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

space-infix-ops: need to exclude "a|0" pattern from warnings #1295

Closed
puzrin opened this issue Sep 25, 2014 · 12 comments
Closed

space-infix-ops: need to exclude "a|0" pattern from warnings #1295

puzrin opened this issue Sep 25, 2014 · 12 comments
Labels
archived due to age This issue has been archived; please open a new issue for any further discussion needs bikeshedding Minor details about this change need to be discussed rule Relates to ESLint's core rules triage An ESLint team member will look at this issue soon

Comments

@puzrin
Copy link
Contributor

puzrin commented Sep 25, 2014

"a|0" is well known pattern, when result is forced to be signed 32 bit integer (makes sense when writing high speed js). It should not give warning about missed spaces.

Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

@michaelficarra
Copy link
Member

a | 0 works just as well.

@puzrin
Copy link
Contributor Author

puzrin commented Sep 25, 2014

I know. But all write a|0, without space. That help to understand, that it's a type cast, not binary operation.

@puzrin
Copy link
Contributor Author

puzrin commented Sep 25, 2014

Here is example from real code https://github.com/nodeca/pica/blob/adb2b2c78f784b2385c6aa00e973c1b2063e056a/lib/pure/resize.js#L182-L185 . That helps JIT to remove some types check in generated inline caches.

@nzakas nzakas added triage An ESLint team member will look at this issue soon rule Relates to ESLint's core rules labels Sep 25, 2014
@nzakas
Copy link
Member

nzakas commented Sep 25, 2014

We definitely can't support this by default, this would have to be some sort of exception if we end up supporting it. I'd like to hear more opinions p.

@nzakas nzakas added the needs bikeshedding Minor details about this change need to be discussed label Sep 25, 2014
@lo1tuma
Copy link
Member

lo1tuma commented Sep 25, 2014

I don’t think it is a good idea to write such code by hand. JavaScript doesn’t have 32bit integers.
Use a statically typed language that can be transpiled to asm.js or something like sweet.js macros.

@puzrin
Copy link
Contributor Author

puzrin commented Sep 25, 2014

@lo1tuma , be in - sync - with - real - life :) . I'm one of those who write such code, when needed. There is nothing wrong in deep understanding of JITs principles.

@nzakas
Copy link
Member

nzakas commented Sep 26, 2014

Okay, let's stop the could vs. should conversation, as that's not very helpful.

I took a look at the asm.js spec and it makes a clear distinction between f|0 as a type annotation and f | 0 as a binary operation. This is also the only type annotation for which the default coding style doesn't really apply.

So I think we should support this exception as an option. What should it be called? asmInt?

@puzrin
Copy link
Contributor Author

puzrin commented Sep 26, 2014

Don't know, is this syntax initially related to asm.js or not. Also, _|0 can be used (with leading space). Key point is no space between | and 0.

May be, int32? I'd prefer to avoid promoting terms intersection via naming - there is nothing common between writing speed-optimized monomophic code and asm.js. Writing asm.js manually is mad idea.

As far as i know, we have only one widely known exclusion. Sometime it's possible to cast Uint32 with >>> 0, but i haven't seen it widely in practice, and never used myself. Uint32 is not supported well in different JITs, and attempt to force it can cause boxing.

@nzakas
Copy link
Member

nzakas commented Sep 26, 2014

int32 isn't specific enough, it makes it seem like it has something to do with numeric literals. int32Hint?

@puzrin
Copy link
Contributor Author

puzrin commented Sep 26, 2014

Argee. int32Hint is much better.

@michaelficarra
Copy link
Member

The name should have no semantic meaning. It should refer only to the exceptional syntactic form: bitwiseOrZero.

@nzakas
Copy link
Member

nzakas commented Sep 26, 2014

bitwiseOrZero is confusing because why not also bitwiseOrOne?

The only real reason to use this option is for type hinting, so having the option refer to this is important IMHO.

@nzakas nzakas closed this as completed in 09bd11c Mar 18, 2015
nzakas added a commit that referenced this issue Mar 18, 2015
New: Added option int32Hint for space-infix-ops (fixes #1295)
@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators Feb 7, 2018
@eslint-deprecated eslint-deprecated bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Feb 7, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
archived due to age This issue has been archived; please open a new issue for any further discussion needs bikeshedding Minor details about this change need to be discussed rule Relates to ESLint's core rules triage An ESLint team member will look at this issue soon
Projects
None yet
Development

No branches or pull requests

4 participants