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

Clement Courbet via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Jan 2 08:26:34 PST 2019


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 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");
----------------
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.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55932





More information about the cfe-commits mailing list