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

Missing argument compiles without error and throws CLR error during runtime #15955

Closed
kdurkin77 opened this issue Sep 8, 2023 · 5 comments · Fixed by #15999
Closed

Missing argument compiles without error and throws CLR error during runtime #15955

kdurkin77 opened this issue Sep 8, 2023 · 5 comments · Fixed by #15999
Labels
Area-Compiler-Syntax lexfilter, indentation and parsing Bug Impact-High (Internal MS Team use only) Describes an issue with extreme impact on existing code. Regression
Milestone

Comments

@kdurkin77
Copy link

If we leave out an argument by mistake the compiler doesn't error and we can even run the code, but on runtime it crashes

This code compiles successfully when it shouldn't:
let x = doesnt.exist(, nor.this.either, also.nope)
image

This code does not compile successfully as it shouldn't:
let x = doesnt.exist(nope, nor.this.either, also.nope)
image

Repro steps

  1. Open fs file in Visual Studio Code
    MissingArgumentShouldNotCompile.zip
  2. See no errors
  3. Try to run and see a CLR error and crash

Expected behavior
The code should not compile

Actual behavior
The code compiles

Known workarounds
None

Related information
Seen in both Visual Studio Code 1.82.0 and Visual Studio 2022 17.7.3 (both up to date)

@kerams
Copy link
Contributor

kerams commented Sep 8, 2023

Almost certainly a regression in recovery. The binding is ignored completely, so I guess FSI is trying to evaluate something that does not produce any code?

No parsing error is raised despite the presence of SynExpr.ArbitraryAfterError.

@vzarytovskii
Copy link
Member

Huh, @auduchinok, does this ring a bell?

@kdurkin77
Copy link
Author

Also, this has happened in a full VS fsproj as well as the fsi like in the example

@T-Gro T-Gro added Area-Compiler-Syntax lexfilter, indentation and parsing Impact-High (Internal MS Team use only) Describes an issue with extreme impact on existing code. labels Sep 11, 2023
@vzarytovskii
Copy link
Member

Introduced in #15227
@auduchinok FYI

Bisect script used:

dotnet build FSharp.Compiler.Service.sln -c Release
.\artifacts\bin\fsi\Release\net7.0\fsi.exe ..\reg15955.fsx

@echo off
if %ERRORLEVEL% EQU -1073741819 (
   echo Failure Reason Given is %errorlevel%
   exit /b 1
) else (
   echo Success
   exit /b 0
)

reg15955.fsx:

let x = doesnt.exist(, nor.this.either, also.nope)
git bisect start
git bisect bad e1131e2
git bisect good 124ca6bd0257ac8c594abe0390befd0d039d7f13
git bisect run ./bisect.cmd

@smoothdeveloper
Copy link
Contributor

@vzarytovskii @auduchinok, should there be a list of AST nodes that are known to not be allowed to "pass" to the code generation part?

I see ArbitraryAfterError is present in many of the test cases baselines, maybe an invariant check before transforming to the typed AST, that would be checked and kept in the context during type checking and later stages.

This contextual flag would then be used to prevent code generation.

Would that be a good approach to prevent similar mishaps?

Also, the infrastructure that runs those parser recovery tests could generate the IL baseline files, expecting them to be empty, and make the test to fail if not.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Compiler-Syntax lexfilter, indentation and parsing Bug Impact-High (Internal MS Team use only) Describes an issue with extreme impact on existing code. Regression
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

6 participants