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

Wed Nov 5 02:58:55 PST 2014

Hi,

I think Martin is right. Preprocessor is only textual replacement, compiler should not know about this step and compiles code resulting from preprocess step.

I used Martin’s source code I have got different number of warnings if preprocess step was made as extra step before compilation, in my opinion it is a bug.

Common sense tells me that this “equation” should be true every time: clang++ -c = clang++ -E + clang++ -c.

Mira

d:\Tmp>clang++ -v

clang version 3.6.0 (220682)

Target: i686-pc-windows-gnu

Thread model: posix

d:\Tmp>clang++ -c foo.cpp

foo.cpp:9:13: 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

          ~ ^~~~

foo.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

           ^    ~

foo.cpp:30:8: 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]

  RET(&r != 0);  // does NOT warn

       ^    ~

foo.cpp:2:13: note: expanded from macro 'RET'

  return (!(expr))

            ^

3 warnings generated.

d:\Tmp>clang++ -E foo.cpp > foo2.cpp

d:\Tmp>clang++ -c foo2.cpp

foo.cpp:9:13: 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));

          ~ ^~~~

foo.cpp:14:13: 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));

          ~ ^~~~

foo.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);

           ^    ~

foo.cpp:30:14: 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));

             ^    ~

4 warnings generated.

From: cfe-dev-bounces at cs.uiuc.edu [mailto:cfe-dev-bounces at cs.uiuc.edu] On Behalf Of Nick Lewycky
Sent: Wednesday, November 05, 2014 8:05 AM
To: Richtarsky, Martin
Cc: cfe-dev at cs.uiuc.edu
Subject: Re: [cfe-dev] Warnings not raised from a macro: -Wundefined-bool-conversion/-Wtautological-undefined-compare

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/20141105/d6ca6b1e/attachment.html>