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

Don't panic when function parameters share names #1017

Merged
merged 2 commits into from
Jan 2, 2021

Conversation

AnnikaCodes
Copy link
Contributor

This PR works on #823.

It changes the following:

  • Adds an allow_name_reuse parameter to EnvironmentRecordTrait::create_mutable_binding
  • Fixes the panic in S10.2.1_A3.js from the test262 test suite
  • Allows the following code to run correctly without crashing Boa:
function test(x, x) {
    return x;
}
test(0, 1); // should return `1`

@codecov
Copy link

codecov bot commented Jan 1, 2021

Codecov Report

Merging #1017 (e424fdb) into master (4625c1a) will increase coverage by 0.01%.
The diff coverage is 75.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1017      +/-   ##
==========================================
+ Coverage   60.10%   60.11%   +0.01%     
==========================================
  Files         167      167              
  Lines       11142    11145       +3     
==========================================
+ Hits         6697     6700       +3     
  Misses       4445     4445              
Impacted Files Coverage Δ
boa/src/environment/object_environment_record.rs 23.72% <0.00%> (ø)
.../src/environment/declarative_environment_record.rs 41.11% <66.66%> (+0.66%) ⬆️
boa/src/environment/function_environment_record.rs 34.78% <66.66%> (+0.57%) ⬆️
boa/src/environment/global_environment_record.rs 21.05% <75.00%> (ø)
boa/src/builtins/function/mod.rs 74.74% <100.00%> (ø)
boa/src/environment/lexical_environment.rs 67.61% <100.00%> (+0.31%) ⬆️
boa/src/object/gcobject.rs 65.86% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4625c1a...e424fdb. Read the comment docs.

@Razican
Copy link
Member

Razican commented Jan 1, 2021

Test262 conformance changes:

Test result master count PR count difference
Total 78,493 78,493 0
Passed 20,882 20,884 +2
Ignored 15,585 15,585 0
Failed 42,026 42,024 -2
Panics 388 386 -2
Conformance 26.60 26.61 +0.00%

@Razican Razican added bug Something isn't working test Issues and PRs related to the tests. labels Jan 1, 2021
@Razican Razican added this to the v0.11.0 milestone Jan 1, 2021
Copy link
Member

@Razican Razican left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix! I think this looks pretty good :)

@Razican Razican merged commit 90a8587 into boa-dev:master Jan 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants