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

Warn when obj is inferred #13298

Merged
merged 50 commits into from
Jul 21, 2023
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
50 commits
Select commit Hold shift + click to select a range
4a24440
First pass at a warning when obj is inferred
Smaug123 Jun 14, 2022
ef5090f
Add more tests, some of which are failing
Smaug123 Jun 14, 2022
9e1c8c2
A couple more tests
Smaug123 Jun 14, 2022
4f0390c
Add test for simpler quotation case
Smaug123 Jun 15, 2022
4d48759
Turn diagnostic off by default
Smaug123 Jun 15, 2022
5794221
Add a test which is failing, no idea why
Smaug123 Jun 15, 2022
c09ac18
Update text
Smaug123 Jul 14, 2022
55fd658
Fix up messages
Smaug123 Jul 29, 2022
2200ddd
Merge main
Smaug123 Jul 29, 2022
b15bd8b
Restructure tests
Smaug123 Jul 29, 2022
97ffb68
Speculative commit to restore range information where required
Smaug123 Jul 29, 2022
ee81996
Another failing test
Smaug123 Jan 22, 2023
e8d64fa
Resolve merge
Smaug123 Jan 23, 2023
2235295
Merge main, sloppily
Smaug123 Jan 23, 2023
aa155b0
Merge main
Smaug123 Jan 23, 2023
030a5cb
Tidy
Smaug123 Jan 23, 2023
76ea258
Take 2
Smaug123 Jan 23, 2023
849a97f
Add another test
Smaug123 Jan 23, 2023
5bfef32
Merge main
Smaug123 Feb 1, 2023
25f3ba1
Add prospective test
Smaug123 Feb 3, 2023
248b7a5
Merge branch 'obj-inference-warning' into obj-inference-take-2
Smaug123 Apr 12, 2023
c47dce2
Merge branch 'main' into obj-inference-warning
Smaug123 Apr 12, 2023
c6242c6
Merge remote-tracking branch 'origin/main' into obj-inference-warning
Smaug123 May 30, 2023
286fe21
Fix build
Smaug123 May 31, 2023
49db33f
Adjust message
Smaug123 May 31, 2023
52dc05d
Add extra fallback typar ranges
Smaug123 Jun 2, 2023
88b7780
WIP extra tests
Smaug123 Jun 2, 2023
8485188
Add fallback range
Smaug123 Jun 2, 2023
609fe05
Revert
Smaug123 Jun 2, 2023
407040b
Reinstate
Smaug123 Jun 2, 2023
d6479c5
Merge branch 'add-fallback-range' into obj-inference-warning
Smaug123 Jun 2, 2023
a2e7f1e
More tests
Smaug123 Jun 2, 2023
adea31c
Merge branch 'main' into obj-inference-warning
Smaug123 Jun 2, 2023
5c991f9
Merge main
Smaug123 Jun 5, 2023
3ee4928
Downgrade warning
Smaug123 Jun 5, 2023
79e4de5
Merge branch 'obj-inference-warning' of github.com:Smaug123/fsharp-1 …
Smaug123 Jun 5, 2023
3356912
Fix tests after merge
Smaug123 Jun 5, 2023
d99753b
Merge branch 'main' into obj-inference-warning
Smaug123 Jun 6, 2023
862c87d
Gate the diagnostic
Smaug123 Jun 6, 2023
2a7d7d9
Fix error
Smaug123 Jun 8, 2023
5f4288d
Merge branch 'main' into obj-inference-warning
Smaug123 Jun 8, 2023
710bab0
Enable language version in tests
Smaug123 Jun 8, 2023
23601d5
Merge branch 'main' into obj-inference-warning
Smaug123 Jun 8, 2023
a055b18
Merge branch 'main' into obj-inference-warning
T-Gro Jun 13, 2023
ac53fb6
Markups
Smaug123 Jun 14, 2023
7cde8eb
Merge branch 'obj-inference-warning' of github.com:Smaug123/fsharp-1 …
Smaug123 Jun 14, 2023
c1cb455
Merge remote-tracking branch 'origin/main' into obj-inference-warning
Smaug123 Jun 14, 2023
f1404c0
Merge branch 'main' into obj-inference-warning
Smaug123 Jul 4, 2023
8beaa75
Merge branch 'main' into obj-inference-warning
Smaug123 Jul 6, 2023
5e99aae
Merge branch 'obj-inference-warning' of github.com:Smaug123/fsharp-1 …
Smaug123 Jul 6, 2023
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
11 changes: 10 additions & 1 deletion docs/diagnostics.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ Adding or adjusting diagnostics emitted by the compiler is usually straightforwa
4. Use another search tool or a tool like Find All References / Find Usages to see where it's used in the compiler source code.
5. Set a breakpoint at the location in source you found. If you debug the compiler with the same steps, it should trigger the breakpoint you set. This verifies that the location you found is the one that emits the error or warning you want to improve.

From here, you can either simply update the error test, or you can use some of the information at the point in the source code you identified to see if there is more information to include in the error message. For example, if the error message doesn't contain information about the identifier the user is using incorrectly, you may be able to include the name of the identifier based on data the compiler has available at that stage of compilation.
From here, you can either simply update the error text, or you can use some of the information at the point in the source code you identified to see if there is more information to include in the error message. For example, if the error message doesn't contain information about the identifier the user is using incorrectly, you may be able to include the name of the identifier based on data the compiler has available at that stage of compilation.

If you're including data from user code in an error message, it's important to also write a test that verifies the exact error message for a given string of F# code.

Expand All @@ -55,3 +55,12 @@ Diagnostics must often format types.
* When displaying multiple types in a comparative way, for example, two types that didn't match, you will want to display the minimal amount of infomation to convey the fact that the two types are different, for example, `NicePrint.minimalStringsOfTwoTypes`.

* When displaying a type, you have the option of displaying the constraints implied by any type variables mentioned in the types, appended as `when ...`. For example, `NicePrint.layoutPrettifiedTypeAndConstraints`.

## Localization

Smaug123 marked this conversation as resolved.
Show resolved Hide resolved
The file `FSComp.txt` contains the canonical listing of diagnostic messages, but there are also `.xlf` localization files for various languages.
When changing `FSComp.txt`, you can automatically put placeholder entries in all these `.xlf` files by running `dotnet msbuild /t:UpdateXlf FSharp.sln`.

## Enabling a warning or error by default

The file `CompilerDiagnostics.fs` contains the function `IsWarningOrInfoEnabled`, which determines whether a given diagnostic is emitted.
67 changes: 36 additions & 31 deletions src/Compiler/Checking/TypeRelations.fs
Original file line number Diff line number Diff line change
Expand Up @@ -134,50 +134,55 @@ let rec TypeFeasiblySubsumesType ndeep g amap m ty1 canCoerce ty2 =
/// Here x gets a generalized type "list<'T>".
let ChooseTyparSolutionAndRange (g: TcGlobals) amap (tp:Typar) =
let m = tp.Range
let maxTy, m =
let initialTy =
match tp.Kind with
| TyparKind.Type -> g.obj_ty
let (maxTy, isRefined), m =
let initialTy =
match tp.Kind with
| TyparKind.Type -> g.obj_ty
| TyparKind.Measure -> TType_measure Measure.One
// Loop through the constraints computing the lub
((initialTy, m), tp.Constraints) ||> List.fold (fun (maxTy, _) tpc ->
let join m x =
if TypeFeasiblySubsumesType 0 g amap m x CanCoerce maxTy then maxTy
elif TypeFeasiblySubsumesType 0 g amap m maxTy CanCoerce x then x
else errorR(Error(FSComp.SR.typrelCannotResolveImplicitGenericInstantiation((DebugPrint.showType x), (DebugPrint.showType maxTy)), m)); maxTy
// Don't continue if an error occurred and we set the value eagerly
if tp.IsSolved then maxTy, m else
match tpc with
(((initialTy, false), m), tp.Constraints) ||> List.fold (fun ((maxTy, haveRefined), _) tpc ->
let join m x =
if TypeFeasiblySubsumesType 0 g amap m x CanCoerce maxTy then maxTy, haveRefined
elif TypeFeasiblySubsumesType 0 g amap m maxTy CanCoerce x then x, true
else errorR(Error(FSComp.SR.typrelCannotResolveImplicitGenericInstantiation((DebugPrint.showType x), (DebugPrint.showType maxTy)), m)); maxTy, haveRefined
// Don't continue if an error occurred and we set the value eagerly
if tp.IsSolved then (maxTy, haveRefined), m else
match tpc with
| TyparConstraint.CoercesTo(x, m) ->
join m x, m
| TyparConstraint.MayResolveMember(_traitInfo, m) ->
maxTy, m
| TyparConstraint.SimpleChoice(_, m) ->
(maxTy, haveRefined), m
| TyparConstraint.SimpleChoice(_, m) ->
errorR(Error(FSComp.SR.typrelCannotResolveAmbiguityInPrintf(), m))
maxTy, m
| TyparConstraint.SupportsNull m ->
maxTy, m
| TyparConstraint.SupportsComparison m ->
(maxTy, haveRefined), m
| TyparConstraint.SupportsNull m ->
(maxTy, haveRefined), m
| TyparConstraint.SupportsComparison m ->
join m g.mk_IComparable_ty, m
| TyparConstraint.SupportsEquality m ->
maxTy, m
| TyparConstraint.IsEnum(_, m) ->
(maxTy, haveRefined), m
| TyparConstraint.IsEnum(_, m) ->
errorR(Error(FSComp.SR.typrelCannotResolveAmbiguityInEnum(), m))
maxTy, m
| TyparConstraint.IsDelegate(_, _, m) ->
(maxTy, haveRefined), m
| TyparConstraint.IsDelegate(_, _, m) ->
errorR(Error(FSComp.SR.typrelCannotResolveAmbiguityInDelegate(), m))
maxTy, m
| TyparConstraint.IsNonNullableStruct m ->
(maxTy, haveRefined), m
| TyparConstraint.IsNonNullableStruct m ->
join m g.int_ty, m
| TyparConstraint.IsUnmanaged m ->
errorR(Error(FSComp.SR.typrelCannotResolveAmbiguityInUnmanaged(), m))
maxTy, m
| TyparConstraint.RequiresDefaultConstructor m ->
maxTy, m
| TyparConstraint.IsReferenceType m ->
maxTy, m
| TyparConstraint.DefaultsTo(_priority, _ty, m) ->
maxTy, m)
(maxTy, haveRefined), m
| TyparConstraint.RequiresDefaultConstructor m ->
(maxTy, haveRefined), m
| TyparConstraint.IsReferenceType m ->
(maxTy, haveRefined), m
| TyparConstraint.DefaultsTo(_priority, _ty, m) ->
(maxTy, haveRefined), m)
match tp.Kind with
| TyparKind.Type ->
if not isRefined then
warning(Error(FSComp.SR.typrelNeverRefinedAwayFromTop(), m))
Smaug123 marked this conversation as resolved.
Show resolved Hide resolved
| _ -> ()
maxTy, m

let ChooseTyparSolution g amap tp =
Expand Down
3 changes: 2 additions & 1 deletion src/Compiler/Driver/CompilerDiagnostics.fs
Original file line number Diff line number Diff line change
Expand Up @@ -368,7 +368,7 @@ let GetWarningLevel diagnostic =
let IsWarningOrInfoEnabled (diagnostic, severity) n level specificWarnOn =
List.contains n specificWarnOn
||
// Some specific warnings/informational are never on by default, i.e. unused variable warnings
// Some specific warnings/informational are never on by default, e.g. unused variable warnings
match n with
| 1182 -> false // chkUnusedValue - off by default
| 3180 -> false // abImplicitHeapAllocation - off by default
Expand All @@ -379,6 +379,7 @@ let IsWarningOrInfoEnabled (diagnostic, severity) n level specificWarnOn =
| 3389 -> false // tcBuiltInImplicitConversionUsed - off by default
| 3390 -> false // xmlDocBadlyFormed - off by default
| 3395 -> false // tcImplicitConversionUsedForMethodArg - off by default
| 3525 -> false // typrelNeverRefinedAwayFromTop - off by default
| _ ->
(severity = FSharpDiagnosticSeverity.Info)
|| (severity = FSharpDiagnosticSeverity.Warning
Expand Down
3 changes: 2 additions & 1 deletion src/Compiler/FSComp.txt
Original file line number Diff line number Diff line change
Expand Up @@ -1631,4 +1631,5 @@ reprStateMachineInvalidForm,"The state machine has an unexpected form"
3522,tcAnonRecdDuplicateFieldId,"The field '%s' appears multiple times in this record expression."
3523,tcAnonRecdTypeDuplicateFieldId,"The field '%s' appears multiple times in this anonymous record type."
3524,parsExpectingExpressionInTuple,"Expecting expression"
3545,tcMissingRequiredMembers,"The following required properties have to be initalized:%s"
3525,typrelNeverRefinedAwayFromTop,"A type inference variable has been implicitly inferred to have type `obj`. Consider adding explicit type annotations. This warning is off by default and has been explicitly enabled for this project. You may suppress this warning by using #nowarn \"3525\"."
3545,tcMissingRequiredMembers,"The following required properties have to be initalized:%s"
7 changes: 6 additions & 1 deletion src/Compiler/xlf/FSComp.txt.cs.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Compiler/xlf/FSComp.txt.de.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Compiler/xlf/FSComp.txt.es.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Compiler/xlf/FSComp.txt.fr.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Compiler/xlf/FSComp.txt.it.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Compiler/xlf/FSComp.txt.ja.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Compiler/xlf/FSComp.txt.ko.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Compiler/xlf/FSComp.txt.pl.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Compiler/xlf/FSComp.txt.pt-BR.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading