Re: [PATCH] D20369: [ASTMatcher] Fix a ASTMatcher test failure on Windows.

2016-05-18 Thread Haojian Wu via cfe-commits
hokein marked 2 inline comments as done. Comment at: unittests/ASTMatchers/ASTMatchersNodeTest.cpp:187 @@ +186,3 @@ + "}", + unresolvedLookupExpr(), true, +

Re: [PATCH] D20369: [ASTMatcher] Fix a ASTMatcher test failure on Windows.

2016-05-18 Thread Alexander Kornienko via cfe-commits
alexfh added inline comments. Comment at: unittests/ASTMatchers/ASTMatchersNodeTest.cpp:187 @@ +186,3 @@ + "}", + unresolvedLookupExpr(), true, +

Re: [PATCH] D20369: [ASTMatcher] Fix a ASTMatcher test failure on Windows.

2016-05-18 Thread Haojian Wu via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL269936: [ASTMatcher] Fix a ASTMatcher test failure on Windows. (authored by hokein). Changed prior to commit: http://reviews.llvm.org/D20369?vs=57631=57633#toc Repository: rL LLVM

Re: [PATCH] D20369: [ASTMatcher] Fix a ASTMatcher test failure on Windows.

2016-05-18 Thread Haojian Wu via cfe-commits
hokein updated this revision to Diff 57631. hokein added a comment. Only use -fno-delayed-template-parsing in the testcase. http://reviews.llvm.org/D20369 Files: unittests/ASTMatchers/ASTMatchersNodeTest.cpp Index: unittests/ASTMatchers/ASTMatchersNodeTest.cpp

Re: [PATCH] D20369: [ASTMatcher] Fix a ASTMatcher test failure on Windows.

2016-05-18 Thread Nico Weber via cfe-commits
thakis added a comment. (If so, maybe add a FIXME comment to make things work without delayed template parsing. In any case, getting the bot green is the most important thing, so landing this as is is definitely fine.) http://reviews.llvm.org/D20369

Re: [PATCH] D20369: [ASTMatcher] Fix a ASTMatcher test failure on Windows.

2016-05-18 Thread Nico Weber via cfe-commits
thakis added a comment. Ah, you answered my question while I was writing it. Doesn't that mean whatever feature this test is testing is broken on Windows? http://reviews.llvm.org/D20369 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

Re: [PATCH] D20369: [ASTMatcher] Fix a ASTMatcher test failure on Windows.

2016-05-18 Thread Nico Weber via cfe-commits
thakis added inline comments. Comment at: unittests/ASTMatchers/ASTMatchersNodeTest.cpp:187 @@ +186,3 @@ + "}", + unresolvedLookupExpr(), true, +

Re: [PATCH] D20369: [ASTMatcher] Fix a ASTMatcher test failure on Windows.

2016-05-18 Thread Haojian Wu via cfe-commits
hokein added a comment. In http://reviews.llvm.org/D20369#433141, @thakis wrote: > Maybe you could change Matcher.UnresolvedLookupExpr to call bar() from a > new function foo() so that it gets instantiated? This will break the intention of the testcase. We don't want the function get

Re: [PATCH] D20369: [ASTMatcher] Fix a ASTMatcher test failure on Windows.

2016-05-18 Thread Aaron Ballman via cfe-commits
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. I like this approach much better, thank you. LGTM! http://reviews.llvm.org/D20369 ___ cfe-commits mailing list

Re: [PATCH] D20369: [ASTMatcher] Fix a ASTMatcher test failure on Windows.

2016-05-18 Thread Nico Weber via cfe-commits
thakis added a subscriber: thakis. thakis added a comment. I agree with Aaron. Maybe you could change Matcher.UnresolvedLookupExpr to call bar() from a new function foo() so that it gets instantiated? http://reviews.llvm.org/D20369 ___ cfe-commits

Re: [PATCH] D20369: [ASTMatcher] Fix a ASTMatcher test failure on Windows.

2016-05-18 Thread Aaron Ballman via cfe-commits
aaron.ballman added a comment. Hrm, I kind of worry about this masking bugs when delayed template parsing is enabled (which it is by default on MSVC-built versions of clang). http://reviews.llvm.org/D20369 ___ cfe-commits mailing list

[PATCH] D20369: [ASTMatcher] Fix a ASTMatcher test failure on Windows.

2016-05-18 Thread Haojian Wu via cfe-commits
hokein created this revision. hokein added reviewers: alexfh, aaron.ballman. hokein added a subscriber: cfe-commits. Herald added a subscriber: klimek. http://reviews.llvm.org/D20369 Files: unittests/ASTMatchers/ASTMatchersTest.h Index: unittests/ASTMatchers/ASTMatchersTest.h