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

[New module system] [Remote API] includes field of CryptolConnection.file_deps instead returns foreign dependencies #1496

Closed
WeeknightMVP opened this issue Feb 7, 2023 · 2 comments

Comments

@WeeknightMVP
Copy link

cryptol$ git status
git status
On branch functors-merge
Your branch is up to date with 'origin/functors-merge'.

nothing to commit, working tree clean
cryptol$ ./cry build cryptol-remote-api
...
cryptol$ export CRYPTOL_SERVER=$(cabal v2-exec which cryptol-remote-api)
cryptol$ cd examples/SuiteB_FFI
cryptol/examples/SuiteB_FFI$ make
cc -O3 -Wall -Wextra -Werror -march=native -fPIC -shared SuiteB_FFI.c -o SuiteB_FFI.so
cryptol/examples/SuiteB_FFI$ cd ../..
cryptol$ export CRYPTOLPATH=$(pwd)/examples/SuiteB_FFI
cryptol$ cd cryptol-remote-api/python
cryptol/cryptol-remote-api/python$ poetry update
...
cryptol/cryptol-remote-api/python$ poetry shell
...
(...) cryptol/cryptol-remote-api/python$ python3
>>> from pprint import pprint
>>> from cryptol import connect
>>> c = connect(reset_server=True)
>>> x = c.file_deps("SuiteB_FFI", False)  # load module
>>> pprint(x.result())
{'fingerprint': 'EAF629FA181D4A780FE070A8129CB555ADAC0902',
 'foreign': ['.../cryptol/examples/SuiteB_FFI/SuiteB_FFI.so'],
 'imports': ['Cryptol', 'SuiteB'],
 'includes': ['.../cryptol/examples/SuiteB_FFI/SuiteB_FFI.so'],
 'source': '.../cryptol/examples/SuiteB_FFI/SuiteB_FFI.cry'}

I think this stems from Line 69 of src/CryptolServer/FileDeps.hs, which calls fiForeignDeps to fill the includes field:

instance ToJSON FileDeps where
  toJSON fd =
    JSON.object
      [ "source"      .= case fdSource fd of
                           InFile f  -> toJSON f
                           InMem l _ -> JSON.object [ "internal" .= l ]
      , "fingerprint" .= fingerprintHexString (fiFingerprint fi)
      , "includes"    .= Set.toList (fiForeignDeps fi)  -- should be `fiIncludeDeps`?
      , "imports"     .= map (show . pp) (Set.toList (fiImportDeps fi))
      , "foreign"     .= Set.toList (fiForeignDeps fi)
      ]
    where
    fi = fdInfo fd
yav added a commit that referenced this issue Feb 9, 2023
To help with 1 look ahead parsing, type declarations are parsed as types,
which are then post processed into actual declarations.   This
helps with #1496, and avoids the ambiguity that previously required
parens in `type constraint (C)`.
@RyanGlScott
Copy link
Contributor

Yes, I think this is a simple oversight. Do you agree, @yav?

yav added a commit that referenced this issue Feb 10, 2023
@yav
Copy link
Member

yav commented Feb 10, 2023

Should be fixed now

@yav yav closed this as completed Feb 10, 2023
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

3 participants