Skip to content

Commit

Permalink
Don't allow __VA_OPT__ to be detected by #ifdef.
Browse files Browse the repository at this point in the history
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.

(cherry picked from commit 5dfa37a)
  • Loading branch information
zygoloid committed Jan 27, 2021
1 parent ea99c88 commit 9ea2a10
Show file tree
Hide file tree
Showing 6 changed files with 11 additions and 64 deletions.
19 changes: 0 additions & 19 deletions clang/include/clang/Lex/Preprocessor.h
Expand Up @@ -447,25 +447,6 @@ class Preprocessor {
ElseLoc(ElseLoc) {}
};

class IfdefMacroNameScopeRAII {
Preprocessor &PP;
bool VAOPTWasPoisoned;

public:
IfdefMacroNameScopeRAII(Preprocessor &PP)
: PP(PP), VAOPTWasPoisoned(PP.Ident__VA_OPT__->isPoisoned()) {
PP.Ident__VA_OPT__->setIsPoisoned(false);
}
IfdefMacroNameScopeRAII(const IfdefMacroNameScopeRAII&) = delete;
IfdefMacroNameScopeRAII &operator=(const IfdefMacroNameScopeRAII&) = delete;
~IfdefMacroNameScopeRAII() { Exit(); }

void Exit() {
if (VAOPTWasPoisoned)
PP.Ident__VA_OPT__->setIsPoisoned(true);
}
};

private:
friend class ASTReader;
friend class MacroArgs;
Expand Down
5 changes: 0 additions & 5 deletions clang/lib/Lex/PPDirectives.cpp
Expand Up @@ -2928,14 +2928,9 @@ void Preprocessor::HandleIfdefDirective(Token &Result,
++NumIf;
Token DirectiveTok = Result;

// __VA_OPT__ is allowed as the operand of #if[n]def.
IfdefMacroNameScopeRAII IfdefMacroNameScope(*this);

Token MacroNameTok;
ReadMacroName(MacroNameTok);

IfdefMacroNameScope.Exit();

// Error reading macro name? If so, diagnostic already issued.
if (MacroNameTok.is(tok::eod)) {
// Skip code until we get to #endif. This helps with recovery by not
Expand Down
5 changes: 0 additions & 5 deletions clang/lib/Lex/PPExpressions.cpp
Expand Up @@ -104,9 +104,6 @@ static bool EvaluateDefined(PPValue &Result, Token &PeekTok, DefinedTracker &DT,
SourceLocation beginLoc(PeekTok.getLocation());
Result.setBegin(beginLoc);

// __VA_OPT__ is allowed as the operand of 'defined'.
Preprocessor::IfdefMacroNameScopeRAII IfdefMacroNameScope(PP);

// Get the next token, don't expand it.
PP.LexUnexpandedNonComment(PeekTok);

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

IfdefMacroNameScope.Exit();

// If we don't have a pp-identifier now, this is an error.
if (PP.CheckMacroName(PeekTok, MU_Other))
return true;
Expand Down
6 changes: 1 addition & 5 deletions clang/lib/Lex/PPMacroExpansion.cpp
Expand Up @@ -323,16 +323,13 @@ void Preprocessor::dumpMacroInfo(const IdentifierInfo *II) {

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

// Mark it as being a macro that is builtin.
MacroInfo *MI = PP.AllocateMacroInfo(SourceLocation());
MI->setIsBuiltinMacro();
if (Disabled)
MI->DisableMacro();
PP.appendDefMacroDirective(Id, MI);
return Id;
}
Expand All @@ -346,7 +343,6 @@ void Preprocessor::RegisterBuiltinMacros() {
Ident__TIME__ = RegisterBuiltinMacro(*this, "__TIME__");
Ident__COUNTER__ = RegisterBuiltinMacro(*this, "__COUNTER__");
Ident_Pragma = RegisterBuiltinMacro(*this, "_Pragma");
Ident__VA_OPT__ = RegisterBuiltinMacro(*this, "__VA_OPT__", true);

// C++ Standing Document Extensions.
if (getLangOpts().CPlusPlus)
Expand Down
15 changes: 7 additions & 8 deletions clang/lib/Lex/Preprocessor.cpp
Expand Up @@ -115,20 +115,19 @@ Preprocessor::Preprocessor(std::shared_ptr<PreprocessorOptions> PPOpts,

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

// "Poison" __VA_ARGS__, __VA_OPT__ which can only appear in the expansion of
// a macro. They get unpoisoned where it is allowed.
(Ident__VA_ARGS__ = getIdentifierInfo("__VA_ARGS__"))->setIsPoisoned();
SetPoisonReason(Ident__VA_ARGS__,diag::ext_pp_bad_vaargs_use);
(Ident__VA_OPT__ = getIdentifierInfo("__VA_OPT__"))->setIsPoisoned();
SetPoisonReason(Ident__VA_OPT__,diag::ext_pp_bad_vaopt_use);

// Initialize the pragma handlers.
RegisterBuiltinPragmas();

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

// "Poison" __VA_ARGS__, __VA_OPT__ which can only appear in the expansion of
// a macro. They get unpoisoned where it is allowed. Note that we model
// __VA_OPT__ as a builtin macro to allow #ifdef and friends to detect it.
(Ident__VA_ARGS__ = getIdentifierInfo("__VA_ARGS__"))->setIsPoisoned();
SetPoisonReason(Ident__VA_ARGS__, diag::ext_pp_bad_vaargs_use);
Ident__VA_OPT__->setIsPoisoned();
SetPoisonReason(Ident__VA_OPT__, diag::ext_pp_bad_vaopt_use);

if(LangOpts.Borland) {
Ident__exception_info = getIdentifierInfo("_exception_info");
Ident___exception_info = getIdentifierInfo("__exception_info");
Expand Down
25 changes: 3 additions & 22 deletions clang/test/Preprocessor/macro_vaopt_check.cpp
Expand Up @@ -2,20 +2,6 @@
// RUN: %clang_cc1 %s -Eonly -verify -Wno-all -pedantic -std=c++11
// RUN: %clang_cc1 -x c %s -Eonly -verify -Wno-all -pedantic -std=c99

// Check that support for __VA_OPT__ can be detected by #ifdef.
#ifndef __VA_OPT__
#error should be defined
#endif

#ifdef __VA_OPT__
#else
#error should be defined
#endif

#if !defined(__VA_OPT__)
#error should be defined
#endif

//expected-error@+1{{missing '('}}
#define V1(...) __VA_OPT__
#undef V1
Expand Down Expand Up @@ -82,12 +68,7 @@
#if __VA_OPT__ // expected-warning {{__VA_OPT__ can only appear in the expansion of a variadic macro}}
#endif

#define BAD __VA_OPT__ // expected-warning {{__VA_OPT__ can only appear in the expansion of a variadic macro}}

// Check defined(__VA_OPT__) doesn't leave __VA_OPT__ poisoned.
#define Z(...) (0 __VA_OPT__(|| 1))
#if defined(__VA_OPT__) && Z(hello)
// OK
#else
#error bad
#ifdef __VA_OPT__ // expected-warning {{__VA_OPT__ can only appear in the expansion of a variadic macro}}
#endif

#define BAD __VA_OPT__ // expected-warning {{__VA_OPT__ can only appear in the expansion of a variadic macro}}

0 comments on commit 9ea2a10

Please sign in to comment.