[PATCH] D65744: [PR42707][OpenCL] Fix addr space deduction for auto

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.


================
Comment at: clang/lib/Sema/SemaDecl.cpp:6721
 
+  if (getLangOpts().OpenCL) {
+
----------------
rjmccall wrote:
> Anastasia wrote:
> > rjmccall wrote:
> > > Since you're moving this code anyway, can this be split into its own function?  I'm not sure if it's actually important that some of these failures return immediately and some of them fall through to later checks.
> > >  I'm not sure if it's actually important that some of these failures return immediately and some of them fall through to later checks.
> > 
> > Yes, it looks a bit random. Do we have any guideline whether we should return or keep diagnosing as long as possible?
> > 
> > 
> If the user is likely to have made multiple independent errors, it's good to diagnose as many of them as possible.  But if it's just as likely that the user messed up in a single way that should've meant we didn't take this code path, then it's better to bail out early.
> 
> In this case, most of these diagnostics are looking for different special OpenCL types, and those are probably all independent  of each other.
> 
> ...it does occur to me to wonder if more of these checks should be looking through array types the way that the check for `half` does.  Presumably you shouldn't be able to declare global arrays of images or pipes if you can't declare non-array variables of them.
> ...it does occur to me to wonder if more of these checks should be looking through array types the way that the check for half does. Presumably you shouldn't be able to declare global arrays of images or pipes if you can't declare non-array variables of them.

We actually provide dedicated diagnostic for all other types in `Sema::BuildArrayType`. No idea why half is handled here I will try to refactor that in a separate patch. 


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

https://reviews.llvm.org/D65744