On 29.07.2013, at 21:58, Michael Gottesman wrote:

> Hey Sebastian.
> 
> Your patch looks fine to me except that you should indent in the inferring 
> section so it looks more like this:
> 
> if cxx_under_test is None:
>     cxx_under_test = getattr(config, 'cxx_under_test', None)
> 
> <-- Indent more in starting here
>  +if cxx_under_test is None:
>       # If no specific cxx_under_test was given, attempt to infer it as 
> clang++.
>       clangxx = lit.util.which('clang++', config.environment['PATH'])
>       if clangxx is not None:
>           cxx_under_test = clangxx
>           lit.note("inferred cxx_under_test as: %r" % (cxx_under_test,))
> <-- And stopping here.
> 
> Otherwise if cxx_under_test is initially not None, you are checking one more 
> time that cxx_under_test is None than you need to.

That was intentional. I consider less indentation and the readability bonus 
from it more important than avoiding one redundant check.

Of course, if you want it the other way, I can easily change it.

Sebastian
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to