1848325 - Rename special CommonPropertyNames

Status: RESOLVED FIXED

(Core :: JavaScript Engine, task, P3)

Description

[Tooru Fujisawa [:arai]]

CommonPropertyNames contains some entries where the id and the content different.
before exposing it to public API, we should rename them to clarify the id is different than the content.

https://searchfox.org/mozilla-central/rev/77dd6aa3810610949a5ff925e24de2f8c11377fd/js/src/vm/CommonPropertyNames.h#533

MACRO1(star, star, "*")                                                      \

https://searchfox.org/mozilla-central/rev/77dd6aa3810610949a5ff925e24de2f8c11377fd/js/src/vm/CommonPropertyNames.h#166

MACRO0(empty, empty, "")                                                     \

https://searchfox.org/mozilla-central/rev/77dd6aa3810610949a5ff925e24de2f8c11377fd/js/src/vm/CommonPropertyNames.h#140

MACRO_(defineGetter, defineGetter, "__defineGetter__")                       \

Also, if we're to use JS::WellKnownAtomId with gecko atoms, we might be better aligning the underline position, for simplicity:

https://searchfox.org/mozilla-central/rev/77dd6aa3810610949a5ff925e24de2f8c11377fd/js/src/vm/CommonPropertyNames.h#209

MACRO_(for, for_, "for")                                                   \

https://searchfox.org/mozilla-central/source/__GENERATED__/xpcom/ds/nsGkAtomList.h#438

GK_ATOM(_for, "for", 0xb87ea068, true, nsStaticAtom, Atom)

Comment 1

[Tooru Fujisawa [:arai]]

WIP: https://treeherder.mozilla.org/jobs?repo=try&revision=e9903f7a29ed349ff3a8ba59eeb04063228fb3f4

Comment 2

[André Bargull [:anba]]

We should try to avoid _ followed by an upper-case character, because all identifiers which start with _[A-Z] are reserved by implementations. Same goes for identifiers which include two consecutive _ characters.

From https://en.cppreference.com/w/cpp/language/identifiers:

Identifiers that appear as appearing as a token or preprocessing token (i.e., not in user-defined-string-literal like operator ""id) (since C++11) of one of the following forms are reserved:

• identifiers with a double underscore anywhere;
• identifiers that begin with an underscore followed by an uppercase letter;
• in the global namespace, identifiers that begin with an underscore.

Comment 3

[Tooru Fujisawa [:arai]]

thank you for pointing that out :)

so, if double underscore cannot be used, we cannot use leading or trailing underscore unless we stop using js_*_str format identifier.

given the js_*_str is used in very limited places, we could switch to something else, or maybe directly use js::wellKnownAtomInfos[n].content, without declaring js_*_str variables.
(bug 1847677 will remove most of the consumers anyway)

https://searchfox.org/mozilla-central/rev/77dd6aa3810610949a5ff925e24de2f8c11377fd/js/src/vm/WellKnownAtom.cpp#9,21-23

#define DECLARE_CONST_CHAR_STR(IDPART, _, TEXT) char js_##IDPART##_str[] = TEXT;
...
js::WellKnownAtomInfo js::wellKnownAtomInfos[] = {
#define ENUM_ENTRY_(IDPART, _, _2)                                \
  {uint32_t(sizeof(js_##IDPART##_str) - 1),                       \

"underscore + uppercase" matches the following. maybe using trailing underscore is safer? or perhaps we should look into some other way to clarify the text content is different than the id.

  MACRO_(_Array_Iterator, _Array_Iterator, "Array Iterator")                 \
  MACRO_(_Async_from_Sync_Iterator, _Async_from_Sync_Iterator,               \
  MACRO_(_Invalid_Date, _Invalid_Date, "Invalid Date")                       \
  MACRO_(_Iterator_Helper, _Iterator_Helper, "Iterator Helper")              \
  MACRO_(_Map_Iterator, _Map_Iterator, "Map Iterator")                       \
  MACRO_(_NegativeInfinity, _NegativeInfinity, "-Infinity")                  \
  MACRO_(_RegExp_String_Iterator, _RegExp_String_Iterator,                   \
  MACRO_(_Set_Iterator, _Set_Iterator, "Set Iterator")                       \
  MACRO_(_String_Iterator, _String_Iterator, "String Iterator")              \

Comment 4

[Tooru Fujisawa [:arai]]

actually, I was wrong. the underscore isn't used in js_*_str in current set because the IDPART is used instead, but that's still misleading.
I'll add class static in order to use the same identifier between the enum class and the strings.

Comment 5

[Tooru Fujisawa [:arai]]

Attachment: Bug 1848325 - Part 1: Rename CommonPropertyNames empty to empty_. r?jandem!

Comment 6

[Tooru Fujisawa [:arai]]

Attachment: Bug 1848325 - Part 2: Rename CommonPropertyNames with $-prefix. r?jandem!

Depends on D186288

Comment 7

[Tooru Fujisawa [:arai]]

Attachment: Bug 1848325 - Part 3: Rename CommonPropertyNames with dot-prefix. r?jandem!

Depends on D186289

Comment 8

[Tooru Fujisawa [:arai]]

Attachment: Bug 1848325 - Part 4: Rename CommonPropertyNames with [object *] content. r?jandem!

Depends on D186290

Comment 9

[Tooru Fujisawa [:arai]]

Attachment: Bug 1848325 - Part 5: Rename CommonPropertyNames with *-prefix. r?jandem!

Depends on D186291

Comment 10

[Tooru Fujisawa [:arai]]

Attachment: Bug 1848325 - Part 6: Rename CommonPropertyNames with " Iterator" suffix. r?jandem!

Depends on D186292

Comment 11

[Tooru Fujisawa [:arai]]

Attachment: Bug 1848325 - Part 7: Rename CommonPropertyNames with __-prefix. r?jandem!

Depends on D186293

Comment 12

[Tooru Fujisawa [:arai]]

Attachment: Bug 1848325 - Part 8: Rename CommonPropertyNames comma to comma_. r?jandem!

Depends on D186294

Comment 13

[Tooru Fujisawa [:arai]]

Attachment: Bug 1848325 - Part 9: Rename CommonPropertyNames for futex. r?jandem!

Depends on D186295

Comment 14

[Tooru Fujisawa [:arai]]

Attachment: Bug 1848325 - Part 10: Rename CommonPropertyNames for use-prefix. r?jandem!

Depends on D186296

Comment 15

[Tooru Fujisawa [:arai]]

Attachment: Bug 1848325 - Part 11: Rename CommonPropertyNames with #-prefix. r?jandem!

Depends on D186297

Comment 16

[Tooru Fujisawa [:arai]]

Attachment: Bug 1848325 - Part 12: Rename CommonPropertyNames for self-hosted. r?jandem!

Depends on D186298

Comment 17

[Tooru Fujisawa [:arai]]

Attachment: Bug 1848325 - Part 13: Rename CommonPropertyNames with spaces. r?jandem!

Depends on D186299

Comment 18

[Tooru Fujisawa [:arai]]

Attachment: Bug 1848325 - Part 14: Rename remaining special CommonPropertyNames. r?jandem!

Depends on D186300

Comment 19

[Tooru Fujisawa [:arai]]

Attachment: Bug 1848325 - Part 15: Rename defineDataPropertyIntrinsic to DefineDataProperty. r?jandem!

Depends on D186301

Comment 20

[Tooru Fujisawa [:arai]]

Attachment: Bug 1848325 - Part 16: Rename more CommonPropertyNames with __-prefix. r?jandem!

defineGetter etc are currently unused, but will be used again in bug 1847677.

Depends on D186302

Comment 21

[Tooru Fujisawa [:arai]]

Attachment: Bug 1848325 - Part 17: Remove unused CommonPropertyNames entries. r?jandem!

Depends on D186303

Comment 22

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/10b649b5c9fd
Part 1: Rename CommonPropertyNames empty to empty_. r=jandem
https://hg.mozilla.org/integration/autoland/rev/26b92aa12caf
Part 2: Rename CommonPropertyNames with $-prefix. r=jandem
https://hg.mozilla.org/integration/autoland/rev/8f78f353f710
Part 3: Rename CommonPropertyNames with dot-prefix. r=jandem
https://hg.mozilla.org/integration/autoland/rev/ab946ba7672e
Part 4: Rename CommonPropertyNames with [object *] content. r=jandem
https://hg.mozilla.org/integration/autoland/rev/945782fb21a5
Part 5: Rename CommonPropertyNames with *-prefix. r=jandem
https://hg.mozilla.org/integration/autoland/rev/c95fdd620848
Part 6: Rename CommonPropertyNames with " Iterator" suffix. r=jandem
https://hg.mozilla.org/integration/autoland/rev/91993e04a2f1
Part 7: Rename CommonPropertyNames with __-prefix. r=jandem
https://hg.mozilla.org/integration/autoland/rev/a302a27df9f8
Part 8: Rename CommonPropertyNames comma to comma_. r=jandem
https://hg.mozilla.org/integration/autoland/rev/04e9a9e2a084
Part 9: Rename CommonPropertyNames for futex. r=jandem
https://hg.mozilla.org/integration/autoland/rev/11997cbcead7
Part 10: Rename CommonPropertyNames for use-prefix. r=jandem
https://hg.mozilla.org/integration/autoland/rev/67d32f150e98
Part 11: Rename CommonPropertyNames with #-prefix. r=jandem
https://hg.mozilla.org/integration/autoland/rev/e44e553c9fd9
Part 12: Rename CommonPropertyNames for self-hosted. r=jandem
https://hg.mozilla.org/integration/autoland/rev/6b4381c2af09
Part 13: Rename CommonPropertyNames with spaces. r=jandem
https://hg.mozilla.org/integration/autoland/rev/8b9e0d861cc5
Part 14: Rename remaining special CommonPropertyNames. r=jandem
https://hg.mozilla.org/integration/autoland/rev/fc61e359ad38
Part 15: Rename defineDataPropertyIntrinsic to DefineDataProperty. r=jandem
https://hg.mozilla.org/integration/autoland/rev/ce9f337c7f60
Part 16: Rename more CommonPropertyNames with __-prefix. r=jandem
https://hg.mozilla.org/integration/autoland/rev/d86693cc6243
Part 17: Remove unused CommonPropertyNames entries. r=jandem

Comment 23

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/10b649b5c9fd
https://hg.mozilla.org/mozilla-central/rev/26b92aa12caf
https://hg.mozilla.org/mozilla-central/rev/8f78f353f710
https://hg.mozilla.org/mozilla-central/rev/ab946ba7672e
https://hg.mozilla.org/mozilla-central/rev/945782fb21a5
https://hg.mozilla.org/mozilla-central/rev/c95fdd620848
https://hg.mozilla.org/mozilla-central/rev/91993e04a2f1
https://hg.mozilla.org/mozilla-central/rev/a302a27df9f8
https://hg.mozilla.org/mozilla-central/rev/04e9a9e2a084
https://hg.mozilla.org/mozilla-central/rev/11997cbcead7
https://hg.mozilla.org/mozilla-central/rev/67d32f150e98
https://hg.mozilla.org/mozilla-central/rev/e44e553c9fd9
https://hg.mozilla.org/mozilla-central/rev/6b4381c2af09
https://hg.mozilla.org/mozilla-central/rev/8b9e0d861cc5
https://hg.mozilla.org/mozilla-central/rev/fc61e359ad38
https://hg.mozilla.org/mozilla-central/rev/ce9f337c7f60
https://hg.mozilla.org/mozilla-central/rev/d86693cc6243