Skip to content

Commit 3e53e25

Browse files
committed
Address reviewer comments:
- change references to C-style casting to just casting - add comments to cast enumeration types - use early-return
1 parent 44cf7ad commit 3e53e25

File tree

5 files changed

+46
-45
lines changed

5 files changed

+46
-45
lines changed

lldb/include/lldb/ValueObject/DILAST.h

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ enum class NodeKind {
2121
eArraySubscriptNode,
2222
eBitExtractionNode,
2323
eBooleanLiteralNode,
24-
eCStyleCastNode,
24+
eCastNode,
2525
eErrorNode,
2626
eFloatLiteralNode,
2727
eIdentifierNode,
@@ -36,12 +36,12 @@ enum class UnaryOpKind {
3636
Deref, // "*"
3737
};
3838

39-
/// The C-Style casts allowed by DIL.
40-
enum class CStyleCastKind {
41-
eEnumeration,
42-
eNullptr,
43-
eReference,
44-
eNone,
39+
/// The type casts allowed by DIL.
40+
enum class CastKind {
41+
eEnumeration, /// Casting from a scalar to an enumeration type
42+
eNullptr, /// Casting to a nullptr type
43+
eReference, /// Casting to a reference type
44+
eNone, /// Type promotion casting
4545
};
4646

4747
/// Forward declaration, for use in DIL AST nodes. Definition is at the very
@@ -253,27 +253,27 @@ class BooleanLiteralNode : public ASTNode {
253253
bool m_value;
254254
};
255255

256-
class CStyleCastNode : public ASTNode {
256+
class CastNode : public ASTNode {
257257
public:
258-
CStyleCastNode(uint32_t location, CompilerType type, ASTNodeUP operand,
259-
CStyleCastKind kind)
260-
: ASTNode(location, NodeKind::eCStyleCastNode), m_type(type),
258+
CastNode(uint32_t location, CompilerType type, ASTNodeUP operand,
259+
CastKind kind)
260+
: ASTNode(location, NodeKind::eCastNode), m_type(type),
261261
m_operand(std::move(operand)), m_cast_kind(kind) {}
262262

263263
llvm::Expected<lldb::ValueObjectSP> Accept(Visitor *v) const override;
264264

265265
CompilerType GetType() const { return m_type; }
266266
ASTNode *GetOperand() const { return m_operand.get(); }
267-
CStyleCastKind GetCastKind() const { return m_cast_kind; }
267+
CastKind GetCastKind() const { return m_cast_kind; }
268268

269269
static bool classof(const ASTNode *node) {
270-
return node->GetKind() == NodeKind::eCStyleCastNode;
270+
return node->GetKind() == NodeKind::eCastNode;
271271
}
272272

273273
private:
274274
CompilerType m_type;
275275
ASTNodeUP m_operand;
276-
CStyleCastKind m_cast_kind;
276+
CastKind m_cast_kind;
277277
};
278278

279279
/// This class contains one Visit method for each specialized type of
@@ -300,7 +300,7 @@ class Visitor {
300300
virtual llvm::Expected<lldb::ValueObjectSP>
301301
Visit(const BooleanLiteralNode *node) = 0;
302302
virtual llvm::Expected<lldb::ValueObjectSP>
303-
Visit(const CStyleCastNode *node) = 0;
303+
Visit(const CastNode *node) = 0;
304304
};
305305

306306
} // namespace lldb_private::dil

lldb/include/lldb/ValueObject/DILEval.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ class Interpreter : Visitor {
6161
llvm::Expected<lldb::ValueObjectSP>
6262
Visit(const BooleanLiteralNode *node) override;
6363
llvm::Expected<lldb::ValueObjectSP>
64-
Visit(const CStyleCastNode *node) override;
64+
Visit(const CastNode *node) override;
6565

6666
llvm::Expected<CompilerType>
6767
PickIntegerType(lldb::TypeSystemSP type_system,

lldb/source/ValueObject/DILAST.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ BooleanLiteralNode::Accept(Visitor *v) const {
5151
return v->Visit(this);
5252
}
5353

54-
llvm::Expected<lldb::ValueObjectSP> CStyleCastNode::Accept(Visitor *v) const {
54+
llvm::Expected<lldb::ValueObjectSP> CastNode::Accept(Visitor *v) const {
5555
return v->Visit(this);
5656
}
5757

lldb/source/ValueObject/DILEval.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -609,7 +609,7 @@ Interpreter::Visit(const BooleanLiteralNode *node) {
609609
}
610610

611611
llvm::Expected<lldb::ValueObjectSP>
612-
Interpreter::Visit(const CStyleCastNode *node) {
612+
Interpreter::Visit(const CastNode *node) {
613613
auto operand_or_err = Evaluate(node->GetOperand());
614614
if (!operand_or_err)
615615
return operand_or_err;

lldb/source/ValueObject/DILParser.cpp

Lines changed: 28 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -93,43 +93,44 @@ ASTNodeUP DILParser::ParseExpression() { return ParseCastExpression(); }
9393
// "(" type_id ")" cast_expression
9494

9595
ASTNodeUP DILParser::ParseCastExpression() {
96-
// This can be a C-style cast, try parsing the contents as a type declaration.
97-
if (CurToken().Is(Token::l_paren)) {
98-
Token token = CurToken();
99-
uint32_t loc = token.GetLocation();
96+
if (!CurToken().Is(Token::l_paren))
97+
return ParseUnaryExpression();
10098

101-
// Enable lexer backtracking, so that we can rollback in case it's not
102-
// actually a type declaration.
99+
// This could be a type cast, try parsing the contents as a type declaration.
100+
Token token = CurToken();
101+
uint32_t loc = token.GetLocation();
103102

104-
// Start tentative parsing (save token location/idx, for possible rollback).
105-
uint32_t save_token_idx = m_dil_lexer.GetCurrentTokenIdx();
103+
// Enable lexer backtracking, so that we can rollback in case it's not
104+
// actually a type declaration.
106105

107-
// Consume the token only after enabling the backtracking.
108-
m_dil_lexer.Advance();
106+
// Start tentative parsing (save token location/idx, for possible rollback).
107+
uint32_t save_token_idx = m_dil_lexer.GetCurrentTokenIdx();
109108

110-
// Try parsing the type declaration. If the returned value is not valid,
111-
// then we should rollback and try parsing the expression.
112-
auto type_id = ParseTypeId();
113-
if (type_id) {
114-
// Successfully parsed the type declaration. Commit the backtracked
115-
// tokens and parse the cast_expression.
109+
// Consume the token only after enabling the backtracking.
110+
m_dil_lexer.Advance();
116111

117-
if (!type_id.value().IsValid())
118-
return std::make_unique<ErrorNode>();
112+
// Try parsing the type declaration. If the returned value is not valid,
113+
// then we should rollback and try parsing the expression.
114+
auto type_id = ParseTypeId();
115+
if (type_id) {
116+
// Successfully parsed the type declaration. Commit the backtracked
117+
// tokens and parse the cast_expression.
119118

120-
Expect(Token::r_paren);
121-
m_dil_lexer.Advance();
122-
auto rhs = ParseCastExpression();
119+
if (!type_id.value().IsValid())
120+
return std::make_unique<ErrorNode>();
123121

124-
return std::make_unique<CStyleCastNode>(
125-
loc, type_id.value(), std::move(rhs), CStyleCastKind::eNone);
126-
}
122+
Expect(Token::r_paren);
123+
m_dil_lexer.Advance();
124+
auto rhs = ParseCastExpression();
127125

128-
// Failed to parse the contents of the parentheses as a type declaration.
129-
// Rollback the lexer and try parsing it as unary_expression.
130-
TentativeParsingRollback(save_token_idx);
126+
return std::make_unique<CastNode>(
127+
loc, type_id.value(), std::move(rhs), CastKind::eNone);
131128
}
132129

130+
// Failed to parse the contents of the parentheses as a type declaration.
131+
// Rollback the lexer and try parsing it as unary_expression.
132+
TentativeParsingRollback(save_token_idx);
133+
133134
return ParseUnaryExpression();
134135
}
135136

0 commit comments

Comments
 (0)