1074911 - Clean-up: Replace JS_ASSERT by MOZ_ASSERT.

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Nicolas B. Pierron [:nbp]]

The goal is to make a simple script to replace JS_ASSERT by MOZ_ASSERT and re-indent it if the content is spread on multiple lines.

Then apply it on all the sources, and remove the definition of JS_ASSERT macro.

Comment 1

[Benjamin Bouvier [:bbouvier] (inactive)]

Please please please.

Comment 2

[Jeff Walden [:Waldo]]

A simple script should be mostly unnecessary here.  I did a one-liner in bug 712873 once, and manual style fixups afterwards were small enough to be easily done.

Alternatively, I bet bug 348748's attachment 271413 [details] could be fairly simply adjusted to perform this rewrite in an alignment-sensitive manner.  It wouldn't handle last-column-aligned continuation characters, but other than that it'll probably do everything we'd want.

Comment 3

[Nicolas B. Pierron [:nbp]]

This command should be modified for each macro, and it replace a JS_ASSERT by a MOZ_ASSERT

$ git grep -l 'JS_ASSERT' | xargs sed -i ' /JS_ASSERT(/ { s/JS_ASSERT(/MOZ_ASSERT(/; :b; s/ \\$/\\/; /;/ { p; d; }; n; s/^/ /; b b; };  s/JS_ASSERT (/MOZ_ASSERT(/;'

This has 1 corner cases in js/src/gc/Marking.cpp, where the JS_ASSERT does not end with a semi-colon.

I will do the same for JS_ASSERT_IF and others.

Comment 4

[Nicolas B. Pierron [:nbp]]

Attachment: Replace JS_ASSERT by MOZ_ASSERT.

Apply the following script

sed -i '
   /JS_ASSERT(/ {
     s/JS_ASSERT(/MOZ_ASSERT(/;
     :b;
     s/ \\$/\\/;
     /;/ { p; d; };
     n;
     s/^/ /;
     b b;
  };
  s/JS_ASSERT (/MOZ_ASSERT(/;
'

except for js/src/gc/marking.cpp where the JS_ASSERT macro does not end with a
semi-colon.

Comment 5

[Nicolas B. Pierron [:nbp]]

Attachment: Replace JS_ASSERT_IF by MOZ_ASSERT_IF.

Apply the following script

sed -i '
   /JS_ASSERT_IF(/ {
     s/JS_ASSERT_IF(/MOZ_ASSERT_IF(/;
     :b;
     s/ \\$/\\/;
     /;/ { p; d; };
     n;
     s/^/ /;
     b b;
  };
  s/JS_ASSERT_IF (/MOZ_ASSERT_IF(/;
'

Comment 6

[Nicolas B. Pierron [:nbp]]

Attachment: Remove JS_ASSERT and JS_ASSERT_IF macros.

Comment 7

[Jason Orendorff [:jorendorff]]

Comment on attachment 8498126 [details] [diff] [review]
Replace JS_ASSERT by MOZ_ASSERT.

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

Great! Thank you.

::: js/src/jit/BaselineIC.h
@@ +719,5 @@
>  
>  #define KIND_METHODS(kindName)   \
>      inline bool is##kindName() const { return kind() == kindName; } \
>      inline const IC##kindName *to##kindName() const { \
> +        MOZ_ASSERT(is##kindName());\

Leave the space before the backslash here.

@@ +724,4 @@
>          return reinterpret_cast<const IC##kindName *>(this); \
>      } \
>      inline IC##kindName *to##kindName() { \
> +        MOZ_ASSERT(is##kindName());\

And here.

::: js/src/jit/IonBuilder.cpp
@@ +1345,5 @@
>                  // FALL THROUGH
>  
>                default:
> +                MOZ_ASSERT(popped[i]->isImplicitlyUsed() ||
> + 

Remove the space on this blank line.

@@ +1351,5 @@
> +                           // often dead unless they escape from the
> +                           // fn. See IonBuilder::loadTypedObjectData()
> +                           // for more details.
> +                           popped[i]->isNewDerivedTypedObject() ||
> + 

And here.

::: js/src/jsfriendapi.h
@@ +1779,5 @@
>  #define JS_DEFINE_DATA_AND_LENGTH_ACCESSOR(Type, type) \
>  inline void \
>  Get ## Type ## ArrayLengthAndData(JSObject *obj, uint32_t *length, type **data) \
>  { \
> +    MOZ_ASSERT(GetObjectClass(obj) == detail::Type ## ArrayClassPtr);\

Leave the space before the backslash here.

::: js/src/shell/jsoptparse.cpp
@@ +25,5 @@
>      } \
>      __cls##Option * \
>      Option::as##__cls##Option() \
>      { \
> +        MOZ_ASSERT(is##__cls##Option());\

And here.

::: js/src/vm/Debugger.cpp
@@ +5076,1 @@
>      RootedObject obj(cx, DebuggerObject_checkThis(cx, args, fnname));         \

The script messed up here and added an extra space to 10 or 15 lines.

@@ +5984,5 @@
> +    MOZ_ASSERT(env);                                                          \
> +    MOZ_ASSERT(!env->is<ScopeObject>())
> + 
> + #define THIS_DEBUGENV_OWNER(cx, argc, vp, fnname, args, envobj, env, dbg)    \
> +     THIS_DEBUGENV(cx, argc, vp, fnname, args, envobj, env);                  \

Same thing here.

::: js/src/vm/Stack.cpp
@@ +206,5 @@
>      /*
> +     * Ideally, we'd MOZ_ASSERT(!scope->is<ScopeObject>()) but the enclosing
> +      * lexical scope chain stops at eval() boundaries. See StaticScopeIter
> +      * comment.
> +      */

And one more place where the script inserted a space at the beginning of 10 lines or so.

Comment 8

[Jason Orendorff [:jorendorff]]

Comment on attachment 8498135 [details] [diff] [review]
Replace JS_ASSERT_IF by MOZ_ASSERT_IF.

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

Looks perfect.

Comment 9

[Jason Orendorff [:jorendorff]]

Comment on attachment 8498141 [details] [diff] [review]
Remove JS_ASSERT and JS_ASSERT_IF macros.

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

Nice.

Comment 10

[Nicolas B. Pierron [:nbp]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ed4b995667b5
https://hg.mozilla.org/integration/mozilla-inbound/rev/020a62efb303
https://hg.mozilla.org/integration/mozilla-inbound/rev/99be24fa7141

Comment 11

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/ed4b995667b5
https://hg.mozilla.org/mozilla-central/rev/020a62efb303
https://hg.mozilla.org/mozilla-central/rev/99be24fa7141