PreParsing inner functions: Fix declaration-only variables, part 2.

src/ast/scopes.cc

Index: src/ast/scopes.cc
diff --git a/src/ast/scopes.cc b/src/ast/scopes.cc
index 22402b9c692d35000fd667c1f3d347c560b3d94a..5ed0e5f3dd372a956ceb7ec5fb61d82914122564 100644
--- a/src/ast/scopes.cc
+++ b/src/ast/scopes.cc
@@ -998,7 +998,7 @@ bool Scope::RemoveUnresolved(VariableProxy* var) {
 }
 
 bool Scope::RemoveUnresolved(const AstRawString* name) {
-  if (unresolved_->raw_name() == name) {
+  if (unresolved_ != nullptr && unresolved_->raw_name() == name) {
     VariableProxy* removed = unresolved_;
     unresolved_ = unresolved_->next_unresolved();
     removed->set_next_unresolved(nullptr);
@@ -1007,7 +1007,7 @@ bool Scope::RemoveUnresolved(const AstRawString* name) {
   VariableProxy* current = unresolved_;
   while (current != nullptr) {
     VariableProxy* next = current->next_unresolved();
-    if (next->raw_name() == name) {
+    if (next != nullptr && next->raw_name() == name) {

[adamk, 2016/10/07 18:35:55]

Why was this null check (and the one above) not needed before?

[marja, 2016/10/07 19:00:27]

- This function is only used by the PreParser
- PreParser shouldn't RemoveUnresolved(sth_that_doesnt_exist) even after this
CL, so it's strictly not needed
- However, this code is much more sensemaking w/ the nullptr check so it makes
sense to have it
- I'll add a DCHECK to PreParser to check the return value.

       current->set_next_unresolved(next->next_unresolved());
       next->set_next_unresolved(nullptr);
       return true;