[lldb] Use AST nodes as Subscript and BitField arguments in DIL

lldb/docs/dil-expr-lang.ebnf

@@ -11,7 +11,8 @@ unary_expression = postfix_expression
 unary_operator = "*" | "&" | "+" | "-";
 
 postfix_expression = primary_expression
-                   | postfix_expression "[" integer_literal "]"
+                   | postfix_expression "[" expression "]"
+                   | postfix_expression "[" expression "-" expression "]"
                    | postfix_expression "." id_expression
                    | postfix_expression "->" id_expression ;
 
@@ -31,8 +32,6 @@ qualified_id = ["::"] [nested_name_specifier] unqualified_id
 
 identifier = ? C99 Identifier ? ;
 
-integer_literal = ? Integer constant: hexademical, decimal, octal, binary ? ;
-
 numeric_literal = ? Integer constant: hexademical, decimal, octal, binary ?
                 | ? Floating constant ? ;
 

lldb/include/lldb/ValueObject/DILAST.h

@@ -141,46 +141,46 @@ class UnaryOpNode : public ASTNode {
 
 class ArraySubscriptNode : public ASTNode {
 public:
-  ArraySubscriptNode(uint32_t location, ASTNodeUP base, int64_t index)
+  ArraySubscriptNode(uint32_t location, ASTNodeUP base, ASTNodeUP index)
       : ASTNode(location, NodeKind::eArraySubscriptNode),
-        m_base(std::move(base)), m_index(index) {}
+        m_base(std::move(base)), m_index(std::move(index)) {}
 
   llvm::Expected<lldb::ValueObjectSP> Accept(Visitor *v) const override;
 
   ASTNode *GetBase() const { return m_base.get(); }
-  int64_t GetIndex() const { return m_index; }
+  ASTNode *GetIndex() const { return m_index.get(); }
 
   static bool classof(const ASTNode *node) {
     return node->GetKind() == NodeKind::eArraySubscriptNode;
   }
 
 private:
   ASTNodeUP m_base;
-  int64_t m_index;
+  ASTNodeUP m_index;
 };
 
 class BitFieldExtractionNode : public ASTNode {
 public:
-  BitFieldExtractionNode(uint32_t location, ASTNodeUP base, int64_t first_index,
-                         int64_t last_index)
+  BitFieldExtractionNode(uint32_t location, ASTNodeUP base,
+                         ASTNodeUP first_index, ASTNodeUP last_index)
       : ASTNode(location, NodeKind::eBitExtractionNode),
-        m_base(std::move(base)), m_first_index(first_index),
-        m_last_index(last_index) {}
+        m_base(std::move(base)), m_first_index(std::move(first_index)),
+        m_last_index(std::move(last_index)) {}
 
   llvm::Expected<lldb::ValueObjectSP> Accept(Visitor *v) const override;
 
   ASTNode *GetBase() const { return m_base.get(); }
-  int64_t GetFirstIndex() const { return m_first_index; }
-  int64_t GetLastIndex() const { return m_last_index; }
+  ASTNode *GetFirstIndex() const { return m_first_index.get(); }
+  ASTNode *GetLastIndex() const { return m_last_index.get(); }
 
   static bool classof(const ASTNode *node) {
     return node->GetKind() == NodeKind::eBitExtractionNode;
   }
 
 private:
   ASTNodeUP m_base;
-  int64_t m_first_index;
-  int64_t m_last_index;
+  ASTNodeUP m_first_index;
+  ASTNodeUP m_last_index;
 };
 
 enum class IntegerTypeSuffix { None, Long, LongLong };

lldb/include/lldb/ValueObject/DILEval.h

@@ -46,6 +46,8 @@ class Interpreter : Visitor {
   llvm::Expected<lldb::ValueObjectSP> Evaluate(const ASTNode *node);
 
 private:
+  llvm::Expected<lldb::ValueObjectSP>
+  EvaluateAndDereference(const ASTNode *node);
   llvm::Expected<lldb::ValueObjectSP>
   Visit(const IdentifierNode *node) override;
   llvm::Expected<lldb::ValueObjectSP> Visit(const MemberOfNode *node) override;

lldb/include/lldb/ValueObject/DILParser.h

@@ -95,7 +95,6 @@ class DILParser {
 
   std::string ParseIdExpression();
   std::string ParseUnqualifiedId();
-  std::optional<int64_t> ParseIntegerConstant();
   ASTNodeUP ParseNumericLiteral();
   ASTNodeUP ParseIntegerLiteral();
   ASTNodeUP ParseFloatingPointLiteral();

lldb/source/ValueObject/DILEval.cpp

@@ -251,6 +251,22 @@ llvm::Expected<lldb::ValueObjectSP> Interpreter::Evaluate(const ASTNode *node) {
   return value_or_error;
 }
 
+llvm::Expected<lldb::ValueObjectSP>
+Interpreter::EvaluateAndDereference(const ASTNode *node) {
+  auto valobj_or_err = Evaluate(node);
+  if (!valobj_or_err)
+    return valobj_or_err;
+  lldb::ValueObjectSP valobj = *valobj_or_err;
+
+  Status error;
+  if (valobj->GetCompilerType().IsReferenceType()) {
+    valobj = valobj->Dereference(error);
+    if (error.Fail())
+      return error.ToError();
+  }
+  return valobj;
+}
+
 llvm::Expected<lldb::ValueObjectSP>
 Interpreter::Visit(const IdentifierNode *node) {
   lldb::DynamicValueType use_dynamic = m_use_dynamic;
@@ -477,13 +493,22 @@ Interpreter::Visit(const MemberOfNode *node) {
 
 llvm::Expected<lldb::ValueObjectSP>
 Interpreter::Visit(const ArraySubscriptNode *node) {
-  auto lhs_or_err = Evaluate(node->GetBase());
-  if (!lhs_or_err)
-    return lhs_or_err;
-  lldb::ValueObjectSP base = *lhs_or_err;
+  auto base_or_err = Evaluate(node->GetBase());
+  if (!base_or_err)
+    return base_or_err;
+  lldb::ValueObjectSP base = *base_or_err;
+  auto idx_or_err = EvaluateAndDereference(node->GetIndex());
+  if (!idx_or_err)
+    return idx_or_err;
+  lldb::ValueObjectSP idx = *idx_or_err;
+
+  if (!idx->GetCompilerType().IsIntegerOrUnscopedEnumerationType()) {
+    return llvm::make_error<DILDiagnosticError>(
+        m_expr, "array subscript is not an integer", node->GetLocation());
+  }
 
   StreamString var_expr_path_strm;
-  uint64_t child_idx = node->GetIndex();
+  uint64_t child_idx = idx->GetValueAsUnsigned(0);
   lldb::ValueObjectSP child_valobj_sp;
 
   bool is_incomplete_array = false;
@@ -613,29 +638,38 @@ Interpreter::Visit(const ArraySubscriptNode *node) {
     return child_valobj_sp;
   }
 
-  int64_t signed_child_idx = node->GetIndex();
+  int64_t signed_child_idx = idx->GetValueAsSigned(0);
   return base->GetSyntheticArrayMember(signed_child_idx, true);
 }
 
 llvm::Expected<lldb::ValueObjectSP>
 Interpreter::Visit(const BitFieldExtractionNode *node) {
-  auto lhs_or_err = Evaluate(node->GetBase());
-  if (!lhs_or_err)
-    return lhs_or_err;
-  lldb::ValueObjectSP base = *lhs_or_err;
-  int64_t first_index = node->GetFirstIndex();
-  int64_t last_index = node->GetLastIndex();
+  auto base_or_err = EvaluateAndDereference(node->GetBase());
+  if (!base_or_err)
+    return base_or_err;
+  lldb::ValueObjectSP base = *base_or_err;
+  auto first_idx_or_err = EvaluateAndDereference(node->GetFirstIndex());
+  if (!first_idx_or_err)
+    return first_idx_or_err;
+  lldb::ValueObjectSP first_idx = *first_idx_or_err;
+  auto last_idx_or_err = EvaluateAndDereference(node->GetLastIndex());
+  if (!last_idx_or_err)
+    return last_idx_or_err;
+  lldb::ValueObjectSP last_idx = *last_idx_or_err;
+
+  if (!first_idx->GetCompilerType().IsIntegerOrUnscopedEnumerationType() ||
+      !last_idx->GetCompilerType().IsIntegerOrUnscopedEnumerationType()) {
+    return llvm::make_error<DILDiagnosticError>(
+        m_expr, "bit index is not an integer", node->GetLocation());
+  }
+
+  int64_t first_index = first_idx->GetValueAsSigned(0);
+  int64_t last_index = last_idx->GetValueAsSigned(0);
 
   // if the format given is [high-low], swap range
   if (first_index > last_index)
     std::swap(first_index, last_index);
 
-  Status error;
-  if (base->GetCompilerType().IsReferenceType()) {
-    base = base->Dereference(error);
-    if (error.Fail())
-      return error.ToError();
-  }
   lldb::ValueObjectSP child_valobj_sp =
       base->GetSyntheticBitFieldChild(first_index, last_index, true);
   if (!child_valobj_sp) {

lldb/source/ValueObject/DILParser.cpp

@@ -127,8 +127,8 @@ ASTNodeUP DILParser::ParseUnaryExpression() {
 //
 //  postfix_expression:
 //    primary_expression
-//    postfix_expression "[" integer_literal "]"
-//    postfix_expression "[" integer_literal "-" integer_literal "]"
+//    postfix_expression "[" expression "]"
+//    postfix_expression "[" expression "-" expression "]"
 //    postfix_expression "." id_expression
 //    postfix_expression "->" id_expression
 //
@@ -140,27 +140,15 @@ ASTNodeUP DILParser::ParsePostfixExpression() {
     switch (token.GetKind()) {
     case Token::l_square: {
       m_dil_lexer.Advance();
-      std::optional<int64_t> index = ParseIntegerConstant();
-      if (!index) {
-        BailOut(
-            llvm::formatv("failed to parse integer constant: {0}", CurToken()),
-            CurToken().GetLocation(), CurToken().GetSpelling().length());
-        return std::make_unique<ErrorNode>();
-      }
+      ASTNodeUP index = ParseExpression();
       if (CurToken().GetKind() == Token::minus) {
         m_dil_lexer.Advance();
-        std::optional<int64_t> last_index = ParseIntegerConstant();
-        if (!last_index) {
-          BailOut(llvm::formatv("failed to parse integer constant: {0}",
-                                CurToken()),
-                  CurToken().GetLocation(), CurToken().GetSpelling().length());
-          return std::make_unique<ErrorNode>();
-        }
+        ASTNodeUP last_index = ParseExpression();
         lhs = std::make_unique<BitFieldExtractionNode>(
-            loc, std::move(lhs), std::move(*index), std::move(*last_index));
+            loc, std::move(lhs), std::move(index), std::move(last_index));
       } else {
         lhs = std::make_unique<ArraySubscriptNode>(loc, std::move(lhs),
-                                                   std::move(*index));
+                                                   std::move(index));
       }
       Expect(Token::r_square);
       m_dil_lexer.Advance();
@@ -374,31 +362,6 @@ void DILParser::BailOut(const std::string &error, uint32_t loc,
   m_dil_lexer.ResetTokenIdx(m_dil_lexer.NumLexedTokens() - 1);
 }
 
-// FIXME: Remove this once subscript operator uses ScalarLiteralNode.
-// Parse a integer_literal.
-//
-//  integer_literal:
-//    ? Integer constant ?
-//
-std::optional<int64_t> DILParser::ParseIntegerConstant() {
-  std::string number_spelling;
-  if (CurToken().GetKind() == Token::minus) {
-    // StringRef::getAsInteger<>() can parse negative numbers.
-    // FIXME: Remove this once unary minus operator is added.
-    number_spelling = "-";
-    m_dil_lexer.Advance();
-  }
-  number_spelling.append(CurToken().GetSpelling());
-  llvm::StringRef spelling_ref = number_spelling;
-  int64_t raw_value;
-  if (!spelling_ref.getAsInteger<int64_t>(0, raw_value)) {
-    m_dil_lexer.Advance();
-    return raw_value;
-  }
-
-  return std::nullopt;
-}
-
 // Parse a numeric_literal.
 //
 //  numeric_literal:

lldb/test/API/commands/frame/var-dil/basics/ArraySubscript/TestFrameVarDILArraySubscript.py

@@ -11,14 +11,6 @@
 class TestFrameVarDILArraySubscript(TestBase):
     NO_DEBUG_INFO_TESTCASE = True
 
-    def expect_var_path(self, expr, compare_to_framevar=False, value=None, type=None):
-        value_dil = super().expect_var_path(expr, value=value, type=type)
-        if compare_to_framevar:
-            self.runCmd("settings set target.experimental.use-DIL false")
-            value_frv = super().expect_var_path(expr, value=value, type=type)
-            self.runCmd("settings set target.experimental.use-DIL true")
-            self.assertEqual(value_dil.GetValue(), value_frv.GetValue())
-
     def test_subscript(self):
         self.build()
         lldbutil.run_to_source_breakpoint(
@@ -28,45 +20,45 @@ def test_subscript(self):
         self.runCmd("settings set target.experimental.use-DIL true")
 
         # Test int[] and int*
-        self.expect_var_path("int_arr[0]", True, value="1")
-        self.expect_var_path("int_ptr[1]", True, value="2")
-        self.expect("frame var 'int_arr[enum_one]'", error=True)
+        self.expect_var_path("int_arr[0]", value="1")
+        self.expect_var_path("int_ptr[1]", value="2")
+        self.expect_var_path("int_ptr[enum_one]", value="2")
 
         # Test when base and index are references.
-        self.expect_var_path("int_arr[0]", True, value="1")
-        self.expect("frame var 'int_arr[idx_1_ref]'", error=True)
-        self.expect("frame var 'int_arr[enum_ref]'", error=True)
+        self.expect_var_path("int_arr[0]", value="1")
+        self.expect_var_path("int_arr[idx_1_ref]", value="2")
+        self.expect_var_path("int_arr[enum_ref]", value="2")
         self.expect_var_path("int_arr_ref[0]", value="1")
-        self.expect("frame var 'int_arr_ref[idx_1_ref]'", error=True)
-        self.expect("frame var 'int_arr_ref[enum_ref]'", error=True)
+        self.expect_var_path("int_arr_ref[idx_1_ref]", value="2")
+        self.expect_var_path("int_arr_ref[enum_ref]", value="2")
 
         # Test when base and index are typedefs.
-        self.expect_var_path("td_int_arr[0]", True, value="1")
-        self.expect("frame var 'td_int_arr[td_int_idx_1]'", error=True)
-        self.expect("frame var 'td_int_arr[td_td_int_idx_2]'", error=True)
-        self.expect_var_path("td_int_ptr[0]", True, value="1")
-        self.expect("frame var 'td_int_ptr[td_int_idx_1]'", error=True)
-        self.expect("frame var 'td_int_ptr[td_td_int_idx_2]'", error=True)
+        self.expect_var_path("td_int_arr[0]", value="1")
+        self.expect_var_path("td_int_arr[td_int_idx_1]", value="2")
+        self.expect_var_path("td_int_arr[td_td_int_idx_2]", value="3")
+        self.expect_var_path("td_int_ptr[0]", value="1")
+        self.expect_var_path("td_int_ptr[td_int_idx_1]", value="2")
+        self.expect_var_path("td_int_ptr[td_td_int_idx_2]", value="3")
 
         # Both typedefs and refs
-        self.expect("frame var 'td_int_arr_ref[td_int_idx_1_ref]'", error=True)
+        self.expect_var_path("td_int_arr_ref[td_int_idx_1_ref]", value="2")
 
         # Test for index out of bounds. 1 beyond the end.
-        self.expect_var_path("int_arr[3]", True, type="int")
+        self.expect_var_path("int_arr[3]", type="int")
         # Far beyond the end (but not far enough to be off the top of the stack).
-        self.expect_var_path("int_arr[10]", True, type="int")
+        self.expect_var_path("int_arr[10]", type="int")
 
         # Test address-of of the subscripted value.
         self.expect_var_path("*(&int_arr[1])", value="2")
 
         # Test for negative index.
-        self.expect_var_path("int_ptr_1[-1]", True, value="1")
+        self.expect_var_path("int_ptr_1[-1]", value="1")
 
         # Test for floating point index
         self.expect(
             "frame var 'int_arr[1.0]'",
             error=True,
-            substrs=["failed to parse integer constant: <'1.0' (float_constant)>"],
+            substrs=["array subscript is not an integer"],
         )
 
         # Test accessing bits in scalar types.
@@ -85,7 +77,7 @@ def test_subscript(self):
         self.expect(
             "frame var 'int_arr[int_ptr]'",
             error=True,
-            substrs=["failed to parse integer constant"],
+            substrs=["array subscript is not an integer"],
         )
 
         # Base should not be a pointer to void

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

@@ -6,6 +6,8 @@ class myArray {
   int m_arr_size = 4;
 };
 
+void stop() {}
+
 int main(int argc, char **argv) {
   int int_arr[] = {1, 2, 3};
   int *int_ptr = int_arr;
@@ -38,5 +40,6 @@ int main(int argc, char **argv) {
   myArray ma;
   myArray *ma_ptr = &ma;
 
-  return 0; // Set a breakpoint here
+  stop(); // Set a breakpoint here
+  return 0;
 }