lichray marked 2 inline comments as done.
lichray added inline comments.

================
Comment at: clang/include/clang/AST/PrettyPrinter.h:288
+  /// template parameters, no matter how many elements there are.
+  unsigned EntireContentsOfLargeArray : 1;
+
----------------
rsmith wrote:
> It looks like nothing is setting this to `true` yet, so that part of this 
> patch seems untested. The places I think we want to set this to `true` are:
> 
> 1) When forming types in a diagnostic, if we have two types that differ only 
> in the contents of large arrays. That's checked in here: 
> https://github.com/llvm/llvm-project/blob/main/clang/lib/AST/ASTDiagnostic.cpp#L259
> 
>     I think what we'd want is: if we still have a collision between type 
> names after comparing the canonical strings 
> (https://github.com/llvm/llvm-project/blob/main/clang/lib/AST/ASTDiagnostic.cpp#L291),
>  then set this policy to `true` for the rest of the function, and recompute 
> `S` and `CanS`.
> 
>     This should be detectable in a case such as:
> 
> ```
> void f(X<{"some very long string that differs here: x"}>);
> void g() {
>   f(X<{"some very long string that differs here: y"}>());
> }
> ```
> 
> ... where the diagnostic should include the whole string, not elide the 
> differing portion at the end.
> 
> 2) In the `-ast-print` output: when pretty-printing as code, we want to 
> produce correct, non-elided template arguments. I think this will require 
> passing a printing policy into `TemplateParamObjectDecl::printAsInit` and 
> `::printAsExpr` 
> (https://github.com/llvm/llvm-project/blob/main/clang/lib/AST/DeclTemplate.cpp#L1509).
>  This should be detectable in the output of `-ast-print`, where we do not 
> want to elide any part of template arguments. Perhaps `StmtPrinter` and 
> `DeclPrinter` should enable this `PrintingPolicy` flag?
> 
> 3) When generating debug information: 
> https://github.com/llvm/llvm-project/blob/main/clang/lib/CodeGen/CGDebugInfo.cpp#L230
> 
>     It's generally important that debug information has complete type names.
Added `PrintingPolicy` parameter to `TemplateParamObjectDecl::printAsInit` and 
`printAsExpr`, fixed the implementation, and added tests for the effect of the 
new `EntireContentsOfLargeArray` bit.

Whether how diagnosis, codegen, and higher-level printers should make use of 
them, I think they're outside the scope of this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115031

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

Reply via email to