Merge pull request #1125 from hzeller/bubble-up-nullptr

Have GetSubtreeAsLeaf() return a pointer instead of a reference.
chipsalliance · Dec 21, 2021 · 034c05e · 034c05e
2 parents 9f0278e + e84b636
commit 034c05e
Show file tree

Hide file tree

Showing 34 changed files with 289 additions and 179 deletions.
diff --git a/common/text/tree_utils.h b/common/text/tree_utils.h
@@ -199,18 +199,16 @@ typename match_const<SyntaxTreeNode, S>::type& GetSubtreeAsNode(
 }
 
 // Same as GetSubtreeAsSymbol, but casts the result to a leaf.
+// If subtree does not exist, returns nullptr.
 template <class S, class E>
-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 <class S, class E>

diff --git 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
@@ -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
@@ -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
@@ -287,10 +287,10 @@ TEST(GetClassConstructorTest, GetNewKeyword) {
 
           std::vector<TreeSearchMatch> 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
@@ -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
@@ -52,11 +52,14 @@ std::vector<verible::TreeSearchMatch> 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
@@ -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
@@ -219,8 +219,8 @@ TEST(FindAllMacroDefinitions, MacroName) {
           const auto decls = FindAllMacroDefinitions(*ABSL_DIE_IF_NULL(root));
           std::vector<TreeSearchMatch> 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
@@ -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
@@ -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
@@ -124,8 +124,8 @@ TEST(GetModuleNameTokenTest, ValidModule) {
 
           std::vector<TreeSearchMatch> 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<TreeSearchMatch> 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<TreeSearchMatch> 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
@@ -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
@@ -38,12 +38,12 @@ std::vector<verible::TreeSearchMatch> FindAllNetDeclarations(
 std::vector<const verible::TokenInfo*> 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
@@ -310,9 +310,9 @@ TEST(GetNameLeafOfNetVariableTest, Various) {
 
           std::vector<TreeSearchMatch> 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<TreeSearchMatch> 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
@@ -39,11 +39,11 @@ std::vector<verible::TreeSearchMatch> 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
@@ -38,7 +38,7 @@ std::vector<verible::TreeSearchMatch> 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.