We started seeing some use-after-frees starting with
https://github.com/llvm/llvm-project/pull/125433.
This patch ensures we explicitly block for the async task, if there's
one, before destructing `std::future`, independent of the stdlib
implementation.
The new config option is a more flexible version of
--function-arg-placeholders, allowing users more detailed control of
what is inserted in argument list position when clangd completes the
name of a function in a function call context.
Fixes https://github.com/llvm/llvm-project/issues/63565
Co-authored-by: MK-Alias <ImNotReadingThis@maininator.com>
Alternatives to https://reviews.llvm.org/D153114.
Try to address https://github.com/clangd/clangd/issues/1293.
See the links for design ideas and the consensus so far. We want to have
some initial support in clang18.
This is the initial support for C++20 Modules in clangd.
As suggested by sammccall in https://reviews.llvm.org/D153114,
we should minimize the scope of the initial patch to make it easier
to review and understand so that every one are in the same page:
> Don't attempt any cross-file or cross-version coordination: i.e. don't
> try to reuse BMIs between different files, don't try to reuse BMIs
> between (preamble) reparses of the same file, don't try to persist the
> module graph. Instead, when building a preamble, synchronously scan
> for the module graph, build the required PCMs on the single preamble
> thread with filenames private to that preamble, and then proceed to
> build the preamble.
This patch reflects the above opinions.
# Testing in real-world project
I tested this with a modularized library:
https://github.com/alibaba/async_simple/tree/CXX20Modules. This library
has 3 modules (async_simple, std and asio) and 65 module units. (Note
that a module consists of multiple module units). Both `std` module and
`asio` module have 100k+ lines of code (maybe more, I didn't count). And
async_simple itself has 8k lines of code. This is the scale of the
project.
The result shows that it works pretty well, ..., well, except I need to
wait roughly 10s after opening/editing any file. And this falls in our
expectations. We know it is hard to make it perfect in the first move.
# What this patch does in detail
- Introduced an option `--experimental-modules-support` for the support
for C++20 Modules. So that no matter how bad this is, it wouldn't affect
current users. Following off the page, we'll assume the option is
enabled.
- Introduced two classes `ModuleFilesInfo` and
`ModuleDependencyScanner`. Now `ModuleDependencyScanner` is only used by
`ModuleFilesInfo`.
- The class `ModuleFilesInfo` records the built module files for
specific single source file. The module files can only be built by the
static member function `ModuleFilesInfo::buildModuleFilesInfoFor(PathRef
File, ...)`.
- The class `PreambleData` adds a new member variable with type
`ModuleFilesInfo`. This refers to the needed module files for the
current file. It means the module files info is part of the preamble,
which is suggested in the first patch too.
- In `isPreambleCompatible()`, we add a call to
`ModuleFilesInfo::CanReuse()` to check if the built module files are
still up to date.
- When we build the AST for a source file, we will load the built module
files from ModuleFilesInfo.
# What we need to do next
Let's split the TODOs into clang part and clangd part to make things
more clear.
The TODOs in the clangd part include:
1. Enable reusing module files across source files. The may require us
to bring a ModulesManager like thing which need to handle `scheduling`,
`the possibility of BMI version conflicts` and `various events that can
invalidate the module graph`.
2. Get a more efficient method to get the `<module-name> ->
<module-unit-source>` map. Currently we always scan the whole project
during `ModuleFilesInfo::buildModuleFilesInfoFor(PathRef File, ...)`.
This is clearly inefficient even if the scanning process is pretty fast.
I think the potential solutions include:
- Make a global scanner to monitor the state of every source file like I
did in the first patch. The pain point is that we need to take care of
the data races.
- Ask the build systems to provide the map just like we ask them to
provide the compilation database.
3. Persist the module files. So that we can reuse module files across
clangd invocations or even across clangd instances.
TODOs in the clang part include:
1. Clang should offer an option/mode to skip writing/reading the bodies
of the functions. Or even if we can requrie the parser to skip parsing
the function bodies.
And it looks like we can say the support for C++20 Modules is initially
workable after we made (1) and (2) (or even without (2)).
This avoids a known libFormat bug where the heuristic can OOM on certain
large files (particularly single-header libraries such as miniaudio.h).
The OOM will still happen on affected files if you actually try to
format them (this is harder to avoid since the underlyting issue affects
the actual formatting logic, not just the language-guessing heuristic),
but at least it's avoided during non-modifying operations like hover,
and modifying operations that do local formatting like code completion.
Fixes https://github.com/clangd/clangd/issues/719
Fixes https://github.com/clangd/clangd/issues/1384
Fixes https://github.com/llvm/llvm-project/issues/70945
This patch replaces uses of StringRef::{starts,ends}with with
StringRef::{starts,ends}_with for consistency with
std::{string,string_view}::{starts,ends}_with in C++20.
I'm planning to deprecate and eventually remove
StringRef::{starts,ends}with.
`clang::runWithSufficientStackSpace` requires the address of the
initial stack bottom to prevent potential stack overflows.
In addition, add a fallback to ASTFrontendAction in case any client
forgets to call it when not through CompilerInstance::ExecuteAction,
which is rare.
Fixes https://github.com/clangd/clangd/issues/1745.
Reviewed By: sammccall
Differential Revision: https://reviews.llvm.org/D158967
This reverts commit 92c0546941190973e9201a08fa10edf27cb6992d.
Prevents tsan issues by resetting ref-counted-pointers eagerly, before
passing the copies into a new thread.
/Users/jiefu/llvm-project/clang-tools-extra/clangd/ClangdServer.cpp:167:32: error: private field 'Opts' is not used [-Werror,-Wunused-private-field]
const ClangdServer::Options &Opts;
^
1 error generated.
This has been the default in our production setup for weeks now,
showing great improvements to latency and no problems around stability or
correctness of the results.
Differential Revision: https://reviews.llvm.org/D155619
- No longer store the diagnostic fixits in the clangdLSPServer
- When propagating the fixit via the code action, we use the Diag
information stored in the ParsedAST (in clangdServer.cpp)
Differential Revision: https://reviews.llvm.org/D155173
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
We would like to move the preamble index out of the critical path.
This patch is an RFC to get feedback on the correct implementation and potential pitfalls to keep into consideration.
I am not entirely sure if the lazy AST initialisation would create using Preamble AST in parallel. I tried with tsan enabled clangd but it seems to work OK (at least for the cases I tried)
Reviewed By: kadircet
Differential Revision: https://reviews.llvm.org/D148088
These are still disabled by default, but will work in ObjC code if you
enable the `-import-insertions` flag.
Completion requires ASTSignals to be available; before ASTSignals are
available, we will always use #include. Once they are available, the
behavior varies as follows:
- For source files, use #import if the ObjC language flag is enabled
- For header files:
- If the ObjC language flag is disabled, use #include
- If the header file contains any #imports, use #import
- If the header file references any ObjC decls, use #import
- Otherwise, use #include
IncludeFixer support is similar, but it does not rely upon ASTSignals,
instead it does the above checks excluding the scan for ObjC symbols.
Differential Revision: https://reviews.llvm.org/D139458
In principle it's OK for stdlib-indexing tasks to run after the TUScheduler is
destroyed, as mostly they just update the dynamic index. We do drain the
stdlib-indexing queue before destroying the index.
However the task captures references to the PreambleCallbacks object, which is
owned by the TUScheduler. Once this is destroyed (explicitly, early in
~ClangdServer) an outstanding stdlib-indexing task may use-after-free.
The fix here is to avoid capturing references to the PreambleCallbacks.
Alternatives would be to have TUScheduler (exclusively) not own its callbacks
so they could live longer, or explicitly stopping the TUScheduler instead of
early-destroying it. These both seem more invasive.
See https://reviews.llvm.org/D115232 for some more context.
Differential Revision: https://reviews.llvm.org/D140486
std::optional::value() has undesired exception checking semantics and is
unavailable in older Xcode (see _LIBCPP_AVAILABILITY_BAD_OPTIONAL_ACCESS). The
call sites block std::optional migration.
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
This is mostly a mechanical change to adapt standard type hierarchy
support proposed in LSP 3.17 on top of clangd's existing extension support.
This does mainly two things:
- Incorporate symbolids for all the parents inside resolution parameters, so
that they can be retrieved from index later on. This is a new code path, as
extension always resolved them eagerly.
- Propogate parent information when resolving children, so that at least one
branch of parents is always preserved. This is to address a shortcoming in the
extension.
This doesn't drop support for the extension, but it's deprecated from now on and
will be deleted in upcoming releases. Currently we use the same struct
internally but don't serialize extra fields.
Fixes https://github.com/clangd/clangd/issues/826.
Differential Revision: https://reviews.llvm.org/D131385
Building preambles is the most resource-intensive thing clangd does, driving
peak RAM and sustained CPU usage.
In a hosted environment where multiple clangd instances are packed into the same
container, it's useful to be able to limit the *aggregate* resource peaks.
Differential Revision: https://reviews.llvm.org/D129100
