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

jhuber6 abandoned this revision. jhuber6 added a comment. In 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

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

jdoerfert requested changes to this revision. jdoerfert added a comment. This revision now requires changes to proceed. 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

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

ivanrodriguez3753 added a comment. In 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

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

jhuber6 added a comment. In D131639#3749582 , @ivanrodriguez3753 wrote: > They are defining their own `bool`, which aliases to the built-in `_Bool` > (which is reserved, as you noted with `_[A-Z]`). I thought `bool` was fair > game unless they included

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

ivanrodriguez3753 added a comment. They are defining their own `bool`, which aliases to the built-in `_Bool` (which is reserved, as you noted with `_[A-Z]`). I thought `bool` was fair game unless they included `stdbool.h`? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION http

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

jhuber6 added a comment. In D131639#3749563 , @ivanrodriguez3753 wrote: > The user didn't define any `__` or `_[A-Z]` identifiers, though? Am I misunderstanding the test input? /* Visual Studio < 2013 does not have stdbool.h so here it is a replaceme

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

ivanrodriguez3753 added a comment. The user didn't define any `__` or `_[A-Z]` identifiers, though? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131639/new/ https://reviews.llvm.org/D131639 ___ cfe-comm

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

jhuber6 added a comment. In D131639#3749408 , @ivanrodriguez3753 wrote: > This looks good to me, and I agree we should document what this is fixing. > Any update on if/when this will land? > > In my opinion, there's nothing broken about the user code (d

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

ivanrodriguez3753 added a comment. This looks good to me, and I agree we should document what this is fixing. Any update on if/when this will land? In my opinion, there's nothing broken about the user code (definitely contrived, though). They didn't ask for `stdbool.h` so there should not be a

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

JonChesterfield added a comment. We should write down (maybe in the commit message) what this is fixing. The linked bug has someone defining bool as a workaround for msvc which shouldn't be applied when not compiling with msvc, so superficially it looks like this patch works around broken appli

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

tianshilei1992 added a comment. I'm fine with the change. @JonChesterfield WDYT? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131639/new/ https://reviews.llvm.org/D131639 ___ cfe-commits mailing list cf

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

jhuber6 updated this revision to Diff 451875. jhuber6 added a comment. Changing bool usage to use `_Bool` if compiling for C so we don't need to include the extra header for this single declaration. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131

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

tianshilei1992 added a comment. `bool` is used by a function declaration `__ockl_fdot2` in `clang/lib/Headers/__clang_hip_libdevice_declares.h`, which is included by `clang/lib/Headers/openmp_wrappers/__clang_openmp_device_functions.h`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

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

jhuber6 created this revision. jhuber6 added reviewers: jdoerfert, tianshilei1992, JonChesterfield. Herald added subscribers: guansong, yaxunl. Herald added a project: All. jhuber6 requested review of this revision. Herald added subscribers: cfe-commits, sstefan1. Herald added a project: clang. Th