Skip to content

ClangImporter: Take ownership of clang::TargetOptions used for code generation #82912

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
merged 1 commit into from
Jul 9, 2025
Merged

ClangImporter: Take ownership of clang::TargetOptions used for code generation #82912

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
55 changes: 38 additions & 17 deletions lib/ClangImporter/ClangImporter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1427,22 +1427,12 @@ ClangImporter::create(ASTContext &ctx,
importer.get(), importerOpts, VFS, *swiftTargetClangArgs);
if (!swiftTargetClangInvocation)
return nullptr;
auto targetInfo = clang::TargetInfo::CreateTargetInfo(
clangDiags, swiftTargetClangInvocation->getTargetOpts());
// Ensure the target info has configured target-specific defines
std::string defineBuffer;
llvm::raw_string_ostream predefines(defineBuffer);
clang::MacroBuilder builder(predefines);
targetInfo->getTargetDefines(instance.getLangOpts(), builder);
importer->Impl.setSwiftTargetInfo(targetInfo);
importer->Impl.setSwiftCodeGenOptions(new clang::CodeGenOptions(
swiftTargetClangInvocation->getCodeGenOpts()));

importer->Impl.configureOptionsForCodeGen(clangDiags,
swiftTargetClangInvocation.get());
} else {
// Just use the existing Invocation's directly
importer->Impl.setSwiftTargetInfo(clang::TargetInfo::CreateTargetInfo(
clangDiags, importer->Impl.Invocation->getTargetOpts()));
importer->Impl.setSwiftCodeGenOptions(
new clang::CodeGenOptions(importer->Impl.Invocation->getCodeGenOpts()));
// Set using the existing invocation.
importer->Impl.configureOptionsForCodeGen(clangDiags);
}

// Create the associated action.
Expand Down Expand Up @@ -4132,7 +4122,7 @@ clang::TargetInfo &ClangImporter::getModuleAvailabilityTarget() const {
}

clang::TargetInfo &ClangImporter::getTargetInfo() const {
return *Impl.getSwiftTargetInfo();
return Impl.getCodeGenTargetInfo();
}

clang::ASTContext &ClangImporter::getClangASTContext() const {
Expand Down Expand Up @@ -4165,7 +4155,7 @@ clang::Sema &ClangImporter::getClangSema() const {
}

clang::CodeGenOptions &ClangImporter::getCodeGenOpts() const {
return *Impl.getSwiftCodeGenOptions();
return Impl.getCodeGenOptions();
}

std::string ClangImporter::getClangModuleHash() const {
Expand Down Expand Up @@ -4612,6 +4602,37 @@ void ClangImporter::Implementation::getMangledName(
}
}

void ClangImporter::Implementation::configureOptionsForCodeGen(
clang::DiagnosticsEngine &Diags, clang::CompilerInvocation *CI) {
clang::TargetInfo *targetInfo = nullptr;
if (CI) {
TargetOpts.reset(new clang::TargetOptions(std::move(CI->getTargetOpts())));
CodeGenOpts.reset(
new clang::CodeGenOptions(std::move(CI->getCodeGenOpts())));
targetInfo = clang::TargetInfo::CreateTargetInfo(Diags, *TargetOpts);

// Ensure the target info has configured target-specific defines
std::string defineBuffer;
llvm::raw_string_ostream predefines(defineBuffer);
clang::MacroBuilder builder(predefines);
targetInfo->getTargetDefines(Instance->getLangOpts(), builder);
} else {
targetInfo =
Copy link
Contributor

@Xazax-hun Xazax-hun Jul 9, 2025

Choose a reason for hiding this comment

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

Is my understanding right that TargetOpts remains nullptr when we use the existing invocation? I wonder if this can get a bit confusing that we don't have one unique way to refer to TargetOpts regardless of which path we took.

Copy link
Collaborator Author

@AnthonyLatsis AnthonyLatsis Jul 9, 2025

Choose a reason for hiding this comment

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

Yes, correct. That’s pretty much why I didn’t add a getter for TargetOpts. It’s just there to keep the options alive for as long as CodeGenTargetInfo when they differ from the importer invocation’s. You’re still supposed to access them through the invocation or getCodeGenTargetInfo, depending on which you want.

Copy link
Collaborator Author

@AnthonyLatsis AnthonyLatsis Jul 9, 2025

Choose a reason for hiding this comment

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

Added a doc comment to TargetOpts.

clang::TargetInfo::CreateTargetInfo(Diags, Invocation->getTargetOpts());
}

CodeGenTargetInfo.reset(targetInfo);
}

clang::CodeGenOptions &
ClangImporter::Implementation::getCodeGenOptions() const {
if (CodeGenOpts) {
return *CodeGenOpts.get();
}

return Invocation->getCodeGenOpts();
}

// ---------------------------------------------------------------------------
// Swift lookup tables
// ---------------------------------------------------------------------------
Expand Down
26 changes: 13 additions & 13 deletions lib/ClangImporter/ImporterImpl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -634,22 +634,22 @@ class LLVM_LIBRARY_VISIBILITY ClangImporter::Implementation
/// corresponding to the instantiating Swift compilation's triple. These are
/// to be used by all IRGen/CodeGen clients of `ClangImporter`.
std::unique_ptr<clang::TargetInfo> CodeGenTargetInfo;
/// - Important: Do not access directly. This field exists only to make sure
/// we own the target options stored in `CodeGenTargetInfo` because
/// `clang::TargetOptions` no longer co-owns them:
/// https://github.com/llvm/llvm-project/pull/106271.
std::unique_ptr<clang::TargetOptions> TargetOpts;
std::unique_ptr<clang::CodeGenOptions> CodeGenOpts;

public:
void setSwiftTargetInfo(clang::TargetInfo *SwiftTargetInfo) {
CodeGenTargetInfo.reset(SwiftTargetInfo);
}
clang::TargetInfo *getSwiftTargetInfo() const {
return CodeGenTargetInfo.get();
}
/// Sets the target & code generation options for use by IRGen/CodeGen
/// clients of `ClangImporter`. If `CI` is null, the data is drawn from the
/// importer's invocation.
void configureOptionsForCodeGen(clang::DiagnosticsEngine &Diags,
clang::CompilerInvocation *CI = nullptr);

void setSwiftCodeGenOptions(clang::CodeGenOptions *SwiftCodeGenOpts) {
CodeGenOpts.reset(SwiftCodeGenOpts);
}
clang::CodeGenOptions *getSwiftCodeGenOptions() const {
return CodeGenOpts.get();
}
clang::TargetInfo &getCodeGenTargetInfo() const { return *CodeGenTargetInfo; }

clang::CodeGenOptions &getCodeGenOptions() const;

private:
/// Generation number that is used for crude versioning.
Expand Down