<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Mon, Sep 12, 2016 at 5:20 PM, David Blaikie <span dir="ltr"><<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.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"><div class="gmail_quote"><span class=""><div dir="ltr">On Mon, Sep 12, 2016 at 5:09 PM Reid Kleckner via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: rnk<br class="m_-2460366013717952950gmail_msg">
Date: Mon Sep 12 19:01:23 2016<br class="m_-2460366013717952950gmail_msg">
New Revision: 281278<br class="m_-2460366013717952950gmail_msg">
<br class="m_-2460366013717952950gmail_msg">
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=281278&view=rev" rel="noreferrer" class="m_-2460366013717952950gmail_msg" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project?rev=281278&view=rev</a><br class="m_-2460366013717952950gmail_msg">
Log:<br class="m_-2460366013717952950gmail_msg">
[DebugInfo] Deduplicate debug info limiting logic<br class="m_-2460366013717952950gmail_msg">
<br class="m_-2460366013717952950gmail_msg">
We should be doing the same checks when a type is completed as we do<br class="m_-2460366013717952950gmail_msg">
when a complete type is used during emission. Previously, we duplicated<br class="m_-2460366013717952950gmail_msg">
the logic, and it got out of sync. This could be observed with<br class="m_-2460366013717952950gmail_msg">
dllimported classes.<br class="m_-2460366013717952950gmail_msg"></blockquote></span><div><br>Thanks for having a go at this!<br><br>The logic in completeType (& maybe also completeClassData) still seems to duplicate some of the logic in shouldOmitDefinition - I think the original idea I had was a series of cascading entry points (completeType checking the "oh, the type now has a definition", then falling into completeRequiredType, etc)<br><br>In theory the "shouldOmitDefinition" could be split into the extra conditions for each of the parts of that cascade, so the checks wouldn't be redundantly executed when we were already being called about the completion or required completion of a type, etc. But I'm not sure the performance gain would be measurable, etc.<br><br>If not, we might as well just have one entry point that's something like "revisit type because something interesting happened to it" (it became complete, it became required to be complete, etc)... but that seems a bit weird too.<br></div></div></div></blockquote><div><br></div><div>Seems reasonable, but I didn't see a nice, clear, obvious refactoring to do, so I left it here. Seems better than it was before.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div></div><span class=""><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br class="m_-2460366013717952950gmail_msg">
Also reduce a test case for this slightly.<br class="m_-2460366013717952950gmail_msg"></blockquote></span><div><br>The UseCompleteType is still there/unnecessary (as is the 'private' in UnicodeString) - and I'd probably rename it to foo/bar and throw it in a namespace like the rest for consistency - having semantic names in a test when they're just remnants from whatever code it was reduced from seems confusing to me. But I realize I'm a bit more pedantic about that than most.<br></div></div></div></blockquote><div><br></div><div>The private was necessary at some point during the reduction process for some reason.</div></div></div></div>