From 6c89dc639c1ee59e8cfdd69719780e0185f1ec08 Mon Sep 17 00:00:00 2001 From: Nicolas Cornu Date: Wed, 22 Nov 2023 08:34:22 +0100 Subject: [PATCH] Semantic: check there is at most one DERIVATIVE block (#1098) This is not forbidden to have more, but due to a bug this fail in nrn. Disable shalow clone as git describe --tags might fail with > fatal: No names found, cannot describe anything. --------- Co-authored-by: Pramod S Kumbhar --- .github/workflows/nmodl-doc.yml | 2 ++ src/visitors/semantic_analysis_visitor.cpp | 31 ++++++++-------------- src/visitors/semantic_analysis_visitor.hpp | 12 +++------ test/unit/visitor/semantic_analysis.cpp | 31 +++++++--------------- 4 files changed, 27 insertions(+), 49 deletions(-) diff --git a/.github/workflows/nmodl-doc.yml b/.github/workflows/nmodl-doc.yml index 0c8481b2b7..56a8df8e86 100644 --- a/.github/workflows/nmodl-doc.yml +++ b/.github/workflows/nmodl-doc.yml @@ -52,6 +52,8 @@ jobs: python-version: ${{ env.PYTHON_VERSION }} - uses: actions/checkout@v3 + with: + fetch-depth: 0 - name: Install Python3 dependencies working-directory: ${{runner.workspace}}/nmodl diff --git a/src/visitors/semantic_analysis_visitor.cpp b/src/visitors/semantic_analysis_visitor.cpp index 365fac7ad1..22220f3b8d 100644 --- a/src/visitors/semantic_analysis_visitor.cpp +++ b/src/visitors/semantic_analysis_visitor.cpp @@ -60,6 +60,17 @@ bool SemanticAnalysisVisitor::check(const ast::Program& node) { return check_fail; } +void SemanticAnalysisVisitor::visit_program(const ast::Program& node) { + /// <-- This code is for check 8 + const auto& derivative_block_nodes = collect_nodes(node, {ast::AstNodeType::DERIVATIVE_BLOCK}); + if (derivative_block_nodes.size() > 1) { + logger->critical("It is not supported to have several DERIVATIVE blocks"); + check_fail = true; + } + /// --> + node.visit_children(*this); +} + void SemanticAnalysisVisitor::visit_procedure_block(const ast::ProcedureBlock& node) { /// <-- This code is for check 1 in_procedure = true; @@ -171,25 +182,5 @@ void SemanticAnalysisVisitor::visit_mutex_unlock(const ast::MutexUnlock& /* node /// --> } -void SemanticAnalysisVisitor::visit_breakpoint_block(const ast::BreakpointBlock& node) { - /// <-- This code is for check 8 - solve_block_found_in_this_breakpoint_block = false; - node.visit_children(*this); - solve_block_found_in_this_breakpoint_block = false; - /// --> -} - -void SemanticAnalysisVisitor::visit_solve_block(const ast::SolveBlock& /* node */) { - /// <-- This code is for check 8 - if (solve_block_found_in_this_breakpoint_block) { - logger->critical( - "It is not allowed to have several solve blocks in the same breakpoint block"); - check_fail = true; - } else { - solve_block_found_in_this_breakpoint_block = true; - } - /// --> -} - } // namespace visitor } // namespace nmodl diff --git a/src/visitors/semantic_analysis_visitor.hpp b/src/visitors/semantic_analysis_visitor.hpp index f51b10dfed..1f9c2b7e98 100644 --- a/src/visitors/semantic_analysis_visitor.hpp +++ b/src/visitors/semantic_analysis_visitor.hpp @@ -31,7 +31,7 @@ * 5. Check if an independent variable is not 't'. * 6. Check that mutex are not badly use * 7. Check than function table got at least one argument. - * 8. Check that at most one solve block is present per breakpoint block. + * 8. Check that at most one derivative block is present. */ #include "ast/ast.hpp" #include "visitors/ast_visitor.hpp" @@ -55,8 +55,9 @@ class SemanticAnalysisVisitor: public ConstAstVisitor { bool is_point_process = false; /// true if we are inside a mutex locked part bool in_mutex = false; - /// true if we already found a solve block - bool solve_block_found_in_this_breakpoint_block = false; + + /// Check number of DERIVATIVE blocks + void visit_program(const ast::Program& node) override; /// Store if we are in a procedure and if the arity of this is 1 void visit_procedure_block(const ast::ProcedureBlock& node) override; @@ -85,11 +86,6 @@ class SemanticAnalysisVisitor: public ConstAstVisitor { /// Look if MUTEXUNLOCK is outside a locked block void visit_mutex_unlock(const ast::MutexUnlock& node) override; - void visit_breakpoint_block(const ast::BreakpointBlock& node) override; - - /// Check how many solve block we got - void visit_solve_block(const ast::SolveBlock& node) override; - public: SemanticAnalysisVisitor(bool accel_backend = false) : accel_backend(accel_backend) {} diff --git a/test/unit/visitor/semantic_analysis.cpp b/test/unit/visitor/semantic_analysis.cpp index 0d51aeb758..df3a6f4452 100644 --- a/test/unit/visitor/semantic_analysis.cpp +++ b/test/unit/visitor/semantic_analysis.cpp @@ -167,38 +167,27 @@ SCENARIO("FUNCTION_TABLE block", "[visitor][semantic_analysis]") { } -SCENARIO("At most one solve block per breakpoint block", "[visitor][semantic_analysis]") { - GIVEN("A breakpoint block with only one solve block") { +SCENARIO("At most one DERIVATIVE block", "[visitor][semantic_analysis]") { + GIVEN("Only one DERIVATIVE block") { std::string nmodl_text = R"( - BREAKPOINT { - SOLVE dX METHOD cnexp + DERIVATIVE states { + m' = m/mTau } )"; THEN("Semantic analysis should success") { REQUIRE_FALSE(run_semantic_analysis_visitor(nmodl_text)); } } - GIVEN("2 breakpoints block with one solve block each") { + GIVEN("2 DERIVATIVE blocks") { std::string nmodl_text = R"( - PROCEDURE foo() { - SOLVE dX METHOD cnexp + DERIVATIVE states1 { + m' = m/mTau } - BREAKPOINT { - SOLVE dY METHOD cnexp + DERIVATIVE states2 { + h' = h/hTau } )"; - THEN("Semantic analysis should success") { - REQUIRE_FALSE(run_semantic_analysis_visitor(nmodl_text)); - } - } - GIVEN("A breakpoint block with two solve blocks") { - std::string nmodl_text = R"( - BREAKPOINT { - SOLVE dX METHOD cnexp - SOLVE dY METHOD cnexp - } - )"; - THEN("Semantic analysis should fail") { + THEN("Semantic analysis should failed") { REQUIRE(run_semantic_analysis_visitor(nmodl_text)); } }