[clang] 02fd002 - [clang-format] Fix bugs in parsing C++20 module import statements

[Owen Pan via cfe-commits]

Author: Owen Pan
Date: 2023-01-23T14:35:15-08:00
New Revision: 02fd0020e5771b2850f892712b8fa51d3b98e270

URL: https://github.com/llvm/llvm-project/commit/02fd0020e5771b2850f892712b8fa51d3b98e270
DIFF: https://github.com/llvm/llvm-project/commit/02fd0020e5771b2850f892712b8fa51d3b98e270.diff

LOG: [clang-format] Fix bugs in parsing C++20 module import statements

Also fixes #60145.

Differential Revision: https://reviews.llvm.org/D142296

Added: 
    

Modified: 
    clang/lib/Format/UnwrappedLineParser.cpp
    clang/lib/Format/UnwrappedLineParser.h
    clang/unittests/Format/FormatTest.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/Format/UnwrappedLineParser.cpp b/clang/lib/Format/UnwrappedLineParser.cpp
index effb0ecb368bb..6d5b92c61c78b 100644
--- a/clang/lib/Format/UnwrappedLineParser.cpp
+++ b/clang/lib/Format/UnwrappedLineParser.cpp
@@ -41,7 +41,7 @@ class FormatTokenSource {
 
   // Returns the token that would be returned by the next call to
   // getNextToken().
-  virtual FormatToken *peekNextToken() = 0;
+  virtual FormatToken *peekNextToken(bool SkipComment = false) = 0;
 
   // Returns whether we are at the end of the file.
   // This can be 
diff erent from whether getNextToken() returned an eof token
@@ -169,10 +169,10 @@ class ScopedMacroState : public FormatTokenSource {
     return PreviousTokenSource->getPreviousToken();
   }
 
-  FormatToken *peekNextToken() override {
+  FormatToken *peekNextToken(bool SkipComment) override {
     if (eof())
       return &FakeEOF;
-    return PreviousTokenSource->peekNextToken();
+    return PreviousTokenSource->peekNextToken(SkipComment);
   }
 
   bool isEOF() override { return PreviousTokenSource->isEOF(); }
@@ -288,8 +288,11 @@ class IndexedTokenSource : public FormatTokenSource {
     return Position > 0 ? Tokens[Position - 1] : nullptr;
   }
 
-  FormatToken *peekNextToken() override {
+  FormatToken *peekNextToken(bool SkipComment) override {
     int Next = Position + 1;
+    if (SkipComment)
+      while (Tokens[Next]->is(tok::comment))
+        ++Next;
     LLVM_DEBUG({
       llvm::dbgs() << "Peeking ";
       dbgToken(Next);
@@ -1435,7 +1438,15 @@ static bool isC78ParameterDecl(const FormatToken *Tok, const FormatToken *Next,
   return Tok->Previous && Tok->Previous->isOneOf(tok::l_paren, tok::comma);
 }
 
-void UnwrappedLineParser::parseModuleImport() {
+bool UnwrappedLineParser::parseModuleImport() {
+  assert(FormatTok->is(Keywords.kw_import) && "'import' expected");
+
+  if (auto Token = Tokens->peekNextToken(/*SkipComment=*/true);
+      !Token->Tok.getIdentifierInfo() &&
+      !Token->isOneOf(tok::colon, tok::less, tok::string_literal)) {
+    return false;
+  }
+
   nextToken();
   while (!eof()) {
     if (FormatTok->is(tok::colon)) {
@@ -1462,6 +1473,7 @@ void UnwrappedLineParser::parseModuleImport() {
   }
 
   addUnwrappedLine();
+  return true;
 }
 
 // readTokenWithJavaScriptASI reads the next token and terminates the current
@@ -1682,14 +1694,12 @@ void UnwrappedLineParser::parseStructuralElement(
     }
     if (Style.isCpp()) {
       nextToken();
-      if (FormatTok->is(Keywords.kw_import)) {
-        parseModuleImport();
-        return;
-      }
       if (FormatTok->is(tok::kw_namespace)) {
         parseNamespace();
         return;
       }
+      if (FormatTok->is(Keywords.kw_import) && parseModuleImport())
+        return;
     }
     break;
   case tok::kw_inline:
@@ -1726,10 +1736,8 @@ void UnwrappedLineParser::parseStructuralElement(
         addUnwrappedLine();
         return;
       }
-      if (Style.isCpp()) {
-        parseModuleImport();
+      if (Style.isCpp() && parseModuleImport())
         return;
-      }
     }
     if (Style.isCpp() &&
         FormatTok->isOneOf(Keywords.kw_signals, Keywords.kw_qsignals,

diff  --git a/clang/lib/Format/UnwrappedLineParser.h b/clang/lib/Format/UnwrappedLineParser.h
index 97c5baf152fef..f043e567eb733 100644
--- a/clang/lib/Format/UnwrappedLineParser.h
+++ b/clang/lib/Format/UnwrappedLineParser.h
@@ -153,7 +153,7 @@ class UnwrappedLineParser {
   void parseCaseLabel();
   void parseSwitch();
   void parseNamespace();
-  void parseModuleImport();
+  bool parseModuleImport();
   void parseNew();
   void parseAccessSpecifier();
   bool parseEnum();

diff  --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index aa7ae12b6502c..b7d0a398dd432 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -12810,6 +12810,7 @@ TEST_F(FormatTest, HandlesIncludeDirectives) {
   // But 'import' might also be a regular C++ namespace.
   verifyFormat("import::SomeFunction(aaaaaaaaaaaaaaaaaaaaaaaaaaa,\n"
                "                     aaaaaaaaaaaaaaaaaaaaaaaaaaaaa);");
+  verifyFormat("import::Bar foo(val ? 2 : 1);");
 }
 
 //===----------------------------------------------------------------------===//
@@ -24628,6 +24629,7 @@ TEST_F(FormatTest, Cpp20ModulesSupport) {
   verifyFormat("import foo.bar;", Style);
   verifyFormat("import foo:bar;", Style);
   verifyFormat("import :bar;", Style);
+  verifyFormat("import /* module partition */ :bar;", Style);
   verifyFormat("import <ctime>;", Style);
   verifyFormat("import \"header\";", Style);
 
@@ -24654,6 +24656,8 @@ TEST_F(FormatTest, Cpp20ModulesSupport) {
   verifyFormat("import", Style);
   verifyFormat("module", Style);
   verifyFormat("export", Style);
+
+  verifyFormat("import /* not keyword */ = val ? 2 : 1;");
 }
 
 TEST_F(FormatTest, CoroutineForCoawait) {