<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Fri, Mar 14, 2014 at 5:36 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 class="">On Fri, Mar 14, 2014 at 3:07 PM, Richard Smith<br>
<<a href="mailto:richard-llvm@metafoo.co.uk">richard-llvm@metafoo.co.uk</a>> wrote:<br>
> Author: rsmith<br>
> Date: Fri Mar 14 17:07:27 2014<br>
> New Revision: 203978<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=203978&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=203978&view=rev</a><br>
> Log:<br>
> Call RequireCompleteType when performing ADL even if the type is already<br>
> complete.<br>
<br>
</div>I'm confused by this - looking at the debug info test cases you<br>
changed it seems odd that the type is required to be complete in those<br>
cases, given that the code still compiles if I replace the definition<br>
with a declaration in those files.</blockquote><div><br></div><div>RequireCompleteType is being called here because the semantics of the program could be affected by the completeness of the type -- in this case, that's because the class is an associated class of the argument-dependent lookup, so we look inside the definition of the class for any friends it might have.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="">
> We hook into this check from a couple of other places (modules,<br>
> debug info) so it's not OK to elide the check if the type was already<br>
> complete.<br>
<br>
</div>It might be helpful to have some modules test cases to cover this<br>
functionality - we might want to try to find some ways to avoid this<br>
for debug info... (though I'm not holding my breath for dealing with<br>
this any time soon)</blockquote><div><br></div><div>You could possibly check whether the RequireCompleteType call is provided with a diagnostic, but even that is probably a bit unreliable.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="HOEnZb"><div class="h5">
><br>
> Modified:<br>
> cfe/trunk/lib/Sema/SemaLookup.cpp<br>
> cfe/trunk/test/CodeGenCXX/debug-info-class-limited.cpp<br>
> cfe/trunk/test/CodeGenCXX/debug-info-limited.cpp<br>
><br>
> Modified: cfe/trunk/lib/Sema/SemaLookup.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaLookup.cpp?rev=203978&r1=203977&r2=203978&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaLookup.cpp?rev=203978&r1=203977&r2=203978&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/lib/Sema/SemaLookup.cpp (original)<br>
> +++ cfe/trunk/lib/Sema/SemaLookup.cpp Fri Mar 14 17:07:27 2014<br>
> @@ -2027,6 +2027,10 @@ addAssociatedClassesAndNamespaces(Associ<br>
><br>
> // Add the class itself. If we've already seen this class, we don't<br>
> // need to visit base classes.<br>
> + //<br>
> + // FIXME: That's not correct, we may have added this class only because it<br>
> + // was the enclosing class of another class, and in that case we won't have<br>
> + // added its base classes yet.<br>
> if (!Result.Classes.insert(Class))<br>
> return;<br>
><br>
> @@ -2053,12 +2057,8 @@ addAssociatedClassesAndNamespaces(Associ<br>
> }<br>
><br>
> // Only recurse into base classes for complete types.<br>
> - if (!Class->hasDefinition()) {<br>
> - QualType type = Result.S.Context.getTypeDeclType(Class);<br>
> - if (Result.S.RequireCompleteType(Result.InstantiationLoc, type,<br>
> - /*no diagnostic*/ 0))<br>
> - return;<br>
> - }<br>
> + if (!Class->hasDefinition())<br>
> + return;<br>
><br>
> // Add direct and indirect base classes along with their associated<br>
> // namespaces.<br>
> @@ -2150,6 +2150,8 @@ addAssociatedClassesAndNamespaces(Associ<br>
> // classes. Its associated namespaces are the namespaces in<br>
> // which its associated classes are defined.<br>
> case Type::Record: {<br>
> + Result.S.RequireCompleteType(Result.InstantiationLoc, QualType(T, 0),<br>
> + /*no diagnostic*/ 0);<br>
> CXXRecordDecl *Class<br>
> = cast<CXXRecordDecl>(cast<RecordType>(T)->getDecl());<br>
> addAssociatedClassesAndNamespaces(Result, Class);<br>
><br>
> Modified: cfe/trunk/test/CodeGenCXX/debug-info-class-limited.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/debug-info-class-limited.cpp?rev=203978&r1=203977&r2=203978&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/debug-info-class-limited.cpp?rev=203978&r1=203977&r2=203978&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/test/CodeGenCXX/debug-info-class-limited.cpp (original)<br>
> +++ cfe/trunk/test/CodeGenCXX/debug-info-class-limited.cpp Fri Mar 14 17:07:27 2014<br>
> @@ -24,7 +24,7 @@ foo *bar(foo *a) {<br>
> }<br>
><br>
> namespace test1 {<br>
> -// CHECK-DAG: [ DW_TAG_structure_type ] [foo] [line [[@LINE+1]], {{.*}} [decl]<br>
> +// CHECK-DAG: [ DW_TAG_structure_type ] [foo] [line [[@LINE+1]], {{.*}} [def]<br>
> struct foo {<br>
> };<br>
><br>
><br>
> Modified: cfe/trunk/test/CodeGenCXX/debug-info-limited.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/debug-info-limited.cpp?rev=203978&r1=203977&r2=203978&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/debug-info-limited.cpp?rev=203978&r1=203977&r2=203978&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/test/CodeGenCXX/debug-info-limited.cpp (original)<br>
> +++ cfe/trunk/test/CodeGenCXX/debug-info-limited.cpp Fri Mar 14 17:07:27 2014<br>
> @@ -11,8 +11,7 @@ A *foo (A* x) {<br>
> return a;<br>
> }<br>
><br>
> -// Verify that we're not emitting a full definition of B in limit debug mode.<br>
> -// CHECK: ; [ DW_TAG_class_type ] [B] {{.*}} [decl]<br>
> +// CHECK: ; [ DW_TAG_class_type ] [B] {{.*}} [def]<br>
><br>
> class B {<br>
> public:<br>
><br>
><br>
> _______________________________________________<br>
> cfe-commits mailing list<br>
> <a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
</div></div></blockquote></div><br></div></div>