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

Parser should not dynamically inject shared builtin program components #511

Closed
robin-aws opened this issue Jan 17, 2020 · 6 comments · Fixed by #4658
Closed

Parser should not dynamically inject shared builtin program components #511

robin-aws opened this issue Jan 17, 2020 · 6 comments · Fixed by #4658
Assignees
Labels
area: build-system Support for dependencies in Dafny, generation of target language build files kind: enhancement Enhancements, feature requests, etc. These are NOT bugs, but ways we can improve Dafny part: parser First phase of Dafny's pipeline part: runtime Happens in Dafny's runtime (Add a `lang` tag if relevant)

Comments

@robin-aws
Copy link
Member

The advantage of dynamically generating elements such as tuple, array, and arrow types is that there's no maximum arity of such generic types. However, this interferes with being able to compile packages separately, since separate packages may decide to generate the same elements, leading to conflicts when loading or linking.

The easiest fix would be to pre-generate these types up to a maximum arity. Scala currently does this (with a cap of 22) but it looks like Scala 3 will no longer have this limit: scala/scala3#1758. It may be possible to have our cake and eat it too if we borrow some of those techniques.

@robin-aws
Copy link
Member Author

Thinking out loud: to have a robust migration path for this and other future changes, we will likely want to depend on versioning the various target language runtimes. We need to ensure that if Dafny is used to build a DLL (for e.g.) after this change, it will declare a dependency on a version of the runtime that will package the builtins the compiler is now assuming are there already.

@robin-aws
Copy link
Member Author

Note that if you use the dafny.msbuild package to create your DLLs (https://github.com/dafny-lang/dafny.msbuild) then you'll be forced to provide an explicit dependency on DafnyRuntime in your csproj file, so the versioning is under user control. This isn't quite adequate, as a Dafny codebase could get away with too old of a runtime until the code happens to change in a way that fails because of missing builtins. There needs to be tooling support to ensure Dafny can enforce the minimum version of the runtime it is compiling for.

@robin-aws robin-aws added the kind: enhancement Enhancements, feature requests, etc. These are NOT bugs, but ways we can improve Dafny label Jun 23, 2020
@acioc acioc added the parsing label Aug 20, 2020
@acioc acioc added this to the Post 3.0 milestone Aug 20, 2020
@keyboardDrummer keyboardDrummer added part: parser First phase of Dafny's pipeline and removed parsing labels Jul 19, 2021
@robin-aws robin-aws removed this from the Post 3.1 milestone Sep 22, 2021
@robin-aws
Copy link
Member Author

I've realized that this is less of a blocking issue for separate compilation than I initially believed, only because many languages allows multiple packages to define the same symbols. For example, if two different jars on your Java classpath both define _System.nat, then it will be loaded from whichever appears earlier on the classpath.

It's still bad practice to allow this state in an application though, since it can lead to very subtle errors when any such ordered lookup behavior is altered (e.g. accidentally reordering the classpath) and the two versions are not actually the identical. Therefore this is still important to fix in the long term, so that packages compiled from Dafny only contain symbols corresponding to the actual Dafny source being compiled.

@cpitclaudel
Copy link
Member

Seeing this issue due to your recent comment: another place where I've run into this is reusing the parser (for the REPL), because it forces you to be very careful about the builtins module (but we don't have a way to clone it at the moment)

@robin-aws
Copy link
Member Author

Just to clarify further (as I was briefly confused myself), _System.nat is particularly harmless because that's only a TypeDescriptor for the nat type, and compiled code will generally not use a dedicated "nat" type. That's why the existing Test/comp/separate-compilation/usesTimesTwo.dfy test case doesn't hit any issues despite _System.nat appearing in both projects.

In C# at least, type descriptors are only used in contexts where a default value for a type is needed. If I add this to the usesTimesTwo.dfy source:

method PickANat() returns (n: nat) {
  n := PickSomething();
}

method PickSomething<T(0)>() returns (t: T) {
}

Then the test commands produce this C# code:

  public static BigInteger PickANat()
  {
    BigInteger n = BigInteger.Zero;
    BigInteger _out0;
    _out0 = LibraryModule_Compile.__default.PickSomething<BigInteger>(_System.nat._TypeDescriptor());
    n = _out0;
    return n;
  }
  public static __T PickSomething<__T>(Dafny.TypeDescriptor<__T> _td___T)
  {
    __T t = _td___T.Default();
    return t;
  }

And you see this warning (I've added line breaks):

/Users/salkeldr/Documents/GitHub/dafny/Source/IntegrationTests/bin/Debug/net6.0/TestFiles/LitTests/LitTest/comp/separate-compilation/consumer/usesTimesTwo.cs(105,74): warning CS0436: 
The type 'nat' in '/Users/salkeldr/Documents/GitHub/dafny/Source/IntegrationTests/bin/Debug/net6.0/TestFiles/LitTests/LitTest/comp/separate-compilation/consumer/usesTimesTwo.cs' conflicts with the imported type 'nat' in 'TimesTwo, Version=3.9.1.41027, Culture=neutral, PublicKeyToken=null'. 
Using the type defined in '/Users/salkeldr/Documents/GitHub/dafny/Source/IntegrationTests/bin/Debug/net6.0/TestFiles/LitTests/LitTest/comp/separate-compilation/consumer/usesTimesTwo.cs'. 
[/Users/salkeldr/Documents/GitHub/dafny/Source/IntegrationTests/bin/Debug/net6.0/TestFiles/LitTests/LitTest/comp/separate-compilation/consumer/Consumer.csproj]

Even then it's only a warning and the code still works.

@keyboardDrummer
Copy link
Member

Note that the tuple part of this issue was resolved with #2284

@robin-aws robin-aws removed their assignment Jul 25, 2023
@alex-chew alex-chew added part: runtime Happens in Dafny's runtime (Add a `lang` tag if relevant) area: build-system Support for dependencies in Dafny, generation of target language build files labels Oct 9, 2023
@robin-aws robin-aws self-assigned this Oct 11, 2023
@robin-aws robin-aws added this to the Dafny Standard Libraries milestone Oct 11, 2023
robin-aws added a commit that referenced this issue Nov 2, 2023
### Description
Fixes #511

Generalizes the approach used in #2284 to pre-generate all built-in
declarations into the various runtimes:

* Adds an internal `--system-module` option to control how the system
module is treated when translating:
* Include (the current default) - emits code for the contents of the
System module
  * Omit (the new default) - emits no code for the System module
  * Populate - emits ONLY the code for the System module
* The third option is used to generate part of each runtime from a
`systemModulePopulator.dfy` file, previously named
`tuplesForDafnyRuntime.dfy`. The output is now present in each runtime
in a separate file/source location, and checked for consistency in CI
just as other Dafny-generated committed source is.
* Also generalized how the runtime files are embedded in `DafnyPipeline`
to support more than one file better (as Go already was).

### How has this been tested?
Mostly existing tests, but also added an internal version of
`--include-runtime` for `dafny run`, so that `%testDafnyForEachCompiler`
can include testing on C# when using the separate `DafnyRuntime.dll`
library, as that behaves significantly different in this case.

<small>By submitting this pull request, I confirm that my contribution
is made under the terms of the [MIT
license](https://github.com/dafny-lang/dafny/blob/master/LICENSE.txt).</small>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: build-system Support for dependencies in Dafny, generation of target language build files kind: enhancement Enhancements, feature requests, etc. These are NOT bugs, but ways we can improve Dafny part: parser First phase of Dafny's pipeline part: runtime Happens in Dafny's runtime (Add a `lang` tag if relevant)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants