[PATCH] D154290: [Clang] Implement P2741R3 - user-generated static_assert messages

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG47ccfd7a89e2: [Clang] Implement P2741R3 - user-generated static_assert messages (authored by cor3ntin). Repository: rG LLVM Github Monorepo CHANG

[PATCH] D154290: [Clang] Implement P2741R3 - user-generated static_assert messages

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM assuming precommit CI doesn't come back with any surprises. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154290/new/ https:/

[PATCH] D154290: [Clang] Implement P2741R3 - user-generated static_assert messages

cor3ntin added inline comments. Comment at: clang/test/SemaCXX/static-assert-cxx26.cpp:269 + //expected-error {{static assertion failed}} \ + // expected-note {{in call to 'InvalidPtr{}.data()'}} aaron.ballman

[PATCH] D154290: [Clang] Implement P2741R3 - user-generated static_assert messages

cor3ntin updated this revision to Diff 542102. cor3ntin marked an inline comment as done. cor3ntin added a comment. - Add test for dependent message - always dump the message even if it's not a string literal - remove superflous isInt - fix diag message Repository: rG LLVM Github Monorepo CHA

[PATCH] D154290: [Clang] Implement P2741R3 - user-generated static_assert messages

cor3ntin added inline comments. Comment at: clang/lib/AST/ExprConstant.cpp:16408 +Char) || +!Char.isInt()) + return false; aaron.ballman wrote: > shafik wrote: > > Why are we specifically checking `!isInt()` wh

[PATCH] D154290: [Clang] Implement P2741R3 - user-generated static_assert messages

aaron.ballman added a comment. This is looking pretty close to good, but there were some outstanding questions still. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:86 "%select{case value|enumerator value|non-type template argument|" - "array size|explicit sp

[PATCH] D154290: [Clang] Implement P2741R3 - user-generated static_assert messages

cor3ntin updated this revision to Diff 542022. cor3ntin marked 17 inline comments as done. cor3ntin added a comment. - Display a warning, defaulted as error, if the message cannot be evaluated in a static assertion that does not fail - Address Shafik's, Timm's and Aaron's feedbacks Repository:

[PATCH] D154290: [Clang] Implement P2741R3 - user-generated static_assert messages

tbaeder added inline comments. Comment at: clang/lib/Sema/SemaDeclCXX.cpp:17059 +// the message is grammatically valid without evaluating it. +if (!Failed && AssertMessage && !!Cond) { + std::string Str; Should work, shouldn't it? Repository: rG

[PATCH] D154290: [Clang] Implement P2741R3 - user-generated static_assert messages

aaron.ballman added inline comments. Comment at: clang/lib/Sema/SemaDeclCXX.cpp:16999-17000 + + if (CheckOnly) +return true; + tbaeder wrote: > aaron.ballman wrote: > > E, this seems like a pretty big surprise because evaluating that char > > range can

[PATCH] D154290: [Clang] Implement P2741R3 - user-generated static_assert messages

tbaeder added inline comments. Comment at: clang/lib/Sema/SemaDeclCXX.cpp:16999-17000 + + if (CheckOnly) +return true; + aaron.ballman wrote: > E, this seems like a pretty big surprise because evaluating that char > range can fail. This means: > ``` > c

[PATCH] D154290: [Clang] Implement P2741R3 - user-generated static_assert messages

cor3ntin added a comment. @aaron.ballman @shafik Thanks! I'll address the small nitpicks one we all agree on a direction on whether to have a warning/error/nothing on `static_assert(true, non_constant_expression)` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://rev

[PATCH] D154290: [Clang] Implement P2741R3 - user-generated static_assert messages

shafik added inline comments. Comment at: clang/lib/AST/ExprConstant.cpp:16408 +Char) || +!Char.isInt()) + return false; Why are we specifically checking `!isInt()` what `Kind` do we expect?

[PATCH] D154290: [Clang] Implement P2741R3 - user-generated static_assert messages

aaron.ballman added inline comments. Comment at: clang/test/SemaCXX/static-assert-cxx26.cpp:240 + +static_assert(false, MessageOverload{}); // expected-error {{static assertion failed: A}} Another test case to consider adding: ``` consteval const char *oops() {

[PATCH] D154290: [Clang] Implement P2741R3 - user-generated static_assert messages

aaron.ballman added inline comments. Comment at: clang/test/SemaCXX/static-assert-cxx26.cpp:110-111 + +static_assert(false, Leaks{}); //expected-error {{the message in a static assertion must be produced by a constant expression}} \ + // expected-err

[PATCH] D154290: [Clang] Implement P2741R3 - user-generated static_assert messages

cor3ntin marked 3 inline comments as done. cor3ntin added inline comments. Comment at: clang/test/SemaCXX/static-assert-cxx26.cpp:110-111 + +static_assert(false, Leaks{}); //expected-error {{the message in a static assertion must be produced by a constant expression}} \ +

[PATCH] D154290: [Clang] Implement P2741R3 - user-generated static_assert messages

aaron.ballman added subscribers: erichkeane, tahonermann, ldionne, hubert.reinterpretcast. aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:1553-1558 +def err_static_assert_invalid_size : Error< + "the message in a static asserti

[PATCH] D154290: [Clang] Implement P2741R3 - user-generated static_assert messages

cor3ntin updated this revision to Diff 539501. cor3ntin added a comment. Add tests for deleted methods and methods whose constraints are not satisfied. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154290/new/ https://reviews.llvm.org/D154290 File

[PATCH] D154290: [Clang] Implement P2741R3 - user-generated static_assert messages

cor3ntin added inline comments. Comment at: clang/lib/AST/ExprConstant.cpp:16417-16418 + } + if (!Scope.destroy()) +return false; + aaron.ballman wrote: > Rather than use an RAII object and destroy it manually, let's use `{}` to > scope the RAII object app

[PATCH] D154290: [Clang] Implement P2741R3 - user-generated static_assert messages

cor3ntin updated this revision to Diff 539467. cor3ntin marked an inline comment as done. cor3ntin added a comment. - Rename EvaluateCharPointerAsString => EvaluateCharRangeAsString - Improve diag messages - Form an overload set to diagnose invalid overloads - Add more tests Repository: rG LLV

[PATCH] D154290: [Clang] Implement P2741R3 - user-generated static_assert messages

barannikov88 added inline comments. Comment at: clang/lib/AST/ExprConstant.cpp:16413 +APSInt C = Char.getInt(); +Result.push_back(static_cast(C.getExtValue())); +if (!HandleLValueArrayAdjustment(Info, PtrExpression, String, CharTy, 1)) aaron.ballman w

[PATCH] D154290: [Clang] Implement P2741R3 - user-generated static_assert messages

shafik added inline comments. Comment at: clang/test/SemaCXX/static-assert-cxx26.cpp:1 +// RUN: %clang_cc1 -std=c++2c -fsyntax-only %s -verify + How about a test case w/ `data()` and or `size()` has default arguments. Repository: rG LLVM Github Monorepo CHAN

[PATCH] D154290: [Clang] Implement P2741R3 - user-generated static_assert messages

shafik added inline comments. Comment at: clang/test/SemaCXX/static-assert-cxx26.cpp:167 +static_assert(false, MessageInvalidSize{}); // expected-error {{static assertion failed}} \ + // expected-error {{the message in a static_asser

[PATCH] D154290: [Clang] Implement P2741R3 - user-generated static_assert messages

aaron.ballman added a comment. (Not a full review, I ran out of steam -- I wanted to get you some feedback that I already found though.) Comment at: clang/include/clang/AST/Expr.h:766-767 + bool EvaluateCharPointerAsString(std::string &Result, +

[PATCH] D154290: [Clang] Implement P2741R3 - user-generated static_assert messages

cor3ntin updated this revision to Diff 538579. cor3ntin marked 3 inline comments as done. cor3ntin added a comment. Address Sergei's feedback - Add more tests - Support non const member functions - Make sure diagnostics messages are never produced twice - Support returning intermediate objects fr

[PATCH] D154290: [Clang] Implement P2741R3 - user-generated static_assert messages

cor3ntin added a comment. > but right now I'm confused by the distinction… Why don't always evaluate the > message? 2 reasons - it would be a rather important breaking change for compiler who don't always use utf-8 at their literal encoding - we do not want to limit static_assert to the capabi

[PATCH] D154290: [Clang] Implement P2741R3 - user-generated static_assert messages

barannikov88 added a comment. In D154290#4483055 , @cor3ntin wrote: > In D154290#4482975 , @barannikov88 > wrote: > >> According to the current wording, the static_assert-message is either >> unevaluated string

[PATCH] D154290: [Clang] Implement P2741R3 - user-generated static_assert messages

cor3ntin added a comment. In D154290#4482975 , @barannikov88 wrote: > According to the current wording, the static_assert-message is either > unevaluated string or an expression evaluated at compile time. > Unevaluated strings don't allow certain escape

[PATCH] D154290: [Clang] Implement P2741R3 - user-generated static_assert messages

barannikov88 added a comment. According to the current wording, the static_assert-message is either unevaluated string or an expression evaluated at compile time. Unevaluated strings don't allow certain escape sequences, but if I wrap the string in a string_view-like class, I'm allowed to use an

[PATCH] D154290: [Clang] Implement P2741R3 - user-generated static_assert messages

barannikov88 added inline comments. Comment at: clang/include/clang/Sema/Sema.h:3883 + ///< message. +CCEK_StaticAssertMessageData, ///< Call to data() in a static assert + ///< message. Appear

[PATCH] D154290: [Clang] Implement P2741R3 - user-generated static_assert messages

cor3ntin added inline comments. Comment at: clang/lib/Sema/SemaDeclCXX.cpp:16898 + } + + QualType T = Message->getType().getNonReferenceType(); tbaeder wrote: > What if the message is ` StringLiteral` but `getCharByteWidth()` doesn't > return `1`? We would get

[PATCH] D154290: [Clang] Implement P2741R3 - user-generated static_assert messages

cor3ntin updated this revision to Diff 538350. cor3ntin marked 4 inline comments as done. cor3ntin added a comment. Address Timm's feedback Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154290/new/ https://reviews.llvm.org/D154290 Files: clang/d