[Merged by Bors] - Prevent breaks without loop or switch from causing panics

[addisoncrump]

This PR changes the following:

• Replaces a panic with a syntax error when a break is used outside of a loop or switch
• Adds a test for that

[codecov]

Codecov Report

Merging #1860 (58fd56a) into main (44b5617) will increase coverage by 0.00%.
The diff coverage is 66.66%.

@@           Coverage Diff           @@
##             main    #1860   +/-   ##
=======================================
  Coverage   46.04%   46.04%           
=======================================
  Files         206      206           
  Lines       17003    17005    +2     
=======================================
+ Hits         7829     7830    +1     
- Misses       9174     9175    +1     

Impacted Files	Coverage Δ
boa_engine/src/bytecompiler.rs	37.22% <66.66%> (+0.01%)	⬆️

---

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 44b5617...58fd56a. Read the comment docs.

[Razican]

Just a small recommendation for readability

[Razican]

Here too, I would use is_err() instead of matches!(), it makes things more clear, and you can directly pass a reference to src instead of calling as_bytes(), if I remember correctly.

[HalidOdat]

Hmmm... Shouldn't this be an early error . According to spec:

14.9.1 Static Semantics: Early Errors
BreakStatement: break ;

It is a Syntax Error if this BreakStatement is not nested, directly or indirectly (but not crossing function or static initialization block boundaries), within an IterationStatement or a SwitchStatement.

In which case it should be in the parser, not compilation.

[Razican]

Hmmm... Shouldn't this be an early error . According to spec:

14.9.1 Static Semantics: Early Errors
BreakStatement: break ;
It is a Syntax Error if this BreakStatement is not nested, directly or indirectly (but not crossing function or static initialization block boundaries), within an IterationStatement or a SwitchStatement.

In which case it should be in the parser, not compilation.

Indeed, this should be in the parser, but that might be a bit more complex to implement, so this could be a quick win to avoid the panic for the 0.14 release.

[HalidOdat]

Indeed, this should be in the parser, but that might be a bit more complex to implement, so this could be a quick win to avoid the panic for the 0.14 release.

In that case we should put FIXMEs in places where we have the quick fix, so we can deal with them later :)

[Razican]

Indeed, this should be in the parser, but that might be a bit more complex to implement, so this could be a quick win to avoid the panic for the 0.14 release.

In that case we should put FIXMEs in places where we have the quick fix, so we can deal with them later :)

Maybe also to open n issue about it

[raskad]

In which case it should be in the parser, not compilation.

#1829 also introduces syntax errors at compilation time. I'm not sure if it makes sense to push all of these errors to the parser. For some we would have to walk the whole AST, and we do that at compilation time anyways.

[addisoncrump]

Are the changes requested still relevant? I'm not sure what I need to change here.

[raskad]

Are the changes requested still relevant? I'm not sure what I need to change here.

The changes are still relevant. Can you rebase the branch on main?

@HalidOdat I created #1907 to track the progress on compile time errors.

[raskad]

@HalidOdat Can you give this another look so we can fix those panics on main?

[HalidOdat]

bors r+

[bors]

Pull request successfully merged into main.

Build succeeded:

• Clippy
• Documentation
• Examples
• Rustfmt
• Tests on Linux
• Tests on MacOS
• Tests on Windows