bader added inline comments.
================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:8263
+def err_atomic_init_addressspace : Error<
+ "initialization of atomic variables is restricted to variables in global
address space">;
def err_atomic_init_constant : Error<
----------------
bader wrote:
> Anastasia wrote:
> > echuraev wrote:
> > > Anastasia wrote:
> > > > Could we combine this error diag with the one below? I guess they are
> > > > semantically very similar apart from one is about initialization and
> > > > another is about assignment?
> > > I'm not sure that it is a good idea to combine these errors. For example,
> > > if developer had declared a variable non-constant and not in global
> > > address space he would have got the same message for both errors. And it
> > > can be difficult to determine what the exact problem is. He can fix one
> > > of the problems but he will still get the same error.
> > Well, I don't actually see that we check for constant anywhere so it's also
> > OK if you want to drop this bit. Although I think the original intension of
> > this message as I understood was to provide the most complete hint.
> >
> > My concern is that these two errors seem to be reporting nearly the same
> > issue and ideally we would like to keep diagnostic list as small as
> > possible. This also makes the file more concise and messages more
> > consistent.
> I suggest adding a test case with non-constant initialization case to
> validate that existing checks cover this case for atomic types already.
> If so, we can adjust existing diagnostic message to cover both cases:
> initialization and assignment expression.
I don't think it's quite true.
There are two requirements here that must be met the same time. Atomic
variables *declared in the global address space* can be initialized only with
"compile time constant'.
If understand the spec correctly this code is also valid:
kernel void foo() {
static global atomic_int a = 42; // although it's not clear if we must use
ATOMIC_VAR_INIT here.
...
}
================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:8267
"variable in constant address space must be initialized">;
-def err_atomic_init_constant : Error<
- "atomic variable can only be assigned to a compile time constant"
- " in the declaration statement in the program scope">;
+// Atomics
+def err_atomic_init: Error<
----------------
I suggest removing this comment.
If you are going to add other diagnostic messages specific to OpenCL atomics,
then separate them from this list of unordered diagnostics similar to pipe
built-in functions below.
https://reviews.llvm.org/D30643
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
