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

[WIP] Data dependencies propagate orphan instances #11022

Closed
wants to merge 7 commits into from

Conversation

akrmn
Copy link
Contributor

@akrmn akrmn commented Sep 24, 2021

Work so far:

I split this task in three parts:

  1. Encoding/decoding the set of required imports, this code will live at DA.Daml.LFConversion.MetadataEncoding.
    • This was fairly simple given the existing helper functions for encoding/decoding similar information to types.
  2. Picking up the encoded set of imports in DA.Daml.Compiler.DataDependencies and adding it to the hsmodImports field in generateSrcFromLf
    • Also simple, especially given the existing modRefImport function.
  3. Generating the encoded imports in DA.Daml.LFConversion.convertModule
    • This is where it got complicated, in particular getting the instances in scope at the module being processed. So far, I've tried:
      • md_insts field of details :: ModDetails argument: only includes instances defined in the current module.

      • md_exports field of details :: ModDetails argument: this includes functions, datatypes and classes, but not instances.

      • I also tried adding a parameter visibleInstances :: [ClsInst] to convertModule, and providing it as the result of calling hptInstances on the available hsc :: HscEnv. Note that one of the callsites of convertModule, generateRawDalfRule, didn't have an available hsc, so I added a line

        hsc <- hscEnv <$> use_ GhcSession file

        However, on running this the value of visibleInstances turned out to be an empty list. I added some trace lines and noticed that the callstack goes through generateRawDalfRule, so perhaps just adding the hsc line isn't enough. The docstring for generateRawDalfRule,

        -- Generates the DALF for a module without adding serializability information
        -- or type checking it.

        makes me think that this is somewhat intentional, and that generateRawDalfRule shouldn't perform too much work, but I'm not sure how to reconcile that with what we need.

    • I believe you also suggested using the mg_inst_env field of ModGuts, but I haven't been able to get my hands on a ModGuts value.

There's also the question of how to test this. For the encoding/decoding, I followed the approach of the other encoder/decoders in DA.Daml.LFConversion.MetadataEncoding. I'd also like to test that the generated $$imports value has the right type, and I found the @QUERY-LF pragma, but I found it hard to read the json file being queried.

Pull Request Checklist

  • Read and understand the contribution guidelines
  • Include appropriate tests
  • Set a descriptive title and thorough description
  • Add a reference to the issue this PR will solve, if appropriate
  • Include changelog additions in one or more commit message bodies between the CHANGELOG_BEGIN and CHANGELOG_END tags
  • Normal production system change, include purpose of change in description

NOTE: CI is not automatically run on non-members pull-requests for security
reasons. The reviewer will have to comment with /AzurePipelines run to
trigger the build.

@akrmn akrmn force-pushed the data-dependencies-propagate-instances branch from 16861df to 13016b4 Compare September 27, 2021 14:28
@akrmn
Copy link
Contributor Author

akrmn commented Sep 28, 2021

As suggested on slack, I split this into #11036, #11037 and #11038

@akrmn akrmn closed this Sep 28, 2021
@akrmn akrmn deleted the data-dependencies-propagate-instances branch September 29, 2022 12:08
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

Successfully merging this pull request may close these issues.

1 participant