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 specifier argument|noexcept specifier argument}0 "
+ "array size|explicit specifier argument|noexcept specifier argument|call to
size()|call to data()}0 "
"is not a constant expression">;
----------------
aaron.ballman wrote:
> This should also be re-flowed to 80 columns.
It looks like these edits were missed.
================
Comment at: clang/lib/AST/DeclPrinter.cpp:952-953
&Context);
- if (StringLiteral *SL = D->getMessage()) {
+ if (StringLiteral *SL =
+ llvm::dyn_cast_if_present<StringLiteral>(D->getMessage())) {
Out << ", ";
----------------
Should there be an `else` clause here that does
`D->getMessage()->prettyPrint()` so that we print out *something*? Otherwise,
there should be a fixme comment about not printing out non-string literal
operands.
================
Comment at: clang/lib/AST/ExprConstant.cpp:16408
+ Char) ||
+ !Char.isInt())
+ return false;
----------------
shafik wrote:
> Why are we specifically checking `!isInt()` what `Kind` do we expect?
I think the caller already validates that we have a pointer to the correct kind
of string, so I kind of think the `isInt()` check is unnecessary. If it's not,
can you show a test case where it kicks in?
================
Comment at: clang/test/SemaCXX/static-assert-cxx26.cpp:269
+ //expected-error {{static assertion failed}} \
+ // expected-note {{in call to
'InvalidPtr{}.data()'}}
----------------
One more test case to consider: do we properly handle dependent expressions in
the assert itself? e.g.,
```
template <typename Ty>
struct S {
static_assert(false, Ty{});
};
struct Frobble {
constexpr int size() const { return 5; }
consterxpr const char *data() const { return "hello"; }
};
S<Frobble> s; // Good, fails with "hello"
S<int> t; // Incorrect
```
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D154290/new/
https://reviews.llvm.org/D154290
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
