Skip to content

TypeSystem: extend the triple sanitization for Swift #1577

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
Aug 3, 2020
Merged

TypeSystem: extend the triple sanitization for Swift #1577

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
23 changes: 16 additions & 7 deletions lldb/source/Plugins/TypeSystem/Swift/SwiftASTContext.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2347,14 +2347,23 @@ llvm::Triple SwiftASTContext::GetTriple() const {
return llvm::Triple(m_compiler_invocation_ap->getTargetTriple());
}

/// Conditions a triple string to be safe for use with Swift. Right
/// now this just strips the Haswell marker off the CPU name.
/// Conditions a triple string to be safe for use with Swift.
///
/// This strips the Haswell marker off the CPU name (for Apple targets).
///
/// It also add the GNU environment for Linux. Although this is technically
/// incorrect, as the `*-unknown-linux` environment represents the bare-metal
/// environment, because Swift is currently hosted only, we can get away with
/// it.
Copy link

Choose a reason for hiding this comment

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

I always thought GNU was a vendor, not an environment? Is it both? Can you add to the comment what the effect of this is / why this is needed?
The aim of this function is to translate the real triple into a triple that we have a stdlib for. If that is what this is enables, I'm on board with the change.

Please cherry-pick to master-rebranch and master-next, too.

Copy link
Member Author

@compnerd compnerd Jul 31, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

GNU isn't a vendor, its an environment - the distributions are the vendors, and they all largely use unknown as they dont have any vendor extensions. I can certainly extend the comment (most of that is in the commit message anyways). An example is SuSE which vends a toolchain as x86_64-suse-linux-gnu.

Yeap, you nailed exactly: this is translating a triple (that is computed by lldb from repl_swift) into a triple that the Swift compiler can work with because it has a standard library for that target.

Ah, the joys of the rebranching, thanks for the reminder on that!

Copy link

Choose a reason for hiding this comment

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

An example is SuSE which vends a toolchain as x86_64-suse-linux-gnu.

Ah that makes perfect sense. Thanks for clearing that up!

///
/// TODO: Make Swift more robust.
static std::string GetSwiftFriendlyTriple(StringRef triple) {
if (triple.consume_front("x86_64h"))
return std::string("x86_64") + triple.str();
return triple.str();
static std::string GetSwiftFriendlyTriple(llvm::Triple triple) {
if (triple.getArchName() == "x86_64h")
triple.setArch(llvm::Triple::x86_64);
if (triple.isOSLinux() &&
triple.getEnvironment() == llvm::Triple::UnknownEnvironment)
triple.setEnvironment(llvm::Triple::GNU);
return triple.normalize();
}

bool SwiftASTContext::SetTriple(const llvm::Triple triple, Module *module) {
Expand All @@ -2372,7 +2381,7 @@ bool SwiftASTContext::SetTriple(const llvm::Triple triple, Module *module) {
}

const unsigned unspecified = 0;
std::string adjusted_triple = GetSwiftFriendlyTriple(triple.str());
std::string adjusted_triple = GetSwiftFriendlyTriple(triple);
// If the OS version is unspecified, do fancy things.
if (triple.getOSMajorVersion() == unspecified) {
// If a triple is "<arch>-apple-darwin" change it to be
Expand Down