Mircea Trofin via llvm-dev
2020-Nov-05 15:30 UTC
[llvm-dev] [RFC] FileCheck: (dis)allowing unused prefixes
There are currently 1350 owner-less failures in the spreadsheet <https://docs.google.com/spreadsheets/d/1o6q3XH1n3DDyyccnYZ_kVfhFbTDzC_S09e973_cwYuw/edit#gid=0>. These seem to be the larger areas there. If you see an area you have ownership or expertise in, please sign up for fixing the tests by Monday, Nov. 9. Otherwise, I will "blanket-add" --allow-unused-prefixes=true to the remaining failing tests. On Fri, Oct 30, 2020 at 12:48 PM Mircea Trofin <mtrofin at google.com> wrote:> An update: as of 871d658c9ceb, the flag is now available, if folks need to > use it. > > On Thu, Oct 29, 2020 at 10:28 AM Mircea Trofin <mtrofin at google.com> wrote: > >> Hello all, >> >> TL;DR; if you used FileCheck --check-prefixes and you missed (misspelled, >> for instance) one of the prefixes in your test, FileCheck silently ignores >> that and the test passes. >> >> 1579 tests have this property. >> >> >> *The details* >> *=========* >> Please refer to https://reviews.llvm.org/D90281 and the discussion there >> for more details (make sure you open "older changes" for full context) >> >> The problem is covered by the TL;DR;. >> >> The proposal is to add an explicit flag to FileCheck, >> --allow-unused-prefixes, to indicate whether the current behavior is >> intended (for instance, jdoerfert contributed a scenario where that is the >> case). >> >> We want the default behavior to be 'strict', i.e. >> --allow-unused-prefixes=false. Doing that right now would lead to 1500 test >> failures. >> >> To get there (thanks, maskray, for suggestion), we propose we: >> * land D90281 where the flag is introduced, but is flipped to match >> today's behavior >> * employ a 'busy beavers' approach, where test maintainers patch their >> tests: >> - either leveraging the flag, to explicitly indicate that unused >> prefixes is intended (i.e. add --allow-unused-patches=true); or >> - fix the test (e.g. maybe there was a misspelling issue/omission/etc). >> >> A spreadsheet with the failing tests is available here >> <https://docs.google.com/spreadsheets/d/1o6q3XH1n3DDyyccnYZ_kVfhFbTDzC_S09e973_cwYuw/edit?usp=sharing> >> [1]. >> >> The request to the community members is to please sign up for their >> respective area in the spreadsheet, and then mark it completed when that's >> the case (yes/no in the respective column). >> >> When all the tests are fixed, we will then flip --allow-unused-prefixes >> to false by default. >> >> Meanwhile, please consider leveraging the flag explicitly when you author >> new tests that use --check-prefixes. That can be then cleaned up easily >> after we switch to the 'strict' behavior. >> >> Thanks! >> >> [1] >> https://docs.google.com/spreadsheets/d/1o6q3XH1n3DDyyccnYZ_kVfhFbTDzC_S09e973_cwYuw/edit?usp=sharing >> >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20201105/7d2f4f72/attachment.html>
David Blaikie via llvm-dev
2020-Nov-05 18:40 UTC
[llvm-dev] [RFC] FileCheck: (dis)allowing unused prefixes
On Thu, Nov 5, 2020 at 7:30 AM Mircea Trofin via llvm-dev < llvm-dev at lists.llvm.org> wrote:> There are currently 1350 owner-less failures in the spreadsheet > <https://docs.google.com/spreadsheets/d/1o6q3XH1n3DDyyccnYZ_kVfhFbTDzC_S09e973_cwYuw/edit#gid=0>. > These seem to be the larger areas there. > > If you see an area you have ownership or expertise in, please sign up for > fixing the tests by Monday, Nov. 9. > > Otherwise, I will "blanket-add" --allow-unused-prefixes=true to the > remaining failing tests. >If/when you do that, probably worth adding a comment at each site to clarify that this was added automatically, not vetted/intentionally added by a human. Something like "// FIXME: Verify that unused prefixes are used intentionally" or the like.> > > On Fri, Oct 30, 2020 at 12:48 PM Mircea Trofin <mtrofin at google.com> wrote: > >> An update: as of 871d658c9ceb, the flag is now available, if folks need >> to use it. >> >> On Thu, Oct 29, 2020 at 10:28 AM Mircea Trofin <mtrofin at google.com> >> wrote: >> >>> Hello all, >>> >>> TL;DR; if you used FileCheck --check-prefixes and you missed >>> (misspelled, for instance) one of the prefixes in your test, FileCheck >>> silently ignores that and the test passes. >>> >>> 1579 tests have this property. >>> >>> >>> *The details* >>> *=========* >>> Please refer to https://reviews.llvm.org/D90281 and the discussion >>> there for more details (make sure you open "older changes" for full context) >>> >>> The problem is covered by the TL;DR;. >>> >>> The proposal is to add an explicit flag to FileCheck, >>> --allow-unused-prefixes, to indicate whether the current behavior is >>> intended (for instance, jdoerfert contributed a scenario where that is the >>> case). >>> >>> We want the default behavior to be 'strict', i.e. >>> --allow-unused-prefixes=false. Doing that right now would lead to 1500 test >>> failures. >>> >>> To get there (thanks, maskray, for suggestion), we propose we: >>> * land D90281 where the flag is introduced, but is flipped to match >>> today's behavior >>> * employ a 'busy beavers' approach, where test maintainers patch their >>> tests: >>> - either leveraging the flag, to explicitly indicate that unused >>> prefixes is intended (i.e. add --allow-unused-patches=true); or >>> - fix the test (e.g. maybe there was a misspelling issue/omission/etc). >>> >>> A spreadsheet with the failing tests is available here >>> <https://docs.google.com/spreadsheets/d/1o6q3XH1n3DDyyccnYZ_kVfhFbTDzC_S09e973_cwYuw/edit?usp=sharing> >>> [1]. >>> >>> The request to the community members is to please sign up for their >>> respective area in the spreadsheet, and then mark it completed when that's >>> the case (yes/no in the respective column). >>> >>> When all the tests are fixed, we will then flip --allow-unused-prefixes >>> to false by default. >>> >>> Meanwhile, please consider leveraging the flag explicitly when you >>> author new tests that use --check-prefixes. That can be then cleaned up >>> easily after we switch to the 'strict' behavior. >>> >>> Thanks! >>> >>> [1] >>> https://docs.google.com/spreadsheets/d/1o6q3XH1n3DDyyccnYZ_kVfhFbTDzC_S09e973_cwYuw/edit?usp=sharing >>> >> _______________________________________________ > LLVM Developers mailing list > llvm-dev at lists.llvm.org > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20201105/0809e2c7/attachment.html>
Mircea Trofin via llvm-dev
2020-Nov-05 18:45 UTC
[llvm-dev] [RFC] FileCheck: (dis)allowing unused prefixes
On Thu, Nov 5, 2020 at 10:40 AM David Blaikie <dblaikie at gmail.com> wrote:> > > On Thu, Nov 5, 2020 at 7:30 AM Mircea Trofin via llvm-dev < > llvm-dev at lists.llvm.org> wrote: > >> There are currently 1350 owner-less failures in the spreadsheet >> <https://docs.google.com/spreadsheets/d/1o6q3XH1n3DDyyccnYZ_kVfhFbTDzC_S09e973_cwYuw/edit#gid=0>. >> These seem to be the larger areas there. >> >> If you see an area you have ownership or expertise in, please sign up for >> fixing the tests by Monday, Nov. 9. >> >> Otherwise, I will "blanket-add" --allow-unused-prefixes=true to the >> remaining failing tests. >> > > If/when you do that, probably worth adding a comment at each site to > clarify that this was added automatically, not vetted/intentionally added > by a human. Something like "// FIXME: Verify that unused prefixes are used > intentionally" or the like. >Ack. or, we can grep for -allow-unused-prefixes=true, wdyt?> > >> >> >> On Fri, Oct 30, 2020 at 12:48 PM Mircea Trofin <mtrofin at google.com> >> wrote: >> >>> An update: as of 871d658c9ceb, the flag is now available, if folks need >>> to use it. >>> >>> On Thu, Oct 29, 2020 at 10:28 AM Mircea Trofin <mtrofin at google.com> >>> wrote: >>> >>>> Hello all, >>>> >>>> TL;DR; if you used FileCheck --check-prefixes and you missed >>>> (misspelled, for instance) one of the prefixes in your test, FileCheck >>>> silently ignores that and the test passes. >>>> >>>> 1579 tests have this property. >>>> >>>> >>>> *The details* >>>> *=========* >>>> Please refer to https://reviews.llvm.org/D90281 and the discussion >>>> there for more details (make sure you open "older changes" for full context) >>>> >>>> The problem is covered by the TL;DR;. >>>> >>>> The proposal is to add an explicit flag to FileCheck, >>>> --allow-unused-prefixes, to indicate whether the current behavior is >>>> intended (for instance, jdoerfert contributed a scenario where that is the >>>> case). >>>> >>>> We want the default behavior to be 'strict', i.e. >>>> --allow-unused-prefixes=false. Doing that right now would lead to 1500 test >>>> failures. >>>> >>>> To get there (thanks, maskray, for suggestion), we propose we: >>>> * land D90281 where the flag is introduced, but is flipped to match >>>> today's behavior >>>> * employ a 'busy beavers' approach, where test maintainers patch their >>>> tests: >>>> - either leveraging the flag, to explicitly indicate that unused >>>> prefixes is intended (i.e. add --allow-unused-patches=true); or >>>> - fix the test (e.g. maybe there was a misspelling >>>> issue/omission/etc). >>>> >>>> A spreadsheet with the failing tests is available here >>>> <https://docs.google.com/spreadsheets/d/1o6q3XH1n3DDyyccnYZ_kVfhFbTDzC_S09e973_cwYuw/edit?usp=sharing> >>>> [1]. >>>> >>>> The request to the community members is to please sign up for their >>>> respective area in the spreadsheet, and then mark it completed when that's >>>> the case (yes/no in the respective column). >>>> >>>> When all the tests are fixed, we will then flip --allow-unused-prefixes >>>> to false by default. >>>> >>>> Meanwhile, please consider leveraging the flag explicitly when you >>>> author new tests that use --check-prefixes. That can be then cleaned up >>>> easily after we switch to the 'strict' behavior. >>>> >>>> Thanks! >>>> >>>> [1] >>>> https://docs.google.com/spreadsheets/d/1o6q3XH1n3DDyyccnYZ_kVfhFbTDzC_S09e973_cwYuw/edit?usp=sharing >>>> >>> _______________________________________________ >> LLVM Developers mailing list >> llvm-dev at lists.llvm.org >> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >> >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20201105/b09c424e/attachment.html>