Fix the source range for a member access expression that includes a

nested-name-specifier and improve the detection of implicit 'this'
bases. Fixes <rdar://problem/8750392>.


git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@126880 91177308-0d34-0410-b5e6-96231b3b80d8

5 files changed, 88 insertions, 57 deletions

diff --git a/include/clang/AST/Expr.h b/include/clang/AST/Expr.h
index a1157c09c0..548568127a 100644
--- a/include/clang/AST/Expr.h
+++ b/include/clang/AST/Expr.h
@@ -536,6 +536,9 @@ public:
   /// temporary object of the given class type.
   bool isTemporaryObject(ASTContext &Ctx, const CXXRecordDecl *TempTy) const;
 
+  /// \brief Whether this expression is an implicit reference to 'this' in C++.
+  bool isImplicitCXXThis() const;
+  
   const Expr *IgnoreParens() const {
     return const_cast<Expr*>(this)->IgnoreParens();
   }
@@ -2070,21 +2073,15 @@ public:
   SourceLocation getMemberLoc() const { return MemberLoc; }
   void setMemberLoc(SourceLocation L) { MemberLoc = L; }
 
-  SourceRange getSourceRange() const {
-    // If we have an implicit base (like a C++ implicit this),
-    // make sure not to return its location
-    // FIXME: This isn't the way to do the above.
-    SourceLocation EndLoc = (HasExplicitTemplateArgumentList)
-      ? getRAngleLoc() : getMemberNameInfo().getEndLoc();
-
-    SourceLocation BaseLoc = getBase()->getLocStart();
-    if (BaseLoc.isInvalid())
-      return SourceRange(MemberLoc, EndLoc);
-    return SourceRange(BaseLoc, EndLoc);
-  }
-
+  SourceRange getSourceRange() const;
+  
   SourceLocation getExprLoc() const { return MemberLoc; }
 
+  /// \brief Determine whether the base of this explicit is implicit.
+  bool isImplicitAccess() const {
+    return getBase() && getBase()->isImplicitCXXThis();
+  }
+  
   static bool classof(const Stmt *T) {
     return T->getStmtClass() == MemberExprClass;
   }
diff --git a/lib/AST/Expr.cpp b/lib/AST/Expr.cpp
index 930457c249..99ce611394 100644
--- a/lib/AST/Expr.cpp
+++ b/lib/AST/Expr.cpp
@@ -943,6 +943,28 @@ MemberExpr *MemberExpr::Create(ASTContext &C, Expr *base, bool isarrow,
   return E;
 }
 
+SourceRange MemberExpr::getSourceRange() const {
+  SourceLocation StartLoc;
+  if (isImplicitAccess()) {
+    if (hasQualifier())
+      StartLoc = getQualifierLoc().getBeginLoc();
+    else
+      StartLoc = MemberLoc;
+  } else {
+    // FIXME: We don't want this to happen. Rather, we should be able to
+    // detect all kinds of implicit accesses more cleanly.
+    StartLoc = getBase()->getLocStart();
+    if (StartLoc.isInvalid())
+      StartLoc = MemberLoc;
+  }
+  
+  SourceLocation EndLoc = 
+    HasExplicitTemplateArgumentList? getRAngleLoc() 
+                                   : getMemberNameInfo().getEndLoc();
+  
+  return SourceRange(StartLoc, EndLoc);
+}
+
 const char *CastExpr::getCastKindName() const {
   switch (getCastKind()) {
   case CK_Dependent:
@@ -2017,6 +2039,42 @@ bool Expr::isTemporaryObject(ASTContext &C, const CXXRecordDecl *TempTy) const {
   return true;
 }
 
+bool Expr::isImplicitCXXThis() const {
+  const Expr *E = this;
+  
+  // Strip away parentheses and casts we don't care about.
+  while (true) {
+    if (const ParenExpr *Paren = dyn_cast<ParenExpr>(E)) {
+      E = Paren->getSubExpr();
+      continue;
+    }
+    
+    if (const ImplicitCastExpr *ICE = dyn_cast<ImplicitCastExpr>(E)) {
+      if (ICE->getCastKind() == CK_NoOp ||
+          ICE->getCastKind() == CK_LValueToRValue ||
+          ICE->getCastKind() == CK_DerivedToBase || 
+          ICE->getCastKind() == CK_UncheckedDerivedToBase) {
+        E = ICE->getSubExpr();
+        continue;
+      }
+    }
+    
+    if (const UnaryOperator* UnOp = dyn_cast<UnaryOperator>(E)) {
+      if (UnOp->getOpcode() == UO_Extension) {
+        E = UnOp->getSubExpr();
+        continue;
+      }
+    }
+    
+    break;
+  }
+  
+  if (const CXXThisExpr *This = dyn_cast<CXXThisExpr>(E))
+    return This->isImplicit();
+  
+  return false;
+}
+
 /// hasAnyTypeDependentArguments - Determines if any of the expressions
 /// in Exprs is type-dependent.
 bool Expr::hasAnyTypeDependentArguments(Expr** Exprs, unsigned NumExprs) {
diff --git a/lib/AST/ExprCXX.cpp b/lib/AST/ExprCXX.cpp
index fb99dc9c86..3dac125990 100644
--- a/lib/AST/ExprCXX.cpp
+++ b/lib/AST/ExprCXX.cpp
@@ -783,46 +783,11 @@ CXXDependentScopeMemberExpr::CreateEmpty(ASTContext &C,
   return E;
 }
 
-/// \brief Determine whether this expression is an implicit C++ 'this'.
-static bool isImplicitThis(const Expr *E) {
-  // Strip away parentheses and casts we don't care about.
-  while (true) {
-    if (const ParenExpr *Paren = dyn_cast<ParenExpr>(E)) {
-      E = Paren->getSubExpr();
-      continue;
-    }
-    
-    if (const ImplicitCastExpr *ICE = dyn_cast<ImplicitCastExpr>(E)) {
-      if (ICE->getCastKind() == CK_NoOp ||
-          ICE->getCastKind() == CK_LValueToRValue ||
-          ICE->getCastKind() == CK_DerivedToBase || 
-          ICE->getCastKind() == CK_UncheckedDerivedToBase) {
-        E = ICE->getSubExpr();
-        continue;
-      }
-    }
-    
-    if (const UnaryOperator* UnOp = dyn_cast<UnaryOperator>(E)) {
-      if (UnOp->getOpcode() == UO_Extension) {
-        E = UnOp->getSubExpr();
-        continue;
-      }
-    }
-    
-    break;
-  }
-  
-  if (const CXXThisExpr *This = dyn_cast<CXXThisExpr>(E))
-    return This->isImplicit();
-  
-  return false;
-}
-
 bool CXXDependentScopeMemberExpr::isImplicitAccess() const {
   if (Base == 0)
     return true;
   
-  return isImplicitThis(cast<Expr>(Base));
+  return cast<Expr>(Base)->isImplicitCXXThis();
 }
 
 UnresolvedMemberExpr::UnresolvedMemberExpr(ASTContext &C, 
@@ -851,7 +816,7 @@ bool UnresolvedMemberExpr::isImplicitAccess() const {
   if (Base == 0)
     return true;
   
-  return isImplicitThis(cast<Expr>(Base));
+  return cast<Expr>(Base)->isImplicitCXXThis();
 }
 
 UnresolvedMemberExpr *
diff --git a/test/Index/annotate-nested-name-specifier.cpp b/test/Index/annotate-nested-name-specifier.cpp
index b315b7eb19..b7570d97e1 100644
--- a/test/Index/annotate-nested-name-specifier.cpp
+++ b/test/Index/annotate-nested-name-specifier.cpp
@@ -126,7 +126,18 @@ struct X7 {
   typedef outer_alias::inner::apply_meta<T_type, U_type::template apply> type;
 };
 
-// RUN: c-index-test -test-annotate-tokens=%s:13:1:128:1 %s | FileCheck %s
+struct X8 {
+  void f();
+};
+
+struct X9 : X8 {
+  typedef X8 inherited;
+  void f() { 
+    inherited::f();
+  }
+};
+
+// RUN: c-index-test -test-annotate-tokens=%s:13:1:137:1 %s | FileCheck %s
 
 // CHECK: Keyword: "using" [14:1 - 14:6] UsingDeclaration=vector[4:12]
 // CHECK: Identifier: "outer_alias" [14:7 - 14:18] NamespaceRef=outer_alias:10:11
@@ -449,3 +460,7 @@ struct X7 {
 // CHECK: Punctuation: ">" [126:72 - 126:73] TypedefDecl=type:126:74 (Definition)
 // CHECK: Identifier: "type" [126:74 - 126:78] TypedefDecl=type:126:74 (Definition)
 
+// Member access expressions
+// CHECK: Identifier: "inherited" [136:5 - 136:14] TypeRef=inherited:134:14
+// CHECK: Punctuation: "::" [136:14 - 136:16] MemberRefExpr=f:130:8
+// CHECK: Identifier: "f" [136:16 - 136:17] MemberRefExpr=f:130:8
diff --git a/tools/libclang/CIndex.cpp b/tools/libclang/CIndex.cpp
index d29e459827..21251e09b1 100644
--- a/tools/libclang/CIndex.cpp
+++ b/tools/libclang/CIndex.cpp
@@ -1979,12 +1979,8 @@ void EnqueueVisitor::VisitMemberExpr(MemberExpr *M) {
   // visit it.
   // FIXME: If we ever want to show these implicit accesses, this will be
   // unfortunate. However, clang_getCursor() relies on this behavior.
-  if (CXXThisExpr *This
-            = llvm::dyn_cast<CXXThisExpr>(M->getBase()->IgnoreParenImpCasts()))
-    if (This->isImplicit())
-      return;
-  
-  AddStmt(M->getBase());
+  if (!M->isImplicitAccess())
+    AddStmt(M->getBase());
 }
 void EnqueueVisitor::VisitObjCEncodeExpr(ObjCEncodeExpr *E) {
   AddTypeLoc(E->getEncodedTypeSourceInfo());