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 <typename>
>           ~~~~~~~~~
> ```
> 
> 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

Reply via email to