Fix build on targets where int32_t is not int (LIST_VALUE) - #165
Open
jerrytron wants to merge 1 commit into
Open
Fix build on targets where int32_t is not int (LIST_VALUE)#165jerrytron wants to merge 1 commit into
jerrytron wants to merge 1 commit into
Conversation
value::set<value_type::int32> is specialized on int32_t, and int32_t is
not int on every target: on Xtensa it is long. So set<int32>(0) and
set<int32>(get_flag_value(...)), which returns int, select the primary
template on those platforms rather than the specialization, and the
build stops on
static assertion failed: No setter for this type defined!
It compiles on x86-64 and arm64 Linux/macOS, where int32_t is int, so
the four call sites added with LIST_VALUE went unnoticed.
Every other set<value_type::int32> call site in the tree already casts
explicitly - numeric_operations.h, container_operations.cpp,
runner_impl.cpp - so this just follows the existing convention.
Found building for ESP32-S3.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Owner
|
Thanks, good catch |
Ink Proof ResultsThese results are obtained by running the Ink-Proof Testing Suite on the compiled binaries in this pull request.
|
Owner
|
Would you mind applying this patch? I do not have access rights to your branch, and my OCD would like a green pipeline. index eb8200a..fa27c3c 100644
--- a/inkcpp/list_operations.h
+++ b/inkcpp/list_operations.h
@@ -336,7 +336,9 @@ public:
if (flag.list_id < 0 || flag.flag < 0) {
stack.push(value{}.set<value_type::int32>(int32_t{0}));
} else {
- stack.push(value{}.set<value_type::int32>(static_cast<int32_t>(_list_table.get_flag_value(flag))));
+ stack.push(
+ value{}.set<value_type::int32>(static_cast<int32_t>(_list_table.get_flag_value(flag)))
+ );
}
}
};
@@ -354,7 +356,9 @@ public:
if (max_flag.list_id < 0 || max_flag.flag < 0) {
stack.push(value{}.set<value_type::int32>(int32_t{0}));
} else {
- stack.push(value{}.set<value_type::int32>(static_cast<int32_t>(_list_table.get_flag_value(max_flag))));
+ stack.push(
+ value{}.set<value_type::int32>(static_cast<int32_t>(_list_table.get_flag_value(max_flag)))
+ );
}
}
}; |
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
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.
value::set<value_type::int32>is specialized onint32_t, andint32_tis notinton every target — on Xtensa (ESP32) it islong.The four
LIST_VALUEcall sites added in #162 pass a bare0and theintreturned byget_flag_value(), so on those platforms overload resolution selects the primary template rather than the specialization and the build stops:It compiles on x86-64 and arm64 Linux/macOS, where
int32_tisint, which is why CI did not catch it.Every other
set<value_type::int32>call site in the tree already converts explicitly —numeric_operations.h,container_operations.cpp,runner_impl.cpp— so this just follows the existing convention rather than introducing one.Found building for ESP32-S3.
ctestpasses.