This is an archive of the discontinued LLVM Phabricator instance.

[test] Modify test case so it verifies the fix from D126396
ClosedPublic

Authored by ppluzhnikov on Jun 3 2022, 3:22 PM.

Diff Detail

Event Timeline

ppluzhnikov created this revision.Jun 3 2022, 3:22 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 3 2022, 3:22 PM
Herald added a subscriber: StephenFan. · View Herald Transcript
ppluzhnikov requested review of this revision.Jun 3 2022, 3:22 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 3 2022, 3:22 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript
MaskRay accepted this revision.EditedJun 3 2022, 4:07 PM

Thanks! The convention is to add [test] to test-only patches.
The subject line doesn't need a trailing .

You can change the Phabricator title, then run arc amend to amend the local commit.
I believe on Debian testing arc is somewhat broken with php 8, so you may need to get arc from https://github.com/phacility/arcanist (bin/arc)

clang/test/Preprocessor/file_test.c
16

This ./ line needs a comment about what is being tested.

This revision is now accepted and ready to land.Jun 3 2022, 4:07 PM
ppluzhnikov marked an inline comment as done.

Add comment and suppress clang-format.

Make comment better.

ppluzhnikov retitled this revision from Modify test case so it verifies the fix from D126396. to [test] Modify test case so it verifies the fix from D126396.Jun 3 2022, 4:32 PM

I think I've done the cleanups requested.

If this looks good, please push it (I don't have commit rights).

Thanks,