[Lldb-commits] [PATCH] D151683: [clang] Enable C++11-style attributes in all language modes

erichkeane added a comment. In D151683#4382408 , @philnik wrote: > In D151683#4380877 , @erichkeane > wrote: > >> What is the justification for this? > > What exactly are you asking for? Why I'd like to back

[Lldb-commits] [PATCH] D151683: [clang] Enable C++11-style attributes in all language modes

erichkeane added a comment. What is the justification for this? Do other compilers do this? Was there an RFC? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151683/new/ https://reviews.llvm.org/D151683

[Lldb-commits] [PATCH] D140996: [c++20] P1907R1: Support for generalized non-type template arguments of scalar type.

erichkeane added a subscriber: asl. erichkeane added a comment. I've got the 1 concern with the mangling that I REALLY want one of our codegen owners to chime in on, otherwise this LGTM. Comment at: clang/lib/AST/ItaniumMangle.cpp:4397 +// argument. +// As proposed in

[Lldb-commits] [PATCH] D140996: [c++20] P1907R1: Support for generalized non-type template arguments of scalar type.

erichkeane added inline comments. Comment at: clang/www/cxx_status.html:1067 + Reference type template arguments referring to instantiation-dependent objects and subobjects + (i.e. delcared inside a template but neither type- nor value-dependent) aren't fully

[Lldb-commits] [PATCH] D140996: [c++20] P1907R1: Support for generalized non-type template arguments of scalar type.

erichkeane added subscribers: eli.friedman, gribozavr2, akyrtzi, gribozavr. erichkeane added inline comments. Comment at: clang/include/clang/AST/TemplateBase.h:88 +/// so cannot be dependent. +UncommonValue, + bolshakov-a wrote: > shafik wrote: > >

[Lldb-commits] [PATCH] D140996: [c++20] P1907R1: Support for generalized non-type template arguments of scalar type.

erichkeane added a comment. In D140996#4189452 , @bolshakov-a wrote: > Sorry! It's my first time using Phabricator. Maybe, the problem occurs > because I've solved the issue with Arcanist just by means of copy-pasting > patches into "Update Diff" Web

[Lldb-commits] [PATCH] D140996: [c++20] P1907R1: Support for generalized non-type template arguments of scalar type.

erichkeane added a comment. Patch seems to be missing all the context. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140996/new/ https://reviews.llvm.org/D140996 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D140996: [c++20] P1907R1: Support for generalized non-type template arguments of scalar type.

erichkeane added inline comments. Comment at: clang/include/clang/AST/ASTContext.h:3036 if (!std::is_trivially_destructible::value) { - auto DestroyPtr = [](void *V) { static_cast(V)->~T(); }; - AddDeallocation(DestroyPtr, Ptr); + auto DestroyPtr = [](void

[Lldb-commits] [PATCH] D140996: [c++20] P1907R1: Support for generalized non-type template arguments of scalar type.

erichkeane added inline comments. Comment at: clang/include/clang/AST/ASTContext.h:3036 if (!std::is_trivially_destructible::value) { - auto DestroyPtr = [](void *V) { static_cast(V)->~T(); }; - AddDeallocation(DestroyPtr, Ptr); + auto DestroyPtr = [](void

[Lldb-commits] [PATCH] D131858: [clang] Track the templated entity in type substitution.

erichkeane added a comment. In D131858#4054348 , @v.g.vassilev wrote: > In D131858#4052031 , @erichkeane > wrote: > >> In D131858#4052026 , @v.g.vassilev >> wrote: >>

[Lldb-commits] [PATCH] D131858: [clang] Track the templated entity in type substitution.

erichkeane added a comment. In D131858#4052026 , @v.g.vassilev wrote: > Thanks a lot @rsmith for providing a fix and thanks a lot @aaron.ballman and > @erichkeane for the efforts saving @mizvekov work over the summer. I believe > he has sporadic

[Lldb-commits] [PATCH] D131858: [clang] Track the templated entity in type substitution.

erichkeane added a comment. Note, I just did the revert of the workaround myself here: 498bf3424d011235d420e02e80e135fae646d537 I won't see the failure on https://green.lab.llvm.org/green/job/clang-stage2-Rthinlto/ until

[Lldb-commits] [PATCH] D131858: [clang] Track the templated entity in type substitution.

erichkeane added a comment. @arphaman I see you're part of the original report too, if you could try to revert the workaround and test it, it would be nice too! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131858/new/

[Lldb-commits] [PATCH] D131858: [clang] Track the templated entity in type substitution.

erichkeane added a comment. In D131858#4051336 , @aaron.ballman wrote: > In D131858#4050112 , @rsmith wrote: > >> In D131858#3957630 , @arphaman >> wrote: >> >>> This

[Lldb-commits] [PATCH] D131858: [clang] Track the templated entity in type substitution.

erichkeane added a comment. In D131858#4047799 , @v.g.vassilev wrote: > In D131858#4047785 , @erichkeane > wrote: > >> @mizvekov : I'm looking at the revert of this since no work seems to have >> been done to

[Lldb-commits] [PATCH] D131858: [clang] Track the templated entity in type substitution.

erichkeane added a comment. @mizvekov : I'm looking at the revert of this since no work seems to have been done to fix @steven_wu 's problem. Unfortunately, it seems that reverting this requires reverting https://reviews.llvm.org/D134604 plus all of the "Instantiate" patches, which undoes

[Lldb-commits] [PATCH] D136565: [clang] Instantiate alias templates with sugar

erichkeane added a comment. In D136565#4032150 , @erichkeane wrote: > Note that the Clang 16 branch is coming up approximately on January 24, and > this needs to be reverted or perf-regression fixed by then. @mizvekov : If > you or someone else don't

[Lldb-commits] [PATCH] D136565: [clang] Instantiate alias templates with sugar

erichkeane added a comment. Note that the Clang 16 branch is coming up approximately on January 24, and this needs to be reverted or perf-regression fixed by then. @mizvekov : If you or someone else don't have a solution/revert to this by January 13th, @aaron.ballman and I will begin the

[Lldb-commits] [PATCH] D131858: [clang] Track the templated entity in type substitution.

erichkeane added a comment. Note that the Clang 16 branch is coming up approximately on January 24, and this needs to be reverted or have the issue reported by @steven_wu fixed by then. @mizvekov : If you or someone else don't have a solution/revert to this by January 13th, @aaron.ballman and

[Lldb-commits] [PATCH] D138939: [WIP][clang] adds a way to provide user-oriented reasons

erichkeane added inline comments. Comment at: clang/test/Frontend/sarif-reason.cpp:15 +void g() { + f1<0>(); // expected-error{{no matching function for call to 'f1'}} + f1(); // expected-error{{no matching function for call to 'f1'}} cjdb wrote: > erichkeane

[Lldb-commits] [PATCH] D138939: [WIP][clang] adds a way to provide user-oriented reasons

erichkeane added a comment. In D138939#3964439 , @erichkeane wrote: > In D138939#3964404 , @cjdb wrote: > >> In D138939#3963496 , @erichkeane >> wrote: >> >>> In

[Lldb-commits] [PATCH] D138939: [WIP][clang] adds a way to provide user-oriented reasons

erichkeane added a comment. In D138939#3964404 , @cjdb wrote: > In D138939#3963496 , @erichkeane > wrote: > >> In D138939#3963473 , @tschuett >> wrote: >> >>> Maybe

[Lldb-commits] [PATCH] D138939: [WIP][clang] adds a way to provide user-oriented reasons

erichkeane added a comment. In D138939#3963473 , @tschuett wrote: > Maybe `void FormatDiagnostic(SmallVectorImpl , DiagnosticMode > mode)`instead of `void FormatDiagnostic(SmallVectorImpl )`? > To make the transition easer and future proof. I like this

[Lldb-commits] [PATCH] D138939: [WIP][clang] adds a way to provide user-oriented reasons

erichkeane added a comment. In D138939#3958185 , @cjdb wrote: >> The clang-side interface to this seems a touch clunky, and I fear it'll make >> continuing its use/generalizing its use less likely. > > Is this w.r.t. the `FormatDiagnostic` being split

[Lldb-commits] [PATCH] D138939: [WIP][clang] adds a way to provide user-oriented reasons

erichkeane added a comment. Herald added subscribers: Michael137, JDevlieghere. A few comments. I don't mind the approach, though I find myself wondering if the "FormatDiagnostic" call should stay the same, and choose the legacy-reason only when a sarif reason doesn't exist? Or for some level

[Lldb-commits] [PATCH] D134604: [clang] Implement sugared substitution changes to infrastructure

erichkeane accepted this revision. erichkeane added a comment. A couple of nits, otherwise I don't have to see this again. Comment at: clang/include/clang/AST/TemplateName.h:158 + // When true the substitution will be finalized (subst node won't be placed). + bool getFinal()

[Lldb-commits] [PATCH] D131858: [clang] Track the templated entity in type substitution.

erichkeane accepted this revision. erichkeane added a comment. This LGTM, but please give @davrec time (a few days?) to do another review before merging. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131858/new/ https://reviews.llvm.org/D131858

[Lldb-commits] [PATCH] D84126: Enable -Wsuggest-override in the LLVM build

erichkeane added a comment. Ive got a bit a of a problem here: This flag (suggest-override) on GCC 8.3 (and others) warns on missing-override EVEN WHEN the function is marked 'final'. This was not fixed until GCC 9.2: https://godbolt.org/z/55KeM3 The result is my build gave a few thousand