aaron.ballman added inline comments.
================ Comment at: clang/lib/Sema/SemaInit.cpp:3863 + SourceLocation EndLoc = VD->getEndLoc(); + if (const auto *VTSD = dyn_cast<VarTemplateSpecializationDecl>(VD)) { ---------------- erichkeane wrote: > v1nh1shungry wrote: > > aaron.ballman wrote: > > > erichkeane wrote: > > > > Hmm... it is strange to me that the variables 'endloc' is the end of > > > > the identifier and not the end of the variable declaration. I > > > > unfortunately don't have a good feeling as to the 'correct' behavior > > > > for that (Aaron is typically the one who understands source locations > > > > better than me!), so hopefully he can come along and see. > > > I don't think we should have to do this dance here, this is something > > > that `getEndLoc()` should be able to tell us. The way that source > > > locations work is that AST nodes can override `getSourceRange()` to > > > produce the correct range information for the node, and then they expose > > > different accessors for any other source location information. In this > > > case, I think `VarTemplateSpecializationDecl` isn't overloading > > > `getSourceRange()` and so we're getting the default behavior from > > > `VarDecl`. > > Sorry! I'm confused here. Do you mean we should use `getEndLoc()` directly > > as the location where the fix-hint insert? But isn't the original code > > using `getEndLoc()`, it turns out to add the fix-hint after the end of the > > identifier instead of the right angle of `VarTemplateSpecializationDecl`. > Right, we should use `getEndLoc` directly for hte fixit location, and 'fix' > `getEndLoc` by having `VarTemplateSpecializationDecl` override > `getSourceRange` to have the right `end loc`. > Sorry! I'm confused here. Do you mean we should use getEndLoc() directly as > the location where the fix-hint insert? But isn't the original code using > getEndLoc(), it turns out to add the fix-hint after the end of the identifier > instead of the right angle of VarTemplateSpecializationDecl. Thanks for asking for clarification! I think the code here in SemaInit.cpp is correct as-is and we should instead be implementing `VarTemplateSpecializationDecl::getSourceRange()` to calculate the correct begin and end location for the variable declaration. This should fix SemaInit.cpp because `getEndLoc()` is implemented by calling `getSourceRange()` and returning the end location: https://github.com/llvm/llvm-project/blob/main/clang/include/clang/AST/DeclBase.h#L428 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139705/new/ https://reviews.llvm.org/D139705 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits