https://github.com/aeubanks updated https://github.com/llvm/llvm-project/pull/66951
>From a956ea32c145d84ee771c985ceb3becbd03f4022 Mon Sep 17 00:00:00 2001 From: Arthur Eubanks <aeuba...@google.com> Date: Wed, 20 Sep 2023 13:42:20 -0700 Subject: [PATCH 1/4] [clang-format] Don't split "DPI"/"DPI-C" in imports The spec doesn't allow splitting these strings and we're seeing compile issues with splitting it. String splitting was enabled for Verilog in https://reviews.llvm.org/D154093. --- clang/lib/Format/ContinuationIndenter.cpp | 8 ++++++++ clang/unittests/Format/FormatTestVerilog.cpp | 6 ++++++ 2 files changed, 14 insertions(+) diff --git a/clang/lib/Format/ContinuationIndenter.cpp b/clang/lib/Format/ContinuationIndenter.cpp index deb3e554fdc124b..0bdf339d8df5827 100644 --- a/clang/lib/Format/ContinuationIndenter.cpp +++ b/clang/lib/Format/ContinuationIndenter.cpp @@ -2270,7 +2270,15 @@ ContinuationIndenter::createBreakableToken(const FormatToken &Current, if (State.Stack.back().IsInsideObjCArrayLiteral) return nullptr; + // The "DPI" (or "DPI-C") in SystemVerilog direct programming interface + // imports cannot be split, e.g. + // `import "DPI" function foo();` + // FIXME: We should see if this is an import statement instead of hardcoding + // "DPI"/"DPI-C". StringRef Text = Current.TokenText; + if (Style.isVerilog() && (Text == "\"DPI\"" || Text == "\"DPI-C\"")) + return nullptr; + // We need this to address the case where there is an unbreakable tail only // if certain other formatting decisions have been taken. The // UnbreakableTailLength of Current is an overapproximation in that case and diff --git a/clang/unittests/Format/FormatTestVerilog.cpp b/clang/unittests/Format/FormatTestVerilog.cpp index 945e06143ccc3f1..56a8d19a31e919c 100644 --- a/clang/unittests/Format/FormatTestVerilog.cpp +++ b/clang/unittests/Format/FormatTestVerilog.cpp @@ -1253,6 +1253,12 @@ TEST_F(FormatTestVerilog, StringLiteral) { "xxxx"});)", R"(x({"xxxxxxxxxxxxxxxx xxxxxxxxxxxxxxxx ", "xxxx"});)", getStyleWithColumns(getDefaultStyle(), 23)); + // "DPI"/"DPI-C" in imports cannot be split. + verifyFormat(R"(import + "DPI-C" function t foo + ();)", + R"(import "DPI-C" function t foo();)", + getStyleWithColumns(getDefaultStyle(), 23)); // These kinds of strings don't exist in Verilog. verifyNoCrash(R"(x(@"xxxxxxxxxxxxxxxx xxxx");)", getStyleWithColumns(getDefaultStyle(), 23)); >From 23d1b3c175b2b0d057014766cb558e9c6cfa67b4 Mon Sep 17 00:00:00 2001 From: Arthur Eubanks <aeuba...@google.com> Date: Wed, 20 Sep 2023 23:08:42 -0700 Subject: [PATCH 2/4] Check if this is an import statement instead --- clang/lib/Format/ContinuationIndenter.cpp | 15 +++++++++------ clang/unittests/Format/FormatTestVerilog.cpp | 2 +- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/clang/lib/Format/ContinuationIndenter.cpp b/clang/lib/Format/ContinuationIndenter.cpp index 0bdf339d8df5827..e279b42f1607345 100644 --- a/clang/lib/Format/ContinuationIndenter.cpp +++ b/clang/lib/Format/ContinuationIndenter.cpp @@ -2270,14 +2270,17 @@ ContinuationIndenter::createBreakableToken(const FormatToken &Current, if (State.Stack.back().IsInsideObjCArrayLiteral) return nullptr; - // The "DPI" (or "DPI-C") in SystemVerilog direct programming interface - // imports cannot be split, e.g. + // The "DPI"/"DPI-C" in SystemVerilog direct programming interface imports + // cannot be split, e.g. // `import "DPI" function foo();` - // FIXME: We should see if this is an import statement instead of hardcoding - // "DPI"/"DPI-C". StringRef Text = Current.TokenText; - if (Style.isVerilog() && (Text == "\"DPI\"" || Text == "\"DPI-C\"")) - return nullptr; + if (Style.isVerilog()) { + const FormatToken *Prev = Current.getPreviousNonComment(); + if (Prev && Prev == State.Line->getFirstNonComment() && + Prev->TokenText == "import") { + return nullptr; + } + } // We need this to address the case where there is an unbreakable tail only // if certain other formatting decisions have been taken. The diff --git a/clang/unittests/Format/FormatTestVerilog.cpp b/clang/unittests/Format/FormatTestVerilog.cpp index 56a8d19a31e919c..c313046af23144a 100644 --- a/clang/unittests/Format/FormatTestVerilog.cpp +++ b/clang/unittests/Format/FormatTestVerilog.cpp @@ -1253,7 +1253,7 @@ TEST_F(FormatTestVerilog, StringLiteral) { "xxxx"});)", R"(x({"xxxxxxxxxxxxxxxx xxxxxxxxxxxxxxxx ", "xxxx"});)", getStyleWithColumns(getDefaultStyle(), 23)); - // "DPI"/"DPI-C" in imports cannot be split. + // import "DPI"/"DPI-C" cannot be split. verifyFormat(R"(import "DPI-C" function t foo ();)", >From b9548587b2928ade3875a75abea891a6c8e666a1 Mon Sep 17 00:00:00 2001 From: Arthur Eubanks <aeuba...@google.com> Date: Thu, 21 Sep 2023 09:53:55 -0700 Subject: [PATCH 3/4] Check kf prev token is import/export keyword --- clang/lib/Format/ContinuationIndenter.cpp | 11 ++++------- clang/unittests/Format/FormatTestVerilog.cpp | 9 ++++++--- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/clang/lib/Format/ContinuationIndenter.cpp b/clang/lib/Format/ContinuationIndenter.cpp index e279b42f1607345..21a0efe23e72580 100644 --- a/clang/lib/Format/ContinuationIndenter.cpp +++ b/clang/lib/Format/ContinuationIndenter.cpp @@ -2273,14 +2273,11 @@ ContinuationIndenter::createBreakableToken(const FormatToken &Current, // The "DPI"/"DPI-C" in SystemVerilog direct programming interface imports // cannot be split, e.g. // `import "DPI" function foo();` - StringRef Text = Current.TokenText; - if (Style.isVerilog()) { - const FormatToken *Prev = Current.getPreviousNonComment(); - if (Prev && Prev == State.Line->getFirstNonComment() && - Prev->TokenText == "import") { - return nullptr; - } + if (Style.isVerilog() && Current.Previous && + Current.Previous->isOneOf(tok::kw_export, Keywords.kw_import)) { + return nullptr; } + StringRef Text = Current.TokenText; // We need this to address the case where there is an unbreakable tail only // if certain other formatting decisions have been taken. The diff --git a/clang/unittests/Format/FormatTestVerilog.cpp b/clang/unittests/Format/FormatTestVerilog.cpp index c313046af23144a..4c0a065daadd9ad 100644 --- a/clang/unittests/Format/FormatTestVerilog.cpp +++ b/clang/unittests/Format/FormatTestVerilog.cpp @@ -1253,11 +1253,14 @@ TEST_F(FormatTestVerilog, StringLiteral) { "xxxx"});)", R"(x({"xxxxxxxxxxxxxxxx xxxxxxxxxxxxxxxx ", "xxxx"});)", getStyleWithColumns(getDefaultStyle(), 23)); - // import "DPI"/"DPI-C" cannot be split. + // import/export "DPI"/"DPI-C" cannot be split. verifyFormat(R"(import - "DPI-C" function t foo + "DPI-C" function void foo ();)", - R"(import "DPI-C" function t foo();)", + R"(import "DPI-C" function void foo();)", + getStyleWithColumns(getDefaultStyle(), 23)); + verifyFormat(R"(export "DPI-C" function foo;)", + R"(export "DPI-C" function foo;)", getStyleWithColumns(getDefaultStyle(), 23)); // These kinds of strings don't exist in Verilog. verifyNoCrash(R"(x(@"xxxxxxxxxxxxxxxx xxxx");)", >From c78c547edc3fb4ace9d6e758a40aef16701283f9 Mon Sep 17 00:00:00 2001 From: Arthur Eubanks <aeuba...@google.com> Date: Thu, 21 Sep 2023 09:55:33 -0700 Subject: [PATCH 4/4] Update comment --- clang/lib/Format/ContinuationIndenter.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/clang/lib/Format/ContinuationIndenter.cpp b/clang/lib/Format/ContinuationIndenter.cpp index 21a0efe23e72580..68103c45b0b9463 100644 --- a/clang/lib/Format/ContinuationIndenter.cpp +++ b/clang/lib/Format/ContinuationIndenter.cpp @@ -2270,9 +2270,10 @@ ContinuationIndenter::createBreakableToken(const FormatToken &Current, if (State.Stack.back().IsInsideObjCArrayLiteral) return nullptr; - // The "DPI"/"DPI-C" in SystemVerilog direct programming interface imports - // cannot be split, e.g. + // The "DPI"/"DPI-C" in SystemVerilog direct programming interface + // imports/exports cannot be split, e.g. // `import "DPI" function foo();` + // FIXME: make this use same infra as C++ import checks if (Style.isVerilog() && Current.Previous && Current.Previous->isOneOf(tok::kw_export, Keywords.kw_import)) { return nullptr; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
