Page MenuHomePhabricator

[NFC] [FileCheck] Fix init of objects in unit tests
ClosedPublic

Authored by thopre on Oct 3 2019, 2:31 PM.

Diff Detail

Repository
rL LLVM

Event Timeline

thopre created this revision.Oct 3 2019, 2:31 PM
Herald added a project: Restricted Project. · View Herald TranscriptOct 3 2019, 2:31 PM
jhenderson accepted this revision.Oct 4 2019, 1:07 AM

LGTM, since it makes things more readable. Beyond that, is there any particular motivation for this?

This revision is now accepted and ready to land.Oct 4 2019, 1:07 AM
grimar added a comment.Oct 4 2019, 1:38 AM

LGTM, this makes the code cleaner.
(btw, you can add "NFC" to the title it seems).

grimar accepted this revision.Oct 4 2019, 1:38 AM
thopre added a comment.Oct 4 2019, 1:47 AM

LGTM, since it makes things more readable. Beyond that, is there any particular motivation for this?

Several comments in https://reviews.llvm.org/D60389 asking to fix the same issue. Since there was already several instances of the issue in existing code I thought I'd fix them.

thopre retitled this revision from [FileCheck] Fix init of stack objects in unit tests to [NFC] [FileCheck] Fix init of stack objects in unit tests.Oct 4 2019, 1:57 AM
This revision was automatically updated to reflect the committed changes.

Sorry, but this change broke the build: http://lab.llvm.org:8011/builders/clang-x86_64-debian-fast/builds/18721

Please always run ninja check-all before committing.

I reverted this change in r373722.

thopre added a comment.Oct 4 2019, 6:00 AM

Sorry, but this change broke the build: http://lab.llvm.org:8011/builders/clang-x86_64-debian-fast/builds/18721

Please always run ninja check-all before committing.

I reverted this change in r373722.

I did ran check check-llvm-unit which runs this test. Is it possible that the unit test is not run using the compiler just built?

thopre added a comment.Oct 4 2019, 6:55 AM

Sorry, but this change broke the build: http://lab.llvm.org:8011/builders/clang-x86_64-debian-fast/builds/18721

Please always run ninja check-all before committing.

I reverted this change in r373722.

I did ran check check-llvm-unit which runs this test. Is it possible that the unit test is not run using the compiler just built?

My bad, I can reproduce this, I might have just run ninja which of course does not do anything here.

thopre reopened this revision.Oct 4 2019, 7:00 AM
This revision is now accepted and ready to land.Oct 4 2019, 7:00 AM
thopre updated this revision to Diff 223211.Oct 4 2019, 7:01 AM

Fix syntax for member initialization

thopre retitled this revision from [NFC] [FileCheck] Fix init of stack objects in unit tests to [NFC] [FileCheck] Fix init of objects in unit tests.Oct 4 2019, 7:02 AM
thopre edited the summary of this revision. (Show Details)
thopre requested review of this revision.Oct 4 2019, 7:04 AM

Ok to commit this revised change? I double checked that I ran the unit tests this time and all tests pass.

Ok to commit this revised change? I double checked that I ran the unit tests this time and all tests pass.

For simple changes to fix a bot breakage, it's generally fine to just recommit the updated fix; people also like to see a comment in the commit log for the recommit that says something about what's different this time or why it's okay to recommit.

thopre added a comment.Oct 4 2019, 8:46 AM

Ok to commit this revised change? I double checked that I ran the unit tests this time and all tests pass.

For simple changes to fix a bot breakage, it's generally fine to just recommit the updated fix; people also like to see a comment in the commit log for the recommit that says something about what's different this time or why it's okay to recommit.

Noted, thanks!

This revision was not accepted when it landed; it landed in state Needs Review.Oct 4 2019, 8:46 AM
This revision was automatically updated to reflect the committed changes.