[libc++][test] Run clang-tidy during CI

[philnik]

Summary

I'm trying to get libc++ to the point of being able to run clang-tidy. This is a PR to see if clang-tidy is happy with all the CI configs.

[Mordante]

I really like this idea! I see it fails since there's no clang-tidy installed in the Docker image.
Best make a separate review to add the clang-tidy in the Docker file, it takes some time for these changes to propagate to the CI agents.

[• Quuxplusone]

libcxx/.clang-tidy

2

One thing we should be wary of here is that by doing this, we're permitting the clang-tidy devs to break libc++'s unit tests. Of course Clang and GCC devs already have this power, and we accept that.
I'm guessing that if we discover clang-tidy suddenly diagnosing something we disagree with or don't have time to fix, then we can simply and quickly add another -foo to this list; or absolute worst case, simply UNSUPPORTED the test; so we should be quite capable of dealing with that kind of thing.
I think this seems good (as long as the problem of installing clang-tidy on all relevant platforms/builders/Dockers gets solved; I have no special knowledge about that).

[Mordante]

libcxx/.clang-tidy

2

That can happen but I don't expect that to become an issue. In Docker we can pick a nightly build of clang-tidy-13 which should be stable. After clang-tidy-14 is released we can switch to that. In order to switch we need to update the Docker file. So the moment of the update is in our hands. (We already use this method to update to the different Clang and clang-format releases.)

I also wonder whether we need to enable it in all CI builds or only in a few. Especially since the clang-tidy run may not be cheap in resources. Once we have clang-tidy working we can see which options we like and dislike.

[ldionne]

libcxx/.clang-tidy

2

Why are we removing llvm-include-order? What does this check do?

[philnik]

libcxx/.clang-tidy

2

I'm actually adding llvm-include-order (note the - in front). It checks that the include order is correct (essentialy the same that @Quuxplusone 's include-order checker does).

[ldionne]

libcxx/.clang-tidy

2

Oh, of course, it makes sense now!

[philnik]

• Rebased
• Run clang-tidy with %{compile_flags}

[Mordante]

I had a look at the output and it seems the Docker changes haven't been available on the build bots, or am I mistaken?

[philnik]

@Mordante Yes, CI should fail. I guess we'll have to wait a bit longer.

[philnik]

Rebased

[ldionne]

The Apple CI failure is something that has been fixed now. This LGTM, however would it make sense to remove some of the checks we did using Python scripts now?

[philnik]

@ldionne I would wait until the CI nodes have clang-tidy installed. Otherwise this would break the build randomly at some point.

[• Quuxplusone]

In D117174#3284177, @ldionne wrote:

This LGTM, however would it make sense to remove some of the checks we did using Python scripts now?

I would (of course ;)) recommend against removing the Python checks, because they're simpler and more maintainable. In the sense that if they ever became annoying in some small way (like, I dunno, we decide to move <__config> back to the top of every include list) we could modify the single relevant line of Python code to fix the issue, whereas with clang-tidy it'd be much more of an all-or-nothing situation — there might be no way to "slightly adjust" a clang-tidy check without making a PR against clang-tidy itself. It's also easier to see that we test a certain invariant if it's right there in libcxx/test/, versus having to poke around in the build system. (Heck, it might even make sense to move the clang-tidy check itself into libcxx/test/, in the form of a test that verifies clang-tidy exists and then calls into it! But I don't know.)
I guess I'm philosophically puzzled: what if anything is the invariant here? Are we committing to "libc++ 14 compiles clean with clang-tidy 14" as an invariant? (If so, having a test that calls into clang-tidy would make a lot of sense.) Or are we just using clang-tidy as a handy tool because it happens to detect a few of our existing invariants out of the box (and so we're actually Hyrum's-Law-ing clang-tidy such that if it changed its behavior in certain ways we'd have to move off it again, just like we're currently Hyrum's-Law-ing python and grep)?

[philnik]

@Quuxplusone I would remove the python lint. clang-tidy is less flexible, but objectively better. It also detects #include-order problems in #ifdefs and knows what #include_next is. Putting that into a python script is not really feasible in my opinion, but prove me wrong if you want to. I also wouldn't leave them side by side, because that could potentially mean they contradict each other.

[• Quuxplusone]

In D117174#3286034, @philnik wrote:

@Quuxplusone I would remove the python lint. clang-tidy is less flexible, but objectively better. It also detects #include-order problems in #ifdefs and knows what #include_next is. Putting that into a python script is not really feasible in my opinion, but prove me wrong if you want to. I also wouldn't leave them side by side, because that could potentially mean they contradict each other.

The point of my last comment was that if they contradict each other, then the Python script is correct by definition, but clang-tidy's misbehavior might be due to a bug that's out of our control.

"It also detects #include-order problems in #ifdefs" — I'm not sure I understand this one. We certainly want to detect every consecutive run of #includes and alphabetize them, regardless of whether they're #if 0'ed (or even commented out). The Python script does this by definition (because it's not parsing C++, just looking for consecutive runs of #include lines). I don't know whether clang-tidy can handle #if or commented-out code (like our synopsis comments). I've been assuming it cannot. But anyway, "Don't make me think!" — rather than depend on reading clang-tidy's documentation to find out its behavior on #if and so on, it's strictly simpler to just say "here's a script that checks the invariant we care about." (I just checked again to make sure: the loop in lint_headers.py is only 12 lines long.)

OTOH, for tasks where we want to skip commented-out synopsis code, such as making sure all our parameter names outside the synopsis are properly _Uglified, that would be a great fit for clang-tidy and a relatively worse fit for Python (although certainly still doable — thank goodness comments are easy to parse (because another libc++ invariant is that we don't use R"(aw)" strings)).

[philnik]

In D117174#3287755, @Quuxplusone wrote:

In D117174#3286034, @philnik wrote:

@Quuxplusone I would remove the python lint. clang-tidy is less flexible, but objectively better. It also detects #include-order problems in #ifdefs and knows what #include_next is. Putting that into a python script is not really feasible in my opinion, but prove me wrong if you want to. I also wouldn't leave them side by side, because that could potentially mean they contradict each other.

The point of my last comment was that if they contradict each other, then the Python script is correct by definition, but clang-tidy's misbehavior might be due to a bug that's out of our control.

"It also detects #include-order problems in #ifdefs" — I'm not sure I understand this one. We certainly want to detect every consecutive run of #includes and alphabetize them, regardless of whether they're #if 0'ed (or even commented out). The Python script does this by definition (because it's not parsing C++, just looking for consecutive runs of #include lines). I don't know whether clang-tidy can handle #if or commented-out code (like our synopsis comments). I've been assuming it cannot. But anyway, "Don't make me think!" — rather than depend on reading clang-tidy's documentation to find out its behavior on #if and so on, it's strictly simpler to just say "here's a script that checks the invariant we care about." (I just checked again to make sure: the loop in lint_headers.py is only 12 lines long.)

OTOH, for tasks where we want to skip commented-out synopsis code, such as making sure all our parameter names outside the synopsis are properly _Uglified, that would be a great fit for clang-tidy and a relatively worse fit for Python (although certainly still doable — thank goodness comments are easy to parse (because another libc++ invariant is that we don't use R"(aw)" strings)).

I thought there was a case where your script didn't check, but I can't reproduce it. I'd be happy to disable llvm-include-order if you add support for #include_next in lint_headers.sh.py.

[philnik]

Rebased

[ldionne]

If clang-tidy does provide coverage for all that libcxx/test/libcxx/lint/lint_headers.sh.py does (and I think it does), I would remove it too. Maintaining less infrastructure/testing code is better in the long term.

@philnik Can you do that in this patch?

We will have to wait for the nodes to get the updated image before this can be checked-in... for some reason this is terribly long..

[philnik]

In D117174#3298131, @ldionne wrote:

If clang-tidy does provide coverage for all that libcxx/test/libcxx/lint/lint_headers.sh.py does (and I think it does), I would remove it too. Maintaining less infrastructure/testing code is better in the long term.

@philnik Can you do that in this patch?

We will have to wait for the nodes to get the updated image before this can be checked-in... for some reason this is terribly long..

I'll update the patch tomorrow. I don't want to block CI for other patches right now. Is there any way to check if the nodes have clang-tidy installed without re-running CI?

[philnik]

• Remove lint_headers.sh.py

[• Quuxplusone]

libcxx/test/libcxx/clang_tidy.sh.cpp

13–14

What does this pragma do, and what goes wrong if you omit it?

libcxx/test/libcxx/lint/lint_headers.sh.py

1

Please don't delete lint_headers.sh.py. It does things that clang-tidy can't, such as check for the existence of # pragma GCC system_header and (soon) check for the existence of #include <__config>.
We can discuss whether the 12 lines concerned with include-sorting are now permanently obsoleted (because we trust clang-tidy to check include-sorting forever more), but FYI personally I don't trust clang-tidy and personally I don't think 12 lines is too much to pay for peace of mind. Again, if clang-tidy ever disagreed with this script, clang-tidy would be wrong by definition.

libcxx/utils/libcxx/test/features.py

109–110

Consider indenting to match the other lines.

[philnik]

libcxx/test/libcxx/clang_tidy.sh.cpp

13–14

It ignores #warnings warnings. This test also checks <ext/hash_map> and <ext/hash_set>, which warn, because they have been replaced by <unordered_map> and <unordered_set>.

libcxx/test/libcxx/lint/lint_headers.sh.py

1

clang-tidy does exactly the same thing as this script. I don't think it's much of an effort to re-create the parts of this script that do the include sorting, in case we want something else. One advantage of clang-tidy is that it is shown in your IDE. If you don't use an IDE OK, but for the people that do use and IDE (with clang-tidy integration), clang-tidy will warn on include-order in the IDE and you don't have to run the tests to check that. That is simply not possible to do with a script portably.

Why don't you trust clang-tidy? Do you think it will break some code? Why would clang-tidy be wrong by definition?
I understand that you want to keep this script. It does things clang-tidy is not designed to do. But why don't you want clang-tidy to check for includes?

[• Quuxplusone]

libcxx/test/libcxx/lint/lint_headers.sh.py

1

clang-tidy does exactly the same thing as this script.

That's untrue.

I don't think it's much of an effort to re-create the parts of this script that do the include sorting

Right. It's like 12 lines. Just leave them in place. If we later find out there's a problem with them, we can fix them. I'm not objecting to your adding the dependency on clang-tidy; I'm objecting to removing this lint script (which checks for include ordering in 12 lines of code, and checks for #pragma GCC system_header in 4 more lines, and will soon check for #include <__config> in another 4.) Go ahead and add clang-tidy to your workflow. Just please don't remove this script, that's all.

Why would clang-tidy be wrong by definition?

If clang-tidy ever disagrees with this script, then clang-tidy is wrong by definition, because this script (by definition) will always express exactly the invariants that libc++ wants to check for. If clang-tidy (hypothetically) ever failed to catch something that this script caught, that would be a false negative in clang-tidy (and maybe we'd want to file a bug upstream for it). If clang-tidy (hypothetically) ever complained about something that this script considered OK, that would be a false positive in clang-tidy (and maybe we'd want to file a bug upstream for it). I'm not claiming that clang-tidy is necessarily wrong; I'm just saying that libc++'s own custom invariant-checking script is correct by definition, so if clang-tidy ever disagreed with this script then obviously (by definition) clang-tidy would be producing the "wrong" output.

libcxx/utils/generate_header_tests.py

209

a-z order plz

[philnik]

libcxx/test/libcxx/lint/lint_headers.sh.py

1

(whenever I say 'script' I mean the include-order check)
My problem with leaving them both in is that it would be possible that they contradict each other, and I don't see a good reason to have this possibility. You could just as well define that clang-tidy is always right and that would make the script wrong. If we remove the script we have eliminated the possibility of contradiction while having no downside AFAICT.

[philnik]

• Rebased
• Only remove include-order check from lint_headers.sh.py

[ldionne]

Is there any way to check if the nodes have clang-tidy installed without re-running CI?

Unfortunately, no.

@Quuxplusone
Honestly, I don't understand the objection with removing the part of lint_headers.py that deals with include ordering. IMO, we should remove it (as the current patch does) and change the top-level comment from

# Verify that each run of consecutive #include directives
# in each libcxx/include/ header is maintained in alphabetical order.

to something like

# This script contains a few consistency checks for the libc++ headers that
# can't be enforced with clang-tidy, such as checking that we use
# pragma system_header consistently, etc.

We normally strive to have one canonical way of doing things, and the canonical way of checking for include order is to use clang-tidy. We don't need a second one -- I think the only thing it adds is potential for confusion.

The only argument I could imagine in favour of this is that it allows "dropping" a dependency on clang-tidy for local development, however I think we should actually go the other way and encourage people to install clang-tidy so that this test is run locally.

[• Quuxplusone]

libcxx/test/libcxx/lint/lint_headers.sh.py

59

I'd still very strongly prefer to keep these 12 lines, as a check against clang-tidy's behavior (which we're not sure what it is). After all, right now it's possible that clang-tidy doesn't check include order at all; or checks it only in the public headers; or fails to check anything not-included or guarded under an #ifdef. (And more importantly from the engineering POV, we don't know if it will behave the same 3 years from now.)
However, removing these 12 lines is at least acceptable, as a conscious compromise to get D117174 closed out.

[philnik]

Rebased

[philnik]

• Fix GCC, XFAIL modules

[• Quuxplusone]

libcxx/test/libcxx/clang_tidy.sh.cpp

10–13

XFAIL: modules

What's the problem with modules? (Preferably in the form of an English explanation plus a link to the failing buildkite.)

13–14

The right way to do this is

// Prevent <ext/hash_map> from generating deprecated warnings for this test.
#if defined(__DEPRECATED)
#    undef __DEPRECATED
#endif

Look at some of the other generated header tests for inspiration.

[philnik]

• Fix CI

libcxx/test/libcxx/clang_tidy.sh.cpp

10–13

https://buildkite.com/llvm-project/libcxx-ci/builds/8619
It complains with use of private header from outside its module and function-like macro '__has_keyword' is not defined. Also a few other macros.

libcxx/utils/libcxx/test/features.py

109–110

Moved it up to 'executor-has-no-bash'.

[philnik]

• Rebased
• Fix modules

[philnik]

re-upload correct diff

[• Quuxplusone]

Looks acceptable to me mod this last round of comments. It should be rebased after D119295 and re-uploaded, but then (if that works, which is why I'm still "requesting changes" on this round) I don't see a really compelling reason not to land it...

libcxx/test/libcxx/clang_tidy.sh.cpp

10

I think this should just be REQUIRES: has-clang-tidy

10–13

Hmm, looks like clang-tidy doesn't play well with modules, huh? OK.

This reminds me that clang-tidy doesn't look inside #ifdefs either. So we're losing test coverage for include ordering in any detail header that's not included on a platform we test in buildkite. I don't like that, but I'm willing to ignore it for now and just revisit once it becomes a problem. (The obvious way to do that is for me to just keep running the original lint-headers.sh.py locally every month until it fails, and then complain about it at that point. But I'm almost certainly too lazy for that.)

13

The number of -s on this line will bit-rot quickly. Consider replacing with a full-sentence comment like

// -Wno-unknown-warning-option tells clang-tidy to ignore "#pragma GCC diagnostic" options it doesn't recognize.

Although, that shouldn't be a problem now that D119295 has landed. So can you remove -Wno-unknown-warning-option at this point?

[philnik]

• Rebased
• Address comments

[philnik]

libcxx/test/libcxx/clang_tidy.sh.cpp

13

-Wno-unknown-warning-options is necessary so that clang-tidy ignores -W* command line arguments it doesn't know. I hope the new comment makes that clear.

[philnik]

• Update comment

[• Quuxplusone]

libcxx/utils/libcxx/test/features.py

86–87

The only remaining thing to keep me up at night is: How sure can we be that this feature is ever enabled? Would we notice if has-clang-tidy were never true on any buildbot?

No action required, I'm just curious if anyone has a reassuring answer.

[philnik]

libcxx/utils/libcxx/test/features.py

86–87

I guess people would notice if the tests fail locally, but all CI runs pass. But I don't think there is any guarantee.

[uabelho]

HI,

No idea what's happening but I get this error on trunk

FAIL: libc++ :: libcxx/clang_tidy.sh.cpp (600 of 7776)
******************** TEST 'libc++ :: libcxx/clang_tidy.sh.cpp' FAILED ********************
Script:
--
: 'RUN: at line 11';   clang-tidy /repo/uabelho/master-github/libcxx/test/libcxx/clang_tidy.sh.cpp --warnings-as-errors=* -header-filter=.* -- -Wno-unknown-warning-option -nostdinc++ -isystem /repo/uabelho/master-github/llvm/build-all-builtins/include/x86_64-unknown-linux-gnu/c++/v1 -isystem /repo/uabelho/master-github/llvm/build-all-builtins/include/c++/v1 -I/repo/uabelho/master-github/llvm/build-all-builtins/runtimes/runtimes-x86_64-unknown-linux-gnu-bins/libcxx/include/c++build -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -D__STDC_CONSTANT_MACROS -I/repo/uabelho/master-github/libcxx/test/support -std=c++2b -Werror -Wall -Wextra -Wshadow -Wundef -Wno-unused-command-line-argument -Wno-attributes -Wno-pessimizing-move -Wno-c++11-extensions -Wno-user-defined-literals -Wno-noexcept-type -Wno-atomic-alignment -Wsign-compare -Wunused-variable -Wunused-parameter -Wunreachable-code -Wno-unused-local-typedef -D_LIBCPP_DISABLE_AVAILABILITY -fcoroutines-ts -Werror=thread-safety -Wuser-defined-warnings -D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER
--
Exit Code: 1

Command Output (stdout):
--
$ ":" "RUN: at line 11"
note: command had no output on stdout or stderr
$ "clang-tidy" "/repo/uabelho/master-github/libcxx/test/libcxx/clang_tidy.sh.cpp" "--warnings-as-errors=*" "-header-filter=.*" "--" "-Wno-unknown-warning-option" "-nostdinc++" "-isystem" "/repo/uabelho/master-github/llvm/build-all-builtins/include/x86_64-unknown-linux-gnu/c++/v1" "-isystem" "/repo/uabelho/master-github/llvm/build-all-builtins/include/c++/v1" "-I/repo/uabelho/master-github/llvm/build-all-builtins/runtimes/runtimes-x86_64-unknown-linux-gnu-bins/libcxx/include/c++build" "-D__STDC_FORMAT_MACROS" "-D__STDC_LIMIT_MACROS" "-D__STDC_CONSTANT_MACROS" "-I/repo/uabelho/master-github/libcxx/test/support" "-std=c++2b" "-Werror" "-Wall" "-Wextra" "-Wshadow" "-Wundef" "-Wno-unused-command-line-argument" "-Wno-attributes" "-Wno-pessimizing-move" "-Wno-c++11-extensions" "-Wno-user-defined-literals" "-Wno-noexcept-type" "-Wno-atomic-alignment" "-Wsign-compare" "-Wunused-variable" "-Wunused-parameter" "-Wunreachable-code" "-Wno-unused-local-typedef" "-D_LIBCPP_DISABLE_AVAILABILITY" "-fcoroutines-ts" "-Werror=thread-safety" "-Wuser-defined-warnings" "-D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER"
# command output:
error: invalid value 'c++2b' in '-std=c++2b' [clang-diagnostic-error]
note: use 'c++98' or 'c++03' for 'ISO C++ 1998 with amendments' standard
note: use 'gnu++98' or 'gnu++03' for 'ISO C++ 1998 with amendments and GNU extensions' standard
note: use 'c++11' for 'ISO C++ 2011 with amendments' standard
note: use 'gnu++11' for 'ISO C++ 2011 with amendments and GNU extensions' standard
note: use 'c++14' for 'ISO C++ 2014 with amendments' standard
note: use 'gnu++14' for 'ISO C++ 2014 with amendments and GNU extensions' standard
note: use 'c++17' for 'ISO C++ 2017 with amendments' standard
note: use 'gnu++17' for 'ISO C++ 2017 with amendments and GNU extensions' standard
note: use 'c++2a' for 'Working draft for ISO C++ 2020' standard
note: use 'gnu++2a' for 'Working draft for ISO C++ 2020 with GNU extensions' standard
/repo/uabelho/master-github/llvm/build-all-builtins/include/c++/v1/type_traits:4092:58: error: use of undeclared identifier '__builtin_is_constant_evaluated'; did you mean '__libcpp_is_constant_evaluated'? [clang-diagnostic-error]
bool __libcpp_is_constant_evaluated() _NOEXCEPT { return __builtin_is_constant_evaluated(); }
                                                         ^
/repo/uabelho/master-github/llvm/build-all-builtins/include/c++/v1/type_traits:4092:6: note: '__libcpp_is_constant_evaluated' declared here
bool __libcpp_is_constant_evaluated() _NOEXCEPT { return __builtin_is_constant_evaluated(); }
     ^

# command stderr:
YAML:1:22: error: unknown key 'InheritParentConfig'
InheritParentConfig: true
                     ^~~~
Error parsing /repo/uabelho/master-github/libcxx/.clang-tidy: Invalid argument
YAML:1:22: error: unknown key 'InheritParentConfig'
InheritParentConfig: true
                     ^~~~
Error parsing /repo/uabelho/master-github/llvm/.clang-tidy: Invalid argument
25374 warnings and 2 errors generated.
Error while processing /repo/uabelho/master-github/libcxx/test/libcxx/clang_tidy.sh.cpp.
Suppressed 25374 warnings (25374 in non-user code).
Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
Found compiler error(s).

error: command failed with exit status: 1

Any idea?

[bjope]

In D117174#3325369, @uabelho wrote:

HI,

No idea what's happening but I get this error on trunk

FAIL: libc++ :: libcxx/clang_tidy.sh.cpp (600 of 7776)
******************** TEST 'libc++ :: libcxx/clang_tidy.sh.cpp' FAILED ********************
Script:
--
: 'RUN: at line 11';   clang-tidy /repo/uabelho/master-github/libcxx/test/libcxx/clang_tidy.sh.cpp --warnings-as-errors=* -header-filter=.* -- -Wno-unknown-warning-option -nostdinc++ -isystem /repo/uabelho/master-github/llvm/build-all-builtins/include/x86_64-unknown-linux-gnu/c++/v1 -isystem /repo/uabelho/master-github/llvm/build-all-builtins/include/c++/v1 -I/repo/uabelho/master-github/llvm/build-all-builtins/runtimes/runtimes-x86_64-unknown-linux-gnu-bins/libcxx/include/c++build -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -D__STDC_CONSTANT_MACROS -I/repo/uabelho/master-github/libcxx/test/support -std=c++2b -Werror -Wall -Wextra -Wshadow -Wundef -Wno-unused-command-line-argument -Wno-attributes -Wno-pessimizing-move -Wno-c++11-extensions -Wno-user-defined-literals -Wno-noexcept-type -Wno-atomic-alignment -Wsign-compare -Wunused-variable -Wunused-parameter -Wunreachable-code -Wno-unused-local-typedef -D_LIBCPP_DISABLE_AVAILABILITY -fcoroutines-ts -Werror=thread-safety -Wuser-defined-warnings -D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER
--
Exit Code: 1

Command Output (stdout):
--
$ ":" "RUN: at line 11"
note: command had no output on stdout or stderr
$ "clang-tidy" "/repo/uabelho/master-github/libcxx/test/libcxx/clang_tidy.sh.cpp" "--warnings-as-errors=*" "-header-filter=.*" "--" "-Wno-unknown-warning-option" "-nostdinc++" "-isystem" "/repo/uabelho/master-github/llvm/build-all-builtins/include/x86_64-unknown-linux-gnu/c++/v1" "-isystem" "/repo/uabelho/master-github/llvm/build-all-builtins/include/c++/v1" "-I/repo/uabelho/master-github/llvm/build-all-builtins/runtimes/runtimes-x86_64-unknown-linux-gnu-bins/libcxx/include/c++build" "-D__STDC_FORMAT_MACROS" "-D__STDC_LIMIT_MACROS" "-D__STDC_CONSTANT_MACROS" "-I/repo/uabelho/master-github/libcxx/test/support" "-std=c++2b" "-Werror" "-Wall" "-Wextra" "-Wshadow" "-Wundef" "-Wno-unused-command-line-argument" "-Wno-attributes" "-Wno-pessimizing-move" "-Wno-c++11-extensions" "-Wno-user-defined-literals" "-Wno-noexcept-type" "-Wno-atomic-alignment" "-Wsign-compare" "-Wunused-variable" "-Wunused-parameter" "-Wunreachable-code" "-Wno-unused-local-typedef" "-D_LIBCPP_DISABLE_AVAILABILITY" "-fcoroutines-ts" "-Werror=thread-safety" "-Wuser-defined-warnings" "-D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER"
# command output:
error: invalid value 'c++2b' in '-std=c++2b' [clang-diagnostic-error]
note: use 'c++98' or 'c++03' for 'ISO C++ 1998 with amendments' standard
note: use 'gnu++98' or 'gnu++03' for 'ISO C++ 1998 with amendments and GNU extensions' standard
note: use 'c++11' for 'ISO C++ 2011 with amendments' standard
note: use 'gnu++11' for 'ISO C++ 2011 with amendments and GNU extensions' standard
note: use 'c++14' for 'ISO C++ 2014 with amendments' standard
note: use 'gnu++14' for 'ISO C++ 2014 with amendments and GNU extensions' standard
note: use 'c++17' for 'ISO C++ 2017 with amendments' standard
note: use 'gnu++17' for 'ISO C++ 2017 with amendments and GNU extensions' standard
note: use 'c++2a' for 'Working draft for ISO C++ 2020' standard
note: use 'gnu++2a' for 'Working draft for ISO C++ 2020 with GNU extensions' standard
/repo/uabelho/master-github/llvm/build-all-builtins/include/c++/v1/type_traits:4092:58: error: use of undeclared identifier '__builtin_is_constant_evaluated'; did you mean '__libcpp_is_constant_evaluated'? [clang-diagnostic-error]
bool __libcpp_is_constant_evaluated() _NOEXCEPT { return __builtin_is_constant_evaluated(); }
                                                         ^
/repo/uabelho/master-github/llvm/build-all-builtins/include/c++/v1/type_traits:4092:6: note: '__libcpp_is_constant_evaluated' declared here
bool __libcpp_is_constant_evaluated() _NOEXCEPT { return __builtin_is_constant_evaluated(); }
     ^

# command stderr:
YAML:1:22: error: unknown key 'InheritParentConfig'
InheritParentConfig: true
                     ^~~~
Error parsing /repo/uabelho/master-github/libcxx/.clang-tidy: Invalid argument
YAML:1:22: error: unknown key 'InheritParentConfig'
InheritParentConfig: true
                     ^~~~
Error parsing /repo/uabelho/master-github/llvm/.clang-tidy: Invalid argument
25374 warnings and 2 errors generated.
Error while processing /repo/uabelho/master-github/libcxx/test/libcxx/clang_tidy.sh.cpp.
Suppressed 25374 warnings (25374 in non-user code).
Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
Found compiler error(s).

error: command failed with exit status: 1

Any idea?

I guess the new test ends up using clang-tidy from the PATH(?) rather than from the just built compiler. And that version might be too old if for example using llvm8.0 when building the compiler.
So should perhaps the has-clang-tidy check be more picky about which version of clang-tidy that is found?

[uabelho]

In D117174#3325739, @bjope wrote:

In D117174#3325369, @uabelho wrote:

Any idea?

I guess the new test ends up using clang-tidy from the PATH(?) rather than from the just built compiler. And that version might be too old if for example using llvm8.0 when building the compiler.
So should perhaps the has-clang-tidy check be more picky about which version of clang-tidy that is found?

Indeed it seems like that. If I run the clang-tidy RUN line explicitly using clang-tidy from the clang installation used to build the compiler I get exactly the errors printed when running the lit test.

If I run the failing command explicitly using the newly built clang-tidy it works.

So in some way the wrong clang-tidy binary is used.

[philnik]

In D117174#3325861, @uabelho wrote:

In D117174#3325739, @bjope wrote:

In D117174#3325369, @uabelho wrote:

Any idea?

I guess the new test ends up using clang-tidy from the PATH(?) rather than from the just built compiler. And that version might be too old if for example using llvm8.0 when building the compiler.
So should perhaps the has-clang-tidy check be more picky about which version of clang-tidy that is found?

Indeed it seems like that. If I run the clang-tidy RUN line explicitly using clang-tidy from the clang installation used to build the compiler I get exactly the errors printed when running the lit test.

If I run the failing command explicitly using the newly built clang-tidy it works.

So in some way the wrong clang-tidy binary is used.

I'll look into using the just built clang-tidy and checking the clang-tidy version, but I don't think it's worth reverting this patch for that. If anybody thinks it should be reverted LMK.

[ldionne]

In D117174#3326677, @philnik wrote:

I'll look into using the just built clang-tidy and checking the clang-tidy version, but I don't think it's worth reverting this patch for that. If anybody thinks it should be reverted LMK.

@philnik I think what we should do is check in features.py that the clang-tidy version is sufficiently recent. We shouldn't try to use the just-built clang-tidy unless it doesn't add many dependencies to the build (but I doubt that).

libcxx/.clang-tidy

 InheritParentConfig: true
-Checks: '-readability-identifier-naming,-llvm-header-guard,-llvm-include-order,-misc-unconventional-assign-operator,-llvm-else-after-return'
+Checks: '-readability-identifier-naming,-llvm-header-guard,-misc-unconventional-assign-operator,-llvm-else-after-return'

libcxx/test/libcxx/clang_tidy.sh.cpp

+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// REQUIRES: has-clang-tidy
+// XFAIL: modules-build
+// RUN: clang-tidy %s --warnings-as-errors=* -header-filter=.* -- -Wno-unknown-warning-option %{compile_flags}
+// -Wno-unknown-warning-option tells clang-tidy to ignore '-W' command-line arguments that it doesn't know.
+// There are some GCC-specific ones where clang-tidy would warn otherwise.
+
+// Prevent <ext/hash_map> from generating deprecated warnings for this test.
+#if defined(__DEPRECATED)
+#    undef __DEPRECATED
+#endif
+
+////////////////////////////////////////////////////////////////////////////////
+// BEGIN-GENERATED-HEADERS
+////////////////////////////////////////////////////////////////////////////////
+
+// clang-format off
+
+// WARNING: This test was generated by generate_header_tests.py
+// and should not be edited manually.
+
+// Top level headers
+#include <algorithm>
+#include <any>
+#include <array>
+#include <atomic>
+#ifndef _LIBCPP_HAS_NO_THREADS
+#    include <barrier>
+#endif
+#include <bit>
+#include <bitset>
+#include <cassert>
+#include <ccomplex>
+#include <cctype>
+#include <cerrno>
+#include <cfenv>
+#include <cfloat>
+#include <charconv>
+#include <chrono>
+#include <cinttypes>
+#include <ciso646>
+#include <climits>
+#ifndef _LIBCPP_HAS_NO_LOCALIZATION
+#    include <clocale>
+#endif
+#include <cmath>
+#ifndef _LIBCPP_HAS_NO_LOCALIZATION
+#    include <codecvt>
+#endif
+#include <compare>
+#include <complex>
+#include <complex.h>
+#include <concepts>
+#include <condition_variable>
+#ifndef _LIBCPP_HAS_NO_CXX20_COROUTINES
+#    include <coroutine>
+#endif
+#include <csetjmp>
+#include <csignal>
+#include <cstdarg>
+#include <cstdbool>
+#include <cstddef>
+#include <cstdint>
+#include <cstdio>
+#include <cstdlib>
+#include <cstring>
+#include <ctgmath>
+#include <ctime>
+#include <ctype.h>
+#ifndef _LIBCPP_HAS_NO_WIDE_CHARACTERS
+#    include <cwchar>
+#endif
+#ifndef _LIBCPP_HAS_NO_WIDE_CHARACTERS
+#    include <cwctype>
+#endif
+#include <deque>
+#include <errno.h>
+#include <exception>
+#include <execution>
+#include <fenv.h>
+#ifndef _LIBCPP_HAS_NO_FILESYSTEM_LIBRARY
+#    include <filesystem>
+#endif
+#include <float.h>
+#ifndef _LIBCPP_HAS_NO_INCOMPLETE_FORMAT
+#    include <format>
+#endif
+#include <forward_list>
+#ifndef _LIBCPP_HAS_NO_LOCALIZATION
+#    include <fstream>
+#endif
+#include <functional>
+#ifndef _LIBCPP_HAS_NO_THREADS
+#    include <future>
+#endif
+#include <initializer_list>
+#include <inttypes.h>
+#ifndef _LIBCPP_HAS_NO_LOCALIZATION
+#    include <iomanip>
+#endif
+#ifndef _LIBCPP_HAS_NO_LOCALIZATION
+#    include <ios>
+#endif
+#include <iosfwd>
+#ifndef _LIBCPP_HAS_NO_LOCALIZATION
+#    include <iostream>
+#endif
+#ifndef _LIBCPP_HAS_NO_LOCALIZATION
+#    include <istream>
+#endif
+#include <iterator>
+#ifndef _LIBCPP_HAS_NO_THREADS
+#    include <latch>
+#endif
+#include <limits>
+#include <limits.h>
+#include <list>
+#ifndef _LIBCPP_HAS_NO_LOCALIZATION
+#    include <locale>
+#endif
+#ifndef _LIBCPP_HAS_NO_LOCALIZATION
+#    include <locale.h>
+#endif
+#include <map>
+#include <math.h>
+#include <memory>
+#ifndef _LIBCPP_HAS_NO_THREADS
+#    include <mutex>
+#endif
+#include <new>
+#include <numbers>
+#include <numeric>
+#include <optional>
+#ifndef _LIBCPP_HAS_NO_LOCALIZATION
+#    include <ostream>
+#endif
+#include <queue>
+#include <random>
+#ifndef _LIBCPP_HAS_NO_INCOMPLETE_RANGES
+#    include <ranges>
+#endif
+#include <ratio>
+#ifndef _LIBCPP_HAS_NO_LOCALIZATION
+#    include <regex>
+#endif
+#include <scoped_allocator>
+#ifndef _LIBCPP_HAS_NO_THREADS
+#    include <semaphore>
+#endif
+#include <set>
+#include <setjmp.h>
+#ifndef _LIBCPP_HAS_NO_THREADS
+#    include <shared_mutex>
+#endif
+#include <span>
+#ifndef _LIBCPP_HAS_NO_LOCALIZATION
+#    include <sstream>
+#endif
+#include <stack>
+#include <stdbool.h>
+#include <stddef.h>
+#include <stdexcept>
+#include <stdint.h>
+#include <stdio.h>
+#include <stdlib.h>
+#ifndef _LIBCPP_HAS_NO_LOCALIZATION
+#    include <streambuf>
+#endif
+#include <string>
+#include <string.h>
+#include <string_view>
+#ifndef _LIBCPP_HAS_NO_LOCALIZATION
+#    include <strstream>
+#endif
+#include <system_error>
+#include <tgmath.h>
+#ifndef _LIBCPP_HAS_NO_THREADS
+#    include <thread>
+#endif
+#include <tuple>
+#include <type_traits>
+#include <typeindex>
+#include <typeinfo>
+#include <unordered_map>
+#include <unordered_set>
+#include <utility>
+#include <valarray>
+#include <variant>
+#include <vector>
+#include <version>
+#ifndef _LIBCPP_HAS_NO_WIDE_CHARACTERS
+#    include <wchar.h>
+#endif
+#ifndef _LIBCPP_HAS_NO_WIDE_CHARACTERS
+#    include <wctype.h>
+#endif
+
+// experimental headers
+#if __cplusplus >= 201103L
+#    include <experimental/algorithm>
+#    ifndef _LIBCPP_HAS_NO_EXPERIMENTAL_COROUTINES
+#        include <experimental/coroutine>
+#    endif
+#    include <experimental/deque>
+#    include <experimental/forward_list>
+#    include <experimental/functional>
+#    include <experimental/iterator>
+#    include <experimental/list>
+#    include <experimental/map>
+#    include <experimental/memory_resource>
+#    include <experimental/propagate_const>
+#    ifndef _LIBCPP_HAS_NO_LOCALIZATION
+#        include <experimental/regex>
+#    endif
+#    include <experimental/set>
+#    include <experimental/simd>
+#    include <experimental/string>
+#    include <experimental/type_traits>
+#    include <experimental/unordered_map>
+#    include <experimental/unordered_set>
+#    include <experimental/utility>
+#    include <experimental/vector>
+#endif // __cplusplus >= 201103L
+
+// extended headers
+#include <ext/hash_map>
+#include <ext/hash_set>
+
+// clang-format on
+
+////////////////////////////////////////////////////////////////////////////////
+// END-GENERATED-HEADERS
+////////////////////////////////////////////////////////////////////////////////

libcxx/test/libcxx/lint/lint_headers.sh.py

 # RUN: %{python} %s
 
 # Verify that each run of consecutive #include directives
 # in each libcxx/include/ header is maintained in alphabetical order.
 
 import glob
 import os
 import re
 
if __name__ == '__main__':
     for fname in all_headers:
         with open(fname, 'r') as f:
             lines = f.readlines()
 
         if '#  pragma GCC system_header\n' not in lines:
             if pretty(fname) not in ['__undef_macros']:
                 okay = False
                 print('FAILED TO FIND #  pragma GCC system_header in libcxx/include/%s!' % pretty(fname))
-
-        # Examine each consecutive run of #include directives.
-        prevline = None
-        for line in lines:
-            if re.match(r'^\s*#\s*include ', line):
-                if (prevline is not None) and (line < prevline):
-                    okay = False
-                    print('LINES OUT OF ORDER in libcxx/include/%s!' % pretty(fname))
-                    print(prevline)
-                    print(line)
-                prevline = line
-            else:
-                prevline = None
     assert okay

libcxx/utils/generate_header_tests.py

test_str = headers_template.format(
         ),
     )
 
     replace_generated_headers(os.path.join(
         libcxx_test_path, test_filename), test_str)
 
 
 def main():
+    produce_test('clang_tidy.sh.cpp')
     produce_test('double_include.sh.cpp')
     produce_test('min_max_macros.compile.pass.cpp', post_include='TEST_MACROS();')
     produce_test('nasty_macros.compile.pass.cpp')
     produce_test('no_assert_include.compile.pass.cpp', exclusions=['cassert'])
 
 
 if __name__ == '__main__':
     main()

libcxx/utils/libcxx/test/features.py

DEFAULT_FEATURES = [
   #
   # For the corner case of bash existing, but it being missing in the path
   # set in %{exec} as "--env PATH=one-single-dir", the executor does find
   # and executes bash, but bash then can't find any other common shell
   # utilities. Test executing "bash -c 'bash --version'" to see if bash
   # manages to find binaries to execute.
   Feature(name='executor-has-no-bash',
           when=lambda cfg: runScriptExitCode(cfg, ['%{exec} bash -c \'bash --version\'']) != 0),
+  Feature(name='has-clang-tidy',
+          when=lambda cfg: runScriptExitCode(cfg, ['clang-tidy --version']) == 0),
 
   Feature(name='apple-clang',                                                                                                      when=_isAppleClang),
   Feature(name=lambda cfg: 'apple-clang-{__clang_major__}'.format(**compilerMacros(cfg)),                                          when=_isAppleClang),
   Feature(name=lambda cfg: 'apple-clang-{__clang_major__}.{__clang_minor__}'.format(**compilerMacros(cfg)),                        when=_isAppleClang),
   Feature(name=lambda cfg: 'apple-clang-{__clang_major__}.{__clang_minor__}.{__clang_patchlevel__}'.format(**compilerMacros(cfg)), when=_isAppleClang),
 
   Feature(name='clang',                                                                                                            when=_isClang,
           actions=[AddCompileFlag('-D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER')]),
   Feature(name=lambda cfg: 'clang-{__clang_major__}'.format(**compilerMacros(cfg)),                                                when=_isClang),
   Feature(name=lambda cfg: 'clang-{__clang_major__}.{__clang_minor__}'.format(**compilerMacros(cfg)),                              when=_isClang),
   Feature(name=lambda cfg: 'clang-{__clang_major__}.{__clang_minor__}.{__clang_patchlevel__}'.format(**compilerMacros(cfg)),       when=_isClang),
 
   Feature(name='gcc',                                                                                                              when=_isGCC),
   Feature(name=lambda cfg: 'gcc-{__GNUC__}'.format(**compilerMacros(cfg)),                                                         when=_isGCC),
   Feature(name=lambda cfg: 'gcc-{__GNUC__}.{__GNUC_MINOR__}'.format(**compilerMacros(cfg)),                                        when=_isGCC),
   Feature(name=lambda cfg: 'gcc-{__GNUC__}.{__GNUC_MINOR__}.{__GNUC_PATCHLEVEL__}'.format(**compilerMacros(cfg)),                  when=_isGCC),
 
   Feature(name='msvc',                                                                                                             when=_isMSVC),
   Feature(name=lambda cfg: 'msvc-{}'.format(*_msvcVersion(cfg)),                                                                   when=_isMSVC),
   Feature(name=lambda cfg: 'msvc-{}.{}'.format(*_msvcVersion(cfg)),                                                                when=_isMSVC),
 ]
 
 # Deduce and add the test features that that are implied by the #defines in
 # the <__config_site> header.
 #
 # For each macro of the form `_LIBCPP_XXX_YYY_ZZZ` defined below that
 # is defined after including <__config_site>, add a Lit feature called
 # `libcpp-xxx-yyy-zzz`. When a macro is defined to a specific value
 # (e.g. `_LIBCPP_ABI_VERSION=2`), the feature is `libcpp-xxx-yyy-zzz=<value>`.
 macros = {
   '_LIBCPP_HAS_NO_MONOTONIC_CLOCK': 'libcpp-has-no-monotonic-clock',