Remove array conversion, cut down evaluation of & and *.

Author: kuilpd

lldb/include/lldb/ValueObject/DILAST.h

@@ -51,8 +51,6 @@ class ASTNode {
 
   virtual llvm::Expected<lldb::ValueObjectSP> Accept(Visitor *v) const = 0;
 
-  virtual bool is_rvalue() const { return false; }
-
   uint32_t GetLocation() const { return m_location; }
   NodeKind GetKind() const { return m_kind; }
 
@@ -97,7 +95,6 @@ class UnaryOpNode : public ASTNode {
         m_rhs(std::move(rhs)) {}
 
   llvm::Expected<lldb::ValueObjectSP> Accept(Visitor *v) const override;
-  bool is_rvalue() const override { return m_kind != UnaryOpKind::Deref; }
 
   UnaryOpKind kind() const { return m_kind; }
   ASTNode *rhs() const { return m_rhs.get(); }

lldb/include/lldb/ValueObject/DILEval.h

@@ -38,18 +38,6 @@ lldb::ValueObjectSP LookupGlobalIdentifier(llvm::StringRef name_ref,
                                            lldb::DynamicValueType use_dynamic,
                                            CompilerType *scope_ptr = nullptr);
 
-class FlowAnalysis {
-public:
-  FlowAnalysis(bool address_of_is_pending)
-      : m_address_of_is_pending(address_of_is_pending) {}
-
-  bool AddressOfIsPending() const { return m_address_of_is_pending; }
-  void DiscardAddressOf() { m_address_of_is_pending = false; }
-
-private:
-  bool m_address_of_is_pending;
-};
-
 class Interpreter : Visitor {
 public:
   Interpreter(lldb::TargetSP target, llvm::StringRef expr,
@@ -59,29 +47,13 @@ class Interpreter : Visitor {
   llvm::Expected<lldb::ValueObjectSP> Evaluate(const ASTNode *node);
 
 private:
-  llvm::Expected<lldb::ValueObjectSP>
-  EvaluateNode(const ASTNode *node, FlowAnalysis *flow = nullptr);
-
   llvm::Expected<lldb::ValueObjectSP>
   Visit(const IdentifierNode *node) override;
   llvm::Expected<lldb::ValueObjectSP> Visit(const UnaryOpNode *node) override;
 
-  lldb::ValueObjectSP EvaluateDereference(lldb::ValueObjectSP rhs);
-
-  FlowAnalysis *flow_analysis() { return m_flow_analysis_chain.back(); }
-
   // Used by the interpreter to create objects, perform casts, etc.
   lldb::TargetSP m_target;
   llvm::StringRef m_expr;
-  // Flow analysis chain represents the expression evaluation flow for the
-  // current code branch. Each node in the chain corresponds to an AST node,
-  // describing the semantics of the evaluation for it. Currently, flow analysis
-  // propagates the information about the pending address-of operator, so that
-  // combination of address-of and a subsequent dereference can be eliminated.
-  // End of the chain (i.e. `back()`) contains the flow analysis instance for
-  // the current node. It may be `nullptr` if no relevant information is
-  // available, the caller/user is supposed to check.
-  std::vector<FlowAnalysis *> m_flow_analysis_chain;
   lldb::ValueObjectSP m_scope;
   lldb::DynamicValueType m_default_dynamic;
   std::shared_ptr<StackFrame> m_exe_ctx_scope;

lldb/source/ValueObject/DILEval.cpp

@@ -18,22 +18,6 @@
 
 namespace lldb_private::dil {
 
-static lldb::ValueObjectSP
-ArrayToPointerConversion(lldb::ValueObjectSP valobj,
-                         std::shared_ptr<ExecutionContextScope> ctx) {
-  assert(valobj->IsArrayType() &&
-         "an argument to array-to-pointer conversion must be an array");
-
-  uint64_t addr = valobj->GetLoadAddress();
-  llvm::StringRef name = "result";
-  ExecutionContext exe_ctx;
-  ctx->CalculateExecutionContext(exe_ctx);
-  return ValueObject::CreateValueObjectFromAddress(
-      name, addr, exe_ctx,
-      valobj->GetCompilerType().GetArrayElementType(ctx.get()).GetPointerType(),
-      /* do_deref */ false);
-}
-
 static lldb::ValueObjectSP LookupStaticIdentifier(
     VariableList &variable_list, std::shared_ptr<StackFrame> exe_scope,
     llvm::StringRef name_ref, llvm::StringRef unqualified_name) {
@@ -222,22 +206,9 @@ Interpreter::Interpreter(lldb::TargetSP target, llvm::StringRef expr,
     : m_target(std::move(target)), m_expr(expr), m_default_dynamic(use_dynamic),
       m_exe_ctx_scope(frame_sp) {}
 
-llvm::Expected<lldb::ValueObjectSP> Interpreter::Evaluate(const ASTNode *tree) {
+llvm::Expected<lldb::ValueObjectSP> Interpreter::Evaluate(const ASTNode *node) {
   // Evaluate an AST.
-  auto value_or_error = EvaluateNode(tree);
-
-  // Return the computed result-or-error.
-  return value_or_error;
-}
-
-llvm::Expected<lldb::ValueObjectSP>
-Interpreter::EvaluateNode(const ASTNode *node, FlowAnalysis *flow) {
-  // Set up the evaluation context for the current node.
-  m_flow_analysis_chain.push_back(flow);
-  // Traverse an AST pointed by the `node`.
   auto value_or_error = node->Accept(this);
-  // Cleanup the context.
-  m_flow_analysis_chain.pop_back();
   // Return the computed value-or-error. The caller is responsible for
   // checking if an error occured during the evaluation.
   return value_or_error;
@@ -265,33 +236,29 @@ Interpreter::Visit(const IdentifierNode *node) {
 
 llvm::Expected<lldb::ValueObjectSP>
 Interpreter::Visit(const UnaryOpNode *node) {
-  FlowAnalysis rhs_flow(
-      /* address_of_is_pending */ node->kind() == UnaryOpKind::AddrOf);
-
   Status error;
-  auto rhs_or_err = EvaluateNode(node->rhs(), &rhs_flow);
+  auto rhs_or_err = Evaluate(node->rhs());
   if (!rhs_or_err) {
     return rhs_or_err;
   }
   lldb::ValueObjectSP rhs = *rhs_or_err;
 
-  CompilerType rhs_type = rhs->GetCompilerType();
   switch (node->kind()) {
   case UnaryOpKind::Deref: {
-    if (rhs_type.IsArrayType())
-      rhs = ArrayToPointerConversion(rhs, m_exe_ctx_scope);
-
     lldb::ValueObjectSP dynamic_rhs = rhs->GetDynamicValue(m_default_dynamic);
     if (dynamic_rhs)
       rhs = dynamic_rhs;
 
-    if (rhs->GetCompilerType().IsPointerType()) {
-      if (rhs->GetCompilerType().IsPointerToVoid()) {
-        return llvm::make_error<DILDiagnosticError>(
-            m_expr, "indirection not permitted on operand of type 'void *'",
-            node->GetLocation(), 1);
-      }
-      return EvaluateDereference(rhs);
+    CompilerType rhs_type = rhs->GetCompilerType();
+    if (!rhs_type.IsReferenceType() && !rhs_type.IsPointerType())
+      return llvm::make_error<DILDiagnosticError>(
+          m_expr, "not a pointer or reference type",
+          node->rhs()->GetLocation());
+
+    if (rhs_type.IsPointerToVoid()) {
+      return llvm::make_error<DILDiagnosticError>(
+          m_expr, "indirection not permitted on operand of type 'void *'",
+          node->GetLocation());
     }
     lldb::ValueObjectSP child_sp = rhs->Dereference(error);
     if (error.Success())
@@ -300,68 +267,19 @@ Interpreter::Visit(const UnaryOpNode *node) {
     return rhs;
   }
   case UnaryOpKind::AddrOf: {
-    if (node->rhs()->is_rvalue()) {
-      std::string errMsg =
-          llvm::formatv("cannot take the address of an rvalue of type {0}",
-                        rhs_type.TypeDescription());
-      return llvm::make_error<DILDiagnosticError>(m_expr, errMsg,
+    Status error;
+    lldb::ValueObjectSP value = rhs->AddressOf(error);
+    if (error.Fail()) {
+      return llvm::make_error<DILDiagnosticError>(m_expr, error.AsCString(),
                                                   node->GetLocation());
     }
-    if (rhs->IsBitfield()) {
-      return llvm::make_error<DILDiagnosticError>(
-          m_expr, "address of bit-field requested", node->GetLocation());
-    }
-    // If the address-of operation wasn't cancelled during the evaluation of
-    // RHS (e.g. because of the address-of-a-dereference elision), apply it
-    // here.
-    if (rhs_flow.AddressOfIsPending()) {
-      Status error;
-      lldb::ValueObjectSP value = rhs->AddressOf(error);
-      if (error.Fail()) {
-        return llvm::make_error<DILDiagnosticError>(m_expr, error.AsCString(),
-                                                    node->GetLocation());
-      }
-      return value;
-    }
-    return rhs;
+    return value;
   }
   }
 
   // Unsupported/invalid operation.
   return llvm::make_error<DILDiagnosticError>(
-      m_expr, "invalid ast: unexpected binary operator", node->GetLocation(),
-      1);
-}
-
-lldb::ValueObjectSP Interpreter::EvaluateDereference(lldb::ValueObjectSP rhs) {
-  // If rhs is a reference, dereference it first.
-  Status error;
-  if (rhs->GetCompilerType().IsReferenceType())
-    rhs = rhs->Dereference(error);
-
-  assert(rhs->GetCompilerType().IsPointerType() &&
-         "invalid ast: must be a pointer type");
-
-  if (rhs->GetDerefValobj())
-    return rhs->GetDerefValobj()->GetSP();
-
-  CompilerType pointer_type = rhs->GetCompilerType();
-  lldb::addr_t base_addr = rhs->GetValueAsUnsigned(0);
-
-  llvm::StringRef name = "result";
-  ExecutionContext exe_ctx(m_target.get(), false);
-  lldb::ValueObjectSP value = ValueObject::CreateValueObjectFromAddress(
-      name, base_addr, exe_ctx, pointer_type,
-      /* do_deref */ false);
-
-  // If we're in the address-of context, skip the dereference and cancel the
-  // pending address-of operation as well.
-  if (flow_analysis() && flow_analysis()->AddressOfIsPending()) {
-    flow_analysis()->DiscardAddressOf();
-    return value;
-  }
-
-  return value->Dereference(error);
+      m_expr, "invalid ast: unexpected binary operator", node->GetLocation());
 }
 
 } // namespace lldb_private::dil

lldb/test/API/commands/frame/var-dil/basics/PointerArithmetic/TestFrameVarDILPointerArithmetic.py

@@ -24,18 +24,25 @@ def test_dereference(self):
         lldbutil.run_to_source_breakpoint(
             self, "Set a breakpoint here", lldb.SBFileSpec("main.cpp")
         )
-        is_32bit = self.process().GetAddressByteSize() == 4
 
         self.runCmd("settings set target.experimental.use-DIL true")
         self.expect_var_path("*p_int0", True, value="0")
         self.expect_var_path("*cp_int5", True, value="5")
         self.expect_var_path("*rcp_int0", True, type="const int *")
-        self.expect_var_path("*array", value="0")
-        self.expect_var_path(
-            "&*p_null", value="0x00000000" if is_32bit else "0x0000000000000000"
-        )
+        self.expect_var_path("*offset_p", True, value="5")
+        self.expect_var_path("*offset_pref", True, type="int *")
         self.expect_var_path("**pp_int0", value="0")
         self.expect_var_path("&**pp_int0", type="int *")
+        self.expect(
+            "frame var '*array'",
+            error=True,
+            substrs=["not a pointer or reference type"],
+        )
+        self.expect(
+            "frame var '&*p_null'",
+            error=True,
+            substrs=["doesn't have a valid address"],
+        )
         self.expect(
             "frame var '&*p_void'",
             error=True,

lldb/test/API/commands/frame/var-dil/basics/PointerArithmetic/main.cpp

@@ -6,6 +6,8 @@ int main(int argc, char **argv) {
   my_char_ptr my_p_char1 = p_char1;
 
   int offset = 5;
+  int *offset_p = &offset;
+  int *&offset_pref = offset_p;
   int array[10];
   array[0] = 0;
   array[offset] = offset;