Skip to content

Remove one "impossible" error #5746

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Oct 15, 2018
Merged

Remove one "impossible" error #5746

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2383ff0
Remove one "impossible" error - fixes #5745
forki Oct 9, 2018
c256b80
Fix tests
forki Oct 9, 2018
05ec633
Add test for #5745
forki Oct 9, 2018
7a23ccf
Fix "build from source"
forki Oct 9, 2018
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
9 changes: 9 additions & 0 deletions src/buildfromsource/FSharp.Compiler.Private/FSComp.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3916,6 +3916,15 @@ type internal SR private() =
/// Union case/exception '%s' does not have field named '%s'.
/// (Originally from ..\FSComp.txt:1296)
static member tcUnionCaseConstructorDoesNotHaveFieldWithGivenName(a0 : System.String, a1 : System.String) = (3174, GetStringFunc("tcUnionCaseConstructorDoesNotHaveFieldWithGivenName",",,,%s,,,%s,,,") a0 a1)
/// The exception '%s' does not have a field named '%s'.
/// (Originally from ..\FSComp.txt:1297)
static member tcExceptionConstructorDoesNotHaveFieldWithGivenName(a0 : System.String, a1 : System.String) = (3174, GetStringFunc("tcExceptionConstructorDoesNotHaveFieldWithGivenName",",,,%s,,,%s,,,") a0 a1)
/// Active patterns do not have fields. This syntax is invalid.
/// (Originally from ..\FSComp.txt:1298)
static member tcActivePatternsDoNotHaveFields() = (3174, GetStringFunc("tcActivePatternsDoNotHaveFields",",,,") )
/// The constructor does not have a field named '%s'.
/// (Originally from ..\FSComp.txt:1299)
static member tcConstructorDoesNotHaveFieldWithGivenName(a0 : System.String) = (3174, GetStringFunc("tcConstructorDoesNotHaveFieldWithGivenName",",,,%s,,,") a0)
/// Union case/exception field '%s' cannot be used more than once.
/// (Originally from ..\FSComp.txt:1297)
static member tcUnionCaseFieldCannotBeUsedMoreThanOnce(a0 : System.String) = (3175, GetStringFunc("tcUnionCaseFieldCannotBeUsedMoreThanOnce",",,,%s,,,") a0)
Expand Down
11 changes: 10 additions & 1 deletion src/buildfromsource/FSharp.Compiler.Private/FSComp.resx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3916,7 +3916,16 @@
<value>Array method '{0}' is supplied by the runtime and cannot be directly used in code. For operations with array elements consider using family of GetArray/SetArray functions from LanguagePrimitives.IntrinsicFunctions module.</value>
</data>
<data name="tcUnionCaseConstructorDoesNotHaveFieldWithGivenName" xml:space="preserve">
<value>Union case/exception '{0}' does not have field named '{1}'.</value>
<value>The union case '{0}' does not have a field named '{1}'.</value>
</data>
<data name="tcExceptionConstructorDoesNotHaveFieldWithGivenName" xml:space="preserve">
<value>The exception '{0}' does not have a field named '{1}'.</value>
</data>
<data name="tcActivePatternsDoNotHaveFields" xml:space="preserve">
<value>Active patterns do not have fields. This syntax is invalid.</value>
</data>
<data name="tcConstructorDoesNotHaveFieldWithGivenName" xml:space="preserve">
<value>The constructor does not have a field named '{0}'.</value>
</data>
<data name="tcUnionCaseFieldCannotBeUsedMoreThanOnce" xml:space="preserve">
<value>Union case/exception field '{0}' cannot be used more than once.</value>
Expand Down
5 changes: 4 additions & 1 deletion src/fsharp/FSComp.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1293,7 +1293,10 @@ descriptionUnavailable,"(description unavailable...)"
3171,tcGeneratedTypesShouldBeInternalOrPrivate,"The provided types generated by this use of a type provider may not be used from other F# assemblies and should be marked internal or private. Consider using 'type internal TypeName = ...' or 'type private TypeName = ...'."
3172,chkGetterAndSetterHaveSamePropertyType,"A property's getter and setter must have the same type. Property '%s' has getter of type '%s' but setter of type '%s'."
3173,tcRuntimeSuppliedMethodCannotBeUsedInUserCode,"Array method '%s' is supplied by the runtime and cannot be directly used in code. For operations with array elements consider using family of GetArray/SetArray functions from LanguagePrimitives.IntrinsicFunctions module."
3174,tcUnionCaseConstructorDoesNotHaveFieldWithGivenName,"Union case/exception '%s' does not have field named '%s'."
3174,tcUnionCaseConstructorDoesNotHaveFieldWithGivenName,"The union case '%s' does not have a field named '%s'."
3174,tcExceptionConstructorDoesNotHaveFieldWithGivenName,"The exception '%s' does not have a field named '%s'."
3174,tcActivePatternsDoNotHaveFields,"Active patterns do not have fields. This syntax is invalid."
3174,tcConstructorDoesNotHaveFieldWithGivenName,"The constructor does not have a field named '%s'."
3175,tcUnionCaseFieldCannotBeUsedMoreThanOnce,"Union case/exception field '%s' cannot be used more than once."
3176,tcFieldNameIsUsedModeThanOnce,"Named field '%s' is used more than once."
3176,tcFieldNameConflictsWithGeneratedNameForAnonymousField,"Named field '%s' conflicts with autogenerated name for anonymous field."
Expand Down
31 changes: 18 additions & 13 deletions src/fsharp/TypeChecker.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5316,12 +5316,14 @@ and TcPat warnOnUpper cenv env topValInfo vFlags (tpenv, names, takenNames) ty p
for (id, pat) in pairs do
match argNames |> List.tryFindIndex (fun id2 -> id.idText = id2.idText) with
| None ->
let caseName =
match item with
| Item.UnionCase(uci, _) -> uci.Name
| Item.ExnCase tcref -> tcref.DisplayName
| _ -> failwith "impossible"
error(Error(FSComp.SR.tcUnionCaseConstructorDoesNotHaveFieldWithGivenName(caseName, id.idText), id.idRange))
match item with
| Item.UnionCase(uci, _) ->
error(Error(FSComp.SR.tcUnionCaseConstructorDoesNotHaveFieldWithGivenName(uci.Name, id.idText), id.idRange))
| Item.ExnCase tcref ->
error(Error(FSComp.SR.tcExceptionConstructorDoesNotHaveFieldWithGivenName(tcref.DisplayName, id.idText), id.idRange))
| _ ->
error(Error(FSComp.SR.tcConstructorDoesNotHaveFieldWithGivenName(id.idText), id.idRange))
Copy link
Contributor

Choose a reason for hiding this comment

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

But this case is impossible, correct? Shouldn't we report this somehow?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

"impossible"

Copy link
Contributor

Choose a reason for hiding this comment

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

What I mean is that item is matched some lines above to be one of the two cases and item is immutable? This case means someone has changed something and missed to do some work. Removing failwith means the mistake will be spotted later (or not at all)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

actually that is what already happend. the case was added and nobody changed the failwith "impossible"

Copy link
Contributor

Choose a reason for hiding this comment

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

But line above reads | (Item.UnionCase _ | Item.ExnCase _) as item -> how can this match another case?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it does not today. wait until someone adds the active pattern part ;-)


| Some idx ->
match box result.[idx] with
| null ->
Expand Down Expand Up @@ -8644,13 +8646,16 @@ and TcItemThen cenv overallTy env tpenv (item, mItem, rest, afterResolution) del
assert (isNull(box fittedArgs.[currentIndex]))
fittedArgs.[currentIndex] <- List.item currentIndex args // grab original argument, not item from the list of named parameters
currentIndex <- currentIndex + 1
else
let caseName =
match item with
| Item.UnionCase(uci, _) -> uci.Name
| Item.ExnCase tcref -> tcref.DisplayName
| _ -> failwith "impossible"
error(Error(FSComp.SR.tcUnionCaseConstructorDoesNotHaveFieldWithGivenName(caseName, id.idText), id.idRange))
else
match item with
| Item.UnionCase(uci, _) ->
error(Error(FSComp.SR.tcUnionCaseConstructorDoesNotHaveFieldWithGivenName(uci.Name, id.idText), id.idRange))
| Item.ExnCase tcref ->
error(Error(FSComp.SR.tcExceptionConstructorDoesNotHaveFieldWithGivenName(tcref.DisplayName, id.idText), id.idRange))
| Item.ActivePatternResult(_,_,_,_) ->
error(Error(FSComp.SR.tcActivePatternsDoNotHaveFields(), id.idRange))
| _ ->
error(Error(FSComp.SR.tcConstructorDoesNotHaveFieldWithGivenName(id.idText), id.idRange))
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we still report that this error message might need to be refined (in case it happens)? Would this be an advantage of failwith "impossible" here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

failwith "impossible" is arguably the worst you can do

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes ideally the code would be written in a way to not compile when written incorrectly, but if that cannot be achieved imho throwing is better than doing/reporting potentially incorrect messages

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually I think even having a confusing error message is better than what existed. The current state of the world (without @forki's changes) paints the whole editor red if you use the incorrect code:

screen shot 2018-10-09 at 07 28 16

These internal errors are critical to root out given this, since it's pretty much undiagnosable where the user error is.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok but imho we still should make sure the error message indicates this is not implemented and something which needs to be fixed if it occurs.
Otherwise users get wrong messages on their code which is arguably worse than a compiler crash

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we don't give a wrong message. We just give a very generic one


assert (Seq.forall (box >> ((<>) null) ) fittedArgs)
List.ofArray fittedArgs
Expand Down
19 changes: 17 additions & 2 deletions src/fsharp/xlf/FSComp.txt.cs.xlf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6328,8 +6328,8 @@
<note />
</trans-unit>
<trans-unit id="tcUnionCaseConstructorDoesNotHaveFieldWithGivenName">
<source>Union case/exception '{0}' does not have field named '{1}'.</source>
<target state="translated">Výjimka nebo případ typu union {0} nemá pole s názvem {1}.</target>
<source>The union case '{0}' does not have a field named '{1}'.</source>
<target state="needs-review-translation">Výjimka nebo případ typu union {0} nemá pole s názvem {1}.</target>
<note />
</trans-unit>
<trans-unit id="tcUnionCaseFieldCannotBeUsedMoreThanOnce">
Expand Down Expand Up @@ -7067,6 +7067,21 @@
<target state="new">This type does not inherit Attribute, it will not work correctly with other .NET languages.</target>
<note />
</trans-unit>
<trans-unit id="tcExceptionConstructorDoesNotHaveFieldWithGivenName">
<source>The exception '{0}' does not have a field named '{1}'.</source>
<target state="new">The exception '{0}' does not have a field named '{1}'.</target>
<note />
</trans-unit>
<trans-unit id="tcConstructorDoesNotHaveFieldWithGivenName">
<source>The constructor does not have a field named '{0}'.</source>
<target state="new">The constructor does not have a field named '{0}'.</target>
<note />
</trans-unit>
<trans-unit id="tcActivePatternsDoNotHaveFields">
<source>Active patterns do not have fields. This syntax is invalid.</source>
<target state="new">Active patterns do not have fields. This syntax is invalid.</target>
<note />
</trans-unit>
</body>
</file>
</xliff>
19 changes: 17 additions & 2 deletions src/fsharp/xlf/FSComp.txt.de.xlf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6328,8 +6328,8 @@
<note />
</trans-unit>
<trans-unit id="tcUnionCaseConstructorDoesNotHaveFieldWithGivenName">
<source>Union case/exception '{0}' does not have field named '{1}'.</source>
<target state="translated">Union-Fall/Ausnahme '{0}' verfügt nicht über ein Feld mit dem Namen '{1}'.</target>
<source>The union case '{0}' does not have a field named '{1}'.</source>
<target state="needs-review-translation">Union-Fall/Ausnahme '{0}' verfügt nicht über ein Feld mit dem Namen '{1}'.</target>
<note />
</trans-unit>
<trans-unit id="tcUnionCaseFieldCannotBeUsedMoreThanOnce">
Expand Down Expand Up @@ -7067,6 +7067,21 @@
<target state="new">This type does not inherit Attribute, it will not work correctly with other .NET languages.</target>
<note />
</trans-unit>
<trans-unit id="tcExceptionConstructorDoesNotHaveFieldWithGivenName">
<source>The exception '{0}' does not have a field named '{1}'.</source>
<target state="new">The exception '{0}' does not have a field named '{1}'.</target>
<note />
</trans-unit>
<trans-unit id="tcConstructorDoesNotHaveFieldWithGivenName">
<source>The constructor does not have a field named '{0}'.</source>
<target state="new">The constructor does not have a field named '{0}'.</target>
<note />
</trans-unit>
<trans-unit id="tcActivePatternsDoNotHaveFields">
<source>Active patterns do not have fields. This syntax is invalid.</source>
<target state="new">Active patterns do not have fields. This syntax is invalid.</target>
<note />
</trans-unit>
</body>
</file>
</xliff>
19 changes: 17 additions & 2 deletions src/fsharp/xlf/FSComp.txt.en.xlf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6328,8 +6328,8 @@
<note />
</trans-unit>
<trans-unit id="tcUnionCaseConstructorDoesNotHaveFieldWithGivenName">
<source>Union case/exception '{0}' does not have field named '{1}'.</source>
<target state="new">Union case/exception '{0}' does not have field named '{1}'.</target>
<source>The union case '{0}' does not have a field named '{1}'.</source>
<target state="new">The union case '{0}' does not have a field named '{1}'.</target>
<note />
</trans-unit>
<trans-unit id="tcUnionCaseFieldCannotBeUsedMoreThanOnce">
Expand Down Expand Up @@ -7067,6 +7067,21 @@
<target state="new">This type does not inherit Attribute, it will not work correctly with other .NET languages.</target>
<note />
</trans-unit>
<trans-unit id="tcExceptionConstructorDoesNotHaveFieldWithGivenName">
<source>The exception '{0}' does not have a field named '{1}'.</source>
<target state="new">The exception '{0}' does not have a field named '{1}'.</target>
<note />
</trans-unit>
<trans-unit id="tcConstructorDoesNotHaveFieldWithGivenName">
<source>The constructor does not have a field named '{0}'.</source>
<target state="new">The constructor does not have a field named '{0}'.</target>
<note />
</trans-unit>
<trans-unit id="tcActivePatternsDoNotHaveFields">
<source>Active patterns do not have fields. This syntax is invalid.</source>
<target state="new">Active patterns do not have fields. This syntax is invalid.</target>
<note />
</trans-unit>
</body>
</file>
</xliff>
19 changes: 17 additions & 2 deletions src/fsharp/xlf/FSComp.txt.es.xlf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6328,8 +6328,8 @@
<note />
</trans-unit>
<trans-unit id="tcUnionCaseConstructorDoesNotHaveFieldWithGivenName">
<source>Union case/exception '{0}' does not have field named '{1}'.</source>
<target state="translated">La excepción o el caso de unión '{0}' no tiene un campo denominado '{1}'.</target>
<source>The union case '{0}' does not have a field named '{1}'.</source>
<target state="needs-review-translation">La excepción o el caso de unión '{0}' no tiene un campo denominado '{1}'.</target>
<note />
</trans-unit>
<trans-unit id="tcUnionCaseFieldCannotBeUsedMoreThanOnce">
Expand Down Expand Up @@ -7067,6 +7067,21 @@
<target state="new">This type does not inherit Attribute, it will not work correctly with other .NET languages.</target>
<note />
</trans-unit>
<trans-unit id="tcExceptionConstructorDoesNotHaveFieldWithGivenName">
<source>The exception '{0}' does not have a field named '{1}'.</source>
<target state="new">The exception '{0}' does not have a field named '{1}'.</target>
<note />
</trans-unit>
<trans-unit id="tcConstructorDoesNotHaveFieldWithGivenName">
<source>The constructor does not have a field named '{0}'.</source>
<target state="new">The constructor does not have a field named '{0}'.</target>
<note />
</trans-unit>
<trans-unit id="tcActivePatternsDoNotHaveFields">
<source>Active patterns do not have fields. This syntax is invalid.</source>
<target state="new">Active patterns do not have fields. This syntax is invalid.</target>
<note />
</trans-unit>
</body>
</file>
</xliff>
19 changes: 17 additions & 2 deletions src/fsharp/xlf/FSComp.txt.fr.xlf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6328,8 +6328,8 @@
<note />
</trans-unit>
<trans-unit id="tcUnionCaseConstructorDoesNotHaveFieldWithGivenName">
<source>Union case/exception '{0}' does not have field named '{1}'.</source>
<target state="translated">Le cas ou l'exception d'union '{0}' ne possède pas de champ nommé '{1}'.</target>
<source>The union case '{0}' does not have a field named '{1}'.</source>
<target state="needs-review-translation">Le cas ou l'exception d'union '{0}' ne possède pas de champ nommé '{1}'.</target>
<note />
</trans-unit>
<trans-unit id="tcUnionCaseFieldCannotBeUsedMoreThanOnce">
Expand Down Expand Up @@ -7067,6 +7067,21 @@
<target state="new">This type does not inherit Attribute, it will not work correctly with other .NET languages.</target>
<note />
</trans-unit>
<trans-unit id="tcExceptionConstructorDoesNotHaveFieldWithGivenName">
<source>The exception '{0}' does not have a field named '{1}'.</source>
<target state="new">The exception '{0}' does not have a field named '{1}'.</target>
<note />
</trans-unit>
<trans-unit id="tcConstructorDoesNotHaveFieldWithGivenName">
<source>The constructor does not have a field named '{0}'.</source>
<target state="new">The constructor does not have a field named '{0}'.</target>
<note />
</trans-unit>
<trans-unit id="tcActivePatternsDoNotHaveFields">
<source>Active patterns do not have fields. This syntax is invalid.</source>
<target state="new">Active patterns do not have fields. This syntax is invalid.</target>
<note />
</trans-unit>
</body>
</file>
</xliff>
19 changes: 17 additions & 2 deletions src/fsharp/xlf/FSComp.txt.it.xlf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6328,8 +6328,8 @@
<note />
</trans-unit>
<trans-unit id="tcUnionCaseConstructorDoesNotHaveFieldWithGivenName">
<source>Union case/exception '{0}' does not have field named '{1}'.</source>
<target state="translated">Il case di unione/eccezione '{0}' non contiene il campo denominato '{1}'.</target>
<source>The union case '{0}' does not have a field named '{1}'.</source>
<target state="needs-review-translation">Il case di unione/eccezione '{0}' non contiene il campo denominato '{1}'.</target>
<note />
</trans-unit>
<trans-unit id="tcUnionCaseFieldCannotBeUsedMoreThanOnce">
Expand Down Expand Up @@ -7067,6 +7067,21 @@
<target state="new">This type does not inherit Attribute, it will not work correctly with other .NET languages.</target>
<note />
</trans-unit>
<trans-unit id="tcExceptionConstructorDoesNotHaveFieldWithGivenName">
<source>The exception '{0}' does not have a field named '{1}'.</source>
<target state="new">The exception '{0}' does not have a field named '{1}'.</target>
<note />
</trans-unit>
<trans-unit id="tcConstructorDoesNotHaveFieldWithGivenName">
<source>The constructor does not have a field named '{0}'.</source>
<target state="new">The constructor does not have a field named '{0}'.</target>
<note />
</trans-unit>
<trans-unit id="tcActivePatternsDoNotHaveFields">
<source>Active patterns do not have fields. This syntax is invalid.</source>
<target state="new">Active patterns do not have fields. This syntax is invalid.</target>
<note />
</trans-unit>
</body>
</file>
</xliff>
19 changes: 17 additions & 2 deletions src/fsharp/xlf/FSComp.txt.ja.xlf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6328,8 +6328,8 @@
<note />
</trans-unit>
<trans-unit id="tcUnionCaseConstructorDoesNotHaveFieldWithGivenName">
<source>Union case/exception '{0}' does not have field named '{1}'.</source>
<target state="translated">共用体ケース/例外 '{0}' には、'{1}' という名前のフィールドがありません。</target>
<source>The union case '{0}' does not have a field named '{1}'.</source>
<target state="needs-review-translation">共用体ケース/例外 '{0}' には、'{1}' という名前のフィールドがありません。</target>
<note />
</trans-unit>
<trans-unit id="tcUnionCaseFieldCannotBeUsedMoreThanOnce">
Expand Down Expand Up @@ -7067,6 +7067,21 @@
<target state="new">This type does not inherit Attribute, it will not work correctly with other .NET languages.</target>
<note />
</trans-unit>
<trans-unit id="tcExceptionConstructorDoesNotHaveFieldWithGivenName">
<source>The exception '{0}' does not have a field named '{1}'.</source>
<target state="new">The exception '{0}' does not have a field named '{1}'.</target>
<note />
</trans-unit>
<trans-unit id="tcConstructorDoesNotHaveFieldWithGivenName">
<source>The constructor does not have a field named '{0}'.</source>
<target state="new">The constructor does not have a field named '{0}'.</target>
<note />
</trans-unit>
<trans-unit id="tcActivePatternsDoNotHaveFields">
<source>Active patterns do not have fields. This syntax is invalid.</source>
<target state="new">Active patterns do not have fields. This syntax is invalid.</target>
<note />
</trans-unit>
</body>
</file>
</xliff>
Loading