-
Notifications
You must be signed in to change notification settings - Fork 15.1k
[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 all commits
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 |
|---|---|---|
|
|
@@ -608,4 +608,16 @@ Interpreter::Visit(const BooleanLiteralNode *node) { | |
| return ValueObject::CreateValueObjectFromBool(m_target, value, "result"); | ||
| } | ||
|
|
||
| llvm::Expected<lldb::ValueObjectSP> | ||
| Interpreter::Visit(const CastNode *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; | ||
|
Comment on lines
+618
to
+620
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we aim to merge this PR separately? Is so, this should return an unimplemented error (or something like that) for now, so that the calling code can fall back to full expression evaluator. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes we plan to merge it separately. I don't think we need to advertise that it doesn't work properly yet (since we're not advertising that this feature is going in at all). So I'm ok with 'type casting' silently doing nothing for now. But if others disagree I could return an error here for now... Other opinions? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's just since |
||
| } | ||
|
|
||
| } // 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,63 @@ 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() { | ||
| if (!CurToken().Is(Token::l_paren)) | ||
| return ParseUnaryExpression(); | ||
|
|
||
| // This could be a type cast, try parsing the contents as a type declaration. | ||
| Token token = CurToken(); | ||
| uint32_t loc = token.GetLocation(); | ||
|
|
||
| // Enable lexer backtracking, so that we can rollback in case it's not | ||
| // actually a type declaration. | ||
|
|
||
| // Start tentative parsing (save token location/idx, for possible rollback). | ||
| uint32_t save_token_idx = m_dil_lexer.GetCurrentTokenIdx(); | ||
|
|
||
| // Consume the token only after enabling the backtracking. | ||
| m_dil_lexer.Advance(); | ||
|
|
||
| // Try parsing the type declaration. If the returned value is not valid, | ||
| // then we should rollback and try parsing the expression. | ||
| auto type_id = ParseTypeId(); | ||
| if (type_id) { | ||
| // Successfully parsed the type declaration. Commit the backtracked | ||
| // tokens and parse the cast_expression. | ||
|
|
||
| if (!type_id.value().IsValid()) | ||
| return std::make_unique<ErrorNode>(); | ||
|
|
||
| Expect(Token::r_paren); | ||
| m_dil_lexer.Advance(); | ||
| auto rhs = ParseCastExpression(); | ||
|
|
||
| return std::make_unique<CastNode>( | ||
| loc, type_id.value(), std::move(rhs), CastKind::eNone); | ||
| } | ||
|
|
||
| // Failed to parse the contents of the parentheses as a type declaration. | ||
| // Rollback the lexer and try parsing it as unary_expression. | ||
| TentativeParsingRollback(save_token_idx); | ||
|
|
||
| return ParseUnaryExpression(); | ||
| } | ||
|
|
||
| // Parse an unary_expression. | ||
| // | ||
| // unary_expression: | ||
| // postfix_expression | ||
| // unary_operator expression | ||
| // unary_operator cast_expression | ||
| // | ||
| // unary_operator: | ||
| // "&" | ||
|
|
@@ -99,7 +149,7 @@ ASTNodeUP DILParser::ParseUnaryExpression() { | |
| Token token = CurToken(); | ||
| uint32_t loc = token.GetLocation(); | ||
| m_dil_lexer.Advance(); | ||
| auto rhs = ParseExpression(); | ||
| auto rhs = ParseCastExpression(); | ||
| switch (token.GetKind()) { | ||
| case Token::star: | ||
| return std::make_unique<UnaryOpNode>(loc, UnaryOpKind::Deref, | ||
|
|
@@ -274,6 +324,81 @@ std::string DILParser::ParseNestedNameSpecifier() { | |
| } | ||
| } | ||
|
|
||
| // Parse a type_id. | ||
| // | ||
| // type_id: | ||
| // type_specifier_seq [abstract_declarator] | ||
| // | ||
| // type_specifier_seq: | ||
| // type_specifier [type_specifier] | ||
| // | ||
| // type_specifier: | ||
| // ["::"] [nested_name_specifier] type_name // not handled for now! | ||
| // builtin_typename | ||
| // | ||
| std::optional<CompilerType> DILParser::ParseTypeId() { | ||
| CompilerType type; | ||
| // For now only allow builtin types -- will expand add to this later. | ||
| auto maybe_builtin_type = ParseBuiltinType(); | ||
| if (maybe_builtin_type) { | ||
| type = *maybe_builtin_type; | ||
| } else | ||
| return {}; | ||
|
|
||
| // | ||
| // abstract_declarator: | ||
| // ptr_operator [abstract_declarator] | ||
| // | ||
| std::vector<Token> ptr_operators; | ||
| while (CurToken().IsOneOf({Token::star, Token::amp})) { | ||
| Token tok = CurToken(); | ||
| ptr_operators.push_back(std::move(tok)); | ||
| m_dil_lexer.Advance(); | ||
| } | ||
| type = ResolveTypeDeclarators(type, ptr_operators); | ||
|
|
||
| return type; | ||
| } | ||
|
|
||
| // Parse a built-in type | ||
| // | ||
| // builtin_typename: | ||
| // identifer_seq | ||
| // | ||
| // identifier_seq | ||
| // identifer [identifier_seq] | ||
| // | ||
| // A built-in type can be a single identifier or a space-separated | ||
| // list of identifiers (e.g. "short" or "long long"). | ||
| std::optional<CompilerType> DILParser::ParseBuiltinType() { | ||
| std::string type_name = ""; | ||
| uint32_t save_token_idx = m_dil_lexer.GetCurrentTokenIdx(); | ||
| bool first_word = true; | ||
| while (CurToken().GetKind() == Token::identifier) { | ||
| if (CurToken().GetSpelling() == "const" || | ||
| CurToken().GetSpelling() == "volatile") | ||
|
Comment on lines
+378
to
+379
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we be throwing away the CV-qualifiers here? I realise this is required for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. From what I understand, only There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We are not planning on processing or doing anything with the CV qualifiers. We could give a parsing error here, but it seemed kinder to just ignore them? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually, I take that back. We can't exactly give error messages on them, because we don't know (at this point during the parse) if we're actually handling a type name or not. It's conceivable that this could be an expression in a language where "const" or "volatile" are legal identifier names. Or it could really be a type name and the user threw these in as qualifiers...but we're not handling the CV qualifiers as part of the cast....So silently dropping them seems the best thing we can do here. Or I suppose if we enounter them we could treat it as definitely NOT a type name, but that seems weird, too. |
||
| continue; | ||
| if (!first_word) | ||
| type_name.push_back(' '); | ||
| else | ||
| first_word = false; | ||
| type_name.append(CurToken().GetSpelling()); | ||
| m_dil_lexer.Advance(); | ||
| } | ||
|
|
||
| if (type_name.size() > 0) { | ||
| lldb::TargetSP target_sp = m_ctx_scope->CalculateTarget(); | ||
| ConstString const_type_name(type_name.c_str()); | ||
| for (auto type_system_sp : target_sp->GetScratchTypeSystems()) | ||
| if (auto compiler_type = | ||
| type_system_sp->GetBuiltinTypeByName(const_type_name)) | ||
| return compiler_type; | ||
| } | ||
|
|
||
| TentativeParsingRollback(save_token_idx); | ||
| return {}; | ||
| } | ||
|
|
||
| // Parse an id_expression. | ||
| // | ||
| // id_expression: | ||
|
|
@@ -339,6 +464,40 @@ std::string DILParser::ParseUnqualifiedId() { | |
| return identifier; | ||
| } | ||
|
|
||
| CompilerType | ||
| DILParser::ResolveTypeDeclarators(CompilerType type, | ||
| const std::vector<Token> &ptr_operators) { | ||
| CompilerType bad_type; | ||
| // Resolve pointers/references. | ||
| for (Token tk : ptr_operators) { | ||
| uint32_t loc = tk.GetLocation(); | ||
| if (tk.GetKind() == Token::star) { | ||
| // Pointers to reference types are forbidden. | ||
| if (type.IsReferenceType()) { | ||
| BailOut(llvm::formatv("'type name' declared as a pointer to a " | ||
| "reference of type {0}", | ||
| type.TypeDescription()), | ||
| loc, CurToken().GetSpelling().length()); | ||
| return bad_type; | ||
| } | ||
|
Comment on lines
+475
to
+482
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 commentThe 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 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 commentThe 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 commentThe reason will be displayed to describe this comment to others. Learn more. Is it hurting anything to leave this in for now? |
||
| // Get pointer type for the base type: e.g. int* -> int**. | ||
| type = type.GetPointerType(); | ||
|
|
||
| } else if (tk.GetKind() == Token::amp) { | ||
| // References to references are forbidden. | ||
| if (type.IsReferenceType()) { | ||
| BailOut("type name declared as a reference to a reference", loc, | ||
| CurToken().GetSpelling().length()); | ||
| return bad_type; | ||
| } | ||
| // Get reference type for the base type: e.g. int -> int&. | ||
| type = type.GetLValueReferenceType(); | ||
| } | ||
| } | ||
|
|
||
| return type; | ||
| } | ||
|
|
||
| // Parse an boolean_literal. | ||
| // | ||
| // boolean_literal: | ||
|
|
||
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.
would be the more general form?
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.
In the currently shown grammar yes. But the full grammar for the full DIL expression language has quite a few layers between "expression" and "cast_expression", and I don't think it would be correct to allow some of them here. So I would prefer to keep this as is at the moment, in anticipation of future changes. But I can make your suggested change if you feel strongly about it.