[PATCH] D80878: [clang] Prevent that Decl::dump on a CXXRecordDecl deserialises further declarations.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

As long as JSON dumper still compiles, it looks good to me. Just have a few small nits.



================
Comment at: clang/include/clang/AST/TextNodeDumper.h:47
+  /// Indicates if we can deserialize declarations from the ExternalASTSource.
+  bool Deserialize = true;
+
----------------
Would it be better to make the default value the same as in `ASTNodeTraverser`? I.e.,

```lang=c++
  /// Indicates whether we should trigger deserialization of nodes that had
  /// not already been loaded.
  bool Deserialize = false;
```


================
Comment at: clang/lib/AST/TextNodeDumper.cpp:2031-2034
+      if (!D->hasExternalVisibleStorage() || getDeserialize()) {
+        FLAG(hasConstexprDestructor, constexpr);
+      } else
+        OS << " maybe_constexpr";
----------------
The inconsistency that visually single-statement "if" branch has curly braces but "else" branch doesn't is grating. I understand a macro can expand into multiple statements but visually it looks like a single statement. Probably I would add braces to "else" as well.


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

https://reviews.llvm.org/D80878