[PATCH] D140726: lld: CHECK->LLD_CHECK to reduce chance of conflict with other libraries

Wed Dec 28 10:44:08 PST 2022

dblaikie added a comment.

In D140726#4018580 <https://reviews.llvm.org/D140726#4018580>, @chandlerc wrote:

> In D140726#4018463 <https://reviews.llvm.org/D140726#4018463>, @dblaikie wrote:
>
>> In D140726#4018459 <https://reviews.llvm.org/D140726#4018459>, @thakis wrote:
>>
>>> This is only used in lld cpp files, and lld's header files aren't meant to be used outside it. So halide is doing something unsupported.
>>
>> I don't think Halide's doing something wrong here - it's including `lld/Common/Driver.h` which states:
>>
>>   // Generic entry point when using LLD as a library, safe for re-entry, supports
>>   // crash recovery.
>>
>> And that includes `lld/Common/CommonLinkerContext.h` which includes `lld/Common/ErrorHandler.h`
>>
>>> Could halide undef this macro after including the lld header and before including absl headers instead?
>>
>> Perhaps? I guess alternatively `lld/Common/Driver.h` could undef it at the end, but including any other lld header would risk breakage (since they might expect the macro to still be defined) - would probably make implementing that header tricky - needing to redefine that macro back again in the implementation file, etc.
>
> Not sure this would help. If the colliding header were included *before* LLD's, this would still break.

Ah, yeah, fair point.

> I don't think it's realistic for any code that intends to be usable as a library with other code to be defining an un-prefixed macro. (That goes for *both* of the colliding cases here to be clear, not just LLD.) Not sure why this would be controversial.

Yeah - guess there's minimal chance of ABSL fixing theirs given the wide usage, at least inside Google? (then again, wide scale changes are a thing) - not that that would remove the value in fixing LLD too.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140726