[PATCH] D68143: [clang] Make handling of unnamed template params similar to function params

2019-10-01 Thread Kadir Cetinkaya via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rL373340: [clang] Make handling of unnamed template params 
similar to function params (authored by kadircet, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D68143?vs=222581=222604#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D68143

Files:
  cfe/trunk/lib/AST/DeclTemplate.cpp
  cfe/trunk/lib/Parse/ParseTemplate.cpp
  cfe/trunk/lib/Sema/SemaTemplate.cpp
  cfe/trunk/test/AST/ast-dump-decl.cpp
  cfe/trunk/test/AST/ast-dump-record-definition-data-json.cpp
  cfe/trunk/test/AST/ast-dump-template-decls-json.cpp
  cfe/trunk/test/AST/ast-dump-template-decls.cpp
  cfe/trunk/test/ASTMerge/class-template/test.cpp
  cfe/trunk/test/Index/index-templates.cpp

Index: cfe/trunk/lib/Parse/ParseTemplate.cpp
===
--- cfe/trunk/lib/Parse/ParseTemplate.cpp
+++ cfe/trunk/lib/Parse/ParseTemplate.cpp
@@ -630,11 +630,11 @@
   }
 
   // Grab the template parameter name (if given)
-  SourceLocation NameLoc;
+  SourceLocation NameLoc = Tok.getLocation();
   IdentifierInfo *ParamName = nullptr;
   if (Tok.is(tok::identifier)) {
 ParamName = Tok.getIdentifierInfo();
-NameLoc = ConsumeToken();
+ConsumeToken();
   } else if (Tok.isOneOf(tok::equal, tok::comma, tok::greater,
  tok::greatergreater)) {
 // Unnamed template parameter. Don't have to do anything here, just
@@ -727,11 +727,11 @@
: diag::ext_variadic_templates);
 
   // Get the identifier, if given.
-  SourceLocation NameLoc;
+  SourceLocation NameLoc = Tok.getLocation();
   IdentifierInfo *ParamName = nullptr;
   if (Tok.is(tok::identifier)) {
 ParamName = Tok.getIdentifierInfo();
-NameLoc = ConsumeToken();
+ConsumeToken();
   } else if (Tok.isOneOf(tok::equal, tok::comma, tok::greater,
  tok::greatergreater)) {
 // Unnamed template parameter. Don't have to do anything here, just
Index: cfe/trunk/lib/AST/DeclTemplate.cpp
===
--- cfe/trunk/lib/AST/DeclTemplate.cpp
+++ cfe/trunk/lib/AST/DeclTemplate.cpp
@@ -510,8 +510,12 @@
   if (hasDefaultArgument() && !defaultArgumentWasInherited())
 return SourceRange(getBeginLoc(),
getDefaultArgumentInfo()->getTypeLoc().getEndLoc());
-  else
-return TypeDecl::getSourceRange();
+  // TypeDecl::getSourceRange returns a range containing name location, which is
+  // wrong for unnamed template parameters. e.g:
+  // it will return <[[typename>]] instead of <[[typename]]>
+  else if (getDeclName().isEmpty())
+return SourceRange(getBeginLoc());
+  return TypeDecl::getSourceRange();
 }
 
 unsigned TemplateTypeParmDecl::getDepth() const {
Index: cfe/trunk/lib/Sema/SemaTemplate.cpp
===
--- cfe/trunk/lib/Sema/SemaTemplate.cpp
+++ cfe/trunk/lib/Sema/SemaTemplate.cpp
@@ -1005,15 +1005,10 @@
   assert(S->isTemplateParamScope() &&
  "Template type parameter not in template parameter scope!");
 
-  SourceLocation Loc = ParamNameLoc;
-  if (!ParamName)
-Loc = KeyLoc;
-
   bool IsParameterPack = EllipsisLoc.isValid();
-  TemplateTypeParmDecl *Param
-= TemplateTypeParmDecl::Create(Context, Context.getTranslationUnitDecl(),
-   KeyLoc, Loc, Depth, Position, ParamName,
-   Typename, IsParameterPack);
+  TemplateTypeParmDecl *Param = TemplateTypeParmDecl::Create(
+  Context, Context.getTranslationUnitDecl(), KeyLoc, ParamNameLoc, Depth,
+  Position, ParamName, Typename, IsParameterPack);
   Param->setAccess(AS_public);
 
   if (Param->isParameterPack())
@@ -1044,7 +1039,7 @@
 assert(DefaultTInfo && "expected source information for type");
 
 // Check for unexpanded parameter packs.
-if (DiagnoseUnexpandedParameterPack(Loc, DefaultTInfo,
+if (DiagnoseUnexpandedParameterPack(ParamNameLoc, DefaultTInfo,
 UPPC_DefaultArgument))
   return Param;
 
Index: cfe/trunk/test/ASTMerge/class-template/test.cpp
===
--- cfe/trunk/test/ASTMerge/class-template/test.cpp
+++ cfe/trunk/test/ASTMerge/class-template/test.cpp
@@ -9,13 +9,13 @@
 // CHECK: class-template2.cpp:9:15: note: declared here with type 'long'
 
 // CHECK: class-template1.cpp:12:14: warning: template parameter has different kinds in different translation units
-// CHECK: class-template2.cpp:12:10: note: template parameter declared here
+// CHECK: class-template2.cpp:12:18: note: template parameter declared here
 
 // CHECK: class-template1.cpp:18:23: warning: non-type template parameter declared with incompatible types in different

[PATCH] D68143: [clang] Make handling of unnamed template params similar to function params

2019-10-01 Thread Ilya Biryukov via Phabricator via cfe-commits 
ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.

Many thanks! LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68143



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68143: [clang] Make handling of unnamed template params similar to function params

2019-10-01 Thread Kadir Cetinkaya via Phabricator via cfe-commits 
kadircet updated this revision to Diff 222581.
kadircet added a comment.

- Address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68143

Files:
  clang/lib/AST/DeclTemplate.cpp
  clang/lib/Parse/ParseTemplate.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/test/AST/ast-dump-decl.cpp
  clang/test/AST/ast-dump-record-definition-data-json.cpp
  clang/test/AST/ast-dump-template-decls-json.cpp
  clang/test/AST/ast-dump-template-decls.cpp
  clang/test/ASTMerge/class-template/test.cpp
  clang/test/Index/index-templates.cpp

Index: clang/test/Index/index-templates.cpp
===
--- clang/test/Index/index-templates.cpp
+++ clang/test/Index/index-templates.cpp
@@ -155,7 +155,7 @@
 // CHECK-LOAD: index-templates.cpp:36:44: DeclRefExpr=OneDimension:35:16 Extent=[36:44 - 36:56]
 // CHECK-LOAD: index-templates.cpp:40:8: ClassTemplate=storage:40:8 (Definition) Extent=[39:1 - 40:19]
 // CHECK-LOAD: index-templates.cpp:39:45: TemplateTemplateParameter=DataStructure:39:45 (Definition) Extent=[39:10 - 39:66]
-// CHECK-LOAD: index-templates.cpp:39:19: TemplateTypeParameter=:39:19 (Definition) Extent=[39:19 - 39:27]
+// CHECK-LOAD: index-templates.cpp:39:27: TemplateTypeParameter=:39:27 (Definition) Extent=[39:19 - 39:27]
 // CHECK-LOAD: index-templates.cpp:39:37: NonTypeTemplateParameter=:39:37 (Definition) Extent=[39:29 - 39:37]
 // CHECK-LOAD: index-templates.cpp:39:61: TemplateRef=array:37:8 Extent=[39:61 - 39:66]
 // CHECK-LOAD: index-templates.cpp:42:18: TypedefDecl=Unsigned:42:18 (Definition) Extent=[42:1 - 42:26]
Index: clang/test/ASTMerge/class-template/test.cpp
===
--- clang/test/ASTMerge/class-template/test.cpp
+++ clang/test/ASTMerge/class-template/test.cpp
@@ -9,13 +9,13 @@
 // CHECK: class-template2.cpp:9:15: note: declared here with type 'long'
 
 // CHECK: class-template1.cpp:12:14: warning: template parameter has different kinds in different translation units
-// CHECK: class-template2.cpp:12:10: note: template parameter declared here
+// CHECK: class-template2.cpp:12:18: note: template parameter declared here
 
 // CHECK: class-template1.cpp:18:23: warning: non-type template parameter declared with incompatible types in different translation units ('long' vs. 'int')
 // CHECK: class-template2.cpp:18:23: note: declared here with type 'int'
 
-// CHECK: class-template1.cpp:21:10: warning: template parameter has different kinds in different translation units
-// CHECK: class-template2.cpp:21:10: note: template parameter declared here
+// CHECK: class-template1.cpp:21:18: warning: template parameter has different kinds in different translation units
+// CHECK: class-template2.cpp:21:31: note: template parameter declared here
 
 // CHECK: class-template2.cpp:27:20: warning: external variable 'x0r' declared with incompatible types in different translation units ('X0 *' vs. 'X0 *')
 // CHECK: class-template1.cpp:26:19: note: declared here with type 'X0 *'
Index: clang/test/AST/ast-dump-template-decls.cpp
===
--- clang/test/AST/ast-dump-template-decls.cpp
+++ clang/test/AST/ast-dump-template-decls.cpp
@@ -26,7 +26,7 @@
 // CHECK: FunctionTemplateDecl 0x{{[^ ]*}}  col:6 d
 // CHECK-NEXT: TemplateTypeParmDecl 0x{{[^ ]*}}  col:20 referenced typename depth 0 index 0 Ty
 // CHECK-NEXT: TemplateTemplateParmDecl 0x{{[^ ]*}}  col:52 depth 0 index 1 Uy
-// CHECK-NEXT: TemplateTypeParmDecl 0x{{[^ ]*}}  col:33 typename depth 1 index 0
+// CHECK-NEXT: TemplateTypeParmDecl 0x{{[^ ]*}}  col:41 typename depth 1 index 0
 void d(Ty, Uy);
 
 template 
@@ -47,7 +47,7 @@
 
 template 
 // CHECK: FunctionTemplateDecl 0x{{[^ ]*}}  col:6 h
-// CHECK-NEXT: TemplateTypeParmDecl 0x{{[^ ]*}}  col:11 typename depth 0 index 0
+// CHECK-NEXT: TemplateTypeParmDecl 0x{{[^ ]*}}  col:20 typename depth 0 index 0
 // CHECK-NEXT: TemplateArgument type 'void'
 void h();
 
Index: clang/test/AST/ast-dump-template-decls-json.cpp
===
--- clang/test/AST/ast-dump-template-decls-json.cpp
+++ clang/test/AST/ast-dump-template-decls-json.cpp
@@ -656,8 +656,8 @@
 // CHECK-NEXT:"id": "0x{{.*}}",
 // CHECK-NEXT:"kind": "TemplateTypeParmDecl",
 // CHECK-NEXT:"loc": {
-// CHECK-NEXT: "col": 33,
-// CHECK-NEXT: "tokLen": 8
+// CHECK-NEXT: "col": 41,
+// CHECK-NEXT: "tokLen": 1
 // CHECK-NEXT:},
 // CHECK-NEXT:"range": {
 // CHECK-NEXT: "begin": {
@@ -1099,8 +1099,8 @@
 // CHECK-NEXT:  "kind": "TemplateTypeParmDecl",
 // CHECK-NEXT:  "loc": {
 // CHECK-NEXT:   "line": 27,
-// CHECK-NEXT:   "col": 11,
-// CHECK-NEXT:   "tokLen": 8
+// CHECK-NEXT:   "col": 20,
+// CHECK-NEXT:   "tokLen": 1
 // CHECK-NEXT:  },
 // CHECK-NEXT:

[PATCH] D68143: [clang] Make handling of unnamed template params similar to function params

2019-10-01 Thread Ilya Biryukov via Phabricator via cfe-commits 
ilya-biryukov added inline comments.



Comment at: clang/lib/AST/DeclTemplate.cpp:513
getDefaultArgumentInfo()->getTypeLoc().getEndLoc());
-  else
-return TypeDecl::getSourceRange();
+  else if(getName().empty())
+return SourceRange(getBeginLoc());

kadircet wrote:
> ilya-biryukov wrote:
> > kadircet wrote:
> > > ilya-biryukov wrote:
> > > > Could you provide more details why we need this change?
> > > added comments, you can also see the similar case in 
> > > `DeclaratorDecl::getSourceRange()`
> > Could you provide an example? What the range was before and what is it now?
> > 
> TypeDecl::getSourceRange will also include the next token if template 
> parameter is unnamed for example:
> 
> ```
> template 
>   ~
> ```
> 
> has the following `>` covered inside typerange.
Thanks for clearing this up. Could you mention in the comment why 
`TypeDecl::getSourceRange` is wrong in that case?

From offline discussion, I figured that it should not be pointing to the name 
location in the case of un-named parameters as this would lead to the closing 
angle bracket being included in the range for template parameter, i.e. we'll 
get `<[[class>]]` instead of `<[[class]]>`



Comment at: clang/lib/AST/DeclTemplate.cpp:515
+  // wrong for unnamed template parameters.
+  else if(getName().empty())
+return SourceRange(getBeginLoc());

`getName()` may fail if the name is not an identifier. 
Even though this shouldn't happen for `TypeDecls`, could you please change to 
`getDeclName().isEmpty()`?

It's equivalent and does not have any assertions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68143



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68143: [clang] Make handling of unnamed template params similar to function params

2019-10-01 Thread Kadir Cetinkaya via Phabricator via cfe-commits 
kadircet marked an inline comment as done.
kadircet added inline comments.



Comment at: clang/lib/AST/DeclTemplate.cpp:513
getDefaultArgumentInfo()->getTypeLoc().getEndLoc());
-  else
-return TypeDecl::getSourceRange();
+  else if(getName().empty())
+return SourceRange(getBeginLoc());

ilya-biryukov wrote:
> kadircet wrote:
> > ilya-biryukov wrote:
> > > Could you provide more details why we need this change?
> > added comments, you can also see the similar case in 
> > `DeclaratorDecl::getSourceRange()`
> Could you provide an example? What the range was before and what is it now?
> 
TypeDecl::getSourceRange will also include the next token if template parameter 
is unnamed for example:

```
template 
  ~
```

has the following `>` covered inside typerange.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68143



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68143: [clang] Make handling of unnamed template params similar to function params

2019-10-01 Thread Ilya Biryukov via Phabricator via cfe-commits 
ilya-biryukov added inline comments.



Comment at: clang/lib/AST/DeclTemplate.cpp:513
getDefaultArgumentInfo()->getTypeLoc().getEndLoc());
-  else
-return TypeDecl::getSourceRange();
+  else if(getName().empty())
+return SourceRange(getBeginLoc());

kadircet wrote:
> ilya-biryukov wrote:
> > Could you provide more details why we need this change?
> added comments, you can also see the similar case in 
> `DeclaratorDecl::getSourceRange()`
Could you provide an example? What the range was before and what is it now?




Comment at: clang/lib/Parse/ParseTemplate.cpp:633
   // Grab the template parameter name (if given)
-  SourceLocation NameLoc;
+  SourceLocation NameLoc = Tok.getLocation();
   IdentifierInfo *ParamName = nullptr;

kadircet wrote:
> ilya-biryukov wrote:
> > What happens if there's a comment? E.g.
> > ```
> > template 
> > ```
> > 
> > where would we point to and does that align with the function parameter 
> > case?
> that's same with parmvardecl case, both refer to beginning of the next token 
> that has been lexed.
ok, LG, thanks



Comment at: clang/test/AST/ast-dump-record-definition-data-json.cpp:398
 // CHECK-NEXT:  "range": {
-// CHECK-NEXT:   "begin": {
-// CHECK-NEXT:"col": 29,
-// CHECK-NEXT:"tokLen": 4
-// CHECK-NEXT:   },
-// CHECK-NEXT:   "end": {
-// CHECK-NEXT:"col": 29,
-// CHECK-NEXT:"tokLen": 4
-// CHECK-NEXT:   }
+// CHECK-NEXT:   "begin": {},
+// CHECK-NEXT:   "end": {}

kadircet wrote:
> ilya-biryukov wrote:
> > Does that mean we do not have locations in some cases now?
> > Is that a regression?
> I believe not, this test is a little bit weird, if you look at the code at 
> the beginning, it doesn't contain any "template" stuff explicitly.
> So this is rather coming from implicit nodes(that doesn't have any line 
> numbers but only column numbers)
> 
> Previously test was checking the "class" as the name of the parmdecl, hence 
> the `tokLen: 4`, now it is unnamed therefore no ranges for it.
> though we still have the location marker for `class` (col:29 tokLen: 4)
Agree, these tests do not seem to test this at all, e.g. there are other nodes 
which are output without any locations.
LGTM




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68143



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68143: [clang] Make handling of unnamed template params similar to function params

2019-10-01 Thread Kadir Cetinkaya via Phabricator via cfe-commits 
kadircet added inline comments.



Comment at: clang/lib/AST/DeclTemplate.cpp:513
getDefaultArgumentInfo()->getTypeLoc().getEndLoc());
-  else
-return TypeDecl::getSourceRange();
+  else if(getName().empty())
+return SourceRange(getBeginLoc());

ilya-biryukov wrote:
> Could you provide more details why we need this change?
added comments, you can also see the similar case in 
`DeclaratorDecl::getSourceRange()`



Comment at: clang/lib/Parse/ParseTemplate.cpp:633
   // Grab the template parameter name (if given)
-  SourceLocation NameLoc;
+  SourceLocation NameLoc = Tok.getLocation();
   IdentifierInfo *ParamName = nullptr;

ilya-biryukov wrote:
> What happens if there's a comment? E.g.
> ```
> template 
> ```
> 
> where would we point to and does that align with the function parameter case?
that's same with parmvardecl case, both refer to beginning of the next token 
that has been lexed.



Comment at: clang/test/AST/ast-dump-record-definition-data-json.cpp:398
 // CHECK-NEXT:  "range": {
-// CHECK-NEXT:   "begin": {
-// CHECK-NEXT:"col": 29,
-// CHECK-NEXT:"tokLen": 4
-// CHECK-NEXT:   },
-// CHECK-NEXT:   "end": {
-// CHECK-NEXT:"col": 29,
-// CHECK-NEXT:"tokLen": 4
-// CHECK-NEXT:   }
+// CHECK-NEXT:   "begin": {},
+// CHECK-NEXT:   "end": {}

ilya-biryukov wrote:
> Does that mean we do not have locations in some cases now?
> Is that a regression?
I believe not, this test is a little bit weird, if you look at the code at the 
beginning, it doesn't contain any "template" stuff explicitly.
So this is rather coming from implicit nodes(that doesn't have any line numbers 
but only column numbers)

Previously test was checking the "class" as the name of the parmdecl, hence the 
`tokLen: 4`, now it is unnamed therefore no ranges for it.
though we still have the location marker for `class` (col:29 tokLen: 4)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68143



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68143: [clang] Make handling of unnamed template params similar to function params

2019-10-01 Thread Kadir Cetinkaya via Phabricator via cfe-commits 
kadircet updated this revision to Diff 222567.
kadircet marked 5 inline comments as done.
kadircet added a comment.

- Address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68143

Files:
  clang/lib/AST/DeclTemplate.cpp
  clang/lib/Parse/ParseTemplate.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/test/AST/ast-dump-decl.cpp
  clang/test/AST/ast-dump-record-definition-data-json.cpp
  clang/test/AST/ast-dump-template-decls-json.cpp
  clang/test/AST/ast-dump-template-decls.cpp
  clang/test/ASTMerge/class-template/test.cpp
  clang/test/Index/index-templates.cpp

Index: clang/test/Index/index-templates.cpp
===
--- clang/test/Index/index-templates.cpp
+++ clang/test/Index/index-templates.cpp
@@ -155,7 +155,7 @@
 // CHECK-LOAD: index-templates.cpp:36:44: DeclRefExpr=OneDimension:35:16 Extent=[36:44 - 36:56]
 // CHECK-LOAD: index-templates.cpp:40:8: ClassTemplate=storage:40:8 (Definition) Extent=[39:1 - 40:19]
 // CHECK-LOAD: index-templates.cpp:39:45: TemplateTemplateParameter=DataStructure:39:45 (Definition) Extent=[39:10 - 39:66]
-// CHECK-LOAD: index-templates.cpp:39:19: TemplateTypeParameter=:39:19 (Definition) Extent=[39:19 - 39:27]
+// CHECK-LOAD: index-templates.cpp:39:27: TemplateTypeParameter=:39:27 (Definition) Extent=[39:19 - 39:27]
 // CHECK-LOAD: index-templates.cpp:39:37: NonTypeTemplateParameter=:39:37 (Definition) Extent=[39:29 - 39:37]
 // CHECK-LOAD: index-templates.cpp:39:61: TemplateRef=array:37:8 Extent=[39:61 - 39:66]
 // CHECK-LOAD: index-templates.cpp:42:18: TypedefDecl=Unsigned:42:18 (Definition) Extent=[42:1 - 42:26]
Index: clang/test/ASTMerge/class-template/test.cpp
===
--- clang/test/ASTMerge/class-template/test.cpp
+++ clang/test/ASTMerge/class-template/test.cpp
@@ -9,13 +9,13 @@
 // CHECK: class-template2.cpp:9:15: note: declared here with type 'long'
 
 // CHECK: class-template1.cpp:12:14: warning: template parameter has different kinds in different translation units
-// CHECK: class-template2.cpp:12:10: note: template parameter declared here
+// CHECK: class-template2.cpp:12:18: note: template parameter declared here
 
 // CHECK: class-template1.cpp:18:23: warning: non-type template parameter declared with incompatible types in different translation units ('long' vs. 'int')
 // CHECK: class-template2.cpp:18:23: note: declared here with type 'int'
 
-// CHECK: class-template1.cpp:21:10: warning: template parameter has different kinds in different translation units
-// CHECK: class-template2.cpp:21:10: note: template parameter declared here
+// CHECK: class-template1.cpp:21:18: warning: template parameter has different kinds in different translation units
+// CHECK: class-template2.cpp:21:31: note: template parameter declared here
 
 // CHECK: class-template2.cpp:27:20: warning: external variable 'x0r' declared with incompatible types in different translation units ('X0 *' vs. 'X0 *')
 // CHECK: class-template1.cpp:26:19: note: declared here with type 'X0 *'
Index: clang/test/AST/ast-dump-template-decls.cpp
===
--- clang/test/AST/ast-dump-template-decls.cpp
+++ clang/test/AST/ast-dump-template-decls.cpp
@@ -26,7 +26,7 @@
 // CHECK: FunctionTemplateDecl 0x{{[^ ]*}}  col:6 d
 // CHECK-NEXT: TemplateTypeParmDecl 0x{{[^ ]*}}  col:20 referenced typename depth 0 index 0 Ty
 // CHECK-NEXT: TemplateTemplateParmDecl 0x{{[^ ]*}}  col:52 depth 0 index 1 Uy
-// CHECK-NEXT: TemplateTypeParmDecl 0x{{[^ ]*}}  col:33 typename depth 1 index 0
+// CHECK-NEXT: TemplateTypeParmDecl 0x{{[^ ]*}}  col:41 typename depth 1 index 0
 void d(Ty, Uy);
 
 template 
@@ -47,7 +47,7 @@
 
 template 
 // CHECK: FunctionTemplateDecl 0x{{[^ ]*}}  col:6 h
-// CHECK-NEXT: TemplateTypeParmDecl 0x{{[^ ]*}}  col:11 typename depth 0 index 0
+// CHECK-NEXT: TemplateTypeParmDecl 0x{{[^ ]*}}  col:20 typename depth 0 index 0
 // CHECK-NEXT: TemplateArgument type 'void'
 void h();
 
Index: clang/test/AST/ast-dump-template-decls-json.cpp
===
--- clang/test/AST/ast-dump-template-decls-json.cpp
+++ clang/test/AST/ast-dump-template-decls-json.cpp
@@ -656,8 +656,8 @@
 // CHECK-NEXT:"id": "0x{{.*}}",
 // CHECK-NEXT:"kind": "TemplateTypeParmDecl",
 // CHECK-NEXT:"loc": {
-// CHECK-NEXT: "col": 33,
-// CHECK-NEXT: "tokLen": 8
+// CHECK-NEXT: "col": 41,
+// CHECK-NEXT: "tokLen": 1
 // CHECK-NEXT:},
 // CHECK-NEXT:"range": {
 // CHECK-NEXT: "begin": {
@@ -1099,8 +1099,8 @@
 // CHECK-NEXT:  "kind": "TemplateTypeParmDecl",
 // CHECK-NEXT:  "loc": {
 // CHECK-NEXT:   "line": 27,
-// CHECK-NEXT:   "col": 11,
-// CHECK-NEXT:   "tokLen": 8
+// CHECK-NEXT:   "col": 20,
+// CHECK-NEXT:   "tokLen":

[PATCH] D68143: [clang] Make handling of unnamed template params similar to function params

2019-09-27 Thread Ilya Biryukov via Phabricator via cfe-commits 
ilya-biryukov added inline comments.



Comment at: clang/lib/AST/DeclTemplate.cpp:513
getDefaultArgumentInfo()->getTypeLoc().getEndLoc());
-  else
-return TypeDecl::getSourceRange();
+  else if(getName().empty())
+return SourceRange(getBeginLoc());

Could you provide more details why we need this change?



Comment at: clang/lib/Parse/ParseTemplate.cpp:633
   // Grab the template parameter name (if given)
-  SourceLocation NameLoc;
+  SourceLocation NameLoc = Tok.getLocation();
   IdentifierInfo *ParamName = nullptr;

What happens if there's a comment? E.g.
```
template 
```

where would we point to and does that align with the function parameter case?



Comment at: clang/lib/Sema/SemaTemplate.cpp:1008
 
   SourceLocation Loc = ParamNameLoc;
-  if (!ParamName)

NIT: maybe remove the extra variable and use `ParamNameLoc` everywhere?



Comment at: clang/test/AST/ast-dump-record-definition-data-json.cpp:398
 // CHECK-NEXT:  "range": {
-// CHECK-NEXT:   "begin": {
-// CHECK-NEXT:"col": 29,
-// CHECK-NEXT:"tokLen": 4
-// CHECK-NEXT:   },
-// CHECK-NEXT:   "end": {
-// CHECK-NEXT:"col": 29,
-// CHECK-NEXT:"tokLen": 4
-// CHECK-NEXT:   }
+// CHECK-NEXT:   "begin": {},
+// CHECK-NEXT:   "end": {}

Does that mean we do not have locations in some cases now?
Is that a regression?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68143



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68143: [clang] Make handling of unnamed template params similar to function params

2019-09-27 Thread Kadir Cetinkaya via Phabricator via cfe-commits 
kadircet created this revision.
kadircet added a reviewer: ilya-biryukov.
Herald added subscribers: cfe-commits, arphaman.
Herald added a project: clang.

Clang uses the location identifier should be inserted for declarator
decls when a decl is unnamed. But for type template and template template
paramaters it uses the location of "typename/class" keyword, which makes it hard
for tooling to insert/change parameter names.

This change tries to unify these two cases by making template parameter
parsing and sourcerange operations similar to function params/declarator decls.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D68143

Files:
  clang/lib/AST/DeclTemplate.cpp
  clang/lib/Parse/ParseTemplate.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/test/AST/ast-dump-decl.cpp
  clang/test/AST/ast-dump-record-definition-data-json.cpp
  clang/test/AST/ast-dump-template-decls-json.cpp
  clang/test/AST/ast-dump-template-decls.cpp
  clang/test/ASTMerge/class-template/test.cpp
  clang/test/Index/index-templates.cpp

Index: clang/test/Index/index-templates.cpp
===
--- clang/test/Index/index-templates.cpp
+++ clang/test/Index/index-templates.cpp
@@ -155,7 +155,7 @@
 // CHECK-LOAD: index-templates.cpp:36:44: DeclRefExpr=OneDimension:35:16 Extent=[36:44 - 36:56]
 // CHECK-LOAD: index-templates.cpp:40:8: ClassTemplate=storage:40:8 (Definition) Extent=[39:1 - 40:19]
 // CHECK-LOAD: index-templates.cpp:39:45: TemplateTemplateParameter=DataStructure:39:45 (Definition) Extent=[39:10 - 39:66]
-// CHECK-LOAD: index-templates.cpp:39:19: TemplateTypeParameter=:39:19 (Definition) Extent=[39:19 - 39:27]
+// CHECK-LOAD: index-templates.cpp:39:27: TemplateTypeParameter=:39:27 (Definition) Extent=[39:19 - 39:27]
 // CHECK-LOAD: index-templates.cpp:39:37: NonTypeTemplateParameter=:39:37 (Definition) Extent=[39:29 - 39:37]
 // CHECK-LOAD: index-templates.cpp:39:61: TemplateRef=array:37:8 Extent=[39:61 - 39:66]
 // CHECK-LOAD: index-templates.cpp:42:18: TypedefDecl=Unsigned:42:18 (Definition) Extent=[42:1 - 42:26]
Index: clang/test/ASTMerge/class-template/test.cpp
===
--- clang/test/ASTMerge/class-template/test.cpp
+++ clang/test/ASTMerge/class-template/test.cpp
@@ -9,13 +9,13 @@
 // CHECK: class-template2.cpp:9:15: note: declared here with type 'long'
 
 // CHECK: class-template1.cpp:12:14: warning: template parameter has different kinds in different translation units
-// CHECK: class-template2.cpp:12:10: note: template parameter declared here
+// CHECK: class-template2.cpp:12:18: note: template parameter declared here
 
 // CHECK: class-template1.cpp:18:23: warning: non-type template parameter declared with incompatible types in different translation units ('long' vs. 'int')
 // CHECK: class-template2.cpp:18:23: note: declared here with type 'int'
 
-// CHECK: class-template1.cpp:21:10: warning: template parameter has different kinds in different translation units
-// CHECK: class-template2.cpp:21:10: note: template parameter declared here
+// CHECK: class-template1.cpp:21:18: warning: template parameter has different kinds in different translation units
+// CHECK: class-template2.cpp:21:31: note: template parameter declared here
 
 // CHECK: class-template2.cpp:27:20: warning: external variable 'x0r' declared with incompatible types in different translation units ('X0 *' vs. 'X0 *')
 // CHECK: class-template1.cpp:26:19: note: declared here with type 'X0 *'
Index: clang/test/AST/ast-dump-template-decls.cpp
===
--- clang/test/AST/ast-dump-template-decls.cpp
+++ clang/test/AST/ast-dump-template-decls.cpp
@@ -26,7 +26,7 @@
 // CHECK: FunctionTemplateDecl 0x{{[^ ]*}}  col:6 d
 // CHECK-NEXT: TemplateTypeParmDecl 0x{{[^ ]*}}  col:20 referenced typename depth 0 index 0 Ty
 // CHECK-NEXT: TemplateTemplateParmDecl 0x{{[^ ]*}}  col:52 depth 0 index 1 Uy
-// CHECK-NEXT: TemplateTypeParmDecl 0x{{[^ ]*}}  col:33 typename depth 1 index 0
+// CHECK-NEXT: TemplateTypeParmDecl 0x{{[^ ]*}}  col:41 typename depth 1 index 0
 void d(Ty, Uy);
 
 template 
@@ -47,7 +47,7 @@
 
 template 
 // CHECK: FunctionTemplateDecl 0x{{[^ ]*}}  col:6 h
-// CHECK-NEXT: TemplateTypeParmDecl 0x{{[^ ]*}}  col:11 typename depth 0 index 0
+// CHECK-NEXT: TemplateTypeParmDecl 0x{{[^ ]*}}  col:20 typename depth 0 index 0
 // CHECK-NEXT: TemplateArgument type 'void'
 void h();
 
Index: clang/test/AST/ast-dump-template-decls-json.cpp
===
--- clang/test/AST/ast-dump-template-decls-json.cpp
+++ clang/test/AST/ast-dump-template-decls-json.cpp
@@ -656,8 +656,8 @@
 // CHECK-NEXT:"id": "0x{{.*}}",
 // CHECK-NEXT:"kind": "TemplateTypeParmDecl",
 // CHECK-NEXT:"loc": {
-// CHECK-NEXT: "col": 33,
-// CHECK-NEXT: "tokLen": 8
+// CHECK-NEXT: "col": 41,
+// CHECK-NEXT: