[Feature #20331] Simplify parser warnings for hash keys duplication and when clause duplication

This commit simplifies warnings for hash keys duplication and when clause duplication,
based on the discussion of https://bugs.ruby-lang.org/issues/20331.
Warnings are reported only when strings are same to ohters.
This commit is contained in:
yui-knk 2024-03-24 08:39:27 +09:00 коммит произвёл Yuichiro Kaneko
Родитель 8066e3ea6e
Коммит 799e854897
8 изменённых файлов: 96 добавлений и 151 удалений

Просмотреть файл

@ -3,13 +3,13 @@
#include "ruby/encoding.h"
#include "internal.h"
#include "internal/imemo.h" /* needed by ruby_parser.h */
#include "rubyparser.h"
#define YYSTYPE_IS_DECLARED
#include "parse.h"
#include "internal/parse.h"
#include "internal/ruby_parser.h"
#include "node.h"
#include "rubyparser.h"
#include "eventids1.h"
#define YYSTYPE_IS_DECLARED
#include "parse.h"
#include "eventids2.h"
#include "ripper_init.h"

Просмотреть файл

@ -65,6 +65,7 @@ int rb_ruby_parser_end_seen_p(rb_parser_t *p);
int rb_ruby_parser_set_yydebug(rb_parser_t *p, int flag);
rb_parser_string_t *rb_str_to_parser_string(rb_parser_t *p, VALUE str);
void rb_parser_check_literal_when(struct parser_params *p, NODE *arg, const YYLTYPE *loc);
void rb_parser_warn_duplicate_keys(struct parser_params *p, NODE *hash);
int rb_parser_dvar_defined_ref(struct parser_params*, ID, ID**);
ID rb_parser_internal_id(struct parser_params*);

183
parse.y
Просмотреть файл

@ -19,8 +19,6 @@
#define YYDEBUG 1
#define YYERROR_VERBOSE 1
#define YYSTACK_USE_ALLOCA 0
#define YYLTYPE rb_code_location_t
#define YYLTYPE_IS_DECLARED 1
/* For Ripper */
#ifdef RUBY_EXTCONF_H
@ -73,6 +71,22 @@
#include "ruby/ractor.h"
#include "symbol.h"
#ifndef RIPPER
static VALUE
syntax_error_new(void)
{
return rb_class_new_instance(0, 0, rb_eSyntaxError);
}
#endif
static NODE *reg_named_capture_assign(struct parser_params* p, VALUE regexp, const YYLTYPE *loc);
#define compile_callback rb_suppress_tracing
VALUE rb_io_gets_internal(VALUE io);
VALUE rb_node_case_when_optimizable_literal(const NODE *const node);
#endif /* !UNIVERSAL_PARSER */
#ifndef RIPPER
static int rb_parser_string_hash_cmp(rb_parser_string_t *str1, rb_parser_string_t *str2);
@ -117,15 +131,8 @@ rb_parser_regx_hash_cmp(rb_node_regx_t *n1, rb_node_regx_t *n2)
rb_parser_string_hash_cmp(n1->string, n2->string));
}
static int
node_integer_line_cmp(const NODE *node_i, const NODE *line)
{
VALUE num = rb_node_integer_literal_val(node_i);
return !(FIXNUM_P(num) && line->nd_loc.beg_pos.lineno == FIX2INT(num));
}
static st_index_t rb_parser_str_hash(rb_parser_string_t *str);
static st_index_t rb_char_p_hash(const char *c);
static int
literal_cmp(st_data_t val, st_data_t lit)
@ -137,22 +144,6 @@ literal_cmp(st_data_t val, st_data_t lit)
enum node_type type_val = nd_type(node_val);
enum node_type type_lit = nd_type(node_lit);
/* Special case for Integer and __LINE__ */
if (type_val == NODE_INTEGER && type_lit == NODE_LINE) {
return node_integer_line_cmp(node_val, node_lit);
}
if (type_lit == NODE_INTEGER && type_val == NODE_LINE) {
return node_integer_line_cmp(node_lit, node_val);
}
/* Special case for String and __FILE__ */
if (type_val == NODE_STR && type_lit == NODE_FILE) {
return rb_parser_string_hash_cmp(RNODE_STR(node_val)->string, RNODE_FILE(node_lit)->path);
}
if (type_lit == NODE_STR && type_val == NODE_FILE) {
return rb_parser_string_hash_cmp(RNODE_STR(node_lit)->string, RNODE_FILE(node_val)->path);
}
if (type_val != type_lit) {
return -1;
}
@ -179,7 +170,11 @@ literal_cmp(st_data_t val, st_data_t lit)
case NODE_ENCODING:
return RNODE_ENCODING(node_val)->enc != RNODE_ENCODING(node_lit)->enc;
default:
#ifdef UNIVERSAL_PARSER
abort();
#else
rb_bug("unexpected node: %s, %s", ruby_node_name(type_val), ruby_node_name(type_lit));
#endif
}
}
@ -187,23 +182,17 @@ static st_index_t
literal_hash(st_data_t a)
{
NODE *node = (NODE *)a;
VALUE val;
enum node_type type = nd_type(node);
switch (type) {
case NODE_INTEGER:
val = rb_node_integer_literal_val(node);
if (!FIXNUM_P(val)) val = rb_big_hash(val);
return FIX2LONG(val);
return rb_char_p_hash(RNODE_INTEGER(node)->val);
case NODE_FLOAT:
val = rb_node_float_literal_val(node);
return rb_dbl_long_hash(RFLOAT_VALUE(val));
return rb_char_p_hash(RNODE_FLOAT(node)->val);
case NODE_RATIONAL:
val = rb_node_rational_literal_val(node);
return rb_rational_hash(val);
return rb_char_p_hash(RNODE_RATIONAL(node)->val);
case NODE_IMAGINARY:
val = rb_node_imaginary_literal_val(node);
return rb_complex_hash(val);
return rb_char_p_hash(RNODE_IMAGINARY(node)->val);
case NODE_STR:
return rb_parser_str_hash(RNODE_STR(node)->string);
case NODE_SYM:
@ -211,32 +200,20 @@ literal_hash(st_data_t a)
case NODE_REGX:
return rb_parser_str_hash(RNODE_REGX(node)->string);
case NODE_LINE:
/* Same with NODE_INTEGER FIXNUM case */
return (st_index_t)node->nd_loc.beg_pos.lineno;
case NODE_FILE:
/* Same with NODE_STR */
return rb_parser_str_hash(RNODE_FILE(node)->path);
case NODE_ENCODING:
return (st_index_t)RNODE_ENCODING(node)->enc;
default:
#ifdef UNIVERSAL_PARSER
abort();
#else
rb_bug("unexpected node: %s", ruby_node_name(type));
#endif
}
}
static VALUE
syntax_error_new(void)
{
return rb_class_new_instance(0, 0, rb_eSyntaxError);
}
#endif
static NODE *reg_named_capture_assign(struct parser_params* p, VALUE regexp, const YYLTYPE *loc);
#define compile_callback rb_suppress_tracing
VALUE rb_io_gets_internal(VALUE io);
VALUE rb_node_case_when_optimizable_literal(const NODE *const node);
#endif /* !UNIVERSAL_PARSER */
#endif /* !RIPPER */
static inline int
parse_isascii(int c)
@ -567,7 +544,7 @@ struct parser_params {
VALUE ruby_sourcefile_string;
rb_encoding *enc;
token_info *token_info;
VALUE case_labels;
st_table *case_labels;
rb_node_exits_t *exits;
VALUE debug_buffer;
@ -1532,8 +1509,6 @@ int reg_fragment_check(struct parser_params*, rb_parser_string_t*, int);
static int literal_concat0(struct parser_params *p, rb_parser_string_t *head, rb_parser_string_t *tail);
static NODE *heredoc_dedent(struct parser_params*,NODE*);
static void check_literal_when(struct parser_params *p, NODE *args, const YYLTYPE *loc);
#ifdef RIPPER
static VALUE var_field(struct parser_params *p, VALUE a);
#define get_value(idx) (rb_ary_entry(p->s_value_stack, idx))
@ -1615,6 +1590,9 @@ static void numparam_pop(struct parser_params *p, NODE *prev_inner);
#define RE_OPTION_MASK 0xff
#define RE_OPTION_ARG_ENCODING_NONE 32
#define CHECK_LITERAL_WHEN (st_table *)1
#define CASE_LABELS_ENABLED_P(case_labels) (case_labels && case_labels != CHECK_LITERAL_WHEN)
#define yytnamerr(yyres, yystr) (YYSIZE_T)rb_yytnamerr(p, yyres, yystr)
size_t rb_yytnamerr(struct parser_params *p, char *yyres, const char *yystr);
@ -2079,7 +2057,6 @@ get_nd_args(struct parser_params *p, NODE *node)
}
#ifndef RIPPER
#ifndef UNIVERSAL_PARSER
static st_index_t
djb2(const uint8_t *str, size_t len)
{
@ -2098,7 +2075,6 @@ parser_memhash(const void *ptr, long len)
return djb2(ptr, len);
}
#endif
#endif
#define PARSER_STRING_PTR(str) (str->ptr)
#define PARSER_STRING_LEN(str) (str->len)
@ -2163,13 +2139,17 @@ rb_parser_string_free(rb_parser_t *p, rb_parser_string_t *str)
}
#ifndef RIPPER
#ifndef UNIVERSAL_PARSER
static st_index_t
rb_parser_str_hash(rb_parser_string_t *str)
{
return parser_memhash((const void *)PARSER_STRING_PTR(str), PARSER_STRING_LEN(str));
}
#endif
static st_index_t
rb_char_p_hash(const char *c)
{
return parser_memhash((const void *)c, strlen(c));
}
#endif
static size_t
@ -2567,7 +2547,6 @@ rb_parser_str_resize(struct parser_params *p, rb_parser_string_t *str, long len)
}
#ifndef RIPPER
#ifndef UNIVERSAL_PARSER
# define PARSER_ENC_STRING_GETMEM(str, ptrvar, lenvar, encvar) \
((ptrvar) = str->ptr, \
(lenvar) = str->len, \
@ -2587,7 +2566,6 @@ rb_parser_string_hash_cmp(rb_parser_string_t *str1, rb_parser_string_t *str2)
enc1 != enc2 ||
memcmp(ptr1, ptr2, len1) != 0);
}
#endif
static void
rb_parser_ary_extend(rb_parser_t *p, rb_parser_ary_t *ary, long len)
@ -2694,6 +2672,10 @@ rb_parser_tokens_free(rb_parser_t *p, rb_parser_ary_t *tokens)
rb_parser_printf(p, "$%c", (int)RNODE_BACK_REF($$)->nd_nth);
} tBACK_REF
%destructor {
if (CASE_LABELS_ENABLED_P($$)) st_free_table($$);
} <labels>
%lex-param {struct parser_params *p}
%parse-param {struct parser_params *p}
%initial-action
@ -2707,7 +2689,6 @@ rb_parser_tokens_free(rb_parser_t *p, rb_parser_ary_t *tokens)
%after-pop-stack after_pop_stack
%union {
VALUE val;
NODE *node;
rb_node_fcall_t *node_fcall;
rb_node_args_t *node_args;
@ -2721,6 +2702,7 @@ rb_parser_tokens_free(rb_parser_t *p, rb_parser_ary_t *tokens)
ID id;
int num;
st_table *tbl;
st_table *labels;
const struct vtable *vars;
struct rb_strterm_struct *strterm;
struct lex_context ctxt;
@ -4536,28 +4518,28 @@ primary : literal
}
| k_case expr_value terms?
{
$<val>$ = p->case_labels;
p->case_labels = Qnil;
}
$$ = p->case_labels;
p->case_labels = CHECK_LITERAL_WHEN;
}<labels>
case_body
k_end
{
if (RTEST(p->case_labels)) rb_hash_clear(p->case_labels);
p->case_labels = $<val>4;
if (CASE_LABELS_ENABLED_P(p->case_labels)) st_free_table(p->case_labels);
p->case_labels = $<labels>4;
$$ = NEW_CASE($2, $5, &@$);
fixpos($$, $2);
/*% ripper: case!($:2, $:5) %*/
}
| k_case terms?
{
$<val>$ = p->case_labels;
$$ = p->case_labels;
p->case_labels = 0;
}
}<labels>
case_body
k_end
{
if (RTEST(p->case_labels)) rb_hash_clear(p->case_labels);
p->case_labels = $<val>3;
if (p->case_labels) st_free_table(p->case_labels);
p->case_labels = $<labels>3;
$$ = NEW_CASE2($4, &@$);
/*% ripper: case!(Qnil, $:4) %*/
}
@ -5415,7 +5397,7 @@ do_body : {
case_args : arg_value
{
check_literal_when(p, $1, &@1);
rb_parser_check_literal_when(p, $1, &@1);
$$ = NEW_LIST($1, &@$);
/*% ripper: args_add!(args_new!, $:1) %*/
}
@ -5426,7 +5408,7 @@ case_args : arg_value
}
| case_args ',' arg_value
{
check_literal_when(p, $3, &@3);
rb_parser_check_literal_when(p, $3, &@3);
$$ = last_arg_append(p, $1, $3, &@$);
/*% ripper: args_add!($:1, $:3) %*/
}
@ -11568,7 +11550,7 @@ yylex(YYSTYPE *lval, YYLTYPE *yylloc, struct parser_params *p)
enum yytokentype t;
p->lval = lval;
lval->val = Qundef;
lval->node = 0;
p->yylloc = yylloc;
t = parser_yylex(p);
@ -13487,36 +13469,35 @@ new_xstring(struct parser_params *p, NODE *node, const YYLTYPE *loc)
}
#ifndef RIPPER
VALUE
rb_parser_node_case_when_optimizable_literal(struct parser_params *p, const NODE *const node)
{
return rb_node_case_when_optimizable_literal(node);
}
#endif
static const
struct st_hash_type literal_type = {
literal_cmp,
literal_hash,
};
static void
check_literal_when(struct parser_params *p, NODE *arg, const YYLTYPE *loc)
{
VALUE lit;
static int nd_type_st_key_enable_p(NODE *node);
void
rb_parser_check_literal_when(struct parser_params *p, NODE *arg, const YYLTYPE *loc)
{
/* See https://bugs.ruby-lang.org/issues/20331 for discussion about what is warned. */
if (!arg || !p->case_labels) return;
if (!nd_type_st_key_enable_p(arg)) return;
lit = rb_parser_node_case_when_optimizable_literal(p, arg);
if (UNDEF_P(lit)) return;
if (NIL_P(p->case_labels)) {
p->case_labels = rb_obj_hide(rb_hash_new());
if (p->case_labels == CHECK_LITERAL_WHEN) {
p->case_labels = st_init_table(&literal_type);
}
else {
VALUE line = rb_hash_lookup(p->case_labels, lit);
if (!NIL_P(line)) {
st_data_t line;
if (st_lookup(p->case_labels, (st_data_t)arg, &line)) {
rb_warning1("duplicated 'when' clause with line %d is ignored",
WARN_IVAL(line));
WARN_IVAL(INT2NUM((int)line)));
return;
}
}
rb_hash_aset(p->case_labels, lit, INT2NUM(p->ruby_sourceline));
st_insert(p->case_labels, (st_data_t)arg, (st_data_t)p->ruby_sourceline);
}
#endif
#ifdef RIPPER
static int
@ -15229,14 +15210,7 @@ nd_value(struct parser_params *p, NODE *node)
void
rb_parser_warn_duplicate_keys(struct parser_params *p, NODE *hash)
{
#ifndef UNIVERSAL_PARSER
static const
#endif
struct st_hash_type literal_type = {
literal_cmp,
literal_hash,
};
/* See https://bugs.ruby-lang.org/issues/20331 for discussion about what is warned. */
st_table *literal_keys = st_init_table_with_size(&literal_type, RNODE_LIST(hash)->as.nd_alen / 2);
while (hash && RNODE_LIST(hash)->nd_next) {
NODE *head = RNODE_LIST(hash)->nd_head;
@ -16159,7 +16133,6 @@ rb_ruby_parser_mark(void *ptr)
rb_gc_mark(p->lex.input);
rb_gc_mark(p->ruby_sourcefile_string);
rb_gc_mark((VALUE)p->ast);
rb_gc_mark(p->case_labels);
rb_gc_mark(p->delayed.token);
#ifndef RIPPER
rb_gc_mark(p->debug_lines);
@ -16213,6 +16186,10 @@ rb_ruby_parser_free(void *ptr)
st_free_table(p->pvtbl);
}
if (CASE_LABELS_ENABLED_P(p->case_labels)) {
st_free_table(p->case_labels);
}
xfree(ptr);
}

Просмотреть файл

@ -93,32 +93,6 @@ dvar_defined(ID id, const void *p)
return rb_dvar_defined(id, (const rb_iseq_t *)p);
}
static bool
hash_literal_key_p(VALUE k)
{
switch (OBJ_BUILTIN_TYPE(k)) {
case T_NODE:
return false;
default:
return true;
}
}
static int
literal_cmp(VALUE val, VALUE lit)
{
if (val == lit) return 0;
if (!hash_literal_key_p(val) || !hash_literal_key_p(lit)) return -1;
return rb_iseq_cdhash_cmp(val, lit);
}
static st_index_t
literal_hash(VALUE a)
{
if (!hash_literal_key_p(a)) return (st_index_t)a;
return rb_iseq_cdhash_hash(a);
}
static int
is_usascii_enc(void *enc)
{
@ -611,9 +585,6 @@ static const rb_parser_config_t rb_global_parser_config = {
.local_defined = local_defined,
.dvar_defined = dvar_defined,
.literal_cmp = literal_cmp,
.literal_hash = literal_hash,
.syntax_error_append = syntax_error_append,
.raise = rb_raise,
.syntax_error_new = syntax_error_new,

Просмотреть файл

@ -200,6 +200,8 @@ typedef struct rb_code_location_struct {
rb_code_position_t beg_pos;
rb_code_position_t end_pos;
} rb_code_location_t;
#define YYLTYPE rb_code_location_t
#define YYLTYPE_IS_DECLARED 1
typedef struct rb_parser_ast_token {
int id;
@ -673,7 +675,7 @@ typedef struct RNode_LIT {
typedef struct RNode_INTEGER {
NODE node;
char* val;
char *val;
int minus;
int base;
} rb_node_integer_t;
@ -681,14 +683,14 @@ typedef struct RNode_INTEGER {
typedef struct RNode_FLOAT {
NODE node;
char* val;
char *val;
int minus;
} rb_node_float_t;
typedef struct RNode_RATIONAL {
NODE node;
char* val;
char *val;
int minus;
int base;
int seen_point;
@ -703,7 +705,7 @@ enum rb_numeric_type {
typedef struct RNode_IMAGINARY {
NODE node;
char* val;
char *val;
int minus;
int base;
int seen_point;
@ -1378,10 +1380,6 @@ typedef struct rb_parser_config_struct {
// int rb_dvar_defined(ID id, const rb_iseq_t *iseq);
int (*dvar_defined)(ID, const void*);
/* Compile (parse.y) */
int (*literal_cmp)(VALUE val, VALUE lit);
parser_st_index_t (*literal_hash)(VALUE a);
/* Error (Exception) */
RBIMPL_ATTR_FORMAT(RBIMPL_PRINTF_FORMAT, 6, 0)
VALUE (*syntax_error_append)(VALUE, VALUE, int, int, rb_encoding*, const char*, va_list);

Просмотреть файл

@ -509,9 +509,6 @@ class TestRubyLiteral < Test::Unit::TestCase
) do |key|
assert_warning(/key #{Regexp.quote(eval(key).inspect)} is duplicated/) { eval("{#{key} => :bar, #{key} => :foo}") }
end
assert_warning(/key 1 is duplicated/) { eval("{__LINE__ => :bar, 1 => :foo}") }
assert_warning(/key \"FILENAME\" is duplicated/) { eval("{__FILE__ => :bar, 'FILENAME' => :foo}", binding, "FILENAME") }
end
def test_hash_frozen_key_id

Просмотреть файл

@ -734,7 +734,7 @@ WARN
end
}
}
assert_warning(/3: #{w}.+4: #{w}.+4: #{w}.+5: #{w}.+5: #{w}/m) {
assert_warning(/3: #{w}/m) {
eval %q{
case 1
when __LINE__, __LINE__
@ -743,7 +743,7 @@ WARN
end
}
}
assert_warning(/3: #{w}.+4: #{w}.+4: #{w}.+5: #{w}.+5: #{w}/m) {
assert_warning(/3: #{w}/m) {
eval %q{
case 1
when __FILE__, __FILE__

Просмотреть файл

@ -54,6 +54,10 @@
#define st_delete rb_parser_st_delete
#undef st_is_member
#define st_is_member parser_st_is_member
#undef st_init_table
#define st_init_table rb_parser_st_init_table
#undef st_lookup
#define st_lookup rb_parser_st_lookup
#define rb_encoding void
@ -227,9 +231,6 @@ struct rb_imemo_tmpbuf_struct {
#define rb_local_defined p->config->local_defined
#define rb_dvar_defined p->config->dvar_defined
#define literal_cmp p->config->literal_cmp
#define literal_hash p->config->literal_hash
#define rb_syntax_error_append p->config->syntax_error_append
#define rb_raise p->config->raise
#define syntax_error_new p->config->syntax_error_new