[PATCH] D37042: Teach clang to tolerate the 'p = nullptr + n' idiom used by glibc

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added inline comments.


================
Comment at: lib/AST/Expr.cpp:1857
+  if (!PExp->IgnoreParenCasts()
+          ->isNullPointerConstant(Ctx, Expr::NPC_ValueDependentIsNull))
+    return false;
----------------
andrew.w.kaylor wrote:
> rsmith wrote:
> > If we get here with a value-dependent expression, we should treat it as non-null (do not warn on `(char*)PtrTemplateParameter + N`).
> OK.  I wasn't sure about that.
> 
> So how do I test that?  Is this right?
> ```
> template<typename T, T *P>
> T* f(intptr_t offset) {
>   return P + offset;
> }
> 
> char *test(intptr_t offset) {
>   return f<char, nullptr>(offset);
> }
> ```
You can simplify that slightly by using `template<char *P>`, but yes.


================
Comment at: lib/Sema/SemaExpr.cpp:8807
+    llvm::APSInt KnownVal;
+    if (!getLangOpts().CPlusPlus || !IExp->EvaluateAsInt(KnownVal, Context) ||
+        KnownVal != 0) {
----------------
This talk about value-dependence reminds me: it is an error to call `EvaluateAsInt` on a value-dependent expression (the expression evaluator will probably assert). If `IExp->isValueDependent()`, you should skip the diagnostic, since it might instantiate to zero.


================
Comment at: lib/Sema/SemaExpr.cpp:8877
     if (RHS.get()->getType()->isIntegerType()) {
+      // Subtracting from a null pointer should produce a warning.
+      // The last argument to the diagnose call says this doesn't match the
----------------
andrew.w.kaylor wrote:
> rsmith wrote:
> > Subtracting zero from a null pointer should not warn in C++.
> > 
> > (Conversely, subtracting a non-null pointer from a pointer should warn in C++, and subtracting any pointer from a null pointer should warn in C.)
> Is it OK if I just add a FIXME in the section below that handles pointer-pointer?
Yes, that's fine. (But the `nullptr - 0` case should be handled in this patch.)


https://reviews.llvm.org/D37042