fix(optim): over-fold rules that could be impacted by a contexte

publicodes · Jan 8, 2024 · b92a793 · b92a793
1 parent dcdb1aa
commit b92a793
Show file tree

Hide file tree

Showing 4 changed files with 24 additions and 25 deletions.
diff --git a/source/commons.ts b/source/commons.ts
@@ -67,7 +67,7 @@ export type ImportMacro = {
 /**
  * Represents a non-parsed NGC rule.
  */
-export type RawRule = Omit<Rule, 'nom'> & ImportMacro
+export type RawRule = Omit<Rule, 'nom'> | ImportMacro
 
 /**
  * Represents a non-parsed NGC model.

diff --git a/source/compilation/resolveImports.ts b/source/compilation/resolveImports.ts
@@ -203,7 +203,7 @@ export function resolveImports(
   let neededNamespaces = new Set<string>()
   const resolvedRules = Object.entries(rules).reduce((acc, [name, value]) => {
     if (name === IMPORT_KEYWORD) {
-      const importMacro: ImportMacro = value
+      const importMacro = value as ImportMacro
       const engine = getEngine(filePath, importMacro, verbose)
       const rulesToImport: RuleToImport[] =
         importMacro['les règles']?.map(getRuleToImportInfos)

diff --git a/source/optims/constantFolding.ts b/source/optims/constantFolding.ts
@@ -1,9 +1,4 @@
-import Engine, {
-  reduceAST,
-  ParsedRules,
-  parseExpression,
-  utils,
-} from 'publicodes'
+import Engine, { reduceAST, ParsedRules, parseExpression } from 'publicodes'
 import type { RuleNode, ASTNode, Unit } from 'publicodes'
 import {
   getAllRefsInNode,
@@ -49,7 +44,14 @@ type FoldingCtx = {
    *	    rule4: 20
    * ...
    * ```
-   * In this case, [rule2] should not be folded.
+   * In this case, [rule2] should not be folded (and all its dependencies
+   * should not be folded!).
+   *
+   * TODO(@EmileRolley): currently, all childs of a rule with a [contexte]
+   * mechanism are not folded. However, it could be smarter to keep track of
+   * each contexte rules and fold the child rules that are not impacted by the
+   * contexte. For now we choose to keep it simple and to over-fold instead of
+   * taking the risk to alter the result.
    */
   impactedByContexteRules: Set<RuleName>
 }
@@ -117,10 +119,14 @@ function initFoldingCtx(
   //
   // NOTE(@EmileRolley): contexte rule will be added in the contextRules set.
   // Therefore, they won't be marked as folded. It's a wanted behavior? Not sure.
+  //
+  // WARN(@EmileRolley): the [impactedByContexteRules] is updated while
+  // iterating it's convenient but the semantics may vary depending on the
+  // javascript runtime used.
   for (const ruleName of impactedByContexteRules) {
-    getAllChilds(ruleName, refs.childs).forEach((rule) =>
-      impactedByContexteRules.add(rule),
-    )
+    refs.childs
+      .get(ruleName)
+      ?.forEach((rule) => impactedByContexteRules.add(rule))
   }
 
   return {
@@ -149,19 +155,6 @@ function getAllRefsInNodeImpactedByContexte(
   return impactedRules
 }
 
-function getAllChilds(ruleName: RuleName, childs: RefMap): RuleName[] {
-  const allChilds = []
-
-  for (const child of childs.get(ruleName) ?? []) {
-    if (!allChilds.includes(child)) {
-      allChilds.push(child)
-      allChilds.push(...getAllChilds(child, childs))
-    }
-  }
-
-  return allChilds
-}
-
 function isFoldable(
   rule: RuleNode | undefined,
   contextRules: Set<RuleName>,

diff --git a/test/optims/constantFolding.test.ts b/test/optims/constantFolding.test.ts
@@ -640,6 +640,9 @@ describe('Constant folding [base]', () => {
         formule: 'nested 2 * 4',
       },
       'rule to recompute . nested 2': {
+        formule: 'nested 3 * 4',
+      },
+      'rule to recompute . nested 3': {
         formule: 'constant * 4',
       },
       constant: {
@@ -660,6 +663,9 @@ describe('Constant folding [base]', () => {
         formule: 'nested 2 * 4',
       },
       'rule to recompute . nested 2': {
+        formule: 'nested 3 * 4',
+      },
+      'rule to recompute . nested 3': {
         formule: 'constant * 4',
       },
       constant: {