[PATCH] D58196: [lit][NFC] Small cleanup

[Julian Lettner via Phabricator via llvm-commits]

yln marked 2 inline comments as done.
yln added inline comments.


================
Comment at: llvm/utils/lit/lit/run.py:196
         if pg:
             semaphore = parallelism_semaphores[pg]
             semaphore.acquire()
----------------
rnk wrote:
> Won't we get an unbound local error now if parallelism_semaphores[pg] raises KeyError? I would expect this to raise, hit the except block, then run the finally, where it would raise again.
Yes, it raises, prints a stack trace, and crashes. (Instead of marking tests as unresolved when I misconfigure my parallelism groups.)


================
Comment at: llvm/utils/lit/lit/run.py:199
             semaphore = parallelism_semaphores[pg]
-        if semaphore:
             semaphore.acquire()
----------------
rnk wrote:
> I agree this check seems unnecessary. It seems like it would be a bug to insert None as a value in the parallelism_semaphores dict.
Your comment about None gave me an idea. I think it *would* be useful for intentionally stating "no restrictions" vs "unsure if configuration error" or just not concerned about parallelism. I will update the patch. Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58196