[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
Tue Oct 1 03:34:01 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());
----------------
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
More information about the cfe-commits
mailing list