<div dir="ltr"><div>Looks good, I'm fairly confident this will DTRT.</div><div><br></div><div>As a followup someone should remove the hack at the top of GetAddrOfCXXDestructor.</div><div><br></div><div>+  typedef llvm::StringMap<llvm::TrackingVH<llvm::Constant> > ReplacemensType;</div>
<div>+  ReplacemensType Replacements;</div><div><br></div><div>typo</div><div><br></div><div>Do we prefer Type or Ty for typedefs?</div><div><br></div><div><div>+void CodeGenModule::applyReplacements() {</div><div>+  for (ReplacemensType::iterator I = Replacements.begin(),</div>
<div>+                                 E = Replacements.end();</div></div><div><div>+    StringRef MangledName = I->first();</div></div><div><br></div><div>I don't know enough C++ to know why the ()'s are needed here.</div>
<div><br></div></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Tue, Nov 5, 2013 at 1:04 PM, Rafael Espíndola <span dir="ltr"><<a href="mailto:rafael.espindola@gmail.com" target="_blank">rafael.espindola@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Now with the patch actually attached.<br>
<div class="HOEnZb"><div class="h5"><br>
<br>
On 5 November 2013 00:33, Rafael Espíndola <<a href="mailto:rafael.espindola@gmail.com">rafael.espindola@gmail.com</a>> wrote:<br>
>>> What happens is that at the end of GetOrCreateLLVMFunction we check if<br>
>>> we have a decl defined in class and if so add it to<br>
>>> DeferredDeclsToEmit. The net result is that we end up running<br>
>>> replaceAllUsesWith twice when something in DeferredDeclsToEmit causes<br>
>>> a new use of the destructor/constructor.<br>
>>><br>
>>> Do you think this is OK? If not I can add a list of "DeclsToReplace"<br>
>>> to Codegen and process that after EmitDeferred.<br>
>><br>
>><br>
>> Ah, that makes sense.  Sounds fine, if a little bit silly.<br>
><br>
> You were right. The example I tried worked because there was a call to<br>
> GetOrCreateLLVMFunction before every try to emit the decl, so we would<br>
> recreate the declaration before trying to emit it in EmitDeferred.<br>
><br>
> With<br>
><br>
> struct Option {<br>
>   virtual ~Option() {}<br>
> };<br>
> template <class DataType> class opt : public Option {};<br>
> template class opt<int>;<br>
> bool handleOccurrence() {<br>
>   Option x;<br>
>   return true;<br>
> }<br>
><br>
> the template instantiation will not cause a declaration to be created<br>
> and we assert when trying to replace the value the second time.<br>
><br>
> The attached patch fixes this by doing all replacements after the<br>
> deferred decls have been emitted. With this approach it should then be<br>
> easy to also handle<br>
><br>
> class foo {<br>
>   ~foo();<br>
> }<br>
> class bar : public foo {<br>
> }<br>
><br>
> i.e., when we know a function is equal to one we don't have a definition for.<br>
><br>
> Cheers,<br>
> Rafael<br>
</div></div></blockquote></div><br></div>