<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Mar 16, 2015 at 10:45 AM, Zachary Turner <span dir="ltr"><<a href="mailto:zturner@google.com" target="_blank">zturner@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">So assuming the correct type is passed today, is that semantically equivalent to passing nullptr today? </div></blockquote><div><br>Right - for now the new parameter is not used at all, except in the assertion. So there's no way to get a change in behavior - either nullptr is passed and nothing happens, or a type is passed and if the type matches, nothing happens - if you get the type wrong, the assertion will fail.<br><br>This is a rather large migration effort, so I'm trying to take as small steps as possible to ensure any issues are flushed out at each step before changing the semantics out from underneath users.<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"> 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.<br></div><div class="HOEnZb"><div class="h5"><br><div class="gmail_quote">On Mon, Mar 16, 2015 at 10:43 AM David Blaikie <<a href="mailto:blaikie@google.com" target="_blank">blaikie@google.com</a>> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Mon, Mar 16, 2015 at 10:39 AM, Zachary Turner <span dir="ltr"><<a href="mailto:zturner@google.com" target="_blank">zturner@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr">Hi Sean,<div><br></div><div>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().</div><div><br></div><div>What are your thoughts here?</div><div><br></div><div>Secondly, the change to nullptr does not seem to have caused any test failures.</div></div></blockquote></div></div></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><br>This is expected - for now, nullptr is an acceptable value. The assertion I have in GetElementPtrInst's ctor is:<br><br>assert(!PointeeType || PointeeType == getSourceElementType());<br><br>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.<br><br>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.<br> </div></div></div></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div> 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.</div></div>
</blockquote></div></div></div></blockquote></div>
</div></div></blockquote></div><br></div></div>