Skip to content

[clang] Do not share ownership of PreprocessorOptions #133467

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jansvoboda11 merged 1 commit into from
Apr 4, 2025
Merged

[clang] Do not share ownership of PreprocessorOptions #133467

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 7 additions & 8 deletions clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,15 +89,14 @@ ExpandModularHeadersPPCallbacks::ExpandModularHeadersPPCallbacks(
HeaderInfo = std::make_unique<HeaderSearch>(HSOpts, Sources, Diags, LangOpts,
&Compiler.getTarget());

auto PO = std::make_shared<PreprocessorOptions>();
*PO = Compiler.getPreprocessorOpts();

PP = std::make_unique<clang::Preprocessor>(PO, Diags, LangOpts, Sources,
*HeaderInfo, ModuleLoader,
/*IILookup=*/nullptr,
/*OwnsHeaderSearch=*/false);
PP = std::make_unique<clang::Preprocessor>(Compiler.getPreprocessorOpts(),
Diags, LangOpts, Sources,
*HeaderInfo, ModuleLoader,
/*IILookup=*/nullptr,
/*OwnsHeaderSearch=*/false);
PP->Initialize(Compiler.getTarget(), Compiler.getAuxTarget());
InitializePreprocessor(*PP, *PO, Compiler.getPCHContainerReader(),
InitializePreprocessor(*PP, Compiler.getPreprocessorOpts(),
Compiler.getPCHContainerReader(),
Compiler.getFrontendOpts(), Compiler.getCodeGenOpts());
ApplyHeaderSearchOptions(*HeaderInfo, HSOpts, LangOpts,
Compiler.getTarget().getTriple());
Expand Down
5 changes: 3 additions & 2 deletions clang-tools-extra/clangd/ModulesBuilder.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,9 +202,10 @@ bool IsModuleFileUpToDate(PathRef ModuleFilePath,
HeaderSearch HeaderInfo(HSOpts, SourceMgr, *Diags, LangOpts,
/*Target=*/nullptr);

PreprocessorOptions PPOpts;
TrivialModuleLoader ModuleLoader;
Preprocessor PP(std::make_shared<PreprocessorOptions>(), *Diags, LangOpts,
SourceMgr, HeaderInfo, ModuleLoader);
Preprocessor PP(PPOpts, *Diags, LangOpts, SourceMgr, HeaderInfo,
ModuleLoader);

IntrusiveRefCntPtr<ModuleCache> ModCache = createCrossProcessModuleCache();
PCHContainerOperations PCHOperations;
Expand Down
3 changes: 0 additions & 3 deletions clang/include/clang/Frontend/CompilerInvocation.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -272,9 +272,6 @@ class CompilerInvocation : public CompilerInvocationBase {
std::shared_ptr<HeaderSearchOptions> getHeaderSearchOptsPtr() {
return HSOpts;
}
std::shared_ptr<PreprocessorOptions> getPreprocessorOptsPtr() {
return PPOpts;
}
Comment on lines -275 to -277
Copy link
Contributor Author

@jansvoboda11 jansvoboda11 Mar 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was the primary goal of this PR.

std::shared_ptr<LangOptions> getLangOptsPtr() { return LangOpts; }
/// @}

Expand Down
14 changes: 6 additions & 8 deletions clang/include/clang/Lex/Preprocessor.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ class Preprocessor {
friend class VariadicMacroScopeGuard;

llvm::unique_function<void(const clang::Token &)> OnToken;
std::shared_ptr<const PreprocessorOptions> PPOpts;
const PreprocessorOptions &PPOpts;
DiagnosticsEngine *Diags;
const LangOptions &LangOpts;
const TargetInfo *Target = nullptr;
Expand Down Expand Up @@ -1165,10 +1165,9 @@ class Preprocessor {
void updateOutOfDateIdentifier(const IdentifierInfo &II) const;

public:
Preprocessor(std::shared_ptr<const PreprocessorOptions> PPOpts,
DiagnosticsEngine &diags, const LangOptions &LangOpts,
SourceManager &SM, HeaderSearch &Headers,
ModuleLoader &TheModuleLoader,
Preprocessor(const PreprocessorOptions &PPOpts, DiagnosticsEngine &diags,
const LangOptions &LangOpts, SourceManager &SM,
HeaderSearch &Headers, ModuleLoader &TheModuleLoader,
IdentifierInfoLookup *IILookup = nullptr,
bool OwnsHeaderSearch = false,
TranslationUnitKind TUKind = TU_Complete);
Expand All @@ -1195,9 +1194,8 @@ class Preprocessor {
/// Cleanup after model file parsing
void FinalizeForModelFile();

/// Retrieve the preprocessor options used to initialize this
/// preprocessor.
const PreprocessorOptions &getPreprocessorOpts() const { return *PPOpts; }
/// Retrieve the preprocessor options used to initialize this preprocessor.
const PreprocessorOptions &getPreprocessorOpts() const { return PPOpts; }

DiagnosticsEngine &getDiagnostics() const { return *Diags; }
void setDiagnostics(DiagnosticsEngine &D) { Diags = &D; }
Expand Down
2 changes: 1 addition & 1 deletion clang/lib/Frontend/ASTUnit.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -844,7 +844,7 @@ std::unique_ptr<ASTUnit> ASTUnit::LoadFromASTFile(
HeaderSearch &HeaderInfo = *AST->HeaderInfo;

AST->PP = std::make_shared<Preprocessor>(
AST->PPOpts, AST->getDiagnostics(), *AST->LangOpts,
*AST->PPOpts, AST->getDiagnostics(), *AST->LangOpts,
AST->getSourceManager(), HeaderInfo, AST->ModuleLoader,
/*IILookup=*/nullptr,
/*OwnsHeaderSearch=*/false);
Expand Down
2 changes: 1 addition & 1 deletion clang/lib/Frontend/CompilerInstance.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -452,7 +452,7 @@ void CompilerInstance::createPreprocessor(TranslationUnitKind TUKind) {
HeaderSearch *HeaderInfo =
new HeaderSearch(getHeaderSearchOpts(), getSourceManager(),
getDiagnostics(), getLangOpts(), &getTarget());
PP = std::make_shared<Preprocessor>(Invocation->getPreprocessorOptsPtr(),
PP = std::make_shared<Preprocessor>(Invocation->getPreprocessorOpts(),
getDiagnostics(), getLangOpts(),
getSourceManager(), *HeaderInfo, *this,
/*IdentifierInfoLookup=*/nullptr,
Expand Down
20 changes: 10 additions & 10 deletions clang/lib/Lex/PPDirectives.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1154,7 +1154,7 @@ Preprocessor::LookupEmbedFile(StringRef Filename, bool isAngled, bool OpenFile,
}
}

for (const auto &Entry : PPOpts->EmbedEntries) {
for (const auto &Entry : PPOpts.EmbedEntries) {
LookupPath.clear();
SeparateComponents(LookupPath, Entry, Filename, false);
llvm::Expected<FileEntryRef> ShouldBeEntry = FM.getFileRef(
Expand Down Expand Up @@ -2341,7 +2341,7 @@ Preprocessor::ImportAction Preprocessor::HandleHeaderIncludeOrImport(

enum { Enter, Import, Skip, IncludeLimitReached } Action = Enter;

if (PPOpts->SingleFileParseMode)
if (PPOpts.SingleFileParseMode)
Action = IncludeLimitReached;

// If we've reached the max allowed include depth, it is usually due to an
Expand Down Expand Up @@ -3420,11 +3420,11 @@ void Preprocessor::HandleIfdefDirective(Token &Result,
Callbacks->Ifdef(DirectiveTok.getLocation(), MacroNameTok, MD);
}

bool RetainExcludedCB = PPOpts->RetainExcludedConditionalBlocks &&
bool RetainExcludedCB = PPOpts.RetainExcludedConditionalBlocks &&
getSourceManager().isInMainFile(DirectiveTok.getLocation());

// Should we include the stuff contained by this directive?
if (PPOpts->SingleFileParseMode && !MI) {
if (PPOpts.SingleFileParseMode && !MI) {
// In 'single-file-parse mode' undefined identifiers trigger parsing of all
// the directive blocks.
CurPPLexer->pushConditionalLevel(DirectiveTok.getLocation(),
Expand Down Expand Up @@ -3475,11 +3475,11 @@ void Preprocessor::HandleIfDirective(Token &IfToken,
IfToken.getLocation(), DER.ExprRange,
(ConditionalTrue ? PPCallbacks::CVK_True : PPCallbacks::CVK_False));

bool RetainExcludedCB = PPOpts->RetainExcludedConditionalBlocks &&
bool RetainExcludedCB = PPOpts.RetainExcludedConditionalBlocks &&
getSourceManager().isInMainFile(IfToken.getLocation());

// Should we include the stuff contained by this directive?
if (PPOpts->SingleFileParseMode && DER.IncludedUndefinedIds) {
if (PPOpts.SingleFileParseMode && DER.IncludedUndefinedIds) {
// In 'single-file-parse mode' undefined identifiers trigger parsing of all
// the directive blocks.
CurPPLexer->pushConditionalLevel(IfToken.getLocation(), /*wasskip*/false,
Expand Down Expand Up @@ -3546,10 +3546,10 @@ void Preprocessor::HandleElseDirective(Token &Result, const Token &HashToken) {
if (Callbacks)
Callbacks->Else(Result.getLocation(), CI.IfLoc);

bool RetainExcludedCB = PPOpts->RetainExcludedConditionalBlocks &&
bool RetainExcludedCB = PPOpts.RetainExcludedConditionalBlocks &&
getSourceManager().isInMainFile(Result.getLocation());

if ((PPOpts->SingleFileParseMode && !CI.FoundNonSkip) || RetainExcludedCB) {
if ((PPOpts.SingleFileParseMode && !CI.FoundNonSkip) || RetainExcludedCB) {
// In 'single-file-parse mode' undefined identifiers trigger parsing of all
// the directive blocks.
CurPPLexer->pushConditionalLevel(CI.IfLoc, /*wasskip*/false,
Expand Down Expand Up @@ -3626,10 +3626,10 @@ void Preprocessor::HandleElifFamilyDirective(Token &ElifToken,
}
}

bool RetainExcludedCB = PPOpts->RetainExcludedConditionalBlocks &&
bool RetainExcludedCB = PPOpts.RetainExcludedConditionalBlocks &&
getSourceManager().isInMainFile(ElifToken.getLocation());

if ((PPOpts->SingleFileParseMode && !CI.FoundNonSkip) || RetainExcludedCB) {
if ((PPOpts.SingleFileParseMode && !CI.FoundNonSkip) || RetainExcludedCB) {
// In 'single-file-parse mode' undefined identifiers trigger parsing of all
// the directive blocks.
CurPPLexer->pushConditionalLevel(ElifToken.getLocation(), /*wasskip*/false,
Expand Down
2 changes: 1 addition & 1 deletion clang/lib/Lex/PPLexerChange.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -561,7 +561,7 @@ bool Preprocessor::HandleEndOfFile(Token &Result, bool isEndOfMacro) {
if (creatingPCHWithThroughHeader() && !LeavingPCHThroughHeader) {
// Reached the end of the compilation without finding the through header.
Diag(CurLexer->getFileLoc(), diag::err_pp_through_header_not_seen)
<< PPOpts->PCHThroughHeader << 0;
<< PPOpts.PCHThroughHeader << 0;
}

if (!isIncrementalProcessingEnabled())
Expand Down
28 changes: 14 additions & 14 deletions clang/lib/Lex/Preprocessor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,13 +77,13 @@ LLVM_INSTANTIATE_REGISTRY(PragmaHandlerRegistry)

ExternalPreprocessorSource::~ExternalPreprocessorSource() = default;

Preprocessor::Preprocessor(std::shared_ptr<const PreprocessorOptions> PPOpts,
Preprocessor::Preprocessor(const PreprocessorOptions &PPOpts,
DiagnosticsEngine &diags, const LangOptions &opts,
SourceManager &SM, HeaderSearch &Headers,
ModuleLoader &TheModuleLoader,
IdentifierInfoLookup *IILookup, bool OwnsHeaders,
TranslationUnitKind TUKind)
: PPOpts(std::move(PPOpts)), Diags(&diags), LangOpts(opts),
: PPOpts(PPOpts), Diags(&diags), LangOpts(opts),
FileMgr(Headers.getFileMgr()), SourceMgr(SM),
ScratchBuf(new ScratchBuffer(SourceMgr)), HeaderInfo(Headers),
TheModuleLoader(TheModuleLoader), ExternalSource(nullptr),
Expand Down Expand Up @@ -156,11 +156,11 @@ Preprocessor::Preprocessor(std::shared_ptr<const PreprocessorOptions> PPOpts,
SkippingUntilPragmaHdrStop = true;

// If using a PCH with a through header, start skipping tokens.
if (!this->PPOpts->PCHThroughHeader.empty() &&
!this->PPOpts->ImplicitPCHInclude.empty())
if (!this->PPOpts.PCHThroughHeader.empty() &&
!this->PPOpts.ImplicitPCHInclude.empty())
SkippingUntilPCHThroughHeader = true;

if (this->PPOpts->GeneratePreamble)
if (this->PPOpts.GeneratePreamble)
PreambleConditionalStack.startRecording();

MaxTokens = LangOpts.MaxTokens;
Expand Down Expand Up @@ -577,18 +577,18 @@ void Preprocessor::EnterMainSourceFile() {
// Start parsing the predefines.
EnterSourceFile(FID, nullptr, SourceLocation());

if (!PPOpts->PCHThroughHeader.empty()) {
if (!PPOpts.PCHThroughHeader.empty()) {
// Lookup and save the FileID for the through header. If it isn't found
// in the search path, it's a fatal error.
OptionalFileEntryRef File = LookupFile(
SourceLocation(), PPOpts->PCHThroughHeader,
SourceLocation(), PPOpts.PCHThroughHeader,
/*isAngled=*/false, /*FromDir=*/nullptr, /*FromFile=*/nullptr,
/*CurDir=*/nullptr, /*SearchPath=*/nullptr, /*RelativePath=*/nullptr,
/*SuggestedModule=*/nullptr, /*IsMapped=*/nullptr,
/*IsFrameworkFound=*/nullptr);
if (!File) {
Diag(SourceLocation(), diag::err_pp_through_header_not_found)
<< PPOpts->PCHThroughHeader;
<< PPOpts.PCHThroughHeader;
return;
}
setPCHThroughHeaderFileID(
Expand All @@ -614,21 +614,21 @@ bool Preprocessor::isPCHThroughHeader(const FileEntry *FE) {
}

bool Preprocessor::creatingPCHWithThroughHeader() {
return TUKind == TU_Prefix && !PPOpts->PCHThroughHeader.empty() &&
return TUKind == TU_Prefix && !PPOpts.PCHThroughHeader.empty() &&
PCHThroughHeaderFileID.isValid();
}

bool Preprocessor::usingPCHWithThroughHeader() {
return TUKind != TU_Prefix && !PPOpts->PCHThroughHeader.empty() &&
return TUKind != TU_Prefix && !PPOpts.PCHThroughHeader.empty() &&
PCHThroughHeaderFileID.isValid();
}

bool Preprocessor::creatingPCHWithPragmaHdrStop() {
return TUKind == TU_Prefix && PPOpts->PCHWithHdrStop;
return TUKind == TU_Prefix && PPOpts.PCHWithHdrStop;
}

bool Preprocessor::usingPCHWithPragmaHdrStop() {
return TUKind != TU_Prefix && PPOpts->PCHWithHdrStop;
return TUKind != TU_Prefix && PPOpts.PCHWithHdrStop;
}

/// Skip tokens until after the #include of the through header or
Expand Down Expand Up @@ -657,8 +657,8 @@ void Preprocessor::SkipTokensWhileUsingPCH() {
if (ReachedMainFileEOF) {
if (UsingPCHThroughHeader)
Diag(SourceLocation(), diag::err_pp_through_header_not_seen)
<< PPOpts->PCHThroughHeader << 1;
else if (!PPOpts->PCHWithHdrStopCreate)
<< PPOpts.PCHThroughHeader << 1;
else if (!PPOpts.PCHWithHdrStopCreate)
Diag(SourceLocation(), diag::err_pp_pragma_hdrstop_not_seen);
}
}
Expand Down
7 changes: 3 additions & 4 deletions clang/unittests/Analysis/MacroExpansionContextTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,11 +60,10 @@ class MacroExpansionContextTest : public ::testing::Test {
SourceMgr.setMainFileID(SourceMgr.createFileID(std::move(Buf)));
HeaderSearchOptions HSOpts;
TrivialModuleLoader ModLoader;
PreprocessorOptions PPOpts;
HeaderSearch HeaderInfo(HSOpts, SourceMgr, Diags, LangOpts, Target.get());
Preprocessor PP(std::make_shared<PreprocessorOptions>(), Diags, LangOpts,
SourceMgr, HeaderInfo, ModLoader,
/*IILookup =*/nullptr,
/*OwnsHeaderSearch =*/false);
Preprocessor PP(PPOpts, Diags, LangOpts, SourceMgr, HeaderInfo, ModLoader,
/*IILookup=*/nullptr, /*OwnsHeaderSearch=*/false);

PP.Initialize(*Target);
auto Ctx = std::make_unique<MacroExpansionContext>(LangOpts);
Expand Down
35 changes: 15 additions & 20 deletions clang/unittests/Basic/SourceManagerTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,12 +136,11 @@ TEST_F(SourceManagerTest, isBeforeInTranslationUnit) {
SourceMgr.setMainFileID(mainFileID);

HeaderSearchOptions HSOpts;
PreprocessorOptions PPOpts;
TrivialModuleLoader ModLoader;
HeaderSearch HeaderInfo(HSOpts, SourceMgr, Diags, LangOpts, &*Target);
Preprocessor PP(std::make_shared<PreprocessorOptions>(), Diags, LangOpts,
SourceMgr, HeaderInfo, ModLoader,
/*IILookup =*/nullptr,
/*OwnsHeaderSearch =*/false);
Preprocessor PP(PPOpts, Diags, LangOpts, SourceMgr, HeaderInfo, ModLoader,
/*IILookup =*/nullptr, /*OwnsHeaderSearch =*/false);
PP.Initialize(*Target);
PP.EnterMainSourceFile();

Expand Down Expand Up @@ -186,12 +185,11 @@ TEST_F(SourceManagerTest, isBeforeInTranslationUnitWithTokenSplit) {
SourceMgr.createFileID(llvm::MemoryBuffer::getMemBuffer(main)));

HeaderSearchOptions HSOpts;
PreprocessorOptions PPOpts;
TrivialModuleLoader ModLoader;
HeaderSearch HeaderInfo(HSOpts, SourceMgr, Diags, LangOpts, &*Target);
Preprocessor PP(std::make_shared<PreprocessorOptions>(), Diags, LangOpts,
SourceMgr, HeaderInfo, ModLoader,
/*IILookup =*/nullptr,
/*OwnsHeaderSearch =*/false);
Preprocessor PP(PPOpts, Diags, LangOpts, SourceMgr, HeaderInfo, ModLoader,
/*IILookup=*/nullptr, /*OwnsHeaderSearch=*/false);
PP.Initialize(*Target);
PP.EnterMainSourceFile();
llvm::SmallString<8> Scratch;
Expand Down Expand Up @@ -462,11 +460,10 @@ TEST_F(SourceManagerTest, ResetsIncludeLocMap) {
auto ParseFile = [&] {
TrivialModuleLoader ModLoader;
HeaderSearchOptions HSOpts;
PreprocessorOptions PPOpts;
HeaderSearch HeaderInfo(HSOpts, SourceMgr, Diags, LangOpts, &*Target);
Preprocessor PP(std::make_shared<PreprocessorOptions>(), Diags, LangOpts,
SourceMgr, HeaderInfo, ModLoader,
/*IILookup =*/nullptr,
/*OwnsHeaderSearch =*/false);
Preprocessor PP(PPOpts, Diags, LangOpts, SourceMgr, HeaderInfo, ModLoader,
/*IILookup=*/nullptr, /*OwnsHeaderSearch=*/false);
PP.Initialize(*Target);
PP.EnterMainSourceFile();
PP.LexTokensUntilEOF();
Expand Down Expand Up @@ -538,13 +535,12 @@ TEST_F(SourceManagerTest, getMacroArgExpandedLocation) {
SourceMgr.overrideFileContents(headerFile, std::move(HeaderBuf));

HeaderSearchOptions HSOpts;
PreprocessorOptions PPOpts;
TrivialModuleLoader ModLoader;
HeaderSearch HeaderInfo(HSOpts, SourceMgr, Diags, LangOpts, &*Target);

Preprocessor PP(std::make_shared<PreprocessorOptions>(), Diags, LangOpts,
SourceMgr, HeaderInfo, ModLoader,
/*IILookup =*/nullptr,
/*OwnsHeaderSearch =*/false);
Preprocessor PP(PPOpts, Diags, LangOpts, SourceMgr, HeaderInfo, ModLoader,
/*IILookup=*/nullptr, /*OwnsHeaderSearch=*/false);
// Ensure we can get expanded locations in presence of implicit includes.
// These are different than normal includes since predefines buffer doesn't
// have a valid insertion location.
Expand Down Expand Up @@ -657,12 +653,11 @@ TEST_F(SourceManagerTest, isBeforeInTranslationUnitWithMacroInInclude) {
SourceMgr.overrideFileContents(headerFile, std::move(HeaderBuf));

HeaderSearchOptions HSOpts;
PreprocessorOptions PPOpts;
TrivialModuleLoader ModLoader;
HeaderSearch HeaderInfo(HSOpts, SourceMgr, Diags, LangOpts, &*Target);
Preprocessor PP(std::make_shared<PreprocessorOptions>(), Diags, LangOpts,
SourceMgr, HeaderInfo, ModLoader,
/*IILookup =*/nullptr,
/*OwnsHeaderSearch =*/false);
Preprocessor PP(PPOpts, Diags, LangOpts, SourceMgr, HeaderInfo, ModLoader,
/*IILookup=*/nullptr, /*OwnsHeaderSearch=*/false);
PP.Initialize(*Target);

std::vector<MacroAction> Macros;
Expand Down
4 changes: 2 additions & 2 deletions clang/unittests/Lex/LexerTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,9 @@ class LexerTest : public ::testing::Test {

HeaderSearchOptions HSOpts;
HeaderSearch HeaderInfo(HSOpts, SourceMgr, Diags, LangOpts, Target.get());
PreprocessorOptions PPOpts;
std::unique_ptr<Preprocessor> PP = std::make_unique<Preprocessor>(
std::make_shared<PreprocessorOptions>(), Diags, LangOpts, SourceMgr,
HeaderInfo, ModLoader,
PPOpts, Diags, LangOpts, SourceMgr, HeaderInfo, ModLoader,
/*IILookup =*/nullptr,
/*OwnsHeaderSearch =*/false);
PP->Initialize(*Target);
Expand Down
Loading
Loading