Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
clang/test/SemaOpenCL/storageclass.cl | ||
---|---|---|
71 | Not sure if this is the best way to restructure this test? Now the diagnostics are quite far away from the declarations, and it is harder to see which diagnostic belongs to which declaration. I wonder if the following would be a better structure instead: int G3 = 0; #ifndef __opencl_c_program_scope_global_variables // expected-error@-2 {{program scope variable must reside in constant address space}} #endif global int G4 = 0; #ifndef __opencl_c_program_scope_global_variables // expected-error@-2 {{program scope variable must reside in constant address space}} #endif ... etc. ... The downside is that it is a bit more verbose due to the repetition of the #if guards, but it makes the test more readable/maintainable in my opinion. |
clang/test/SemaOpenCL/storageclass.cl | ||
---|---|---|
71 | Yeah, that's reasonable. I'll change that in the following patch. Thanks. |
LGTM! Thanks!
clang/include/clang/Basic/OpenCLOptions.h | ||
---|---|---|
83 | While it is not critical, I would slightly prefer that we are checking for == 300. Checking >= 300 assumes that the next versions are going to be backward compatible. But it is not always the case if we look at the OpenCL evolution. |
Fixed latest comment about checking for == 300. Btw, there already exists a few places where >= 300 is checked. Should fix them as well.
While it is not critical, I would slightly prefer that we are checking for == 300. Checking >= 300 assumes that the next versions are going to be backward compatible. But it is not always the case if we look at the OpenCL evolution.