[PATCH] D35271: Fix printing policy for AST context loaded from file

2017-08-25 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL311787: [Frontend] Fix printing policy for AST context loaded from file (authored by vedantk). Changed prior to commit: https://reviews.llvm.org/D35271?vs=112334&id=112718#toc Repository: rL LLVM ht

[PATCH] D35271: Fix printing policy for AST context loaded from file

2017-08-25 Thread Johann Klähn via Phabricator via cfe-commits
jklaehn marked 2 inline comments as done. jklaehn added a comment. In https://reviews.llvm.org/D35271#850472, @vsk wrote: > Thanks, LGTM! This seems like a pretty straightforward bug fix. Since it's > not my usual area maybe it'd be worth waiting a day or so for more feedback. Thanks! I do not

[PATCH] D35271: Fix printing policy for AST context loaded from file

2017-08-23 Thread Vedant Kumar via Phabricator via cfe-commits
vsk accepted this revision. vsk added a comment. This revision is now accepted and ready to land. Thanks, LGTM! This seems like a pretty straightforward bug fix. Since it's not my usual area maybe it'd be worth waiting a day or so for more feedback. https://reviews.llvm.org/D35271 __

[PATCH] D35271: Fix printing policy for AST context loaded from file

2017-08-23 Thread Johann Klähn via Phabricator via cfe-commits
jklaehn updated this revision to Diff 112334. jklaehn added a comment. Update regression test to use `createTemporaryFile()` and `tool_output_file` as suggested. https://reviews.llvm.org/D35271 Files: lib/Frontend/ASTUnit.cpp unittests/Frontend/ASTUnitTest.cpp unittests/Frontend/CMakeLis

[PATCH] D35271: Fix printing policy for AST context loaded from file

2017-08-22 Thread Vedant Kumar via Phabricator via cfe-commits
vsk added a comment. Thanks for adding the test! This is looking good. Comment at: unittests/Frontend/ASTUnitTest.cpp:32 +llvm::SmallString<256> Dir; +ASSERT_FALSE(llvm::sys::fs::createUniqueDirectory("astunit-test", Dir)); +TestDir = Dir.str(); If

[PATCH] D35271: Fix printing policy for AST context loaded from file

2017-08-22 Thread Johann Klähn via Phabricator via cfe-commits
jklaehn updated this revision to Diff 112139. jklaehn added a reviewer: akyrtzi. jklaehn added a comment. Herald added a subscriber: mgorny. Added regression test. https://reviews.llvm.org/D35271 Files: lib/Frontend/ASTUnit.cpp unittests/Frontend/ASTUnitTest.cpp unittests/Frontend/CMakeLi

[PATCH] D35271: Fix printing policy for AST context loaded from file

2017-08-21 Thread Vedant Kumar via Phabricator via cfe-commits
vsk added a comment. In https://reviews.llvm.org/D35271#847306, @jklaehn wrote: > In https://reviews.llvm.org/D35271#809159, @vsk wrote: > > > I wonder if it's possible to do away with the calls to 'updated()'... it > > seems strange that we initialize the same preprocessor repeatedly. Is there

[PATCH] D35271: Fix printing policy for AST context loaded from file

2017-08-21 Thread Johann Klähn via Phabricator via cfe-commits
jklaehn added a comment. In https://reviews.llvm.org/D35271#809159, @vsk wrote: > I wonder if it's possible to do away with the calls to 'updated()'... it > seems strange that we initialize the same preprocessor repeatedly. Is there > any way to finalize an ASTInfoCollector after ReadAST happen

[PATCH] D35271: Fix printing policy for AST context loaded from file

2017-07-13 Thread Vedant Kumar via Phabricator via cfe-commits
vsk added a comment. I wonder if it's possible to do away with the calls to 'updated()'... it seems strange that we initialize the same preprocessor repeatedly. Is there any way to finalize an ASTInfoCollector after ReadAST happens (or ASTReaderListeners in general)? https://reviews.llvm.org/

[PATCH] D35271: Fix printing policy for AST context loaded from file

2017-07-11 Thread Johann Klähn via Phabricator via cfe-commits
jklaehn created this revision. jklaehn added a project: clang. In `ASTUnit::LoadFromASTFile`, the context object is set up using default-constructed `LangOptions` (which only later get populated). As the language options are used in the constructor of `PrintingPolicy`, this needs to be updated