[lldb] Use AST nodes as Subscript and BitField arguments in DIL #169363
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kuilpd
requested review from
Michael137,
adrian-prantl,
asl,
cmtice and
jimingham
Member
|
@llvm/pr-subscribers-lldb Author: Ilia Kuklin (kuilpd) ChangesUse AST nodes as Subscript and BitField arguments instead of bare integers. This enables using any supported expression as an array or bit index. Patch is 20.31 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/169363.diff 10 Files Affected:
diff --git a/lldb/docs/dil-expr-lang.ebnf b/lldb/docs/dil-expr-lang.ebnf
index ccd2b00223910..554b107c117e0 100644
--- a/lldb/docs/dil-expr-lang.ebnf
+++ b/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 ? ;
diff --git a/lldb/include/lldb/ValueObject/DILAST.h b/lldb/include/lldb/ValueObject/DILAST.h
index 91f8d93c09622..31de575f25fb0 100644
--- a/lldb/include/lldb/ValueObject/DILAST.h
+++ b/lldb/include/lldb/ValueObject/DILAST.h
@@ -141,14 +141,14 @@ 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;
@@ -156,22 +156,22 @@ class ArraySubscriptNode : public ASTNode {
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;
@@ -179,8 +179,8 @@ class BitFieldExtractionNode : public ASTNode {
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 };
diff --git a/lldb/include/lldb/ValueObject/DILEval.h b/lldb/include/lldb/ValueObject/DILEval.h
index a65edc58cc4e7..b88177a269ab5 100644
--- a/lldb/include/lldb/ValueObject/DILEval.h
+++ b/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;
diff --git a/lldb/include/lldb/ValueObject/DILParser.h b/lldb/include/lldb/ValueObject/DILParser.h
index c9d28333ffed1..0633ab08e849b 100644
--- a/lldb/include/lldb/ValueObject/DILParser.h
+++ b/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();
diff --git a/lldb/source/ValueObject/DILEval.cpp b/lldb/source/ValueObject/DILEval.cpp
index 40a05a467f883..46d949a52f880 100644
--- a/lldb/source/ValueObject/DILEval.cpp
+++ b/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) {
diff --git a/lldb/source/ValueObject/DILParser.cpp b/lldb/source/ValueObject/DILParser.cpp
index 072ddff1e28d2..b23c667e883d8 100644
--- a/lldb/source/ValueObject/DILParser.cpp
+++ b/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:
diff --git a/lldb/test/API/commands/frame/var-dil/basics/ArraySubscript/TestFrameVarDILArraySubscript.py b/lldb/test/API/commands/frame/var-dil/basics/ArraySubscript/TestFrameVarDILArraySubscript.py
index f47e86266f474..33d2e3c4fc2b2 100644
--- a/lldb/test/API/commands/frame/var-dil/basics/ArraySubscript/TestFrameVarDILArraySubscript.py
+++ b/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
diff --git a/lldb/test/API/commands/frame/var-dil/basics/ArraySubscript/main.cpp b/lldb/test/API/commands/frame/var-dil/basics/ArraySubscript/main.cpp
index 03ad3e872ca76..241e93a0240cb 100644
--- a/lldb/test/API/commands/frame/var-dil/basics/ArraySubscript/main.cpp
+++ b/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;
}
diff --git a/lldb/test/API/commands/frame/var-dil/basics/BitFieldExtraction/TestFrameVarDILBitFieldExtraction.py b/lldb/test/API/commands/frame/var-dil/basics/BitFieldExtraction/TestFrameVarDILBitFieldExtraction.py
index 7b5ef0650b6e1..476c072b1cdc8 100644
--- a/lldb/test/API/commands/frame/var-dil/basics/BitFieldExtraction/TestFrameVarDILBitFieldExtraction.py
+++ b/lldb/test/API/commands/frame/var-dil/basics/BitFieldExtraction/TestFrameVarDILBitFieldExtraction.py
@@ -11,14 +11,6 @@
class TestFrameVarDILBitFieldExtraction(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_bitfield_extraction(self):
self.build()
lldbutil.run_to_source_breakpoint(
@@ -28,14 +20,21 @@ def test_bitfield_extraction(self):
self.runCmd("settings set target.experimental.use-DIL true")
# Test ranges and type
- self.expect_var_path("value[0-1]", True, value="3", type="int:2")
- self.expect_var_path("value[4-7]", True, value="7", type="int:4")
- self.expect_var_path("value[7-0]", True, value="115", type="int:8")
+ self.expect_var_path("value[0-1]", value="3", type="int:2")
+ self.expect_var_path("value[4-7]", value="7", type="int:4")
+ self.expect_var_path("value[7-0]", value="115", type="int:8")
# Test reference and dereferenced pointer
self.expect_var_path("value_ref[0-1]", value="3", type="int:2")
self.expect_var_path("(*value_ptr)[0-1]", value="3", type="int:2")
+ # Test ranges as variable, reference, enum
+ self.expect_var_path("value[idx_0-idx_1]", value="3", type="int:2")
+ self.expect_var_path("value[0-idx_1_ref]", value="3", type="int:2")
+ self.expect_var_path("value[idx_1_ref-0]", value="3", type="int:2")
+ self.expect_var_path("value[0-enum_one]", value="3", type="int:2")
+ self.expect_var_path("value[enum_one-0]", value="3", type="int:2")
+
# Test array and pointer
self.expect(
"frame var 'int_arr[0-2]'",
@@ -52,5 +51,10 @@ def test_bitfield_extraction(self):
self.expect(
"frame var 'value[1-]'",
error=True,
- substrs=["failed to parse integer constant"],
+ substrs=["Unexpected token: <']' (r_square)>"],
+ )
+ self.expect(
+ "frame var 'value[1-2.0]'",
+ error=True,
+ substrs=["bit index is not an integer"],
)
diff --git a/lldb/test/API/commands/frame/var-dil/basics/BitFieldExtraction/main.cpp b/lldb/test/API/commands/frame/var-dil/basics/BitFieldExtraction/main.cpp
index a35f68a9e30a8..122ea4e1acded 100644
--- a/lldb/test/API/commands/frame/var-dil/basics/BitFieldExtraction/main.cpp
+++ b/lldb/test/API/commands/frame/var-dil/basics/BitFieldExtraction/main.cpp
@@ -1,9 +1,17 @@
+void stop() {}
+
int main(int argc, char **argv) {
int ...
[truncated]
|
Contributor
Author
|
@Michael137 @jimingham |
| postfix_expression = primary_expression | ||
| | postfix_expression "[" integer_literal "]" | ||
| | postfix_expression "[" expression "]" | ||
| | postfix_expression "[" expression "-" expression "]" |
Collaborator
There was a problem hiding this comment.
What is this production representing?
Contributor
Author
There was a problem hiding this comment.
@adrian-prantl
- is for the bit range in bitfield extraction, e.g. value[0-7], it was just missing from the grammar before. I also plan to discuss in the next PR changing - here to a more common in other languages : character, since - wouldn't work anymore once we add binary subtraction.
Contributor
Author
|
@Michael137 @jimingham |
Michael137
reviewed
Dec 9, 2025
Comment on lines
+167
to
+168
| m_base(std::move(base)), m_first_index(std::move(first_index)), | ||
| m_last_index(std::move(last_index)) {} |
Member
There was a problem hiding this comment.
Can these ever be null? If not, we could assert. And then hand out ASTNode& instead of pointers. (same question applies to ArraySubscriptNode).
Member
There was a problem hiding this comment.
Based on Interpreter::Evaluate, we unconditionally dereference (we should probably add an assert there). So these can't ever be nullptr
Contributor
Author
There was a problem hiding this comment.
Can these ever be null? If not, we could assert. And then hand out
ASTNode&instead of pointers. (same question applies toArraySubscriptNode).
Do you mean assert all ASTNodeUP arguments every time a new node is created, or put an assert in every getter? Either way, this should probably be a separate PR, since all existing nodes are already set up like this.
Michael137
reviewed
Dec 9, 2025
lldb/source/ValueObject/DILEval.cpp
Outdated
| auto base_or_err = Evaluate(node->GetBase()); | ||
| if (!base_or_err) | ||
| return base_or_err; | ||
| lldb::ValueObjectSP base = *base_or_err; |
Member
There was a problem hiding this comment.
Can we move this closer to the point of usage?
Also do we need to have a null-check here?
Contributor
Author
There was a problem hiding this comment.
The check is in Evaluate itself so that we don't have to do it every time it's used.
Michael137
reviewed
Dec 9, 2025
| 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; |
Michael137
reviewed
Dec 9, 2025
lldb/source/ValueObject/DILEval.cpp
Outdated
Comment on lines
666
to
667
| int64_t first_index = first_idx->GetValueAsSigned(0); | ||
| int64_t last_index = last_idx->GetValueAsSigned(0); |
Member
There was a problem hiding this comment.
We should probably check the success value (i.e., pass it the bool output parameter). Otherwise we'd return some range that the user didn't ask for
Michael137
reviewed
Dec 9, 2025
Comment on lines
+56
to
+59
| self.expect( | ||
| "frame var 'value[1-2.0]'", | ||
| error=True, | ||
| substrs=["bit index is not an integer"], |
Member
There was a problem hiding this comment.
Can we test this error for both sides of the expression? E.g., 2.0 - 3
Michael137
reviewed
Dec 9, 2025
| class TestFrameVarDILArraySubscript(TestBase): | ||
| NO_DEBUG_INFO_TESTCASE = True | ||
|
|
||
| def expect_var_path(self, expr, compare_to_framevar=False, value=None, type=None): |
Member
There was a problem hiding this comment.
Is the removal of this tied to this PR? Or a drive-by cleanup?
Contributor
Author
There was a problem hiding this comment.
Drive-by, this isn't really used in anymore
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Use AST nodes as Subscript and BitField arguments instead of bare integers. This enables using any supported expression as an array or bit index.