[Lldb-commits] [lldb] [lldb][draft] All ValueObjectSP instances are now valid (non-null) but have an error state (PR #74912)

[Adrian Prantl via lldb-commits]

================
@@ -1984,84 +1984,75 @@ void ValueObject::GetExpressionPath(Stream &s,
   }
 }
 
-ValueObjectSP ValueObject::GetValueForExpressionPath(
-    llvm::StringRef expression, ExpressionPathScanEndReason *reason_to_stop,
-    ExpressionPathEndResultType *final_value_type,
+std::optional<lldb::ValueObjectSP> ValueObject::GetValueForExpressionPath(
+    llvm::StringRef expression, ExpressionPathScanEndReason *reason_to_stop_ptr,
+    ExpressionPathEndResultType *final_value_type_ptr,
     const GetValueForExpressionPathOptions &options,
-    ExpressionPathAftermath *final_task_on_target) {
-
-  ExpressionPathScanEndReason dummy_reason_to_stop =
-      ValueObject::eExpressionPathScanEndReasonUnknown;
-  ExpressionPathEndResultType dummy_final_value_type =
-      ValueObject::eExpressionPathEndResultTypeInvalid;
-  ExpressionPathAftermath dummy_final_task_on_target =
-      ValueObject::eExpressionPathAftermathNothing;
-
-  ValueObjectSP ret_val = GetValueForExpressionPath_Impl(
-      expression, reason_to_stop ? reason_to_stop : &dummy_reason_to_stop,
-      final_value_type ? final_value_type : &dummy_final_value_type, options,
-      final_task_on_target ? final_task_on_target
-                           : &dummy_final_task_on_target);
-
-  if (!final_task_on_target ||
-      *final_task_on_target == ValueObject::eExpressionPathAftermathNothing)
-    return ret_val;
-
-  if (ret_val.get() &&
-      ((final_value_type ? *final_value_type : dummy_final_value_type) ==
-       eExpressionPathEndResultTypePlain)) // I can only deref and takeaddress
-                                           // of plain objects
-  {
-    if ((final_task_on_target ? *final_task_on_target
-                              : dummy_final_task_on_target) ==
-        ValueObject::eExpressionPathAftermathDereference) {
-      Status error;
-      ValueObjectSP final_value = ret_val->Dereference(error);
-      if (error.Fail() || !final_value.get()) {
-        if (reason_to_stop)
-          *reason_to_stop =
-              ValueObject::eExpressionPathScanEndReasonDereferencingFailed;
-        if (final_value_type)
-          *final_value_type = ValueObject::eExpressionPathEndResultTypeInvalid;
-        return ValueObjectSP();
-      } else {
-        if (final_task_on_target)
-          *final_task_on_target = ValueObject::eExpressionPathAftermathNothing;
-        return final_value;
-      }
-    }
-    if (*final_task_on_target ==
-        ValueObject::eExpressionPathAftermathTakeAddress) {
-      Status error;
-      ValueObjectSP final_value = ret_val->AddressOf(error);
-      if (error.Fail() || !final_value.get()) {
-        if (reason_to_stop)
-          *reason_to_stop =
-              ValueObject::eExpressionPathScanEndReasonTakingAddressFailed;
-        if (final_value_type)
-          *final_value_type = ValueObject::eExpressionPathEndResultTypeInvalid;
-        return ValueObjectSP();
-      } else {
-        if (final_task_on_target)
-          *final_task_on_target = ValueObject::eExpressionPathAftermathNothing;
-        return final_value;
-      }
-    }
+    ExpressionPathAftermath *final_task_on_target_ptr) {
+
+  auto stop_reason_unknown = eExpressionPathScanEndReasonUnknown;
+  auto value_type_invalid = eExpressionPathEndResultTypeInvalid;
+  auto final_task_nothing = eExpressionPathAftermathNothing;
+
+  auto ret_value = GetValueForExpressionPath_Impl(
+      expression,
+      reason_to_stop_ptr ? reason_to_stop_ptr : &stop_reason_unknown,
+      final_value_type_ptr ? final_value_type_ptr : &value_type_invalid,
+      options,
+      final_task_on_target_ptr ? final_task_on_target_ptr
+                               : &final_task_nothing);
+
+  // The caller knows that nothing happened because `final_task_on_target` still
+  // has its original value.
+  if (!ret_value)
+    return {};
+
+  if (!final_value_type_ptr)
+    return ret_value;
+
+  if ((*final_value_type_ptr) != eExpressionPathEndResultTypePlain)
+    return ret_value;
+
+  if (!final_task_on_target_ptr)
+    return ret_value;
+
+  ExpressionPathAftermath &final_task_on_target = (*final_task_on_target_ptr);
+  ExpressionPathScanEndReason stop_reason_for_error;
+  // The method can only dereference and take the address of plain objects.
+  switch (final_task_on_target) {
+  case eExpressionPathAftermathNothing: {
+    return ret_value;
+  }
+  case eExpressionPathAftermathDereference: {
+    ret_value = ret_value.value()->Dereference();
+    stop_reason_for_error = eExpressionPathScanEndReasonDereferencingFailed;
+  }
+  case eExpressionPathAftermathTakeAddress: {
+    ret_value = ret_value.value()->AddressOf();
+    stop_reason_for_error = eExpressionPathScanEndReasonTakingAddressFailed;
+  }
+  }
+
+  if (ret_value && ret_value.value()->GetError().Success()) {
+    final_task_on_target = eExpressionPathAftermathNothing;
+    return ret_value;
+  } else {
----------------
adrian-prantl wrote:

LLVM coding style prefers leaving off `else` after `return`.

https://github.com/llvm/llvm-project/pull/74912