Skip to content

Conversation

ReneSac
Copy link
Contributor

@ReneSac ReneSac commented Feb 3, 2014

No description provided.

One for 'do notation' in a single function in an expression, another the trick of using the method call syntax to pass two parameters.
@ReneSac
Copy link
Contributor Author

ReneSac commented Feb 3, 2014

Return is a bit shorter and easier to type than 'result =', and it is used that way in many places in the manual and Nimrod codebase itself. I see no problem with this use of return. I use 'result=' where it can simplify/shave one or more return calls.

But if you want I can change it, change it in the tutorial 2 from where I copied, and in the "Procedures" section that I also saw it but didn't touch.

@Araq
Copy link
Member

Araq commented Feb 3, 2014

I agree with reactormonk, but it's a minor thing.

Araq added a commit that referenced this pull request Feb 3, 2014
Additions to the language manual and tests of new procedure call syntax.
@Araq Araq merged commit a15ad34 into nim-lang:devel Feb 3, 2014
Araq added a commit that referenced this pull request Apr 7, 2014
Additions to the language manual and tests of new procedure call syntax.
Clyybber pushed a commit to Clyybber/Nim that referenced this pull request Sep 16, 2023
## Summary

Perform constant folding at the end of the first semantic pass, prior to
`sempass2`. Inlining constants and folding constant expression can
produce errors and, more generally, decides program behaviour, meaning
that performing these steps in `transf` is too late. In addition to
reducing the amount of repeated work for the compiler, this change
should also make further improvements to `semfold` easier.

**Fixes/Improvements**:
- the compiler doesn't crash when errors are detected during constant
  folding
- static range, array index, conversion, and other constant folding
  errors can now be detected via `compiles`, and they're now also
  reported with `nim check`
- legacy not-nil annotations and the `ProveInit` warnings now work
  better and more reliably in code using enum values

**Breaking changes**:
- the AST returned by `getImpl` now contains the result of constant
  folding

## Details

The core of the change is the introduction of `foldInAst` and
`foldConstExprAux` (the 'Aux' postfix is used to make the future
introduction of a public `foldConstExpr` easier). They implement a
depth-first traversal of the AST and inline/fold constants in almost the
same way as `transf` previously did. Compared to `transf`, they do
support error propagation.

### Implementation of the pass

A standard AST traversal is used, but expressions and statements are
separated as a preparation for further changes to the pass. The
introduced `Folded` type both tracks whether there's an error in a tree
and also implements a simple copy-on-write mechanism that prevents
unnecessary copies when nothing changes in a sub-tree. Same as `transf`,
`getConstExpr` is called on each sub-expression, with the result being
inlined, if allowed.

### Differences to `transf`

* statement-list expressions where the first sub-node is a doc comment
  are not folded away -- the documentation generator still needs them
* `getConstExpr` is not called on nodes of literal values (`nkIntLit`,
  `nkStrLit`, etc.) as that would only introduce unnecessary copies
  (potentially forcing a whole tree to be copied)

### Updating the compiler

* all places where AST is passed on to either `sempass2` or `transf`
  now run the constant folding pass (`foldInAst`)
* since the AST is passed on to the VM (and thus `transf`) in case the
  expression is not fully constant, `evalConstExpr` and `tryConstExpr`
  first run the folding pass before calling `getConstExpr`. This is
  slightly more efficient than the other way around
* `getConstExpr2` is a convenience wrapper around `getConstExpr` that
  allows passing in `nkError` nodes

With constant expressions being already folded in AST reaching
`sempass2`, two simplifications are possible:
* `sempass2` doesn't have to tentatively fold a call in order to know
  whether a call expression should be analyzed or not
* `semcomptime` doesn't have to special-case `mLengthArray` magics

Since the constant folding pass is now *not* run for the bodies of
lifted operations (e.g., lifetime hooks), the lifting has to make sure
that the output can be consumed by logic that expects constant-folded
AST:
* `liftdestructors` attempts to directly fold `offsetOf` and `alignOf`
* `genEnumToStrProc` inserts enum literals directly as `nkIntLit`
  instead of as `skEnumField` symbols

In `semGenericArgs`, a defensive copy is of the evaluated constant
expression is created prior to assigning the created `static` type. This
prevents recursive types when `opr` is the exact same node as
`evaluated`.

Finally, the `semfold` integration is removed from `transf`. The logic
for collapsing `nkCheckedFieldExpr`s is obsolete (done by the folding
pass) and the labels in `nkOfBranch` trees can be ignored.

### Other changes

* remove support for folding `nkChckRange`, `nkStringToCString`, and
  `nkCStringToString` nodes -- these nodes are first introduced in
  `transf`, which folding now happens prior to
* remove `adSemfoldInvalidConversion` diagnostic and report associated
  with range-conversion folding errors
* remove a leftover usage of `nkPar` for meaning "tuple constructor"

The `guards` module (which implements facts and implications used in
guarded data-flow analysis) already supported enum operation (e.g.,
`==`, `in`, etc.) but since enum values reached `sempass2` as
`skEnumField` symbols (which `guards` doesn't understand), facts
involving enum values generally didn't apply/work. With enum values now
lowered into integer literals, `guards` can automatically work with
them. An existing test for the `guards` module that now works properly
is update and changed in way so that silent regressions are prevented.

Since `nkHiddenConv` nodes in the index slot of `nkBracketExpr` nodes
now still exist when folding (the nodes are removed in `transf`),
different errors are produced for static out-of-bounds array access. The
associated tests are marked as known issues until it's clearer how this
should ideally work.

---------

Co-authored-by: Saem Ghani <saemghani+github@gmail.com>
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.

2 participants