Page MenuHomePhabricator

[FileCheck] Factor some parsing checks out

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



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


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.

251 ↗(On Diff #208093)

Nit: is this needed here?

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.
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
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.