[PATCH] D49033: [test-suite] Add a decorator for the lack of libstdcxx on the system.

Pavel Labath via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jul 9 06:12:16 PDT 2018


labath added a comment.

Besides the problems I mention in inline comments, the other issue I have with this patch is that it does things completely different than how we implement the check for libc++, which is an identical problem. (libc++ detection is done by marking all libc++ tests as with a special category, and then deciding in dotest.py (`checkLibcxxSupport`) whether to run the category as a whole).

So, I think this should be reverted and replaced by a solution mirroring the libc++ approach. I'm not saying that one is perfect, but it is in many ways better than this one (e.g., if the default skipping logic gets things wrong, you can override it by manually skipping the corresponding category), and most importantly, it makes things consistent.



================
Comment at: lldb/packages/Python/lldbsuite/test/decorators.py:696
+        f = tempfile.NamedTemporaryFile()
+        cmd = "echo '#include <string> | %s -x c++ -stdlib=libstdc++ -o %s -" % (compiler_path, f.name)
+        if os.popen(cmd).close() is not None:
----------------
This won't work in several scenarios:
- gcc: I'm not sure how you tested on gcc, but my gcc(-7.3.0) gives me an "unrecognised command line option" on the -stdlib argument.
- cross-compilation: just because the default target has/hasn't libstdc++ headers+libs available, it does not mean the target we selected for cross compilation will be the same.

On top of all of that you have a mismatched `'` inside the executed command...


https://reviews.llvm.org/D49033





More information about the llvm-commits mailing list