[PATCH] D148953: [llvm][ADT] Fix Any linker error with multiple compilers

[David Blaikie via Phabricator via llvm-commits]

dblaikie added a comment.

In D148953#4295013 <https://reviews.llvm.org/D148953#4295013>, @sebastian-ne wrote:

> For future reference, this patch was ported to mingw here: https://github.com/msys2/MINGW-packages/pull/16855
>
> In D148953#4293674 <https://reviews.llvm.org/D148953#4293674>, @dblaikie wrote:
>
>> In D148953#4291600 <https://reviews.llvm.org/D148953#4291600>, @sebastian-ne wrote:
>>
>>> Thanks for the quote from wikipedia!
>>>
>>>> Would this not be an ABI bug in those compilers that needs to be fixed there?
>>>
>>> One could stand on the point that if objects from different compilers are linked, the user needs to supply `-f[no-]zero-initialized-in-bss` to make sure they behave the same.
>>> As this instance of such behavior is easy enough to fix in code, I think we can do that in LLVM.
>>
>> I don't think we should be working around ABI mismatches in compilers - the longer-term solution should be that we only use ABI compatible compilers, or use compilers in ABI-compatible ways (with the right flags, if that's what's required).
>>
>> What are the specifically incompatible compilers/versions/flags?
>
> From the ticket, it seems to be the mingw clang version and mingw gcc where the problem occurs.
> At the time of writing this comment, mingw-w64-gcc is at version 12.2.0 and mingw-w64-clang is at 16.0.1.

Is there/could you file bugs on one or both of these compilers to track/address this issue (& if we're going to keep this patch - link to the bug(s) from the code)

Oh, that made me look - this commit includes a link to the bug it's fixing in the source. We don't usually do that. The bug should be linked from the commit message (which it is, thanks!) which can be found via version control, but generally not linked in the source itself (too noisy if we put bug links in the source all the time - conversely, if the code is there to /workaround/ a bug, linking that bug makes sense as we'll want to check on it repeatedly to see if the bug is addressed yet so we can remove the workaround).

>> I don't think we should be working around ABI mismatches in compilers
>
> Sure, I agree. The reason I still made this patch, is because it is only a one character change from a zero to a one :)

Sure, but the explanation/ongoing cost of people trying to understand it/read the comment/etc is more significant than the one character change...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148953