[libc++][patch] Fix a lit.cfg bug: the CMake build compiler is ignored

[Michael Gottesman]

Ok. I think you should make the change given that the checks around it follow that convention (see libcxx_src_root, libcxx_obj_root, etc).

Once that is fixed, LGTM.

Michael

On Jul 29, 2013, at 4:05 PM, Sebastian Redl <sebastian.redl at getdesigned.at> wrote:

> 
> 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