PR #447: Module port data type forwarding Fixes #419 GitHub PR https://github.com/google/verible/pull/447 Copybara import of the project: - 92798399785eeb230894e1b967a78bcc5e77e67a Propagating Port Type in Module by MinaToma <minatoma1999@gmail.com> - 55a9364fe71c02c39ce462afbde6bf886707a563 More Tests for Port Data Type Forwarding in Modules by MinaToma <minatoma1999@gmail.com> - 94c9da0a480d1c6ce0b9a0a31a8128787b4995d5 More Tests by MinaToma <minatoma1999@gmail.com> Closes #447 PiperOrigin-RevId: 331856863
diff --git a/verilog/CST/port.cc b/verilog/CST/port.cc index 8eebfcd..bab8765 100644 --- a/verilog/CST/port.cc +++ b/verilog/CST/port.cc
@@ -39,7 +39,7 @@ std::vector<verible::TreeSearchMatch> FindAllPortReferences( const verible::Symbol& root) { - return SearchSyntaxTree(root, NodekPortReference()); + return SearchSyntaxTree(root, NodekPort()); } std::vector<verible::TreeSearchMatch> FindAllTaskFunctionPortDeclarations( @@ -61,6 +61,12 @@ return AutoUnwrapIdentifier(*ABSL_DIE_IF_NULL(identifier_symbol)); } +const verible::SyntaxTreeNode& GetPortReferenceFromPort( + const verible::Symbol& port) { + return verible::GetSubtreeAsNode(port, NodeEnum::kPort, 0, + NodeEnum::kPortReference); +} + static const verible::SyntaxTreeNode& GetTypeIdDimensionsFromTaskFunctionPortItem(const Symbol& symbol) { return verible::GetSubtreeAsNode(
diff --git a/verilog/CST/port.h b/verilog/CST/port.h index d8827d7..ce11eaf 100644 --- a/verilog/CST/port.h +++ b/verilog/CST/port.h
@@ -33,7 +33,7 @@ std::vector<verible::TreeSearchMatch> FindAllModulePortDeclarations( const verible::Symbol&); -// Find all individual port references. +// Find all nodes tagged with kPort. std::vector<verible::TreeSearchMatch> FindAllPortReferences( const verible::Symbol&); @@ -46,6 +46,9 @@ const verible::SyntaxTreeLeaf* GetIdentifierFromPortReference( const verible::Symbol&); +// Extracts the node tagged with kPortReference from a node tagged with kPort. +const verible::SyntaxTreeNode& GetPortReferenceFromPort(const verible::Symbol&); + // Find all task/function port declarations. std::vector<verible::TreeSearchMatch> FindAllTaskFunctionPortDeclarations( const verible::Symbol&);
diff --git a/verilog/CST/port_test.cc b/verilog/CST/port_test.cc index 5505364..8168378 100644 --- a/verilog/CST/port_test.cc +++ b/verilog/CST/port_test.cc
@@ -282,7 +282,8 @@ std::vector<TreeSearchMatch> types; for (const auto& decl : decls) { - const auto* type = GetIdentifierFromPortReference(*decl.match); + const auto* type = + GetIdentifierFromPortReference(GetPortReferenceFromPort(*decl.match)); types.push_back(TreeSearchMatch{type, {/* ignored context */}}); }
diff --git a/verilog/tools/kythe/indexing_facts_tree_extractor.cc b/verilog/tools/kythe/indexing_facts_tree_extractor.cc index 28f23aa..1198279 100644 --- a/verilog/tools/kythe/indexing_facts_tree_extractor.cc +++ b/verilog/tools/kythe/indexing_facts_tree_extractor.cc
@@ -119,11 +119,6 @@ break; } - case NodeEnum::kPortDeclaration: - case NodeEnum::kPortReference: { - ExtractModulePort(node); - break; - } case NodeEnum::kIdentifierUnpackedDimensions: { ExtractInputOutputDeclaration(node); break; @@ -217,17 +212,39 @@ return; } - Visit(*port_list); + // This boolean is used to distinguish between ANSI and Non-ANSI module ports. + // e.g in this case: + // module m(a, b); + // has_propagated_type will be false as no type has been countered. + // + // in case like: + // module m(a, b, input x, y) + // for "a", "b" the boolean will be false but for "x", "y" the boolean will be + // true. + // + // The boolean is used to determine whether this the fact for this variable + // should be a reference or a defintiion. + bool has_propagated_type = false; + for (const auto& port : port_list->children()) { + if (port->Kind() == verible::SymbolKind::kLeaf) continue; + + const SyntaxTreeNode& port_node = verible::SymbolCastToNode(*port); + const auto tag = static_cast<verilog::NodeEnum>(port_node.Tag().tag); + + if (tag == NodeEnum::kPortDeclaration) { + has_propagated_type = true; + ExtractModulePort(port_node, has_propagated_type); + } else if (tag == NodeEnum::kPort) { + ExtractModulePort(GetPortReferenceFromPort(port_node), + has_propagated_type); + } + } } void IndexingFactsTreeExtractor::ExtractModulePort( - const SyntaxTreeNode& module_port_node) { + const SyntaxTreeNode& module_port_node, bool has_propagated_type) { const auto tag = static_cast<verilog::NodeEnum>(module_port_node.Tag().tag); - // TODO(minatoma): Fix case like: - // module m(input a, b); --> b is treated as a reference but should be a - // definition. - // For extracting cases like: // module m(input a, input b); if (tag == NodeEnum::kPortDeclaration) { @@ -237,15 +254,16 @@ facts_tree_context_.top().NewChild( IndexingNodeData({Anchor(leaf->get(), context_.base)}, IndexingFactType::kVariableDefinition)); - } else { + } else if (tag == NodeEnum::kPortReference) { // For extracting Non-ANSI style ports: // module m(a, b); const SyntaxTreeLeaf* leaf = GetIdentifierFromPortReference(module_port_node); - facts_tree_context_.top().NewChild( - IndexingNodeData({Anchor(leaf->get(), context_.base)}, - IndexingFactType::kVariableReference)); + facts_tree_context_.top().NewChild(IndexingNodeData( + {Anchor(leaf->get(), context_.base)}, + has_propagated_type ? IndexingFactType::kVariableDefinition + : IndexingFactType::kVariableReference)); } }
diff --git a/verilog/tools/kythe/indexing_facts_tree_extractor.h b/verilog/tools/kythe/indexing_facts_tree_extractor.h index 3184a1f..ffd3ad5 100644 --- a/verilog/tools/kythe/indexing_facts_tree_extractor.h +++ b/verilog/tools/kythe/indexing_facts_tree_extractor.h
@@ -58,7 +58,8 @@ void ExtractModuleHeader(const verible::SyntaxTreeNode& module_header_node); // Extracts modules ports and creates its corresponding fact tree. - void ExtractModulePort(const verible::SyntaxTreeNode& module_port_node); + void ExtractModulePort(const verible::SyntaxTreeNode& module_port_node, + bool has_propagated_type); // Extracts "a" from input a, output a and creates its corresponding fact // tree.
diff --git a/verilog/tools/kythe/indexing_facts_tree_extractor_test.cc b/verilog/tools/kythe/indexing_facts_tree_extractor_test.cc index 968d57f..be77c69 100644 --- a/verilog/tools/kythe/indexing_facts_tree_extractor_test.cc +++ b/verilog/tools/kythe/indexing_facts_tree_extractor_test.cc
@@ -448,6 +448,10 @@ {kTag, "a"}, ", ", {kTag, "b"}, + ", input wire ", + {kTag, "z"}, + ", ", + {kTag, "h"}, ");\nendmodule: ", {kTag, "foo"}}}; @@ -468,23 +472,114 @@ { { Anchor(kTestCase.expected_tokens[1], kTestCase.code), - Anchor(kTestCase.expected_tokens[7], kTestCase.code), + Anchor(kTestCase.expected_tokens[11], kTestCase.code), }, IndexingFactType::kModule, }, - // refers to input a. + // refers to a. T({ { Anchor(kTestCase.expected_tokens[3], kTestCase.code), }, IndexingFactType::kVariableReference, }), - // refers to output b. + // refers to b. T({ { Anchor(kTestCase.expected_tokens[5], kTestCase.code), }, IndexingFactType::kVariableReference, + }), + // refers to input z. + T({ + { + Anchor(kTestCase.expected_tokens[7], kTestCase.code), + }, + IndexingFactType::kVariableDefinition, + }), + // refers to h. + T({ + { + Anchor(kTestCase.expected_tokens[9], kTestCase.code), + }, + IndexingFactType::kVariableDefinition, + }))); + + const auto facts_tree = + ExtractOneFile(kTestCase.code, file_name, exit_status, parse_ok); + + const auto result_pair = DeepEqual(facts_tree, expected); + EXPECT_EQ(result_pair.left, nullptr) << *result_pair.left; + EXPECT_EQ(result_pair.right, nullptr) << *result_pair.right; +} + +TEST(FactsTreeExtractor, ModuleWithPortsDataTypeForwarding) { + constexpr int kTag = 1; // value doesn't matter + + // Normally, tools will reject non-ANSI port declarations that are missing + // their full definitions inside the body like "input a", but here we don't + // care and are just checking for references, even if they are dangling. + const verible::SyntaxTreeSearchTestCase kTestCase = {{"module ", + {kTag, "foo"}, + "(input wire ", + {kTag, "a"}, + ", ", + {kTag, "b"}, + ", output wire ", + {kTag, "z"}, + ", ", + {kTag, "h"}, + ");\nendmodule: ", + {kTag, "foo"}}}; + + constexpr absl::string_view file_name = "verilog.v"; + int exit_status = 0; + bool parse_ok = false; + + const IndexingFactNode expected( + { + { + Anchor(file_name, 0, kTestCase.code.size()), + Anchor(kTestCase.code, 0, kTestCase.code.size()), + }, + IndexingFactType ::kFile, + }, + // refers to module foo. + T( + { + { + Anchor(kTestCase.expected_tokens[1], kTestCase.code), + Anchor(kTestCase.expected_tokens[11], kTestCase.code), + }, + IndexingFactType::kModule, + }, + // refers to a. + T({ + { + Anchor(kTestCase.expected_tokens[3], kTestCase.code), + }, + IndexingFactType::kVariableDefinition, + }), + // refers to b. + T({ + { + Anchor(kTestCase.expected_tokens[5], kTestCase.code), + }, + IndexingFactType::kVariableDefinition, + }), + // refers to input z. + T({ + { + Anchor(kTestCase.expected_tokens[7], kTestCase.code), + }, + IndexingFactType::kVariableDefinition, + }), + // refers to h. + T({ + { + Anchor(kTestCase.expected_tokens[9], kTestCase.code), + }, + IndexingFactType::kVariableDefinition, }))); const auto facts_tree =