[cfe-dev] Warnings not raised from a macro: -Wundefined-bool-conversion/-Wtautological-undefined-compare

[Nick Lewycky]

On 4 November 2014 02:15, Richtarsky, Martin <martin.richtarsky at sap.com>
wrote:

> Hi,
>
> while cleaning some code of
> "-Wundefined-bool-conversion"/"-Wtautological-undefined-compare" warnings I
> found that these are never raised when coming from a macro. I know this is
> intentional in clang, but I wonder whether the benefits of this really
> outweigh the risks.
>
> clang 3.5 optimizes these checks away, so it is crucial to find all places
> that rely on them in the codebase. But when the code comes from a macro
> there is suddenly no way to do so at compile time. See functions f2() below.
>

I think this is correct behaviour. How is the macro supposed to know
whether the pointer you gave it is the magically non-nullable kind or the
regular flavour? Are you going to ask people to use two different macros
depending on whether the argument is nullable or not?

How are others dealing with this problem e.g. at Google and Apple with
> large codebases likely affected by this?
>

With a similar macro, CHECK, I've been seeing callers doing "CHECK(this !=
nullptr);" and that gets flagged by the warning. I haven't seen any
failures due to a pattern like you describe, and only something like a
dozen or so code changes at Google.

One alternative is to build with
-fsanitize=null,nonnull-attribute,returns-nonnull-attribute and run their
tests.

But maybe you don't have tests. You could gin up a patched clang that
comments out the "Don't warn inside macros" logic in
Sema::DiagnoseAlwaysNonNullPointer. But maybe you can't easily vend a
deliberately-non-production compiler. The last trick up my sleeve is
clang-query.

clang-query> match
implicitCastExpr(hasImplicitDestinationType(asString("_Bool")),
hasSourceExpression(thisExpr()))

Match #1:

/usr/local/google/home/nlewycky/sap.cc:3:12: note: "root" binds here
    return this;
           ^~~~
1 match.


Hrm. I tried writing the obvious thing for address of reference, but this
doesn't work:

clang-query> match
implicitCastExpr(hasImplicitDestinationType(asString("_Bool")),
hasSourceExpression(unaryOperator(hasOperatorName("&"),
hasUnaryOperand(hasType(referenceType())))))
0 matches.

because apparently nothing has referenceType():

clang-query> match expr(hasType(referenceType()))
0 matches.
clang-query> match expr(hasType(qualType(referenceType())))
0 matches.

The testcase is:

bool test2(int &i) {
  return &i;
}

Not sure what I'm doing wrong with clang-query.

Nick

$ cat undefined+macro.cpp
> ----
> #define RET(expr) \
>   return (!(expr))
>
> class A
> {
>
> int f1()
> {
>   return (!this);  // warns
> }
>
> int f2()
> {
>   RET(this);  // does NOT warn
> }
>
> };
>
> int f1()
> {
>   int a = 1;
>   int &r = a;
>   return (&r != 0);  // warns
> }
>
> int f2()
> {
>   int a = 1;
>   int &r = a;
>   RET(&r != 0);  // does NOT warn
> }
> ----
>
> $ clang++ -c  undefined+macro.cpp
>
>
> undefined+macro.cpp:9:12: warning: 'this' pointer cannot be null in
> well-defined C++ code; pointer may be assumed to always convert to true
> [-Wundefined-bool-conversion]
>   return (!this);  // warns
>           ~^~~~
> undefined+macro.cpp:23:12: warning: reference cannot be bound to
> dereferenced null pointer in well-defined C++ code; comparison may be
> assumed to always evaluate to true [-Wtautological-undefined-compare]
>   return (&r != 0);  // warns
>            ^    ~
> 2 warnings generated.
>
>
> Best regards,
> Martin
>
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20141104/9a291134/attachment.html>