From d72022ba040f08df0c01d9fee1633bb78e5809b2 Mon Sep 17 00:00:00 2001
From: davidot <davidot@serenityos.org>
Date: Thu, 30 Dec 2021 13:19:05 +0100
Subject: [PATCH] LibJS: Convert get_identifier_reference() to
 ThrowCompletionOr

And while we're here add spec comments.
---
 Userland/Libraries/LibJS/Runtime/VM.cpp | 17 +++++++++++++----
 Userland/Libraries/LibJS/Runtime/VM.h   |  2 +-
 2 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/Userland/Libraries/LibJS/Runtime/VM.cpp b/Userland/Libraries/LibJS/Runtime/VM.cpp
index 0a94ee27e6..8aebd2fab9 100644
--- a/Userland/Libraries/LibJS/Runtime/VM.cpp
+++ b/Userland/Libraries/LibJS/Runtime/VM.cpp
@@ -426,7 +426,7 @@ ThrowCompletionOr<void> VM::iterator_binding_initialization(BindingPattern const
 }
 
 // 9.1.2.1 GetIdentifierReference ( env, name, strict ), https://tc39.es/ecma262/#sec-getidentifierreference
-Reference VM::get_identifier_reference(Environment* environment, FlyString name, bool strict, size_t hops)
+ThrowCompletionOr<Reference> VM::get_identifier_reference(Environment* environment, FlyString name, bool strict, size_t hops)
 {
     // 1. If env is the value null, then
     if (!environment) {
@@ -434,17 +434,26 @@ Reference VM::get_identifier_reference(Environment* environment, FlyString name,
         return Reference { Reference::BaseType::Unresolvable, move(name), strict };
     }
 
+    // 2. Let exists be ? env.HasBinding(name).
     Optional<size_t> index;
-    auto exists = TRY_OR_DISCARD(environment->has_binding(name, &index));
+    auto exists = TRY(environment->has_binding(name, &index));
 
+    // Note: This is an optimization for looking up the same reference.
     Optional<EnvironmentCoordinate> environment_coordinate;
     if (index.has_value())
         environment_coordinate = EnvironmentCoordinate { .hops = hops, .index = index.value() };
 
-    if (exists)
+    // 3. If exists is true, then
+    if (exists) {
+        // a. Return the Reference Record { [[Base]]: env, [[ReferencedName]]: name, [[Strict]]: strict, [[ThisValue]]: empty }.
         return Reference { *environment, move(name), strict, environment_coordinate };
-    else
+    }
+    // 4. Else,
+    else {
+        // a. Let outer be env.[[OuterEnv]].
+        // b. Return ? GetIdentifierReference(outer, name, strict).
         return get_identifier_reference(environment->outer_environment(), move(name), strict, hops + 1);
+    }
 }
 
 // 9.4.2 ResolveBinding ( name [ , env ] ), https://tc39.es/ecma262/#sec-resolvebinding
diff --git a/Userland/Libraries/LibJS/Runtime/VM.h b/Userland/Libraries/LibJS/Runtime/VM.h
index d10c1ac5a0..46529ef884 100644
--- a/Userland/Libraries/LibJS/Runtime/VM.h
+++ b/Userland/Libraries/LibJS/Runtime/VM.h
@@ -193,7 +193,7 @@ public:
     FlyString unwind_until_label() const { return m_unwind_until_label; }
 
     ThrowCompletionOr<Reference> resolve_binding(FlyString const&, Environment* = nullptr);
-    Reference get_identifier_reference(Environment*, FlyString, bool strict, size_t hops = 0);
+    ThrowCompletionOr<Reference> get_identifier_reference(Environment*, FlyString, bool strict, size_t hops = 0);
 
     template<typename T, typename... Args>
     void throw_exception(GlobalObject& global_object, Args&&... args)