[lldb][DataFormatters] Change ExtractIndexFromString to return std::optional #138297
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: Charles Zablit (charles-zablit) ChangesThis PR is in continuation of #136693. It upgrades Patch is 28.14 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/138297.diff 21 Files Affected:
diff --git a/lldb/include/lldb/DataFormatters/FormattersHelpers.h b/lldb/include/lldb/DataFormatters/FormattersHelpers.h
index a98042fd40f93..82aae705e2a59 100644
--- a/lldb/include/lldb/DataFormatters/FormattersHelpers.h
+++ b/lldb/include/lldb/DataFormatters/FormattersHelpers.h
@@ -53,7 +53,7 @@ void AddFilter(TypeCategoryImpl::SharedPointer category_sp,
llvm::StringRef type_name,
ScriptedSyntheticChildren::Flags flags, bool regex = false);
-size_t ExtractIndexFromString(const char *item_name);
+llvm::Expected<size_t> ExtractIndexFromString(const char *item_name);
Address GetArrayAddressOrPointerValue(ValueObject &valobj);
diff --git a/lldb/source/DataFormatters/FormattersHelpers.cpp b/lldb/source/DataFormatters/FormattersHelpers.cpp
index 085ed3d0a2f29..5e29794c291be 100644
--- a/lldb/source/DataFormatters/FormattersHelpers.cpp
+++ b/lldb/source/DataFormatters/FormattersHelpers.cpp
@@ -97,18 +97,19 @@ void lldb_private::formatters::AddFilter(
category_sp->AddTypeFilter(type_name, match_type, filter_sp);
}
-size_t lldb_private::formatters::ExtractIndexFromString(const char *item_name) {
+llvm::Expected<size_t>
+lldb_private::formatters::ExtractIndexFromString(const char *item_name) {
if (!item_name || !*item_name)
- return UINT32_MAX;
+ return llvm::createStringError("String has no item named '%s'", item_name);
if (*item_name != '[')
- return UINT32_MAX;
+ return llvm::createStringError("String has no item named '%s'", item_name);
item_name++;
char *endptr = nullptr;
unsigned long int idx = ::strtoul(item_name, &endptr, 0);
if (idx == 0 && endptr == item_name)
- return UINT32_MAX;
+ return llvm::createStringError("String has no item named '%s'", item_name);
if (idx == ULONG_MAX)
- return UINT32_MAX;
+ return llvm::createStringError("String has no item named '%s'", item_name);
return idx;
}
diff --git a/lldb/source/DataFormatters/VectorType.cpp b/lldb/source/DataFormatters/VectorType.cpp
index eab2612d1e941..3fa616b9d9b14 100644
--- a/lldb/source/DataFormatters/VectorType.cpp
+++ b/lldb/source/DataFormatters/VectorType.cpp
@@ -270,10 +270,14 @@ class VectorTypeSyntheticFrontEnd : public SyntheticChildrenFrontEnd {
}
llvm::Expected<size_t> GetIndexOfChildWithName(ConstString name) override {
- const char *item_name = name.GetCString();
- uint32_t idx = ExtractIndexFromString(item_name);
- if (idx == UINT32_MAX ||
- (idx < UINT32_MAX && idx >= CalculateNumChildrenIgnoringErrors()))
+ auto idx_or_err = ExtractIndexFromString(name.AsCString());
+ if (!idx_or_err) {
+ llvm::consumeError(idx_or_err.takeError());
+ return llvm::createStringError("Type has no child named '%s'",
+ name.AsCString());
+ }
+ uint32_t idx = *idx_or_err;
+ if (idx >= CalculateNumChildrenIgnoringErrors())
return llvm::createStringError("Type has no child named '%s'",
name.AsCString());
return idx;
diff --git a/lldb/source/Plugins/Language/CPlusPlus/GenericBitset.cpp b/lldb/source/Plugins/Language/CPlusPlus/GenericBitset.cpp
index 234471d5ba518..ebcac0a48bd30 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/GenericBitset.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/GenericBitset.cpp
@@ -29,11 +29,13 @@ class GenericBitsetFrontEnd : public SyntheticChildrenFrontEnd {
GenericBitsetFrontEnd(ValueObject &valobj, StdLib stdlib);
llvm::Expected<size_t> GetIndexOfChildWithName(ConstString name) override {
- size_t idx = formatters::ExtractIndexFromString(name.GetCString());
- if (idx == UINT32_MAX)
+ auto idx_or_err = formatters::ExtractIndexFromString(name.GetCString());
+ if (!idx_or_err) {
+ llvm::consumeError(idx_or_err.takeError());
return llvm::createStringError("Type has no child named '%s'",
name.AsCString());
- return idx;
+ }
+ return *idx_or_err;
}
lldb::ChildCacheState Update() override;
diff --git a/lldb/source/Plugins/Language/CPlusPlus/GenericOptional.cpp b/lldb/source/Plugins/Language/CPlusPlus/GenericOptional.cpp
index 6f1b2ee3fd9e3..bfcc15ae805e7 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/GenericOptional.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/GenericOptional.cpp
@@ -39,11 +39,13 @@ class GenericOptionalFrontend : public SyntheticChildrenFrontEnd {
llvm::Expected<size_t> GetIndexOfChildWithName(ConstString name) override {
if (name == "$$dereference$$")
return 0;
- size_t idx = formatters::ExtractIndexFromString(name.GetCString());
- if (idx == UINT32_MAX)
+ auto idx_or_err = formatters::ExtractIndexFromString(name.GetCString());
+ if (!idx_or_err) {
+ llvm::consumeError(idx_or_err.takeError());
return llvm::createStringError("Type has no child named '%s'",
name.AsCString());
- return idx;
+ }
+ return *idx_or_err;
}
llvm::Expected<uint32_t> CalculateNumChildren() override {
diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxxInitializerList.cpp b/lldb/source/Plugins/Language/CPlusPlus/LibCxxInitializerList.cpp
index e8a886dd71821..04891d7d094f3 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxxInitializerList.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxxInitializerList.cpp
@@ -108,12 +108,13 @@ lldb_private::formatters::LibcxxInitializerListSyntheticFrontEnd::
return llvm::createStringError("Type has no child named '%s'",
name.AsCString());
}
- size_t idx = ExtractIndexFromString(name.GetCString());
- if (idx == UINT32_MAX) {
+ auto idx_or_err = formatters::ExtractIndexFromString(name.GetCString());
+ if (!idx_or_err) {
+ llvm::consumeError(idx_or_err.takeError());
return llvm::createStringError("Type has no child named '%s'",
name.AsCString());
}
- return idx;
+ return *idx_or_err;
}
lldb_private::SyntheticChildrenFrontEnd *
diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxxList.cpp b/lldb/source/Plugins/Language/CPlusPlus/LibCxxList.cpp
index ea18c02ee6591..9d4773c56b3e3 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxxList.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxxList.cpp
@@ -107,11 +107,13 @@ class ListIterator {
class AbstractListFrontEnd : public SyntheticChildrenFrontEnd {
public:
llvm::Expected<size_t> GetIndexOfChildWithName(ConstString name) override {
- size_t idx = ExtractIndexFromString(name.GetCString());
- if (idx == UINT32_MAX)
+ auto idx_or_err = formatters::ExtractIndexFromString(name.GetCString());
+ if (!idx_or_err) {
+ llvm::consumeError(idx_or_err.takeError());
return llvm::createStringError("Type has no child named '%s'",
name.AsCString());
- return idx;
+ }
+ return *idx_or_err;
}
lldb::ChildCacheState Update() override;
diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp b/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
index fec2d6f29ca54..9c3694e2448fc 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
@@ -395,12 +395,13 @@ lldb_private::formatters::LibcxxStdMapSyntheticFrontEnd::Update() {
llvm::Expected<size_t> lldb_private::formatters::LibcxxStdMapSyntheticFrontEnd::
GetIndexOfChildWithName(ConstString name) {
- size_t idx = ExtractIndexFromString(name.GetCString());
- if (idx == UINT32_MAX) {
+ auto idx_or_err = formatters::ExtractIndexFromString(name.GetCString());
+ if (!idx_or_err) {
+ llvm::consumeError(idx_or_err.takeError());
return llvm::createStringError("Type has no child named '%s'",
name.AsCString());
}
- return idx;
+ return *idx_or_err;
}
SyntheticChildrenFrontEnd *
diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxxProxyArray.cpp b/lldb/source/Plugins/Language/CPlusPlus/LibCxxProxyArray.cpp
index 41fc704d5886f..a4ded6e0d7d85 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxxProxyArray.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxxProxyArray.cpp
@@ -179,12 +179,13 @@ lldb_private::formatters::LibcxxStdProxyArraySyntheticFrontEnd::
if (!m_base)
return llvm::createStringError("Type has no child named '%s'",
name.AsCString());
- size_t idx = ExtractIndexFromString(name.GetCString());
- if (idx == UINT32_MAX) {
+ auto idx_or_err = formatters::ExtractIndexFromString(name.GetCString());
+ if (!idx_or_err) {
+ llvm::consumeError(idx_or_err.takeError());
return llvm::createStringError("Type has no child named '%s'",
name.AsCString());
}
- return idx;
+ return *idx_or_err;
}
lldb_private::SyntheticChildrenFrontEnd *
diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxxSliceArray.cpp b/lldb/source/Plugins/Language/CPlusPlus/LibCxxSliceArray.cpp
index 1c4d8509374f1..309d91ae02ecb 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxxSliceArray.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxxSliceArray.cpp
@@ -150,11 +150,13 @@ lldb_private::formatters::LibcxxStdSliceArraySyntheticFrontEnd::
if (!m_start)
return llvm::createStringError("Type has no child named '%s'",
name.AsCString());
- size_t idx = ExtractIndexFromString(name.GetCString());
- if (idx == UINT32_MAX)
+ auto idx_or_err = formatters::ExtractIndexFromString(name.GetCString());
+ if (!idx_or_err) {
+ llvm::consumeError(idx_or_err.takeError());
return llvm::createStringError("Type has no child named '%s'",
name.AsCString());
- return idx;
+ }
+ return *idx_or_err;
}
lldb_private::SyntheticChildrenFrontEnd *
diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxxSpan.cpp b/lldb/source/Plugins/Language/CPlusPlus/LibCxxSpan.cpp
index 43721e4b41fb8..26e8c48b4f8eb 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxxSpan.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxxSpan.cpp
@@ -133,12 +133,13 @@ llvm::Expected<size_t> lldb_private::formatters::
if (!m_start)
return llvm::createStringError("Type has no child named '%s'",
name.AsCString());
- size_t idx = ExtractIndexFromString(name.GetCString());
- if (idx == UINT32_MAX) {
+ auto idx_or_err = formatters::ExtractIndexFromString(name.GetCString());
+ if (!idx_or_err) {
+ llvm::consumeError(idx_or_err.takeError());
return llvm::createStringError("Type has no child named '%s'",
name.AsCString());
}
- return idx;
+ return *idx_or_err;
}
lldb_private::SyntheticChildrenFrontEnd *
diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxxTuple.cpp b/lldb/source/Plugins/Language/CPlusPlus/LibCxxTuple.cpp
index 270aad5ea9310..235a26da50a5f 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxxTuple.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxxTuple.cpp
@@ -21,11 +21,13 @@ class TupleFrontEnd: public SyntheticChildrenFrontEnd {
}
llvm::Expected<size_t> GetIndexOfChildWithName(ConstString name) override {
- size_t idx = formatters::ExtractIndexFromString(name.GetCString());
- if (idx == UINT32_MAX)
+ auto idx_or_err = formatters::ExtractIndexFromString(name.GetCString());
+ if (!idx_or_err) {
+ llvm::consumeError(idx_or_err.takeError());
return llvm::createStringError("Type has no child named '%s'",
name.AsCString());
- return idx;
+ }
+ return *idx_or_err;
}
lldb::ChildCacheState Update() override;
diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxxUnorderedMap.cpp b/lldb/source/Plugins/Language/CPlusPlus/LibCxxUnorderedMap.cpp
index 313664e691c9f..de37aeda25168 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxxUnorderedMap.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxxUnorderedMap.cpp
@@ -294,12 +294,13 @@ lldb_private::formatters::LibcxxStdUnorderedMapSyntheticFrontEnd::Update() {
llvm::Expected<size_t>
lldb_private::formatters::LibcxxStdUnorderedMapSyntheticFrontEnd::
GetIndexOfChildWithName(ConstString name) {
- size_t idx = ExtractIndexFromString(name.GetCString());
- if (idx == UINT32_MAX) {
+ auto idx_or_err = formatters::ExtractIndexFromString(name.GetCString());
+ if (!idx_or_err) {
+ llvm::consumeError(idx_or_err.takeError());
return llvm::createStringError("Type has no child named '%s'",
name.AsCString());
}
- return idx;
+ return *idx_or_err;
}
SyntheticChildrenFrontEnd *
diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxxValarray.cpp b/lldb/source/Plugins/Language/CPlusPlus/LibCxxValarray.cpp
index 172df6358789e..4c87e167589da 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxxValarray.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxxValarray.cpp
@@ -129,12 +129,13 @@ lldb_private::formatters::LibcxxStdValarraySyntheticFrontEnd::
if (!m_start || !m_finish)
return llvm::createStringError("Type has no child named '%s'",
name.AsCString());
- size_t idx = ExtractIndexFromString(name.GetCString());
- if (idx == UINT32_MAX) {
+ auto idx_or_err = formatters::ExtractIndexFromString(name.GetCString());
+ if (!idx_or_err) {
+ llvm::consumeError(idx_or_err.takeError());
return llvm::createStringError("Type has no child named '%s'",
name.AsCString());
}
- return idx;
+ return *idx_or_err;
}
lldb_private::SyntheticChildrenFrontEnd *
diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxxVariant.cpp b/lldb/source/Plugins/Language/CPlusPlus/LibCxxVariant.cpp
index 2419683a4d713..12071321189b9 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxxVariant.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxxVariant.cpp
@@ -203,11 +203,13 @@ class VariantFrontEnd : public SyntheticChildrenFrontEnd {
}
llvm::Expected<size_t> GetIndexOfChildWithName(ConstString name) override {
- size_t index = formatters::ExtractIndexFromString(name.GetCString());
- if (index == UINT32_MAX)
+ auto idx_or_err = formatters::ExtractIndexFromString(name.GetCString());
+ if (!idx_or_err) {
+ llvm::consumeError(idx_or_err.takeError());
return llvm::createStringError("Type has no child named '%s'",
name.AsCString());
- return index;
+ }
+ return *idx_or_err;
}
lldb::ChildCacheState Update() override;
diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxxVector.cpp b/lldb/source/Plugins/Language/CPlusPlus/LibCxxVector.cpp
index abbf1ed935b6b..228647b8961b1 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxxVector.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxxVector.cpp
@@ -170,12 +170,13 @@ lldb_private::formatters::LibcxxStdVectorSyntheticFrontEnd::
if (!m_start || !m_finish)
return llvm::createStringError("Type has no child named '%s'",
name.AsCString());
- size_t index = formatters::ExtractIndexFromString(name.GetCString());
- if (index == UINT32_MAX) {
+ auto idx_or_err = formatters::ExtractIndexFromString(name.GetCString());
+ if (!idx_or_err) {
+ llvm::consumeError(idx_or_err.takeError());
return llvm::createStringError("Type has no child named '%s'",
name.AsCString());
}
- return index;
+ return *idx_or_err;
}
lldb_private::formatters::LibcxxVectorBoolSyntheticFrontEnd::
@@ -273,10 +274,14 @@ lldb_private::formatters::LibcxxVectorBoolSyntheticFrontEnd::
if (!m_count || !m_base_data_address)
return llvm::createStringError("Type has no child named '%s'",
name.AsCString());
- const char *item_name = name.GetCString();
- uint32_t idx = ExtractIndexFromString(item_name);
- if (idx == UINT32_MAX ||
- (idx < UINT32_MAX && idx >= CalculateNumChildrenIgnoringErrors()))
+ auto idx_or_err = ExtractIndexFromString(name.AsCString());
+ if (!idx_or_err) {
+ llvm::consumeError(idx_or_err.takeError());
+ return llvm::createStringError("Type has no child named '%s'",
+ name.AsCString());
+ }
+ uint32_t idx = *idx_or_err;
+ if (idx >= CalculateNumChildrenIgnoringErrors())
return llvm::createStringError("Type has no child named '%s'",
name.AsCString());
return idx;
diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibStdcppTuple.cpp b/lldb/source/Plugins/Language/CPlusPlus/LibStdcppTuple.cpp
index cae5aa379b245..4f05dc1934958 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibStdcppTuple.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibStdcppTuple.cpp
@@ -98,12 +98,13 @@ LibStdcppTupleSyntheticFrontEnd::CalculateNumChildren() {
llvm::Expected<size_t>
LibStdcppTupleSyntheticFrontEnd::GetIndexOfChildWithName(ConstString name) {
- size_t index = formatters::ExtractIndexFromString(name.GetCString());
- if (index == UINT32_MAX) {
+ auto idx_or_err = formatters::ExtractIndexFromString(name.GetCString());
+ if (!idx_or_err) {
+ llvm::consumeError(idx_or_err.takeError());
return llvm::createStringError("Type has no child named '%s'",
name.AsCString());
}
- return index;
+ return *idx_or_err;
}
SyntheticChildrenFrontEnd *
diff --git a/lldb/source/Plugins/Language/ObjC/NSArray.cpp b/lldb/source/Plugins/Language/ObjC/NSArray.cpp
index 5e72bd5a6bab6..465639a21d943 100644
--- a/lldb/source/Plugins/Language/ObjC/NSArray.cpp
+++ b/lldb/source/Plugins/Language/ObjC/NSArray.cpp
@@ -528,10 +528,14 @@ lldb_private::formatters::GenericNSArrayMSyntheticFrontEnd<D32, D64>::Update() {
llvm::Expected<size_t> lldb_private::formatters::NSArrayMSyntheticFrontEndBase::
GetIndexOfChildWithName(ConstString name) {
- const char *item_name = name.GetCString();
- size_t idx = ExtractIndexFromString(item_name);
- if (idx == UINT32_MAX ||
- (idx < UINT32_MAX && idx >= CalculateNumChildrenIgnoringErrors()))
+ auto idx_or_err = ExtractIndexFromString(name.AsCString());
+ if (!idx_or_err) {
+ llvm::consumeError(idx_or_err.takeError());
+ return llvm::createStringError("Type has no child named '%s'",
+ name.AsCString());
+ }
+ uint32_t idx = *idx_or_err;
+ if (idx >= CalculateNumChildrenIgnoringErrors())
return llvm::createStringError("Type has no child named '%s'",
name.AsCString());
return idx;
@@ -616,10 +620,14 @@ template <typename D32, typename D64, bool Inline>
llvm::Expected<size_t>
lldb_private::formatters::GenericNSArrayISyntheticFrontEnd<
D32, D64, Inline>::GetIndexOfChildWithName(ConstString name) {
- const char *item_name = name.GetCString();
- uint32_t idx = ExtractIndexFromString(item_name);
- if (idx == UINT32_MAX ||
- (idx < UINT32_MAX && idx >= CalculateNumChildrenIgnoringErrors()))
+ auto idx_or_err = ExtractIndexFromString(name.AsCString());
+ if (!idx_or_err) {
+ llvm::consumeError(idx_or_err.takeError());
+ return llvm::createStringError("Type has no child named '%s'",
+ name.AsCString());
+ }
+ uint32_t idx = *idx_or_err;
+ if (idx >= CalculateNumChildrenIgnoringErrors())
return llvm::createStringError("Type has no child named '%s'",
name.AsCString());
return idx;
diff --git a/lldb/source/Plugins/Language/ObjC/NSDictionary.cpp b/lldb/source/Plugins/Language/ObjC/NSDictionary.cpp
index b04b4610a0bf4..babf638a54ee6 100644
--- a/lldb/source/Plugins/Language/ObjC/NSDictionary.cpp
+++ b/lldb/source/Plugins/Language/ObjC/NSDictionary.cpp
@@ -587,10 +587,14 @@ lldb_private::formatters::NSDictionaryISyntheticFrontEnd::
llvm::Expected<size_t> lldb_private::formatters::
NSDictionaryISyntheticFrontEnd::GetIndexOfChildWithName(ConstString name) {
- const char *item_name = name.GetCString();
- uint32_t idx = ExtractIndexFromString(item_name);
- if (idx == UINT32_MAX ||
- (idx < UINT32_MAX && idx >= CalculateNumChildrenIgnoringErrors()))
+ auto idx_or_err = ExtractIndexFromString(name.AsCString());
+ if (!idx_or_err) {
+ llvm::c...
[truncated]
|
bulbazord
reviewed
May 2, 2025
There was a problem hiding this comment.
I like the idea of this PR (big fan of llvm::Expected) but the way this is implemented contains an anti-pattern IMO. Specifically, all callers consume the error from ExtractIndexFromString only to then create a similar but different error string afterwards. It would be great if the callers could just propagate the error from ExtractIndexFromString instead of doing this dance.
Perhaps ExtractIndexFromString could return the error string Type has no child named '%s' and the function could be named something more descriptive? Something like ExtractIndexOfChildFromString?
+1 This is what I was afraid of when we discussed what kind of error we want to put into these |
| @@ -97,18 +97,19 @@ void lldb_private::formatters::AddFilter( | |||
| category_sp->AddTypeFilter(type_name, match_type, filter_sp); | |||
| } | |||
|
|
|||
| size_t lldb_private::formatters::ExtractIndexFromString(const char *item_name) { | |||
| llvm::Expected<size_t> | |||
There was a problem hiding this comment.
I think here I would just return a std::optional<>, since all of the StringErrors just say "this failed" without any specific other information, and that's something the caller can also piece together.
| auto idx_or_err = ExtractIndexFromString(name.AsCString()); | ||
| if (!idx_or_err) { | ||
| llvm::consumeError(idx_or_err.takeError()); | ||
| return llvm::createStringError("Type has no child named '%s'", |
There was a problem hiding this comment.
See my comment above. I would probably just return optional from ExtractIndexFromString, since the error message isn't that useful. But if it were useful, I wanted to point out that we also have llvm::joinErrors()
There was a problem hiding this comment.
I ended up switching to std::optional following the different discussions. The error message are not very descriptive, and not finding an index did not mean there was an error, but rather that the name was not found in the string.
Michael137
changed the title
Michael137
approved these changes
May 7, 2025
charles-zablit
added a commit
to charles-zablit/llvm-project
that referenced
this pull request
May 8, 2025
…ptional (llvm#138297) This PR is in continuation of llvm#136693.
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 PR is in continuation of #136693.
It upgrades
ExtractIndexFromStringto usellvm::Expected.