[PATCH] D54862: [OpenCL] Add generic AS to 'this' pointer

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.


================
Comment at: lib/Sema/SemaType.cpp:3929
+static TypeSourceInfo *
+GetFullTypeForDeclarator(TypeProcessingState &state, QualType declSpecType,
+                         TypeSourceInfo *TInfo,
----------------
>From the `state` you can use `getDeclarator()` that then call a method `getContext()`.


================
Comment at: lib/Sema/SemaType.cpp:5169
+TypeSourceInfo *Sema::GetTypeForDeclarator(Declarator &D, Scope *S,
+                                           const DeclContext *DC) {
   // Determine the type of the declarator. Not all forms of declarator
----------------
Rather than passing `DeclContext` you can get it from the `Declarator` using `getContext()` method.


================
Comment at: lib/Sema/TreeTransform.h:4277
   //   cv-qualifiers are ignored.
-  if (T->isFunctionType())
+  // OpenCL: The address space should not be ignored.
+  if (T->isFunctionType()) {
----------------
mikael wrote:
> Another place where the address space was removed.
Why OpenCL in the comment, since the code seems to be rather generic.

Also not sure this comments adds anything that can't be understood from the code you are adding.


================
Comment at: test/SemaOpenCLCXX/address-space-templates.cl:7
   T f1();     // expected-error{{function type may not be qualified with an address space}}
-  void f2(T); // expected-error{{parameter may not be qualified with an address space}}
+  // FIXME: Should only get the error message once.
+  void f2(T); // expected-error{{parameter may not be qualified with an address space}} expected-error{{parameter may not be qualified with an address space}}
----------------
mikael wrote:
> This was the remaining issue that I have not solved yet. It looked like this issue was not so trivial so I think it makes sense to postpone it.
Do you know why it appears twice?


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

https://reviews.llvm.org/D54862