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

Mon Feb 27 07:52:53 PST 2023

erichkeane added a subscriber: shafik.
erichkeane added a comment.

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

> In D143803#4155266 <https://reviews.llvm.org/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 correct there, since what we really care is that it is the name-as-emitted to the linker. I don't have an idea on exactly what to call it.  If we could come up with a better phrase, it probably makes the diagnostic on the thing not being a function.
>
> First few that come to mind:
>
> - ABI-mangled symbol
> - Linked symbol
> - Externally-visible name

I don't think any of those are accurate with internal names.  ABI-mangled is incorrect as the ABI says nothing about these.  "Linked Symbol" isnt really accurate, as these aren't really 'linked'.  AND, they aren't externally-visible.  I wish I was better at this part :D  Perhaps @shafik might have a better ability to bikeshed here?

> It may also be worth splitting the diagnostic for `ifunc`s and `alias`es to be something like:
>
> - `ifunc`: the function specified in an `ifunc` must refer to its <mangled name>
> - `alias`: the function or variable specified in an `alias` must refer to its <mangled name>
>
> I feel externally-visible name fits the best, though it doesn't really make sense with internal linkage.

>> ALSO, it would be worth updating the error here to mention that it must point to a defined function, <x, y, Z> (where x,y,z are the things that it is allowed to target).
>
> Hmm, I do not really understand this point, could you please clarify it?

Exactly what you were saying above with the 'splitting the diagnostic', I was saying in less accurate words, that I'd prefer that.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143803