Page MenuHomePhabricator

[ASTImporter] Eliminated some unittest warnings.
ClosedPublic

Authored by balazske on May 29 2018, 1:59 AM.

Details

Summary

When running the ASTTests test, warnings produced by the compiler can be
distracting when looking for test errors. A part of the warnings is removed
by setting extra compiler options.

Diff Detail

Repository
rL LLVM

Event Timeline

balazske created this revision.May 29 2018, 1:59 AM
a.sidorin requested changes to this revision.May 29 2018, 3:00 AM

Hello Balázs!

Thank you for addressing this problem, it is really cool. However, it will be much better if we don't suppress warnings but fix them. Could you modify the tests instead?

This revision now requires changes to proceed.May 29 2018, 3:00 AM

Original idea was to not modify the test code to keep it as simple as possible. If you like it better I will change the test code.

balazske updated this revision to Diff 149079.May 30 2018, 3:44 AM

[ASTImporter] Fixed test code in ASTImporter tests.

Test code is fixed instead of turn off the warnings.
For this to work change of match expressions was needed.

martong removed a reviewer: balazske.

Hello Balázs,

The patch is mostly LG now, thank you!

unittests/AST/ASTImporterTest.cpp
495 ↗(On Diff #149079)

Redundant space after cast.

541 ↗(On Diff #149079)

Commented code should be removed.

802 ↗(On Diff #149079)

Sometimes we turn to usehasDescendant(), sometimes we add a cast matcher. Can we make it consistent?

balazske updated this revision to Diff 152632.Jun 25 2018, 1:15 AM

[ASTImporter] Fixed test code in ASTImporter tests.

Reformatted some test code, changed to use isDescendant.

balazske marked 2 inline comments as done.Jun 25 2018, 1:17 AM

Some of the source code was reformatted for more consistency. At least a part of the code is now better formatted.

a.sidorin accepted this revision.Jun 26 2018, 2:56 AM

Thank you!

When committing, please change the commit message (the current review description is out-of-date) and mention the reformatting done.

This revision is now accepted and ready to land.Jun 26 2018, 2:56 AM
balazske updated this revision to Diff 153015.Jun 27 2018, 1:15 AM

Rebase to newest master.

This revision was automatically updated to reflect the committed changes.