[PATCH] D95829: [Utils] Add a switch controlling prefix warnings in UpdateTestChecks

Tue Feb 2 06:58:48 PST 2021

mtrofin added a comment.

In D95829#2535179 <https://reviews.llvm.org/D95829#2535179>, @jdoerfert wrote:

> this deserves a test I think

In D95829#2536222 <https://reviews.llvm.org/D95829#2536222>, @RKSimon wrote:

> @mtrofin This is causing warning spam on lots of x86 test regenerations - are we sure its actually disabled by default like its supposed to be?

I'm not sure why it'd be supposed to be disabled by default. The old behavior (pre e2dc306b1ac71258e6ce40a66e778527f282c355 <https://reviews.llvm.org/rGe2dc306b1ac71258e6ce40a66e778527f282c355>) - while buggy - seemed to have the intention to warn frequently (but hard to tell, since it was buggy, see the description and discussion in https://reviews.llvm.org/D93078).

Separately, @craig.topper pointed at how his expectation was for this level of verbosity.

I would argue that the warnings have some value: they highlight functions that end up with no asserts under a prefix in a more compact way than visiting the generated file function by function - but happy to switch the default to whatever the broader community agrees to.

================
Comment at: llvm/utils/UpdateTestChecks/common.py:274
     self._func_order = {}
+    self._flags = flags
     for tuple in run_list:
----------------
jdoerfert wrote:
> leftover?
yes - done.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95829