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

FS0971: copyOfStruct not defined #8069

Closed
giuliohome opened this issue Jan 1, 2020 · 16 comments
Closed

FS0971: copyOfStruct not defined #8069

giuliohome opened this issue Jan 1, 2020 · 16 comments
Labels
Bug Impact-Low (Internal MS Team use only) Describes an issue with limited impact on existing code.

Comments

@giuliohome
Copy link

giuliohome commented Jan 1, 2020

Please rewrite it with your words if you need to keep it open. Thank you.

@giuliohome giuliohome reopened this Jan 1, 2020
giuliohome referenced this issue in giuliohome/AdventOfCode Jan 1, 2020
giuliohome referenced this issue in giuliohome/AdventOfCode Jan 2, 2020
@abelbraaksma
Copy link
Contributor

abelbraaksma commented Jan 3, 2020

(way1,way2) <- ([|1|],[|2|])

That line creates a tuple of two mutable values, and then you use the <- operator to set the value of the tuple, but the tuple itself is not mutable, only its values are.

Just saying: I understand that the compiler complains, but:

  • I'd expect it to do so in the editor
  • I'd expect an error that is more easily understandable (though it does have ref in it, which it cannot find, this is not very clear).

I'd assume this is a bug, but someone from the MS's F# team will have to have their say in it ;).

Btw, you seem to want to set two values at once, why not just do:

let mutable a, b = ([|1|],[|2|])

This will set a to [|1|] and b to hold [|2|]. Isn't that your original intend?

If you want to create a mutable value from the return of a function (i.e., hold a reference to the original computed value), instead of using mutable, your go-to technique would be reference cells, something like:

let f x =
    let a = ref (2 * x)
    let b = ref (3 * x)
    b, a

let (x, y) = f 12    // x and y are now mutable reference cells and will color yellow in VS

Editing Tools Visual Studio 2017

VS 2019 has been out for a while and a lot has improved in the F# area, perhaps time to upgrade? ;)

@abelbraaksma
Copy link
Contributor

Forgot to add, arrays are mutable by default in F#, you an use the reverse arrow syntax to set an item at an index inside the array. The variables in the let binding merely hold a reference to the array, making them mutable usually doesn't have much benefit:

let x = [|3;4:5|]
x.[2] <- 9    // 5 changed to 9

@cartermp
Copy link
Contributor

cartermp commented Jan 4, 2020

Labeling as a bug for the error message, which should be better.

This pattern is not supported since <- is assigning to the tuple, not assigning individual values; but since there is no declared tuple to re-assign in the first place you get an error. The error message should reflect that.

giuliohome added a commit to giuliohome/AdventOfCode that referenced this issue Jan 4, 2020
@abelbraaksma
Copy link
Contributor

abelbraaksma commented Jan 4, 2020

Enough of it, it's closed from my side

Sorry you feel that way. Your search for a proper algorithm that is tail recursive is a good one, and anything can be made tail recursive, or imperative, with F#.

The easiest you can do to avoid copying, is to use byref/outref/inref, but this is only available in VS 2019, which you aren't using yet. If you're stuck with VS 2017, there are still many other ways to get to the function calling paradigm you want (one such way is perhaps with an [<Out>] parameter).

I'll have a look at your function you showed above by tomorrow (it's late now).

On the feature suggestion: that may be a good thing, but a function always returns only one value (which can be a tuple, which is two values wrapped in a type), which is why it isn't as easy as it seems from a technical standpoint. Hence my suggestion to use inref/outref/byref to overcome that issue the easy way.

My goal is avoiding to copy an array of many values for many times

Disclaimer: The example code you showed is without arrays and without a lot of copying and only uses reference types, perhaps with the exception of List.rev. This was from a cursery read, but I'm well aware that I may be missing, or fail to understand the sweet spot you're trying to solve.

@cartermp
Copy link
Contributor

cartermp commented Jan 4, 2020

@giuliohome I'm re-opening this so that we have a open bug tracking the error message, which is not correct. Please feel free to unsubscribe from the thread if you'd prefer not to get notifications.

@cartermp cartermp reopened this Jan 4, 2020
@abelbraaksma
Copy link
Contributor

So it is a completely completely satisfying solution

Glad to hear that! Note that this syntax doesn't create a mutable tuple per se, but two individual values that are mutable. The assignment operator, contrary to the original syntax, recognizes that it should decompose the tuple from the r-side. You can see that when you use reflector to inspect the generated code.

I'm assuming then that you won't need further help with your algorithm?

It is strange that stackoverflow doesn't mention this approach: it should be added as a solution.

Indeed ;)

BTW, this report is best left open so we can address the confusing error message.

@abelbraaksma
Copy link
Contributor

abelbraaksma commented Jan 5, 2020

so I won't change my modified dequeue algorithm with byref because I want a single instruction.

You don't need assignment anymore when you use outref (or byref, but it seems that outref is jouw suitable here). So you would still have a single instruction, and the actual assignment would happen inside your function.

And you're right, you cannot use let with a record field. However, you can create a record field that is a mutable tuple, which would solve your original issue (but then we're back at assignment of the tuple, not the individual fields).

For the algorithm, we'd need to choose: either it returns a single tuple, with the content set in the function, which can be assigned to either a mutable tuple, or auto-deconstructed into two mutable values. Or you choose that the function must update the values that are referenced, in which case these should be given as outref or byref arguments (proper usage requires VS 2019, though).

The magic deconstruction can also be used if the 2nd param is outref, and you use CLI style parameter passing (meaning, no curried parameters). This magic can be seen in practice with the Int32.TryParse, where the out param can appear on the lh-side of the assignment operator. It's a little trick that may be just what you need, though I'm unsure it'll work the same with multiple out params.

@abelbraaksma
Copy link
Contributor

abelbraaksma commented Jan 5, 2020

I'm using VS2017 and byref as shown above without any issue...

Yes, I know. But your original issue was (if I understood it correctly) to return values that are references. Returning byrefs (and having ByRefLike structs, and a whole lot more) was not supported prior to F# 4.5. Besides, many stability enhancements have appeared in the past 2.5 years, value tuples and value records have been introduced, and anonymous records and much more, so it is always a good idea, esp. when you are doing things "at the edge of the language abilities", to upgrade to the latest version. And not unimportant, the compiler creates increasingly more performant code. And it is a free product anyway, so why not upgrade?

I may be wrong, but I think the notion of managed pointers did not exist prior to F# 4.5, apart from some workarounds you could use in your code for interop by using attributes. Such highly efficient memory-manipulating code is now possible directly from F# without resorting to C# and the proper IL is now emitted.

Part of the discussion on byref etc for 4.5 is here: fsharp/fslang-design#287. This is the relevant RFC-1053. A lot of details, including an example where a function returns a managed pointer to an items inside an array (a technique which may be useful to you as well) is explained in the docs: https://docs.microsoft.com/en-us/dotnet/fsharp/language-reference/byrefs.

@abelbraaksma
Copy link
Contributor

Edit, I just read:

F# 4.1 added return byrefs.

So, while F# 4.5 got a lot, some of it already existed before. I have to admit, I don't know exactly what went in when, but you can always check the changes log, or try your code with a recent version to see if you have benefits by upgrading (you can install side-by-side, meaning you'll never lose your old VS 2017 and can always go back).

@abelbraaksma
Copy link
Contributor

It is quite different from

Indeed it is, one shadows the existing variables, the other doesn't. The variable to start the loop with should already be mutable and not be redeclared.

I wholeheartedly agree to the language suggestion, it seems to be a natural thing to support.

@abelbraaksma
Copy link
Contributor

This has become a rather strange thread now that @giuliohome removed all his comments. It was a valuable discussion, but that's now gone.

I believe the original issue is still a bug, and if I remember correctly, we should do something about it. But without the original discussion, it's a bit hard to reconstruct it.

@cartermp, I haven't seen this happening on other threads before (people deleting their own valuable insights), should we close this issue as it appears the original poster has lost interest in pursuing it, or should we re-create it with the knowledge we still have? The reported error message was most certainly a bug...

@Martin521
Copy link
Contributor

Good that the thread exists, because I ran into the same issue and found the answer here.
BTW, it is not only the text of the error message, but also he fact that the compiler service does not bring it up in the IDE (I am using VS Code 1.42.1, Ionide 4.6.0, F#4.5).
The error message appears only during compile.

@cartermp
Copy link
Contributor

@Martin521 Note that your environmental settings are different here; Visual Studio is up to date but it sounds like your Ionide environment is not.

@dsyme
Copy link
Contributor

dsyme commented Aug 31, 2020

@abelbraaksma @cartermp @Martin521 Do you know a repro for this problem?

@dsyme dsyme added needs repro Impact-Low (Internal MS Team use only) Describes an issue with limited impact on existing code. labels Aug 31, 2020
@abelbraaksma
Copy link
Contributor

abelbraaksma commented Aug 31, 2020

@dsyme, the problem here is that @giuliohome removed his original issue (still there in the history) and a lot of a very valuable discussion about assignment and deconstruction with tuples containing mutables. The removed posts makes this thread look rather odd and one-sided now. IIRC, there was discussion about his example, because it was possibly invalid code that just didn't show a compile error. I could be off, it's been half a year since.

I also believe there was some discussion about upgrading VS 2017, which he (or she) wasn't inclined to do. But again, I may not remember it correctly, as a lot of the discussion is lost.

The problem was deconstruction while using the assignment operator, which isn't supported and doesn't show an error until you compile:

(x, y) <- ([|1|],[|2|])    // where x and y are mutable

The error: error FS0971: Undefined value 'copyOfStruct : (int [] * int []) ref'

The original post was this, the error still reproduces in VS 2019, but is shown when compiling, not in the editor:


Repro steps

A simple Console pgm

[<EntryPoint>]
let main argv = 

    let mutable way1 = [|0|]
    let mutable way2 = [|0|]
    (way1,way2) <- ([|1|],[|2|])
    printfn "%A" argv
    0 // return an integer exit code

Expected behavior

It should be equivalent of

[<EntryPoint>]
let main argv = 

    let mutable way1 = [|0|]
    let mutable way2 = [|0|]
    let (_way1,_way2) = ([|1|],[|2|])
    way1 <- _way1
    way2 <- _way2
    printfn "%A" argv
    0 // return an integer exit code

Actual behavior

I'm getting a compile error:

errore FS0971: Valore non definito 'copyOfStruct : (int [] * int []) ref'

Known workarounds

My second equivalent version above.

Related information

Operating system: Windows 64 bits
.NET Runtime NET Framework 4.7
Editing Tools Visual Studio 2017

@dsyme
Copy link
Contributor

dsyme commented Sep 1, 2020

Cool, thanks for the info. Duplicate of #8899 I believe.

@dsyme dsyme closed this as completed Sep 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Impact-Low (Internal MS Team use only) Describes an issue with limited impact on existing code.
Projects
None yet
Development

No branches or pull requests

5 participants