[lldb][TypeSystemClang] Add support for floating point template argument constants #127206
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
|
@llvm/pr-subscribers-lldb Author: Michael Buch (Michael137) ChangesThis patch adds support for the recently added Anywhere LLDB assumed a We can rely on the fact that any Full diff: https://github.com/llvm/llvm-project/pull/127206.diff 9 Files Affected:
diff --git a/lldb/include/lldb/Symbol/CompilerType.h b/lldb/include/lldb/Symbol/CompilerType.h
index 096a8f1ab68e8..f7e3e552f3e45 100644
--- a/lldb/include/lldb/Symbol/CompilerType.h
+++ b/lldb/include/lldb/Symbol/CompilerType.h
@@ -15,6 +15,7 @@
#include <vector>
#include "lldb/lldb-private.h"
+#include "clang/AST/APValue.h"
#include "llvm/ADT/APSInt.h"
#include "llvm/Support/Casting.h"
@@ -544,7 +545,7 @@ bool operator==(const CompilerType &lhs, const CompilerType &rhs);
bool operator!=(const CompilerType &lhs, const CompilerType &rhs);
struct CompilerType::IntegralTemplateArgument {
- llvm::APSInt value;
+ clang::APValue value;
CompilerType type;
};
diff --git a/lldb/source/API/SBType.cpp b/lldb/source/API/SBType.cpp
index 6401d32c85795..72f590947dff6 100644
--- a/lldb/source/API/SBType.cpp
+++ b/lldb/source/API/SBType.cpp
@@ -697,6 +697,7 @@ lldb::SBValue SBType::GetTemplateArgumentValue(lldb::SBTarget target,
std::optional<CompilerType::IntegralTemplateArgument> arg;
const bool expand_pack = true;
switch (GetTemplateArgumentKind(idx)) {
+ case eTemplateArgumentKindStructuralValue:
case eTemplateArgumentKindIntegral:
arg = m_opaque_sp->GetCompilerType(false).GetIntegralTemplateArgument(
idx, expand_pack);
@@ -708,7 +709,12 @@ lldb::SBValue SBType::GetTemplateArgumentValue(lldb::SBTarget target,
if (!arg)
return {};
- Scalar value{arg->value};
+ Scalar value;
+ if (arg->value.isFloat())
+ value = arg->value.getFloat();
+ else
+ value = arg->value.getInt();
+
DataExtractor data;
value.GetData(data);
diff --git a/lldb/source/Plugins/Language/CPlusPlus/GenericBitset.cpp b/lldb/source/Plugins/Language/CPlusPlus/GenericBitset.cpp
index 33955dccb6ccc..99ff975825c71 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/GenericBitset.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/GenericBitset.cpp
@@ -91,7 +91,7 @@ lldb::ChildCacheState GenericBitsetFrontEnd::Update() {
size_t size = 0;
if (auto arg = m_backend.GetCompilerType().GetIntegralTemplateArgument(0))
- size = arg->value.getLimitedValue();
+ size = arg->value.getInt().getLimitedValue();
m_elements.assign(size, ValueObjectSP());
m_first =
diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxxSpan.cpp b/lldb/source/Plugins/Language/CPlusPlus/LibCxxSpan.cpp
index 15040295efe6d..687ef1739ad11 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxxSpan.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxxSpan.cpp
@@ -119,7 +119,7 @@ lldb_private::formatters::LibcxxStdSpanSyntheticFrontEnd::Update() {
} else if (auto arg =
m_backend.GetCompilerType().GetIntegralTemplateArgument(1)) {
- m_num_elements = arg->value.getLimitedValue();
+ m_num_elements = arg->value.getInt().getLimitedValue();
}
}
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index ec0004c70c6da..70af283ab7443 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -1973,6 +1973,27 @@ class DWARFASTParserClang::DelayedAddObjCClassProperty {
ClangASTMetadata m_metadata;
};
+static clang::APValue MakeAPValue(CompilerType clang_type, uint64_t bit_width,
+ uint64_t value) {
+ bool is_signed = false;
+ const bool is_integral = clang_type.IsIntegerOrEnumerationType(is_signed);
+
+ llvm::APSInt apint(bit_width, !is_signed);
+ apint = value;
+
+ if (is_integral)
+ return clang::APValue(apint);
+
+ uint32_t count;
+ bool is_complex;
+ assert(clang_type.IsFloatingPointType(count, is_complex));
+
+ if (bit_width == 32)
+ return clang::APValue(llvm::APFloat(apint.bitsToFloat()));
+
+ return clang::APValue(llvm::APFloat(apint.bitsToDouble()));
+}
+
bool DWARFASTParserClang::ParseTemplateDIE(
const DWARFDIE &die,
TypeSystemClang::TemplateParameterInfos &template_param_infos) {
@@ -2050,9 +2071,6 @@ bool DWARFASTParserClang::ParseTemplateDIE(
clang_type = m_ast.GetBasicType(eBasicTypeVoid);
if (!is_template_template_argument) {
- bool is_signed = false;
- // Get the signed value for any integer or enumeration if available
- clang_type.IsIntegerOrEnumerationType(is_signed);
if (name && !name[0])
name = nullptr;
@@ -2061,11 +2079,12 @@ bool DWARFASTParserClang::ParseTemplateDIE(
std::optional<uint64_t> size = clang_type.GetBitSize(nullptr);
if (!size)
return false;
- llvm::APInt apint(*size, uval64, is_signed);
+
template_param_infos.InsertArg(
- name, clang::TemplateArgument(ast, llvm::APSInt(apint, !is_signed),
- ClangUtil::GetQualType(clang_type),
- is_default_template_arg));
+ name,
+ clang::TemplateArgument(ast, ClangUtil::GetQualType(clang_type),
+ MakeAPValue(clang_type, *size, uval64),
+ is_default_template_arg));
} else {
template_param_infos.InsertArg(
name, clang::TemplateArgument(ClangUtil::GetQualType(clang_type),
diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
index bcb63f719de10..567819fc2572c 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -1311,10 +1311,18 @@ CompilerType TypeSystemClang::CreateRecordType(
}
namespace {
-/// Returns true iff the given TemplateArgument should be represented as an
-/// NonTypeTemplateParmDecl in the AST.
-bool IsValueParam(const clang::TemplateArgument &argument) {
- return argument.getKind() == TemplateArgument::Integral;
+/// Returns the type of the template argument iff the given TemplateArgument
+/// should be represented as an NonTypeTemplateParmDecl in the AST. Returns
+/// a null QualType otherwise.
+QualType GetValueParamType(const clang::TemplateArgument &argument) {
+ switch (argument.getKind()) {
+ case TemplateArgument::Integral:
+ return argument.getIntegralType();
+ case TemplateArgument::StructuralValue:
+ return argument.getStructuralValueType();
+ default:
+ return {};
+ }
}
void AddAccessSpecifierDecl(clang::CXXRecordDecl *cxx_record_decl,
@@ -1361,8 +1369,8 @@ static TemplateParameterList *CreateTemplateParameterList(
if (name && name[0])
identifier_info = &ast.Idents.get(name);
TemplateArgument const &targ = args[i];
- if (IsValueParam(targ)) {
- QualType template_param_type = targ.getIntegralType();
+ QualType template_param_type = GetValueParamType(targ);
+ if (!template_param_type.isNull()) {
template_param_decls.push_back(NonTypeTemplateParmDecl::Create(
ast, decl_context, SourceLocation(), SourceLocation(), depth, i,
identifier_info, template_param_type, parameter_pack,
@@ -1380,10 +1388,11 @@ static TemplateParameterList *CreateTemplateParameterList(
identifier_info = &ast.Idents.get(template_param_infos.GetPackName());
const bool parameter_pack_true = true;
- if (!template_param_infos.GetParameterPack().IsEmpty() &&
- IsValueParam(template_param_infos.GetParameterPack().Front())) {
- QualType template_param_type =
- template_param_infos.GetParameterPack().Front().getIntegralType();
+ QualType template_param_type =
+ !template_param_infos.GetParameterPack().IsEmpty()
+ ? GetValueParamType(template_param_infos.GetParameterPack().Front())
+ : QualType();
+ if (!template_param_type.isNull()) {
template_param_decls.push_back(NonTypeTemplateParmDecl::Create(
ast, decl_context, SourceLocation(), SourceLocation(), depth,
num_template_params, identifier_info, template_param_type,
@@ -1458,10 +1467,9 @@ static bool TemplateParameterAllowsValue(NamedDecl *param,
} else if (auto *type_param =
llvm::dyn_cast<NonTypeTemplateParmDecl>(param)) {
// Compare the argument kind, i.e. ensure that <typename> != <int>.
- if (!IsValueParam(value))
- return false;
+ QualType value_param_type = GetValueParamType(value);
// Compare the integral type, i.e. ensure that <int> != <char>.
- if (type_param->getType() != value.getIntegralType())
+ if (type_param->getType() != value_param_type)
return false;
} else {
// There is no way to create other parameter decls at the moment, so we
@@ -7351,10 +7359,19 @@ TypeSystemClang::GetIntegralTemplateArgument(lldb::opaque_compiler_type_t type,
return std::nullopt;
const auto *arg = GetNthTemplateArgument(template_decl, idx, expand_pack);
- if (!arg || arg->getKind() != clang::TemplateArgument::Integral)
+ if (!arg)
return std::nullopt;
- return {{arg->getAsIntegral(), GetType(arg->getIntegralType())}};
+ switch (arg->getKind()) {
+ case clang::TemplateArgument::Integral:
+ return {{clang::APValue(arg->getAsIntegral()),
+ GetType(arg->getIntegralType())}};
+ case clang::TemplateArgument::StructuralValue:
+ return {
+ {arg->getAsStructuralValue(), GetType(arg->getStructuralValueType())}};
+ default:
+ return std::nullopt;
+ }
}
CompilerType TypeSystemClang::GetTypeForFormatters(void *type) {
diff --git a/lldb/test/API/lang/cpp/template-arguments/TestCppTemplateArguments.py b/lldb/test/API/lang/cpp/template-arguments/TestCppTemplateArguments.py
index db5388b8bcc6d..fbfc7d1ca9868 100644
--- a/lldb/test/API/lang/cpp/template-arguments/TestCppTemplateArguments.py
+++ b/lldb/test/API/lang/cpp/template-arguments/TestCppTemplateArguments.py
@@ -62,10 +62,35 @@ def test(self):
self.assertEqual(template_param_value.GetTypeName(), "char")
self.assertEqual(chr(template_param_value.GetValueAsSigned()), "v")
- # FIXME: type should be Foo<float, 2.0f>
- # FIXME: double/float NTTP parameter values currently not supported.
- value = self.expect_expr("temp4", result_type="Foo<float, 1073741824>")
+ value = self.expect_expr("temp4", result_type="Foo<float, 2.000000e+00>")
template_param_value = value.GetType().GetTemplateArgumentValue(target, 1)
self.assertEqual(template_param_value.GetTypeName(), "float")
# FIXME: this should return a float
self.assertEqual(template_param_value.GetValueAsSigned(), 2)
+
+ value = self.expect_expr("temp5", result_type="Foo<double, -2.505000e+02>")
+ template_param_value = value.GetType().GetTemplateArgumentValue(target, 1)
+ self.assertEqual(template_param_value.GetTypeName(), "double")
+ # FIXME: this should return a float
+ self.assertEqual(template_param_value.GetValueAsSigned(), -250)
+
+ # FIXME: type should be Foo<int *, &temp1.member>
+ value = self.expect_expr("temp6", result_type="Foo<int *, int *>")
+ self.assertFalse(value.GetType().GetTemplateArgumentValue(target, 1))
+
+ value = self.expect_expr("temp7", result_type="Bar<double, 1.200000e+00>")
+ template_param_value = value.GetType().GetTemplateArgumentValue(target, 1)
+ self.assertEqual(template_param_value.GetTypeName(), "double")
+ # FIXME: this should return a float
+ self.assertEqual(template_param_value.GetValueAsSigned(), 1)
+
+ value = self.expect_expr("temp8", result_type="Bar<float, 1.000000e+00, 2.000000e+00>")
+ template_param_value = value.GetType().GetTemplateArgumentValue(target, 1)
+ self.assertEqual(template_param_value.GetTypeName(), "float")
+ # FIXME: this should return a float
+ self.assertEqual(template_param_value.GetValueAsSigned(), 1)
+
+ template_param_value = value.GetType().GetTemplateArgumentValue(target, 2)
+ self.assertEqual(template_param_value.GetTypeName(), "float")
+ # FIXME: this should return a float
+ self.assertEqual(template_param_value.GetValueAsSigned(), 2)
diff --git a/lldb/test/API/lang/cpp/template-arguments/main.cpp b/lldb/test/API/lang/cpp/template-arguments/main.cpp
index 0c0eb97cbc858..e1add12170b54 100644
--- a/lldb/test/API/lang/cpp/template-arguments/main.cpp
+++ b/lldb/test/API/lang/cpp/template-arguments/main.cpp
@@ -9,5 +9,11 @@ template <typename T, T value> struct Foo {};
Foo<short, -2> temp2;
Foo<char, 'v'> temp3;
Foo<float, 2.0f> temp4;
+Foo<double, -250.5> temp5;
+Foo<int *, &temp1.member> temp6;
+
+template <typename T, T... values> struct Bar {};
+Bar<double, 1.2> temp7;
+Bar<float, 1.0f, 2.0f> temp8;
int main() {}
diff --git a/lldb/unittests/Symbol/TestTypeSystemClang.cpp b/lldb/unittests/Symbol/TestTypeSystemClang.cpp
index 23374062127e0..446d1976481db 100644
--- a/lldb/unittests/Symbol/TestTypeSystemClang.cpp
+++ b/lldb/unittests/Symbol/TestTypeSystemClang.cpp
@@ -525,7 +525,17 @@ TEST_F(TestTypeSystemClang, TemplateArguments) {
infos.InsertArg("I", TemplateArgument(m_ast->getASTContext(), arg,
m_ast->getASTContext().IntTy));
- // template<typename T, int I> struct foo;
+ llvm::APFloat float_arg(5.5f);
+ infos.InsertArg("F", TemplateArgument(m_ast->getASTContext(),
+ m_ast->getASTContext().FloatTy,
+ clang::APValue(float_arg)));
+
+ llvm::APFloat double_arg(-15.2);
+ infos.InsertArg("D", TemplateArgument(m_ast->getASTContext(),
+ m_ast->getASTContext().DoubleTy,
+ clang::APValue(double_arg)));
+
+ // template<typename T, int I, float F, double D> struct foo;
ClassTemplateDecl *decl = m_ast->CreateClassTemplateDecl(
m_ast->GetTranslationUnitDecl(), OptionalClangModuleID(), eAccessPublic,
"foo", llvm::to_underlying(clang::TagTypeKind::Struct), infos);
@@ -555,6 +565,10 @@ TEST_F(TestTypeSystemClang, TemplateArguments) {
CompilerType int_type(m_ast->weak_from_this(),
m_ast->getASTContext().IntTy.getAsOpaquePtr());
+ CompilerType float_type(m_ast->weak_from_this(),
+ m_ast->getASTContext().FloatTy.getAsOpaquePtr());
+ CompilerType double_type(m_ast->weak_from_this(),
+ m_ast->getASTContext().DoubleTy.getAsOpaquePtr());
for (CompilerType t : {type, typedef_type, auto_type}) {
SCOPED_TRACE(t.GetTypeName().AsCString());
@@ -577,8 +591,32 @@ TEST_F(TestTypeSystemClang, TemplateArguments) {
auto result = m_ast->GetIntegralTemplateArgument(t.GetOpaqueQualType(), 1,
expand_pack);
ASSERT_NE(std::nullopt, result);
- EXPECT_EQ(arg, result->value);
+ EXPECT_EQ(arg, result->value.getInt());
EXPECT_EQ(int_type, result->type);
+
+ EXPECT_EQ(
+ m_ast->GetTemplateArgumentKind(t.GetOpaqueQualType(), 2, expand_pack),
+ eTemplateArgumentKindStructuralValue);
+ EXPECT_EQ(
+ m_ast->GetTypeTemplateArgument(t.GetOpaqueQualType(), 2, expand_pack),
+ CompilerType());
+ auto float_result = m_ast->GetIntegralTemplateArgument(
+ t.GetOpaqueQualType(), 2, expand_pack);
+ ASSERT_NE(std::nullopt, float_result);
+ EXPECT_EQ(float_arg, float_result->value.getFloat());
+ EXPECT_EQ(float_type, float_result->type);
+
+ EXPECT_EQ(
+ m_ast->GetTemplateArgumentKind(t.GetOpaqueQualType(), 3, expand_pack),
+ eTemplateArgumentKindStructuralValue);
+ EXPECT_EQ(
+ m_ast->GetTypeTemplateArgument(t.GetOpaqueQualType(), 3, expand_pack),
+ CompilerType());
+ auto double_result = m_ast->GetIntegralTemplateArgument(
+ t.GetOpaqueQualType(), 3, expand_pack);
+ ASSERT_NE(std::nullopt, double_result);
+ EXPECT_EQ(double_arg, double_result->value.getFloat());
+ EXPECT_EQ(double_type, double_result->type);
}
}
|
|
✅ With the latest revision this PR passed the Python code formatter. |
labath
reviewed
Feb 14, 2025
Comment on lines
1991
to
1994
| if (bit_width == 32) | ||
| return clang::APValue(llvm::APFloat(apint.bitsToFloat())); | ||
|
|
||
| return clang::APValue(llvm::APFloat(apint.bitsToDouble())); |
There was a problem hiding this comment.
Any chance this could be something like return clang::APValue(llvm::APFloat(fltSemantics_I_somehow_got_from_clang_type, apint))
Reason being floats and doubles aren't the only floating point type, so this will likely not work on any of the fancier float types.
There was a problem hiding this comment.
Hmmm good question
The only differentiator between the different float types we have in DWARF is the DW_AT_name of the type and the DW_AT_byte_size. We could set the semantics based on the floating point type, but we probably won't be able to support all the different available fltSemantics. We already create Clang types from the DW_AT_name, so we could, e.g., differentiate between _Float16 and __bf16 semantics by looking at the typename. Then 32-bits and 64-bits we would assume IEEE. And if none of those work then we could return a std::nullopt here? Wdyt?
There was a problem hiding this comment.
I don't think we need to solve the problem of parsing all floating point types here. What I'm mainly interested in is seeing if there's a way to write this code such that it is automatically correct for any floating point type that we do support. I'm assuming/hoping that there is a way to get floating point semantics object out of a clang type without relying on the bitwidth...
There was a problem hiding this comment.
Ahh I misunderstood the suggestion. Yes, getting the semantics from the QualType makes sense here. Done in the latest commit
| @@ -91,7 +91,7 @@ lldb::ChildCacheState GenericBitsetFrontEnd::Update() { | |||
| size_t size = 0; | |||
|
|
|||
| if (auto arg = m_backend.GetCompilerType().GetIntegralTemplateArgument(0)) | |||
| size = arg->value.getLimitedValue(); | |||
| size = arg->value.getInt().getLimitedValue(); | |||
There was a problem hiding this comment.
Technically I think these need to check that the value is an integral type as a malicious binary could crash lldb by declaring a std::bitset with a floating point template argument.
…dd defensive check in GenericBitset
labath
reviewed
Feb 17, 2025
| @@ -544,7 +545,7 @@ bool operator==(const CompilerType &lhs, const CompilerType &rhs); | |||
| bool operator!=(const CompilerType &lhs, const CompilerType &rhs); | |||
|
|
|||
| struct CompilerType::IntegralTemplateArgument { | |||
| llvm::APSInt value; | |||
| clang::APValue value; | |||
There was a problem hiding this comment.
I just realized this is adding a clang dependency on lldb core (non-plugin) code. What would you say to making this a Scalar instead?
There was a problem hiding this comment.
Ah good point. Changed to Scalar, wasn't a difficult transformation to make
labath
reviewed
Feb 17, 2025
Comment on lines
2082
to
2084
| std::optional<uint64_t> size = clang_type.GetBitSize(nullptr); | ||
| if (!size) | ||
| return false; |
There was a problem hiding this comment.
It looks like this exists only to be passed to MakeAPValue. Since it already gets clang_type as an argument (and uses it to retrieve things like bit width and float semantics), could we move this there as well?
labath
reviewed
Feb 17, 2025
Michael137
commented
Feb 17, 2025
| @@ -119,7 +119,7 @@ lldb_private::formatters::LibcxxStdSpanSyntheticFrontEnd::Update() { | |||
| } else if (auto arg = | |||
| m_backend.GetCompilerType().GetIntegralTemplateArgument(1)) { | |||
|
|
|||
| m_num_elements = arg->value.getLimitedValue(); | |||
| m_num_elements = arg->value.GetAPSInt().getLimitedValue(); | |||
There was a problem hiding this comment.
Since Scalar default initializes both float and integer members, i removed the isInt call from prior revisions here
Michael137
deleted the
lldb/floating-point-template-argument-constants
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This patch adds support for template arguments of
clang::TemplateArgument::ArgKind::StructuralValuekind (added in #78041). These are used for non-type template parameters such as floating point constants. When LLDB createdclang::NonTypeTemplateParmDecls, it previously assumed integral values, this patch accounts for structural values too.Anywhere LLDB assumed a
DW_TAG_template_value_parameterwasIntegral, it will now also check forStructuralValue, and will unpack theTemplateArgumentvalue and type accordingly.We can rely on the fact that any
TemplateArgumentofStructuralValuekind that theDWARFASTParserClangcreates will have a valid value, because it gets those fromDW_AT_const_value.