Page MenuHomePhabricator

[FileCheck] Factor some parsing checks out
ClosedPublic

Authored by thopre on Jul 4 2019, 4:48 PM.

Details

Summary

Both callers of parseNumericVariableDefinition() perform the same extra
check that no character is found after the variable name. This patch
factors out this check into parseNumericVariableDefinition().

Diff Detail

Repository
rL LLVM

Event Timeline

thopre created this revision.Jul 4 2019, 4:48 PM
jhenderson accepted this revision.Jul 5 2019, 1:26 AM

LGTM, assuming the error position difference is not an issue.

llvm/lib/Support/FileCheck.cpp
251 ↗(On Diff #208093)

Nit: is this needed here?

llvm/test/FileCheck/numeric-defines-diagnostics.txt
23 ↗(On Diff #208093)

It's slightly disconcerting that the column number here has changed. I assume that it's just some difference between the start and end of the string or something?

This revision is now accepted and ready to land.Jul 5 2019, 1:26 AM
arichardson accepted this revision.Jul 5 2019, 1:37 AM
arichardson added inline comments.
llvm/test/FileCheck/numeric-defines-diagnostics.txt
23 ↗(On Diff #208093)

It seems like the diagnostic now points to the invalid character (+) instead of the start of the name. I think this makes more sense.

thopre updated this revision to Diff 208148.Jul 5 2019, 4:58 AM
thopre marked 4 inline comments as done.

Address review comments

thopre added inline comments.Jul 5 2019, 5:01 AM
llvm/test/FileCheck/numeric-defines-diagnostics.txt
23 ↗(On Diff #208093)

As Alexander said, it was pointing to the start of the variable before (saying basically that the whole "VALUE+2" is an invalid variable name) and is now pointing to the + operator because that's the first unexpected character.

This revision was automatically updated to reflect the committed changes.