[Clang] Add __STDCPP_THREADS__ to standard predefine macros

[zequanwu]

Summary

According to https://eel.is/c++draft/cpp.predefined#2.6, __STDCPP_THREADS__ is a predefined macro.

[zequanwu]

Add a test case.

[rnk]

clang/include/clang/Frontend/FrontendOptions.h

307

This doesn't seem to fit with the other frontend options. This seems more like a language option. See the existing PosixThreads langopt.

If you add a langopt, you should be able to remove the option from CodeGenOptions, since codegen can check language options.

[zequanwu]

Add ThreadModel to LangOptions and remove it from CodegenOption.

[rnk]

clang/test/CXX/cpp/cpp.predefined/p2.cpp

1 ↗

(On Diff #306724)

Let's expand on this:

• test that we don't set the macro when compiling C (-x c)
• test that we don't set the macro when -mthread-model single is passed

Pass an extra macro def on the command line to control the preprocessor test expectations similar to p1.cpp

[zequanwu]

Update tests.

[zequanwu]

clang/test/CXX/cpp/cpp.predefined/p2.cpp

1 ↗

(On Diff #306724)

I found split-file is handy to separate different test expectations.

[rnk]

lgtm, thanks!

[leonardchan]

Hi, it seems that p2.cpp is failing on our production builder (https://luci-milo.appspot.com/p/fuchsia/builders/prod/clang-linux-x64/b8862835022166171808):

FAIL: Clang :: CXX/cpp/cpp.predefined/p2.cpp (1210 of 26900)
******************** TEST 'Clang :: CXX/cpp/cpp.predefined/p2.cpp' FAILED ********************
Script:
--
: 'RUN: at line 1';   split-file /b/s/w/ir/x/w/llvm-project/clang/test/CXX/cpp/cpp.predefined/p2.cpp /b/s/w/ir/x/w/staging/llvm_build/tools/clang/stage2-bins/tools/clang/test/CXX/cpp/cpp.predefined/Output/p2.cpp.tmp.dir
: 'RUN: at line 2';   /b/s/w/ir/x/w/staging/llvm_build/tools/clang/stage2-bins/bin/clang -cc1 -internal-isystem /b/s/w/ir/x/w/staging/llvm_build/tools/clang/stage2-bins/lib/clang/12.0.0/include -nostdsysteminc -verify /b/s/w/ir/x/w/staging/llvm_build/tools/clang/stage2-bins/tools/clang/test/CXX/cpp/cpp.predefined/Output/p2.cpp.tmp.dir/defined.cpp
: 'RUN: at line 3';   /b/s/w/ir/x/w/staging/llvm_build/tools/clang/stage2-bins/bin/clang -cc1 -internal-isystem /b/s/w/ir/x/w/staging/llvm_build/tools/clang/stage2-bins/lib/clang/12.0.0/include -nostdsysteminc -verify -mthread-model posix /b/s/w/ir/x/w/staging/llvm_build/tools/clang/stage2-bins/tools/clang/test/CXX/cpp/cpp.predefined/Output/p2.cpp.tmp.dir/defined.cpp
: 'RUN: at line 4';   /b/s/w/ir/x/w/staging/llvm_build/tools/clang/stage2-bins/bin/clang -cc1 -internal-isystem /b/s/w/ir/x/w/staging/llvm_build/tools/clang/stage2-bins/lib/clang/12.0.0/include -nostdsysteminc -verify -mthread-model single /b/s/w/ir/x/w/staging/llvm_build/tools/clang/stage2-bins/tools/clang/test/CXX/cpp/cpp.predefined/Output/p2.cpp.tmp.dir/not-defined.cpp
: 'RUN: at line 5';   /b/s/w/ir/x/w/staging/llvm_build/tools/clang/stage2-bins/bin/clang -cc1 -internal-isystem /b/s/w/ir/x/w/staging/llvm_build/tools/clang/stage2-bins/lib/clang/12.0.0/include -nostdsysteminc -verify -x c /b/s/w/ir/x/w/staging/llvm_build/tools/clang/stage2-bins/tools/clang/test/CXX/cpp/cpp.predefined/Output/p2.cpp.tmp.dir/not-defined.cpp
--
Exit Code: 127

Command Output (stderr):
--
/b/s/w/ir/x/w/staging/llvm_build/tools/clang/stage2-bins/tools/clang/test/CXX/cpp/cpp.predefined/Output/p2.cpp.script: line 1: split-file: command not found

--

********************
Testing:  0.. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90.. 
********************
Failed Tests (1):
  Clang :: CXX/cpp/cpp.predefined/p2.cpp

This can be reproduced if you run clang tests before building llvm tools (running check-clang without running check-llvm). Should split-file be used here in the first place since it's an llvm tool? If so, should there be a way to force check-clang to build split-file before running tests?

[rnk]

I'll push a fix to add it to the cmake deps list.

[rnk]

Does rG64802d48d51d651bd2e4567b2f228f8795569542 fix it?

[leonardchan]

In D91747#2412212, @rnk wrote:

Does rG64802d48d51d651bd2e4567b2f228f8795569542 fix it?

Yup, looks like it does. Thanks for the quick fix!

[alanphipps]

Looks like _LIBCPP_HAS_NO_THREADS is being set for libcxxabi, and the build now fails with this change:

llvm-project/libcxxabi/../libcxx/include/__config:1172:2: error: _LIBCPP_HAS_NO_THREADS cannot be set when STDCPP_THREADS is set

[dmajor]

In D91747#2412346, @alanphipps wrote:

Looks like _LIBCPP_HAS_NO_THREADS is being set for libcxxabi, and the build now fails with this change:

llvm-project/libcxxabi/../libcxx/include/__config:1172:2: error: _LIBCPP_HAS_NO_THREADS cannot be set when STDCPP_THREADS is set

Indeed: LIBCXX_ENABLE_THREADS is off by default.

@zequanwu, what's your take on this... which part of the system needs to adapt?

[rnk]

If we believe the standard says that the compiler is supposed to set __STDCPP_THREADS__, then I think the libc++ #error needs to be adjusted. libcxxabi, or any other client, should be able to define _LIBCPP_HAS_NO_THREADS, and it should work, even if the compiler thinks they are allowed.

[zequanwu]

In D91747#2423960, @rnk wrote:

If we believe the standard says that the compiler is supposed to set __STDCPP_THREADS__, then I think the libc++ #error needs to be adjusted. libcxxabi, or any other client, should be able to define _LIBCPP_HAS_NO_THREADS, and it should work, even if the compiler thinks they are allowed.

So, we could remove the checking for if __STDCPP_THREADS__ and _LIBCPP_HAS_NO_THREADS are both set. And let libcxx adds flag -mthread-model single to use single thread (but this is compiler specific flag, might need a better solution).

[rnk]

In D91747#2423987, @zequanwu wrote:

So, we could remove the checking for if __STDCPP_THREADS__ and _LIBCPP_HAS_NO_THREADS are both set. And let libcxx adds flag -mthread-model single to use single thread (but this is compiler specific flag, might need a better solution).

Yes, and I think we also need to audit to see if there is any code in libc++ that checks __STDCPP_THREADS__. Actually, I went ahead and did that. It like no other code checks it, so we're probably OK to remove the libc++ check.

It's worth noting that this change will prevent users of older libc++ versions from disabling thread support with new clang. I think that's probably OK.

[dmajor]

To connect the dots for posterity: the followup was landed in D92349.

clang/include/clang/Frontend/FrontendOptions.h

public:
   unsigned IncludeTimestamps : 1;
 
   /// Should a temporary file be used during compilation.
   unsigned UseTemporary : 1;
 
   /// When using -emit-module, treat the modulemap as a system module.
   unsigned IsSystemModule : 1;
 
+  /// Wheather using -mthread-model single.
+  unsigned IsSingleThreadModel : 1;
+
   CodeCompleteOptions CodeCompleteOpts;
 
   /// Specifies the output format of the AST.
   ASTDumpOutputFormat ASTDumpFormat = ADOF_Default;
 
   enum {
     ARCMT_None,
     ARCMT_Check,

clang/lib/Frontend/CompilerInvocation.cpp

for (const auto *A : Args.filtered(OPT_fmodule_file)) {
     if (Val.find('=') == StringRef::npos)
       Opts.ModuleFiles.push_back(std::string(Val));
   }
   Opts.ModulesEmbedFiles = Args.getAllArgValues(OPT_fmodules_embed_file_EQ);
   Opts.ModulesEmbedAllFiles = Args.hasArg(OPT_fmodules_embed_all_files);
   Opts.IncludeTimestamps = !Args.hasArg(OPT_fno_pch_timestamp);
   Opts.UseTemporary = !Args.hasArg(OPT_fno_temp_file);
   Opts.IsSystemModule = Args.hasArg(OPT_fsystem_module);
+  Opts.IsSingleThreadModel =
+      Args.getLastArgValue(OPT_mthread_model, "") == "single";
 
   if (Opts.ProgramAction != frontend::GenerateModule && Opts.IsSystemModule)
     Diags.Report(diag::err_drv_argument_only_allowed_with) << "-fsystem-module"
                                                            << "-emit-module";
 
   Opts.CodeCompleteOpts.IncludeMacros
     = Args.hasArg(OPT_code_completion_macros);
   Opts.CodeCompleteOpts.IncludeCodePatterns

clang/lib/Frontend/InitPreprocessor.cpp

if (!LangOpts.CPlusPlus) {
     //   -- __STDCPP_DEFAULT_NEW_ALIGNMENT__
     //      [C++17] An integer literal of type std::size_t whose value is the
     //      alignment guaranteed by a call to operator new(std::size_t)
     //
     // We provide this in all language modes, since it seems generally useful.
     Builder.defineMacro("__STDCPP_DEFAULT_NEW_ALIGNMENT__",
                         Twine(TI.getNewAlign() / TI.getCharWidth()) +
                             TI.getTypeConstantSuffix(TI.getSizeType()));
+
+    //   -- __STDCPP_­THREADS__
+    //      Defined, and has the value integer literal 1, if and only if a
+    //      program can have more than one thread of execution.
+    if (!FEOpts.IsSingleThreadModel)
+      Builder.defineMacro("__STDCPP_THREADS__", "1");
   }
 
   // In C11 these are environment macros. In C++11 they are only defined
   // as part of <cuchar>. To prevent breakage when mixing C and C++
   // code, define these macros unconditionally. We can define them
   // unconditionally, as Clang always uses UTF-16 and UTF-32 for 16-bit
   // and 32-bit character literals.
   Builder.defineMacro("__STDC_UTF_16__", "1");

clang/test/Preprocessor/init-aarch64.c

 // AARCH64-NEXT: #define __SIZE_FMTX__ "lX"
 // AARCH64-NEXT: #define __SIZE_FMTo__ "lo"
 // AARCH64-NEXT: #define __SIZE_FMTu__ "lu"
 // AARCH64-NEXT: #define __SIZE_FMTx__ "lx"
 // AARCH64-NEXT: #define __SIZE_MAX__ 18446744073709551615UL
 // AARCH64-NEXT: #define __SIZE_TYPE__ long unsigned int
 // AARCH64-NEXT: #define __SIZE_WIDTH__ 64
 // AARCH64_CXX: #define __STDCPP_DEFAULT_NEW_ALIGNMENT__ 16UL
+// AARCH64_CXX: #define __STDCPP_THREADS__ 1
 // AARCH64-NEXT: #define __STDC_HOSTED__ 1
 // AARCH64-NEXT: #define __STDC_UTF_16__ 1
 // AARCH64-NEXT: #define __STDC_UTF_32__ 1
 // AARCH64_C: #define __STDC_VERSION__ 201710L
 // AARCH64-NEXT: #define __STDC__ 1
 // AARCH64-NEXT: #define __UINT16_C_SUFFIX__ 
 // AARCH64-NEXT: #define __UINT16_FMTX__ "hX"
 // AARCH64-NEXT: #define __UINT16_FMTo__ "ho"