[PATCH] Add function pre- and post-conditions.

classic Classic list List threaded Threaded
22 messages Options
12
Reply | Threaded
Open this post in threaded view
|

[PATCH] Add function pre- and post-conditions.

Taahir Ahmed
Our previous discussion about asserts and testing in general led me to
draw up this patch, which I thought I would send around for comment
before pursuing further.

This message should be directly usable by `git am`.  If it's preferable,
I can instead make a pull request on github.

Taahir Ahmed

From 5595c58f57e39b848c504f2cafc2643241588dbe Mon Sep 17 00:00:00 2001
From: Taahir Ahmed <[hidden email]>
Date: Tue, 26 Aug 2014 23:25:01 -0500
Subject: [PATCH] Add function pre- and post-conditions.

Change description
==================

This change adds optional preconditions and postconditions to OpenSCAD
functions.  Multiple preconditions and postconditions may be attached to
a given function.  Optional messages can be attached to each
precondition and postcondition, to aid the user in debugging.

Usage
-----

Conditions are added to functions after the close of the argument list
and before the defining `=`:

```
function f(x)
    pre(x > 0, "The argument must be positive.")
    post(__retval > 0, "The return value must be positive.")
    = x;
```

The syntax is whitespace-insensitive, and as many preconditions and
postconditions may be attached as are needed, in any order.
Preconditions are declared using `pre(expr)` or `pre(expr,expr)`.
Postconditions are declared using `post(expr)` or `post(expr,expr)`.

  * The first argument is the condition.  It can be any single
    expression.  If it is true when coerced to a boolean, then the
    precondition passes.  Otherwise, the precondition fails.

  * The optional second argument is the message.  It can be any single
    expression.  Its value is coerced to a string, which then forms
    part of the error message.

Semantics
---------

All the involved expressions have access to the full lexical scope in
which the function is defined, including the formal parameters.  In
addition, postconditions have access to the return value of the function
as the special symbol `__retval`.

Function evaluation proceeds in the following order:

  - Preconditions are evaluated in the order in which they appear.
    If any precondition fails, then no subsequent precondition is
    evaluated, and the function returns `undef`.

  - If all preconditions succeed, then the body of the function is
    evaluated.

  - Postconditions are evaluated in the order in which they appear.  If
    any postcondition fails, then no subsequent postcondition is
    evaluated, and the function returns `undef`.

  - If all postconditions succeed, the function returns the value
    produced by the body.

Deficiencies and gotchas
========================

  - The behavior of `value.toBool()` leads to some unexpected behavior
    --- for example, the following code will not trigger a precondition
    failure:

    ```
    function f(x) pre(x) = x;
    z = f(0/0);
    ```

    Internally, `toBool` checks that `x != 0`, and `NaN != 0`, so the
    precondition passes.  I figured it was better to stick with uniform
    semantics, even if they are a little strange.

  - Ideally, condition failure would halt compilation.  This currently
    does not happen.  Compilation continues as if the function returned
    `undef`.

  - There are no tests associated with this feature yet.  The existing
    testing framework seems to be set up around testing geometry output,
    not control flow / console output.
---
 src/dxfdim.cc  |   4 +-
 src/func.cc    | 269
+++++++++++++++++++++++++++++++++++++++++++++++++--------
 src/function.h | 115 +++++++++++++++++++++---
 src/lexer.l    |   4 +
 src/module.cc  |  28 ++++++
 src/module.h   |  24 +++--
 src/parser.y   |  78 +++++++++++++++--
 7 files changed, 459 insertions(+), 63 deletions(-)

diff --git a/src/dxfdim.cc b/src/dxfdim.cc
index 8f55a2f..898f21a 100644
--- a/src/dxfdim.cc
+++ b/src/dxfdim.cc
@@ -215,6 +215,6 @@ Value builtin_dxf_cross(const Context *ctx, const
EvalContext *evalctx)
 
 void initialize_builtin_dxf_dim()
 {
- Builtins::init("dxf_dim", new BuiltinFunction(&builtin_dxf_dim));
- Builtins::init("dxf_cross", new BuiltinFunction(&builtin_dxf_cross));
+ Builtins::init("dxf_dim", new BuiltinFunction(&builtin_dxf_dim,
"dxf_dim"));
+ Builtins::init("dxf_cross", new BuiltinFunction(&builtin_dxf_cross,
"dxf_cross"));
 }
diff --git a/src/func.cc b/src/func.cc
index f26f44c..53c359b 100644
--- a/src/func.cc
+++ b/src/func.cc
@@ -65,10 +65,30 @@ int process_id = getpid();
 boost::mt19937 deterministic_rng;
 boost::mt19937 lessdeterministic_rng( std::time(0) + process_id );
 
+AbstractFunction::AbstractFunction()
+    : feature(NULL)
+{
+}
+
+AbstractFunction::AbstractFunction(const Feature &feature_in)
+    : feature(&feature_in)
+{
+}
+
 AbstractFunction::~AbstractFunction()
 {
 }
 
+bool AbstractFunction::is_experimental() const
+{
+    return feature != NULL;
+}
+
+bool AbstractFunction::is_enabled() const
+{
+    return (feature == NULL) || feature->is_enabled();
+}
+
 Value AbstractFunction::evaluate(const Context*, const EvalContext *evalctx)
const
 {
  (void)evalctx; // unusued parameter
@@ -82,17 +102,159 @@ std::string AbstractFunction::dump(const std::string
&indent, const std::string
  return dump.str();
 }
 
+condition::condition(
+    const type_tag type_in,
+    boost::shared_ptr<Expression> expr_in
+)
+    : type(type_in),
+      expr(expr_in)
+{
+}
+
+condition::condition(
+    const type_tag type_in,
+    boost::shared_ptr<Expression> expr_in,
+    boost::shared_ptr<Expression> msg_in
+)
+    : type(type_in),
+      expr(expr_in),
+      msg(msg_in)
+{
+}
+
+std::string condition::typestring() const
+{
+    switch(type) {
+    case condition::precondition:
+        return std::string("precondition");
+    case condition::postcondition:
+        return std::string("postcondition");
+    default:
+        return std::string("");
+    }
+}
+
+namespace
+{
+
+template<condition::type_tag type>
+struct condition_matcher
+{
+    bool operator()(const condition &cond)
+    {
+        return cond.type == type;
+    }
+};
+
+}
+
+Function::Function(
+    const AssignmentList                &formal_args_in,
+          std::vector<condition>         conditions_in,
+    const boost::shared_ptr<Expression>  body_in,
+    const std::string                   &name_in,
+    const Module                        &enclosing_in
+)
+    : definition_arguments(formal_args_in),
+      conds(conditions_in),
+      // Split up pre- and post- so we don't have to do it every time.
+      precond_end(
+          std::stable_partition(
+              conds.begin(),
+              conds.end(),
+              (condition_matcher<condition::precondition>())
+          )
+      ),
+      body(body_in),
+      name(name_in),
+      enclosing(enclosing_in)
+{
+}
+
 Function::~Function()
 {
- delete expr;
+}
+
+namespace
+{
+
+// Run a pre- or post-condition in the given context.
+struct condition_runner
+{
+    condition_runner(const Context &ctx_in, const std::string &fullname_in)
+        : ctx(ctx_in),
+          fullname(fullname_in),
+          all_passed(true)
+    {
+    }
+
+    operator bool () const
+    {
+        return all_passed;
+    }
+
+    void operator()(const condition &cond)
+    {
+        // Break out early if an earlier condition has failed.
+        if(! all_passed) {
+            return;
+        }
+
+        const Value condition_value = cond.expr->evaluate(&ctx);
+        if(!condition_value.toBool()) {
+            all_passed = false;
+
+            std::stringstream msg;
+            msg << "Error: " << cond.typestring() << " violation in "
+                << fullname;
+
+            if(cond.msg) {
+                const Value usermsg = cond.msg->evaluate(&ctx);
+                msg << ": " << usermsg.toString();
+            }
+
+            PRINT(msg.str().c_str());
+        }
+    }
+
+    const Context &ctx;
+    const std::string fullname;
+    bool all_passed;
+};
+
 }
 
 Value Function::evaluate(const Context *ctx, const EvalContext *evalctx)
const
 {
- if (!expr) return Value();
- Context c(ctx);
- c.setVariables(definition_arguments, evalctx);
- return expr->evaluate(&c);
+    // definition_arguments contains any default parameters values.
+    // setVariables is an ad-hoc function that sets up inner appropriately
for
+    // the body of a function call.
+    Context inner(ctx);
+ inner.setVariables(definition_arguments, evalctx);
+
+    condition_runner precond_runner(inner, fullname());
+    std::for_each(conds.begin(), precond_end, precond_runner);
+    if(!precond_runner) {
+        // TODO: make a precondition failure halt compilation.
+        return (Value());
+    }
+
+    // Evaluate the body.
+    const Value produced = body ? (*body).evaluate(&inner) : (Value());
+
+    // Create a context derived from the inner context, with the variable
+    // __retval set to the function return value.
+    Context post_context(&inner);
+    post_context.set_variable("__retval", produced);
+
+    condition_runner postcond_runner(post_context, fullname());
+    std::for_each(precond_end, conds.end(), postcond_runner);
+    if(!postcond_runner) {
+        // TODO: make a postcondition failure halt compilation.
+        return (Value());
+    }
+
+    return produced;
 }
 
 std::string Function::dump(const std::string &indent, const std::string
&name) const
@@ -105,10 +267,38 @@ std::string Function::dump(const std::string &indent,
const std::string &name) c
  dump << arg.first;
  if (arg.second) dump << " = " << *arg.second;
  }
- dump << ") = " << *expr << ";\n";
+ dump << ") = " << body << ";\n";
  return dump.str();
 }
 
+std::string Function::fullname() const
+{
+    std::stringstream namestream;
+    namestream << enclosing.fullname() << "::" << name;
+
+    return namestream.str();
+}
+
+BuiltinFunction::BuiltinFunction(
+    eval_func_t eval_func_in,
+    const std::string name_in
+)
+    : eval_func(eval_func_in),
+      name(name_in)
+{
+}
+
+BuiltinFunction::BuiltinFunction(
+    eval_func_t       eval_func_in,
+    const Feature     &feature_in,
+    const std::string name_in
+)
+    : AbstractFunction(feature_in),
+      eval_func(eval_func_in),
+      name(name_in)
+{
+}
+
 BuiltinFunction::~BuiltinFunction()
 {
 }
@@ -125,6 +315,11 @@ std::string BuiltinFunction::dump(const std::string
&indent, const std::string &
  return dump.str();
 }
 
+std::string BuiltinFunction::fullname() const
+{
+    return "builtin::" + name;
+}
+
 static inline double deg2rad(double x)
 {
  return x * M_PI / 180.0;
@@ -917,35 +1112,35 @@ Value builtin_cross(const Context *, const EvalContext
*evalctx)
 
 void register_builtin_functions()
 {
- Builtins::init("abs", new BuiltinFunction(&builtin_abs));
- Builtins::init("sign", new BuiltinFunction(&builtin_sign));
- Builtins::init("rands", new BuiltinFunction(&builtin_rands));
- Builtins::init("min", new BuiltinFunction(&builtin_min));
- Builtins::init("max", new BuiltinFunction(&builtin_max));
- Builtins::init("sin", new BuiltinFunction(&builtin_sin));
- Builtins::init("cos", new BuiltinFunction(&builtin_cos));
- Builtins::init("asin", new BuiltinFunction(&builtin_asin));
- Builtins::init("acos", new BuiltinFunction(&builtin_acos));
- Builtins::init("tan", new BuiltinFunction(&builtin_tan));
- Builtins::init("atan", new BuiltinFunction(&builtin_atan));
- Builtins::init("atan2", new BuiltinFunction(&builtin_atan2));
- Builtins::init("round", new BuiltinFunction(&builtin_round));
- Builtins::init("ceil", new BuiltinFunction(&builtin_ceil));
- Builtins::init("floor", new BuiltinFunction(&builtin_floor));
- Builtins::init("pow", new BuiltinFunction(&builtin_pow));
- Builtins::init("sqrt", new BuiltinFunction(&builtin_sqrt));
- Builtins::init("exp", new BuiltinFunction(&builtin_exp));
- Builtins::init("len", new BuiltinFunction(&builtin_length));
- Builtins::init("log", new BuiltinFunction(&builtin_log));
- Builtins::init("ln", new BuiltinFunction(&builtin_ln));
- Builtins::init("str", new BuiltinFunction(&builtin_str));
- Builtins::init("chr", new BuiltinFunction(&builtin_chr));
- Builtins::init("concat", new BuiltinFunction(&builtin_concat));
- Builtins::init("lookup", new BuiltinFunction(&builtin_lookup));
- Builtins::init("search", new BuiltinFunction(&builtin_search));
- Builtins::init("version", new BuiltinFunction(&builtin_version));
- Builtins::init("version_num", new BuiltinFunction(&builtin_version_num));
- Builtins::init("norm", new BuiltinFunction(&builtin_norm));
- Builtins::init("cross", new BuiltinFunction(&builtin_cross));
- Builtins::init("parent_module", new
BuiltinFunction(&builtin_parent_module));
+ Builtins::init("abs", new BuiltinFunction(&builtin_abs, "abs"));
+ Builtins::init("sign", new BuiltinFunction(&builtin_sign, "sign"));
+ Builtins::init("rands", new BuiltinFunction(&builtin_rands, "rands"));
+ Builtins::init("min", new BuiltinFunction(&builtin_min, "min"));
+ Builtins::init("max", new BuiltinFunction(&builtin_max, "max"));
+ Builtins::init("sin", new BuiltinFunction(&builtin_sin, "sin"));
+ Builtins::init("cos", new BuiltinFunction(&builtin_cos, "cos"));
+ Builtins::init("asin", new BuiltinFunction(&builtin_asin, "asin"));
+ Builtins::init("acos", new BuiltinFunction(&builtin_acos, "acos"));
+ Builtins::init("tan", new BuiltinFunction(&builtin_tan, "tan"));
+ Builtins::init("atan", new BuiltinFunction(&builtin_atan, "atan"));
+ Builtins::init("atan2", new BuiltinFunction(&builtin_atan2, "atan2"));
+ Builtins::init("round", new BuiltinFunction(&builtin_round, "round"));
+ Builtins::init("ceil", new BuiltinFunction(&builtin_ceil, "ceil"));
+ Builtins::init("floor", new BuiltinFunction(&builtin_floor, "floor"));
+ Builtins::init("pow", new BuiltinFunction(&builtin_pow, "pow"));
+ Builtins::init("sqrt", new BuiltinFunction(&builtin_sqrt, "sqrt"));
+ Builtins::init("exp", new BuiltinFunction(&builtin_exp, "exp"));
+ Builtins::init("len", new BuiltinFunction(&builtin_length, "length"));
+ Builtins::init("log", new BuiltinFunction(&builtin_log, "log"));
+ Builtins::init("ln", new BuiltinFunction(&builtin_ln, "ln"));
+ Builtins::init("str", new BuiltinFunction(&builtin_str, "str"));
+ Builtins::init("chr", new BuiltinFunction(&builtin_chr, "chr"));
+ Builtins::init("concat", new BuiltinFunction(&builtin_concat, "concat"));
+ Builtins::init("lookup", new BuiltinFunction(&builtin_lookup, "lookup"));
+ Builtins::init("search", new BuiltinFunction(&builtin_search, "search"));
+ Builtins::init("version", new BuiltinFunction(&builtin_version,
"version"));
+ Builtins::init("version_num", new BuiltinFunction(&builtin_version_num,
"version_num"));
+ Builtins::init("norm", new BuiltinFunction(&builtin_norm, "norm"));
+ Builtins::init("cross", new BuiltinFunction(&builtin_cross, "cross"));
+ Builtins::init("parent_module", new
BuiltinFunction(&builtin_parent_module, "parent_module"));
 }
diff --git a/src/function.h b/src/function.h
index d8e3ad7..e47c01d 100644
--- a/src/function.h
+++ b/src/function.h
@@ -1,25 +1,29 @@
 #pragma once
 
+#include "expression.h"
+#include "module.h"
 #include "value.h"
 #include "typedefs.h"
 #include "feature.h"
 
 #include <string>
+#include <utility>
 #include <vector>
 
-
 class AbstractFunction
 {
 private:
-        const Feature *feature;
+    const Feature *feature;
 public:
-        AbstractFunction() : feature(NULL) {}
-        AbstractFunction(const Feature& feature) : feature(&feature) {}
+    AbstractFunction();
+    AbstractFunction(const Feature& feature);
  virtual ~AbstractFunction();
-        virtual bool is_experimental() const { return feature != NULL; }
-        virtual bool is_enabled() const { return (feature == NULL) ||
feature->is_enabled(); }
+    virtual bool is_experimental() const;
+    virtual bool is_enabled() const;
  virtual Value evaluate(const class Context *ctx, const class EvalContext
*evalctx) const;
  virtual std::string dump(const std::string &indent, const std::string
&name) const;
+
+    virtual std::string fullname() const = 0;
 };
 
 class BuiltinFunction : public AbstractFunction
@@ -28,12 +32,56 @@ public:
  typedef Value (*eval_func_t)(const Context *ctx, const EvalContext
*evalctx);
  eval_func_t eval_func;
 
- BuiltinFunction(eval_func_t f) : eval_func(f) { }
- BuiltinFunction(eval_func_t f, const Feature& feature) :
AbstractFunction(feature), eval_func(f) { }
+ BuiltinFunction(eval_func_t f, const std::string name_in);
+
+ BuiltinFunction(
+        eval_func_t f,
+        const Feature& feature,
+        const std::string name_in
+    );
+
  virtual ~BuiltinFunction();
 
  virtual Value evaluate(const Context *ctx, const EvalContext *evalctx)
const;
  virtual std::string dump(const std::string &indent, const std::string
&name) const;
+
+    virtual std::string fullname() const;
+private:
+    const std::string name;
+};
+
+// Pre- and post-condition container.
+//
+// Bundles an expression together with the message that should be printed if
the
+// expression is violated.  Not structured for inheritance.
+class condition
+{
+public:
+    enum type_tag
+    {
+        precondition,
+        postcondition
+    };
+
+    type_tag type;
+
+    // TODO: These members should be made into std::unique_ptrs at the
earliest
+    // opportunity.
+    boost::shared_ptr<Expression> expr;
+    boost::shared_ptr<Expression> msg;
+
+    condition(
+        const type_tag type_in,
+        const boost::shared_ptr<Expression> expr_in
+    );
+
+    condition(
+        const type_tag type_in,
+        const boost::shared_ptr<Expression> expr_in,
+        const boost::shared_ptr<Expression> msg_in
+    );
+
+    std::string typestring() const;
 };
 
 class Function : public AbstractFunction
@@ -41,11 +89,56 @@ class Function : public AbstractFunction
 public:
  AssignmentList definition_arguments;
 
- Expression *expr;
+    // The pre- and post-conditions.
+    //
+    // After construction of the object, preconditions will be at the front,
and
+    // postconditions will be at the back.
+    std::vector<condition> conds;
+
+    // Iterator delimiting pre- and post-conditions.
+    //
+    // [conds.begin(), precond_end): preconditions.
+    //
+    // [precond_end, conds.end()): postconditions.
+    std::vector<condition>::const_iterator precond_end;
 
- Function() { }
- virtual ~Function();
+    // TODO: Should be made into a std::unique_ptr at the earliest
opportunity.
+    const boost::shared_ptr<Expression> body;
+
+    const std::string name;
+
+    const Module &enclosing;
+
+    // Create a function.
+    //
+    // Parameters:
+    //
+    //   formal_args_in: The list of formal arguments for the function,
+    //     including any default values.
+    //
+    //   conditions_in: The list of pre- and post- conditions.
+    //
+    //   body_in: The function definition.
+    //
+    //   name_in: The base name of the function.
+    //
+    //   enclosing_in: The module in which this function is defined.
+    //
+    // The precondition, postcondition, and body are all evaluated in an
inner
+    // context that has bound the formal parameter list to their concrete
+    // values.
+    Function(
+        const AssignmentList                &formal_args_in,
+              std::vector<condition>        conditions_in,
+        const boost::shared_ptr<Expression> body_in,
+        const std::string                   &name_in,
+        const Module                        &enclosing_in
+    );
+
+    virtual ~Function();
 
  virtual Value evaluate(const Context *ctx, const EvalContext *evalctx)
const;
  virtual std::string dump(const std::string &indent, const std::string
&name) const;
+
+    virtual std::string fullname() const;
 };
diff --git a/src/lexer.l b/src/lexer.l
index c873be8..815c605 100644
--- a/src/lexer.l
+++ b/src/lexer.l
@@ -28,6 +28,7 @@
 
 #include <glib.h>
 #include "typedefs.h"
+#include "function.h" // For condition.
 #include "handle_dep.h"
 #include "printutils.h"
 #include "parsersettings.h"
@@ -158,6 +159,9 @@ use[ \t\r\n>]*"<" { BEGIN(cond_use); }
 "false" return TOK_FALSE;
 "undef" return TOK_UNDEF;
 
+"pre"       return TOK_PRECONDITION;
+"post"      return TOK_POSTCONDITION;
+
 %{/*
  U+00A0 (UTF-8 encoded: C2A0) is no-break space. We support it since Qt's
QTextEdit
  automatically converts these to spaces and we want to be able to process the
same
diff --git a/src/module.cc b/src/module.cc
index 95224be..3da0e38 100644
--- a/src/module.cc
+++ b/src/module.cc
@@ -165,6 +165,16 @@ std::vector<AbstractNode*>
IfElseModuleInstantiation::instantiateElseChildren(co
  return this->else_scope.instantiateChildren(evalctx);
 }
 
+std::string AbstractModule::fullname() const
+{
+    // Default implementation just dumps the object address to ensure that
the
+    // fullname is unique.
+
+    std::stringstream namestream;
+    namestream << this;
+    return namestream.str();
+}
+
 std::deque<std::string> Module::module_stack;
 
 Module::~Module()
@@ -234,6 +244,24 @@ std::string Module::dump(const std::string &indent, const
std::string &name) con
  return dump.str();
 }
 
+std::string Module::fullname() const
+{
+    using std::deque;
+    using std::string;
+    using std::stringstream;
+
+    stringstream fullname;
+
+    for(deque<string>::const_iterator point = module_stack.begin();
+        point != module_stack.end();
+        ++point
+    ) {
+        fullname << "::" << (*point);
+    }
+
+    return fullname.str();
+}
+
 void FileModule::registerUse(const std::string path) {
  std::string extraw = boosty::extension_str(fs::path(path));
  std::string ext = boost::algorithm::to_lower_copy(extraw);
diff --git a/src/module.h b/src/module.h
index 7962ec2..46a8512 100644
--- a/src/module.h
+++ b/src/module.h
@@ -61,17 +61,20 @@ public:
 class AbstractModule
 {
 private:
-        const Feature *feature;
+    const Feature *feature;
 public:
-        AbstractModule() : feature(NULL) {}
-        AbstractModule(const Feature& feature) : feature(&feature) {}
+    AbstractModule() : feature(NULL) {}
+    AbstractModule(const Feature& feature) : feature(&feature) {}
  virtual ~AbstractModule();
-        virtual bool is_experimental() const { return feature != NULL; }
-        virtual bool is_enabled() const { return (feature == NULL) ||
feature->is_enabled(); }
+    virtual bool is_experimental() const { return feature != NULL; }
+    virtual bool is_enabled() const { return (feature == NULL) || feature-
>is_enabled(); }
  virtual class AbstractNode *instantiate(const Context *ctx, const
ModuleInstantiation *inst, const class EvalContext *evalctx = NULL) const;
  virtual std::string dump(const std::string &indent, const std::string
&name) const;
-        virtual double lookup_double_variable_with_default(Context &c,
std::string variable, double def) const;
-        virtual std::string lookup_string_variable_with_default(Context &c,
std::string variable, std::string def) const;
+    virtual double lookup_double_variable_with_default(Context &c,
std::string variable, double def) const;
+    virtual std::string lookup_string_variable_with_default(Context &c,
std::string variable, std::string def) const;
+
+    // Get the fully-qualified name of this module.
+    virtual std::string fullname() const;
 };
 
 class Module : public AbstractModule
@@ -86,6 +89,13 @@ public:
  static const std::string& stack_element(int n) { return module_stack[n];
};
  static int stack_size() { return module_stack.size(); };
 
+    // Get the fully-qualified name of this module.
+    //
+    // Get the fully-qualified name of this module (a unique string that
+    // represents the full scope of this module.  The name takes the form
+    // "::a::b::c", where c is nested inside b, which is nested inside a.
+    virtual std::string fullname() const;
+
  AssignmentList definition_arguments;
 
  LocalScope scope;
diff --git a/src/parser.y b/src/parser.y
index 6c0d537..08eb5ae 100644
--- a/src/parser.y
+++ b/src/parser.y
@@ -61,6 +61,8 @@ int lexerlex(void);
 std::stack<LocalScope *> scope_stack;
 FileModule *rootmodule;
 
+std::stack<Module*> module_stack;
+
 extern void lexerdestroy();
 extern FILE *lexerin;
 extern const char *parser_input_buffer;
@@ -78,6 +80,9 @@ std::string parser_source_path;
   class IfElseModuleInstantiation *ifelse;
   Assignment *arg;
   AssignmentList *args;
+
+  condition              *cond_type;
+  std::vector<condition> *conds_type;
 }
 
 %token TOK_ERROR
@@ -89,6 +94,9 @@ std::string parser_source_path;
 %token TOK_FOR
 %token TOK_LET
 
+%token TOK_PRECONDITION pre
+%token TOK_POSTCONDITION post
+
 %token <text> TOK_ID
 %token <text> TOK_STRING
 %token <text> TOK_USE
@@ -120,6 +128,9 @@ std::string parser_source_path;
 %type <expr> list_comprehension_elements
 %type <expr> list_comprehension_elements_or_expr
 
+%type <cond_type> condition
+%type <conds_type> conditions
+
 %type <inst> module_instantiation
 %type <ifelse> if_statement
 %type <ifelse> ifelse_statement
@@ -157,25 +168,79 @@ statement:
                 newmodule->definition_arguments = *$4;
                 scope_stack.top()->modules[$2] = newmodule;
                 scope_stack.push(&newmodule->scope);
+
+                module_stack.push(newmodule);
+
                 free($2);
                 delete $4;
             }
           statement
             {
                 scope_stack.pop();
-            }
-        | TOK_FUNCTION TOK_ID '(' arguments_decl optional_commas ')' '=' expr
-            {
-                Function *func = new Function();
-                func->definition_arguments = *$4;
-                func->expr = $8;
+                module_stack.pop();
+            }
+        | TOK_FUNCTION TOK_ID
+          '(' arguments_decl optional_commas ')'
+          conditions
+          '=' expr
+            {
+                Function *func = new Function(
+                    *$4,                               // Formal arguments
+                    *$7,                               // Conditions
+                    boost::shared_ptr<Expression>($9), // Body
+                    (std::string($2)),                 // Base name
+                    *(module_stack.top())              // Enclosing module.
+                );
+
+                // Record the function in this scope.
                 scope_stack.top()->functions[$2] = func;
+
                 free($2);
                 delete $4;
+                delete $7;
             }
           ';'
         ;
 
+conditions:
+      /* Nothing */ {
+          $$ = new std::vector<condition>();
+      }
+    | condition conditions {
+          $2->push_back(*$1);
+          delete $1;
+          $$ = $2;
+      }
+
+condition:
+      TOK_PRECONDITION '(' expr ')' {
+          $$ = new condition(
+              condition::precondition,
+              boost::shared_ptr<Expression>($3)
+          );
+      }
+    | TOK_PRECONDITION '(' expr ',' expr ')' {
+          $$ = new condition(
+              condition::precondition,
+              boost::shared_ptr<Expression>($3),
+              boost::shared_ptr<Expression>($5)
+          );
+      }
+    | TOK_POSTCONDITION '(' expr ')' {
+          $$ = new condition(
+              condition::postcondition,
+              boost::shared_ptr<Expression>($3)
+          );
+      }
+    | TOK_POSTCONDITION '(' expr ',' expr ')' {
+          $$ = new condition(
+              condition::postcondition,
+              boost::shared_ptr<Expression>($3),
+              boost::shared_ptr<Expression>($5)
+          );
+      }
+    ;
+
 inner_input:
           /* empty */
         | statement inner_input
@@ -606,6 +671,7 @@ FileModule *parse(const char *text, const char *path, int
debug)
   rootmodule = new FileModule();
   rootmodule->setModulePath(path);
   scope_stack.push(&rootmodule->scope);
+  module_stack.push(rootmodule);
   //        PRINTB_NOCACHE("New module: %s %p", "root" % rootmodule);
 
   parserdebug = debug;
--
2.0.2


_______________________________________________
OpenSCAD mailing list
[hidden email]
http://rocklinux.net/mailman/listinfo/openscad
http://openscad.org - https://flattr.com/thing/121566

signature.asc (484 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Add function pre- and post-conditions.

Felipe Sanches

Does it work well with a scad script that declares a module or function called pre or called post ?

Can such a function called pre or post be safely used as part of a precondition or as parte of a post condition statement ?

Em 27/08/2014 04:42, "Taahir Ahmed" <[hidden email]> escreveu:
Our previous discussion about asserts and testing in general led me to
draw up this patch, which I thought I would send around for comment
before pursuing further.

This message should be directly usable by `git am`.  If it's preferable,
I can instead make a pull request on github.

Taahir Ahmed

>From 5595c58f57e39b848c504f2cafc2643241588dbe Mon Sep 17 00:00:00 2001
From: Taahir Ahmed <[hidden email]>
Date: Tue, 26 Aug 2014 23:25:01 -0500
Subject: [PATCH] Add function pre- and post-conditions.

Change description
==================

This change adds optional preconditions and postconditions to OpenSCAD
functions.  Multiple preconditions and postconditions may be attached to
a given function.  Optional messages can be attached to each
precondition and postcondition, to aid the user in debugging.

Usage
-----

Conditions are added to functions after the close of the argument list
and before the defining `=`:

```
function f(x)
    pre(x > 0, "The argument must be positive.")
    post(__retval > 0, "The return value must be positive.")
    = x;
```

The syntax is whitespace-insensitive, and as many preconditions and
postconditions may be attached as are needed, in any order.
Preconditions are declared using `pre(expr)` or `pre(expr,expr)`.
Postconditions are declared using `post(expr)` or `post(expr,expr)`.

  * The first argument is the condition.  It can be any single
    expression.  If it is true when coerced to a boolean, then the
    precondition passes.  Otherwise, the precondition fails.

  * The optional second argument is the message.  It can be any single
    expression.  Its value is coerced to a string, which then forms
    part of the error message.

Semantics
---------

All the involved expressions have access to the full lexical scope in
which the function is defined, including the formal parameters.  In
addition, postconditions have access to the return value of the function
as the special symbol `__retval`.

Function evaluation proceeds in the following order:

  - Preconditions are evaluated in the order in which they appear.
    If any precondition fails, then no subsequent precondition is
    evaluated, and the function returns `undef`.

  - If all preconditions succeed, then the body of the function is
    evaluated.

  - Postconditions are evaluated in the order in which they appear.  If
    any postcondition fails, then no subsequent postcondition is
    evaluated, and the function returns `undef`.

  - If all postconditions succeed, the function returns the value
    produced by the body.

Deficiencies and gotchas
========================

  - The behavior of `value.toBool()` leads to some unexpected behavior
    --- for example, the following code will not trigger a precondition
    failure:

    ```
    function f(x) pre(x) = x;
    z = f(0/0);
    ```

    Internally, `toBool` checks that `x != 0`, and `NaN != 0`, so the
    precondition passes.  I figured it was better to stick with uniform
    semantics, even if they are a little strange.

  - Ideally, condition failure would halt compilation.  This currently
    does not happen.  Compilation continues as if the function returned
    `undef`.

  - There are no tests associated with this feature yet.  The existing
    testing framework seems to be set up around testing geometry output,
    not control flow / console output.
---
 src/dxfdim.cc  |   4 +-
 src/func.cc    | 269
+++++++++++++++++++++++++++++++++++++++++++++++++--------
 src/function.h | 115 +++++++++++++++++++++---
 src/lexer.l    |   4 +
 src/module.cc  |  28 ++++++
 src/module.h   |  24 +++--
 src/parser.y   |  78 +++++++++++++++--
 7 files changed, 459 insertions(+), 63 deletions(-)

diff --git a/src/dxfdim.cc b/src/dxfdim.cc
index 8f55a2f..898f21a 100644
--- a/src/dxfdim.cc
+++ b/src/dxfdim.cc
@@ -215,6 +215,6 @@ Value builtin_dxf_cross(const Context *ctx, const
EvalContext *evalctx)

 void initialize_builtin_dxf_dim()
 {
-       Builtins::init("dxf_dim", new BuiltinFunction(&builtin_dxf_dim));
-       Builtins::init("dxf_cross", new BuiltinFunction(&builtin_dxf_cross));
+       Builtins::init("dxf_dim", new BuiltinFunction(&builtin_dxf_dim,
"dxf_dim"));
+       Builtins::init("dxf_cross", new BuiltinFunction(&builtin_dxf_cross,
"dxf_cross"));
 }
diff --git a/src/func.cc b/src/func.cc
index f26f44c..53c359b 100644
--- a/src/func.cc
+++ b/src/func.cc
@@ -65,10 +65,30 @@ int process_id = getpid();
 boost::mt19937 deterministic_rng;
 boost::mt19937 lessdeterministic_rng( std::time(0) + process_id );

+AbstractFunction::AbstractFunction()
+    : feature(NULL)
+{
+}
+
+AbstractFunction::AbstractFunction(const Feature &feature_in)
+    : feature(&feature_in)
+{
+}
+
 AbstractFunction::~AbstractFunction()
 {
 }

+bool AbstractFunction::is_experimental() const
+{
+    return feature != NULL;
+}
+
+bool AbstractFunction::is_enabled() const
+{
+    return (feature == NULL) || feature->is_enabled();
+}
+
 Value AbstractFunction::evaluate(const Context*, const EvalContext *evalctx)
const
 {
        (void)evalctx; // unusued parameter
@@ -82,17 +102,159 @@ std::string AbstractFunction::dump(const std::string
&indent, const std::string
        return dump.str();
 }

+condition::condition(
+    const type_tag type_in,
+    boost::shared_ptr<Expression> expr_in
+)
+    : type(type_in),
+      expr(expr_in)
+{
+}
+
+condition::condition(
+    const type_tag type_in,
+    boost::shared_ptr<Expression> expr_in,
+    boost::shared_ptr<Expression> msg_in
+)
+    : type(type_in),
+      expr(expr_in),
+      msg(msg_in)
+{
+}
+
+std::string condition::typestring() const
+{
+    switch(type) {
+    case condition::precondition:
+        return std::string("precondition");
+    case condition::postcondition:
+        return std::string("postcondition");
+    default:
+        return std::string("");
+    }
+}
+
+namespace
+{
+
+template<condition::type_tag type>
+struct condition_matcher
+{
+    bool operator()(const condition &cond)
+    {
+        return cond.type == type;
+    }
+};
+
+}
+
+Function::Function(
+    const AssignmentList                &formal_args_in,
+          std::vector<condition>         conditions_in,
+    const boost::shared_ptr<Expression>  body_in,
+    const std::string                   &name_in,
+    const Module                        &enclosing_in
+)
+    : definition_arguments(formal_args_in),
+      conds(conditions_in),
+      // Split up pre- and post- so we don't have to do it every time.
+      precond_end(
+          std::stable_partition(
+              conds.begin(),
+              conds.end(),
+              (condition_matcher<condition::precondition>())
+          )
+      ),
+      body(body_in),
+      name(name_in),
+      enclosing(enclosing_in)
+{
+}
+
 Function::~Function()
 {
-       delete expr;
+}
+
+namespace
+{
+
+// Run a pre- or post-condition in the given context.
+struct condition_runner
+{
+    condition_runner(const Context &ctx_in, const std::string &fullname_in)
+        : ctx(ctx_in),
+          fullname(fullname_in),
+          all_passed(true)
+    {
+    }
+
+    operator bool () const
+    {
+        return all_passed;
+    }
+
+    void operator()(const condition &cond)
+    {
+        // Break out early if an earlier condition has failed.
+        if(! all_passed) {
+            return;
+        }
+
+        const Value condition_value = cond.expr->evaluate(&ctx);
+        if(!condition_value.toBool()) {
+            all_passed = false;
+
+            std::stringstream msg;
+            msg << "Error: " << cond.typestring() << " violation in "
+                << fullname;
+
+            if(cond.msg) {
+                const Value usermsg = cond.msg->evaluate(&ctx);
+                msg << ": " << usermsg.toString();
+            }
+
+            PRINT(msg.str().c_str());
+        }
+    }
+
+    const Context &ctx;
+    const std::string fullname;
+    bool all_passed;
+};
+
 }

 Value Function::evaluate(const Context *ctx, const EvalContext *evalctx)
const
 {
-       if (!expr) return Value();
-       Context c(ctx);
-       c.setVariables(definition_arguments, evalctx);
-       return expr->evaluate(&c);
+    // definition_arguments contains any default parameters values.
+    // setVariables is an ad-hoc function that sets up inner appropriately
for
+    // the body of a function call.
+    Context inner(ctx);
+       inner.setVariables(definition_arguments, evalctx);
+
+    condition_runner precond_runner(inner, fullname());
+    std::for_each(conds.begin(), precond_end, precond_runner);
+    if(!precond_runner) {
+        // TODO: make a precondition failure halt compilation.
+        return (Value());
+    }
+
+    // Evaluate the body.
+    const Value produced = body ? (*body).evaluate(&inner) : (Value());
+
+    // Create a context derived from the inner context, with the variable
+    // __retval set to the function return value.
+    Context post_context(&inner);
+    post_context.set_variable("__retval", produced);
+
+    condition_runner postcond_runner(post_context, fullname());
+    std::for_each(precond_end, conds.end(), postcond_runner);
+    if(!postcond_runner) {
+        // TODO: make a postcondition failure halt compilation.
+        return (Value());
+    }
+
+    return produced;
 }

 std::string Function::dump(const std::string &indent, const std::string
&name) const
@@ -105,10 +267,38 @@ std::string Function::dump(const std::string &indent,
const std::string &name) c
                dump << arg.first;
                if (arg.second) dump << " = " << *arg.second;
        }
-       dump << ") = " << *expr << ";\n";
+       dump << ") = " << body << ";\n";
        return dump.str();
 }

+std::string Function::fullname() const
+{
+    std::stringstream namestream;
+    namestream << enclosing.fullname() << "::" << name;
+
+    return namestream.str();
+}
+
+BuiltinFunction::BuiltinFunction(
+    eval_func_t eval_func_in,
+    const std::string name_in
+)
+    : eval_func(eval_func_in),
+      name(name_in)
+{
+}
+
+BuiltinFunction::BuiltinFunction(
+    eval_func_t       eval_func_in,
+    const Feature     &feature_in,
+    const std::string name_in
+)
+    : AbstractFunction(feature_in),
+      eval_func(eval_func_in),
+      name(name_in)
+{
+}
+
 BuiltinFunction::~BuiltinFunction()
 {
 }
@@ -125,6 +315,11 @@ std::string BuiltinFunction::dump(const std::string
&indent, const std::string &
        return dump.str();
 }

+std::string BuiltinFunction::fullname() const
+{
+    return "builtin::" + name;
+}
+
 static inline double deg2rad(double x)
 {
        return x * M_PI / 180.0;
@@ -917,35 +1112,35 @@ Value builtin_cross(const Context *, const EvalContext
*evalctx)

 void register_builtin_functions()
 {
-       Builtins::init("abs", new BuiltinFunction(&builtin_abs));
-       Builtins::init("sign", new BuiltinFunction(&builtin_sign));
-       Builtins::init("rands", new BuiltinFunction(&builtin_rands));
-       Builtins::init("min", new BuiltinFunction(&builtin_min));
-       Builtins::init("max", new BuiltinFunction(&builtin_max));
-       Builtins::init("sin", new BuiltinFunction(&builtin_sin));
-       Builtins::init("cos", new BuiltinFunction(&builtin_cos));
-       Builtins::init("asin", new BuiltinFunction(&builtin_asin));
-       Builtins::init("acos", new BuiltinFunction(&builtin_acos));
-       Builtins::init("tan", new BuiltinFunction(&builtin_tan));
-       Builtins::init("atan", new BuiltinFunction(&builtin_atan));
-       Builtins::init("atan2", new BuiltinFunction(&builtin_atan2));
-       Builtins::init("round", new BuiltinFunction(&builtin_round));
-       Builtins::init("ceil", new BuiltinFunction(&builtin_ceil));
-       Builtins::init("floor", new BuiltinFunction(&builtin_floor));
-       Builtins::init("pow", new BuiltinFunction(&builtin_pow));
-       Builtins::init("sqrt", new BuiltinFunction(&builtin_sqrt));
-       Builtins::init("exp", new BuiltinFunction(&builtin_exp));
-       Builtins::init("len", new BuiltinFunction(&builtin_length));
-       Builtins::init("log", new BuiltinFunction(&builtin_log));
-       Builtins::init("ln", new BuiltinFunction(&builtin_ln));
-       Builtins::init("str", new BuiltinFunction(&builtin_str));
-       Builtins::init("chr", new BuiltinFunction(&builtin_chr));
-       Builtins::init("concat", new BuiltinFunction(&builtin_concat));
-       Builtins::init("lookup", new BuiltinFunction(&builtin_lookup));
-       Builtins::init("search", new BuiltinFunction(&builtin_search));
-       Builtins::init("version", new BuiltinFunction(&builtin_version));
-       Builtins::init("version_num", new BuiltinFunction(&builtin_version_num));
-       Builtins::init("norm", new BuiltinFunction(&builtin_norm));
-       Builtins::init("cross", new BuiltinFunction(&builtin_cross));
-       Builtins::init("parent_module", new
BuiltinFunction(&builtin_parent_module));
+       Builtins::init("abs", new BuiltinFunction(&builtin_abs, "abs"));
+       Builtins::init("sign", new BuiltinFunction(&builtin_sign, "sign"));
+       Builtins::init("rands", new BuiltinFunction(&builtin_rands, "rands"));
+       Builtins::init("min", new BuiltinFunction(&builtin_min, "min"));
+       Builtins::init("max", new BuiltinFunction(&builtin_max, "max"));
+       Builtins::init("sin", new BuiltinFunction(&builtin_sin, "sin"));
+       Builtins::init("cos", new BuiltinFunction(&builtin_cos, "cos"));
+       Builtins::init("asin", new BuiltinFunction(&builtin_asin, "asin"));
+       Builtins::init("acos", new BuiltinFunction(&builtin_acos, "acos"));
+       Builtins::init("tan", new BuiltinFunction(&builtin_tan, "tan"));
+       Builtins::init("atan", new BuiltinFunction(&builtin_atan, "atan"));
+       Builtins::init("atan2", new BuiltinFunction(&builtin_atan2, "atan2"));
+       Builtins::init("round", new BuiltinFunction(&builtin_round, "round"));
+       Builtins::init("ceil", new BuiltinFunction(&builtin_ceil, "ceil"));
+       Builtins::init("floor", new BuiltinFunction(&builtin_floor, "floor"));
+       Builtins::init("pow", new BuiltinFunction(&builtin_pow, "pow"));
+       Builtins::init("sqrt", new BuiltinFunction(&builtin_sqrt, "sqrt"));
+       Builtins::init("exp", new BuiltinFunction(&builtin_exp, "exp"));
+       Builtins::init("len", new BuiltinFunction(&builtin_length, "length"));
+       Builtins::init("log", new BuiltinFunction(&builtin_log, "log"));
+       Builtins::init("ln", new BuiltinFunction(&builtin_ln, "ln"));
+       Builtins::init("str", new BuiltinFunction(&builtin_str, "str"));
+       Builtins::init("chr", new BuiltinFunction(&builtin_chr, "chr"));
+       Builtins::init("concat", new BuiltinFunction(&builtin_concat, "concat"));
+       Builtins::init("lookup", new BuiltinFunction(&builtin_lookup, "lookup"));
+       Builtins::init("search", new BuiltinFunction(&builtin_search, "search"));
+       Builtins::init("version", new BuiltinFunction(&builtin_version,
"version"));
+       Builtins::init("version_num", new BuiltinFunction(&builtin_version_num,
"version_num"));
+       Builtins::init("norm", new BuiltinFunction(&builtin_norm, "norm"));
+       Builtins::init("cross", new BuiltinFunction(&builtin_cross, "cross"));
+       Builtins::init("parent_module", new
BuiltinFunction(&builtin_parent_module, "parent_module"));
 }
diff --git a/src/function.h b/src/function.h
index d8e3ad7..e47c01d 100644
--- a/src/function.h
+++ b/src/function.h
@@ -1,25 +1,29 @@
 #pragma once

+#include "expression.h"
+#include "module.h"
 #include "value.h"
 #include "typedefs.h"
 #include "feature.h"

 #include <string>
+#include <utility>
 #include <vector>

-
 class AbstractFunction
 {
 private:
-        const Feature *feature;
+    const Feature *feature;
 public:
-        AbstractFunction() : feature(NULL) {}
-        AbstractFunction(const Feature& feature) : feature(&feature) {}
+    AbstractFunction();
+    AbstractFunction(const Feature& feature);
        virtual ~AbstractFunction();
-        virtual bool is_experimental() const { return feature != NULL; }
-        virtual bool is_enabled() const { return (feature == NULL) ||
feature->is_enabled(); }
+    virtual bool is_experimental() const;
+    virtual bool is_enabled() const;
        virtual Value evaluate(const class Context *ctx, const class EvalContext
*evalctx) const;
        virtual std::string dump(const std::string &indent, const std::string
&name) const;
+
+    virtual std::string fullname() const = 0;
 };

 class BuiltinFunction : public AbstractFunction
@@ -28,12 +32,56 @@ public:
        typedef Value (*eval_func_t)(const Context *ctx, const EvalContext
*evalctx);
        eval_func_t eval_func;

-       BuiltinFunction(eval_func_t f) : eval_func(f) { }
-       BuiltinFunction(eval_func_t f, const Feature& feature) :
AbstractFunction(feature), eval_func(f) { }
+       BuiltinFunction(eval_func_t f, const std::string name_in);
+
+       BuiltinFunction(
+        eval_func_t f,
+        const Feature& feature,
+        const std::string name_in
+    );
+
        virtual ~BuiltinFunction();

        virtual Value evaluate(const Context *ctx, const EvalContext *evalctx)
const;
        virtual std::string dump(const std::string &indent, const std::string
&name) const;
+
+    virtual std::string fullname() const;
+private:
+    const std::string name;
+};
+
+// Pre- and post-condition container.
+//
+// Bundles an expression together with the message that should be printed if
the
+// expression is violated.  Not structured for inheritance.
+class condition
+{
+public:
+    enum type_tag
+    {
+        precondition,
+        postcondition
+    };
+
+    type_tag type;
+
+    // TODO: These members should be made into std::unique_ptrs at the
earliest
+    // opportunity.
+    boost::shared_ptr<Expression> expr;
+    boost::shared_ptr<Expression> msg;
+
+    condition(
+        const type_tag type_in,
+        const boost::shared_ptr<Expression> expr_in
+    );
+
+    condition(
+        const type_tag type_in,
+        const boost::shared_ptr<Expression> expr_in,
+        const boost::shared_ptr<Expression> msg_in
+    );
+
+    std::string typestring() const;
 };

 class Function : public AbstractFunction
@@ -41,11 +89,56 @@ class Function : public AbstractFunction
 public:
        AssignmentList definition_arguments;

-       Expression *expr;
+    // The pre- and post-conditions.
+    //
+    // After construction of the object, preconditions will be at the front,
and
+    // postconditions will be at the back.
+    std::vector<condition> conds;
+
+    // Iterator delimiting pre- and post-conditions.
+    //
+    // [conds.begin(), precond_end): preconditions.
+    //
+    // [precond_end, conds.end()): postconditions.
+    std::vector<condition>::const_iterator precond_end;

-       Function() { }
-       virtual ~Function();
+    // TODO: Should be made into a std::unique_ptr at the earliest
opportunity.
+    const boost::shared_ptr<Expression> body;
+
+    const std::string name;
+
+    const Module &enclosing;
+
+    // Create a function.
+    //
+    // Parameters:
+    //
+    //   formal_args_in: The list of formal arguments for the function,
+    //     including any default values.
+    //
+    //   conditions_in: The list of pre- and post- conditions.
+    //
+    //   body_in: The function definition.
+    //
+    //   name_in: The base name of the function.
+    //
+    //   enclosing_in: The module in which this function is defined.
+    //
+    // The precondition, postcondition, and body are all evaluated in an
inner
+    // context that has bound the formal parameter list to their concrete
+    // values.
+    Function(
+        const AssignmentList                &formal_args_in,
+              std::vector<condition>        conditions_in,
+        const boost::shared_ptr<Expression> body_in,
+        const std::string                   &name_in,
+        const Module                        &enclosing_in
+    );
+
+    virtual ~Function();

        virtual Value evaluate(const Context *ctx, const EvalContext *evalctx)
const;
        virtual std::string dump(const std::string &indent, const std::string
&name) const;
+
+    virtual std::string fullname() const;
 };
diff --git a/src/lexer.l b/src/lexer.l
index c873be8..815c605 100644
--- a/src/lexer.l
+++ b/src/lexer.l
@@ -28,6 +28,7 @@

 #include <glib.h>
 #include "typedefs.h"
+#include "function.h" // For condition.
 #include "handle_dep.h"
 #include "printutils.h"
 #include "parsersettings.h"
@@ -158,6 +159,9 @@ use[ \t\r\n>]*"<"   { BEGIN(cond_use); }
 "false"                return TOK_FALSE;
 "undef"                return TOK_UNDEF;

+"pre"       return TOK_PRECONDITION;
+"post"      return TOK_POSTCONDITION;
+
 %{/*
  U+00A0 (UTF-8 encoded: C2A0) is no-break space. We support it since Qt's
QTextEdit
  automatically converts these to spaces and we want to be able to process the
same
diff --git a/src/module.cc b/src/module.cc
index 95224be..3da0e38 100644
--- a/src/module.cc
+++ b/src/module.cc
@@ -165,6 +165,16 @@ std::vector<AbstractNode*>
IfElseModuleInstantiation::instantiateElseChildren(co
        return this->else_scope.instantiateChildren(evalctx);
 }

+std::string AbstractModule::fullname() const
+{
+    // Default implementation just dumps the object address to ensure that
the
+    // fullname is unique.
+
+    std::stringstream namestream;
+    namestream << this;
+    return namestream.str();
+}
+
 std::deque<std::string> Module::module_stack;

 Module::~Module()
@@ -234,6 +244,24 @@ std::string Module::dump(const std::string &indent, const
std::string &name) con
        return dump.str();
 }

+std::string Module::fullname() const
+{
+    using std::deque;
+    using std::string;
+    using std::stringstream;
+
+    stringstream fullname;
+
+    for(deque<string>::const_iterator point = module_stack.begin();
+        point != module_stack.end();
+        ++point
+    ) {
+        fullname << "::" << (*point);
+    }
+
+    return fullname.str();
+}
+
 void FileModule::registerUse(const std::string path) {
        std::string extraw = boosty::extension_str(fs::path(path));
        std::string ext = boost::algorithm::to_lower_copy(extraw);
diff --git a/src/module.h b/src/module.h
index 7962ec2..46a8512 100644
--- a/src/module.h
+++ b/src/module.h
@@ -61,17 +61,20 @@ public:
 class AbstractModule
 {
 private:
-        const Feature *feature;
+    const Feature *feature;
 public:
-        AbstractModule() : feature(NULL) {}
-        AbstractModule(const Feature& feature) : feature(&feature) {}
+    AbstractModule() : feature(NULL) {}
+    AbstractModule(const Feature& feature) : feature(&feature) {}
        virtual ~AbstractModule();
-        virtual bool is_experimental() const { return feature != NULL; }
-        virtual bool is_enabled() const { return (feature == NULL) ||
feature->is_enabled(); }
+    virtual bool is_experimental() const { return feature != NULL; }
+    virtual bool is_enabled() const { return (feature == NULL) || feature-
>is_enabled(); }
        virtual class AbstractNode *instantiate(const Context *ctx, const
ModuleInstantiation *inst, const class EvalContext *evalctx = NULL) const;
        virtual std::string dump(const std::string &indent, const std::string
&name) const;
-        virtual double lookup_double_variable_with_default(Context &c,
std::string variable, double def) const;
-        virtual std::string lookup_string_variable_with_default(Context &c,
std::string variable, std::string def) const;
+    virtual double lookup_double_variable_with_default(Context &c,
std::string variable, double def) const;
+    virtual std::string lookup_string_variable_with_default(Context &c,
std::string variable, std::string def) const;
+
+    // Get the fully-qualified name of this module.
+    virtual std::string fullname() const;
 };

 class Module : public AbstractModule
@@ -86,6 +89,13 @@ public:
        static const std::string& stack_element(int n) { return module_stack[n];
};
        static int stack_size() { return module_stack.size(); };

+    // Get the fully-qualified name of this module.
+    //
+    // Get the fully-qualified name of this module (a unique string that
+    // represents the full scope of this module.  The name takes the form
+    // "::a::b::c", where c is nested inside b, which is nested inside a.
+    virtual std::string fullname() const;
+
        AssignmentList definition_arguments;

        LocalScope scope;
diff --git a/src/parser.y b/src/parser.y
index 6c0d537..08eb5ae 100644
--- a/src/parser.y
+++ b/src/parser.y
@@ -61,6 +61,8 @@ int lexerlex(void);
 std::stack<LocalScope *> scope_stack;
 FileModule *rootmodule;

+std::stack<Module*> module_stack;
+
 extern void lexerdestroy();
 extern FILE *lexerin;
 extern const char *parser_input_buffer;
@@ -78,6 +80,9 @@ std::string parser_source_path;
   class IfElseModuleInstantiation *ifelse;
   Assignment *arg;
   AssignmentList *args;
+
+  condition              *cond_type;
+  std::vector<condition> *conds_type;
 }

 %token TOK_ERROR
@@ -89,6 +94,9 @@ std::string parser_source_path;
 %token TOK_FOR
 %token TOK_LET

+%token TOK_PRECONDITION pre
+%token TOK_POSTCONDITION post
+
 %token <text> TOK_ID
 %token <text> TOK_STRING
 %token <text> TOK_USE
@@ -120,6 +128,9 @@ std::string parser_source_path;
 %type <expr> list_comprehension_elements
 %type <expr> list_comprehension_elements_or_expr

+%type <cond_type> condition
+%type <conds_type> conditions
+
 %type <inst> module_instantiation
 %type <ifelse> if_statement
 %type <ifelse> ifelse_statement
@@ -157,25 +168,79 @@ statement:
                 newmodule->definition_arguments = *$4;
                 scope_stack.top()->modules[$2] = newmodule;
                 scope_stack.push(&newmodule->scope);
+
+                module_stack.push(newmodule);
+
                 free($2);
                 delete $4;
             }
           statement
             {
                 scope_stack.pop();
-            }
-        | TOK_FUNCTION TOK_ID '(' arguments_decl optional_commas ')' '=' expr
-            {
-                Function *func = new Function();
-                func->definition_arguments = *$4;
-                func->expr = $8;
+                module_stack.pop();
+            }
+        | TOK_FUNCTION TOK_ID
+          '(' arguments_decl optional_commas ')'
+          conditions
+          '=' expr
+            {
+                Function *func = new Function(
+                    *$4,                               // Formal arguments
+                    *$7,                               // Conditions
+                    boost::shared_ptr<Expression>($9), // Body
+                    (std::string($2)),                 // Base name
+                    *(module_stack.top())              // Enclosing module.
+                );
+
+                // Record the function in this scope.
                 scope_stack.top()->functions[$2] = func;
+
                 free($2);
                 delete $4;
+                delete $7;
             }
           ';'
         ;

+conditions:
+      /* Nothing */ {
+          $$ = new std::vector<condition>();
+      }
+    | condition conditions {
+          $2->push_back(*$1);
+          delete $1;
+          $$ = $2;
+      }
+
+condition:
+      TOK_PRECONDITION '(' expr ')' {
+          $$ = new condition(
+              condition::precondition,
+              boost::shared_ptr<Expression>($3)
+          );
+      }
+    | TOK_PRECONDITION '(' expr ',' expr ')' {
+          $$ = new condition(
+              condition::precondition,
+              boost::shared_ptr<Expression>($3),
+              boost::shared_ptr<Expression>($5)
+          );
+      }
+    | TOK_POSTCONDITION '(' expr ')' {
+          $$ = new condition(
+              condition::postcondition,
+              boost::shared_ptr<Expression>($3)
+          );
+      }
+    | TOK_POSTCONDITION '(' expr ',' expr ')' {
+          $$ = new condition(
+              condition::postcondition,
+              boost::shared_ptr<Expression>($3),
+              boost::shared_ptr<Expression>($5)
+          );
+      }
+    ;
+
 inner_input:
           /* empty */
         | statement inner_input
@@ -606,6 +671,7 @@ FileModule *parse(const char *text, const char *path, int
debug)
   rootmodule = new FileModule();
   rootmodule->setModulePath(path);
   scope_stack.push(&rootmodule->scope);
+  module_stack.push(rootmodule);
   //        PRINTB_NOCACHE("New module: %s %p", "root" % rootmodule);

   parserdebug = debug;
--
2.0.2


_______________________________________________
OpenSCAD mailing list
[hidden email]
http://rocklinux.net/mailman/listinfo/openscad
http://openscad.org - https://flattr.com/thing/121566

_______________________________________________
OpenSCAD mailing list
[hidden email]
http://rocklinux.net/mailman/listinfo/openscad
http://openscad.org - https://flattr.com/thing/121566
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Add function pre- and post-conditions.

Felipe Sanches

Also,  in terms of syntax, I would preffer to have the pre and post statements after the function declaration.

Perhaps something like:

function my_fun(x, y) = sqrt(x*x + y*y) {
     post(__retval >= 0, "the resulting value should be positive")
}

   

Em 27/08/2014 08:59, "Felipe Sanches" <[hidden email]> escreveu:

Does it work well with a scad script that declares a module or function called pre or called post ?

Can such a function called pre or post be safely used as part of a precondition or as parte of a post condition statement ?

Em 27/08/2014 04:42, "Taahir Ahmed" <[hidden email]> escreveu:
Our previous discussion about asserts and testing in general led me to
draw up this patch, which I thought I would send around for comment
before pursuing further.

This message should be directly usable by `git am`.  If it's preferable,
I can instead make a pull request on github.

Taahir Ahmed

>From 5595c58f57e39b848c504f2cafc2643241588dbe Mon Sep 17 00:00:00 2001
From: Taahir Ahmed <[hidden email]>
Date: Tue, 26 Aug 2014 23:25:01 -0500
Subject: [PATCH] Add function pre- and post-conditions.

Change description
==================

This change adds optional preconditions and postconditions to OpenSCAD
functions.  Multiple preconditions and postconditions may be attached to
a given function.  Optional messages can be attached to each
precondition and postcondition, to aid the user in debugging.

Usage
-----

Conditions are added to functions after the close of the argument list
and before the defining `=`:

```
function f(x)
    pre(x > 0, "The argument must be positive.")
    post(__retval > 0, "The return value must be positive.")
    = x;
```

The syntax is whitespace-insensitive, and as many preconditions and
postconditions may be attached as are needed, in any order.
Preconditions are declared using `pre(expr)` or `pre(expr,expr)`.
Postconditions are declared using `post(expr)` or `post(expr,expr)`.

  * The first argument is the condition.  It can be any single
    expression.  If it is true when coerced to a boolean, then the
    precondition passes.  Otherwise, the precondition fails.

  * The optional second argument is the message.  It can be any single
    expression.  Its value is coerced to a string, which then forms
    part of the error message.

Semantics
---------

All the involved expressions have access to the full lexical scope in
which the function is defined, including the formal parameters.  In
addition, postconditions have access to the return value of the function
as the special symbol `__retval`.

Function evaluation proceeds in the following order:

  - Preconditions are evaluated in the order in which they appear.
    If any precondition fails, then no subsequent precondition is
    evaluated, and the function returns `undef`.

  - If all preconditions succeed, then the body of the function is
    evaluated.

  - Postconditions are evaluated in the order in which they appear.  If
    any postcondition fails, then no subsequent postcondition is
    evaluated, and the function returns `undef`.

  - If all postconditions succeed, the function returns the value
    produced by the body.

Deficiencies and gotchas
========================

  - The behavior of `value.toBool()` leads to some unexpected behavior
    --- for example, the following code will not trigger a precondition
    failure:

    ```
    function f(x) pre(x) = x;
    z = f(0/0);
    ```

    Internally, `toBool` checks that `x != 0`, and `NaN != 0`, so the
    precondition passes.  I figured it was better to stick with uniform
    semantics, even if they are a little strange.

  - Ideally, condition failure would halt compilation.  This currently
    does not happen.  Compilation continues as if the function returned
    `undef`.

  - There are no tests associated with this feature yet.  The existing
    testing framework seems to be set up around testing geometry output,
    not control flow / console output.
---
 src/dxfdim.cc  |   4 +-
 src/func.cc    | 269
+++++++++++++++++++++++++++++++++++++++++++++++++--------
 src/function.h | 115 +++++++++++++++++++++---
 src/lexer.l    |   4 +
 src/module.cc  |  28 ++++++
 src/module.h   |  24 +++--
 src/parser.y   |  78 +++++++++++++++--
 7 files changed, 459 insertions(+), 63 deletions(-)

diff --git a/src/dxfdim.cc b/src/dxfdim.cc
index 8f55a2f..898f21a 100644
--- a/src/dxfdim.cc
+++ b/src/dxfdim.cc
@@ -215,6 +215,6 @@ Value builtin_dxf_cross(const Context *ctx, const
EvalContext *evalctx)

 void initialize_builtin_dxf_dim()
 {
-       Builtins::init("dxf_dim", new BuiltinFunction(&builtin_dxf_dim));
-       Builtins::init("dxf_cross", new BuiltinFunction(&builtin_dxf_cross));
+       Builtins::init("dxf_dim", new BuiltinFunction(&builtin_dxf_dim,
"dxf_dim"));
+       Builtins::init("dxf_cross", new BuiltinFunction(&builtin_dxf_cross,
"dxf_cross"));
 }
diff --git a/src/func.cc b/src/func.cc
index f26f44c..53c359b 100644
--- a/src/func.cc
+++ b/src/func.cc
@@ -65,10 +65,30 @@ int process_id = getpid();
 boost::mt19937 deterministic_rng;
 boost::mt19937 lessdeterministic_rng( std::time(0) + process_id );

+AbstractFunction::AbstractFunction()
+    : feature(NULL)
+{
+}
+
+AbstractFunction::AbstractFunction(const Feature &feature_in)
+    : feature(&feature_in)
+{
+}
+
 AbstractFunction::~AbstractFunction()
 {
 }

+bool AbstractFunction::is_experimental() const
+{
+    return feature != NULL;
+}
+
+bool AbstractFunction::is_enabled() const
+{
+    return (feature == NULL) || feature->is_enabled();
+}
+
 Value AbstractFunction::evaluate(const Context*, const EvalContext *evalctx)
const
 {
        (void)evalctx; // unusued parameter
@@ -82,17 +102,159 @@ std::string AbstractFunction::dump(const std::string
&indent, const std::string
        return dump.str();
 }

+condition::condition(
+    const type_tag type_in,
+    boost::shared_ptr<Expression> expr_in
+)
+    : type(type_in),
+      expr(expr_in)
+{
+}
+
+condition::condition(
+    const type_tag type_in,
+    boost::shared_ptr<Expression> expr_in,
+    boost::shared_ptr<Expression> msg_in
+)
+    : type(type_in),
+      expr(expr_in),
+      msg(msg_in)
+{
+}
+
+std::string condition::typestring() const
+{
+    switch(type) {
+    case condition::precondition:
+        return std::string("precondition");
+    case condition::postcondition:
+        return std::string("postcondition");
+    default:
+        return std::string("");
+    }
+}
+
+namespace
+{
+
+template<condition::type_tag type>
+struct condition_matcher
+{
+    bool operator()(const condition &cond)
+    {
+        return cond.type == type;
+    }
+};
+
+}
+
+Function::Function(
+    const AssignmentList                &formal_args_in,
+          std::vector<condition>         conditions_in,
+    const boost::shared_ptr<Expression>  body_in,
+    const std::string                   &name_in,
+    const Module                        &enclosing_in
+)
+    : definition_arguments(formal_args_in),
+      conds(conditions_in),
+      // Split up pre- and post- so we don't have to do it every time.
+      precond_end(
+          std::stable_partition(
+              conds.begin(),
+              conds.end(),
+              (condition_matcher<condition::precondition>())
+          )
+      ),
+      body(body_in),
+      name(name_in),
+      enclosing(enclosing_in)
+{
+}
+
 Function::~Function()
 {
-       delete expr;
+}
+
+namespace
+{
+
+// Run a pre- or post-condition in the given context.
+struct condition_runner
+{
+    condition_runner(const Context &ctx_in, const std::string &fullname_in)
+        : ctx(ctx_in),
+          fullname(fullname_in),
+          all_passed(true)
+    {
+    }
+
+    operator bool () const
+    {
+        return all_passed;
+    }
+
+    void operator()(const condition &cond)
+    {
+        // Break out early if an earlier condition has failed.
+        if(! all_passed) {
+            return;
+        }
+
+        const Value condition_value = cond.expr->evaluate(&ctx);
+        if(!condition_value.toBool()) {
+            all_passed = false;
+
+            std::stringstream msg;
+            msg << "Error: " << cond.typestring() << " violation in "
+                << fullname;
+
+            if(cond.msg) {
+                const Value usermsg = cond.msg->evaluate(&ctx);
+                msg << ": " << usermsg.toString();
+            }
+
+            PRINT(msg.str().c_str());
+        }
+    }
+
+    const Context &ctx;
+    const std::string fullname;
+    bool all_passed;
+};
+
 }

 Value Function::evaluate(const Context *ctx, const EvalContext *evalctx)
const
 {
-       if (!expr) return Value();
-       Context c(ctx);
-       c.setVariables(definition_arguments, evalctx);
-       return expr->evaluate(&c);
+    // definition_arguments contains any default parameters values.
+    // setVariables is an ad-hoc function that sets up inner appropriately
for
+    // the body of a function call.
+    Context inner(ctx);
+       inner.setVariables(definition_arguments, evalctx);
+
+    condition_runner precond_runner(inner, fullname());
+    std::for_each(conds.begin(), precond_end, precond_runner);
+    if(!precond_runner) {
+        // TODO: make a precondition failure halt compilation.
+        return (Value());
+    }
+
+    // Evaluate the body.
+    const Value produced = body ? (*body).evaluate(&inner) : (Value());
+
+    // Create a context derived from the inner context, with the variable
+    // __retval set to the function return value.
+    Context post_context(&inner);
+    post_context.set_variable("__retval", produced);
+
+    condition_runner postcond_runner(post_context, fullname());
+    std::for_each(precond_end, conds.end(), postcond_runner);
+    if(!postcond_runner) {
+        // TODO: make a postcondition failure halt compilation.
+        return (Value());
+    }
+
+    return produced;
 }

 std::string Function::dump(const std::string &indent, const std::string
&name) const
@@ -105,10 +267,38 @@ std::string Function::dump(const std::string &indent,
const std::string &name) c
                dump << arg.first;
                if (arg.second) dump << " = " << *arg.second;
        }
-       dump << ") = " << *expr << ";\n";
+       dump << ") = " << body << ";\n";
        return dump.str();
 }

+std::string Function::fullname() const
+{
+    std::stringstream namestream;
+    namestream << enclosing.fullname() << "::" << name;
+
+    return namestream.str();
+}
+
+BuiltinFunction::BuiltinFunction(
+    eval_func_t eval_func_in,
+    const std::string name_in
+)
+    : eval_func(eval_func_in),
+      name(name_in)
+{
+}
+
+BuiltinFunction::BuiltinFunction(
+    eval_func_t       eval_func_in,
+    const Feature     &feature_in,
+    const std::string name_in
+)
+    : AbstractFunction(feature_in),
+      eval_func(eval_func_in),
+      name(name_in)
+{
+}
+
 BuiltinFunction::~BuiltinFunction()
 {
 }
@@ -125,6 +315,11 @@ std::string BuiltinFunction::dump(const std::string
&indent, const std::string &
        return dump.str();
 }

+std::string BuiltinFunction::fullname() const
+{
+    return "builtin::" + name;
+}
+
 static inline double deg2rad(double x)
 {
        return x * M_PI / 180.0;
@@ -917,35 +1112,35 @@ Value builtin_cross(const Context *, const EvalContext
*evalctx)

 void register_builtin_functions()
 {
-       Builtins::init("abs", new BuiltinFunction(&builtin_abs));
-       Builtins::init("sign", new BuiltinFunction(&builtin_sign));
-       Builtins::init("rands", new BuiltinFunction(&builtin_rands));
-       Builtins::init("min", new BuiltinFunction(&builtin_min));
-       Builtins::init("max", new BuiltinFunction(&builtin_max));
-       Builtins::init("sin", new BuiltinFunction(&builtin_sin));
-       Builtins::init("cos", new BuiltinFunction(&builtin_cos));
-       Builtins::init("asin", new BuiltinFunction(&builtin_asin));
-       Builtins::init("acos", new BuiltinFunction(&builtin_acos));
-       Builtins::init("tan", new BuiltinFunction(&builtin_tan));
-       Builtins::init("atan", new BuiltinFunction(&builtin_atan));
-       Builtins::init("atan2", new BuiltinFunction(&builtin_atan2));
-       Builtins::init("round", new BuiltinFunction(&builtin_round));
-       Builtins::init("ceil", new BuiltinFunction(&builtin_ceil));
-       Builtins::init("floor", new BuiltinFunction(&builtin_floor));
-       Builtins::init("pow", new BuiltinFunction(&builtin_pow));
-       Builtins::init("sqrt", new BuiltinFunction(&builtin_sqrt));
-       Builtins::init("exp", new BuiltinFunction(&builtin_exp));
-       Builtins::init("len", new BuiltinFunction(&builtin_length));
-       Builtins::init("log", new BuiltinFunction(&builtin_log));
-       Builtins::init("ln", new BuiltinFunction(&builtin_ln));
-       Builtins::init("str", new BuiltinFunction(&builtin_str));
-       Builtins::init("chr", new BuiltinFunction(&builtin_chr));
-       Builtins::init("concat", new BuiltinFunction(&builtin_concat));
-       Builtins::init("lookup", new BuiltinFunction(&builtin_lookup));
-       Builtins::init("search", new BuiltinFunction(&builtin_search));
-       Builtins::init("version", new BuiltinFunction(&builtin_version));
-       Builtins::init("version_num", new BuiltinFunction(&builtin_version_num));
-       Builtins::init("norm", new BuiltinFunction(&builtin_norm));
-       Builtins::init("cross", new BuiltinFunction(&builtin_cross));
-       Builtins::init("parent_module", new
BuiltinFunction(&builtin_parent_module));
+       Builtins::init("abs", new BuiltinFunction(&builtin_abs, "abs"));
+       Builtins::init("sign", new BuiltinFunction(&builtin_sign, "sign"));
+       Builtins::init("rands", new BuiltinFunction(&builtin_rands, "rands"));
+       Builtins::init("min", new BuiltinFunction(&builtin_min, "min"));
+       Builtins::init("max", new BuiltinFunction(&builtin_max, "max"));
+       Builtins::init("sin", new BuiltinFunction(&builtin_sin, "sin"));
+       Builtins::init("cos", new BuiltinFunction(&builtin_cos, "cos"));
+       Builtins::init("asin", new BuiltinFunction(&builtin_asin, "asin"));
+       Builtins::init("acos", new BuiltinFunction(&builtin_acos, "acos"));
+       Builtins::init("tan", new BuiltinFunction(&builtin_tan, "tan"));
+       Builtins::init("atan", new BuiltinFunction(&builtin_atan, "atan"));
+       Builtins::init("atan2", new BuiltinFunction(&builtin_atan2, "atan2"));
+       Builtins::init("round", new BuiltinFunction(&builtin_round, "round"));
+       Builtins::init("ceil", new BuiltinFunction(&builtin_ceil, "ceil"));
+       Builtins::init("floor", new BuiltinFunction(&builtin_floor, "floor"));
+       Builtins::init("pow", new BuiltinFunction(&builtin_pow, "pow"));
+       Builtins::init("sqrt", new BuiltinFunction(&builtin_sqrt, "sqrt"));
+       Builtins::init("exp", new BuiltinFunction(&builtin_exp, "exp"));
+       Builtins::init("len", new BuiltinFunction(&builtin_length, "length"));
+       Builtins::init("log", new BuiltinFunction(&builtin_log, "log"));
+       Builtins::init("ln", new BuiltinFunction(&builtin_ln, "ln"));
+       Builtins::init("str", new BuiltinFunction(&builtin_str, "str"));
+       Builtins::init("chr", new BuiltinFunction(&builtin_chr, "chr"));
+       Builtins::init("concat", new BuiltinFunction(&builtin_concat, "concat"));
+       Builtins::init("lookup", new BuiltinFunction(&builtin_lookup, "lookup"));
+       Builtins::init("search", new BuiltinFunction(&builtin_search, "search"));
+       Builtins::init("version", new BuiltinFunction(&builtin_version,
"version"));
+       Builtins::init("version_num", new BuiltinFunction(&builtin_version_num,
"version_num"));
+       Builtins::init("norm", new BuiltinFunction(&builtin_norm, "norm"));
+       Builtins::init("cross", new BuiltinFunction(&builtin_cross, "cross"));
+       Builtins::init("parent_module", new
BuiltinFunction(&builtin_parent_module, "parent_module"));
 }
diff --git a/src/function.h b/src/function.h
index d8e3ad7..e47c01d 100644
--- a/src/function.h
+++ b/src/function.h
@@ -1,25 +1,29 @@
 #pragma once

+#include "expression.h"
+#include "module.h"
 #include "value.h"
 #include "typedefs.h"
 #include "feature.h"

 #include <string>
+#include <utility>
 #include <vector>

-
 class AbstractFunction
 {
 private:
-        const Feature *feature;
+    const Feature *feature;
 public:
-        AbstractFunction() : feature(NULL) {}
-        AbstractFunction(const Feature& feature) : feature(&feature) {}
+    AbstractFunction();
+    AbstractFunction(const Feature& feature);
        virtual ~AbstractFunction();
-        virtual bool is_experimental() const { return feature != NULL; }
-        virtual bool is_enabled() const { return (feature == NULL) ||
feature->is_enabled(); }
+    virtual bool is_experimental() const;
+    virtual bool is_enabled() const;
        virtual Value evaluate(const class Context *ctx, const class EvalContext
*evalctx) const;
        virtual std::string dump(const std::string &indent, const std::string
&name) const;
+
+    virtual std::string fullname() const = 0;
 };

 class BuiltinFunction : public AbstractFunction
@@ -28,12 +32,56 @@ public:
        typedef Value (*eval_func_t)(const Context *ctx, const EvalContext
*evalctx);
        eval_func_t eval_func;

-       BuiltinFunction(eval_func_t f) : eval_func(f) { }
-       BuiltinFunction(eval_func_t f, const Feature& feature) :
AbstractFunction(feature), eval_func(f) { }
+       BuiltinFunction(eval_func_t f, const std::string name_in);
+
+       BuiltinFunction(
+        eval_func_t f,
+        const Feature& feature,
+        const std::string name_in
+    );
+
        virtual ~BuiltinFunction();

        virtual Value evaluate(const Context *ctx, const EvalContext *evalctx)
const;
        virtual std::string dump(const std::string &indent, const std::string
&name) const;
+
+    virtual std::string fullname() const;
+private:
+    const std::string name;
+};
+
+// Pre- and post-condition container.
+//
+// Bundles an expression together with the message that should be printed if
the
+// expression is violated.  Not structured for inheritance.
+class condition
+{
+public:
+    enum type_tag
+    {
+        precondition,
+        postcondition
+    };
+
+    type_tag type;
+
+    // TODO: These members should be made into std::unique_ptrs at the
earliest
+    // opportunity.
+    boost::shared_ptr<Expression> expr;
+    boost::shared_ptr<Expression> msg;
+
+    condition(
+        const type_tag type_in,
+        const boost::shared_ptr<Expression> expr_in
+    );
+
+    condition(
+        const type_tag type_in,
+        const boost::shared_ptr<Expression> expr_in,
+        const boost::shared_ptr<Expression> msg_in
+    );
+
+    std::string typestring() const;
 };

 class Function : public AbstractFunction
@@ -41,11 +89,56 @@ class Function : public AbstractFunction
 public:
        AssignmentList definition_arguments;

-       Expression *expr;
+    // The pre- and post-conditions.
+    //
+    // After construction of the object, preconditions will be at the front,
and
+    // postconditions will be at the back.
+    std::vector<condition> conds;
+
+    // Iterator delimiting pre- and post-conditions.
+    //
+    // [conds.begin(), precond_end): preconditions.
+    //
+    // [precond_end, conds.end()): postconditions.
+    std::vector<condition>::const_iterator precond_end;

-       Function() { }
-       virtual ~Function();
+    // TODO: Should be made into a std::unique_ptr at the earliest
opportunity.
+    const boost::shared_ptr<Expression> body;
+
+    const std::string name;
+
+    const Module &enclosing;
+
+    // Create a function.
+    //
+    // Parameters:
+    //
+    //   formal_args_in: The list of formal arguments for the function,
+    //     including any default values.
+    //
+    //   conditions_in: The list of pre- and post- conditions.
+    //
+    //   body_in: The function definition.
+    //
+    //   name_in: The base name of the function.
+    //
+    //   enclosing_in: The module in which this function is defined.
+    //
+    // The precondition, postcondition, and body are all evaluated in an
inner
+    // context that has bound the formal parameter list to their concrete
+    // values.
+    Function(
+        const AssignmentList                &formal_args_in,
+              std::vector<condition>        conditions_in,
+        const boost::shared_ptr<Expression> body_in,
+        const std::string                   &name_in,
+        const Module                        &enclosing_in
+    );
+
+    virtual ~Function();

        virtual Value evaluate(const Context *ctx, const EvalContext *evalctx)
const;
        virtual std::string dump(const std::string &indent, const std::string
&name) const;
+
+    virtual std::string fullname() const;
 };
diff --git a/src/lexer.l b/src/lexer.l
index c873be8..815c605 100644
--- a/src/lexer.l
+++ b/src/lexer.l
@@ -28,6 +28,7 @@

 #include <glib.h>
 #include "typedefs.h"
+#include "function.h" // For condition.
 #include "handle_dep.h"
 #include "printutils.h"
 #include "parsersettings.h"
@@ -158,6 +159,9 @@ use[ \t\r\n>]*"<"   { BEGIN(cond_use); }
 "false"                return TOK_FALSE;
 "undef"                return TOK_UNDEF;

+"pre"       return TOK_PRECONDITION;
+"post"      return TOK_POSTCONDITION;
+
 %{/*
  U+00A0 (UTF-8 encoded: C2A0) is no-break space. We support it since Qt's
QTextEdit
  automatically converts these to spaces and we want to be able to process the
same
diff --git a/src/module.cc b/src/module.cc
index 95224be..3da0e38 100644
--- a/src/module.cc
+++ b/src/module.cc
@@ -165,6 +165,16 @@ std::vector<AbstractNode*>
IfElseModuleInstantiation::instantiateElseChildren(co
        return this->else_scope.instantiateChildren(evalctx);
 }

+std::string AbstractModule::fullname() const
+{
+    // Default implementation just dumps the object address to ensure that
the
+    // fullname is unique.
+
+    std::stringstream namestream;
+    namestream << this;
+    return namestream.str();
+}
+
 std::deque<std::string> Module::module_stack;

 Module::~Module()
@@ -234,6 +244,24 @@ std::string Module::dump(const std::string &indent, const
std::string &name) con
        return dump.str();
 }

+std::string Module::fullname() const
+{
+    using std::deque;
+    using std::string;
+    using std::stringstream;
+
+    stringstream fullname;
+
+    for(deque<string>::const_iterator point = module_stack.begin();
+        point != module_stack.end();
+        ++point
+    ) {
+        fullname << "::" << (*point);
+    }
+
+    return fullname.str();
+}
+
 void FileModule::registerUse(const std::string path) {
        std::string extraw = boosty::extension_str(fs::path(path));
        std::string ext = boost::algorithm::to_lower_copy(extraw);
diff --git a/src/module.h b/src/module.h
index 7962ec2..46a8512 100644
--- a/src/module.h
+++ b/src/module.h
@@ -61,17 +61,20 @@ public:
 class AbstractModule
 {
 private:
-        const Feature *feature;
+    const Feature *feature;
 public:
-        AbstractModule() : feature(NULL) {}
-        AbstractModule(const Feature& feature) : feature(&feature) {}
+    AbstractModule() : feature(NULL) {}
+    AbstractModule(const Feature& feature) : feature(&feature) {}
        virtual ~AbstractModule();
-        virtual bool is_experimental() const { return feature != NULL; }
-        virtual bool is_enabled() const { return (feature == NULL) ||
feature->is_enabled(); }
+    virtual bool is_experimental() const { return feature != NULL; }
+    virtual bool is_enabled() const { return (feature == NULL) || feature-
>is_enabled(); }
        virtual class AbstractNode *instantiate(const Context *ctx, const
ModuleInstantiation *inst, const class EvalContext *evalctx = NULL) const;
        virtual std::string dump(const std::string &indent, const std::string
&name) const;
-        virtual double lookup_double_variable_with_default(Context &c,
std::string variable, double def) const;
-        virtual std::string lookup_string_variable_with_default(Context &c,
std::string variable, std::string def) const;
+    virtual double lookup_double_variable_with_default(Context &c,
std::string variable, double def) const;
+    virtual std::string lookup_string_variable_with_default(Context &c,
std::string variable, std::string def) const;
+
+    // Get the fully-qualified name of this module.
+    virtual std::string fullname() const;
 };

 class Module : public AbstractModule
@@ -86,6 +89,13 @@ public:
        static const std::string& stack_element(int n) { return module_stack[n];
};
        static int stack_size() { return module_stack.size(); };

+    // Get the fully-qualified name of this module.
+    //
+    // Get the fully-qualified name of this module (a unique string that
+    // represents the full scope of this module.  The name takes the form
+    // "::a::b::c", where c is nested inside b, which is nested inside a.
+    virtual std::string fullname() const;
+
        AssignmentList definition_arguments;

        LocalScope scope;
diff --git a/src/parser.y b/src/parser.y
index 6c0d537..08eb5ae 100644
--- a/src/parser.y
+++ b/src/parser.y
@@ -61,6 +61,8 @@ int lexerlex(void);
 std::stack<LocalScope *> scope_stack;
 FileModule *rootmodule;

+std::stack<Module*> module_stack;
+
 extern void lexerdestroy();
 extern FILE *lexerin;
 extern const char *parser_input_buffer;
@@ -78,6 +80,9 @@ std::string parser_source_path;
   class IfElseModuleInstantiation *ifelse;
   Assignment *arg;
   AssignmentList *args;
+
+  condition              *cond_type;
+  std::vector<condition> *conds_type;
 }

 %token TOK_ERROR
@@ -89,6 +94,9 @@ std::string parser_source_path;
 %token TOK_FOR
 %token TOK_LET

+%token TOK_PRECONDITION pre
+%token TOK_POSTCONDITION post
+
 %token <text> TOK_ID
 %token <text> TOK_STRING
 %token <text> TOK_USE
@@ -120,6 +128,9 @@ std::string parser_source_path;
 %type <expr> list_comprehension_elements
 %type <expr> list_comprehension_elements_or_expr

+%type <cond_type> condition
+%type <conds_type> conditions
+
 %type <inst> module_instantiation
 %type <ifelse> if_statement
 %type <ifelse> ifelse_statement
@@ -157,25 +168,79 @@ statement:
                 newmodule->definition_arguments = *$4;
                 scope_stack.top()->modules[$2] = newmodule;
                 scope_stack.push(&newmodule->scope);
+
+                module_stack.push(newmodule);
+
                 free($2);
                 delete $4;
             }
           statement
             {
                 scope_stack.pop();
-            }
-        | TOK_FUNCTION TOK_ID '(' arguments_decl optional_commas ')' '=' expr
-            {
-                Function *func = new Function();
-                func->definition_arguments = *$4;
-                func->expr = $8;
+                module_stack.pop();
+            }
+        | TOK_FUNCTION TOK_ID
+          '(' arguments_decl optional_commas ')'
+          conditions
+          '=' expr
+            {
+                Function *func = new Function(
+                    *$4,                               // Formal arguments
+                    *$7,                               // Conditions
+                    boost::shared_ptr<Expression>($9), // Body
+                    (std::string($2)),                 // Base name
+                    *(module_stack.top())              // Enclosing module.
+                );
+
+                // Record the function in this scope.
                 scope_stack.top()->functions[$2] = func;
+
                 free($2);
                 delete $4;
+                delete $7;
             }
           ';'
         ;

+conditions:
+      /* Nothing */ {
+          $$ = new std::vector<condition>();
+      }
+    | condition conditions {
+          $2->push_back(*$1);
+          delete $1;
+          $$ = $2;
+      }
+
+condition:
+      TOK_PRECONDITION '(' expr ')' {
+          $$ = new condition(
+              condition::precondition,
+              boost::shared_ptr<Expression>($3)
+          );
+      }
+    | TOK_PRECONDITION '(' expr ',' expr ')' {
+          $$ = new condition(
+              condition::precondition,
+              boost::shared_ptr<Expression>($3),
+              boost::shared_ptr<Expression>($5)
+          );
+      }
+    | TOK_POSTCONDITION '(' expr ')' {
+          $$ = new condition(
+              condition::postcondition,
+              boost::shared_ptr<Expression>($3)
+          );
+      }
+    | TOK_POSTCONDITION '(' expr ',' expr ')' {
+          $$ = new condition(
+              condition::postcondition,
+              boost::shared_ptr<Expression>($3),
+              boost::shared_ptr<Expression>($5)
+          );
+      }
+    ;
+
 inner_input:
           /* empty */
         | statement inner_input
@@ -606,6 +671,7 @@ FileModule *parse(const char *text, const char *path, int
debug)
   rootmodule = new FileModule();
   rootmodule->setModulePath(path);
   scope_stack.push(&rootmodule->scope);
+  module_stack.push(rootmodule);
   //        PRINTB_NOCACHE("New module: %s %p", "root" % rootmodule);

   parserdebug = debug;
--
2.0.2


_______________________________________________
OpenSCAD mailing list
[hidden email]
http://rocklinux.net/mailman/listinfo/openscad
http://openscad.org - https://flattr.com/thing/121566

_______________________________________________
OpenSCAD mailing list
[hidden email]
http://rocklinux.net/mailman/listinfo/openscad
http://openscad.org - https://flattr.com/thing/121566
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Add function pre- and post-conditions.

Oskar
In reply to this post by Taahir Ahmed
Taahir,

I really like the idea, but think the syntax could be made more general. A few random thoughts:

* Since every expression evaluation in OpenSCAD is side-effect free, there is nothing that could change during a function call, and the only information that isn't known prior to the evaluation is the "return" value. That means that "__retval" will be present in every reasonable post-contition, and the syntax should probably be streamlined.

* For the same reason (that functions are pure), I'm not sure it really make sense to talk about it as a temporal sequence with pre and post evaluation. Function domain might be more appropriate.

* Unit tests are closely related to function contracts. Could they be included?

* As far as I see, these contracts serve multiple purposes, being 1) a debugging aid – making sure that a function behaves as intended, 2) a way of documenting a function, and 3) to give a user clear and helpful error messages for incorrect parameter values. In terms of documentation, it seems to me that the only thing one would like in addition is a structured (string) describing the function itself.

Best,

/Oskar
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Add function pre- and post-conditions.

Taahir Ahmed
In reply to this post by Felipe Sanches
> Does it work well with a scad script that declares a module or function
> called pre or called post ?

I thought it would, but it does not.  The pre- and post-conditions don't muck
with the symbol table (except for the `__retval`), but it looks like

function pre(x) pre(x > 0) = x;

doesn't work, since the lexer cuts up all literal 'pre's into
TOK_PRECONDITION.

> Can such a function called pre or post be safely used as part of a
> precondition or as part of a post condition statement ?

No, as a consequence of the above.

Syntax like you suggest in your next mail should fix this, and since Oskar
would like docstrings as well, I should probably be using plain identifiers to
identify function attributes, rather than plain TOK_PRECONDITION and
TOK_POSTCONDITION.

Taahir

On 27 August 2014 08:59 Felipe Sanches wrote:

> Em 27/08/2014 04:42, "Taahir Ahmed" <[hidden email]> escreveu:
>
> > Our previous discussion about asserts and testing in general led me to
> > draw up this patch, which I thought I would send around for comment
> > before pursuing further.
> >
> > This message should be directly usable by `git am`.  If it's preferable,
> > I can instead make a pull request on github.
> >
> > Taahir Ahmed
> >
> > From 5595c58f57e39b848c504f2cafc2643241588dbe Mon Sep 17 00:00:00 2001
> > From: Taahir Ahmed <[hidden email]>
> > Date: Tue, 26 Aug 2014 23:25:01 -0500
> > Subject: [PATCH] Add function pre- and post-conditions.
> >
> > Change description
> > ==================
> >
> > This change adds optional preconditions and postconditions to OpenSCAD
> > functions.  Multiple preconditions and postconditions may be attached to
> > a given function.  Optional messages can be attached to each
> > precondition and postcondition, to aid the user in debugging.
> >
> > Usage
> > -----
> >
> > Conditions are added to functions after the close of the argument list
> > and before the defining `=`:
> >
> > ```
> > function f(x)
> >     pre(x > 0, "The argument must be positive.")
> >     post(__retval > 0, "The return value must be positive.")
> >     = x;
> > ```
> >
> > The syntax is whitespace-insensitive, and as many preconditions and
> > postconditions may be attached as are needed, in any order.
> > Preconditions are declared using `pre(expr)` or `pre(expr,expr)`.
> > Postconditions are declared using `post(expr)` or `post(expr,expr)`.
> >
> >   * The first argument is the condition.  It can be any single
> >     expression.  If it is true when coerced to a boolean, then the
> >     precondition passes.  Otherwise, the precondition fails.
> >
> >   * The optional second argument is the message.  It can be any single
> >     expression.  Its value is coerced to a string, which then forms
> >     part of the error message.
> >
> > Semantics
> > ---------
> >
> > All the involved expressions have access to the full lexical scope in
> > which the function is defined, including the formal parameters.  In
> > addition, postconditions have access to the return value of the function
> > as the special symbol `__retval`.
> >
> > Function evaluation proceeds in the following order:
> >
> >   - Preconditions are evaluated in the order in which they appear.
> >     If any precondition fails, then no subsequent precondition is
> >     evaluated, and the function returns `undef`.
> >
> >   - If all preconditions succeed, then the body of the function is
> >     evaluated.
> >
> >   - Postconditions are evaluated in the order in which they appear.  If
> >     any postcondition fails, then no subsequent postcondition is
> >     evaluated, and the function returns `undef`.
> >
> >   - If all postconditions succeed, the function returns the value
> >     produced by the body.
> >
> > Deficiencies and gotchas
> > ========================
> >
> >   - The behavior of `value.toBool()` leads to some unexpected behavior
> >     --- for example, the following code will not trigger a precondition
> >     failure:
> >
> >     ```
> >     function f(x) pre(x) = x;
> >     z = f(0/0);
> >     ```
> >
> >     Internally, `toBool` checks that `x != 0`, and `NaN != 0`, so the
> >     precondition passes.  I figured it was better to stick with uniform
> >     semantics, even if they are a little strange.
> >
> >   - Ideally, condition failure would halt compilation.  This currently
> >     does not happen.  Compilation continues as if the function returned
> >     `undef`.
> >
> >   - There are no tests associated with this feature yet.  The existing
> >     testing framework seems to be set up around testing geometry output,
> >     not control flow / console output.
> > ---
> >  src/dxfdim.cc  |   4 +-
> >  src/func.cc    | 269
> > +++++++++++++++++++++++++++++++++++++++++++++++++--------
> >  src/function.h | 115 +++++++++++++++++++++---
> >  src/lexer.l    |   4 +
> >  src/module.cc  |  28 ++++++
> >  src/module.h   |  24 +++--
> >  src/parser.y   |  78 +++++++++++++++--
> >  7 files changed, 459 insertions(+), 63 deletions(-)
> >
> > diff --git a/src/dxfdim.cc b/src/dxfdim.cc
> > index 8f55a2f..898f21a 100644
> > --- a/src/dxfdim.cc
> > +++ b/src/dxfdim.cc
> > @@ -215,6 +215,6 @@ Value builtin_dxf_cross(const Context *ctx, const
> > EvalContext *evalctx)
> >
> >  void initialize_builtin_dxf_dim()
> >  {
> > -       Builtins::init("dxf_dim", new BuiltinFunction(&builtin_dxf_dim));
> > -       Builtins::init("dxf_cross", new
> > BuiltinFunction(&builtin_dxf_cross));
> > +       Builtins::init("dxf_dim", new BuiltinFunction(&builtin_dxf_dim,
> > "dxf_dim"));
> > +       Builtins::init("dxf_cross", new
BuiltinFunction(&builtin_dxf_cross,

> > "dxf_cross"));
> >  }
> > diff --git a/src/func.cc b/src/func.cc
> > index f26f44c..53c359b 100644
> > --- a/src/func.cc
> > +++ b/src/func.cc
> > @@ -65,10 +65,30 @@ int process_id = getpid();
> >  boost::mt19937 deterministic_rng;
> >  boost::mt19937 lessdeterministic_rng( std::time(0) + process_id );
> >
> > +AbstractFunction::AbstractFunction()
> > +    : feature(NULL)
> > +{
> > +}
> > +
> > +AbstractFunction::AbstractFunction(const Feature &feature_in)
> > +    : feature(&feature_in)
> > +{
> > +}
> > +
> >  AbstractFunction::~AbstractFunction()
> >  {
> >  }
> >
> > +bool AbstractFunction::is_experimental() const
> > +{
> > +    return feature != NULL;
> > +}
> > +
> > +bool AbstractFunction::is_enabled() const
> > +{
> > +    return (feature == NULL) || feature->is_enabled();
> > +}
> > +
> >  Value AbstractFunction::evaluate(const Context*, const EvalContext
> > *evalctx)
> > const
> >  {
> >         (void)evalctx; // unusued parameter
> > @@ -82,17 +102,159 @@ std::string AbstractFunction::dump(const std::string
> > &indent, const std::string
> >         return dump.str();
> >  }
> >
> > +condition::condition(
> > +    const type_tag type_in,
> > +    boost::shared_ptr<Expression> expr_in
> > +)
> > +    : type(type_in),
> > +      expr(expr_in)
> > +{
> > +}
> > +
> > +condition::condition(
> > +    const type_tag type_in,
> > +    boost::shared_ptr<Expression> expr_in,
> > +    boost::shared_ptr<Expression> msg_in
> > +)
> > +    : type(type_in),
> > +      expr(expr_in),
> > +      msg(msg_in)
> > +{
> > +}
> > +
> > +std::string condition::typestring() const
> > +{
> > +    switch(type) {
> > +    case condition::precondition:
> > +        return std::string("precondition");
> > +    case condition::postcondition:
> > +        return std::string("postcondition");
> > +    default:
> > +        return std::string("");
> > +    }
> > +}
> > +
> > +namespace
> > +{
> > +
> > +template<condition::type_tag type>
> > +struct condition_matcher
> > +{
> > +    bool operator()(const condition &cond)
> > +    {
> > +        return cond.type == type;
> > +    }
> > +};
> > +
> > +}
> > +
> > +Function::Function(
> > +    const AssignmentList                &formal_args_in,
> > +          std::vector<condition>         conditions_in,
> > +    const boost::shared_ptr<Expression>  body_in,
> > +    const std::string                   &name_in,
> > +    const Module                        &enclosing_in
> > +)
> > +    : definition_arguments(formal_args_in),
> > +      conds(conditions_in),
> > +      // Split up pre- and post- so we don't have to do it every time.
> > +      precond_end(
> > +          std::stable_partition(
> > +              conds.begin(),
> > +              conds.end(),
> > +              (condition_matcher<condition::precondition>())
> > +          )
> > +      ),
> > +      body(body_in),
> > +      name(name_in),
> > +      enclosing(enclosing_in)
> > +{
> > +}
> > +
> >  Function::~Function()
> >  {
> > -       delete expr;
> > +}
> > +
> > +namespace
> > +{
> > +
> > +// Run a pre- or post-condition in the given context.
> > +struct condition_runner
> > +{
> > +    condition_runner(const Context &ctx_in, const std::string
> > &fullname_in)
> > +        : ctx(ctx_in),
> > +          fullname(fullname_in),
> > +          all_passed(true)
> > +    {
> > +    }
> > +
> > +    operator bool () const
> > +    {
> > +        return all_passed;
> > +    }
> > +
> > +    void operator()(const condition &cond)
> > +    {
> > +        // Break out early if an earlier condition has failed.
> > +        if(! all_passed) {
> > +            return;
> > +        }
> > +
> > +        const Value condition_value = cond.expr->evaluate(&ctx);
> > +        if(!condition_value.toBool()) {
> > +            all_passed = false;
> > +
> > +            std::stringstream msg;
> > +            msg << "Error: " << cond.typestring() << " violation in "
> > +                << fullname;
> > +
> > +            if(cond.msg) {
> > +                const Value usermsg = cond.msg->evaluate(&ctx);
> > +                msg << ": " << usermsg.toString();
> > +            }
> > +
> > +            PRINT(msg.str().c_str());
> > +        }
> > +    }
> > +
> > +    const Context &ctx;
> > +    const std::string fullname;
> > +    bool all_passed;
> > +};
> > +
> >  }
> >
> >  Value Function::evaluate(const Context *ctx, const EvalContext *evalctx)
> > const
> >  {
> > -       if (!expr) return Value();
> > -       Context c(ctx);
> > -       c.setVariables(definition_arguments, evalctx);
> > -       return expr->evaluate(&c);
> > +    // definition_arguments contains any default parameters values.
> > +    // setVariables is an ad-hoc function that sets up inner
appropriately

> > for
> > +    // the body of a function call.
> > +    Context inner(ctx);
> > +       inner.setVariables(definition_arguments, evalctx);
> > +
> > +    condition_runner precond_runner(inner, fullname());
> > +    std::for_each(conds.begin(), precond_end, precond_runner);
> > +    if(!precond_runner) {
> > +        // TODO: make a precondition failure halt compilation.
> > +        return (Value());
> > +    }
> > +
> > +    // Evaluate the body.
> > +    const Value produced = body ? (*body).evaluate(&inner) : (Value());
> > +
> > +    // Create a context derived from the inner context, with the variable
> > +    // __retval set to the function return value.
> > +    Context post_context(&inner);
> > +    post_context.set_variable("__retval", produced);
> > +
> > +    condition_runner postcond_runner(post_context, fullname());
> > +    std::for_each(precond_end, conds.end(), postcond_runner);
> > +    if(!postcond_runner) {
> > +        // TODO: make a postcondition failure halt compilation.
> > +        return (Value());
> > +    }
> > +
> > +    return produced;
> >  }
> >
> >  std::string Function::dump(const std::string &indent, const std::string
> > &name) const
> > @@ -105,10 +267,38 @@ std::string Function::dump(const std::string
&indent,

> > const std::string &name) c
> >                 dump << arg.first;
> >                 if (arg.second) dump << " = " << *arg.second;
> >         }
> > -       dump << ") = " << *expr << ";\n";
> > +       dump << ") = " << body << ";\n";
> >         return dump.str();
> >  }
> >
> > +std::string Function::fullname() const
> > +{
> > +    std::stringstream namestream;
> > +    namestream << enclosing.fullname() << "::" << name;
> > +
> > +    return namestream.str();
> > +}
> > +
> > +BuiltinFunction::BuiltinFunction(
> > +    eval_func_t eval_func_in,
> > +    const std::string name_in
> > +)
> > +    : eval_func(eval_func_in),
> > +      name(name_in)
> > +{
> > +}
> > +
> > +BuiltinFunction::BuiltinFunction(
> > +    eval_func_t       eval_func_in,
> > +    const Feature     &feature_in,
> > +    const std::string name_in
> > +)
> > +    : AbstractFunction(feature_in),
> > +      eval_func(eval_func_in),
> > +      name(name_in)
> > +{
> > +}
> > +
> >  BuiltinFunction::~BuiltinFunction()
> >  {
> >  }
> > @@ -125,6 +315,11 @@ std::string BuiltinFunction::dump(const std::string
> > &indent, const std::string &
> >         return dump.str();
> >  }
> >
> > +std::string BuiltinFunction::fullname() const
> > +{
> > +    return "builtin::" + name;
> > +}
> > +
> >  static inline double deg2rad(double x)
> >  {
> >         return x * M_PI / 180.0;
> > @@ -917,35 +1112,35 @@ Value builtin_cross(const Context *, const
> > EvalContext
> > *evalctx)
> >
> >  void register_builtin_functions()
> >  {
> > -       Builtins::init("abs", new BuiltinFunction(&builtin_abs));
> > -       Builtins::init("sign", new BuiltinFunction(&builtin_sign));
> > -       Builtins::init("rands", new BuiltinFunction(&builtin_rands));
> > -       Builtins::init("min", new BuiltinFunction(&builtin_min));
> > -       Builtins::init("max", new BuiltinFunction(&builtin_max));
> > -       Builtins::init("sin", new BuiltinFunction(&builtin_sin));
> > -       Builtins::init("cos", new BuiltinFunction(&builtin_cos));
> > -       Builtins::init("asin", new BuiltinFunction(&builtin_asin));
> > -       Builtins::init("acos", new BuiltinFunction(&builtin_acos));
> > -       Builtins::init("tan", new BuiltinFunction(&builtin_tan));
> > -       Builtins::init("atan", new BuiltinFunction(&builtin_atan));
> > -       Builtins::init("atan2", new BuiltinFunction(&builtin_atan2));
> > -       Builtins::init("round", new BuiltinFunction(&builtin_round));
> > -       Builtins::init("ceil", new BuiltinFunction(&builtin_ceil));
> > -       Builtins::init("floor", new BuiltinFunction(&builtin_floor));
> > -       Builtins::init("pow", new BuiltinFunction(&builtin_pow));
> > -       Builtins::init("sqrt", new BuiltinFunction(&builtin_sqrt));
> > -       Builtins::init("exp", new BuiltinFunction(&builtin_exp));
> > -       Builtins::init("len", new BuiltinFunction(&builtin_length));
> > -       Builtins::init("log", new BuiltinFunction(&builtin_log));
> > -       Builtins::init("ln", new BuiltinFunction(&builtin_ln));
> > -       Builtins::init("str", new BuiltinFunction(&builtin_str));
> > -       Builtins::init("chr", new BuiltinFunction(&builtin_chr));
> > -       Builtins::init("concat", new BuiltinFunction(&builtin_concat));
> > -       Builtins::init("lookup", new BuiltinFunction(&builtin_lookup));
> > -       Builtins::init("search", new BuiltinFunction(&builtin_search));
> > -       Builtins::init("version", new BuiltinFunction(&builtin_version));
> > -       Builtins::init("version_num", new
> > BuiltinFunction(&builtin_version_num));
> > -       Builtins::init("norm", new BuiltinFunction(&builtin_norm));
> > -       Builtins::init("cross", new BuiltinFunction(&builtin_cross));
> > -       Builtins::init("parent_module", new
> > BuiltinFunction(&builtin_parent_module));
> > +       Builtins::init("abs", new BuiltinFunction(&builtin_abs, "abs"));
> > +       Builtins::init("sign", new BuiltinFunction(&builtin_sign,
"sign"));
> > +       Builtins::init("rands", new BuiltinFunction(&builtin_rands,
> > "rands"));
> > +       Builtins::init("min", new BuiltinFunction(&builtin_min, "min"));
> > +       Builtins::init("max", new BuiltinFunction(&builtin_max, "max"));
> > +       Builtins::init("sin", new BuiltinFunction(&builtin_sin, "sin"));
> > +       Builtins::init("cos", new BuiltinFunction(&builtin_cos, "cos"));
> > +       Builtins::init("asin", new BuiltinFunction(&builtin_asin,
"asin"));
> > +       Builtins::init("acos", new BuiltinFunction(&builtin_acos,
"acos"));
> > +       Builtins::init("tan", new BuiltinFunction(&builtin_tan, "tan"));
> > +       Builtins::init("atan", new BuiltinFunction(&builtin_atan,
"atan"));
> > +       Builtins::init("atan2", new BuiltinFunction(&builtin_atan2,
> > "atan2"));
> > +       Builtins::init("round", new BuiltinFunction(&builtin_round,
> > "round"));
> > +       Builtins::init("ceil", new BuiltinFunction(&builtin_ceil,
"ceil"));
> > +       Builtins::init("floor", new BuiltinFunction(&builtin_floor,
> > "floor"));
> > +       Builtins::init("pow", new BuiltinFunction(&builtin_pow, "pow"));
> > +       Builtins::init("sqrt", new BuiltinFunction(&builtin_sqrt,
"sqrt"));

> > +       Builtins::init("exp", new BuiltinFunction(&builtin_exp, "exp"));
> > +       Builtins::init("len", new BuiltinFunction(&builtin_length,
> > "length"));
> > +       Builtins::init("log", new BuiltinFunction(&builtin_log, "log"));
> > +       Builtins::init("ln", new BuiltinFunction(&builtin_ln, "ln"));
> > +       Builtins::init("str", new BuiltinFunction(&builtin_str, "str"));
> > +       Builtins::init("chr", new BuiltinFunction(&builtin_chr, "chr"));
> > +       Builtins::init("concat", new BuiltinFunction(&builtin_concat,
> > "concat"));
> > +       Builtins::init("lookup", new BuiltinFunction(&builtin_lookup,
> > "lookup"));
> > +       Builtins::init("search", new BuiltinFunction(&builtin_search,
> > "search"));
> > +       Builtins::init("version", new BuiltinFunction(&builtin_version,
> > "version"));
> > +       Builtins::init("version_num", new
> > BuiltinFunction(&builtin_version_num,
> > "version_num"));
> > +       Builtins::init("norm", new BuiltinFunction(&builtin_norm,
"norm"));

> > +       Builtins::init("cross", new BuiltinFunction(&builtin_cross,
> > "cross"));
> > +       Builtins::init("parent_module", new
> > BuiltinFunction(&builtin_parent_module, "parent_module"));
> >  }
> > diff --git a/src/function.h b/src/function.h
> > index d8e3ad7..e47c01d 100644
> > --- a/src/function.h
> > +++ b/src/function.h
> > @@ -1,25 +1,29 @@
> >  #pragma once
> >
> > +#include "expression.h"
> > +#include "module.h"
> >  #include "value.h"
> >  #include "typedefs.h"
> >  #include "feature.h"
> >
> >  #include <string>
> > +#include <utility>
> >  #include <vector>
> >
> > -
> >  class AbstractFunction
> >  {
> >  private:
> > -        const Feature *feature;
> > +    const Feature *feature;
> >  public:
> > -        AbstractFunction() : feature(NULL) {}
> > -        AbstractFunction(const Feature& feature) : feature(&feature) {}
> > +    AbstractFunction();
> > +    AbstractFunction(const Feature& feature);
> >         virtual ~AbstractFunction();
> > -        virtual bool is_experimental() const { return feature != NULL; }
> > -        virtual bool is_enabled() const { return (feature == NULL) ||
> > feature->is_enabled(); }
> > +    virtual bool is_experimental() const;
> > +    virtual bool is_enabled() const;
> >         virtual Value evaluate(const class Context *ctx, const class
> > EvalContext
> > *evalctx) const;
> >         virtual std::string dump(const std::string &indent, const
> > std::string
> > &name) const;
> > +
> > +    virtual std::string fullname() const = 0;
> >  };
> >
> >  class BuiltinFunction : public AbstractFunction
> > @@ -28,12 +32,56 @@ public:
> >         typedef Value (*eval_func_t)(const Context *ctx, const EvalContext
> > *evalctx);
> >         eval_func_t eval_func;
> >
> > -       BuiltinFunction(eval_func_t f) : eval_func(f) { }
> > -       BuiltinFunction(eval_func_t f, const Feature& feature) :
> > AbstractFunction(feature), eval_func(f) { }
> > +       BuiltinFunction(eval_func_t f, const std::string name_in);
> > +
> > +       BuiltinFunction(
> > +        eval_func_t f,
> > +        const Feature& feature,
> > +        const std::string name_in
> > +    );
> > +
> >         virtual ~BuiltinFunction();
> >
> >         virtual Value evaluate(const Context *ctx, const EvalContext
> > *evalctx)
> > const;
> >         virtual std::string dump(const std::string &indent, const
> > std::string
> > &name) const;
> > +
> > +    virtual std::string fullname() const;
> > +private:
> > +    const std::string name;
> > +};
> > +
> > +// Pre- and post-condition container.
> > +//
> > +// Bundles an expression together with the message that should be printed
> > if
> > the
> > +// expression is violated.  Not structured for inheritance.
> > +class condition
> > +{
> > +public:
> > +    enum type_tag
> > +    {
> > +        precondition,
> > +        postcondition
> > +    };
> > +
> > +    type_tag type;
> > +
> > +    // TODO: These members should be made into std::unique_ptrs at the
> > earliest
> > +    // opportunity.
> > +    boost::shared_ptr<Expression> expr;
> > +    boost::shared_ptr<Expression> msg;
> > +
> > +    condition(
> > +        const type_tag type_in,
> > +        const boost::shared_ptr<Expression> expr_in
> > +    );
> > +
> > +    condition(
> > +        const type_tag type_in,
> > +        const boost::shared_ptr<Expression> expr_in,
> > +        const boost::shared_ptr<Expression> msg_in
> > +    );
> > +
> > +    std::string typestring() const;
> >  };
> >
> >  class Function : public AbstractFunction
> > @@ -41,11 +89,56 @@ class Function : public AbstractFunction
> >  public:
> >         AssignmentList definition_arguments;
> >
> > -       Expression *expr;
> > +    // The pre- and post-conditions.
> > +    //
> > +    // After construction of the object, preconditions will be at the
> > front,
> > and
> > +    // postconditions will be at the back.
> > +    std::vector<condition> conds;
> > +
> > +    // Iterator delimiting pre- and post-conditions.
> > +    //
> > +    // [conds.begin(), precond_end): preconditions.
> > +    //
> > +    // [precond_end, conds.end()): postconditions.
> > +    std::vector<condition>::const_iterator precond_end;
> >
> > -       Function() { }
> > -       virtual ~Function();
> > +    // TODO: Should be made into a std::unique_ptr at the earliest
> > opportunity.
> > +    const boost::shared_ptr<Expression> body;
> > +
> > +    const std::string name;
> > +
> > +    const Module &enclosing;
> > +
> > +    // Create a function.
> > +    //
> > +    // Parameters:
> > +    //
> > +    //   formal_args_in: The list of formal arguments for the function,
> > +    //     including any default values.
> > +    //
> > +    //   conditions_in: The list of pre- and post- conditions.
> > +    //
> > +    //   body_in: The function definition.
> > +    //
> > +    //   name_in: The base name of the function.
> > +    //
> > +    //   enclosing_in: The module in which this function is defined.
> > +    //
> > +    // The precondition, postcondition, and body are all evaluated in an
> > inner
> > +    // context that has bound the formal parameter list to their concrete
> > +    // values.
> > +    Function(
> > +        const AssignmentList                &formal_args_in,
> > +              std::vector<condition>        conditions_in,
> > +        const boost::shared_ptr<Expression> body_in,
> > +        const std::string                   &name_in,
> > +        const Module                        &enclosing_in
> > +    );
> > +
> > +    virtual ~Function();
> >
> >         virtual Value evaluate(const Context *ctx, const EvalContext
> > *evalctx)
> > const;
> >         virtual std::string dump(const std::string &indent, const
> > std::string
> > &name) const;
> > +
> > +    virtual std::string fullname() const;
> >  };
> > diff --git a/src/lexer.l b/src/lexer.l
> > index c873be8..815c605 100644
> > --- a/src/lexer.l
> > +++ b/src/lexer.l
> > @@ -28,6 +28,7 @@
> >
> >  #include <glib.h>
> >  #include "typedefs.h"
> > +#include "function.h" // For condition.
> >  #include "handle_dep.h"
> >  #include "printutils.h"
> >  #include "parsersettings.h"
> > @@ -158,6 +159,9 @@ use[ \t\r\n>]*"<"   { BEGIN(cond_use); }
> >  "false"                return TOK_FALSE;
> >  "undef"                return TOK_UNDEF;
> >
> > +"pre"       return TOK_PRECONDITION;
> > +"post"      return TOK_POSTCONDITION;
> > +
> >  %{/*
> >   U+00A0 (UTF-8 encoded: C2A0) is no-break space. We support it since Qt's
> > QTextEdit
> >   automatically converts these to spaces and we want to be able to process
> > the
> > same
> > diff --git a/src/module.cc b/src/module.cc
> > index 95224be..3da0e38 100644
> > --- a/src/module.cc
> > +++ b/src/module.cc
> > @@ -165,6 +165,16 @@ std::vector<AbstractNode*>
> > IfElseModuleInstantiation::instantiateElseChildren(co
> >         return this->else_scope.instantiateChildren(evalctx);
> >  }
> >
> > +std::string AbstractModule::fullname() const
> > +{
> > +    // Default implementation just dumps the object address to ensure
that

> > the
> > +    // fullname is unique.
> > +
> > +    std::stringstream namestream;
> > +    namestream << this;
> > +    return namestream.str();
> > +}
> > +
> >  std::deque<std::string> Module::module_stack;
> >
> >  Module::~Module()
> > @@ -234,6 +244,24 @@ std::string Module::dump(const std::string &indent,
> > const
> > std::string &name) con
> >         return dump.str();
> >  }
> >
> > +std::string Module::fullname() const
> > +{
> > +    using std::deque;
> > +    using std::string;
> > +    using std::stringstream;
> > +
> > +    stringstream fullname;
> > +
> > +    for(deque<string>::const_iterator point = module_stack.begin();
> > +        point != module_stack.end();
> > +        ++point
> > +    ) {
> > +        fullname << "::" << (*point);
> > +    }
> > +
> > +    return fullname.str();
> > +}
> > +
> >  void FileModule::registerUse(const std::string path) {
> >         std::string extraw = boosty::extension_str(fs::path(path));
> >         std::string ext = boost::algorithm::to_lower_copy(extraw);
> > diff --git a/src/module.h b/src/module.h
> > index 7962ec2..46a8512 100644
> > --- a/src/module.h
> > +++ b/src/module.h
> > @@ -61,17 +61,20 @@ public:
> >  class AbstractModule
> >  {
> >  private:
> > -        const Feature *feature;
> > +    const Feature *feature;
> >  public:
> > -        AbstractModule() : feature(NULL) {}
> > -        AbstractModule(const Feature& feature) : feature(&feature) {}
> > +    AbstractModule() : feature(NULL) {}
> > +    AbstractModule(const Feature& feature) : feature(&feature) {}
> >         virtual ~AbstractModule();
> > -        virtual bool is_experimental() const { return feature != NULL; }
> > -        virtual bool is_enabled() const { return (feature == NULL) ||
> > feature->is_enabled(); }
> > +    virtual bool is_experimental() const { return feature != NULL; }
> > +    virtual bool is_enabled() const { return (feature == NULL) ||
feature-

> > >is_enabled(); }
> >         virtual class AbstractNode *instantiate(const Context *ctx, const
> > ModuleInstantiation *inst, const class EvalContext *evalctx = NULL) const;
> >         virtual std::string dump(const std::string &indent, const
> > std::string
> > &name) const;
> > -        virtual double lookup_double_variable_with_default(Context &c,
> > std::string variable, double def) const;
> > -        virtual std::string lookup_string_variable_with_default(Context
> > &c,
> > std::string variable, std::string def) const;
> > +    virtual double lookup_double_variable_with_default(Context &c,
> > std::string variable, double def) const;
> > +    virtual std::string lookup_string_variable_with_default(Context &c,
> > std::string variable, std::string def) const;
> > +
> > +    // Get the fully-qualified name of this module.
> > +    virtual std::string fullname() const;
> >  };
> >
> >  class Module : public AbstractModule
> > @@ -86,6 +89,13 @@ public:
> >         static const std::string& stack_element(int n) { return
> > module_stack[n];
> > };
> >         static int stack_size() { return module_stack.size(); };
> >
> > +    // Get the fully-qualified name of this module.
> > +    //
> > +    // Get the fully-qualified name of this module (a unique string that
> > +    // represents the full scope of this module.  The name takes the form
> > +    // "::a::b::c", where c is nested inside b, which is nested inside a.
> > +    virtual std::string fullname() const;
> > +
> >         AssignmentList definition_arguments;
> >
> >         LocalScope scope;
> > diff --git a/src/parser.y b/src/parser.y
> > index 6c0d537..08eb5ae 100644
> > --- a/src/parser.y
> > +++ b/src/parser.y
> > @@ -61,6 +61,8 @@ int lexerlex(void);
> >  std::stack<LocalScope *> scope_stack;
> >  FileModule *rootmodule;
> >
> > +std::stack<Module*> module_stack;
> > +
> >  extern void lexerdestroy();
> >  extern FILE *lexerin;
> >  extern const char *parser_input_buffer;
> > @@ -78,6 +80,9 @@ std::string parser_source_path;
> >    class IfElseModuleInstantiation *ifelse;
> >    Assignment *arg;
> >    AssignmentList *args;
> > +
> > +  condition              *cond_type;
> > +  std::vector<condition> *conds_type;
> >  }
> >
> >  %token TOK_ERROR
> > @@ -89,6 +94,9 @@ std::string parser_source_path;
> >  %token TOK_FOR
> >  %token TOK_LET
> >
> > +%token TOK_PRECONDITION pre
> > +%token TOK_POSTCONDITION post
> > +
> >  %token <text> TOK_ID
> >  %token <text> TOK_STRING
> >  %token <text> TOK_USE
> > @@ -120,6 +128,9 @@ std::string parser_source_path;
> >  %type <expr> list_comprehension_elements
> >  %type <expr> list_comprehension_elements_or_expr
> >
> > +%type <cond_type> condition
> > +%type <conds_type> conditions
> > +
> >  %type <inst> module_instantiation
> >  %type <ifelse> if_statement
> >  %type <ifelse> ifelse_statement
> > @@ -157,25 +168,79 @@ statement:
> >                  newmodule->definition_arguments = *$4;
> >                  scope_stack.top()->modules[$2] = newmodule;
> >                  scope_stack.push(&newmodule->scope);
> > +
> > +                module_stack.push(newmodule);
> > +
> >                  free($2);
> >                  delete $4;
> >              }
> >            statement
> >              {
> >                  scope_stack.pop();
> > -            }
> > -        | TOK_FUNCTION TOK_ID '(' arguments_decl optional_commas ')' '='
> > expr
> > -            {
> > -                Function *func = new Function();
> > -                func->definition_arguments = *$4;
> > -                func->expr = $8;
> > +                module_stack.pop();
> > +            }
> > +        | TOK_FUNCTION TOK_ID
> > +          '(' arguments_decl optional_commas ')'
> > +          conditions
> > +          '=' expr
> > +            {
> > +                Function *func = new Function(
> > +                    *$4,                               // Formal
arguments

> > +                    *$7,                               // Conditions
> > +                    boost::shared_ptr<Expression>($9), // Body
> > +                    (std::string($2)),                 // Base name
> > +                    *(module_stack.top())              // Enclosing
> > module.
> > +                );
> > +
> > +                // Record the function in this scope.
> >                  scope_stack.top()->functions[$2] = func;
> > +
> >                  free($2);
> >                  delete $4;
> > +                delete $7;
> >              }
> >            ';'
> >          ;
> >
> > +conditions:
> > +      /* Nothing */ {
> > +          $$ = new std::vector<condition>();
> > +      }
> > +    | condition conditions {
> > +          $2->push_back(*$1);
> > +          delete $1;
> > +          $$ = $2;
> > +      }
> > +
> > +condition:
> > +      TOK_PRECONDITION '(' expr ')' {
> > +          $$ = new condition(
> > +              condition::precondition,
> > +              boost::shared_ptr<Expression>($3)
> > +          );
> > +      }
> > +    | TOK_PRECONDITION '(' expr ',' expr ')' {
> > +          $$ = new condition(
> > +              condition::precondition,
> > +              boost::shared_ptr<Expression>($3),
> > +              boost::shared_ptr<Expression>($5)
> > +          );
> > +      }
> > +    | TOK_POSTCONDITION '(' expr ')' {
> > +          $$ = new condition(
> > +              condition::postcondition,
> > +              boost::shared_ptr<Expression>($3)
> > +          );
> > +      }
> > +    | TOK_POSTCONDITION '(' expr ',' expr ')' {
> > +          $$ = new condition(
> > +              condition::postcondition,
> > +              boost::shared_ptr<Expression>($3),
> > +              boost::shared_ptr<Expression>($5)
> > +          );
> > +      }
> > +    ;
> > +
> >  inner_input:
> >            /* empty */
> >          | statement inner_input
> > @@ -606,6 +671,7 @@ FileModule *parse(const char *text, const char *path,
> > int
> > debug)
> >    rootmodule = new FileModule();
> >    rootmodule->setModulePath(path);
> >    scope_stack.push(&rootmodule->scope);
> > +  module_stack.push(rootmodule);
> >    //        PRINTB_NOCACHE("New module: %s %p", "root" % rootmodule);
> >
> >    parserdebug = debug;
> > --
> > 2.0.2
> >
> >
> > _______________________________________________
> > OpenSCAD mailing list
> > [hidden email]
> > http://rocklinux.net/mailman/listinfo/openscad
> > http://openscad.org - https://flattr.com/thing/121566
> >

_______________________________________________
OpenSCAD mailing list
[hidden email]
http://rocklinux.net/mailman/listinfo/openscad
http://openscad.org - https://flattr.com/thing/121566

signature.asc (484 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Add function pre- and post-conditions.

Taahir Ahmed
In reply to this post by Oskar
> * Since every expression evaluation in OpenSCAD is side-effect free, there
> is nothing that could change during a function call, and the only
> information that isn't known prior to the evaluation is the "return" value.
> That means that "__retval" will be present in every reasonable
> post-contition, and the syntax should probably be streamlined.

Correct me if I am wrong, but I believe that rands is not side-effect free.

I was also thinking that there might be some expensive dynamic programming
function defined, for which there might not be a termination if you give it
the wrong inputs.  (Fibonacci generation, for example)

> * Unit tests are closely related to function contracts. Could they be
> included?

I don't see why not.  ELisp-like docstrings might also be a useful addition.

Given Felipe's suggested syntax, we might then see a function definitions that
looks like this:

```
function fib(n) = (n == 0) ? 1 : (n == 1) ? 1 : f(n-1) + f(n-2) {
    pre(n >= 0, "n must be positive to ensure termination")
    post(
        __retval == round(__retval),
        "Your requested n was too large to give a precise answer."
    )
    doc("Compute the nth Fibonacci number.

Computes the nth fibonacci number using a recursive method.  Since OpenSCAD
doesn't memoize intermediate function results, you might have to wait a long
time for large values of n.

Parameters:

  n: The 0-based index of the Fibonacci number you want.

Return value:  The nth fibonacci number.")
   
    test(fib(2) == 2, "The second Fibonacci number is 2.")
}
```

The doc-string is a little overkill, but it would just be free-form text.

It's important to note that that the items between the braces are not true
statements.  Should they be?

Taahir

On 27 August 2014 06:32 Oskar wrote:
> Taahir,
>
> I really like the idea, but think the syntax could be made more general. A
> few random thoughts:
>

>
> * For the same reason (that functions are pure), I'm not sure it really make
> sense to talk about it as a temporal sequence with pre and post evaluation.
> Function domain might be more appropriate.
>

>
> * As far as I see, these contracts serve multiple purposes, being 1) a
> debugging aid – making sure that a function behaves as intended, 2) a way of
> documenting a function, and 3) to give a user clear and helpful error
> messages for incorrect parameter values. In terms of documentation, it seems
> to me that the only thing one would like in addition is a structured
> (string) describing the function itself.
>
> Best,
>
> /Oskar
>
>
>
> --
> View this message in context: http://forum.openscad.org/PATCH-Add-function-pre-and-post-conditions-tp9493p9496.html
> Sent from the OpenSCAD mailing list archive at Nabble.com.
> _______________________________________________
> OpenSCAD mailing list
> [hidden email]
> http://rocklinux.net/mailman/listinfo/openscad
> http://openscad.org - https://flattr.com/thing/121566

_______________________________________________
OpenSCAD mailing list
[hidden email]
http://rocklinux.net/mailman/listinfo/openscad
http://openscad.org - https://flattr.com/thing/121566

signature.asc (484 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Add function pre- and post-conditions.

MichaelAtOz
Administrator
It is unclear what happens with recursion, are the pre/post conditions checked for each call or just the first/last?

Could it be expanded to do the fecho()'s job?

function recur(a,b, _i=0) = recursion_code_goes_here {
    pre(n >= 0, "n must be positive to ensure termination")
    post(
        __retval == round(__retval),
        "Your requested n was too large to give a precise answer."
    )
    debug( (a > 10 && i < 5), str(a,b,i), ret_val )  
}

where if ret_val, if specified ( != undef),  overrides the return value instead of evaluating the function instance, if not specified (or ==undef), it keeps going, evaluating the function expression.

Could do similar for pre() post(), thus, depending on the condition you have control of returning,  NaN (0/0), 0, -1, false, true, 42, "whatever you want" etc; or alternatively, issue a warning, but still let the function proceed.

e.g.
pre( n > 1000, "This will take a while...",) // warn and run v's
pre( n > 1000, "Too big, sorry", undef) // error and stop

post( _retval > 100 ), "Max box size is 100, trimming to 100",100) // last ret_val if multiples.
// I know you could do this with conditional (?:), but not with a message.

debug( i % 100, str(i,x,y,z),) // keeps running, msg every 100
debug( x < 0, "oops thats not right", false)  // stop recursion


Thirdly, re syntax, if it is going to be curly brackets, then shouldn't there be semi-colons?

function whatever = whatever {
    pre();
    post();
}
Admin - email* me if you need anything, or if I've done something stupid...
* click on my MichaelAtOz label, there is a link to email me.

Unless specifically shown otherwise above, my contribution is in the Public Domain; to the extent possible under law, I have waived all copyright and related or neighbouring rights to this work.
Obviously inclusion of works of previous authors is not included in the above.
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Add function pre- and post-conditions.

MichaelAtOz
Administrator
> pre( n > 1000, "Too big, sorry", undef) // error and stop

rather :/

pre( n > 1000, "Too big, sorry", false) // error and stop
Admin - email* me if you need anything, or if I've done something stupid...
* click on my MichaelAtOz label, there is a link to email me.

Unless specifically shown otherwise above, my contribution is in the Public Domain; to the extent possible under law, I have waived all copyright and related or neighbouring rights to this work.
Obviously inclusion of works of previous authors is not included in the above.
Ivo
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Add function pre- and post-conditions.

Ivo
In reply to this post by MichaelAtOz
Some of my own openscad scripts contain hacky checks and this idea would clean up this code.

I have some questions on this :
 * Can we set aside a prefix for builtin functions (like pre and post here) so they can be easily identified as such and never collide with user defined names ?

* I'd like to have pre and post condition checking in modules as well.

* There is some overlap with these functions and the echo, can we clean up both at the same time ? I'd like echo to echo without "echo" in front.
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Add function pre- and post-conditions.

minuti
I must be missing something. What's the advantage to these pre and post things over something using if statements and echo? If and echo exist in virtually all languages, so it's familiar syntax to most people.

Since OpenSCAD is a description language, the idea of values changing as a program "runs" is a bit off, so from what I understand of pre and post, they're somewhat out of place here. Yes, you can calculate intermediate values and use assign(), but there's not really any while() structure, nor is there a need.
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Add function pre- and post-conditions.

MichaelAtOz
Administrator
I did a bit of looking, I think it relates to:
new fangled ideas

> Since OpenSCAD is a description language, the idea of values changing as a program "runs" is a bit off,

Yes I agree, but since the introduction of recursion, there is a need to debug it. Recursion IS Turing equivalent of iterative, even tho it can be done in infinite parallelism, EACH instance has a consecutive value of the control variable, so is still manageable.

I proposed the fecho(), but unfortunately it is viewed as having side-effects. !!! like any debug does......

MUST NEED Debug...somehow... help me........drowning(level) = drowning(level+1);
Admin - email* me if you need anything, or if I've done something stupid...
* click on my MichaelAtOz label, there is a link to email me.

Unless specifically shown otherwise above, my contribution is in the Public Domain; to the extent possible under law, I have waived all copyright and related or neighbouring rights to this work.
Obviously inclusion of works of previous authors is not included in the above.
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Add function pre- and post-conditions.

runsun
In reply to this post by Taahir Ahmed
Taahir Ahmed wrote
```
function fib(n) = (n == 0) ? 1 : (n == 1) ? 1 : f(n-1) + f(n-2) {
    pre(n >= 0, "n must be positive to ensure termination")
    post(
        __retval == round(__retval),
        "Your requested n was too large to give a precise answer."
    )
    doc("Compute the nth Fibonacci number.

Computes the nth fibonacci number using a recursive method.  Since OpenSCAD
doesn't memoize intermediate function results, you might have to wait a long
time for large values of n.

Parameters:

  n: The 0-based index of the Fibonacci number you want.

Return value:  The nth fibonacci number.")
   
    test(fib(2) == 2, "The second Fibonacci number is 2.")
}
```

The doc-string is a little overkill, but it would just be free-form text.

It's important to note that that the items between the braces are not true
statements.  Should they be?
I sure like this doc test idea :) We then have to deal with new problems though:

Are doc() and test() run in every single call ? In test() it will result in endless recursion.

In python the doc test string is just a string right following the function definition;
def fib( ...):
    """ this is the fib function
        >>> fib(2) == 2
    """
A solution to this could be: fib(... doc,test) where doc/test is either false, or a number
to specify how they are gonna run (like the "mode" in my OpenScad_DocTest lib ).
$ Runsun Pan, PhD
$ libs: scadx, doctest, faces(git), offline doc(git), runscad.py(2,git), editor of choice: CudaText ( OpenSCAD lexer); $ Tips; $ Snippets
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Add function pre- and post-conditions.

MichaelAtOz
Administrator
In reply to this post by MichaelAtOz
OK, lazy question, without research, HOW do functional languages DO debug? (without 'side-effects')
Admin - email* me if you need anything, or if I've done something stupid...
* click on my MichaelAtOz label, there is a link to email me.

Unless specifically shown otherwise above, my contribution is in the Public Domain; to the extent possible under law, I have waived all copyright and related or neighbouring rights to this work.
Obviously inclusion of works of previous authors is not included in the above.
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Add function pre- and post-conditions.

runsun
In reply to this post by MichaelAtOz
MichaelAtOz wrote
I proposed the fecho(),
+1
$ Runsun Pan, PhD
$ libs: scadx, doctest, faces(git), offline doc(git), runscad.py(2,git), editor of choice: CudaText ( OpenSCAD lexer); $ Tips; $ Snippets
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Add function pre- and post-conditions.

nophead
Since the only side effect is to output a string (which has no effect on the rest of the script) the only issue is that the function results could be cached in future so it is only ever called once for each input value. That would mean you would not get the expected fecho output. The simple solution is to cache the the string produced by fecho() as well as the return value so that it can be output again without calling the function if it is called again with the same arguments.

As mentioned before the same applies to echo() in modules if they get run in parallel.




On 28 August 2014 14:33, runsun <[hidden email]> wrote:
MichaelAtOz wrote
> I proposed the fecho(),

+1



-----
$ Runsun Pan, PhD
$ -- OpenScad_DocTest: doc and unit test ( Github , Thingiverse  )
$ -- Linux Mint 17 MATE 64bit  + OpenSCAD 2014.05.31 snapshot
$ -- Linux Mint 17 MATE 64bit + Wine 1.6.2 + OpenSCAD for Windows 2014.07.22 snapshot


--
View this message in context: http://forum.openscad.org/PATCH-Add-function-pre-and-post-conditions-tp9493p9509.html
Sent from the OpenSCAD mailing list archive at Nabble.com.
_______________________________________________
OpenSCAD mailing list
[hidden email]
http://rocklinux.net/mailman/listinfo/openscad
http://openscad.org - https://flattr.com/thing/121566


_______________________________________________
OpenSCAD mailing list
[hidden email]
http://rocklinux.net/mailman/listinfo/openscad
http://openscad.org - https://flattr.com/thing/121566
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Add function pre- and post-conditions.

kintel
Administrator
In reply to this post by Taahir Ahmed
On Aug 27, 2014, at 03:39 AM, Taahir Ahmed <[hidden email]> wrote:

> Our previous discussion about asserts and testing in general led me to
> draw up this patch, […] If it's preferable,I can instead make a pull request on github.
>
Thanks for the patch!

I like the idea, but before including this in OpenSCAD I’d like to see more discussion on the topic.
In particular, how would this work in the context of modules. Since modules and function syntax is interleaved, perhaps we need a syntactically similar way of describing modules?

I’m planning to revamp the language a bit soon. I feel that these additions belong in a new language spec rather than being tagged onto the existing one. Once we have a clear idea of how the new syntax differs from the old, it would be easier to decide how to proceed and which new features can be backported to the old syntax in the shorter term.

I would suggest (once this discussion leads to some sort of consensus) posting a branch on github, and start a design document on the wiki as an OEP (https://github.com/openscad/openscad/wiki/OpenSCAD-Enhancement-Proposals).

Cheers,

 -Marius


_______________________________________________
OpenSCAD mailing list
[hidden email]
http://rocklinux.net/mailman/listinfo/openscad
http://openscad.org - https://flattr.com/thing/121566

signature.asc (210 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Add function pre- and post-conditions.

kintel
Administrator
In reply to this post by nophead
On Aug 28, 2014, at 10:08 AM, nop head <[hidden email]> wrote:

> Since the only side effect is to output a string (which has no effect on the rest of the script) the only issue is that the function results could be cached in future so it is only ever called once for each input value. That would mean you would not get the expected fecho output. The simple solution is to cache the the string produced by fecho() as well as the return value so that it can be output again without calling the function if it is called again with the same arguments.
>
> As mentioned before the same applies to echo() in modules if they get run in parallel.
>
I agree. Caching of function results is becoming more important now that more processing gets put into functions.
While the concept is simple, this is a fair bit of work to get right. We should probably open a new issue on the topic of function caching.

 -Marius

_______________________________________________
OpenSCAD mailing list
[hidden email]
http://rocklinux.net/mailman/listinfo/openscad
http://openscad.org - https://flattr.com/thing/121566
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Add function pre- and post-conditions.

Taahir Ahmed
In reply to this post by kintel
I have just picked a good random spot to reply, to address several
questions that were posed above.

Q: Should `pre`, `post`, etc. be namespaced?

A: In the current form, they are neither functions (and bare functions
   cannot appear in this context), nor are they statements like module
   children.

   They are more akin to decorators, in that they are explicitly
   recognized as providing metadata.

   Thus `doc` and `test` wouldn't be executed on every recursive call,
   since they are not functions.  Rather, the contents of `doc` would be
   used to provide contextual help, sort of like emacs `C-h f`.  `test`
   would only be invoked if the user requested it through the UI or via
   a command-line flag.

Q: How should this be extended to modules?

A: It's not quite clear --- the easiest way would probably be to add
   another set of '{' braces after the module body.  This would be
   pretty ugly.

   Another, more elegant way would be to make a new kind of decorator
   statement.  Then, on parsing, the compiler would pick up all of these
   decorator statements.

   In this case, it would be best to give them an unambiguous prefix.
   One easy one (and the one I already used for `__retval`), is the
   C/C++ convention that names prefixed with `__` (two underscored) are
   reserved for the compiler and standard library implementers.

Q: What about `debug`/`fecho`?

A: I think this might make more sense as a builtin function `__debug`
   that takes an arbitrary number of parameters and returns `true`.
   Then it can be worked directly into a function body:

   ```
   function f(x) = __debug("x=", x) ? x :;
   ```

   The question of output memoization has two approaches:

     1) The easy one: just emit a compile warning that debug output will
        not memoize correctly.

     2) Monad by another name: Instead of having all builtins use
        `PRINT`\`PRINTB` to do their output, give them a boost tee
        stream that copies to both a stringstream and to stdout.  Then
        memoize the string produced by the stringstream.

------------------------------------------------------------------------

I propose a new syntax that looks like this:

```
funtion fib(n) = (n == 0 || n == 1) ? 1 : fib(n-1) + fib(n-2) {
    __pre(n >= 0, "n must be nonnegative to ensure termination", __fatal);
    __pre(n < 50, "Large values of n will take a long time.", __warn);
    __post(__retval == round(__retval), "Accuracy lost.", __fatal);
    __test(fib(2) == 2, "fib(2) should be 2");
    __doc("Calculate the nth fibonacci number.");
}

module tube(ri, t, l) {
   __doc("Create a tube of length l, inner radius ri, and thickness t");
   __pre(ri >= 0, "ri must be nonnegative.", __fatal);
}
```

In this form, each decorator is a decorator-statement, so they are
followed by semicolons.  The compiler picks out decorator-statements for
special processing during parsing.  `__pre` and `__post` take an
optional third argument that can be `__fatal` or `__warn`, defaulting to
`__fatal`.

Functions gain a statement body, but it is a compile-time error for them
to contain anything other than decorator statements.

As my omission of `__post` and `__test` for the module case might
suggest, there are some open questions about what it means to test the
effects of a module.  I'm open to suggestions on this point.

Taahir Ahmed
_______________________________________________
OpenSCAD mailing list
[hidden email]
http://rocklinux.net/mailman/listinfo/openscad
http://openscad.org - https://flattr.com/thing/121566

signature.asc (484 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Add function pre- and post-conditions.

Taahir Ahmed
In reply to this post by kintel
> I’m planning to revamp the language a bit soon. I feel that these
> additions belong in a new language spec rather than being tagged onto
> the existing one. Once we have a clear idea of how the new syntax
> differs from the old, it would be easier to decide how to proceed and
> which new features can be backported to the old syntax in the shorter
> term.

For a revamp, might I suggest changing the parser infrastructure.

Both flex and bison have C++ support now.  In particular, bison knows
how to use a boost::variant workalike as a semantic value, so it's
possible to do away with %union and all the manual allocations in the
parser.

I'm working on a branch where I do just this, keeping the grammar the
same.  However, even in C++ output mode, flex is still very frightful to
work with, and I'm not sure that their two C++ modes are meant to work
together.

------------------------------------------------------------------------

Another alternative, perhaps a bit more palatable, it to use pegtl ---
it's a C++ template library for producing packrat parsers, which are a
sort of memoizing recursive descent parser.  Instead of LR/GLR/LALR,
they work with PEGs (parsing expression grammars).

The main difference between PEG and L*R is that the alternation operator
is ordered, rather than unordered, so there is no shift-reduce conflict.
In addition, packrat parsers also run in linear time, though they may
need to hold the entire input in memory.

The main selling point of pegtl is that you get full type safety and
RAII out of the box, rather than after a lot of terrible hacking at flex
and bison.  However, it does require C++11.

Taahir Ahmed
_______________________________________________
OpenSCAD mailing list
[hidden email]
http://rocklinux.net/mailman/listinfo/openscad
http://openscad.org - https://flattr.com/thing/121566

signature.asc (484 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Add function pre- and post-conditions.

kintel
Administrator
On Aug 28, 2014, at 14:04 PM, Taahir Ahmed <[hidden email]> wrote:
>
> For a revamp, might I suggest changing the parser infrastructure.
>
That wouldn’t hurt :)
Care should be taken to deal with backwards compatibility though.

> Both flex and bison have C++ support now.

Is this only available in bison 3? What limitation does that impose in terms of compiling on various platforms which may not have the latest versions of these tools? This might “just” be another build script challenge though.

> Another alternative, perhaps a bit more palatable, it to use pegtl —
> […] However, it does require C++11.
>
I’d love to move to C++11. We still need to figure out how realistic that is, for similar reasons as requiring bison 3.

 -Marius


_______________________________________________
OpenSCAD mailing list
[hidden email]
http://rocklinux.net/mailman/listinfo/openscad
http://openscad.org - https://flattr.com/thing/121566

signature.asc (210 bytes) Download Attachment
12