erichkeane added a comment. In D139705#4216530 <https://reviews.llvm.org/D139705#4216530>, @tomasz-kaminski-sonarsource wrote:
>> Since the motivation for this patch here was "make sure we're pointing to >> the 'end' so we can suggest an init fixit", perhaps this SHOULD be changed >> to the 'end' still, but just fix the case where the initializer was omitted. >> So >> >> /// What USED to happen: >> template<> double temp<double> = 1; >> //End is here: ^ >> template<> double temp<double>; >> //End is here: ^ >> >> //What is happening now: >> template<> double temp<double> = 1; >> //End is here: ^ >> template<> double temp<double>; >> //End is here: ^ >> >> // What I think we want: >> template<> double temp<double> = 1; >> //End is here: ^ >> template<> double temp<double>; >> //End is here: ^ >> >> Right? So this isn't so much as a separate function, its just to make sure >> we get the 'source range' to include 'everything', including the >> initializer, if present. > > Indeed, this would address our concern, and allow properly inserting > initializer. This would build down to repeating the condition from here: > https://github.com/llvm/llvm-project/blob/bbe1394c9602ab9a939d9b17199d5f538cac9d0c/clang/lib/AST/Decl.cpp#L1207. > > I was suggesting adding an additional function, as it would cover additional > use cases like replacing or removing the initializer, and then > `VarDecl::getSourceRange` could be defined as: > > SourceRange VarDecl::getSourceRange() const { > if (const Expr *Init = getInit()) { > SourceLocation InitEnd = Init->getEndLoc(); > // If Init is implicit, ignore its source range and fallback on > // DeclaratorDecl::getSourceRange() to handle postfix elements. > if (InitEnd.isValid() && InitEnd != getLocation()) > return SourceRange(getOuterLocStart(), InitEnd); > } > return getDeclatorRange(); > } That would make sense to me. Feel free to submit a patch (assuming @v1nh1shungry doesn't respond/get to it first). 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
