James Henderson via llvm-dev
2020-Nov-06 15:05 UTC
[llvm-dev] [RFC] FileCheck: (dis)allowing unused prefixes
I recently discovered that multi-line RUN statements can actually be interrupted with non-RUN lines, without changing the behaviour. In other words, you can do something like: # RUN: some command --option1 \ ## Comment # CHECK: check something # RUN: --option2 And you'd end up with "some command --option1 --option2" being run. It's rather surprising behaviour, and not one I'd generally recommend exercising, but maybe in this context it would be okay? On Fri, 6 Nov 2020 at 15:00, Mircea Trofin <mtrofin at google.com> wrote:> > > On Fri, Nov 6, 2020 at 2:22 AM James Henderson < > jh7370.2008 at my.bristol.ac.uk> wrote: > >> I think it would make more sense to add it at each individual call site. >> This ensures that all cases are fixed, rather than just one in a file. It >> also ensures that in the (hopefully unlikely) event that there are both >> intentional and unintentional use-cases within a file, each one gets >> checked. >> >> That would be better indeed, but may be trickier to automate - the > challenge, in my mind, comes from multi-line RUN statements, but maybe I'm > missing something / maybe there aren't that many. If folks have > suggestions, please don't hesitate to throw them my way! > > Thanks! > > >> On Thu, 5 Nov 2020 at 20:29, Mircea Trofin via llvm-dev < >> llvm-dev at lists.llvm.org> wrote: >> >>> Thanks! >>> >>> On Thu, Nov 5, 2020 at 11:50 AM Fāng-ruì Sòng <maskray at google.com> >>> wrote: >>> >>>> On Thu, Nov 5, 2020 at 11:36 AM David Blaikie via llvm-dev >>>> <llvm-dev at lists.llvm.org> wrote: >>>> > >>>> > >>>> > >>>> > On Thu, Nov 5, 2020 at 10:46 AM Mircea Trofin <mtrofin at google.com> >>>> wrote: >>>> >> >>>> >> >>>> >> >>>> >> 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. >>>> 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? >>>> > >>>> > >>>> > Not sure I understand who/when <https://teams.googleplex.com/u/when> >>>> they would grep for that? >>>> >>> >>> AAh! Nevermind me :) forgot that there are reasonable cases using it. >>> Yes, it makes sense to add a FIXME, perhaps at the start of each file >>> >>> >>>> > >>>> > I was suggesting adding an explicit "this use of >>>> --allow-unused-prefixes=true hasn't been confirmed as intentional" so that >>>> the backwards compatibility cases can be distinguished from the intentional >>>> cases when someone is reading the test case, rather than puzzling over why >>>> this flag was added (which looks intentional) though the unused prefix may >>>> not make sense in that particular test. It'll make it easier in the future >>>> when someone does look at the test for them to not feel like they're being >>>> implicitly told "this use of unused prefixes is intentional" (by the >>>> presence of an explicit flag requesting such support) while staring at the >>>> test and not being able to see why someone would've done that intentionally. >>>> >>>> Directly CCing some folks who can fix or find people to fix some >>>> directories (*/AMDGPU, */X86, */OpenMP, */AArch64) on >>>> >>>> https://docs.google.com/spreadsheets/d/1o6q3XH1n3DDyyccnYZ_kVfhFbTDzC_S09e973_cwYuw/edit#gid=0 >>>> After these big directories are cleaned up, the remaining tests should >>>> be manageable in amount. >>>> >>>> How to reproduce: >>>> >>>> sed -i '/allow-unused-prefixes/s/true/false/' >>>> llvm/utils/FileCheck/FileCheck.cpp >>>> git update-index --assume-unchanged llvm/utils/FileCheck/FileCheck.cpp >>>> ninja check-llvm # or check-clang ... >>>> >>>> >>>> >>>> >>>> >>> >>>> >>> >>>> >>>> >>>> >>>> >>>> >>>> >>>> >>>> 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 [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 >>>> > >>>> > _______________________________________________ >>>> > LLVM Developers mailing list >>>> > llvm-dev at lists.llvm.org >>>> > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >>>> >>>> >>>> >>>> -- >>>> 宋方睿 >>>> >>> _______________________________________________ >>> 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/20201106/fa41c79c/attachment.html>
Mircea Trofin via llvm-dev
2020-Nov-06 15:13 UTC
[llvm-dev] [RFC] FileCheck: (dis)allowing unused prefixes
Oh! Perfect - thanks! (plus, if it becomes unsupported, there's another nudge to fix :) ) On Fri, Nov 6, 2020 at 7:05 AM James Henderson <jh7370.2008 at my.bristol.ac.uk> wrote:> I recently discovered that multi-line RUN statements can actually be > interrupted with non-RUN lines, without changing the behaviour. In other > words, you can do something like: > > # RUN: some command --option1 \ > ## Comment > # CHECK: check something > # RUN: --option2 > > And you'd end up with "some command --option1 --option2" being run. It's > rather surprising behaviour, and not one I'd generally recommend > exercising, but maybe in this context it would be okay? > > On Fri, 6 Nov 2020 at 15:00, Mircea Trofin <mtrofin at google.com> wrote: > >> >> >> On Fri, Nov 6, 2020 at 2:22 AM James Henderson < >> jh7370.2008 at my.bristol.ac.uk> wrote: >> >>> I think it would make more sense to add it at each individual call site. >>> This ensures that all cases are fixed, rather than just one in a file. It >>> also ensures that in the (hopefully unlikely) event that there are both >>> intentional and unintentional use-cases within a file, each one gets >>> checked. >>> >>> That would be better indeed, but may be trickier to automate - the >> challenge, in my mind, comes from multi-line RUN statements, but maybe I'm >> missing something / maybe there aren't that many. If folks have >> suggestions, please don't hesitate to throw them my way! >> >> Thanks! >> >> >>> On Thu, 5 Nov 2020 at 20:29, Mircea Trofin via llvm-dev < >>> llvm-dev at lists.llvm.org> wrote: >>> >>>> Thanks! >>>> >>>> On Thu, Nov 5, 2020 at 11:50 AM Fāng-ruì Sòng <maskray at google.com> >>>> wrote: >>>> >>>>> On Thu, Nov 5, 2020 at 11:36 AM David Blaikie via llvm-dev >>>>> <llvm-dev at lists.llvm.org> wrote: >>>>> > >>>>> > >>>>> > >>>>> > On Thu, Nov 5, 2020 at 10:46 AM Mircea Trofin <mtrofin at google.com> >>>>> wrote: >>>>> >> >>>>> >> >>>>> >> >>>>> >> 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. >>>>> 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? >>>>> > >>>>> > >>>>> > Not sure I understand who/when <https://teams.googleplex.com/u/when> >>>>> they would grep for that? >>>>> >>>> >>>> AAh! Nevermind me :) forgot that there are reasonable cases using it. >>>> Yes, it makes sense to add a FIXME, perhaps at the start of each file >>>> >>>> >>>>> > >>>>> > I was suggesting adding an explicit "this use of >>>>> --allow-unused-prefixes=true hasn't been confirmed as intentional" so that >>>>> the backwards compatibility cases can be distinguished from the intentional >>>>> cases when someone is reading the test case, rather than puzzling over why >>>>> this flag was added (which looks intentional) though the unused prefix may >>>>> not make sense in that particular test. It'll make it easier in the future >>>>> when someone does look at the test for them to not feel like they're being >>>>> implicitly told "this use of unused prefixes is intentional" (by the >>>>> presence of an explicit flag requesting such support) while staring at the >>>>> test and not being able to see why someone would've done that intentionally. >>>>> >>>>> Directly CCing some folks who can fix or find people to fix some >>>>> directories (*/AMDGPU, */X86, */OpenMP, */AArch64) on >>>>> >>>>> https://docs.google.com/spreadsheets/d/1o6q3XH1n3DDyyccnYZ_kVfhFbTDzC_S09e973_cwYuw/edit#gid=0 >>>>> After these big directories are cleaned up, the remaining tests should >>>>> be manageable in amount. >>>>> >>>>> How to reproduce: >>>>> >>>>> sed -i '/allow-unused-prefixes/s/true/false/' >>>>> llvm/utils/FileCheck/FileCheck.cpp >>>>> git update-index --assume-unchanged llvm/utils/FileCheck/FileCheck.cpp >>>>> ninja check-llvm # or check-clang ... >>>>> >>>>> >>>>> >>>>> >>>>> >>> >>>>> >>> >>>>> >>>> >>>>> >>>> >>>>> >>>> >>>>> >>>> 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 [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 >>>>> > >>>>> > _______________________________________________ >>>>> > LLVM Developers mailing list >>>>> > llvm-dev at lists.llvm.org >>>>> > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >>>>> >>>>> >>>>> >>>>> -- >>>>> 宋方睿 >>>>> >>>> _______________________________________________ >>>> 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/20201106/c370158c/attachment.html>
Mircea Trofin via llvm-dev
2020-Nov-09 18:18 UTC
[llvm-dev] [RFC] FileCheck: (dis)allowing unused prefixes
There's a wrinkle in this: some tests (clang ones, for instance) have output checks depending on the line position of the input. For example, they check debug info. Adding // FIXME: comments shift that. If the goal is easy identification of auto-inserted -allow-unused-prefixes directives, how about: - we make the flag an enum: true, false, and auto_inserted - we use -allow-unused-prefixes=auto_inserted for the scripts This allows easy identification, and should be easier to script without requiring more significant test by test surgery. WDYT? On Fri, Nov 6, 2020 at 7:13 AM Mircea Trofin <mtrofin at google.com> wrote:> Oh! Perfect - thanks! > > (plus, if it becomes unsupported, there's another nudge to fix :) ) > > On Fri, Nov 6, 2020 at 7:05 AM James Henderson < > jh7370.2008 at my.bristol.ac.uk> wrote: > >> I recently discovered that multi-line RUN statements can actually be >> interrupted with non-RUN lines, without changing the behaviour. In other >> words, you can do something like: >> >> # RUN: some command --option1 \ >> ## Comment >> # CHECK: check something >> # RUN: --option2 >> >> And you'd end up with "some command --option1 --option2" being run. It's >> rather surprising behaviour, and not one I'd generally recommend >> exercising, but maybe in this context it would be okay? >> >> On Fri, 6 Nov 2020 at 15:00, Mircea Trofin <mtrofin at google.com> wrote: >> >>> >>> >>> On Fri, Nov 6, 2020 at 2:22 AM James Henderson < >>> jh7370.2008 at my.bristol.ac.uk> wrote: >>> >>>> I think it would make more sense to add it at each individual call >>>> site. This ensures that all cases are fixed, rather than just one in a >>>> file. It also ensures that in the (hopefully unlikely) event that there are >>>> both intentional and unintentional use-cases within a file, each one gets >>>> checked. >>>> >>>> That would be better indeed, but may be trickier to automate - the >>> challenge, in my mind, comes from multi-line RUN statements, but maybe I'm >>> missing something / maybe there aren't that many. If folks have >>> suggestions, please don't hesitate to throw them my way! >>> >>> Thanks! >>> >>> >>>> On Thu, 5 Nov 2020 at 20:29, Mircea Trofin via llvm-dev < >>>> llvm-dev at lists.llvm.org> wrote: >>>> >>>>> Thanks! >>>>> >>>>> On Thu, Nov 5, 2020 at 11:50 AM Fāng-ruì Sòng <maskray at google.com> >>>>> wrote: >>>>> >>>>>> On Thu, Nov 5, 2020 at 11:36 AM David Blaikie via llvm-dev >>>>>> <llvm-dev at lists.llvm.org> wrote: >>>>>> > >>>>>> > >>>>>> > >>>>>> > On Thu, Nov 5, 2020 at 10:46 AM Mircea Trofin <mtrofin at google.com> >>>>>> wrote: >>>>>> >> >>>>>> >> >>>>>> >> >>>>>> >> 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. >>>>>> 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? >>>>>> > >>>>>> > >>>>>> > Not sure I understand who/when >>>>>> <https://teams.googleplex.com/u/when> they would grep for that? >>>>>> >>>>> >>>>> AAh! Nevermind me :) forgot that there are reasonable cases using it. >>>>> Yes, it makes sense to add a FIXME, perhaps at the start of each file >>>>> >>>>> >>>>>> > >>>>>> > I was suggesting adding an explicit "this use of >>>>>> --allow-unused-prefixes=true hasn't been confirmed as intentional" so that >>>>>> the backwards compatibility cases can be distinguished from the intentional >>>>>> cases when someone is reading the test case, rather than puzzling over why >>>>>> this flag was added (which looks intentional) though the unused prefix may >>>>>> not make sense in that particular test. It'll make it easier in the future >>>>>> when someone does look at the test for them to not feel like they're being >>>>>> implicitly told "this use of unused prefixes is intentional" (by the >>>>>> presence of an explicit flag requesting such support) while staring at the >>>>>> test and not being able to see why someone would've done that intentionally. >>>>>> >>>>>> Directly CCing some folks who can fix or find people to fix some >>>>>> directories (*/AMDGPU, */X86, */OpenMP, */AArch64) on >>>>>> >>>>>> https://docs.google.com/spreadsheets/d/1o6q3XH1n3DDyyccnYZ_kVfhFbTDzC_S09e973_cwYuw/edit#gid=0 >>>>>> After these big directories are cleaned up, the remaining tests should >>>>>> be manageable in amount. >>>>>> >>>>>> How to reproduce: >>>>>> >>>>>> sed -i '/allow-unused-prefixes/s/true/false/' >>>>>> llvm/utils/FileCheck/FileCheck.cpp >>>>>> git update-index --assume-unchanged llvm/utils/FileCheck/FileCheck.cpp >>>>>> ninja check-llvm # or check-clang ... >>>>>> >>>>>> >>>>>> >>>>>> >>>>>> >>> >>>>>> >>> >>>>>> >>>> >>>>>> >>>> >>>>>> >>>> >>>>>> >>>> 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 [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 >>>>>> > >>>>>> > _______________________________________________ >>>>>> > LLVM Developers mailing list >>>>>> > llvm-dev at lists.llvm.org >>>>>> > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >>>>>> >>>>>> >>>>>> >>>>>> -- >>>>>> 宋方睿 >>>>>> >>>>> _______________________________________________ >>>>> 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/20201109/3f085738/attachment.html>