[PATCH] D46919: [libclang] Deprecate CXPrintingPolicy_IncludeTagDefinition

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

The deprecated enumerator is also referenced by `tools/c-index-test/c-index-test.c`

This looks fine to me, but let's leave this review thread open for a week or so, to give people a bit more time to object in case they're using the flag for something.



================
Comment at: include/clang/AST/PrettyPrinter.h:97-99
+  /// This flag is deprecated and no longer has any effect.
+  bool IncludeTagDefinition : 1;
+
----------------
There's no need to keep this around. We have no API stability guarantees for our C++ API.


================
Comment at: include/clang/AST/PrettyPrinter.h:100-108
+  /// When true, print any tag declaration owned by an elaborated type.
   ///
-  /// This is used to place the definition of a struct
-  /// in the middle of another declaration as with:
+  /// This is used to faithfully print the exact tag declaration that appears
+  /// within another declaration.  For example:
   ///
   /// \code
+  /// typedef struct T { int x, y; } Point;
----------------
Please explicitly mark this as being transient state that's internal to the printing algorithm and not part of its external configuration, so that people don't try to expose it again in the future. The same is true for a bunch of the other flags around here -- if we're going to do this, we should systematically examine these flags to see which ones are external configuration and which ones are internal state that we're (hackily) passing between printing steps via the `PrintingPolicy`. It would also make sense to move the internal state flags to a separate struct from the policy.


https://reviews.llvm.org/D46919