jdoerfert added a comment. In D93078#2500122 <https://reviews.llvm.org/D93078#2500122>, @mtrofin wrote:
> In D93078#2500114 <https://reviews.llvm.org/D93078#2500114>, @jdoerfert wrote: > >> In D93078#2500040 <https://reviews.llvm.org/D93078#2500040>, @mtrofin wrote: >> >>> 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: >>>>> >>>>>> 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" >> >> I can add the option explicitly (D94744 <https://reviews.llvm.org/D94744>). >> We should look for the filecheck one if possible, two options means double >> the hassle. That said, why are we warning in both FileCheck and >> update_test_check, it seems to be unnecessary to do the latter. > > update_test_prefix.py's role is temporary: once we flip FileCheck to disallow > unused prefixes by default, we don't need to keep it around. At that point, > it becomes important for the update_<xyz>_test_check scripts to warn. I don't follow. I would assume it's the opposite. If FileCheck doesn't allow unused prefixes why warn in the update script as well. Anyway, there should be a way to opt-out. 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
