Skip to content

Commit 0ff8b79

Browse files
committed
[clang-format] Stop crashing on slightly off Verilog module headers (#116000)
This piece of code made the program crash. ```Verilog function pkg::t get (int t = 2, int f = 2); ``` The way the code is supposed to be parsed is that UnwrappedLineParser should identify the function header, and then TokenAnnotator should recognize the result. But the code in UnwrappedLineParser would mistakenly not recognize it due to the `::`. Then TokenAnnotator would recognize the comma both as TT_VerilogInstancePortComma and TT_VerilogTypeComma. The code for annotating the instance port comma used `setFinalizedType`. The program would crash when it tried to set it to another type. The code in UnwrappedLineParser now recognizes the `::` token. The are other cases in which TokenAnnotator would recognize the comma as both of those types, for example if the `function` keyword is removed. The type is now set using `setType` instead so that the program does not crash. The developer no longer knows why he used `setFinalizedType` back then.
1 parent 27d25d1 commit 0ff8b79

File tree

4 files changed

+30
-3
lines changed

4 files changed

+30
-3
lines changed

clang/lib/Format/TokenAnnotator.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1554,7 +1554,7 @@ class AnnotatingParser {
15541554
};
15551555

15561556
if (IsInstancePort())
1557-
Tok->setFinalizedType(TT_VerilogInstancePortLParen);
1557+
Tok->setType(TT_VerilogInstancePortLParen);
15581558
}
15591559

15601560
if (!parseParens())
@@ -1730,7 +1730,7 @@ class AnnotatingParser {
17301730
Tok->setType(TT_InheritanceComma);
17311731
break;
17321732
case Context::VerilogInstancePortList:
1733-
Tok->setFinalizedType(TT_VerilogInstancePortComma);
1733+
Tok->setType(TT_VerilogInstancePortComma);
17341734
break;
17351735
default:
17361736
if (Style.isVerilog() && Contexts.size() == 1 &&

clang/lib/Format/UnwrappedLineParser.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4441,7 +4441,8 @@ unsigned UnwrappedLineParser::parseVerilogHierarchyHeader() {
44414441
Prev->setFinalizedType(TT_VerilogDimensionedTypeName);
44424442
parseSquare();
44434443
} else if (Keywords.isVerilogIdentifier(*FormatTok) ||
4444-
FormatTok->isOneOf(Keywords.kw_automatic, tok::kw_static)) {
4444+
FormatTok->isOneOf(tok::hash, tok::hashhash, tok::coloncolon,
4445+
Keywords.kw_automatic, tok::kw_static)) {
44454446
nextToken();
44464447
} else {
44474448
break;

clang/unittests/Format/FormatTestVerilog.cpp

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -702,6 +702,18 @@ TEST_F(FormatTestVerilog, Hierarchy) {
702702
" generate\n"
703703
" endgenerate\n"
704704
"endfunction : x");
705+
// Type names with '::' should be recognized.
706+
verifyFormat("function automatic x::x x\n"
707+
" (input x);\n"
708+
"endfunction : x");
709+
// Names having to do macros should be recognized.
710+
verifyFormat("function automatic x::x x``x\n"
711+
" (input x);\n"
712+
"endfunction : x");
713+
verifyFormat("function automatic x::x `x\n"
714+
" (input x);\n"
715+
"endfunction : x");
716+
verifyNoCrash("x x(x x, x x);");
705717
}
706718

707719
TEST_F(FormatTestVerilog, Identifiers) {

clang/unittests/Format/TokenAnnotatorTest.cpp

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2598,6 +2598,20 @@ TEST_F(TokenAnnotatorTest, UnderstandsVerilogOperators) {
25982598
Tokens = Annotate("x = '{\"\"};");
25992599
ASSERT_EQ(Tokens.size(), 8u) << Tokens;
26002600
EXPECT_TOKEN(Tokens[4], tok::string_literal, TT_Unknown);
2601+
2602+
// Module headers.
2603+
Tokens = Annotate("module x();\nendmodule");
2604+
ASSERT_EQ(Tokens.size(), 7u) << Tokens;
2605+
EXPECT_TOKEN(Tokens[2], tok::l_paren, TT_VerilogMultiLineListLParen);
2606+
Tokens = Annotate("function automatic `x x();\nendmodule");
2607+
ASSERT_EQ(Tokens.size(), 10u) << Tokens;
2608+
EXPECT_TOKEN(Tokens[5], tok::l_paren, TT_VerilogMultiLineListLParen);
2609+
Tokens = Annotate("function automatic x``x x();\nendmodule");
2610+
ASSERT_EQ(Tokens.size(), 11u) << Tokens;
2611+
EXPECT_TOKEN(Tokens[6], tok::l_paren, TT_VerilogMultiLineListLParen);
2612+
Tokens = Annotate("function automatic x::x x();\nendmodule");
2613+
ASSERT_EQ(Tokens.size(), 11u) << Tokens;
2614+
EXPECT_TOKEN(Tokens[6], tok::l_paren, TT_VerilogMultiLineListLParen);
26012615
}
26022616

26032617
TEST_F(TokenAnnotatorTest, UnderstandTableGenTokens) {

0 commit comments

Comments
 (0)