[clang] [clang-tools-extra] Fix OOM in FormatDiagnostic (PR #108187)
Vakhurin Sergei via cfe-commits
cfe-commits at lists.llvm.org
Thu Sep 12 10:47:47 PDT 2024
================
@@ -538,24 +511,51 @@ bool DiagnosticsEngine::EmitCurrentDiagnostic(bool Force) {
Emitted = (DiagLevel != DiagnosticIDs::Ignored);
if (Emitted) {
// Emit the diagnostic regardless of suppression level.
- Diags->EmitDiag(*this, DiagLevel);
+ Diags->EmitDiag(*this, DB, DiagLevel);
}
} else {
// Process the diagnostic, sending the accumulated information to the
// DiagnosticConsumer.
- Emitted = ProcessDiag();
+ Emitted = ProcessDiag(DB);
}
- // Clear out the current diagnostic object.
- Clear();
+ return Emitted;
+}
- // If there was a delayed diagnostic, emit it now.
- if (!Force && DelayedDiagID)
- ReportDelayed();
+DiagnosticBuilder::DiagnosticBuilder(DiagnosticsEngine *diagObj,
+ SourceLocation CurDiagLoc,
+ unsigned CurDiagID)
+ : StreamingDiagnostic(diagObj->DiagAllocator), DiagObj(diagObj),
+ CurDiagLoc(CurDiagLoc), CurDiagID(CurDiagID), IsActive(true) {
+ assert(diagObj && "DiagnosticBuilder requires a valid DiagnosticsEngine!");
+}
- return Emitted;
+DiagnosticBuilder::DiagnosticBuilder(const DiagnosticBuilder &D)
+ : StreamingDiagnostic() {
+ CurDiagLoc = D.CurDiagLoc;
+ CurDiagID = D.CurDiagID;
+ FlagValue = D.FlagValue;
+ DiagObj = D.DiagObj;
+ DiagStorage = D.DiagStorage;
+ D.DiagStorage = nullptr;
----------------
igelbox wrote:
Just tried to turn copy-constructor into a moving one and got a whole bunch of errors like:
```
/home/igelbox/llvm-project/clang/lib/Basic/SourceMgrAdapter.cpp: In member function ‘void clang::SourceMgrAdapter::handleDiag(const llvm::SMDiagnostic&)’:
/home/igelbox/llvm-project/clang/lib/Basic/SourceMgrAdapter.cpp:120:66: error: use of deleted function ‘clang::DiagnosticBuilder::DiagnosticBuilder(const clang::DiagnosticBuilder&)’
120 | DiagnosticBuilder Builder = Diagnostics.Report(Loc, DiagID) << Message;
| ^~~~~~~
In file included from /home/igelbox/llvm-project/clang/include/clang/Basic/SourceManager.h:37,
from /home/igelbox/llvm-project/clang/include/clang/Basic/SourceMgrAdapter.h:17,
from /home/igelbox/llvm-project/clang/lib/Basic/SourceMgrAdapter.cpp:14:
/home/igelbox/llvm-project/clang/include/clang/Basic/Diagnostic.h:1192:7: note: ‘clang::DiagnosticBuilder::DiagnosticBuilder(const clang::DiagnosticBuilder&)’ is implicitly declared as deleted because ‘clang::DiagnosticBuilder’ declares a move constructor or move assignment operator
1192 | class DiagnosticBuilder : public StreamingDiagnostic {
| ^~~~~~~~~~~~~~~~~
[ 14%] Building R600GenCallingConv.inc...
make[3]: *** [tools/clang/lib/Basic/CMakeFiles/obj.clangBasic.dir/build.make:530: tools/clang/lib/Basic/CMakeFiles/obj.clangBasic.dir/SourceMgrAdapter.cpp.o] Error 1
make[2]: *** [CMakeFiles/Makefile2:41994: tools/clang/lib/Basic/CMakeFiles/obj.clangBasic.dir/all] Error 2
```
I think they caused by `<<` operators which take `const Builder&` and return `const Builder&` as well that cannot match the moving constructor argument (which is a mutable value of course). Then tried to make them all non-const but it's still failing to compile in several places. I believe it's doable but afraid it would blow up the amount of changes in this PR. So, keeping in mind the original behavior was the same ("and neuters it" original comment) I suggest leaving this surprise-included copy-constructor as it is (like as it was).
https://github.com/llvm/llvm-project/pull/108187
More information about the cfe-commits
mailing list