This is an archive of the discontinued LLVM Phabricator instance.

[FileCheck] Added --enable-var-scope option to enable scope for regex variables.
ClosedPublic

Authored by tra on Mar 8 2017, 10:44 AM.

Details

Summary

This implements Hal's idea suggested in D30725.

If `--enable-var-scope` is in effect, variables with names that
start with $ are considered to be global. All others variables are
local. All local variables get undefined at the beginning of each
CHECK-LABEL block. Global variables are not affected by CHECK-LABEL.
This makes it easier to ensure that individual tests are not affected
by variables set in preceding tests.

The feature is not enabled by default as there are few hundred tests that depend on variables remaining set across CHECK-LABEL.
Once tests are cleaned up we can flip the flag to be enabled by default.

Event Timeline

tra created this revision.Mar 8 2017, 10:44 AM
jlebar accepted this revision.Mar 8 2017, 1:49 PM
jlebar added inline comments.
docs/CommandGuide/FileCheck.rst
357

Nit, "blocks" aren't really a concept in FileCheck. Maybe we should just say "If `--enable-var-scope` is in effect, all local variables (i.e. variables that don't start with "$") are cleared when a CHECK-LABEL is encountered."

454

Same comment as above about "block".

utils/FileCheck/FileCheck.cpp
80

Same here about "block"

1285

No need for braces on this one.

This revision is now accepted and ready to land.Mar 8 2017, 1:49 PM
tra updated this revision to Diff 91097.Mar 8 2017, 4:03 PM

Removed unneeded {}

tra marked an inline comment as done.Mar 8 2017, 4:04 PM
tra added inline comments.
docs/CommandGuide/FileCheck.rst
357

I've used the block because CHECK-LABEL documentation says (lines 353-355):

...the presence of ``CHECK-LABEL`` divides
the input stream into separate blocks, each of which is processed independently,
preventing a ``CHECK:`` directive in one block matching a line in another block.

I did start with a sentence along the lines of the one you've suggested, but decided to stick with blocks as it seems to be a better fit for something with 'scope'.

jlebar added a comment.Mar 8 2017, 4:39 PM

Heh, that's what I get for not reading the existing docs. sgtm.

hfinkel accepted this revision.Mar 8 2017, 4:48 PM

LGTM too.

This revision was automatically updated to reflect the committed changes.