[PATCH] D52301: [clang] Set TypeSourceInfo for vardecl's in addition to type when we can deduce.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

A few more comments, mostly marking places of unintentional changes that we need to revert.
Hope it's not going past the point where the number of comments are not useful.



================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:8616
   "use of %select{type|declaration}0 %1 requires %2 extension to be enabled">;
-def warn_opencl_generic_address_space_arg : Warning<
-  "passing non-generic address space pointer to %0"
----------------
This looks unintentional. Should we revert?


================
Comment at: lib/Sema/SemaChecking.cpp:852
 
-  if (RT->getPointeeType().getAddressSpace() != LangAS::opencl_generic) {
-    S.Diag(Call->getArg(0)->getBeginLoc(),
----------------
aaron.ballman wrote:
> kadircet wrote:
> > aaron.ballman wrote:
> > > I'm not certain I understand why this code has been removed?
> > It shouldn't have been, tried rebasing but it didn't go away. I think it was deleted at some point by a different change and somehow ended up showing in here as well. (Tried to revert, got an error stating warn_opencl_generic_address_space_arg doesn't exist)
> That's truly strange. I bring it up because these sort of changes make it harder for reviewers to test changes locally by applying the patch themselves.
+1, we definitely need to revert this, happy to help figure out why this happened.


================
Comment at: lib/Sema/SemaTemplateDeduction.cpp:4348
       TLB.reserve(TL.getFullDataSize());
-      return TransformType(TLB, TL);
+      QualType Result = TransformType(TLB, TL);
+      return TLB.getTypeSourceInfo(getSema().Context, Result);
----------------
Maybe remove the intermediate variable?


================
Comment at: lib/Tooling/Refactoring/Rename/USRLocFinder.cpp:354
   bool VisitTypeLoc(TypeLoc Loc) {
+    if(Loc.getTypeLocClass() == TypeLoc::Auto)
+      return true;
----------------
Why do we need this change? Are we fixing some particular regressions?
A comment would be useful.


================
Comment at: test/SemaOpenCL/to_addr_builtin.cl:2
 // RUN: %clang_cc1 -verify -fsyntax-only %s
-// RUN: %clang_cc1 -Wconversion -verify -fsyntax-only -cl-std=CL2.0 %s
 
----------------
Unrelated change.


================
Comment at: test/SemaOpenCL/to_addr_builtin.cl:48
   // expected-error at -4{{assigning '__global int *' to '__local int *' changes address space of pointer}}
-  // expected-warning at -5{{passing non-generic address space pointer to to_global may cause dynamic conversion affecting performance}}
 #endif
----------------
Unrelated change


================
Comment at: test/SemaOpenCL/to_addr_builtin.cl:122
   // expected-warning at -4{{incompatible pointer types initializing '__global char *' with an expression of type '__global int *'}}
-  // expected-warning at -5{{passing non-generic address space pointer to to_global may cause dynamic conversion affecting performance}}
 #endif
----------------
Another unrelated change


Repository:
  rC Clang

https://reviews.llvm.org/D52301