[Clang] Invert emptiness check for correctness#178068
Conversation
zwuis on github pointed out a bug [1] in a deferred call which was supposed to clear the DeclForInitializer field but was doing so incorrectly because the emptiness check needs to be inverted. [1]: https://github.com/llvm/llvm-project/pull/163885/changes/BASE..082c9769209e80504a2205581a4387bcf39ca5df#r2726301659 Signed-off-by: Justin Stitt <justinstitt@google.com>
|
@llvm/pr-subscribers-clang Author: Justin Stitt (JustinStitt) ChangesAfter #163885 landed, @zwuis pointed out a bug in a deferred call which was supposed to clear the DeclForInitializer field but was doing so incorrectly because the emptiness check is wrong. Invert the check since the current check is wrong and an access to It is hard to add a test specifically for this change. It seems rather difficult to achieve the right circumstances to trigger this cleanup function with any valid source input. This check is more of a defensive check to make sure we aren't calling cc @zwuis Full diff: https://github.com/llvm/llvm-project/pull/178068.diff 1 Files Affected:
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 0b3416a12f9f4..907b7b367f19b 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -13825,7 +13825,7 @@ void Sema::DiagnoseUniqueObjectDuplication(const VarDecl *VD) {
void Sema::AddInitializerToDecl(Decl *RealDecl, Expr *Init, bool DirectInit) {
llvm::scope_exit ResetDeclForInitializer([this]() {
- if (this->ExprEvalContexts.empty())
+ if (!this->ExprEvalContexts.empty())
this->ExprEvalContexts.back().DeclForInitializer = nullptr;
});
|
Fix a bug in a deferred call which was supposed to clear the `DeclForInitializer` field but was doing so incorrectly because the emptiness check needs to be inverted.
Fix a bug in a deferred call which was supposed to clear the `DeclForInitializer` field but was doing so incorrectly because the emptiness check needs to be inverted.
After #163885 landed, @zwuis pointed out a bug in a deferred call which was supposed to clear the DeclForInitializer field but was doing so incorrectly because the emptiness check is wrong.
Invert the check since the current check is wrong and an access to
.back()is undefined behavior when the container is empty.It is hard to add a test specifically for this change. It seems rather difficult to achieve the right circumstances to trigger this cleanup function with any valid source input. This check is more of a defensive check to make sure we aren't calling
back()on an empty thing.cc @zwuis