[PATCH] D45463: [AST] Print correct tag decl for tag specifier

Richard Smith - zygoloid via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu May 3 16:49:35 PDT 2018


rsmith added a comment.

> TagSpecifierAs expands sizeof(PrintingPolicy) from 8 to 16 bytes. My concern is the comments on PrintingPolicy about how PrintingPolicy is intended to be small. My guess it that 16 bytes is still small enough, but perhaps Richard Smith, who wrote that comment, knows better.

This seems fine. See r270009 for some background for that comment -- we used to store a copy of the `LangOptions` in the `PrintingPolicy` (among other things), and copying these objects (including the potentially-large vectors within the `LangOptions`) was a double-digit percentage of the compile time of some compilations. That's a very different ball park from a change from 8 bytes to 16 bytes.



================
Comment at: include/clang-c/Index.h:4116
   CXPrintingPolicy_SuppressTagKeyword,
-  CXPrintingPolicy_IncludeTagDefinition,
   CXPrintingPolicy_SuppressScope,
----------------
This is not OK: this would be an ABI break for the stable libclang ABI.


================
Comment at: lib/AST/DeclPrinter.cpp:180-181
     if (isFirst) {
-      if(TD)
-        SubPolicy.IncludeTagDefinition = true;
+      if (TD)
+        SubPolicy.TagSpecifierAs = TD;
       SubPolicy.SuppressSpecifiers = false;
----------------
Instead of the changes in this patch, can you address your use case by just changing the `if` here to `if (TD && TD->isThisDeclarationADefinition())`?


https://reviews.llvm.org/D45463





More information about the cfe-commits mailing list