Skip to content

[clang][Darwin] Remove legacy framework search path logic in the frontend #75841

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 2 commits into from
Dec 31, 2023
Merged

[clang][Darwin] Remove legacy framework search path logic in the frontend #75841

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ca2a858
[clang][Darwin] Remove legacy framework search path logic in the fron…
ldionne Dec 18, 2023
ee38b41
Tentatively remove cuda test that is not relevant anymore since the l…
ldionne Dec 18, 2023
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
35 changes: 22 additions & 13 deletions clang/lib/Driver/ToolChains/Darwin.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -758,9 +758,14 @@ void darwin::Linker::ConstructJob(Compilation &C, const JobAction &JA,
}
}

// Add non-standard, platform-specific search paths, e.g., for DriverKit:
// -L<sysroot>/System/DriverKit/usr/lib
// -F<sysroot>/System/DriverKit/System/Library/Framework
// Add framework include paths and library search paths.
// There are two flavors:
// 1. The "non-standard" paths, e.g. for DriverKit:
// -L<sysroot>/System/DriverKit/usr/lib
// -F<sysroot>/System/DriverKit/System/Library/Frameworks
// 2. The "standard" paths, e.g. for macOS and iOS:
// -F<sysroot>/System/Library/Frameworks
// -F<sysroot>/Library/Frameworks
{
bool NonStandardSearchPath = false;
Copy link
Member Author

@ldionne ldionne Dec 18, 2023

Choose a reason for hiding this comment

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

@yln Note that I think this whole block could almost go away now. I think we always do the right thing in the linker on Apple platforms, so I don't think the driver needs any special logic for -F and -L anymore, but I could be wrong. I also wanted to keep this change minimal.

Copy link
Collaborator

@yln yln Dec 18, 2023

Choose a reason for hiding this comment

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

Do you mean that ld64-605.1 is old enough / has "aged out" by now so we can drop this special case?

Copy link
Member Author

@ldionne ldionne Dec 18, 2023

Choose a reason for hiding this comment

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

Yes, that is what I suspect.

Copy link
Collaborator

@yln yln Dec 18, 2023

Choose a reason for hiding this comment

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

Got it, that would be another good cleanup! I recommend doing this as a separate PR.

const auto &Triple = getToolChain().getTriple();
Expand All @@ -771,18 +776,22 @@ void darwin::Linker::ConstructJob(Compilation &C, const JobAction &JA,
(Version.getMajor() == 605 && Version.getMinor().value_or(0) < 1);
}

if (NonStandardSearchPath) {
if (auto *Sysroot = Args.getLastArg(options::OPT_isysroot)) {
auto AddSearchPath = [&](StringRef Flag, StringRef SearchPath) {
SmallString<128> P(Sysroot->getValue());
AppendPlatformPrefix(P, Triple);
llvm::sys::path::append(P, SearchPath);
if (getToolChain().getVFS().exists(P)) {
CmdArgs.push_back(Args.MakeArgString(Flag + P));
}
};
if (auto *Sysroot = Args.getLastArg(options::OPT_isysroot)) {
auto AddSearchPath = [&](StringRef Flag, StringRef SearchPath) {
SmallString<128> P(Sysroot->getValue());
AppendPlatformPrefix(P, Triple);
llvm::sys::path::append(P, SearchPath);
if (getToolChain().getVFS().exists(P)) {
CmdArgs.push_back(Args.MakeArgString(Flag + P));
}
};

if (NonStandardSearchPath) {
AddSearchPath("-L", "/usr/lib");
AddSearchPath("-F", "/System/Library/Frameworks");
} else if (!Triple.isDriverKit()) {
AddSearchPath("-F", "/System/Library/Frameworks");
AddSearchPath("-F", "/Library/Frameworks");
}
}
}
Expand Down
18 changes: 3 additions & 15 deletions clang/lib/Lex/InitHeaderSearch.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -324,6 +324,9 @@ bool InitHeaderSearch::ShouldAddDefaultIncludePaths(
break;
}

if (triple.isOSDarwin())
return false;

return true; // Everything else uses AddDefaultIncludePaths().
}

Expand All @@ -338,21 +341,6 @@ void InitHeaderSearch::AddDefaultIncludePaths(
if (!ShouldAddDefaultIncludePaths(triple))
return;

// NOTE: some additional header search logic is handled in the driver for
// Darwin.
if (triple.isOSDarwin()) {
if (HSOpts.UseStandardSystemIncludes) {
// Add the default framework include paths on Darwin.
if (triple.isDriverKit()) {
AddPath("/System/DriverKit/System/Library/Frameworks", System, true);
} else {
AddPath("/System/Library/Frameworks", System, true);
AddPath("/Library/Frameworks", System, true);
}
}
return;
}

if (Lang.CPlusPlus && !Lang.AsmPreprocessor &&
HSOpts.UseStandardCXXIncludes && HSOpts.UseStandardSystemIncludes) {
if (HSOpts.UseLibcxx) {
Expand Down
1 change: 0 additions & 1 deletion clang/test/Driver/driverkit-path.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,4 +31,3 @@ int main() { return 0; }
// INC: [[PATH]]/System/DriverKit/usr/local/include
// INC: /lib{{(64)?}}/clang/{{[^/ ]+}}/include
// INC: [[PATH]]/System/DriverKit/usr/include
// INC: [[PATH]]/System/DriverKit/System/Library/Frameworks (framework directory)
13 changes: 0 additions & 13 deletions clang/test/Preprocessor/cuda-macos-includes.cu
View file
Open in desktop

This file was deleted.