courbet marked an inline comment as done.
courbet added a comment.
Thanks for the comments.
================
Comment at: lib/Sema/SemaTemplate.cpp:3070
+// and returns true.
+static bool prettyPrintTypeTrait(const NestedNameSpecifier *const NNS,
+ llvm::raw_string_ostream &OS,
----------------
aaron.ballman wrote:
> No need for the pointer itself to be `const` qualified -- drop the top-level
> `const` qualifier (here and elsewhere).
constness prevents me from accidentally reassigning the variable. But I won't
fight over it :)
================
Comment at: lib/Sema/SemaTemplate.cpp:3115
+ // This might be `std::some_type_trait<U,V>::value`.
+ if (Var && Var->isStaticDataMember() && Var->getName() == "value" &&
+ prettyPrintTypeTrait(DR->getQualifier(), OS, PrintPolicy)) {
----------------
aaron.ballman wrote:
> You can also check `Var->isInStdNamespace()` here to simplify the logic above.
Thanks for the pointer ! I was looking for something like this :)
I still have to check this on the qualifier and not the variable though, but
that does make the logic a lot simpler.
================
Comment at: test/SemaCXX/static-assert.cpp:111
+static_assert(std::is_same<ExampleTypes::T, ExampleTypes::U>::value,
"message"); // expected-error{{static_assert failed due to requirement
'std::is_same<int, float>::value' "message"}}
+static_assert(std::is_const<ExampleTypes::T>::value, "message");
// expected-error{{static_assert failed due to requirement
'std::is_const<int>::value' "message"}}
----------------
Quuxplusone wrote:
> I would like to see some more realistic test cases. I suggest this test case
> for example:
> ```
> struct BI_tag {};
> struct RAI_tag : BI_tag {};
> struct MyIterator {
> using tag = BI_tag;
> };
> struct MyContainer {
> using iterator = MyIterator;
> };
> template<class Container>
> void foo() {
> static_assert(std::is_base_of_v<RAI_tag, typename
> Container::iterator::tag>);
> }
> ```
> This is an example where as a programmer I would not want to see //only//
> `failed due to requirement std::is_base_of_v<RAI_tag, BI_tag>` — that doesn't
> help me solve the issue. OTOH, since every diagnostic includes a cursor to
> the exact text of the `static_assert` already, I think it's fair to say that
> the current diagnostic message is redundant, and therefore it's okay to
> replace it (as you propose to do) with something that is not redundant.
> I think it's fair to say that the current diagnostic message is redundant,
> and therefore it's okay to replace it (as you propose to do) with something
> that is not redundant.
Yes, the proposal here might not be the *best* possible diagnostic for all
cases, but it's already a huge improvement on the existing one, and solves a
significant proportion of use cases.
Here, the programmer will see:
```
test.cc:13:5: error: static_assert failed due to requirement
'std::is_base_of<RAI_tag, BI_tag>::value'
static_assert(std::is_base_of<RAI_tag, typename
Container::iterator::tag>::value);
^
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
```
which I think is a reasonable help for debugging.
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D54903/new/
https://reviews.llvm.org/D54903
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
