[libcxx] Add numerous options to libc++ LIT test suite configuration.

[EricWF]

Summary

In order to fully replace the testit script we need to update LIT so it provides the same functionality.
This patch adds a number of different configuration options to LIT to do that. It also adds documentation for all of the command line parameters that LIT supports.

Generic options added:

• libcxx_headers
• libcxx_library
• compile_flags

Generic options modified:

• link_flags: Changed from overriding the default args to adding extra args instead (to match compile flags)
• use_sanitizer: Renamed from llvm_use_sanitizer

Please see the added documentation for more information about the switches. As for the actual documentation I'm not sure if it should be kept in libc++ forever since it adds an undue maintenance burden, but I think it should be added for the time being while the changes are new. I'm verify unskilled with HTML so if the documentation needs any changes please let me know.

Hopefully this will kill testit.

[alexfh]

test/lit.cfg

578

I wouldn't like this to depend on the default set of checks. And I don't think you want all static analyzer checks. I'd better change this to:

-checks=-*,llvm*,clang-analyzer*,-clang-analyzer-alpha*

Also, could you just use an appropriate .clang-tidy file instead?

[EricWF]

Thanks for the information on the default clang tidy settings, I wasn't quite sure what to use :D

The reason I didn't use a clang-tidy is because much of the clang-tidy invocation is sensative to information that could change from run to run due to different configurations.
I'll take a look into it further to see if there is any information that is consistent.

[EricWF]

Update patch so it applies cleanly to TOT. Also change the default clang-tidy checkers. I didn't add a .clang-tidy file since a user may want to disable the default checkers by overriding them on the command line.

[alexfh]

test/lit.cfg

572

It should be -checks=-*,llvm*,... i.e. the first item in the list should be "-*" which means "disable all checks".

[EricWF]

Woops. Fix clang-tidy default args again. Thanks.

[EricWF]

Update docs for last change. Sorry for the spam.

[alexfh]

I know nothing about libcxx testing, but clang-tidy-related bits look good now ;) (assuming you have run this already and it works).

[EricWF]

Taking this offline.

Thanks for the input. Using clang-tidy is a nice feature. (even though
every single test fails w/ clang-tidy and the default options).
I guess we have plenty of work to do.

/Eric

[EricWF]

Add support for building and generating code coverage using lcov and genhtml. To produce code coverage you must:

• configure with '-DLIBCXX_GENERATE_COVERAGE=ON'
• make
• make check-libcxx
• make check-libcxx-coverage

The resulting HTML can be found in <build-libcxx>/test/coverage/libcxx_coverage

This also adds support for UNSUPPORTED-XFAIL and REQUIRES-XFAIL tags that ensure compilation fail but report the test as UNSUPPORTED instead of XFAIL.

[danalbert]

Mega patches make me sad. I see 5 distinct changes here:

1. Add missing testit features.
2. scan-build support.
3. clang-tidy support.
4. Coverage support.
5. REQUIRES-XFAIL/UNSUPPORTED-XFAIL.

Please split up this change into separate patches. Voltron-esque patches are hard to review, since we have to context switch so many times, and they're less useful when looking at git history because it isn't always immediately obvious which of the many things a given change was for.

cmake/Modules/LibcxxCodeCoverage.cmake

2 ↗

(On Diff #15968)

This looks like a leftover.

10 ↗

(On Diff #15968)

iirc --coverage implies -fprofile-arcs and -ftest-coverage.

13 ↗

(On Diff #15968)

Can move these two checks up to where find_program is called

20 ↗

(On Diff #15968)

MAKE_DIRECTORY?

22 ↗

(On Diff #15968)

Don't we already have a cxx target?

37 ↗

(On Diff #15968)

Why?

test/lit.cfg

33

Python style is kwarg=value (no space)

47

What was it before if not a string?

49

I don't think this does what you think it does.

You're probably looking for coverage_flags.split(' ')

50

Same as 47

51

Is this actually interesting to run on the tests? I would think this is the kind of thing we should be running on libc++ itself.

53

Same as 49

54

Same as 51

55

Same as 49

57

Diff fail :(

I think the new name is actually a better match for the old behavior (since this doesn't actually do the reporting now). I'd change the name back.

Hard to see what else changed... (should be more clear if the name changes back).

105

Do we actually need these? It looks like it's just complicating things.

183

Variable style changed here.

We're kind of inconsistent here anyway, but we should probably drift toward actually matching PEP8.

200

Hmm. I thought the driver would dump the gcno in the same directory as the output file (ditto for the gcd). What is this for?

234

spaces after ,

306

Why do we need on/off? Doesn't that just mean cmake is missing a pythonize_bool?

313

Why do we have two of these?

496

Should say which one is being used if we aren't making this an error

501

Doesn't rpath need an argument?

572

Don't think you meant to unalign them.

597

I don't quite understand why '' means yes, but 'true' means no. Shouldn't this just be

if not self.use_scan_build:
    ...
else:
    ...

? It changes the meaning slightly, but I think this behavior makes more sense. Am I missing something?

[EricWF]

In D5877#18, @danalbert wrote:

Mega patches make me sad. I see 5 distinct changes here:

1. Add missing testit features.
2. scan-build support.
3. clang-tidy support.
4. Coverage support.
5. REQUIRES-XFAIL/UNSUPPORTED-XFAIL.

Please split up this change into separate patches. Voltron-esque patches are hard to review, since we have to context switch so many times, and they're less useful when looking at git history because it isn't always immediately obvious which of the many things a given change was for.

Sorry about that. I'll split them up. I was hoping to get away with it this once. It's hard to maintain and merge 5 separate patches that exist in such a small space.

cmake/Modules/LibcxxCodeCoverage.cmake

2 ↗

(On Diff #15968)

lcov depends on gcov. I thought we should check if gcov is found and issue a hard error otherwise.

10 ↗

(On Diff #15968)

Yep. My mistake. Thanks.

20 ↗

(On Diff #15968)

It creates a directory where all of the coverage files will live.

22 ↗

(On Diff #15968)

Yep. This adds a command that executes before the cxx target.

37 ↗

(On Diff #15968)

In case the resulting HTML is hosted on a web server this minimizes the permissions.

test/lit.cfg

47

I was being dumb.

49

coverage_flags should already be a list. This performs a copy of the list so we have our own.

51

Yes. The tests instantiate the templates. Without the instantiations the static analyzer has nothing to do.

57

Ok. Will do.

105

I'll pull these out for now, but the change is something I would like. It gives the ability to file the test as unsupported while ensuring it fails. It would be helpful for ignoring the std::tuple tests in c++03 mode.

200

The driver dumps the gcno file in the cwd used to invoke the compiler. Since the gcno will be named <test-name>.gcno and not the unique executable output name we need to mirror the test directory structure to ensure that gcno files for each test don't conflict.

306

This input could also come from the command line. I'm not opposed to only using one name for true and false but this gives flexibility.

313

Different semantics. I'm not a fan of the get_lit_bool semantics.

get_lit_bool:

• None -> None
• "" -> False

get_lit_switch:

• None -> False
• "" -> True

get_lit_switch allows the no-argument case to return True, and always returns a bool.

496

I'll make it an error.

501

It gets its argument from the second -Wl flag but I'll join them for clarity.

597

The semantics of use_scan_build are intended to be:

--param=use_scan_build[=<path/to/scan-build>]

Where the argument defaults to scan-build if omitted.

[EricWF]

I've stripped the patch down to a minimal set of changes so that it has the same functionality as the testit script.

I've removed:

• UNSUPPORTED-XFAIL and REQUIRES-XFAIL
• support for scan-build
• support for clang-tidy
• support for code coverage.

[EricWF]

Changed enable_warnings so it sets -Werror. Otherwise the switch is pretty useless.

[danalbert]

A bunch of style nits and a few doc changes, but otherwise looks good.

test/lit.cfg

41

Spaces after commas.

42

Why the docstring? I see you didn't actually put them here, but what's the point?

Same goes for the rest of them.

55

Spaces after commas.

128

Spaces after commas.

142

Spaces after commas.

154

Again.

219

Don't need the trailing \ when it's inside parentheses.

344

Alignment.

359

Agreed. Fatal makes more sense.

370

Alignment.

www/lit_usage.html

16

Space after colon.

54

s/it's/its/

62

s/"/'/

136

Should say how the list is delimited. Same for the others.

143

1z should probably come after 14.

[mclow.lists]

General comments:

1. I'm liking how this is going. A lot.
2. The symlink thing in using lit seems to be misguided to me. You should be able to specify a lit.site.cfg file on the command line. This will make it much easier to support multiple test configurations. I can think of 32 right off the bat: c++03/11/14/1z vs. 32/64 bit vs. No Sanitizer/ASAN/TSAN/UBSAN

3. Did I mention that I like the fast that it's much faster than testit? :-)

[EricWF]

Fix more of @danalbert's comments. I would like to go forward with this patch even though there is a conflicting patch out against LIT.

[danalbert]

LGTM with the one doc change.

www/lit_usage.html

128

I had meant explicitly state that it is space delimited.

[EricWF]

I've rewritten the patch against TOT. There are a couple of changes.

• Removed ability to enable warnings for now.
• Added documentation of the libcxx_site_config option.
• Added documentation about how compile_flags and link_flags are delimited.

[danalbert]

LGTM. Thanks!

test/lit.cfg

         self.configure_use_clang_verify()
         self.configure_ccache()
         self.configure_env()
-        self.configure_std_flag()
         self.configure_compile_flags()
         self.configure_link_flags()
         self.configure_sanitizer()

             self.config.available_features.add('long_tests')
 
     def configure_compile_flags(self):
-        # Configure extra compiler flags.
+        # Try and get the std version from the command line. Fall back to
+        # default given in lit.site.cfg is not present. If default is not
+        # present then force c++11.
+        std = self.get_lit_conf('std', 'c++11')
+        self.compile_flags += ['-std={0}'.format(std)]
+        self.config.available_features.add(std)
+        # Configure include paths
         self.compile_flags += ['-nostdinc++']
-        self.compile_flags += ['-I' + self.src_root + '/include',
-                               '-I' + self.src_root + '/test/support']
+        self.compile_flags += ['-I' + self.src_root + '/test/support']
+        libcxx_headers = self.get_lit_conf('libcxx_headers',
+                                           self.src_root + '/include')
+        if not os.path.isdir(libcxx_headers):
+            self.lit_config.fatal("libcxx_headers='%s' is not a directory."
+                                  % libcxx_headers)
+        self.compile_flags += ['-I' + libcxx_headers]
         if sys.platform.startswith('linux'):
             self.compile_flags += ['-D__STDC_FORMAT_MACROS',
                                    '-D__STDC_LIMIT_MACROS',
                                    '-D__STDC_CONSTANT_MACROS']
+        # Configure feature flags.
         enable_exceptions = self.get_lit_bool('enable_exceptions', True)
         if enable_exceptions:
             self.config.available_features.add('exceptions')

         elif not enable_monotonic_clock:
             self.lit_config.fatal('enable_monotonic_clock cannot be false when'
                                   ' enable_threads is true.')
+        # Use verbose output for better errors
+        self.compile_flags += ['-v']
+        # Configure extra compile flags.
+        compile_flags_str = self.get_lit_conf('compile_flags', '')
+        self.compile_flags += shlex.split(compile_flags_str)
 
 
     def configure_link_flags(self):
         self.link_flags += ['-nodefaultlibs']
-        # Configure library search paths
+        libcxx_library = self.get_lit_conf('libcxx_library')
+        # Configure libc++ library paths.
+        if libcxx_library is not None:
+            # Check that the given value for libcxx_library is valid.
+            if not os.path.isfile(libcxx_library):
+                self.lit_config.fatal(
+                    "libcxx_library='%s' is not a valid file." % libcxx_library)
+            if self.use_system_lib:
+                self.lit_config.fatal("Conflicting options: 'libcxx_library'"
+                    " cannot be used with 'use_system_lib=true'")
+            self.link_flags += ['-Wl,-rpath,' + os.path.dirname(libcxx_library)]
+        elif not self.use_system_lib:
+            self.link_flags += ['-L' + self.library_root,
+                                '-Wl,-rpath,' + self.library_root]
+        # Configure ABI library paths.
         abi_library_path = self.get_lit_conf('abi_library_path', '')
-        if not self.use_system_lib:
-            self.link_flags += ['-L' + self.library_root]
-            self.link_flags += ['-Wl,-rpath,' + self.library_root]
         if abi_library_path:
             self.link_flags += ['-L' + abi_library_path,
                                 '-Wl,-rpath,' + abi_library_path]
         # Configure libraries
-        self.link_flags += ['-lc++']
-        link_flags_str = self.get_lit_conf('link_flags')
-        if link_flags_str is None:
-            cxx_abi = self.get_lit_conf('cxx_abi', 'libcxxabi')
-            if cxx_abi == 'libstdc++':
-                self.link_flags += ['-lstdc++']
-            elif cxx_abi == 'libsupc++':
-                self.link_flags += ['-lsupc++']
-            elif cxx_abi == 'libcxxabi':
-                self.link_flags += ['-lc++abi']
-            elif cxx_abi == 'libcxxrt':
-                self.link_flags += ['-lcxxrt']
-            elif cxx_abi == 'none':
-                pass
-            else:
-                self.lit_config.fatal(
-                    'C++ ABI setting %s unsupported for tests' % cxx_abi)
-
-            if sys.platform == 'darwin':
-                self.link_flags += ['-lSystem']
-            elif sys.platform.startswith('linux'):
-                self.link_flags += ['-lgcc_eh', '-lc', '-lm', '-lpthread',
-                                    '-lrt', '-lgcc_s']
-            elif sys.platform.startswith('freebsd'):
-                self.link_flags += ['-lc', '-lm', '-pthread', '-lgcc_s']
-            else:
-                self.lit_config.fatal("unrecognized system: %r" % sys.platform)
-
-        if link_flags_str:
-            self.link_flags += shlex.split(link_flags_str)
+        if libcxx_library:
+            self.link_flags += [libcxx_library]
+        else:
+            self.link_flags += ['-lc++']
+        cxx_abi = self.get_lit_conf('cxx_abi', 'libcxxabi')
+        if cxx_abi == 'libstdc++':
+            self.link_flags += ['-lstdc++']
+        elif cxx_abi == 'libsupc++':
+            self.link_flags += ['-lsupc++']
+        elif cxx_abi == 'libcxxabi':
+            self.link_flags += ['-lc++abi']
+        elif cxx_abi == 'libcxxrt':
+            self.link_flags += ['-lcxxrt']
+        elif cxx_abi == 'none':
+            pass
+        else:
+            self.lit_config.fatal(
+                'C++ ABI setting %s unsupported for tests' % cxx_abi)
+        # Configure extra libraries.
+        if sys.platform == 'darwin':
+            self.link_flags += ['-lSystem']
+        elif sys.platform.startswith('linux'):
+            self.link_flags += ['-lgcc_eh', '-lc', '-lm', '-lpthread',
+                                '-lrt', '-lgcc_s']
+        elif sys.platform.startswith('freebsd'):
+            self.link_flags += ['-lc', '-lm', '-pthread', '-lgcc_s']
+        else:
+            self.lit_config.fatal("unrecognized system: %r" % sys.platform)
 
+        link_flags_str = self.get_lit_conf('link_flags', '')
+        self.link_flags += shlex.split(link_flags_str)
 
-    def configure_std_flag(self):
-        # Try and get the std version from the command line. Fall back to
-        # default given in lit.site.cfg is not present. If default is not
-        # present then force c++11.
-        std = self.get_lit_conf('std')
-        if std is None:
-            std = 'c++11'
-            self.lit_config.note('using default std: \'-std=c++11\'')
-        self.compile_flags += ['-std={0}'.format(std)]
-        self.config.available_features.add(std)
 
     def configure_sanitizer(self):
-        san = self.get_lit_conf('llvm_use_sanitizer', '').strip()
+        san = self.get_lit_conf('use_sanitizer', '').strip()
         if san:
             # Search for llvm-symbolizer along the compiler path first
             # and then along the PATH env variable.

 
     def configure_env(self):
         if sys.platform == 'darwin' and not self.use_system_lib:
-            self.env['DYLD_LIBRARY_PATH'] = self.library_root
+            libcxx_library = self.get_lit_conf('libcxx_library')
+            if libcxx_library:
+                library_root = os.path.dirname(libcxx_library)
+            else:
+                library_root = self.library_root
+            self.env['DYLD_LIBRARY_PATH'] = library_root
 
 # name: The name of this test suite.
 config.name = 'libc++'

test/lit.site.cfg.in

 config.enable_threads           = "@LIBCXX_ENABLE_THREADS@"
 config.enable_monotonic_clock   = "@LIBCXX_ENABLE_MONOTONIC_CLOCK@"
 config.cxx_abi                  = "@LIBCXX_CXX_ABI_LIBNAME@"
-config.llvm_use_sanitizer       = "@LLVM_USE_SANITIZER@"
+config.use_sanitizer            = "@LLVM_USE_SANITIZER@"
 config.abi_library_path         = "@LIBCXX_CXX_ABI_LIBRARY_PATH@"
 
 # Let the main config do the real work.

www/index.html

 <li><a href="type_traits_design.html"><tt>&lt;type_traits&gt;</tt></a></li>
 <li><a href="http://cplusplusmusings.wordpress.com/2012/07/05/clang-and-standard-libraries-on-mac-os-x/">Excellent notes by Marshall Clow</a></li>
 <li><a href="debug_mode.html">Status of debug mode</a></li>
+<li><a href="lit_usage.html">LIT usage guide</a></li>
 </ul>
 
 </div>

www/lit_usage.html

+<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01//EN"
+          "http://www.w3.org/TR/html4/strict.dtd">
+<!-- Material used from: HTML 4.01 specs: http://www.w3.org/TR/html401/ -->
+<html>
+<head>
+  <META http-equiv="Content-Type" content="text/html; charset=ISO-8859-1">
+  <title>Testing libc++ using LIT</title>
+  <link type="text/css" rel="stylesheet" href="menu.css">
+  <link type="text/css" rel="stylesheet" href="content.css">
+  <style>
+    .lit-option {
+      padding-top: 0.5em;
+      margin-bottom: 0.0em;
+      font-size: medium;
+      color: #2d58b7
+    }
+    .lit-option-desc {
+      display: block;
+      margin-top: 0em;
+      margin-bottom: 0em;
+      margin-left: 20px;
+      margin-right: 20px;
+    }
+  </style>
+</head>
+
+<body>
+<div id="menu">
+  <div>
+    <a href="http://llvm.org/">LLVM Home</a>
+  </div>
+
+  <div class="submenu">
+    <label>libc++ Info</label>
+    <a href="/index.html">About</a>
+  </div>
+
+  <div class="submenu">
+    <label>Quick Links</label>
+    <a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev">cfe-dev</a>
+    <a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits">cfe-commits</a>
+    <a href="http://llvm.org/bugs/">Bug Reports</a>
+    <a href="http://llvm.org/svn/llvm-project/libcxx/trunk/">Browse SVN</a>
+    <a href="http://llvm.org/viewvc/llvm-project/libcxx/trunk/">Browse ViewVC</a>
+  </div>
+</div>
+
+<div id="content">
+<!--*********************************************************************-->
+                     <h1>Testing libc++ using LIT</h1>
+<!--*********************************************************************-->
+<p>
+libc++ uses LIT to configure and run its tests. The primary way to run the
+libc++ tests is by using <code>make check-libcxx</code>. However since libc++
+can be used in any number of possible configurations it is important to
+customize the way LIT builds and runs the tests. This guide provides
+information on how to use LIT directly to test libc++.
+</p>
+<p>
+Documentation for LIT can be found
+<a href="http://llvm.org/docs/CommandGuide/lit.html">here</a>.
+</p>
+
+<!--*********************************************************************-->
+                        <h2>Getting Started</h2>
+<!--*********************************************************************-->
+<p>
+After building libc++ use the following commands before you start using LIT to
+test.
+</p>
+<ul>
+  <li><code>alias lit='python path/to/llvm/utils/lit/lit.py'</code></li>
+  <li><code>export LIBCXX_SITE_CONFIG=path/to/build-libcxx/test/lit.site.cfg</code></li>
+</ul>
+<p>
+You can now run the libc++ tests by running:
+</p>
+<ul>
+  <li><code>cd path/to/libcxx</code></li>
+  <li><code>lit -sv ./test</code></li>
+</ul>
+<p>
+To only run a subsection of the tests use:
+<ul>
+  <li><code>lit -sv test/std/numerics # Run only the numeric tests</code></li>
+</ul>
+
+<!--*********************************************************************-->
+                     <h2>Customization Options</h2>
+<!--*********************************************************************-->
+<p>
+libc++'s testsuite provides multiple options to configure the way the tests
+are build and run. To use these options you pass them on the LIT command line
+as <code>--param NAME</code> or <code>--param NAME=VALUE</code>. Some options
+have default values specified during CMake's configuration. Passing the option
+on the command line will override the default.
+</p>
+
+
+<p>
+<h3 class="lit-option">libcxx_site_config=&lt;path/to/lit.site.cfg&gt;</h3>
+<blockquote class="lit-option-desc">
+Specify the site configuration to use when running the tests. This option
+overrides the enviroment variable <code>LIBCXX_SITE_CONFIG</code>
+</blockquote>
+</p>
+
+<p>
+<h3 class="lit-option">libcxx_headers=&lt;path/to/headers&gt;</h3>
+<blockquote class="lit-option-desc">
+Specify the libc++ headers that are tested. By default the headers in the source
+tree are used.
+</blockquote>
+</p>
+
+<p>
+<h3 class="lit-option">libcxx_library=&lt;path/to/libc++.so&gt;</h3>
+<blockquote class="lit-option-desc">
+Specify the libc++ library that is tested. By default the library in the build
+directory is used. This option cannot be used when <code>use_system_lib</code>
+is provided.
+</blockquote>
+</p>
+
+<p>
+<h3 class="lit-option">use_system_lib=&lt;bool&gt;</h3>
+<blockquote class="lit-option-desc">
+<b>Default: </b><code>False</code></br>
+Enable or disable testing against the installed version of libc++ library.
+Note: This does not use the installed headers.
+</blockquote>
+</p>
+
+<p>
+<h3 class="lit-option">compile_flags="&lt;list-of-args&gt;"</h3>
+<blockquote class="lit-option-desc">
+Specify additional compile flags as a space delimited string.
+Note: This options should not be used to change the standard version used.
+</blockquote>
+</p>
+
+<p>
+<h3 class="lit-option">link_flags="&lt;list-of-args&gt;"</h3>
+<blockquote class="lit-option-desc">
+Specify additional link  flags as a space delimited string.
+</blockquote>
+</p>
+
+<p>
+<h3 class="lit-option">std=&lt;standard version&gt;</h3>
+<blockquote class="lit-option-desc">
+<b>Values: </b><code>c++98, c++03, c++11, c++14, c++1z</code></br>
+Change the standard version used when building the tests.
+</blockquote>
+</p>
+
+<p>
+<h3 class="lit-option">use_sanitizer=&lt;sanitizer name&gt;</h3>
+<blockquote class="lit-option-desc">
+<b>Values: </b><code>Memory, MemoryWithOrigins, Address, Undefined</code></br>
+Run the tests using the given sanitizer. If <code>LLVM_USE_SANITIZER</code>
+was given when building libc++ then that sanitizer will be used by default.
+</blockquote>
+</p>
+
+</div>
+</body>
+</html>