This is an archive of the discontinued LLVM Phabricator instance.

Dividied ASTMatcherTests into 4 files
ClosedPublic

Authored by Prazek on May 12 2016, 9:07 AM.

Details

Reviewers
klimek
alexfh
Summary

Fix for 20061

Diff Detail

Event Timeline

Prazek updated this revision to Diff 57050.May 12 2016, 9:07 AM
Prazek retitled this revision from to Dividied ASTMatcherTests into 4 files.
Prazek updated this object.
Prazek added reviewers: alexfh, klimek.
Prazek set the repository for this revision to rL LLVM.
alexfh requested changes to this revision.May 12 2016, 9:20 AM
alexfh edited edge metadata.
alexfh added inline comments.
unittests/ASTMatchers/ASTTraversalMatchersTest.cpp
21 ↗(On Diff #57050)

Too many empty lines.

1982 ↗(On Diff #57050)

Ensure there's a newline at the end of file.

unittests/ASTMatchers/NarrowingMatchersTest.cpp
316 ↗(On Diff #57050)

Too many empty lines.

This revision now requires changes to proceed.May 12 2016, 9:20 AM

BTW can I reformat the files? there are some places that clang-format find inappropriate.

Prazek marked 3 inline comments as done.May 12 2016, 9:44 AM
Prazek updated this revision to Diff 57061.May 12 2016, 9:46 AM
Prazek edited edge metadata.
Prazek removed rL LLVM as the repository for this revision.

We should have split the file loooong ago. Thanks for doing this!

One comment re: file names. Would it be better to leave "ASTMatchersTest" or "ASTMatchers" prefix for all file names you create? E.g. "ASTMatchersTestTraversal.cpp", "ASTMatchersTestNarrowing.cpp", ....

We should have split the file loooong ago. Thanks for doing this!

One comment re: file names. Would it be better to leave "ASTMatchersTest" or "ASTMatchers" prefix for all file names you create? E.g. "ASTMatchersTestTraversal.cpp", "ASTMatchersTestNarrowing.cpp", ....

I guess it would be better to do it the same way as in unit tests of AST - like ASTContextParentMapTest.cpp or ASTVectorTest.cpp
I think "Test" suffix is must have, so I can chcange it to ASTMatchersTraversalTest.cpp, ASTMatchersNarrowingTest.cpp

We should have split the file loooong ago. Thanks for doing this!

One comment re: file names. Would it be better to leave "ASTMatchersTest" or "ASTMatchers" prefix for all file names you create? E.g. "ASTMatchersTestTraversal.cpp", "ASTMatchersTestNarrowing.cpp", ....

I guess it would be better to do it the same way as in unit tests of AST - like ASTContextParentMapTest.cpp or ASTVectorTest.cpp
I think "Test" suffix is must have, so I can chcange it to ASTMatchersTraversalTest.cpp, ASTMatchersNarrowingTest.cpp

SG

Prazek updated this revision to Diff 57447.May 17 2016, 2:35 AM
Prazek edited edge metadata.

rebased with master
Added tests for matchers hasCastKind and hasDynamicExceptionSpec()

alexfh accepted this revision.May 17 2016, 11:56 AM
alexfh edited edge metadata.

LG. Thank you!

unittests/ASTMatchers/ASTMatchersNodeTest.cpp
21

nit: Too many empty lines.

34

nit: Too many empty lines.

290

nit: Too many empty lines.

524

nit: Too many empty lines.

This revision is now accepted and ready to land.May 17 2016, 11:56 AM
Prazek closed this revision.May 17 2016, 12:33 PM
Prazek marked 4 inline comments as done.
unittests/ASTMatchers/ASTMatchersTest.h