efriedma added inline comments.
================
Comment at: clang/lib/Sema/SemaExpr.cpp:10723-10733
+ bool LHSIsNullPtr = LHS.get()->IgnoreParenCasts()->isNullPointerConstant(
+ Context, Expr::NPC_ValueDependentIsNotNull);
+ bool RHSIsNullPtr = RHS.get()->IgnoreParenCasts()->isNullPointerConstant(
+ Context, Expr::NPC_ValueDependentIsNotNull);
+
+ // Subtracting nullptr or from nullptr should produce
+ // a warning expect nullptr - nullptr is valid in C++ [expr.add]p7
----------------
nickdesaulniers wrote:
> Might it be better to move the C++ check to the top; have all of this within
> a `if (!getLangOpts().CPlusPlus) {` block? Perhaps I'm misunderstanding what
> the `||` is doing?
The getLangOpts().CPlusPlus check is specifically so we don't warn on
null-null, which the C++ standard allows.
================
Comment at: clang/test/Sema/pointer-addition.c:34
+ f = (char*)(f - (char*)0); // expected-warning {{performing pointer
arithmetic on a null pointer has undefined behavior}}
+ f = (char*)((char*)0 - (char*)0); // expected-warning {{performing pointer
arithmetic on a null pointer has undefined behavior}} expected-warning
{{performing pointer arithmetic on a null pointer has undefined behavior}}
}
----------------
jamieschmeiser wrote:
> jamieschmeiser wrote:
> > efriedma wrote:
> > > This is what I was afraid would happen.
> > >
> > > C++ has a specific carveout in the standard that "null-null" isn't
> > > undefined. C doesn't, but I'm not sure warning is practically useful
> > > there.
> > >
> > > In any case, printing the same warning twice isn't useful.
> > Yes, I see that [expr.add] p 7 says nullptr-nullptr is defined to be 0. I
> > will suppress the warning for this.
> I disagree with the comment that the two warnings are not useful. While it
> is the same warning, the section of code indicated by each warning is
> different and both need to be corrected to clear the code of warnings. A
> single warning would likely be more confusing in that a user would see the
> same warning and not notice that a different section of code is indicated
> after fixing the indicated problem. I would expect the typical response to
> be the user assuming that they had made a mistake and removing the initial
> fix and fixing the second, only to again receive a same warning again with a
> different section of code. The different code sections indicated would
> likely be overlooked, leading to frustration whereas two warnings clearly
> indicates there are 2 problems.
I see what you mean about the two warnings; printing twice is probably okay.
In terms of whether we actually want the null-null warning in C, I could go
either way. I usually like to avoid differences between C and C++ where
possible. But the standards are actually written differently here. And it's
unlikely that explicitly written null-null shows up in practice, so probably
nobody will notice either way.
Needs a testcase for the C++ behavior.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D98798/new/
https://reviews.llvm.org/D98798
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
