[PATCH] D55127: [OpenCL] Diagnose conflicting address spaces between template definition and its instantiation

John McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Dec 4 10:49:17 PST 2018


rjmccall accepted this revision.
rjmccall added a comment.
This revision is now accepted and ready to land.

LGTM with minor revisions.



================
Comment at: lib/Sema/SemaType.cpp:7206
+      // Do not deduce address spaces in templates.
+      T->isDependentType())
     return;
----------------
I think a better comment here would be something like "Don't deduce address spaces for dependent types because they might end up instantiating to a type with a different address space qualifier."


================
Comment at: lib/Sema/SemaType.cpp:7232
+      if (D.getContext() == DeclaratorContext::TemplateArgContext)
+        // Do not deduce address space for non-pointee type in template arg.
+        ;
----------------
Anastasia wrote:
> rjmccall wrote:
> > I don't understand what you're trying to do here.  Template arguments may need to be directly qualified with an address-space sometimes, and if you prevent that unconditionally you're going to break all sorts of things, like partially-specializing a template based on the presence of an address-space qualifier.
> I want to prevent deduction of address spaces for template arguments (when they are not specified explicitly).
> 
> Without this change, this example won't compile:
>   template <typename T>
>   void foo() {
>     static __global T i;
>   }
>   foo<int>(); // error because int here is deduced to __private int (so i will have conflicting addr space quals)
> But I think it's perfectly reasonable to compile this example because the addr space qual of `i` is specified to be `__global`.
> 
> Basically I am just trying to fix OpenCL C deduction rules that didn't account for the logic of templates.
> 
Okay.  Please use braces instead of a `;`.


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

https://reviews.llvm.org/D55127





More information about the cfe-commits mailing list