tomasz-kaminski-sonarsource added a comment. In D139705#4216417 <https://reviews.llvm.org/D139705#4216417>, @erichkeane wrote:
> 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? Before the change, for both `x` and `temp<double>`, prior the change the `getSourceRange()` on the `VarDecl` that represents them include initializer (they end just before `;`). But now for the variable template specialization, we end up just after template arguments. This means that fixit/report that previously covered the whole definition, will now not include initializer. Or in our example: ` template<typename T> T temp = 1; // v getSourceRange() ends on this token before and after the change int x = 10; // v getSourceRange() ends on this token prior the change, consistently with normal variables template<> double temp<double> = 1; // ^ getSurceRange() ends on this token after the change, making it inconsistient ` 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
