rnk added inline comments.

================
Comment at: clang/lib/AST/TemplateBase.cpp:71-72
 
   if (T->isBooleanType() && !Policy.MSVCFormatting) {
     Out << (Val.getBoolValue() ? "true" : "false");
   } else if (T->isCharType()) {
----------------
rsmith wrote:
> reikdas wrote:
> > rsmith wrote:
> > > rsmith wrote:
> > > > It looks like `MSVCFormatting` wants `bool` values to be printed as `0` 
> > > > and `1`, and this patch presumably changes that (along with the 
> > > > printing of other builtin types). I wonder if this is a problem in 
> > > > practice (eg, if such things are used as keys for debug info or 
> > > > similar)...
> > > Do we need to suppress printing the suffixes below in `MSVCFormatting` 
> > > mode too?
> > @rsmith The tests pass, so that is reassuring at least. Is there any other 
> > way to find out whether we need to suppress printing the suffixes for other 
> > types in MSVCFormatting mode?
> @rnk Can you take a look at this? Per 
> rG60103383f097b6580ecb4519eeb87defdb7c05c9 and PR21528 it seems like there 
> might be specific requirements for how template arguments are formatted for 
> CodeView support; we presumably need to make sure we still satisfy those 
> requirements.
Probably. This change looks like it preserves behavior from before when 
`MSVCFormatting` is set, so I think this is OK. I checked, my version of MSVC 
still uses 1/0 in debug info for boolean template args.


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

https://reviews.llvm.org/D77598

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
  • [PATCH] D77598: I... Richard Smith - zygoloid via Phabricator via cfe-commits
    • [PATCH] D775... Pratyush Das via Phabricator via cfe-commits
    • [PATCH] D775... Pratyush Das via Phabricator via cfe-commits
    • [PATCH] D775... Reid Kleckner via Phabricator via cfe-commits

Reply via email to