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

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia marked an inline comment as done.
Anastasia added a comment.

missing test case

  1 auto* accumulator()
  2 {
  3   __global int * glptr;
  4   return glptr;
  5 }
  6 int i;
  7 auto* ai = &i;
  8 int* ii = &i;

In D65744#1646348 <https://reviews.llvm.org/D65744#1646348>, @rjmccall wrote:

> In D65744#1629055 <https://reviews.llvm.org/D65744#1629055>, @Anastasia wrote:
>
> > In D65744#1627589 <https://reviews.llvm.org/D65744#1627589>, @rjmccall wrote:
> >
> > > I see.  Is the deduction rule recursive or something, where a pointer to pointer is inferred to point to the same address space as the pointee?
> >
> >
> > It is recursive indeed and we currently deduce all pointees to generic AS.
>
>
> Is that likely to change?  If all pointees are inferred to be generic (in the absence of an explicit qualifier) when building a pointer type, this seems very over-thought vs. just adding the generic AS whenever you build a pointer to an unqualified but non-dependent type.


I don't think this is likely to change. Are you suggesting to move the deduction logic for pointee of pointers, references and block pointers into ASTContext helper that creates a pointer/reference/block pointer type? I guess it should work if the only way to construct the pointer is by using this helper. I can certainly prototype this change and see.



================
Comment at: lib/Sema/SemaType.cpp:7441
+      // the initializing expression type during the type deduction.
+      (T->isAutoType() && IsPointee) || (IsAutoPointee) ||
       // OpenCL spec v2.0 s6.9.b:
----------------
mantognini wrote:
> mantognini wrote:
> > Shouldn't the parentheses around `IsAutoPointee` be removed for style consistency?
> With the `if` statement introduced above, `IsAutoPointee` can be true only in C++ mode. Could it be an issue to not guard `(T->isAutoType() && IsPointee)` for non-C++ mode? (I guess not, but I couldn't convince myself.)
I think `TreeTransforms` will only be used in C++ mode. But also `isAutoType` should only be true in C++ mode. So I think we should be fine.


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

https://reviews.llvm.org/D65744