commit c9dc037f1cccca15a836bca12ffad93109b62669
parent b90ed1b56cfe85e9707aa2f107d429a13e9fdcec
Author: Nicolò Ribaudo <nribaudo@igalia.com>
Date: Wed, 22 Oct 2025 14:18:32 +0000
Bug 1995751 - Do not store dfsIndex on modules r=jonco
It can be replaced with a simpler local variable in InnerModuleLinking
and InnerModuleEvaluation.
A similar spec change was landed in https://github.com/tc39/ecma262/pull/3625.
Differential Revision: https://phabricator.services.mozilla.com/D269579
Diffstat:
6 files changed, 19 insertions(+), 58 deletions(-)
diff --git a/js/src/builtin/ModuleObject.cpp b/js/src/builtin/ModuleObject.cpp
@@ -770,12 +770,10 @@ class js::CyclicModuleFields {
private:
// Flag bits that determine whether other fields are present.
- bool hasDfsIndex : 1;
bool hasDfsAncestorIndex : 1;
bool hasPendingAsyncDependencies : 1;
// Fields whose presence is conditional on the flag bits above.
- uint32_t dfsIndex = 0;
uint32_t dfsAncestorIndex = 0;
uint32_t pendingAsyncDependencies = 0;
@@ -811,11 +809,9 @@ class js::CyclicModuleFields {
Span<const ExportEntry> indirectExportEntries() const;
Span<const ExportEntry> starExportEntries() const;
- void setDfsIndex(uint32_t index);
- Maybe<uint32_t> maybeDfsIndex() const;
void setDfsAncestorIndex(uint32_t index);
Maybe<uint32_t> maybeDfsAncestorIndex() const;
- void clearDfsIndexes();
+ void clearDfsAncestorIndex();
void setPendingAsyncDependencies(uint32_t newValue);
Maybe<uint32_t> maybePendingAsyncDependencies() const;
@@ -823,7 +819,6 @@ class js::CyclicModuleFields {
CyclicModuleFields::CyclicModuleFields()
: hasTopLevelAwait(false),
- hasDfsIndex(false),
hasDfsAncestorIndex(false),
hasPendingAsyncDependencies(false) {}
@@ -874,15 +869,6 @@ Span<const ExportEntry> CyclicModuleFields::starExportEntries() const {
exportEntries.end());
}
-void CyclicModuleFields::setDfsIndex(uint32_t index) {
- dfsIndex = index;
- hasDfsIndex = true;
-}
-
-Maybe<uint32_t> CyclicModuleFields::maybeDfsIndex() const {
- return hasDfsIndex ? Some(dfsIndex) : Nothing();
-}
-
void CyclicModuleFields::setDfsAncestorIndex(uint32_t index) {
dfsAncestorIndex = index;
hasDfsAncestorIndex = true;
@@ -892,9 +878,7 @@ Maybe<uint32_t> CyclicModuleFields::maybeDfsAncestorIndex() const {
return hasDfsAncestorIndex ? Some(dfsAncestorIndex) : Nothing();
}
-void CyclicModuleFields::clearDfsIndexes() {
- dfsIndex = 0;
- hasDfsIndex = false;
+void CyclicModuleFields::clearDfsAncestorIndex() {
dfsAncestorIndex = 0;
hasDfsAncestorIndex = false;
}
@@ -1193,16 +1177,6 @@ AsyncEvaluationOrder const& ModuleObject::asyncEvaluationOrder() const {
return cyclicModuleFields()->asyncEvaluationOrder;
}
-Maybe<uint32_t> ModuleObject::maybeDfsIndex() const {
- return cyclicModuleFields()->maybeDfsIndex();
-}
-
-uint32_t ModuleObject::dfsIndex() const { return maybeDfsIndex().value(); }
-
-void ModuleObject::setDfsIndex(uint32_t index) {
- cyclicModuleFields()->setDfsIndex(index);
-}
-
Maybe<uint32_t> ModuleObject::maybeDfsAncestorIndex() const {
return cyclicModuleFields()->maybeDfsAncestorIndex();
}
@@ -1215,8 +1189,8 @@ void ModuleObject::setDfsAncestorIndex(uint32_t index) {
cyclicModuleFields()->setDfsAncestorIndex(index);
}
-void ModuleObject::clearDfsIndexes() {
- cyclicModuleFields()->clearDfsIndexes();
+void ModuleObject::clearDfsAncestorIndex() {
+ cyclicModuleFields()->clearDfsAncestorIndex();
}
PromiseObject* ModuleObject::maybeTopLevelCapability() const {
diff --git a/js/src/builtin/ModuleObject.h b/js/src/builtin/ModuleObject.h
@@ -426,8 +426,6 @@ class ModuleObject : public NativeObject {
ModuleEnvironmentObject* environment() const;
ModuleNamespaceObject* namespace_();
ModuleStatus status() const;
- mozilla::Maybe<uint32_t> maybeDfsIndex() const;
- uint32_t dfsIndex() const;
mozilla::Maybe<uint32_t> maybeDfsAncestorIndex() const;
uint32_t dfsAncestorIndex() const;
bool hadEvaluationError() const;
@@ -445,9 +443,8 @@ class ModuleObject : public NativeObject {
IndirectBindingMap& importBindings();
void setStatus(ModuleStatus newStatus);
- void setDfsIndex(uint32_t index);
void setDfsAncestorIndex(uint32_t index);
- void clearDfsIndexes();
+ void clearDfsAncestorIndex();
static PromiseObject* createTopLevelCapability(JSContext* cx,
Handle<ModuleObject*> module);
diff --git a/js/src/jit-test/tests/modules/bug-1928654.js b/js/src/jit-test/tests/modules/bug-1928654.js
@@ -5,7 +5,6 @@ assertEq(m.importEntries, undefined);
assertEq(m.localExportEntries, undefined);
assertEq(m.indirectExportEntries, undefined);
assertEq(m.starExportEntries, undefined);
-assertEq(m.dfsIndex, undefined);
assertEq(m.dfsAncestorIndex, undefined);
assertEq(m.hasTopLevelAwait, undefined);
assertEq(m.topLevelCapability, undefined);
diff --git a/js/src/jit-test/tests/modules/shell-wrapper.js b/js/src/jit-test/tests/modules/shell-wrapper.js
@@ -156,7 +156,7 @@ assertEq(i.starExportEntries[0].localName, null);
assertEq(i.starExportEntries[0].lineNumber, 2);
assertEq(i.starExportEntries[0].columnNumber, 8);
-// ==== dfsIndex and dfsAncestorIndex getters ====
+// ==== dfsAncestorIndex getter ====
const j = registerModule('j', parseModule(`
export const v1 = 10;
import {v2} from 'k'
@@ -170,18 +170,12 @@ export const v3 = 10;
import {v2} from 'k'
import {v1} from 'j'
`));
-assertEq(j.dfsIndex, undefined);
assertEq(j.dfsAncestorIndex, undefined);
-assertEq(k.dfsIndex, undefined);
assertEq(k.dfsAncestorIndex, undefined);
-assertEq(l.dfsIndex, undefined);
assertEq(l.dfsAncestorIndex, undefined);
moduleLink(l);
-assertEq(j.dfsIndex, 2);
assertEq(j.dfsAncestorIndex, 1);
-assertEq(k.dfsIndex, 1);
assertEq(k.dfsAncestorIndex, 1);
-assertEq(l.dfsIndex, 0);
assertEq(l.dfsAncestorIndex, 0);
// ==== async and promises getters ====
diff --git a/js/src/shell/ShellModuleObjectWrapper.cpp b/js/src/shell/ShellModuleObjectWrapper.cpp
@@ -481,8 +481,6 @@ DEFINE_NATIVE_GETTER_FUNCTIONS(ModuleObject, indirectExportEntries,
SpanToArrayFilter<ShellExportEntryWrapper>)
DEFINE_NATIVE_GETTER_FUNCTIONS(ModuleObject, starExportEntries,
SpanToArrayFilter<ShellExportEntryWrapper>)
-DEFINE_GETTER_FUNCTIONS(ModuleObject, maybeDfsIndex, Uint32OrUndefinedValue,
- IdentFilter)
DEFINE_GETTER_FUNCTIONS(ModuleObject, maybeDfsAncestorIndex,
Uint32OrUndefinedValue, IdentFilter)
DEFINE_GETTER_FUNCTIONS(ModuleObject, hasTopLevelAwait, BooleanValue,
@@ -510,7 +508,6 @@ static const JSPropertySpec ShellModuleObjectWrapper_accessors[] = {
ShellModuleObjectWrapper_indirectExportEntriesGetter, 0),
JS_PSG("starExportEntries",
ShellModuleObjectWrapper_starExportEntriesGetter, 0),
- JS_PSG("dfsIndex", ShellModuleObjectWrapper_maybeDfsIndexGetter, 0),
JS_PSG("dfsAncestorIndex",
ShellModuleObjectWrapper_maybeDfsAncestorIndexGetter, 0),
JS_PSG("hasTopLevelAwait", ShellModuleObjectWrapper_hasTopLevelAwaitGetter,
diff --git a/js/src/vm/Modules.cpp b/js/src/vm/Modules.cpp
@@ -1734,7 +1734,7 @@ static bool ModuleLink(JSContext* cx, Handle<ModuleObject*> module) {
MOZ_ASSERT(m->status() == ModuleStatus::Linking);
// Step 4.a.ii. Set m.[[Status]] to unlinked.
m->setStatus(ModuleStatus::Unlinked);
- m->clearDfsIndexes();
+ m->clearDfsAncestorIndex();
}
// Step 4.b. Assert: module.[[Status]] is unlinked.
@@ -1797,8 +1797,8 @@ static bool InnerModuleLinking(JSContext* cx, Handle<ModuleObject*> module,
// Step 4. Set module.[[Status]] to linking.
module->setStatus(ModuleStatus::Linking);
- // Step 5. Set module.[[DFSIndex]] to index.
- module->setDfsIndex(index);
+ // Step 5. Let moduleIndex be index.
+ size_t moduleIndex = index;
// Step 6. Set module.[[DFSAncestorIndex]] to index.
module->setDfsAncestorIndex(index);
@@ -1865,11 +1865,11 @@ static bool InnerModuleLinking(JSContext* cx, Handle<ModuleObject*> module,
// Step 11. Assert: module occurs exactly once in stack.
MOZ_ASSERT(CountElements(stack, module) == 1);
- // Step 12. Assert: module.[[DFSAncestorIndex]] <= module.[[DFSIndex]].
- MOZ_ASSERT(module->dfsAncestorIndex() <= module->dfsIndex());
+ // Step 12. Assert: module.[[DFSAncestorIndex]] <= moduleIndex.
+ MOZ_ASSERT(module->dfsAncestorIndex() <= moduleIndex);
- // Step 13. If module.[[DFSAncestorIndex]] = module.[[DFSIndex]], then
- if (module->dfsAncestorIndex() == module->dfsIndex()) {
+ // Step 13. If module.[[DFSAncestorIndex]] = moduleIndex, then
+ if (module->dfsAncestorIndex() == moduleIndex) {
// Step 13.a.
bool done = false;
@@ -2100,8 +2100,8 @@ static bool InnerModuleEvaluation(JSContext* cx, Handle<ModuleObject*> module,
// Step 5. Set module.[[Status]] to evaluating.
module->setStatus(ModuleStatus::Evaluating);
- // Step 6. Set module.[[DFSIndex]] to index.
- module->setDfsIndex(index);
+ // Step 6. Let moduleIndex be index.
+ size_t moduleIndex = index;
// Step 7. Set module.[[DFSAncestorIndex]] to index.
module->setDfsAncestorIndex(index);
@@ -2215,11 +2215,11 @@ static bool InnerModuleEvaluation(JSContext* cx, Handle<ModuleObject*> module,
// Step 14. Assert: module occurs exactly once in stack.
MOZ_ASSERT(CountElements(stack, module) == 1);
- // Step 15. Assert: module.[[DFSAncestorIndex]] <= module.[[DFSIndex]].
- MOZ_ASSERT(module->dfsAncestorIndex() <= module->dfsIndex());
+ // Step 15. Assert: module.[[DFSAncestorIndex]] <= moduleIndex.
+ MOZ_ASSERT(module->dfsAncestorIndex() <= moduleIndex);
- // Step 16. If module.[[DFSAncestorIndex]] = module.[[DFSIndex]], then:
- if (module->dfsAncestorIndex() == module->dfsIndex()) {
+ // Step 16. If module.[[DFSAncestorIndex]] = momoduleIndex, then:
+ if (module->dfsAncestorIndex() == moduleIndex) {
// Step 16.a. Let done be false.
bool done = false;
