erichkeane added a comment. In D139705#4215653 <https://reviews.llvm.org/D139705#4215653>, @tomasz-kaminski-sonarsource wrote:
> As a downstream, we have concerns with this change. From what I saw it breaks > the behavior of the fix-it that wants to remove the whole variable definition > (including the initializer). For example, I have unused, that I want to > remove variable `x` and unnecessary explicit specialization `temp<double>`: > > template<typename T> > T temp = 1; > > int x = 10; > template<> double temp<double> = 1; > > Previously, this could be handled (in case of single variable declaration) by > simply removing up the `var->getSourceRange()` with succeeding coma. However, > after the change, such code does not work, and in general if > `getSourceRange()` is used on `VarDecl` (or even `Decl`), special > consideration needs to be taken for the situation when `VarDecl` refers to > variable template specialization. > > As an alternative, I would suggest introducing an additional function to > `VarDecl` (which could be moved up in the hierarchy), that would report a > source range that corresponds to //declarator-id//, i.e. for template > variable it would include template arguments. Hmm... I'm being a little dense here I guess, I don't understand what you mean? Does this result in one of our fixits being wrong here? With the old range, wouldn't it have left the `<double>` in that case, and now is removing it? Or what am I missing? 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
