[Lldb-commits] [PATCH] D98370: [lldb] Fix SBValue::Persist() for constant values

[Andy Yankovsky via Phabricator via lldb-commits]

werat added a comment.
Herald added a subscriber: JDevlieghere.

Hi @teemperor , here's an attempt to fix `SBValue::Persist` method. I've highlighted a few moments in the patch I'm not so sure about, please let me know what you think. Thanks!



================
Comment at: lldb/source/Expression/Materializer.cpp:301
 
-        m_persistent_variable_sp->m_flags &=
-            ~ExpressionVariable::EVNeedsFreezeDry;
+        m_persistent_variable_sp->m_frozen_sp = ValueObjectConstResult::Create(
+            map.GetBestExecutionContextScope(),
----------------
Writing data directly to `m_persistent_variable_sp->GetValueBytes()` is not enough, because the underlying `ValueObject` also stores the data in `m_value` and I didn't find a way to update that one too. Overall there seems to be an assumption that `ValueObjectConstResult` doesn't change (which makes sense), so creating a new one here seems more in line with the design.


================
Comment at: lldb/source/Expression/Materializer.cpp:303-304
 
-        m_persistent_variable_sp->m_flags &=
-            ~ExpressionVariable::EVNeedsFreezeDry;
       }
----------------
I am not sure how to deal with this. "ConstResult" variables needs to be dematerialized every time, so they should either have `EVNeedsFreezeDry` flag, or the check should be different. Removing this flag doesn't seem to break anything though...


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98370/new/

https://reviews.llvm.org/D98370