Static analysis flagged the columns - 1 code, it was correct but the
assumption was not obvious. I document the assumption w/ assertions.
While digging through related code I found getColumnNumber that looks
wrong at first inspection and adding parentheses makes it clearer.
Revert llvm/llvm-project#143520 for now since it’s causing issues for
people who are using symlinks and prefer to preserve the original path
(i.e. looks like we’ll have to make this configurable after all; I just
need to figure out how to pass `-no-canonical-prefixes` down through the
driver); I’m planning to refactor this a bit and reland it in a few
days.
This can significantly shorten file paths to standard library headers,
e.g. on my system, `<ranges>` is currently printed as
```console
/usr/lib/gcc/x86_64-redhat-linux/15/../../../../include/c++/15/ranges
```
but with this change, we instead print
```console
/usr/include/c++/15/ranges
```
This is of course just a heuristic; there are paths that would get longer
as a result of this, so we use whichever path ends up being shorter.
@AaronBallman pointed out that this might be problematic for network
file systems since path resolution might take a while, so this is enabled
only for paths that are part of a local filesystem—though not on Windows
since there we noticed that the check itself is slow.
The file names are cached in `SourceManager`.
Introduce a type alias for the commonly used `std::pair<FileID,
unsigned>` to improve code readability, and make it easier for future
updates (64-bit source locations).
These are identified by misc-include-cleaner. I've filtered out those
that break builds. Also, I'm staying away from llvm-config.h,
config.h, and Compiler.h, which likely cause platform- or
compiler-specific build failures.
This reverts commit e2a885537f11f8d9ced1c80c2c90069ab5adeb1d. Build failures were fixed right away and reverting the original commit without the fixes breaks the build again.
The `DiagnosticOptions` class is currently intrusively
reference-counted, which makes reasoning about its lifetime very
difficult in some cases. For example, `CompilerInvocation` owns the
`DiagnosticOptions` instance (wrapped in `llvm::IntrusiveRefCntPtr`) and
only exposes an accessor returning `DiagnosticOptions &`. One would
think this gives `CompilerInvocation` exclusive ownership of the object,
but that's not the case:
```c++
void shareOwnership(CompilerInvocation &CI) {
llvm::IntrusiveRefCntPtr<DiagnosticOptions> CoOwner = &CI.getDiagnosticOptions();
// ...
}
```
This is a perfectly valid pattern that is being actually used in the
codebase.
I would like to ensure the ownership of `DiagnosticOptions` by
`CompilerInvocation` is guaranteed to be exclusive. This can be
leveraged for a copy-on-write optimization later on. This PR changes
usages of `DiagnosticOptions` across `clang`, `clang-tools-extra` and
`lldb` to not be intrusively reference-counted.
This manifested as an assertion failure in Clang built against libc++
with
hardening enabled (e.g.
-D_LIBCPP_HARDENING_MODE=_LIBCPP_HARDENING_MODE_DEBUG):
`libcxx/include/__memory/unique_ptr.h:596: assertion
__checker_.__in_bounds(std::__to_address(__ptr_), __i) failed:
unique_ptr<T[]>::operator[](index): index out of range`
Implements the fix proposed by Evgeny Eltsin on
https://github.com/llvm/llvm-project/pull/66514#issuecomment-1924039038.
No test case provided, since the bug is extremely sensitive to the
preprocessor
state (headers, macros, including the ones defined on command line), and
it
turned out to be non-trivial to create an isolated test.
Add some primitive syntax highlighting to our code snippet output.
This adds "checkpoints" to the Preprocessor, which we can use to start lexing from. When printing a code snippet, we lex from the nearest checkpoint and highlight the tokens based on their token type.
This reapplies ddbcc10b9e26b18f6a70e23d0611b9da75ffa52f, except for a tiny part that was reverted separately: 65331da0032ab4253a4bc0ddcb2da67664bd86a9. That will be reapplied later on, since it turned out to be more involved.
This commit is enabled by 5523fefb01c282c4cbcaf6314a9aaf658c6c145f and f0f548a65a215c450d956dbcedb03656449705b9, specifically the part that makes 'clang-tidy/checkers/misc/header-include-cycle.cpp' separator agnostic.
This commit replaces some calls to the deprecated `FileEntry::getName()` with `FileEntryRef::getName()` by swapping current usages of `SourceManager::getFileEntryForID()` with `SourceManager::getFileEntryRefForID()`. This lowers the number of usages of the deprecated `FileEntry::getName()` from 95 to 50.
When the caret location is lower than the lowest source range, clang is
printing wrong line numbers. The first line number should consider caret
location line even when there are source ranges provided.
Current wrong line example: https://godbolt.org/z/aj4qEjzs4
The first line of the code snippet we print is potentially lower than
the caret line, so handle that case.
Fixes#63524
Differential Revision: https://reviews.llvm.org/D153849
This commit fixes "TextDiagnostic::emitIncludeLocation" when compiling
with "-fdiagnostics-absolute-paths" flag enabled by emitting the absolute
path of the included file.
Fixes#63026
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D151833
... emitting them.
This makes later code easier to understand, since we emit the code
snippets line by line anyway.
It also fixes the weird underlinig of multi-line source ranges.
Differential Revision: https://reviews.llvm.org/D151215
... emitting them.
This makes later code easier to understand, since we emit the code
snippets line by line anyway.
It also fixes the weird underlinig of multi-line source ranges.
Differential Revision: https://reviews.llvm.org/D151215
Seems unnecessary to create a StringRef here just so we can drop the
trailing null bytes. We can do that with the std::string we
create anyway.
Differential Revision: https://reviews.llvm.org/D151300
Instead of creating a CaretLine the size of the SourceLine, just leave
it empty at first, let HighlightRange resize it to fit all the ~, then
resize it to fit the ^. Then we can save ourselves the work to remove
the trailing whitespace again.
Differential Revision: https://reviews.llvm.org/D151286
printWordWrapped() is only called in one place, which passes all
parameters except `Indentation`. So, remove that parameter and use its
default value instead. Also remove the other default parameter values,
since those are unneeded.
They were both only called from one place and did very similar things.
Merge them into one, so we only have to iterate the source line once to
generate the SourceMap.
Differential Revision: https://reviews.llvm.org/D151100
We don't use the offset returned from SourceManager::getDecomposedLoc
here, so we might as well just use getFileID().
Differential Revision: https://reviews.llvm.org/D151093
Don't try to minimize the times we invoke operator<< on the output
stream by keeping a ToPrint string around. Instead, just print the
characters as we iterate over them.
Differential Revision: https://reviews.llvm.org/D151075
Rename parameters and local variables and reorder things a bit to be
closer to their first point of use.
Differential Revision: https://reviews.llvm.org/D150840
Show line numbers to the left of diagnostic code snippets and increase
the numbers of lines shown from 1 to 16.
Differential Revision: https://reviews.llvm.org/D147875
Show line numbers to the left of diagnostic code snippets and increase
the numbers of lines shown from 1 to 16.
Differential Revision: https://reviews.llvm.org/D147875
This patch mechanically replaces None with std::nullopt where the
compiler would warn if None were deprecated. The intent is to reduce
the amount of manual work required in migrating from Optional to
std::optional.
This is part of an effort to migrate from llvm::Optional to
std::optional:
https://discourse.llvm.org/t/deprecating-llvm-optional-x-hasvalue-getvalue-getvalueor/63716
Adds `sarif` option to the existing `-fdiagnostics-format` flag
for intended future work with SARIF diagnostics. Currently issues a warning
against the use of diagnostics in SARIF mode, then defaults to clang style for
diagnostics.
Reviewed By: cjdb, denik, aaron.ballman
Differential Revision: https://reviews.llvm.org/D129886
- isValid: FileManager only ever returns valid FileEntries (see next point)
- construction from outside FileManager (both FileEntry and DirectoryEntry).
It's not possible to create a useful FileEntry this way, there are no setters.
This was only used in FileEntryTest, added a friend to enable this.
A real constructor is cleaner but requires larger changes to FileManager.
Differential Revision: https://reviews.llvm.org/D123197