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

[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 <class /*Something*/>
> ```
> 
> 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