Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Test case for recursive instantiation #33

Open
nponeccop opened this issue Apr 10, 2017 · 7 comments
Open

Test case for recursive instantiation #33

nponeccop opened this issue Apr 10, 2017 · 7 comments
Assignees
Labels

Comments

@nponeccop
Copy link
Collaborator

Непонятно, зачем рекурсия в функции:

renameVarsInExpr :: Expr -> Infer Expr
renameVarsInExpr x = flip cataM x $ \case
      ExprTypeF tp -> ExprType <$> renameVars vars tp
      x -> return $ embed x
    where
    vars = flip cata x $ \case
        ExprTypeF tp -> setToList $ ftv tp
        x -> ffold x

renameVarsInExpr :: Expr -> Infer Expr

Функцию почти не трогали, разве что Fix/unfix заменили на embed/project. Однако она на первый взгляд не имеет смысла - алгоритму W такой обход не требуется.

Замена на более логичный вариант не приводит к падению тестов.

renameVarsInExpr :: Expr -> Infer Expr
renameVarsInExpr (ExprType tp) = ExprType <$> renameVars vars tp where
   vars = setToList $ ftv tp

renameVarsInExpr x = return x

Как понять задумку исходного рекурсивного обхода? Или придумать тест кейс чтобы второй вариант упал?

@sirikid
Copy link

sirikid commented Apr 10, 2017

На мой неискушенный взгляд похоже на говнокод, особенно куча разных переменных с именем x.

@nponeccop
Copy link
Collaborator Author

Классика хаскеля же. Однобуквенные локальные идентификаторы.

@nponeccop
Copy link
Collaborator Author

nponeccop commented Apr 10, 2017

Upd: Этот код вообще вызывается только из одного места, где он логически не нужен

(Function param guard body) <- renameVarsInExpr ast

И комментирование его там тоже не приводит к падению тестов. Похоже что можно просто удалить саму функцию и её вызов

@ptol
Copy link
Owner

ptol commented Apr 10, 2017

Похоже просто дублирование. Так как реализация ftv уже рекурсивно проходит по TypeExpr и собирается все TypeVar, то проходить рекурсивно еще раз смысла нет.

@ptol
Copy link
Owner

ptol commented Apr 10, 2017

На мой неискушенный взгляд похоже на говнокод, особенно куча разных переменных с именем x.

Синтаксис Хаскеля не позволяет избавить от переменных в которых нет необходимости.
Например в oczor case синтаксически просто список функций, поэтому можно заменить

case
  ExprTypeF tp -> setToList $ ftv tp
  x -> ffold x

на

case
  ExprTypeF tp -> setToList $ ftv 
  ffold

@ptol
Copy link
Owner

ptol commented Apr 10, 2017

Этот код вообще вызывается только из одного места, где он логически не нужен

Это на случай если внутри функции будет описание типа с TypeVar который уже есть в контексте.

@nponeccop
Copy link
Collaborator Author

nponeccop commented Apr 10, 2017

Так это нормально. Называется (в пейпере Карделли который я сейчас не могу найти) non-generic type variables. Они связаны наружным forall и их "освобождать" нельзя.

В любом случае нужен тест! Ниже аналоги в HNC:

https://github.com/nponeccop/HNC/blob/master/hn_tests/comp-2.hn
https://github.com/nponeccop/HNC/blob/master/hn_tests/comp-2.cpp

Короче тебе задание придумать тест, который упадёт после комментирования renameVarsInExpr. Я там трогать не буду сейчас.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants