This is an archive of the discontinued LLVM Phabricator instance.

[FileCheck] Flip -allow-unused-prefixes to false by default
Needs ReviewPublic

Authored by mtrofin on Nov 10 2020, 8:21 PM.

Details

Summary

Automatically inserted --allow-unused-prefixes=true for tests that are
still failing.

Added a FIXME above each line where this was inserted.

RFC: http://lists.llvm.org/pipermail/llvm-dev/2020-October/146162.html

Diff Detail

Event Timeline

mtrofin created this revision.Nov 10 2020, 8:21 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptNov 10 2020, 8:21 PM
mtrofin requested review of this revision.Nov 10 2020, 8:21 PM

FYI - I realize the change is enormous. I don't necessarily mean to land it as-is, we can chunk it by directories and iteratively update this as those land.

mtrofin edited the summary of this revision. (Show Details)Nov 10 2020, 8:24 PM
mtrofin added a reviewer: dblaikie.

Maybe one way to do this is to do what @RKSimon suggests in the D90281, and to enable it on a per-directory basis using lit.local.cfg. That would potentially require changing the "0 or 1" occurrences limitation of the option, in favour of "last one wins" (or first one). Then, you make the change in the lit.local.cfg, flipping the default in each directory as you go. Eventually, once all tests that need it are covered, you can just change the default in FileCheck itself. How does that sound?

Maybe one way to do this is to do what @RKSimon suggests in the D90281, and to enable it on a per-directory basis using lit.local.cfg. That would potentially require changing the "0 or 1" occurrences limitation of the option, in favour of "last one wins" (or first one). Then, you make the change in the lit.local.cfg, flipping the default in each directory as you go. Eventually, once all tests that need it are covered, you can just change the default in FileCheck itself. How does that sound?

I haven't tried that yet. @RKSimon's case was that the folder was fixed, and we'd want to stop the bleeding there. Now, thinking more about it: because FileCheck won't accept more than one occurrence of --allow-unused-prefixes, if a directory has cases where we want to allow duplicates, we'd probably want lit.local.cfg to do 2 things:

  • set --allow-unused-prefixes to false, to stop the bleeding - this would be removed when we switch defaults
  • define a %UnusedPrefixesFileCheck (or better name) which is FileCheck with the flag set to true, which we'd then use where needed instead of FileCheck. This could stay like this even after we switch defaults.

I'm not very versed with lit.local.cfg - does the above capture what you imagined? (@RKSimon too)

because FileCheck won't accept more than one occurrence of --allow-unused-prefixes

Perhaps this is a fixable issue?

because FileCheck won't accept more than one occurrence of --allow-unused-prefixes

Perhaps this is a fixable issue?

I think I was able to circumvent it in D91275