[libclang, bindings]: add spelling location

[frutiger]

Summary

• Add a 'Location' class that represents the four properties of a physical location
• Enhance 'SourceLocation' to provide 'expansion' and 'spelling' locations, maintaining backwards compatibility with existing code by forwarding the four properties to 'expansion'.
• Update the implementation to use 'clang_getExpansionLocation' instead of the deprecated 'clang_getInstantiationLocation', which has been present since 2011.
• Update the implementation of 'clang_getSpellingLocation' to actually obtain spelling location instead of file location.
• Update test cases to account for changes to the result of 'clang_getSpellingLocation'.

Note: this commit is a reapplication of r316278 along with fixes to the
test cases.

[frutiger]

Add context to the patch.

[frutiger]

I would very much appreciate some guidance on whether or not this kind of a change in behaviour for clang_getSpellingLocation is an acceptable thing to do.

[compnerd]

I think that this is reasonable given that it is addressing a long standing issue. CC'ing @akyrtzi for his opinion as well.

[akyrtzi]

CC'ed @arphaman.

[jklaehn]

test/Index/annotate-tokens.c

226

Those look like an improvement to me since the MemberRefs of y and z no longer refer to the same line?

test/Index/c-index-getCursor-test.m

167

This does not seem to refer to a valid location? (line 2 only contains a comment)

[frutiger]

test/Index/c-index-getCursor-test.m

167

I don't really understand this output. What is -test-file-scan supposed to show?

[jklaehn]

test/Index/c-index-getCursor-test.m

167

Given this argument c-index-test will use clang_getCursor to iterate through all cursors in the file (by calling it on each character location and checking if it is equal to the previous cursor).
The numbers in brackets (e.g. [58:4 - 58:8]) indicate the locations for which clang_getCursor returned the same cursor. VarDecl=my_var:2:1 (Definition) is the output of PrintCursor. In this case :2:1 is IIUC produced by

Referenced = clang_getCursorReferenced(Cursor);
if (!clang_equalCursors(Referenced, clang_getNullCursor())) {
  if (clang_getCursorKind(Referenced) == CXCursor_OverloadedDeclRef) {
    // [...]
  } else {
    CXSourceLocation Loc = clang_getCursorLocation(Referenced);
    clang_getSpellingLocation(Loc, 0, &line, &column, 0);
    printf(":%d:%d", line, column);
  }

  // [...]
}

So it is affected by the change to clang_getSpellingLocation. I'm not sure what the referenced cursor is in this case. Maybe it would be helpful to also print its kind.

[frutiger]

It looks like the my_var:2:1 refers to the result from clang_getCursorReferenced: https://github.com/llvm-mirror/clang/blob/d454549fce04dfedda6cc2825b66efca94effe3f/tools/c-index-test/c-index-test.c#L709-L711. I'm not sure what a referenced cursor is in this context, but the line and column appear to be related to the referenced cursor. As you say, there's nothing relevant at that location, so I'm not sure why it's returning 2 and 1.

[frutiger]

@jklaehn do you know why the referenced cursor would point to line 2?

bindings/python/clang/cindex.py

         assert isinstance(res, _CXString)
         return conf.lib.clang_getCString(res)
 
+class Location(object):
+    """A Location is a specific kind of source location.  A SourceLocation
+    refers to several kinds of locations (e.g.  spelling location vs. expansion
+    location)."""
+
+    def __init__(self, file, line, column, offset):
+        self._file   = File(file) if file else None
+        self._line   = int(line.value)
+        self._column = int(column.value)
+        self._offset = int(offset.value)
+
+
+    @property
+    def file(self):
+        """Get the file represented by this source location."""
+        return self._file
+
+    @property
+    def line(self):
+        """Get the line represented by this source location."""
+        return self._line
+
+    @property
+    def column(self):
+        """Get the column represented by this source location."""
+        return self._column
+
+    @property
+    def offset(self):
+        """Get the file offset represented by this source location."""
+        return self._offset
 
 class SourceLocation(Structure):
     """
     A SourceLocation represents a particular location within a source file.
     """
     _fields_ = [("ptr_data", c_void_p * 2), ("int_data", c_uint)]
-    _data = None
-
-    def _get_instantiation(self):
-        if self._data is None:
-            f, l, c, o = c_object_p(), c_uint(), c_uint(), c_uint()
-            conf.lib.clang_getInstantiationLocation(self, byref(f), byref(l),
-                    byref(c), byref(o))
-            if f:
-                f = File(f)
-            else:
-                f = None
-            self._data = (f, int(l.value), int(c.value), int(o.value))
-        return self._data
+    _expansion = None
+    _spelling = None
 
     @staticmethod
     def from_position(tu, file, line, column):

         """
         return conf.lib.clang_getLocationForOffset(tu, file, offset)
 
+    @property
+    def expansion(self):
+        """
+        The source location where then entity this object is referring to is
+        expanded.
+        """
+        if not self._expansion:
+            file   = c_object_p()
+            line   = c_uint()
+            column = c_uint()
+            offset = c_uint()
+            conf.lib.clang_getExpansionLocation(self,
+                                                byref(file),
+                                                byref(line),
+                                                byref(column),
+                                                byref(offset))
+
+            self._expansion = Location(file, line, column, offset)
+        return self._expansion
+
+    @property
+    def spelling(self):
+        """
+        The source location where then entity this object is referring to is
+        written.
+        """
+        if not self._spelling:
+            file   = c_object_p()
+            line   = c_uint()
+            column = c_uint()
+            offset = c_uint()
+            conf.lib.clang_getSpellingLocation(self,
+                                               byref(file),
+                                               byref(line),
+                                               byref(column),
+                                               byref(offset))
+
+            self._spelling = Location(file, line, column, offset)
+        return self._spelling
+
     @property
     def file(self):
-        """Get the file represented by this source location."""
-        return self._get_instantiation()[0]
+        """Get the file represented by this source location.
+
+        DEPRECATED: use expansion.file."""
+        return self.expansion.file
 
     @property
     def line(self):
-        """Get the line represented by this source location."""
-        return self._get_instantiation()[1]
+        """Get the line represented by this source location.
+
+        DEPRECATED: use expansion.line."""
+        return self.expansion.line
 
     @property
     def column(self):
-        """Get the column represented by this source location."""
-        return self._get_instantiation()[2]
+        """Get the column represented by this source location.
+
+        DEPRECATED: use expansion.column."""
+        return self.expansion.column
 
     @property
     def offset(self):
-        """Get the file offset represented by this source location."""
-        return self._get_instantiation()[3]
+        """Get the file offset represented by this source location.
+
+        DEPRECATED: use expansion.offset."""
+        return self.expansion.offset
 
     def __eq__(self, other):
         return conf.lib.clang_equalLocations(self, other)

     @property
     def location(self):
         """
-        Return the source location (the starting character) of the entity
-        pointed at by the cursor.
+        Return the source locations of the entity pointed at by the cursor.
         """
         if not hasattr(self, '_loc'):
             self._loc = conf.lib.clang_getCursorLocation(self)

   ("clang_getInclusions",
    [TranslationUnit, callbacks['translation_unit_includes'], py_object]),
 
-  ("clang_getInstantiationLocation",
+  ("clang_getExpansionLocation",
+   [SourceLocation, POINTER(c_object_p), POINTER(c_uint), POINTER(c_uint),
+    POINTER(c_uint)]),
+
+  ("clang_getSpellingLocation",
    [SourceLocation, POINTER(c_object_p), POINTER(c_uint), POINTER(c_uint),
     POINTER(c_uint)]),
 

     'FixIt',
     'Index',
     'LinkageKind',
+    'Location',
     'SourceLocation',
     'SourceRange',
     'TLSKind',

bindings/python/tests/cindex/test_location.py

     location3 = SourceLocation.from_position(tu, file, 1, 6)
     range3 = SourceRange.from_locations(location1, location3)
     assert range1 != range3
+
+def test_spelling_location():
+    tu = get_tu('''#define ONE int one
+ONE;''')
+    one = get_cursor(tu, 'one')
+    assert one.location.spelling.line == 1
+    assert one.location.expansion.line == 2

test/Index/annotate-tokens.c

 // CHECK-RANGE1: Identifier: "Foo" [52:11 - 52:14] VarDecl=Foo:52:11 (Definition)
 // CHECK-RANGE1: Punctuation: "," [52:14 - 52:15]
 // CHECK-RANGE1: Punctuation: "." [53:5 - 53:6] UnexposedExpr=
-// CHECK-RANGE1: Identifier: "x" [53:6 - 53:7] MemberRef=x:52:1
+// CHECK-RANGE1: Identifier: "x" [53:6 - 53:7] MemberRef=x:39:9
 // CHECK-RANGE1: Punctuation: "=" [53:8 - 53:9] UnexposedExpr=
 // CHECK-RANGE1: Literal: "0" [53:10 - 53:11] IntegerLiteral=
 // CHECK-RANGE1: Punctuation: "," [53:11 - 53:12] InitListExpr=
 // CHECK-RANGE1: Punctuation: "." [54:5 - 54:6] UnexposedExpr=
-// CHECK-RANGE1: Identifier: "y" [54:6 - 54:7] MemberRef=y:52:1
+// CHECK-RANGE1: Identifier: "y" [54:6 - 54:7] MemberRef=y:40:9
 // CHECK-RANGE1: Punctuation: "=" [54:8 - 54:9] UnexposedExpr=
 // CHECK-RANGE1: Literal: "1" [54:10 - 54:11] IntegerLiteral=
 // CHECK-RANGE1: Punctuation: "," [54:11 - 54:12] InitListExpr=
 
 // RUN: c-index-test -test-annotate-tokens=%s:54:1:70:1 %s | FileCheck %s -check-prefix=CHECK-RANGE2
 // CHECK-RANGE2: Punctuation: "." [54:5 - 54:6] UnexposedExpr=
-// CHECK-RANGE2: Identifier: "y" [54:6 - 54:7] MemberRef=y:52:1
+// CHECK-RANGE2: Identifier: "y" [54:6 - 54:7] MemberRef=y:40:9
 // CHECK-RANGE2: Punctuation: "=" [54:8 - 54:9] UnexposedExpr=
 // CHECK-RANGE2: Literal: "1" [54:10 - 54:11] IntegerLiteral=
 // CHECK-RANGE2: Punctuation: "," [54:11 - 54:12] InitListExpr=
 // CHECK-RANGE2: Punctuation: "." [55:5 - 55:6] UnexposedExpr=
-// CHECK-RANGE2: Identifier: "z" [55:6 - 55:7] MemberRef=z:52:1
+// CHECK-RANGE2: Identifier: "z" [55:6 - 55:7] MemberRef=z:41:9
 // CHECK-RANGE2: Punctuation: "=" [55:8 - 55:9] UnexposedExpr=
 // CHECK-RANGE2: Literal: "2" [55:10 - 55:11] IntegerLiteral=
 // CHECK-RANGE2: Punctuation: "," [55:11 - 55:12] InitListExpr=

test/Index/blocks.c

 // CHECK: blocks.c:7:3: DeclStmt= Extent=[7:3 - 7:26]
 // CHECK: blocks.c:7:21: VarDecl=_foo:7:21 (Definition) Extent=[7:3 - 7:25]
 // CHECK: blocks.c:7:17: TypeRef=struct foo:4:8 Extent=[7:17 - 7:20]
-// CHECK: blocks.c:8:11: VarDecl=i:8:11 (Definition) Extent=[8:3 - 8:16]
+// CHECK: blocks.c:8:11: VarDecl=i:8:11 (Definition)
 // CHECK: blocks.c:8:15: IntegerLiteral= Extent=[8:15 - 8:16]
 // CHECK: blocks.c:9:3: CallExpr= Extent=[9:3 - 9:65]
 // CHECK: blocks.c:9:3: BlockExpr= Extent=[9:3 - 9:58]

test/Index/c-index-api-loadTU-test.m

 // CHECK: c-index-api-loadTU-test.m:54:33: UnexposedExpr=bee:47:8 Extent=[54:33 - 54:36]
 // CHECK: c-index-api-loadTU-test.m:54:33: DeclRefExpr=bee:47:8 Extent=[54:33 - 54:36]
 // CHECK: c-index-api-loadTU-test.m:62:12: ObjCInterfaceDecl=TestAttributes:62:12 Extent=[62:1 - 67:5]
-// CHECK: c-index-api-loadTU-test.m:63:15: ObjCIvarDecl=anOutlet:63:15 (Definition) Extent=[63:3 - 63:23]
-// CHECK: c-index-api-loadTU-test.m:63:3: attribute(iboutlet)= Extent=[63:3 - 63:11]
+// CHECK: c-index-api-loadTU-test.m:63:15: ObjCIvarDecl=anOutlet:63:15 (Definition) Extent=[58:18 - 63:23]
+// CHECK: c-index-api-loadTU-test.m:58:33: attribute(iboutlet)= Extent=[58:33 - 63:11]
 // CHECK: c-index-api-loadTU-test.m:63:12: TypeRef=id:0:0 Extent=[63:12 - 63:14]
-// CHECK: c-index-api-loadTU-test.m:64:29: ObjCIvarDecl=anOutletCollection:64:29 (Definition) Extent=[64:3 - 64:47]
-// CHECK: c-index-api-loadTU-test.m:64:3: attribute(iboutletcollection)= [IBOutletCollection=ObjCObjectPointer] Extent=[64:3 - 64:25]
+// CHECK: c-index-api-loadTU-test.m:64:29: ObjCIvarDecl=anOutletCollection:64:29 (Definition) Extent=[59:39 - 64:47]
+// CHECK: c-index-api-loadTU-test.m:59:54: attribute(iboutletcollection)= [IBOutletCollection=ObjCObjectPointer] Extent=[59:54 - 64:25]
 // CHECK: c-index-api-loadTU-test.m:64:26: TypeRef=id:0:0 Extent=[64:26 - 64:28]
 // CHECK: c-index-api-loadTU-test.m:66:14: ObjCInstanceMethodDecl=actionMethod::66:14 Extent=[66:1 - 66:35]
-// CHECK: c-index-api-loadTU-test.m:66:4: attribute(ibaction)= Extent=[66:4 - 66:12]
+// CHECK: c-index-api-loadTU-test.m:60:38: attribute(ibaction)= Extent=[60:38 - 66:12]
 // CHECK: c-index-api-loadTU-test.m:66:31: ParmDecl=arg:66:31 (Definition) Extent=[66:28 - 66:34]
 // CHECK: c-index-api-loadTU-test.m:66:28: TypeRef=id:0:0 Extent=[66:28 - 66:30]
 // CHECK: c-index-api-loadTU-test.m:69:16: StructDecl=X0:69:16 Extent=[69:9 - 69:18]

 // CHECK: c-index-api-loadTU-test.m:73:12: ObjCCategoryDecl=:73:12 Extent=[73:1 - 77:5]
 // CHECK: c-index-api-loadTU-test.m:73:12: ObjCClassRef=TestAttributes:62:12 Extent=[73:12 - 73:26]
 // CHECK: c-index-api-loadTU-test.m:75:32: ObjCPropertyDecl=anotherOutlet:75:32 [retain,] Extent=[75:1 - 75:45]
-// CHECK: c-index-api-loadTU-test.m:75:20: attribute(iboutlet)= Extent=[75:20 - 75:28]
+// CHECK: c-index-api-loadTU-test.m:58:33: attribute(iboutlet)= Extent=[58:33 - 75:28]
 // CHECK: c-index-api-loadTU-test.m:75:29: TypeRef=id:0:0 Extent=[75:29 - 75:31]
 // CHECK: c-index-api-loadTU-test.m:75:32: ObjCInstanceMethodDecl=anotherOutlet:75:32 Extent=[75:32 - 75:45]
 // CHECK: c-index-api-loadTU-test.m:75:32: ObjCInstanceMethodDecl=setAnotherOutlet::75:32 Extent=[75:32 - 75:45]

test/Index/c-index-getCursor-test.m

 // CHECK: [52:37 - 53:2] CompoundStmt=
 // CHECK: [55:9 - 55:26] macro definition=CONCAT
 // CHECK: [57:1 - 57:10] FunctionDecl=f:57:6 (Definition)
-// CHECK: [58:4 - 58:8] VarDecl=my_var:58:8 (Definition)
+// CHECK: [58:4 - 58:8] VarDecl=my_var:2:1 (Definition)
 // CHECK: [58:8 - 58:15] macro expansion=CONCAT:55:9
 // REQUIRES: x86-registered-target

test/Index/get-cursor.cpp

 // CHECK-TEMPLPARAM: 55:23 TypeRef=struct X:3:8 Extent=[55:23 - 55:24] Spelling=struct X ([55:23 - 55:24])
 
 // RUN: c-index-test -cursor-at=%s:66:23 %s | FileCheck -check-prefix=CHECK-TEMPLSPEC %s
-// CHECK-TEMPLSPEC: 66:23 ClassDecl=TC:66:23 (Definition) [Specialization of TC:59:7] Extent=[66:1 - 66:31] Spelling=TC ([66:23 - 66:25])
+// CHECK-TEMPLSPEC: 66:23 ClassDecl=TC:66:23 (Definition) [Specialization of TC:59:7] Extent=[65:30 - 66:31] Spelling=TC ([66:23 - 66:25])
 
 // RUN: c-index-test -cursor-at=%s:69:3 -cursor-at=%s:70:11 -cursor-at=%s:73:6 -cursor-at=%s:74:6 -cursor-at=%s:77:8 -cursor-at=%s:78:8 -cursor-at=%s:79:8 -cursor-at=%s:80:8 -cursor-at=%s:81:8 -cursor-at=%s:82:8 -cursor-at=%s:85:6 -cursor-at=%s:86:6 -cursor-at=%s:87:6 -cursor-at=%s:88:6 -cursor-at=%s:91:5 -cursor-at=%s:92:5 -cursor-at=%s:93:5 -cursor-at=%s:94:5 -cursor-at=%s:95:5 -cursor-at=%s:96:5 -cursor-at=%s:97:5 -cursor-at=%s:98:5 -cursor-at=%s:100:5 -cursor-at=%s:101:5 -cursor-at=%s:104:6 -cursor-at=%s:105:6 -cursor-at=%s:106:6 -cursor-at=%s:107:6 -cursor-at=%s:108:6 -cursor-at=%s:109:6 -cursor-at=%s:110:6 -cursor-at=%s:111:6 -cursor-at=%s:113:6 -cursor-at=%s:114:6 -cursor-at=%s:117:8 -cursor-at=%s:118:8 -cursor-at=%s:120:8 -cursor-at=%s:121:8 -cursor-at=%s:122:8 -cursor-at=%s:123:8 -cursor-at=%s:124:8 -cursor-at=%s:125:8 -cursor-at=%s:128:6 -cursor-at=%s:129:6 -cursor-at=%s:130:6 -cursor-at=%s:132:3 -cursor-at=%s:146:15 -cursor-at=%s:149:6 -cursor-at=%s:150:25 -cursor-at=%s:151:6 -cursor-at=%s:152:6 -cursor-at=%s:153:6 -std=c++11 %s | FileCheck -check-prefix=CHECK-SPELLING %s
 // CHECK-SPELLING: 69:3 CXXConstructor=A:69:3 (default constructor) Extent=[69:3 - 69:6] Spelling=A ([69:3 - 69:4])

test/Index/get-cursor.m

 // CHECK-TRANSPARENT: 139:1 TypeRef=TestTransparent:133:17 (Transparent: enum TestTransparent) Extent=[139:1 - 139:16] Spelling=TestTransparent ([139:1 - 139:16])
 // CHECK-TRANSPARENT: 140:6 TypeRef=enum TestTransparent:133:17 Extent=[140:6 - 140:21] Spelling=enum TestTransparent ([140:6 - 140:21])
 // CHECK-TRANSPARENT: 141:1 TypeRef=NotTransparent:137:30 Extent=[141:1 - 141:15] Spelling=NotTransparent ([141:1 - 141:15])
-// CHECK-TRANSPARENT: 147:1 TypeRef=TokenPaste_t:144:9 Extent=[147:1 - 147:13] Spelling=TokenPaste_t ([147:1 - 147:13])
+// CHECK-TRANSPARENT: 147:1 TypeRef=TokenPaste_t:2:1 Extent=[147:1 - 147:13] Spelling=TokenPaste_t ([147:1 - 147:13])
 // CHECK-TRANSPARENT: 151:1 TypeRef=SomeT:150:17 (Transparent: struct SomeT) Extent=[151:1 - 151:6] Spelling=SomeT ([151:1 - 151:6])
 // CHECK-TRANSPARENT: 155:1 TypeRef=SomeT2:154:18 Extent=[155:1 - 155:7] Spelling=SomeT2 ([155:1 - 155:7])
 
 // RUN: c-index-test -cursor-at=%s:160:12 -cursor-at=%s:161:8 %s | FileCheck -check-prefix=CHECK-EXTERNAL %s
 // CHECK-EXTERNAL: 160:12 ObjCInterfaceDecl=ExtCls:160:12 (external lang: Swift, defined: some_module, gen: 1)
 // CHECK-EXTERNAL: 161:8 ObjCInstanceMethodDecl=method:161:8 (external lang: Swift, defined: some_module, gen: 1)
-C
+C
 

test/Index/load-exprs.c

 // CHECK: load-exprs.c:7:23: DeclRefExpr=x:6:12 Extent=[7:23 - 7:24]
 // CHECK: load-exprs.c:10:5: FunctionDecl=test_blocks:10:5 (Definition) Extent=[10:1 - 21:2]
 // CHECK: load-exprs.c:10:21: ParmDecl=x:10:21 (Definition) Extent=[10:17 - 10:22]
-// CHECK: load-exprs.c:11:15: VarDecl=y:11:15 (Definition) Extent=[11:3 - 11:20]
+// CHECK: load-exprs.c:11:15: VarDecl=y:11:15 (Definition)
 // CHECK: load-exprs.c:11:19: DeclRefExpr=x:10:21 Extent=[11:19 - 11:20]
 // CHECK: load-exprs.c:12:3: CallExpr= Extent=[12:3 - 19:7]
 // CHECK: load-exprs.c:13:17: VarDecl=z:13:17 (Definition) Extent=[13:6 - 13:22]

test/Index/preamble-conditionals-crash.cpp

 // RUN: | FileCheck %s --implicit-check-not "libclang: crash detected" \
 // RUN:                --implicit-check-not "error:"
 // CHECK: macro expansion=FOO:3:9 Extent=[4:1 - 4:4]
-// CHECK: VarDecl=aba:4:1 (Definition) Extent=[4:1 - 4:4]
+// CHECK: VarDecl=aba:3:17 (Definition) Extent=[3:13 - 4:4]

test/Index/preamble_macro_template.cpp

 // CHECK: preamble_macro_template.h:4:13: ParmDecl=p:4:13 (Definition) Extent=[4:10 - 4:14]
 // CHECK: preamble_macro_template.h:4:16: CompoundStmt= Extent=[4:16 - 6:2]
 // CHECK: preamble_macro_template.h:5:3: CStyleCastExpr= Extent=[5:3 - 5:27]
-// CHECK: preamble_macro_template.h:5:9: CXXStaticCastExpr= Extent=[5:9 - 5:27]
+// CHECK: preamble_macro_template.h:1:21: CXXStaticCastExpr= Extent=[1:21 - 5:27]
 // CHECK: preamble_macro_template.h:5:25: UnexposedExpr= Extent=[5:25 - 5:26]
 // CHECK: preamble_macro_template.h:5:25: IntegerLiteral= Extent=[5:25 - 5:26]
 // CHECK: preamble_macro_template.cpp:3:5: FunctionDecl=main:3:5 (Definition) Extent=[3:1 - 3:15]

tools/libclang/CXSourceLocation.cpp

   
   const SourceManager &SM =
   *static_cast<const SourceManager*>(location.ptr_data[0]);
-  // FIXME: This should call SourceManager::getSpellingLoc().
-  SourceLocation SpellLoc = SM.getFileLoc(Loc);
+  SourceLocation SpellLoc = SM.getSpellingLoc(Loc);
   std::pair<FileID, unsigned> LocInfo = SM.getDecomposedLoc(SpellLoc);
   FileID FID = LocInfo.first;
   unsigned FileOffset = LocInfo.second;