Improve generated C++ header files in the nmodl::ast namespace (#1080)

* Improve generated C++ headers:
  * Use nested namespace definitions
  * Improve indentation for readability
  * Prefer passing by reference rather than copy when possible for `std::shared_ptr` and `std::vector`
* CI:
  * CodeCov: ignore jinja templates
  * SonarSource: ignore a couple of directories / files (submodules, ...)
* in bison parser, catch exception by reference, not copy

codecov.yaml

@@ -0,0 +1,2 @@
+ignore:
+- src/language/templates/

docs/DoxygenLayout.xml

@@ -4,7 +4,6 @@
   <!-- Navigation index tabs for HTML output -->
   <navindex>
     <tab type="mainpage" visible="yes" title="Overview"/>
-    <!-- <tab type="pages" visible="yes" title="Tutorials" intro=""/> -->
     <tab type="modules" visible="yes" title="Components" intro=""/>
     <tab type="namespaces" visible="yes" title="">
       <tab type="namespacelist" visible="yes" title="" intro=""/>

sonar-project.properties

@@ -6,6 +6,7 @@ sonar.organization=bluebrain
 #sonar.projectVersion=1.0
 
 sonar.python.version=3.10.12
+sonar.exclusions=ext/**, nmodl/ext/**, docs/notebooks/tree.js
 
 # Path is relative to the sonar-project.properties file. Replace "\" by "/" on Windows.
 #sonar.sources=.

src/codegen/codegen_cpp_visitor.cpp

@@ -473,7 +473,6 @@ bool CodegenCppVisitor::need_semicolon(const Statement& node) {
         || node.is_else_statement()
         || node.is_from_statement()
         || node.is_verbatim()
-        || node.is_from_statement()
         || node.is_conductance_hint()
         || node.is_while_statement()
         || node.is_protect_statement()

src/codegen/codegen_transform_visitor.cpp

@@ -21,7 +21,8 @@ void CodegenTransformVisitor::visit_function_block(FunctionBlock& node) {
     auto table_statements = collect_nodes(node, {AstNodeType::TABLE_STATEMENT});
     for (auto t: table_statements) {
         auto t_ = std::dynamic_pointer_cast<TableStatement>(t);
-        t_->set_table_vars({std::make_shared<Name>(new String(node.get_node_name()))});
+        t_->set_table_vars(
+            {std::make_shared<Name>(std::make_shared<String>(node.get_node_name()))});
     }
 }
 }  // namespace nmodl

src/language/nodes.py

@@ -199,6 +199,8 @@ def get_typename(self):
     def get_rvalue_typename(self):
         """returns rvalue reference type of the node"""
         typename = self.get_typename()
+        if self.is_vector:
+            return "const " + typename + "&"
         if self.is_base_type_node and not self.is_integral_type_node or self.is_ptr_excluded_node:
             return "const " + typename + "&"
         return typename
@@ -242,63 +244,63 @@ def _is_member_type_wrapped_as_shared_pointer(self):
     def member_rvalue_typename(self):
         """returns rvalue reference type when used as returned or parameter type"""
         typename = self.member_typename
-        if not self.is_integral_type_node and not self._is_member_type_wrapped_as_shared_pointer:
+        if not self.is_integral_type_node:
             return "const " + typename + "&"
         return typename
 
     def get_add_methods_declaration(self):
         s = ''
         if self.add_method:
             method = f"""
-                         /**
-                          * \\brief Add member to {self.varname} by raw pointer
-                          */
-                         void emplace_back_{to_snake_case(self.class_name)}({self.class_name} *n);
-
-                         /**
-                          * \\brief Add member to {self.varname} by shared_ptr
-                          */
-                         void emplace_back_{to_snake_case(self.class_name)}(std::shared_ptr<{self.class_name}> n);
-
-                         /**
-                          * \\brief Erase member to {self.varname}
-                          */
-                         {self.class_name}Vector::const_iterator erase_{to_snake_case(self.class_name)}({self.class_name}Vector::const_iterator first);
-
-                         /**
-                          * \\brief Erase members to {self.varname}
-                          */
-                         {self.class_name}Vector::const_iterator erase_{to_snake_case(self.class_name)}({self.class_name}Vector::const_iterator first, {self.class_name}Vector::const_iterator last);
-
-                         /**
-                          * \\brief Erase non-consecutive members to {self.varname}
-                          *
-                          * loosely following the cpp reference of remove_if
-                          */
-                         size_t erase_{to_snake_case(self.class_name)}(std::unordered_set<{self.class_name}*>& to_be_erased);
-
-                         /**
-                          * \\brief Insert member to {self.varname}
-                          */
-                         {self.class_name}Vector::const_iterator insert_{to_snake_case(self.class_name)}({self.class_name}Vector::const_iterator position, const std::shared_ptr<{self.class_name}>& n);
-
-                         /**
-                          * \\brief Insert members to {self.varname}
-                          */
-                         template <class NodeType, class InputIterator>
-                         void insert_{to_snake_case(self.class_name)}({self.class_name}Vector::const_iterator position, NodeType& to, InputIterator first, InputIterator last);
-
-                         /**
-                          * \\brief Reset member to {self.varname}
-                          */
-                         void reset_{to_snake_case(self.class_name)}({self.class_name}Vector::const_iterator position, {self.class_name}* n);
-
-                         /**
-                          * \\brief Reset member to {self.varname}
-                          */
-                         void reset_{to_snake_case(self.class_name)}({self.class_name}Vector::const_iterator position, std::shared_ptr<{self.class_name}> n);
-                    """
-            s = textwrap.dedent(method)
+              /**
+               * \\brief Add member to {self.varname} by raw pointer
+               */
+              void emplace_back_{to_snake_case(self.class_name)}({self.class_name} *n);
+
+              /**
+               * \\brief Add member to {self.varname} by shared_ptr
+               */
+              void emplace_back_{to_snake_case(self.class_name)}(std::shared_ptr<{self.class_name}> n);
+
+              /**
+               * \\brief Erase member to {self.varname}
+               */
+              {self.class_name}Vector::const_iterator erase_{to_snake_case(self.class_name)}({self.class_name}Vector::const_iterator first);
+
+              /**
+               * \\brief Erase members to {self.varname}
+               */
+              {self.class_name}Vector::const_iterator erase_{to_snake_case(self.class_name)}({self.class_name}Vector::const_iterator first, {self.class_name}Vector::const_iterator last);
+
+              /**
+               * \\brief Erase non-consecutive members to {self.varname}
+               *
+               * loosely following the cpp reference of remove_if
+               */
+              size_t erase_{to_snake_case(self.class_name)}(std::unordered_set<{self.class_name}*>& to_be_erased);
+
+              /**
+               * \\brief Insert member to {self.varname}
+               */
+              {self.class_name}Vector::const_iterator insert_{to_snake_case(self.class_name)}({self.class_name}Vector::const_iterator position, const std::shared_ptr<{self.class_name}>& n);
+
+              /**
+               * \\brief Insert members to {self.varname}
+               */
+              template <class NodeType, class InputIterator>
+              void insert_{to_snake_case(self.class_name)}({self.class_name}Vector::const_iterator position, NodeType& to, InputIterator first, InputIterator last);
+
+              /**
+               * \\brief Reset member to {self.varname}
+               */
+              void reset_{to_snake_case(self.class_name)}({self.class_name}Vector::const_iterator position, {self.class_name}* n);
+
+              /**
+               * \\brief Reset member to {self.varname}
+               */
+              void reset_{to_snake_case(self.class_name)}({self.class_name}Vector::const_iterator position, std::shared_ptr<{self.class_name}> n);
+            """
+            s = textwrap.indent(textwrap.dedent(method), '  ')
         return s
 
     def get_add_methods_definition(self, parent):
@@ -440,12 +442,12 @@ def get_node_name_method_declaration(self):
                   * in case of this ast::{self.class_name} has {self.varname} designated as a
                   * node name.
                   *
-                  * @return name of the node as std::string
+                  * \\return name of the node as std::string
                   *
                   * \\sa Ast::get_node_type_name
                   */
                  std::string get_node_name() const override;"""
-            s = textwrap.dedent(method)
+            s = textwrap.indent(textwrap.dedent(method), '  ')
         return s
 
     def get_node_name_method_definition(self, parent):
@@ -461,42 +463,40 @@ def get_node_name_method_definition(self, parent):
 
     def get_getter_method(self, class_name):
         getter_method = self.getter_method if self.getter_method else "get_" + to_snake_case(self.varname)
-        getter_override = " override" if self.getter_override else ""
+        getter_override = "override" if self.getter_override else ""
         return_type = self.member_rvalue_typename
-        return f"""
-                   /**
-                    * \\brief Getter for member variable \\ref {class_name}.{self.varname}
-                    */
-                   {return_type} {getter_method}() const noexcept {getter_override} {{
-                       return {self.varname};
-                   }}
-                """
+        return textwrap.indent(textwrap.dedent(f"""
+          /**
+           * \\brief Getter for member variable \\ref {class_name}.{self.varname}
+           */
+          {return_type} {getter_method}() const noexcept {getter_override} {{
+            return {self.varname};
+          }}
+        """), '  ')
 
     def get_setter_method_declaration(self, class_name):
         setter_method = "set_" + to_snake_case(self.varname)
         setter_type = self.member_typename
         reference = "" if self.is_base_type_node else "&&"
         if self.is_base_type_node:
-            return f"""
-                       /**
-                        * \\brief Setter for member variable \\ref {class_name}.{self.varname}
-                        */
-                       void {setter_method}({setter_type} {self.varname});
-                    """
+            return textwrap.indent(textwrap.dedent(f"""
+              /**
+               * \\brief Setter for member variable \\ref {class_name}.{self.varname}
+               */
+              void {setter_method}({setter_type} {self.varname});
+            """), '  ')
         else:
-            return f"""
-                       /**
-                        * \\brief Setter for member variable \\ref {class_name}.{self.varname} (rvalue reference)
-                        */
-                       void {setter_method}({setter_type}&& {self.varname});
-
-                       /**
-                        * \\brief Setter for member variable \\ref {class_name}.{self.varname}
-                        */
-                       void {setter_method}(const {setter_type}& {self.varname});
-                    """
-
-
+            return textwrap.indent(textwrap.dedent(f"""
+              /**
+               * \\brief Setter for member variable \\ref {class_name}.{self.varname} (rvalue reference)
+               */
+              void {setter_method}({setter_type}&& {self.varname});
+
+              /**
+               * \\brief Setter for member variable \\ref {class_name}.{self.varname}
+               */
+              void {setter_method}(const {setter_type}& {self.varname});
+            """), '  ')
 
     def get_setter_method_definition(self, class_name):
         setter_method = "set_" + to_snake_case(self.varname)

src/language/templates/ast/ast.cpp

@@ -17,8 +17,7 @@
  * \brief Auto generated AST classes implementations
  */
 
-namespace nmodl {
-namespace ast {
+namespace nmodl::ast {
 
 ///
 ///  Ast member function definition
@@ -30,7 +29,7 @@ std::string Ast::get_node_name() const {
   throw std::logic_error("get_node_name() not implemented");
 }
 
-std::shared_ptr<StatementBlock> Ast::get_statement_block() const {
+const std::shared_ptr<StatementBlock>& Ast::get_statement_block() const {
   throw std::runtime_error("get_statement_block not implemented");
 }
 
@@ -219,7 +218,5 @@ void Ast::set_parent(Ast* p) {
   {% endfor %}
 
   {% endfor %}
-
-}  // namespace ast
-}  // namespace nmodl
+}  // namespace nmodl::ast
 

src/language/templates/ast/ast.hpp

@@ -29,8 +29,7 @@
 #include "utils/common_utils.hpp"
 #include "visitors/visitor.hpp"
 
-namespace nmodl {
-namespace ast {
+namespace nmodl::ast {
 
 /**
  * \page ast_design Design of Abstract Syntax Tree (AST)
@@ -50,10 +49,10 @@ namespace ast {
  */
 
 /**
- * @defgroup ast_class AST Classes
- * @ingroup ast
- * @brief Classes for implementing Abstract Syntax Tree (AST)
- * @{
+ * \defgroup ast_class AST Classes
+ * \ingroup ast
+ * \brief Classes for implementing Abstract Syntax Tree (AST)
+ * \{
  */
 
 /**
@@ -122,7 +121,7 @@ struct Ast: public std::enable_shared_from_this<Ast> {
    * This type name can be returned as a std::string for printing
    * ast to text/json form.
    *
-   * @return name of the node type as a string
+   * \return name of the node type as a string
    *
    * \sa Ast::get_node_name
    */
@@ -273,7 +272,7 @@ struct Ast: public std::enable_shared_from_this<Ast> {
    *
    * \sa ast::StatementBlock
    */
-  virtual std::shared_ptr<StatementBlock> get_statement_block() const;
+  virtual const std::shared_ptr<StatementBlock>& get_statement_block() const;
 
   /**
    * \brief Set symbol table for the AST node
@@ -342,7 +341,7 @@ struct Ast: public std::enable_shared_from_this<Ast> {
   /**
    *\brief Parent setter
    *
-   * Usually, the parent parent pointer cannot be set in the constructor
+   * Usually, the parent pointer cannot be set in the constructor
    * because children are generally build BEFORE the parent. Conversely,
    * we set children parents directly in the parent constructor using
    * set_parent_in_children()
@@ -352,5 +351,4 @@ struct Ast: public std::enable_shared_from_this<Ast> {
   virtual void set_parent(Ast* p);
 };
 
-}  // namespace ast
-}  // namespace nmodl
+}  // namespace nmodl::ast