[cfe-dev] libclang and MemberRefExpr

[Douglas Gregor]

On Jun 15, 2011, at 4:38 AM, Erik Verbruggen wrote:

> Attached is a set of patches against svn trunk 133049.
> 
> 0001: stores the location of the brackets for calls to array subscription expressions when the caller is a C++ operator. I tried to make the least intrusive patch as possible, but I am not sure if there are more cases where the DeclRefExpr should store a DeclarationNameLoc then only the overloaded array subscription. I send it to the list previously, but did not receive feedback yet.

This looks pretty good, but please feed this information through template instantiation (in lib/Sema/TreeTransform.h, where we end up eventually calling CreateOverloadedArraySubscriptExpr).

> 0002: the patch to add clang_getCursorReferenceNameRange, including a change to c-index-test which will print "SingleRefName=.." when the flag WantSinglePiece is used, and "RefName=..." for the separate pieces.

Cool. A few comments here:

+enum CXCursor_RefNameFlags {
+  /**
+   * \brief Include the nested-name-specifier, e.g. Foo:: in x.Foo::y, in the
+   * range.
+   */
+  CXCursor_WantQualifier = 0x1,
+  

I find the CXCursor_ prefix here to be weird. How about the enum be named CXNameRefFlags and the enumerators be named CXNameRange_*?

+CXSourceRange clang_getCursorReferenceNameRange(CXCursor C, unsigned NameFlags,
+                                                unsigned PieceIndex) {
+  SourceRange R;
+  const bool WantQualifier = NameFlags & CXCursor_WantQualifier;
+  const bool WantTemplateArgs = NameFlags & CXCursor_WantTemplateArgs;
+  const bool WantSinglePiece = NameFlags & CXCursor_WantSinglePiece;
+  
+  switch (C.kind) {
+  case CXCursor_MemberRefExpr:
+    if (MemberExpr *E = dyn_cast<MemberExpr>(getCursorExpr(C))) {
+      DeclarationNameInfo NI = E->getMemberNameInfo();
+      DeclarationName::NameKind Kind = NI.getName().getNameKind();
+      if (Kind == DeclarationName::Identifier) {
+        if (PieceIndex == 0) {
+          R = NI.getLoc();
+          if (E->hasQualifier() && WantQualifier)
+            R.setBegin(E->getQualifierLoc().getBeginLoc());
+        }
+      } else if (Kind == DeclarationName::CXXOperatorName) {
+        if (WantSinglePiece) {
+          if (PieceIndex == 0) {
+            if (E->hasQualifier() && WantQualifier)
+              R.setBegin(E->getQualifierLoc().getBeginLoc());
+            else
+              R.setBegin(E->getMemberLoc());
+            R.setEnd(NI.getCXXOperatorNameRange().getEnd());
+          }
+        } else {
+          if (PieceIndex == 0) {
+            R = NI.getLoc();
+            if (E->hasQualifier() && WantQualifier)
+              R.setBegin(E->getQualifierLoc().getBeginLoc());
+          } else if (PieceIndex == 1) {
+            R = SourceLocation::getFromRawEncoding(NI.getInfo().CXXOperatorName.BeginOpNameLoc);
+          } else if (PieceIndex == 2) {
+            R = SourceLocation::getFromRawEncoding(NI.getInfo().CXXOperatorName.EndOpNameLoc);
+          }
+        }
+      }
+    } 
+    break;

This looks good, but it's really repetitive. How about abstracting out the functionality that gets the appropriate piece of a possibly-qualified, possibly-template-argumented name? Passing through the NestedNameSpecifierLoc, DeclarationNameInfo, and explicit template arguments should do for most of these expression types.

And, once that's abstracted, it becomes easy to add all of the other kinds that have this kind of information, e.g., function declarations, dependent-scoped member access expressions, and so on.

+    
+  default: 
+    break;

It seems like we can have a fallback here, e.g., if we have a NamedDecl with a non-empty name, then the source range covers the token referenced by NamedDecl::getLocation().

@@ -288,6 +303,31 @@ static void PrintCursor(CXTranslationUnit TU, CXCursor Cursor) {
       if (clang_isFileMultipleIncludeGuarded(TU, File))
         printf("  [multi-include guarded]");
     }
+    
+    if (Cursor.kind == CXCursor_MemberRefExpr
+        || Cursor.kind == CXCursor_DeclRefExpr
+        || Cursor.kind == CXCursor_CallExpr) {

We'll probably want to do this for all cursors, to get better coverage. However, I suggest adding some smarts so that when there's a trivial 1-piece range, we emit output that's identical to what we emit today. We should only emit more interesting output when there are multiple pieces or there is a qualifier/explicit template argument list, so not *all* of the tests have to change :)

If the user requests individual pieces and also wants the qualifier and/or explicit template arguments, should the qualifier and the template-argument-list could as separate "pieces"?

> 0003: a test-case for clang_getCursorReferenceNameRange.
> 0004: fixes for the other test-cases.

Very cool. Thanks for working on this!

	- Doug