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

inconsistent-return-statements with assert False in function call #4019

Closed
douglas-raillard-arm opened this issue Jan 8, 2021 · 2 comments · Fixed by #4311
Closed

inconsistent-return-statements with assert False in function call #4019

douglas-raillard-arm opened this issue Jan 8, 2021 · 2 comments · Fixed by #4311
Assignees
Labels
Needs investigation 🔬 A bug or crash where it's not immediately obvious what is happenning

Comments

@douglas-raillard-arm
Copy link

Hello,

Steps to reproduce

def f(x):
    if x == 1:
        return 42
    assert False

Current behavior

example.py:2:0: R1710: Either all return statements in a function should return an expression, or none of them should. (inconsistent-return-statements)

Expected behavior

No error. This is pretty similar to this issue #1782, except that assert False is being used. I can currently work around with something like raise RuntimeError('Unreachable') but assert False is a common idiom to mark unreachable paths, and should be pretty easy to match.

pylint --version output

pylint 2.6.0
astroid 2.4.2
Python 3.8.7 (default, Dec 24 2020, 17:53:09) 
[GCC 10.2.0]
@PCManticore
Copy link
Contributor

While this might be a false positive, do note that the assert False might be stripped if you are running Python with -O, in which case an exception might be the best solution for asserting that a code path is never reached.

@PCManticore PCManticore added the Needs investigation 🔬 A bug or crash where it's not immediately obvious what is happenning label Feb 7, 2021
@douglas-raillard-arm
Copy link
Author

Sure, I'm not trying to catch AssertionError anywhere so I don't rely on the exception actually being raised, this is only a safety check in case some other bug means this line ends up being executed, as well as some sort of enforced documentation.

From static analysis point of view, this is a good way to allow the user to pass extra knowledge on the data and control flow in cases that would be almost impossible to guess for the tool. For example, I've seen assembly code generation changes with GCC when using some assert() to add properties about a value (something like unrolling a loop when the induction variable is bounded with a small enough constant or something like that).

@hippo91 hippo91 self-assigned this Apr 7, 2021
@hippo91 hippo91 mentioned this issue Apr 7, 2021
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs investigation 🔬 A bug or crash where it's not immediately obvious what is happenning
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants