Re: [PATCH] D56829: Move decl context dumping to TextNodeDumper

2019-01-20 Thread Stephen Kelly via cfe-commits
On 20/01/2019 16:57, Stella Stamenova via Phabricator via cfe-commits wrote: stella.stamenova added a comment. It looks like this change broke a number of the LLDB tests: lab.llvm.org:8011/builders/lldb-x64-windows-ninja/builds/781 Could you look into this or revert the change, so that the bot

[PATCH] D56829: Move decl context dumping to TextNodeDumper

2019-01-20 Thread Stella Stamenova via Phabricator via cfe-commits
stella.stamenova added a comment. It looks like this change broke a number of the LLDB tests: lab.llvm.org:8011/builders/lldb-x64-windows-ninja/builds/781 Could you look into this or revert the change, so that the bot can go back to green? Repository: rC Clang CHANGES SINCE LAST ACTION h

[PATCH] D56829: Move decl context dumping to TextNodeDumper

2019-01-19 Thread Stephen Kelly via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC351637: Move decl context dumping to TextNodeDumper (authored by steveire, committed by ). Changed prior to commit: https://reviews.llvm.org/D56829?vs=182607&id=182669#toc Repository: rC Clang CHANG

[PATCH] D56829: Move decl context dumping to TextNodeDumper

2019-01-18 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM! At some point, we may want to revisit how we dump `TranslationUnitDecl` to make it a bit less inscrutable (I have no idea what those "invalid sloc" strings are telling me). At that point, we may want to turn `` into `unde

[PATCH] D56829: Move decl context dumping to TextNodeDumper

2019-01-18 Thread Stephen Kelly via Phabricator via cfe-commits
steveire updated this revision to Diff 182607. steveire marked an inline comment as done. steveire added a comment. Update Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56829/new/ https://reviews.llvm.org/D56829 Files: lib/AST/ASTDumper.cpp lib/AST/TextNodeD

[PATCH] D56829: Move decl context dumping to TextNodeDumper

2019-01-18 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM aside from the testing situation -- so long as the tests come out without changes, this is good to go. If there is a change in test behavior, let's do another round of revie

[PATCH] D56829: Move decl context dumping to TextNodeDumper

2019-01-18 Thread Stephen Kelly via Phabricator via cfe-commits
steveire updated this revision to Diff 182482. steveire added a comment. Update Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56829/new/ https://reviews.llvm.org/D56829 Files: lib/AST/ASTDumper.cpp lib/AST/TextNodeDumper.cpp Index: lib/AST/TextNodeDumper.c

[PATCH] D56829: Move decl context dumping to TextNodeDumper

2019-01-18 Thread Stephen Kelly via Phabricator via cfe-commits
steveire updated this revision to Diff 182478. steveire added a comment. Update Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56829/new/ https://reviews.llvm.org/D56829 Files: lib/AST/ASTDumper.cpp lib/AST/TextNodeDumper.cpp Index: lib/AST/TextNodeDumper.c

[PATCH] D56829: Move decl context dumping to TextNodeDumper

2019-01-17 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D56829#1361465 , @steveire wrote: > I don't know what C++ code results in the `undeserialized declarations` > output. Can you suggest some? I haven't the foggiest idea. I'd recommend doing an svn blame to see what comm

[PATCH] D56829: Move decl context dumping to TextNodeDumper

2019-01-17 Thread Stephen Kelly via Phabricator via cfe-commits
steveire marked an inline comment as done. steveire added a comment. I don't know what C++ code results in the `undeserialized declarations` output. Can you suggest some? Comment at: lib/AST/ASTDumper.cpp:519-520 - (DC->hasExternalLexicalStorage() || - (Dese

[PATCH] D56829: Move decl context dumping to TextNodeDumper

2019-01-17 Thread Aaron Ballman via Phabricator via cfe-commits
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(*D) && !isa(*D)) { auto DC = dyn_cast(D); - if (DC && - (DC->hasExternalLexicalStorage() || -

[PATCH] D56829: Move decl context dumping to TextNodeDumper

2019-01-17 Thread Stephen Kelly via Phabricator via cfe-commits
steveire created this revision. steveire added a reviewer: aaron.ballman. Herald added a subscriber: cfe-commits. Only an obscure case is moved. Repository: rC Clang https://reviews.llvm.org/D56829 Files: lib/AST/ASTDumper.cpp lib/AST/TextNodeDumper.cpp Index: lib/AST/TextNodeDumper.cp