[llvm] r243996 - Avoid passing nullptr to std::equal.

[Yaron Keren via llvm-commits]

+aaron, reid

Here is a patch disabling the iterator null pointer checking in Visual C++
debug mode. This makes std::equal behave as expected instead of asserting
on std::equal(nullpt,r nullptr, nullptr), saving an extra check just for
Visual C++ debug mode here and there.

It should not cause any practical problem debugging as if a null iterator
is actually dereferenced (as opposed to compared) the debugger will stop on
location.

What do you think?


2015-08-17 23:45 GMT+03:00 Duncan P. N. Exon Smith <dexonsmith at apple.com>:

>
> > On 2015-Aug-17, at 13:43, Yaron Keren <yaron.keren at gmail.com> wrote:
> >
> > I'm trying another approach. The non-null validation goes through a
> macro defined to a function which possibly may be predefined to nothing.
> This will disable all null checking on iterators in debug mode which is
> relevant to additional functions beyond std::equal. In practice it should
> not be a problem since if a null iterator is actually dereferenced (not
> only compared) then the debugger stops with invalid access anyhow.
>
> Well, this would be ideal :).
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150818/5ae4e293/attachment.html>
-------------- next part --------------
Index: cmake/modules/HandleLLVMOptions.cmake
===================================================================
--- cmake/modules/HandleLLVMOptions.cmake	(revision 245244)
+++ cmake/modules/HandleLLVMOptions.cmake	(working copy)
@@ -247,6 +247,10 @@
 endif()
 
 if( MSVC )
+  # Disable iterator null pointer checking in debug mode, especially
+  # so std::equal(nullptr, nullptr, nullptr) will not assert.
+  add_llvm_definitions("-D_DEBUG_POINTER_IMPL=")
+  
   include(ChooseMSVCCRT)
 
   if( NOT (${CMAKE_VERSION} VERSION_LESS 2.8.11) )
Index: include/llvm/ADT/ArrayRef.h
===================================================================
--- include/llvm/ADT/ArrayRef.h	(revision 245244)
+++ include/llvm/ADT/ArrayRef.h	(working copy)
@@ -156,8 +156,6 @@
     bool equals(ArrayRef RHS) const {
       if (Length != RHS.Length)
         return false;
-      if (Length == 0)
-        return true;
       return std::equal(begin(), end(), RHS.begin());
     }
 
Index: lib/CodeGen/SelectionDAG/SelectionDAG.cpp
===================================================================
--- lib/CodeGen/SelectionDAG/SelectionDAG.cpp	(revision 245244)
+++ lib/CodeGen/SelectionDAG/SelectionDAG.cpp	(working copy)
@@ -5668,7 +5668,7 @@
          "Update with wrong number of operands");
 
   // If no operands changed just return the input node.
-  if (Ops.empty() || std::equal(Ops.begin(), Ops.end(), N->op_begin()))
+  if (std::equal(Ops.begin(), Ops.end(), N->op_begin()))
     return N;
 
   // See if the modified node already exists.
Index: unittests/IR/TypesTest.cpp
===================================================================
--- unittests/IR/TypesTest.cpp	(revision 245244)
+++ unittests/IR/TypesTest.cpp	(working copy)
@@ -27,4 +27,12 @@
   EXPECT_FALSE(Struct->hasName());
 }
 
+TEST(TypesTest, LayoutIdenticalEmptyStructs) {
+  LLVMContext C;
+
+  StructType *Foo = StructType::create(C, "Foo");
+  StructType *Bar = StructType::create(C, "Bar");
+  EXPECT_TRUE(Foo->isLayoutIdentical(Bar));
+}
+
 }  // end anonymous namespace