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

Warn about variable exports from subexpressions #9134

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

richcarl
Copy link
Contributor

@richcarl richcarl commented Dec 2, 2024

Except for "block structures" like case, receive, etc., there is really no good reason to allow variable bindings to be exported from a subexpression. It is very rarely done in practice, and the resulting code is always confusing, having a distinct C flavour. For example:

        case file:open(File,AllOpts=[write,{encoding,utf8}]) of

instead of

        AllOpts=[write,{encoding,utf8}],
        case file:open(File,AllOpts) of

At the same time, the requirement that the compiler must handle the exporting of variable bindings from any and every subexpression is an unnecessary tax on all compiler code and development tooling, which has to trace these possible bindings correctly. Doing this is complicated and error prone. It would be good if such exports could be phased out completely.

This patch adds source information to variables exported from non-block subexpressions, such as arguments to calls, tuples, etc., and adds an unconditional warning (irrespective of the warn_export_vars compiler flag) when such exports are used. It also fixes all such instances in the OTP code (about 20 files in the entire code base - looks like it's been done by a few individual authors only).

Copy link
Contributor

github-actions bot commented Dec 2, 2024

CT Test Results

   13 files    245 suites   4h 9m 11s ⏱️
3 958 tests 3 533 ✅ 425 💤 0 ❌
5 734 runs  5 111 ✅ 623 💤 0 ❌

Results for commit 3e0a184.

♻️ This comment has been updated with latest results.

To speed up review, make sure that you have read Contributing to Erlang/OTP and that all checks pass.

See the TESTING and DEVELOPMENT HowTo guides for details about how to run test locally.

Artifacts

// Erlang/OTP Github Action Bot

lib/mnesia/src/mnesia_recover.erl Outdated Show resolved Hide resolved
lib/mnesia/src/mnesia_recover.erl Outdated Show resolved Hide resolved
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.

2 participants