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