[libclang] Fix cursors for arguments of Subscript and Call operators

[nik]

Summary

The DeclRefExpr of CXXOperatorCallExpr refering to the custom operator
is visited before the arguments to the operator call. For the Call and
Subscript operator the range of this DeclRefExpr includes the whole call
expression, so that all tokens in that range were mapped to the operator
function, even the tokens of the arguments.

Fix this by ensuring that this particular DeclRefExpr is visited last.

Fixes PR25775.

[yvvan]

tools/libclang/CIndex.cpp

6445 ↗

(On Diff #124339)

4 spaces instead of 2

6535 ↗

(On Diff #124339)

missing {}

6666 ↗

(On Diff #124339)

4 spaces indentation instead of 2

6860 ↗

(On Diff #124339)

cover with {}

6888 ↗

(On Diff #124339)

4 spaces indentation instead of 2

6892 ↗

(On Diff #124339)

same

[nik]

tools/libclang/CIndex.cpp

6888 ↗

(On Diff #124339)

That's not me, but clang-format, so I guess it's fine as is.

6892 ↗

(On Diff #124339)

That's not me, but clang-format, so I guess it's fine as is.

[nik]

Addressed coding style issues.

[yvvan]

tools/libclang/CIndex.cpp

6888 ↗

(On Diff #124339)

oh, you're right. it's ok here and in the next one

[nik]

Ping

[nik]

Ping.

[nik]

New year, new hope - ping :)

[nik]

Ping...

[nik]

Ping!

[nik]

Ping...

[nik]

This one still applies and tests pass.

Please review.

[nik]

Ping.

[yvvan]

Tests run fine but the solution feels like a workaround.

Nevertheless if we are sure that extending the subscript/call operator range does not work properly or breaks too many other things than it's probably fine to have this workaround.

[yvvan]

Let's proceed with this one. I really see that it's going to be useful.

cfe/trunk/test/Index/annotate-operator-call-expr.cpp

+struct Foo {
+  int operator[](int key);
+  int operator()(int key = 2);
+};
+
+void testFoo(Foo foo, int index) {
+  foo();
+  foo(index);
+
+  foo[index];
+  foo[index + index];
+
+  foo[foo[index]];
+  foo[foo() + foo[index]];
+  foo[foo(index) + foo[index]];
+}
+
+// RUN: c-index-test -test-annotate-tokens=%s:7:1:7:100 %s -std=c++11 -Wno-unused-value | FileCheck %s -check-prefix=CHECK1
+// CHECK1: Identifier: "foo" [7:3 - 7:6] DeclRefExpr=foo:6:18
+// CHECK1: Punctuation: "(" [7:6 - 7:7] DeclRefExpr=operator():3:7 RefName=[7:6 - 7:7] RefName=[7:7 - 7:8]
+// CHECK1: Punctuation: ")" [7:7 - 7:8] DeclRefExpr=operator():3:7 RefName=[7:6 - 7:7] RefName=[7:7 - 7:8]
+// CHECK1: Punctuation: ";" [7:8 - 7:9] CompoundStmt=
+
+// RUN: c-index-test -test-annotate-tokens=%s:8:1:8:100 %s -std=c++11 -Wno-unused-value | FileCheck %s -check-prefix=CHECK2
+// CHECK2: Punctuation: "(" [8:6 - 8:7] DeclRefExpr=operator():3:7 RefName=[8:6 - 8:7] RefName=[8:12 - 8:13]
+// CHECK2: Identifier: "index" [8:7 - 8:12] DeclRefExpr=index:6:27
+// CHECK2: Punctuation: ")" [8:12 - 8:13] DeclRefExpr=operator():3:7 RefName=[8:6 - 8:7] RefName=[8:12 - 8:13]
+// CHECK2: Punctuation: ";" [8:13 - 8:14] CompoundStmt=
+
+// RUN: c-index-test -test-annotate-tokens=%s:10:1:10:100 %s -std=c++11 -Wno-unused-value | FileCheck %s -check-prefix=CHECK3
+// CHECK3: Identifier: "foo" [10:3 - 10:6] DeclRefExpr=foo:6:18
+// CHECK3: Punctuation: "[" [10:6 - 10:7] DeclRefExpr=operator[]:2:7 RefName=[10:6 - 10:7] RefName=[10:12 - 10:13]
+// CHECK3: Identifier: "index" [10:7 - 10:12] DeclRefExpr=index:6:27
+// CHECK3: Punctuation: "]" [10:12 - 10:13] DeclRefExpr=operator[]:2:7 RefName=[10:6 - 10:7] RefName=[10:12 - 10:13]
+// CHECK3: Punctuation: ";" [10:13 - 10:14] CompoundStmt=
+
+// RUN: c-index-test -test-annotate-tokens=%s:11:1:11:100 %s -std=c++11 -Wno-unused-value | FileCheck %s -check-prefix=CHECK4
+// CHECK4: Identifier: "foo" [11:3 - 11:6] DeclRefExpr=foo:6:18
+// CHECK4: Punctuation: "[" [11:6 - 11:7] DeclRefExpr=operator[]:2:7 RefName=[11:6 - 11:7] RefName=[11:20 - 11:21]
+// CHECK4: Identifier: "index" [11:7 - 11:12] DeclRefExpr=index:6:27
+// CHECK4: Punctuation: "+" [11:13 - 11:14] BinaryOperator=
+// CHECK4: Identifier: "index" [11:15 - 11:20] DeclRefExpr=index:6:27
+// CHECK4: Punctuation: "]" [11:20 - 11:21] DeclRefExpr=operator[]:2:7 RefName=[11:6 - 11:7] RefName=[11:20 - 11:21]
+// CHECK4: Punctuation: ";" [11:21 - 11:22] CompoundStmt=
+
+// RUN: c-index-test -test-annotate-tokens=%s:13:1:13:100 %s -std=c++11 -Wno-unused-value | FileCheck %s -check-prefix=CHECK5
+// CHECK5: Identifier: "foo" [13:3 - 13:6] DeclRefExpr=foo:6:18
+// CHECK5: Punctuation: "[" [13:6 - 13:7] DeclRefExpr=operator[]:2:7 RefName=[13:6 - 13:7] RefName=[13:17 - 13:18]
+// CHECK5: Identifier: "foo" [13:7 - 13:10] DeclRefExpr=foo:6:18
+// CHECK5: Punctuation: "[" [13:10 - 13:11] DeclRefExpr=operator[]:2:7 RefName=[13:10 - 13:11] RefName=[13:16 - 13:17]
+// CHECK5: Identifier: "index" [13:11 - 13:16] DeclRefExpr=index:6:27
+// CHECK5: Punctuation: "]" [13:16 - 13:17] DeclRefExpr=operator[]:2:7 RefName=[13:10 - 13:11] RefName=[13:16 - 13:17]
+// CHECK5: Punctuation: "]" [13:17 - 13:18] DeclRefExpr=operator[]:2:7 RefName=[13:6 - 13:7] RefName=[13:17 - 13:18]
+// CHECK5: Punctuation: ";" [13:18 - 13:19] CompoundStmt=
+
+// RUN: c-index-test -test-annotate-tokens=%s:14:1:14:100 %s -std=c++11 -Wno-unused-value | FileCheck %s -check-prefix=CHECK6
+// CHECK6: Identifier: "foo" [14:3 - 14:6] DeclRefExpr=foo:6:18
+// CHECK6: Punctuation: "[" [14:6 - 14:7] DeclRefExpr=operator[]:2:7 RefName=[14:6 - 14:7] RefName=[14:25 - 14:26]
+// CHECK6: Identifier: "foo" [14:7 - 14:10] DeclRefExpr=foo:6:18
+// CHECK6: Punctuation: "(" [14:10 - 14:11] DeclRefExpr=operator():3:7 RefName=[14:10 - 14:11] RefName=[14:11 - 14:12]
+// CHECK6: Punctuation: ")" [14:11 - 14:12] DeclRefExpr=operator():3:7 RefName=[14:10 - 14:11] RefName=[14:11 - 14:12]
+// CHECK6: Punctuation: "+" [14:13 - 14:14] BinaryOperator=
+// CHECK6: Identifier: "foo" [14:15 - 14:18] DeclRefExpr=foo:6:18
+// CHECK6: Punctuation: "[" [14:18 - 14:19] DeclRefExpr=operator[]:2:7 RefName=[14:18 - 14:19] RefName=[14:24 - 14:25]
+// CHECK6: Identifier: "index" [14:19 - 14:24] DeclRefExpr=operator[]:2:7 RefName=[14:6 - 14:7] RefName=[14:25 - 14:26]
+// CHECK6: Punctuation: "]" [14:24 - 14:25] DeclRefExpr=operator[]:2:7 RefName=[14:18 - 14:19] RefName=[14:24 - 14:25]
+// CHECK6: Punctuation: "]" [14:25 - 14:26] DeclRefExpr=operator[]:2:7 RefName=[14:6 - 14:7] RefName=[14:25 - 14:26]
+// CHECK6: Punctuation: ";" [14:26 - 14:27] CompoundStmt=
+
+// RUN: c-index-test -test-annotate-tokens=%s:15:1:15:100 %s -std=c++11 -Wno-unused-value | FileCheck %s -check-prefix=CHECK7
+// CHECK7: Identifier: "foo" [15:3 - 15:6] DeclRefExpr=foo:6:18
+// CHECK7: Punctuation: "[" [15:6 - 15:7] DeclRefExpr=operator[]:2:7 RefName=[15:6 - 15:7] RefName=[15:30 - 15:31]
+// CHECK7: Identifier: "foo" [15:7 - 15:10] DeclRefExpr=foo:6:18
+// CHECK7: Punctuation: "(" [15:10 - 15:11] DeclRefExpr=operator():3:7 RefName=[15:10 - 15:11] RefName=[15:16 - 15:17]
+// CHECK7: Identifier: "index" [15:11 - 15:16] DeclRefExpr=index:6:27
+// CHECK7: Punctuation: ")" [15:16 - 15:17] DeclRefExpr=operator():3:7 RefName=[15:10 - 15:11] RefName=[15:16 - 15:17]
+// CHECK7: Punctuation: "+" [15:18 - 15:19] BinaryOperator=
+// CHECK7: Identifier: "foo" [15:20 - 15:23] DeclRefExpr=foo:6:18
+// CHECK7: Punctuation: "[" [15:23 - 15:24] DeclRefExpr=operator[]:2:7 RefName=[15:23 - 15:24] RefName=[15:29 - 15:30]
+// CHECK7: Identifier: "index" [15:24 - 15:29] DeclRefExpr=index:6:27
+// CHECK7: Punctuation: "]" [15:29 - 15:30] DeclRefExpr=operator[]:2:7 RefName=[15:23 - 15:24] RefName=[15:29 - 15:30]
+// CHECK7: Punctuation: "]" [15:30 - 15:31] DeclRefExpr=operator[]:2:7 RefName=[15:6 - 15:7] RefName=[15:30 - 15:31]
+// CHECK7: Punctuation: ";" [15:31 - 15:32] CompoundStmt=
+

cfe/trunk/tools/libclang/CIndex.cpp

class AnnotateTokensWorker {
   CXCursor *Cursors;
   unsigned NumTokens;
   unsigned TokIdx;
   unsigned PreprocessingTokIdx;
   CursorVisitor AnnotateVis;
   SourceManager &SrcMgr;
   bool HasContextSensitiveKeywords;
 
+  struct PostChildrenAction {
+    CXCursor cursor;
+    enum Action { Invalid, Ignore, Postpone } action;
+  };
+  using PostChildrenActions = SmallVector<PostChildrenAction, 0>;
+
   struct PostChildrenInfo {
     CXCursor Cursor;
     SourceRange CursorRange;
     unsigned BeforeReachingCursorIdx;
     unsigned BeforeChildrenTokenIdx;
+    PostChildrenActions ChildActions;
   };
   SmallVector<PostChildrenInfo, 8> PostChildrenInfos;
 
   CXToken &getTok(unsigned Idx) {
     assert(Idx < NumTokens);
     return Tokens[Idx];
   }
   const CXToken &getTok(unsigned Idx) const {
: Tokens(tokens), Cursors(cursors),
                   RegionOfInterest,
                   /*VisitDeclsOnly=*/false,
                   AnnotateTokensPostChildrenVisitor),
       SrcMgr(cxtu::getASTUnit(TU)->getSourceManager()),
       HasContextSensitiveKeywords(false) { }
 
   void VisitChildren(CXCursor C) { AnnotateVis.VisitChildren(C); }
   enum CXChildVisitResult Visit(CXCursor cursor, CXCursor parent);
+  bool IsIgnoredChildCursor(CXCursor cursor) const;
+  PostChildrenActions DetermineChildActions(CXCursor Cursor) const;
+
   bool postVisitChildren(CXCursor cursor);
+  void HandlePostPonedChildCursors(const PostChildrenInfo &Info);
+  void HandlePostPonedChildCursor(CXCursor Cursor, unsigned StartTokenIndex);
+
   void AnnotateTokens();
   
   /// Determine whether the annotator saw any cursors that have 
   /// context-sensitive keywords.
   bool hasContextSensitiveKeywords() const {
     return HasContextSensitiveKeywords;
   }
 
   ~AnnotateTokensWorker() {
     assert(PostChildrenInfos.empty());
   }
 };
 }
 
 void AnnotateTokensWorker::AnnotateTokens() {
   // Walk the AST within the region of interest, annotating tokens
   // along the way.
   AnnotateVis.visitFileRegion();
 }
 
+bool AnnotateTokensWorker::IsIgnoredChildCursor(CXCursor cursor) const {
+  if (PostChildrenInfos.empty())
+    return false;
+
+  for (const auto &ChildAction : PostChildrenInfos.back().ChildActions) {
+    if (ChildAction.cursor == cursor &&
+        ChildAction.action == PostChildrenAction::Ignore) {
+      return true;
+    }
+  }
+
+  return false;
+}
+
+const CXXOperatorCallExpr *GetSubscriptOrCallOperator(CXCursor Cursor) {
+  if (!clang_isExpression(Cursor.kind))
+    return nullptr;
+
+  const Expr *E = getCursorExpr(Cursor);
+  if (const auto *OCE = dyn_cast<CXXOperatorCallExpr>(E)) {
+    const OverloadedOperatorKind Kind = OCE->getOperator();
+    if (Kind == OO_Call || Kind == OO_Subscript)
+      return OCE;
+  }
+
+  return nullptr;
+}
+
+AnnotateTokensWorker::PostChildrenActions
+AnnotateTokensWorker::DetermineChildActions(CXCursor Cursor) const {
+  PostChildrenActions actions;
+
+  // The DeclRefExpr of CXXOperatorCallExpr refering to the custom operator is
+  // visited before the arguments to the operator call. For the Call and
+  // Subscript operator the range of this DeclRefExpr includes the whole call
+  // expression, so that all tokens in that range would be mapped to the
+  // operator function, including the tokens of the arguments. To avoid that,
+  // ensure to visit this DeclRefExpr as last node.
+  if (const auto *OCE = GetSubscriptOrCallOperator(Cursor)) {
+    const Expr *Callee = OCE->getCallee();
+    if (const ImplicitCastExpr *ICE = dyn_cast<ImplicitCastExpr>(Callee)) {
+      const Expr *SubExpr = ICE->getSubExpr();
+      if (const DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(SubExpr)) {
+        const Decl *parentDecl = getCursorParentDecl(Cursor);
+        CXTranslationUnit TU = clang_Cursor_getTranslationUnit(Cursor);
+
+        // Visit the DeclRefExpr as last.
+        CXCursor cxChild = MakeCXCursor(DRE, parentDecl, TU);
+        actions.push_back({cxChild, PostChildrenAction::Postpone});
+
+        // The parent of the DeclRefExpr, an ImplicitCastExpr, has an equally
+        // wide range as the DeclRefExpr. We can skip visiting this entirely.
+        cxChild = MakeCXCursor(ICE, parentDecl, TU);
+        actions.push_back({cxChild, PostChildrenAction::Ignore});
+      }
+    }
+  }
+
+  return actions;
+}
+
 static inline void updateCursorAnnotation(CXCursor &Cursor,
                                           const CXCursor &updateC) {
   if (clang_isInvalid(updateC.kind) || !clang_isInvalid(Cursor.kind))
     return;
   Cursor = updateC;
 }
 
 /// It annotates and advances tokens with a cursor until the comparison
bool AnnotateTokensWorker::annotateAndAdvanceFunctionMacroTokens(
   return true;
 }
 
 enum CXChildVisitResult
 AnnotateTokensWorker::Visit(CXCursor cursor, CXCursor parent) {  
   SourceRange cursorRange = getRawCursorExtent(cursor);
   if (cursorRange.isInvalid())
     return CXChildVisit_Recurse;
-      
+  if (IsIgnoredChildCursor(cursor))
+    return CXChildVisit_Continue;
+
   if (!HasContextSensitiveKeywords) {
     // Objective-C properties can have context-sensitive keywords.
     if (cursor.kind == CXCursor_ObjCPropertyDecl) {
       if (const ObjCPropertyDecl *Property
                   = dyn_cast_or_null<ObjCPropertyDecl>(getCursorDecl(cursor)))
         HasContextSensitiveKeywords = Property->getPropertyAttributesAsWritten() != 0;
     }
     // Objective-C methods can have context-sensitive keywords.
AnnotateTokensWorker::Visit(CXCursor cursor, CXCursor parent) {
   // Note that we don't call VisitChildren here to avoid traversing statements
   // code-recursively which can blow the stack.
 
   PostChildrenInfo Info;
   Info.Cursor = cursor;
   Info.CursorRange = cursorRange;
   Info.BeforeReachingCursorIdx = BeforeReachingCursorIdx;
   Info.BeforeChildrenTokenIdx = NextToken();
+  Info.ChildActions = DetermineChildActions(cursor);
   PostChildrenInfos.push_back(Info);
 
   return CXChildVisit_Recurse;
 }
 
 bool AnnotateTokensWorker::postVisitChildren(CXCursor cursor) {
   if (PostChildrenInfos.empty())
     return false;
   const PostChildrenInfo &Info = PostChildrenInfos.back();
   if (!clang_equalCursors(Info.Cursor, cursor))
     return false;
 
+  HandlePostPonedChildCursors(Info);
+
   const unsigned BeforeChildren = Info.BeforeChildrenTokenIdx;
   const unsigned AfterChildren = NextToken();
   SourceRange cursorRange = Info.CursorRange;
 
   // Scan the tokens that are at the end of the cursor, but are not captured
   // but the child cursors.
   annotateAndAdvanceTokens(cursor, RangeOverlap, cursorRange);
 
bool AnnotateTokensWorker::postVisitChildren(CXCursor cursor) {
   // encountered the attribute cursor.
   if (clang_isAttribute(cursor.kind))
     TokIdx = Info.BeforeReachingCursorIdx;
 
   PostChildrenInfos.pop_back();
   return false;
 }
 
+void AnnotateTokensWorker::HandlePostPonedChildCursors(
+    const PostChildrenInfo &Info) {
+  for (const auto &ChildAction : Info.ChildActions) {
+    if (ChildAction.action == PostChildrenAction::Postpone) {
+      HandlePostPonedChildCursor(ChildAction.cursor,
+                                 Info.BeforeChildrenTokenIdx);
+    }
+  }
+}
+
+void AnnotateTokensWorker::HandlePostPonedChildCursor(
+    CXCursor Cursor, unsigned StartTokenIndex) {
+  const auto flags = CXNameRange_WantQualifier | CXNameRange_WantQualifier;
+  unsigned I = StartTokenIndex;
+
+  // The bracket tokens of a Call or Subscript operator are mapped to
+  // CallExpr/CXXOperatorCallExpr because we skipped visiting the corresponding
+  // DeclRefExpr. Remap these tokens to the DeclRefExpr cursors.
+  for (unsigned RefNameRangeNr = 0; I < NumTokens; RefNameRangeNr++) {
+    const CXSourceRange CXRefNameRange =
+        clang_getCursorReferenceNameRange(Cursor, flags, RefNameRangeNr);
+    if (clang_Range_isNull(CXRefNameRange))
+      break; // All ranges handled.
+
+    SourceRange RefNameRange = cxloc::translateCXSourceRange(CXRefNameRange);
+    while (I < NumTokens) {
+      const SourceLocation TokenLocation = GetTokenLoc(I);
+      if (!TokenLocation.isValid())
+        break;
+
+      // Adapt the end range, because LocationCompare() reports
+      // RangeOverlap even for the not-inclusive end location.
+      const SourceLocation fixedEnd =
+          RefNameRange.getEnd().getLocWithOffset(-1);
+      RefNameRange = SourceRange(RefNameRange.getBegin(), fixedEnd);
+
+      const RangeComparisonResult ComparisonResult =
+          LocationCompare(SrcMgr, TokenLocation, RefNameRange);
+
+      if (ComparisonResult == RangeOverlap) {
+        Cursors[I++] = Cursor;
+      } else if (ComparisonResult == RangeBefore) {
+        ++I; // Not relevant token, check next one.
+      } else if (ComparisonResult == RangeAfter) {
+        break; // All tokens updated for current range, check next.
+      }
+    }
+  }
+}
+
 static enum CXChildVisitResult AnnotateTokensVisitor(CXCursor cursor,
                                                      CXCursor parent,
                                                      CXClientData client_data) {
   return static_cast<AnnotateTokensWorker*>(client_data)->Visit(cursor, parent);
 }
 
 static bool AnnotateTokensPostChildrenVisitor(CXCursor cursor,
                                               CXClientData client_data) {