Change Fwd of Df to Maybe instead of Data

[t-wallet]

Also fixed some linter warnings. HLS was very useful for this job, but it was not configured in the nix shell. I'd like to hear your opinions on adding HLS to shell.nix? We used this in the clash-ethernet repo and it worked pretty well with vscode.

[rowanG077]

Don't forget to update the readme.

[martijnbastiaan]

HLS was very useful for this job, but it was not configured in the nix shell. I'd like to hear your opinions on adding HLS to shell.nix?

Go for it. I never got it working within Nix though, do you need to start vscode in a specific way?

[t-wallet]

HLS was very useful for this job, but it was not configured in the nix shell. I'd like to hear your opinions on adding HLS to shell.nix?

Go for it. I never got it working within Nix though, do you need to start vscode in a specific way?

Yes, you need to start vscode from the terminal while in nix-shell (and have the vscode hls extension of course).

[martijnbastiaan]

Could you check whether this breaks underscore notation in the circuit plugin?

[t-wallet]

Could you check whether this breaks underscore notation in the circuit plugin?

Tests.Protocols.Plugin still compiles, so I don't think it breaks

[martijnbastiaan]

Ah of course, that's because you did Data -> Maybe, but not Ack -> Bool. Makes sense, sorry for the noise.

[christiaanb]

Do we have some benchmarks that show there’s no space leaks, or at least no simulation performance degradation from this change?

The issue that is closed by this PR just says that “Maybe is lazy, but that’s not a problem any longer”. So what was the original problem that is no longer there?

I guess a benefit of this change is that we no longer have an unlawful Applicative Data instance.

[martijnbastiaan]

The issue that is closed by this PR just says that “Maybe is lazy, but that’s not a problem any longer”. So what was the original problem that is no longer there?

This was more based on a gut feeling back then, after having experienced many space leaks at a client. The idea was that it doesn't make much sense to have an X-value when you have a Data, hence making the seqX in Signal's fmap/app actually do something / do more.

[t-wallet]

Do we have some benchmarks that show there’s no space leaks, or at least no simulation performance degradation from this change?

The issue that is closed by this PR just says that “Maybe is lazy, but that’s not a problem any longer”. So what was the original problem that is no longer there?

I guess a benefit of this change is that we no longer have an unlawful Applicative Data instance.

There was no huge difference in the runtime of the Df tests (actually, the strict version ran 5% slower on average). The point of this PR is to make things uniform, right now we use Data for Df and Maybe for PacketStream and Axi4Stream. I'm also open to doing things the other way around, i.e. using the strict version for every protocol.

[martijnbastiaan]

Let's add an issue discussing a similar Ack -> Bool migration.