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

Marco Antognini via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Aug 21 05:55:37 PDT 2019


mantognini added a comment.

I think this looks good. Maybe the tests should be extended to test `auto` as function return type, and if there's some special handling around `decltype(auto)`, then it should be tested too, but I'm not sure it's actually needed here. What do you think?



================
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:
----------------
Shouldn't the parentheses around `IsAutoPointee` be removed for style consistency?


================
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:
> 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.)


================
Comment at: lib/Sema/TreeTransform.h:4550
+    Pointee = Pointee->getPointeeType();
+  }  while (!Pointee.isNull());
+  if (!IsAuto && PointeeType.getAddressSpace() == LangAS::Default)
----------------
Nitpicking: there are two spaces between `}` and `while`.


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

https://reviews.llvm.org/D65744





More information about the cfe-commits mailing list