Page MenuHomePhabricator

FileCheck [12/12]: Support use of var defined on same line
Changes PlannedPublic

Authored by thopre on Apr 7 2019, 4:24 PM.

Details

Summary

This patch is part of a patch series to add support for FileCheck
numeric expressions. This specific patch adds support for using a
numeric variable with empty numeric expression defined on the same line
either directly or indirectly.

The regex language used to represent a constraint corresponding to a
given CHECK directive does not allow to express a numeric relation. It
is therefore necessary to initially match any number in the appropriate
matching format and check the constraints are verified in a follow-up
step. Such a process must then be repeated as long as the constraints
fail because of cases with a check directive like #N: #N+1
where the input contains lines such as:
10 12
10 11

The first line would match the regex "[0-9]+ [0-9]+" generated for the
CHECK directive but the constraint would fail. The second line would
then need to be tried since it both matches successfully and verifies
the constraints of the CHECK directive. Note that this process would
still fail to match if the input was:
10 12 13
because the regex would match 10 and 12 which fail to verify the
constraints and the process would then skip to the next line since there
is no way to ask the regex engine if the line had other possible
matches. This caveat is documented.

Copyright:

  • Linaro (changes up to diff 183612 of revision D55940)
  • GraphCore (changes in later versions of revision D55940 and in new revision created off D55940)

Event Timeline

thopre created this revision.Apr 7 2019, 4:24 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 7 2019, 4:24 PM
Herald added a subscriber: hiraditya. · View Herald Transcript
thopre updated this revision to Diff 194323.Apr 9 2019, 7:35 AM

Use camel casing for new functions

thopre updated this revision to Diff 194488.Apr 10 2019, 4:45 AM

Make numeric value constructor explicit

tra removed a subscriber: tra.Apr 11 2019, 1:16 PM
thopre updated this revision to Diff 197124.Apr 29 2019, 8:42 AM

Rebase on latest changes

thopre updated this revision to Diff 198039.May 3 2019, 9:50 AM

Rebase on latest changes

thopre updated this revision to Diff 198101.May 3 2019, 3:03 PM

Rebase on latest unittest changes

thopre updated this revision to Diff 198448.May 7 2019, 5:36 AM

Rebase on top of latest changes

thopre updated this revision to Diff 198554.May 7 2019, 3:32 PM

Rebase on top of latest changes

thopre updated this revision to Diff 198650.May 8 2019, 6:54 AM

Rebase on top of latest changes

thopre updated this revision to Diff 198955.May 9 2019, 4:56 PM

Rebase on top of latest changes

thopre updated this revision to Diff 199143.May 11 2019, 7:18 AM

Rebase on top of latest changes

thopre updated this revision to Diff 199241.May 13 2019, 3:59 AM

Rebase on top of latest changes

thopre updated this revision to Diff 199401.May 14 2019, 4:08 AM

Rebase on top of latest changes

rnk removed a reviewer: rnk.May 14 2019, 11:28 AM
thopre updated this revision to Diff 200478.May 21 2019, 5:56 AM

rebase on top of latest changes

thopre updated this revision to Diff 200555.May 21 2019, 11:15 AM

rebase on top of latest changes

thopre updated this revision to Diff 201987.May 29 2019, 10:53 AM

Rebase on top of latest changes in patch series

thopre updated this revision to Diff 203214.Jun 5 2019, 11:22 AM

Rebase on top of latest changes in the series

thopre updated this revision to Diff 205685.Jun 19 2019, 3:23 PM

Rebase on top of latest changes in patch series

thopre planned changes to this revision.May 11 2020, 3:09 PM

Codebase has changed considerably since last time this review was updated. Patch will need some reworking and before that I want to build consensus on whether the feature should be done given the caveats it has.

I don't have a clear solution to the problems you're describing, but I have already run into one or two places where I wanted the behaviour of being able to use a numeric variable on the same line it was defined. CHECK-SAME is a valid workaround, but has its own issues (for example, it doesn't inherently ensure the next match starts immediately after the previous one, and is just simply more verbose). My instinct is that the right approach is to ignore the constraint initially, match a number, and then verify that it is the right number, failing if not, without further search attempts. I think it's relatively easy to workaround that issue in most cases, with additional checks elsewhere, although I can't be positive that is the case.

I don't have a clear solution to the problems you're describing, but I have already run into one or two places where I wanted the behaviour of being able to use a numeric variable on the same line it was defined. CHECK-SAME is a valid workaround, but has its own issues (for example, it doesn't inherently ensure the next match starts immediately after the previous one, and is just simply more verbose). My instinct is that the right approach is to ignore the constraint initially, match a number, and then verify that it is the right number, failing if not, without further search attempts. I think it's relatively easy to workaround that issue in most cases, with additional checks elsewhere, although I can't be positive that is the case.

What worries me is the effect on a CHECK-NOT directive since it could lead to the directive being fulfilled in a case where the intent was to forbid it. Consider the following:

CHECK-NOT: [[#VAR:]] [[#VAR+1]]

Say the author of the test was looking a a bug where the output being checked was 10 11 14. The test would pass. Now if the bug wasn't fully fixed, perhaps this doesn't happen anymore but we can have cases like 10 13 14. The above directive will turn into ([[:digit:]]+) ([[:digit:]]+) which will match 10 and 13 and will then check if they are consecutive numbers. They are not and thus the CHECK-NOT is satisfied even though there was 13 14 later on the line. Of course here we could retry after the first number matched but that doesn't work in the general case where regex can appear anywhere (e.g. #VAR: {{.*}} #VAR+1 where the input is now 10 8 11).

Perhaps the solution is to forbid using a variable defined on the same line for a CHECK-NOT directive? In all other cases we'll get a false negative (directive not satisfied) instead of a false positive (directive satisfied).

Perhaps the solution is to forbid using a variable defined on the same line for a CHECK-NOT directive? In all other cases we'll get a false negative (directive not satisfied) instead of a false positive (directive satisfied).

Hmm, you make a fair argument, and I think this suggestion might work. I'll need to give it more thought though. Perhaps it needs a mailing list thread to get more eyes on this?

Perhaps the solution is to forbid using a variable defined on the same line for a CHECK-NOT directive? In all other cases we'll get a false negative (directive not satisfied) instead of a false positive (directive satisfied).

Hmm, you make a fair argument, and I think this suggestion might work. I'll need to give it more thought though. Perhaps it needs a mailing list thread to get more eyes on this?

That was my intention. I'm focusing on D60390 for now, then I'll start a thread.