Merge pull request #4433 from thehans/issue4432

Fix for issue #4432

src/core/LinearExtrudeNode.cc

@@ -75,6 +75,7 @@ static std::shared_ptr<AbstractNode> builtin_linear_extrude(const ModuleInstanti
   auto node = std::make_shared<LinearExtrudeNode>(inst);
 
   Parameters parameters = parse_parameters(std::move(arguments), inst->location());
+  parameters.set_caller("linear_extrude");
 
   node->fn = parameters["$fn"].toDouble();
   node->fs = parameters["$fs"].toDouble();
@@ -115,8 +116,8 @@ static std::shared_ptr<AbstractNode> builtin_linear_extrude(const ModuleInstanti
   if (node->scale_x < 0) node->scale_x = 0;
   if (node->scale_y < 0) node->scale_y = 0;
 
-  node->has_slices = parameters["slices"].getUnsignedInt(node->slices);
-  node->has_segments = parameters["segments"].getUnsignedInt(node->segments);
+  node->has_slices = parameters.validate_integral("slices", node->slices, 1u);
+  node->has_segments = parameters.validate_integral("segments", node->segments, 0u);
 
   node->twist = 0.0;
   parameters["twist"].getFiniteDouble(node->twist);

src/core/Parameters.cc

@@ -102,6 +102,23 @@ bool Parameters::valid(const std::string& name, Value::Type type)
   return valid(name, *value, type);
 }
 
+// Handle all general warnings and return true if a valid number is found.
+bool Parameters::validate_number(const std::string& name, double& out)
+{
+  boost::optional<const Value&> value = lookup(name);
+  if (!value || value->isUndefined()) {
+    return false;
+  } else if (valid(name, *value, Value::Type::NUMBER)) {
+    if (value->getFiniteDouble(out)) {
+      return true;
+    } else {
+      LOG(message_group::Warning, loc, documentRoot(), "%1$s(..., %2$s=%3$s) argument cannot be infinite or nan", caller, name, value->toString());
+      return false;
+    }
+  }
+  return false;
+}
+
 ContextFrame Parameters::to_context_frame() &&
 {
   handle.release();
@@ -234,20 +251,44 @@ void print_argCnt_warning(
   const std::string& expected,
   const Location& loc,
   const std::string& documentRoot
-  ){
+  ) {
   LOG(message_group::Warning, loc, documentRoot, "%1$s() number of parameters does not match: expected %2$s, found %3$i", name, expected, found);
 }
 
-void print_argConvert_warning(
-  const std::string& name,
+void print_argConvert_positioned_warning(
+  const std::string& calledName,
   const std::string& where,
   const Value& found,
   std::vector<Value::Type> expected,
   const Location& loc,
   const std::string& documentRoot
   ){
   std::stringstream message;
-  message << name << "() parameter could not be converted: " << where << ": expected ";
+  message << calledName << "() parameter could not be converted: " << where << ": expected ";
+  if (expected.size() == 1) {
+    message << Value::typeName(expected[0]);
+  } else {
+    assert(expected.size() > 0);
+    message << "one of (" << Value::typeName(expected[0]);
+    for (size_t i = 1; i < expected.size(); i++) {
+      message << ", " << Value::typeName(expected[i]);
+    }
+    message << ")";
+  }
+  message << ", found " << found.typeName()) << " " << "(" << found.toEchoStringNoThrow() << ")";
+  LOG(message_group::Warning, loc, documentRoot, "%1$s", message.str());
+}
+
+void print_argConvert_warning(
+  const std::string& calledName,
+  const std::string& argName,
+  const Value& found,
+  std::vector<Value::Type> expected,
+  const Location& loc,
+  const std::string& documentRoot
+  ) {
+  std::stringstream message;
+  message << calledName << "(..., " << argName << "=" << found.toEchoStringNoThrow() << ") Invalid type: expected ";
   if (expected.size() == 1) {
     message << Value::typeName(expected[0]);
   } else {
@@ -258,6 +299,6 @@ void print_argConvert_warning(
     }
     message << ")";
   }
-  message << ", found " << Value::typeName(found.type()) << " " << "(" << found.toEchoStringNoThrow() << ")";
+  message << ", found " << found.typeName();
   LOG(message_group::Warning, loc, documentRoot, "%1$s", message.str());
 }

src/core/Parameters.h

@@ -56,6 +56,10 @@ class Parameters
   const Value& operator[](const std::string& name) const { return get(name); }
   bool valid(const std::string& name, Value::Type type);
   bool valid_required(const std::string& name, Value::Type type);
+  bool validate_number(const std::string& name, double& out);
+  template <typename T> bool validate_integral(const std::string& name, T& out,
+                                               T lo = std::numeric_limits<T>::min(),
+                                               T hi = std::numeric_limits<T>::max());
 
   ContextFrame to_context_frame() &&;
 
@@ -70,9 +74,31 @@ class Parameters
   Location loc;
 };
 
+// Silently clamp to the given range(defaults to numeric_limits)
+// as long as param is a finite number.
+template <typename T>
+bool Parameters::validate_integral(const std::string& name, T& out, T lo, T hi)
+{
+  double temp;
+  if (validate_number(name, temp)) {
+    if (temp < lo) {
+      out = lo;
+    } else if (temp > hi) {
+      out = hi;
+    } else {
+      out = static_cast<T>(temp);
+    }
+    return true;
+  }
+  return false;
+}
+
 void print_argCnt_warning(const std::string& name, int found,
                           const std::string& expected, const Location& loc,
                           const std::string& documentRoot);
-void print_argConvert_warning(const std::string& name, const std::string& where,
+void print_argConvert_positioned_warning(const std::string& calledName, const std::string& where,
+                                         const Value& found, std::vector<Value::Type> expected,
+                                         const Location& loc, const std::string& documentRoot);
+void print_argConvert_warning(const std::string& calledName, const std::string& argName,
                               const Value& found, std::vector<Value::Type> expected,
                               const Location& loc, const std::string& documentRoot);

src/core/builtin_functions.cc

@@ -73,11 +73,11 @@ static inline bool check_arguments(const char *function_name, const Arguments& a
   return true;
 }
 /* // Commented due to compiler warning of unused function.
-static inline bool try_check_arguments(const Arguments& arguments, int expected_count)
-{
-  return check_arguments(nullptr, arguments, Location::NONE, expected_count, false);
-}
-*/
+   static inline bool try_check_arguments(const Arguments& arguments, int expected_count)
+   {
+   return check_arguments(nullptr, arguments, Location::NONE, expected_count, false);
+   }
+ */
 template <size_t N>
 static inline bool check_arguments(const char *function_name, const Arguments& arguments, const Location& loc, const Value::Type (& expected_types) [N], bool warn = true)
 {
@@ -87,7 +87,7 @@ static inline bool check_arguments(const char *function_name, const Arguments& a
   for (size_t i = 0; i < N; i++) {
     if (arguments[i]->type() != expected_types[i]) {
       if (warn) {
-        print_argConvert_warning(function_name, "argument " + STR(i), arguments[i]->clone(), {expected_types[i]}, loc, arguments.documentRoot());
+        print_argConvert_positioned_warning(function_name, "argument " + STR(i), arguments[i]->clone(), {expected_types[i]}, loc, arguments.documentRoot());
       }
       return false;
     }
@@ -195,7 +195,7 @@ static std::vector<double> min_max_arguments(const Arguments& arguments, const L
       // 4/20/14 semantic change per discussion:
       // break on any non-number
       if (element.type() != Value::Type::NUMBER) {
-        print_argConvert_warning(function_name, "vector element " + STR(i), element, {Value::Type::NUMBER}, loc, arguments.documentRoot());
+        print_argConvert_positioned_warning(function_name, "vector element " + STR(i), element, {Value::Type::NUMBER}, loc, arguments.documentRoot());
         return {};
       }
       output.push_back(element.toDouble());
@@ -206,7 +206,7 @@ static std::vector<double> min_max_arguments(const Arguments& arguments, const L
       // 4/20/14 semantic change per discussion:
       // break on any non-number
       if (argument->type() != Value::Type::NUMBER) {
-        print_argConvert_warning(function_name, "argument " + STR(i), argument->clone(), {Value::Type::NUMBER}, loc, arguments.documentRoot());
+        print_argConvert_positioned_warning(function_name, "argument " + STR(i), argument->clone(), {Value::Type::NUMBER}, loc, arguments.documentRoot());
         return {};
       }
       output.push_back(argument->toDouble());

tests/CMakeLists.txt

@@ -523,6 +523,7 @@ list(APPEND FAILING_FILES
 list(APPEND ECHO_FILES ${FUNCTION_FILES} ${MISC_FILES} ${REDEFINITION_FILES}
   ${TEST_SCAD_DIR}/3D/features/for-tests.scad
   ${TEST_SCAD_DIR}/3D/features/rotate-parameters.scad
+  ${TEST_SCAD_DIR}/3D/features/linear_extrude-parameter-tests.scad
   ${TEST_SCAD_DIR}/misc/expression-evaluation-tests.scad
   ${TEST_SCAD_DIR}/misc/echo-tests.scad
   ${TEST_SCAD_DIR}/misc/assert-fail1-test.scad

tests/data/scad/3D/issues/issue4432.scad

@@ -0,0 +1 @@
+linear_extrude(height=10, twist=45, slices=0) square(10,center=true);

tests/regression/echotest/linear_extrude-parameter-tests-expected.echo

@@ -0,0 +1,18 @@
+WARNING: linear_extrude(..., slices=inf) argument cannot be infinite or nan in file linear_extrude-parameter-tests.scad, line 14
+WARNING: linear_extrude(..., slices=-inf) argument cannot be infinite or nan in file linear_extrude-parameter-tests.scad, line 14
+WARNING: linear_extrude(..., slices=nan) argument cannot be infinite or nan in file linear_extrude-parameter-tests.scad, line 14
+WARNING: linear_extrude(..., slices="") Invalid type: expected number, found string in file linear_extrude-parameter-tests.scad, line 14
+WARNING: linear_extrude(..., slices=true) Invalid type: expected number, found bool in file linear_extrude-parameter-tests.scad, line 14
+WARNING: linear_extrude(..., slices=[1 : 1 : 3]) Invalid type: expected number, found range in file linear_extrude-parameter-tests.scad, line 14
+WARNING: linear_extrude(..., slices=inf) argument cannot be infinite or nan in file linear_extrude-parameter-tests.scad, line 17
+WARNING: linear_extrude(..., slices=-inf) argument cannot be infinite or nan in file linear_extrude-parameter-tests.scad, line 17
+WARNING: linear_extrude(..., slices=nan) argument cannot be infinite or nan in file linear_extrude-parameter-tests.scad, line 17
+WARNING: linear_extrude(..., slices="") Invalid type: expected number, found string in file linear_extrude-parameter-tests.scad, line 17
+WARNING: linear_extrude(..., slices=true) Invalid type: expected number, found bool in file linear_extrude-parameter-tests.scad, line 17
+WARNING: linear_extrude(..., slices=[1 : 1 : 3]) Invalid type: expected number, found range in file linear_extrude-parameter-tests.scad, line 17
+WARNING: linear_extrude(..., scale=inf) could not be converted in file linear_extrude-parameter-tests.scad, line 20
+WARNING: linear_extrude(..., scale=-inf) could not be converted in file linear_extrude-parameter-tests.scad, line 20
+WARNING: linear_extrude(..., scale=nan) could not be converted in file linear_extrude-parameter-tests.scad, line 20
+WARNING: linear_extrude(..., scale="") could not be converted in file linear_extrude-parameter-tests.scad, line 20
+WARNING: linear_extrude(..., scale=true) could not be converted in file linear_extrude-parameter-tests.scad, line 20
+WARNING: linear_extrude(..., scale=[1 : 1 : 3]) could not be converted in file linear_extrude-parameter-tests.scad, line 20

tests/regression/echotest/text-metrics-test-expected.echo

@@ -4,17 +4,17 @@ ECHO: { position = [-116.212, -10.208]; size = [98.96, 20.416]; ascent = 20.1344
 ECHO: { nominal = { ascent = 12.5733; descent = -2.9433; }; max = { ascent = 13.6109; descent = -4.2114; }; interline = 15.9709; font = { family = "Liberation Sans"; style = "Regular"; }; }
 ECHO: { nominal = { ascent = 25.1466; descent = -5.8866; }; max = { ascent = 27.2218; descent = -8.4228; }; interline = 31.9418; font = { family = "Liberation Sans"; style = "Regular"; }; }
 ECHO: "Errors..."
-WARNING: textmetrics() parameter could not be converted: size: expected number, found vector ([]) in file text-metrics-test.scad, line 20
-WARNING: textmetrics() parameter could not be converted: text: expected string, found number (123) in file text-metrics-test.scad, line 20
-WARNING: textmetrics() parameter could not be converted: spacing: expected number, found string ("") in file text-metrics-test.scad, line 20
-WARNING: textmetrics() parameter could not be converted: font: expected string, found bool (true) in file text-metrics-test.scad, line 20
-WARNING: textmetrics() parameter could not be converted: direction: expected string, found number (0) in file text-metrics-test.scad, line 20
-WARNING: textmetrics() parameter could not be converted: language: expected string, found range ([0 : 1 : 10]) in file text-metrics-test.scad, line 20
-WARNING: textmetrics() parameter could not be converted: script: expected string, found number (0) in file text-metrics-test.scad, line 20
-WARNING: textmetrics() parameter could not be converted: halign: expected string, found number (0) in file text-metrics-test.scad, line 20
-WARNING: textmetrics() parameter could not be converted: valign: expected string, found number (0) in file text-metrics-test.scad, line 20
+WARNING: textmetrics(..., size=[]) Invalid type: expected number, found vector in file text-metrics-test.scad, line 20
+WARNING: textmetrics(..., text=123) Invalid type: expected string, found number in file text-metrics-test.scad, line 20
+WARNING: textmetrics(..., spacing="") Invalid type: expected number, found string in file text-metrics-test.scad, line 20
+WARNING: textmetrics(..., font=true) Invalid type: expected string, found bool in file text-metrics-test.scad, line 20
+WARNING: textmetrics(..., direction=0) Invalid type: expected string, found number in file text-metrics-test.scad, line 20
+WARNING: textmetrics(..., language=[0 : 1 : 10]) Invalid type: expected string, found range in file text-metrics-test.scad, line 20
+WARNING: textmetrics(..., script=0) Invalid type: expected string, found number in file text-metrics-test.scad, line 20
+WARNING: textmetrics(..., halign=0) Invalid type: expected string, found number in file text-metrics-test.scad, line 20
+WARNING: textmetrics(..., valign=0) Invalid type: expected string, found number in file text-metrics-test.scad, line 20
 ECHO: { position = [0, 0]; size = [0, 0]; ascent = 0; descent = 0; offset = [0, 0]; advance = [0, 0]; }
 WARNING: variable text not specified as parameter in file text-metrics-test.scad, line 25
-WARNING: fontmetrics() parameter could not be converted: size: expected number, found bool (true) in file text-metrics-test.scad, line 25
-WARNING: fontmetrics() parameter could not be converted: font: expected string, found number (0) in file text-metrics-test.scad, line 25
+WARNING: fontmetrics(..., size=true) Invalid type: expected number, found bool in file text-metrics-test.scad, line 25
+WARNING: fontmetrics(..., font=0) Invalid type: expected string, found number in file text-metrics-test.scad, line 25
 ECHO: { nominal = { ascent = 12.5733; descent = -2.9433; }; max = { ascent = 13.6109; descent = -4.2114; }; interline = 15.9709; font = { family = "Liberation Sans"; style = "Regular"; }; }