[Clang][Darwin] Introduce `SubFrameworks` as a SDK default location #115048

cyndyishida · 2024-11-05T19:09:14Z

Have clang always append & pass System/Library/SubFrameworks when determining default sdk search paths.
Teach clang-installapi to traverse there for framework input.
Teach llvm-readtapi that the library files (TBD or binary) in there should be considered private.

resolves: rdar://137457006

llvmbot · 2024-11-05T19:09:52Z

@llvm/pr-subscribers-clang

Author: Cyndy Ishida (cyndyishida)

Changes

Have clang always append & pass System/Library/SubFrameworks when determining default sdk search paths.
Teach clang-installapi to traverse there for framework input.
Teach llvm-readtapi that the library files (TBD or binary) in there should be considered private.

resolves: <rdar://137457006>

Full diff: https://github.com/llvm/llvm-project/pull/115048.diff

5 Files Affected:

(modified) clang/lib/InstallAPI/DirectoryScanner.cpp (+2-1)
(modified) clang/lib/Lex/InitHeaderSearch.cpp (+1)
(added) clang/test/Driver/darwin-subframeworks.c (+15)
(modified) llvm/lib/TextAPI/Utils.cpp (+3)
(modified) llvm/test/tools/llvm-readtapi/stubify-delete.test (+5-2)

diff --git a/clang/lib/InstallAPI/DirectoryScanner.cpp b/clang/lib/InstallAPI/DirectoryScanner.cpp
index 03a8208c7364e9..be43a96f3d97d1 100644
--- a/clang/lib/InstallAPI/DirectoryScanner.cpp
+++ b/clang/lib/InstallAPI/DirectoryScanner.cpp
@@ -277,7 +277,8 @@ llvm::Error DirectoryScanner::scanForFrameworks(StringRef Directory) {
   // Expect a certain directory structure and naming convention to find
   // frameworks.
   static const char *SubDirectories[] = {"System/Library/Frameworks/",
-                                         "System/Library/PrivateFrameworks/"};
+                                         "System/Library/PrivateFrameworks/",
+                                         "System/Library/SubFrameworks"};
 
   // Check if the directory is already a framework.
   if (isFramework(Directory)) {
diff --git a/clang/lib/Lex/InitHeaderSearch.cpp b/clang/lib/Lex/InitHeaderSearch.cpp
index 86c2ecdf9e36eb..c1769c84887b5f 100644
--- a/clang/lib/Lex/InitHeaderSearch.cpp
+++ b/clang/lib/Lex/InitHeaderSearch.cpp
@@ -347,6 +347,7 @@ void InitHeaderSearch::AddDefaultIncludePaths(
       } else {
         AddPath("/System/Library/Frameworks", System, true);
         AddPath("/Library/Frameworks", System, true);
+        AddPath("/System/Library/SubFrameworks", System, true);
       }
     }
     return;
diff --git a/clang/test/Driver/darwin-subframeworks.c b/clang/test/Driver/darwin-subframeworks.c
new file mode 100644
index 00000000000000..62d3ab50f44741
--- /dev/null
+++ b/clang/test/Driver/darwin-subframeworks.c
@@ -0,0 +1,15 @@
+// Add default directories before running clang to check default 
+// search paths. 
+// RUN: rm -rf %t && mkdir -p %t
+// RUN: cp -R %S/Inputs/MacOSX15.1.sdk %t/
+// RUN: mkdir -p %t/MacOSX15.1.sdk/System/Library/Frameworks
+// RUN: mkdir -p %t/MacOSX15.1.sdk/System/Library/SubFrameworks
+// RUN: mkdir -p %t/MacOSX15.1.sdk/usr/include
+
+// RUN: %clang %s -target arm64-apple-darwin13.0 -isysroot %t/MacOSX15.1.sdk -E -v 2>&1 | FileCheck %s
+
+// CHECK:    -isysroot [[PATH:[^ ]*/MacOSX15.1.sdk]]
+// CHECK:    #include <...> search starts here:
+// CHECK:    [[PATH]]/usr/include
+// CHECK:    [[PATH]]/System/Library/Frameworks (framework directory)
+// CHECK:    [[PATH]]/System/Library/SubFrameworks (framework directory)
diff --git a/llvm/lib/TextAPI/Utils.cpp b/llvm/lib/TextAPI/Utils.cpp
index 8a06d53942a947..68d73eac86691d 100644
--- a/llvm/lib/TextAPI/Utils.cpp
+++ b/llvm/lib/TextAPI/Utils.cpp
@@ -120,6 +120,9 @@ bool llvm::MachO::isPrivateLibrary(StringRef Path, bool IsSymLink) {
   if (Path.starts_with("/System/Library/PrivateFrameworks"))
     return true;
 
+  if (Path.starts_with("/System/Library/SubFrameworks"))
+    return true;
+
   // Everything in /usr/lib/swift (including sub-directories) are considered
   // public.
   if (Path.consume_front("/usr/lib/swift/"))
diff --git a/llvm/test/tools/llvm-readtapi/stubify-delete.test b/llvm/test/tools/llvm-readtapi/stubify-delete.test
index 666d740560cbfe..d91b0df06d3d8f 100644
--- a/llvm/test/tools/llvm-readtapi/stubify-delete.test
+++ b/llvm/test/tools/llvm-readtapi/stubify-delete.test
@@ -2,17 +2,20 @@
 # Setup a mix of public and private libraries that resemble apple sdk.
 ; RUN: mkdir -p %t/sysroot/usr/local/lib/ %t/sysroot/usr/lib/
 ; RUN: mkdir -p %t/sysroot/System/Library/Frameworks/System.framework %t/sysroot/System/Library/PrivateFrameworks/Fat.framework
+; RUN: mkdir -p %t/sysroot/System/Library/SubFrameworks/Fat.framework/Headers
 ; RUN: yaml2obj %S/Inputs/libSystem.1.yaml -o %t/sysroot/System/Library/Frameworks/System.framework/System
 ; RUN: yaml2obj %S/Inputs/objc.yaml -o %t/sysroot/usr/lib/libobjc.dylib
 ; RUN: cp %t/sysroot/usr/lib/libobjc.dylib %t/sysroot/usr/local/lib/libobjc-unstable.dylib
 ; RUN: yaml2obj %S/Inputs/universal.yaml -o %t/sysroot/System/Library/PrivateFrameworks/Fat.framework/Fat
+; RUN: cp %t/sysroot/System/Library/PrivateFrameworks/Fat.framework/Fat %t/sysroot/System/Library/SubFrameworks/Fat.framework/Fat
+; RUN: touch %t/sysroot/System/Library/SubFrameworks/Fat.framework/Headers/Fat.h
 ; RUN: llvm-readtapi -stubify %t/sysroot --delete-input --delete-private-libraries 2>&1 | FileCheck %s --allow-empty  --implicit-check-not warning: --implicit-check-not error:
 # Validate expected files are removed.
 ; RUN: not test -f %t/sysroot/System/Library/PrivateFrameworks
 ; RUN: not test -f %t/sysroot/usr/local
 ; RUN: not test -f %t/sysroot/usr/lib/libobjc.dylib
 ; RUN: not test -f %t/sysroot/System/Library/Frameworks/System.framework/System
+; RUN: not test -f %t/sysroot/System/Library/SubFrameworks/Fat.framework/Fat
 ; RUN: test -f %t/sysroot/System/Library/Frameworks/System.framework/System.tbd
 ; RUN: test -f %t/sysroot/usr/lib/libobjc.tbd
-
-
+; RUN: test -f %t/sysroot/System/Library/SubFrameworks/Fat.framework/Headers/Fat.h

llvmbot · 2024-11-05T19:09:53Z

@llvm/pr-subscribers-clang-driver

Author: Cyndy Ishida (cyndyishida)

Changes

Have clang always append & pass System/Library/SubFrameworks when determining default sdk search paths.
Teach clang-installapi to traverse there for framework input.
Teach llvm-readtapi that the library files (TBD or binary) in there should be considered private.

resolves: <rdar://137457006>

Full diff: https://github.com/llvm/llvm-project/pull/115048.diff

5 Files Affected:

(modified) clang/lib/InstallAPI/DirectoryScanner.cpp (+2-1)
(modified) clang/lib/Lex/InitHeaderSearch.cpp (+1)
(added) clang/test/Driver/darwin-subframeworks.c (+15)
(modified) llvm/lib/TextAPI/Utils.cpp (+3)
(modified) llvm/test/tools/llvm-readtapi/stubify-delete.test (+5-2)

diff --git a/clang/lib/InstallAPI/DirectoryScanner.cpp b/clang/lib/InstallAPI/DirectoryScanner.cpp
index 03a8208c7364e9..be43a96f3d97d1 100644
--- a/clang/lib/InstallAPI/DirectoryScanner.cpp
+++ b/clang/lib/InstallAPI/DirectoryScanner.cpp
@@ -277,7 +277,8 @@ llvm::Error DirectoryScanner::scanForFrameworks(StringRef Directory) {
   // Expect a certain directory structure and naming convention to find
   // frameworks.
   static const char *SubDirectories[] = {"System/Library/Frameworks/",
-                                         "System/Library/PrivateFrameworks/"};
+                                         "System/Library/PrivateFrameworks/",
+                                         "System/Library/SubFrameworks"};
 
   // Check if the directory is already a framework.
   if (isFramework(Directory)) {
diff --git a/clang/lib/Lex/InitHeaderSearch.cpp b/clang/lib/Lex/InitHeaderSearch.cpp
index 86c2ecdf9e36eb..c1769c84887b5f 100644
--- a/clang/lib/Lex/InitHeaderSearch.cpp
+++ b/clang/lib/Lex/InitHeaderSearch.cpp
@@ -347,6 +347,7 @@ void InitHeaderSearch::AddDefaultIncludePaths(
       } else {
         AddPath("/System/Library/Frameworks", System, true);
         AddPath("/Library/Frameworks", System, true);
+        AddPath("/System/Library/SubFrameworks", System, true);
       }
     }
     return;
diff --git a/clang/test/Driver/darwin-subframeworks.c b/clang/test/Driver/darwin-subframeworks.c
new file mode 100644
index 00000000000000..62d3ab50f44741
--- /dev/null
+++ b/clang/test/Driver/darwin-subframeworks.c
@@ -0,0 +1,15 @@
+// Add default directories before running clang to check default 
+// search paths. 
+// RUN: rm -rf %t && mkdir -p %t
+// RUN: cp -R %S/Inputs/MacOSX15.1.sdk %t/
+// RUN: mkdir -p %t/MacOSX15.1.sdk/System/Library/Frameworks
+// RUN: mkdir -p %t/MacOSX15.1.sdk/System/Library/SubFrameworks
+// RUN: mkdir -p %t/MacOSX15.1.sdk/usr/include
+
+// RUN: %clang %s -target arm64-apple-darwin13.0 -isysroot %t/MacOSX15.1.sdk -E -v 2>&1 | FileCheck %s
+
+// CHECK:    -isysroot [[PATH:[^ ]*/MacOSX15.1.sdk]]
+// CHECK:    #include <...> search starts here:
+// CHECK:    [[PATH]]/usr/include
+// CHECK:    [[PATH]]/System/Library/Frameworks (framework directory)
+// CHECK:    [[PATH]]/System/Library/SubFrameworks (framework directory)
diff --git a/llvm/lib/TextAPI/Utils.cpp b/llvm/lib/TextAPI/Utils.cpp
index 8a06d53942a947..68d73eac86691d 100644
--- a/llvm/lib/TextAPI/Utils.cpp
+++ b/llvm/lib/TextAPI/Utils.cpp
@@ -120,6 +120,9 @@ bool llvm::MachO::isPrivateLibrary(StringRef Path, bool IsSymLink) {
   if (Path.starts_with("/System/Library/PrivateFrameworks"))
     return true;
 
+  if (Path.starts_with("/System/Library/SubFrameworks"))
+    return true;
+
   // Everything in /usr/lib/swift (including sub-directories) are considered
   // public.
   if (Path.consume_front("/usr/lib/swift/"))
diff --git a/llvm/test/tools/llvm-readtapi/stubify-delete.test b/llvm/test/tools/llvm-readtapi/stubify-delete.test
index 666d740560cbfe..d91b0df06d3d8f 100644
--- a/llvm/test/tools/llvm-readtapi/stubify-delete.test
+++ b/llvm/test/tools/llvm-readtapi/stubify-delete.test
@@ -2,17 +2,20 @@
 # Setup a mix of public and private libraries that resemble apple sdk.
 ; RUN: mkdir -p %t/sysroot/usr/local/lib/ %t/sysroot/usr/lib/
 ; RUN: mkdir -p %t/sysroot/System/Library/Frameworks/System.framework %t/sysroot/System/Library/PrivateFrameworks/Fat.framework
+; RUN: mkdir -p %t/sysroot/System/Library/SubFrameworks/Fat.framework/Headers
 ; RUN: yaml2obj %S/Inputs/libSystem.1.yaml -o %t/sysroot/System/Library/Frameworks/System.framework/System
 ; RUN: yaml2obj %S/Inputs/objc.yaml -o %t/sysroot/usr/lib/libobjc.dylib
 ; RUN: cp %t/sysroot/usr/lib/libobjc.dylib %t/sysroot/usr/local/lib/libobjc-unstable.dylib
 ; RUN: yaml2obj %S/Inputs/universal.yaml -o %t/sysroot/System/Library/PrivateFrameworks/Fat.framework/Fat
+; RUN: cp %t/sysroot/System/Library/PrivateFrameworks/Fat.framework/Fat %t/sysroot/System/Library/SubFrameworks/Fat.framework/Fat
+; RUN: touch %t/sysroot/System/Library/SubFrameworks/Fat.framework/Headers/Fat.h
 ; RUN: llvm-readtapi -stubify %t/sysroot --delete-input --delete-private-libraries 2>&1 | FileCheck %s --allow-empty  --implicit-check-not warning: --implicit-check-not error:
 # Validate expected files are removed.
 ; RUN: not test -f %t/sysroot/System/Library/PrivateFrameworks
 ; RUN: not test -f %t/sysroot/usr/local
 ; RUN: not test -f %t/sysroot/usr/lib/libobjc.dylib
 ; RUN: not test -f %t/sysroot/System/Library/Frameworks/System.framework/System
+; RUN: not test -f %t/sysroot/System/Library/SubFrameworks/Fat.framework/Fat
 ; RUN: test -f %t/sysroot/System/Library/Frameworks/System.framework/System.tbd
 ; RUN: test -f %t/sysroot/usr/lib/libobjc.tbd
-
-
+; RUN: test -f %t/sysroot/System/Library/SubFrameworks/Fat.framework/Headers/Fat.h

ian-twilightcoder · 2024-11-05T19:13:43Z

Do we also need to update darwin::Linker::ConstructJob(...)?

cyndyishida · 2024-11-05T19:33:43Z

Do we also need to update darwin::Linker::ConstructJob(...)?

I thought about it, but clang already doesn't for today's default paths so I decided against it. It seems only pass extra paths to the linker for platform-specific directories like DriverKit. Xcode's ld adds its own default path to S/L/F

* Have clang always append & pass `System/Library/SubFrameworks` when determining default sdk search paths. * Teach `clang-installapi` to traverse there for framework input. * Teach `llvm-readtapi` that the library files (TBD or binary) in there should be considered private. resolves: <rdar://137457006>

cyndyishida · 2024-11-05T21:22:02Z

> git checkout -f baecb5c50287efebf2482739bfbe320c8147b7ff
error: unable to unlink old 'clang/lib/AST/Decl.cpp': Invalid argument
error: unable to unlink old 'clang/lib/Serialization/ASTReaderDecl.cpp': Invalid argument

Seems like an unrelated issue is happening on the windows bot, noticed the same in a different PR. https://buildkite.com/llvm-project/github-pull-requests/builds/116688#0192fe1c-1484-47e8-b14c-7f3da7038b76

styles

cyndyishida · 2024-11-08T16:06:25Z

ping

clang/lib/Lex/InitHeaderSearch.cpp

llvm/tools/llvm-readtapi/llvm-readtapi.cpp

cyndyishida · 2024-11-13T19:44:03Z

ping

ributzka

LGTM

clang/lib/Lex/InitHeaderSearch.cpp

…lvm#115048) * Have clang always append & pass System/Library/SubFrameworks when determining default sdk search paths. * Teach clang-installapi to traverse there for framework input. * Teach llvm-readtapi that the library files (TBD or binary) in there should be considered private. resolves: rdar://137457006 (cherry picked from commit 2d48489)

cyndyishida requested review from ahatanak, cachemeifyoucan, ian-twilightcoder, ributzka and zixu-w November 5, 2024 19:09

llvmbot added clang Clang issues not falling into any other category clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Nov 5, 2024

cyndyishida closed this Nov 5, 2024

cyndyishida reopened this Nov 5, 2024

cyndyishida force-pushed the eng/pr-newDefaultFrameworks branch from baecb5c to 25e30d6 Compare November 5, 2024 21:17

[llvm-readtapi] Make directory traversal more reslient to different path

4207ffe

styles

cyndyishida force-pushed the eng/pr-newDefaultFrameworks branch from 25e30d6 to 4207ffe Compare November 6, 2024 16:12

ributzka reviewed Nov 8, 2024

View reviewed changes

clang/lib/Lex/InitHeaderSearch.cpp Show resolved Hide resolved

llvm/tools/llvm-readtapi/llvm-readtapi.cpp Show resolved Hide resolved

cyndyishida added 2 commits November 8, 2024 11:29

Swap order of search paths.

3f45b42

Disable darwin-subframeworks.c test on windows like driverkit-path test

f7d617a

ributzka approved these changes Nov 14, 2024

View reviewed changes

jakepetroules reviewed Nov 15, 2024

View reviewed changes

clang/lib/Lex/InitHeaderSearch.cpp Show resolved Hide resolved

cyndyishida merged commit 2d48489 into llvm:main Nov 15, 2024
5 of 8 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[Clang][Darwin] Introduce `SubFrameworks` as a SDK default location #115048

[Clang][Darwin] Introduce `SubFrameworks` as a SDK default location #115048

Uh oh!

cyndyishida commented Nov 5, 2024

Uh oh!

llvmbot commented Nov 5, 2024

Uh oh!

llvmbot commented Nov 5, 2024

Uh oh!

ian-twilightcoder commented Nov 5, 2024

Uh oh!

cyndyishida commented Nov 5, 2024 •

edited

Loading

Uh oh!

cyndyishida commented Nov 5, 2024

Uh oh!

cyndyishida commented Nov 8, 2024

Uh oh!

Uh oh!

Uh oh!

cyndyishida commented Nov 13, 2024

Uh oh!

ributzka left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

[Clang][Darwin] Introduce SubFrameworks as a SDK default location #115048

[Clang][Darwin] Introduce SubFrameworks as a SDK default location #115048

Uh oh!

Conversation

cyndyishida commented Nov 5, 2024

Uh oh!

llvmbot commented Nov 5, 2024

Uh oh!

llvmbot commented Nov 5, 2024

Uh oh!

ian-twilightcoder commented Nov 5, 2024

Uh oh!

cyndyishida commented Nov 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cyndyishida commented Nov 5, 2024

Uh oh!

cyndyishida commented Nov 8, 2024

Uh oh!

Uh oh!

Uh oh!

cyndyishida commented Nov 13, 2024

Uh oh!

ributzka left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

[Clang][Darwin] Introduce `SubFrameworks` as a SDK default location #115048

[Clang][Darwin] Introduce `SubFrameworks` as a SDK default location #115048

cyndyishida commented Nov 5, 2024 •

edited

Loading