<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Fri, Jan 30, 2015 at 2:48 PM, jahanian <span dir="ltr"><<a href="mailto:fjahanian@apple.com" target="_blank">fjahanian@apple.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">This patch attempts to fix a crash during instantiation of a direct destructor call; as in foo::~foo().<br>
Please review.</blockquote><div><br></div><div>Hmm, we're looking up the wrong name. D->getDeclName() will be the name of the primary template's destructor; we should transform that name prior to the call to lookup so that we look up the destructor of the specialization.</div><div><br></div><div>While playing with this, I found we also assert for:<br></div><div><br></div><div>  template<typename T> struct X { operator X(); void f() { &operator X; } };<br></div><div><br></div><div>... which may or may not be related.</div><div><br></div><div>We also reject-valid on this reduced case:</div><div><br></div><div><div>  template<typename T> struct X { void f() { X::~X(); } };</div></div><div><br></div><div>... which is a different bug (lookup fails because we've not actually declared ~X() yet, and we choose to not do so because we're in a dependent context), but we should first figure out how we'll tackle it because that informs the change we make here. In particular:</div><div><br></div><div>+      if (isa<CXXDestructorDecl>(D)) {<br></div><div><div>+        if (CXXRecordDecl *Spec = dyn_cast<CXXRecordDecl>(ParentDC))</div><div>+          if (!Spec->isDependentContext())</div><div>+            Result = LookupDestructor(Spec);</div><div><br></div><div>LookupDestructor may implicitly declare the destructor, which should never be necessary if the template had a declared destructor; calling lookup / findInstantiationOf with the appropriate DeclarationName would be better in that case. If on the other hand the template might have an implicitly-declared destructor, LookupDestructor is appropriate here (but might be problematic for other reasons, if the class is not yet complete).</div><div><br></div><div>+      } else {</div><div>+          DeclContext::lookup_result Found = ParentDC->lookup(D->getDeclName());</div><div>+          Result = findInstantiationOf(Context, D, Found.begin(), Found.end());</div><div><br></div><div>Weird indentation.</div><div><br></div><div>+    DeclarationNameInfo NameInfo = !isa<CXXDestructorDecl>(FoundDecl)<br></div><div>+                                      ? E->getMemberNameInfo()</div><div>+                                      : DeclarationNameInfo(FoundDecl->getDeclName(),</div><div>+                                                            FoundDecl->getLocation());</div><div><br></div><div>This is wrong: you want the location of the destructor-name here, not the location of the declaration of the destructor. Instead of this change, you should call TransformDeclarationNameInfo. (It looks like we get conversion function names wrong here too.)</div></div></div></div></div>