This is an archive of the discontinued LLVM Phabricator instance.

[NFC] Make file offsets a regex to handle CRLF
ClosedPublic

Authored by beanz on Feb 9 2022, 11:37 AM.

Details

Summary

None of these tests are really intended to test the file offset as much
as to test the structure. Making the regex allows this test to work
even if the file is checked out with CRLF line endings.

Diff Detail

Event Timeline

beanz requested review of this revision.Feb 9 2022, 11:37 AM
beanz created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptFeb 9 2022, 11:37 AM
beanz added a comment.Feb 9 2022, 11:41 AM

I realize this is a _big_ silly NFC change, but I figured I'd blast it out there in case anyone objects to the premise.

My goal is to eventually get to the point where Windows developers can use autocrlf (see the note here: https://llvm.org/docs/GettingStarted.html#checkout-llvm-from-git).

My assertion is that the file offset isn't the important thing any of these tests are testing, so making that match fuzzy makes the tests less fragile and get them working with CRLF line endings.

aaron.ballman accepted this revision.Feb 9 2022, 11:59 AM

I think this NFC change is reasonable. We do lose a bit of JSON dumping test coverage from it (I don't see any tests for the offset there now), but I am okay with that as I don't think offsets are something that can be relied upon to be stable values anyway (we pick new source locations for things as we improve fidelity in the compiler).

LGTM, thank you for this!

This revision is now accepted and ready to land.Feb 9 2022, 11:59 AM
This revision was landed with ongoing or failed builds.Feb 10 2022, 6:39 PM
This revision was automatically updated to reflect the committed changes.