[PATCH] D144285: [Clang] Implement CWG2518 - static_assert(false)

cor3ntin added a comment. In D144285#4176850 , @rupprecht wrote: > In D144285#4163004 , @cor3ntin > wrote: > >> If however we find this change to disruptive, we should inform WG21. > > Thanks for the explanation,

[PATCH] D144285: [Clang] Implement CWG2518 - static_assert(false)

rupprecht added a comment. In D144285#4163004 , @cor3ntin wrote: > If however we find this change to disruptive, we should inform WG21. Thanks for the explanation, I think I understand the issue now. I got totally thrown off by the title -- it's not abo

[PATCH] D144285: [Clang] Implement CWG2518 - static_assert(false)

cor3ntin added a comment. @rupprecht This expands to template class MyMock { static_assert( static_assert( ::testing::tuple_size >::ArgumentTuple>::value == 2, "This method does not take " "2" " arguments. Parenthesize all types with unprotected commas."); }; (some stuff omitted)

[PATCH] D144285: [Clang] Implement CWG2518 - static_assert(false)

rupprecht added a comment. Here's one change this patch causes on "real" code (invalid code, but something a user might try to compile): we see is a static_assert in gmock that now fails to report a useful error message: https://godbolt.org/z/sPr1PYT9d Previously we saw `error: static assertion

[PATCH] D144285: [Clang] Implement CWG2518 - static_assert(false)

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG00e2098bf49f: [Clang] Implement CWG2518 - static_assert(false) (authored by cor3ntin). Changed prior to commit: https://reviews.llvm.org/D144285?v

[PATCH] D144285: [Clang] Implement CWG2518 - static_assert(false)

aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM! Comment at: clang/test/SemaTemplate/instantiate-var-template.cpp:36 } + } :: gasps :: spurious whitespace change! :-D Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTI

[PATCH] D144285: [Clang] Implement CWG2518 - static_assert(false)

cor3ntin updated this revision to Diff 500928. cor3ntin added a comment. Remove the #error / #warning tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144285/new/ https://reviews.llvm.org/D144285 Files: clang/docs/ReleaseNotes.rst clang/lib

[PATCH] D144285: [Clang] Implement CWG2518 - static_assert(false)

aaron.ballman added inline comments. Comment at: clang/test/CXX/drs/dr25xx.cpp:5-14 +#error one +// expected-error@-1 {{one}} +#if 0 +#error skip +#warning skip // expected-error {{skip}} +#endif +#error two cor3ntin wrote: > aaron.ballman wrote: > > What do thes

[PATCH] D144285: [Clang] Implement CWG2518 - static_assert(false)

cor3ntin added inline comments. Comment at: clang/test/CXX/drs/dr25xx.cpp:5-14 +#error one +// expected-error@-1 {{one}} +#if 0 +#error skip +#warning skip // expected-error {{skip}} +#endif +#error two aaron.ballman wrote: > What do these tests have to do with t

[PATCH] D144285: [Clang] Implement CWG2518 - static_assert(false)

cor3ntin updated this revision to Diff 500866. cor3ntin marked an inline comment as done. cor3ntin added a comment. Address Aaron's comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144285/new/ https://reviews.llvm.org/D144285 Files: clang/

[PATCH] D144285: [Clang] Implement CWG2518 - static_assert(false)

aaron.ballman added inline comments. Comment at: clang/lib/Sema/SemaDeclCXX.cpp:16824-16841 if (InnerCond && isa(InnerCond)) { // Drill down into concept specialization expressions to see why they // weren't satisfied. Diag(StaticAssertLoc, diag:

[PATCH] D144285: [Clang] Implement CWG2518 - static_assert(false)

cor3ntin updated this revision to Diff 499898. cor3ntin added a comment. I played with different diagnostics, we already specialize the literal case, and I didn't find that not saying a static_assert failed improved things. Instead, printing, in addition of that diagnostic, an instantiation stack

[PATCH] D144285: [Clang] Implement CWG2518 - static_assert(false)

cor3ntin added inline comments. Comment at: clang/lib/Sema/SemaDeclCXX.cpp:16824-16841 if (InnerCond && isa(InnerCond)) { // Drill down into concept specialization expressions to see why they // weren't satisfied. Diag(StaticAssertLoc, diag::err_

[PATCH] D144285: [Clang] Implement CWG2518 - static_assert(false)

aaron.ballman added a comment. In D144285#4135807 , @erichkeane wrote: > In D144285#4135775 , @Endill wrote: > >> Thank you for the patch. >> Any plans to backport this to 16.x branch? > > I would not really want

[PATCH] D144285: [Clang] Implement CWG2518 - static_assert(false)

cor3ntin added inline comments. Comment at: clang/docs/ReleaseNotes.rst:62 directly rather than instantiating the definition from the standard library. +- Implemented `CWG2518 `_ which allows ``static_assert(false)`` + not to be ill-formed when its

[PATCH] D144285: [Clang] Implement CWG2518 - static_assert(false)

cor3ntin updated this revision to Diff 498573. cor3ntin marked 2 inline comments as done. cor3ntin added a comment. - Reword Release note - Restore dependent instantiation tests using Richard's suggestion - Add additional test to check that diagnostics for static_assert are emitted once per insta

[PATCH] D144285: [Clang] Implement CWG2518 - static_assert(false)

rsmith added inline comments. Comment at: clang/lib/Sema/SemaDeclCXX.cpp:16824-16841 if (InnerCond && isa(InnerCond)) { // Drill down into concept specialization expressions to see why they // weren't satisfied. Diag(StaticAssertLoc, diag::err_st

[PATCH] D144285: [Clang] Implement CWG2518 - static_assert(false)

erichkeane added inline comments. Comment at: clang/lib/Sema/SemaDeclCXX.cpp:16806 +bool InTemplateDefinition = +getLangOpts().CPlusPlus && CurContext->isDependentContext(); + shafik wrote: > erichkeane wrote: > > cor3ntin wrote: > > > erichkeane wrot

[PATCH] D144285: [Clang] Implement CWG2518 - static_assert(false)

shafik added inline comments. Comment at: clang/docs/ReleaseNotes.rst:62 directly rather than instantiating the definition from the standard library. +- Implemented `CWG2518 `_ which allows ``static_assert(false)`` + not to be ill-formed when its c

[PATCH] D144285: [Clang] Implement CWG2518 - static_assert(false)

erichkeane added inline comments. Comment at: clang/lib/Sema/SemaDeclCXX.cpp:16806 +bool InTemplateDefinition = +getLangOpts().CPlusPlus && CurContext->isDependentContext(); + cor3ntin wrote: > erichkeane wrote: > > CplusPlus check is now not really b

[PATCH] D144285: [Clang] Implement CWG2518 - static_assert(false)

cor3ntin added inline comments. Comment at: clang/lib/Sema/SemaDeclCXX.cpp:16806 +bool InTemplateDefinition = +getLangOpts().CPlusPlus && CurContext->isDependentContext(); + erichkeane wrote: > CplusPlus check is now not really beneficial? I'm not su

[PATCH] D144285: [Clang] Implement CWG2518 - static_assert(false)

erichkeane accepted this revision. erichkeane added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/Sema/SemaDeclCXX.cpp:16806 +bool InTemplateDefinition = +getLangOpts().CPlusPlus && CurContext->isDependentContext(); +

[PATCH] D144285: [Clang] Implement CWG2518 - static_assert(false)

cor3ntin marked 2 inline comments as done. cor3ntin added inline comments. Comment at: clang/lib/Sema/SemaDeclCXX.cpp:16805 +// definition, the declaration has no effect. +bool InTemplateDefinition = getLangOpts().CPlusPlus && getTemplateDepth(getCurScope()) != 0; +

[PATCH] D144285: [Clang] Implement CWG2518 - static_assert(false)

cor3ntin updated this revision to Diff 498455. cor3ntin added a comment. - Simplify how we check we are in a dependent context - Fix typos Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144285/new/ https://reviews.llvm.org/D144285 Files: clang/do

[PATCH] D144285: [Clang] Implement CWG2518 - static_assert(false)

erichkeane added a comment. In D144285#4135775 , @Endill wrote: > Thank you for the patch. > Any plans to backport this to 16.x branch? I would not really want us to do that, the breaking change here is concerning, and I'd like this to spend time in tru

[PATCH] D144285: [Clang] Implement CWG2518 - static_assert(false)

cor3ntin added a comment. In D144285#4135775 , @Endill wrote: > Thank you for the patch. > Any plans to backport this to 16.x branch? I think that's something we'd need to discuss with more folks but maybe a bit of time to see how this behaves on real c

[PATCH] D144285: [Clang] Implement CWG2518 - static_assert(false)

cor3ntin added inline comments. Comment at: clang/lib/Sema/SemaDeclCXX.cpp:16805 +// definition, the declaration has no effect. +bool InTemplateDefinition = getLangOpts().CPlusPlus && getTemplateDepth(getCurScope()) != 0; + erichkeane wrote: > Hmm... int

[PATCH] D144285: [Clang] Implement CWG2518 - static_assert(false)

Endill added a comment. Thank you for the patch. Any plans to backport this to 16.x branch? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144285/new/ https://reviews.llvm.org/D144285 ___ cfe-commits mail

[PATCH] D144285: [Clang] Implement CWG2518 - static_assert(false)

erichkeane added inline comments. Comment at: clang/lib/Sema/SemaDeclCXX.cpp:16805 +// definition, the declaration has no effect. +bool InTemplateDefinition = getLangOpts().CPlusPlus && getTemplateDepth(getCurScope()) != 0; + Hmm... interesting way to ca

[PATCH] D144285: [Clang] Implement CWG2518 - static_assert(false)

cor3ntin updated this revision to Diff 498448. cor3ntin added a comment. Forgot to run clang-format Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144285/new/ https://reviews.llvm.org/D144285 Files: clang/docs/ReleaseNotes.rst clang/lib/Sema/Se

[PATCH] D144285: [Clang] Implement CWG2518 - static_assert(false)

cor3ntin created this revision. Herald added a project: All. cor3ntin requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This allows `static_assert(false)` to not be ill-formed in template definitions. This change is applied as a DR in all C+