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



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

Reply via email to