[PATCH] D96290: [tools] UpdateTestPrefix: improved verbosity

Mon Feb 15 10:22:43 PST 2021

mtrofin added a comment.

In D96290#2563763 <https://reviews.llvm.org/D96290#2563763>, @RKSimon wrote:

> In D96290#2563750 <https://reviews.llvm.org/D96290#2563750>, @spatel wrote:
>
>> I think this is an improvement from current behavior, so no objections from me, but hopefully someone else can take a look at the implementation.
>> @RKSimon might have some comments based on:
>> https://llvm.org/PR49189
>
> I've replied on the bug , but we seem to have lost the original warning as part of D93078 <https://reviews.llvm.org/D93078>

As detailed on the bug, and on D93078 <https://reviews.llvm.org/D93078>, that warning was buggy. In particular, in the case of PR49189, it wouldn't have worked (tried by sync-ing before the patch and running the tool).

Are we confident this (i.e. warn if a RUN line's outputs aren't used) was the intended behavior? Perhaps it would be worth taking a step back and defining what the desired behavior of the tool is - independent from previous state of the code which, again, was buggy and untested (and, thus, can't serve as a specification).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D96290/new/

https://reviews.llvm.org/D96290