From 91bf7eb274e0b6e431b4f89a6dc814701a4d8739 Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Fri, 15 Mar 2024 12:38:39 +0100 Subject: [PATCH] Refactor frozen_string_literal check during compilation In preparation for https://bugs.ruby-lang.org/issues/20205. The `frozen_string_literal` compilation option will no longer be a boolean but a tri-state: `on/off/default`. --- bootstraptest/runner.rb | 2 +- bootstraptest/test_eval.rb | 31 +++++++++++++++++++++++++++++++ bootstraptest/test_ractor.rb | 2 +- bootstraptest/test_syntax.rb | 32 ++++++++++++++++++++++++++++++++ compile.c | 14 ++++++++++---- iseq.c | 8 ++++++++ iseq.h | 1 + prism_compile.c | 13 ++++++------- ruby.c | 2 ++ vm_eval.c | 2 ++ 10 files changed, 94 insertions(+), 13 deletions(-) diff --git a/bootstraptest/runner.rb b/bootstraptest/runner.rb index 1851640d1f..2135f14276 100755 --- a/bootstraptest/runner.rb +++ b/bootstraptest/runner.rb @@ -548,7 +548,7 @@ class Assertion < Struct.new(:src, :path, :lineno, :proc) end end - def make_srcfile(frozen_string_literal: true) + def make_srcfile(frozen_string_literal: nil) filename = "bootstraptest.#{self.path}_#{self.lineno}_#{self.id}.rb" File.open(filename, 'w') {|f| f.puts "#frozen_string_literal:#{frozen_string_literal}" unless frozen_string_literal.nil? diff --git a/bootstraptest/test_eval.rb b/bootstraptest/test_eval.rb index 47e2924846..d923a957bc 100644 --- a/bootstraptest/test_eval.rb +++ b/bootstraptest/test_eval.rb @@ -364,3 +364,34 @@ assert_normal_exit %q{ end }, 'check escaping the internal value th->base_block' +assert_equal "false", <<~RUBY, "literal strings are mutable", "--disable-frozen-string-literal" + eval("'test'").frozen? +RUBY + +assert_equal "false", <<~RUBY, "literal strings are mutable", "--disable-frozen-string-literal", frozen_string_literal: true + eval("'test'").frozen? +RUBY + +assert_equal "true", <<~RUBY, "literal strings are frozen", "--enable-frozen-string-literal" + eval("'test'").frozen? +RUBY + +assert_equal "true", <<~RUBY, "literal strings are frozen", "--enable-frozen-string-literal", frozen_string_literal: false + eval("'test'").frozen? +RUBY + +assert_equal "false", <<~RUBY, "__FILE__ is mutable", "--disable-frozen-string-literal" + eval("__FILE__").frozen? +RUBY + +assert_equal "false", <<~RUBY, "__FILE__ is mutable", "--disable-frozen-string-literal", frozen_string_literal: true + eval("__FILE__").frozen? +RUBY + +assert_equal "true", <<~RUBY, "__FILE__ is frozen", "--enable-frozen-string-literal" + eval("__FILE__").frozen? +RUBY + +assert_equal "true", <<~RUBY, "__FILE__ is frozen", "--enable-frozen-string-literal", frozen_string_literal: false + eval("__FILE__").frozen? +RUBY diff --git a/bootstraptest/test_ractor.rb b/bootstraptest/test_ractor.rb index 63164b8785..3298ff1443 100644 --- a/bootstraptest/test_ractor.rb +++ b/bootstraptest/test_ractor.rb @@ -1421,7 +1421,7 @@ assert_equal '[false, false, true, true]', %q{ } # TracePoint with normal Proc should be Ractor local -assert_equal '[7, 11]', %q{ +assert_equal '[6, 10]', %q{ rs = [] TracePoint.new(:line){|tp| rs << tp.lineno if tp.path == __FILE__}.enable do Ractor.new{ # line 5 diff --git a/bootstraptest/test_syntax.rb b/bootstraptest/test_syntax.rb index bd147c248b..18528db7a5 100644 --- a/bootstraptest/test_syntax.rb +++ b/bootstraptest/test_syntax.rb @@ -904,3 +904,35 @@ assert_normal_exit %q{ Class end }, '[ruby-core:30293]' + +assert_equal "false", <<~RUBY, "literal strings are mutable", "--disable-frozen-string-literal" + 'test'.frozen? +RUBY + +assert_equal "true", <<~RUBY, "literal strings are frozen", "--disable-frozen-string-literal", frozen_string_literal: true + 'test'.frozen? +RUBY + +assert_equal "true", <<~RUBY, "literal strings are frozen", "--enable-frozen-string-literal" + 'test'.frozen? +RUBY + +assert_equal "false", <<~RUBY, "literal strings are mutable", "--enable-frozen-string-literal", frozen_string_literal: false + 'test'.frozen? +RUBY + +assert_equal "false", <<~RUBY, "__FILE__ is mutable", "--disable-frozen-string-literal" + __FILE__.frozen? +RUBY + +assert_equal "true", <<~RUBY, "__FILE__ is frozen", "--disable-frozen-string-literal", frozen_string_literal: true + __FILE__.frozen? +RUBY + +assert_equal "true", <<~RUBY, "__FILE__ is frozen", "--enable-frozen-string-literal" + __FILE__.frozen? +RUBY + +assert_equal "false", <<~RUBY, "__FILE__ is mutable", "--enable-frozen-string-literal", frozen_string_literal: false + __FILE__.frozen? +RUBY diff --git a/compile.c b/compile.c index fbe122c9ed..3990e03fed 100644 --- a/compile.c +++ b/compile.c @@ -4707,6 +4707,12 @@ compile_args(rb_iseq_t *iseq, LINK_ANCHOR *const ret, const NODE *node, NODE **k return len; } +static inline bool +frozen_string_literal_p(const rb_iseq_t *iseq) +{ + return ISEQ_COMPILE_DATA(iseq)->option->frozen_string_literal > 0; +} + static inline int static_literal_node_p(const NODE *node, const rb_iseq_t *iseq, bool hash_key) { @@ -4726,7 +4732,7 @@ static_literal_node_p(const NODE *node, const rb_iseq_t *iseq, bool hash_key) return TRUE; case NODE_STR: case NODE_FILE: - return hash_key || ISEQ_COMPILE_DATA(iseq)->option->frozen_string_literal; + return hash_key || frozen_string_literal_p(iseq); default: return FALSE; } @@ -8478,7 +8484,7 @@ compile_call_precheck_freeze(rb_iseq_t *iseq, LINK_ANCHOR *const ret, const NODE nd_type_p(get_nd_args(node), NODE_LIST) && RNODE_LIST(get_nd_args(node))->as.nd_alen == 1 && (nd_type_p(RNODE_LIST(get_nd_args(node))->nd_head, NODE_STR) || nd_type_p(RNODE_LIST(get_nd_args(node))->nd_head, NODE_FILE)) && ISEQ_COMPILE_DATA(iseq)->current_block == NULL && - !ISEQ_COMPILE_DATA(iseq)->option->frozen_string_literal && + !frozen_string_literal_p(iseq) && ISEQ_COMPILE_DATA(iseq)->option->specialized_instruction) { VALUE str = rb_fstring(get_string_value(RNODE_LIST(get_nd_args(node))->nd_head)); CHECK(COMPILE(ret, "recv", get_nd_recv(node))); @@ -9862,7 +9868,7 @@ compile_attrasgn(rb_iseq_t *iseq, LINK_ANCHOR *const ret, const NODE *const node nd_type_p(RNODE_ATTRASGN(node)->nd_args, NODE_LIST) && RNODE_LIST(RNODE_ATTRASGN(node)->nd_args)->as.nd_alen == 2 && (nd_type_p(RNODE_LIST(RNODE_ATTRASGN(node)->nd_args)->nd_head, NODE_STR) || nd_type_p(RNODE_LIST(RNODE_ATTRASGN(node)->nd_args)->nd_head, NODE_FILE)) && ISEQ_COMPILE_DATA(iseq)->current_block == NULL && - !ISEQ_COMPILE_DATA(iseq)->option->frozen_string_literal && + !frozen_string_literal_p(iseq) && ISEQ_COMPILE_DATA(iseq)->option->specialized_instruction) { VALUE str = rb_fstring(get_string_value(RNODE_LIST(RNODE_ATTRASGN(node)->nd_args)->nd_head)); @@ -10349,7 +10355,7 @@ iseq_compile_each0(rb_iseq_t *iseq, LINK_ANCHOR *const ret, const NODE *const no debugp_param("nd_lit", get_string_value(node)); if (!popped) { VALUE lit = get_string_value(node); - if (!ISEQ_COMPILE_DATA(iseq)->option->frozen_string_literal) { + if (!frozen_string_literal_p(iseq)) { lit = rb_fstring(lit); ADD_INSN1(ret, node, putstring, lit); RB_OBJ_WRITTEN(iseq, Qundef, lit); diff --git a/iseq.c b/iseq.c index 9e9f609323..1d903f9591 100644 --- a/iseq.c +++ b/iseq.c @@ -733,6 +733,12 @@ static rb_compile_option_t COMPILE_OPTION_DEFAULT = { static const rb_compile_option_t COMPILE_OPTION_FALSE = {0}; +int +rb_iseq_opt_frozen_string_literal(void) +{ + return COMPILE_OPTION_DEFAULT.frozen_string_literal; +} + static void set_compile_option_from_hash(rb_compile_option_t *option, VALUE opt) { @@ -1242,6 +1248,8 @@ pm_iseq_compile_with_option(VALUE src, VALUE file, VALUE realpath, VALUE line, V pm_parse_result_t result = { 0 }; pm_options_line_set(&result.options, NUM2INT(line)); + pm_options_frozen_string_literal_set(&result.options, option.frozen_string_literal); + VALUE error; if (RB_TYPE_P(src, T_FILE)) { VALUE filepath = rb_io_path(src); diff --git a/iseq.h b/iseq.h index ec5b145f43..78e0d900db 100644 --- a/iseq.h +++ b/iseq.h @@ -172,6 +172,7 @@ void rb_iseq_init_trace(rb_iseq_t *iseq); int rb_iseq_add_local_tracepoint_recursively(const rb_iseq_t *iseq, rb_event_flag_t turnon_events, VALUE tpval, unsigned int target_line, bool target_bmethod); int rb_iseq_remove_local_tracepoint_recursively(const rb_iseq_t *iseq, VALUE tpval); const rb_iseq_t *rb_iseq_load_iseq(VALUE fname); +int rb_iseq_opt_frozen_string_literal(void); #if VM_INSN_INFO_TABLE_IMPL == 2 unsigned int *rb_iseq_insns_info_decode_positions(const struct rb_iseq_constant_body *body); diff --git a/prism_compile.c b/prism_compile.c index 8417d09bd1..4e4ec40ec1 100644 --- a/prism_compile.c +++ b/prism_compile.c @@ -820,7 +820,7 @@ pm_interpolated_node_compile(pm_node_list_t *parts, rb_iseq_t *iseq, NODE dummy_ current_string = rb_enc_str_new(NULL, 0, scope_node->encoding); } - if (ISEQ_COMPILE_DATA(iseq)->option->frozen_string_literal) { + if (frozen_string_literal_p(iseq)) { ADD_INSN1(ret, &dummy_line_node, putobject, rb_str_freeze(current_string)); } else { @@ -842,7 +842,7 @@ pm_interpolated_node_compile(pm_node_list_t *parts, rb_iseq_t *iseq, NODE dummy_ if (RTEST(current_string)) { current_string = rb_fstring(current_string); - if (ISEQ_COMPILE_DATA(iseq)->option->frozen_string_literal) { + if (frozen_string_literal_p(iseq)) { ADD_INSN1(ret, &dummy_line_node, putobject, current_string); } else { @@ -4019,7 +4019,7 @@ pm_opt_aref_with_p(const rb_iseq_t *iseq, const pm_call_node_t *node) ((const pm_arguments_node_t *) node->arguments)->arguments.size == 1 && PM_NODE_TYPE_P(((const pm_arguments_node_t *) node->arguments)->arguments.nodes[0], PM_STRING_NODE) && node->block == NULL && - !ISEQ_COMPILE_DATA(iseq)->option->frozen_string_literal && + !frozen_string_literal_p(iseq) && ISEQ_COMPILE_DATA(iseq)->option->specialized_instruction ); } @@ -4038,7 +4038,7 @@ pm_opt_aset_with_p(const rb_iseq_t *iseq, const pm_call_node_t *node) ((const pm_arguments_node_t *) node->arguments)->arguments.size == 2 && PM_NODE_TYPE_P(((const pm_arguments_node_t *) node->arguments)->arguments.nodes[0], PM_STRING_NODE) && node->block == NULL && - !ISEQ_COMPILE_DATA(iseq)->option->frozen_string_literal && + !frozen_string_literal_p(iseq) && ISEQ_COMPILE_DATA(iseq)->option->specialized_instruction ); } @@ -7920,10 +7920,9 @@ pm_compile_node(rb_iseq_t *iseq, const pm_node_t *node, LINK_ANCHOR *const ret, // ^^^^^^^^ if (!popped) { const pm_source_file_node_t *cast = (const pm_source_file_node_t *) node; - VALUE string = parse_string(scope_node, &cast->filepath); + VALUE string = rb_fstring(parse_string(scope_node, &cast->filepath)); if (PM_NODE_FLAG_P(cast, PM_STRING_FLAGS_FROZEN)) { - string = rb_fstring(string); PUSH_INSN1(ret, location, putobject, string); } else { @@ -7977,7 +7976,7 @@ pm_compile_node(rb_iseq_t *iseq, const pm_node_t *node, LINK_ANCHOR *const ret, const pm_string_node_t *cast = (const pm_string_node_t *) node; VALUE value = rb_fstring(parse_string_encoded(scope_node, node, &cast->unescaped)); - if (PM_NODE_FLAG_P(node, PM_STRING_FLAGS_FROZEN) || ISEQ_COMPILE_DATA(iseq)->option->frozen_string_literal) { + if (PM_NODE_FLAG_P(node, PM_STRING_FLAGS_FROZEN)) { PUSH_INSN1(ret, location, putobject, value); } else { diff --git a/ruby.c b/ruby.c index d1ae574cdf..f91b65c729 100644 --- a/ruby.c +++ b/ruby.c @@ -2116,6 +2116,8 @@ prism_script(ruby_cmdline_options_t *opt, pm_parse_result_t *result) pm_options_t *options = &result->options; pm_options_line_set(options, 1); + pm_options_frozen_string_literal_set(&result->options, rb_iseq_opt_frozen_string_literal()); + uint8_t command_line = 0; if (opt->do_split) command_line |= PM_OPTIONS_COMMAND_LINE_A; if (opt->do_line) command_line |= PM_OPTIONS_COMMAND_LINE_L; diff --git a/vm_eval.c b/vm_eval.c index f7e23a691b..c52244ed1e 100644 --- a/vm_eval.c +++ b/vm_eval.c @@ -1663,6 +1663,8 @@ pm_eval_make_iseq(VALUE src, VALUE fname, int line, pm_parse_result_t result = { 0 }; pm_options_line_set(&result.options, line); + pm_options_frozen_string_literal_set(&result.options, rb_iseq_opt_frozen_string_literal()); + // Cout scopes, one for each parent iseq, plus one for our local scope int scopes_count = 0; do {