Guard against errors in linearity test. (#1964)

- Print better modcc errors
- Catch errors in symbolic diff in linearity test
- Abort upon such errors and advise for a different solver

modcc/error.hpp

@@ -71,6 +71,3 @@ public:
 private:
     error_entry error_info_;
 };
-
-
-

modcc/expression.hpp

@@ -18,6 +18,7 @@
 class Visitor;
 
 class ARB_LIBMODCC_API Expression;
+struct ARB_LIBMODCC_API ErrorExpression;
 class ARB_LIBMODCC_API CallExpression;
 class ARB_LIBMODCC_API BlockExpression;
 class ARB_LIBMODCC_API IfExpression;
@@ -209,6 +210,20 @@ protected:
     scope_ptr scope_;
 };
 
+struct ARB_LIBMODCC_API ErrorExpression : public Expression {
+    explicit ErrorExpression(Location location): Expression(location)
+    {}
+
+    std::string to_string() const override {
+        return "Error" + error_string_;
+    }
+
+    void accept(Visitor *) override {
+        throw compiler_exception{"Attempted to visit error expression.", location()};
+    }
+};
+
+
 class ARB_LIBMODCC_API Symbol : public Expression {
 public :
     Symbol(Location loc, std::string name, symbolKind kind)

modcc/modcc.cpp

@@ -25,12 +25,12 @@ using std::cerr;
 // Options and option parsing:
 
 int report_error(const std::string& message) {
-    cerr << red("error: ") << message << "\n";
+    cerr << red("error trace:\n") << message << "\n";
     return 1;
 }
 
 int report_ice(const std::string& message) {
-    cerr << red("internal compiler error: ") << message << "\n"
+    cerr << red("internal compiler error:\n") << message << "\n"
          << "\nPlease report this error to the modcc developers.\n";
     return 1;
 }
@@ -223,6 +223,7 @@ int main(int argc, char **argv) {
         emit_header("semantic analysis");
         m.semantic();
         if (m.has_warning()) {
+            cerr << yellow("Warnings:\n");
             cerr << m.warning_string() << "\n";
         }
         if (m.has_error()) {

modcc/module.cpp

@@ -119,7 +119,7 @@ std::string Module::error_string() const {
     std::string str;
     for (const error_entry& entry: errors()) {
         if (!str.empty()) str += '\n';
-        str += red("error   ");
+        str += red("  * ");
         str += white(pprintf("%:% ", source_name(), entry.location));
         str += entry.message;
     }
@@ -130,7 +130,7 @@ std::string Module::warning_string() const {
     std::string str;
     for (auto& entry: warnings()) {
         if (!str.empty()) str += '\n';
-        str += purple("warning   ");
+        str += purple("  * ");
         str += white(pprintf("%:% ", source_name(), entry.location));
         str += entry.message;
     }

modcc/solvers.cpp

@@ -41,7 +41,13 @@ void CnexpSolverVisitor::visit(AssignmentExpression *e) {
     }
 
     auto s = deriv->name();
+
     linear_test_result r = linear_test(rhs, dvars_);
+    if (r.has_error()) {
+        append_errors(r.errors());
+        error({"CNExp: Could not determine linearity, maybe use a different solver?", loc});
+        return;
+    }
 
     if (!r.monolinear(s)) {
         error({"System not diagonal linear for cnexp", loc});
@@ -52,7 +58,6 @@ void CnexpSolverVisitor::visit(AssignmentExpression *e) {
     if (!coef || is_zero(coef)) {
         // s' = b becomes s = s + b*dt; use b_ as a local variable for
         // the constant term b.
-
         auto local_b_term = make_unique_local_assign(scope, r.constant.get(), "b_");
         statements_.push_back(std::move(local_b_term.local_decl));
         statements_.push_back(std::move(local_b_term.assignment));

modcc/symdiff.cpp

@@ -562,7 +562,17 @@ ARB_LIBMODCC_API expression_ptr symbolic_pdiff(Expression* e, const std::string&
     SymPDiffVisitor pdiff_visitor(id);
     e->accept(&pdiff_visitor);
 
-    if (pdiff_visitor.has_error()) return nullptr;
+    if (pdiff_visitor.has_error()) {
+        std::string errors, sep = "";
+
+        for (const auto& error: pdiff_visitor.errors()) {
+            errors += sep + error.message;
+            sep = "\n";
+        }
+        auto res = std::make_unique<ErrorExpression>(e->location());
+        res->error(errors);
+        return res;
+    }
 
     return constant_simplify(pdiff_visitor.result());
 }
@@ -666,17 +676,20 @@ ARB_LIBMODCC_API linear_test_result linear_test(Expression* e, const std::vector
     result.constant = e->clone();
     for (const auto& id: vars) {
         auto coef = symbolic_pdiff(e, id);
-        if (!coef) {
-            return linear_test_result{};
+        if (coef->has_error()) {
+            auto res = linear_test_result{};
+            res.error({coef->error_message(), loc});
+            return res;
         }
+        if (!coef) return linear_test_result{};
         if (!is_zero(coef)) result.coef[id] = std::move(coef);
-
         result.constant = substitute(result.constant, id, zero());
     }
 
     ConstantSimplifyVisitor csimp_visitor;
     result.constant->accept(&csimp_visitor);
     result.constant = csimp_visitor.result();
+    if (result.constant.get() == nullptr) throw compiler_exception{"Linear test: simplification of the constant term failed.", loc};
 
     // linearity test: take second order derivatives, test against zero.
     result.is_linear = true;

modcc/symdiff.hpp

@@ -81,7 +81,7 @@ inline expression_ptr substitute(const expression_ptr& e, const substitute_map&
 
 // Linearity testing
 
-struct linear_test_result {
+struct linear_test_result: public error_stack {
     bool is_linear = false;
     bool is_homogeneous = false;
     expression_ptr constant;

test/unit-modcc/mod_files/bug-1893-bad.mod

+NEURON {
+    POINT_PROCESS bug_1893
+}
+
+INITIAL {
+    c = 0
+    rho = 0
+    theta_p = 0
+}
+
+STATE {
+    c
+    rho
+    theta_p
+}
+
+PARAMETER {
+    tau_c = 150 (ms)
+}
+
+BREAKPOINT {
+    SOLVE state METHOD cnexp
+}
+
+DERIVATIVE state {
+    c' = -c/tau_c
+    rho' = (c - theta_p) > 0
+}

test/unit-modcc/mod_files/bug-1893.mod

+NEURON {
+    POINT_PROCESS bug_1893
+}
+
+INITIAL {
+    c = 0
+    rho = 0
+    theta_p = 0
+}
+
+STATE {
+    c
+    rho
+    theta_p
+}
+
+PARAMETER {
+    tau_c = 150 (ms)
+}
+
+BREAKPOINT {
+    SOLVE state METHOD sparse
+}
+
+DERIVATIVE state {
+    c' = -c/tau_c
+    rho' = (c - theta_p) > 0
+}

test/unit-modcc/test_module.cpp

@@ -122,3 +122,24 @@ TEST(Module, read_write_ion) {
     EXPECT_TRUE(p.parse());
     EXPECT_TRUE(m.semantic());
 }
+
+// Regression test in #1893 we found that the solver segfaults when handed a
+// naked comparison statement.
+TEST(Module, solver_bug_1893) {
+    {
+        Module m(io::read_all(DATADIR "/mod_files/bug-1893.mod"), "bug-1893.mod");
+        EXPECT_NE(m.buffer().size(), 0);
+
+        Parser p(m, false);
+        EXPECT_TRUE(p.parse());
+        EXPECT_TRUE(m.semantic());
+    }
+    {
+        Module m(io::read_all(DATADIR "/mod_files/bug-1893-bad.mod"), "bug-1893.mod");
+        EXPECT_NE(m.buffer().size(), 0);
+
+        Parser p(m, false);
+        EXPECT_TRUE(p.parse());
+        EXPECT_FALSE(m.semantic());
+    }
+}