fixes#24450
The new concepts were previously not included in
[containsGenericType][1] which prevents them from being instantiated.
Here they are included by being added to `tyTypeClasses` though this
doesn't have to be done, they can also be added manually to
`containsGenericTypeIter`, but this might be too specific.
[1]:
a2031ec6cf/compiler/types.nim (L1507-L1517)
(cherry picked from commit e28d2f42e9)
closes#24372, refs #20091
This was added in #20091 for some reason but doesn't actually work and
only makes error messages more obscure. So for now, it's disabled.
Can also be backported to 2.0 if necessary.
(cherry picked from commit a610f23060)
follows up #24425, fixes#18861, fixes#22445
Since #24425 generic object and distinct types now accurately link back
to their generic instantiations. To work around this issue previously,
type matching checked if generic objects/distinct types were
*structurally* equal, which caused false positives with types that
didn't use generic parameters in their structures. This structural check
is now removed, in cases where generic objects/distinct types require a
nontrivial equality check, the generic parameters of the `typeInst`
fields are checked for equality instead.
The check is copied from `tyGenericInst`, but the check in
`tyGenericInst` is not always sufficient as this type can be skipped or
unreachable in the case of `ref object`s.
(cherry picked from commit a2031ec6cf)
fixes#22479, fixes#24374, depends on #24429 and #24430
When instantiating generic types which directly have nominal types
(object, distinct, ref/ptr object but not enums[^1]) as their values,
the nominal type is now copied (in the case of ref objects, its child as
well) so that it receives a fresh ID and `typeInst` field. Previously
this only happened if it contained any generic types in its structure,
as is the case for all other types.
This solves #22479 and #24374 by virtue of the IDs being unique, which
is what destructors check for. Technically types containing generic
param fields work for the same reason. There is also the benefit that
the `typeInst` field is correct. However issues like #22445 aren't
solved because the compiler still uses structural object equality checks
for inheritance etc. which could be removed in a later PR.
Also fixes a pre-existing issue where destructors bound to object types
with generic fields would not error when attempting to define a user
destructor after the fact, but the error message doesn't show where the
implicit destructor was created now since it was only created for
another instance. To do this, a type flag is used that marks the generic
type symbol when a generic instance has a destructor created. Reusing
`tfCheckedForDestructor` for this doesn't work.
Maybe there is a nicer design that isn't an overreliance on the ID
mechanism, but the shortcomings of `tyGenericInst` are too ingrained in
the compiler to use for this. I thought about maybe adding something
like `tyNominalGenericInst`, but it's really much easier if the nominal
type itself directly contains the information of its generic parameters,
or at least its "symbol", which the design is heading towards.
[^1]: See [this
test](21420d8b09/lib/std/enumutils.nim (L102))
in enumutils. The field symbols `b0`/`b1` always have the uninstantiated
type `B` because enum fields don't expect to be generic, so no generic
instance of `B` matches its own symbols. Wouldn't expect anyone to use
generic enums but maybe someone does.
(cherry picked from commit 05c74d6844)
fixes#17571
Objects in the VM are represented as object constructor nodes that
contain every single field, including ones in different case branches.
This is so that every field has a unique invariant index in the object
constructor that can be written to and read from. However when
converting this node back into semantic code, fields from inactive case
branches can remain in the constructor which causes bad codegen,
generating assignments to fields from other case branches.
To fix this, fields from inactive branches are now detected in
`semmacrosanity.annotateType` (called in `fixupTypeAfterEval`) and
marked to prevent the codegen of their assignments. In #24441 these
fields were excluded from the resulting node, but this causes issues
when the node is directly supposed to go back into the VM, for example
as `const` values. I don't know if this is the only case where this
happens, so I wasn't sure about how to keep that implementation working.
(cherry picked from commit 75b512bc6a)
fixes#16413
`semIterator` checks if the original iterator passed to it has no body,
but it should check the processed node created by `semProcAux`.
(cherry picked from commit e239968b80)
fixes#24402
```nim
iterator myPairsInline*[T](twoDarray: seq[seq[T]]): (int, seq[T]) {.inline.} =
for indexValuePair in twoDarray.pairs:
yield indexValuePair
proc innerTestTotalMem() =
var my2dArray: seq[seq[int32]] = @[]
# fill with some data...
for i in 0'i32..100:
var z = @[i, i+1]
my2dArray.add z
for oneDindex, innerArray in myPairsInline(my2dArray):
discard
innerTestTotalMem()
```
In `for oneDindex, innerArray in myPairsInline(my2dArray)`, `oneDindex`
and `innerArray` becomes `cursors` because they satisfy the criterion of
`isSimpleIteratorVar`. On the one hand, it is not correct to have them
point to the temporary generated by tuple unpacking, which left the
memory of the temporary uncleaned up. On the other hand, we don't need
to generate a temporary for a symbol node when unpacking the tuple.
(cherry picked from commit 21420d8b09)
split from #24425
Matching `tyGenericBody` performs a match on the last child of the
generic body, in this case the uninstantied `tyObject` type. If the
object contains no generic fields, this ends up being the same type as
all instantiated ones, but if it does, a new type is created. This fails
the `sameObjectTypes` check that subtype matching for object types uses.
To fix this, also consider that the pattern type could be the generic
uninstantiated object type of the matched type in subtype matching.
(cherry picked from commit 511ab72342)
fixes#17958
In `transf`, conversions in subscript expressions are skipped (with
`skipConv`'s rules). This is because array indexing can produce
conversions to the range type that is the array's index type, which
causes a `RangeDefect` rather than an `IndexDefect` (and also
`--rangeChecks` and `--indexChecks` are both considered). However this
causes problems when explicit conversions are used, between types of
different bitsizes, because those also get skipped.
To fix this, we only skip the conversion if:
* it's a hidden (implicit) conversion
* it's a range check conversion (produces `nkChckRange`)
* the subscript is on an array type and the result type of the
conversion has the same bounds as the array index type
And `skipConv` rules also still apply (int/float classification).
Another idea would be to prevent the implicit conversion to the array
index type from being generated. But there is no good way to do this:
matching to the base type instead prevents types like `uint32` from
implicitly converting (i.e. it can convert to `range[0..3]` but not
`int`), and analyzing whether this is an array bound check is easier in
`transf`, since `sigmatch` just produces a type conversion.
The rules for skipping the conversion could also receive some other
tweaks: We could add a rule that changing bitsizes also doesn't skip the
conversion, but this breaks the `uint32` case. We could simplify it to
only removing implicit skips to specifically fix#17958, but this is
wrong in general.
We could also add something like `nkChckIndex` that generates index
errors instead but this is weird when it doesn't have access to the
collection type and it might be overkill.
(cherry picked from commit 76c5f16ac5)
fixes#14067, fixes#15004, fixes#19019
Anonymous procs are [added to
scope](8091d76306/compiler/semstmts.nim (L2466))
with the name `:anonymous`. This means that if they have the same
signature in a scope, they can consider each other as redefinitions. To
prevent this, mark their symbols as `sfGenSym` so they do not get added
to scope or cause any name conflicts. The commented out `and not isAnon`
check wouldn't work because `isAnon` would not be true if the proc is
being resemmed, in which case the name field in the proc AST would have
the symbol of the anonymous proc rather than being empty.
There is a separate problem of default values in generic/normal procs
not opening new scopes which is partially responsible for #19019.
(cherry picked from commit 3e47725c08)
fixes#24415
Since #23978 (which is in 2.0), all generic types that alias to another
type now insert a `tyAlias` layer in their value. However the
`skipGenericAlias` etc code which `sigmatch` uses is not updated for
this, so `tyAlias` is now skipped in these.
The relevant code in sigmatch is:
67ad1ae159/compiler/sigmatch.nim (L1668-L1673)
This behavior is also suspicious IMO, not skipping a structural
`tyGenericInst` alias can be useful for code like #10220, but this is
currently arbitrarily decided based on "depth" and whether the alias is
to another `tyGenericInst` type or not. Maybe in the future we could
enforce the use of a nominal type.
(cherry picked from commit 45b8434c7d)
fixes issue described in https://forum.nim-lang.org/t/12579
In #24065 explicit generic parameter matching was made to fail matches
on arguments with unresolved types in generic contexts (the sigmatch
diff, following #24010), similar to what is done for regular calls since
#22029. However unlike regular calls, a failed match in a generic
context for a standalone explicit generic instantiation did not convert
the expression into one with `tyFromExpr` type, which means it would
error immediately given any unresolved parameter. This is now done to
fix the issue.
For explicit generic instantiations on single non-overloaded symbols, a
successful match is still instantiated. For multiple overloads (i.e.
symchoice), if any of the overloads fail the match, the entire
expression is considered untyped and any instantiations are not used, so
as to not void overloads that would match later. This means even
symchoices without unresolved arguments aren't instantiated, which may
be too restrictive, but it could also be too lenient and we might need
to make symchoice instantiations always untyped. The behavior for
symchoice is not sound anyway given it causes #9997 so this is something
to consider for a redesign.
Diff follows #24276.
(cherry picked from commit 67ad1ae159)
fixes#24378
```nim
type Win = typeof(`body`)
doAssert not supportsCopyMem((int, Win))
```
`semAfterMacroCall` doesn't skip the children aliases types in the tuple
typedesc construction while the normal program seem to skip the aliases
types somewhere
`(int, Win)` is kept as `(int, alias string)` instead of expected `(int,
string)`
(cherry picked from commit 5e56f0a356)
refs #23625, refs #24289
Encountered in #24360 but could not reproduce minimally: overloading on
static parameters can work with the normal compile commands but crash
`nim check`. Static overloading relies on `tryConstExpr` which recovers
from things like `globalError` and fails softly, in this case this can
happen when a variable etc. is not available to evaluate in the VM. But
with `nim check`, the compiler does not throw an exception in this case,
and instead tries to keep generating the entire expression in the VM,
which can cause crashes.
To fix this, when the compiler has no error outputs even on `nim check`,
we raise a global error so that the VM code generation stops early. This
fixes both `tryConstExpr` and speeds up `nim check`, because no error
outputs means we don't need cascading errors.
(cherry picked from commit efd603eb28)
pre-existing issues:
```nim
block:
type
FooObj = object
data: int
Foo = ref ref FooObj
proc delete(self: Foo) =
echo self.data
var s: Foo
new(s, delete)
```
it crashed with arc/orc in 1.6.x and 2.x.x
```nim
block:
type
Foo = ref int
proc delete(self: Foo) =
echo self[]
var s: Foo
new(s, delete)
```
The simple fix is to add a type restriction for the type `T` for arc/orc
versions
```nim
proc new*[T: object](a: var ref T, finalizer: proc (x: T) {.nimcall.})
```
(cherry picked from commit 2af602a5c8)
fixes#13417, fixes#19703
When passing an expression to an `openarray` iterator parameter: If the
expression is a statement list (considered "complex"), it's assigned in
a non-deep-copying way to a temporary variable first, then this variable
is used as a parameter. If it's not a statement list, i.e. a call or a
symbol, the parameter is substituted directly with the given expression.
In the case of calls, this results in the call potentially being
executed more than once, or can cause redefined variables in the
codegen.
To fix this, calls are also considered as "complex" assignments to
openarrays, as long as the return type of the call is not `openarray` as
the generated assignment in that case has issues/is unimplemented
(caused a segfault [here in
datamancer](47ba4d81bf/src/datamancer/dataframe.nim (L1580))).
As for why creating a temporary isn't the default only with exceptions
for things like `nkSym`, the "non-deep-copying" way of assignment
apparently still causes arrays to be copied according to a comment in
the code. I'm not sure to what extent this is true: if it still happens
on ARC/ORC, if it happens for every array length, or if we can fix it by
passing arrays by reference. Otherwise, a more general way to assign to
openarrays might be needed, but I'm not sure if the compiler can easily
do this.
(cherry picked from commit d303c289fa)
fixes#23952
It reorders `type Foo = enum A, B = -1` to `type Foo = enum B = -1, A`
so that `firstOrd` etc. continue to work.
(cherry picked from commit 294b1566e7)
closes https://github.com/nim-lang/RFCs/issues/380, fixes#4773, fixes
#14729, fixes#16755, fixes#18150, fixes#22984, refs #11167 (only some
comments fixed), refs #12620 (needs tiny workaround)
The compiler gains a concept of root "nominal" types (i.e. objects,
enums, distincts, direct `Foo = ref object`s, generic versions of all of
these). Exported top-level routines in the same module as the nominal
types that their parameter types derive from (i.e. with
`var`/`sink`/`typedesc`/generic constraints) are considered attached to
the respective type, as the RFC states. This happens for every argument
regardless of placement.
When a call is overloaded and overload matching starts, for all
arguments in the call that already have a type, we add any operation
with the same name in the scope of the root nominal type of each
argument (if it exists) to the overload match. This also happens as
arguments gradually get typed after every overload match. This restricts
the considered overloads to ones attached to the given arguments, as
well as preventing `untyped` arguments from being forcefully typed due
to unrelated overloads. There are some caveats:
* If no overloads with a name are in scope, type bound ops are not
triggered, i.e. if `foo` is not declared, `foo(x)` will not consider a
type bound op for `x`.
* If overloads in scope do not have enough parameters up to the argument
which needs its type bound op considered, then type bound ops are also
not added. For example, if only `foo()` is in scope, `foo(x)` will not
consider a type bound op for `x`.
In the cases of "generic interfaces" like `hash`, `$`, `items` etc. this
is not really a problem since any code using it will have at least one
typed overload imported. For arbitrary versions of these though, as in
the test case for #12620, a workaround is to declare a temporary
"template" overload that never matches:
```nim
# neither have to be exported, just needed for any use of `foo`:
type Placeholder = object
proc foo(_: Placeholder) = discard
```
I don't know what a "proper" version of this could be, maybe something
to do with the new concepts.
Possible directions:
A limitation with the proposal is that parameters like `a: ref Foo` are
not attached to any type, even if `Foo` is nominal. Fixing this for just
`ptr`/`ref` would be a special case, parameters like `seq[Foo]` would
still not be attached to `Foo`. We could also skip any *structural* type
but this could produce more than one nominal type, i.e. `(Foo, Bar)`
(not that this is hard to implement, it just might be unexpected).
Converters do not use type bound ops, they still need to be in scope to
implicitly convert. But maybe they could also participate in the nominal
type consideration: if `Generic[T] = distinct T` has a converter to `T`,
both `Generic` and `T` can be considered as nominal roots.
The other restriction in the proposal, being in the same scope as the
nominal type, could maybe be worked around by explicitly attaching to
the type, i.e.: `proc foo(x: T) {.attach: T.}`, similar to class
extensions in newer OOP languages. The given type `T` needs to be
obtainable from the type of the given argument `x` however, i.e.
something like `proc foo(x: ref T) {.attach: T.}` doesn't work to fix
the `ref` issue since the compiler never obtains `T` from a given `ref
T` argument. Edit: Since the module is queried now, this is likely not
possible.
---------
Co-authored-by: Andreas Rumpf <rumpf_a@web.de>
(cherry picked from commit 2864830941)
fixes#24338
When unrolling each iteration of a `fields` iterator, the compiler only
opens a new scope for semchecking, but doesn't generate a node that
signals to the codegen that a new scope should be created. This causes
issues for reused template instantiations that reuse variable symbols
between each iteration, which causes the codegen to generate multiple
declarations for them in the same scope (regardless of `inject` or
`gensym`). To fix this, we wrap the unrolled iterations in an `if true:
body` node, which both opens a new scope and doesn't interfere with
`break`.
(cherry picked from commit ca5df9ab25)
Running `ctags` on files with quoted symbols (e.g. `$`) would list \`
instead of the full ident. Issue was the result getting reassigned at
the end to a \` instead of appending
(cherry picked from commit 93c24fe1c5)
closes https://github.com/nim-lang/RFCs/issues/554
Adds a symmetric difference operation to the language bitset type. This
maps to a simple `xor` operation on the backend and thus is likely
faster than the current alternatives, namely `(a - b) + (b - a)` or `a +
b - a * b`. The compiler VM implementation of bitsets already
implemented this via `symdiffSets` but it was never used.
The standalone binary operation is added to `setutils`, named
`symmetricDifference` in line with [hash
sets](https://nim-lang.org/docs/sets.html#symmetricDifference%2CHashSet%5BA%5D%2CHashSet%5BA%5D).
An operator version `-+-` and an in-place version like `toggle` as
described in the RFC are also added, implemented as trivial sugar.
(cherry picked from commit ae9287c4f3)
refs #8064, refs #24010
Error messages for standalone explicit generic instantiations are
revamped. Failing standalone explicit generic instantiations now only
error after overloading has finished and resolved to the default `[]`
magic (this means `[]` can now be overloaded for procs but this isn't
necessarily intentional, in #24010 it was documented that it isn't
possible). The error messages for failed instantiations are also no
longer a simple `cannot instantiate: foo` message, instead they now give
the same type mismatch error message as overloads with mismatching
explicit generic parameters.
This is now possible due to the changes in #24010 that delegate all
explicit generic proc instantiations to overload resolution. Old code
that worked around this is now removed. `maybeInstantiateGeneric` could
maybe also be removed in favor of just `explicitGenericSym`, the `result
== n` case is due to `explicitGenericInstError` which is only for niche
cases.
Also, to cause "ambiguous identifier" error messages when the explicit
instantiation is a symchoice and the expression context doesn't allow
symchoices, we semcheck the sym/symchoice created by
`explicitGenericSym` with the given expression flags.
#8064 isn't entirely fixed because the error message is still misleading
for the original case which does `values[1]`, as a consequence of
#12664.
(cherry picked from commit 0a058a6b8f)
The first commit reverts the revert of #23787.
The second fixes lambdalifting in convolutedly nested
closures/closureiters. This is considered to be the reason of #24094,
though I can't tell for sure, as I was not able to reproduce #24094 for
complicated but irrelevant reasons. Therefore I ask @jmgomez, @metagn or
anyone who could reproduce it to try it again with this PR.
I would suggest this PR to not be squashed if possible, as the history
is already messy enough.
Some theory behind the lambdalifting fix:
- A closureiter that captures anything outside its body will always have
`:up` in its env. This property is now used as a trigger to lift any
proc that captures such a closureiter.
- Instantiating a closureiter involves filling out its `:up`, which was
previously done incorrectly. The fixed algorithm is to use "current" env
if it is the owner of the iter declaration, or traverse through `:up`s
of env param until the common ancestor is found.
---------
Co-authored-by: Andreas Rumpf <rumpf_a@web.de>
(cherry picked from commit 5fa96ef270)
fixes#24305, refs #23807
Since #23014 `nkHiddenAddr` is produced to fast assign array elements in
iterators. However the array access inside this `nkHiddenAddr` can get
folded at compile time, generating invalid code. In #23807, compile time
folding of regular `addr` expressions was changed to be prevented in
`transf` but `nkHiddenAddr` was not updated alongside it.
The method for preventing folding in `addr` in #23807 was also faulty,
it should only trigger on the immediate child node of the address rather
than all nodes nested inside it. This caused a regression as outlined in
[this
comment](https://github.com/nim-lang/Nim/pull/24322#issuecomment-2419560182).
To fix both issues, `addr` and `nkHiddenAddr` now both shallowly prevent
constant folding for their immediate children.
(cherry picked from commit 52cf7dfde0)
fixes#24319
`byRefLoc` (`mapType`) requires the Loc `a` to have the right type.
Without `lfEnforceDeref`, it produces the wrong type for `deref (var
array)`, which may come from `mitems`.
(cherry picked from commit 0347536ff2)
fixes#18896fixes#20886
```nim
type
PFile {.importc: "FILE*", header: "<stdio.h>".} = distinct pointer
# import C's FILE* type; Nim will treat it as a new pointer type
```
This is an excerpt from the Nim manual. In the old Nim versions, it
produces a void pointer type instead of the `FILE*` type that should
have been generated. Because these C types tend to be opaque and adapt
to changes on different platforms. It might affect the portability of
Nim on various OS, i.e. `csource_v2` cannot build on the apline platform
because of `Time` relies on Nim types instead of the `time_t` type.
ref https://github.com/nim-lang/Nim/pull/18851
(cherry picked from commit 8be82c36c9)
fixes#24258
It uses conditionals to guard against ill formed AST to produce better
error messages, rather than crashing
(cherry picked from commit 8b39b2df7d)
fixes#24296fixes#24295
Templates use `expectedType` for type inference. It's justified that
when templates don't have an actual return type, i.e., `untyped` etc.
When the return type of templates is specified, we should not infer the
type
```nim
template g(): string = ""
let c: cstring = g()
```
In this example, it is not reasonable to annotate the templates
expression with the `cstring` type before the `fitNode` check with its
specified return type.
(cherry picked from commit 80e6b35721)
fixes#24288, refs #23625
Since #23625 "cannot evaluate" errors during VM code generation are
"soft" errors with `nim check`, i.e. the code generation isn't halted
(except for the current proc which `return`s which can cause wrong
codegen) and the expression is still attempted to be evaluated. Now,
these errors signal to the VM that the current generated VM code cannot
be evaluated, and so instead of evaluating, an error node is returned.
This keeps the benefit of the "soft" errors without potentially crashing
the compiler on improperly generated VM code. Although maybe the
compiler might not be able to handle the generated error node in some
cases.
This fixes the chame example in #24288 but this is not tested in CI.
Presumably it or the compiler was doing something like `compiles()` on
code that can't run in the VM.
I would accept nicer ways of tracking non-evaluability than
`c.cannotEval = true` but I tried to keep it as harmless as possible.
(cherry picked from commit def1fea43a)
fixes#24021
The field checking for case object branches at some point generates a
negated set `contains` check for the object discriminator. For enum
types, this tries to generate a complement set and convert to a
`contains` check in that instead. It obtains this type from the type of
the element node in the `contains` check.
`buildProperFieldCheck` creates the element node by changing a field
access expression like `foo.z` into `foo.kind`. In order to do this, it
copies the node `foo.z` and sets the field name in the node to the
symbol `kind`. But when copying the node, the type of the original
`foo.z` is retained. This means that the complement is performed on the
type of the accessed field rather than the type of the discriminator,
which causes problems when the accessed field is also an enum.
To fix this, we properly set the type of the copied node to the type of
the kind field. An alternative is just to make a new node instead.
A lot of text for a single line change, I know, but this part of the
codebase could use more explanation.
(cherry picked from commit 1bebc236bd)
fixes#24087, refs https://forum.nim-lang.org/t/341, refs #14222, refs
#14221
The Nim compiler calls `cl` for linking as well as compilation. This
means that options to the linker have to be passed after a `/link`
argument. But the Nim compiler doesn't include this option normally,
because users may still want to pass non-linker options to `cl` at link
time.
To deal with this, a workaround is used: every single library link
option adds `/link` before it. The linker simply ignores extraneous
`/link` arguments and gives a warning instead, since it's an
unrecognized option to the linker. This is really hacky but otherwise we
need to separate linker arguments into arguments passed either to the
compiler or to the linker at link time, and this behavior wouldn't be
meaningful outside of MSVC.
I can't really test this manually but I did test that the linker ignores
`/link`. I also can't really do more than this, I don't really use MSVC
so I wouldn't know how to navigate it, or how people use it. Ideally
someone who knows more about/uses MSVC can give their input or take
over.
(cherry picked from commit 449106a5a4)
fixes#24269, refs #20095
Instead of checking the package of the *used sym* to determine whether a
stylecheck should trigger, we check the package of the lineinfo instead.
Before #20095 this checked for the current compilation context module
instead which caused issues with generic procs, but the lineinfo should
more closely match the AST.
I figured this might cause issues with includes etc but the foreign
package test specifically tests for an include and passes, so maybe the
package determining logic accounts for this already. This still might
not be the correct logic, I'm not too familiar with the package handling
in the compiler.
Package PRs, both merged:
- json_rpc: https://github.com/status-im/nim-json-rpc/pull/226
- json_serialization:
https://github.com/status-im/nim-json-serialization/pull/99
(cherry picked from commit aaf6c408c6)
fixes#24274
The code in the `if` branch replaces the current destination `d` with a
new one. But the location `d` can be an assignment location, in which
case the provided expression isn't generated. To fix this, don't trigger
this code for when the location already exists. An alternative would be
to call `putIntoDest` in this case as is done below.
(cherry picked from commit 9c85f4fd07)
fixes#24091, refs #24092
Any instantiations resolving to a generic body type now gives an error.
Due to #24092, this does not error in cases like matching against `type
M` in generics because generic body type symbols are just not
instantiated. But this prevents parameters with type `type M` from being
used, although there doesn't seem to be any code which does this. Just
in case such code exists, we still allow `typedesc` types resolving to
generic body types.
(cherry picked from commit 2f904535d0)
