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

Type checker: don't suppress errors while checking expressions #18311

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/release-notes/.FSharp.Compiler.Service/9.0.300.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
* Add support for C# `Experimental` attribute. ([PR #18253](https://github.com/dotnet/fsharp/pull/18253))
* Nullness warnings are issued for signature<>implementation conformance ([PR #18186](https://github.com/dotnet/fsharp/pull/18186))
* Symbols: Add FSharpAssembly.IsFSharp ([PR #18290](https://github.com/dotnet/fsharp/pull/18290))
* Type checker: don't suppress errors while checking expressions ([PR #18311](https://github.com/dotnet/fsharp/pull/18311))
Copy link
Member

Choose a reason for hiding this comment

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

I think we will need better real-life IDE tests and what it does when typing incomplete snippets of code.
I assume there was a reason to suppress typecheck errors in case of parser errors, to prevent a wall of semantic errors jumping around when typing.

I admit the reasons behind it might have changed, but it would be safer if we could either test this more while typing.


### Changed

Expand Down
62 changes: 27 additions & 35 deletions src/Compiler/Checking/Expressions/CheckExpressions.fs
Original file line number Diff line number Diff line change
Expand Up @@ -5958,7 +5958,7 @@ and TcExprUndelayed (cenv: cenv) (overallTy: OverallTy) env tpenv (synExpr: SynE

| SynExpr.FromParseError (expr1, m) ->
//SolveTypeAsError cenv env.DisplayEnv m overallTy
let _, tpenv = suppressErrorReporting (fun () -> TcExpr cenv overallTy env tpenv expr1)
let _, tpenv = TcExpr cenv overallTy env tpenv expr1
mkDefault(m, overallTy.Commit), tpenv

| SynExpr.Sequential (sp, dir, synExpr1, synExpr2, m, _) ->
Expand Down Expand Up @@ -6487,9 +6487,7 @@ and TcIteratedLambdas (cenv: cenv) isFirst (env: TcEnv) overallTy takenNames tpe

| e ->
let env = { env with eIsControlFlow = true }
// Dive into the expression to check for syntax errors and suppress them if they show.
conditionallySuppressErrorReporting (not isFirst && synExprContainsError e) (fun () ->
TcExpr cenv overallTy env tpenv e)
TcExpr cenv overallTy env tpenv e

and TcTyparExprThen (cenv: cenv) overallTy env tpenv synTypar m delayed =
match delayed with
Expand Down Expand Up @@ -11093,39 +11091,33 @@ and TcNormalizedBinding declKind (cenv: cenv) env tpenv overallTy safeThisValOpt
// At each module binding, dive into the expression to check for syntax errors and suppress them if they show.
// Don't do this for lambdas, because we always check for suppression for all lambda bodies in TcIteratedLambdas
let rhsExprChecked, tpenv =
let atTopNonLambdaDefn =
declKind.IsModuleOrMemberOrExtensionBinding &&
(match rhsExpr with SynExpr.Lambda _ -> false | _ -> true) &&
synExprContainsError rhsExpr

conditionallySuppressErrorReporting atTopNonLambdaDefn (fun () ->

// Save the arginfos away to match them up in the lambda
let (PrelimValReprInfo(argInfos, _)) = prelimValReprInfo

// The right-hand-side is control flow (has an implicit debug point) in any situation where we
// haven't extended the debug point to include the 'let', that is, there is a debug point noted
// at the binding.
//
// This includes
// let _ = expr
// let () = expr
// which are transformed to sequential expressions in TcLetBinding
//
let rhsIsControlFlow =
match pat with
| SynPat.Wild _
| SynPat.Const (SynConst.Unit, _)
| SynPat.Paren (SynPat.Const (SynConst.Unit, _), _) -> true
| _ ->
match debugPoint with
| DebugPointAtBinding.Yes _ -> false
| _ -> true
// Save the arginfos away to match them up in the lambda
let (PrelimValReprInfo(argInfos, _)) = prelimValReprInfo

// The right-hand-side is control flow (has an implicit debug point) in any situation where we
// haven't extended the debug point to include the 'let', that is, there is a debug point noted
// at the binding.
//
// This includes
// let _ = expr
// let () = expr
// which are transformed to sequential expressions in TcLetBinding
//
let rhsIsControlFlow =
match pat with
| SynPat.Wild _
| SynPat.Const (SynConst.Unit, _)
| SynPat.Paren (SynPat.Const (SynConst.Unit, _), _) -> true
| _ ->

match debugPoint with
| DebugPointAtBinding.Yes _ -> false
| _ -> true

let envinner = { envinner with eLambdaArgInfos = argInfos; eIsControlFlow = rhsIsControlFlow }
let envinner = { envinner with eLambdaArgInfos = argInfos; eIsControlFlow = rhsIsControlFlow }

if isCtor then TcExprThatIsCtorBody (safeThisValOpt, safeInitInfo) cenv (MustEqual overallExprTy) envinner tpenv rhsExpr
else TcExprThatCantBeCtorBody cenv (MustConvertTo (false, overallExprTy)) envinner tpenv rhsExpr)
if isCtor then TcExprThatIsCtorBody (safeThisValOpt, safeInitInfo) cenv (MustEqual overallExprTy) envinner tpenv rhsExpr
else TcExprThatCantBeCtorBody cenv (MustConvertTo (false, overallExprTy)) envinner tpenv rhsExpr

if kind = SynBindingKind.StandaloneExpression && not cenv.isScript then
UnifyUnitType cenv env mBinding overallPatTy rhsExprChecked |> ignore<bool>
Expand Down
2 changes: 2 additions & 0 deletions tests/fsharp/typecheck/sigs/neg104.vsbsl
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ neg104.fs(8,9,8,15): typecheck error FS0750: This construct may only be used wit

neg104.fs(10,9,10,15): typecheck error FS0750: This construct may only be used within computation expressions

neg104.fs(13,21,13,26): typecheck error FS0003: This value is not a function and cannot be applied.

neg104.fs(20,9,20,15): typecheck error FS0025: Incomplete pattern matches on this expression.

neg104.fs(23,9,23,15): typecheck error FS0025: Incomplete pattern matches on this expression.
Expand Down
2 changes: 1 addition & 1 deletion tests/fsharp/typecheck/sigs/neg110.bsl
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
neg110.fs(5,3,5,15): typecheck error FS3242: This type does not inherit Attribute, it will not work correctly with other .NET languages.

neg110.fs(5,3,5,15): typecheck error FS3242: This type does not inherit Attribute, it will not work correctly with other .NET languages.
4 changes: 4 additions & 0 deletions tests/fsharp/typecheck/sigs/neg135.bsl
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,7 @@
neg135.fs(6,1,6,2): parse error FS0010: Unexpected symbol '}' in expression

neg135.fs(4,5,4,11): parse error FS3122: Missing 'do' in 'while' expression. Expected 'while <expr> do <expr>'.

neg135.fs(4,12,4,17): typecheck error FS0003: This value is not a function and cannot be applied.

neg135.fs(4,12,4,17): typecheck error FS0003: This value is not a function and cannot be applied.
4 changes: 4 additions & 0 deletions tests/fsharp/typecheck/sigs/neg64.vsbsl
Original file line number Diff line number Diff line change
@@ -1,2 +1,6 @@

neg64.fsx(32,32,32,33): parse error FS0010: Unexpected symbol ')' in if/then/else expression

neg64.fsx(32,17,32,31): typecheck error FS0001: All branches of an 'if' expression must return values implicitly convertible to the type of the first branch, which here is 'string'. This branch returns a value of type 'bool'.

neg64.fsx(27,11,27,13): typecheck error FS0025: Incomplete pattern matches on this expression. For example, the value 'Horizontal (_, _)' may indicate a case not covered by the pattern(s).
20 changes: 20 additions & 0 deletions tests/fsharp/typecheck/sigs/neg68.vsbsl
Original file line number Diff line number Diff line change
@@ -1,6 +1,26 @@

neg68.fsx(71,46,71,47): parse error FS0010: Unexpected symbol ')' in binding. Expected incomplete structured construct at or before this point or other token.

neg68.fsx(71,32,71,33): typecheck error FS0062: This construct is for ML compatibility. Consider using the '+' operator instead. This may require a type annotation to indicate it acts on strings. This message can be disabled using '--nowarn:62' or '#nowarn "62"'.

neg68.fsx(71,30,71,46): typecheck error FS0001: This expression was expected to have type
'float'
but here has type
'string'

neg68.fsx(71,30,71,31): typecheck error FS1133: No constructors are available for the type 'm'

neg68.fsx(71,43,71,44): typecheck error FS0062: This construct is for ML compatibility. Consider using the '+' operator instead. This may require a type annotation to indicate it acts on strings. This message can be disabled using '--nowarn:62' or '#nowarn "62"'.

neg68.fsx(71,39,71,45): typecheck error FS0001: The type 'string' does not match the type 'int'

neg68.fsx(71,39,71,41): typecheck error FS1133: No constructors are available for the type 'kg'

neg68.fsx(71,44,71,45): typecheck error FS0001: This expression was expected to have type
'string'
but here has type
'int'

neg68.fsx(123,40,123,41): typecheck error FS0001: The type 'bool' does not match the type 'float<'u>'

neg68.fsx(123,38,123,39): typecheck error FS0043: The type 'bool' does not match the type 'float<'u>'
12 changes: 12 additions & 0 deletions tests/fsharp/typecheck/sigs/neg70.vsbsl
Original file line number Diff line number Diff line change
@@ -1,2 +1,14 @@

neg70.fsx(109,64,109,65): parse error FS0010: Unexpected symbol ')' in binding. Expected incomplete structured construct at or before this point or other token.

neg70.fsx(107,29,107,69): typecheck error FS0507: No accessible member or object constructor named 'Rectangle' takes 0 arguments. Note the call to this member also provides 1 named arguments.

neg70.fsx(108,39,108,40): typecheck error FS0039: The value or constructor 'y' is not defined.

neg70.fsx(110,18,110,43): typecheck error FS0041: No overloads match for method 'FillEllipse'.

Known types of arguments: Brush * (int * bool * bool * bool)

Available overloads:
- Graphics.FillEllipse(brush: Brush, rect: Rectangle) : unit // Argument 'rect' doesn't match
- Graphics.FillEllipse(brush: Brush, rect: RectangleF) : unit // Argument 'rect' doesn't match
4 changes: 4 additions & 0 deletions tests/fsharp/typecheck/sigs/neg79.vsbsl
Original file line number Diff line number Diff line change
@@ -1,2 +1,6 @@

neg79.fsx(31,32,31,33): parse error FS0010: Unexpected symbol ')' in if/then/else expression

neg79.fsx(31,17,31,31): typecheck error FS0001: All branches of an 'if' expression must return values implicitly convertible to the type of the first branch, which here is 'string'. This branch returns a value of type 'bool'.

neg79.fsx(26,11,26,13): typecheck error FS0025: Incomplete pattern matches on this expression. For example, the value 'Horizontal (_, _)' may indicate a case not covered by the pattern(s).
2 changes: 2 additions & 0 deletions tests/fsharp/typecheck/sigs/neg80.vsbsl
Original file line number Diff line number Diff line change
@@ -1,2 +1,4 @@

neg80.fsx(79,5,79,6): parse error FS0010: Unexpected symbol '|' in pattern matching

neg80.fsx(79,7,79,28): typecheck error FS0026: This rule will never be matched
2 changes: 2 additions & 0 deletions tests/fsharp/typecheck/sigs/neg82.vsbsl
Original file line number Diff line number Diff line change
Expand Up @@ -26,4 +26,6 @@ To continue using non-conforming indentation, pass the '--strict-indentation-' f
neg82.fsx(138,1,138,4): parse error FS0058: Unexpected syntax or possible incorrect indentation: this token is offside of context started at position (102:1). Try indenting this further.
To continue using non-conforming indentation, pass the '--strict-indentation-' flag to the compiler, or set the language version to F# 7.

neg82.fsx(76,11,76,13): typecheck error FS0025: Incomplete pattern matches on this expression. For example, the value 'Horizontal (_, _)' may indicate a case not covered by the pattern(s).

neg82.fsx(93,5,93,7): typecheck error FS0039: The value, namespace, type or module 'sb' is not defined.
18 changes: 18 additions & 0 deletions tests/fsharp/typecheck/sigs/neg83.vsbsl
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,21 @@ neg83.fsx(13,2,13,5): parse error FS0058: Unexpected syntax or possible incorrec
To continue using non-conforming indentation, pass the '--strict-indentation-' flag to the compiler, or set the language version to F# 7.

neg83.fsx(16,1,16,1): parse error FS0010: Incomplete structured construct at or before this point in expression

neg83.fsx(8,12,8,39): typecheck error FS0001: Type mismatch. Expecting a
'('a -> 'a) -> string -> 'b'
but given a
''c list -> 'd list'
The type ''a list' does not match the type ''b -> 'b'

neg83.fsx(8,12,8,39): typecheck error FS0001: Type mismatch. Expecting a
'('a -> 'a) -> string -> 'b'
but given a
''c list -> 'd list'
The type ''a list' does not match the type ''c -> 'c'

neg83.fsx(10,15,10,17): typecheck error FS3217: This expression is not a function and cannot be applied. Did you intend to access the indexer via 'expr[index]'?

neg83.fsx(15,4,15,31): typecheck error FS0001: The type ''a list' does not match the type ''b -> 'b'

neg83.fsx(15,4,15,31): typecheck error FS0001: The type ''a list' does not match the type ''c -> 'c'
2 changes: 2 additions & 0 deletions tests/fsharp/typecheck/sigs/neg84.vsbsl
Original file line number Diff line number Diff line change
@@ -1,2 +1,4 @@

neg84.fsx(11,1,11,1): parse error FS0010: Incomplete structured construct at or before this point in expression

neg84.fsx(4,10,4,12): typecheck error FS0025: Incomplete pattern matches on this expression. For example, the value '[_]' may indicate a case not covered by the pattern(s).
7 changes: 7 additions & 0 deletions tests/fsharp/typecheck/sigs/neg86.vsbsl
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,13 @@ neg86.fsx(8,13,8,17): typecheck error FS3167: 'join' must be followed by 'in'. U

neg86.fsx(9,21,9,22): typecheck error FS0039: The value or constructor 'e' is not defined.

neg86.fsx(18,20,18,22): typecheck error FS3145: This is not a known query operator. Query operators are identifiers such as 'select', 'where', 'sortBy', 'thenBy', 'groupBy', 'groupValBy', 'join', 'groupJoin', 'sumBy' and 'averageBy', defined using corresponding methods on the 'QueryBuilder' type.

neg86.fsx(18,20,18,22): typecheck error FS0039: The value or constructor 'op_Atin' is not defined. Maybe you want one of the following:
Option
option
Option

neg86.fsx(23,13,23,17): typecheck error FS3097: Incorrect syntax for 'join'. Usage: join var in collection on (outerKey = innerKey). Note that parentheses are required after 'on'.

neg86.fsx(28,13,28,17): typecheck error FS3097: Incorrect syntax for 'join'. Usage: join var in collection on (outerKey = innerKey). Note that parentheses are required after 'on'.
Expand Down
Loading