https://github.com/Bigcheese created https://github.com/llvm/llvm-project/pull/206199
Before this patch `__has_feature(modules)` is true for any compilation with `-std=c++20` as it was trigged by `LangOpts.Modules`. This is unexpected and breaks some usages of `__has_feature(modules)`, which is intended to only be true when Clang modules are in use. Public code contains many examples of the form: ``` #if __has_feature(modules) @import Foundation; #else #import <Foundation/Foundation.h> #endif ``` which will never work if module maps are not in use, but is still evaluated as true. This patch adds a separate `LangOpts.ClangModules` which is set by `-fmodules`. This indicates that the user intended the Clang modules machinery to be used. We should be extremely careful in usage of this LangOpt. Clang modules and C++20 named modules are intended to work together, so this flag really only means that module maps are expected to be in use. Fixes rdar://177715262 Assisted-by Claude Code: opus-4-8 >From bae2b0b5b2bc7c6473a6f877bb7ea750ff11c437 Mon Sep 17 00:00:00 2001 From: Michael Spencer <[email protected]> Date: Wed, 17 Jun 2026 14:20:23 -0700 Subject: [PATCH] [clang] __has_feature(modules) should only be true for Clang modules Before this patch `__has_feature(modules)` is true for any compilation with `-std=c++20` as it was trigged by `LangOpts.Modules`. This is unexpected and breaks some usages of `__has_feature(modules)`, which is intended to only be true when Clang modules are in use. Public code contains many examples of the form: ``` #if __has_feature(modules) @import Foundation; #else #import <Foundation/Foundation.h> #endif ``` which will never work if module maps are not in use, but is still evaluated as true. This patch adds a separate `LangOpts.ClangModules` which is set by `-fmodules`. This indicates that the user intended the Clang modules machinery to be used. We should be extremely careful in usage of this LangOpt. Clang modules and C++20 named modules are intended to work together, so this flag really only means that module maps are expected to be in use. Fixes rdar://177715262 Assisted-by Claude Code: opus-4-8 --- clang/docs/ReleaseNotes.rst | 18 ++++++++++++++++++ clang/include/clang/Basic/Features.def | 4 ++-- clang/include/clang/Basic/LangOptions.def | 1 + clang/include/clang/Options/Options.td | 2 +- clang/lib/Frontend/CompilerInvocation.cpp | 5 +++++ clang/test/Lexer/has_feature_modules.m | 7 +++++++ 6 files changed, 34 insertions(+), 3 deletions(-) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index a2439ccb0452a..0bd82deaf5e6f 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -71,6 +71,24 @@ C/C++ Language Potentially Breaking Changes Clang would previously ``break`` out of the ``while`` loop, whereas GCC (since version 9) would ``break`` out of the ``for`` loop here. Now, Clang and GCC both break out of the ``for`` loop. +- ``__has_feature(modules)`` is no longer true when just ``-std=c++20`` + (or higher) is passed. It's only true if ``-fmodules`` is passed, which + enables Clang's module map semantics. Code of the form + + .. code-block:: c++ + + #if __has_feature(modules) + @import Foundation; + #else + #import <Foundation/Foundation.h> + #endif + + previously took the ``@import`` branch under ``-std=c++20`` even though no + module maps were in use, which would always fail. + + ``__cpp_modules`` continues to be the standard macro to use to check if C++20 + modules are available. + C++ Specific Potentially Breaking Changes ----------------------------------------- diff --git a/clang/include/clang/Basic/Features.def b/clang/include/clang/Basic/Features.def index 2f7741ff74f9b..f0a9c4889ea2d 100644 --- a/clang/include/clang/Basic/Features.def +++ b/clang/include/clang/Basic/Features.def @@ -186,7 +186,7 @@ FEATURE(objc_default_synthesize_properties, LangOpts.ObjC) FEATURE(objc_fixed_enum, LangOpts.ObjC) FEATURE(objc_instancetype, LangOpts.ObjC) FEATURE(objc_kindof, LangOpts.ObjC) -FEATURE(objc_modules, LangOpts.ObjC && LangOpts.Modules) +FEATURE(objc_modules, LangOpts.ObjC && LangOpts.ClangModules) FEATURE(objc_nonfragile_abi, LangOpts.ObjCRuntime.isNonFragile()) FEATURE(objc_property_explicit_atomic, true) FEATURE(objc_protocol_qualifier_mangling, true) @@ -318,7 +318,7 @@ FEATURE(cfi_nvcall_sanitizer, LangOpts.Sanitize.has(SanitizerKind::CFINVCall)) FEATURE(cfi_vcall_sanitizer, LangOpts.Sanitize.has(SanitizerKind::CFIVCall)) FEATURE(kcfi, LangOpts.Sanitize.has(SanitizerKind::KCFI)) FEATURE(kcfi_arity, LangOpts.Sanitize.has(SanitizerKind::KCFI)) -FEATURE(modules, LangOpts.Modules) +FEATURE(modules, LangOpts.ClangModules) FEATURE(safe_stack, LangOpts.Sanitize.has(SanitizerKind::SafeStack)) FEATURE(shadow_call_stack, LangOpts.Sanitize.has(SanitizerKind::ShadowCallStack)) diff --git a/clang/include/clang/Basic/LangOptions.def b/clang/include/clang/Basic/LangOptions.def index d68784b7efbcd..b4d03a1a3e0b5 100644 --- a/clang/include/clang/Basic/LangOptions.def +++ b/clang/include/clang/Basic/LangOptions.def @@ -153,6 +153,7 @@ LANGOPT(Blocks , 1, 0, NotCompatible, "blocks extension to C") LANGOPT(EmitAllDecls , 1, 0, Benign, "emitting all declarations") LANGOPT(MathErrno , 1, 1, NotCompatible, "errno in math functions") LANGOPT(Modules , 1, 0, NotCompatible, "modules semantics") +LANGOPT(ClangModules , 1, 0, Compatible, "Clang header modules") LANGOPT(CPlusPlusModules , 1, 0, Compatible, "C++ modules syntax") LANGOPT(SkipODRCheckInGMF , 1, 0, NotCompatible, "Skip ODR checks for decls in the global module fragment") LANGOPT(BuiltinHeadersInSystemModules, 1, 0, NotCompatible, "builtin headers belong to system modules, and _Builtin_ modules are ignored for cstdlib headers") diff --git a/clang/include/clang/Options/Options.td b/clang/include/clang/Options/Options.td index 0a864acfb6a38..e0d3c4232f5bc 100644 --- a/clang/include/clang/Options/Options.td +++ b/clang/include/clang/Options/Options.td @@ -3760,7 +3760,7 @@ defm modulemap_allow_subdirectory_search : BoolFOption <"modulemap-allow-subdire PosFlag<SetTrue, [], [], "Allow to search for module maps in subdirectories of search paths">, NegFlag<SetFalse>, BothFlags<[NoXarchOption], [ClangOption, CC1Option]>>; defm modules : BoolFOption<"modules", - LangOpts<"Modules">, Default<fcxx_modules.KeyPath>, + LangOpts<"ClangModules">, DefaultFalse, PosFlag<SetTrue, [], [ClangOption, CC1Option], "Enable the 'modules' language feature">, NegFlag<SetFalse>, BothFlags< diff --git a/clang/lib/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp index dfde7b756dbff..4aa6e67739449 100644 --- a/clang/lib/Frontend/CompilerInvocation.cpp +++ b/clang/lib/Frontend/CompilerInvocation.cpp @@ -4130,6 +4130,11 @@ bool CompilerInvocation::ParseLangArgs(LangOptions &Opts, ArgList &Args, #include "clang/Options/Options.inc" #undef LANG_OPTION_WITH_MARSHALLING + // "Modules semantics" (e.g. cross-translation-unit declaration merging) are + // needed for both Clang (header) modules and C++20 modules, so enable them + // for either. + Opts.Modules = Opts.ClangModules || Opts.CPlusPlusModules; + if (const Arg *A = Args.getLastArg(OPT_fcf_protection_EQ)) { StringRef Name = A->getValue(); if (Name == "full") { diff --git a/clang/test/Lexer/has_feature_modules.m b/clang/test/Lexer/has_feature_modules.m index 6cea3246892ad..e1f52aedd0793 100644 --- a/clang/test/Lexer/has_feature_modules.m +++ b/clang/test/Lexer/has_feature_modules.m @@ -6,6 +6,13 @@ // RUN: %clang_cc1 -E %s -o - | FileCheck --check-prefix=CHECK-NO-MODULES %s // RUN: %clang_cc1 -E -x c -fmodules %s -o - | FileCheck --check-prefix=CHECK-HAS-MODULES %s +// -std=c++20 enables C++ modules but not Clang modules, so __has_feature(modules) +// must only be set when -fmodules is also passed. +// RUN: %clang_cc1 -E -x c++ -std=c++20 %s -o - | FileCheck --check-prefix=CHECK-NO-MODULES %s +// RUN: %clang_cc1 -E -x c++ -std=c++20 -fmodules %s -o - | FileCheck --check-prefix=CHECK-HAS-MODULES %s +// RUN: %clang_cc1 -E -x objective-c++ -std=c++20 %s -o - | FileCheck --check-prefix=CHECK-NO-MODULES %s + + #if __has_feature(modules) int has_modules(); #else _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
