mtrofin added a comment. In D93078#2500032 <https://reviews.llvm.org/D93078#2500032>, @jdoerfert wrote:
> In D93078#2499996 <https://reviews.llvm.org/D93078#2499996>, @mtrofin wrote: > >> In D93078#2499995 <https://reviews.llvm.org/D93078#2499995>, @jdoerfert >> wrote: >> >>> In D93078#2499982 <https://reviews.llvm.org/D93078#2499982>, @mtrofin wrote: >>> >>>> In D93078#2499666 <https://reviews.llvm.org/D93078#2499666>, @jdoerfert >>>> wrote: >>>> >>>>> I think this caused a lot of problems for the Attributor tests. I get the >>>>> "conflict output" warning all the time now :( >>>>> >>>>> These two `UpdateTestChecks` tests also generate the warning. I would >>>>> think that they haven't before. >>>>> >>>>> LLVM :: tools/UpdateTestChecks/update_test_checks/check_attrs.test >>>>> LLVM :: >>>>> tools/UpdateTestChecks/update_test_checks/prefix-never-matches.test >>>>> >>>>> I suspect the `--check-attributes` flag effect are not taken into account >>>>> somewhere but I might be wrong. >>>> >>>> The warnings are correct, but their purpose is to inform. For example, >>>> take check_attrs. The third and fourth opt run produce different function >>>> attributes from those produced by the first two, so the function is >>>> different insofar as update_test_prefix is concerned. The warnings are >>>> there to indicate that the listed prefixes end up not being used. But, >>>> that's 'by design' for the attributor tests, IIRC. >>>> >>>> If the warnings are noise for you, we could add a flag to quiet them down, >>>> which you'd flip when regenerating attributor tests? >>> >>> TBH, before, the warnings meant there is a problem that needs fixing. Now >>> they mean, there might be one or not. So, depending on your setup it's just >>> noise while it was pure signal before. >>> I'm not sure how this is more helpful. What is the use case where this way >>> of warning helps? >> >> For tests other than attributor, that explicitly set FileCheck >> --allow-unused-prefixes=true, these warnings mean that there will be unused >> prefixes (those listed) > > Should not we check for that flag in the RUN line then and only warn for > unused prefixes when it is set. If there is no prefix we should obviously > always warn. That's a good idea. Probably we'd need to also make sure that the unused prefixes are all on RUN lines with --allow-unused-prefixes=true. I'm also not sure how lit.local.cfg interacts with the test prefix updater: currently, the only cases where we bulk-want to allow unused prefixes is the Attributor tests. If you were going to add the flag explicitly, that'd also work. Or just the option to the update_test_prefix that says "ok with duplicates, don't warn" Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93078/new/ https://reviews.llvm.org/D93078 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
