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

Implement unbounded ints/rationals in the Rust backend #4438

Merged
merged 2 commits into from
Aug 22, 2023

Conversation

shadaj
Copy link
Collaborator

@shadaj shadaj commented Aug 17, 2023

Implement unbounded ints/rationals in the Rust backend

By submitting this pull request, I confirm that my contribution is made under the terms of the MIT
license
.


Stack created with Sapling. Best reviewed with ReviewStack.

@shadaj shadaj requested a review from jtristan August 17, 2023 20:17
@shadaj shadaj force-pushed the pr4438 branch 2 times, most recently from 5df2dda to 66f19a6 Compare August 18, 2023 16:25
jtristan pushed a commit that referenced this pull request Aug 18, 2023
Support subset types in the Rust backend








<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>

---
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/dafny-lang/dafny/pull/4422).
* #4438
* #4429
* __->__ #4422
@shadaj shadaj force-pushed the pr4438 branch 3 times, most recently from 78a7da5 to c408443 Compare August 18, 2023 19:15
Copy link
Member

@keyboardDrummer keyboardDrummer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One question that I have, which doesn't necessarily relate to this PR, but to the approach taken in Compiler-dafny.cs, is whether you've considered not just using ConcreteSyntaxTree instead of string, but to immediately generate the desired AST nodes.

I made a PR with a prototype of a SinglePassCompiler that doesn't use ConcreteSyntaxTree or string here: #3396

The idea is that SinglePassCompiler is made generic:

public abstract class GenericSinglePassCompiler<TExpression, TStatements, TStatement, TType, etc...>

Existing backends that want to use ConcreteSyntaxTree/ICanRender will inherit from a new type ConcreteSinglePassCompiler:

public abstract class ConcreteSinglePassCompiler 
  : GenericSinglePassCompiler<ICanRender, ConcreteSyntaxTree, ICanRender, ICanRender>, Plugins.Compiler

I think it does require more refactoring of SinglePassCompiler then you're currently doing though, although I haven't taken the time yet to understand the BuilderSyntaxTree<T> technique that you're currently using. For the GenericSinglePassCompiler approach, many methods have to be refactored so that the argument that is currently ConcreteSyntaxTree becomes a result value instead, so these functions can return AST nodes.

Maybe such as refactoring is not the best investment of your time, but I think it would be awesome to have what's currently SinglePassCompiler in a state where it doesn't relate to string or ConcreteSyntaxTree.

jtristan pushed a commit that referenced this pull request Aug 18, 2023
Support class fields in the Rust backend



Right now, we still use reference counted pointers, which means that
cycles will result in memory leaks. I plan to address this with cycle
collection in a later PR.

<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>

---
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/dafny-lang/dafny/pull/4429).
* #4438
* __->__ #4429
@shadaj
Copy link
Collaborator Author

shadaj commented Aug 18, 2023

@keyboardDrummer yeah, making SinglePassCompiler generic wrt ConcreteSyntaxTree would be quite nice. Even for our approach, there are situations where the compiler internally creates new ConcreteSyntaxTrees that cause trouble, so many of the refactoring steps have been to eliminate that. Would also be nice to eliminate assumptions about parenthesizing expressions, or how function calls are generated (right now the compiler unnecessarily assumes an order of receiver, then name, then args).

It's effectively what the Dafny-implemented backends look like, since there we have similar methods to generate statements/expressions/etc but the implementation is free to generate strings or its own representations of the target syntax.

@shadaj shadaj marked this pull request as ready for review August 18, 2023 20:45
@shadaj shadaj force-pushed the pr4438 branch 3 times, most recently from 5504542 to d0305e6 Compare August 20, 2023 14:12
<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>
<DataCollectors>
<DataCollector friendlyName="XPlat Code Coverage">
<Configuration>
<Exclude>[*]DAST.*,[*]DCOMP.*</Exclude>
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because of the code blowup in the Dafny-to-C# compilation, instrumentation hangs and causes the CI to timeout. Since we ignore coverage of generated code anyways, this is safe.

@jtristan jtristan enabled auto-merge (squash) August 22, 2023 14:28
@jtristan jtristan merged commit bc898a9 into dafny-lang:master Aug 22, 2023
keyboardDrummer pushed a commit to keyboardDrummer/dafny that referenced this pull request Sep 15, 2023
Support subset types in the Rust backend








<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>

---
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/dafny-lang/dafny/pull/4422).
* dafny-lang#4438
* dafny-lang#4429
* __->__ dafny-lang#4422
keyboardDrummer pushed a commit to keyboardDrummer/dafny that referenced this pull request Sep 15, 2023
Support class fields in the Rust backend



Right now, we still use reference counted pointers, which means that
cycles will result in memory leaks. I plan to address this with cycle
collection in a later PR.

<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>

---
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/dafny-lang/dafny/pull/4429).
* dafny-lang#4438
* __->__ dafny-lang#4429
keyboardDrummer pushed a commit to keyboardDrummer/dafny that referenced this pull request Sep 15, 2023
Implement unbounded ints/rationals in the Rust backend









<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>

---
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/dafny-lang/dafny/pull/4438).
* dafny-lang#4450
* __->__ dafny-lang#4438
keyboardDrummer pushed a commit that referenced this pull request Sep 19, 2023
Support subset types in the Rust backend








<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>

---
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/dafny-lang/dafny/pull/4422).
* #4438
* #4429
* __->__ #4422
keyboardDrummer pushed a commit that referenced this pull request Sep 19, 2023
Support class fields in the Rust backend



Right now, we still use reference counted pointers, which means that
cycles will result in memory leaks. I plan to address this with cycle
collection in a later PR.

<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>

---
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/dafny-lang/dafny/pull/4429).
* #4438
* __->__ #4429
keyboardDrummer pushed a commit that referenced this pull request Sep 19, 2023
Implement unbounded ints/rationals in the Rust backend









<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>

---
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/dafny-lang/dafny/pull/4438).
* #4450
* __->__ #4438
robin-aws added a commit that referenced this pull request Sep 20, 2023
#4568)

Addresses xunit/xunit#2549 to reduce our
integration test shard execution time from > 1 hour to <= 30 mins.

The TL;DR is that xUnit has an extension mechanism that attempts to load
every class in every available DLL to see if it extends a given
interface. This manifests in a huge delay on startup before any tests
begin to run, e.g. 52 mins in this case:
https://github.com/dafny-lang/dafny/actions/runs/6241813664/job/16944685069#step:22:146
(make sure you turn on Show Timestamps!) This was steadily getting worse
as we added more classes, especially
#4438 as it added a bunch of
Dafny-generated code.

Fortunately xUnit 2.5.1 added a knob to switch off this discovery, which
we're definitely not using.

<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>
keyboardDrummer pushed a commit that referenced this pull request Sep 26, 2023
#4568)

Addresses xunit/xunit#2549 to reduce our
integration test shard execution time from > 1 hour to <= 30 mins.

The TL;DR is that xUnit has an extension mechanism that attempts to load
every class in every available DLL to see if it extends a given
interface. This manifests in a huge delay on startup before any tests
begin to run, e.g. 52 mins in this case:
https://github.com/dafny-lang/dafny/actions/runs/6241813664/job/16944685069#step:22:146
(make sure you turn on Show Timestamps!) This was steadily getting worse
as we added more classes, especially
#4438 as it added a bunch of
Dafny-generated code.

Fortunately xUnit 2.5.1 added a knob to switch off this discovery, which
we're definitely not using.

<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>
@shadaj shadaj deleted the pr4438 branch January 17, 2025 02:13
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.

3 participants