Out of interest: The RecursiveASTVisitorTests are part of the ToolingTests binary while this adds a new binary TokensTest. Can you say why?

Another comment: The new binary is called TokensTest but is in a directory "Syntax". For consistency with all other unit test binaries, please either rename the binary to SyntaxTests, or rename the directory to "Tokens". (From the patch description, the former seems more appropriate.) Note the missing trailing "s" in the binary name too.

As said on IRC, a nicer fix would be to make clang-x86_64-debian-fast and friends use use CMAKE_C_COMPILER_LAUNCHER / CMAKE_CXX_COMPILER_LAUNCHER for ccache instead of setting CC / CXX – but it's also good to make the pre-cmake-3.4 way of configuring this work.

Any chance this could've caused the build breakage starting at http://lab.llvm.org:8011/builders/clang-x86_64-debian-fast/builds/14855 ?

For posterity, https://bugs.chromium.org/p/chromium/issues/detail?id=960881#c31 has the explanation for what exactly was happening in the Chromium/Android build. It's a bit different from rnk's example since no cross-TU imports are happening. It likely explains why rnk couldn't find an example that doesn't use thinlto though: To get the bug, you need to require debug info in a TU but all actual code of the TU needs to be stripped. In regular builds this can't happen, but in thinlto builds it can happen in several cases:

The test added here doesn't pass on our bots. I filed https://bugs.llvm.org/show_bug.cgi?id=41923 for that.

The motivation here is being able to build libc++ against system libc++abi on macOS. We can explicitly define a few variables to get this to work, but with the old repo layout it Just Works. This is an attempt to make it just work with the monorepo layout as well.

echristo's comments on the mail thread for this didn't make it to phab. I apologize for repeating more or less everything he said, I had looked on phab before reading email.

Thanks for the revert!

This breaking both debug info in thinlto builds and fmodules-debuginfo is probably enough to revert and go back to the drawing board.

Having a bot for this is great, under these conditions:

• It doesn't send email to regular contributors when it goes red (it can email you and me and others who'd like to opt in)
• It doesn't show up on http://lab.llvm.org:8011/console

minor tweaks for landing

Actually, "the first comment below" – phab showed them in a different order in the preview. I meant this comment: https://reviews.llvm.org/D61821#inline-549166

Like with the Lanai target: lgtm with the usual two comments (1. https://reviews.llvm.org/D61859 treatment for MCTargetDesc; 2. Consider omitting unused llvm_build_Hexago variable)

lgtm with the usual two comments (1. https://reviews.llvm.org/D61859 treatment for MCTargetDesc; 2. Consider omitting unused llvm_build_Lanai variable)

Landed in r360600, but forgot to add "Differential Revision:" line to commit message :/

I'll abandon this for now, since as said above it doesn't address the use case as I have, and I agree with Rui that it feels like the wrong approach. If I find time, I'll try to make a CL to make /force warnings ignorable.

Nice!

Awesome, thanks! Just one real question, the last comment below:

Add a test that fails without the code change and that passes with it. I don't know under which conditions this change is needed, so I apologize that I can't make a more concrete suggestion.

comments

Now with more boilerplate!

lgtm, but shouldn't we set this globally for all projects?

Looks like the new test is failing: http://lab.llvm.org:8011/builders/clang-cmake-x86_64-avx2-linux/builds/9439/steps/ninja%20check%201/logs/FAIL%3A%20LLVM%3A%3Areassociate-after-unroll.ll

In D61710#1495938, @ruiu wrote:

I'm not confident that this is what the user wants. This might be useful, but at the same time I can imagine that people want to see an entire log rather than the first part of it.

Does clang or other tools have a similar feature?

That problem only seems to happen when (thin) lto is enabled.

I agree with that.

Hello, in a bit of a https://xkcd.com/1172/ moment this breaks the chromium/android build. We have a list of "resources" (strings, bitmaps, etc) that we list in an XML file which then generates a header with lots of "IDR_foo" constants. As it turns out, now all of these resources are used on all platforms, so we use the following technique to only keep the ones actually used on android:

Did I get the bug right that this adds almost 400kB to every file that includes stddef.h?

Are you going to work on those tests?

Thanks for the context. Sounds great :)

I happened to see this go by. Is there an explanation of the overall goal somewhere? In general, requiring -- for long flags sounds like a great change to me, but there are a few exceptions: For example. lld-link should keep accepting long flags with a single dash for link.exe compatibility.

Done in r359907, thanks!

This test is broken with Xcode 9:

Merged by now.

This landed in r359825.

r359829; forgot to put "Differential Revision" in the commit message :/

Thanks! This is incomplete:

• Create clangd_lit_site_cfg_files.gni next to this build file, but path to generated files there
• Edit llvm/utils/gn/secondary/llvm/utils/llvm-lit/BUILD.gn and add remapping there