pengfei added inline comments.

================
Comment at: llvm/utils/update_analyze_test_checks.py:129
+    
+    common.warn_on_failed_prefixes(func_dict)
     is_in_function = False
----------------
mtrofin wrote:
> pengfei wrote:
> > Can we move these warn to common.py?
> Come to think of it, maybe moving it to common.py was not quite ideal:
> - once a warning is issued in the middle of the list of RUN lines, it'll just 
> be re-issued next time around; we could warn and exit, but that'd be annoying 
> if there's another failure later in the RUN list.
> - if we do it at the end, we can additionally distinguish the case where a 
> prefix has an empty dict of funcs associated with it -> and the warning would 
> be "there are unused prefixes - please remove %s'. This is more discoverable 
> than llvm-lit -a, and also than relying on the user knowing to run 
> uplate_test_prefix.
> 
> I think that the benefits (discoverability, better, more concise warnings) 
> make the extra 2-3 lines worth it; and we already have tests for these other 
> tools - wdyt?
Make sence for me. Thanks for continuing to improve it.


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

Reply via email to