[lldb][ClangExpressionParser] Reset DiagnosticManager before we create persistent variables #11500
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Here's an example crash that we've seen sporadically over the years:
Here
ASTImporter::IsStructurallyEquivalent
is trying to emit a diagnostic usingClangExpressionParser
'sClangDiagnosticManagerAdapter
. ButTextDiagnosticPrinter::TextDiag
seems to benullptr
. This can only happen when we callHandleDiagnostic
onClangDiagnosticManagerAdapter
after we calledEndSourceFile
. Specifically, it looks like when moving a type fromExpression
AST toScratch
AST (inCommitPersistentDecls
), something went wrong, so the ASTImporter tried to emit a diagnostic, but we already calledEndSourceFile
at that point.This patch moves the call to
ResetManager
to beforeCommitPersistentDecls
, so if we calledHandleDiagnostic
, we would correctly short-circuit out of it. This seems to have been intended anyway based on this comment:https://github.com/llvm/llvm-project/blob/cecdff92838f3c049548e3445a15d8c9c7a49205/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp#L200-L204
But something must have broken that during a refactor.
I'm not 100% sure how best to test this because we need a scenario where moving a type into the scratch AST fails, but the expression itself succeeded.
rdar://161198130
(cherry picked from commit cbfa5c8)