Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[libclang] Compute the right spelling location #72400

Merged
merged 1 commit into from
Apr 24, 2024

Conversation

sebastianpoeplau
Copy link
Contributor

Locations inside macro expansions have different spelling/expansion locations. Apply a FIXME to make the libclang function clang_getSpellingLocation return the right spelling location, and adapt the testsuite driver code to use the file location rather than the spelling location to compute source ranges.

@llvmbot llvmbot added the clang Clang issues not falling into any other category label Nov 15, 2023
@llvmbot
Copy link
Collaborator

llvmbot commented Nov 15, 2023

@llvm/pr-subscribers-clang

Author: Sebastian Poeplau (sebastianpoeplau)

Changes

Locations inside macro expansions have different spelling/expansion locations. Apply a FIXME to make the libclang function clang_getSpellingLocation return the right spelling location, and adapt the testsuite driver code to use the file location rather than the spelling location to compute source ranges.


Full diff: https://github.com/llvm/llvm-project/pull/72400.diff

2 Files Affected:

  • (modified) clang/tools/c-index-test/c-index-test.c (+27-28)
  • (modified) clang/tools/libclang/CXSourceLocation.cpp (+1-2)
diff --git a/clang/tools/c-index-test/c-index-test.c b/clang/tools/c-index-test/c-index-test.c
index 9d66a22f3b43b55..e73c73ce59e0f72 100644
--- a/clang/tools/c-index-test/c-index-test.c
+++ b/clang/tools/c-index-test/c-index-test.c
@@ -451,10 +451,10 @@ static void PrintRange(CXSourceRange R, const char *str) {
   CXFile begin_file, end_file;
   unsigned begin_line, begin_column, end_line, end_column;
 
-  clang_getSpellingLocation(clang_getRangeStart(R),
-                            &begin_file, &begin_line, &begin_column, 0);
-  clang_getSpellingLocation(clang_getRangeEnd(R),
-                            &end_file, &end_line, &end_column, 0);
+  clang_getFileLocation(clang_getRangeStart(R),
+                        &begin_file, &begin_line, &begin_column, 0);
+  clang_getFileLocation(clang_getRangeEnd(R),
+                        &end_file, &end_line, &end_column, 0);
   if (!begin_file || !end_file)
     return;
 
@@ -836,13 +836,13 @@ static void PrintCursor(CXCursor Cursor, const char *CommentSchemaFile) {
             printf(", ");
           
           Loc = clang_getCursorLocation(Ovl);
-          clang_getSpellingLocation(Loc, 0, &line, &column, 0);
+          clang_getFileLocation(Loc, 0, &line, &column, 0);
           printf("%d:%d", line, column);          
         }
         printf("]");
       } else {
         CXSourceLocation Loc = clang_getCursorLocation(Referenced);
-        clang_getSpellingLocation(Loc, 0, &line, &column, 0);
+        clang_getFileLocation(Loc, 0, &line, &column, 0);
         printf(":%d:%d", line, column);
       }
 
@@ -1034,7 +1034,7 @@ static void PrintCursor(CXCursor Cursor, const char *CommentSchemaFile) {
     if (!clang_equalCursors(SpecializationOf, clang_getNullCursor())) {
       CXSourceLocation Loc = clang_getCursorLocation(SpecializationOf);
       CXString Name = clang_getCursorSpelling(SpecializationOf);
-      clang_getSpellingLocation(Loc, 0, &line, &column, 0);
+      clang_getFileLocation(Loc, 0, &line, &column, 0);
       printf(" [Specialization of %s:%d:%d]",
              clang_getCString(Name), line, column);
       clang_disposeString(Name);
@@ -1081,7 +1081,7 @@ static void PrintCursor(CXCursor Cursor, const char *CommentSchemaFile) {
       printf(" [Overrides ");
       for (I = 0; I != num_overridden; ++I) {
         CXSourceLocation Loc = clang_getCursorLocation(overridden[I]);
-        clang_getSpellingLocation(Loc, 0, &line, &column, 0);
+        clang_getFileLocation(Loc, 0, &line, &column, 0);
         lineCols[I].line = line;
         lineCols[I].col = column;
       }
@@ -1244,8 +1244,8 @@ void PrintDiagnostic(CXDiagnostic Diagnostic) {
   fprintf(stderr, "%s\n", clang_getCString(Msg));
   clang_disposeString(Msg);
 
-  clang_getSpellingLocation(clang_getDiagnosticLocation(Diagnostic),
-                            &file, 0, 0, 0);
+  clang_getFileLocation(clang_getDiagnosticLocation(Diagnostic),
+                        &file, 0, 0, 0);
   if (!file)
     return;
 
@@ -1258,9 +1258,8 @@ void PrintDiagnostic(CXDiagnostic Diagnostic) {
     CXSourceLocation end = clang_getRangeEnd(range);
     unsigned start_line, start_column, end_line, end_column;
     CXFile start_file, end_file;
-    clang_getSpellingLocation(start, &start_file, &start_line,
-                              &start_column, 0);
-    clang_getSpellingLocation(end, &end_file, &end_line, &end_column, 0);
+    clang_getFileLocation(start, &start_file, &start_line, &start_column, 0);
+    clang_getFileLocation(end, &end_file, &end_line, &end_column, 0);
     if (clang_equalLocations(start, end)) {
       /* Insertion. */
       if (start_file == file)
@@ -1343,7 +1342,7 @@ enum CXChildVisitResult FilteredPrintingVisitor(CXCursor Cursor,
   if (!Data->Filter || (Cursor.kind == *(enum CXCursorKind *)Data->Filter)) {
     CXSourceLocation Loc = clang_getCursorLocation(Cursor);
     unsigned line, column;
-    clang_getSpellingLocation(Loc, 0, &line, &column, 0);
+    clang_getFileLocation(Loc, 0, &line, &column, 0);
     printf("// %s: %s:%d:%d: ", FileCheckPrefix,
            GetCursorSource(Cursor), line, column);
     PrintCursor(Cursor, Data->CommentSchemaFile);
@@ -1404,7 +1403,7 @@ static enum CXChildVisitResult FunctionScanVisitor(CXCursor Cursor,
       curColumn++;
 
     Loc = clang_getCursorLocation(Cursor);
-    clang_getSpellingLocation(Loc, &file, 0, 0, 0);
+    clang_getFileLocation(Loc, &file, 0, 0, 0);
 
     source = clang_getFileName(file);
     if (clang_getCString(source)) {
@@ -1470,8 +1469,8 @@ void InclusionVisitor(CXFile includedFile, CXSourceLocation *includeStack,
   for (i = 0; i < includeStackLen; ++i) {
     CXFile includingFile;
     unsigned line, column;
-    clang_getSpellingLocation(includeStack[i], &includingFile, &line,
-                              &column, 0);
+    clang_getFileLocation(includeStack[i], &includingFile, &line,
+                          &column, 0);
     fname = clang_getFileName(includingFile);
     printf("  %s:%d:%d\n", clang_getCString(fname), line, column);
     clang_disposeString(fname);
@@ -2967,7 +2966,7 @@ static void inspect_print_cursor(CXCursor Cursor) {
   CXString Spelling;
   const char *cspell;
   unsigned line, column;
-  clang_getSpellingLocation(CursorLoc, 0, &line, &column, 0);
+  clang_getFileLocation(CursorLoc, 0, &line, &column, 0);
   printf("%d:%d ", line, column);
   PrintCursor(Cursor, NULL);
   PrintCursorExtent(Cursor);
@@ -3083,7 +3082,7 @@ static void inspect_evaluate_cursor(CXCursor Cursor) {
   unsigned line, column;
   CXEvalResult ER;
 
-  clang_getSpellingLocation(CursorLoc, 0, &line, &column, 0);
+  clang_getFileLocation(CursorLoc, 0, &line, &column, 0);
   printf("%d:%d ", line, column);
   PrintCursor(Cursor, NULL);
   PrintCursorExtent(Cursor);
@@ -3118,7 +3117,7 @@ static void inspect_macroinfo_cursor(CXCursor Cursor) {
   CXString Spelling;
   const char *cspell;
   unsigned line, column;
-  clang_getSpellingLocation(CursorLoc, 0, &line, &column, 0);
+  clang_getFileLocation(CursorLoc, 0, &line, &column, 0);
   printf("%d:%d ", line, column);
   PrintCursor(Cursor, NULL);
   PrintCursorExtent(Cursor);
@@ -4311,10 +4310,10 @@ int perform_token_annotation(int argc, const char **argv) {
   skipped_ranges = clang_getSkippedRanges(TU, file);
   for (i = 0; i != skipped_ranges->count; ++i) {
     unsigned start_line, start_column, end_line, end_column;
-    clang_getSpellingLocation(clang_getRangeStart(skipped_ranges->ranges[i]),
-                              0, &start_line, &start_column, 0);
-    clang_getSpellingLocation(clang_getRangeEnd(skipped_ranges->ranges[i]),
-                              0, &end_line, &end_column, 0);
+    clang_getFileLocation(clang_getRangeStart(skipped_ranges->ranges[i]),
+                          0, &start_line, &start_column, 0);
+    clang_getFileLocation(clang_getRangeEnd(skipped_ranges->ranges[i]),
+                          0, &end_line, &end_column, 0);
     printf("Skipping: ");
     PrintExtent(stdout, start_line, start_column, end_line, end_column);
     printf("\n");
@@ -4334,10 +4333,10 @@ int perform_token_annotation(int argc, const char **argv) {
     case CXToken_Literal: kind = "Literal"; break;
     case CXToken_Comment: kind = "Comment"; break;
     }
-    clang_getSpellingLocation(clang_getRangeStart(extent),
-                              0, &start_line, &start_column, 0);
-    clang_getSpellingLocation(clang_getRangeEnd(extent),
-                              0, &end_line, &end_column, 0);
+    clang_getFileLocation(clang_getRangeStart(extent),
+                          0, &start_line, &start_column, 0);
+    clang_getFileLocation(clang_getRangeEnd(extent),
+                          0, &end_line, &end_column, 0);
     printf("%s: \"%s\" ", kind, clang_getCString(spelling));
     clang_disposeString(spelling);
     PrintExtent(stdout, start_line, start_column, end_line, end_column);
diff --git a/clang/tools/libclang/CXSourceLocation.cpp b/clang/tools/libclang/CXSourceLocation.cpp
index ba70cbfee8995f2..53cb71f7276f299 100644
--- a/clang/tools/libclang/CXSourceLocation.cpp
+++ b/clang/tools/libclang/CXSourceLocation.cpp
@@ -319,8 +319,7 @@ void clang_getSpellingLocation(CXSourceLocation location,
   
   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;

Copy link

github-actions bot commented Nov 15, 2023

✅ With the latest revision this PR passed the C/C++ code formatter.

@sebastianpoeplau
Copy link
Contributor Author

I just discovered #28205 which should be fixed by this change.

@sebastianpoeplau
Copy link
Contributor Author

Ping

@sebastianpoeplau
Copy link
Contributor Author

@jansvoboda11 since you've most recently touched this part of the code, would you mind having a look at the change?

@jansvoboda11
Copy link
Contributor

The change itself looks good, but I think we should have a test and an entry in release notes.

@sebastianpoeplau
Copy link
Contributor Author

Thanks for the review @jansvoboda11; I've added an entry in the release notes and a unit test.

Locations inside macro expansions have different spelling/expansion
locations. Apply a FIXME to make the libclang function
clang_getSpellingLocation return the right spelling location, and adapt
the testsuite driver code to use the file location rather than the
spelling location to compute source ranges.
@sebastianpoeplau
Copy link
Contributor Author

Thanks for the approval @jansvoboda11! Would you mind merging the change? I don't have the required permissions...

@jansvoboda11 jansvoboda11 merged commit 2e770ed into llvm:main Apr 24, 2024
5 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants