Skip to content

[clang] Add /Zc:__STDC__ flag to clang-cl #68690

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 25, 2024
Merged

[clang] Add /Zc:__STDC__ flag to clang-cl #68690

merged 1 commit into from
May 25, 2024

Conversation

xbjfk
Copy link
Contributor

@xbjfk xbjfk commented Oct 10, 2023

This commit adds the /Zc:__STDC__ argument from MSVC, which defines __STDC__.
This means, alongside stronger feature parity with MSVC, that things that rely on __STDC__, such as autoconf, can work.
Link to MSVC documentation of this flag: https://learn.microsoft.com/en-us/cpp/build/reference/zc-stdc?view=msvc-170

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Oct 10, 2023
@llvmbot
Copy link
Member

llvmbot commented Oct 10, 2023

@llvm/pr-subscribers-clang-driver

@llvm/pr-subscribers-clang

Author: Reagan (xbjfk)

Changes

This commit adds the /Zc:__STDC__ argument from MSVC, which defines __STDC__.
This means, alongside stronger feature parity with MSVC, that things that rely on __STDC__, such as autoconf, can work.
Link to MSVC documentation of this flag: https://learn.microsoft.com/en-us/cpp/build/reference/zc-stdc?view=msvc-170


Full diff: https://github.com/llvm/llvm-project/pull/68690.diff

5 Files Affected:

  • (modified) clang/include/clang/Basic/LangOptions.def (+1-1)
  • (modified) clang/include/clang/Driver/Options.td (+7)
  • (modified) clang/lib/Driver/ToolChains/Clang.cpp (+10-1)
  • (modified) clang/lib/Frontend/InitPreprocessor.cpp (+1-1)
  • (modified) clang/test/Driver/cl-zc.cpp (+4)
diff --git a/clang/include/clang/Basic/LangOptions.def b/clang/include/clang/Basic/LangOptions.def
index c0ea4ecb9806a5b..97101d0166a135a 100644
--- a/clang/include/clang/Basic/LangOptions.def
+++ b/clang/include/clang/Basic/LangOptions.def
@@ -282,7 +282,7 @@ LANGOPT(HIPUseNewLaunchAPI, 1, 0, "Use new kernel launching API for HIP")
 LANGOPT(OffloadUniformBlock, 1, 0, "Assume that kernels are launched with uniform block sizes (default true for CUDA/HIP and false otherwise)")
 LANGOPT(HIPStdPar, 1, 0, "Enable Standard Parallel Algorithm Acceleration for HIP (experimental)")
 LANGOPT(HIPStdParInterposeAlloc, 1, 0, "Replace allocations / deallocations with HIP RT calls when Standard Parallel Algorithm Acceleration for HIP is enabled (Experimental)")
-
+LANGOPT(MSVCEnableStdcMacro , 1, 0, "define __STDC__ with -fms-compatability")
 LANGOPT(SizedDeallocation , 1, 0, "sized deallocation")
 LANGOPT(AlignedAllocation , 1, 0, "aligned allocation")
 LANGOPT(AlignedAllocationUnavailable, 1, 0, "aligned allocation functions are unavailable")
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index c272a7f1c398aa6..fae1dcc2b5117f7 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -2821,6 +2821,10 @@ def fms_compatibility : Flag<["-"], "fms-compatibility">, Group<f_Group>,
   Visibility<[ClangOption, CC1Option, CLOption]>,
   HelpText<"Enable full Microsoft Visual C++ compatibility">,
   MarshallingInfoFlag<LangOpts<"MSVCCompat">>;
+def fms_define_stdc : Flag<["-"], "fms-define-stdc">, Group<f_Group>,
+  Visibility<[ClangOption, CC1Option]>,
+  HelpText<"Define __STDC__ in MSVC Compatibility mode">,
+  MarshallingInfoFlag<LangOpts<"MSVCEnableStdcMacro">>;
 def fms_extensions : Flag<["-"], "fms-extensions">, Group<f_Group>,
   Visibility<[ClangOption, CC1Option, CLOption]>,
   HelpText<"Accept some non-standard constructs supported by the Microsoft compiler">,
@@ -7933,6 +7937,9 @@ def _SLASH_vd : CLJoined<"vd">, HelpText<"Control vtordisp placement">,
   Alias<vtordisp_mode_EQ>;
 def _SLASH_X : CLFlag<"X">,
   HelpText<"Do not add %INCLUDE% to include search path">, Alias<nostdlibinc>;
+def _SLASH_Zc___STDC__ : CLFlag<"Zc:__STDC__">,
+  HelpText<"Define __STDC__">,
+  Alias<fms_define_stdc>;
 def _SLASH_Zc_sizedDealloc : CLFlag<"Zc:sizedDealloc">,
   HelpText<"Enable C++14 sized global deallocation functions">,
   Alias<fsized_deallocation>;
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index bfd6c5c2864abf7..b8f3ff6cd4c91e4 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -6744,8 +6744,13 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
       options::OPT_fms_compatibility, options::OPT_fno_ms_compatibility,
       (IsWindowsMSVC && Args.hasFlag(options::OPT_fms_extensions,
                                      options::OPT_fno_ms_extensions, true)));
-  if (IsMSVCCompat)
+  if (IsMSVCCompat) {
     CmdArgs.push_back("-fms-compatibility");
+    bool IsMSVCDefineStdc = Args.hasArg(options::OPT_fms_define_stdc);
+    if (IsMSVCDefineStdc)
+      CmdArgs.push_back("-fms-define-stdc");
+  }
+
 
   if (Triple.isWindowsMSVCEnvironment() && !D.IsCLMode() &&
       Args.hasArg(options::OPT_fms_runtime_lib_EQ))
@@ -7922,6 +7927,10 @@ void Clang::AddClangCLArgs(const ArgList &Args, types::ID InputType,
    CmdArgs.push_back("-fno-wchar");
  }
 
+ if (Args.hasArg(options::OPT_fms_define_stdc)) {
+   CmdArgs.push_back("-fms-define-stdc");
+ }
+
  if (Args.hasArg(options::OPT__SLASH_kernel)) {
    llvm::Triple::ArchType Arch = getToolChain().getArch();
    std::vector<std::string> Values =
diff --git a/clang/lib/Frontend/InitPreprocessor.cpp b/clang/lib/Frontend/InitPreprocessor.cpp
index 846e5fce6de7b2c..46399f613257a74 100644
--- a/clang/lib/Frontend/InitPreprocessor.cpp
+++ b/clang/lib/Frontend/InitPreprocessor.cpp
@@ -423,7 +423,7 @@ static void InitializeStandardPredefinedMacros(const TargetInfo &TI,
   //      [C++] Whether __STDC__ is predefined and if so, what its value is,
   //      are implementation-defined.
   // (Removed in C++20.)
-  if (!LangOpts.MSVCCompat && !LangOpts.TraditionalCPP)
+  if ((!LangOpts.MSVCCompat || LangOpts.MSVCEnableStdcMacro) && !LangOpts.TraditionalCPP)
     Builder.defineMacro("__STDC__");
   //   -- __STDC_HOSTED__
   //      The integer literal 1 if the implementation is a hosted
diff --git a/clang/test/Driver/cl-zc.cpp b/clang/test/Driver/cl-zc.cpp
index c7cf5b1b6525bea..bb955222c647af3 100644
--- a/clang/test/Driver/cl-zc.cpp
+++ b/clang/test/Driver/cl-zc.cpp
@@ -123,6 +123,10 @@
 // CHECK-CHAR8_T_: "-fno-char8_t"
 
 
+// RUN: %clang_cl /dev/null /E -Xclang -dM 2> /dev/null | FileCheck -match-full-lines %s --check-prefix=STDCOFF
+// RUN: %clang_cl /dev/null /E -Xclang -dM /Zc:__STDC__ 2> /dev/null | FileCheck -match-full-lines %s --check-prefix=STDCON
+// STDCON: #define __STDC__ 1
+// STDCOFF-NOT: #define __STDC__ 1
 
 // These never warn, but don't have an effect yet.
 

@github-actions
Copy link

github-actions bot commented Oct 10, 2023

✅ With the latest revision this PR passed the C/C++ code formatter.

Copy link
Collaborator

@AaronBallman AaronBallman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for working on this! Please be sure to add a release note to clang/docs/ReleaseNotes.rst so users know about the new functionality.

@xbjfk
Copy link
Contributor Author

xbjfk commented Oct 12, 2023

Okay, I made sure it says argument unused during compilation: '/Zc:__STDC__' and never defines STDC in C++ mode, but I have couple of questions:

  • Where should I put the test to make sure -fno-ms-define-stdc is not accepted?
  • !IsCXX feels kind of wrong - is this a justified case for creating isC in clang/lib/Driver/Types.cpp?
    • now that I think about it, what should the behavior be with Objective-C? I imagine the number of people compiling Objective-C with -fms-compatibility is very small

Thank you

@xbjfk xbjfk requested a review from AaronBallman October 12, 2023 11:53
@AaronBallman
Copy link
Collaborator

Okay, I made sure it says argument unused during compilation: '/Zc:__STDC__' and never defines STDC in C++ mode, but I have couple of questions:

* Where should I put the test to make sure `-fno-ms-define-stdc` is not accepted?

I think that would make sense in clang/test/Frontend to ensure you can't do -Xclang -fno-ms-define-stdc, probably in a new test file. (I don't see an existing test that would make sense to augment.)

* !IsCXX feels kind of wrong - is this a justified case for creating `isC` in `clang/lib/Driver/Types.cpp`?

I know what you mean about it feeling kind of wrong, but it mirrors how the compiler's language options work. Not C++ mode means C mode, so I would probably not add an isC() interface. However, if either @MaskRay and @jansvoboda11 feel differently as code owners of that area, I wouldn't be opposed.

  * now that I think about it, what should the behavior be with Objective-C? I imagine the number of people compiling Objective-C with `-fms-compatibility` is very small

Objective-C is a pure extension over C, so whatever is allowed in C should generally be allowed in Objective-C. Similarly for Objective-C++ and C++.

Copy link
Collaborator

@AaronBallman AaronBallman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@cor3ntin
Copy link
Contributor

cor3ntin commented Mar 7, 2024

@xbjfk Once you fix the conflicts, would you like us to merge that for you?
Thanks!

// ZCSTDCIGNORED-NOT: #define __STDC__ 1
// ZCSTDCIGNORED: argument unused during compilation

// RUN: not %clang -Xclang -fno-ms-define-stdc %s 2>&1 | FileCheck %s --check-prefix="NOARG"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This tests that -fno-ms-define-stdc is not a cc1 option. We almost never do it. There could be a value to test whether the driver -fno-ms-define-stdc option is present.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm, this test was requested by @AaronBallman - what would you do in this case?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@MaskRay -- see #68690 (comment) as to why I think it's valuable to have this test (but if you think it's not a useful test, I don't insist on it).

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@MaskRay are you good now or do we need a change to move this forward?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the late reply. If you want to show the behavior of the driver option -fno-ms-define-stdc, feel free to add it when you think there is sufficient value.

For CC1 options, I believe we perform a negative test: hey, we check that the negative form is not present.
The value testing that seems low.

@xbjfk xbjfk force-pushed the zc-stdc branch 2 times, most recently from adfced0 to 391eb28 Compare March 10, 2024 04:18
This commit adds the /Zc:__STDC__ argument from MSVC, which defines __STDC__.
This means, alongside stronger feature parity with MSVC,
that things that rely on __STDC__, such as autoconf, can work.
Copy link
Contributor

@MaxEW707 MaxEW707 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@xbjfk Let us know if you require one of us to commit on your behalf due to not having commit access.

@xbjfk
Copy link
Contributor Author

xbjfk commented May 24, 2024

Hi @MaxEW707
Yes, I will need someone to commit this for me.
Thanks!

@MaxEW707 MaxEW707 merged commit 778dbcb into llvm:main May 25, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants