[clang] Fix objc_sel_{name,types} missing an underscore (PR #88713)
https://github.com/Midar created https://github.com/llvm/llvm-project/pull/88713
The other places all use the underscore.
This is just a quick drive-by commit, LMK if more is needed.
>From e39e2fc218c588d66937211f98a25eb2acf00a6b Mon Sep 17 00:00:00 2001
From: Jonathan Schleifer
Date: Mon, 15 Apr 2024 13:36:05 +0200
Subject: [PATCH] Fix objc_sel_{name,types} missing an underscore
The other places all use the underscore.
---
clang/lib/CodeGen/CGObjCGNU.cpp | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/clang/lib/CodeGen/CGObjCGNU.cpp b/clang/lib/CodeGen/CGObjCGNU.cpp
index 4e7f777ba1d916..97df8c5ae18c64 100644
--- a/clang/lib/CodeGen/CGObjCGNU.cpp
+++ b/clang/lib/CodeGen/CGObjCGNU.cpp
@@ -3873,13 +3873,14 @@ llvm::Function *CGObjCGNU::ModuleInitFunction() {
for (auto &untypedSel : allSelectors) {
std::string selNameStr = untypedSel.getAsString();
- llvm::Constant *selName = ExportUniqueString(selNameStr,
".objc_sel_name");
+ llvm::Constant *selName = ExportUniqueString(selNameStr,
+".objc_sel_name_");
for (TypedSelector &sel : table[untypedSel]) {
llvm::Constant *selectorTypeEncoding = NULLPtr;
if (!sel.first.empty())
selectorTypeEncoding =
-MakeConstantString(sel.first, ".objc_sel_types");
+MakeConstantString(sel.first, ".objc_sel_types_");
auto selStruct = selectors.beginStruct(selStructTy);
selStruct.add(selName);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Fix objc_sel_{name,types} missing an underscore (PR #88713)
github-actions[bot] wrote: Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using `@` followed by their GitHub username. If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the [LLVM GitHub User Guide](https://llvm.org/docs/GitHub.html). You can also ask questions in a comment on this PR, on the [LLVM Discord](https://discord.com/invite/xS7Z362) or on the [forums](https://discourse.llvm.org/). https://github.com/llvm/llvm-project/pull/88713 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Fix objc_sel_{name,types} missing an underscore (PR #88713)
llvmbot wrote:
@llvm/pr-subscribers-clang
Author: Jonathan Schleifer (Midar)
Changes
The other places all use the underscore.
This is just a quick drive-by commit, LMK if more is needed.
---
Full diff: https://github.com/llvm/llvm-project/pull/88713.diff
1 Files Affected:
- (modified) clang/lib/CodeGen/CGObjCGNU.cpp (+3-2)
``diff
diff --git a/clang/lib/CodeGen/CGObjCGNU.cpp b/clang/lib/CodeGen/CGObjCGNU.cpp
index 4e7f777ba1d916..97df8c5ae18c64 100644
--- a/clang/lib/CodeGen/CGObjCGNU.cpp
+++ b/clang/lib/CodeGen/CGObjCGNU.cpp
@@ -3873,13 +3873,14 @@ llvm::Function *CGObjCGNU::ModuleInitFunction() {
for (auto &untypedSel : allSelectors) {
std::string selNameStr = untypedSel.getAsString();
- llvm::Constant *selName = ExportUniqueString(selNameStr,
".objc_sel_name");
+ llvm::Constant *selName = ExportUniqueString(selNameStr,
+".objc_sel_name_");
for (TypedSelector &sel : table[untypedSel]) {
llvm::Constant *selectorTypeEncoding = NULLPtr;
if (!sel.first.empty())
selectorTypeEncoding =
-MakeConstantString(sel.first, ".objc_sel_types");
+MakeConstantString(sel.first, ".objc_sel_types_");
auto selStruct = selectors.beginStruct(selStructTy);
selStruct.add(selName);
``
https://github.com/llvm/llvm-project/pull/88713
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Fix objc_sel_{name,types} missing an underscore (PR #88713)
https://github.com/Midar updated https://github.com/llvm/llvm-project/pull/88713
>From 8dc7333c0e9c757bba91ebfbe57280ad0635afa9 Mon Sep 17 00:00:00 2001
From: Jonathan Schleifer
Date: Mon, 15 Apr 2024 13:36:05 +0200
Subject: [PATCH] Fix objc_sel_{name,types} missing an underscore
The other places all use the underscore.
This also makes .objc_sel_name_* consistently private.
---
clang/lib/CodeGen/CGObjCGNU.cpp | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/clang/lib/CodeGen/CGObjCGNU.cpp b/clang/lib/CodeGen/CGObjCGNU.cpp
index 4e7f777ba1d916..d2823b7ee3de46 100644
--- a/clang/lib/CodeGen/CGObjCGNU.cpp
+++ b/clang/lib/CodeGen/CGObjCGNU.cpp
@@ -3873,13 +3873,14 @@ llvm::Function *CGObjCGNU::ModuleInitFunction() {
for (auto &untypedSel : allSelectors) {
std::string selNameStr = untypedSel.getAsString();
- llvm::Constant *selName = ExportUniqueString(selNameStr,
".objc_sel_name");
+ llvm::Constant *selName = ExportUniqueString(selNameStr,
+".objc_sel_name_", true);
for (TypedSelector &sel : table[untypedSel]) {
llvm::Constant *selectorTypeEncoding = NULLPtr;
if (!sel.first.empty())
selectorTypeEncoding =
-MakeConstantString(sel.first, ".objc_sel_types");
+MakeConstantString(sel.first, ".objc_sel_types_");
auto selStruct = selectors.beginStruct(selStructTy);
selStruct.add(selName);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Fix objc_sel_{name,types} missing an underscore (PR #88713)
Midar wrote: Maybe @rjmccall would be a good reviewer? https://github.com/llvm/llvm-project/pull/88713 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Fix objc_sel_{name,types} missing an underscore (PR #88713)
https://github.com/rjmccall commented: At first, I was worried that this was an ABI break. Then I noticed the internal inconsistency within this single file, and so I became worried that it was an ABI *fix*. But then I noticed that the normal selector-emission code actually makes these strings hidden by default, and so now my guess is that the uniqueness of these strings is at best an optimization. Still, it's an optimization we should do. There are actually three differences between the code emission here and the code emission in the "primary" paths in `GetConstantSelector` and `GetTypeString`: - the primary path uses an underscore in the symbol name prefix - the primary path adjusts the symbol name to avoid restricted characters in the target object file format - the primary path generates hidden symbols My assumption is that, on all three of these points, we should just do what the primary path would do. Please extract common functions as necessary in order to reuse the logic. @davidchisnall, could you verify my reasoning here? https://github.com/llvm/llvm-project/pull/88713 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Fix objc_sel_{name,types} missing an underscore (PR #88713)
https://github.com/rjmccall edited https://github.com/llvm/llvm-project/pull/88713 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Fix objc_sel_{name,types} missing an underscore (PR #88713)
Midar wrote: These symbols are just used to deduplicate strings: That's why they're hidden and in COMDAT. So I don't think it'll break or fix the ABI ;). https://github.com/llvm/llvm-project/pull/88713 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Fix objc_sel_{name,types} missing an underscore (PR #88713)
davidchisnall wrote: Yup, it’s just an optimisation. The runtime does a slightly more complex deduplication pass during loading, this just does a best effort merge so that we don’t end up with hundreds of copies of common selectors as we did with the GCC ABI. If we don’t do it, it isn’t an ABI break, it’s only an ABI break if we merge two strings that are not the same. Some of this code accreted and so deduplicating the code, as well as the selectors, is a good idea. https://github.com/llvm/llvm-project/pull/88713 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Fix objc_sel_{name,types} missing an underscore (PR #88713)
rjmccall wrote: Great, thanks! @Midar, if you can fix this code up so that these strings are created in one place (and test the new output, since this is not NFC), I'd be happy to sign off. https://github.com/llvm/llvm-project/pull/88713 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
