[PATCH] D39217: [libclang, bindings]: add spelling location

2017-11-08 Thread Masud Rahman via Phabricator via cfe-commits 
frutiger added a comment.

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


https://reviews.llvm.org/D39217



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39217: [libclang, bindings]: add spelling location

2017-10-25 Thread Masud Rahman via Phabricator via cfe-commits 
frutiger added a comment.

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.


https://reviews.llvm.org/D39217



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39217: [libclang, bindings]: add spelling location

2017-10-25 Thread Johann Klähn via Phabricator via cfe-commits 
jklaehn added inline comments.



Comment at: test/Index/c-index-getCursor-test.m:167
 // 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

frutiger wrote:
> jklaehn wrote:
> > This does not seem to refer to a valid location? (line 2 only contains a 
> > comment)
> I don't really understand this output.  What is `-test-file-scan` supposed to 
> show?
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.


https://reviews.llvm.org/D39217



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39217: [libclang, bindings]: add spelling location

2017-10-24 Thread Masud Rahman via Phabricator via cfe-commits 
frutiger added inline comments.



Comment at: test/Index/c-index-getCursor-test.m:167
 // 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

jklaehn wrote:
> This does not seem to refer to a valid location? (line 2 only contains a 
> comment)
I don't really understand this output.  What is `-test-file-scan` supposed to 
show?


https://reviews.llvm.org/D39217



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39217: [libclang, bindings]: add spelling location

2017-10-24 Thread Johann Klähn via Phabricator via cfe-commits 
jklaehn added inline comments.



Comment at: test/Index/annotate-tokens.c:226
 // 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=

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



Comment at: test/Index/c-index-getCursor-test.m:167
 // 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

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


https://reviews.llvm.org/D39217



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39217: [libclang, bindings]: add spelling location

2017-10-23 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits 
akyrtzi added a subscriber: arphaman.
akyrtzi added a comment.

CC'ed @arphaman.


https://reviews.llvm.org/D39217



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39217: [libclang, bindings]: add spelling location

2017-10-23 Thread Saleem Abdulrasool via Phabricator via cfe-commits 
compnerd added a subscriber: akyrtzi.
compnerd added a comment.

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


https://reviews.llvm.org/D39217



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39217: [libclang, bindings]: add spelling location

2017-10-23 Thread Masud Rahman via Phabricator via cfe-commits 
frutiger added a comment.

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.


https://reviews.llvm.org/D39217



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39217: [libclang, bindings]: add spelling location

2017-10-23 Thread Masud Rahman via Phabricator via cfe-commits 
frutiger updated this revision to Diff 119971.
frutiger added a comment.

Add context to the patch.


https://reviews.llvm.org/D39217

Files:
  bindings/python/clang/cindex.py
  bindings/python/tests/cindex/test_location.py
  test/Index/annotate-tokens.c
  test/Index/blocks.c
  test/Index/c-index-api-loadTU-test.m
  test/Index/c-index-getCursor-test.m
  test/Index/get-cursor.cpp
  test/Index/get-cursor.m
  test/Index/load-exprs.c
  test/Index/preamble-conditionals-crash.cpp
  test/Index/preamble_macro_template.cpp
  tools/libclang/CXSourceLocation.cpp

Index: tools/libclang/CXSourceLocation.cpp
===
--- tools/libclang/CXSourceLocation.cpp
+++ tools/libclang/CXSourceLocation.cpp
@@ -318,8 +318,7 @@
   
   const SourceManager &SM =
   *static_cast(location.ptr_data[0]);
-  // FIXME: This should call SourceManager::getSpellingLoc().
-  SourceLocation SpellLoc = SM.getFileLoc(Loc);
+  SourceLocation SpellLoc = SM.getSpellingLoc(Loc);
   std::pair LocInfo = SM.getDecomposedLoc(SpellLoc);
   FileID FID = LocInfo.first;
   unsigned FileOffset = LocInfo.second;
Index: test/Index/preamble_macro_template.cpp
===
--- test/Index/preamble_macro_template.cpp
+++ test/Index/preamble_macro_template.cpp
@@ -8,7 +8,7 @@
 // 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]
Index: test/Index/preamble-conditionals-crash.cpp
===
--- test/Index/preamble-conditionals-crash.cpp
+++ test/Index/preamble-conditionals-crash.cpp
@@ -9,4 +9,4 @@
 // 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]
Index: test/Index/load-exprs.c
===
--- test/Index/load-exprs.c
+++ test/Index/load-exprs.c
@@ -52,7 +52,7 @@
 // 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]
Index: test/Index/get-cursor.m
===
--- test/Index/get-cursor.m
+++ test/Index/get-cursor.m
@@ -229,11 +229,11 @@
 // 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
\ No newline at end of file
+C
Index: test/Index/get-cursor.cpp
===
--- test/Index/get-cursor.cpp
+++ test/Index/get-cursor.cpp
@@ -214,7 +214,7 @@
 // CHECK-TEMPLPARAM: 55:23 T

[PATCH] D39217: [libclang, bindings]: add spelling location

2017-10-23 Thread Masud Rahman via Phabricator via cfe-commits 
frutiger created this revision.

- 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.


https://reviews.llvm.org/D39217

Files:
  bindings/python/clang/cindex.py
  bindings/python/tests/cindex/test_location.py
  test/Index/annotate-tokens.c
  test/Index/blocks.c
  test/Index/c-index-api-loadTU-test.m
  test/Index/c-index-getCursor-test.m
  test/Index/get-cursor.cpp
  test/Index/get-cursor.m
  test/Index/load-exprs.c
  test/Index/preamble-conditionals-crash.cpp
  test/Index/preamble_macro_template.cpp
  tools/libclang/CXSourceLocation.cpp

Index: tools/libclang/CXSourceLocation.cpp
===
--- tools/libclang/CXSourceLocation.cpp
+++ tools/libclang/CXSourceLocation.cpp
@@ -318,8 +318,7 @@
   
   const SourceManager &SM =
   *static_cast(location.ptr_data[0]);
-  // FIXME: This should call SourceManager::getSpellingLoc().
-  SourceLocation SpellLoc = SM.getFileLoc(Loc);
+  SourceLocation SpellLoc = SM.getSpellingLoc(Loc);
   std::pair LocInfo = SM.getDecomposedLoc(SpellLoc);
   FileID FID = LocInfo.first;
   unsigned FileOffset = LocInfo.second;
Index: test/Index/preamble_macro_template.cpp
===
--- test/Index/preamble_macro_template.cpp
+++ test/Index/preamble_macro_template.cpp
@@ -8,7 +8,7 @@
 // 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]
Index: test/Index/preamble-conditionals-crash.cpp
===
--- test/Index/preamble-conditionals-crash.cpp
+++ test/Index/preamble-conditionals-crash.cpp
@@ -9,4 +9,4 @@
 // 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]
Index: test/Index/load-exprs.c
===
--- test/Index/load-exprs.c
+++ test/Index/load-exprs.c
@@ -52,7 +52,7 @@
 // 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]
Index: test/Index/get-cursor.m
===
--- test/Index/get-cursor.m
+++ test/Index/get-cursor.m
@@ -229,11 +229,11 @@
 // 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 Typ