Skip to content

Add ff_pipeline:do parse transform - #111

Open
ciiol wants to merge 3 commits into
masterfrom
ft/parse_trasform_do
Open

Add ff_pipeline:do parse transform #111
ciiol wants to merge 3 commits into
masterfrom
ft/parse_trasform_do

Conversation

@ciiol

@ciiol ciiol commented Aug 14, 2019

Copy link
Copy Markdown
Contributor

Продолжение #26

Dialyzer порой не может проверить типы в конструкциях вида do(fun() -> bar end), он часто определяет тип возвращаемого внутренней функцией значения как any().

Как решение этой проблемы, в этом PR предлагается автоматически преобразовать такие конструкции в более дружественную к dialyzer форму.

@ciiol
ciiol requested a review from keynslug August 14, 2019 12:06

@keynslug keynslug left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔥

transform_do(Tree) ->
TreePos = erl_syntax:get_pos(Tree),
case Tree of
?Q("do(_@Tag, fun() -> _@@Body end)") ->

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Мне кажется не очень корректным перефигачивать конструкции с do(...), даже в том случае, если эту псевдофункцию не импортировали в модуль. Кажется лучше было бы пойти по одному из двух путей:

  • перефигачивать, только если был импорт;
  • перефигачивать вообще все do, не требуя никаких имортов, только включения parse transform (не пытаться сделать вид, что do ‒ это функция в модуле ff_pipeline).

@ciiol ciiol Aug 14, 2019

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Да, согласен, это не достаточно явно.

перефигачивать, только если был импорт;

Попробую сделать.

перефигачивать вообще все do, не требуя никаких имортов

На это агрятся IDE. Плагин, что я использовал, ожидаемо не смог обработать parse transform.

Ну и, справедливости ради, do это все же функция ff_pipeline, тут делается что-то вроде инлайнинга частных случаев.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Попробую сделать.

👍

На это агрятся IDE. Плагин, что я использовал, ожидаемо не смог обработать parse transform.

Забавно, я не считаю это ожидаемым поведением. Это же просто ещё одна опция компиляции.

Ну и, справедливости ради, do это все же функция ff_pipeline, тут делается что-то вроде инлайнинга.

О, это я упустил.

?Q("-import(ff_pipeline, ['@_@FilteredImports'/0]).").

is_import_not_replaced(ImportItem) ->
case ?Q("-export(['@_ImportItem'/0]).") of

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Очень долго думал, почему тут export. Может комментом описать, что тут происходит и почему так?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Да, добавлю. Или посмотрю, чем из erl_syntax можно воспользоваться чтобы стало понятнее.

Это нужно потому, что грамматика языка не позволяет однозначно распарсить do/1. И, по-умолчанию, приоритет отдается предположению, что это деление атома на число. Как конструкцию "нечто с арностью" парсер интерпретирует это выражение только когда оно находится внутри атрибутов модуля. Причем не всех, а только известных ему.

@dinama

dinama commented Dec 7, 2020

Copy link
Copy Markdown
Contributor

если это место переделывать, то может быть есть смысл добавить кумулятивный контекст (номер строки, имя файла) возникновения/пробрасывания ошибки. иначе затруднительно анализировать без полного знания кода. {error,exists} badarg и вот это вот всё

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants