1333020 - Wasm baseline: Move some whitespace around in expressions

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect, P5)

Description

[Lars T Hansen [:lth]]

Some of us are more concerned than others about where we put our whitespace, especially in expressions and loop headers.

Since I belong to the 100%-unconcerned faction (by temperament), and since an idiosyncratic and heterodox use of whitespace is built into my fingers (by decades of programming), my code tends to annoy those on the more concerned + orthodox end of the spectrum.  Anyone who wants to go over the baseline compiler and clean up whitespace is welcome to do so but please, please, please -- make modest, meaningful changes, and submit them for review in modest chunks.

Comment 1

[Benjamin Bouvier [:bbouvier] (inactive)]

As part of the witch hunters faction, here are a few more comments:

- please do change no more than 5 or 6 functions at a time *in a single patch*, to not make rebasing painful for other programmers.
- an example of the required changes is the following: whenever we have `x+y`, we want to have `x + y`.
- other spacing changes could impact line sizes: per our guidelines, a code line may fill up to 100 columns, but a comment may fill up to 80 chars only, for some reason. There are a few comments that should be wrapped in the baseline compiler file (js/src/wasm/WasmBaselineCompile.cpp).

If you have any questions, feel free to reach us on irc (irc.mozilla.org, channel #ionmonkey or #jsapi; I'm bbouvier there).

Comment 2

[Jeff Walden [:Waldo]]

(In reply to Benjamin Bouvier [:bbouvier] from comment #1)
> per our guidelines, a code
> line may fill up to 100 columns, but a comment may fill up to 80 chars only,
> for some reason.

Actually it's 99ch and 79ch, the off-by-one to accommodate line-continuation glyphs in Emacs, apparently.  I kinda think pandering to Emacs this way is dumb.  But also, whatevs.

As for why comments are less, it's to have less-long lines to aid readability and simplify the visual reset-to-text-start when reading paragraphs.

Comment 3

[Abhishek Tripathi]

I seek guidance from mentors Benjamin and Lars to get started. I am interested to work on this.

Comment 4

[Benjamin Bouvier [:bbouvier] (inactive)]

(In reply to Abhishek Tripathi from comment #3)
> I seek guidance from mentors Benjamin and Lars to get started. I am
> interested to work on this.

Hi Abhishek, thanks for jumping in! See comment 1 for some guidance. If you have made all the following to start hacking on Spidermonkey [1], you can start hacking on the /js/src/wasm/WasmBaselineCompile.{h,cpp} files and change whitespaces there.

If you get stuck or need any help, feel free to drop by on IRC (irc.mozilla.org), in the #ionmonkey or #jsapi channels; my nick there is bbouvier.

[1] https://wiki.mozilla.org/JavaScript:New_to_SpiderMonkey

Comment 5

[meghasharma4910]

I am new in this open source world, can I take up this bug?

Comment 6

[Lars T Hansen [:lth]]

Hi meghasharma4910, you can indeed work on this if you like, it's a good way to get started.  See comment 4 for more guidance, and do feel free to ask if you are stuck.

Comment 7

[Gauri]

May I also work on this bug? Or is it only to be assigned to one person?

Comment 8

[meghasharma4910]

I've started working on the bug, though was facing some problems with the setup. I'll try to send the patch as soon as possible.

Comment 9

[Lars T Hansen [:lth]]

Hi Gauri,

it's best if only one person works at a bug at a time, and especially in this case, as only one file is involved.  However it looks like bug 1333018 has been idle for a while, perhaps you want to take a look at that?

Comment 10

[insiyeah]

Is this issue still available? I'd like to work on this.

Comment 11

[Lars T Hansen [:lth]]

Yes, the bug is unassigned and I don't know about anyone actually working on it.

Comment 12

[insiyeah]

Where can I find the files I've to work on? Please guide me further.

Comment 13

[Lars T Hansen [:lth]]

Hi insiyeah, see comment 4 for more guidance.

Comment 14

[George Prince]

I am a new beginner to open source world, can I also try to work on it?

Comment 15

[Lars T Hansen [:lth]]

Hi George, I propose you look at some other bug, I think this one is going to be resolved shortly with work that Benjamin is doing over in bug 1447261.

Comment 16

[Lars T Hansen [:lth]]

Resolved by bug 1447261.