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