Skip to content

Conversation

ributzka
Copy link
Collaborator

@ributzka ributzka commented Jan 5, 2024

  • [clang][modules] Remove no longer needed autolink test for TBD files.
  • [clang][modules] Remove _Private suffix from framework auto-link hints.

test for TBD files.

The autolink feature no longer checks for the existence of a binary or
TBD file. Hence, the autolink TBD test can be removed.
…nts.

The auto-link hint for a framework is generated from the module name.
For a given framework <name>, the public module name is <name>, while the
private module name is <name>_Private.

This change removes the `_Private` suffix from the module name to get the
actual framework name for the auto-link hint.
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:modules C++20 modules and Clang Header Modules labels Jan 5, 2024
@llvmbot
Copy link
Member

llvmbot commented Jan 5, 2024

@llvm/pr-subscribers-clang-modules

Author: Juergen Ributzka (ributzka)

Changes
  • [clang][modules] Remove no longer needed autolink test for TBD files.
  • [clang][modules] Remove _Private suffix from framework auto-link hints.

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

5 Files Affected:

  • (modified) clang/lib/Lex/ModuleMap.cpp (+3-1)
  • (removed) clang/test/Modules/Inputs/AutolinkTBD.framework/AutolinkTBD.tbd (-1)
  • (removed) clang/test/Modules/Inputs/AutolinkTBD.framework/Headers/AutolinkTBD.h (-1)
  • (removed) clang/test/Modules/autolinkTBD.m (-16)
  • (added) clang/test/Modules/autolink_private_module.m (+25)
diff --git a/clang/lib/Lex/ModuleMap.cpp b/clang/lib/Lex/ModuleMap.cpp
index ea5d13deb11470..42d55d09ea5a13 100644
--- a/clang/lib/Lex/ModuleMap.cpp
+++ b/clang/lib/Lex/ModuleMap.cpp
@@ -984,7 +984,9 @@ static void inferFrameworkLink(Module *Mod) {
   assert(!Mod->isSubFramework() &&
          "Can only infer linking for top-level frameworks");
 
-  Mod->LinkLibraries.push_back(Module::LinkLibrary(Mod->Name,
+  StringRef FrameworkName(Mod->Name);
+  FrameworkName.consume_back("_Private");
+  Mod->LinkLibraries.push_back(Module::LinkLibrary(FrameworkName.str(),
                                                    /*IsFramework=*/true));
 }
 
diff --git a/clang/test/Modules/Inputs/AutolinkTBD.framework/AutolinkTBD.tbd b/clang/test/Modules/Inputs/AutolinkTBD.framework/AutolinkTBD.tbd
deleted file mode 100644
index 4aa0f85d0d56aa..00000000000000
--- a/clang/test/Modules/Inputs/AutolinkTBD.framework/AutolinkTBD.tbd
+++ /dev/null
@@ -1 +0,0 @@
-empty file - clang only needs to check if it exists.
diff --git a/clang/test/Modules/Inputs/AutolinkTBD.framework/Headers/AutolinkTBD.h b/clang/test/Modules/Inputs/AutolinkTBD.framework/Headers/AutolinkTBD.h
deleted file mode 100644
index 914983c49636c2..00000000000000
--- a/clang/test/Modules/Inputs/AutolinkTBD.framework/Headers/AutolinkTBD.h
+++ /dev/null
@@ -1 +0,0 @@
-extern int foo(void);
diff --git a/clang/test/Modules/autolinkTBD.m b/clang/test/Modules/autolinkTBD.m
deleted file mode 100644
index 69253294f7b816..00000000000000
--- a/clang/test/Modules/autolinkTBD.m
+++ /dev/null
@@ -1,16 +0,0 @@
-// UNSUPPORTED: target={{.*}}-zos{{.*}}, target={{.*}}-aix{{.*}}
-// RUN: rm -rf %t
-// RUN: %clang_cc1 -emit-llvm -o - -fmodules-cache-path=%t -fmodules -fimplicit-module-maps -F %S/Inputs %s | FileCheck %s
-// RUN: %clang_cc1 -emit-llvm -fno-autolink -o - -fmodules-cache-path=%t -fmodules -fimplicit-module-maps -F %S/Inputs %s | FileCheck --check-prefix=CHECK-AUTOLINK-DISABLED %s
-
-@import AutolinkTBD;
-
-int f(void) {
-  return foo();
-}
-
-// CHECK: !llvm.linker.options = !{![[AUTOLINK_FRAMEWORK:[0-9]+]]}
-// CHECK: ![[AUTOLINK_FRAMEWORK]] = !{!"-framework", !"AutolinkTBD"}
-
-// CHECK-AUTOLINK-DISABLED: !llvm.module.flags
-// CHECK-AUTOLINK-DISABLED-NOT: !llvm.linker.options
diff --git a/clang/test/Modules/autolink_private_module.m b/clang/test/Modules/autolink_private_module.m
new file mode 100644
index 00000000000000..54bebc3a587b1b
--- /dev/null
+++ b/clang/test/Modules/autolink_private_module.m
@@ -0,0 +1,25 @@
+// Test that autolink hints for frameworks don't use the private module name.
+// RUN: rm -rf %t && mkdir %t
+// RUN: split-file %s %t
+
+// RUN: %clang_cc1 -emit-llvm -o - -fmodules-cache-path=%t/ModuleCache -fmodules -fimplicit-module-maps -F %t/Frameworks %t/test.m | FileCheck %s
+
+// CHECK:     !{!"-framework", !"Autolink"}
+// CHECK-NOT: !{!"-framework", !"Autolink_Private"}
+
+//--- test.m
+#include <Autolink/Autolink.h>
+#include <Autolink/Autolink_Private.h>
+
+//--- Frameworks/Autolink.framework/Headers/Autolink.h
+void public();
+
+//--- Frameworks/Autolink.framework/PrivateHeaders/Autolink_Private.h
+void private();
+
+//--- Frameworks/Autolink.framework/Modules/module.modulemap
+framework module Autolink { header "Autolink.h"}
+
+//--- Frameworks/Autolink.framework/Modules/module.private.modulemap
+framework module Autolink_Private { header "Autolink_Private.h"}
+

@llvmbot
Copy link
Member

llvmbot commented Jan 5, 2024

@llvm/pr-subscribers-clang

Author: Juergen Ributzka (ributzka)

Changes
  • [clang][modules] Remove no longer needed autolink test for TBD files.
  • [clang][modules] Remove _Private suffix from framework auto-link hints.

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

5 Files Affected:

  • (modified) clang/lib/Lex/ModuleMap.cpp (+3-1)
  • (removed) clang/test/Modules/Inputs/AutolinkTBD.framework/AutolinkTBD.tbd (-1)
  • (removed) clang/test/Modules/Inputs/AutolinkTBD.framework/Headers/AutolinkTBD.h (-1)
  • (removed) clang/test/Modules/autolinkTBD.m (-16)
  • (added) clang/test/Modules/autolink_private_module.m (+25)
diff --git a/clang/lib/Lex/ModuleMap.cpp b/clang/lib/Lex/ModuleMap.cpp
index ea5d13deb11470..42d55d09ea5a13 100644
--- a/clang/lib/Lex/ModuleMap.cpp
+++ b/clang/lib/Lex/ModuleMap.cpp
@@ -984,7 +984,9 @@ static void inferFrameworkLink(Module *Mod) {
   assert(!Mod->isSubFramework() &&
          "Can only infer linking for top-level frameworks");
 
-  Mod->LinkLibraries.push_back(Module::LinkLibrary(Mod->Name,
+  StringRef FrameworkName(Mod->Name);
+  FrameworkName.consume_back("_Private");
+  Mod->LinkLibraries.push_back(Module::LinkLibrary(FrameworkName.str(),
                                                    /*IsFramework=*/true));
 }
 
diff --git a/clang/test/Modules/Inputs/AutolinkTBD.framework/AutolinkTBD.tbd b/clang/test/Modules/Inputs/AutolinkTBD.framework/AutolinkTBD.tbd
deleted file mode 100644
index 4aa0f85d0d56aa..00000000000000
--- a/clang/test/Modules/Inputs/AutolinkTBD.framework/AutolinkTBD.tbd
+++ /dev/null
@@ -1 +0,0 @@
-empty file - clang only needs to check if it exists.
diff --git a/clang/test/Modules/Inputs/AutolinkTBD.framework/Headers/AutolinkTBD.h b/clang/test/Modules/Inputs/AutolinkTBD.framework/Headers/AutolinkTBD.h
deleted file mode 100644
index 914983c49636c2..00000000000000
--- a/clang/test/Modules/Inputs/AutolinkTBD.framework/Headers/AutolinkTBD.h
+++ /dev/null
@@ -1 +0,0 @@
-extern int foo(void);
diff --git a/clang/test/Modules/autolinkTBD.m b/clang/test/Modules/autolinkTBD.m
deleted file mode 100644
index 69253294f7b816..00000000000000
--- a/clang/test/Modules/autolinkTBD.m
+++ /dev/null
@@ -1,16 +0,0 @@
-// UNSUPPORTED: target={{.*}}-zos{{.*}}, target={{.*}}-aix{{.*}}
-// RUN: rm -rf %t
-// RUN: %clang_cc1 -emit-llvm -o - -fmodules-cache-path=%t -fmodules -fimplicit-module-maps -F %S/Inputs %s | FileCheck %s
-// RUN: %clang_cc1 -emit-llvm -fno-autolink -o - -fmodules-cache-path=%t -fmodules -fimplicit-module-maps -F %S/Inputs %s | FileCheck --check-prefix=CHECK-AUTOLINK-DISABLED %s
-
-@import AutolinkTBD;
-
-int f(void) {
-  return foo();
-}
-
-// CHECK: !llvm.linker.options = !{![[AUTOLINK_FRAMEWORK:[0-9]+]]}
-// CHECK: ![[AUTOLINK_FRAMEWORK]] = !{!"-framework", !"AutolinkTBD"}
-
-// CHECK-AUTOLINK-DISABLED: !llvm.module.flags
-// CHECK-AUTOLINK-DISABLED-NOT: !llvm.linker.options
diff --git a/clang/test/Modules/autolink_private_module.m b/clang/test/Modules/autolink_private_module.m
new file mode 100644
index 00000000000000..54bebc3a587b1b
--- /dev/null
+++ b/clang/test/Modules/autolink_private_module.m
@@ -0,0 +1,25 @@
+// Test that autolink hints for frameworks don't use the private module name.
+// RUN: rm -rf %t && mkdir %t
+// RUN: split-file %s %t
+
+// RUN: %clang_cc1 -emit-llvm -o - -fmodules-cache-path=%t/ModuleCache -fmodules -fimplicit-module-maps -F %t/Frameworks %t/test.m | FileCheck %s
+
+// CHECK:     !{!"-framework", !"Autolink"}
+// CHECK-NOT: !{!"-framework", !"Autolink_Private"}
+
+//--- test.m
+#include <Autolink/Autolink.h>
+#include <Autolink/Autolink_Private.h>
+
+//--- Frameworks/Autolink.framework/Headers/Autolink.h
+void public();
+
+//--- Frameworks/Autolink.framework/PrivateHeaders/Autolink_Private.h
+void private();
+
+//--- Frameworks/Autolink.framework/Modules/module.modulemap
+framework module Autolink { header "Autolink.h"}
+
+//--- Frameworks/Autolink.framework/Modules/module.private.modulemap
+framework module Autolink_Private { header "Autolink_Private.h"}
+

@ributzka
Copy link
Collaborator Author

ributzka commented Jan 8, 2024

Thanks for the review Jan.

@ributzka ributzka merged commit f4bc70e into llvm:main Jan 8, 2024
@ributzka ributzka deleted the pr/autolink branch January 8, 2024 19:04
akyrtzi added a commit to swiftlang/llvm-project that referenced this pull request Jan 11, 2024
ributzka added a commit to swiftlang/llvm-project that referenced this pull request Jan 23, 2024
…nts. (llvm#77120)

- [clang][modules] Remove no longer needed autolink test for TBD files.
- [clang][modules] Remove `_Private` suffix from framework auto-link
hints.

(cherry picked from commit f4bc70e)
ributzka added a commit to swiftlang/llvm-project that referenced this pull request Jan 24, 2024
…nts. (llvm#77120)

- [clang][modules] Remove no longer needed autolink test for TBD files.
- [clang][modules] Remove `_Private` suffix from framework auto-link
hints.

(cherry picked from commit f4bc70e)
akyrtzi added a commit to akyrtzi/llvm-project that referenced this pull request Jan 24, 2024
Update checking for test output after llvm#77120.

(cherry picked from commit ccb1e72)
benlangmuir pushed a commit to benlangmuir/llvm-project that referenced this pull request Jan 25, 2024
Update checking for test output after llvm#77120.

(cherry picked from commit ccb1e72)
justinfargnoli pushed a commit to justinfargnoli/llvm-project that referenced this pull request Jan 28, 2024
…nts. (llvm#77120)

- [clang][modules] Remove no longer needed autolink test for TBD files.
- [clang][modules] Remove `_Private` suffix from framework auto-link
hints.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:modules C++20 modules and Clang Header Modules clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants