Skip to content

Commit

Permalink
[modules] Don't try to use the definition of a class if
Browse files Browse the repository at this point in the history 
RequireCompleteType(..., 0) says we're not permitted to do so. The definition
might not be visible, even though we know what it is.

llvm-svn: 256045
  • Loading branch information
@zygoloid
zygoloid committed Dec 18, 2015
1 parent e78b3e6 commit 82b8d4e
Show file tree
Hide file tree
Showing 8 changed files with 126 additions and 106 deletions.
3 changes: 2 additions & 1 deletion clang/include/clang/Sema/Sema.h
View file
Expand Up @@ -1348,7 +1348,7 @@ class Sema {
void emit(const SemaDiagnosticBuilder &DB,
llvm::index_sequence<Is...>) const {
// Apply all tuple elements to the builder in order.
bool Dummy[] = {(DB << getPrintable(std::get<Is>(Args)))...};
bool Dummy[] = {false, (DB << getPrintable(std::get<Is>(Args)))...};
(void)Dummy;
}

Expand Down Expand Up @@ -1425,6 +1425,7 @@ class Sema {
return RequireCompleteType(Loc, T, Diagnoser);
}

void completeExprArrayBound(Expr *E);
bool RequireCompleteExprType(Expr *E, TypeDiagnoser &Diagnoser);
bool RequireCompleteExprType(Expr *E, unsigned DiagID);

Expand Down
12 changes: 6 additions & 6 deletions clang/lib/Sema/SemaLookup.cpp
View file
Expand Up @@ -2428,7 +2428,8 @@ addAssociatedClassesAndNamespaces(AssociatedLookup &Result,
}

// Only recurse into base classes for complete types.
if (!Class->hasDefinition())
if (Result.S.RequireCompleteType(Result.InstantiationLoc,
Result.S.Context.getRecordType(Class), 0))
return;

// Add direct and indirect base classes along with their associated
Expand Down Expand Up @@ -2521,10 +2522,8 @@ addAssociatedClassesAndNamespaces(AssociatedLookup &Result, QualType Ty) {
// 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());
CXXRecordDecl *Class =
cast<CXXRecordDecl>(cast<RecordType>(T)->getDecl());
addAssociatedClassesAndNamespaces(Result, Class);
break;
}
Expand Down Expand Up @@ -3208,7 +3207,8 @@ void Sema::ArgumentDependentLookup(DeclarationName Name, SourceLocation Loc,
for (Decl *DI = D; DI; DI = DI->getPreviousDecl()) {
DeclContext *LexDC = DI->getLexicalDeclContext();
if (isa<CXXRecordDecl>(LexDC) &&
AssociatedClasses.count(cast<CXXRecordDecl>(LexDC))) {
AssociatedClasses.count(cast<CXXRecordDecl>(LexDC)) &&
isVisible(cast<NamedDecl>(DI))) {
DeclaredInAssociatedClass = true;
break;
}
Expand Down
9 changes: 3 additions & 6 deletions clang/lib/Sema/SemaOverload.cpp
View file
Expand Up @@ -3085,11 +3085,7 @@ IsUserDefinedConversion(Sema &S, Expr *From, QualType ToType,
S.IsDerivedFrom(From->getLocStart(), From->getType(), ToType)))
ConstructorsOnly = true;

S.RequireCompleteType(From->getExprLoc(), ToType, 0);
// RequireCompleteType may have returned true due to some invalid decl
// during template instantiation, but ToType may be complete enough now
// to try to recover.
if (ToType->isIncompleteType()) {
if (S.RequireCompleteType(From->getExprLoc(), ToType, 0)) {
// We're not going to find any constructors.
} else if (CXXRecordDecl *ToRecordDecl
= dyn_cast<CXXRecordDecl>(ToRecordType->getDecl())) {
Expand Down Expand Up @@ -6685,7 +6681,8 @@ void Sema::AddMemberOperatorCandidates(OverloadedOperatorKind Op,
// the set of member candidates is empty.
if (const RecordType *T1Rec = T1->getAs<RecordType>()) {
// Complete the type if it can be completed.
RequireCompleteType(OpLoc, T1, 0);
if (RequireCompleteType(OpLoc, T1, 0) && !T1Rec->isBeingDefined())
return;
// If the type is neither complete nor being defined, bail out now.
if (!T1Rec->getDecl()->getDefinition())
return;
Expand Down
182 changes: 94 additions & 88 deletions clang/lib/Sema/SemaType.cpp
View file
Expand Up @@ -6366,6 +6366,50 @@ static void processTypeAttrs(TypeProcessingState &state, QualType &type,
}
}

void Sema::completeExprArrayBound(Expr *E) {
if (DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(E->IgnoreParens())) {
if (VarDecl *Var = dyn_cast<VarDecl>(DRE->getDecl())) {
if (isTemplateInstantiation(Var->getTemplateSpecializationKind())) {
SourceLocation PointOfInstantiation = E->getExprLoc();

if (MemberSpecializationInfo *MSInfo =
Var->getMemberSpecializationInfo()) {
// If we don't already have a point of instantiation, this is it.
if (MSInfo->getPointOfInstantiation().isInvalid()) {
MSInfo->setPointOfInstantiation(PointOfInstantiation);

// This is a modification of an existing AST node. Notify
// listeners.
if (ASTMutationListener *L = getASTMutationListener())
L->StaticDataMemberInstantiated(Var);
}
} else {
VarTemplateSpecializationDecl *VarSpec =
cast<VarTemplateSpecializationDecl>(Var);
if (VarSpec->getPointOfInstantiation().isInvalid())
VarSpec->setPointOfInstantiation(PointOfInstantiation);
}

InstantiateVariableDefinition(PointOfInstantiation, Var);

// Update the type to the newly instantiated definition's type both
// here and within the expression.
if (VarDecl *Def = Var->getDefinition()) {
DRE->setDecl(Def);
QualType T = Def->getType();
DRE->setType(T);
// FIXME: Update the type on all intervening expressions.
E->setType(T);
}

// We still go on to try to complete the type independently, as it
// may also require instantiations or diagnostics if it remains
// incomplete.
}
}
}
}

/// \brief Ensure that the type of the given expression is complete.
///
/// This routine checks whether the expression \p E has a complete type. If the
Expand All @@ -6380,87 +6424,26 @@ static void processTypeAttrs(TypeProcessingState &state, QualType &type,
///
/// \returns \c true if the type of \p E is incomplete and diagnosed, \c false
/// otherwise.
bool Sema::RequireCompleteExprType(Expr *E, TypeDiagnoser &Diagnoser){
bool Sema::RequireCompleteExprType(Expr *E, TypeDiagnoser &Diagnoser) {
QualType T = E->getType();

// Fast path the case where the type is already complete.
if (!T->isIncompleteType())
// FIXME: The definition might not be visible.
return false;

// Incomplete array types may be completed by the initializer attached to
// their definitions. For static data members of class templates and for
// variable templates, we need to instantiate the definition to get this
// initializer and complete the type.
if (T->isIncompleteArrayType()) {
if (DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(E->IgnoreParens())) {
if (VarDecl *Var = dyn_cast<VarDecl>(DRE->getDecl())) {
if (isTemplateInstantiation(Var->getTemplateSpecializationKind())) {
SourceLocation PointOfInstantiation = E->getExprLoc();

if (MemberSpecializationInfo *MSInfo =
Var->getMemberSpecializationInfo()) {
// If we don't already have a point of instantiation, this is it.
if (MSInfo->getPointOfInstantiation().isInvalid()) {
MSInfo->setPointOfInstantiation(PointOfInstantiation);

// This is a modification of an existing AST node. Notify
// listeners.
if (ASTMutationListener *L = getASTMutationListener())
L->StaticDataMemberInstantiated(Var);
}
} else {
VarTemplateSpecializationDecl *VarSpec =
cast<VarTemplateSpecializationDecl>(Var);
if (VarSpec->getPointOfInstantiation().isInvalid())
VarSpec->setPointOfInstantiation(PointOfInstantiation);
}

InstantiateVariableDefinition(PointOfInstantiation, Var);

// Update the type to the newly instantiated definition's type both
// here and within the expression.
if (VarDecl *Def = Var->getDefinition()) {
DRE->setDecl(Def);
T = Def->getType();
DRE->setType(T);
E->setType(T);
}

// We still go on to try to complete the type independently, as it
// may also require instantiations or diagnostics if it remains
// incomplete.
}
}
}
completeExprArrayBound(E);
T = E->getType();
}

// FIXME: Are there other cases which require instantiating something other
// than the type to complete the type of an expression?

// Look through reference types and complete the referred type.
if (const ReferenceType *Ref = T->getAs<ReferenceType>())
T = Ref->getPointeeType();

return RequireCompleteType(E->getExprLoc(), T, Diagnoser);
}

namespace {
struct TypeDiagnoserDiag : Sema::TypeDiagnoser {
unsigned DiagID;

TypeDiagnoserDiag(unsigned DiagID)
: Sema::TypeDiagnoser(DiagID == 0), DiagID(DiagID) {}

void diagnose(Sema &S, SourceLocation Loc, QualType T) override {
if (Suppressed) return;
S.Diag(Loc, DiagID) << T;
}
};
}

bool Sema::RequireCompleteExprType(Expr *E, unsigned DiagID) {
TypeDiagnoserDiag Diagnoser(DiagID);
BoundTypeDiagnoser<> Diagnoser(DiagID);
return RequireCompleteExprType(E, Diagnoser);
}

Expand Down Expand Up @@ -6612,9 +6595,16 @@ bool Sema::RequireCompleteTypeImpl(SourceLocation Loc, QualType T,
if (!T->isIncompleteType(&Def)) {
// If we know about the definition but it is not visible, complain.
NamedDecl *SuggestedDef = nullptr;
if (!Diagnoser.Suppressed && Def &&
!hasVisibleDefinition(Def, &SuggestedDef, /*OnlyNeedComplete*/true))
diagnoseMissingImport(Loc, SuggestedDef, /*NeedDefinition*/true);
if (Def &&
!hasVisibleDefinition(Def, &SuggestedDef, /*OnlyNeedComplete*/true)) {
// If the user is going to see an error here, recover by making the
// definition visible.
bool TreatAsComplete = !Diagnoser.Suppressed && !isSFINAEContext();
if (!Diagnoser.Suppressed)
diagnoseMissingImport(Loc, SuggestedDef, /*NeedDefinition*/true,
/*Recover*/TreatAsComplete);
return !TreatAsComplete;
}

return false;
}
Expand All @@ -6630,6 +6620,9 @@ bool Sema::RequireCompleteTypeImpl(SourceLocation Loc, QualType T,
// chain for a declaration that can be accessed through a mechanism other
// than name lookup (eg, referenced in a template, or a variable whose type
// could be completed by the module)?
//
// FIXME: Should we map through to the base array element type before
// checking for a tag type?
if (Tag || IFace) {
NamedDecl *D =
Tag ? static_cast<NamedDecl *>(Tag->getDecl()) : IFace->getDecl();
Expand Down Expand Up @@ -6660,26 +6653,45 @@ bool Sema::RequireCompleteTypeImpl(SourceLocation Loc, QualType T,
= Context.getAsConstantArrayType(MaybeTemplate))
MaybeTemplate = Array->getElementType();
if (const RecordType *Record = MaybeTemplate->getAs<RecordType>()) {
bool Instantiated = false;
bool Diagnosed = false;
if (ClassTemplateSpecializationDecl *ClassTemplateSpec
= dyn_cast<ClassTemplateSpecializationDecl>(Record->getDecl())) {
if (ClassTemplateSpec->getSpecializationKind() == TSK_Undeclared)
return InstantiateClassTemplateSpecialization(Loc, ClassTemplateSpec,
TSK_ImplicitInstantiation,
/*Complain=*/!Diagnoser.Suppressed);
if (ClassTemplateSpec->getSpecializationKind() == TSK_Undeclared) {
Diagnosed = InstantiateClassTemplateSpecialization(
Loc, ClassTemplateSpec, TSK_ImplicitInstantiation,
/*Complain=*/!Diagnoser.Suppressed);
Instantiated = true;
}
} else if (CXXRecordDecl *Rec
= dyn_cast<CXXRecordDecl>(Record->getDecl())) {
CXXRecordDecl *Pattern = Rec->getInstantiatedFromMemberClass();
if (!Rec->isBeingDefined() && Pattern) {
MemberSpecializationInfo *MSI = Rec->getMemberSpecializationInfo();
assert(MSI && "Missing member specialization information?");
// This record was instantiated from a class within a template.
if (MSI->getTemplateSpecializationKind() != TSK_ExplicitSpecialization)
return InstantiateClass(Loc, Rec, Pattern,
getTemplateInstantiationArgs(Rec),
TSK_ImplicitInstantiation,
/*Complain=*/!Diagnoser.Suppressed);
if (MSI->getTemplateSpecializationKind() !=
TSK_ExplicitSpecialization) {
Diagnosed = InstantiateClass(Loc, Rec, Pattern,
getTemplateInstantiationArgs(Rec),
TSK_ImplicitInstantiation,
/*Complain=*/!Diagnoser.Suppressed);
Instantiated = true;
}
}
}

if (Instantiated) {
// Instantiate* might have already complained that the template is not
// defined, if we asked it to.
if (!Diagnoser.Suppressed && Diagnosed)
return true;
// If we instantiated a definition, check that it's usable, even if
// instantiation produced an error, so that repeated calls to this
// function give consistent answers.
if (!T->isIncompleteType())
return RequireCompleteTypeImpl(Loc, T, Diagnoser);
}
}

if (Diagnoser.Suppressed)
Expand Down Expand Up @@ -6716,7 +6728,7 @@ bool Sema::RequireCompleteTypeImpl(SourceLocation Loc, QualType T,

bool Sema::RequireCompleteType(SourceLocation Loc, QualType T,
unsigned DiagID) {
TypeDiagnoserDiag Diagnoser(DiagID);
BoundTypeDiagnoser<> Diagnoser(DiagID);
return RequireCompleteType(Loc, T, Diagnoser);
}

Expand Down Expand Up @@ -6757,14 +6769,10 @@ bool Sema::RequireLiteralType(SourceLocation Loc, QualType T,
assert(!T->isDependentType() && "type should not be dependent");

QualType ElemType = Context.getBaseElementType(T);
RequireCompleteType(Loc, ElemType, 0);

if (T->isLiteralType(Context))
if ((!RequireCompleteType(Loc, ElemType, 0) || ElemType->isVoidType()) &&
T->isLiteralType(Context))
return false;

if (Diagnoser.Suppressed)
return true;

Diagnoser.diagnose(*this, Loc, T);

if (T->isVariableArrayType())
Expand All @@ -6779,10 +6787,8 @@ bool Sema::RequireLiteralType(SourceLocation Loc, QualType T,
// A partially-defined class type can't be a literal type, because a literal
// class type must have a trivial destructor (which can't be checked until
// the class definition is complete).
if (!RD->isCompleteDefinition()) {
RequireCompleteType(Loc, ElemType, diag::note_non_literal_incomplete, T);
if (RequireCompleteType(Loc, ElemType, diag::note_non_literal_incomplete, T))
return true;
}

// If the class has virtual base classes, then it's not an aggregate, and
// cannot have any constexpr constructors or a trivial default constructor,
Expand Down Expand Up @@ -6834,7 +6840,7 @@ bool Sema::RequireLiteralType(SourceLocation Loc, QualType T,
}

bool Sema::RequireLiteralType(SourceLocation Loc, QualType T, unsigned DiagID) {
TypeDiagnoserDiag Diagnoser(DiagID);
BoundTypeDiagnoser<> Diagnoser(DiagID);
return RequireLiteralType(Loc, T, Diagnoser);
}

Expand Down
4 changes: 2 additions & 2 deletions clang/test/Modules/cxx-templates.cpp
View file
Expand Up @@ -105,8 +105,8 @@ void g() {

TemplateInstantiationVisibility<char[1]> tiv1;
TemplateInstantiationVisibility<char[2]> tiv2;
TemplateInstantiationVisibility<char[3]> tiv3; // expected-error {{must be imported from module 'cxx_templates_b_impl'}}
// expected-note@cxx-templates-b-impl.h:10 {{previous definition is here}}
TemplateInstantiationVisibility<char[3]> tiv3; // expected-error 2{{must be imported from module 'cxx_templates_b_impl'}}
// expected-note@cxx-templates-b-impl.h:10 2{{previous definition is here}}
TemplateInstantiationVisibility<char[4]> tiv4;

int &p = WithPartialSpecializationUse().f();
Expand Down
16 changes: 16 additions & 0 deletions clang/test/Modules/hidden-definition.cpp
View file
@@ -0,0 +1,16 @@
// RUN: rm -rf %t
// RUN: mkdir %t
// RUN: echo 'struct X {}; struct Y : X { friend int f(Y); };' > %t/a.h
// RUN: echo 'module a { header "a.h" }' > %t/map
// RUN: %clang_cc1 -fmodules -x c++ -emit-module -fmodule-name=a %t/map -o %t/a.pcm
// RUN: %clang_cc1 -fmodules -x c++ -verify -fmodule-file=%t/a.pcm %s -fno-modules-error-recovery

struct X;
struct Y;

// Ensure that we can't use the definitions of X and Y, since we've not imported module a.
Y *yp;
X *xp = yp; // expected-error {{cannot initialize}}
_Static_assert(!__is_convertible(Y*, X*), "");
X &xr = *yp; // expected-error {{unrelated type}}
int g(Y &y) { f(y); } // expected-error {{undeclared identifier 'f'}}
4 changes: 2 additions & 2 deletions clang/test/Modules/submodules-merge-defs.cpp
View file
Expand Up @@ -22,7 +22,7 @@ A pre_a;
#endif
// expected-note@defs.h:1 +{{here}}
extern class A pre_a2;
int pre_use_a = use_a(pre_a2); // expected-error {{'A' must be imported}} expected-error {{'use_a' must be imported}}
int pre_use_a = use_a(pre_a2); // expected-error 2{{'A' must be imported}} expected-error {{'use_a' must be imported}}
// expected-note@defs.h:2 +{{here}}

B::Inner2 pre_bi; // expected-error +{{must be imported}}
Expand All @@ -48,7 +48,7 @@ int pre_e = E(0); // expected-error {{must be imported}}
// expected-note@defs.h:32 +{{here}}

int pre_ff = F<int>().f(); // expected-error +{{must be imported}}
int pre_fg = F<int>().g<int>(); // expected-error +{{must be imported}}
int pre_fg = F<int>().g<int>(); // expected-error +{{must be imported}} expected-error 2{{expected}}
// expected-note@defs.h:34 +{{here}}

G::A pre_ga // expected-error +{{must be imported}}
Expand Down

0 comments on commit 82b8d4e

Please sign in to comment.