Oops, désolé, j'ai laissé passer ce courriel sous mon radar!
Pas de problème! J'aurais aimé faire un merge request pour solliciter du feedback, mais je n'ai pas trouvé l'option dans Gitlab puis j'ai été distrait. Ça semble ne pas être possible pour le projet Typer. Pourquoi ne pas les activer?
- src/serialization_types: SVP ajoute une extension à ce nom de fichier
pour qu'on puisse plus facilement y associer un `major-mode`.
Bonne idée! Comme la syntaxe ressemble beaucoup à OCaml, est-ce que .ml serait approprié? Sinon, aurais-tu une autre suggestion? Petit problème est que atdgen n'ignore pas l'extension lorsqu'il génère ses fichiers. Ça donne des modules nommés src/serialization_types.ml_j.mli ce qui n'est pas idéal et atdgen ne semble pas permettre de changer le fichier de destination. Je vais voir ce qui est possible.
Pourrait-on le générer automatiquement à partir du code OCaml, ou
l'inverse (histoire d'éviter cette duplication)?
Il existe des façons de dériver automatiquement le code de sérialisation et désérialisation. Par contre, il ne tient pas en contre du hash consing et ne préserve pas le partage, comme tu l'as spécifié dans un ancien courriel. J'aimerais bien mais à moins d'écrire des macros générant les types et tout le reste du code, je vois mal comment je pourrais faire. Peut-être introduire un paramètre de type à lexp, à la https://www.youtube.com/watch?v=BHjIl81HgfE, mais ça demanderait encore là des changement importants dans le reste du code et je ne suis pas prêt à faire ces changements.
De plus, comme on ne sait toujours pas si les résultats seront satisfaisants, je préfère laisser cela comme un TODO pour le moment.
Peut-être qu'une autre option est de demander à ATD de créer non pas
des types similaires aux nôtres, mais plutôt un "universal type" (un peu comme Sexp) qui couvre nos besoin. Ça augmenterait un peu la complexité de `serialization.ml`, mais simplifierait `serialization_types`.
Je ne suis pas certain de bien comprendre de quoi le type universel aurait l'air. Peux-tu élaborer?
- tu dépasses beaucoup les 80 colonnes. SVP évite-ça autant que
possible.
En effet, et ça dépasse même souvent 120. Je corrige. C'est plus difficile dans les courriel, je vais m'en tenir à ~120.
LexpMap: on a déjà une table de hashage dans lexp.ml. Tu ne peux
pas vraiment la réutiliser paske c'est une table qui ne fait que du hash-consing (tu ne peux pas en extraire un index pour chaque objet), mais elle vient avec un commentaire "relevant":
J'ai vu ce commentaire! J'utilise déjà (==) alors je ne suis pas certain de comprendre ce qui est préférable. De plus, comme tout est hash-consed, ça devrait être équivalent en pratique non?
- si je comprends bien, tu numérotes les nœuds en
"preorder" alors qu'il serait préférable de le faire en "postorder".
On "alloue" le id d'un noeud avant de le parcourir. De plus, comme le noeud pourrait potentiellement avoir une référence à soi-même (même si tu as dis que ce n'est pas possible, j'imagine que ça pourrait changer), on l'insère dans la map avant de construire sa valeur sérialisée. C'est pour ça que les valeurs optionnelles dans le type *sctx*.
Pourquoi est-ce qu'un parcours postorder serait mieux?
- il semble que chaque branche se termine par `addToCtx`, donc il
serait préférable d'avoir un seul appel à la fin plutôt qu'un appel dans chaque branche (forme de CSE).
En effet. Ça va aider (un peu) à réduire la largeur des lignes aussi. Qu'est-ce que CSE? Constant Subexpression Elimination?
- deserialize_lexp:
- évite l'identifiant `ref` pour une variable.
Index alors? Par curiosité, c'est une convention du project/ocaml ou simplement parce que c'est peu descriptif?
je ne comprends pas à quoi sert la table de hachage.
Il faut un endroit pour contenir les valeurs déjà créées pendant qu'on désérialise. Autrement, on créerait des copies. Cependant, ceci a été ajouté avant que je réalise que tout je devais utiliser le hash-consing défini dans le module lexp, alors je suis d'accord que c'est redondant.
je ne comprends pas pourquoi on a *List.assoc_opt ref store.lexp_refs*
Je suis paresseux et devrais transformer ça en map alors tu as raison que la complexité pourrait être améliorée. Une fois que le tout va fonctionner, ce sera une optimisation très facile à faire.
- "cache": en fait, je pensais plutôt rendre ces fichiers explicites,
pensés plus comme "la version compilée" du fichier, genre `foo.o` ou `foo.cmo`. Je te laisse choisir l'extension (qui ne devrait être ni `.o` ni `.cmo`).
Alors .tco, pour Tail Call Optimization *Typer Compiled Object* :)
As-tu essayé de faire fonctionner mes changements? ` make typer && _build/typer samples/acc.typer` donne cette erreur lorsque les fichiers générés sont utilisés (la 2e fois qu'on démarre le repl):
./btl/pervasive.typer:210:44:Error:(TC) Type mismatch for (cons[132]
##TypeLevel.z ##Sexp v1[0] (cons[132] ##TypeLevel.z ##Sexp v0[1] (nil[133] ##TypeLevel.z ##Sexp))) : (List[134] ##TypeLevel.z ##Sexp) != (let List : (ℓ : ##TypeLevel) ≡> ((##Type_ ℓ[0]) -> (##Type_ ℓ[1])); List = typecons (List (ℓ ::: ##TypeLevel) (a : (##Type_ ℓ[0]))) (cons a[0] (List[3] ℓ[2] a[1])) (nil) in typecons (List (ℓ ::: ##TypeLevel) (a : (##Type_ ℓ[0]))) (cons a[0] (List[3] ℓ[2] a[1])) (nil) ##TypeLevel.z ##Sexp)
./btl/pervasive.typer:210:44:Error:(TC) Type mismatch for (cons[132]
##TypeLevel.z ##Sexp (##Sexp.node (##Sexp.symbol "_=_") (cons[132] ##TypeLevel.z ##Sexp v1[0] (cons[132] ##TypeLevel.z ##Sexp v0[1] (nil[133] ##TypeLevel.z ##Sexp)))) (cons[132] ##TypeLevel.z ##Sexp fun[2] (nil[133] ##TypeLevel.z ##Sexp))) : (List[134] ##TypeLevel.z ##Sexp) != (let List : (ℓ : ##TypeLevel) ≡> ((##Type_ ℓ[0]) -> (##Type_ ℓ[1])); List = typecons (List (ℓ ::: ##TypeLevel) (a : (##Type_ ℓ[0]))) (cons a[0] (List[3] ℓ[2] a[1])) (nil) in typecons (List (ℓ ::: ##TypeLevel) (a : (##Type_ ℓ[0]))) (cons a[0] (List[3] ℓ[2] a[1])) (nil) ##TypeLevel.z ##Sexp)
Error:(ELAB) Compilation stopped in default context Fatal error: exception Log.Stop_Compilation("Compiler stopped after: 2
errors
Je n'ai pas réussi à comprendre cette erreur. J'imagine que c'est parce que le lexp généré n'est pas valide, mais l'erreur n'est pas très informative et ça réduit pas tant l'espace de recherche. As-tu une idée?
Merci des commentaires! Bonne fin de semaine, Laurent
Le mer. 19 août 2020, à 13 h 32, Stefan Monnier monnier@iro.umontreal.ca a écrit :
Salut Laurent,
Laurent Huberdeau pushed new branch laurent/elab_cache at Stefan / Typer
Oops, désolé, j'ai laissé passer ce courriel sous mon radar!
Je viens de jeter un coup d'œil à cette branche. Voici mes commentaires à chaud:
src/serialization_types: SVP ajoute une extension à ce nom de fichier pour qu'on puisse plus facilement y associer un `major-mode`. Pourrait-on le générer automatiquement à partir du code OCaml, ou l'inverse (histoire d'éviter cette duplication)?
Peut-être qu'une autre option est de demander à ATD de créer non pas des types similaires aux nôtres, mais plutôt un "universal type" (un peu comme Sexp) qui couvre nos besoin. Ça augmenterait un peu la complexité de `serialization.ml`, mais simplifierait `serialization_types`.
src/serialization.ml:
- tu dépasses beaucoup les 80 colonnes. SVP évite-ça autant que
possible.
LexpMap: on a déjà une table de hashage dans lexp.ml. Tu ne peux pas vraiment la réutiliser paske c'est une table qui ne fait que du hash-consing (tu ne peux pas en extraire un index pour chaque objet), mais elle vient avec un commentaire "relevant":
module WHC = Weak.Make (struct type t = lexp (* Using (=) instead of `compare` results * in an *enormous* slowdown. Apparently * `compare` checks == before recursing * but (=) doesn't? *) let equal x y = (compare x y = 0) let hash = Hashtbl.hash
serialize_lexp:
- si je comprends bien, tu numérotes les nœuds en "preorder" alors qu'il serait préférable de le faire en "postorder".
- il semble que chaque branche se termine par `addToCtx`, donc il serait préférable d'avoir un seul appel à la fin plutôt qu'un appel dans chaque branche (forme de CSE).
deserialize_lexp:
évite l'identifiant `ref` pour une variable.
je ne comprends pas à quoi sert la table de hachage.
je ne comprends pas pourquoi on a
List.assoc_opt ref store.lexp_refs
ça ressemble à une opération de complexité O(N) pour chacun des N nœuds, pour un total de O(N²). Me trompe-je?
"cache": en fait, je pensais plutôt rendre ces fichiers explicites, pensés plus comme "la version compilée" du fichier, genre `foo.o` ou `foo.cmo`. Je te laisse choisir l'extension (qui ne devrait être ni `.o` ni `.cmo`).
-- Stefan