Hi John. Ok, I propose two versions then.
1. Add "bool SuppressDelayedDiagnostics" param to the CheckLookupAccess
method, that will "false" by default. IMHO looks like a workaround.
2. Extend DelayedDiagnostics with enable() feature.
#1 is attached as pr12328.patch
#2 is attached as pr12328-2.patch
Also I attached new + fixed tests in pr12328-testcases.patch.
-Stepan.
John McCall wrote:
On Apr 19, 2012, at 3:45 AM, Stepan Dyatkovskiy wrote:
ping.
Stepan Dyatkovskiy wrote:
ping.
Stepan Dyatkovskiy wrote:
ping.
Stepan Dyatkovskiy wrote:
Hi all.
Please find patch in attachment for review.
I suppose that this check was not implemented yet, since I couldn't
found it anywhere. The friend method availability check was inserted in
Sema::ActOnFriendFunctionDecl.
I also added "DeclContext *FromContext" to the AccessedEntity class. It
allows to ask DelayedDiagnostic to use context that was "current" when
this diagnostics was requested. It is usefull in our case, since we
requested diagnostics when we're parsing class with "friend"
declaration, but diagnostics is invoked for class with definition of
friend member itself.
Sorry for the delay.
Please include test cases when you submit patches. Also, I'm afraid your
patch is out-of-date.
+ if (Previous.getResultKind() == LookupResult::Found&&
+ Previous.getFoundDecl()->isCXXClassMember())
+ CheckLookupAccess(Previous);
This is a bit silly, because we know exactly what we're checking access to,
and we're getting nothing out of delaying the diagnostic. It would be better
to provide a different CheckFriendAccess entrypoint that happens to
suppress diagnostic delay.
Also, you'll need to do this same access check in SemaTemplateInstantiateDecl
so that it happens in template instantiations.
John.
Index: include/clang/Sema/Sema.h
===================================================================
--- include/clang/Sema/Sema.h (revision 155311)
+++ include/clang/Sema/Sema.h (working copy)
@@ -4217,7 +4217,8 @@
unsigned DiagID,
bool ForceCheck = false,
bool ForceUnprivileged = false);
- void CheckLookupAccess(const LookupResult &R);
+ void CheckLookupAccess(const LookupResult &R,
+ bool SuppressDelayedDiagnostics = false);
bool IsSimplyAccessible(NamedDecl *decl, DeclContext *Ctx);
bool isSpecialMemberAccessibleForDeletion(CXXMethodDecl *decl,
AccessSpecifier access,
Index: lib/Sema/SemaDeclCXX.cpp
===================================================================
--- lib/Sema/SemaDeclCXX.cpp (revision 155311)
+++ lib/Sema/SemaDeclCXX.cpp (working copy)
@@ -10400,6 +10400,10 @@
}
}
+ if (Previous.getResultKind() == LookupResult::Found &&
+ Previous.getFoundDecl()->isCXXClassMember())
+ CheckLookupAccess(Previous, true /*Suppress Delayed Diagnostics*/);
+
// 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 155311)
+++ lib/Sema/SemaAccess.cpp (working copy)
@@ -1386,7 +1386,9 @@
}
static Sema::AccessResult CheckAccess(Sema &S, SourceLocation Loc,
- AccessTarget &Entity) {
+ AccessTarget &Entity,
+ bool SuppressDelayedDiagnostics = false
+ ) {
// If the access path is public, it's accessible everywhere.
if (Entity.getAccess() == AS_public)
return Sema::AR_accessible;
@@ -1405,7 +1407,8 @@
// Or we might be parsing something that will turn out to be a friend:
// void foo(A::private_type);
// void B::foo(A::private_type);
- if (S.DelayedDiagnostics.shouldDelayDiagnostics()) {
+ if (!SuppressDelayedDiagnostics &&
+ S.DelayedDiagnostics.shouldDelayDiagnostics()) {
S.DelayedDiagnostics.add(DelayedDiagnostic::makeAccess(Loc, Entity));
return Sema::AR_delayed;
}
@@ -1756,7 +1759,8 @@
}
/// Checks access to all the declarations in the given result set.
-void Sema::CheckLookupAccess(const LookupResult &R) {
+void Sema::CheckLookupAccess(const LookupResult &R,
+ bool SuppressDelayedDiagnostics) {
assert(getLangOpts().AccessControl
&& "performing access check without access control");
assert(R.getNamingClass() && "performing access check without naming class");
@@ -1767,7 +1771,7 @@
R.getNamingClass(), I.getPair(),
R.getBaseObjectType());
Entity.setDiag(diag::err_access);
- CheckAccess(*this, R.getNameLoc(), Entity);
+ CheckAccess(*this, R.getNameLoc(), Entity, SuppressDelayedDiagnostics);
}
}
}
Index: include/clang/Sema/Sema.h
===================================================================
--- include/clang/Sema/Sema.h (revision 155311)
+++ include/clang/Sema/Sema.h (working copy)
@@ -387,10 +387,14 @@
/// \brief The depth of the declarations we're currently parsing.
/// This gets saved and reset whenever we enter a class definition.
unsigned ParsingDepth;
+
+ /// \brief Enable delayed diagnostics (true by default).
+ /// Use for temporary disabling DD.
+ bool Enabled;
public:
DelayedDiagnostics() : Stack(0), StackSize(0), StackCapacity(0),
- ActiveStackBase(0), ParsingDepth(0) {}
+ ActiveStackBase(0), ParsingDepth(0), Enabled(true) {}
~DelayedDiagnostics() {
delete[] reinterpret_cast<char*>(Stack);
@@ -400,8 +404,12 @@
void add(const sema::DelayedDiagnostic &diag);
/// Determines whether diagnostics should be delayed.
- bool shouldDelayDiagnostics() { return ParsingDepth > 0; }
+ bool shouldDelayDiagnostics() { return Enabled && ParsingDepth > 0; }
+ /// Enable/Disable delayed diagnostics. Note it is only affects on
+ /// shouldDelayDiagnostics method.
+ void enable(bool Val) { Enabled = Val; }
+
/// Observe that we've started parsing a declaration. Access and
/// deprecation diagnostics will be delayed; when the declaration
/// is completed, all active delayed diagnostics will be evaluated
Index: lib/Sema/SemaDeclCXX.cpp
===================================================================
--- lib/Sema/SemaDeclCXX.cpp (revision 155311)
+++ lib/Sema/SemaDeclCXX.cpp (working copy)
@@ -10400,6 +10400,13 @@
}
}
+ if (Previous.getResultKind() == LookupResult::Found &&
+ Previous.getFoundDecl()->isCXXClassMember()) {
+ DelayedDiagnostics.enable(false);
+ CheckLookupAccess(Previous);
+ DelayedDiagnostics.enable(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: 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
