[Lldb-commits] [lldb] [LLDB] Add DIL code for handling plain variable names. (PR #120971)

[via lldb-commits]

================
@@ -0,0 +1,106 @@
+//===-- DILEval.h -----------------------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLDB_VALUEOBJECT_DILEVAL_H_
+#define LLDB_VALUEOBJECT_DILEVAL_H_
+
+#include "lldb/ValueObject/DILAST.h"
+#include "lldb/ValueObject/DILParser.h"
+#include <memory>
+#include <vector>
+
+namespace lldb_private {
+
+namespace dil {
+
+/// Class used to store & manipulate information about identifiers.
+class IdentifierInfo {
+public:
+  enum class Kind {
+    eValue,
+    eContextArg,
+  };
+
+  static std::unique_ptr<IdentifierInfo> FromValue(ValueObject &valobj) {
+    CompilerType type;
+    type = valobj.GetCompilerType();
+    return std::unique_ptr<IdentifierInfo>(
+        new IdentifierInfo(Kind::eValue, type, valobj.GetSP(), {}));
+  }
+
+  static std::unique_ptr<IdentifierInfo> FromContextArg(CompilerType type) {
+    lldb::ValueObjectSP empty_value;
+    return std::unique_ptr<IdentifierInfo>(
+        new IdentifierInfo(Kind::eContextArg, type, empty_value, {}));
+  }
+
+  Kind GetKind() const { return m_kind; }
+  lldb::ValueObjectSP GetValue() const { return m_value; }
+
+  CompilerType GetType() { return m_type; }
+  bool IsValid() const { return m_type.IsValid(); }
+
+  IdentifierInfo(Kind kind, CompilerType type, lldb::ValueObjectSP value,
+                 std::vector<uint32_t> path)
+      : m_kind(kind), m_type(type), m_value(std::move(value)) {}
+
+private:
+  Kind m_kind;
+  CompilerType m_type;
+  lldb::ValueObjectSP m_value;
+};
+
+/// Given the name of an identifier (variable name, member name, type name,
+/// etc.), find the ValueObject for that name (if it exists) and create and
+/// return an IdentifierInfo object containing all the relevant information
+/// about that object (for DIL parsing and evaluating).
+std::unique_ptr<IdentifierInfo> LookupIdentifier(
+    const std::string &name, std::shared_ptr<ExecutionContextScope> ctx_scope,
+    lldb::DynamicValueType use_dynamic, CompilerType *scope_ptr = nullptr);
+
+class DILInterpreter : Visitor {
+public:
+  DILInterpreter(lldb::TargetSP target, llvm::StringRef expr,
+                 lldb::DynamicValueType use_dynamic,
+                 std::shared_ptr<ExecutionContextScope> exe_ctx_scope);
+
+  llvm::Expected<lldb::ValueObjectSP> DILEval(const DILASTNode *tree,
+                                              lldb::TargetSP target_sp);
+
+private:
+  lldb::ValueObjectSP DILEvalNode(const DILASTNode *node);
+
+  bool Success() { return m_error.Success(); }
+
+  void SetError(ErrorCode error_code, std::string error, uint32_t loc);
+
+  void Visit(const ErrorNode *node) override;
+  void Visit(const IdentifierNode *node) override;
+
+private:
+  // Used by the interpreter to create objects, perform casts, etc.
+  lldb::TargetSP m_target;
+
+  llvm::StringRef m_expr;
+
+  lldb::ValueObjectSP m_result;
+
+  lldb::ValueObjectSP m_scope;
+
+  lldb::DynamicValueType m_default_dynamic;
+
+  Status m_error;
----------------
cmtice wrote:

It turns out that I can get by without using a Status member object in the DILIntepreter. I have updated the DILInterpreter code to use llvm::Expected instead.

The use of the Status member in the DILParser will be much harder to remove and replace with llvm::Expected. But the good news is that the m_error status member in DILParser is only ever updated by the BailOut methods, which also immediately terminate parsing by setting the current token to EOF, and the top level call (Run) checks the error before returning, so I am hoping that in this case I will be allowed to keep the Status member variable?  

Among the reasons for not trying to convert all calls to BailOut to returning an llvm::Expected...: It's called from many functions with many different return types, including void. So at a minimum I would have to add return types to functions that currently don't return anything (as well as. changing the return types of nearly every function in the parser); I would have to do error type conversions in functions where they call something that fails with llvm::expected<type A> but the calling function returns llvm::expected<type b>. I would also have to add code to many of these sites to also update the current token to EOF, to terminate the parsing, not to mention the additional code at all the parse function call sites to see what value was returned & get the 'real' (non error) value out of the expected return.  It will actually make the code much messier and more complicated than it is now.

I would be happy to add comments both at the declaration of the 'Status m_error;' variable in DILParser.h and in the BailOut functions, to the effect that this variable must ONLY be set in the BailOut functions (as it currently is).

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