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

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.

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

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/Dec

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

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

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

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 'e

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

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

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

erichkeane added a comment. In D139705#4216480 , @aaron.ballman wrote: > In D139705#4216449 , > @tomasz-kaminski-sonarsource wrote: > >> In D139705#4216417 , @erichkeane

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

aaron.ballman added a comment. In D139705#4216449 , @tomasz-kaminski-sonarsource wrote: > In D139705#4216417 , @erichkeane > wrote: > >> In D139705#4215653 , >> @tomasz

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

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 wha

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

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 > (incl

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

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

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

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/ ht

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

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

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

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@lis

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

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 Fri

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

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

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

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 Git

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

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 refus

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

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 co

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

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/DeclT

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

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

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

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 co

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

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? Alternat

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

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 _

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

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/cla

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

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" --

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

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://revi

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

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 associa

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

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 u

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

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 wrot

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

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/

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

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 wrot

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

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/

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

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 wrot

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

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 wrot

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

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: >

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

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: > >

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

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 m

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

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 'en

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

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

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

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-com

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

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 *VT

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

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

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

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

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

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 cla

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

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 `VarTemplatePa

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

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