[cfe-commits] [Patch]

[erikjv]

On 15 Sep, 2011,at 05:23 PM, Douglas Gregor <dgregor at apple.com> wrote:


On Sep 14, 2011, at 9:31 AM, Erik Verbruggen wrote:

> Hello,
> 
> Attached is a patch to add CXAvailability_NotAccessible to indicate that a declaration is available, but not accessible from the current code completion context. Included is a test-case which checks this with public/protected/private members/methods for C++. This can be used with code-completion to indicate availability/accessibility of completed items.
> 
> The patch is against trunk rev. #139681. As always, feedback is welcome.

Cool! A few comments:

diff --git a/include/clang/Sema/Lookup.h b/include/clang/Sema/Lookup.h
index ce762b8..0f7dfea 100644
--- a/include/clang/Sema/Lookup.h
+++ b/include/clang/Sema/Lookup.h
@@ -655,7 +655,7 @@ private:
/// \param InBaseClass whether this declaration was found in base
/// class of the context we searched.
virtual void FoundDecl(NamedDecl *ND, NamedDecl *Hiding, 
- bool InBaseClass) = 0;
+ bool InBaseClass, bool Accessible = true) = 0;
};


I'm not thrilled about adding the Accessible flag here, because I don't want to pay the cost of access checking for every declaration we visit with this generic interface. For example, spell-checking shouldn't pay the cost of access checking for every declaration, because spell-checking only ends up examining a very small number of declarations for further consideration.
 
This flag is only used so to propagate the value down to the CodeCompletionResult. So, I will see if I can move the access-checking code into the CodeCompletionDeclConsumer.


--- a/include/clang/Sema/Sema.h
+++ b/include/clang/Sema/Sema.h
@@ -3648,6 +3648,7 @@ public:
bool ForceCheck = false,
bool ForceUnprivileged = false);
void CheckLookupAccess(const LookupResult &R);
+ bool IsAccessible(NamedDecl *decl, CXXRecordDecl *Class, bool isUsingDecl);

Can the name of this routine have some sort of "Simple" modifier tacked on to it? Full access-checking involves a bunch of madness in member access expressions and such, which we aren't doing here.
 
Will do that.


--- a/tools/c-index-test/c-index-testc
+++ b/tools/c-index-test/c-index-test.c
@@ -237,6 +237,10 @@ static void PrintCursor(CXTranslationUnit TU, CXCursor Cursor) {
case CXAvailability_NotAvailable:
printf(" (unavailable)");
break;
+
+ case CXAvailability_NotAccessible:
+ printf(" (unaccessible)");
+ break;
}

if (clang_CXXMethod_isStatic(Cursor))
@@ -1050,6 +1054,10 @@ void print_completion_result(CXCompletionResult *completion_result,
case CXAvailability_NotAvailable:
fprintf(file, " (unavailable)");
break;
+
+ case CXAvailability_NotAccessible:
+ fprintf(file, " (unaccessible)");
+ break;
}
fprintf(file, "\n");
}
-- 
1.7.4.4

"inaccessible" rather than "unaccessible", please.
 
Oops...

-- Erik.


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20110915/86f82ae1/attachment.html>