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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
