Skip to content

[lldb][ClangExpressionParser] Set BuiltinHeadersInSystemModules depending on SDK version #9059

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.

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
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include "clang/AST/ExternalASTSource.h"
#include "clang/AST/PrettyPrinter.h"
#include "clang/Basic/Builtins.h"
#include "clang/Basic/DarwinSDKInfo.h"
#include "clang/Basic/DiagnosticIDs.h"
#include "clang/Basic/SourceLocation.h"
#include "clang/Basic/TargetInfo.h"
Expand Down Expand Up @@ -352,6 +353,73 @@ static void SetupDefaultClangDiagnostics(CompilerInstance &compiler) {
}
}

// NOTE: should be kept in sync with sdkSupportsBuiltinModules in
// Toolchains/Darwin.cpp
static bool
sdkSupportsBuiltinModulesImpl(const llvm::Triple &triple,

Choose a reason for hiding this comment

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

I'm about to change the implementation of this function in the Darwin driver

Copy link
Author

Choose a reason for hiding this comment

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

Implementation in what sense? Changes to the SDK version checks?

Choose a reason for hiding this comment

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

Posted llvm#102239 for it.

Copy link
Author

Choose a reason for hiding this comment

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

Ah yes I accounted for that in this patch already

const std::optional<DarwinSDKInfo> &SDKInfo) {
if (!SDKInfo)
return false;

VersionTuple SDKVersion = SDKInfo->getVersion();
switch (triple.getOS()) {
case Triple::OSType::MacOSX:
return SDKVersion >= VersionTuple(15U);
case Triple::OSType::IOS:
return SDKVersion >= VersionTuple(18U);
case Triple::OSType::TvOS:
return SDKVersion >= VersionTuple(18U);
case Triple::OSType::WatchOS:
return SDKVersion >= VersionTuple(11U);
case Triple::OSType::XROS:
return SDKVersion >= VersionTuple(2U);
default:
// New SDKs support builtin modules from the start.
return true;
}
}

/// Returns true if the SDK for the specified triple supports
/// builtin modules in system headers. This is used to decide
/// whether to pass -fbuiltin-headers-in-system-modules to
/// the compiler instance when compiling the `std` module.
///
/// This function assumes one of the directories in \ref include_dirs
/// is an SDK path that exists on the host. The SDK version is
/// read from the SDKSettings.json in that directory.
///
/// \param[in] triple The target triple.
/// \param[in] include_dirs The include directories the compiler will use
/// during header search.
///
static bool
sdkSupportsBuiltinModules(llvm::Triple const &triple,
std::vector<std::string> const &include_dirs) {
// Find an SDK directory.
static constexpr std::string_view s_sdk_suffix = ".sdk";
auto it = llvm::find_if(include_dirs, [](std::string const &path) {
return path.find(s_sdk_suffix) != std::string::npos;
});
if (it == include_dirs.end())
return false;

auto VFS = FileSystem::Instance().GetVirtualFileSystem();
if (!VFS)
return false;

// Extract: /path/to/some.sdk
size_t suffix = it->find(s_sdk_suffix);
assert(suffix != std::string::npos);
std::string sdk_path = it->substr(0, suffix + s_sdk_suffix.size());

// Extract SDK version from the /path/to/some.sdk/SDKSettings.json
auto parsed = clang::parseDarwinSDKInfo(*VFS, sdk_path);
if (!parsed)
return false;

return sdkSupportsBuiltinModulesImpl(triple, *parsed);
}

//===----------------------------------------------------------------------===//
// Implementation of ClangExpressionParser
//===----------------------------------------------------------------------===//
Expand Down Expand Up @@ -576,7 +644,8 @@ ClangExpressionParser::ClangExpressionParser(
lang_opts.GNUMode = true;
lang_opts.GNUKeywords = true;
lang_opts.CPlusPlus11 = true;
lang_opts.BuiltinHeadersInSystemModules = true;
lang_opts.BuiltinHeadersInSystemModules = !sdkSupportsBuiltinModules(

Choose a reason for hiding this comment

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

This ! is the intended direction, right?

Choose a reason for hiding this comment

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

Yes. The builtin headers have to go in the system modules if the sdk doesn't support the builtin modules.

target_sp->GetArchitecture().GetTriple(), m_include_directories);

// The Darwin libc expects this macro to be set.
lang_opts.GNUCVersion = 40201;
Expand Down