9 Commits

Author SHA1 Message Date
James Y Knight
c7f3437507
NFC: Clean up of IntrusiveRefCntPtr construction from raw pointers. (#151545)
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
2025-07-31 15:07:35 -04:00
Jan Svoboda
13e1a2cb22 Reapply "[clang] Remove intrusive reference count from DiagnosticOptions (#139584)"
This reverts commit e2a885537f11f8d9ced1c80c2c90069ab5adeb1d. Build failures were fixed right away and reverting the original commit without the fixes breaks the build again.
2025-05-22 12:52:03 -07:00
Kazu Hirata
e2a885537f Revert "[clang] Remove intrusive reference count from DiagnosticOptions (#139584)"
This reverts commit 9e306ad4600c4d3392c194a8be88919ee758425c.

Multiple builtbot failures have been reported:
https://github.com/llvm/llvm-project/pull/139584
2025-05-22 12:44:20 -07:00
Jan Svoboda
9e306ad460
[clang] Remove intrusive reference count from DiagnosticOptions (#139584)
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.
2025-05-22 12:33:52 -07:00
Jan Svoboda
b69dcb8734
[clang][frontend] Require invocation to construct CompilerInstance (#137668)
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).
2025-05-01 07:31:30 -07:00
Kadir Cetinkaya
df9a14d7bb
Reapply "[NFC] Explicitly pass a VFS when creating DiagnosticsEngine (#115852)"
This reverts commit a1153cd6fedd4c906a9840987934ca4712e34cb2 with fixes
to lldb breakages.

Fixes https://github.com/llvm/llvm-project/issues/117145.
2024-11-21 14:55:30 +01:00
Sylvestre Ledru
a1153cd6fe Revert "[NFC] Explicitly pass a VFS when creating DiagnosticsEngine (#115852)"
Reverted for causing:
https://github.com/llvm/llvm-project/issues/117145

This reverts commit bdd10d9d249bd1c2a45e3de56a5accd97e953458.
2024-11-21 13:04:30 +01:00
kadir çetinkaya
bdd10d9d24
[NFC] Explicitly pass a VFS when creating DiagnosticsEngine (#115852)
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.
2024-11-21 12:11:41 +01:00
Chuanqi Xu
49775b1dc0
[Serialization] Record whether the ODR is skipped (#82302)
Close https://github.com/llvm/llvm-project/issues/80570.

In

a0b6747804,
we skipped ODR checks for decls in GMF. Then it should be natural to
skip storing the ODR values in BMI.

Generally it should be fine as long as the writer and the reader keep
consistent.

However, the use of preamble in clangd shows the tricky part.

For,

```
// test.cpp
module;

// any one off these is enough to crash clangd
// #include <iostream>
// #include <string_view>
// #include <cmath>
// #include <system_error>
// #include <new>
// #include <bit>
// probably many more

// only ok with libc++, not the system provided libstdc++ 13.2.1

// these are ok

export module test;
```

clangd will store the headers as preamble to speedup the parsing and the
preamble reuses the serialization techniques. (Generally we'd call the
preamble as PCH. However it is not true strictly. I've tested the PCH
wouldn't be problematic.) However, the tricky part is that the preamble
is not modules. It literally serialiaze and deserialize things. So
before clangd parsing the above test module, clangd will serialize the
headers into the preamble. Note that there is no concept like GMF now.
So the ODR bits are stored. However, when clangd parse the file
actually, the decls from preamble are thought as in GMF literally, then
hte ODR bits are skipped. Then mismatch happens.

To solve the problem, this patch adds another bit for decls to record
whether or not the ODR bits are skipped.
2024-02-20 13:31:28 +08:00