diff options
author | Jan Svoboda <jan_svoboda@apple.com> | 2025-05-22 12:33:52 -0700 |
---|---|---|
committer | GitHub <noreply@github.com> | 2025-05-22 12:33:52 -0700 |
commit | 9e306ad4600c4d3392c194a8be88919ee758425c (patch) | |
tree | f8b15171e92d056e9006bcbe36d534a9d73155d9 /clang/unittests/Interpreter/InterpreterTest.cpp | |
parent | 2b8bff6f66fd90ac658d0ae0d7f9a83ffadfd77f (diff) | |
download | llvm-9e306ad4600c4d3392c194a8be88919ee758425c.zip llvm-9e306ad4600c4d3392c194a8be88919ee758425c.tar.gz llvm-9e306ad4600c4d3392c194a8be88919ee758425c.tar.bz2 |
[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.
Diffstat (limited to 'clang/unittests/Interpreter/InterpreterTest.cpp')
-rw-r--r-- | clang/unittests/Interpreter/InterpreterTest.cpp | 15 |
1 files changed, 9 insertions, 6 deletions
diff --git a/clang/unittests/Interpreter/InterpreterTest.cpp b/clang/unittests/Interpreter/InterpreterTest.cpp index 03b9c33..b97f5ae 100644 --- a/clang/unittests/Interpreter/InterpreterTest.cpp +++ b/clang/unittests/Interpreter/InterpreterTest.cpp @@ -95,8 +95,9 @@ TEST_F(InterpreterTest, Errors) { // Create the diagnostic engine with unowned consumer. std::string DiagnosticOutput; llvm::raw_string_ostream DiagnosticsOS(DiagnosticOutput); - auto DiagPrinter = std::make_unique<TextDiagnosticPrinter>( - DiagnosticsOS, new DiagnosticOptions()); + DiagnosticOptions DiagOpts; + auto DiagPrinter = + std::make_unique<TextDiagnosticPrinter>(DiagnosticsOS, DiagOpts); auto Interp = createInterpreter(ExtraArgs, DiagPrinter.get()); auto Err = Interp->Parse("intentional_error v1 = 42; ").takeError(); @@ -126,8 +127,9 @@ TEST_F(InterpreterTest, DeclsAndStatements) { // Create the diagnostic engine with unowned consumer. std::string DiagnosticOutput; llvm::raw_string_ostream DiagnosticsOS(DiagnosticOutput); - auto DiagPrinter = std::make_unique<TextDiagnosticPrinter>( - DiagnosticsOS, new DiagnosticOptions()); + DiagnosticOptions DiagOpts; + auto DiagPrinter = + std::make_unique<TextDiagnosticPrinter>(DiagnosticsOS, DiagOpts); auto Interp = createInterpreter(ExtraArgs, DiagPrinter.get()); auto R1 = Interp->Parse( @@ -148,8 +150,9 @@ TEST_F(InterpreterTest, UndoCommand) { // Create the diagnostic engine with unowned consumer. std::string DiagnosticOutput; llvm::raw_string_ostream DiagnosticsOS(DiagnosticOutput); - auto DiagPrinter = std::make_unique<TextDiagnosticPrinter>( - DiagnosticsOS, new DiagnosticOptions()); + DiagnosticOptions DiagOpts; + auto DiagPrinter = + std::make_unique<TextDiagnosticPrinter>(DiagnosticsOS, DiagOpts); auto Interp = createInterpreter(ExtraArgs, DiagPrinter.get()); |