[PATCH] D55932: [Sema] Simplfy static_assert diagnostic code.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman 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 at -1{{static_assert failed due to requirement 'std::is_same<std::is_const<const int>, int>::value' "message"}}
+// expected-error at -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");
----------------
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).


Repository:
  rC Clang

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

https://reviews.llvm.org/D55932