courbet marked an inline comment as done.
courbet added inline comments.

================
Comment at: test/SemaCXX/static-assert.cpp:130
 static_assert(std::is_same<decltype(std::is_const<const ExampleTypes::T>()), 
int>::value, "message");
-// expected-error@-1{{static_assert failed due to requirement 
'std::is_same<std::is_const<const int>, int>::value' "message"}}
+// expected-error@-1{{static_assert failed due to requirement 
'std::is_same<is_const<const int>, int>::value' "message"}}
 static_assert(std::is_const<decltype(ExampleTypes::T(3))>::value, "message");
----------------
aaron.ballman wrote:
> courbet wrote:
> > aaron.ballman wrote:
> > > courbet wrote:
> > > > aaron.ballman wrote:
> > > > > Any idea why the `std::` was dropped here?
> > > > `NestedNameSpecifier::print()` explicitly does:
> > > > 
> > > > ```
> > > >  PrintingPolicy InnerPolicy(Policy);
> > > >  InnerPolicy.SuppressScope = true;
> > > > ```
> > > > 
> > > Ah, good point, but is that a good behavioral change? I slightly prefer 
> > > printing the namespace name there -- it will likely be redundant 
> > > information most of the time, but when the namespace actually matters, 
> > > having it printed could save someone a lot of head scratching.
> > > I slightly prefer printing the namespace name there
> > 
> > I tend to agree, so it's more a trade-off of code complexity vs better 
> > diagnostic - I tend to err on the side of simplifying the code :)
> > 
> > Another option is to add yet another boolean to PrintingPolicy, but I htink 
> > this is too narrow a use case.
> Heh, I tend to err on the side of helping the user unless the code will be 
> truly awful. I agree that another option on PrintingPolicy may not be the 
> best approach. Do you know why the namespace is being suppressed in the first 
> place? Another option would be to always print the namespace, but I don't 
> know what that might regress (if anything).
Unfortunately always printing the qualification breaks 30 tests, including some 
in a very bad way:

```
/usr/local/google/home/courbet/llvm/llvm/tools/clang/test/AST/ast-print-out-of-line-func.cpp:29:11:
 error: CHECK: expected string not found in input
// CHECK: void Wrapper::Inner::operator+=(int)
          ^
<stdin>:14:43: note: scanning from here
 ns::Wrapper::ns::Wrapper::Inner::Inner() {
                                          ^
<stdin>:16:19: note: possible intended match here
 void ns::Wrapper::ns::Wrapper::Inner::operator+=(int) {
```


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55932/new/

https://reviews.llvm.org/D55932



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to