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

basilisp.core/nil? SyntaxWarning with numeric constants #996

Closed
ikappaki opened this issue Aug 24, 2024 · 1 comment · Fixed by #998
Closed

basilisp.core/nil? SyntaxWarning with numeric constants #996

ikappaki opened this issue Aug 24, 2024 · 1 comment · Fixed by #998
Labels
component:compiler Issue pertaining to compiler issue-type:bug Something isn't working

Comments

@ikappaki
Copy link
Contributor

ikappaki commented Aug 24, 2024

Hi,

starting with Python 3.8, using basilisp.core/nil? with a numeric literal triggers a SyntaxWarning:

SyntaxWarning: "is" with 'int' literal. Did you mean "=="?

To reproduce, open up the REPL and call nil? with a numeric argument, you should see the warning

> poetry run basilisp repl
basilisp.user=> (nil? 4)
<REPL Input>:1: SyntaxWarning: "is" with 'int' literal. Did you mean "=="?
false

This warning also appears during running the test suite, as some macro-expanded code includes nil? with a literal as its first argument. For example:

tests\basilisp\test_core_macros.lpy:598
  C:\src\basilisp\tests\basilisp\test_core_macros.lpy:598: SyntaxWarning: "is" with 'int' literal. Did you mean "=="?
    (is (= 2 (some-> 1 inc)))

The underlying issue is that basilisp.core/nil? is defined using Python’s is operator

(defn ^:inline nil?
  "Return ``true`` if ``x`` is ``nil``\\, otherwise ``false``\\."
  [x]
  (operator/is- x nil))

However, the is operator should not be used with string or numeric literals, as outlined in https://bugs.python.org/issue34850

Gregory have noticed that the "is" and "is not" operator sometimes is used with string and numerical literals. This code "works" on CPython by accident, because of caching on different levels (small integers and strings caches, interned strings, deduplicating constants at compile time). But it shouldn't work on other implementations, and can not work even on early or future CPython versions.

and made it into a SyntaxWarning since Python 3.8.

Although comparing a literal with nil/None (as we do in this function) might not cause problems, I believe the warning needs to be addressed.

A solution can be to use the == operator instead in nil?, but PEP-8 suggests comparison to None should only be done with is:

Comparisons to singletons like None should always be done with is or is not, never the equality operators.

Perhaps a better solution could be to implement `nil? in the compiler, where the analyzer can check if the argument is a literal and decide accordingly?

Thanks

@chrisrink10
Copy link
Member

I think I know what's causing this and the fix shouldn't be too tough. Let me get back to you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:compiler Issue pertaining to compiler issue-type:bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants