[PATCH] D67814: [LNT] Python 3 support: adapt to dict method returning views

[Thomas Preud'homme via Phabricator via llvm-commits]

thopre added inline comments.


================
Comment at: lnt/lnttool/admin.py:50
             config = yaml.load(open(filename))
-            for key, value in config.items():
+            for key, value in list(config.items()):
                 self._set(key, value)
----------------
hubert.reinterpretcast wrote:
> thopre wrote:
> > hubert.reinterpretcast wrote:
> > > I believe the change on this line is unnecessary. I'm not marking further instances, but we generally shouldn't form a list just to iterate.
> > Is there no multithreading going on in LNT?
> `config` is local to the function, so I don't think it is being concurrently modified here.
I meant in general, I don't know enough the code to make a good judgement call. However IIRW iterating on a views while it is being updated raises an assertion so in the worse case it should be quite obvious what is going on.


================
Comment at: lnt/tests/nt.py:1488
     logger.info(summary.format(len(rerunable_benches),
-                               len(collated_results.values())))
+                               len(list(collated_results.values()))))
 
----------------
hubert.reinterpretcast wrote:
> hubert.reinterpretcast wrote:
> > `len(collated_results)`
> I'm not seeing the change. `len` applied to the dictionary directly works.
Oh my bad, missed your point.


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

https://reviews.llvm.org/D67814