Sirraide wrote:
> In that case WYGIWYD (what you get is what you deserve) applies.
Sgtm; I’ll leave the clang-tidy side of things as-is then and just update the
test to reflect this.
https://github.com/llvm/llvm-project/pull/143520
___
cfe-commits ma
LegalizeAdulthood wrote:
> What about symlinks though?
In that case WYGIWYD (what you get is what you deserve) applies.
https://github.com/llvm/llvm-project/pull/143520
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi
AaronBallman wrote:
> My expectation would be that if I specify a header filter I'm not going to
> use weird paths like a/b/../foo.h, but just a/foo.h because that is where
> foo.h lives.
What about symlinks though? Would you expect that passing `path/to/file` fails
because `path` is a symlin
LegalizeAdulthood wrote:
My expectation would be that if I specify a header filter I'm not going to use
weird paths like a/b/../foo.h, but just a/foo.h because that is where foo.h
lives.
https://github.com/llvm/llvm-project/pull/143520
___
cfe-commit
AaronBallman wrote:
> Ok, looks like the clang-tidy test failure is related to the `-header-filter`
> option:
>
> ```c++
> // Check that `-header-filter` operates on the same file paths as paths in
> // diagnostics printed by ClangTidy.
> #include "dir1/dir2/../header_alias.h"
> // CHECK_HEADER
Sirraide wrote:
> should the header filter apply to the original filename,
I mean, I guess this beacuse it’s what the user specified and it’s what we’re
currently doing? It’s just that the end result might be weird, e.g. if a user
writes `-exclude-header=filter='a/foo.h'` and then we print dia
https://github.com/Sirraide updated
https://github.com/llvm/llvm-project/pull/143520
>From 15c0a79d6a0cd65d88fbe152275b224201e632a1 Mon Sep 17 00:00:00 2001
From: Sirraide
Date: Tue, 10 Jun 2025 14:32:32 +0200
Subject: [PATCH 1/5] [Clang] [Diagnostics] Simplify filenames that contain
'..'
---
Sirraide wrote:
Ok, looks like the clang-tidy test failure is related to the `-header-filter`
option:
```C++
// Check that `-header-filter` operates on the same file paths as paths in
// diagnostics printed by ClangTidy.
#include "dir1/dir2/../header_alias.h"
// CHECK_HEADER_ALIAS: dir1/dir2/../
@@ -2403,3 +2403,75 @@ SourceManagerForFile::SourceManagerForFile(StringRef
FileName,
assert(ID.isValid());
SourceMgr->setMainFileID(ID);
}
+
+StringRef
+SourceManager::getNameForDiagnostic(StringRef Filename,
+const DiagnosticOptions &O
@@ -2403,3 +2403,75 @@ SourceManagerForFile::SourceManagerForFile(StringRef
FileName,
assert(ID.isValid());
SourceMgr->setMainFileID(ID);
}
+
+StringRef
+SourceManager::getNameForDiagnostic(StringRef Filename,
+const DiagnosticOptions &O
@@ -35,6 +35,7 @@ namespace clang {
class TextDiagnostic : public DiagnosticRenderer {
raw_ostream &OS;
const Preprocessor *PP;
+ llvm::StringMap> SimplifiedFileNameCache;
Sirraide wrote:
Not anymore; forgot to remove that.
https://github.com/llvm/llvm-p
@@ -2403,3 +2403,75 @@ SourceManagerForFile::SourceManagerForFile(StringRef
FileName,
assert(ID.isValid());
SourceMgr->setMainFileID(ID);
}
+
+StringRef
+SourceManager::getNameForDiagnostic(StringRef Filename,
+const DiagnosticOptions &O
@@ -35,6 +35,7 @@ namespace clang {
class TextDiagnostic : public DiagnosticRenderer {
raw_ostream &OS;
const Preprocessor *PP;
+ llvm::StringMap> SimplifiedFileNameCache;
cor3ntin wrote:
Is this used?
https://github.com/llvm/llvm-project/pull/143520
___
@@ -2403,3 +2403,75 @@ SourceManagerForFile::SourceManagerForFile(StringRef
FileName,
assert(ID.isValid());
SourceMgr->setMainFileID(ID);
}
+
+StringRef
+SourceManager::getNameForDiagnostic(StringRef Filename,
+const DiagnosticOptions &O
https://github.com/AaronBallman commented:
In general, I'm in favor of this patch. Precommit CI found relevant failures
that need to be fixed, but I think this is otherwise good to go once those are
addressed.
https://github.com/llvm/llvm-project/pull/143520
___
@@ -738,12 +738,20 @@ void TextDiagnostic::printDiagnosticMessage(raw_ostream
&OS,
}
void TextDiagnostic::emitFilename(StringRef Filename, const SourceManager &SM)
{
-#ifdef _WIN32
- SmallString<4096> TmpFilename;
-#endif
- if (DiagOpts.AbsolutePath) {
-auto File = SM.
@@ -738,12 +738,20 @@ void TextDiagnostic::printDiagnosticMessage(raw_ostream
&OS,
}
void TextDiagnostic::emitFilename(StringRef Filename, const SourceManager &SM)
{
-#ifdef _WIN32
- SmallString<4096> TmpFilename;
-#endif
- if (DiagOpts.AbsolutePath) {
-auto File = SM.
Sirraide wrote:
Actually, we could also just put it in `SourceManager` because that already has
a reference to the `DiagnosticsEngine` and then a single
`getNameForDiagnostic(StringRef Filename)` function would do.
https://github.com/llvm/llvm-project/pull/143520
_
@@ -738,12 +738,20 @@ void TextDiagnostic::printDiagnosticMessage(raw_ostream
&OS,
}
void TextDiagnostic::emitFilename(StringRef Filename, const SourceManager &SM)
{
-#ifdef _WIN32
- SmallString<4096> TmpFilename;
-#endif
- if (DiagOpts.AbsolutePath) {
-auto File = SM.
Sirraide wrote:
> The downside to it being in `TextDiagnostic` is that consumers then all have
> to normalize the path themselves (some file system APIs on some systems are
> better about relative paths than others). If the paths are always equivalent,
> it might be kinder to pass the resolved
@@ -738,12 +738,20 @@ void TextDiagnostic::printDiagnosticMessage(raw_ostream
&OS,
}
void TextDiagnostic::emitFilename(StringRef Filename, const SourceManager &SM)
{
-#ifdef _WIN32
- SmallString<4096> TmpFilename;
-#endif
- if (DiagOpts.AbsolutePath) {
-auto File = SM.
@@ -738,12 +738,20 @@ void TextDiagnostic::printDiagnosticMessage(raw_ostream
&OS,
}
void TextDiagnostic::emitFilename(StringRef Filename, const SourceManager &SM)
{
-#ifdef _WIN32
- SmallString<4096> TmpFilename;
-#endif
- if (DiagOpts.AbsolutePath) {
-auto File = SM.
@@ -738,12 +738,20 @@ void TextDiagnostic::printDiagnosticMessage(raw_ostream
&OS,
}
void TextDiagnostic::emitFilename(StringRef Filename, const SourceManager &SM)
{
-#ifdef _WIN32
- SmallString<4096> TmpFilename;
-#endif
- if (DiagOpts.AbsolutePath) {
-auto File = SM.
AaronBallman wrote:
> I’m not exactly sure how to test this change since this is not only
> platform-dependent but also path-dependent since we may end up producing
> absolute paths here.
I think this is a case where maybe we want to use unit tests. We have
`clang/unittests/Basic/DiagnosticTe
https://github.com/Sirraide edited
https://github.com/llvm/llvm-project/pull/143520
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Sirraide wrote:
I’m not exactly sure how to test this change since this is not only
platform-dependent but also path-dependent since we may end up producing
absolute paths here.
https://github.com/llvm/llvm-project/pull/143520
___
cfe-commits mailing
https://github.com/Sirraide updated
https://github.com/llvm/llvm-project/pull/143520
>From 15c0a79d6a0cd65d88fbe152275b224201e632a1 Mon Sep 17 00:00:00 2001
From: Sirraide
Date: Tue, 10 Jun 2025 14:32:32 +0200
Subject: [PATCH 1/2] [Clang] [Diagnostics] Simplify filenames that contain
'..'
---
27 matches
Mail list logo
