[PATCH] D54903: [Sema] Improve static_assert diagnostics.

[Clement Courbet via Phabricator via cfe-commits]

courbet marked 4 inline comments as done.
courbet added inline comments.


================
Comment at: lib/AST/NestedNameSpecifier.cpp:308-310
+    if (ResolveTemplateArguments && getAsRecordDecl()) {
+      if (const auto *Record =
+              dyn_cast<ClassTemplateSpecializationDecl>(getAsRecordDecl())) {
----------------
aaron.ballman wrote:
> I'd remove the `getAsRecordDecl()` from the first `if` and instead use `dyn_cast_or_null` in the second `if`. Probably something like:
> ```
> const auto *Record = dyn_cast_or_null<ClassTemplateSpecializationDecl>(getAsRecordDecl());
> if (ResolveTemplateArguments && Record) {
> }
> ```
SG, thanks for the pointer to `dyn_cast_or_null`.


================
Comment at: lib/Sema/SemaTemplate.cpp:3071
+      printTemplateArgumentList(OS, IV->getTemplateArgs().asArray(), Policy);
+    }
+    return;
----------------
aaron.ballman wrote:
> Quuxplusone wrote:
> > Checking my understanding: Am I correct that this code currently does not pretty-print
> > 
> >     static_assert(std::is_same<T, int>(), "");
> > 
> > (creating an object of the trait type and then using its constexpr `operator bool` to convert it to bool)? This is a rare idiom and doesn't need to be supported AFAIC.
> I'm fine worrying about that situation for a follow-up patch if it isn't currently supported.
This is not supported indeed. There are a bunch of other expr types that we could support.
This is the most frequent one (11.2% of our codebase), the next one I plan to address (in a follow-up patch) is `static_assert(!std;:type_trait<stuff>::value)` with 6.6%.
The one you're mentioning here accounts for 2.8%.


Repository:
  rC Clang

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

https://reviews.llvm.org/D54903