So assuming the correct type is passed today, is that semantically equivalent to passing nullptr today? Because if not, then I would have expected a test to fail somewhere. Not because an assertion fired, but because something somewhere had the wrong idea about a type.
On Mon, Mar 16, 2015 at 10:43 AM David Blaikie <blai...@google.com> wrote: > On Mon, Mar 16, 2015 at 10:39 AM, Zachary Turner <ztur...@google.com> > wrote: > >> Hi Sean, >> >> A change went in over the weekend to change the >> way GetElementPtrInst::Create works. Now, for pointer types, you need to >> manually specify the type of the pointee, as pointerType->getType() won't >> work anymore. This broke the build, so a workaround was employed to simply >> pass nullptr for this argument. But this is likely to be incorrect, and >> instead we need the actual llvm::Type for the pointee. I've looked over >> the code, and I *think* that on line 2209 we can replace the nullptr with >> old_constant->getType(), and on line 2396 we can place the nullptr with >> value->getType(). >> >> What are your thoughts here? >> >> Secondly, the change to nullptr does not seem to have caused any test >> failures. >> > > This is expected - for now, nullptr is an acceptable value. The assertion > I have in GetElementPtrInst's ctor is: > > assert(!PointeeType || PointeeType == getSourceElementType()); > > Eventually I'll need to remove the nullptr case once I believe I've > flushed out all the callers - this will verify that the callers are passing > the type that matches the pointee type of the pointer operand. (at this > point the current lldb code will assert-fail, if it is tested) You can try > removing the first part of that assert locally to see if the LLDB code path > is tested currently. > > Once similar changes have been applied to all pointer-element-type-aware > uses of pointers in LLVM IR, I'll start trying to remove the ability to > query a pointer type for its element type. > > >> Is there any way we can add some tests for this? If this is not >> something that is testable through the public interface, I recently added >> gtest unit tests to the build. It's not enabled in the Xcode build (but >> will be soon), but if you tell me how to set up a test, call the right >> function, and what outputs to expect, I can write it. >> >
_______________________________________________ lldb-dev mailing list lldb-dev@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/lldb-dev
