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

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia marked 2 inline comments as done.
Anastasia added inline comments.


================
Comment at: clang/lib/Sema/SemaDecl.cpp:5998
+    QualType Type = Var->getType();
+    if (Type->isSamplerT() || Type->isVoidType())
+      return;
----------------
I don't seem to need a check for dependent or auto types because the substitution happens using type info rather than getting type from the declaration. Not sure if I should explain it here or add the checks just in case?


================
Comment at: clang/lib/Sema/SemaType.cpp:7460
+      // the initializing expression type during the type deduction.
+      (T->isUndeducedAutoType() && IsPointee) ||
       // OpenCL spec v2.0 s6.9.b:
----------------
rjmccall wrote:
> Okay, I understand why you're doing this now, and it makes sense.  I would like to propose changing the entire way `deduceOpenCLImplicitAddrSpace` works.  Why don't we do it more like how ObjC ARC infers its implicit ownership qualifiers:
> 
> - In SemaType, we add the default address space to non-qualified, non-dependent, non-undeduced-`auto` pointees when parsing a pointer or reference type.
> - In SemaType, we add the default address space to non-qualified pointees when building a pointer or reference type.
> - We add the default address space at the top level when when building a variable.
> 
> Then all of this context-specific logic where we're looking at different declarator chunks and trying to infer the relationship of the current chunk to the overall type being parsed can just go away or get pushed into a more appropriate position.
Ok, it mainly works, however I still need a bit of parsing horribleness when deducing addr space of declarations with parenthesis  in `GetFullTypeForDeclarator`. This is the case for block pointers or pointers/references to arrays. It is incorrect to add address spaces on ParenType while building a pointer or references so I have to detect this as special case.


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

https://reviews.llvm.org/D65744