rsmith added a comment.

In https://reviews.llvm.org/D37042#855713, @efriedma wrote:

> I didn't think of this earlier, but strictly speaking, I think 
> "(char*)nullptr+0" isn't undefined in C++?


Yes, that's correct. (C++'s model is basically equivalent to having an object 
of size zero at the null address, so things like `(char*)nullptr - 
(char*)nullptr` and `(char*)nullptr + 0` are valid.)

> But probably worth emitting the warning anyway; anyone writing out arithmetic 
> on null is probably doing something suspicious, even if it isn't technically 
> undefined.

I agree. We should probably suppress the warning if the non-pointer operand is 
known to be zero in C++, and weaken the wording slightly "[..] has undefined 
behavior if offset is nonzero" otherwise, though.



================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:6032
+def ext_gnu_null_ptr_arith : Extension<
+  "inttoptr casting using arithmetic on a null pointer is a GNU extension">,
+  InGroup<PointerArith>;
----------------
efriedma wrote:
> The keyword "inttoptr" is part of LLVM, not something we expect users to 
> understand.
How about something like "arithmetic on null pointer treated as cast from 
integer to pointer as a GNU extension"?


================
Comment at: lib/Sema/SemaExpr.cpp:8799
 
+  // Adding to a null pointer is never an error, but should warn.
+  if (PExp->IgnoreParenCasts()->isNullPointerConstant(Context, 
----------------
Maybe `// Adding to a null pointer results in undefined behavior.` to explain 
why we warn (a reader of the code can already see that we do warn in this case).


================
Comment at: lib/Sema/SemaExpr.cpp:8805
+    const PointerType *pointerType = PExp->getType()->getAs<PointerType>();
+    if (!IExp->isIntegerConstantExpr(Context) &&
+        pointerType->getPointeeType()->isCharType() &&
----------------
It seems strange to me to disable this when the RHS is an ICE. If we're going 
to support this as an extension, we should make the rules for it as simple as 
we reasonably can; this ICE check seems like an unnecessary complication.


https://reviews.llvm.org/D37042



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to