Hi James,
Hi all. I’ve been following the clang project passively for a little while, and figured it was time to start contributing a bit.
Great!
Currently, the patch does not support qualified-id lookup for dependent types or for classes with an overloaded operator->. The latter should be straightforward to add, but the former is a bit trickier, since I’m not familiar with how clang handles class templates yet. There are also likely some bugs with handling templates inside the qualified-id, but I haven’t had a chance to look at that yet. Finally, I haven’t handled the actual lookup of the members, only the nested-name-specifier part.
Okay. This is a rather non-trivial feature to tackle, so I don’t expect templates to work on the first try.
Hopefully this patch is more-or-less correct, even though it’s not complete yet. Comments are, of course, welcome (in fact, they’re requested!).
- Jim
Index: include/clang/Parse/Action.h
===================================================================
— include/clang/Parse/Action.h (revision 74890)
+++ include/clang/Parse/Action.h (working copy)
@@ -235,6 +235,14 @@
return 0;
}
- virtual bool ActOnCXXEnterMemberScope(Scope *S, ExprArg &exp) {
- return false;
- }
-
- virtual void ActOnCXXExitMemberScope(Scope *S, ExprArg &exp,
- CXXScopeSpec &SS) {
- }
-
The idea here is that the “exp” argument isn’t really being consumed by ActOnCXXEnterMemberScope or ActOnCXXExitMemberScope, but that “exp” is there to help these actions enter or exit that scope, right? If so, I suggest passing an ExprTy* rather than a reference to an ExprArg. ExprArg is a smart pointer that’s meant to be passed by value, so the signatures above look like a mistake for those of us used to the smart pointers.
Please add some comments on these two actions, describing their purpose.
[snip]
Index: lib/Sema/SemaCXXScopeSpec.cpp
— lib/Sema/SemaCXXScopeSpec.cpp (revision 74890)
+++ lib/Sema/SemaCXXScopeSpec.cpp (working copy)
@@ -14,6 +14,7 @@
#include “Sema.h”
#include “clang/AST/ASTContext.h”
#include “clang/AST/DeclTemplate.h”
+#include “clang/AST/ExprCXX.h”
#include “clang/AST/NestedNameSpecifier.h”
#include “clang/Parse/DeclSpec.h”
#include “llvm/ADT/STLExtras.h”
@@ -278,6 +279,65 @@
T.getTypePtr());
}
+bool Sema::ActOnCXXEnterMemberScope(Scope *S, ExprArg &Base) {
- Expr BaseExpr = static_cast<Expr>(Base.get());
- if(BaseExpr == 0)
- return false;
-
- QualType BaseType = BaseExpr->getType();
- if (BaseExpr->getType()->isPointerType())
- BaseType = BaseType->getAsPointerType()->getPointeeType();
This will compute a base type even for something like:
class C { int foo; };
void f(C *ptr) {
return ptr.foo;
}
I think that ActOnCXXEnterMemberScope needs to know whether the member access is with a “->” or a “.”, so it knows whether to expect BaseExpr’s type to be a pointer or not. (It’s also important when you get to implementing support for an overloaded operator->).
- if (BaseType->isDependentType()) {
- // FIXME: handle dependent types
- return false;
FIW, just creating a NestedNameSpecifier from the type pointer should suffice for dependent types (at least for now).
- } else if (!BaseType->isRecordType()) {
- Diag(BaseExpr->getExprLoc(),
- diag::err_typecheck_member_reference_struct_union)
- << BaseType << BaseExpr->getSourceRange();
- return false;
- }
Okay, good.
- CXXScopeSpec SS;
- SS.setRange(BaseExpr->getSourceRange());
- SS.setScopeRep(
- NestedNameSpecifier::Create(Context, 0, false, BaseType.getTypePtr())
- );
-
- ActOnCXXEnterDeclaratorScope(S,SS);
- return true;
+}
+void Sema::ActOnCXXExitMemberScope(Scope *S, ExprArg &Base, CXXScopeSpec &SS) {
- ExitDeclaratorContext(S);
Okay.
- Expr BaseExpr = static_cast<Expr>(Base.get());
- assert(BaseExpr && “Unable to look up base expression”);
-
- QualType BaseType = BaseExpr->getType();
- if (BaseExpr->getType()->isPointerType())
- BaseType = BaseType->getAsPointerType()->getPointeeType();
-
- RecordDecl *Rec = BaseType->getAsRecordType()->getDecl();
It’s unfortunate that we have to recompute this base type information 
- if (SS.isSet()) {
- NestedNameSpecifier *Prefix
- = static_cast<NestedNameSpecifier *>(SS.getScopeRep());
- assert(Prefix && Prefix->getKind() == NestedNameSpecifier::TypeSpec
- && “Nested name specifier does not name a type”);
-
- Type *T = Prefix->getAsType();
- DeclarationName Name = T->getAsRecordType()->getDecl()->getDeclName();
- LookupResult Result = LookupQualifiedName(Rec, Name, LookupMemberName,
- false);
- if (!Result.getAsDecl()) {
- Diag(SS.getBeginLoc(), diag::err_not_direct_base_or_virtual)
- << Name.getAsIdentifierInfo() << BaseType;
- }
- }
+}
This seems a little strange to me. Why isn’t this semantic check done in ActOnMemberExpr?
/// ActOnCXXEnterDeclaratorScope - Called when a C++ scope specifier (global
/// scope or nested-name-specifier) is parsed, part of a declarator-id.
/// After this method is called, according to [C++ 3.4.3p3], names should be
Index: lib/Sema/SemaExpr.cpp
— lib/Sema/SemaExpr.cpp (revision 74890)
+++ lib/Sema/SemaExpr.cpp (working copy)
@@ -2067,7 +2067,11 @@
Sema::ActOnMemberReferenceExpr(Scope *S, ExprArg Base, SourceLocation OpLoc,
tok::TokenKind OpKind, SourceLocation MemberLoc,
IdentifierInfo &Member,
- DeclPtrTy ObjCImpDecl,const CXXScopeSpec *SS) {
- // FIXME: handle the CXXScopeSpec for proper lookup of qualified-ids
- if (SS && SS->isInvalid())
- return ExprError();
-
Expr *BaseExpr = Base.takeAs();
assert(BaseExpr && “no record expression”);
Shouldn’t SS be used to perform name lookup for the Member somewhere in ActOnMemberReferenceExpr?
Index: lib/Parse/ParseExpr.cpp
— lib/Parse/ParseExpr.cpp (revision 74890)
+++ lib/Parse/ParseExpr.cpp (working copy)
@@ -915,6 +915,14 @@
tok::TokenKind OpKind = Tok.getKind();
SourceLocation OpLoc = ConsumeToken(); // Eat the “.” or “->” token.
- CXXScopeSpec SS;
- if (getLang().CPlusPlus) {
- if( !Actions.ActOnCXXEnterMemberScope(CurScope,LHS) )
- return ExprError();
- ParseOptionalCXXScopeSpecifier(SS);
- Actions.ActOnCXXExitMemberScope(CurScope,LHS,SS);
- }
-
Oh, this is interesting. I completely expected to have something like:
- CXXScopeSpec SS;
- if (getLang().CPlusPlus) {
- if( !Actions.ActOnCXXEnterMemberScope(CurScope,LHS) )
- return ExprError();
- ParseOptionalCXXScopeSpecifier(SS);
followed by a call to ActOnMemberReferenceExpr, then
if (getLang().CPlusPlus)
Actions.ActOnCXXExitMemberScope(CurScope,LHS,SS);
In other words, I expected that we would enter the member scope, process the member-reference expression within that scope (so that name lookup would be based on that scope), then exit the member scope at the end. \
// RUN: clang-cc -fsyntax-only -verify %s
Thanks for including test cases. You might also consider adding in some tests that involve templates (even if there are no dependent types in the expression).
This is a good start, and I think you’re on the right track. I think the biggest question at the moment is how to make ActOnMemberExpr interact with its CXXScopeSpec parameter.
