Hi, Nikola.
Thanks you again for updating the patch. The only thing you missed to do
is to pass additional parameter to CheckLookupAccess in SemaDeclCXX.cpp:
CheckLookupAccess(Previous, *true*);
"true" means - use current declaration context in delayed check. Since
as I wrote before we're parsing class with "friend" declaration, but
diagnostics is invoked for class with definition of friend member
itself, and improper context is used by default.
Well, I attached two patches. The first is without any fixes in delayed
diagnostics. I just implemented CheckLookupAccessNow, that doesn't use
DD. Ss McCall said: "because we know exactly what we're checking access
to, and we're getting nothing out of delaying the diagnostic."
The patch name is pr12328-checknow.patch
But IMHO, the DD works a little bit improperly. I think it is not proper
to use decl context for class for which it was invoked. We should use
decl context for class/method for which it was *requested*.
The patch name is ppr12328-fixedDD.patch
Also I patch with testcases is attached.
-Stepan.
Nikola Smiljanic wrote:
I was just looking for something to work on myself when I saw your
patch and gave it a try. I'm not in position to comment the
implementation but I guess that John McCall is the right person to CC
when (or if) you get back to this issue.
Index: include/clang/Sema/Sema.h
===================================================================
--- include/clang/Sema/Sema.h (revision 161486)
+++ include/clang/Sema/Sema.h (working copy)
@@ -4444,6 +4444,7 @@
bool ForceCheck = false,
bool ForceUnprivileged = false);
void CheckLookupAccess(const LookupResult &R);
+ void CheckLookupAccessNow(const LookupResult &R);
bool IsSimplyAccessible(NamedDecl *decl, DeclContext *Ctx);
bool isSpecialMemberAccessibleForDeletion(CXXMethodDecl *decl,
AccessSpecifier access,
Index: lib/Sema/SemaDeclCXX.cpp
===================================================================
--- lib/Sema/SemaDeclCXX.cpp (revision 161486)
+++ lib/Sema/SemaDeclCXX.cpp (working copy)
@@ -10306,6 +10306,10 @@
}
}
+ if (Previous.getResultKind() == LookupResult::Found &&
+ Previous.getFoundDecl()->isCXXClassMember())
+ CheckLookupAccessNow(Previous);
+
// FIXME: This is an egregious hack to cope with cases where the scope stack
// does not contain the declaration context, i.e., in an out-of-line
// definition of a class.
Index: lib/Sema/SemaAccess.cpp
===================================================================
--- lib/Sema/SemaAccess.cpp (revision 161486)
+++ lib/Sema/SemaAccess.cpp (working copy)
@@ -1418,6 +1418,24 @@
llvm_unreachable("falling off end");
}
+/// CheckAccessNow - does the same as CheckAccess, but doesn't perform
+/// delayed diagnostics. It is supposed that user knows that diagnostics
+/// shouldn't be delayed.
+static Sema::AccessResult CheckAccessNow(Sema &S, SourceLocation Loc,
+ AccessTarget &Entity) {
+ // If the access path is public, it's accessible everywhere.
+ if (Entity.getAccess() == AS_public)
+ return Sema::AR_accessible;
+
+ EffectiveContext EC(S.CurContext);
+ switch (CheckEffectiveAccess(S, EC, Loc, Entity)) {
+ case AR_accessible: return Sema::AR_accessible;
+ case AR_inaccessible: return Sema::AR_inaccessible;
+ case AR_dependent: return Sema::AR_dependent;
+ }
+ llvm_unreachable("falling off end");
+}
+
void Sema::HandleDelayedAccessCheck(DelayedDiagnostic &DD, Decl *decl) {
// Access control for names used in the declarations of functions
// and function templates should normally be evaluated in the context
@@ -1768,6 +1786,23 @@
}
}
+/// Checks access to all the declarations in the given result set.
+void Sema::CheckLookupAccessNow(const LookupResult &R) {
+ assert(getLangOpts().AccessControl
+ && "performing access check without access control");
+ assert(R.getNamingClass() && "performing access check without naming class");
+
+ for (LookupResult::iterator I = R.begin(), E = R.end(); I != E; ++I) {
+ if (I.getAccess() != AS_public) {
+ AccessTarget Entity(Context, AccessedEntity::Member,
+ R.getNamingClass(), I.getPair(),
+ R.getBaseObjectType());
+ Entity.setDiag(diag::err_access);
+ CheckAccessNow(*this, R.getNameLoc(), Entity);
+ }
+ }
+}
+
/// Checks access to Decl from the given class. The check will take access
/// specifiers into account, but no member access expressions and such.
///
Index: include/clang/Sema/DelayedDiagnostic.h
===================================================================
--- include/clang/Sema/DelayedDiagnostic.h (revision 161486)
+++ include/clang/Sema/DelayedDiagnostic.h (working copy)
@@ -44,10 +44,13 @@
MemberNonce _,
CXXRecordDecl *NamingClass,
DeclAccessPair FoundDecl,
- QualType BaseObjectType)
+ QualType BaseObjectType,
+ DeclContext *FromContext = 0)
: Access(FoundDecl.getAccess()), IsMember(true),
Target(FoundDecl.getDecl()), NamingClass(NamingClass),
- BaseObjectType(BaseObjectType), Diag(0, Allocator) {
+ BaseObjectType(BaseObjectType),
+ FromContext(FromContext),
+ Diag(0, Allocator) {
}
AccessedEntity(PartialDiagnostic::StorageAllocator &Allocator,
@@ -58,6 +61,7 @@
: Access(Access), IsMember(false),
Target(BaseClass),
NamingClass(DerivedClass),
+ FromContext(0),
Diag(0, Allocator) {
}
@@ -78,6 +82,10 @@
/// Retrieves the base object type, important when accessing
/// an instance member.
QualType getBaseObjectType() const { return BaseObjectType; }
+
+ /// Retrieves the specified DeclarationContext from which
+ /// we want to access the target.
+ DeclContext *getFromContext() const { return FromContext; }
/// Sets a diagnostic to be performed. The diagnostic is given
/// four (additional) arguments:
@@ -105,6 +113,7 @@
NamedDecl *Target;
CXXRecordDecl *NamingClass;
QualType BaseObjectType;
+ DeclContext *FromContext;
PartialDiagnostic Diag;
};
Index: include/clang/Sema/Sema.h
===================================================================
--- include/clang/Sema/Sema.h (revision 161486)
+++ include/clang/Sema/Sema.h (working copy)
@@ -4443,7 +4443,7 @@
unsigned DiagID,
bool ForceCheck = false,
bool ForceUnprivileged = false);
- void CheckLookupAccess(const LookupResult &R);
+ void CheckLookupAccess(const LookupResult &R, bool UseCurContext = false);
bool IsSimplyAccessible(NamedDecl *decl, DeclContext *Ctx);
bool isSpecialMemberAccessibleForDeletion(CXXMethodDecl *decl,
AccessSpecifier access,
Index: lib/Sema/SemaDeclCXX.cpp
===================================================================
--- lib/Sema/SemaDeclCXX.cpp (revision 161486)
+++ lib/Sema/SemaDeclCXX.cpp (working copy)
@@ -10306,6 +10306,10 @@
}
}
+ if (Previous.getResultKind() == LookupResult::Found &&
+ Previous.getFoundDecl()->isCXXClassMember())
+ CheckLookupAccess(Previous, true);
+
// FIXME: This is an egregious hack to cope with cases where the scope stack
// does not contain the declaration context, i.e., in an out-of-line
// definition of a class.
Index: lib/Sema/SemaAccess.cpp
===================================================================
--- lib/Sema/SemaAccess.cpp (revision 161486)
+++ lib/Sema/SemaAccess.cpp (working copy)
@@ -151,9 +151,10 @@
MemberNonce _,
CXXRecordDecl *NamingClass,
DeclAccessPair FoundDecl,
- QualType BaseObjectType)
+ QualType BaseObjectType,
+ DeclContext *FromContext = 0)
: AccessedEntity(Context.getDiagAllocator(), Member, NamingClass,
- FoundDecl, BaseObjectType) {
+ FoundDecl, BaseObjectType, FromContext) {
initialize();
}
@@ -1424,12 +1425,15 @@
// of the declaration, just in case it's a friend of something.
// However, this does not apply to local extern declarations.
- DeclContext *DC = decl->getDeclContext();
- if (FunctionDecl *fn = dyn_cast<FunctionDecl>(decl)) {
- if (!DC->isFunctionOrMethod()) DC = fn;
- } else if (FunctionTemplateDecl *fnt = dyn_cast<FunctionTemplateDecl>(decl)) {
- // Never a local declaration.
- DC = fnt->getTemplatedDecl();
+ DeclContext *DC = DD.getAccessData().getFromContext();
+ if (!DC) {
+ DC = decl->getDeclContext();
+ if (FunctionDecl *fn = dyn_cast<FunctionDecl>(decl)) {
+ if (!DC->isFunctionOrMethod()) DC = fn;
+ } else if (FunctionTemplateDecl *fnt = dyn_cast<FunctionTemplateDecl>(decl)) {
+ // Never a local declaration.
+ DC = fnt->getTemplatedDecl();
+ }
}
EffectiveContext EC(DC);
@@ -1752,7 +1756,7 @@
}
/// Checks access to all the declarations in the given result set.
-void Sema::CheckLookupAccess(const LookupResult &R) {
+void Sema::CheckLookupAccess(const LookupResult &R, bool UseCurContext) {
assert(getLangOpts().AccessControl
&& "performing access check without access control");
assert(R.getNamingClass() && "performing access check without naming class");
@@ -1761,7 +1765,8 @@
if (I.getAccess() != AS_public) {
AccessTarget Entity(Context, AccessedEntity::Member,
R.getNamingClass(), I.getPair(),
- R.getBaseObjectType());
+ R.getBaseObjectType(),
+ UseCurContext ? CurContext : 0);
Entity.setDiag(diag::err_access);
CheckAccess(*this, R.getNameLoc(), Entity);
}
Index: test/CXX/class.access/class.friend/p1.cpp
===================================================================
--- test/CXX/class.access/class.friend/p1.cpp (revision 155311)
+++ test/CXX/class.access/class.friend/p1.cpp (working copy)
@@ -64,6 +64,7 @@
};
class MemberFriend {
+ public:
void test();
};
@@ -309,6 +310,7 @@
// PR8705
namespace test11 {
class A {
+ public:
void test0(int);
void test1(int);
void test2(int);
Index: test/SemaCXX/friend.cpp
===================================================================
--- test/SemaCXX/friend.cpp (revision 155311)
+++ test/SemaCXX/friend.cpp (working copy)
@@ -138,3 +138,13 @@
};
}
}
+
+// PR12328
+namespace test8 {
+ class X {
+ void f(); // expected-note {{implicitly declared private here}}
+ };
+ class Y {
+ friend void X::f(); // expected-error {{'f' is a private member of 'test8::X'}}
+ };
+}
\ No newline at end of file
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
