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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D143803#4150624 <https://reviews.llvm.org/D143803#4150624>, @0xdc03 wrote:

> In D143803#4150423 <https://reviews.llvm.org/D143803#4150423>, @aaron.ballman wrote:
>
>> Btw, these changes should come with a release note as well.
>
> Are all functional changes logged in the release notes? I am not really aware of how they work.

Not quite all, but somewhat close to it -- if the changes are "user facing", we usually try to write a release note for it: https://llvm.org/docs/DeveloperPolicy.html#release-notes



================
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:345
+                               const llvm::GlobalValue *&GV,
+                               const llvm::MapVector<GlobalDecl, StringRef> &MangledDeclNames) {
   GV = getAliasedGlobal(Alias);
----------------
0xdc03 wrote:
> aaron.ballman wrote:
> > Does clang-format do this? It looks far beyond the usual 80 col limit we use.
> It is beyond the 80 column limit because I formatted it by hand. The linting part of `arc diff` did not complain so it seemed fine to me. I suppose I was supposed to use `git clang-format`, I was not aware of it. I will use that command from now on.
I'd recommend using the patch formatting script: https://clang.llvm.org/docs/ClangFormat.html#script-for-patch-reformatting


================
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;
+        }
----------------
0xdc03 wrote:
> aaron.ballman wrote:
> > Should this come with a fix-it to switch the attribute to using the mangled name instead?
> I did consider it, however I have no idea how to implement that 😅 
We have some documentation on that: https://clang.llvm.org/docs/InternalsManual.html#fix-it-hints

The basic idea here is that you'd create a replacement range so you can replace the old name with the new one.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D143803/new/

https://reviews.llvm.org/D143803