Page MenuHomePhabricator

[FileCheck] Document FILECHECK_OPTS in -help
ClosedPublic

Authored by jdenny on Aug 3 2019, 4:16 PM.

Diff Detail

Repository
rL LLVM

Event Timeline

jdenny created this revision.Aug 3 2019, 4:16 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 3 2019, 4:16 PM
Herald added a subscriber: thopre. · View Herald Transcript
thopre added inline comments.Aug 5 2019, 8:16 AM
llvm/utils/FileCheck/FileCheck.cpp
27–29 ↗(On Diff #213215)

Do I read this right and the options in effect are the union of those specified in FILECHECK_OPTS and those on the command-line?

jdenny marked an inline comment as done.Aug 5 2019, 8:35 AM
jdenny added inline comments.
llvm/utils/FileCheck/FileCheck.cpp
27–29 ↗(On Diff #213215)

Yes.

Interestingly, FILECHECK_OPTS is parsed before command-line options. However, at the time I implemented FILECHECK_OPTS, the only option I found where the order of multiple occurrences mattered was -D, and it gives precedence to earlier occurrences. That's surely wrong, and I think multiple definitions for the same variable should be an error instead, but I haven't found time to propose something.

thopre accepted this revision.Aug 5 2019, 1:44 PM
thopre marked an inline comment as done.

LGTM.

llvm/utils/FileCheck/FileCheck.cpp
27–29 ↗(On Diff #213215)

Alright, LGTM then. I'm not sure about forbidding multiple definitions of the same variable, I think it makes sense to redefine a variable in a CHECK directive (eg. lots of tests with REG being defined, although --enable-var-scope should probably be used in that case). It makes less sense on the command-line for sure and I would agree making it an error.

This revision is now accepted and ready to land.Aug 5 2019, 1:44 PM
jdenny marked 2 inline comments as done.Aug 5 2019, 1:52 PM
jdenny added inline comments.
llvm/utils/FileCheck/FileCheck.cpp
27–29 ↗(On Diff #213215)

I just meant on the command line. Thanks for the review!

This revision was automatically updated to reflect the committed changes.