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

Ilya Biryukov via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Sep 27 11:10:09 PDT 2019


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

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





More information about the cfe-commits mailing list