From a02f5eb56a53e6799e73cfbb96fc8ccd68fa5651 Mon Sep 17 00:00:00 2001 From: Jemma Issroff Date: Wed, 12 Jul 2023 12:46:38 -0400 Subject: [PATCH] YARP resync (#8059) --- lib/yarp/node.rb | 2 +- test/yarp/errors_test.rb | 9 ++- test/yarp/library_symbols_test.rb | 101 ++++++++++++++++++++++++++++++ yarp/defines.h | 8 +-- yarp/yarp.c | 45 +++++++------ 5 files changed, 140 insertions(+), 25 deletions(-) create mode 100644 test/yarp/library_symbols_test.rb diff --git a/lib/yarp/node.rb b/lib/yarp/node.rb index 393030d12d..d2f8863091 100644 --- a/lib/yarp/node.rb +++ b/lib/yarp/node.rb @@ -4427,7 +4427,7 @@ module YARP end end - # Represents a parentesized expression + # Represents a parenthesized expression # # (10 + 34) # ^^^^^^^^^ diff --git a/test/yarp/errors_test.rb b/test/yarp/errors_test.rb index 4e5f8cb936..03c0c7ce64 100644 --- a/test/yarp/errors_test.rb +++ b/test/yarp/errors_test.rb @@ -426,9 +426,14 @@ class ErrorsTest < Test::Unit::TestCase expected = DefNode( Location(), nil, - ParametersNode([], [], [], nil, [], nil, nil), + ParametersNode([ + RequiredParameterNode(:A), + RequiredParameterNode(:@a), + RequiredParameterNode(:$A), + RequiredParameterNode(:@@a), + ], [], [], nil, [], nil, nil), nil, - [], + [:A, :@a, :$A, :@@a], Location(), nil, Location(), diff --git a/test/yarp/library_symbols_test.rb b/test/yarp/library_symbols_test.rb new file mode 100644 index 0000000000..276b42a028 --- /dev/null +++ b/test/yarp/library_symbols_test.rb @@ -0,0 +1,101 @@ +# frozen_string_literal: true + +require "yarp_test_helper" + +if RUBY_PLATFORM =~ /linux/ + # + # examine a yarp dll or static archive for expected external symbols. + # these tests only work on a linux system right now. + # + class LibrarySymbolsTest < Test::Unit::TestCase + def setup + super + + @librubyparser_a = File.expand_path(File.join(__dir__, "..", "build", "librubyparser.a")) + @librubyparser_so = File.expand_path(File.join(__dir__, "..", "build", "librubyparser.so")) + @yarp_so = File.expand_path(File.join(__dir__, "..", "lib", "yarp.so")) + end + + # objdump runner and helpers + def objdump(path) + assert_path_exist(path) + %x(objdump --section=.text --syms #{path}).split("\n") + end + + def global_objdump_symbols(path) + objdump(path).select { |line| line[17] == "g" } + end + + def hidden_global_objdump_symbols(path) + global_objdump_symbols(path).select { |line| line =~ / \.hidden / } + end + + def visible_global_objdump_symbols(path) + global_objdump_symbols(path).reject { |line| line =~ / \.hidden / } + end + + # nm runner and helpers + def nm(path) + assert_path_exist(path) + %x(nm #{path}).split("\n") + end + + def global_nm_symbols(path) + nm(path).select { |line| line[17] == "T" } + end + + def local_nm_symbols(path) + nm(path).select { |line| line[17] == "t" } + end + + # dig the symbol name out of each line. works for both `objdump` and `nm` output. + def names(symbol_lines) + symbol_lines.map { |line| line.split(/\s+/).last } + end + + # + # static archive - librubyparser.a + # + def test_librubyparser_a_contains_nothing_globally_visible + omit("librubyparser.a is not built") unless File.exist?(@librubyparser_a) + + assert_empty(names(visible_global_objdump_symbols(@librubyparser_a))) + end + + def test_librubyparser_a_contains_hidden_yp_symbols + omit("librubyparser.a is not built") unless File.exist?(@librubyparser_a) + + names(hidden_global_objdump_symbols(@librubyparser_a)).tap do |symbols| + assert_includes(symbols, "yp_parse") + assert_includes(symbols, "yp_version") + end + end + + # + # shared object - librubyparser.so + # + def test_librubyparser_so_exports_only_the_necessary_functions + omit("librubyparser.so is not built") unless File.exist?(@librubyparser_so) + + names(global_nm_symbols(@librubyparser_so)).tap do |symbols| + assert_includes(symbols, "yp_parse") + assert_includes(symbols, "yp_version") + end + names(local_nm_symbols(@librubyparser_so)).tap do |symbols| + assert_includes(symbols, "yp_encoding_shift_jis_isupper_char") + end + # TODO: someone who uses this library needs to finish this test + end + + # + # shared object - yarp.so + # + def test_yarp_so_exports_only_the_C_extension_init_function + omit("yarp.so is not built") unless File.exist?(@yarp_so) + + names(global_nm_symbols(@yarp_so)).tap do |symbols| + assert_equal(["Init_yarp"], symbols) + end + end + end +end diff --git a/yarp/defines.h b/yarp/defines.h index e6a90bb518..5dba303a3d 100644 --- a/yarp/defines.h +++ b/yarp/defines.h @@ -12,11 +12,9 @@ #include // YP_EXPORTED_FUNCTION -#if defined(YP_STATIC) -# define YP_EXPORTED_FUNCTION -#elif defined(_WIN32) +#if defined(_WIN32) # define YP_EXPORTED_FUNCTION __declspec(dllexport) extern -#else +#elif defined(YP_EXPORT_SYMBOLS) # ifndef YP_EXPORTED_FUNCTION # ifndef RUBY_FUNC_EXPORTED # define YP_EXPORTED_FUNCTION __attribute__((__visibility__("default"))) extern @@ -24,6 +22,8 @@ # define YP_EXPORTED_FUNCTION RUBY_FUNC_EXPORTED # endif # endif +#else +# define YP_EXPORTED_FUNCTION #endif // YP_ATTRIBUTE_UNUSED diff --git a/yarp/yarp.c b/yarp/yarp.c index 97b698279d..8e920b3571 100644 --- a/yarp/yarp.c +++ b/yarp/yarp.c @@ -3412,7 +3412,6 @@ yp_required_destructured_parameter_node_closing_set(yp_required_destructured_par // Allocate a new RequiredParameterNode node. static yp_required_parameter_node_t * yp_required_parameter_node_create(yp_parser_t *parser, const yp_token_t *token) { - assert(token->type == YP_TOKEN_MISSING || token->type == YP_TOKEN_IDENTIFIER); yp_required_parameter_node_t *node = YP_ALLOC_NODE(parser, yp_required_parameter_node_t); *node = (yp_required_parameter_node_t) { @@ -8244,8 +8243,27 @@ parse_parameters( } break; } - case YP_TOKEN_IDENTIFIER: { + case YP_TOKEN_CLASS_VARIABLE: + case YP_TOKEN_IDENTIFIER: + case YP_TOKEN_CONSTANT: + case YP_TOKEN_INSTANCE_VARIABLE: + case YP_TOKEN_GLOBAL_VARIABLE: { parser_lex(parser); + switch (parser->previous.type) { + case YP_TOKEN_CONSTANT: + yp_diagnostic_list_append(&parser->error_list, parser->previous.start, parser->previous.end, "Formal argument cannot be a constant"); + break; + case YP_TOKEN_INSTANCE_VARIABLE: + yp_diagnostic_list_append(&parser->error_list, parser->previous.start, parser->previous.end, "Formal argument cannot be an instance variable"); + break; + case YP_TOKEN_GLOBAL_VARIABLE: + yp_diagnostic_list_append(&parser->error_list, parser->previous.start, parser->previous.end, "Formal argument cannot be a global variable"); + break; + case YP_TOKEN_CLASS_VARIABLE: + yp_diagnostic_list_append(&parser->error_list, parser->previous.start, parser->previous.end, "Formal argument cannot be a class variable"); + break; + default: break; + } if (parser->current.type == YP_TOKEN_EQUAL) { update_parameter_state(parser, &parser->current, &order); @@ -8387,22 +8405,6 @@ parse_parameters( yp_parameters_node_keyword_rest_set(params, param); break; } - case YP_TOKEN_CONSTANT: - parser_lex(parser); - yp_diagnostic_list_append(&parser->error_list, parser->previous.start, parser->previous.end, "Formal argument cannot be a constant"); - break; - case YP_TOKEN_INSTANCE_VARIABLE: - parser_lex(parser); - yp_diagnostic_list_append(&parser->error_list, parser->previous.start, parser->previous.end, "Formal argument cannot be an instance variable"); - break; - case YP_TOKEN_GLOBAL_VARIABLE: - parser_lex(parser); - yp_diagnostic_list_append(&parser->error_list, parser->previous.start, parser->previous.end, "Formal argument cannot be a global variable"); - break; - case YP_TOKEN_CLASS_VARIABLE: - parser_lex(parser); - yp_diagnostic_list_append(&parser->error_list, parser->previous.start, parser->previous.end, "Formal argument cannot be a class variable"); - break; default: if (parser->previous.type == YP_TOKEN_COMMA) { if (allows_trailing_comma) { @@ -8427,6 +8429,13 @@ parse_parameters( } while (looping && accept(parser, YP_TOKEN_COMMA)); yp_do_loop_stack_pop(parser); + + // If we don't have any parameters, return `NULL` instead of an empty `ParametersNode`. + if (params->base.location.start == params->base.location.end) { + yp_node_destroy(parser, (yp_node_t *) params); + return NULL; + } + return params; }