[clang] [clang-format] Don't split "DPI"/"DPI-C" in Verilog imports (PR #66951)

[Arthur Eubanks via cfe-commits]

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 <aeubanks at google.com>
Date: Wed, 20 Sep 2023 13:42:20 -0700
Subject: [PATCH 1/2] [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 <aeubanks at google.com>
Date: Wed, 20 Sep 2023 23:08:42 -0700
Subject: [PATCH 2/2] 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
     ();)",