aboutsummaryrefslogtreecommitdiff
path: root/ink_stroke_modeler/internal/wobble_smoother_test.cc
diff options
context:
space:
mode:
authorSamuel Freilich <sfreilich@google.com>2022-03-04 11:25:11 -0800
committerCopybara-Service <copybara-worker@google.com>2022-03-04 11:25:41 -0800
commit8628205a2c2ff704f89f8d9cbab05e8732d4a480 (patch)
tree457063e4031a992aba70e5a75e2925599c1b3c59 /ink_stroke_modeler/internal/wobble_smoother_test.cc
parentb5393869c12fc4a485b91bf8acf244c140c8f182 (diff)
downloadink-stroke-modeler-8628205a2c2ff704f89f8d9cbab05e8732d4a480.tar.gz
Fix some build warnings and some errors under GCC
This involved fixing three types of issues: 1. warning: control reaches end of non-void function These were instances of exhaustive switch statements with nothing following to handle invalid values. Even if your switch is exhaustive, it's possible for an enum type to have a value that's not one of the specific enum values and thus not hit any of the cases in the switch statement. If that causes execution to hit the end of a non-void function without a return statement, that's undefined behavior. See https://abseil.io/tips/147. This was a real (though hopefully irrelevant) bug in our handling of invalid input in `StrokeModeler::Update` and the output operator for `Input::EventType`. 2. warning: comparison of integer expressions of different signedness Comparing unsigned and signed integer types does not work as you'd expect numeric comparisons between the two numbers in question to work, since the unsigned value gets naively cast to signed. Most of the cases of the warning cropping up were due to looping from zero to the size of a container, where the container size type is unsigned. In those cases, I use the container size type I'm comparing against, replacing: `for(int i = 0; i < container.size(); i++)` With: `for(decltype(container.size()) i = 0; i < container.size(); i++)` In a few cases, we're comparing an unsigned container size type with a user-provided int, which seems like a real bug. In those cases, I changed the logic to explicitly check for negative values, replacing: `if (x < container.size())` With: `if (x < 0 || (uint) x < container.size())` 3. error: converting to 'ink::stroke_model::Duration' from initializer list would use explicit constructor (Similar for Time.) This is showing up in GCC in cases like: ``` const KalmanPredictorParams params{ ... .prediction_interval{1}} ``` Here we have a struct being initialized with aggregate initialization using designated initializers: https://en.cppreference.com/w/cpp/language/aggregate_initialization "Each direct non-static data member named by the designated initializer is initialized from the corresponding brace-or-equals initializer that follows the designator. Narrowing conversions are prohibited." So here we should be initializing from the brace initializer. This is a class type and there's no equals sign, so it's direct list initialization: https://en.cppreference.com/w/cpp/language/list_initialization And that should allow explicit constructors. This is a known bug in GCC: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91319 But that can be worked around by being explicit about those types in the aggregate initialization, replacing something like: `{.time{1}}` With something like: `{.time{Time(1)}}` (Sometimes changing that to the copy-list-initializer form with the equals for readability.) PiperOrigin-RevId: 432492904
Diffstat (limited to 'ink_stroke_modeler/internal/wobble_smoother_test.cc')
-rw-r--r--ink_stroke_modeler/internal/wobble_smoother_test.cc2
1 files changed, 1 insertions, 1 deletions
diff --git a/ink_stroke_modeler/internal/wobble_smoother_test.cc b/ink_stroke_modeler/internal/wobble_smoother_test.cc
index 64a8965..f31ac9d 100644
--- a/ink_stroke_modeler/internal/wobble_smoother_test.cc
+++ b/ink_stroke_modeler/internal/wobble_smoother_test.cc
@@ -25,7 +25,7 @@ namespace stroke_model {
namespace {
const WobbleSmootherParams kDefaultParams{
- .timeout{.04}, .speed_floor = 1.31, .speed_ceiling = 1.44};
+ .timeout = Duration(.04), .speed_floor = 1.31, .speed_ceiling = 1.44};
TEST(WobbleSmootherTest, SlowStraightLine) {
// The line moves at 1 cm/s, which is below the floor of 1.31 cm/s.