On Thu, Oct 4, 2012 at 4:48 PM, Villmow, Micah <span dir="ltr"><<a href="mailto:Micah.Villmow@amd.com" target="_blank" class="cremed">Micah.Villmow@amd.com</a>></span> wrote:<br><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 lang="EN-US" link="blue" vlink="purple">
<div>
<p class=""><span style="font-size:11pt;font-family:Calibri,sans-serif">Here is my first run at implementing TargetData with DataLayout.<u></u><u></u></span></p>
<p class=""><span style="font-size:11pt;font-family:Calibri,sans-serif"><u></u> <u></u></span></p>
<p class=""><span style="font-size:11pt;font-family:Calibri,sans-serif">This should allow all the clients to start switching over to datalayout without any functionality breaking.</span></p></div></div></blockquote><div>
There are comments in TargetData that may need updating.</div><div><br></div><div>You've made the destructor virtual. I don't think you should need this. The code responsible for creating and destroying these will need to be the last migrated (which is fine) and that code should use TargetData consistently until every client is updated to use DataLayout. Then it can switch, and the virtual destructor won't matter.</div>
<div><br></div><div>Some comments:</div><div><br></div><div><span style="color:rgb(0,0,0);white-space:pre-wrap">+    DataLayout(*reinterpret_cast<const DataLayout*>(&TD))</span></div><div><span style="color:rgb(0,0,0);white-space:pre-wrap"><br>
</span></div><div><span style="color:rgb(0,0,0);white-space:pre-wrap">No, you should just call DataLayout(TD). Derived-to-base conversion will handle the rest.</span></div><div><font color="#000000"><span style="white-space:pre-wrap"><br>
</span></font>With those changes, if everything builds and tests pass, LGTM.</div></div></div>