The notion of dead record label is not clear. about reanalyze

[cristianoc]

@jchavarri see example in #63.
The results on this example seem correct as the function userData takes as parameter a tabState.

So this example might not be an accurate repro.

[jchavarri]

@cristianoc Thanks for looking into this.

userData.b is a record label never used to read a value

Maybe i'm not fully understanding the meaning of what means to be "dead". Why there is a distinction between "read" a value and "write" a value? In the example, the userData record is used to create a value of that type, so it is "used" (regardless if it is for read or write) so I would not consider it dead as long as the function userData is used somewhere. Could you help me understand, so I can provide a better repro?

[cristianoc]

@jchavarri clearly the records check is not explained well.

With this definition:

type r = {x:int, y:int};

This keeps the label alive:

let bar = r => r.x

This does not keep the label alive:

let bar = (n) => {x:n, y:n}

If it's created and never read, then you can just delete the label as the program does not depend on it at all. (And you'd delete it from the creation too).

So you'd get this program, which is indistinguishable from the original one:

type r = {x:int};

let foo = r => r.x

let bar = (n) => {x:n}

[jchavarri]

If it's created and never read, then you can just delete the label as the program does not depend on it at all.

Interesting. I wonder if this assumption is not too strict? I mean, there are cases where the label is not explicitly read but it can be used.

An example, which actually led to opening this issue. I took a subset of tabState record and put it into userData. Then I proceed to compare deeply some values from userData.

The full code would look like:

type userData = {
  a: bool,
  b: int,
  c: string,
};

type tabState = {
  a: bool,
  b: int,
  c: string,
  d: int,
  e: int,
  f: string,
};

let userData =
    ({a, b, c}): userData => {
  a,
  b,
  c,
};

let one = {
  a: true,
  b: 2,
  c: "foo",
  d: 1,
  e: 3,
  f: "bar",
}
let two = {
  a: true,
  b: 1,
  c: "foo",
  d: 1,
  e: 3,
  f: "baz",
}
let changed = userData(one) != userData(two);

In that example, I'm not explicitly reading the labels, but I'm definitely making use of the record type userData, so I would not say its labels are dead.

Does that make sense?

[jchavarri]

Ooh I guess I understand. What the analysis is telling you is that if nothing is reading from that label, you could use a tuple or any other structure that doesn't have named access to fields? 🤔

[cristianoc]

That code is redundant.

[cristianoc]

And you might want it like that, because it's not finished or something.
The analysis indicates you can remove the label. But it's up to you whether you want to.

[jchavarri]

Where's the redundancy? I want to deeply compare just a subset of the fields of the original record, not all the fields.

[cristianoc]

As an analogy, it's not any different than this program:

let x = 35;
Js.log("hello");

You're "writing" to x but never "reading" from it.

[cristianoc]

@jchavarri remember we're talking about a global program analysis.
This is your entire program.
If that's your entire program, then the labels marked as unused are redundant.

[jchavarri]

I don't see how it's the same example. I am reading one and two values of type userData in the example above, while x is not being read in your example.

I guess what is confusing me is that I would assume reanalyze should include a transitive dependency that marks all record fields as alive if at least one place is using a value of that record type.

[cristianoc]

Use == "read". Not "create". That's how the analysis is defined.

[cristianoc]

The analysis would not be very useful otherwise. Most redundant fields are in fact created. Just not read.

[cristianoc]

If this was data coming from the network, the term one would use is "overfetching".

[jchavarri]

I see. I understand what you mean now.

Let me ask for a counter example. If I do include A in some module B, would the analysis mark all the bindings in A as live?

[cristianoc]

In theory include is treated as copy-paste, as that's its semantics.
But sometimes it's kind of tricky, so the analysis might get it wrong.
We do have examples where it gets it right.

[jchavarri]

Gotcha. I think I see the point where what I'm suggesting (mark record labels as alive if a value of that record type is used/read) would actually remove information from the analysis, which would make the tool less useful. The downside is that the analysis might have more "noise" as these "implicit dependencies" are not considered and the algorithm is very exhaustive.

I think this issue can be closed. Maybe there could be a section "Liveliness" in the readme that includes specifics about the different data types, but I see now how the case we discussed is just a specific application of the more general design, so adding specific cases might be more confusing.

[cristianoc]

Great idea re a section explaining more.
I suspect we'll have another issue with exactly the same title. But this one is not it.