[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