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

[Vincent Hong via Phabricator via cfe-commits]

v1nh1shungry added a comment.

In D139705#4216539 <https://reviews.llvm.org/D139705#4216539>, @erichkeane wrote:

> 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).

Thank you for the suggestion! This makes sense to me too. But I'm sorry for being busy recently. Please feel free to submit a patch. Thank you!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D139705/new/

https://reviews.llvm.org/D139705