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

Dead Types wrt consumed types #169

Open
AlexMoutonNoble opened this issue May 25, 2022 · 7 comments
Open

Dead Types wrt consumed types #169

AlexMoutonNoble opened this issue May 25, 2022 · 7 comments

Comments

@AlexMoutonNoble
Copy link

AlexMoutonNoble commented May 25, 2022

Hi again Cristiano.
Just added dce to our CI process and going through the output i get:

Warning Dead Type
  File "/home/circleci/noble/web/src/requests.res", line 200, characters 2-17
  argsClone.with_data is a record label never used to read a value
  <-- line 200
    @dead("argsClone.with_data") with_data: bool,

argsClone is consumed by an external Axios.post, and this type is a preposition for our clone action, so these warnings seem like false positive to me. Its not wrong, granted, but suggests I can remove that field which is wrong.

Are you doing any work in this direction?

Thanks!
Alex

@AlexMoutonNoble AlexMoutonNoble changed the title Dead Types wrt contraviarance Dead Types wrt consumed types May 25, 2022
@cristianoc
Copy link
Collaborator

cristianoc commented May 25, 2022

How does the Axis post get access to it? Is argsClone exported to JS, and if so is genType used?

@cristianoc
Copy link
Collaborator

cristianoc commented May 25, 2022

added dce to our CI process

Great!

@AlexMoutonNoble
Copy link
Author

AlexMoutonNoble commented May 25, 2022

@send
external postImpl: (
  axios,
  string,
  'data,
  config,
) => Js.Promise.t<response<'datasend, 'datarecv, 'headersrecv, 'request>> = "post"

let post = (url, data, config) => axios->postImpl(url, data, config)
type argsClone<'id> = {
  name: string,
  description: string,
  with_data: bool,
  clone_id: 'id,
}

type clone = {id: int}

let clone = (data: argsClone<int>): Js.Promise.t<
  Axios.response<'datasend, cloneCapability, 'headers, 'request>,
> => {
  Axios.post(`/api/capability/`, data, configJson)
}

@cristianoc
Copy link
Collaborator

cristianoc commented May 25, 2022

Currently there's no magic that tries to understand flow of values to externals.
The only magic is values annotated @genType are automatically treated as not dead. But there's no special provision at the moment for types used in externals.

In these cases, it's best to annotate the type argsClone live manually: @live type argsClone....

@cristianoc
Copy link
Collaborator

E.g.

@live("sent to Axios")
type argsClone<'id> = {
  name: string,
  description: string,
  with_data: bool,
  clone_id: 'id,
}

@cristianoc
Copy link
Collaborator

The other thing is just because a value is sent to an external, that does not mean that the external function. is going to read all its fields, so any automatic analysis would be pretty coarse.

@AlexMoutonNoble
Copy link
Author

AlexMoutonNoble commented May 26, 2022

That seems pretty reasonable to me. False positives and aggressive annotations are a burden

adding just for context that -dce produces 8500 lines of output for our codebase, so basically too much for our small team

(anyways thanks again!)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants