aaron.ballman added a comment.
It appears we have no test coverage for this case -- can you introduce some?
================
Comment at: lib/AST/ASTDumper.cpp:506-507
if (!isa<FunctionDecl>(*D) && !isa<ObjCMethodDecl>(*D)) {
auto DC = dyn_cast<DeclContext>(D);
- if (DC &&
- (DC->hasExternalLexicalStorage() ||
- (Deserialize ? DC->decls_begin() != DC->decls_end()
- : DC->noload_decls_begin() != DC->noload_decls_end())))
+ if (DC)
dumpDeclContext(DC);
----------------
Might as well clean this up to be `if (const auto *DC =
dyn_cast<DeclContext>(D))`
================
Comment at: lib/AST/ASTDumper.cpp:519-520
- (DC->hasExternalLexicalStorage() ||
- (Deserialize ? DC->decls_begin() != DC->decls_end()
- : DC->noload_decls_begin() != DC->noload_decls_end())))
dumpDeclContext(DC);
----------------
Why did this condition get dropped?
================
Comment at: lib/AST/ASTDumper.cpp:1228
- if (D->isThisDeclarationADefinition()) {
+ if (isa<DeclContext>(D) && D->isThisDeclarationADefinition())
dumpDeclContext(D);
----------------
Why is the isa<> test needed? `D` must be an `ObjCMethodDecl` which inherits
from `DeclContext`, so this seems unneeded.
================
Comment at: lib/AST/TextNodeDumper.cpp:261
+ if (!isa<FunctionDecl>(*D)) {
+ auto MD = dyn_cast<ObjCMethodDecl>(D);
+ if (!MD || !MD->isThisDeclarationADefinition()) {
----------------
`const auto *`
================
Comment at: lib/AST/TextNodeDumper.cpp:263
+ if (!MD || !MD->isThisDeclarationADefinition()) {
+ auto DC = dyn_cast<DeclContext>(D);
+ if (DC && DC->hasExternalLexicalStorage()) {
----------------
`const auto *`
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D56829/new/
https://reviews.llvm.org/D56829
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
