[clang] [NFC][Clang] Add test for P2843R3 - Preprocessing is never undefined (PR #192073)

Aaron Ballman via cfe-commits cfe-commits at lists.llvm.org
Wed Apr 15 05:01:55 PDT 2026


================
@@ -1362,20 +1368,30 @@ void Preprocessor::HandleDirective(Token &Result) {
       case tok::pp_module:
       case tok::pp___preprocessed_module:
       case tok::pp___preprocessed_import:
-        Diag(Result, diag::err_embedded_directive)
-            << (getLangOpts().CPlusPlusModules &&
-                Introducer.isModuleContextualKeyword(
-                    /*AllowExport=*/false))
-            << II->getName();
-        Diag(*ArgMacro, diag::note_macro_expansion_here)
-            << ArgMacro->getIdentifierInfo();
-        DiscardUntilEndOfDirective();
-        return;
+        return true;
       default:
-        break;
+        return false;
       }
+    };
+
+    // [cpp.replace.general] makes any embedded directive ill-formed in
+    // C++26; in earlier modes the construct is undefined behavior and only
+    // pedantically warned about.
+    const bool IsError = LangOpts.CPlusPlus26 ||
+                         (II && IsAlwaysUnsupported(II->getPPKeywordID()));
+
----------------
AaronBallman wrote:

I think it's reasonable as an error in all language modes but I don't think the changes here are quite correct. This is implementing https://eel.is/c++draft/cpp.replace#general-14.sentence-3 which has similar wording in C (undefined rather than requiring a diagnostic), and that's not dependent on the keyword. e.g., this should also be rejected:
```
#define FUNCTION_MACRO(...)

FUNCTION_MACRO(
#
)
```
because it's still a directive. I think the only time we need to look at identifier are the cases which don't start with a leading `#` here: https://eel.is/c++draft/cpp#pre-2 (because we already diagnose non-directives as an error: https://godbolt.org/z/s1dY3Kc4T) But those special cases also aren't handled correctly because this should be accepted (doesn't match the requirements to make it a directive):
```
#define FUNCTION_MACRO(...)

FUNCTION_MACRO(
module
)
```
but I don't think we even get into `HandleDirective` for the cases that don't start with `#` because we don't warn currently: https://godbolt.org/z/qKjjdr8sx

So I think there's a little bit more work to be done getting the diagnostics just right, but I think it's reasonable to diagnose as an error in C as well as C++. (Maybe drop the embedded directive changes from this PR so we can land it and then do those in a separate PR?)

https://github.com/llvm/llvm-project/pull/192073


More information about the cfe-commits mailing list