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

Short-circuit `if` expression for always true/always false vars and MYPY/TYPE_CHECKING #5965

Merged
merged 1 commit into from Dec 20, 2018

Conversation

Projects
None yet
2 participants
@samertm
Copy link
Contributor

samertm commented Nov 28, 2018

  • Always short-circuit in the if's expression, instead of only
    short-circuiting for "ALWAYS_TRUE/FALSE" constants (e.g. sys.platform
    checks) and not "MYPY_TRUE/FALSE" constants (e.g. MYPY)

  • Make --always-true and --always-false map to ALWAYS_TRUE and
    ALWAYS_FALSE, respectively, instead of MYPY_TRUE and MYPY_FALSE.
    The only difference between the two is a change in import priority,
    but semantically uses of "--always-true" are probably closer to
    "this condition should be true at compile time and runtime" than
    "this condition should be true at compile time but false at
    runtime".

  • Fixes #5963

@samertm

This comment has been minimized.

Copy link
Contributor Author

samertm commented Nov 28, 2018

I haven't run or tested this locally yet, I'll do that tomorrow :o)

@samertm samertm force-pushed the samertm:shortcircuit branch from b86b2ec to 0b64679 Nov 28, 2018

@msullivan
Copy link
Collaborator

msullivan left a comment

This looks good! If you rebase it to fix the merge conflicts we should be good to go.

@samertm samertm force-pushed the samertm:shortcircuit branch 2 times, most recently from 6a2587f to a00906d Dec 18, 2018

@samertm

This comment has been minimized.

Copy link
Contributor Author

samertm commented Dec 18, 2018

Just fixed the merge conflicts, should be ready to merge if the tests pass.

@msullivan

This comment has been minimized.

Copy link
Collaborator

msullivan commented Dec 18, 2018

Unfortunately they did not

@samertm samertm force-pushed the samertm:shortcircuit branch from a00906d to 268b99c Dec 19, 2018

Short-circuit `if` expression for always true/always false vars and M…
…YPY/TYPE_CHECKING

 - Always short-circuit in the if's expression, instead of only
   short-circuiting for "ALWAYS_TRUE/FALSE" constants (e.g. sys.platform
   checks) and not "MYPY_TRUE/FALSE" constants (e.g. `MYPY`)

 - Make --always-true and --always-false map to ALWAYS_TRUE and
   ALWAYS_FALSE, respectively, instead of MYPY_TRUE and MYPY_FALSE.
   The only difference between the two is a change in import priority,
   but semantically uses of "--always-true" are probably closer to
   "this condition should be true at compile time and runtime" than
   "this condition should be true at compile time but false at
   runtime".

 - Fixes #5963

@samertm samertm force-pushed the samertm:shortcircuit branch from 268b99c to dd63fcb Dec 19, 2018

@samertm

This comment has been minimized.

Copy link
Contributor Author

samertm commented Dec 19, 2018

Looks like tests are passing now :)

@msullivan msullivan merged commit 833517d into python:master Dec 20, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@msullivan

This comment has been minimized.

Copy link
Collaborator

msullivan commented Dec 20, 2018

Thanks!

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