Skip to content

Commit 2b0bf9e

Browse files
committed
feedback changes and suppressing link that is failing CI
1 parent c06ed78 commit 2b0bf9e

File tree

2 files changed

+51
-44
lines changed

2 files changed

+51
-44
lines changed

rfcs/0005-collection-context.md

Lines changed: 27 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,13 @@ With its current design, the Sway compiler faces challenges regarding how nodes
1111
with one another during type checking. These include:
1212

1313
1. Mutually recursive dependencies are not supported, and could not be supported without
14-
extensive special casing.
14+
extensive special casing.
1515
2. Recursive AST nodes are not currently supported, and could not be supported without
16-
extensive special casing.
16+
extensive special casing.
1717
3. This inhibits the introduction of recursive functions, complex application development
18-
requiring circular dependencies, and more.
18+
requiring circular dependencies, and more.
1919
4. If a user forgets to import a dependency, it is not tractable to suggest an import to
20-
them at the compiler level without extensive special-casing.
20+
them at the compiler level without extensive special-casing.
2121

2222
This RFC proposes a two-themed solution to this problem. At a high level, theme one of this
2323
RFC is a change to the internal design of the compiler to disentangle AST transformation
@@ -26,7 +26,7 @@ context" that allows the compiler to introduce steps for graph collection and ty
2626
collection, in addition to type inference and type checking. All together, this allows the
2727
compiler to solve the issues listed above.
2828

29-
[*prototype system*](https://github.com/emilyaherbert/declaration-engine-and-collection-context-demo/tree/master/de)
29+
[_prototype system_](https://github.com/emilyaherbert/declaration-engine-and-collection-context-demo/tree/master/de)
3030

3131
# Motivation
3232

@@ -182,21 +182,21 @@ The [proposed stages of the compiler](https://github.com/emilyaherbert/declarati
182182

183183
1. parsing
184184
2. [AST transformation from the untyped AST to the typeable AST and graph collection](https://github.com/emilyaherbert/declaration-engine-and-collection-context-demo/blob/master/de_cc/src/semantic_analysis/graph_collection/mod.rs)
185-
- insert instances of `TypeInfo` into the `TypeEngine`
186-
- insert declarations into the `DeclarationEngine`
187-
- create the collection context
188-
- NOT evaluate types in any way
189-
- NOT create constraints on types or perform type unification
190-
- NOT resolve instances of custom types
185+
- insert instances of `TypeInfo` into the `TypeEngine`
186+
- insert declarations into the `DeclarationEngine`
187+
- create the collection context
188+
- NOT evaluate types in any way
189+
- NOT create constraints on types or perform type unification
190+
- NOT resolve instances of custom types
191191
3. [type collection](https://github.com/emilyaherbert/declaration-engine-and-collection-context-demo/blob/master/de_cc/src/semantic_analysis/type_collection/mod.rs)
192-
- visiting all intraprocedural definitions (struct/enum/function/trait/etc declarations)
193-
- resolving custom types
194-
- associating type parameters with generics
195-
- NOT visiting non-intraprocedural definitions (function bodies are not visited)
192+
- visiting all intraprocedural definitions (struct/enum/function/trait/etc declarations)
193+
- resolving custom types
194+
- associating type parameters with generics
195+
- NOT visiting non-intraprocedural definitions (function bodies are not visited)
196196
4. [type inference (+ type checking)](https://github.com/emilyaherbert/declaration-engine-and-collection-context-demo/blob/master/de_cc/src/semantic_analysis/type_inference/mod.rs)
197-
- visiting all function bodies and expressions
198-
- resolving custom types
199-
- monomorphizing as needed
197+
- visiting all function bodies and expressions
198+
- resolving custom types
199+
- monomorphizing as needed
200200

201201
## Typeable AST
202202

@@ -389,11 +389,15 @@ these `TypeId`'s resolve in the `TypeEngine` to concrete types.
389389

390390
## Recursive Data Structures
391391

392+
<!-- markdown-link-check-disable -->
393+
392394
From my understanding, it's impossible to do type inference on truly recursive data structures (i.e. those
393395
that would be of infinite size) as you need to
394-
[fulfill these requirements](https://papl.cs.brown.edu/2019/types.html#(part._.Recursive_.Datatype_.Definitions)).
396+
[fulfill these requirements](<https://papl.cs.brown.edu/2019/types.html#(part._.Recursive_.Datatype_.Definitions)>).
395397
For example:
396398

399+
<!-- markdown-link-check-enable -->
400+
397401
```rust
398402
struct Data1 {
399403
other: Data2
@@ -404,9 +408,13 @@ struct Data2 {
404408
}
405409
```
406410

411+
<!-- markdown-link-check-disable -->
412+
407413
In these instances, the compiler performs an [occurs check](https://papl.cs.brown.edu/2019/Type_Inference.html)
408414
to check whether the same type variable occurs on both sides and, if it does, it produces a compile error.
409415

416+
<!-- markdown-link-check-enable -->
417+
410418
I propose that we use an occurs check detect to recursive data structures until we fully support an equivalent
411419
to something like the Rust `Box` type, which would eliminate this problem.
412420

rfcs/0013-changes-lifecyle.md

Lines changed: 24 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ To maximize user experience when updating the compiler, we need a guide on how t
1919
[guide-level-explanation]: #guide-level-explanation
2020

2121
The compiler will follow a "rolling release" scheme, which means that periodically (to be specified) a
22-
new major version will be released.
22+
new version will be released.
2323

2424
This means that as soon as the compiler reaches version "1.0.0", the next **version** will be "1.1.0"; and
2525
after that and if needed, the next **patch** would be "1.1.1".
@@ -66,13 +66,13 @@ Each item will follow the template:
6666
...
6767
```
6868

69-
Inside each section, items will follow the template of a user friendly one line description nad link to the PR.
69+
Inside each section, items will follow the template of a user friendly one line description and a link to the PR.
7070

7171
```
7272
- Index operator using Index trait [#6356](https://github.com/FuelLabs/sway/pull/6356)
7373
```
7474

75-
Given that all PRs will touch this file, to avoid conflicts and decrease the experience when merging them, we will use "git custom merge driver" (see https://git-scm.com/docs/gitattributes#_defining_a_custom_merge_driver).
75+
Given that all PRs will touch this file, to avoid conflicts and decrease the experience when merging them, we will use "git custom merge driver" ([https://git-scm.com/docs/gitattributes#\_defining_a_custom_merge_driver](https://git-scm.com/docs/gitattributes#_defining_a_custom_merge_driver)).
7676

7777
This allows a custom merge strategy to a specific file using ".gitattributes".
7878

@@ -82,7 +82,7 @@ ReleaseNotes.md merge=union
8282

8383
## Breaking changes
8484

85-
A breaking change is a difference in functionality from the previous version of the compiler that may require an update to sway code in order for it to compile.
85+
A breaking change is a difference in functionality from the previous version of the compiler that may require an update to Sway code in order for it to compile.
8686

8787
The following changes are defined to be breaking changes and will need to follow the process of being gated by feature flags.
8888

@@ -95,11 +95,10 @@ The following changes are defined to be breaking changes and will need to follow
9595
- How `contract method` arguments are encoded;
9696
- How `log` data is encoded;
9797
- How `message` data is encoded;
98-
4. Utilization of new VM opcodes;
99-
5. IR changes
100-
6. Receipt parsers to break;
101-
7. When a compiler feature or a standard library produces different behavior for the same code (semantic changes);
102-
8. When `storage` is impacted. Particularly addresses.
98+
4. IR changes;
99+
5. Receipt parsers to break;
100+
6. When a compiler feature or a standard library produces different behavior for the same code (semantic changes);
101+
7. When `storage` is impacted. Particularly addresses.
103102

104103
## Feature flags
105104

@@ -108,7 +107,7 @@ Any complex change that needs to be gated will follow these steps.
108107
1. A specific GitHub issue labeled with `feature-*` and `track-feature`. This issue should be the umbrella for everything related to this feature;
109108
2. A preliminary PR enabling the feature flag should be created and linked to the umbrella issue; This PR will enable the feature flag `--experimental <comma-separated-list>` on all tools.
110109
3. As many PRs will be created and merged as normal;
111-
4. A chapter inside `Sway Unstable book` will be created and updated as needed;
110+
4. A chapter inside `Sway Unstable Book` will be created and updated as needed;
112111
5. When the feature is ready, a closing PR will be created and wait until the feature flag is enabled by default.
113112
6. On a later date, the feature flag can be removed making the feature the default behavior of the compiler.
114113

@@ -161,7 +160,7 @@ A special token `*` will mean "all features" in the sense that all features can
161160
> forc ... --experimental *
162161
```
163162

164-
This is specially useful to control what features are enabled
163+
This is specially useful to control which features are enabled
165164

166165
```
167166
> forc .. --no-experimental * --experimental some_feature
@@ -192,7 +191,7 @@ This section contains the suggested guide on how to introduce changes to differe
192191

193192
## `sway-features` crate
194193

195-
A new crate named `sway-features` will be created and will contain **ALL** features and their metadata. The best suggestion is a macro to define features where enums, documentation, etc... will be generated.
194+
A new crate named `sway-features` will be created and will contain **ALL** features and their metadata. The best suggestion is a macro to define features where enums, documentation, etc., will be generated.
196195

197196
This macro also needs to generate code for the errors and warnings related to each error.
198197

@@ -219,7 +218,7 @@ To allow as user-friendly error messages as possible, in all possible cases, we
219218

220219
After that, the lexer and the parser will mark that an experimental feature was lexed/parsed; and a check will guarantee that no disabled experimental was parsed.
221220

222-
The error message will have a message explaining that the feature is experimental and a Github link for more details on the stabilization lifecycle.
221+
The error message will have a message explaining that the feature is experimental and a GitHub link for more details on the stabilization lifecycle.
223222

224223
```rust
225224
// always parse new syntax
@@ -237,9 +236,9 @@ Formatting should always format new syntax, even when the flag is off. To avoid
237236

238237
LSP can take advantage of specific error messages, and suggest users to enable the corresponding feature.
239238

240-
## CST, AST and Typed Tree
239+
## CST, AST, and Typed Tree
241240

242-
All trees must always support new features, which means that new nodes will always exist. Their specific behavior, desugaring, etc... will be gated by the experimental feature.
241+
All trees must always support new features, which means that new nodes will always exist. Their specific behavior, desugaring, etc., will be gated by the experimental feature.
243242

244243
More specifically, this means that variants should not be behind compiler flags.
245244

@@ -304,15 +303,15 @@ These are the sources and stacks used as motivation for this RFC.
304303

305304
https://rustc-dev-guide.rust-lang.org/implementing_new_features.html#stability-in-code
306305

307-
1. - Open a tracking issue;
308-
2. - Pick a name for the feature gate;
309-
3. - Add the feature gate to the compiler code;
310-
4. - If the feature gate is not set, you should either maintain the pre-feature behavior or raise an error, depending on what makes sense;
311-
5. - Add a test to ensure the feature cannot be used without a feature gate;
312-
6. - Add a section to the unstable book;
313-
7. - Write a lot of tests for the new feature;
314-
8. - Get your PR reviewed and land it.
315-
9. - Add a section to the unstable book;
306+
1. Open a tracking issue;
307+
2. Pick a name for the feature gate;
308+
3. Add the feature gate to the compiler code;
309+
4. If the feature gate is not set, you should either maintain the pre-feature behavior or raise an error, depending on what makes sense;
310+
5. Add a test to ensure the feature cannot be used without a feature gate;
311+
6. Add a section to the unstable book;
312+
7. Write a lot of tests for the new feature;
313+
8. Get your PR reviewed and land it.
314+
9. Add a section to the unstable book;
316315

317316
## changeset
318317

@@ -336,7 +335,7 @@ Changes will categorized following https://keepachangelog.com/en/1.1.0/
336335
## Should new warnings be considered breaking changes?
337336

338337
1. Normally warnings should not be considered breaking changes, because they do not break anything.
339-
2. on the other hand, if the team treats warnings as errors, new warnings will break CIs, and demand
338+
2. On the other hand, if the team treats warnings as errors, new warnings will break CIs, and demand
340339
developers' attention. Not all fixes are trivial and can demand bigger code changes than the user would expect
341340
from a minor update from the compiler.
342341

0 commit comments

Comments
 (0)