-
Notifications
You must be signed in to change notification settings - Fork 15.3k
[LLDB] Add type casting to DIL, part 1 of 3. #165199
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 1 commit
44cf7ad
3e53e25
9b3d261
f6c7946
eb27812
215160a
e682895
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,6 +21,7 @@ enum class NodeKind { | |
| eArraySubscriptNode, | ||
| eBitExtractionNode, | ||
| eBooleanLiteralNode, | ||
| eCStyleCastNode, | ||
| eErrorNode, | ||
| eFloatLiteralNode, | ||
| eIdentifierNode, | ||
|
|
@@ -35,6 +36,14 @@ enum class UnaryOpKind { | |
| Deref, // "*" | ||
| }; | ||
|
|
||
| /// The C-Style casts allowed by DIL. | ||
|
||
| enum class CStyleCastKind { | ||
| eEnumeration, | ||
| eNullptr, | ||
| eReference, | ||
| eNone, | ||
|
||
| }; | ||
|
|
||
| /// Forward declaration, for use in DIL AST nodes. Definition is at the very | ||
| /// end of this file. | ||
| class Visitor; | ||
|
|
@@ -244,6 +253,29 @@ class BooleanLiteralNode : public ASTNode { | |
| bool m_value; | ||
| }; | ||
|
|
||
| class CStyleCastNode : public ASTNode { | ||
| public: | ||
| CStyleCastNode(uint32_t location, CompilerType type, ASTNodeUP operand, | ||
| CStyleCastKind kind) | ||
| : ASTNode(location, NodeKind::eCStyleCastNode), m_type(type), | ||
| m_operand(std::move(operand)), m_cast_kind(kind) {} | ||
|
|
||
| llvm::Expected<lldb::ValueObjectSP> Accept(Visitor *v) const override; | ||
|
|
||
| CompilerType GetType() const { return m_type; } | ||
| ASTNode *GetOperand() const { return m_operand.get(); } | ||
| CStyleCastKind GetCastKind() const { return m_cast_kind; } | ||
|
|
||
| static bool classof(const ASTNode *node) { | ||
| return node->GetKind() == NodeKind::eCStyleCastNode; | ||
| } | ||
|
|
||
| private: | ||
| CompilerType m_type; | ||
| ASTNodeUP m_operand; | ||
| CStyleCastKind m_cast_kind; | ||
| }; | ||
|
|
||
| /// This class contains one Visit method for each specialized type of | ||
| /// DIL AST node. The Visit methods are used to dispatch a DIL AST node to | ||
| /// the correct function in the DIL expression evaluator for evaluating that | ||
|
|
@@ -267,6 +299,8 @@ class Visitor { | |
| Visit(const FloatLiteralNode *node) = 0; | ||
| virtual llvm::Expected<lldb::ValueObjectSP> | ||
| Visit(const BooleanLiteralNode *node) = 0; | ||
| virtual llvm::Expected<lldb::ValueObjectSP> | ||
| Visit(const CStyleCastNode *node) = 0; | ||
| }; | ||
|
|
||
| } // namespace lldb_private::dil | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -608,4 +608,16 @@ Interpreter::Visit(const BooleanLiteralNode *node) { | |
| return ValueObject::CreateValueObjectFromBool(m_target, value, "result"); | ||
| } | ||
|
|
||
| llvm::Expected<lldb::ValueObjectSP> | ||
| Interpreter::Visit(const CStyleCastNode *node) { | ||
| auto operand_or_err = Evaluate(node->GetOperand()); | ||
| if (!operand_or_err) | ||
| return operand_or_err; | ||
|
|
||
| lldb::ValueObjectSP operand = *operand_or_err; | ||
| // Don't actually do the cast for now -- that code will be added later. | ||
| // For now just return the original operand, unchanged. | ||
| return operand; | ||
|
||
| } | ||
|
|
||
| } // namespace lldb_private::dil | ||
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -12,7 +12,9 @@ | |||||||||||
| //===----------------------------------------------------------------------===// | ||||||||||||
|
|
||||||||||||
| #include "lldb/ValueObject/DILParser.h" | ||||||||||||
| #include "lldb/Symbol/CompileUnit.h" | ||||||||||||
| #include "lldb/Target/ExecutionContextScope.h" | ||||||||||||
| #include "lldb/Target/LanguageRuntime.h" | ||||||||||||
| #include "lldb/Utility/DiagnosticsRendering.h" | ||||||||||||
| #include "lldb/ValueObject/DILAST.h" | ||||||||||||
| #include "lldb/ValueObject/DILEval.h" | ||||||||||||
|
|
@@ -80,15 +82,62 @@ ASTNodeUP DILParser::Run() { | |||||||||||
| // Parse an expression. | ||||||||||||
| // | ||||||||||||
| // expression: | ||||||||||||
| // unary_expression | ||||||||||||
| // cast_expression | ||||||||||||
| // | ||||||||||||
| ASTNodeUP DILParser::ParseExpression() { return ParseUnaryExpression(); } | ||||||||||||
| ASTNodeUP DILParser::ParseExpression() { return ParseCastExpression(); } | ||||||||||||
|
|
||||||||||||
| // Parse a cast_expression. | ||||||||||||
| // | ||||||||||||
| // cast_expression: | ||||||||||||
| // unary_expression | ||||||||||||
| // "(" type_id ")" cast_expression | ||||||||||||
|
|
||||||||||||
| ASTNodeUP DILParser::ParseCastExpression() { | ||||||||||||
| // This can be a C-style cast, try parsing the contents as a type declaration. | ||||||||||||
| if (CurToken().Is(Token::l_paren)) { | ||||||||||||
|
||||||||||||
| if (CurToken().Is(Token::l_paren)) { | |
| if (!CurToken().Is(Token::l_paren)) | |
| return ParseUnaryExpression(); | |
| ... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will do.
Michael137 marked this conversation as resolved.
Show resolved
Hide resolved
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lets get rid of this and just return {} in the failure branches.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this too C++ specific? Maybe this should live in TypeSystem?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes me think, should we support references at all for now? Might be wrong, but I'd think casting to a reference type is not a super common use-case. Once we add & as a ptr_operator, we're saying & is the reference syntax for all languages.
Again, if this has already been discussed elsewhere, feel free to ignore
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess we could drop cast to &, we don't have an assignment operator yet anyway. The check should stay regardless though, because if DIL cannot handle something, it must return an error rather then an incorrect value.
But in general, this is a C-style cast with C-style syntax, I think it should be expected to be used only with C/C++ code, since DIL will be mostly used automatically without the programmer knowing that DIL will try to evaluate the expression before the compiler.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it hurting anything to leave this in for now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But in general, this is a C-style cast with C-style syntax, I think it should be expected to be used only with C/C++ code
But isn't DIL supposed to be the de-facto inspection language? Regardless of what language you're debugging. Yes it follows mostly C/C++ syntax but here you are checking explicitly C++ language semantics, not just syntax.
Maybe I'm incorrectly delineating DIL and the language plugins.
E.g., an alternative would be to say:
if (tk.GetKind() == Token::star)
type = type.GetPointerType();
if (!type) {
BailOut("couldn't get a pointer type to ....");
return {};
}
And the language plugin decide whether it can create a pointer to a reference type.
Wdyt?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is the intention of the DIL to be the general introspection language. We should try to make it useable to people who aren't just debugging C but the overall idea is that most people are familiar with C so making the syntax look C-ish is a good way to achieve that goal. Remember that this is not the expression parser, whose job it is to capture all the subtleties of any given language. This is primarily a way to examine data values.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My concern was that if we have something like:
(lldb) frame var (Foo &*)foo
Why is the DIL parser deciding that this is an invalid cast for all languages? Shouldn't that be the TypeSystem's job? In my mind the DIL parser just consumes tokens and dispatches to the relevant type system to conjure up the types that it thinks it needs to create. And if the TypeSystem doesn't allow making a pointer type to a reference type, then so be it.
Am I being overly pedantic here? I don't mind keeping this here, just wanted to make sure I understand the interaction between TypeSystem and DIL.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the DIL is interpreting: (Typename <operator>) Identifier to mean "re-interpret Identifier as an <operator> variant of "Typename" regardless of how the underlying language would actually spell that operation, similarly * and & can't just mean "pass a * to the language and see what it does with it" since the underlying language may very well not spell it that way.
So if the DIL can convince itself some combination of primitive operations in a cast doesn't make sense in the abstract, it should be free to reject it. But if it just isn't allowed when spelled that way in the languages we know, then it should pass that off to the type system.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But in general, this is a C-style cast with C-style syntax, I think it should be expected to be used only with C/C++ code, since DIL will be mostly used automatically without the programmer knowing that DIL will try to evaluate the expression before the compiler.
But isn't DIL supposed to be the de-facto inspection language? Regardless of what language you're debugging. Yes it follows mostly C/C++ syntax but here you are checking explicitly C++ language semantics, not just syntax.
It is the intention of the DIL to be the general introspection language. We should try to make it useable to people who aren't just debugging C but the overall idea is that most people are familiar with C so making the syntax look C-ish is a good way to achieve that goal. Remember that this is not the expression parser, whose job it is to capture all the subtleties of any given language. This is primarily a way to examine data values.
@Michael137 @jimingham
What I meant to say is that considering how we're integrating DIL into LLDB, most people won't even know that DIL exists. We just replaced the implementation of frame var with DIL and made people use it without realizing it. And this implementation is used as a first attempt in evaluating expressions when using an IDE via lldb-dap (for example), so people will be just writing expressions in the language that they're debugging.
So, on the one hand, what's the point going out of the way supporting C-style cast on languages with different reference symbol, or no explicit references at all (like Swift, if I understand correctly)? For someone to do this, they would need to know about DIL and to know that it is allowed to replace a reference symbol & with something else specifically in a C-style cast. On the other hand, the debugged code could be a mixture of C++ and Swift objects, so maybe this could be useful to have anyway. I'm really not sure, I thought we could eventually just support different syntax constructions from different languages which underneath could do the same thing, or at least very similar ones. For example, having both a C-style cast and a keyword as for Swift and Rust, which is why I also suggested to keep the name "CStyleCastNode" in the code. This hasn't really come up in any meetings about DIL afaik, so maybe we need to discuss this at some point.
But for now it's probably a good idea anyway to check pointer/reference operators during evaluation, although disallowing reference to pointer and reference to reference is exclusive to this cast. Normally, we can do that, so existing type system functions like GetPointerType won't work here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The DIL implements the inspection of values in memory. The operations it supports aren't intended to be "language accurate", for instance we aren't calling overloaded operators, etc. And because of the ways you can name the children provided by synthetic child providers, it may even not describe the objects as they might appear in the source language.
So for that reason, I don't want to give the impression that this IS the expression parser. That's one reason why I think it isn't terribly important that we support each (or even the current) language's way to express cast, dereference, etc. That's just doing work to make an impression that isn't correct.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we add a FIXME for rvalue references (i.e., &&)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
Uh oh!
There was an error while loading. Please reload this page.