[PATCH] D139705: [clang] fix zero-initialization fix-it for variable template

[Erich Keane via Phabricator via cfe-commits]

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