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
The `ConceptReference`'s `FoundDecl` claims it "can differ from
`NamedConcept` when, for example, the concept was found through a
`UsingShadowDecl`", but such the contract was not previously respected.
Fixes https://github.com/llvm/llvm-project/issues/82628
We used to consider the `DeclContext` for selection nodes inside a
lambda as the enclosing scope of the lambda expression, rather than the
lambda itself.
For example,
```cpp
void foo();
auto lambda = [] {
return ^foo();
};
```
where `N` is the selection node for the expression `foo()`,
`N.getDeclContext()` returns the `TranslationUnitDecl` previously, which
IMO is wrong, since the method `operator()` of the lambda is closer.
Incidentally, this fixes a glitch in add-using-declaration tweaks.
(Thanks @HighCommander4 for the test case.)
Now we can store it in DynTypedNode, we can target these nodes
(SelectionTree) and resolve them (FindTarget).
This makes Hover, go-to-def etc work in all(?) cases.
Also support it in DumpAST.
Differential Revision: https://reviews.llvm.org/D159299
We've been running this internally for months now, without any
stability or correctness concerns. It has ~40% speed up on incremental
diagnostics latencies (as preamble can get invalidated through code completion
etc.).
Differential Revision: https://reviews.llvm.org/D153882
This reverts commit e70ca7b35319a3621f9d9c6475926428f8c5c000 and the
followup patch "[clang] Fix the location of UsingTypeLoc"
(ebbeb164c25a40cb6ba9c6b18dce5dcd06c0bb07).
The patch causes an incorrect lookup result:
```
namespace ns { struct Foo { };}
using ns::Foo;
void test() {
struct Foo {
} k; // the type of k refers to ns::Foo, rather than the local Foo!
}
```
It is revealed by the https://reviews.llvm.org/D141280.
```
namespace ns { class Foo {}; }
using ns::Foo;
// Before the fix, the Location of UsingTypeLoc Foo points to the
token "class", slection on ^Foo will result in the VarDecl abc.
class Foo abc;
```
Differential Revision: https://reviews.llvm.org/D142125
This behavior was once deliberate, but i've yet to find someone who likes it.
The reference behavior is unchanged: the `foo` within ~foo is still considered
a reference to the type. This means rename etc still works.
fixes https://github.com/clangd/clangd/issues/179
Differential Revision: https://reviews.llvm.org/D136212
- store NestedNameSpecifier & Loc for the qualifiers
This information was entirely missing from the AST.
- expose the location information for qualifier/identifier/typedefs as typeloc
This allows many traversals/astmatchers etc to handle these generically along
with other references. The decl vs type split can help preserve typedef
sugar when https://github.com/llvm/llvm-project/issues/57659 is resolved.
- fix the SourceRange of UsingEnumDecl to include 'using'.
Fixes https://github.com/clangd/clangd/issues/1283
Differential Revision: https://reviews.llvm.org/D134303
isBeforeInTranslationUnit compares SourceLocations across FileIDs by
mapping them onto a common ancestor file, following include/expansion edges.
It is possible to get a tie in the common ancestor, because multiple
"chunks" of a macro arg will expand to the same macro param token in the body:
#define ID(X) X
#define TWO 2
ID(1 TWO)
Here two FileIDs both expand into `X` in ID's expansion:
- one containing `1` and spelled on line 3
- one containing `2` and spelled by the macro expansion of TWO
isBeforeInTranslationUnit breaks this tie by comparing the two FileIDs:
the one "on the left" is always created first and is numerically smaller.
This seems correct so far.
Prior to this patch it also takes a shortcut (unclear if intentionally).
Instead of comparing the two FileIDs that directly expand to the same location,
it compares the original FileIDs being compared. These may not be the
same if there are multiple macro expansions in between.
This *almost* always yields the right answer, because macro expansion
yields "trees" of FileIDs allocated in a contiguous range: when comparing tree A
to tree B, it doesn't matter what representative you pick.
However, the splitting of >> tokens is modeled as macro expansion (as if
the first '>' was a macro that expands to a '>' spelled a scratch buffer).
This splitting occurs retroactively when parsing, so the FileID allocated is
larger than expected if it were a real macro expansion performed during lexing.
As a result, macro tree A can be on the left of tree B, and yet contain
a token-split FileID whose numeric value is *greator* than those in B.
In this case the tiebreak gives the wrong answer.
Concretely:
#define ID(X) X
template <typename> class S{};
ID(
ID(S<S<int>> x);
int y;
)
Given Greater = (typeloc of S<int>).getEndLoc();
Y = (decl of y).getLocation();
isBeforeInTranslationUnit(Greater, Y) should return true, but returns false.
Here the common FileID of (Greater, Y) is the body of the outer ID
expansion, and they both expand to X within it.
With the current tiebreak rules, we compare the FileID of Greater (a split)
to the FileID of Y (a macro arg expansion into X of the outer ID).
The former is larger because the token split occurred relatively late.
This patch fixes the issue by removing the shortcut. It tracks the immediate
FileIDs used to reach the common file, and uses these IDs to break ties.
In the example, we now compare the macro arg expansion of the inner ID()
to the macro arg expansion of Y, and find that it is smaller.
This requires some changes to the InBeforeInTUCacheEntry (sic).
We store a little more data so it's probably slightly slower.
It was difficult to resist more invasive changes:
- performance: the sizing is very suspicious, and once the cache "fills up"
we're thrashing a single entry
- API: the class seems to be needlessly complicated
However I tried to avoid mixing these with subtle behavior changes, and
will send a followup instead.
Differential Revision: https://reviews.llvm.org/D134685
17 vs 14 have different ASTs, this causes D131465 to have to touch this test.
While here, make sure we're being clear about *which* nodes we're matching.
Differential Revision: https://reviews.llvm.org/D133423
Expose these as variables as that's what the standard calls them (and D131175).
To make this work, we also fix a bug in SelectionTree: PredefinedExpr has
an implicit/invisible StringLiteral, and SelectionTree should not traverse
implicit things.
Reviewed By: ckandeler
Differential Revision: https://reviews.llvm.org/D132135
The patch was reverted because it caused a crash during PCH build -- we
missed to update the RParenLoc in TreeTransform<Derived>::TransformAutoType.
This relands 55d96ac and 37ec65e with a test and fix.
During pop() we convert nodes into spans of expanded syntax::Tokens.
If we precompute a range of plausible (expanded) tokens, then we can do an
extremely cheap approximate hit-test against it, because syntax::Tokens are
ordered by pointer.
This would seem not to buy anything (we don't enter nodes unless they overlap
the selection), but in fact the spans we have are for *newly* claimed ranges
(i.e. those unclaimed by any child node).
So if you have:
{ { [[2+2]]; } }
then all of the CompoundStmts pass the hit test and are pushed, but we skip
full hit-testing of the brackets during pop() as they lie outside the range.
This is ~10x average speedup for selectiontree on a bad case I've seen
(large gtest file).
Differential Revision: https://reviews.llvm.org/D117107
The AST doesn't track their locations, and the default behavior of attributing
them to the lexically-enclosing node is sloppy and often inaccurate.
Also add a couple of passing test cases for declarators that weren't obvious.
Differential Revision: https://reviews.llvm.org/D117185
This breaks a clang-tidy check, needs to investigate and fix. Revert
them to bring the buildbot back.
This reverts commit 55d96ac3dc56bdebea854952a724c2a50d96ce19 and
37ec65e1d705f56fe5551de1dfcbac1e071588a2
Because declarators nest inside-out, we logically need to claim tokens for
parent declarators logically before child ones.
This is the ultimate reason we had problems with DeclaratorDecl, ArrayType etc.
However actually changing the order of traversal is hard, especially for nodes
that have both declarator and non-declarator children.
Since there's only a few TypeLocs corresponding to declarators, we just
have them claim the exact tokens rather than rely on nesting.
This fixes handling of complex declarators, like
`int (*Fun(OuterT^ype))(InnerType);`.
This avoids the need for the DeclaratorDecl early-claim hack, which is
removed.
Unfortunately the DeclaratorDecl early-claims were covering up an AST
anomaly around CXXConstructExpr, so we need to fix that up too.
Based on D116623 and D116618
Differential Revision: https://reviews.llvm.org/D116630
Previously, it was in canSafelySkipNode, which is only used to decide
whether we should descend into it and its children, and we still used
the incomplete Decltypeloc.getSourceRange() to claim tokens, which will
cause some tokens were not claimed correctly.
Separate a change of https://reviews.llvm.org/D116536
Reviewed By: sammccall
Differential Revision: https://reviews.llvm.org/D116586
This involves separating out the concepts of "which tokens should we
descend into this node for" vs "which tokens should this node claim".
Reviewed By: sammccall
Differential Revision: https://reviews.llvm.org/D116218
This is important especially for code that tries to traverse scopes as
written in code, which is the contract SelectionTree tries to satisfy.
Differential Revision: https://reviews.llvm.org/D112712
These aren't terribly common, but we currently mishandle them badly.
Not only do we not recogize the attributes themselves, but we often end up
selecting some node other than the parent (because source ranges aren't accurate
in the presence of attributes).
Differential Revision: https://reviews.llvm.org/D89785
Implement initial support for pull-based diagnostics in ClangdServer.
This is planned for LSP 3.17, and initial proposal is in
d15eb0671e/protocol/src/common/proposed.diagnostic.ts (L111).
We chose to serve the requests only when clangd has a fresh preamble
available. In case of a stale preamble we just drop the request on the
floor.
This patch doesn't plumb this to LSP layer yet, as pullDiags is still a
proposal with only an implementation in vscode.
Differential Revision: https://reviews.llvm.org/D98623
Selection now includes the virtual and access modifier as part of their range for cxx base specifiers.
Reviewed By: sammccall
Differential Revision: https://reviews.llvm.org/D95231
Nullability annotations are implmented using attributes; previusly
clangd would skip over AttributedTypeLoc since their location
points to the attribute instead of the modified type.
Also add some test cases for this.
Differential Revision: https://reviews.llvm.org/D89579
This prevents selection of empty preprocessor entities (like #define directives,
or text in disabled sections) creating a selection in the parent element.
Summary: Based on D83508 by Aleksandr Platonov.
Reviewers: ArcsinX, kadircet
Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, usaxena95, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D84012
Summary:
Previously, the range for "->" CXXOperatorCallExpr is the range of the
class object (not including the operator!), e.g. "[[vector_ptr]]->size()".
This patch includes the range of the operator, which fixes the issue
where clangd doesn't go to the overloaded operator "->" definition.
Reviewers: sammccall
Reviewed By: sammccall
Subscribers: ilya-biryukov, jkorous, arphaman, kadircet, usaxena95, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D76128
Summary:
Selection tree was performing an early claim only for VarDecls, but
there are other cases where we can have declarators, e.g. FieldDecls. This patch
extends the early claim logic to all types of declarators.
Fixes https://github.com/clangd/clangd/issues/292
Reviewers: sammccall
Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, usaxena95, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D75106
This reverts commit b4b9706d5da368c81b86867b1c11a2e17b4472ac.
Now avoiding expected<vector<selection>> in favor of expected<vector<unique_ptr<selection>>>
This reverts commit a2ce807eb72a8e154abca09b1e968b2d99ba6933.
Buildbot failures on GCC due to SelectionTree not being copyable, and
instantiating vector<Selection> in the tweak-handling in ClangdServer.
This is how it should've been and brings it more in line with
std::string_view. There should be no functional change here.
This is mostly mechanical from a custom clang-tidy check, with a lot of
manual fixups. It uncovers a lot of minor inefficiencies.
This doesn't actually modify StringRef yet, I'll do that in a follow-up.
