This is a major change on how we represent nested name qualifications in
the AST.
* The nested name specifier itself and how it's stored is changed. The
prefixes for types are handled within the type hierarchy, which makes
canonicalization for them super cheap, no memory allocation required.
Also translating a type into nested name specifier form becomes a no-op.
An identifier is stored as a DependentNameType. The nested name
specifier gains a lightweight handle class, to be used instead of
passing around pointers, which is similar to what is implemented for
TemplateName. There is still one free bit available, and this handle can
be used within a PointerUnion and PointerIntPair, which should keep
bit-packing aficionados happy.
* The ElaboratedType node is removed, all type nodes in which it could
previously apply to can now store the elaborated keyword and name
qualifier, tail allocating when present.
* TagTypes can now point to the exact declaration found when producing
these, as opposed to the previous situation of there only existing one
TagType per entity. This increases the amount of type sugar retained,
and can have several applications, for example in tracking module
ownership, and other tools which care about source file origins, such as
IWYU. These TagTypes are lazily allocated, in order to limit the
increase in AST size.
This patch offers a great performance benefit.
It greatly improves compilation time for
[stdexec](https://github.com/NVIDIA/stdexec). For one datapoint, for
`test_on2.cpp` in that project, which is the slowest compiling test,
this patch improves `-c` compilation time by about 7.2%, with the
`-fsyntax-only` improvement being at ~12%.
This has great results on compile-time-tracker as well:
This patch also further enables other optimziations in the future, and
will reduce the performance impact of template specialization resugaring
when that lands.
It has some other miscelaneous drive-by fixes.
About the review: Yes the patch is huge, sorry about that. Part of the
reason is that I started by the nested name specifier part, before the
ElaboratedType part, but that had a huge performance downside, as
ElaboratedType is a big performance hog. I didn't have the steam to go
back and change the patch after the fact.
There is also a lot of internal API changes, and it made sense to remove
ElaboratedType in one go, versus removing it from one type at a time, as
that would present much more churn to the users. Also, the nested name
specifier having a different API avoids missing changes related to how
prefixes work now, which could make existing code compile but not work.
How to review: The important changes are all in
`clang/include/clang/AST` and `clang/lib/AST`, with also important
changes in `clang/lib/Sema/TreeTransform.h`.
The rest and bulk of the changes are mostly consequences of the changes
in API.
PS: TagType::getDecl is renamed to `getOriginalDecl` in this patch, just
for easier to rebasing. I plan to rename it back after this lands.
Fixes#136624
Fixes https://github.com/llvm/llvm-project/issues/43179
Fixes https://github.com/llvm/llvm-project/issues/68670
Fixes https://github.com/llvm/llvm-project/issues/92757
This commit handles the following types:
- clang::ExternalASTSource
- clang::TargetInfo
- clang::ASTContext
- clang::SourceManager
- clang::FileManager
Part of cleanup #151026
Handles clang::DiagnosticsEngine and clang::DiagnosticIDs.
For DiagnosticIDs, this mostly migrates from `new DiagnosticIDs` to
convenience method `DiagnosticIDs::create()`.
Part of cleanup https://github.com/llvm/llvm-project/issues/151026
This switches to `makeIntrusiveRefCnt<FileSystem>` where creating a new
object, and to passing/returning by `IntrusiveRefCntPtr<FileSystem>`
instead of `FileSystem*` or `FileSystem&`, when dealing with existing
objects.
Part of cleanup #151026.
Add `NamespaceBaseDecl` as common base class of `NamespaceDecl` and
`NamespaceAliasDecl`. This simplifies `NestedNameSpecifier` a bit.
Co-authored-by: Matheus Izvekov <mizvekov@gmail.com>
We noticed that when a directory's content changes, its size reported by
`status` can change as well. We do not want to include such "false
positive" cases. This PR revises the computation so that only regular
file size changes are considered out-of-date.
rdar://152247357
`DependencyScanningFileSystemSharedCache` can currently report
out-of-date negatively stat cached paths. This PR enhances the reporting
with two modifications.
1. The reported path are now null terminated char arrays instead of
`StringRef`s. This way the API's user can avoid copying `StringRef`s to
other containers because the char arrays can be used directly.
2. The API now reports out-of-date cache entry due to file size changes.
Specifically, we check each file's cached size against the size of the
same file on the underlying FS. If the sizes are different, diagnostics
will be reported.
rdar://152247357
While the source code isn't supposed to change during a build, in some
environments it does. This adds an option that disables caching of stat
failures, meaning that source files can be added to the build during
scanning.
This adds a `-no-cache-negative-stats` option to clang-scan-deps to
enable this behavior. There are no tests for clang-scan-deps as there's
no reliable way to do so from it. A unit test has been added that
modifies the filesystem between scans to test it.
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.
Use `getParenOrBraceRange()` to get the location of the opening paren or
braces instead of searching backwards from the first argument.
For implicit construction, get the range surrounding the first and last
arguments.
Recently commit dc17429ae6 removed some code related to template
arguments in NestedNameSpecifier::print that would not pass on the
TemplateParameterList. This would cause printIntegral to add type
suffixes for the unsigned parameter, but only for the prefix. Add a
regression test to prevent such problems from coming back.
This PR makes it so that `CompilerInvocation` needs to be provided to
`CompilerInstance` on construction. There are a couple of benefits in my
view:
* Making it impossible to mis-use some `CompilerInstance` APIs. For
example there are cases, where `createDiagnostics()` was called before
`setInvocation()`, causing the `DiagnosticEngine` to use the
default-constructed `DiagnosticOptions` instead of the intended ones.
* This shrinks `CompilerInstance`'s state space.
* This makes it possible to access **the** invocation in
`CompilerInstance`'s constructor (to be used in a follow-up).
We have had numerous situations where the negatively stat cached paths
are invalidated during the build, and such invalidations lead to build
errors.
This PR adds an API to diagnose such cases.
`DependencyScanningFilesystemSharedCache::diagnoseNegativeStatCachedPaths`
allows users of the cache to ask the cache to examine all negatively
stat cached paths, and re-stat the paths using the passed-in file
system. If any re-stat succeeds, the API emits diagnostics.
rdar://149147920
Before this change the code used to add extra qualifiers, e.g.
`std::unique_ptr<int> _Nonnull` became `::std::std::unique_ptr<int>
_Nonnull`
when adding a global namespace qualifier was requested.
This reapplies 5ffd9bdb50b57 (#133545) with fixes.
The BUILD_SHARED_LIBS=ON build was fixed by adding missing LLVM
dependencies to the InterpTests binary in
unittests/AST/ByteCode/CMakeLists.txt .
Pass all the dependencies into add_clang_unittest. This is consistent
with how it is done for LLDB. I borrowed the same named argument list
structure from add_lldb_unittest. This is a necessary step towards
consolidating unit tests into fewer binaries, but seems like a good
refactoring in its own right.
When using the clang dependency scanner with an arbitrary
DiagnosticConsumer, it is important that we always call finish().
Previously, if there was an error preventing us from reaching the
scanning action, or if the command line contained no scannable actions
we would fail to finish(), which would break some consumers (e.g.
serialized diag consumer).
In preparation of making `-Wreturn-type` default to an error (as there
is virtually no situation where you’d *want* to fall off the end of a
function that is supposed to return a value), this patch fixes tests
that have relied on this being only a warning, of which there seem
to be 3 kinds:
1. Tests which for no apparent reason have a function that triggers the
warning.
I suspect that a lot of these were on accident (or from before the
warning was introduced), since a lot of people will open issues w/ their
problematic code in the `main` function (which is the one case where you
don’t need to return from a non-void function, after all...), which
someone will then copy, possibly into a namespace, possibly renaming it,
the end result of that being that you end up w/ something that
definitely is not `main` anymore, but which still is declared as
returning `int`, and which still has no return statement (another reason
why I think this might apply to a lot of these is because usually the
actual return type of such problematic functions is quite literally
`int`).
A lot of these are really old tests that don’t use `-verify`, which is
why no-one noticed or had to care about the extra warning that was
already being emitted by them until now.
2. Tests which test either `-Wreturn-type`, `[[noreturn]]`, or what
codegen and sanitisers do whenever you do fall off the end of a
function.
3. Tests where I struggle to figure out what is even being tested
(usually because they’re Objective-C tests, and I don’t know
Objective-C), whether falling off the end of a function matters in the
first place, and tests where actually spelling out an expression to
return would be rather cumbersome (e.g. matrix types currently don’t
support list initialisation, so I can’t write e.g. `return {}`).
For tests that fall into categories 2 and 3, I just added
`-Wno-error=return-type` to the `RUN` lines and called it a day. This
was especially necessary for the former since `-Wreturn-type` is an
analysis-based warning, meaning that it is currently impossible to test
for more than one occurrence of it in the same compilation if it
defaults to an error since the analysis pass is skipped for subsequent
functions as soon as an error is emitted.
I’ve also added `-Werror=return-type` to a few tests that I had already
updated as this patch was previously already making the warning an error
by default, but we’ve decided to split that into two patches instead.
Starting with 41e3919ded78d8870f7c95e9181c7f7e29aa3cc4 DiagnosticsEngine
creation might perform IO. It was implicitly defaulting to
getRealFileSystem. This patch makes it explicit by pushing the decision
making to callers.
It uses ambient VFS if one is available, and keeps using
`getRealFileSystem` if there aren't any VFS.
This pr refactors most tests that use RAV to use DRAV instead; this also
has the nice effect of testing both the RAV and DRAV implementations at
the same time w/o having to duplicate all of our AST visitor tests.
Some tests rely on features that DRAV doesn’t support (mainly post-order
traversal), so those haven’t been migrated. At the same time,
`TestVisitor` is now a DRAV, so I’ve had to introduce a new
`CTRPTestVisitor` for any tests that need to use RAV directly.
Adds a def file to have a single location where tested language versions
are specified. Removes the need to update multiple locations in the
testing infrastructure to add a new language version to be tested. Test
instatiation can now include all languages without needing to specify
them.
This patch also adds pretty printing for instantiated test names. That
means, that a test instantiated with C++23 will have the name
`...TestSuite/TestName/CXX23` instead ending with some number (index of
the argument for instantiation of the test), which provides a better
experience when encountering a test failure with a specific language
version. The suffix will also contain an `_win` if the target contains
`win`.
---------
Co-authored-by: Sirraide <aeternalmail@gmail.com>
This patch adds an IsText parameter to the following functions
openFileForRead, getBufferForFile, getBufferForFileImpl and determines
whether a file is text by querying the file tag on z/OS. The default is
set to OF_Text instead of OF_None, this change in value does not affect
any other platforms other than z/OS.
LLVM-based tools often use the `llvm::vfs::FileSystem` instrastructure
to access the file system. This patch adds new kind of a VFS that
performs lightweight tracing of file system operations on an underlying
VFS. This is supposed to aid in investigating file system traffic
without resorting to instrumentation on the operating system level.
There will be follow-up patches that integrate this into Clang and its
dependency scanner.
The scanning VFS doesn't cache stat failures of paths with no extension.
This was originally implemented to avoid caching the non-existence of
the modules cache directory that the modular scanner will eventually
create if it does not exist.
However, this prevents caching of the non-existence of all directories
and notably also header files from the standard C++ library, which can
lead to sub-par performance.
This patch adds an API to the scanning VFS that allows clients to
configure path prefix for which to bypass the scanning VFS and use the
underlying VFS directly.
Emission status seems to be only used by cuda/openmp/hip compiles, to
figure out
when to emit diagnostics. Current logic emits "uknown" when definition
is
missing, so i extended that to skipped-function-bodies as well.
Since `raw_string_ostream` doesn't own the string buffer, it is
desirable (in terms of memory safety) for users to directly reference
the string buffer rather than use `raw_string_ostream::str()`.
Work towards TODO item to remove `raw_string_ostream::str()`.
