In-source template expansions

.vscode/settings.json

@@ -6,10 +6,11 @@
     "search.exclude": {
         "out": true // set this to false to include "out" folder in search results
     },
-    "python-envs.defaultEnvManager": "ms-python.python:system",
+    "python-envs.defaultEnvManager": "ms-python.python:venv",
     "python.defaultInterpreterPath": "${workspaceFolder}/fpp_python/venv/bin/python",
     "python.analysis.extraPaths": [
-        "${workspaceFolder}/fpp_python"
+        "${workspaceFolder}/fpp_python",
+        "./fpp_python/python"
     ],
     "python.testing.pytestArgs": [
         "fpp_python"

fpp_analysis/src/analysis.rs

@@ -104,12 +104,16 @@ pub struct Analysis {
     pub(crate) tlm_packet_set: Option<TlmPacketSet>,
     /// The mapping from nodes to implied uses
     pub implied_use_map: HashMap<fpp_core::Node, ImpliedUseSet>,
-    /// The mapping from template expansion specifier nodes to their expansions,
-    /// in the order the expansions are entered
-    pub template_expansion_map: IndexMap<fpp_core::Node, TemplateExpansion>,
+    /// Node of the current template definition
+    pub(crate) template_def_id: Option<fpp_core::Node>,
     /// The node of the template expansion specifier whose expansion we are
     /// currently analyzing inside of
     pub(crate) template_expansion: Option<fpp_core::Node>,
+    /// Node of the current template expansion specifier
+    pub(crate) template_expansion_id: Option<fpp_core::Node>,
+    /// The mapping from template expansion specifier nodes to their expansions,
+    /// in the order the expansions are entered
+    pub template_expansion_map: IndexMap<fpp_core::Node, TemplateExpansion>,
     /// The set of symbols defined with a dictionary specifier
     pub dictionary_symbol_set: HashSet<Symbol>,
     /// The mapping from system symbols to systems
@@ -174,8 +178,10 @@ impl Analysis {
             dictionary: None,
             tlm_packet_set: None,
             implied_use_map: Default::default(),
-            template_expansion_map: Default::default(),
+            template_def_id: None,
             template_expansion: None,
+            template_expansion_id: None,
+            template_expansion_map: Default::default(),
             dictionary_symbol_set: Default::default(),
             system_map: Default::default(),
             unknown_type: None,

fpp_analysis/src/analyzers/module_analyzer.rs

@@ -41,6 +41,11 @@ where
 /// names inside it resolve against the definitions the expansion introduced
 /// rather than same-named definitions at the expansion site.
 ///
+/// The members are visited only once the analysis has recorded the expansion,
+/// which [`crate::passes::EnterTemplateExpansionSymbols`] does. Before that, the
+/// members of an expansion written in the source, in the form `expand T { ... }`,
+/// have no expansion to resolve against, so they are not visited.
+///
 /// This is the analysis of a template expansion specifier by
 /// [`ModuleAnalyzer`]. A pass that visits module members without a
 /// [`ModuleAnalyzer`] calls it from [`Visitor::visit_spec_template_expand`].
@@ -52,11 +57,15 @@ pub fn visit_expansion<'ast, V>(
 where
     V: Visitor<'ast, State = Analysis>,
 {
-    // Check if this template has been expanded
     let Some(members) = &node.members else {
         return ControlFlow::Continue(());
     };
-    // Mark the analysis with the proper expansion
+    if !a.template_expansion_map.contains_key(&node.node_id) {
+        // This node has not yet been expanded by analysis
+        return ControlFlow::Continue(());
+    }
+    // This node has been expanded by analysis.
+    // Mark the analysis with the proper expansion.
     let saved = a.template_expansion.replace(node.node_id);
     let out = members.visit(a, visitor);
     a.template_expansion = saved;

fpp_analysis/src/errors.rs

@@ -402,15 +402,17 @@ pub enum SemanticError {
         param_loc: Span,
         def_loc: Span,
     },
-    /// Nested template definition
-    NestedTemplateDefinition {
-        nested_loc: Span,
-        parent_loc: Span,
+    /// Error in a member of a module template definition
+    TemplateDefError {
+        loc: Span,
+        msg: String,
+        def_loc: Span,
     },
-    /// Template expansion specifier nested inside a template
-    TemplateExpansionInTemplate {
+    /// Error in a member of a template expansion
+    TemplateExpandError {
+        loc: Span,
+        msg: String,
         expand_loc: Span,
-        template_loc: Span,
     },
     /// Missing async input
     MissingAsync {
@@ -1195,24 +1197,16 @@ impl From<SemanticError> for Diagnostic {
                 ),
             )
             .span_note(def_loc, "conflicting definition is here"),
-            SemanticError::NestedTemplateDefinition {
-                nested_loc,
-                parent_loc,
-            } => Diagnostic::new(
-                nested_loc,
-                Level::Error,
-                "module template definitions cannot be nested",
-            )
-            .span_note(parent_loc, "parent template defined here"),
-            SemanticError::TemplateExpansionInTemplate {
-                expand_loc,
-                template_loc,
-            } => Diagnostic::new(
+            SemanticError::TemplateDefError { loc, msg, def_loc } => {
+                Diagnostic::new(loc, Level::Error, msg)
+                    .span_note(def_loc, "enclosing template is defined here")
+            }
+            SemanticError::TemplateExpandError {
+                loc,
+                msg,
                 expand_loc,
-                Level::Error,
-                "template expansion specifiers cannot appear inside a module template",
-            )
-            .span_note(template_loc, "enclosing template defined here"),
+            } => Diagnostic::new(loc, Level::Error, msg)
+                .span_note(expand_loc, "enclosing template expansion is here"),
             SemanticError::MissingAsync { kind, loc } => Diagnostic::new(
                 loc,
                 Level::Error,

fpp_analysis/src/lib.rs

@@ -92,6 +92,12 @@ pub mod passes {
     mod check_uses_helpers;
     pub use check_uses_helpers::*;
 
+    mod check_template_defs;
+    pub use check_template_defs::*;
+
+    mod check_template_expansions;
+    pub use check_template_expansions::*;
+
     mod check_template_uses;
     pub use check_template_uses::*;
 
@@ -336,8 +342,8 @@ pub fn resolve_includes<Reader: FileReader>(
 /// driver that resolves the includes and adds the state enums of each
 /// translation unit itself ([`resolve_includes`], [`add_state_enums`]). It
 /// runs before [`check_semantics`], on the same analysis, and it rewrites the
-/// translation units, replacing each template expansion specifier with the
-/// expanded template. A `Break` result means the caller must not go on to
+/// translation units, replacing each unexpanded template expansion specifier
+/// with the expanded template. A `Break` result means the caller must not go on to
 /// [`check_semantics`].
 pub fn resolve_templates(a: &mut Analysis, tul: &mut [fpp_ast::TransUnit]) -> ControlFlow<()> {
     ExpandSyntaxAndEnterSymbols::enter_symbols_and_expand_templates(a, tul)

fpp_analysis/src/passes/check_template_defs.rs

@@ -0,0 +1,72 @@
+use crate::Analysis;
+use crate::errors::SemanticError;
+use fpp_ast::{
+    DefModule, DefModuleTemplate, SpecTemplateExpand, TransUnit, Visitable, Visitor, Walkable,
+};
+use fpp_core::Spanned;
+use std::ops::ControlFlow;
+
+/// Check template definitions
+///
+/// No module template definition may contain another module template definition
+/// or a template expansion specifier, so expanding a template never defines
+/// another template and never causes another expansion.
+///
+/// The first error stops the pass: template expansion relies on these rules.
+pub struct CheckTemplateDefs;
+
+impl<'ast> Visitor<'ast> for CheckTemplateDefs {
+    type Break = ();
+    type State = Analysis;
+
+    fn visit_trans_unit(&self, a: &mut Analysis, node: &'ast TransUnit) -> ControlFlow<()> {
+        node.walk(a, self)
+    }
+
+    fn visit_def_module(&self, a: &mut Analysis, def: &'ast DefModule) -> ControlFlow<()> {
+        def.walk(a, self)
+    }
+
+    fn visit_def_module_template(
+        &self,
+        a: &mut Analysis,
+        def: &'ast DefModuleTemplate,
+    ) -> ControlFlow<()> {
+        if let Some(def_id) = a.template_def_id {
+            SemanticError::TemplateDefError {
+                loc: def.span(),
+                msg: "module template definitions cannot be nested".to_string(),
+                def_loc: def_id.span(),
+            }
+            .emit();
+            return ControlFlow::Break(());
+        }
+        a.template_def_id = Some(def.node_id);
+        let result = def
+            .members
+            .iter()
+            .try_for_each(|member| member.visit(a, self));
+        a.template_def_id = None;
+        result
+    }
+
+    fn visit_spec_template_expand(
+        &self,
+        a: &mut Analysis,
+        node: &'ast SpecTemplateExpand,
+    ) -> ControlFlow<()> {
+        match a.template_def_id {
+            Some(def_id) => {
+                SemanticError::TemplateDefError {
+                    loc: node.span(),
+                    msg: "template expansion specifiers cannot appear inside a module template"
+                        .to_string(),
+                    def_loc: def_id.span(),
+                }
+                .emit();
+                ControlFlow::Break(())
+            }
+            None => ControlFlow::Continue(()),
+        }
+    }
+}

fpp_analysis/src/passes/check_template_expansions.rs

@@ -0,0 +1,86 @@
+use crate::Analysis;
+use crate::errors::SemanticError;
+use fpp_ast::{
+    DefModule, DefModuleTemplate, SpecTemplateExpand, TransUnit, Visitable, Visitor, Walkable,
+};
+use fpp_core::Spanned;
+use std::ops::ControlFlow;
+
+/// Check template expansions
+///
+/// The members of an expanded template, whether written in the source or produced
+/// by template expansion, may contain no module template definition and no
+/// template expansion specifier. The rule ensures that the members written in the
+/// source could have been produced by expanding some template.
+///
+/// The first error stops the pass: template expansion relies on these rules.
+pub struct CheckTemplateExpansions;
+
+impl CheckTemplateExpansions {
+    fn spec_template_expand_unexpanded(
+        &self,
+        a: &mut Analysis,
+        node: &SpecTemplateExpand,
+    ) -> ControlFlow<()> {
+        match a.template_expansion_id {
+            Some(expand_id) => {
+                SemanticError::TemplateExpandError {
+                    loc: node.span(),
+                    msg: "template expansions cannot be nested".to_string(),
+                    expand_loc: expand_id.span(),
+                }
+                .emit();
+                ControlFlow::Break(())
+            }
+            None => ControlFlow::Continue(()),
+        }
+    }
+}
+
+impl<'ast> Visitor<'ast> for CheckTemplateExpansions {
+    type Break = ();
+    type State = Analysis;
+
+    fn visit_trans_unit(&self, a: &mut Analysis, node: &'ast TransUnit) -> ControlFlow<()> {
+        node.walk(a, self)
+    }
+
+    fn visit_def_module(&self, a: &mut Analysis, def: &'ast DefModule) -> ControlFlow<()> {
+        def.walk(a, self)
+    }
+
+    fn visit_def_module_template(
+        &self,
+        a: &mut Analysis,
+        def: &'ast DefModuleTemplate,
+    ) -> ControlFlow<()> {
+        match a.template_expansion_id {
+            Some(expand_id) => {
+                SemanticError::TemplateExpandError {
+                    loc: def.span(),
+                    msg: "module template definitions cannot appear inside a template expansion"
+                        .to_string(),
+                    expand_loc: expand_id.span(),
+                }
+                .emit();
+                ControlFlow::Break(())
+            }
+            None => ControlFlow::Continue(()),
+        }
+    }
+
+    fn visit_spec_template_expand(
+        &self,
+        a: &mut Analysis,
+        node: &'ast SpecTemplateExpand,
+    ) -> ControlFlow<()> {
+        self.spec_template_expand_unexpanded(a, node)?;
+        let Some(members) = &node.members else {
+            return ControlFlow::Continue(());
+        };
+        a.template_expansion_id = Some(node.node_id);
+        let result = members.iter().try_for_each(|member| member.visit(a, self));
+        a.template_expansion_id = None;
+        result
+    }
+}

fpp_analysis/src/passes/enter_template_expansion_symbols.rs

@@ -53,6 +53,13 @@ impl<'ast> Visitor<'ast> for EnterTemplateExpansionSymbols {
             // resolve unexpanded, so there is nothing to enter
             return ControlFlow::Continue(());
         };
+        let Ok(Some(Symbol::Template(tmpl))) = a.get_template_symbol(node.template.id()) else {
+            // CheckTemplateUses reports a template name that does not resolve,
+            // and the analysis goes on past that error. The members of an
+            // expansion written in the source as `expand T { ... }` have no
+            // parameters to bind then, so there is nothing to enter.
+            return ControlFlow::Continue(());
+        };
 
         if a.template_expansion_map.contains_key(&node.node_id) {
             // We already entered this expansion
@@ -65,7 +72,7 @@ impl<'ast> Visitor<'ast> for EnterTemplateExpansionSymbols {
             return result;
         }
 
-        if let Err(err) = self.enter_expansion(a, node, members) {
+        if let Err(err) = self.enter_expansion(a, &tmpl, node, members) {
             err.emit();
             return ControlFlow::Break(());
         }
@@ -80,16 +87,12 @@ impl EnterTemplateExpansionSymbols {
     fn enter_expansion(
         &self,
         a: &mut Analysis,
+        tmpl: &Arc<DefModuleTemplate>,
         node: &SpecTemplateExpand,
         members: &[ModuleMember],
     ) -> SemanticResult {
-        let tmpl = match a.get_template_symbol(node.template.id()) {
-            Ok(Some(Symbol::Template(tmpl))) => tmpl,
-            _ => panic!("template symbol should already be resolved"),
-        };
-
         // Build the parameter set of the expansion
-        let params = Self::bind_params(&tmpl, node)?;
+        let params = Self::bind_params(tmpl, node)?;
 
         // The parameter scope and the expansion scope are reached through the
         // expansion, so record it before filling either one in

fpp_analysis/src/passes/expand_syntax_and_enter_symbols.rs

@@ -1,5 +1,8 @@
 use crate::analysis::Analysis;
-use crate::passes::{CheckTemplateUses, EnterSymbols, EnterTemplateExpansionSymbols};
+use crate::passes::{
+    CheckTemplateDefs, CheckTemplateExpansions, CheckTemplateUses, EnterSymbols,
+    EnterTemplateExpansionSymbols,
+};
 use crate::transform::ExpandTemplates;
 use fpp_ast::{TransUnit, Visitor};
 use fpp_core::{Error, FileReader, SourceFile};
@@ -26,9 +29,10 @@ impl ExpandSyntaxAndEnterSymbols {
     }
 
     /// The steps of [`ExpandSyntaxAndEnterSymbols::tu_list`] that follow include
-    /// resolution and the synthesis of the state enums: enter the symbols, match
-    /// each template expansion specifier to its template, expand the templates,
-    /// and enter the symbols the expansions introduce.
+    /// resolution and the synthesis of the state enums: enter the symbols, check
+    /// the template definitions and the expanded templates, match each template
+    /// expansion specifier to its template, expand the templates, and enter the
+    /// symbols the expansions introduce.
     ///
     /// The language server parses each file once and keeps the parsed unit, with
     /// its includes resolved and its state enums added, across analyses. Each
@@ -38,6 +42,8 @@ impl ExpandSyntaxAndEnterSymbols {
         tul: &mut [TransUnit],
     ) -> ControlFlow<()> {
         EnterSymbols.visit_trans_units(a, tul.iter())?;
+        CheckTemplateDefs.visit_trans_units(a, tul.iter())?;
+        CheckTemplateExpansions.visit_trans_units(a, tul.iter())?;
         CheckTemplateUses.visit_trans_units(a, tul.iter())?;
         if let Err(err) = ExpandTemplates::transform_list(a, tul) {
             err.emit();