1508095 - Fix clang-format around JS_FN_HELP

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Ted Campbell [:tcampbell]]

These JS_FN_HELP macro uses have their own line breaking for the messages that gets mangled by clang-format.

These probably should be |clang-format off| for line breaking, but probably could stand to have their formatting manually fixed to be more consistent (eg. add missing leading whitespace). Similarly I wonder if parameter 5 should be consistently on the first line.

Comment 1

[Jan de Mooij [:jandem]]

I wonder if this (from clang-format):

------
  JS_FN_HELP("setSavedStacksRNGState", SetSavedStacksRNGState, 1, 0,
             "setSavedStacksRNGState(seed)",
             "  Set this compartment's SavedStacks' RNG state.\n"),
------

is actually nicer than what we have now:

------
    JS_FN_HELP("setSavedStacksRNGState", SetSavedStacksRNGState, 1, 0,
"setSavedStacksRNGState(seed)",
"  Set this compartment's SavedStacks' RNG state.\n"),
------

It might make sense to use the first style everywhere and ensure 80 cols so clang-format won't touch it?

Comment 2

[Jan de Mooij [:jandem]]

It might be nice to use C++11 raw string literals so we don't have to add \n ourselves. Something like:

R"(
  Run the garbage collector. When obj is given, GC only its zone.
  If 'zone' is given, GC any zones that were scheduled for
  GC via schedulegc.
  If 'shrinking' is passed as the optional second argument, perform a
  shrinking GC rather than a normal GC.)"),

Unfortunately this adds a leading \n but maybe we can skip that somehow.

Comment 3

[Ted Campbell [:tcampbell]]

I'd also support just a |clang-format off| to get it off our plate. In the future we can change how \n are used in string too to make it a little prettier.

Comment 4

[Jan de Mooij [:jandem]]

Attachment: Bug 1508095 - Add clang-format off/on annotations for JSFunctionSpecWithHelp arrays. r?tcampbell

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/07b82a902b85
Add clang-format off/on annotations for JSFunctionSpecWithHelp arrays. r=tcampbell

Comment 6

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/07b82a902b85

Comment 7

[(no longer active)]

Attachment: yyyyyy

[ESR Uplift Approval Request]

If this is not a sec:{high,crit} bug, please state case for ESR consideration: This is required for easier backporting of patches after the reformatting of ESR using clang-format.

User impact if declined: Declining this will negatively impact our developers' ability to easily backport their patches to ESR in the future.

Fix Landed on Version: 65

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): comment only

String or UUID changes made by this patch: None

Comment 8

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 9030795 [details] [diff] [review]
yyyyyy

OK for uplift to ESR60 as part of the clang-format project.

Comment 9

[(no longer active)]

https://hg.mozilla.org/releases/mozilla-esr60/rev/8d17cb9df6f4