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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: include/clang/AST/NestedNameSpecifier.h:220
+  void print(raw_ostream &OS, const PrintingPolicy &Policy,
+             bool ResolveTemplateArguments = false) const;
 
----------------
Quuxplusone wrote:
> Peanut gallery says: Should `ResolveTemplateArguments` really be here, or should it be a property of the `PrintingPolicy` the same way e.g. `ConstantsAsWritten` is a property of the `PrintingPolicy`?  I don't know what a `PrintingPolicy` is, really, but I know that I hate defaulted boolean function parameters with a passion. ;)
> 
> Furthermore, I note that the doc-comment for `ConstantsAsWritten`, at line ~207 of include/clang/AST/PrettyPrinter.h, is nonsensical and maybe someone should give it some love. (That is totally not //your// problem, though.)
I think this feels like a printing policy decision and should live there.


================
Comment at: lib/Sema/SemaTemplate.cpp:3071
+      printTemplateArgumentList(OS, IV->getTemplateArgs().asArray(), Policy);
+    }
+    return;
----------------
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.


Repository:
  rC Clang

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

https://reviews.llvm.org/D54903