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

[Tomasz Kamiński via Phabricator via cfe-commits]

tomasz-kaminski-sonarsource added a comment.

While creating a test for the fix, I noticed the getSourceRange behavior is not 
stable after the serialization, as illustrated here: 
https://reviews.llvm.org/D146784


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

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

[Tomasz Kamiński via Phabricator via cfe-commits]

tomasz-kaminski-sonarsource added a comment.

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

I https://reviews.llvm.org/D146733 I have submitted a smaller patch, that 
addresses the regression and the concern. Unfortunately, at this time I am not 
able to commit to the introduction of the additional method, and its proper 
handling for AST nodes.


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

[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 , @erichkeane 
wrote:

> In 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 = 1;
>>>   //End is here:   ^
>>>   template<> double temp;
>>>   //End is here:   ^
>>>   
>>>   //What is happening now:
>>>   template<> double temp = 1;
>>>   //End is here:   ^
>>>   template<> double temp;
>>>   //End is here:   ^
>>>   
>>>   // What I think we want:
>>>   template<> double temp = 1;
>>>   //End is here:   ^
>>>   template<> double temp;
>>>   //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

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[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 , 
@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 = 1;
>>   //End is here:   ^
>>   template<> double temp;
>>   //End is here:   ^
>>   
>>   //What is happening now:
>>   template<> double temp = 1;
>>   //End is here:   ^
>>   template<> double temp;
>>   //End is here:   ^
>>   
>>   // What I think we want:
>>   template<> double temp = 1;
>>   //End is here:   ^
>>   template<> double temp;
>>   //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

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

[Tomasz Kamiński via Phabricator via cfe-commits]

tomasz-kaminski-sonarsource added a comment.

> 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 = 1;
>   //End is here:   ^
>   template<> double temp;
>   //End is here:   ^
>   
>   //What is happening now:
>   template<> double temp = 1;
>   //End is here:   ^
>   template<> double temp;
>   //End is here:   ^
>   
>   // What I think we want:
>   template<> double temp = 1;
>   //End is here:   ^
>   template<> double temp;
>   //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();
  }


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

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

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In D139705#4216480 , @aaron.ballman 
wrote:

> In D139705#4216449 , 
> @tomasz-kaminski-sonarsource wrote:
>
>> In D139705#4216417 , @erichkeane 
>> wrote:
>>
>>> In 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`:

   template
   T temp = 1;
   
   int x  = 10;
   template<> double temp = 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 `` in that case, and now is 
>>> removing it?  Or what am I missing?
>>
>> Before the change, for both `x` and `temp`, prior to the change the 
>> `getSourceRange()` on the `VarDecl` that represents them includes an 
>> 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 an initializer.
>> Or in our example:
>>
>>   template
>>   T temp = 1;
>>// v getSourceRange() ends on this token before and after the change
>>   int x = 10;
>> // v getSourceRange() ends on this token 
>> prior to the change, consistently with normal variables
>>   template<> double temp = 1;
>> // ^ getSourceRange() ends on this token after 
>> the change, making it inconsistent
>
> Thank you for the further explanation, that clarified the concern for me as 
> well. I think I agree with you -- we used to cover the full source range for 
> the AST node, and now we only cover part of it because we're missing the 
> initializer. We want `getSourceRange()` to cover the full range of text for 
> the node, so we should have a different function to access the more limited 
> range. @v1nh1shungry is this something you'd feel comfortable fixing up?

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 = 1;
  //End is here:   ^
  template<> double temp;
  //End is here:   ^
  
  //What is happening now:
  template<> double temp = 1;
  //End is here:   ^
  template<> double temp;
  //End is here:   ^
  
  // What I think we want:
  template<> double temp = 1;
  //End is here:   ^
  template<> double temp;
  //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.


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

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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D139705#4216449 , 
@tomasz-kaminski-sonarsource wrote:

> In D139705#4216417 , @erichkeane 
> wrote:
>
>> In 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`:
>>>
>>>   template
>>>   T temp = 1;
>>>   
>>>   int x  = 10;
>>>   template<> double temp = 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 `` in that case, and now is 
>> removing it?  Or what am I missing?
>
> Before the change, for both `x` and `temp`, prior to the change the 
> `getSourceRange()` on the `VarDecl` that represents them includes an 
> 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 an initializer.
> Or in our example:
>
>   template
>   T temp = 1;
>// v getSourceRange() ends on this token before and after the change
>   int x = 10;
> // v getSourceRange() ends on this token 
> prior to the change, consistently with normal variables
>   template<> double temp = 1;
> // ^ getSourceRange() ends on this token after 
> the change, making it inconsistent

Thank you for the further explanation, that clarified the concern for me as 
well. I think I agree with you -- we used to cover the full source range for 
the AST node, and now we only cover part of it because we're missing the 
initializer. We want `getSourceRange()` to cover the full range of text for the 
node, so we should have a different function to access the more limited range. 
@v1nh1shungry is this something you'd feel comfortable fixing up?


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

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

[Tomasz Kamiński via Phabricator via cfe-commits]

tomasz-kaminski-sonarsource added a comment.

In D139705#4216417 , @erichkeane 
wrote:

> In 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`:
>>
>>   template
>>   T temp = 1;
>>   
>>   int x  = 10;
>>   template<> double temp = 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 `` in that case, and now is removing 
> it?  Or what am I missing?

Before the change, for both `x` and `temp`, 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
  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 = 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

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

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In 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`:
>
>   template
>   T temp = 1;
>   
>   int x  = 10;
>   template<> double temp = 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 `` 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

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

[Tomasz Kamiński via Phabricator via cfe-commits]

tomasz-kaminski-sonarsource added a comment.

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
  T temp = 1;
  
  int x  = 10;
  template<> double temp = 1;

Previously, this could be handled (in case of single variable declaration) by 
simply removing the `var->getSourceRange()` with succeeding coma. However, such 
code does not work for variable templates (and only for them).

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.


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

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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D139705#4067774 , @lattner wrote:

> Got it this time, sorry for the confusion!

Thank you for the help, Chris!


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

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

[Vincent Hong via Phabricator via cfe-commits]

v1nh1shungry added a comment.

In D139705#4067774 , @lattner wrote:

> Got it this time, sorry for the confusion!

Confirmed that I received the invitation. Thanks a lot!


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

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

[Chris Lattner via Phabricator via cfe-commits]

lattner added a comment.

Got it this time, sorry for the confusion!


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

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

[Vincent Hong via Phabricator via cfe-commits]

v1nh1shungry added a comment.

In D139705#4067370 , @lattner wrote:

> I'm pretty sure I'm on top of commit access requests now, plz let me know if 
> I missed you or something!  Could be spam filter or who knows what

I sent an email from  to 
 on last Friday, and I've received no reply so far. I've 
checked the email address many times so it was not likely to be sent to the 
wrong person.

I can resend the email. Or maybe we can deal with it here? If so, my GitHub 
username is v1nh1shungry (https://github.com/v1nh1shungry).

Thank you so much!


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

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

[Chris Lattner via Phabricator via cfe-commits]

lattner added a comment.

I'm pretty sure I'm on top of commit access requests now, plz let me know if I 
missed you or something!  Could be spam filter or who knows what


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

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

[Aaron Ballman via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGb3faa1a87ac3: Fix zero-initialization fix-it for variable 
template (authored by v1nh1shungry, committed by aaron.ballman).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139705

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/AST/DeclTemplate.h
  clang/test/FixIt/fixit-const-var-init.cpp


Index: clang/test/FixIt/fixit-const-var-init.cpp
===
--- /dev/null
+++ clang/test/FixIt/fixit-const-var-init.cpp
@@ -0,0 +1,28 @@
+// RUN: not %clang_cc1 -fdiagnostics-parseable-fixits -x c++ -std=c++14 %s 
2>&1 | FileCheck %s
+
+const int a; // expected-error {{default initialization of an object of const 
type}}
+// CHECK: fix-it:"{{.*}}":{3:12-3:12}:" = 0"
+
+template  const int b; // expected-error {{default 
initialization of an object of const type}}
+// CHECK: fix-it:"{{.*}}":{6:36-6:36}:" = 0"
+
+template  const int b; // expected-error {{default 
initialization of an object of const type}}
+// CHECK: fix-it:"{{.*}}":{9:39-9:39}:" = 0"
+
+template <> const int b; // expected-error {{default 
initialization of an object of const type}}
+// CHECK: fix-it:"{{.*}}":{12:36-12:36}:" = 0"
+
+constexpr float c; // expected-error {{must be initialized by a constant 
expression}}
+// CHECK: fix-it:"{{.*}}":{15:18-15:18}:" = 0.0"
+
+template  constexpr float d; // expected-error {{must be 
initialized by a constant expression}}
+// CHECK: fix-it:"{{.*}}":{18:42-18:42}:" = 0.0"
+
+template  constexpr float d; // expected-error {{must be 
initialized by a constant expression}}
+// CHECK: fix-it:"{{.*}}":{21:45-21:45}:" = 0.0"
+
+template <> constexpr float d; // expected-error {{must be 
initialized by a constant expression}}
+// CHECK: fix-it:"{{.*}}":{24:42-24:42}:" = 0.0"
+
+void (* const func)(int, int); // expected-error {{default initialization of 
an object of const type}}
+// CHECK: fix-it:"{{.*}}":{27:30-27:30}:" = nullptr"
Index: clang/include/clang/AST/DeclTemplate.h
===
--- clang/include/clang/AST/DeclTemplate.h
+++ clang/include/clang/AST/DeclTemplate.h
@@ -2926,6 +2926,14 @@
 return ExplicitInfo ? ExplicitInfo->TemplateKeywordLoc : SourceLocation();
   }
 
+  SourceRange getSourceRange() const override LLVM_READONLY {
+if (isExplicitSpecialization()) {
+  if (const ASTTemplateArgumentListInfo *Info = getTemplateArgsInfo())
+return SourceRange(getOuterLocStart(), Info->getRAngleLoc());
+}
+return VarDecl::getSourceRange();
+  }
+
   void Profile(llvm::FoldingSetNodeID ) const {
 Profile(ID, TemplateArgs->asArray(), getASTContext());
   }
@@ -3083,6 +3091,14 @@
 return First->InstantiatedFromMember.setInt(true);
   }
 
+  SourceRange getSourceRange() const override LLVM_READONLY {
+if (isExplicitSpecialization()) {
+  if (const ASTTemplateArgumentListInfo *Info = getTemplateArgsAsWritten())
+return SourceRange(getOuterLocStart(), Info->getRAngleLoc());
+}
+return VarDecl::getSourceRange();
+  }
+
   void Profile(llvm::FoldingSetNodeID ) const {
 Profile(ID, getTemplateArgs().asArray(), getTemplateParameters(),
 getASTContext());
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -350,6 +350,8 @@
   This fixes `Issue 59765 `_
 - Reject in-class defaulting of previosly declared comparison operators. Fixes
   `Issue 51227 `_.
+- Fix the bug of inserting the ``ZeroInitializationFixit`` before the template
+  argument list of ``VarTemplateSpecializationDecl``.
 
 Improvements to Clang's diagnostics
 ^^^


Index: clang/test/FixIt/fixit-const-var-init.cpp
===
--- /dev/null
+++ clang/test/FixIt/fixit-const-var-init.cpp
@@ -0,0 +1,28 @@
+// RUN: not %clang_cc1 -fdiagnostics-parseable-fixits -x c++ -std=c++14 %s 2>&1 | FileCheck %s
+
+const int a; // expected-error {{default initialization of an object of const type}}
+// CHECK: fix-it:"{{.*}}":{3:12-3:12}:" = 0"
+
+template  const int b; // expected-error {{default initialization of an object of const type}}
+// CHECK: fix-it:"{{.*}}":{6:36-6:36}:" = 0"
+
+template  const int b; // expected-error {{default initialization of an object of const type}}
+// CHECK: fix-it:"{{.*}}":{9:39-9:39}:" = 0"
+
+template <> const int b; // expected-error {{default initialization of an object of const type}}
+// CHECK: fix-it:"{{.*}}":{12:36-12:36}:" = 0"
+
+constexpr float c; // expected-error {{must be initialized by a constant 

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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a subscriber: lattner.
aaron.ballman added a comment.

In D139705#4065905 , @v1nh1shungry 
wrote:

> Sorry to disturb you again! It's been a week since I sent an email to Chris, 
> and I have received no reply. Maybe my request was refused. It'd be great to 
> land this and https://reviews.llvm.org/D140838 before the 16 release, right? 
> If so, could you please help me commit them? I'd like "v1nh1shungry 
> ". Thank you so much!

I doubt your request was refused, my guess is that Chris is busy at the moment 
(CC @lattner). I'm happy to land this for you though!


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

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

[Vincent Hong via Phabricator via cfe-commits]

v1nh1shungry added a comment.

Sorry to disturb you again! It's been a week since I sent an email to Chris, 
and I have received no reply. Maybe my request was refused. It'd be great to 
land this and https://reviews.llvm.org/D140838 before the 16 release, right? If 
so, could you please help me commit them? I'd like "v1nh1shungry 
". Thank you so much!


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

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

[Vincent Hong via Phabricator via cfe-commits]

v1nh1shungry updated this revision to Diff 490525.
v1nh1shungry added a comment.

rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139705

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/AST/DeclTemplate.h
  clang/test/FixIt/fixit-const-var-init.cpp


Index: clang/test/FixIt/fixit-const-var-init.cpp
===
--- /dev/null
+++ clang/test/FixIt/fixit-const-var-init.cpp
@@ -0,0 +1,28 @@
+// RUN: not %clang_cc1 -fdiagnostics-parseable-fixits -x c++ -std=c++14 %s 
2>&1 | FileCheck %s
+
+const int a; // expected-error {{default initialization of an object of const 
type}}
+// CHECK: fix-it:"{{.*}}":{3:12-3:12}:" = 0"
+
+template  const int b; // expected-error {{default 
initialization of an object of const type}}
+// CHECK: fix-it:"{{.*}}":{6:36-6:36}:" = 0"
+
+template  const int b; // expected-error {{default 
initialization of an object of const type}}
+// CHECK: fix-it:"{{.*}}":{9:39-9:39}:" = 0"
+
+template <> const int b; // expected-error {{default 
initialization of an object of const type}}
+// CHECK: fix-it:"{{.*}}":{12:36-12:36}:" = 0"
+
+constexpr float c; // expected-error {{must be initialized by a constant 
expression}}
+// CHECK: fix-it:"{{.*}}":{15:18-15:18}:" = 0.0"
+
+template  constexpr float d; // expected-error {{must be 
initialized by a constant expression}}
+// CHECK: fix-it:"{{.*}}":{18:42-18:42}:" = 0.0"
+
+template  constexpr float d; // expected-error {{must be 
initialized by a constant expression}}
+// CHECK: fix-it:"{{.*}}":{21:45-21:45}:" = 0.0"
+
+template <> constexpr float d; // expected-error {{must be 
initialized by a constant expression}}
+// CHECK: fix-it:"{{.*}}":{24:42-24:42}:" = 0.0"
+
+void (* const func)(int, int); // expected-error {{default initialization of 
an object of const type}}
+// CHECK: fix-it:"{{.*}}":{27:30-27:30}:" = nullptr"
Index: clang/include/clang/AST/DeclTemplate.h
===
--- clang/include/clang/AST/DeclTemplate.h
+++ clang/include/clang/AST/DeclTemplate.h
@@ -2926,6 +2926,14 @@
 return ExplicitInfo ? ExplicitInfo->TemplateKeywordLoc : SourceLocation();
   }
 
+  SourceRange getSourceRange() const override LLVM_READONLY {
+if (isExplicitSpecialization()) {
+  if (const ASTTemplateArgumentListInfo *Info = getTemplateArgsInfo())
+return SourceRange(getOuterLocStart(), Info->getRAngleLoc());
+}
+return VarDecl::getSourceRange();
+  }
+
   void Profile(llvm::FoldingSetNodeID ) const {
 Profile(ID, TemplateArgs->asArray(), getASTContext());
   }
@@ -3083,6 +3091,14 @@
 return First->InstantiatedFromMember.setInt(true);
   }
 
+  SourceRange getSourceRange() const override LLVM_READONLY {
+if (isExplicitSpecialization()) {
+  if (const ASTTemplateArgumentListInfo *Info = getTemplateArgsAsWritten())
+return SourceRange(getOuterLocStart(), Info->getRAngleLoc());
+}
+return VarDecl::getSourceRange();
+  }
+
   void Profile(llvm::FoldingSetNodeID ) const {
 Profile(ID, getTemplateArgs().asArray(), getTemplateParameters(),
 getASTContext());
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -350,6 +350,8 @@
   This fixes `Issue 59765 `_
 - Reject in-class defaulting of previosly declared comparison operators. Fixes
   `Issue 51227 `_.
+- Fix the bug of inserting the ``ZeroInitializationFixit`` before the template
+  argument list of ``VarTemplateSpecializationDecl``.
 
 Improvements to Clang's diagnostics
 ^^^


Index: clang/test/FixIt/fixit-const-var-init.cpp
===
--- /dev/null
+++ clang/test/FixIt/fixit-const-var-init.cpp
@@ -0,0 +1,28 @@
+// RUN: not %clang_cc1 -fdiagnostics-parseable-fixits -x c++ -std=c++14 %s 2>&1 | FileCheck %s
+
+const int a; // expected-error {{default initialization of an object of const type}}
+// CHECK: fix-it:"{{.*}}":{3:12-3:12}:" = 0"
+
+template  const int b; // expected-error {{default initialization of an object of const type}}
+// CHECK: fix-it:"{{.*}}":{6:36-6:36}:" = 0"
+
+template  const int b; // expected-error {{default initialization of an object of const type}}
+// CHECK: fix-it:"{{.*}}":{9:39-9:39}:" = 0"
+
+template <> const int b; // expected-error {{default initialization of an object of const type}}
+// CHECK: fix-it:"{{.*}}":{12:36-12:36}:" = 0"
+
+constexpr float c; // expected-error {{must be initialized by a constant expression}}
+// CHECK: fix-it:"{{.*}}":{15:18-15:18}:" = 0.0"
+
+template  constexpr float d; // expected-error {{must be initialized by a constant expression}}
+// CHECK: 

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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D139705#4051178 , @v1nh1shungry 
wrote:

>> I'm happy to, what name and email address would you like used for patch 
>> attribution?
>
> I'd like "v1nh1shungry" and "v1nh1shun...@outlook.com". Thank you!
>
>> Alternatively, it seems that you've had a few patches accepted in the 
>> project, so you could apply for commit access yourself if you'd like: 
>> https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access
>
> Cool! I'll give it a try. Thanks!

Great! I'll hold off on landing this, but let me know if you run into any 
troubles.


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

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

[Vincent Hong via Phabricator via cfe-commits]

v1nh1shungry added a comment.

> I'm happy to, what name and email address would you like used for patch 
> attribution?

I'd like "v1nh1shungry" and "v1nh1shun...@outlook.com". Thank you!

> Alternatively, it seems that you've had a few patches accepted in the 
> project, so you could apply for commit access yourself if you'd like: 
> https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access

Cool! I'll give it a try. Thanks!


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

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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D139705#4050216 , @v1nh1shungry 
wrote:

> If this patch is okay to land, could you please help me commit it? Thanks a 
> lot!

I'm happy to, what name and email address would you like used for patch 
attribution? Alternatively, it seems that you've had a few patches accepted in 
the project, so you could apply for commit access yourself if you'd like: 
https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access


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

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

[Vincent Hong via Phabricator via cfe-commits]

v1nh1shungry added a comment.

If this patch is okay to land, could you please help me commit it? Thanks a lot!


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

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

[Vincent Hong via Phabricator via cfe-commits]

v1nh1shungry updated this revision to Diff 488189.
v1nh1shungry added a comment.

add a release note


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139705

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/AST/DeclTemplate.h
  clang/test/FixIt/fixit-const-var-init.cpp


Index: clang/test/FixIt/fixit-const-var-init.cpp
===
--- /dev/null
+++ clang/test/FixIt/fixit-const-var-init.cpp
@@ -0,0 +1,28 @@
+// RUN: not %clang_cc1 -fdiagnostics-parseable-fixits -x c++ -std=c++14 %s 
2>&1 | FileCheck %s
+
+const int a; // expected-error {{default initialization of an object of const 
type}}
+// CHECK: fix-it:"{{.*}}":{3:12-3:12}:" = 0"
+
+template  const int b; // expected-error {{default 
initialization of an object of const type}}
+// CHECK: fix-it:"{{.*}}":{6:36-6:36}:" = 0"
+
+template  const int b; // expected-error {{default 
initialization of an object of const type}}
+// CHECK: fix-it:"{{.*}}":{9:39-9:39}:" = 0"
+
+template <> const int b; // expected-error {{default 
initialization of an object of const type}}
+// CHECK: fix-it:"{{.*}}":{12:36-12:36}:" = 0"
+
+constexpr float c; // expected-error {{must be initialized by a constant 
expression}}
+// CHECK: fix-it:"{{.*}}":{15:18-15:18}:" = 0.0"
+
+template  constexpr float d; // expected-error {{must be 
initialized by a constant expression}}
+// CHECK: fix-it:"{{.*}}":{18:42-18:42}:" = 0.0"
+
+template  constexpr float d; // expected-error {{must be 
initialized by a constant expression}}
+// CHECK: fix-it:"{{.*}}":{21:45-21:45}:" = 0.0"
+
+template <> constexpr float d; // expected-error {{must be 
initialized by a constant expression}}
+// CHECK: fix-it:"{{.*}}":{24:42-24:42}:" = 0.0"
+
+void (* const func)(int, int); // expected-error {{default initialization of 
an object of const type}}
+// CHECK: fix-it:"{{.*}}":{27:30-27:30}:" = nullptr"
Index: clang/include/clang/AST/DeclTemplate.h
===
--- clang/include/clang/AST/DeclTemplate.h
+++ clang/include/clang/AST/DeclTemplate.h
@@ -2924,6 +2924,14 @@
 return ExplicitInfo ? ExplicitInfo->TemplateKeywordLoc : SourceLocation();
   }
 
+  SourceRange getSourceRange() const override LLVM_READONLY {
+if (isExplicitSpecialization()) {
+  if (const ASTTemplateArgumentListInfo *Info = getTemplateArgsInfo())
+return SourceRange(getOuterLocStart(), Info->getRAngleLoc());
+}
+return VarDecl::getSourceRange();
+  }
+
   void Profile(llvm::FoldingSetNodeID ) const {
 Profile(ID, TemplateArgs->asArray(), getASTContext());
   }
@@ -3081,6 +3089,14 @@
 return First->InstantiatedFromMember.setInt(true);
   }
 
+  SourceRange getSourceRange() const override LLVM_READONLY {
+if (isExplicitSpecialization()) {
+  if (const ASTTemplateArgumentListInfo *Info = getTemplateArgsAsWritten())
+return SourceRange(getOuterLocStart(), Info->getRAngleLoc());
+}
+return VarDecl::getSourceRange();
+  }
+
   void Profile(llvm::FoldingSetNodeID ) const {
 Profile(ID, getTemplateArgs().asArray(), getTemplateParameters(),
 getASTContext());
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -348,6 +348,8 @@
 - Fix issue that the standard C++ modules importer will call global
   constructor/destructor for the global varaibles in the importing modules.
   This fixes `Issue 59765 `_
+- Fix the bug of inserting the ``ZeroInitializationFixit`` before the template
+  argument list of ``VarTemplateSpecializationDecl``.
 
 Improvements to Clang's diagnostics
 ^^^


Index: clang/test/FixIt/fixit-const-var-init.cpp
===
--- /dev/null
+++ clang/test/FixIt/fixit-const-var-init.cpp
@@ -0,0 +1,28 @@
+// RUN: not %clang_cc1 -fdiagnostics-parseable-fixits -x c++ -std=c++14 %s 2>&1 | FileCheck %s
+
+const int a; // expected-error {{default initialization of an object of const type}}
+// CHECK: fix-it:"{{.*}}":{3:12-3:12}:" = 0"
+
+template  const int b; // expected-error {{default initialization of an object of const type}}
+// CHECK: fix-it:"{{.*}}":{6:36-6:36}:" = 0"
+
+template  const int b; // expected-error {{default initialization of an object of const type}}
+// CHECK: fix-it:"{{.*}}":{9:39-9:39}:" = 0"
+
+template <> const int b; // expected-error {{default initialization of an object of const type}}
+// CHECK: fix-it:"{{.*}}":{12:36-12:36}:" = 0"
+
+constexpr float c; // expected-error {{must be initialized by a constant expression}}
+// CHECK: fix-it:"{{.*}}":{15:18-15:18}:" = 0.0"
+
+template  constexpr float d; // expected-error {{must be initialized by a constant expression}}
+// CHECK: 

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

[Erich Keane via Phabricator via cfe-commits]

erichkeane accepted this revision.
erichkeane added inline comments.



Comment at: clang/test/FixIt/fixit-const-var-init.cpp:24
+
+template <> constexpr float d; // expected-error {{must be 
initialized by a constant expression}}
+// CHECK: fix-it:"{{.*}}":{24:42-24:42}:" = 0.0"

aaron.ballman wrote:
> v1nh1shungry wrote:
> > aaron.ballman wrote:
> > > v1nh1shungry wrote:
> > > > aaron.ballman wrote:
> > > > > I'd like to see some additional test coverage for more complicated 
> > > > > declarators:
> > > > > ```
> > > > > void (* const func)(int, int);
> > > > > const int var [[clang::annotate("")]];
> > > > > ```
> > > > One more question: I just did the second test
> > > > 
> > > > ```
> > > > const int var [[clang::annotate("")]];
> > > > ```
> > > > 
> > > > and it inserted the fix hint after the identifier.
> > > > 
> > > > Is this the expected behavior of `getEndLoc()`?
> > > @erichkeane and I talked about this off-list because it's a bit of an 
> > > academic question as to whether an attribute is "part" of a declaration 
> > > or not. We ultimately decided that we think it's best for the attribute 
> > > to *not* be considered part of the source range of the variable 
> > > declaration; they are wholly separate AST nodes.
> > > 
> > > So based on that, we think you will need some changes to SemaInit.cpp 
> > > after all. And you'll have to use 
> > > `SourceManager::isBeforeInTranslationUnit()` to determine if the begin 
> > > source location of the attribute is after the end source location of the 
> > > variable or not (because the attribute could go before the type 
> > > information or after the variable name).
> > > 
> > > There's another test case to consider that hopefully will Just Work with 
> > > this design, but is worth testing explicitly:
> > > ```
> > > void (* const func [[clang::annotate("test")]])(int, int);
> > > ```
> > I have trouble getting the location of the right bracket of the attribute 
> > list, which may be used to insert the fix hint. Could you please give me 
> > some hints? Thanks a lot!
> > I have trouble getting the location of the right bracket of the attribute 
> > list, which may be used to insert the fix hint. Could you please give me 
> > some hints? Thanks a lot!
> 
> Oh boy, that's an interesting problem, isn't it! We keep all of the attribute 
> AST nodes with the declaration, and each attribute knows its own source 
> range, but we *don't* keep the source locations for where the full attribute 
> list appears. e.g.,
> ```
> __attribute__((foo)) const int var [[bar, baz]];
> ```
> `var` will have three attributes associated with it, but the only source 
> location information you have access to is for the `foo`, `bar`, and `baz` 
> tokens. Each of those attributes also keeps track of what syntax was used, so 
> you could tell that `foo` was a GNU-style attribute while `bar` and `baz` 
> were C++. But we don't keep enough information about `bar` and `baz` being 
> part of the same attribute list or whether that attribute list is leading or 
> trailing. You have to calculate all of this yourself and some of it might not 
> even be possible to calculate (for example, the user could have done 
> `__attribute__((foo)) const int var [[bar]] [[baz]];` and the AST would come 
> out the same as the previous example).
> 
> I'd say that's way more work than is reasonable for this patch, so my 
> suggestion is to file a GitHub issue about the problem with attributes, note 
> the implementation issues, and then we'll ignore attributes for this patch.
> 
> @erichkeane -- something for us to consider is how to improve this in the 
> AST. We run into problems when doing AST pretty printing because of 
> attributes for these same reasons. It seems to me that the 
> `Decl`/`Stmt`/`TypeLoc` nodes need to keep a list of source ranges for 
> attributes around (since there can be multiple ranges for declarations). WDYT?
I took conversation to the bug here: 
https://github.com/llvm/llvm-project/issues/59935


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

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

[Vincent Hong via Phabricator via cfe-commits]

v1nh1shungry added a comment.

Thank you for reviewing! I have opened an issue on GitHub: 
https://github.com/llvm/llvm-project/issues/59935. Hope my description is 
appropriate.


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

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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

In D139705#4043225 , @v1nh1shungry 
wrote:

> Thanks for the reply! I'll raise a GitHub issue.
>
>> var will have three attributes associated with it, but the only source 
>> location information you have access to is for the foo, bar, and baz tokens. 
>> Each of those attributes also keeps track of what syntax was used, so you 
>> could tell that foo was a GNU-style attribute while bar and baz were C++. 
>> But we don't keep enough information about bar and baz being part of the 
>> same attribute list or whether that attribute list is leading or trailing. 
>> You have to calculate all of this yourself and some of it might not even be 
>> possible to calculate (for example, the user could have done 
>> __attribute__((foo)) const int var [[bar]] [[baz]]; and the AST would come 
>> out the same as the previous example).
>
> Can I post your reply there? I think it will help.

Yes, absolutely!

>> and then we'll ignore attributes for this patch.
>
> Then I think this patch is ready for a review.

Changes LGTM though you should add a release note to tell users about the fix. 
Thank you!


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

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

[Vincent Hong via Phabricator via cfe-commits]

v1nh1shungry added a comment.

Thanks for the reply! I'll raise a GitHub issue.

> var will have three attributes associated with it, but the only source 
> location information you have access to is for the foo, bar, and baz tokens. 
> Each of those attributes also keeps track of what syntax was used, so you 
> could tell that foo was a GNU-style attribute while bar and baz were C++. But 
> we don't keep enough information about bar and baz being part of the same 
> attribute list or whether that attribute list is leading or trailing. You 
> have to calculate all of this yourself and some of it might not even be 
> possible to calculate (for example, the user could have done 
> __attribute__((foo)) const int var [[bar]] [[baz]]; and the AST would come 
> out the same as the previous example).

Can I post your reply there? I think it will help.

> and then we'll ignore attributes for this patch.

Then I think this patch is ready for a review.


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

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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/test/FixIt/fixit-const-var-init.cpp:24
+
+template <> constexpr float d; // expected-error {{must be 
initialized by a constant expression}}
+// CHECK: fix-it:"{{.*}}":{24:42-24:42}:" = 0.0"

v1nh1shungry wrote:
> aaron.ballman wrote:
> > v1nh1shungry wrote:
> > > aaron.ballman wrote:
> > > > I'd like to see some additional test coverage for more complicated 
> > > > declarators:
> > > > ```
> > > > void (* const func)(int, int);
> > > > const int var [[clang::annotate("")]];
> > > > ```
> > > One more question: I just did the second test
> > > 
> > > ```
> > > const int var [[clang::annotate("")]];
> > > ```
> > > 
> > > and it inserted the fix hint after the identifier.
> > > 
> > > Is this the expected behavior of `getEndLoc()`?
> > @erichkeane and I talked about this off-list because it's a bit of an 
> > academic question as to whether an attribute is "part" of a declaration or 
> > not. We ultimately decided that we think it's best for the attribute to 
> > *not* be considered part of the source range of the variable declaration; 
> > they are wholly separate AST nodes.
> > 
> > So based on that, we think you will need some changes to SemaInit.cpp after 
> > all. And you'll have to use `SourceManager::isBeforeInTranslationUnit()` to 
> > determine if the begin source location of the attribute is after the end 
> > source location of the variable or not (because the attribute could go 
> > before the type information or after the variable name).
> > 
> > There's another test case to consider that hopefully will Just Work with 
> > this design, but is worth testing explicitly:
> > ```
> > void (* const func [[clang::annotate("test")]])(int, int);
> > ```
> I have trouble getting the location of the right bracket of the attribute 
> list, which may be used to insert the fix hint. Could you please give me some 
> hints? Thanks a lot!
> I have trouble getting the location of the right bracket of the attribute 
> list, which may be used to insert the fix hint. Could you please give me some 
> hints? Thanks a lot!

Oh boy, that's an interesting problem, isn't it! We keep all of the attribute 
AST nodes with the declaration, and each attribute knows its own source range, 
but we *don't* keep the source locations for where the full attribute list 
appears. e.g.,
```
__attribute__((foo)) const int var [[bar, baz]];
```
`var` will have three attributes associated with it, but the only source 
location information you have access to is for the `foo`, `bar`, and `baz` 
tokens. Each of those attributes also keeps track of what syntax was used, so 
you could tell that `foo` was a GNU-style attribute while `bar` and `baz` were 
C++. But we don't keep enough information about `bar` and `baz` being part of 
the same attribute list or whether that attribute list is leading or trailing. 
You have to calculate all of this yourself and some of it might not even be 
possible to calculate (for example, the user could have done 
`__attribute__((foo)) const int var [[bar]] [[baz]];` and the AST would come 
out the same as the previous example).

I'd say that's way more work than is reasonable for this patch, so my 
suggestion is to file a GitHub issue about the problem with attributes, note 
the implementation issues, and then we'll ignore attributes for this patch.

@erichkeane -- something for us to consider is how to improve this in the AST. 
We run into problems when doing AST pretty printing because of attributes for 
these same reasons. It seems to me that the `Decl`/`Stmt`/`TypeLoc` nodes need 
to keep a list of source ranges for attributes around (since there can be 
multiple ranges for declarations). WDYT?


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

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

[Vincent Hong via Phabricator via cfe-commits]

v1nh1shungry updated this revision to Diff 488138.
v1nh1shungry added a comment.

add `LLVM_READONLY` to `getSourceRange()`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139705

Files:
  clang/include/clang/AST/DeclTemplate.h
  clang/test/FixIt/fixit-const-var-init.cpp


Index: clang/test/FixIt/fixit-const-var-init.cpp
===
--- /dev/null
+++ clang/test/FixIt/fixit-const-var-init.cpp
@@ -0,0 +1,28 @@
+// RUN: not %clang_cc1 -fdiagnostics-parseable-fixits -x c++ -std=c++14 %s 
2>&1 | FileCheck %s
+
+const int a; // expected-error {{default initialization of an object of const 
type}}
+// CHECK: fix-it:"{{.*}}":{3:12-3:12}:" = 0"
+
+template  const int b; // expected-error {{default 
initialization of an object of const type}}
+// CHECK: fix-it:"{{.*}}":{6:36-6:36}:" = 0"
+
+template  const int b; // expected-error {{default 
initialization of an object of const type}}
+// CHECK: fix-it:"{{.*}}":{9:39-9:39}:" = 0"
+
+template <> const int b; // expected-error {{default 
initialization of an object of const type}}
+// CHECK: fix-it:"{{.*}}":{12:36-12:36}:" = 0"
+
+constexpr float c; // expected-error {{must be initialized by a constant 
expression}}
+// CHECK: fix-it:"{{.*}}":{15:18-15:18}:" = 0.0"
+
+template  constexpr float d; // expected-error {{must be 
initialized by a constant expression}}
+// CHECK: fix-it:"{{.*}}":{18:42-18:42}:" = 0.0"
+
+template  constexpr float d; // expected-error {{must be 
initialized by a constant expression}}
+// CHECK: fix-it:"{{.*}}":{21:45-21:45}:" = 0.0"
+
+template <> constexpr float d; // expected-error {{must be 
initialized by a constant expression}}
+// CHECK: fix-it:"{{.*}}":{24:42-24:42}:" = 0.0"
+
+void (* const func)(int, int); // expected-error {{default initialization of 
an object of const type}}
+// CHECK: fix-it:"{{.*}}":{27:30-27:30}:" = nullptr"
Index: clang/include/clang/AST/DeclTemplate.h
===
--- clang/include/clang/AST/DeclTemplate.h
+++ clang/include/clang/AST/DeclTemplate.h
@@ -2924,6 +2924,14 @@
 return ExplicitInfo ? ExplicitInfo->TemplateKeywordLoc : SourceLocation();
   }
 
+  SourceRange getSourceRange() const override LLVM_READONLY {
+if (isExplicitSpecialization()) {
+  if (const ASTTemplateArgumentListInfo *Info = getTemplateArgsInfo())
+return SourceRange(getOuterLocStart(), Info->getRAngleLoc());
+}
+return VarDecl::getSourceRange();
+  }
+
   void Profile(llvm::FoldingSetNodeID ) const {
 Profile(ID, TemplateArgs->asArray(), getASTContext());
   }
@@ -3081,6 +3089,14 @@
 return First->InstantiatedFromMember.setInt(true);
   }
 
+  SourceRange getSourceRange() const override LLVM_READONLY {
+if (isExplicitSpecialization()) {
+  if (const ASTTemplateArgumentListInfo *Info = getTemplateArgsAsWritten())
+return SourceRange(getOuterLocStart(), Info->getRAngleLoc());
+}
+return VarDecl::getSourceRange();
+  }
+
   void Profile(llvm::FoldingSetNodeID ) const {
 Profile(ID, getTemplateArgs().asArray(), getTemplateParameters(),
 getASTContext());


Index: clang/test/FixIt/fixit-const-var-init.cpp
===
--- /dev/null
+++ clang/test/FixIt/fixit-const-var-init.cpp
@@ -0,0 +1,28 @@
+// RUN: not %clang_cc1 -fdiagnostics-parseable-fixits -x c++ -std=c++14 %s 2>&1 | FileCheck %s
+
+const int a; // expected-error {{default initialization of an object of const type}}
+// CHECK: fix-it:"{{.*}}":{3:12-3:12}:" = 0"
+
+template  const int b; // expected-error {{default initialization of an object of const type}}
+// CHECK: fix-it:"{{.*}}":{6:36-6:36}:" = 0"
+
+template  const int b; // expected-error {{default initialization of an object of const type}}
+// CHECK: fix-it:"{{.*}}":{9:39-9:39}:" = 0"
+
+template <> const int b; // expected-error {{default initialization of an object of const type}}
+// CHECK: fix-it:"{{.*}}":{12:36-12:36}:" = 0"
+
+constexpr float c; // expected-error {{must be initialized by a constant expression}}
+// CHECK: fix-it:"{{.*}}":{15:18-15:18}:" = 0.0"
+
+template  constexpr float d; // expected-error {{must be initialized by a constant expression}}
+// CHECK: fix-it:"{{.*}}":{18:42-18:42}:" = 0.0"
+
+template  constexpr float d; // expected-error {{must be initialized by a constant expression}}
+// CHECK: fix-it:"{{.*}}":{21:45-21:45}:" = 0.0"
+
+template <> constexpr float d; // expected-error {{must be initialized by a constant expression}}
+// CHECK: fix-it:"{{.*}}":{24:42-24:42}:" = 0.0"
+
+void (* const func)(int, int); // expected-error {{default initialization of an object of const type}}
+// CHECK: fix-it:"{{.*}}":{27:30-27:30}:" = nullptr"
Index: clang/include/clang/AST/DeclTemplate.h
===
--- 

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

[Vincent Hong via Phabricator via cfe-commits]

v1nh1shungry added inline comments.



Comment at: clang/test/FixIt/fixit-const-var-init.cpp:24
+
+template <> constexpr float d; // expected-error {{must be 
initialized by a constant expression}}
+// CHECK: fix-it:"{{.*}}":{24:42-24:42}:" = 0.0"

aaron.ballman wrote:
> v1nh1shungry wrote:
> > aaron.ballman wrote:
> > > I'd like to see some additional test coverage for more complicated 
> > > declarators:
> > > ```
> > > void (* const func)(int, int);
> > > const int var [[clang::annotate("")]];
> > > ```
> > One more question: I just did the second test
> > 
> > ```
> > const int var [[clang::annotate("")]];
> > ```
> > 
> > and it inserted the fix hint after the identifier.
> > 
> > Is this the expected behavior of `getEndLoc()`?
> @erichkeane and I talked about this off-list because it's a bit of an 
> academic question as to whether an attribute is "part" of a declaration or 
> not. We ultimately decided that we think it's best for the attribute to *not* 
> be considered part of the source range of the variable declaration; they are 
> wholly separate AST nodes.
> 
> So based on that, we think you will need some changes to SemaInit.cpp after 
> all. And you'll have to use `SourceManager::isBeforeInTranslationUnit()` to 
> determine if the begin source location of the attribute is after the end 
> source location of the variable or not (because the attribute could go before 
> the type information or after the variable name).
> 
> There's another test case to consider that hopefully will Just Work with this 
> design, but is worth testing explicitly:
> ```
> void (* const func [[clang::annotate("test")]])(int, int);
> ```
I have trouble getting the location of the right bracket of the attribute list, 
which may be used to insert the fix hint. Could you please give me some hints? 
Thanks a lot!


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

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

[Vincent Hong via Phabricator via cfe-commits]

v1nh1shungry updated this revision to Diff 488081.
v1nh1shungry added a comment.

implement `VarTemplateSpecializationDecl::getSourceRange()`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139705

Files:
  clang/include/clang/AST/DeclTemplate.h
  clang/test/FixIt/fixit-const-var-init.cpp


Index: clang/test/FixIt/fixit-const-var-init.cpp
===
--- /dev/null
+++ clang/test/FixIt/fixit-const-var-init.cpp
@@ -0,0 +1,28 @@
+// RUN: not %clang_cc1 -fdiagnostics-parseable-fixits -x c++ -std=c++14 %s 
2>&1 | FileCheck %s
+
+const int a; // expected-error {{default initialization of an object of const 
type}}
+// CHECK: fix-it:"{{.*}}":{3:12-3:12}:" = 0"
+
+template  const int b; // expected-error {{default 
initialization of an object of const type}}
+// CHECK: fix-it:"{{.*}}":{6:36-6:36}:" = 0"
+
+template  const int b; // expected-error {{default 
initialization of an object of const type}}
+// CHECK: fix-it:"{{.*}}":{9:39-9:39}:" = 0"
+
+template <> const int b; // expected-error {{default 
initialization of an object of const type}}
+// CHECK: fix-it:"{{.*}}":{12:36-12:36}:" = 0"
+
+constexpr float c; // expected-error {{must be initialized by a constant 
expression}}
+// CHECK: fix-it:"{{.*}}":{15:18-15:18}:" = 0.0"
+
+template  constexpr float d; // expected-error {{must be 
initialized by a constant expression}}
+// CHECK: fix-it:"{{.*}}":{18:42-18:42}:" = 0.0"
+
+template  constexpr float d; // expected-error {{must be 
initialized by a constant expression}}
+// CHECK: fix-it:"{{.*}}":{21:45-21:45}:" = 0.0"
+
+template <> constexpr float d; // expected-error {{must be 
initialized by a constant expression}}
+// CHECK: fix-it:"{{.*}}":{24:42-24:42}:" = 0.0"
+
+void (* const func)(int, int); // expected-error {{default initialization of 
an object of const type}}
+// CHECK: fix-it:"{{.*}}":{27:30-27:30}:" = nullptr"
Index: clang/include/clang/AST/DeclTemplate.h
===
--- clang/include/clang/AST/DeclTemplate.h
+++ clang/include/clang/AST/DeclTemplate.h
@@ -2924,6 +2924,14 @@
 return ExplicitInfo ? ExplicitInfo->TemplateKeywordLoc : SourceLocation();
   }
 
+  SourceRange getSourceRange() const override {
+if (isExplicitSpecialization()) {
+  if (const ASTTemplateArgumentListInfo *Info = getTemplateArgsInfo())
+return SourceRange(getOuterLocStart(), Info->getRAngleLoc());
+}
+return VarDecl::getSourceRange();
+  }
+
   void Profile(llvm::FoldingSetNodeID ) const {
 Profile(ID, TemplateArgs->asArray(), getASTContext());
   }
@@ -3081,6 +3089,14 @@
 return First->InstantiatedFromMember.setInt(true);
   }
 
+  SourceRange getSourceRange() const override {
+if (isExplicitSpecialization()) {
+  if (const ASTTemplateArgumentListInfo *Info = getTemplateArgsAsWritten())
+return SourceRange(getOuterLocStart(), Info->getRAngleLoc());
+}
+return VarDecl::getSourceRange();
+  }
+
   void Profile(llvm::FoldingSetNodeID ) const {
 Profile(ID, getTemplateArgs().asArray(), getTemplateParameters(),
 getASTContext());


Index: clang/test/FixIt/fixit-const-var-init.cpp
===
--- /dev/null
+++ clang/test/FixIt/fixit-const-var-init.cpp
@@ -0,0 +1,28 @@
+// RUN: not %clang_cc1 -fdiagnostics-parseable-fixits -x c++ -std=c++14 %s 2>&1 | FileCheck %s
+
+const int a; // expected-error {{default initialization of an object of const type}}
+// CHECK: fix-it:"{{.*}}":{3:12-3:12}:" = 0"
+
+template  const int b; // expected-error {{default initialization of an object of const type}}
+// CHECK: fix-it:"{{.*}}":{6:36-6:36}:" = 0"
+
+template  const int b; // expected-error {{default initialization of an object of const type}}
+// CHECK: fix-it:"{{.*}}":{9:39-9:39}:" = 0"
+
+template <> const int b; // expected-error {{default initialization of an object of const type}}
+// CHECK: fix-it:"{{.*}}":{12:36-12:36}:" = 0"
+
+constexpr float c; // expected-error {{must be initialized by a constant expression}}
+// CHECK: fix-it:"{{.*}}":{15:18-15:18}:" = 0.0"
+
+template  constexpr float d; // expected-error {{must be initialized by a constant expression}}
+// CHECK: fix-it:"{{.*}}":{18:42-18:42}:" = 0.0"
+
+template  constexpr float d; // expected-error {{must be initialized by a constant expression}}
+// CHECK: fix-it:"{{.*}}":{21:45-21:45}:" = 0.0"
+
+template <> constexpr float d; // expected-error {{must be initialized by a constant expression}}
+// CHECK: fix-it:"{{.*}}":{24:42-24:42}:" = 0.0"
+
+void (* const func)(int, int); // expected-error {{default initialization of an object of const type}}
+// CHECK: fix-it:"{{.*}}":{27:30-27:30}:" = nullptr"
Index: clang/include/clang/AST/DeclTemplate.h
===
--- 

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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/test/FixIt/fixit-const-var-init.cpp:24
+
+template <> constexpr float d; // expected-error {{must be 
initialized by a constant expression}}
+// CHECK: fix-it:"{{.*}}":{24:42-24:42}:" = 0.0"

v1nh1shungry wrote:
> aaron.ballman wrote:
> > I'd like to see some additional test coverage for more complicated 
> > declarators:
> > ```
> > void (* const func)(int, int);
> > const int var [[clang::annotate("")]];
> > ```
> One more question: I just did the second test
> 
> ```
> const int var [[clang::annotate("")]];
> ```
> 
> and it inserted the fix hint after the identifier.
> 
> Is this the expected behavior of `getEndLoc()`?
@erichkeane and I talked about this off-list because it's a bit of an academic 
question as to whether an attribute is "part" of a declaration or not. We 
ultimately decided that we think it's best for the attribute to *not* be 
considered part of the source range of the variable declaration; they are 
wholly separate AST nodes.

So based on that, we think you will need some changes to SemaInit.cpp after 
all. And you'll have to use `SourceManager::isBeforeInTranslationUnit()` to 
determine if the begin source location of the attribute is after the end source 
location of the variable or not (because the attribute could go before the type 
information or after the variable name).

There's another test case to consider that hopefully will Just Work with this 
design, but is worth testing explicitly:
```
void (* const func [[clang::annotate("test")]])(int, int);
```


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

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

[Vincent Hong via Phabricator via cfe-commits]

v1nh1shungry added inline comments.



Comment at: clang/test/FixIt/fixit-const-var-init.cpp:24
+
+template <> constexpr float d; // expected-error {{must be 
initialized by a constant expression}}
+// CHECK: fix-it:"{{.*}}":{24:42-24:42}:" = 0.0"

aaron.ballman wrote:
> I'd like to see some additional test coverage for more complicated 
> declarators:
> ```
> void (* const func)(int, int);
> const int var [[clang::annotate("")]];
> ```
One more question: I just did the second test

```
const int var [[clang::annotate("")]];
```

and it inserted the fix hint after the identifier.

Is this the expected behavior of `getEndLoc()`?


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

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

[Vincent Hong via Phabricator via cfe-commits]

v1nh1shungry added inline comments.



Comment at: clang/lib/Sema/SemaInit.cpp:3863
 
+  SourceLocation EndLoc = VD->getEndLoc();
+  if (const auto *VTSD = dyn_cast(VD)) {

aaron.ballman wrote:
> erichkeane wrote:
> > v1nh1shungry wrote:
> > > aaron.ballman wrote:
> > > > erichkeane wrote:
> > > > > Hmm... it is strange to me that the variables 'endloc' is the end of 
> > > > > the identifier and not the end of the variable declaration.  I 
> > > > > unfortunately don't have a good feeling as to the 'correct' behavior 
> > > > > for that (Aaron is typically the one who understands source locations 
> > > > > better than me!), so hopefully he can come along and see.  
> > > > I don't think we should have to do this dance here, this is something 
> > > > that `getEndLoc()` should be able to tell us. The way that source 
> > > > locations work is that AST nodes can override `getSourceRange()` to 
> > > > produce the correct range information for the node, and then they 
> > > > expose different accessors for any other source location information. 
> > > > In this case, I think `VarTemplateSpecializationDecl` isn't overloading 
> > > > `getSourceRange()` and so we're getting the default behavior from 
> > > > `VarDecl`.
> > > Sorry! I'm confused here. Do you mean we should use `getEndLoc()` 
> > > directly as the location where the fix-hint insert? But isn't the 
> > > original code using `getEndLoc()`, it turns out to add the fix-hint after 
> > > the end of the identifier instead of the right angle of 
> > > `VarTemplateSpecializationDecl`.
> > Right, we should use `getEndLoc` directly for hte fixit location, and 'fix' 
> > `getEndLoc` by having `VarTemplateSpecializationDecl` override 
> > `getSourceRange` to have the right `end loc`.
> > Sorry! I'm confused here. Do you mean we should use getEndLoc() directly as 
> > the location where the fix-hint insert? But isn't the original code using 
> > getEndLoc(), it turns out to add the fix-hint after the end of the 
> > identifier instead of the right angle of VarTemplateSpecializationDecl.
> 
> Thanks for asking for clarification! I think the code here in SemaInit.cpp is 
> correct as-is and we should instead be implementing 
> `VarTemplateSpecializationDecl::getSourceRange()` to calculate the correct 
> begin and end location for the variable declaration. This should fix 
> SemaInit.cpp because `getEndLoc()` is implemented by calling 
> `getSourceRange()` and returning the end location: 
> https://github.com/llvm/llvm-project/blob/main/clang/include/clang/AST/DeclBase.h#L428
I see. Thanks for the clarification!


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

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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/Sema/SemaInit.cpp:3863
 
+  SourceLocation EndLoc = VD->getEndLoc();
+  if (const auto *VTSD = dyn_cast(VD)) {

erichkeane wrote:
> v1nh1shungry wrote:
> > aaron.ballman wrote:
> > > erichkeane wrote:
> > > > Hmm... it is strange to me that the variables 'endloc' is the end of 
> > > > the identifier and not the end of the variable declaration.  I 
> > > > unfortunately don't have a good feeling as to the 'correct' behavior 
> > > > for that (Aaron is typically the one who understands source locations 
> > > > better than me!), so hopefully he can come along and see.  
> > > I don't think we should have to do this dance here, this is something 
> > > that `getEndLoc()` should be able to tell us. The way that source 
> > > locations work is that AST nodes can override `getSourceRange()` to 
> > > produce the correct range information for the node, and then they expose 
> > > different accessors for any other source location information. In this 
> > > case, I think `VarTemplateSpecializationDecl` isn't overloading 
> > > `getSourceRange()` and so we're getting the default behavior from 
> > > `VarDecl`.
> > Sorry! I'm confused here. Do you mean we should use `getEndLoc()` directly 
> > as the location where the fix-hint insert? But isn't the original code 
> > using `getEndLoc()`, it turns out to add the fix-hint after the end of the 
> > identifier instead of the right angle of `VarTemplateSpecializationDecl`.
> Right, we should use `getEndLoc` directly for hte fixit location, and 'fix' 
> `getEndLoc` by having `VarTemplateSpecializationDecl` override 
> `getSourceRange` to have the right `end loc`.
> Sorry! I'm confused here. Do you mean we should use getEndLoc() directly as 
> the location where the fix-hint insert? But isn't the original code using 
> getEndLoc(), it turns out to add the fix-hint after the end of the identifier 
> instead of the right angle of VarTemplateSpecializationDecl.

Thanks for asking for clarification! I think the code here in SemaInit.cpp is 
correct as-is and we should instead be implementing 
`VarTemplateSpecializationDecl::getSourceRange()` to calculate the correct 
begin and end location for the variable declaration. This should fix 
SemaInit.cpp because `getEndLoc()` is implemented by calling `getSourceRange()` 
and returning the end location: 
https://github.com/llvm/llvm-project/blob/main/clang/include/clang/AST/DeclBase.h#L428


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

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

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: clang/lib/Sema/SemaInit.cpp:3863
 
+  SourceLocation EndLoc = VD->getEndLoc();
+  if (const auto *VTSD = dyn_cast(VD)) {

v1nh1shungry wrote:
> aaron.ballman wrote:
> > erichkeane wrote:
> > > Hmm... it is strange to me that the variables 'endloc' is the end of the 
> > > identifier and not the end of the variable declaration.  I unfortunately 
> > > don't have a good feeling as to the 'correct' behavior for that (Aaron is 
> > > typically the one who understands source locations better than me!), so 
> > > hopefully he can come along and see.  
> > I don't think we should have to do this dance here, this is something that 
> > `getEndLoc()` should be able to tell us. The way that source locations work 
> > is that AST nodes can override `getSourceRange()` to produce the correct 
> > range information for the node, and then they expose different accessors 
> > for any other source location information. In this case, I think 
> > `VarTemplateSpecializationDecl` isn't overloading `getSourceRange()` and so 
> > we're getting the default behavior from `VarDecl`.
> Sorry! I'm confused here. Do you mean we should use `getEndLoc()` directly as 
> the location where the fix-hint insert? But isn't the original code using 
> `getEndLoc()`, it turns out to add the fix-hint after the end of the 
> identifier instead of the right angle of `VarTemplateSpecializationDecl`.
Right, we should use `getEndLoc` directly for hte fixit location, and 'fix' 
`getEndLoc` by having `VarTemplateSpecializationDecl` override `getSourceRange` 
to have the right `end loc`.


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

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

[Vincent Hong via Phabricator via cfe-commits]

v1nh1shungry added inline comments.



Comment at: clang/lib/Sema/SemaInit.cpp:3863
 
+  SourceLocation EndLoc = VD->getEndLoc();
+  if (const auto *VTSD = dyn_cast(VD)) {

aaron.ballman wrote:
> erichkeane wrote:
> > Hmm... it is strange to me that the variables 'endloc' is the end of the 
> > identifier and not the end of the variable declaration.  I unfortunately 
> > don't have a good feeling as to the 'correct' behavior for that (Aaron is 
> > typically the one who understands source locations better than me!), so 
> > hopefully he can come along and see.  
> I don't think we should have to do this dance here, this is something that 
> `getEndLoc()` should be able to tell us. The way that source locations work 
> is that AST nodes can override `getSourceRange()` to produce the correct 
> range information for the node, and then they expose different accessors for 
> any other source location information. In this case, I think 
> `VarTemplateSpecializationDecl` isn't overloading `getSourceRange()` and so 
> we're getting the default behavior from `VarDecl`.
Sorry! I'm confused here. Do you mean we should use `getEndLoc()` directly as 
the location where the fix-hint insert? But isn't the original code using 
`getEndLoc()`, it turns out to add the fix-hint after the end of the identifier 
instead of the right angle of `VarTemplateSpecializationDecl`.


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

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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/Sema/SemaInit.cpp:3863
 
+  SourceLocation EndLoc = VD->getEndLoc();
+  if (const auto *VTSD = dyn_cast(VD)) {

erichkeane wrote:
> Hmm... it is strange to me that the variables 'endloc' is the end of the 
> identifier and not the end of the variable declaration.  I unfortunately 
> don't have a good feeling as to the 'correct' behavior for that (Aaron is 
> typically the one who understands source locations better than me!), so 
> hopefully he can come along and see.  
I don't think we should have to do this dance here, this is something that 
`getEndLoc()` should be able to tell us. The way that source locations work is 
that AST nodes can override `getSourceRange()` to produce the correct range 
information for the node, and then they expose different accessors for any 
other source location information. In this case, I think 
`VarTemplateSpecializationDecl` isn't overloading `getSourceRange()` and so 
we're getting the default behavior from `VarDecl`.



Comment at: clang/test/FixIt/fixit-const-var-init.cpp:24
+
+template <> constexpr float d; // expected-error {{must be 
initialized by a constant expression}}
+// CHECK: fix-it:"{{.*}}":{24:42-24:42}:" = 0.0"

I'd like to see some additional test coverage for more complicated declarators:
```
void (* const func)(int, int);
const int var [[clang::annotate("")]];
```


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

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

[Vincent Hong via Phabricator via cfe-commits]

v1nh1shungry added a comment.

Thanks for reviewing, @erichkeane! And a gentle ping to @aaron.ballman


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

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

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

As far as teh fix itself, I think this is fine.  BUT i think there is value in 
waiting for Aaron to see if there is a deeper issue here.




Comment at: clang/lib/Sema/SemaInit.cpp:3863
 
+  SourceLocation EndLoc = VD->getEndLoc();
+  if (const auto *VTSD = dyn_cast(VD)) {

Hmm... it is strange to me that the variables 'endloc' is the end of the 
identifier and not the end of the variable declaration.  I unfortunately don't 
have a good feeling as to the 'correct' behavior for that (Aaron is typically 
the one who understands source locations better than me!), so hopefully he can 
come along and see.  


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

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

[Vincent Hong via Phabricator via cfe-commits]

v1nh1shungry added a comment.

Ping?


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

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

[v1nh1shungry via Phabricator via cfe-commits]

v1nh1shungry added a reviewer: shafik.
v1nh1shungry marked an inline comment as done.
v1nh1shungry added a comment.

@shafik Thank you for reviewing and giving suggestions!


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

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

[v1nh1shungry via Phabricator via cfe-commits]

v1nh1shungry updated this revision to Diff 481819.
v1nh1shungry added a comment.

address the comment's suggestion


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139705

Files:
  clang/lib/Sema/SemaInit.cpp
  clang/test/FixIt/fixit-const-var-init.cpp


Index: clang/test/FixIt/fixit-const-var-init.cpp
===
--- /dev/null
+++ clang/test/FixIt/fixit-const-var-init.cpp
@@ -0,0 +1,25 @@
+// RUN: not %clang_cc1 -fdiagnostics-parseable-fixits -x c++ -std=c++14 %s 
2>&1 | FileCheck %s
+
+const int a; // expected-error {{default initialization of an object of const 
type}}
+// CHECK: fix-it:"{{.*}}":{3:12-3:12}:" = 0"
+
+template  const int b; // expected-error {{default 
initialization of an object of const type}}
+// CHECK: fix-it:"{{.*}}":{6:36-6:36}:" = 0"
+
+template  const int b; // expected-error {{default 
initialization of an object of const type}}
+// CHECK: fix-it:"{{.*}}":{9:39-9:39}:" = 0"
+
+template <> const int b; // expected-error {{default 
initialization of an object of const type}}
+// CHECK: fix-it:"{{.*}}":{12:36-12:36}:" = 0"
+
+constexpr float c; // expected-error {{must be initialized by a constant 
expression}}
+// CHECK: fix-it:"{{.*}}":{15:18-15:18}:" = 0.0"
+
+template  constexpr float d; // expected-error {{must be 
initialized by a constant expression}}
+// CHECK: fix-it:"{{.*}}":{18:42-18:42}:" = 0.0"
+
+template  constexpr float d; // expected-error {{must be 
initialized by a constant expression}}
+// CHECK: fix-it:"{{.*}}":{21:45-21:45}:" = 0.0"
+
+template <> constexpr float d; // expected-error {{must be 
initialized by a constant expression}}
+// CHECK: fix-it:"{{.*}}":{24:42-24:42}:" = 0.0"
Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -3860,8 +3860,21 @@
   if (VD->getInit() || VD->getEndLoc().isMacroID())
 return false;
 
+  SourceLocation EndLoc = VD->getEndLoc();
+  if (const auto *VTSD = dyn_cast(VD)) {
+if (const auto *VTPSD =
+dyn_cast(VD)) {
+  if (const ASTTemplateArgumentListInfo *Info =
+  VTPSD->getTemplateArgsAsWritten())
+EndLoc = Info->getRAngleLoc();
+} else if (const ASTTemplateArgumentListInfo *Info =
+   VTSD->getTemplateArgsInfo()) {
+  EndLoc = Info->getRAngleLoc();
+}
+  }
+
   QualType VariableTy = VD->getType().getCanonicalType();
-  SourceLocation Loc = S.getLocForEndOfToken(VD->getEndLoc());
+  SourceLocation Loc = S.getLocForEndOfToken(EndLoc);
   std::string Init = S.getFixItZeroInitializerForType(VariableTy, Loc);
   if (!Init.empty()) {
 Sequence.AddZeroInitializationStep(Entity.getType());


Index: clang/test/FixIt/fixit-const-var-init.cpp
===
--- /dev/null
+++ clang/test/FixIt/fixit-const-var-init.cpp
@@ -0,0 +1,25 @@
+// RUN: not %clang_cc1 -fdiagnostics-parseable-fixits -x c++ -std=c++14 %s 2>&1 | FileCheck %s
+
+const int a; // expected-error {{default initialization of an object of const type}}
+// CHECK: fix-it:"{{.*}}":{3:12-3:12}:" = 0"
+
+template  const int b; // expected-error {{default initialization of an object of const type}}
+// CHECK: fix-it:"{{.*}}":{6:36-6:36}:" = 0"
+
+template  const int b; // expected-error {{default initialization of an object of const type}}
+// CHECK: fix-it:"{{.*}}":{9:39-9:39}:" = 0"
+
+template <> const int b; // expected-error {{default initialization of an object of const type}}
+// CHECK: fix-it:"{{.*}}":{12:36-12:36}:" = 0"
+
+constexpr float c; // expected-error {{must be initialized by a constant expression}}
+// CHECK: fix-it:"{{.*}}":{15:18-15:18}:" = 0.0"
+
+template  constexpr float d; // expected-error {{must be initialized by a constant expression}}
+// CHECK: fix-it:"{{.*}}":{18:42-18:42}:" = 0.0"
+
+template  constexpr float d; // expected-error {{must be initialized by a constant expression}}
+// CHECK: fix-it:"{{.*}}":{21:45-21:45}:" = 0.0"
+
+template <> constexpr float d; // expected-error {{must be initialized by a constant expression}}
+// CHECK: fix-it:"{{.*}}":{24:42-24:42}:" = 0.0"
Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -3860,8 +3860,21 @@
   if (VD->getInit() || VD->getEndLoc().isMacroID())
 return false;
 
+  SourceLocation EndLoc = VD->getEndLoc();
+  if (const auto *VTSD = dyn_cast(VD)) {
+if (const auto *VTPSD =
+dyn_cast(VD)) {
+  if (const ASTTemplateArgumentListInfo *Info =
+  VTPSD->getTemplateArgsAsWritten())
+EndLoc = Info->getRAngleLoc();
+} else if (const ASTTemplateArgumentListInfo *Info =
+   VTSD->getTemplateArgsInfo()) {
+  EndLoc = Info->getRAngleLoc();
+}

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

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added inline comments.



Comment at: clang/lib/Sema/SemaInit.cpp:3870-3872
+}
+if (const ASTTemplateArgumentListInfo *Info = VTSD->getTemplateArgsInfo())
+  EndLoc = Info->getRAngleLoc();

If I understand correctly we either have a 
`VarTemplatePartialSpecializationDecl` or `ASTTemplateArgumentListInfo ` so we 
should use an `else if` and if we do then we should add a brace to [make the 
else if consistent with the 
else](https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements)


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

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

[v1nh1shungry via Phabricator via cfe-commits]

v1nh1shungry created this revision.
v1nh1shungry added reviewers: aaron.ballman, erichkeane.
Herald added a project: All.
v1nh1shungry requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Current version there is a fix-it for

  cpp
  template  constexpr int x = 0;
  template <> constexpr int x; // fix-it here

but it will cause

  cpp
  template <> constexpr int x = 0;


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D139705

Files:
  clang/lib/Sema/SemaInit.cpp
  clang/test/FixIt/fixit-const-var-init.cpp


Index: clang/test/FixIt/fixit-const-var-init.cpp
===
--- /dev/null
+++ clang/test/FixIt/fixit-const-var-init.cpp
@@ -0,0 +1,25 @@
+// RUN: not %clang_cc1 -fdiagnostics-parseable-fixits -x c++ -std=c++14 %s 
2>&1 | FileCheck %s
+
+const int a; // expected-error {{default initialization of an object of const 
type}}
+// CHECK: fix-it:"{{.*}}":{3:12-3:12}:" = 0"
+
+template  const int b; // expected-error {{default 
initialization of an object of const type}}
+// CHECK: fix-it:"{{.*}}":{6:36-6:36}:" = 0"
+
+template  const int b; // expected-error {{default 
initialization of an object of const type}}
+// CHECK: fix-it:"{{.*}}":{9:39-9:39}:" = 0"
+
+template <> const int b; // expected-error {{default 
initialization of an object of const type}}
+// CHECK: fix-it:"{{.*}}":{12:36-12:36}:" = 0"
+
+constexpr float c; // expected-error {{must be initialized by a constant 
expression}}
+// CHECK: fix-it:"{{.*}}":{15:18-15:18}:" = 0.0"
+
+template  constexpr float d; // expected-error {{must be 
initialized by a constant expression}}
+// CHECK: fix-it:"{{.*}}":{18:42-18:42}:" = 0.0"
+
+template  constexpr float d; // expected-error {{must be 
initialized by a constant expression}}
+// CHECK: fix-it:"{{.*}}":{21:45-21:45}:" = 0.0"
+
+template <> constexpr float d; // expected-error {{must be 
initialized by a constant expression}}
+// CHECK: fix-it:"{{.*}}":{24:42-24:42}:" = 0.0"
Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -3860,8 +3860,20 @@
   if (VD->getInit() || VD->getEndLoc().isMacroID())
 return false;
 
+  SourceLocation EndLoc = VD->getEndLoc();
+  if (const auto *VTSD = dyn_cast(VD)) {
+if (const auto *VTPSD =
+dyn_cast(VD)) {
+  if (const ASTTemplateArgumentListInfo *Info =
+  VTPSD->getTemplateArgsAsWritten())
+EndLoc = Info->getRAngleLoc();
+}
+if (const ASTTemplateArgumentListInfo *Info = VTSD->getTemplateArgsInfo())
+  EndLoc = Info->getRAngleLoc();
+  }
+
   QualType VariableTy = VD->getType().getCanonicalType();
-  SourceLocation Loc = S.getLocForEndOfToken(VD->getEndLoc());
+  SourceLocation Loc = S.getLocForEndOfToken(EndLoc);
   std::string Init = S.getFixItZeroInitializerForType(VariableTy, Loc);
   if (!Init.empty()) {
 Sequence.AddZeroInitializationStep(Entity.getType());


Index: clang/test/FixIt/fixit-const-var-init.cpp
===
--- /dev/null
+++ clang/test/FixIt/fixit-const-var-init.cpp
@@ -0,0 +1,25 @@
+// RUN: not %clang_cc1 -fdiagnostics-parseable-fixits -x c++ -std=c++14 %s 2>&1 | FileCheck %s
+
+const int a; // expected-error {{default initialization of an object of const type}}
+// CHECK: fix-it:"{{.*}}":{3:12-3:12}:" = 0"
+
+template  const int b; // expected-error {{default initialization of an object of const type}}
+// CHECK: fix-it:"{{.*}}":{6:36-6:36}:" = 0"
+
+template  const int b; // expected-error {{default initialization of an object of const type}}
+// CHECK: fix-it:"{{.*}}":{9:39-9:39}:" = 0"
+
+template <> const int b; // expected-error {{default initialization of an object of const type}}
+// CHECK: fix-it:"{{.*}}":{12:36-12:36}:" = 0"
+
+constexpr float c; // expected-error {{must be initialized by a constant expression}}
+// CHECK: fix-it:"{{.*}}":{15:18-15:18}:" = 0.0"
+
+template  constexpr float d; // expected-error {{must be initialized by a constant expression}}
+// CHECK: fix-it:"{{.*}}":{18:42-18:42}:" = 0.0"
+
+template  constexpr float d; // expected-error {{must be initialized by a constant expression}}
+// CHECK: fix-it:"{{.*}}":{21:45-21:45}:" = 0.0"
+
+template <> constexpr float d; // expected-error {{must be initialized by a constant expression}}
+// CHECK: fix-it:"{{.*}}":{24:42-24:42}:" = 0.0"
Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -3860,8 +3860,20 @@
   if (VD->getInit() || VD->getEndLoc().isMacroID())
 return false;
 
+  SourceLocation EndLoc = VD->getEndLoc();
+  if (const auto *VTSD = dyn_cast(VD)) {
+if (const auto *VTPSD =
+dyn_cast(VD)) {
+  if (const ASTTemplateArgumentListInfo *Info =
+