https://github.com/SergejSalnikov updated https://github.com/llvm/llvm-project/pull/166255
>From fef8fb267ff7a7ad52b2c5dabd4b1dff1fc8394b Mon Sep 17 00:00:00 2001 From: skill <[email protected]> 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 <[email protected]> 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 <[email protected]> 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 6134ab1235049f7fe6b765320ef5fadbde612e27 Mon Sep 17 00:00:00 2001 From: SKill <[email protected]> Date: Wed, 5 Nov 2025 17:01:22 +0100 Subject: [PATCH 4/4] Update comment for getPresumedLoc --- clang/include/clang/Basic/SourceManager.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/clang/include/clang/Basic/SourceManager.h b/clang/include/clang/Basic/SourceManager.h index ed967fd47dc83..2e2917a40a1b2 100644 --- a/clang/include/clang/Basic/SourceManager.h +++ b/clang/include/clang/Basic/SourceManager.h @@ -1469,8 +1469,9 @@ class SourceManager : public RefCountedBase<SourceManager> { /// directives. This provides a view on the data that a user should see /// in diagnostics, for example. /// - /// Note that a presumed location is always given as the expansion point of - /// an expansion location, not at the spelling location. + /// If \p Loc is a macro expansion location, the presumed location + /// computation uses the spelling location for macro arguments and the + /// expansion location for other macro expansions. /// /// \returns The presumed location of the specified SourceLocation. If the /// presumed location cannot be calculated (e.g., because \p Loc is invalid _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
