Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Panic trying to re-define the "arguments" identifier in a function #1062

Closed
Razican opened this issue Jan 12, 2021 · 2 comments · Fixed by #1273
Closed

Panic trying to re-define the "arguments" identifier in a function #1062

Razican opened this issue Jan 12, 2021 · 2 comments · Fixed by #1273
Assignees
Labels
bug Something isn't working execution Issues or PRs related to code execution
Milestone

Comments

@Razican
Copy link
Member

Razican commented Jan 12, 2021

When we try to define an identifier inside a function with the name arguments, this fails with a panic saying that "Identifier arguments has already been declared". You can test this with the following Test262 tests:

To run any of them, you just need to run:

RUST_BACKTRACE=1 cargo run --release --bin boa_tester -- run -v -s {file}

Where {file} is one of the paths in the list above.

You will get a backtrace like this one:

   0: rust_begin_unwind
             at /rustc/e1884a8e3c3e813aada8254edfa120e85bf5ffca/library/std/src/panicking.rs:495:5
   1: std::panicking::begin_panic_fmt
             at /rustc/e1884a8e3c3e813aada8254edfa120e85bf5ffca/library/std/src/panicking.rs:437:5
   2: <boa::environment::function_environment_record::FunctionEnvironmentRecord as boa::environment::environment_record_trait::EnvironmentRecordTrait>::create_mutable_binding
   3: boa::environment::lexical_environment::LexicalEnvironment::create_mutable_binding
   4: <boa::syntax::ast::node::Node as boa::exec::Executable>::run
   5: <boa::syntax::ast::node::statement_list::StatementList as boa::exec::Executable>::run
   6: boa::object::gcobject::GcObject::call
   7: <boa::syntax::ast::node::call::Call as boa::exec::Executable>::run
   8: <boa::syntax::ast::node::block::Block as boa::exec::Executable>::run
   9: <boa::syntax::ast::node::Node as boa::exec::Executable>::run
  10: <boa::syntax::ast::node::statement_list::StatementList as boa::exec::Executable>::run
  11: boa::object::gcobject::GcObject::call
  12: <boa::syntax::ast::node::call::Call as boa::exec::Executable>::run
  13: <boa::syntax::ast::node::statement_list::StatementList as boa::exec::Executable>::run
  14: boa::context::Context::eval
  15: boa_tester::exec::<impl boa_tester::Test>::run_once
  16: boa_tester::exec::<impl boa_tester::Test>::run
  17: boa_tester::main

The panic would be this one:

thread 'main' panicked at 'Identifier arguments has already been declared', boa/src/environment/function_environment_record.rs:108:13

This happens because we assert here that a binding that doesn't allow re-declaration (such as a let binding) won't be able to get re-declared. The issue is that in every function, an arguments variable gets declared here and here. Even if that variable can be re-declared, the let binding cannot, and this makes it panic.

Our environment representation is not quite spec compliant, but the thing is that if a variable can be re-declared, then it should be in the new declaration when we check if that variable can be re-declared, not when declaring the variable gets declared for the first time, so this information should be persisted.

@Razican Razican added bug Something isn't working execution Issues or PRs related to code execution labels Jan 12, 2021
@Razican Razican added this to the v0.12.0 milestone Jan 12, 2021
@Razican Razican mentioned this issue Jan 12, 2021
@0x7D2B
Copy link
Contributor

0x7D2B commented Mar 12, 2021

I can fix this in #988 after #1156 (for proper environments) is merged.

@Razican
Copy link
Member Author

Razican commented Mar 14, 2021

I can fix this in #988 after #1156 (for proper environments) is merged.

Cool, let's assign this to you :) I will try to get some free time in the next days to review those PRs.

@Razican Razican linked a pull request May 22, 2021 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working execution Issues or PRs related to code execution
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants