[clang] [Clang][Lex] Only start a header-name at a bare '<' after 'import' (PR #210944)

[via cfe-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang-modules

Author: Akash Agrawal (akashagrwl)

<details>
<summary>Changes</summary>

Per [cpp.pre]p2, `import` at the start of a logical line introduces a module directive only if it is immediately followed by a header-name, `<`, an identifier, or `:`. HandleModuleContextualKeyword unconditionally set ParsingFilename = true before peeking the next token, forcing the lexer to scan any leading `<` as an angled header-name. A compound punctuator that merely starts with `<` (`<<`, `<=`, `<<=`, `<=>`) is a single preprocessing-token and none of the p2 alternatives, yet this scan misparsed it two ways:

  - No closing `>` later on the line (`import << 1;`): the angled scan falls back to a synthetic bare tok::less (see LexAngledStringLiteral) indistinguishable from a genuine `<`, so `import` was wrongly treated as a header-unit-import directive.
  - A `>` later on the line: the scan folds everything in between into a bogus header-name -- e.g. `import << <foo.h>`, or even the valid shift `import << (a > b)`, which became a spurious "file not found".

Re-peek under ParsingFilename = true only when the ordinary-mode token is a bare `<` or a string literal -- the only spellings that can begin a header-name at that position -- so a compound punctuator is never re-lexed as a header-name. Trust the re-lex only if it produced a `>`-terminated header_name (needed so quoted `import "foo.h"` still imports a header); otherwise keep the ordinary-mode token.

Updates clang/test/CXX/module/cpp.pre/p1.cpp: covers `import` followed by `<<`, `<=`, `<<=` (with and without a later `>`, plus the valid shift and `import << <foo.h>`), and corrects the `import <=>` case, which likewise must treat `import` as an ordinary identifier rather than lexing `=` as a header-name.

Fixes https://github.com/llvm/llvm-project/issues/210935

Co-authored - Claude-Sonnet

---
Full diff: https://github.com/llvm/llvm-project/pull/210944.diff


2 Files Affected:

- (modified) clang/lib/Lex/Preprocessor.cpp (+30-6) 
- (modified) clang/test/CXX/module/cpp.pre/p1.cpp (+40-1) 


``````````diff
diff --git a/clang/lib/Lex/Preprocessor.cpp b/clang/lib/Lex/Preprocessor.cpp
index 427a3826b299a..21fc7235978b8 100644
--- a/clang/lib/Lex/Preprocessor.cpp
+++ b/clang/lib/Lex/Preprocessor.cpp
@@ -1356,19 +1356,43 @@ bool Preprocessor::HandleModuleContextualKeyword(Token &Result) {
   llvm::SaveAndRestore<bool> SavedParsingPreprocessorDirective(
       CurPPLexer->ParsingPreprocessorDirective, true);
 
-  // The next token may be an angled string literal after import keyword.
-  llvm::SaveAndRestore<bool> SavedParsingFilemame(
-      CurPPLexer->ParsingFilename,
-      Result.getIdentifierInfo()->isImportKeyword());
-
+  bool IsImport = Result.getIdentifierInfo()->isImportKeyword();
+
+  // Peek under ordinary lexing rules first. Forcing ParsingFilename
+  // unconditionally (as before) makes the lexer scan any leading '<' as a
+  // header-name. That is wrong for two reasons: (1) if the scan never
+  // reaches a closing '>' on the line -- true for '<<', '<=', '<<=' with no
+  // later '>' -- it falls back to a synthetic bare '<' (see
+  // LexAngledStringLiteral) indistinguishable from a genuine one; (2) if a
+  // '>' does appear later on the line, the scan folds everything in between
+  // into a bogus header-name (e.g. 'import << <foo.h>', or the valid shift
+  // 'import << (a > b)'), even though the token actually following 'import'
+  // is '<<'. Neither '<<'/'<='/'<<='/'<=>' is one of [cpp.pre]/p2's
+  // alternatives ("a header-name, <, identifier, or :").
   std::optional<Token> NextTok = peekNextPPToken();
   if (!NextTok)
     return false;
 
+  // A header-name can only begin exactly at a '<' (or be spelled as a
+  // string literal). So only re-lex when the ordinary-mode token is a bare
+  // '<' or a quoted string -- not a compound punctuator that merely starts
+  // with '<' ('<<', '<=', '<<=', '<=>'), whose leading '<' does not
+  // introduce a header-name. Trust the re-lex only if a header-name
+  // actually terminated; otherwise keep the ordinary-mode token above.
+  if (IsImport && NextTok->isOneOf(tok::less, tok::string_literal)) {
+    llvm::SaveAndRestore<bool> SavedParsingFilename(CurPPLexer->ParsingFilename,
+                                                    true);
+    std::optional<Token> HeaderNameTok = peekNextPPToken();
+    if (!HeaderNameTok)
+      return false;
+    if (HeaderNameTok->is(tok::header_name))
+      NextTok = HeaderNameTok;
+  }
+
   if (NextTok->is(tok::raw_identifier))
     LookUpIdentifierInfo(*NextTok);
 
-  if (Result.getIdentifierInfo()->isImportKeyword()) {
+  if (IsImport) {
     if (NextTok->isOneOf(tok::identifier, tok::less, tok::colon,
                          tok::header_name)) {
       Result.setKind(tok::kw_import);
diff --git a/clang/test/CXX/module/cpp.pre/p1.cpp b/clang/test/CXX/module/cpp.pre/p1.cpp
index 989915004ff57..8ac9bbf96cfdd 100644
--- a/clang/test/CXX/module/cpp.pre/p1.cpp
+++ b/clang/test/CXX/module/cpp.pre/p1.cpp
@@ -20,6 +20,7 @@
 // RUN: %clang_cc1 -std=c++20 %t/foo.cppm -emit-module-interface -o %t/foo.pcm
 // RUN: %clang_cc1 -std=c++20 %t/import_decl_not_in_same_line.cpp -fmodule-file=foo=%t/foo.pcm -fsyntax-only -verify
 // RUN: %clang_cc1 -std=c++20 %t/not_import.cpp -fsyntax-only -verify
+// RUN: %clang_cc1 -std=c++20 %t/not_import_compound_punctuators.cpp -fsyntax-only -verify
 // RUN: %clang_cc1 -std=c++20 %t/import_spaceship.cpp -fsyntax-only -verify
 // RUN: %clang_cc1 -std=c++20 %t/leading_empty_macro.cpp -fsyntax-only -verify
 // RUN: %clang_cc1 -std=c++20 %t/operator_keyword_and.cpp -fsyntax-only -verify
@@ -94,9 +95,47 @@ export module M;
 import ::               // expected-error {{use of undeclared identifier 'import'}}
 import ->               // expected-error {{cannot use arrow operator on a type}}
 
+//--- not_import_compound_punctuators.cpp
+// 'import' followed by a compound punctuator that begins with '<' but is
+// not itself '<' (i.e. '<<', '<=', '<<=') is not one of the [cpp.pre]/p2
+// alternatives ("a header-name, <, identifier, or :") and must not be
+// treated as introducing a header-unit-import directive, even though an
+// angled-header-name lex of the rest of the line would otherwise
+// (incorrectly) fold back to a bare '<' (when no '>' follows) or swallow a
+// later '>' into a bogus header-name. 'import' must be at the start of a
+// logical line to be considered for directive status at all, hence the
+// line breaks below.
+int import = 0;
+int a() { return
+import << 1;
+}
+int b() { return
+import <= 1;
+}
+int c() { return
+import <<= 1;
+}
+// Same, but with a '>' later on the line: the '<'-leading punctuator still
+// does not start a header-name, so 'import' stays an ordinary identifier
+// and the '<' is a shift/comparison operator rather than folding
+// '<< ... >' into a header-name.
+int d(int x) { return
+import << (x > 0); // ok, shift of int 'import'
+}
+int e() { return
+import << <foo.h>; // expected-error {{expected expression}} \
+                   // expected-error {{use of undeclared identifier 'foo'}} \
+                   // expected-error {{expected expression}}
+}
+
 //--- import_spaceship.cpp
 export module M;
-import <=>; // expected-error {{'=' file not found}}
+// '<=>' is a single preprocessing-token (maximal munch), and it is not one
+// of the [cpp.pre]/p2 alternatives ("a header-name, <, identifier, or :"),
+// so 'import <=>' does not introduce a header-unit-import directive: its
+// leading '<' must not be lexed as the start of a header-name.
+import <=>; // expected-error {{unknown type name 'import'}} \
+            // expected-error {{expected unqualified-id}}
 
 //--- leading_empty_macro.cpp
 // expected-no-diagnostics

``````````

</details>


https://github.com/llvm/llvm-project/pull/210944