[libcxx-commits] [PATCH] D81846: [libc++] Allow specifying custom Lit config files

Fri Jun 19 00:29:53 PDT 2020

gargaroff added a comment.

In D81846#2100877 <https://reviews.llvm.org/D81846#2100877>, @ldionne wrote:

> Does your compiler support `-fno-exceptions`? If so, just use `--param enable_exceptions=False` when running Lit and you're golden.

We have disabled exceptions through CMake (`LIBCXX_ENABLE_EXCEPTIONS=OFF`) which also sets `config.enable_exceptions` to `False` but we still see many many unresolved references errors (more than 500) for `_Unwind_*` functions (we don't provide `libunwind` yet). Also, at first we also disabled RTTI but still saw that all? most? just some select few? (didn't do a full investigation) of the RTTI tests were still running, so I was unsure whether the same was true for exception handling tests.

> 
> 
>> Furthermore, I saw that there were some tests which rely on GNU extensions, which are not available in our C library.
> 
> Which ones?

Sorry, I mixed up POSIX and GNU :)
Anyway, I see that some tests are using the `debug_mode_helper.h` which includes `sys/wait.h`. As far as I understand that's a POSIX header and is not available for our target (as well as all other POSIX-only headers).

Speaking of available features. There's another thing that just popped into my head: our C library uses the ISO-8859-X charmap and does not support UTF-8.  Will we need to disable locale tests? When implementation our `TargetInfo` object I had to implement a function which tests whether some locales are supported and all of them where UTF-8 locales.

> I believe it might become useful to support specifying a set of UNSUPPORTED and/or XFAIL annotations externally to the test suite itself. This way, you could add additional annotations out of tree without risking merge conflicts or adding a lot of complexity. You'd have to maintain that out-of-tree list when we add new tests, but that's the case whichever way we decide to do it.

Yes, that would indeed be a nice-to-have feature for us! I think it would make sense to have this as part of the config. I'm thinking something like `config.unsupported_tests = []` and `config.xfail_tests = []` which would take a list of paths (maybe even globs? or regex?). And since it would be part of the config, you could even make this dynamic.

> No, it shouldn't be an issue if you hardcode stuff. What's the problem you're seeing with adding your flags to `%{link_flags}`?

No no, we didn't have any problem with adding it to `%{link_flags}`. We just wondered if it *would* be problematic for some tests if we did.

> Your compiler doesn't support `-Wno-aligned-allocation-unavailable`? Does it fail when one tries to use that option?

Yes, clang complains about it being an unrecognized flag and exits right away.
By the way, setting `config.enable_warnings = False` has the nice side-effect that `libcxx/selftest/newformat/pass.cpp/werror.pass.cpp` and `libcxx/selftest/newformat/sh.cpp/werror.sh.cpp` are unexpectedly passing because `-Werror` is not passed as a flag anymore.

> Since it looks like this is sufficient for your use case, I'll go forward with this patch.

Looking forward to using it! Thanks for the effort.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81846