diff --git a/common/text/tree_utils.h b/common/text/tree_utils.h index 36cc170c3..57f9840c6 100644 --- a/common/text/tree_utils.h +++ b/common/text/tree_utils.h @@ -199,18 +199,16 @@ typename match_const::type& GetSubtreeAsNode( } // Same as GetSubtreeAsSymbol, but casts the result to a leaf. +// If subtree does not exist, returns nullptr. template -const SyntaxTreeLeaf& GetSubtreeAsLeaf(const S& symbol, +const SyntaxTreeLeaf* GetSubtreeAsLeaf(const S& symbol, E parent_must_be_node_enum, size_t child_position) { internal::MustBeCSTSymbolOrNode(symbol); const Symbol* subtree = GetSubtreeAsSymbol(symbol, parent_must_be_node_enum, child_position); - // TODO(ikr): avoid this check-failure. - CHECK(subtree != nullptr) - << symbol.Kind() << " e:" << parent_must_be_node_enum - << " p:" << child_position; - return SymbolCastToLeaf(*subtree); + if (!subtree) return nullptr; + return &SymbolCastToLeaf(*subtree); } template diff --git a/common/text/tree_utils_test.cc b/common/text/tree_utils_test.cc index 2f5416d0b..963b1b324 100644 --- a/common/text/tree_utils_test.cc +++ b/common/text/tree_utils_test.cc @@ -1435,8 +1435,8 @@ TEST(GetSubtreeAsNodeTest, ValidatedFoundNodeEnumChildMismatches) { TEST(GetSubtreeAsLeafTest, ValidatedFoundLeaf) { auto root = TNode(FakeEnum::kZero, Leaf(7, "foo")); - const auto& leaf = GetSubtreeAsLeaf(*root, FakeEnum::kZero, 0); - EXPECT_EQ(leaf.get().token_enum(), 7); + const SyntaxTreeLeaf* leaf = GetSubtreeAsLeaf(*root, FakeEnum::kZero, 0); + EXPECT_EQ(leaf->get().token_enum(), 7); } TEST(GetSubtreeAsLeafTest, GotNodeInsteadOfLeaf) { diff --git a/verilog/CST/class.cc b/verilog/CST/class.cc index cae74eb9f..a9a09a915 100644 --- a/verilog/CST/class.cc +++ b/verilog/CST/class.cc @@ -56,9 +56,10 @@ const verible::SyntaxTreeNode& GetClassHeader( const verible::SyntaxTreeLeaf& GetClassName( const verible::Symbol& class_declaration) { const auto& header_node = GetClassHeader(class_declaration); - const auto& name_leaf = + const verible::SyntaxTreeLeaf* name_leaf = verible::GetSubtreeAsLeaf(header_node, NodeEnum::kClassHeader, 3); - return name_leaf; + // TODO(hzeller): bubble up nullptr. + return *ABSL_DIE_IF_NULL(name_leaf); } const verible::SyntaxTreeNode* GetExtendedClass( @@ -79,9 +80,8 @@ const verible::SyntaxTreeLeaf* GetClassEndLabel( if (label_node == nullptr) { return nullptr; } - const auto& class_name = verible::GetSubtreeAsLeaf( - verible::SymbolCastToNode(*label_node), NodeEnum::kLabel, 1); - return &class_name; + return verible::GetSubtreeAsLeaf(verible::SymbolCastToNode(*label_node), + NodeEnum::kLabel, 1); } const verible::SyntaxTreeNode& GetClassItemList( @@ -112,7 +112,7 @@ const verible::SyntaxTreeNode& GetClassConstructorStatementList( NodeEnum::kClassConstructor, 2); } -const verible::SyntaxTreeLeaf& GetNewKeywordFromClassConstructor( +const verible::SyntaxTreeLeaf* GetNewKeywordFromClassConstructor( const verible::Symbol& class_constructor) { const verible::SyntaxTreeNode& constructor_prototype = verible::GetSubtreeAsNode(class_constructor, NodeEnum::kClassConstructor, diff --git a/verilog/CST/class.h b/verilog/CST/class.h index bccafef19..2c7d190f8 100644 --- a/verilog/CST/class.h +++ b/verilog/CST/class.h @@ -71,7 +71,7 @@ const verible::SyntaxTreeNode& GetClassConstructorStatementList( const verible::Symbol& class_constructor); // Returns the leaf spanning the "new" keyword from class constructor. -const verible::SyntaxTreeLeaf& GetNewKeywordFromClassConstructor( +const verible::SyntaxTreeLeaf* GetNewKeywordFromClassConstructor( const verible::Symbol& class_constructor); } // namespace verilog diff --git a/verilog/CST/class_test.cc b/verilog/CST/class_test.cc index 54cba4c35..5bc002844 100644 --- a/verilog/CST/class_test.cc +++ b/verilog/CST/class_test.cc @@ -287,10 +287,10 @@ TEST(GetClassConstructorTest, GetNewKeyword) { std::vector keywords; for (const auto& constructor : constructors) { - const auto& keyword = + const auto* keyword = GetNewKeywordFromClassConstructor(*constructor.match); keywords.emplace_back( - TreeSearchMatch{&keyword, {/* ignored context */}}); + TreeSearchMatch{keyword, {/* ignored context */}}); } return keywords; }); diff --git a/verilog/CST/declaration.cc b/verilog/CST/declaration.cc index 1667e167b..d9eee6f21 100644 --- a/verilog/CST/declaration.cc +++ b/verilog/CST/declaration.cc @@ -119,16 +119,18 @@ const verible::SyntaxTreeNode* GetParamListFromDataDeclaration( const verible::TokenInfo& GetModuleInstanceNameTokenInfoFromGateInstance( const verible::Symbol& gate_instance) { - const verible::SyntaxTreeLeaf& instance_name = + const verible::SyntaxTreeLeaf* instance_name = GetSubtreeAsLeaf(gate_instance, NodeEnum::kGateInstance, 0); - return instance_name.get(); + // TODO(hzeller): bubble up nullptr + return ABSL_DIE_IF_NULL(instance_name)->get(); } const verible::TokenInfo& GetInstanceNameTokenInfoFromRegisterVariable( const verible::Symbol& regiseter_variable) { - const verible::SyntaxTreeLeaf& instance_name = + const verible::SyntaxTreeLeaf* instance_name = GetSubtreeAsLeaf(regiseter_variable, NodeEnum::kRegisterVariable, 0); - return instance_name.get(); + // TODO(hzeller): bubble up nullptr. + return ABSL_DIE_IF_NULL(instance_name)->get(); } const verible::SyntaxTreeNode& GetParenGroupFromModuleInstantiation( diff --git a/verilog/CST/macro.cc b/verilog/CST/macro.cc index b403d61a4..b24015984 100644 --- a/verilog/CST/macro.cc +++ b/verilog/CST/macro.cc @@ -52,11 +52,14 @@ std::vector FindAllMacroDefinitionsArgs( } const TokenInfo& GetMacroCallId(const Symbol& s) { - return GetSubtreeAsLeaf(s, NodeEnum::kMacroCall, 0).get(); + // TODO(hzeller): bubble up nullptr. + return ABSL_DIE_IF_NULL(GetSubtreeAsLeaf(s, NodeEnum::kMacroCall, 0))->get(); } const TokenInfo& GetMacroGenericItemId(const Symbol& s) { - return GetSubtreeAsLeaf(s, NodeEnum::kMacroGenericItem, 0).get(); + // TODO(hzeller): bubble up nullptr. + return ABSL_DIE_IF_NULL(GetSubtreeAsLeaf(s, NodeEnum::kMacroGenericItem, 0)) + ->get(); } const SyntaxTreeNode& GetMacroCallParenGroup(const Symbol& s) { @@ -77,13 +80,13 @@ bool MacroCallArgsIsEmpty(const SyntaxTreeNode& args) { return sub.front() == nullptr; } -const verible::SyntaxTreeLeaf& GetMacroName( +const verible::SyntaxTreeLeaf* GetMacroName( const verible::Symbol& preprocessor_define) { return GetSubtreeAsLeaf(preprocessor_define, NodeEnum::kPreprocessorDefine, 1); } -const verible::SyntaxTreeLeaf& GetMacroArgName( +const verible::SyntaxTreeLeaf* GetMacroArgName( const verible::Symbol& macro_formal_arg) { return GetSubtreeAsLeaf(macro_formal_arg, NodeEnum::kMacroFormalArg, 0); } diff --git a/verilog/CST/macro.h b/verilog/CST/macro.h index 4962ce65c..5705d7e98 100644 --- a/verilog/CST/macro.h +++ b/verilog/CST/macro.h @@ -64,12 +64,12 @@ const verible::SyntaxTreeNode& GetMacroCallArgs(const verible::Symbol&); bool MacroCallArgsIsEmpty(const verible::SyntaxTreeNode&); // Returns the leaf node containing the macro name from node tagged with -// kPreprocessorDefine. -const verible::SyntaxTreeLeaf& GetMacroName(const verible::Symbol&); +// kPreprocessorDefine or nullptr if it doesn't exist. +const verible::SyntaxTreeLeaf* GetMacroName(const verible::Symbol&); // Returns the leaf node containing the macro arg name from node tagged with -// kMacroFormalArg. -const verible::SyntaxTreeLeaf& GetMacroArgName(const verible::Symbol&); +// kMacroFormalArg or nullptr if it doesn't exist. +const verible::SyntaxTreeLeaf* GetMacroArgName(const verible::Symbol&); // Returns the leaf node containing the filename from the node tagged with // kPreprocessorInclude or nullptr if the argument is not a simple diff --git a/verilog/CST/macro_test.cc b/verilog/CST/macro_test.cc index 15bc8521e..cf8aa4892 100644 --- a/verilog/CST/macro_test.cc +++ b/verilog/CST/macro_test.cc @@ -219,8 +219,8 @@ TEST(FindAllMacroDefinitions, MacroName) { const auto decls = FindAllMacroDefinitions(*ABSL_DIE_IF_NULL(root)); std::vector names; for (const auto& decl : decls) { - const auto& type = GetMacroName(*decl.match); - names.push_back(TreeSearchMatch{&type, {/* ignored context */}}); + const auto* type = GetMacroName(*decl.match); + names.push_back(TreeSearchMatch{type, {/* ignored context */}}); } return names; }); @@ -271,8 +271,8 @@ TEST(FindAllMacroDefinitions, MacroArgsName) { for (const auto& decl : decls) { const auto& args = FindAllMacroDefinitionsArgs(*decl.match); for (const auto& arg : args) { - const auto& name = GetMacroArgName(*arg.match); - names.push_back(TreeSearchMatch{&name, {/* ignored context */}}); + const auto* name = GetMacroArgName(*arg.match); + names.push_back(TreeSearchMatch{name, {/* ignored context */}}); } } return names; diff --git a/verilog/CST/module.cc b/verilog/CST/module.cc index afd2c038a..1de4dec4e 100644 --- a/verilog/CST/module.cc +++ b/verilog/CST/module.cc @@ -71,16 +71,17 @@ const SyntaxTreeNode& GetInterfaceHeader(const Symbol& module_symbol) { NodeEnum::kModuleHeader); } -const verible::SyntaxTreeLeaf& GetModuleName(const Symbol& s) { +const verible::SyntaxTreeLeaf* GetModuleName(const Symbol& s) { const auto& header_node = GetModuleHeader(s); return verible::GetSubtreeAsLeaf(header_node, NodeEnum::kModuleHeader, 2); } const TokenInfo& GetInterfaceNameToken(const Symbol& s) { const auto& header_node = GetInterfaceHeader(s); - const auto& name_leaf = + const verible::SyntaxTreeLeaf* name_leaf = verible::GetSubtreeAsLeaf(header_node, NodeEnum::kModuleHeader, 2); - return name_leaf.get(); + // TODO(hzeller): bubble up nullptr. + return ABSL_DIE_IF_NULL(name_leaf)->get(); } const SyntaxTreeNode* GetModulePortParenGroup( @@ -112,9 +113,8 @@ const verible::SyntaxTreeLeaf* GetModuleEndLabel( if (label_node == nullptr) { return nullptr; } - const auto& module_name = verible::GetSubtreeAsLeaf( - verible::SymbolCastToNode(*label_node), NodeEnum::kLabel, 1); - return &module_name; + return verible::GetSubtreeAsLeaf(verible::SymbolCastToNode(*label_node), + NodeEnum::kLabel, 1); } const verible::SyntaxTreeNode& GetModuleItemList( diff --git a/verilog/CST/module.h b/verilog/CST/module.h index 1428515f7..adb6caa33 100644 --- a/verilog/CST/module.h +++ b/verilog/CST/module.h @@ -76,8 +76,9 @@ const verible::SyntaxTreeNode& GetModuleHeader(const verible::Symbol&); // Returns the full header of an interface (params, ports, etc...). const verible::SyntaxTreeNode& GetInterfaceHeader(const verible::Symbol&); -// Extract the subnode of a module declaration that is the module name. -const verible::SyntaxTreeLeaf& GetModuleName(const verible::Symbol&); +// Extract the subnode of a module declaration that is the module name or +// nullptr if not found. +const verible::SyntaxTreeLeaf* GetModuleName(const verible::Symbol&); // Extract the subnode of an interface declaration that is the module name. const verible::TokenInfo& GetInterfaceNameToken(const verible::Symbol&); diff --git a/verilog/CST/module_test.cc b/verilog/CST/module_test.cc index 7351f3265..5d7bd5488 100644 --- a/verilog/CST/module_test.cc +++ b/verilog/CST/module_test.cc @@ -124,8 +124,8 @@ TEST(GetModuleNameTokenTest, ValidModule) { std::vector names; for (const auto& instance : programs) { - const auto& name = GetModuleName(*instance.match); - names.emplace_back(TreeSearchMatch{&name, {/* ignored context */}}); + const auto* name = GetModuleName(*instance.match); + names.emplace_back(TreeSearchMatch{name, {/* ignored context */}}); } return names; }); @@ -148,8 +148,8 @@ TEST(GetModuleNameTokenTest, ValidInterface) { std::vector names; for (const auto& instance : programs) { - const auto& name = GetModuleName(*instance.match); - names.emplace_back(TreeSearchMatch{&name, {/* ignored context */}}); + const auto* name = GetModuleName(*instance.match); + names.emplace_back(TreeSearchMatch{name, {/* ignored context */}}); } return names; }); @@ -172,8 +172,8 @@ TEST(GetModuleNameTokenTest, ValidProgram) { std::vector names; for (const auto& instance : programs) { - const auto& name = GetModuleName(*instance.match); - names.emplace_back(TreeSearchMatch{&name, {/* ignored context */}}); + const auto* name = GetModuleName(*instance.match); + names.emplace_back(TreeSearchMatch{name, {/* ignored context */}}); } return names; }); diff --git a/verilog/CST/net.cc b/verilog/CST/net.cc index 1c385dfad..8c04c7ec2 100644 --- a/verilog/CST/net.cc +++ b/verilog/CST/net.cc @@ -54,12 +54,12 @@ static bool ExpectedTagPredicate(const Symbol& symbol) { return symbol.Tag() == var_symbol || symbol.Tag() == assign_symbol; } -const verible::SyntaxTreeLeaf& GetNameLeafOfNetVariable( +const verible::SyntaxTreeLeaf* GetNameLeafOfNetVariable( const verible::Symbol& net_variable) { return verible::GetSubtreeAsLeaf(net_variable, NodeEnum::kNetVariable, 0); } -const verible::SyntaxTreeLeaf& GetNameLeafOfRegisterVariable( +const verible::SyntaxTreeLeaf* GetNameLeafOfRegisterVariable( const verible::Symbol& register_variable) { return verible::GetSubtreeAsLeaf(register_variable, NodeEnum::kRegisterVariable, 0); diff --git a/verilog/CST/net.h b/verilog/CST/net.h index fd71c66cd..ba960dccf 100644 --- a/verilog/CST/net.h +++ b/verilog/CST/net.h @@ -38,12 +38,12 @@ std::vector FindAllNetDeclarations( std::vector GetIdentifiersFromNetDeclaration( const verible::Symbol& symbol); -// Returns the declared identifier from a kNetVariable. -const verible::SyntaxTreeLeaf& GetNameLeafOfNetVariable( +// Returns the declared identifier from a kNetVariable or nullptr if invalid. +const verible::SyntaxTreeLeaf* GetNameLeafOfNetVariable( const verible::Symbol& net_variable); // Returns the declared identifier from a kRegisterVariable. -const verible::SyntaxTreeLeaf& GetNameLeafOfRegisterVariable( +const verible::SyntaxTreeLeaf* GetNameLeafOfRegisterVariable( const verible::Symbol& register_variable); } // namespace verilog diff --git a/verilog/CST/net_test.cc b/verilog/CST/net_test.cc index 982a1cf06..c4d14c03e 100644 --- a/verilog/CST/net_test.cc +++ b/verilog/CST/net_test.cc @@ -310,9 +310,9 @@ TEST(GetNameLeafOfNetVariableTest, Various) { std::vector net_ids; for (const auto& decl : net_decls) { - const auto& net_name = GetNameLeafOfNetVariable(*decl.match); + const auto* net_name = GetNameLeafOfNetVariable(*decl.match); net_ids.emplace_back( - TreeSearchMatch{&net_name, {/* ignored context */}}); + TreeSearchMatch{net_name, {/* ignored context */}}); } return net_ids; }); @@ -376,9 +376,9 @@ TEST(GetNameLeafOfRegisterVariableTest, Various) { std::vector net_ids; for (const auto& decl : net_decls) { - const auto& net_name = GetNameLeafOfRegisterVariable(*decl.match); + const auto* net_name = GetNameLeafOfRegisterVariable(*decl.match); net_ids.emplace_back( - TreeSearchMatch{&net_name, {/* ignored context */}}); + TreeSearchMatch{net_name, {/* ignored context */}}); } return net_ids; }); diff --git a/verilog/CST/package.cc b/verilog/CST/package.cc index 7baa6c4ac..5fb7441c3 100644 --- a/verilog/CST/package.cc +++ b/verilog/CST/package.cc @@ -39,11 +39,11 @@ std::vector FindAllPackageImportItems( } const verible::TokenInfo& GetPackageNameToken(const verible::Symbol& s) { - const auto& name_node = GetPackageNameLeaf(s); - return name_node.get(); + // TODO(hzeller): bubble up nullptr. + return ABSL_DIE_IF_NULL(GetPackageNameLeaf(s))->get(); } -const verible::SyntaxTreeLeaf& GetPackageNameLeaf(const verible::Symbol& s) { +const verible::SyntaxTreeLeaf* GetPackageNameLeaf(const verible::Symbol& s) { return verible::GetSubtreeAsLeaf(s, NodeEnum::kPackageDeclaration, 2); } @@ -54,9 +54,8 @@ const verible::SyntaxTreeLeaf* GetPackageNameEndLabel( if (label_node == nullptr) { return nullptr; } - const auto& package_name = verible::GetSubtreeAsLeaf( - verible::SymbolCastToNode(*label_node), NodeEnum::kLabel, 1); - return &package_name; + return verible::GetSubtreeAsLeaf(verible::SymbolCastToNode(*label_node), + NodeEnum::kLabel, 1); } const verible::Symbol* GetPackageItemList( @@ -72,7 +71,7 @@ const verible::SyntaxTreeNode& GetScopePrefixFromPackageImportItem( NodeEnum::kScopePrefix); } -const verible::SyntaxTreeLeaf& GetImportedPackageName( +const verible::SyntaxTreeLeaf* GetImportedPackageName( const verible::Symbol& package_import_item) { return verible::GetSubtreeAsLeaf( GetScopePrefixFromPackageImportItem(package_import_item), @@ -81,14 +80,15 @@ const verible::SyntaxTreeLeaf& GetImportedPackageName( const verible::SyntaxTreeLeaf* GeImportedItemNameFromPackageImportItem( const verible::Symbol& package_import_item) { - const auto& imported_item = verible::GetSubtreeAsLeaf( + const verible::SyntaxTreeLeaf* imported_item = verible::GetSubtreeAsLeaf( package_import_item, NodeEnum::kPackageImportItem, 1); - if (imported_item.get().token_enum() != verilog_tokentype::SymbolIdentifier) { + if (!imported_item || imported_item->get().token_enum() != + verilog_tokentype::SymbolIdentifier) { return nullptr; } - return &imported_item; + return imported_item; } } // namespace verilog diff --git a/verilog/CST/package.h b/verilog/CST/package.h index a1ae63837..1ff143945 100644 --- a/verilog/CST/package.h +++ b/verilog/CST/package.h @@ -38,7 +38,7 @@ std::vector FindAllPackageImportItems( const verible::TokenInfo& GetPackageNameToken(const verible::Symbol&); // Return the node spanning the name of the package. -const verible::SyntaxTreeLeaf& GetPackageNameLeaf(const verible::Symbol& s); +const verible::SyntaxTreeLeaf* GetPackageNameLeaf(const verible::Symbol& s); // Extracts the node that spans the name of the package after endpackage if // exists. @@ -57,7 +57,7 @@ const verible::SyntaxTreeNode& GetScopePrefixFromPackageImportItem( // Extracts package name for package import (node tagged with // kPackageImportItem). // e.g import pkg::my_integer return the node spanning "pkg". -const verible::SyntaxTreeLeaf& GetImportedPackageName( +const verible::SyntaxTreeLeaf* GetImportedPackageName( const verible::Symbol& package_import_item); // Extracts the symbol identifier from PackageImportItem if exits. diff --git a/verilog/CST/package_test.cc b/verilog/CST/package_test.cc index 3db1f901d..6125464d8 100644 --- a/verilog/CST/package_test.cc +++ b/verilog/CST/package_test.cc @@ -302,8 +302,8 @@ TEST(GetPackageNameTokenTest, VariousPackageTokenTests) { std::vector declIdentifiers; for (const auto& decl : declarations) { - const auto& packageToken = GetPackageNameLeaf(*decl.match); - declIdentifiers.push_back(TreeSearchMatch{&packageToken, {}}); + const auto* packageToken = GetPackageNameLeaf(*decl.match); + declIdentifiers.push_back(TreeSearchMatch{packageToken, {}}); } return declIdentifiers; }); @@ -418,8 +418,8 @@ TEST(PackageImportTest, GetImportedPackageName) { std::vector names; for (const auto& decl : decls) { - const auto& name = GetImportedPackageName(*decl.match); - names.emplace_back(TreeSearchMatch{&name, {/* ignored context */}}); + const auto* name = GetImportedPackageName(*decl.match); + names.emplace_back(TreeSearchMatch{name, {/* ignored context */}}); } return names; }); diff --git a/verilog/CST/parameters.cc b/verilog/CST/parameters.cc index d55216eef..27260fa4b 100644 --- a/verilog/CST/parameters.cc +++ b/verilog/CST/parameters.cc @@ -167,7 +167,7 @@ const verible::SyntaxTreeNode* GetTypeAssignmentFromParamDeclaration( const verible::SyntaxTreeLeaf* GetIdentifierLeafFromTypeAssignment( const verible::Symbol& symbol) { - return &verible::GetSubtreeAsLeaf(symbol, NodeEnum::kTypeAssignment, 0); + return verible::GetSubtreeAsLeaf(symbol, NodeEnum::kTypeAssignment, 0); } const verible::SyntaxTreeNode* GetExpressionFromTypeAssignment( @@ -224,8 +224,10 @@ bool IsTypeInfoEmpty(const verible::Symbol& symbol) { const verible::SyntaxTreeLeaf& GetNamedParamFromActualParam( const verible::Symbol& param_by_name) { - return *AutoUnwrapIdentifier( - verible::GetSubtreeAsLeaf(param_by_name, NodeEnum::kParamByName, 1)); + // TODO(hzeller): bubble up nullptr. + const verible::SyntaxTreeLeaf* param_name = + verible::GetSubtreeAsLeaf(param_by_name, NodeEnum::kParamByName, 1); + return *AutoUnwrapIdentifier(*ABSL_DIE_IF_NULL(param_name)); } const verible::SyntaxTreeNode* GetParenGroupFromActualParam( diff --git a/verilog/CST/port.cc b/verilog/CST/port.cc index 877214189..f3b064d9c 100644 --- a/verilog/CST/port.cc +++ b/verilog/CST/port.cc @@ -112,7 +112,7 @@ const SyntaxTreeLeaf* GetIdentifierFromTaskFunctionPortItem( return AutoUnwrapIdentifier(*ABSL_DIE_IF_NULL(type_id_dimensions[1].get())); } -const verible::SyntaxTreeLeaf& GetActualNamedPortName( +const verible::SyntaxTreeLeaf* GetActualNamedPortName( const verible::Symbol& actual_named_port) { return verible::GetSubtreeAsLeaf(actual_named_port, NodeEnum::kActualNamedPort, 1); diff --git a/verilog/CST/port.h b/verilog/CST/port.h index b4d51a752..6eb47bfc4 100644 --- a/verilog/CST/port.h +++ b/verilog/CST/port.h @@ -87,7 +87,8 @@ const verible::SyntaxTreeNode& GetUnpackedDimensionsFromTaskFunctionPortItem( // Returns the leaf node containing the name of the actual named port. // example: from ".x(y)" this returns the leaf spanning "x". -const verible::SyntaxTreeLeaf& GetActualNamedPortName(const verible::Symbol&); +// Returns nullptr if it doesn't exist. +const verible::SyntaxTreeLeaf* GetActualNamedPortName(const verible::Symbol&); // Returns the node containing the paren group of the actual named port (if // exists). diff --git a/verilog/CST/port_test.cc b/verilog/CST/port_test.cc index 37eb52348..74c586124 100644 --- a/verilog/CST/port_test.cc +++ b/verilog/CST/port_test.cc @@ -325,8 +325,8 @@ TEST(GetActualNamedPort, GetActualPortName) { std::vector names; for (const auto& port : ports) { - const auto& name = GetActualNamedPortName(*port.match); - names.emplace_back(TreeSearchMatch{&name, {/* ignored context */}}); + const auto* name = GetActualNamedPortName(*port.match); + names.emplace_back(TreeSearchMatch{name, {/* ignored context */}}); } return names; }); diff --git a/verilog/CST/type.cc b/verilog/CST/type.cc index 952667cf8..1dad0f44d 100644 --- a/verilog/CST/type.cc +++ b/verilog/CST/type.cc @@ -263,8 +263,8 @@ GetSymbolIdentifierFromDataTypeImplicitIdDimensions( identifier->Kind() == verible::SymbolKind::kLeaf) { return {&verible::SymbolCastToLeaf(*identifier), 2}; } - return {&verible::GetSubtreeAsLeaf( - struct_union_member, NodeEnum::kDataTypeImplicitIdDimensions, 1), + return {verible::GetSubtreeAsLeaf(struct_union_member, + NodeEnum::kDataTypeImplicitIdDimensions, 1), 1}; } @@ -288,12 +288,12 @@ const verible::SyntaxTreeNode* GetReferencedTypeOfTypeDeclaration( NodeEnum::kTypeDeclaration, 1); } -const verible::SyntaxTreeLeaf& GetSymbolIdentifierFromEnumName( +const verible::SyntaxTreeLeaf* GetSymbolIdentifierFromEnumName( const verible::Symbol& enum_name) { return verible::GetSubtreeAsLeaf(enum_name, NodeEnum::kEnumName, 0); } -const verible::SyntaxTreeLeaf& GetTypeIdentifierFromInterfaceType( +const verible::SyntaxTreeLeaf* GetTypeIdentifierFromInterfaceType( const verible::Symbol& interface_type) { return verible::GetSubtreeAsLeaf(interface_type, NodeEnum::kInterfaceType, 2); } @@ -306,7 +306,7 @@ const verible::Symbol* GetTypeIdentifierFromInstantiationType( return GetTypeIdentifierFromDataType(data_type); } if (NodeEnum(data_type.Tag().tag) == NodeEnum::kInterfaceType) { - return &GetTypeIdentifierFromInterfaceType(data_type); + return GetTypeIdentifierFromInterfaceType(data_type); } return nullptr; } diff --git a/verilog/CST/type.h b/verilog/CST/type.h index 92fcbab3d..a9e2d26e9 100644 --- a/verilog/CST/type.h +++ b/verilog/CST/type.h @@ -209,9 +209,10 @@ const verible::SyntaxTreeLeaf* GetNonprimitiveTypeOfDataTypeImplicitDimensions( const verible::SyntaxTreeNode* GetParamListFromInstantiationType( const verible::Symbol& instantiation_type); -// Extracts symbol identifier node from node tagged with kEnumName. +// Extracts symbol identifier node from node tagged with kEnumName or +// nullptr if it doesn't exist. // e.g from "enum {first}" extracts "first". -const verible::SyntaxTreeLeaf& GetSymbolIdentifierFromEnumName( +const verible::SyntaxTreeLeaf* GetSymbolIdentifierFromEnumName( const verible::Symbol& enum_name); // Returns symbol identifier node for the type name from node tagged with diff --git a/verilog/CST/type_test.cc b/verilog/CST/type_test.cc index 00622be08..fcdc1450e 100644 --- a/verilog/CST/type_test.cc +++ b/verilog/CST/type_test.cc @@ -535,8 +535,8 @@ TEST(GetEnumName, GetEnumNameIdentifier) { std::vector names; for (const auto& decl : instances) { - const auto& name = GetSymbolIdentifierFromEnumName(*decl.match); - names.emplace_back(TreeSearchMatch{&name, {/* ignored context */}}); + const auto* name = GetSymbolIdentifierFromEnumName(*decl.match); + names.emplace_back(TreeSearchMatch{name, {/* ignored context */}}); } return names; }); diff --git a/verilog/analysis/checkers/always_comb_blocking_rule.cc b/verilog/analysis/checkers/always_comb_blocking_rule.cc index d82e70d58..98ebb68d6 100644 --- a/verilog/analysis/checkers/always_comb_blocking_rule.cc +++ b/verilog/analysis/checkers/always_comb_blocking_rule.cc @@ -72,11 +72,11 @@ void AlwaysCombBlockingRule::HandleSymbol(const verible::Symbol& symbol, if (node == nullptr) continue; - const auto& leaf = verible::GetSubtreeAsLeaf( + const verible::SyntaxTreeLeaf* leaf = verible::GetSubtreeAsLeaf( *node, NodeEnum::kNonblockingAssignmentStatement, 1); - if (leaf.get().token_enum() == TK_LE) - violations_.insert(LintViolation(leaf, kMessage, match.context)); + if (leaf && leaf->get().token_enum() == TK_LE) + violations_.insert(LintViolation(*leaf, kMessage, match.context)); } } } diff --git a/verilog/analysis/checkers/banned_declared_name_patterns_rule.cc b/verilog/analysis/checkers/banned_declared_name_patterns_rule.cc index c13e0ce28..02bfd5d86 100644 --- a/verilog/analysis/checkers/banned_declared_name_patterns_rule.cc +++ b/verilog/analysis/checkers/banned_declared_name_patterns_rule.cc @@ -57,11 +57,13 @@ void BannedDeclaredNamePatternsRule::HandleNode( const auto tag = static_cast(node.Tag().tag); switch (tag) { case NodeEnum::kModuleDeclaration: { - const auto& module_match = GetModuleName(node); - absl::string_view module_id = module_match.get().text(); + const auto* module_match = GetModuleName(node); + if (module_match) { + const absl::string_view module_id = module_match->get().text(); - if (absl::EqualsIgnoreCase(module_id, "ILLEGALNAME")) { - violations_.insert(LintViolation(module_match.get(), kMessage)); + if (absl::EqualsIgnoreCase(module_id, "ILLEGALNAME")) { + violations_.insert(LintViolation(module_match->get(), kMessage)); + } } break; } diff --git a/verilog/analysis/checkers/forbid_defparam_rule.cc b/verilog/analysis/checkers/forbid_defparam_rule.cc index f1b04f6a4..768f63527 100644 --- a/verilog/analysis/checkers/forbid_defparam_rule.cc +++ b/verilog/analysis/checkers/forbid_defparam_rule.cc @@ -56,11 +56,14 @@ void ForbidDefparamRule::HandleSymbol( const verible::Symbol& symbol, const verible::SyntaxTreeContext& context) { verible::matcher::BoundSymbolManager manager; if (OverrideMatcher().Matches(symbol, &manager)) { - const auto& defparam_token = - GetSubtreeAsLeaf(symbol, NodeEnum::kParameterOverride, 0).get(); - CHECK_EQ(defparam_token.token_enum(), TK_defparam); - violations_.insert( - verible::LintViolation(defparam_token, kMessage, context)); + const verible::SyntaxTreeLeaf* defparam = + GetSubtreeAsLeaf(symbol, NodeEnum::kParameterOverride, 0); + if (defparam) { + const auto& defparam_token = defparam->get(); + CHECK_EQ(defparam_token.token_enum(), TK_defparam); + violations_.insert( + verible::LintViolation(defparam_token, kMessage, context)); + } } } diff --git a/verilog/analysis/checkers/module_filename_rule.cc b/verilog/analysis/checkers/module_filename_rule.cc index 84f368ea1..7bb8c9d0c 100644 --- a/verilog/analysis/checkers/module_filename_rule.cc +++ b/verilog/analysis/checkers/module_filename_rule.cc @@ -67,8 +67,8 @@ const LintRuleDescriptor& ModuleFilenameRule::GetDescriptor() { static bool ModuleNameMatches(const verible::Symbol& s, absl::string_view name) { - const auto& token_info = GetModuleName(s).get(); - return token_info.text() == name; + const auto* module_leaf = GetModuleName(s); + return module_leaf && module_leaf->get().text() == name; } void ModuleFilenameRule::Lint(const TextStructureView& text_structure, @@ -114,9 +114,12 @@ void ModuleFilenameRule::Lint(const TextStructureView& text_structure, } // Only report a violation on the last module declaration. - const auto& last_module_id = GetModuleName(*module_cleaned.back().match); - violations_.insert(verible::LintViolation( - last_module_id.get(), absl::StrCat(kMessage, "\"", unitname, "\""))); + const auto* last_module_id = GetModuleName(*module_cleaned.back().match); + if (!last_module_id) LOG(ERROR) << "Couldn't extract module name"; + if (last_module_id) { + violations_.insert(verible::LintViolation( + last_module_id->get(), absl::StrCat(kMessage, "\"", unitname, "\""))); + } } LintRuleStatus ModuleFilenameRule::Report() const { diff --git a/verilog/analysis/checkers/one_module_per_file_rule.cc b/verilog/analysis/checkers/one_module_per_file_rule.cc index 830a8847e..e5499fd37 100644 --- a/verilog/analysis/checkers/one_module_per_file_rule.cc +++ b/verilog/analysis/checkers/one_module_per_file_rule.cc @@ -78,9 +78,13 @@ void OneModulePerFileRule::Lint(const TextStructureView& text_structure, if (module_cleaned.size() > 1) { // Report second module declaration - const auto& second_module_id = GetModuleName(*module_cleaned[1].match); - violations_.insert(verible::LintViolation( - second_module_id.get(), absl::StrCat(kMessage, module_cleaned.size()))); + const auto* second_module_id = GetModuleName(*module_cleaned[1].match); + if (!second_module_id) LOG(ERROR) << "Couldn't extract module name"; + if (second_module_id) { + violations_.insert(verible::LintViolation( + second_module_id->get(), + absl::StrCat(kMessage, module_cleaned.size()))); + } } } diff --git a/verilog/analysis/symbol_table.cc b/verilog/analysis/symbol_table.cc index 4d26f2616..ef711a924 100644 --- a/verilog/analysis/symbol_table.cc +++ b/verilog/analysis/symbol_table.cc @@ -912,7 +912,9 @@ class SymbolTable::Builder : public TreeContextVisitor { } void DeclareModule(const SyntaxTreeNode& module) { - DeclareScopedElementAndDescend(module, GetModuleName(module).get().text(), + const SyntaxTreeLeaf* module_name = GetModuleName(module); + if (!module_name) return; + DeclareScopedElementAndDescend(module, module_name->get().text(), SymbolMetaType::kModule); } @@ -1089,16 +1091,20 @@ class SymbolTable::Builder : public TreeContextVisitor { } void DeclareNet(const SyntaxTreeNode& net_variable) { - const absl::string_view net_name( - GetNameLeafOfNetVariable(net_variable).get().text()); + const SyntaxTreeLeaf* net_variable_name = + GetNameLeafOfNetVariable(net_variable); + if (!net_variable_name) return; + const absl::string_view net_name(net_variable_name->get().text()); EmplaceTypedElementInCurrentScope(net_variable, net_name, SymbolMetaType::kDataNetVariableInstance); Descend(net_variable); } void DeclareRegister(const SyntaxTreeNode& reg_variable) { - const absl::string_view net_name( - GetNameLeafOfRegisterVariable(reg_variable).get().text()); + const SyntaxTreeLeaf* register_variable_name = + GetNameLeafOfRegisterVariable(reg_variable); + if (!register_variable_name) return; + const absl::string_view net_name(register_variable_name->get().text()); EmplaceTypedElementInCurrentScope(reg_variable, net_name, SymbolMetaType::kDataNetVariableInstance); Descend(reg_variable); diff --git a/verilog/tools/kythe/indexing_facts_tree_extractor.cc b/verilog/tools/kythe/indexing_facts_tree_extractor.cc index 863748af6..84a527bfe 100644 --- a/verilog/tools/kythe/indexing_facts_tree_extractor.cc +++ b/verilog/tools/kythe/indexing_facts_tree_extractor.cc @@ -684,10 +684,11 @@ void IndexingFactsTreeExtractor::ExtractModuleOrInterfaceOrProgram( void IndexingFactsTreeExtractor::ExtractModuleOrInterfaceOrProgramHeader( const SyntaxTreeNode& module_declaration_node) { // Extract module name e.g from "module my_module" extracts "my_module". - const SyntaxTreeLeaf& module_name_leaf = + const SyntaxTreeLeaf* module_name_leaf = GetModuleName(module_declaration_node); + if (!module_name_leaf) return; facts_tree_context_.top().Value().AppendAnchor( - Anchor(module_name_leaf.get())); + Anchor(module_name_leaf->get())); // Extract parameters if exist. const SyntaxTreeNode* param_declaration_list = @@ -812,10 +813,10 @@ void IndexingFactsTreeExtractor::ExtractModulePort( void IndexingFactsTreeExtractor::ExtractModuleNamedPort( const SyntaxTreeNode& actual_named_port) { + const SyntaxTreeLeaf* named_port = GetActualNamedPortName(actual_named_port); + if (!named_port) return; IndexingFactNode actual_port_node(IndexingNodeData( - IndexingFactType::kModuleNamedPort, - Anchor(GetActualNamedPortName(actual_named_port).get()))); - + IndexingFactType::kModuleNamedPort, Anchor(named_port->get()))); { const IndexingFactsTreeContext::AutoPop p(&facts_tree_context_, &actual_port_node); @@ -916,8 +917,10 @@ void IndexingFactsTreeExtractor::ExtractPackageDeclaration( const IndexingFactsTreeContext::AutoPop p(&facts_tree_context_, &package_node); // Extract package name. - facts_tree_context_.top().Value().AppendAnchor( - Anchor(GetPackageNameLeaf(package_declaration_node).get())); + const SyntaxTreeLeaf* pname = GetPackageNameLeaf(package_declaration_node); + if (pname) { + facts_tree_context_.top().Value().AppendAnchor(Anchor(pname->get())); + } // Extract package name after endpackage if exists. const SyntaxTreeLeaf* package_end_name = @@ -941,18 +944,22 @@ void IndexingFactsTreeExtractor::ExtractPackageDeclaration( void IndexingFactsTreeExtractor::ExtractMacroDefinition( const SyntaxTreeNode& preprocessor_definition) { + const SyntaxTreeLeaf* macro_name = GetMacroName(preprocessor_definition); + if (!macro_name) return; IndexingFactNode macro_node( - IndexingNodeData(IndexingFactType::kMacro, - Anchor(GetMacroName(preprocessor_definition).get()))); + IndexingNodeData(IndexingFactType::kMacro, Anchor(macro_name->get()))); // TODO(fangism): access directly, instead of searching. const std::vector args = FindAllMacroDefinitionsArgs(preprocessor_definition); for (const TreeSearchMatch& arg : args) { - macro_node.NewChild( - IndexingNodeData(IndexingFactType::kVariableDefinition, - Anchor(GetMacroArgName(*arg.match).get()))); + const SyntaxTreeLeaf* macro_arg_name = GetMacroArgName(*arg.match); + if (macro_arg_name) { + macro_node.NewChild( + IndexingNodeData(IndexingFactType::kVariableDefinition, + Anchor(macro_arg_name->get()))); + } } facts_tree_context_.top().NewChild(std::move(macro_node)); @@ -995,9 +1002,11 @@ void IndexingFactsTreeExtractor::ExtractMacroReference( void IndexingFactsTreeExtractor::ExtractClassConstructor( const SyntaxTreeNode& class_constructor) { + const SyntaxTreeLeaf* new_keyword = + GetNewKeywordFromClassConstructor(class_constructor); + if (!new_keyword) return; IndexingFactNode constructor_node(IndexingNodeData( - IndexingFactType::kConstructor, - Anchor(GetNewKeywordFromClassConstructor(class_constructor).get()))); + IndexingFactType::kConstructor, Anchor(new_keyword->get()))); { const IndexingFactsTreeContext::AutoPop p(&facts_tree_context_, @@ -1534,9 +1543,11 @@ void IndexingFactsTreeExtractor::ExtractParamByName( void IndexingFactsTreeExtractor::ExtractPackageImport( const SyntaxTreeNode& package_import_item) { - IndexingNodeData package_import_data( - IndexingFactType::kPackageImport, - Anchor(GetImportedPackageName(package_import_item).get())); + const SyntaxTreeLeaf* package_name = + GetImportedPackageName(package_import_item); + if (!package_name) return; + IndexingNodeData package_import_data(IndexingFactType::kPackageImport, + Anchor(package_name->get())); // Get the name of the imported item (if exists). // e.g pkg::var1 ==> return var1. @@ -1688,9 +1699,10 @@ void IndexingFactsTreeExtractor::ExtractInclude( void IndexingFactsTreeExtractor::ExtractEnumName( const SyntaxTreeNode& enum_name) { - IndexingFactNode enum_node(IndexingNodeData( - IndexingFactType::kConstant, - Anchor(GetSymbolIdentifierFromEnumName(enum_name).get()))); + const SyntaxTreeLeaf* symbol_id = GetSymbolIdentifierFromEnumName(enum_name); + if (!symbol_id) return; + IndexingFactNode enum_node( + IndexingNodeData(IndexingFactType::kConstant, Anchor(symbol_id->get()))); // Iterate over the children and traverse them to extract facts from inner // nodes and ignore the leaves. diff --git a/verilog/tools/kythe/indexing_facts_tree_extractor_test.cc b/verilog/tools/kythe/indexing_facts_tree_extractor_test.cc index 4e7b30d9b..9963e3315 100644 --- a/verilog/tools/kythe/indexing_facts_tree_extractor_test.cc +++ b/verilog/tools/kythe/indexing_facts_tree_extractor_test.cc @@ -962,47 +962,48 @@ TEST(FactsTreeExtractor, ClassTypeParams) { TEST(FactsTreeExtractor, ModuleInstanceWithActualNamedPorts) { constexpr int kTag = 1; // value doesn't matter - const verible::SyntaxTreeSearchTestCase kTestCase = {{"module ", - {kTag, "foo"}, - "(input ", - {kTag, "a"}, - ", input ", - {kTag, "b"}, - ", input wire ", - {kTag, "z"}, - ", output ", - {kTag, "h"}, - ");\nendmodule: ", - {kTag, "foo"}, - "\nmodule ", - {kTag, "bar"}, - "(input ", - {kTag, "a"}, - ", ", - {kTag, "b"}, - ", ", - {kTag, "c"}, - ", ", - {kTag, "h"}, - ");\n", - {kTag, "foo"}, - " ", - {kTag, "f1"}, - "(.", - {kTag, "a"}, - "(", - {kTag, "a"}, - "), .", - {kTag, "b"}, - "(", - {kTag, "b"}, - "), .", - {kTag, "z"}, - "(", - {kTag, "c"}, - "), .", - {kTag, "h"}, - ");\nendmodule"}}; + const verible::SyntaxTreeSearchTestCase kTestCase = { + {"module ", // 0 + {kTag, "foo"}, // 1 + "(input ", // 2 + {kTag, "a"}, // 3 + ", input ", // 4 + {kTag, "b"}, // 5 + ", input wire ", // 6 + {kTag, "z"}, // 7 + ", output ", // 8 + {kTag, "h"}, // 9 + ");\nendmodule: ", // 10 + {kTag, "foo"}, // 11 + "\nmodule ", // 12 + {kTag, "bar"}, // 13 + "(input ", // 14 + {kTag, "a"}, // 15 + ", ", // 16 + {kTag, "b"}, // 17 + ", ", // 18 + {kTag, "c"}, // 19 + ", ", // 20 + {kTag, "h"}, // 21 + ");\n", // 22 + {kTag, "foo"}, // 23 + " ", // 24 + {kTag, "f1"}, // 25 + "(.", // 26 + {kTag, "a"}, // 27 + "(", // 28 + {kTag, "a"}, // 29 + "), .", // 30 + {kTag, "b"}, // 31 + "(", // 32 + {kTag, "b"}, // 33 + "), .", // 34 + {kTag, "z"}, // 35 + "(", // 36 + {kTag, "c"}, // 37 + "), .", // 38 + {kTag, "h"}, // 39 + ");\nendmodule"}}; SimpleTestProject project(kTestCase.code); @@ -1122,6 +1123,75 @@ TEST(FactsTreeExtractor, ModuleInstanceWithActualNamedPorts) { EXPECT_EQ(result_pair.right, nullptr) << P(*result_pair.right); } +TEST(FactsTreeExtractor, ModuleInstanceWitStarExpandedPorts) { + constexpr int kTag = 1; // value doesn't matter + + const verible::SyntaxTreeSearchTestCase kTestCase = { + {"module ", // 00 + {kTag, "foo"}, // 01 + "(input ", // 02 + {kTag, "a"}, // 03 + ");\nendmodule\n", // 04 + "\nmodule ", // 05 + {kTag, "bar"}, // 06 + "(input ", // 07 + {kTag, "a"}, // 08 + ");\n", // 09 + {kTag, "foo"}, // 10 + " ", // 11 + {kTag, "f1"}, // 12 + "(.", // 13 + {kTag, "*"}, // 14 + ");\nendmodule"}}; + + SimpleTestProject project(kTestCase.code); + const IndexingFactNode expected( // + project.ExpectedFileListData(), + project.XUnit().RebaseFileTree( + T(project.XUnit().ExpectedFileData(), + // refers to module foo. + T( + D{ + IndexingFactType::kModule, + Anchor(kTestCase.expected_tokens[1]), + }, + // refers to a. + T(D{ + IndexingFactType::kVariableDefinition, + Anchor(kTestCase.expected_tokens[3]), + })), + // refers to module bar. + T( + D{ + IndexingFactType::kModule, + Anchor(kTestCase.expected_tokens[6]), + }, + // refers to input a. + T(D{ + IndexingFactType::kVariableDefinition, + Anchor(kTestCase.expected_tokens[8]), + }), + // refers to foo. + T( + D{ + IndexingFactType::kDataTypeReference, + Anchor(kTestCase.expected_tokens[10]), + }, + // refers to f1(.*) + T(D{ + IndexingFactType::kModuleInstance, + Anchor(kTestCase.expected_tokens[12]), + } // The .* is not recorded as named module port. + )))))); + + const auto facts_tree = project.Extract(); + + const auto result_pair = DeepEqual(facts_tree, expected); + const auto P = [&project](const T& t) { return project.TreePrinter(t); }; + EXPECT_EQ(result_pair.left, nullptr) << P(*result_pair.left); + EXPECT_EQ(result_pair.right, nullptr) << P(*result_pair.right); +} + TEST(FactsTreeExtractor, ModuleWithPortsDataTypeForwarding) { constexpr int kTag = 1; // value doesn't matter diff --git a/verilog/tools/ls/document-symbol-filler.cc b/verilog/tools/ls/document-symbol-filler.cc index 59df1fade..6f6fe6c43 100644 --- a/verilog/tools/ls/document-symbol-filler.cc +++ b/verilog/tools/ls/document-symbol-filler.cc @@ -59,11 +59,13 @@ void DocumentSymbolFiller::Visit(const verible::SyntaxTreeNode &node) { bool is_visible_node = false; switch (static_cast(node.Tag().tag)) { case verilog::NodeEnum::kModuleDeclaration: { - is_visible_node = true; - node_symbol.kind = kModuleSymbolKind; - const auto &name_leaf = verilog::GetModuleName(node); - node_symbol.selectionRange = RangeFromLeaf(name_leaf); - node_symbol.name = std::string(name_leaf.get().text()); + const auto *name_leaf = verilog::GetModuleName(node); + if (name_leaf) { + is_visible_node = true; + node_symbol.kind = kModuleSymbolKind; + node_symbol.selectionRange = RangeFromLeaf(*name_leaf); + node_symbol.name = std::string(name_leaf->get().text()); + } break; }