From 971e466521e1cbdacb75d6326af6c495e4875158 Mon Sep 17 00:00:00 2001 From: Ali Mohammad Pur Date: Fri, 16 Feb 2024 04:57:03 +0330 Subject: [PATCH] LibIDL: Emit an error when two decls of the same function are present Duplicates can show up when copy-pasting IDL snippets into existing IDL files, and the resulting error is extremely useless and misleading. This commit makes it so the parser catches these cases, and emits a more helpful error like "Overload set 'instantiate' contains multiple identical declarations". --- Userland/Libraries/LibIDL/IDLParser.cpp | 29 ++++++++++++++++++++++++- Userland/Libraries/LibIDL/IDLParser.h | 2 +- Userland/Libraries/LibIDL/Types.h | 1 + 3 files changed, 30 insertions(+), 2 deletions(-) diff --git a/Userland/Libraries/LibIDL/IDLParser.cpp b/Userland/Libraries/LibIDL/IDLParser.cpp index 53ce400157..8616e41709 100644 --- a/Userland/Libraries/LibIDL/IDLParser.cpp +++ b/Userland/Libraries/LibIDL/IDLParser.cpp @@ -372,6 +372,7 @@ Vector Parser::parse_parameters() Function Parser::parse_function(HashMap& extended_attributes, Interface& interface, IsSpecialOperation is_special_operation) { + auto position = lexer.current_position(); bool static_ = false; if (lexer.consume_specific("static"sv)) { static_ = true; @@ -388,7 +389,7 @@ Function Parser::parse_function(HashMap& extended_attrib consume_whitespace(); assert_specific(';'); - Function function { move(return_type), name, move(parameters), move(extended_attributes), {}, false }; + Function function { move(return_type), name, move(parameters), move(extended_attributes), position, {}, false }; // "Defining a special operation with an identifier is equivalent to separating the special operation out into its own declaration without an identifier." if (is_special_operation == IsSpecialOperation::No || (is_special_operation == IsSpecialOperation::Yes && !name.is_empty())) { @@ -1112,6 +1113,32 @@ Interface& Parser::parse() } // FIXME: Add support for overloading constructors + // Check overload sets for repeated instances of the same function + // as these will produce very cryptic errors if left alone. + for (auto& overload_set : interface.overload_sets) { + auto& functions = overload_set.value; + for (size_t i = 0; i < functions.size(); ++i) { + for (size_t j = i + 1; j < functions.size(); ++j) { + if (functions[i].parameters.size() != functions[j].parameters.size()) + continue; + auto same = true; + for (size_t k = 0; k < functions[i].parameters.size(); ++k) { + if (functions[i].parameters[k].type->is_distinguishable_from(interface, functions[j].parameters[k].type)) { + same = false; + break; + } + } + if (same) { + report_parsing_error( + ByteString::formatted("Overload set '{}' contains multiple identical declarations", overload_set.key), + filename, + input, + functions[j].source_position.offset); + } + } + } + } + if (interface.will_generate_code()) interface.required_imported_paths.set(this_module); interface.imported_modules = move(imports); diff --git a/Userland/Libraries/LibIDL/IDLParser.h b/Userland/Libraries/LibIDL/IDLParser.h index 0168da2861..2320ed17f8 100644 --- a/Userland/Libraries/LibIDL/IDLParser.h +++ b/Userland/Libraries/LibIDL/IDLParser.h @@ -61,7 +61,7 @@ private: ByteString import_base_path; ByteString filename; StringView input; - GenericLexer lexer; + LineTrackingLexer lexer; HashTable>& top_level_interfaces(); HashTable> interfaces; diff --git a/Userland/Libraries/LibIDL/Types.h b/Userland/Libraries/LibIDL/Types.h index d074b812c7..e40144c1ce 100644 --- a/Userland/Libraries/LibIDL/Types.h +++ b/Userland/Libraries/LibIDL/Types.h @@ -158,6 +158,7 @@ struct Function { ByteString name; Vector parameters; HashMap extended_attributes; + LineTrackingLexer::Position source_position; size_t overload_index { 0 }; bool is_overloaded { false };