r258128 - Add -Wexpansion-to-undefined: warn when using `defined` in a macro definition.
Diego Novillo via cfe-commits
cfe-commits at lists.llvm.org
Tue Jan 19 11:29:00 PST 2016
Nico, this is producing tons of warnings on an LLVM build and is actually
breaking our internal builds (we build with -Werror).
I fixed one file that was producing this, but there are several that have
the same problem (e.g., gtest-port.h). Could you fix them or rollback?
Thanks. Diego.
On Tue, Jan 19, 2016 at 10:15 AM, Nico Weber via cfe-commits <
cfe-commits at lists.llvm.org> wrote:
> Author: nico
> Date: Tue Jan 19 09:15:31 2016
> New Revision: 258128
>
> URL: http://llvm.org/viewvc/llvm-project?rev=258128&view=rev
> Log:
> Add -Wexpansion-to-undefined: warn when using `defined` in a macro
> definition.
>
> [cpp.cond]p4:
> Prior to evaluation, macro invocations in the list of preprocessing
> tokens that will become the controlling constant expression are replaced
> (except for those macro names modified by the 'defined' unary operator),
> just as in normal text. If the token 'defined' is generated as a result
> of this replacement process or use of the 'defined' unary operator does
> not match one of the two specified forms prior to macro replacement, the
> behavior is undefined.
>
> This isn't an idle threat, consider this program:
> #define FOO
> #define BAR defined(FOO)
> #if BAR
> ...
> #else
> ...
> #endif
> clang and gcc will pick the #if branch while Visual Studio will take the
> #else branch. Emit a warning about this undefined behavior.
>
> One problem is that this also applies to function-like macros. While the
> example above can be written like
>
> #if defined(FOO) && defined(BAR)
> #defined HAVE_FOO 1
> #else
> #define HAVE_FOO 0
> #endif
>
> there is no easy way to rewrite a function-like macro like `#define FOO(x)
> (defined __foo_##x && __foo_##x)`. Function-like macros like this are
> used in
> practice, and compilers seem to not have differing behavior in that case.
> So
> this a default-on warning only for object-like macros. For function-like
> macros, it is an extension warning that only shows up with `-pedantic`.
> (But it's undefined behavior in both cases.)
>
> Modified:
> cfe/trunk/include/clang/Basic/DiagnosticGroups.td
> cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td
> cfe/trunk/lib/Lex/PPExpressions.cpp
> cfe/trunk/test/Lexer/cxx-features.cpp
> cfe/trunk/test/Preprocessor/expr_define_expansion.c
>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=258128&r1=258127&r2=258128&view=diff
>
> ==============================================================================
> --- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Tue Jan 19 09:15:31
> 2016
> @@ -204,6 +204,7 @@ def OverloadedShiftOpParentheses: DiagGr
> def DanglingElse: DiagGroup<"dangling-else">;
> def DanglingField : DiagGroup<"dangling-field">;
> def DistributedObjectModifiers :
> DiagGroup<"distributed-object-modifiers">;
> +def ExpansionToUndefined : DiagGroup<"expansion-to-undefined">;
> def FlagEnum : DiagGroup<"flag-enum">;
> def IncrementBool : DiagGroup<"increment-bool",
> [DeprecatedIncrementBool]>;
> def InfiniteRecursion : DiagGroup<"infinite-recursion">;
>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td?rev=258128&r1=258127&r2=258128&view=diff
>
> ==============================================================================
> --- cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td Tue Jan 19
> 09:15:31 2016
> @@ -658,6 +658,13 @@ def warn_header_guard : Warning<
> def note_header_guard : Note<
> "%0 is defined here; did you mean %1?">;
>
> +def warn_defined_in_object_type_macro : Warning<
> + "macro expansion producing 'defined' has undefined behavior">,
> + InGroup<ExpansionToUndefined>;
> +def warn_defined_in_function_type_macro : Extension<
> + "macro expansion producing 'defined' has undefined behavior">,
> + InGroup<ExpansionToUndefined>;
> +
> let CategoryName = "Nullability Issue" in {
>
> def err_pp_assume_nonnull_syntax : Error<"expected 'begin' or 'end'">;
>
> Modified: cfe/trunk/lib/Lex/PPExpressions.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/PPExpressions.cpp?rev=258128&r1=258127&r2=258128&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Lex/PPExpressions.cpp (original)
> +++ cfe/trunk/lib/Lex/PPExpressions.cpp Tue Jan 19 09:15:31 2016
> @@ -140,6 +140,51 @@ static bool EvaluateDefined(PPValue &Res
> PP.LexNonComment(PeekTok);
> }
>
> + // [cpp.cond]p4:
> + // Prior to evaluation, macro invocations in the list of preprocessing
> + // tokens that will become the controlling constant expression are
> replaced
> + // (except for those macro names modified by the 'defined' unary
> operator),
> + // just as in normal text. If the token 'defined' is generated as a
> result
> + // of this replacement process or use of the 'defined' unary operator
> does
> + // not match one of the two specified forms prior to macro
> replacement, the
> + // behavior is undefined.
> + // This isn't an idle threat, consider this program:
> + // #define FOO
> + // #define BAR defined(FOO)
> + // #if BAR
> + // ...
> + // #else
> + // ...
> + // #endif
> + // clang and gcc will pick the #if branch while Visual Studio will take
> the
> + // #else branch. Emit a warning about this undefined behavior.
> + if (beginLoc.isMacroID()) {
> + bool IsFunctionTypeMacro =
> + PP.getSourceManager()
> + .getSLocEntry(PP.getSourceManager().getFileID(beginLoc))
> + .getExpansion()
> + .isFunctionMacroExpansion();
> + // For object-type macros, it's easy to replace
> + // #define FOO defined(BAR)
> + // with
> + // #if defined(BAR)
> + // #define FOO 1
> + // #else
> + // #define FOO 0
> + // #endif
> + // and doing so makes sense since compilers handle this differently in
> + // practice (see example further up). But for function-type macros,
> + // there is no good way to write
> + // # define FOO(x) (defined(M_ ## x) && M_ ## x)
> + // in a different way, and compilers seem to agree on how to behave
> here.
> + // So warn by default on object-type macros, but only warn in
> -pedantic
> + // mode on function-type macros.
> + if (IsFunctionTypeMacro)
> + PP.Diag(beginLoc, diag::warn_defined_in_function_type_macro);
> + else
> + PP.Diag(beginLoc, diag::warn_defined_in_object_type_macro);
> + }
> +
> // Invoke the 'defined' callback.
> if (PPCallbacks *Callbacks = PP.getPPCallbacks()) {
> Callbacks->Defined(macroToken, Macro,
> @@ -177,8 +222,8 @@ static bool EvaluateValue(PPValue &Resul
> if (IdentifierInfo *II = PeekTok.getIdentifierInfo()) {
> // Handle "defined X" and "defined(X)".
> if (II->isStr("defined"))
> - return(EvaluateDefined(Result, PeekTok, DT, ValueLive, PP));
> -
> + return EvaluateDefined(Result, PeekTok, DT, ValueLive, PP);
> +
> // If this identifier isn't 'defined' or one of the special
> // preprocessor keywords and it wasn't macro expanded, it turns
> // into a simple 0, unless it is the C++ keyword "true", in which
> case it
>
> Modified: cfe/trunk/test/Lexer/cxx-features.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Lexer/cxx-features.cpp?rev=258128&r1=258127&r2=258128&view=diff
>
> ==============================================================================
> --- cfe/trunk/test/Lexer/cxx-features.cpp (original)
> +++ cfe/trunk/test/Lexer/cxx-features.cpp Tue Jan 19 09:15:31 2016
> @@ -6,6 +6,7 @@
>
> // expected-no-diagnostics
>
> +// FIXME using `defined` in a macro has undefined behavior.
> #if __cplusplus < 201103L
> #define check(macro, cxx98, cxx11, cxx1y) cxx98 == 0 ?
> defined(__cpp_##macro) : __cpp_##macro != cxx98
> #elif __cplusplus < 201304L
>
> Modified: cfe/trunk/test/Preprocessor/expr_define_expansion.c
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Preprocessor/expr_define_expansion.c?rev=258128&r1=258127&r2=258128&view=diff
>
> ==============================================================================
> --- cfe/trunk/test/Preprocessor/expr_define_expansion.c (original)
> +++ cfe/trunk/test/Preprocessor/expr_define_expansion.c Tue Jan 19
> 09:15:31 2016
> @@ -1,6 +1,28 @@
> -// RUN: %clang_cc1 %s -E -CC -pedantic -verify
> -// expected-no-diagnostics
> +// RUN: %clang_cc1 %s -E -CC -verify
> +// RUN: %clang_cc1 %s -E -CC -DPEDANTIC -pedantic -verify
>
> #define FOO && 1
> #if defined FOO FOO
> #endif
> +
> +#define A
> +#define B defined(A)
> +#if B // expected-warning{{macro expansion producing 'defined' has
> undefined behavior}}
> +#endif
> +
> +#define m_foo
> +#define TEST(a) (defined(m_##a) && a)
> +
> +#if defined(PEDANTIC)
> +// expected-warning at +4{{macro expansion producing 'defined' has
> undefined behavior}}
> +#endif
> +
> +// This shouldn't warn by default, only with pedantic:
> +#if TEST(foo)
> +#endif
> +
> +
> +// Only one diagnostic for this case:
> +#define INVALID defined(
> +#if INVALID // expected-error{{macro name missing}}
> +#endif
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160119/c9a1ff39/attachment-0001.html>
More information about the cfe-commits
mailing list