[LLDB] Add unary plus and minus to DIL #155617
Conversation
|
@llvm/pr-subscribers-lldb Author: Ilia Kuklin (kuilpd) ChangesThis patch adds unary nodes plus and minus, and introduces unary type conversions and integral promotion. Patch is 22.54 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/155617.diff 13 Files Affected:
diff --git a/lldb/include/lldb/Symbol/TypeSystem.h b/lldb/include/lldb/Symbol/TypeSystem.h
index 16a2e0b5a52fb..d74e6a0e05625 100644
--- a/lldb/include/lldb/Symbol/TypeSystem.h
+++ b/lldb/include/lldb/Symbol/TypeSystem.h
@@ -412,6 +412,13 @@ class TypeSystem : public PluginInterface,
GetIntegralTemplateArgument(lldb::opaque_compiler_type_t type, size_t idx,
bool expand_pack);
+ // DIL
+
+ virtual bool IsPromotableIntegerType(lldb::opaque_compiler_type_t type);
+
+ virtual llvm::Expected<CompilerType>
+ DoIntegralPromotion(CompilerType from, ExecutionContextScope *exe_scope);
+
// Dumping types
#ifndef NDEBUG
diff --git a/lldb/include/lldb/ValueObject/DILAST.h b/lldb/include/lldb/ValueObject/DILAST.h
index 1d10755c46e39..900eb8a792a1e 100644
--- a/lldb/include/lldb/ValueObject/DILAST.h
+++ b/lldb/include/lldb/ValueObject/DILAST.h
@@ -32,6 +32,8 @@ enum class NodeKind {
enum class UnaryOpKind {
AddrOf, // "&"
Deref, // "*"
+ Minus, // "-"
+ Plus, // "+"
};
/// Forward declaration, for use in DIL AST nodes. Definition is at the very
diff --git a/lldb/include/lldb/ValueObject/DILEval.h b/lldb/include/lldb/ValueObject/DILEval.h
index 5a48c2c989f4d..11f44336cbf55 100644
--- a/lldb/include/lldb/ValueObject/DILEval.h
+++ b/lldb/include/lldb/ValueObject/DILEval.h
@@ -59,6 +59,8 @@ class Interpreter : Visitor {
llvm::Expected<lldb::ValueObjectSP>
Visit(const FloatLiteralNode *node) override;
+ llvm::Expected<lldb::ValueObjectSP>
+ UnaryConversion(lldb::ValueObjectSP valobj);
llvm::Expected<CompilerType>
PickIntegerType(lldb::TypeSystemSP type_system,
std::shared_ptr<ExecutionContextScope> ctx,
diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
index 39aacdb58e694..4d4ca522e052a 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -7320,6 +7320,102 @@ CompilerType TypeSystemClang::GetTypeForFormatters(void *type) {
return CompilerType();
}
+bool TypeSystemClang::IsPromotableIntegerType(
+ lldb::opaque_compiler_type_t type) {
+ // Unscoped enums are always considered as promotable, even if their
+ // underlying type does not need to be promoted (e.g. "int").
+ bool is_signed = false;
+ bool isUnscopedEnumerationType =
+ IsEnumerationType(type, is_signed) && !IsScopedEnumerationType(type);
+ if (isUnscopedEnumerationType)
+ return true;
+
+ switch (GetBasicTypeEnumeration(type)) {
+ case lldb::eBasicTypeBool:
+ case lldb::eBasicTypeChar:
+ case lldb::eBasicTypeSignedChar:
+ case lldb::eBasicTypeUnsignedChar:
+ case lldb::eBasicTypeShort:
+ case lldb::eBasicTypeUnsignedShort:
+ case lldb::eBasicTypeWChar:
+ case lldb::eBasicTypeSignedWChar:
+ case lldb::eBasicTypeUnsignedWChar:
+ case lldb::eBasicTypeChar16:
+ case lldb::eBasicTypeChar32:
+ return true;
+
+ default:
+ return false;
+ }
+
+ llvm_unreachable("All cases handled above.");
+}
+
+llvm::Expected<CompilerType>
+TypeSystemClang::DoIntegralPromotion(CompilerType from,
+ ExecutionContextScope *exe_scope) {
+ if (!from.IsInteger() && !from.IsUnscopedEnumerationType())
+ return from;
+
+ if (!from.IsPromotableIntegerType())
+ return from;
+
+ if (from.IsUnscopedEnumerationType()) {
+ EnumDecl *enum_decl = GetAsEnumDecl(from);
+ CompilerType promotion_type = GetType(enum_decl->getPromotionType());
+ return DoIntegralPromotion(promotion_type, exe_scope);
+ }
+
+ lldb::BasicType builtin_type =
+ from.GetCanonicalType().GetBasicTypeEnumeration();
+ uint64_t from_size = 0;
+ if (builtin_type == lldb::eBasicTypeWChar ||
+ builtin_type == lldb::eBasicTypeSignedWChar ||
+ builtin_type == lldb::eBasicTypeUnsignedWChar ||
+ builtin_type == lldb::eBasicTypeChar16 ||
+ builtin_type == lldb::eBasicTypeChar32) {
+ // Find the type that can hold the entire range of values for our type.
+ bool is_signed = from.IsSigned();
+ llvm::Expected<uint64_t> from_size = from.GetByteSize(exe_scope);
+ if (!from_size)
+ return from_size.takeError();
+ CompilerType promote_types[] = {
+ GetBasicTypeFromAST(lldb::eBasicTypeInt),
+ GetBasicTypeFromAST(lldb::eBasicTypeUnsignedInt),
+ GetBasicTypeFromAST(lldb::eBasicTypeLong),
+ GetBasicTypeFromAST(lldb::eBasicTypeUnsignedLong),
+ GetBasicTypeFromAST(lldb::eBasicTypeLongLong),
+ GetBasicTypeFromAST(lldb::eBasicTypeUnsignedLongLong),
+ };
+ for (CompilerType &type : promote_types) {
+ llvm::Expected<uint64_t> byte_size = type.GetByteSize(exe_scope);
+ if (!byte_size)
+ return byte_size.takeError();
+ if (*from_size < *byte_size ||
+ (*from_size == *byte_size && is_signed == type.IsSigned())) {
+ return type;
+ }
+ }
+ llvm_unreachable("char type should fit into long long");
+ }
+
+ // Here we can promote only to "int" or "unsigned int".
+ CompilerType int_type = GetBasicTypeFromAST(lldb::eBasicTypeInt);
+ llvm::Expected<uint64_t> int_byte_size = int_type.GetByteSize(exe_scope);
+ if (!int_byte_size)
+ return int_byte_size.takeError();
+
+ // Signed integer types can be safely promoted to "int".
+ if (from.IsSigned()) {
+ return int_type;
+ }
+ // Unsigned integer types are promoted to "unsigned int" if "int" cannot hold
+ // their entire value range.
+ return (from_size == *int_byte_size)
+ ? GetBasicTypeFromAST(lldb::eBasicTypeUnsignedInt)
+ : int_type;
+}
+
clang::EnumDecl *TypeSystemClang::GetAsEnumDecl(const CompilerType &type) {
const clang::EnumType *enutype =
llvm::dyn_cast<clang::EnumType>(ClangUtil::GetCanonicalQualType(type));
diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
index 709f89590ba3b..0ca1a1e2578b3 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
@@ -939,6 +939,14 @@ class TypeSystemClang : public TypeSystem {
CompilerType GetTypeForFormatters(void *type) override;
+ // DIL
+
+ bool IsPromotableIntegerType(lldb::opaque_compiler_type_t type) override;
+
+ llvm::Expected<CompilerType>
+ DoIntegralPromotion(CompilerType from,
+ ExecutionContextScope *exe_scope) override;
+
#define LLDB_INVALID_DECL_LEVEL UINT32_MAX
// LLDB_INVALID_DECL_LEVEL is returned by CountDeclLevels if child_decl_ctx
// could not be found in decl_ctx.
diff --git a/lldb/source/Symbol/CompilerType.cpp b/lldb/source/Symbol/CompilerType.cpp
index 62c0ddf51c012..a22e817f818ce 100644
--- a/lldb/source/Symbol/CompilerType.cpp
+++ b/lldb/source/Symbol/CompilerType.cpp
@@ -373,30 +373,10 @@ bool CompilerType::IsScalarOrUnscopedEnumerationType() const {
}
bool CompilerType::IsPromotableIntegerType() const {
- // Unscoped enums are always considered as promotable, even if their
- // underlying type does not need to be promoted (e.g. "int").
- if (IsUnscopedEnumerationType())
- return true;
-
- switch (GetBasicTypeEnumeration()) {
- case lldb::eBasicTypeBool:
- case lldb::eBasicTypeChar:
- case lldb::eBasicTypeSignedChar:
- case lldb::eBasicTypeUnsignedChar:
- case lldb::eBasicTypeShort:
- case lldb::eBasicTypeUnsignedShort:
- case lldb::eBasicTypeWChar:
- case lldb::eBasicTypeSignedWChar:
- case lldb::eBasicTypeUnsignedWChar:
- case lldb::eBasicTypeChar16:
- case lldb::eBasicTypeChar32:
- return true;
-
- default:
- return false;
- }
-
- llvm_unreachable("All cases handled above.");
+ if (IsValid())
+ if (auto type_system_sp = GetTypeSystem())
+ return type_system_sp->IsPromotableIntegerType(m_type);
+ return false;
}
bool CompilerType::IsPointerToVoid() const {
diff --git a/lldb/source/Symbol/TypeSystem.cpp b/lldb/source/Symbol/TypeSystem.cpp
index f7d634ffa2dec..7df049084e6fb 100644
--- a/lldb/source/Symbol/TypeSystem.cpp
+++ b/lldb/source/Symbol/TypeSystem.cpp
@@ -123,6 +123,16 @@ CompilerType TypeSystem::GetTypeForFormatters(void *type) {
return CompilerType(weak_from_this(), type);
}
+bool TypeSystem::IsPromotableIntegerType(lldb::opaque_compiler_type_t type) {
+ return false;
+}
+
+llvm::Expected<CompilerType>
+TypeSystem::DoIntegralPromotion(CompilerType from,
+ ExecutionContextScope *exe_scope) {
+ return CompilerType();
+}
+
bool TypeSystem::IsTemplateType(lldb::opaque_compiler_type_t type) {
return false;
}
diff --git a/lldb/source/ValueObject/DILEval.cpp b/lldb/source/ValueObject/DILEval.cpp
index c6cf41ee9e9ee..c31b90ce83663 100644
--- a/lldb/source/ValueObject/DILEval.cpp
+++ b/lldb/source/ValueObject/DILEval.cpp
@@ -21,6 +21,104 @@
namespace lldb_private::dil {
+static llvm::Expected<lldb::TypeSystemSP>
+GetTypeSystemFromCU(std::shared_ptr<ExecutionContextScope> ctx) {
+ auto stack_frame = ctx->CalculateStackFrame();
+ if (!stack_frame)
+ return llvm::createStringError("no stack frame in this context");
+ SymbolContext symbol_context =
+ stack_frame->GetSymbolContext(lldb::eSymbolContextCompUnit);
+ lldb::LanguageType language = symbol_context.comp_unit->GetLanguage();
+
+ symbol_context = stack_frame->GetSymbolContext(lldb::eSymbolContextModule);
+ return symbol_context.module_sp->GetTypeSystemForLanguage(language);
+}
+
+static CompilerType GetBasicType(lldb::TypeSystemSP type_system,
+ lldb::BasicType basic_type) {
+ if (type_system)
+ return type_system.get()->GetBasicTypeFromAST(basic_type);
+
+ return CompilerType();
+}
+
+static lldb::ValueObjectSP
+ArrayToPointerConversion(lldb::ValueObjectSP valobj,
+ std::shared_ptr<ExecutionContextScope> ctx) {
+ assert(valobj->IsArrayType() &&
+ "an argument to array-to-pointer conversion must be an array");
+
+ uint64_t addr = valobj->GetLoadAddress();
+ ExecutionContext exe_ctx;
+ ctx->CalculateExecutionContext(exe_ctx);
+ return ValueObject::CreateValueObjectFromAddress(
+ "result", addr, exe_ctx,
+ valobj->GetCompilerType().GetArrayElementType(ctx.get()).GetPointerType(),
+ /* do_deref */ false);
+}
+
+llvm::Expected<lldb::ValueObjectSP>
+Interpreter::UnaryConversion(lldb::ValueObjectSP valobj) {
+ // Perform usual conversions for unary operators. At the moment this includes
+ // array-to-pointer and the integral promotion for eligible types.
+ llvm::Expected<lldb::TypeSystemSP> type_system =
+ GetTypeSystemFromCU(m_exe_ctx_scope);
+ if (!type_system)
+ return type_system.takeError();
+ CompilerType in_type = valobj->GetCompilerType();
+ CompilerType result_type;
+ if (valobj->IsBitfield()) {
+ // Promote bitfields. If `int` can represent the bitfield value, it is
+ // converted to `int`. Otherwise, if `unsigned int` can represent it, it
+ // is converted to `unsigned int`. Otherwise, it is treated as its
+ // underlying type.
+ uint32_t bitfield_size = valobj->GetBitfieldBitSize();
+ // Some bitfields have undefined size (e.g. result of ternary operation).
+ // The AST's `bitfield_size` of those is 0, and no promotion takes place.
+ if (bitfield_size > 0 && in_type.IsInteger()) {
+ CompilerType int_type = GetBasicType(*type_system, lldb::eBasicTypeInt);
+ CompilerType uint_type =
+ GetBasicType(*type_system, lldb::eBasicTypeUnsignedInt);
+ llvm::Expected<uint64_t> int_bit_size =
+ int_type.GetBitSize(m_exe_ctx_scope.get());
+ if (!int_bit_size)
+ return int_bit_size.takeError();
+ llvm::Expected<uint64_t> uint_bit_size =
+ uint_type.GetBitSize(m_exe_ctx_scope.get());
+ if (!uint_bit_size)
+ return int_bit_size.takeError();
+ if (bitfield_size < *int_bit_size ||
+ (in_type.IsSigned() && bitfield_size == *int_bit_size))
+ return valobj->CastToBasicType(int_type);
+ if (bitfield_size <= *uint_bit_size)
+ return valobj->CastToBasicType(uint_type);
+ // Re-create as a const value with the same underlying type
+ Scalar scalar;
+ bool resolved = valobj->ResolveValue(scalar);
+ if (!resolved)
+ return llvm::createStringError("invalid scalar value");
+ return ValueObject::CreateValueObjectFromScalar(m_target, scalar, in_type,
+ "result");
+ }
+ }
+
+ if (in_type.IsArrayType())
+ valobj = ArrayToPointerConversion(valobj, m_exe_ctx_scope);
+
+ if (valobj->GetCompilerType().IsInteger() ||
+ valobj->GetCompilerType().IsUnscopedEnumerationType()) {
+ llvm::Expected<CompilerType> promoted_type =
+ type_system.get()->DoIntegralPromotion(valobj->GetCompilerType(),
+ m_exe_ctx_scope.get());
+ if (!promoted_type)
+ return promoted_type.takeError();
+ if (!promoted_type->CompareTypes(valobj->GetCompilerType()))
+ return valobj->CastToBasicType(*promoted_type);
+ }
+
+ return valobj;
+}
+
static lldb::VariableSP DILFindVariable(ConstString name,
VariableList &variable_list) {
lldb::VariableSP exact_match;
@@ -175,21 +273,21 @@ Interpreter::Visit(const IdentifierNode *node) {
llvm::Expected<lldb::ValueObjectSP>
Interpreter::Visit(const UnaryOpNode *node) {
Status error;
- auto rhs_or_err = Evaluate(node->GetOperand());
- if (!rhs_or_err)
- return rhs_or_err;
+ auto op_or_err = Evaluate(node->GetOperand());
+ if (!op_or_err)
+ return op_or_err;
- lldb::ValueObjectSP rhs = *rhs_or_err;
+ lldb::ValueObjectSP operand = *op_or_err;
switch (node->GetKind()) {
case UnaryOpKind::Deref: {
- lldb::ValueObjectSP dynamic_rhs = rhs->GetDynamicValue(m_use_dynamic);
- if (dynamic_rhs)
- rhs = dynamic_rhs;
+ lldb::ValueObjectSP dynamic_op = operand->GetDynamicValue(m_use_dynamic);
+ if (dynamic_op)
+ operand = dynamic_op;
- lldb::ValueObjectSP child_sp = rhs->Dereference(error);
+ lldb::ValueObjectSP child_sp = operand->Dereference(error);
if (!child_sp && m_use_synthetic) {
- if (lldb::ValueObjectSP synth_obj_sp = rhs->GetSyntheticValue()) {
+ if (lldb::ValueObjectSP synth_obj_sp = operand->GetSyntheticValue()) {
error.Clear();
child_sp = synth_obj_sp->Dereference(error);
}
@@ -202,18 +300,57 @@ Interpreter::Visit(const UnaryOpNode *node) {
}
case UnaryOpKind::AddrOf: {
Status error;
- lldb::ValueObjectSP value = rhs->AddressOf(error);
+ lldb::ValueObjectSP value = operand->AddressOf(error);
if (error.Fail())
return llvm::make_error<DILDiagnosticError>(m_expr, error.AsCString(),
node->GetLocation());
return value;
}
+ case UnaryOpKind::Minus: {
+ llvm::Expected<lldb::ValueObjectSP> conv_op = UnaryConversion(operand);
+ if (!conv_op)
+ return conv_op;
+ operand = *conv_op;
+ CompilerType operand_type = operand->GetCompilerType();
+ if (!operand_type.IsScalarType()) {
+ std::string errMsg =
+ llvm::formatv("invalid argument type '{0}' to unary expression",
+ operand_type.GetTypeName());
+ return llvm::make_error<DILDiagnosticError>(m_expr, errMsg,
+ node->GetLocation());
+ }
+ Scalar scalar;
+ bool resolved = operand->ResolveValue(scalar);
+ if (!resolved)
+ break;
+
+ bool negated = scalar.UnaryNegate();
+ if (negated)
+ return ValueObject::CreateValueObjectFromScalar(
+ m_target, scalar, operand->GetCompilerType(), "result");
+ break;
}
-
- // Unsupported/invalid operation.
- return llvm::make_error<DILDiagnosticError>(
- m_expr, "invalid ast: unexpected binary operator", node->GetLocation());
+ case UnaryOpKind::Plus: {
+ llvm::Expected<lldb::ValueObjectSP> conv_op = UnaryConversion(operand);
+ if (!conv_op)
+ return conv_op;
+ operand = *conv_op;
+ CompilerType operand_type = operand->GetCompilerType();
+ if (!operand_type.IsScalarType() &&
+ // Unary plus is allowed for pointers.
+ !operand_type.IsPointerType()) {
+ std::string errMsg =
+ llvm::formatv("invalid argument type '{0}' to unary expression",
+ operand_type.GetTypeName());
+ return llvm::make_error<DILDiagnosticError>(m_expr, errMsg,
+ node->GetLocation());
+ }
+ return operand;
+ }
+ }
+ return llvm::make_error<DILDiagnosticError>(m_expr, "invalid unary operation",
+ node->GetLocation());
}
llvm::Expected<lldb::ValueObjectSP>
@@ -499,24 +636,6 @@ Interpreter::Visit(const BitFieldExtractionNode *node) {
return child_valobj_sp;
}
-static llvm::Expected<lldb::TypeSystemSP>
-GetTypeSystemFromCU(std::shared_ptr<StackFrame> ctx) {
- SymbolContext symbol_context =
- ctx->GetSymbolContext(lldb::eSymbolContextCompUnit);
- lldb::LanguageType language = symbol_context.comp_unit->GetLanguage();
-
- symbol_context = ctx->GetSymbolContext(lldb::eSymbolContextModule);
- return symbol_context.module_sp->GetTypeSystemForLanguage(language);
-}
-
-static CompilerType GetBasicType(lldb::TypeSystemSP type_system,
- lldb::BasicType basic_type) {
- if (type_system)
- return type_system.get()->GetBasicTypeFromAST(basic_type);
-
- return CompilerType();
-}
-
llvm::Expected<CompilerType>
Interpreter::PickIntegerType(lldb::TypeSystemSP type_system,
std::shared_ptr<ExecutionContextScope> ctx,
diff --git a/lldb/source/ValueObject/DILLexer.cpp b/lldb/source/ValueObject/DILLexer.cpp
index 0b2288a9d9230..77879f50cc9ad 100644
--- a/lldb/source/ValueObject/DILLexer.cpp
+++ b/lldb/source/ValueObject/DILLexer.cpp
@@ -42,7 +42,6 @@ llvm::StringRef Token::GetTokenName(Kind kind) {
return "minus";
case Kind::period:
return "period";
- return "l_square";
case Kind::plus:
return "plus";
case Kind::r_paren:
diff --git a/lldb/source/ValueObject/DILParser.cpp b/lldb/source/ValueObject/DILParser.cpp
index 8c4f7fdb25bea..b88345d18a6d4 100644
--- a/lldb/source/ValueObject/DILParser.cpp
+++ b/lldb/source/ValueObject/DILParser.cpp
@@ -95,7 +95,8 @@ ASTNodeUP DILParser::ParseExpression() { return ParseUnaryExpression(); }
// "*"
//
ASTNodeUP DILParser::ParseUnaryExpression() {
- if (CurToken().IsOneOf({Token::amp, Token::star})) {
+ if (CurToken().IsOneOf(
+ {Token::amp, Token::star, Token::minus, Token::plus})) {
Token token = CurToken();
uint32_t loc = token.GetLocation();
m_dil_lexer.Advance();
@@ -107,7 +108,12 @@ ASTNodeUP DILParser::ParseUnaryExpression() {
case Token::amp:
return std::make_unique<UnaryOpNode>(loc, UnaryOpKind::AddrOf,
std::move(rhs));
-
+ case Token::minus:
+ return std::make_unique<UnaryOpNode>(loc, UnaryOpKind::Minus,
+ std::move(rhs));
+ case Token::plus:
+ return std::make_unique<UnaryOpNode>(loc, UnaryOpKind::Plus,
+ std::move(rhs));
default:
llvm_unreachable("invalid token kind");
}
diff --git a/lldb/test/API/commands/frame/var-dil/expr/Arithmetic/Makefile b/lldb/test/API/commands/frame/var-dil/expr/Arithmetic/Makefile
new file mode 100644
index 0000000000000..99998b20bcb05
--- /dev/null
+++ b/lldb/test/API/commands/frame/var-dil/expr/Arithmetic/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
diff --git a/lldb/test/API/commands/frame/var-dil/expr/Arithmetic/TestFrameVarDILArithmetic.py b/lldb/test/API/commands/frame/var-dil/expr/Arithmetic/TestFrameVarDILArithmetic.py
new file mode 100644
index 0000000000000..76edc5837cf2b
--- /dev/null
+++ b/lldb/test/API/commands/frame/var-dil/expr/Arithmetic/TestFrameVarDILArithmetic.py
@@ -0,0 +1,51 @@
+"""
+Test DIL arithmetic.
+"""
+
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.decorators import *
+from lldbsuite.test import lldbutil
+
+
+class TestFrameVarDILArithmetic(TestBase):
+ NO_DEBUG_INFO_TESTCASE = True
+
+ def test_arithmetic(self):
+ self.build...
[truncated]
|
|
I'm not sure if we want to create a separate plugin for DIL-related code to be implemented for different languages. For now, I put the code for integral promotion into TypeSystem plugin, it feels like it belongs there anyway. Plus I was able to directly use info from |
|
Wanted to add: this patch is more about what to put away into a plugin and which plugin to choose. For now, I put only integral promotion into the Again, a more broad question: should we create a separate plugin for DIL-related code or find a spot for such code in existing related plugins, like integral promotion in |
@JDevlieghere , @jimingham , @adrian-prantl Do any of you have any thoughts or preferences about Ilya's questions re the TypeSystem plugin(s) and where's the right place to implement this or put this code? |
|
Do we ever want to support - in the DIL - something like: swift_integer = c_integer + rust_integer We do know how to do that (variations on the details of integer promotion and the like aside) and given these are data objects we can query for their integer values it seems somewhat artificial to disallow it. But in that case, whose promotion rules should we use? That's why I've been arguing that we should have a DIL set of promotion rules where integers are not sized. That way all these sort of computations can be done naturally. So I would argue that the DIL code should be its own plugin, not trying to either be C++ or something in the background, nor trying to figure out based on the expression what "Real" Typesystem we should dispatch to. When people want to answer the question "what would this expression do at this point in my code" they should be using the expression evaluator for that language. The DIL is there to support the widest range of data introspection tasks we can offer on generic structured data objects. |
I agree with that.
.. but I'm not sure what you meant by "its own plugin". Can you elaborate on that? If DIL is going to be language agnostic, then I don't think it needs to be a plugin. What I can imagine is having some sort of a (*) Technically, that also shouldn't be, as it's depended on by core functionality (the DIL), which would break if its plugged "out". However, it would be an implementation of the TypeSystem interface. |
We can do this, because the arithmetic itself is not typed. Math is implemented through
Basically, the promotion rules of DIL math are from
For the reason above, I don't think we have to sacrifice the correctness of result types to achieve the main goal of DIL. Every language will have to implement something in their type system anyway for DIL to work with it. If it doesn't matter to the language, it can just return a default type, like
I would really like DIL to be just compatible with the rest of LLDB as is. I don't see much of a problem with picking a type for the result. Other than primitive types, we have to have a type anyway for the result to be usable. |
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
lldb/source/ValueObject/DILEval.cpp
Outdated
| static lldb::ValueObjectSP | ||
| ArrayToPointerConversion(lldb::ValueObjectSP valobj, | ||
| std::shared_ptr<ExecutionContextScope> ctx) { | ||
| assert(valobj->IsArrayType() && |
There was a problem hiding this comment.
I don't think we should assert on errors in DIL.
There was a problem hiding this comment.
Yes, the correct thing to do would be to return a ValueObject wrapping an Error, although it might be even nicer to return an Expected<ValueObjectSP>.
|
Have you thought about adding any of the Bitwise tests from lldb-eval? Some of them might be appropriate for this PR... |
|
Ping? Could somebody else please review this PR? (As one of the main DIL authors, I don't feel right approving it myself...) |
But this PR has only |
Oh, you're right. I was glancing at the tests and mistook the '~' for '-'. I apologize for the confusion. |
There was a problem hiding this comment.
@kastiglione Do you think the unary +/- could create any parsing ambiguity with ObjC method names?
For example, does image lookup -a take a DIL expression and someone could write image lookup -a +[MyClass myselector]?
| int (&array_ref)[10] = array; | ||
| int *p_int0 = &array[0]; | ||
|
|
||
| return 0; // Set a breakpoint here |
There was a problem hiding this comment.
Generally, it is more reliable to set a breakpoint on a function call, since the return statement itself doesn't always produce an instruction with a line table entry that is outside the epilogue. So it's better to either write something like
void stop() {}
...
stop(); // break here
return 0;
There was a problem hiding this comment.
I haven't been doing this for other tests, should I make a separate commit changing this in all DIL tests?
|
|
||
| // DIL | ||
|
|
||
| virtual bool IsPromotableIntegerType(lldb::opaque_compiler_type_t type); |
There was a problem hiding this comment.
Can you add a Doxygen comment explaining the semantics of this function?
| virtual bool IsPromotableIntegerType(lldb::opaque_compiler_type_t type); | ||
|
|
||
| virtual llvm::Expected<CompilerType> | ||
| DoIntegralPromotion(CompilerType from, ExecutionContextScope *exe_scope); |
There was a problem hiding this comment.
I'm not really sure what to write here, since the semantics is dependent on the type system. I wrote a general idea for C++, but I suspect that for Swift this might do opposite things, like not promoting integers at all and returning an error for Bool.
The |
6 participants
This patch adds unary nodes plus and minus, and introduces unary type conversions and integral promotion.