Skip to content

Commit

Permalink
Support friend function declarations in local classes correctly.
Browse files Browse the repository at this point in the history
Fixes a crash and diagnoses the error condition of an unqualified
friend which doesn't resolve to something.  I'm still not certain how
this is useful.

llvm-svn: 116393
  • Loading branch information
rjmccall committed Oct 13, 2010
1 parent 2473992 commit f7cfb22
Show file tree
Hide file tree
Showing 4 changed files with 108 additions and 51 deletions.
2 changes: 2 additions & 0 deletions clang/include/clang/Basic/DiagnosticSemaKinds.td
Original file line number Diff line number Diff line change
Expand Up @@ -444,6 +444,8 @@ def err_introducing_special_friend : Error<
"destructor|conversion operator}0 as a friend">;
def err_tagless_friend_type_template : Error<
"friend type templates must use an elaborated type">;
def err_no_matching_local_friend : Error<
"no matching function found in local scope">;

def err_abstract_type_in_decl : Error<
"%select{return|parameter|variable|field}0 type %1 is an abstract class">;
Expand Down
110 changes: 65 additions & 45 deletions clang/lib/Sema/SemaDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3153,6 +3153,20 @@ void Sema::AddOverriddenMethods(CXXRecordDecl *DC, CXXMethodDecl *MD) {
}
}

static void DiagnoseInvalidRedeclaration(Sema &S, FunctionDecl *NewFD) {
LookupResult Prev(S, NewFD->getDeclName(), NewFD->getLocation(),
Sema::LookupOrdinaryName, Sema::ForRedeclaration);
S.LookupQualifiedName(Prev, NewFD->getDeclContext());
assert(!Prev.isAmbiguous() &&
"Cannot have an ambiguity in previous-declaration lookup");
for (LookupResult::iterator Func = Prev.begin(), FuncEnd = Prev.end();
Func != FuncEnd; ++Func) {
if (isa<FunctionDecl>(*Func) &&
isNearlyMatchingFunction(S.Context, cast<FunctionDecl>(*Func), NewFD))
S.Diag((*Func)->getLocation(), diag::note_member_def_close_match);
}
}

NamedDecl*
Sema::ActOnFunctionDeclarator(Scope* S, Declarator& D, DeclContext* DC,
QualType R, TypeSourceInfo *TInfo,
Expand Down Expand Up @@ -3470,13 +3484,6 @@ Sema::ActOnFunctionDeclarator(Scope* S, Declarator& D, DeclContext* DC,
FilterLookupForScope(*this, Previous, DC, S, NewFD->hasLinkage());

if (isFriend) {
// DC is the namespace in which the function is being declared.
assert((DC->isFileContext() || !Previous.empty() ||
(D.getCXXScopeSpec().isSet() &&
D.getCXXScopeSpec().getScopeRep()->isDependent())) &&
"previously-undeclared friend function being created "
"in a non-namespace context");

// For now, claim that the objects have no previous declaration.
if (FunctionTemplate) {
FunctionTemplate->setObjectOfFriendDecl(false);
Expand Down Expand Up @@ -3675,24 +3682,36 @@ Sema::ActOnFunctionDeclarator(Scope* S, Declarator& D, DeclContext* DC,
: TPC_FunctionTemplate);
}

if (D.getCXXScopeSpec().isSet() && !NewFD->isInvalidDecl()) {
if (isFriend || !CurContext->isRecord()) {
// Fake up an access specifier if it's supposed to be a class member.
if (!Redeclaration && isa<CXXRecordDecl>(NewFD->getDeclContext()))
NewFD->setAccess(AS_public);

// An out-of-line member function declaration must also be a
// definition (C++ [dcl.meaning]p1).
// Note that this is not the case for explicit specializations of
// function templates or member functions of class templates, per
// C++ [temp.expl.spec]p2. We also allow these declarations as an extension
// for compatibility with old SWIG code which likes to generate them.
if (!IsFunctionDefinition && !isFriend &&
!isFunctionTemplateSpecialization && !isExplicitSpecialization) {
Diag(NewFD->getLocation(), diag::ext_out_of_line_declaration)
<< D.getCXXScopeSpec().getRange();
}
if (!Redeclaration && !(isFriend && CurContext->isDependentContext())) {
if (NewFD->isInvalidDecl()) {
// Ignore all the rest of this.

} else if (CurContext->isRecord() && D.getCXXScopeSpec().isSet() &&
!isFriend) {
// The user provided a superfluous scope specifier inside a class
// definition:
//
// class X {
// void X::f();
// };
Diag(NewFD->getLocation(), diag::warn_member_extra_qualification)
<< Name << FixItHint::CreateRemoval(D.getCXXScopeSpec().getRange());

} else if (!Redeclaration) {
// Fake up an access specifier if it's supposed to be a class member.
if (isa<CXXRecordDecl>(NewFD->getDeclContext()))
NewFD->setAccess(AS_public);

// Qualified decls generally require a previous declaration.
if (D.getCXXScopeSpec().isSet()) {
// ...with the major exception of dependent friend declarations.
// In theory, this condition could be whether the qualifier
// is dependent; in practice, the way we nest template parameters
// prevents this sort of matching from working, so we have to base it
// on the general dependence of the context.
if (isFriend && CurContext->isDependentContext()) {
// ignore these

} else {
// The user tried to provide an out-of-line definition for a
// function that is a member of a class or namespace, but there
// was no such member function declared (C++ [class.mfct]p2,
Expand All @@ -3711,27 +3730,28 @@ Sema::ActOnFunctionDeclarator(Scope* S, Declarator& D, DeclContext* DC,
<< Name << DC << D.getCXXScopeSpec().getRange();
NewFD->setInvalidDecl();

LookupResult Prev(*this, Name, D.getIdentifierLoc(), LookupOrdinaryName,
ForRedeclaration);
LookupQualifiedName(Prev, DC);
assert(!Prev.isAmbiguous() &&
"Cannot have an ambiguity in previous-declaration lookup");
for (LookupResult::iterator Func = Prev.begin(), FuncEnd = Prev.end();
Func != FuncEnd; ++Func) {
if (isa<FunctionDecl>(*Func) &&
isNearlyMatchingFunction(Context, cast<FunctionDecl>(*Func), NewFD))
Diag((*Func)->getLocation(), diag::note_member_def_close_match);
}
DiagnoseInvalidRedeclaration(*this, NewFD);
}
} else {
// The user provided a superfluous scope specifier inside a class definition:
//
// class X {
// void X::f();
// };
Diag(NewFD->getLocation(), diag::warn_member_extra_qualification)
<< Name << FixItHint::CreateRemoval(D.getCXXScopeSpec().getRange());
}

// Unqualified local friend declarations are required to resolve
// to something.
} else if (isFriend && cast<CXXRecordDecl>(CurContext)->isLocalClass()) {
Diag(D.getIdentifierLoc(), diag::err_no_matching_local_friend);
NewFD->setInvalidDecl();
DiagnoseInvalidRedeclaration(*this, NewFD);
}

} else if (!IsFunctionDefinition && D.getCXXScopeSpec().isSet() &&
!isFriend && !isFunctionTemplateSpecialization &&
!isExplicitSpecialization) {
// An out-of-line member function declaration must also be a
// definition (C++ [dcl.meaning]p1).
// Note that this is not the case for explicit specializations of
// function templates or member functions of class templates, per
// C++ [temp.expl.spec]p2. We also allow these declarations as an extension
// for compatibility with old SWIG code which likes to generate them.
Diag(NewFD->getLocation(), diag::ext_out_of_line_declaration)
<< D.getCXXScopeSpec().getRange();
}

// Handle attributes. We need to have merged decls when handling attributes
Expand Down
28 changes: 22 additions & 6 deletions clang/lib/Sema/SemaDeclCXX.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6344,12 +6344,26 @@ Decl *Sema::ActOnFriendFunctionDecl(Scope *S, Declarator &D, bool IsDefinition,

// There are four cases here.
// - There's no scope specifier, in which case we just go to the
// appropriate namespace and create a function or function template
// appropriate scope and look for a function or function template
// there as appropriate.
// Recover from invalid scope qualifiers as if they just weren't there.
if (SS.isInvalid() || !SS.isSet()) {
// Walk out to the nearest namespace scope looking for matches.

// C++0x [namespace.memdef]p3:
// If the name in a friend declaration is neither qualified nor
// a template-id and the declaration is a function or an
// elaborated-type-specifier, the lookup to determine whether
// the entity has been previously declared shall not consider
// any scopes outside the innermost enclosing namespace.
// C++0x [class.friend]p11:
// If a friend declaration appears in a local class and the name
// specified is an unqualified name, a prior declaration is
// looked up without considering scopes that are outside the
// innermost enclosing non-class scope. For a friend function
// declaration, if there is no prior declaration, the program is
// ill-formed.
bool isLocal = cast<CXXRecordDecl>(CurContext)->isLocalClass();

// Find the appropriate context according to the above.
DC = CurContext;
while (true) {
// Skip class contexts. If someone can cite chapter and verse
Expand All @@ -6365,9 +6379,9 @@ Decl *Sema::ActOnFriendFunctionDecl(Scope *S, Declarator &D, bool IsDefinition,
LookupQualifiedName(Previous, DC);

// TODO: decide what we think about using declarations.
if (!Previous.empty())
if (isLocal || !Previous.empty())
break;

if (DC->isFileContext()) break;
DC = DC->getParent();
}
Expand All @@ -6381,6 +6395,8 @@ Decl *Sema::ActOnFriendFunctionDecl(Scope *S, Declarator &D, bool IsDefinition,
&& !getLangOptions().CPlusPlus0x)
Diag(DS.getFriendSpecLoc(), diag::err_friend_is_member);

S = getScopeForDeclContext(S, DC);

// - There's a non-dependent scope specifier, in which case we
// compute it and do a previous lookup there for a function
// or function template.
Expand Down Expand Up @@ -6425,7 +6441,7 @@ Decl *Sema::ActOnFriendFunctionDecl(Scope *S, Declarator &D, bool IsDefinition,
assert(isa<CXXRecordDecl>(DC) && "friend declaration not in class?");
}

if (DC->isFileContext()) {
if (!DC->isRecord()) {
// This implies that it has to be an operator or function.
if (D.getName().getKind() == UnqualifiedId::IK_ConstructorName ||
D.getName().getKind() == UnqualifiedId::IK_DestructorName ||
Expand Down
19 changes: 19 additions & 0 deletions clang/test/CXX/class.access/class.friend/p11.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// RUN: %clang_cc1 -fsyntax-only -verify %s

// rdar://problem/8540720
namespace test0 {
void foo() {
void bar();
class A {
friend void bar();
};
}
}

namespace test1 {
void foo() {
class A {
friend void bar(); // expected-error {{no matching function found in local scope}}
};
}
}

0 comments on commit f7cfb22

Please sign in to comment.