[clang] Revert "[ObjC][Preprocessor] Handle @import directive as a pp-directive" (PR #188806)
via cfe-commits
cfe-commits at lists.llvm.org
Thu Mar 26 10:53:31 PDT 2026
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-modules
Author: Dave Lee (kastiglione)
<details>
<summary>Changes</summary>
Reverts llvm/llvm-project#<!-- -->157726
This is causing a number of lldb test failures, specifically tests that do `@<!-- -->import ...` expression evaluation. See https://green.lab.llvm.org/job/llvm.org/view/LLDB/job/as-lldb-cmake/
---
Patch is 26.09 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/188806.diff
14 Files Affected:
- (modified) clang/include/clang/Basic/DiagnosticLexKinds.td (+1-1)
- (modified) clang/include/clang/Frontend/CompilerInstance.h (+7-4)
- (modified) clang/include/clang/Lex/Preprocessor.h (+10-1)
- (modified) clang/lib/Frontend/CompilerInstance.cpp (+13-14)
- (modified) clang/lib/Lex/DependencyDirectivesScanner.cpp (+11-7)
- (modified) clang/lib/Lex/Lexer.cpp (+3-33)
- (modified) clang/lib/Lex/PPDirectives.cpp (+24-76)
- (modified) clang/lib/Lex/PPLexerChange.cpp (+8-5)
- (modified) clang/lib/Lex/Preprocessor.cpp (+92-1)
- (modified) clang/test/Modules/lookup.cpp (+5-2)
- (modified) clang/test/Modules/no-stale-modtime.m (+1-2)
- (removed) clang/test/Modules/objc-at-import.m (-42)
- (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 2ef13f568667d..5eceeced311f2 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{|#}0%1 directive "
+def err_embedded_directive : Error<"embedding a %select{#|C++ }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 f206d012eacc9..0d684d5c7f9fe 100644
--- a/clang/include/clang/Frontend/CompilerInstance.h
+++ b/clang/include/clang/Frontend/CompilerInstance.h
@@ -168,10 +168,13 @@ class CompilerInstance : public ModuleLoader {
/// Should we delete the BuiltModules when we're done?
bool DeleteBuiltModules = true;
- /// 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;
+ /// The location of the module-import keyword for the last module
+ /// import.
+ SourceLocation LastModuleImportLoc;
+
+ /// The result of the last module import.
+ ///
+ ModuleLoadResult LastModuleImportResult;
/// 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 8830294ea1658..c7e152a75f51f 100644
--- a/clang/include/clang/Lex/Preprocessor.h
+++ b/clang/include/clang/Lex/Preprocessor.h
@@ -381,6 +381,12 @@ 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;
@@ -1851,6 +1857,7 @@ class Preprocessor {
return FirstPPTokenLoc;
}
+ bool LexAfterModuleImport(Token &Result);
void CollectPPImportSuffix(SmallVectorImpl<Token> &Toks,
bool StopUntilEOD = false);
bool CollectPPImportSuffixAndEnterStream(SmallVectorImpl<Token> &Toks,
@@ -2907,7 +2914,6 @@ 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.
@@ -3171,6 +3177,9 @@ 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 1f1b6701c38df..33919bc1c4634 100644
--- a/clang/lib/Frontend/CompilerInstance.cpp
+++ b/clang/lib/Frontend/CompilerInstance.cpp
@@ -985,8 +985,6 @@ 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.
@@ -1982,15 +1980,14 @@ CompilerInstance::loadModule(SourceLocation ImportLoc,
SourceLocation ModuleNameLoc = Path[0].getLoc();
// If we've already handled this import, just return the cached result.
- // 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;
- }
+ // 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;
}
// If we don't already have information on this module, load the module now.
@@ -2166,7 +2163,8 @@ CompilerInstance::loadModule(SourceLocation ImportLoc,
*Module, getDiagnostics())) {
getDiagnostics().Report(ImportLoc, diag::note_module_import_here)
<< SourceRange(Path.front().getLoc(), Path.back().getLoc());
- ModuleImportResults[ImportLoc] = ModuleLoadResult();
+ LastModuleImportLoc = ImportLoc;
+ LastModuleImportResult = ModuleLoadResult();
return ModuleLoadResult();
}
@@ -2179,8 +2177,9 @@ CompilerInstance::loadModule(SourceLocation ImportLoc,
.getModuleMap()
.resolveLinkAsDependencies(Module->getTopLevelModule());
- ModuleImportResults[ImportLoc] = ModuleLoadResult(Module);
- return ModuleLoadResult(Module);
+ LastModuleImportLoc = ImportLoc;
+ LastModuleImportResult = ModuleLoadResult(Module);
+ return LastModuleImportResult;
}
void CompilerInstance::createModuleFromSource(SourceLocation ImportLoc,
diff --git a/clang/lib/Lex/DependencyDirectivesScanner.cpp b/clang/lib/Lex/DependencyDirectivesScanner.cpp
index ede5d49860fa4..f2e0d52c2cbba 100644
--- a/clang/lib/Lex/DependencyDirectivesScanner.cpp
+++ b/clang/lib/Lex/DependencyDirectivesScanner.cpp
@@ -582,12 +582,15 @@ bool Scanner::lexModuleDirectiveBody(DirectiveKind Kind, const char *&First,
return false;
}
- const auto &Tok = lexToken(First, End);
pushDirective(Kind);
- if (Tok.is(tok::eof) || Tok.is(tok::eod))
+ skipWhitespace(First, End);
+ if (First == End)
return false;
- return reportError(DirectiveLoc,
- diag::err_dep_source_scanner_unexpected_tokens_at_import);
+ if (!isVerticalWhitespace(*First))
+ return reportError(
+ DirectiveLoc, diag::err_dep_source_scanner_unexpected_tokens_at_import);
+ skipNewline(First, End);
+ return false;
}
dependency_directives_scan::Token &Scanner::lexToken(const char *&First,
@@ -949,6 +952,10 @@ 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) {
@@ -963,9 +970,6 @@ 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 7a3d79e744ef2..10246552bb13d 100644
--- a/clang/lib/Lex/Lexer.cpp
+++ b/clang/lib/Lex/Lexer.cpp
@@ -34,7 +34,6 @@
#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>
@@ -4449,28 +4448,9 @@ bool Lexer::LexTokenInternal(Token &Result) {
case '@':
// Objective C support.
- 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
+ if (CurPtr[-1] == '@' && LangOpts.ObjC)
+ Kind = tok::at;
+ else
Kind = tok::unknown;
break;
@@ -4632,16 +4612,6 @@ 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 055c946b3322b..c89402fa137c0 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 # 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).
+ // 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).
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 ('#', '@', or import/module in C++20)
- // in case we need to return it later.
+ // Save the directive-introducing token('#' and 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.isOneOf(tok::hash, tok::at))
+ if (Introducer.is(tok::hash))
LexUnexpandedToken(Result);
// C99 6.10.3p11: Is this preprocessor directive in macro invocation? e.g.:
@@ -1360,7 +1360,10 @@ void Preprocessor::HandleDirective(Token &Result) {
case tok::pp___preprocessed_module:
case tok::pp___preprocessed_import:
Diag(Result, diag::err_embedded_directive)
- << Introducer.is(tok::hash) << II->getName();
+ << (getLangOpts().CPlusPlusModules &&
+ Introducer.isModuleContextualKeyword(
+ /*AllowExport=*/false))
+ << II->getName();
Diag(*ArgMacro, diag::note_macro_expansion_here)
<< ArgMacro->getIdentifierInfo();
DiscardUntilEndOfDirective();
@@ -1461,16 +1464,11 @@ void Preprocessor::HandleDirective(Token &Result) {
return HandleCXXImportDirective(Result);
// GNU Extensions.
case tok::pp_import:
- switch (Introducer.getKind()) {
- case tok::hash:
- return HandleImportDirective(Introducer.getLocation(), Result);
- case tok::at:
- return HandleObjCImportDirective(Introducer, Result);
- case tok::kw_import:
+ if (getLangOpts().CPlusPlusModules &&
+ Introducer.isModuleContextualKeyword(
+ /*AllowExport=*/false))
return HandleCXXImportDirective(Result);
- default:
- llvm_unreachable("not a valid import directive");
- }
+ return HandleImportDirective(Introducer.getLocation(), Result);
case tok::pp_include_next:
return HandleIncludeNextDirective(Introducer.getLocation(), Result);
@@ -4194,6 +4192,9 @@ 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))
@@ -4334,7 +4335,13 @@ 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));
- SourceLocation StartLoc = ModuleTok.getLocation();
+ Token Introducer = ModuleTok;
+ if (LastExportKeyword.is(tok::kw_export)) {
+ Introducer = LastExportKeyword;
+ LastExportKeyword.startToken();
+ }
+
+ SourceLocation StartLoc = Introducer.getLocation();
Token Tok;
SourceLocation UseLoc = ModuleTok.getLocation();
@@ -4438,62 +4445,3 @@ 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 named module.
-void Preprocessor::HandleObjCImportDirective(Token &AtTok, Token &ImportTok) {
- assert(getLangOpts().ObjC && AtTok.is(tok::at) &&
- ImportTok.isObjCAtKeyword(tok::objc_import));
- ImportTok.setKind(tok::kw_import);
- SmallVector<Token, 32> DirToks{AtTok, ImportTok};
- SmallVector<IdentifierLoc, 3> Path;
- SourceLocation UseLoc = ImportTok.getLocation();
- ModuleImportLoc = ImportTok.getLocation();
- Token Tok;
- Lex(Tok);
- if (HandleModuleName(ImportTok.getIdentifierInfo()->getName(), UseLoc, Tok,
- Path, DirToks,
- /*AllowMacroExpansion=*/true,
- /*IsPartition=*/false))
- return;
-
- // Consume the pp-import-suffix and expand any macros in it now, if we're not
- // at the semicolon already.
- if (!DirToks.back().isOneOf(tok::semi, tok::eod))
- CollectPPImportSuffix(DirToks);
-
- if (DirToks.back().isNot(tok::eod))
- CheckEndOfDirective(ImportTok.getIdentifierInfo()->getName());
- else
- DirToks.pop_back();
-
- // This is not a pp-import after all.
- if (DirToks.back().isNot(tok::semi)) {
- EnterModuleSuffixTokenStream(DirToks);
- return;
- }
-
- Module *Imported = nullptr;
- SourceLocation SemiLoc = DirToks.back().getLocation();
- if (getLangOpts().Modules) {
- Imported = TheModuleLoader.loadModule(ModuleImportLoc, Path, Module::Hidden,
- /*IsInclusionDirective=*/false);
- if (Imported)
- makeModuleVisible(Imported, SemiLoc);
- }
-
- if (Callbacks)
- Callbacks->moduleImport(ModuleImportLoc, Path, Imported);
-
- EnterModuleSuffixTokenStream(DirToks);
-}
diff --git a/clang/lib/Lex/PPLexerChange.cpp b/clang/lib/Lex/PPLexerChange.cpp
index 35fc23092dcc1..05affedd48a86 100644
--- a/clang/lib/Lex/PPLexerChange.cpp
+++ b/clang/lib/Lex/PPLexerChange.cpp
@@ -115,9 +115,10 @@ void Preprocessor::EnterSourceFileWithLexer(Lexer *TheLexer,
CurPPLexer = TheLexer;
CurDirLookup = CurDir;
CurLexerSubmodule = nullptr;
- CurLexerCallback = TheLexer->isDependencyDirectivesLexer()
- ? CLK_DependencyDirectivesLexer
- : CLK_Lexer;
+ if (CurLexerCallback != CLK_LexAfterModuleImport)
+ CurLexerCallback = TheLexer->isDependencyDirectivesLexer()
+ ? CLK_DependencyDirectivesLexer
+ : CLK_Lexer;
// Notify the client, if desired, that we are in a new source file.
if (Callbacks && !CurLexer->Is_PragmaLexer) {
@@ -153,7 +154,8 @@ void Preprocessor::EnterMacro(Token &Tok, SourceLocation ILEnd,
PushIncludeMacroStack();
CurDirLookup = nullptr;
CurTokenLexer = std::move(TokLexer);
- CurLexerCallback = CLK_TokenLexer;
+ if (CurLexerCallback != CLK_LexAfterModuleImport)
+ CurLexerCallback = CLK_TokenLexer;
}
/// EnterTokenStream - Add a "macro" context to the top of the include stack,
@@ -207,7 +209,8 @@ void Preprocessor::EnterTokenStream(const Token *Toks, unsigned NumToks,
PushIncludeMacroStack();
CurDirLookup = nullptr;
CurTokenLexer = std::move(TokLexer);
- CurLexerCallback = CLK_TokenLexer;
+ if (CurLexerCallback != CLK_LexAfterModuleImport)
+ CurLexerCallback = CLK_TokenLexer;
}
/// Compute the relative path that names the given file relative to
diff --git a/clang/lib/Lex/Preprocessor.cpp b/clang/lib/Lex/Preprocessor.cpp
index 4f5423f0e9b79..2cfefac1052a4 100644
--- a/clang/lib/Lex/Preprocessor.cpp
+++ b/clang/lib/Lex/Preprocessor.cpp
@@ -889,6 +889,23 @@ bool Preprocessor::HandleIdentifier(Token &Identifier) {
return hadModuleLoaderFatalFailure();
}
+ // If this is the 'import' contextual keyword following an '@', note
+ // that the next token indicates a module name.
+ //
+ // Note that we do not treat 'import' as a contextual
+ // keyword when we're in a caching lexer, because caching lexers only get
+ // used in contexts where import declarations are disallowed.
+ //
+ // Likewise if this is the standard C++ import keyword.
+ if (((LastTokenWasAt && II.isImportKeyword()) ||
+ Identifier.is(tok::kw_import)) &&
+ !InMacroArgs &&
+ (!DisableMacroExpansion || MacroExpansionInDirectivesOverride) &&
+ CurLexerCallback != CLK_CachingLexer) {
+ ModuleImportLoc = Identifier.getLocation();
+ IsAtImport = true;
+ CurLexerCallback = CLK_LexAfterModuleImport;
+ }
return true;
}
@@ -988,6 +1005,7 @@ void Preprocessor::Lex(Token &Result) {
CheckPointCounter = 0;
}
+ LastTokenWasAt = Result.is(tok::at);
if (Result.isNot(tok::kw_export))
LastExportKeyword.startToken();
@@ -1326,7 +1344,8 @@ bool Preprocessor::HandleModuleContextualKeyword(Token &Result) {
CurPPLexer->ParsingFilename,
Result.getIdentifierInfo()->isImportKeyword());
- std::optional<Token> NextTok = peekNextPPToken();
+ std::optional<Token> NextTok =
+ CurLexer ? CurLexer->peekNextPPToken() : CurTokenLexer->peekNextPPToken();
if (!NextTok)
return false;
@@ -1338,6 +1357,7 @@ bool Preprocessor::HandleModuleContextualKeyword(Token &Result) {
tok::header_name)) {
Result.setKind(tok::kw_import);
ModuleImportLoc = Result.getLocation();
+ IsAtImport = false;
return true;
}
}
@@ -1394,6 +1414,77 @@ void Preprocessor::EnterModuleSuffixTokenStream(ArrayRef<Token> Toks) {
CurTokenLexer->setLexingCXXModuleDirective();
}
+/// Lex a token following the 'import' contextual keyword.
+///
+/// pp-import: [C++20]
+/// import header-name pp-impor...
[truncated]
``````````
</details>
https://github.com/llvm/llvm-project/pull/188806
More information about the cfe-commits
mailing list