[PATCH] D143803: [clang][alias|ifunc]: Add a diagnostic for mangled names

2023-03-06 Thread Erich Keane via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG9306ef9750b7: [clang][alias|ifunc]: Add a diagnostic for mangled names (authored by 0xdc03, committed by erichkeane). Changed prior to commit: htt

[PATCH] D143803: [clang][alias|ifunc]: Add a diagnostic for mangled names

2023-03-06 Thread Dhruv Chawla via Phabricator via cfe-commits
0xdc03 added a comment. In D143803#4171728 , @erichkeane wrote: > Not thrilled about 'mangled name' still, but I can't think of anything better > after all this time, so LGTM. Oh wow your reply was much faster than I expected 😅. If you can, please land

[PATCH] D143803: [clang][alias|ifunc]: Add a diagnostic for mangled names

2023-03-06 Thread Dhruv Chawla via Phabricator via cfe-commits
0xdc03 added a comment. Okay, I have now modified the diagnostic to look like this: ../bug/ifunc-#59164.cpp:17:16: error: ifunc must point to a defined function __attribute__((ifunc("resolver"))) ^ ../bug/ifunc-#59164.cpp:17:16: note: the function specified in an ifunc must

[PATCH] D143803: [clang][alias|ifunc]: Add a diagnostic for mangled names

2023-03-06 Thread Erich Keane via Phabricator via cfe-commits
erichkeane accepted this revision. erichkeane added a comment. This revision is now accepted and ready to land. Not thrilled about 'mangled name' still, but I can't think of anything better after all this time, so LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://

[PATCH] D143803: [clang][alias|ifunc]: Add a diagnostic for mangled names

2023-03-06 Thread Dhruv Chawla via Phabricator via cfe-commits
0xdc03 updated this revision to Diff 502660. 0xdc03 added a comment. - Reword the diagnostic messages - Refactor tests to match updated diagnostic Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143803/new/ https://reviews.llvm.org/D143803 Files:

[PATCH] D143803: [clang][alias|ifunc]: Add a diagnostic for mangled names

2023-02-27 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment. I think "mangled name" is probably the closest without being overly verbose for the average user. We do use "mangled name" in a few places already, some which look not too unrelated to this one: clang/include/clang/Basic/DiagnosticSemaKinds.td: "mangled name of %0

[PATCH] D143803: [clang][alias|ifunc]: Add a diagnostic for mangled names

2023-02-27 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a subscriber: shafik. erichkeane added a comment. In D143803#4155345 , @0xdc03 wrote: > In D143803#4155266 , @erichkeane > wrote: > >> I think updating that test with this additional note is the

[PATCH] D143803: [clang][alias|ifunc]: Add a diagnostic for mangled names

2023-02-27 Thread Dhruv Chawla via Phabricator via cfe-commits
0xdc03 added a comment. In D143803#4155266 , @erichkeane wrote: > I think updating that test with this additional note is the right thing to do. Will do, will also add checks for the fix-its. > As far as that note, saying 'mangled name' is perhaps not

[PATCH] D143803: [clang][alias|ifunc]: Add a diagnostic for mangled names

2023-02-27 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment. In D143803#4154736 , @0xdc03 wrote: > Okay, I have now modified the diagnostic to look something like (as per the > discussion at > https://discord.com/channels/636084430946959380/636732781086638081/1079356357024694363): > >

[PATCH] D143803: [clang][alias|ifunc]: Add a diagnostic for mangled names

2023-02-27 Thread Dhruv Chawla via Phabricator via cfe-commits
0xdc03 added a comment. Okay, I have now modified the diagnostic to look something like (as per the discussion at https://discord.com/channels/636084430946959380/636732781086638081/1079356357024694363): ../../bug/ifunc-#59164.cpp:17:16: error: ifunc must point to a defined function __attri

[PATCH] D143803: [clang][alias|ifunc]: Add a diagnostic for mangled names

2023-02-27 Thread Dhruv Chawla via Phabricator via cfe-commits
0xdc03 updated this revision to Diff 500715. 0xdc03 edited the summary of this revision. 0xdc03 added a comment. - Update to address reviewer comments - Add release note - Redo notes to match reviewer comments - Make the fix-it highlight the whole attribute - Update tests to check new diagnostics

[PATCH] D143803: [clang][alias|ifunc]: Add a diagnostic for mangled names

2023-02-24 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang/lib/CodeGen/CodeGenModule.cpp:359-360 +if (ND->getName() == GV->getName()) { + Diags.Report(Location, diag::note_alias_requires_mangled_name) + << GV->getName() << Name; +}

[PATCH] D143803: [clang][alias|ifunc]: Add a diagnostic for mangled names

2023-02-24 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added inline comments. Comment at: clang/lib/CodeGen/CodeGenModule.cpp:359-360 +if (ND->getName() == GV->getName()) { + Diags.Report(Location, diag::note_alias_requires_mangled_name) + << GV->getName() << Name; +} ---

[PATCH] D143803: [clang][alias|ifunc]: Add a diagnostic for mangled names

2023-02-24 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D143803#4150624 , @0xdc03 wrote: > In D143803#4150423 , @aaron.ballman > wrote: > >> Btw, these changes should come with a release note as well. > > Are all functional changes lo

[PATCH] D143803: [clang][alias|ifunc]: Add a diagnostic for mangled names

2023-02-24 Thread Dhruv Chawla via Phabricator via cfe-commits
0xdc03 added a comment. I will try to come up with a better diagnostic for this issue and will try and update the patch as soon as I can. In D143803#4150423 , @aaron.ballman wrote: > Btw, these changes should come with a release note as well. Are all

[PATCH] D143803: [clang][alias|ifunc]: Add a diagnostic for mangled names

2023-02-24 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment. >> which confuses me because an extern "C" block is not supposed to mangle any >> names, right? Appreciate any inputs on this. That IS strange, that has internal linkage, but so the 'extern "C"' doesn't do anything to it, so we choose to just mangle it. I guess ther

[PATCH] D143803: [clang][alias|ifunc]: Add a diagnostic for mangled names

2023-02-24 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. Btw, these changes should come with a release note as well. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143803/new/ https://reviews.llvm.org/D143803 ___ cfe-commits maili

[PATCH] D143803: [clang][alias|ifunc]: Add a diagnostic for mangled names

2023-02-24 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a reviewer: erichkeane. aaron.ballman added a comment. Adding Erich as he's more familiar with ifunc and friends. Comment at: clang/lib/CodeGen/CodeGenModule.cpp:345 + const llvm::GlobalValue *&GV, +

[PATCH] D143803: [clang][alias|ifunc]: Add a diagnostic for mangled names

2023-02-18 Thread Dhruv Chawla via Phabricator via cfe-commits
0xdc03 added a comment. nudge! at anyone who knows what to do here Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143803/new/ https://reviews.llvm.org/D143803 ___ cfe-commits mailing list cfe-commits@list

[PATCH] D143803: [clang][alias|ifunc]: Add a diagnostic for mangled names

2023-02-13 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment. In D143803#412 , @0xdc03 wrote: > Note that as it stands currently, this patch cannot be committed because the > test `clang/test/SemaCXX/externc-ifunc-resolver.cpp` fails to run. The > contents of the test are as follows:

[PATCH] D143803: [clang][alias|ifunc]: Add a diagnostic for mangled names

2023-02-10 Thread Dhruv Chawla via Phabricator via cfe-commits
0xdc03 added a comment. Note that as it stands currently, this patch cannot be committed because the test `clang/test/SemaCXX/externc-ifunc-resolver.cpp` fails to run. The contents of the test are as follows: // RUN: %clang_cc1 -emit-llvm-only -triple x86_64-linux-gnu -verify %s extern "

[PATCH] D143803: [clang][alias|ifunc]: Add a diagnostic for mangled names

2023-02-10 Thread Dhruv Chawla via Phabricator via cfe-commits
0xdc03 created this revision. Herald added a subscriber: jeroen.dobbelaere. Herald added a project: All. 0xdc03 requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. When an alias or ifunc attribute refers to a function name that is mangled, a di