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 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
