[clang] All file loc followup (PR #166544)
via cfe-commits
cfe-commits at lists.llvm.org
Wed Nov 5 04:14:35 PST 2025
https://github.com/SergejSalnikov created https://github.com/llvm/llvm-project/pull/166544
None
>From fef8fb267ff7a7ad52b2c5dabd4b1dff1fc8394b Mon Sep 17 00:00:00 2001
From: skill <skill at google.com>
Date: Mon, 3 Nov 2025 23:57:50 +0100
Subject: [PATCH 1/4] Universally use file location as a presumed location.
This change moves away from using expansion location
when computing presumed location.
---
clang/lib/Basic/SourceManager.cpp | 2 +-
clang/lib/CodeGen/CGDebugInfo.cpp | 14 ++++++--------
.../test/Analysis/plist-macros-with-expansion.cpp | 8 ++++----
clang/test/C/C23/n2350.c | 5 ++---
clang/test/ExtractAPI/macro_undefined.c | 4 ++--
clang/test/FixIt/format.cpp | 8 ++++----
clang/test/Preprocessor/macro_arg_directive.c | 4 ++--
clang/test/Preprocessor/print_line_track.c | 11 +++++------
8 files changed, 26 insertions(+), 30 deletions(-)
diff --git a/clang/lib/Basic/SourceManager.cpp b/clang/lib/Basic/SourceManager.cpp
index 97aa0f2aa59b9..88e7b843f0a80 100644
--- a/clang/lib/Basic/SourceManager.cpp
+++ b/clang/lib/Basic/SourceManager.cpp
@@ -1481,7 +1481,7 @@ PresumedLoc SourceManager::getPresumedLoc(SourceLocation Loc,
if (Loc.isInvalid()) return PresumedLoc();
// Presumed locations are always for expansion points.
- FileIDAndOffset LocInfo = getDecomposedExpansionLoc(Loc);
+ FileIDAndOffset LocInfo = getDecomposedLoc(getFileLoc(Loc));
bool Invalid = false;
const SLocEntry &Entry = getSLocEntry(LocInfo.first, &Invalid);
diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp
index ca579c915f49d..fc284b7908b4b 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -345,7 +345,7 @@ void CGDebugInfo::setLocation(SourceLocation Loc) {
if (Loc.isInvalid())
return;
- CurLoc = CGM.getContext().getSourceManager().getFileLoc(Loc);
+ CurLoc = Loc;
// If we've changed files in the middle of a lexical scope go ahead
// and create a new lexical scope with file node if it's different
@@ -572,7 +572,7 @@ llvm::DIFile *CGDebugInfo::getOrCreateFile(SourceLocation Loc) {
FileName = TheCU->getFile()->getFilename();
CSInfo = TheCU->getFile()->getChecksum();
} else {
- PresumedLoc PLoc = SM.getPresumedLoc(SM.getFileLoc(Loc));
+ PresumedLoc PLoc = SM.getPresumedLoc(Loc);
FileName = PLoc.getFilename();
if (FileName.empty()) {
@@ -599,8 +599,7 @@ llvm::DIFile *CGDebugInfo::getOrCreateFile(SourceLocation Loc) {
if (CSKind)
CSInfo.emplace(*CSKind, Checksum);
}
- return createFile(FileName, CSInfo,
- getSource(SM, SM.getFileID(SM.getFileLoc(Loc))));
+ return createFile(FileName, CSInfo, getSource(SM, SM.getFileID(Loc)));
}
llvm::DIFile *CGDebugInfo::createFile(
@@ -655,7 +654,7 @@ unsigned CGDebugInfo::getLineNumber(SourceLocation Loc) {
if (Loc.isInvalid())
return 0;
SourceManager &SM = CGM.getContext().getSourceManager();
- return SM.getPresumedLoc(SM.getFileLoc(Loc)).getLine();
+ return SM.getPresumedLoc(Loc).getLine();
}
unsigned CGDebugInfo::getColumnNumber(SourceLocation Loc, bool Force) {
@@ -667,8 +666,7 @@ unsigned CGDebugInfo::getColumnNumber(SourceLocation Loc, bool Force) {
if (Loc.isInvalid() && CurLoc.isInvalid())
return 0;
SourceManager &SM = CGM.getContext().getSourceManager();
- PresumedLoc PLoc =
- SM.getPresumedLoc(Loc.isValid() ? SM.getFileLoc(Loc) : CurLoc);
+ PresumedLoc PLoc = SM.getPresumedLoc(Loc.isValid() ? Loc : CurLoc);
return PLoc.isValid() ? PLoc.getColumn() : 0;
}
@@ -6281,7 +6279,7 @@ void CGDebugInfo::AddStringLiteralDebugInfo(llvm::GlobalVariable *GV,
const StringLiteral *S) {
SourceLocation Loc = S->getStrTokenLoc(0);
SourceManager &SM = CGM.getContext().getSourceManager();
- PresumedLoc PLoc = SM.getPresumedLoc(SM.getFileLoc(Loc));
+ PresumedLoc PLoc = SM.getPresumedLoc(Loc);
if (!PLoc.isValid())
return;
diff --git a/clang/test/Analysis/plist-macros-with-expansion.cpp b/clang/test/Analysis/plist-macros-with-expansion.cpp
index d57bb0f2dd265..d9a2f94055593 100644
--- a/clang/test/Analysis/plist-macros-with-expansion.cpp
+++ b/clang/test/Analysis/plist-macros-with-expansion.cpp
@@ -405,14 +405,14 @@ void commaInBracketsTest() {
code
void commaInBracesTest() {
- PASTE_CODE({ // expected-warning{{Dereference of null pointer}}
+ PASTE_CODE({
// NOTE: If we were to add a new variable here after a comma, we'd get a
// compilation error, so this test is mainly here to show that this was also
// investigated.
//
// int *ptr = nullptr, a;
int *ptr = nullptr;
- *ptr = 5;
+ *ptr = 5; // expected-warning{{Dereference of null pointer}}
})
}
@@ -425,14 +425,14 @@ void commaInBracesTest() {
// CHECK-NEXT: <key>col</key><integer>3</integer>
// CHECK-NEXT: <key>file</key><integer>0</integer>
// CHECK-NEXT: </dict>
-// CHECK-NEXT: <key>name</key><string>PASTE_CODE({ // expected-
+// CHECK-NEXT: <key>name</key><string>PASTE_CODE({
// CHECK-NEXT: // NOTE: If we were to add a new variable here after a comma, we'd get a
// CHECK-NEXT: // compilation error, so this test is mainly here to show that this was also
// CHECK-NEXT: // investigated.
// CHECK-NEXT: //
// CHECK-NEXT: // int *ptr = nullptr, a;
// CHECK-NEXT: int *ptr = nullptr;
-// CHECK-NEXT: *ptr = 5;
+// CHECK-NEXT: *ptr = 5; // expected-
// CHECK-NEXT: })</string>
// CHECK-NEXT: <key>expansion</key><string>{int *ptr =nullptr ;*ptr =5;}</string>
// CHECK-NEXT: </dict>
diff --git a/clang/test/C/C23/n2350.c b/clang/test/C/C23/n2350.c
index af0ca6d79be5e..96b8c511d5716 100644
--- a/clang/test/C/C23/n2350.c
+++ b/clang/test/C/C23/n2350.c
@@ -47,11 +47,10 @@ int struct_in_second_param(void) {
int macro(void) {
return offsetof(struct A // cpp-error {{'A' cannot be defined in a type specifier}} \
- expected-warning 2 {{defining a type within 'offsetof' is a C23 extension}}
+ expected-warning {{defining a type within 'offsetof' is a C23 extension}}
{
int a;
- struct B // verifier seems to think the error is emitted by the macro
- // In fact the location of the error is "B" on the line above
+ struct B // expected-warning {{defining a type within 'offsetof' is a C23 extension}}
{
int c;
int d;
diff --git a/clang/test/ExtractAPI/macro_undefined.c b/clang/test/ExtractAPI/macro_undefined.c
index 7bb50af380c24..1d697db1e1613 100644
--- a/clang/test/ExtractAPI/macro_undefined.c
+++ b/clang/test/ExtractAPI/macro_undefined.c
@@ -89,7 +89,7 @@ FUNC_GEN(bar, const int *, unsigned);
},
"location": {
"position": {
- "character": 0,
+ "character": 9,
"line": 2
},
"uri": "file://INPUT_DIR/input.h"
@@ -241,7 +241,7 @@ FUNC_GEN(bar, const int *, unsigned);
},
"location": {
"position": {
- "character": 0,
+ "character": 9,
"line": 3
},
"uri": "file://INPUT_DIR/input.h"
diff --git a/clang/test/FixIt/format.cpp b/clang/test/FixIt/format.cpp
index d663c0fb35e13..db642b60ffd95 100644
--- a/clang/test/FixIt/format.cpp
+++ b/clang/test/FixIt/format.cpp
@@ -56,9 +56,9 @@ void a(N::E NEVal, S *SPtr, S &SRef) {
// CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:7-[[@LINE-2]]:7}:"static_cast<int>("
// CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:17-[[@LINE-3]]:17}:")"
- LOG( // expected-warning{{format specifies type 'int' but the argument has type 'N::E'}}
+ LOG(
"%d",
- SPtr->Type
+ SPtr->Type // expected-warning{{format specifies type 'int' but the argument has type 'N::E'}}
);
// CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:7-[[@LINE-2]]:7}:"static_cast<int>("
// CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:17-[[@LINE-3]]:17}:")"
@@ -68,8 +68,8 @@ void a(N::E NEVal, S *SPtr, S &SRef) {
// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:7-[[@LINE-1]]:7}:"static_cast<int>("
// CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:16-[[@LINE-2]]:16}:")"
- LOG("%d", // expected-warning{{format specifies type 'int' but the argument has type 'N::E'}}
- SRef.Type);
+ LOG("%d",
+ SRef.Type); // expected-warning{{format specifies type 'int' but the argument has type 'N::E'}}
// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:7-[[@LINE-1]]:7}:"static_cast<int>("
// CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:16-[[@LINE-2]]:16}:")"
diff --git a/clang/test/Preprocessor/macro_arg_directive.c b/clang/test/Preprocessor/macro_arg_directive.c
index 929a03d70d025..c612aa545a2a9 100644
--- a/clang/test/Preprocessor/macro_arg_directive.c
+++ b/clang/test/Preprocessor/macro_arg_directive.c
@@ -18,7 +18,7 @@ void fail(const char *);
({ int result = 0; __VA_ARGS__; if (!result) { fail(#__VA_ARGS__); }; result })
static inline int f(int k) {
- return MUNCH( // expected-error {{expected ')'}} expected-note {{to match this '('}} expected-error {{returning 'void'}} expected-note {{expansion of macro 'MUNCH' requested here}}
+ return MUNCH( // expected-note {{to match this '('}} expected-error {{returning 'void'}} expected-note {{expansion of macro 'MUNCH' requested here}}
if (k < 3)
result = 24;
else if (k > 4)
@@ -27,6 +27,6 @@ static inline int f(int k) {
#include "macro_arg_directive.h" // expected-error {{embedding a #include directive within macro arguments is not supported}}
-int g(int k) {
+int g(int k) { // expected-error {{expected ')'}}
return f(k) + f(k-1));
}
diff --git a/clang/test/Preprocessor/print_line_track.c b/clang/test/Preprocessor/print_line_track.c
index 156ae22693b85..56f30073e3e86 100644
--- a/clang/test/Preprocessor/print_line_track.c
+++ b/clang/test/Preprocessor/print_line_track.c
@@ -1,9 +1,9 @@
-/* RUN: %clang_cc1 -E %s | grep 'a 3'
- * RUN: %clang_cc1 -E %s | grep 'b 16'
- * RUN: %clang_cc1 -E -P %s | grep 'a 3'
- * RUN: %clang_cc1 -E -P %s | grep 'b 16'
+/* RUN: %clang_cc1 -E %s | grep -z 'a.3'
+ * RUN: %clang_cc1 -E %s | grep -z 'b.16'
+ * RUN: %clang_cc1 -E -P %s | grep -z 'a.3'
+ * RUN: %clang_cc1 -E -P %s | grep -z 'b.16'
* RUN: %clang_cc1 -E %s | not grep '# 0 '
- * RUN: %clang_cc1 -E -P %s | count 2
+ * RUN: %clang_cc1 -E -P %s | count 4
* PR1848 PR3437 PR7360
*/
@@ -14,4 +14,3 @@ t(a
t(b
__LINE__)
-
>From a202000bb7696622d7d994a8bdc0c6b0591a9593 Mon Sep 17 00:00:00 2001
From: SKill <skill at google.com>
Date: Wed, 5 Nov 2025 11:29:34 +0100
Subject: [PATCH 2/4] Fix FID usage in getOrCreateFile
---
clang/lib/CodeGen/CGDebugInfo.cpp | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp
index fc284b7908b4b..6c1e0038a4316 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -571,6 +571,7 @@ llvm::DIFile *CGDebugInfo::getOrCreateFile(SourceLocation Loc) {
// with an absolute path.
FileName = TheCU->getFile()->getFilename();
CSInfo = TheCU->getFile()->getChecksum();
+ FID = SM.getFileID(Loc);
} else {
PresumedLoc PLoc = SM.getPresumedLoc(Loc);
FileName = PLoc.getFilename();
@@ -599,7 +600,7 @@ llvm::DIFile *CGDebugInfo::getOrCreateFile(SourceLocation Loc) {
if (CSKind)
CSInfo.emplace(*CSKind, Checksum);
}
- return createFile(FileName, CSInfo, getSource(SM, SM.getFileID(Loc)));
+ return createFile(FileName, CSInfo, getSource(SM, FID));
}
llvm::DIFile *CGDebugInfo::createFile(
>From 2cb78181e16f4b4345007fdb4d5d3154133874ff Mon Sep 17 00:00:00 2001
From: skill <skill at google.com>
Date: Wed, 5 Nov 2025 13:09:07 +0100
Subject: [PATCH 3/4] Move CGDebugInfo.cc to a new PR.
---
clang/lib/CodeGen/CGDebugInfo.cpp | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp
index 6c1e0038a4316..ca579c915f49d 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -345,7 +345,7 @@ void CGDebugInfo::setLocation(SourceLocation Loc) {
if (Loc.isInvalid())
return;
- CurLoc = Loc;
+ CurLoc = CGM.getContext().getSourceManager().getFileLoc(Loc);
// If we've changed files in the middle of a lexical scope go ahead
// and create a new lexical scope with file node if it's different
@@ -571,9 +571,8 @@ llvm::DIFile *CGDebugInfo::getOrCreateFile(SourceLocation Loc) {
// with an absolute path.
FileName = TheCU->getFile()->getFilename();
CSInfo = TheCU->getFile()->getChecksum();
- FID = SM.getFileID(Loc);
} else {
- PresumedLoc PLoc = SM.getPresumedLoc(Loc);
+ PresumedLoc PLoc = SM.getPresumedLoc(SM.getFileLoc(Loc));
FileName = PLoc.getFilename();
if (FileName.empty()) {
@@ -600,7 +599,8 @@ llvm::DIFile *CGDebugInfo::getOrCreateFile(SourceLocation Loc) {
if (CSKind)
CSInfo.emplace(*CSKind, Checksum);
}
- return createFile(FileName, CSInfo, getSource(SM, FID));
+ return createFile(FileName, CSInfo,
+ getSource(SM, SM.getFileID(SM.getFileLoc(Loc))));
}
llvm::DIFile *CGDebugInfo::createFile(
@@ -655,7 +655,7 @@ unsigned CGDebugInfo::getLineNumber(SourceLocation Loc) {
if (Loc.isInvalid())
return 0;
SourceManager &SM = CGM.getContext().getSourceManager();
- return SM.getPresumedLoc(Loc).getLine();
+ return SM.getPresumedLoc(SM.getFileLoc(Loc)).getLine();
}
unsigned CGDebugInfo::getColumnNumber(SourceLocation Loc, bool Force) {
@@ -667,7 +667,8 @@ unsigned CGDebugInfo::getColumnNumber(SourceLocation Loc, bool Force) {
if (Loc.isInvalid() && CurLoc.isInvalid())
return 0;
SourceManager &SM = CGM.getContext().getSourceManager();
- PresumedLoc PLoc = SM.getPresumedLoc(Loc.isValid() ? Loc : CurLoc);
+ PresumedLoc PLoc =
+ SM.getPresumedLoc(Loc.isValid() ? SM.getFileLoc(Loc) : CurLoc);
return PLoc.isValid() ? PLoc.getColumn() : 0;
}
@@ -6280,7 +6281,7 @@ void CGDebugInfo::AddStringLiteralDebugInfo(llvm::GlobalVariable *GV,
const StringLiteral *S) {
SourceLocation Loc = S->getStrTokenLoc(0);
SourceManager &SM = CGM.getContext().getSourceManager();
- PresumedLoc PLoc = SM.getPresumedLoc(Loc);
+ PresumedLoc PLoc = SM.getPresumedLoc(SM.getFileLoc(Loc));
if (!PLoc.isValid())
return;
>From af81e65b416cce5de8ada7eada63a7d1b0608a0e Mon Sep 17 00:00:00 2001
From: skill <skill at google.com>
Date: Wed, 5 Nov 2025 13:13:23 +0100
Subject: [PATCH 4/4] Remove unnessesary call to SM.getFileLoc when calling
SM.getPresumedLoc.
---
clang/lib/CodeGen/CGDebugInfo.cpp | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp
index ca579c915f49d..6c1e0038a4316 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -345,7 +345,7 @@ void CGDebugInfo::setLocation(SourceLocation Loc) {
if (Loc.isInvalid())
return;
- CurLoc = CGM.getContext().getSourceManager().getFileLoc(Loc);
+ CurLoc = Loc;
// If we've changed files in the middle of a lexical scope go ahead
// and create a new lexical scope with file node if it's different
@@ -571,8 +571,9 @@ llvm::DIFile *CGDebugInfo::getOrCreateFile(SourceLocation Loc) {
// with an absolute path.
FileName = TheCU->getFile()->getFilename();
CSInfo = TheCU->getFile()->getChecksum();
+ FID = SM.getFileID(Loc);
} else {
- PresumedLoc PLoc = SM.getPresumedLoc(SM.getFileLoc(Loc));
+ PresumedLoc PLoc = SM.getPresumedLoc(Loc);
FileName = PLoc.getFilename();
if (FileName.empty()) {
@@ -599,8 +600,7 @@ llvm::DIFile *CGDebugInfo::getOrCreateFile(SourceLocation Loc) {
if (CSKind)
CSInfo.emplace(*CSKind, Checksum);
}
- return createFile(FileName, CSInfo,
- getSource(SM, SM.getFileID(SM.getFileLoc(Loc))));
+ return createFile(FileName, CSInfo, getSource(SM, FID));
}
llvm::DIFile *CGDebugInfo::createFile(
@@ -655,7 +655,7 @@ unsigned CGDebugInfo::getLineNumber(SourceLocation Loc) {
if (Loc.isInvalid())
return 0;
SourceManager &SM = CGM.getContext().getSourceManager();
- return SM.getPresumedLoc(SM.getFileLoc(Loc)).getLine();
+ return SM.getPresumedLoc(Loc).getLine();
}
unsigned CGDebugInfo::getColumnNumber(SourceLocation Loc, bool Force) {
@@ -667,8 +667,7 @@ unsigned CGDebugInfo::getColumnNumber(SourceLocation Loc, bool Force) {
if (Loc.isInvalid() && CurLoc.isInvalid())
return 0;
SourceManager &SM = CGM.getContext().getSourceManager();
- PresumedLoc PLoc =
- SM.getPresumedLoc(Loc.isValid() ? SM.getFileLoc(Loc) : CurLoc);
+ PresumedLoc PLoc = SM.getPresumedLoc(Loc.isValid() ? Loc : CurLoc);
return PLoc.isValid() ? PLoc.getColumn() : 0;
}
@@ -6281,7 +6280,7 @@ void CGDebugInfo::AddStringLiteralDebugInfo(llvm::GlobalVariable *GV,
const StringLiteral *S) {
SourceLocation Loc = S->getStrTokenLoc(0);
SourceManager &SM = CGM.getContext().getSourceManager();
- PresumedLoc PLoc = SM.getPresumedLoc(SM.getFileLoc(Loc));
+ PresumedLoc PLoc = SM.getPresumedLoc(Loc);
if (!PLoc.isValid())
return;
More information about the cfe-commits
mailing list