Anastasia added a comment. In D119560#3322582 <https://reviews.llvm.org/D119560#3322582>, @svenvh wrote:
> In D119560#3322531 <https://reviews.llvm.org/D119560#3322531>, @Anastasia > wrote: > >>> also makes the header no longer "claim" the identifiers "success", >>> "failure", "desired", "value" (such that you can compile with -Dvalue=... >>> when including the header for example, which currently breaks parsing >>> of the header). >> >> I don't get what you mean by this. :) > > Compiling a CL source file with e.g. `clang -cl-std=CL2.0 -Xclang > -finclude-default-header -cl-no-stdinc -Dvalue=1 > clang/test/CodeGenOpenCL/as_type.cl` gives lots of errors such as the > following, because defining `value` as a macro (which is not a reserved > identifier as far as I'm aware) collides with the argument names in the > header: > > In file included from <built-in>:1: > lib/clang/15.0.0/include/opencl-c.h:13277:58: error: expected ')' > void __ovld atomic_init(volatile atomic_int *object, int value); > ^ > <command line>:1:15: note: expanded from here > #define value 1 Oh, I see. Thanks! This is bad indeed!!! >>> This is a big patch and it only touches the OpenCL 2 atomics for now. I >>> wonder if we should remove argument names from the other builtins too? >>> I think it would help unifying the header and tablegen approaches: if we >>> gradually move the header into some canonical form, we might be able >>> to eventually replace it with a tablegen-ed header, while being able to >>> easily confirm equivalence. >> >> The only drawback I see if that we will lose the history a bit in "git >> blame" but: > > Slight nuance: we will not lose any history, but I understand your concern: > someone needs to look through this commit to see the previous commit that > touched the code. Yeah indeed, we don't lose it, but just make it a bit more complicated to look up. > If there are no objections to removing all argument names from the header, > I'll try to prepare patches for doing so. No, it seems rather good to do since it is fixing a bug with the preprocessor macros that you have highlighted. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119560/new/ https://reviews.llvm.org/D119560 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
