Skip to content

Commit 5dfa37a

Browse files
committed
Don't allow __VA_OPT__ to be detected by #ifdef.
More study has discovered this to not actually be useful: because current C++20 implementations reject `#ifdef __VA_OPT__`, this can't really be used as a feature-test mechanism. And it's not too hard to detect __VA_OPT__ without this, for example: #define THIRD_ARG(a, b, c, ...) c #define HAS_VA_OPT(...) THIRD_ARG(__VA_OPT__(,), 1, 0, ) #if HAS_VA_OPT(?) Partially reverts 0436ec2.
1 parent 44f7929 commit 5dfa37a

File tree

6 files changed

+11
-64
lines changed

6 files changed

+11
-64
lines changed

clang/include/clang/Lex/Preprocessor.h

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -447,25 +447,6 @@ class Preprocessor {
447447
ElseLoc(ElseLoc) {}
448448
};
449449

450-
class IfdefMacroNameScopeRAII {
451-
Preprocessor &PP;
452-
bool VAOPTWasPoisoned;
453-
454-
public:
455-
IfdefMacroNameScopeRAII(Preprocessor &PP)
456-
: PP(PP), VAOPTWasPoisoned(PP.Ident__VA_OPT__->isPoisoned()) {
457-
PP.Ident__VA_OPT__->setIsPoisoned(false);
458-
}
459-
IfdefMacroNameScopeRAII(const IfdefMacroNameScopeRAII&) = delete;
460-
IfdefMacroNameScopeRAII &operator=(const IfdefMacroNameScopeRAII&) = delete;
461-
~IfdefMacroNameScopeRAII() { Exit(); }
462-
463-
void Exit() {
464-
if (VAOPTWasPoisoned)
465-
PP.Ident__VA_OPT__->setIsPoisoned(true);
466-
}
467-
};
468-
469450
private:
470451
friend class ASTReader;
471452
friend class MacroArgs;

clang/lib/Lex/PPDirectives.cpp

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2928,14 +2928,9 @@ void Preprocessor::HandleIfdefDirective(Token &Result,
29282928
++NumIf;
29292929
Token DirectiveTok = Result;
29302930

2931-
// __VA_OPT__ is allowed as the operand of #if[n]def.
2932-
IfdefMacroNameScopeRAII IfdefMacroNameScope(*this);
2933-
29342931
Token MacroNameTok;
29352932
ReadMacroName(MacroNameTok);
29362933

2937-
IfdefMacroNameScope.Exit();
2938-
29392934
// Error reading macro name? If so, diagnostic already issued.
29402935
if (MacroNameTok.is(tok::eod)) {
29412936
// Skip code until we get to #endif. This helps with recovery by not

clang/lib/Lex/PPExpressions.cpp

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -104,9 +104,6 @@ static bool EvaluateDefined(PPValue &Result, Token &PeekTok, DefinedTracker &DT,
104104
SourceLocation beginLoc(PeekTok.getLocation());
105105
Result.setBegin(beginLoc);
106106

107-
// __VA_OPT__ is allowed as the operand of 'defined'.
108-
Preprocessor::IfdefMacroNameScopeRAII IfdefMacroNameScope(PP);
109-
110107
// Get the next token, don't expand it.
111108
PP.LexUnexpandedNonComment(PeekTok);
112109

@@ -125,8 +122,6 @@ static bool EvaluateDefined(PPValue &Result, Token &PeekTok, DefinedTracker &DT,
125122
PP.LexUnexpandedNonComment(PeekTok);
126123
}
127124

128-
IfdefMacroNameScope.Exit();
129-
130125
// If we don't have a pp-identifier now, this is an error.
131126
if (PP.CheckMacroName(PeekTok, MU_Other))
132127
return true;

clang/lib/Lex/PPMacroExpansion.cpp

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -323,16 +323,13 @@ void Preprocessor::dumpMacroInfo(const IdentifierInfo *II) {
323323

324324
/// RegisterBuiltinMacro - Register the specified identifier in the identifier
325325
/// table and mark it as a builtin macro to be expanded.
326-
static IdentifierInfo *RegisterBuiltinMacro(Preprocessor &PP, const char *Name,
327-
bool Disabled = false) {
326+
static IdentifierInfo *RegisterBuiltinMacro(Preprocessor &PP, const char *Name){
328327
// Get the identifier.
329328
IdentifierInfo *Id = PP.getIdentifierInfo(Name);
330329

331330
// Mark it as being a macro that is builtin.
332331
MacroInfo *MI = PP.AllocateMacroInfo(SourceLocation());
333332
MI->setIsBuiltinMacro();
334-
if (Disabled)
335-
MI->DisableMacro();
336333
PP.appendDefMacroDirective(Id, MI);
337334
return Id;
338335
}
@@ -346,7 +343,6 @@ void Preprocessor::RegisterBuiltinMacros() {
346343
Ident__TIME__ = RegisterBuiltinMacro(*this, "__TIME__");
347344
Ident__COUNTER__ = RegisterBuiltinMacro(*this, "__COUNTER__");
348345
Ident_Pragma = RegisterBuiltinMacro(*this, "_Pragma");
349-
Ident__VA_OPT__ = RegisterBuiltinMacro(*this, "__VA_OPT__", true);
350346

351347
// C++ Standing Document Extensions.
352348
if (getLangOpts().CPlusPlus)

clang/lib/Lex/Preprocessor.cpp

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -115,20 +115,19 @@ Preprocessor::Preprocessor(std::shared_ptr<PreprocessorOptions> PPOpts,
115115

116116
BuiltinInfo = std::make_unique<Builtin::Context>();
117117

118+
// "Poison" __VA_ARGS__, __VA_OPT__ which can only appear in the expansion of
119+
// a macro. They get unpoisoned where it is allowed.
120+
(Ident__VA_ARGS__ = getIdentifierInfo("__VA_ARGS__"))->setIsPoisoned();
121+
SetPoisonReason(Ident__VA_ARGS__,diag::ext_pp_bad_vaargs_use);
122+
(Ident__VA_OPT__ = getIdentifierInfo("__VA_OPT__"))->setIsPoisoned();
123+
SetPoisonReason(Ident__VA_OPT__,diag::ext_pp_bad_vaopt_use);
124+
118125
// Initialize the pragma handlers.
119126
RegisterBuiltinPragmas();
120127

121128
// Initialize builtin macros like __LINE__ and friends.
122129
RegisterBuiltinMacros();
123130

124-
// "Poison" __VA_ARGS__, __VA_OPT__ which can only appear in the expansion of
125-
// a macro. They get unpoisoned where it is allowed. Note that we model
126-
// __VA_OPT__ as a builtin macro to allow #ifdef and friends to detect it.
127-
(Ident__VA_ARGS__ = getIdentifierInfo("__VA_ARGS__"))->setIsPoisoned();
128-
SetPoisonReason(Ident__VA_ARGS__, diag::ext_pp_bad_vaargs_use);
129-
Ident__VA_OPT__->setIsPoisoned();
130-
SetPoisonReason(Ident__VA_OPT__, diag::ext_pp_bad_vaopt_use);
131-
132131
if(LangOpts.Borland) {
133132
Ident__exception_info = getIdentifierInfo("_exception_info");
134133
Ident___exception_info = getIdentifierInfo("__exception_info");

clang/test/Preprocessor/macro_vaopt_check.cpp

Lines changed: 3 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -2,20 +2,6 @@
22
// RUN: %clang_cc1 %s -Eonly -verify -Wno-all -pedantic -std=c++11
33
// RUN: %clang_cc1 -x c %s -Eonly -verify -Wno-all -pedantic -std=c99
44

5-
// Check that support for __VA_OPT__ can be detected by #ifdef.
6-
#ifndef __VA_OPT__
7-
#error should be defined
8-
#endif
9-
10-
#ifdef __VA_OPT__
11-
#else
12-
#error should be defined
13-
#endif
14-
15-
#if !defined(__VA_OPT__)
16-
#error should be defined
17-
#endif
18-
195
//expected-error@+1{{missing '('}}
206
#define V1(...) __VA_OPT__
217
#undef V1
@@ -82,12 +68,7 @@
8268
#if __VA_OPT__ // expected-warning {{__VA_OPT__ can only appear in the expansion of a variadic macro}}
8369
#endif
8470

85-
#define BAD __VA_OPT__ // expected-warning {{__VA_OPT__ can only appear in the expansion of a variadic macro}}
86-
87-
// Check defined(__VA_OPT__) doesn't leave __VA_OPT__ poisoned.
88-
#define Z(...) (0 __VA_OPT__(|| 1))
89-
#if defined(__VA_OPT__) && Z(hello)
90-
// OK
91-
#else
92-
#error bad
71+
#ifdef __VA_OPT__ // expected-warning {{__VA_OPT__ can only appear in the expansion of a variadic macro}}
9372
#endif
73+
74+
#define BAD __VA_OPT__ // expected-warning {{__VA_OPT__ can only appear in the expansion of a variadic macro}}

0 commit comments

Comments
 (0)