llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: None (yronglin) <details> <summary>Changes</summary> Depends: https://github.com/llvm/llvm-project/pull/107168 This patch handle `@<!-- -->import` as a preprocessing directive, and since this patch, the following import directive will be ill-formed: ``` @<!-- -->import Foo\n; ``` **Only the last commit belongs to this patch, the other commits come from dependent patches** --- Patch is 32.22 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/157726.diff 13 Files Affected: - (modified) clang/include/clang/Basic/DiagnosticLexKinds.td (+1-1) - (modified) clang/include/clang/Frontend/CompilerInstance.h (+4-7) - (modified) clang/include/clang/Lex/Preprocessor.h (+8-12) - (modified) clang/lib/Frontend/CompilerInstance.cpp (+14-13) - (modified) clang/lib/Lex/DependencyDirectivesScanner.cpp (+7-11) - (modified) clang/lib/Lex/Lexer.cpp (+33-3) - (modified) clang/lib/Lex/PPDirectives.cpp (+85-87) - (modified) clang/lib/Lex/PPLexerChange.cpp (+5-8) - (modified) clang/lib/Lex/Preprocessor.cpp (+33-92) - (modified) clang/test/Modules/lookup.cpp (+2-5) - (modified) clang/test/Modules/no-stale-modtime.m (+2-1) - (modified) clang/unittests/Lex/DependencyDirectivesScannerTest.cpp (+1-1) - (modified) clang/utils/ClangVisualizers/clang.natvis (-1) ``````````diff diff --git a/clang/include/clang/Basic/DiagnosticLexKinds.td b/clang/include/clang/Basic/DiagnosticLexKinds.td index 77feea9f869e9..d88aa891dc3e5 100644 --- a/clang/include/clang/Basic/DiagnosticLexKinds.td +++ b/clang/include/clang/Basic/DiagnosticLexKinds.td @@ -503,7 +503,7 @@ def warn_cxx98_compat_variadic_macro : Warning< InGroup<CXX98CompatPedantic>, DefaultIgnore; def ext_named_variadic_macro : Extension< "named variadic macros are a GNU extension">, InGroup<VariadicMacros>; -def err_embedded_directive : Error<"embedding a %select{#|C++ }0%1 directive " +def err_embedded_directive : Error<"embedding a %select{|#}0%1 directive " "within macro arguments is not supported">; def ext_embedded_directive : Extension< "embedding a directive within macro arguments has undefined behavior">, diff --git a/clang/include/clang/Frontend/CompilerInstance.h b/clang/include/clang/Frontend/CompilerInstance.h index 217efa3fe756e..fa811ae4fe8b2 100644 --- a/clang/include/clang/Frontend/CompilerInstance.h +++ b/clang/include/clang/Frontend/CompilerInstance.h @@ -167,13 +167,10 @@ class CompilerInstance : public ModuleLoader { /// Should we delete the BuiltModules when we're done? bool DeleteBuiltModules = true; - /// The location of the module-import keyword for the last module - /// import. - SourceLocation LastModuleImportLoc; - - /// The result of the last module import. - /// - ModuleLoadResult LastModuleImportResult; + /// Cache of module import results keyed by import location. + /// It is important to eliminate redundant diagnostics + /// when both the preprocessor and parser see the same import declaration. + llvm::SmallDenseMap<SourceLocation, ModuleLoadResult, 4> ModuleImportResults; /// Whether we should (re)build the global module index once we /// have finished with this translation unit. diff --git a/clang/include/clang/Lex/Preprocessor.h b/clang/include/clang/Lex/Preprocessor.h index b6e42a6151ac3..1e0b51eb7e7b4 100644 --- a/clang/include/clang/Lex/Preprocessor.h +++ b/clang/include/clang/Lex/Preprocessor.h @@ -381,12 +381,6 @@ class Preprocessor { llvm::DenseMap<FileID, SmallVector<const char *>> CheckPoints; unsigned CheckPointCounter = 0; - /// Whether the import is an `@import` or a standard c++ modules import. - bool IsAtImport = false; - - /// Whether the last token we lexed was an '@'. - bool LastTokenWasAt = false; - /// Whether we're importing a standard C++20 named Modules. bool ImportingCXXNamedModules = false; @@ -1833,8 +1827,13 @@ class Preprocessor { bool LexModuleNameContinue(Token &Tok, SourceLocation UseLoc, SmallVectorImpl<Token> &Suffix, SmallVectorImpl<IdentifierLoc> &Path, - bool AllowMacroExpansion = true, - bool IsPartition = false); + bool IsPartition = false, + bool AllowMacroExpansion = true); + bool HandleModuleName(StringRef DirType, SourceLocation UseLoc, Token &Tok, + SmallVectorImpl<IdentifierLoc> &Path, + SmallVectorImpl<Token> &DirToks, + bool IsPartition = false, + bool AllowMacroExpansion = true); void EnterModuleSuffixTokenStream(ArrayRef<Token> Toks); void HandleCXXImportDirective(Token Import); void HandleCXXModuleDirective(Token Module); @@ -1854,7 +1853,6 @@ class Preprocessor { return FirstPPTokenLoc; } - bool LexAfterModuleImport(Token &Result); void CollectPPImportSuffix(SmallVectorImpl<Token> &Toks, bool StopUntilEOD = false); bool CollectPPImportSuffixAndEnterStream(SmallVectorImpl<Token> &Toks, @@ -2911,6 +2909,7 @@ class Preprocessor { void HandleIncludeMacrosDirective(SourceLocation HashLoc, Token &Tok); void HandleImportDirective(SourceLocation HashLoc, Token &Tok); void HandleMicrosoftImportDirective(Token &Tok); + void HandleObjCImportDirective(Token &AtTok, Token &ImportTok); public: /// Check that the given module is available, producing a diagnostic if not. @@ -3174,9 +3173,6 @@ class Preprocessor { static bool CLK_DependencyDirectivesLexer(Preprocessor &P, Token &Result) { return P.CurLexer->LexDependencyDirectiveToken(Result); } - static bool CLK_LexAfterModuleImport(Preprocessor &P, Token &Result) { - return P.LexAfterModuleImport(Result); - } }; /// Abstract base class that describes a handler that will receive diff --git a/clang/lib/Frontend/CompilerInstance.cpp b/clang/lib/Frontend/CompilerInstance.cpp index 9f1a3c56feec1..f282863e7cb80 100644 --- a/clang/lib/Frontend/CompilerInstance.cpp +++ b/clang/lib/Frontend/CompilerInstance.cpp @@ -1003,6 +1003,8 @@ bool CompilerInstance::ExecuteAction(FrontendAction &Act) { if (hasSourceManager() && !Act.isModelParsingAction()) getSourceManager().clearIDTables(); + ModuleImportResults.clear(); + if (Act.BeginSourceFile(*this, FIF)) { if (llvm::Error Err = Act.Execute()) { consumeError(std::move(Err)); // FIXME this drops errors on the floor. @@ -1955,14 +1957,15 @@ CompilerInstance::loadModule(SourceLocation ImportLoc, SourceLocation ModuleNameLoc = Path[0].getLoc(); // If we've already handled this import, just return the cached result. - // This one-element cache is important to eliminate redundant diagnostics - // when both the preprocessor and parser see the same import declaration. - if (ImportLoc.isValid() && LastModuleImportLoc == ImportLoc) { - // Make the named module visible. - if (LastModuleImportResult && ModuleName != getLangOpts().CurrentModule) - TheASTReader->makeModuleVisible(LastModuleImportResult, Visibility, - ImportLoc); - return LastModuleImportResult; + // This cache eliminates redundant diagnostics when both the preprocessor + // and parser see the same import declaration. + if (ImportLoc.isValid()) { + auto CacheIt = ModuleImportResults.find(ImportLoc); + if (CacheIt != ModuleImportResults.end()) { + if (CacheIt->second && ModuleName != getLangOpts().CurrentModule) + TheASTReader->makeModuleVisible(CacheIt->second, Visibility, ImportLoc); + return CacheIt->second; + } } // If we don't already have information on this module, load the module now. @@ -2138,8 +2141,7 @@ CompilerInstance::loadModule(SourceLocation ImportLoc, *Module, getDiagnostics())) { getDiagnostics().Report(ImportLoc, diag::note_module_import_here) << SourceRange(Path.front().getLoc(), Path.back().getLoc()); - LastModuleImportLoc = ImportLoc; - LastModuleImportResult = ModuleLoadResult(); + ModuleImportResults[ImportLoc] = ModuleLoadResult(); return ModuleLoadResult(); } @@ -2152,9 +2154,8 @@ CompilerInstance::loadModule(SourceLocation ImportLoc, .getModuleMap() .resolveLinkAsDependencies(Module->getTopLevelModule()); - LastModuleImportLoc = ImportLoc; - LastModuleImportResult = ModuleLoadResult(Module); - return LastModuleImportResult; + ModuleImportResults[ImportLoc] = ModuleLoadResult(Module); + return ModuleLoadResult(Module); } void CompilerInstance::createModuleFromSource(SourceLocation ImportLoc, diff --git a/clang/lib/Lex/DependencyDirectivesScanner.cpp b/clang/lib/Lex/DependencyDirectivesScanner.cpp index 8320b3ddbca31..f31c76f9f675b 100644 --- a/clang/lib/Lex/DependencyDirectivesScanner.cpp +++ b/clang/lib/Lex/DependencyDirectivesScanner.cpp @@ -581,15 +581,12 @@ bool Scanner::lexModuleDirectiveBody(DirectiveKind Kind, const char *&First, return false; } + const auto &Tok = lexToken(First, End); pushDirective(Kind); - skipWhitespace(First, End); - if (First == End) + if (Tok.is(tok::eof) || Tok.is(tok::eod)) return false; - if (!isVerticalWhitespace(*First)) - return reportError( - DirectiveLoc, diag::err_dep_source_scanner_unexpected_tokens_at_import); - skipNewline(First, End); - return false; + return reportError(DirectiveLoc, + diag::err_dep_source_scanner_unexpected_tokens_at_import); } dependency_directives_scan::Token &Scanner::lexToken(const char *&First, @@ -951,10 +948,6 @@ bool Scanner::lexPPLine(const char *&First, const char *const End) { CurDirToks.clear(); }); - // FIXME: Shoule we handle @import as a preprocessing directive? - if (*First == '@') - return lexAt(First, End); - bool IsPreprocessedModule = isStartWithPreprocessedModuleDirective(First, End); if (*First == '_' && !IsPreprocessedModule) { @@ -969,6 +962,9 @@ bool Scanner::lexPPLine(const char *&First, const char *const End) { llvm::scope_exit ScEx2( [&]() { TheLexer.setParsingPreprocessorDirective(false); }); + if (*First == '@') + return lexAt(First, End); + // Handle module directives for C++20 modules. if (*First == 'i' || *First == 'e' || *First == 'm' || IsPreprocessedModule) return lexModule(First, End); diff --git a/clang/lib/Lex/Lexer.cpp b/clang/lib/Lex/Lexer.cpp index 92c3046a6fd19..3e4548d76b140 100644 --- a/clang/lib/Lex/Lexer.cpp +++ b/clang/lib/Lex/Lexer.cpp @@ -34,6 +34,7 @@ #include "llvm/Support/ConvertUTF.h" #include "llvm/Support/MemoryBufferRef.h" #include "llvm/Support/NativeFormatting.h" +#include "llvm/Support/SaveAndRestore.h" #include "llvm/Support/Unicode.h" #include "llvm/Support/UnicodeCharRanges.h" #include <algorithm> @@ -4436,9 +4437,28 @@ bool Lexer::LexTokenInternal(Token &Result) { case '@': // Objective C support. - if (CurPtr[-1] == '@' && LangOpts.ObjC) - Kind = tok::at; - else + if (CurPtr[-1] == '@' && LangOpts.ObjC) { + FormTokenWithChars(Result, CurPtr, tok::at); + if (PP && Result.isAtPhysicalStartOfLine() && !LexingRawMode && + !Is_PragmaLexer) { + Token NextPPTok; + NextPPTok.startToken(); + { + llvm::SaveAndRestore<bool> SavedParsingPreprocessorDirective( + this->ParsingPreprocessorDirective, true); + auto NextTokOr = peekNextPPToken(); + if (NextTokOr.has_value()) { + NextPPTok = *NextTokOr; + } + } + if (NextPPTok.is(tok::raw_identifier) && + NextPPTok.getRawIdentifier() == "import") { + PP->HandleDirective(Result); + return false; + } + } + return true; + } else Kind = tok::unknown; break; @@ -4600,6 +4620,16 @@ bool Lexer::LexDependencyDirectiveToken(Token &Result) { return true; return false; } + if (Result.is(tok::at) && Result.isAtStartOfLine()) { + auto NextTok = peekNextPPToken(); + if (NextTok && NextTok->is(tok::raw_identifier) && + NextTok->getRawIdentifier() == "import") { + PP->HandleDirective(Result); + if (PP->hadModuleLoaderFatalFailure()) + return true; + return false; + } + } if (Result.is(tok::raw_identifier)) { Result.setRawIdentifierData(TokPtr); if (!isLexingRawMode()) { diff --git a/clang/lib/Lex/PPDirectives.cpp b/clang/lib/Lex/PPDirectives.cpp index 4a854c213926b..3f2e66e66a458 100644 --- a/clang/lib/Lex/PPDirectives.cpp +++ b/clang/lib/Lex/PPDirectives.cpp @@ -1313,9 +1313,9 @@ void Preprocessor::HandleSkippedDirectiveWhileUsingPCH(Token &Result, void Preprocessor::HandleDirective(Token &Result) { // FIXME: Traditional: # with whitespace before it not recognized by K&R? - // We just parsed a # character at the start of a line, so we're in directive - // mode. Tell the lexer this so any newlines we see will be converted into an - // EOD token (which terminates the directive). + // We just parsed a # or @ character at the start of a line, so we're in + // directive mode. Tell the lexer this so any newlines we see will be + // converted into an EOD token (which terminates the directive). CurPPLexer->ParsingPreprocessorDirective = true; if (CurLexer) CurLexer->SetKeepWhitespaceMode(false); @@ -1330,13 +1330,13 @@ void Preprocessor::HandleDirective(Token &Result) { // pp-directive. bool ReadAnyTokensBeforeDirective =CurPPLexer->MIOpt.getHasReadAnyTokensVal(); - // Save the directive-introducing token('#' and import/module in C++20) in - // case we need to return it later. + // Save the directive-introducing token ('#', '@', or import/module in C++20) + // in case we need to return it later. Token Introducer = Result; // Read the next token, the directive flavor. This isn't expanded due to // C99 6.10.3p8. - if (Introducer.is(tok::hash)) + if (Introducer.isOneOf(tok::hash, tok::at)) LexUnexpandedToken(Result); // C99 6.10.3p11: Is this preprocessor directive in macro invocation? e.g.: @@ -1360,10 +1360,7 @@ void Preprocessor::HandleDirective(Token &Result) { case tok::pp___preprocessed_module: case tok::pp___preprocessed_import: Diag(Result, diag::err_embedded_directive) - << (getLangOpts().CPlusPlusModules && - Introducer.isModuleContextualKeyword( - /*AllowExport=*/false)) - << II->getName(); + << Introducer.is(tok::hash) << II->getName(); Diag(*ArgMacro, diag::note_macro_expansion_here) << ArgMacro->getIdentifierInfo(); DiscardUntilEndOfDirective(); @@ -1464,11 +1461,16 @@ void Preprocessor::HandleDirective(Token &Result) { return HandleCXXImportDirective(Result); // GNU Extensions. case tok::pp_import: - if (getLangOpts().CPlusPlusModules && - Introducer.isModuleContextualKeyword( - /*AllowExport=*/false)) + switch (Introducer.getKind()) { + case tok::hash: + return HandleImportDirective(Introducer.getLocation(), Result); + case tok::at: + return HandleObjCImportDirective(Introducer, Result); + case tok::kw_import: return HandleCXXImportDirective(Result); - return HandleImportDirective(Introducer.getLocation(), Result); + default: + llvm_unreachable("not a valid import directive"); + } case tok::pp_include_next: return HandleIncludeNextDirective(Introducer.getLocation(), Result); @@ -4192,9 +4194,6 @@ void Preprocessor::HandleCXXImportDirective(Token ImportTok) { llvm::SaveAndRestore<bool> SaveImportingCXXModules( this->ImportingCXXNamedModules, true); - if (LastExportKeyword.is(tok::kw_export)) - LastExportKeyword.startToken(); - Token Tok; if (LexHeaderName(Tok)) { if (Tok.isNot(tok::eod)) @@ -4207,7 +4206,7 @@ void Preprocessor::HandleCXXImportDirective(Token ImportTok) { SmallVector<IdentifierLoc, 2> Path; bool ImportingHeader = false; bool IsPartition = false; - std::string FlatName; + switch (Tok.getKind()) { case tok::header_name: ImportingHeader = true; @@ -4221,27 +4220,11 @@ void Preprocessor::HandleCXXImportDirective(Token ImportTok) { Lex(Tok); [[fallthrough]]; case tok::identifier: { - bool LeadingSpace = Tok.hasLeadingSpace(); - unsigned NumToksInDirective = DirToks.size(); - if (LexModuleNameContinue(Tok, UseLoc, DirToks, Path)) { - if (Tok.isNot(tok::eod)) - CheckEndOfDirective(ImportTok.getIdentifierInfo()->getName(), - /*EnableMacros=*/false, &DirToks); - EnterModuleSuffixTokenStream(DirToks); + if (HandleModuleName(ImportTok.getIdentifierInfo()->getName(), UseLoc, Tok, + Path, DirToks, IsPartition)) return; - } - - // Clean the module-name tokens and replace these tokens with - // annot_module_name. - DirToks.resize(NumToksInDirective); - ModuleNameLoc *NameLoc = ModuleNameLoc::Create(*this, Path); - DirToks.emplace_back(); - DirToks.back().setKind(tok::annot_module_name); - DirToks.back().setAnnotationRange(NameLoc->getRange()); - DirToks.back().setAnnotationValue(static_cast<void *>(NameLoc)); - DirToks.back().setFlagValue(Token::LeadingSpace, LeadingSpace); - DirToks.push_back(Tok); + std::string FlatName; bool IsValid = (IsPartition && ModuleDeclState.isNamedModule()) || !IsPartition; if (Callbacks && IsValid) { @@ -4350,13 +4333,7 @@ void Preprocessor::HandleCXXImportDirective(Token ImportTok) { /// The lexed module name are replaced by annot_module_name token. void Preprocessor::HandleCXXModuleDirective(Token ModuleTok) { assert(getLangOpts().CPlusPlusModules && ModuleTok.is(tok::kw_module)); - Token Introducer = ModuleTok; - if (LastExportKeyword.is(tok::kw_export)) { - Introducer = LastExportKeyword; - LastExportKeyword.startToken(); - } - - SourceLocation StartLoc = Introducer.getLocation(); + SourceLocation StartLoc = ModuleTok.getLocation(); Token Tok; SourceLocation UseLoc = ModuleTok.getLocation(); @@ -4382,57 +4359,20 @@ void Preprocessor::HandleCXXModuleDirective(Token ModuleTok) { DirToks.push_back(Tok); break; case tok::identifier: { - bool LeadingSpace = Tok.hasLeadingSpace(); - unsigned NumToksInDirective = DirToks.size(); - - // C++ [cpp.module]p3: Any preprocessing tokens after the module - // preprocessing token in the module directive are processed just as in - // normal text. - // - // P3034R1 Module Declarations Shouldn’t be Macros. - if (LexModuleNameContinue(Tok, UseLoc, DirToks, Path, - /*AllowMacroExpansion=*/false)) { - if (Tok.isNot(tok::eod)) - CheckEndOfDirective(ModuleTok.getIdentifierInfo()->getName(), - /*EnableMacros=*/false, &DirToks); - EnterModuleSuffixTokenStream(DirToks); + if (HandleModuleName(ModuleTok.getIdentifierInfo()->getName(), UseLoc, Tok, + Path, DirToks, /*IsPartition=*/false, + /*AllowMacroExpansion=*/false)) return; - } - - ModuleNameLoc *NameLoc = ModuleNameLoc::Create(*this, Path); - DirToks.resize(NumToksInDirective); - DirToks.emplace_back(); - DirToks.back().setKind(tok::annot_module_name); - DirToks.back().setAnnotationRange(NameLoc->getRange()); - DirToks.back().setAnnotationValue(static_cast<void *>(NameLoc)); - DirToks.back().setFlagValue(Token::LeadingSpace, LeadingSpace); - DirToks.push_back(Tok); // C++20 [cpp.module]p // The pp-tokens, if any, of a pp-module shall be of the form: // pp-module-name pp-module-partition[opt] pp-tokens[opt] if (Tok.is(tok::colon)) { - NumToksInDirective = DirToks.size(); LexUnexpandedToken(Tok); - LeadingSpace = Tok.hasLeadingSpace(); - if (LexModuleNameContinue(Tok, UseLoc, DirToks, Partition, - /*AllowMacroExpansion=*/false, - /*IsPartition=*/true)) { - if (Tok.isNot(tok::eod)) - CheckEndOfDirective(ModuleTok.getIdentifierInfo()->getName(), - /*EnableMacros=*/false, &DirToks); - EnterModuleSuffixTokenStream(DirToks); + if (HandleModuleName(ModuleTok.getIdentifierInfo()->getName(), UseLoc, + Tok, Partition, DirToks, /*IsPartition=*/true, + /*AllowMacroExpansion=*/false)) return; - } - - ModuleNameLoc *PartitionLoc = ModuleNameLoc::Create(*this, Partition); - DirToks.resize(NumToksInDirective); - DirToks.emplace_back(); - DirToks.back().setKind(tok::annot_module_name); - DirToks.back().setAnnotationRange(NameLoc->getRange()); - DirToks.back().setAnnotationValue(static_cast<void *>(PartitionLoc)); - DirToks.back().setFlagValue(Token::LeadingSpace, LeadingSpace); - DirToks.push_back(Tok); } // If the current token is a macro definition, put it back to token stream @@ -4497,3 +4437,61 @@ void Preprocessor::HandleCXXModuleDirective(Token ModuleTok) { } EnterModuleSuffixTokenStream(DirToks); } + +/// Lex a token following the 'import' contextual keyword. +/// +/// pp-import: +/// [ObjC] @ import module-name ; +/// +/// module-name: +/// module-name-qualifier[opt] identifier +/// +/// module-name-qualifier +/// module-name-qualifier[opt] identifier . +/// +/// We respond to a pp-import by importing macros from the nam... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/157726 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
