1471272 - Add [SMDOC] tag for in-source SpiderMonkey documentation

Status: RESOLVED FIXED

(Core :: JavaScript Engine, enhancement)

Description

[Ted Campbell [:tcampbell]]

Attachment: 0001-WIP-Add-SMDOC-tags-for-in-source-documentation.patch

I've run this idea by a few people already without strong objection but now have gotten around to doing it.

This adds [SMDOC] markers with a one line description to various comment docs in the codebase. We have a lot of good information hidden away in tree and this is a simple way to make it easier to find. Devs (new and old) can grep or searchfox for the tag to find the comment blocks we have.

This is an initial collection of them. People should tag more things they find valuable and add comment docs for systems that lack them. As we build up the set, we may evolve the naming convention to something more hierarchical (suggestions welcome!).

Comment 1

[Nicolas B. Pierron [:nbp] {away}]

Comment on attachment 8987864 [details] [diff] [review]
0001-WIP-Add-SMDOC-tags-for-in-source-documentation.patch

Review of attachment 8987864 [details] [diff] [review]:
-----------------------------------------------------------------

Really good idea! A small effort of indexing, and a simple way to grep for documentation.

nit:
https://searchfox.org/mozilla-central/source/js/src/jit/Recover.h#20 (Recover Instruction)
https://searchfox.org/mozilla-central/source/js/src/jit/MacroAssembler.h#44 (MacroAssembler meta-doc)
https://searchfox.org/mozilla-central/source/js/src/jit/RegisterSets.h#469 (Register Sets)
https://searchfox.org/mozilla-central/source/js/src/jit/RegisterSets.h#507 (Allocatable Register Sets)
https://searchfox.org/mozilla-central/source/js/src/jit/RegisterSets.h#623 (Live Register Sets)

Comment 2

[Ted Campbell [:tcampbell]]

Attachment: 0001-WIP-Add-SMDOC-tags-for-in-source-documentation.patch

Updated with feedback from Comment 1

Comment 3

[Jan de Mooij [:jandem]]

Comment on attachment 8987914 [details] [diff] [review]
0001-WIP-Add-SMDOC-tags-for-in-source-documentation.patch

Review of attachment 8987914 [details] [diff] [review]:
-----------------------------------------------------------------

Fantastic, thanks for doing this!

Please add the following:

* gc/Zone.h (line 102)
* jit/CacheIR.h (line 25)
* vm/ArgumentsObject.h (line 100)
* vm/Runtime.h (line 110)
* vm/Stack.h (line 72)

(Also maybe jit/ValueNumbering.cpp (line 17) and AliasAnalysis.cpp (line 120) if you think they qualify.)

::: js/public/CallArgs.h
@@ +4,5 @@
>   * License, v. 2.0. If a copy of the MPL was not distributed with this
>   * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
>  
>  /*
> + * [SMDOC] JS::CallArgs API

Please remove this part of the comment (these APIs are gone):

 * It's also possible to use the supported API of JS_CALLEE, JS_THIS, JS_ARGV,
 * JS_RVAL, and JS_SET_RVAL to the same ends.

Also the "neither" in the next sentence should be fixed then.

::: js/src/jit/Bailouts.h
@@ +21,1 @@
>  // A "bailout" is a condition in which we need to recover an interpreter frame

Nit: s/an interpreter frame/a baseline frame/ here and at least once below.

(Actually it can be multiple Baseline frames because of inlining..)

Comment 4

[Jan de Mooij [:jandem]]

Last ones: gc/AtomMarking.cpp has an overview comment and vm/SymbolType.h (line 125).

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/67c0442a7a44
Add [SMDOC] tags for in-source documentation. r=jandem,nbp

Comment 6

[Eliza Balazs [:ebalazs_]]

https://hg.mozilla.org/mozilla-central/rev/67c0442a7a44