[PATCH] D38060: Remove offset size check in nullptr arithmetic handling

[Andy Kaylor via Phabricator via cfe-commits]

andrew.w.kaylor created this revision.

This patch amends the changes that were committed from https://reviews.llvm.org/D37042 to remove the offset size check in the idiom identification routine.

That check was behaving inconsistently (from target to target) because the offset was implicitly promoted to an integer before the idiom identification routine was called.  The result was that when a smaller-than-int offset was added to a nullptr, the idiom was reported as having been matched on platforms where the int size is the same as the pointer size but the idiom was reported as not having been matched on platforms where the int size and the pointer size were different.

Because this check adds little value, and because the behavior of nullptr arithmetic is undefined in all cases (and therefore at our discretion to implement as we choose), it seems best to remove this check in order to guarantee consistent behavior across all platforms.


https://reviews.llvm.org/D38060

Files:
  lib/AST/Expr.cpp
  test/CodeGen/nullptr-arithmetic.c
  test/Sema/pointer-addition.c
  test/SemaCXX/nullptr-arithmetic.cpp


Index: lib/AST/Expr.cpp
===================================================================
--- lib/AST/Expr.cpp
+++ lib/AST/Expr.cpp
@@ -1862,10 +1862,6 @@
   if (!PTy || !PTy->getPointeeType()->isCharType())
     return false;
 
-  // Check that the integer type is pointer-sized.
-  if (Ctx.getTypeSize(IExp->getType()) != Ctx.getTypeSize(PExp->getType()))
-    return false;
-
   return true;
 }
 InitListExpr::InitListExpr(const ASTContext &C, SourceLocation lbraceloc,
Index: test/SemaCXX/nullptr-arithmetic.cpp
===================================================================
--- test/SemaCXX/nullptr-arithmetic.cpp
+++ test/SemaCXX/nullptr-arithmetic.cpp
@@ -1,4 +1,6 @@
 // RUN: %clang_cc1 %s -fsyntax-only -verify -pedantic -Wextra -std=c++11
+// RUN: %clang_cc1 %s -fsyntax-only -triple i686-unknown-unknown -verify -pedantic -Wextra -std=c++11
+// RUN: %clang_cc1 %s -fsyntax-only -triple x86_64-unknown-unknown -verify -pedantic -Wextra -std=c++11
 
 #include <stdint.h>
 
Index: test/CodeGen/nullptr-arithmetic.c
===================================================================
--- test/CodeGen/nullptr-arithmetic.c
+++ test/CodeGen/nullptr-arithmetic.c
@@ -1,4 +1,6 @@
 // RUN: %clang_cc1 -S %s -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -S %s -emit-llvm -triple i686-unknown-unknown -o - | FileCheck %s
+// RUN: %clang_cc1 -S %s -emit-llvm -triple x86_64-unknown-unknown -o - | FileCheck %s
 
 #include <stdint.h>
 
@@ -32,3 +34,14 @@
 // CHECK-LABEL: test3
 // CHECK: getelementptr
 // CHECK-NOT: inttoptr
+
+// This checks the case where the offset isn't pointer-sized.
+// The front end will implicitly cast the offset to an integer, so we need to
+// make sure that doesn't cause problems on targets where integers and pointers
+// are not the same size.
+int8_t *test4(int8_t b) {
+  return NULLPTRI8 + b;
+}
+// CHECK-LABEL: test4
+// CHECK: inttoptr
+// CHECK-NOT: getelementptr
Index: test/Sema/pointer-addition.c
===================================================================
--- test/Sema/pointer-addition.c
+++ test/Sema/pointer-addition.c
@@ -1,4 +1,6 @@
 // RUN: %clang_cc1 %s -fsyntax-only -verify -pedantic -Wextra -std=c11
+// RUN: %clang_cc1 %s -fsyntax-only -triple i686-unknown-unknown -verify -pedantic -Wextra -std=c11
+// RUN: %clang_cc1 %s -fsyntax-only -triple x86_64-unknown-unknown -verify -pedantic -Wextra -std=c11
 
 #include <stdint.h>
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D38060.115921.patch
Type: text/x-patch
Size: 2403 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170919/677e8861/attachment.bin>