[PATCH] D131639: [OpenMP] Remove 'stdbool.h' from OpenMP header wrappers

Mon Aug 29 09:45:08 PDT 2022

jhuber6 abandoned this revision.
jhuber6 added a comment.

In D131639#3755866 <https://reviews.llvm.org/D131639#3755866>, @jdoerfert wrote:

> I think the code as is upstream is fine. The test input is problematic. There is no guarantee, or even any argument, that stdbool is not included by the compiler or any header (system or not). If the user writes conflicting cod with the system, that calls for problems down the line.
>
> Long story short, I'd abandon this for now.

I agree, I will abandon this revision and close the associated bug.

In D131639#3755699 <https://reviews.llvm.org/D131639#3755699>, @ivanrodriguez3753 wrote:

> In D131639#3749633 <https://reviews.llvm.org/D131639#3749633>, @jhuber6 wrote:
>
>> I think it's perfectly reasonable to include system files as part of a toolchain.
>
> I think it comes down to a matter of inconveniencing the user versus the developer. We usually go with the latter. I don't particularly agree with your statement I quoted above, but I can't say I'm an expert in this area. I'll see what others have to say. I've already pulled this into our fork so it doesn't make a difference to us, but I did think it would be useful to get input from the community on whether or not we should pull this in to upstream.
>
> Thanks for your input and responses.

I'm glad this helped you fix it on your end, thanks for opening an issue for this.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131639