[PATCH] D83004: [UpdateCCTestChecks] Include generated functions if asked

[David Greene via Phabricator via cfe-commits]

greened added inline comments.


================
Comment at: llvm/utils/update_cc_test_checks.py:274-277
       prefixes = p[0]
       for prefix in prefixes:
         func_dict.update({prefix: dict()})
+        func_order.update({prefix: []})
----------------
arichardson wrote:
> This should avoid a few update() calls. Not that performance really matters here.
> This should avoid a few update() calls. Not that performance really matters here.

Wouldn't it iterate over the prefix list twice?  In any event, I was trying to follow existing style and wanted to rewrite as little existing code as possible.


================
Comment at: llvm/utils/update_cc_test_checks.py:331
+            # are ordered by prefix instead of by function as in "normal"
+            # mode.
+            if '-emit-llvm' in clang_args:
----------------
arichardson wrote:
> jdoerfert wrote:
> > This is all unfortunate but at least for OpenMP not easily changeable. Let's go with this.
> How difficult would it be to only use this behaviour if the normal one fails? 
> 
> If we encounter a function that's not in the source code we just append the check lines for the generate function where we last added checks. And if we then see a function where the order doesn't match the source order fall back to just emitting all checks at the end of the file?
> How difficult would it be to only use this behaviour if the normal one fails? 
> 
> If we encounter a function that's not in the source code we just append the check lines for the generate function where we last added checks. And if we then see a function where the order doesn't match the source order fall back to just emitting all checks at the end of the file?

The problem is that this loop is driven by lines as they appear in the source file and uses a dictionary to look up the tool output for that function.  Because of this checks will *always* be placed in the source before each function and there is no way to tell if the checks you're adding are from out-of-order tool output.  Changing this would be a complete rewrite of the algorithm, which I wanted to avoid for this enhancement.  Of course we can always rewrite it later but I think that is best done as a separate change.

The other two tools use a similar algorithm.  In fact it *may* be possible to further refactor this code to share the check output driver.  But again, that involves changing existing code and is best left to a separate change I think.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83004