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#24228, refs #22022
As described in
https://github.com/nim-lang/Nim/issues/24228#issuecomment-2392462221,
instantiating generic routines inside `typeof` causes all code inside to
be treated as being in a typeof context, and thus preventing compile
time proc folding, causing issues when code is generated for the
instantiated routine. Now, instantiated generic procs are treated as
never being inside a `typeof` context.
This is probably an arbitrary special case and more issues with the
`typeof` behavior from #22022 are likely. Ideally this behavior would be
removed but it's necessary to accomodate the current [proc `declval` in
the package `stew`](https://github.com/status-im/nim-stew/pull/190), at
least without changes to `compileTime` that would either break other
code (making it not eagerly fold by default) or still require a change
in stew (adding an option to disable the eager folding).
Alternatively we could also make the eager folding opt-in only for
generic compileTime procs so that #22022 breaks nothing whatsoever, but
a universal solution would be better. Edit: Done in #24230 via
experimental switch
(cherry picked from commit ea9811a4d2)
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)
This is in reference to a [feature
request](https://github.com/nim-lang/Nim/issues/22142) that I posted.
I'm making this PR to demonstrate the suggested change and expect that
this should be scrutinized
---------
Co-authored-by: Bung <crc32@qq.com>
Co-authored-by: Andreas Rumpf <rumpf_a@web.de>
(cherry picked from commit 74fa8ed59a)
Sorry I don't have a test case or issue for this. `injectdestructors` is
supposed to add a final bool argument to `=copy` and `=dup` to mark
cyclic types, as generated by `liftdestructors`. Hence this flag is
added after every call to `genCopy`, but `genCopy` can generate a
`=sink` call when passed the flag `IsExplicitSink` by `nkSinkAsgn`. This
creates a codegen error, saying the sink received an extra argument.
This is fixed by not adding the argument on the flag `IsExplicitSink`.
This is a followup to #20585 which is on the 2.0 branch, hence this is
marked backport.
(cherry picked from commit 7974a2208c)
fixes#24203
`semTypeNode` is called twice for RHS'es of type sections,
[here](b0e6d28782/compiler/semstmts.nim (L1612))
and
[here](b0e6d28782/compiler/semstmts.nim (L1646)).
Each time `prev` is `s.typ`, but the assertion expects `prev == nil`
which is false since `s.typ` is not nil the second time. To fix this,
the `prev == nil` part of the assertion is removed.
The reason this only happens for types like `seq[int]`, `(int, int)` etc
is because they don't have syms: `semTypeIdent` attempts to directly
[replace the typedesc param
itself](b0e6d28782/compiler/semtypes.nim (L1916))
with the sym of the base type of the resolved typedesc type if it
exists, which means `semTypeNode` doesn't receive the typedesc param sym
to perform the assert.
(cherry picked from commit 2a48182288)
fixes#24150, refs #22022
An exception is raised in the `semExprWithType` call, which means `dec
c.inTypeofContext` is never called, but `compiles` allows compilation to
continue. This means `c.inTypeofContext` is left perpetually nonzero,
which prevents `compileTime` evaluation for the rest of the program.
To fix this, `defer:` is used for the `dec c.inTypeofContext` call, as
is done for
[`instCounter`](d51d88700b/compiler/seminst.nim (L374))
in other parts of the compiler.
(cherry picked from commit a1777200c1)
fixes#24164, regression from #20091
The expression `nil` as the default value of template parameter `x:
untyped` is typechecked with expected type `untyped` since #20091. The
expected type is checked if it matches the `nil` literal with a match
better than a subtype match, and the type is set to it if it does.
However `untyped` matches with a generic match which is better, so the
`nil` literal has type `untyped`. This breaks type matching for the
literal. So if the expected type is `untyped` or `typed`, it is now
ignored and the `nil` literal just has the `nil` type.
(cherry picked from commit b9de2bb4f3)
This fixes a logic error in #23870
The inheritance penalty should be -1 if there is no inheritance
relationship. Not sure how to write a test case for this one honestly.
---------
Co-authored-by: Andreas Rumpf <rumpf_a@web.de>
(cherry picked from commit 37dba853c9)
alternative to #24101
enabled in the context of the generic/template declarations capturing
the symbols, not the context of the instantiation of the
generics/templates. This was to be in line with where the compiler gives
the warnings and changes behavior in a potentially breaking way.
However `results` [depends on the old
behavior](71d404b314/results.nim (L1428)),
so that the callers of the macros provided by results always take
advantage of the opensym behavior. To accomodate this, we change the
behavior of the old experimental option that `results` uses,
`genericsOpenSym`, so that ignores the information of whether or not
symbols are intentionally opened and always gives the opensym behavior
as long as it's enabled at instantiation time. This should keep
`results` working as is. However this differs from the normal opensym
switch in that it doesn't generate `nnkOpenSym`.
Before it was just a generics-only version of `openSym` along with
`templateOpenSym` which was only for templates. So `templateOpenSym` is
removed along with this change, but no one appears to have used it.
(cherry picked from commit 0c3573e4a0)
fixes#24112
Sym nodes in templates that could be open are [given `nil`
type](22d2cf2175/compiler/semtempl.nim (L274))
when `--experimentalOpenSym` is disabled so that they can be semchecked
to give a warning since #24007. The first nodes of object constructors
(in this case) and in type conversions don't replace their first node
(the symbol) with a typechecked one, they only call `semTypeNode` on it
and leave it as is.
Effect tracking checks if the type of a sym node has a destructor to
check if the node type should be replaced with the sym type. But this
causes a segfault when the type of the node is nil. To fix this, we
always set the node type to the sym type if the node type is nil.
Alternatively `semObjConstr` and `semConv` could be changed to set the
type of their first node to the found type but I'm not sure if this
would break anything. They could call `semExprWithType` on the first
node but `semTypeNode` would still have to be called (maybe call it
before?). This isn't a problem if the sym node has a type but is just
nested in `nkOpenSym` or `nkOpenSymChoice` which have nil type instead
(i.e. with openSym enabled), so maybe this still is the "most general"
solution, I don't know.
(cherry picked from commit 680a13a142)
split again from #24038, fixes
https://github.com/status-im/nimbus-eth2/pull/6554#issuecomment-2354977102
`var`/pointer types are no longer implicitly convertible to each other
if their element types either:
* require an int conversion or another conversion operation as long as
it's not to `openarray`,
* are subtypes with pointer indirection,
Previously any conversion below a subrange match would match if the
element type wasn't a pointer type, then it would error later in
`analyseIfAddressTaken`.
Different from #24038 in that the preview define that made subrange
matches also fail to match is removed for a simpler diff so that it can
be backported.
(cherry picked from commit 1660ddf98a)
fixes#15314, fixes#24002
The OpenSym behavior first added to generics in #23091 now also applies
to templates, since templates can also capture symbols that are meant to
be replaced by local symbols if the context imports symbols with the
same name, as in the issue #24002. The experimental switch
`templateOpenSym` is added to enable this behavior for templates only,
and the experimental switch `openSym` is added to enable it for both
templates and generics, and the documentation now mainly mentions this
switch.
Additionally the logic for `nkOpenSymChoice` nodes that were previously
wrapped in `nkOpenSym` now apply to all `nkOpenSymChoice` nodes, and so
these nodes aren't wrapped in `nkOpenSym` anymore. This means
`nkOpenSym` can only have children of kind `nkSym` again, so it is more
in line with the structure of symchoice nodes. As for why they aren't
merged with `nkOpenSymChoice` nodes yet, we need some way to signal that
the node shouldn't become ambiguous if other options exist at
instantiation time, we already captured a symbol at the beginning and
another symbol can only replace it if it's closer in scope and
unambiguous.
(cherry picked from commit 770f8d5513)
Close#22826
I am not sure why this code skips generic insts, so letting CI tell me.
Update: It has told me nothing. Maybe someone knows during review.
Issue itself seems to be that the generic instance is skipped thus it
ends up being just `float` which makes it use the wrong generic instance
of the proc because it matches the one in cache
---------
Co-authored-by: SirOlaf <>
(cherry picked from commit c13c48500b)
fixes#24104, refs #23955
The line `result.typ = dstTyp` added in #23955 changes the type of
`result`, which was the type of `n` due to the argument passed to
`newIntNodeT`, to the abstract type skipped `dstTyp`. The line is
removed to just keep the type as abstract.
(cherry picked from commit 6d362e0ffe)
It speeds up
```nim
proc foo =
let piece = cast[seq[char]](newSeqUninit[uint8](5220600386'i64))
foo()
```
Notes that `cast[ref](...)` is excluded because we need to keep the ref
alive if the parameter is something with pointer types (e.g.
`cast[ref](pointer)`or `cast[ref](makePointer(...))`)
---------
Co-authored-by: Andreas Rumpf <rumpf_a@web.de>
(cherry picked from commit 4ef06a5cc5)
fixes#8697, fixes#9620, fixes#23265
When matching a `template` with an `untyped` argument fails because of a
mismatching typed argument, `presentFailedCandidates` tries to sem every
single argument to show their types, but trying to type the `untyped`
argument can fail if it's supposed to use an injected symbol, so we get
an unrelated error message like "undeclared identifier".
Instead we use `tryExpr` as the comment suggests, setting the type to
`untyped` if it fails to compile. We could also maybe check if an
`untyped` argument is expected in its place and not try to compile the
expression if it is but this would require a bit of reorganizing the
code here and IMO it's better to have the information of what type it
would be if it can be typed.
(cherry picked from commit 34719cad9d)
fixes#23977
The problem is that for *any* body of a generic declaration,
[semstmts](2e4d344b43/compiler/semstmts.nim (L1610-L1611))
sets the sym of its value to the generic type name, and
[semtypes](2e4d344b43/compiler/semtypes.nim (L2143))
just directly gives the referenced type *specifically* when the
expression is a generic body. I'm blaming `semtypes` here because it's
responsible for the type given but the exact opposite behavior
specifically written in makes me think generating an alias type here
maybe breaks something.
(cherry picked from commit 58813a3b2e)
fixes#10753, fixes#22021, refs #19365 (was fixed by #22029, but more
faithful test added)
For whatever reason `compileTime` proc calls did not fold if the proc
was generic ([since this folding was
introduced](c25ffbf262 (diff-539da3a63df08fa987f1b0c67d26cdc690753843d110b6bf0805a685eeaffd40))).
I'm guessing the intention was for *unresolved* generic procs to not
fold, which is now the logic.
Non-magic `compileTime` procs also now don't fold at compile time in
`typeof` contexts to avoid possible runtime errors (only the important)
and prevent double/needless evaluation.
(cherry picked from commit f7c11a8978)
fixes#22850
The `is` operator checks the type of the left hand side, and if it's
generic or if it's a `typedesc` type with no base type, it leaves it to
be evaluated later. But `typedesc` types with no base type precisely
describe the default typeclass `type`/`typeclass`, so this condition is
removed. Maybe at some point this represented an unresolved generic
type?
(cherry picked from commit 995081b56a)
fixes#23853
Since #22610 generics turns the `Name` in the `GT.Name` expression in
the test code into a sym choice. The problem is when the compiler tries
to instantiate `GT.Name` it also instantiates the sym choice symbols.
`Name` has type `template (E: type ExtensionField)` which contains the
unresolved generic type `ExtensionField`, which the compiler mistakes as
an uninstantiated node, when it's just part of the type of the template.
The compilation of the node itself and hence overloading will handle the
instantiation of the proc, so we avoid instantiating it in `semtypinst`,
similar to how the first nodes of call nodes aren't instantiated.
(cherry picked from commit 97f5474545)
ref #20653
```nim
Error* = object
case kind*: ErrorType
of ErrorA:
discard
of ErrorB:
discard
```
For an object variants without fields, it shouldn't generate empty
brackets for default values since there are no fields at all in case
branches.
(cherry picked from commit 948bb38335)
fixes#10440, fixes#13871, fixes#14665, fixes#19672, fixes#23677
The false positive in #23677 was caused by behavior in
`implicitlyDiscardable` where only the last node of `if`/`case`/`try`
etc expressions were considered, as in the final node of the final
branch (in this case `else`). To fix this we use the same iteration in
`implicitlyDiscardable` that we use in `endsInNoReturn`, with the
difference that for an `if`/`case`/`try` statement to be implicitly
discardable, all of its branches must be implicitly discardable.
`noreturn` calls are also considered implicitly discardable for this
reason, otherwise stuff like `if true: discardableCall() else: error()`
doesn't compile.
However `endsInNoReturn` also had bugs, one where `finally` was
considered in noreturn checking when it shouldn't, another where only
`nkIfStmt` was checked and not `nkIfExpr`, and the node given for the
error message was bad. So `endsInNoReturn` now skips over
`skipForDiscardable` which no longer contains
`nkIfStmt`/`nkCaseStmt`/`nkTryStmt`, stores the first encountered
returning node in a var parameter for the error message, and handles
`finally` and `nkIfExpr`.
Fixing #23677 already broke a line in `syncio` so some package code
might be affected.
(cherry picked from commit 42e8472ca6)
refs
https://github.com/nim-lang/Nim/issues/23586#issuecomment-2102113750
In #20091 a bad kind of type inference was mistakenly left in where if
an identifier `abc` had an expected type of an enum type `Enum`, and
`Enum` had a member called `abc`, the identifier would change to be that
enum member. This causes bugs where a local symbol can have the same
name as an enum member but have a different value. I had assumed this
behavior was removed since but it wasn't, and CI seems to pass having it
removed.
A separate PR needs to be made for the 2.0 branch because these lines
were moved around during a refactoring in #23123 which is not in 2.0.
(cherry picked from commit c101490a0c)
fixes#18125
Previously a tuple type like `(T, int)` would match an expected tuple
type `(U, int)` if `T` is a subtype of `U`. This is wrong since the
codegen does not handle type conversions of individual tuple elements in
a type conversion of an entire tuple. For this reason the compiler
already does not accept `(float, int)` for a matched type `(int, int)`,
however the code that checked for which relations are unacceptable
checked for `< isSubtype` rather than `<= isSubtype`, so subtypes were
not included in the unacceptable relations.
Update: Now only considered unacceptable when inheritance is used, as in
[`paramTypesMatch`](3379d26629/compiler/sigmatch.nim (L2252-L2254)).
Ideally subtype relations that don't need conversions, like `nil`,
`seq[empty]`, `range[0..5]` etc would be their own relation
`isConcreteSubtype` (which would also allow us to differentiate with
`openArray[T]`), but this is too big of a refactor for now.
To compensate for this making things like `let x: (Parent, int) =
(Child(), 0)` not compile (they would crash codegen before anyway but
should still work in principle), type inference for tuple constructors
is updated such that they call `fitNode` on the fields and their
expected types, so a type conversion is generated for the individual
subtype element.
(cherry picked from commit cfd69bad1a)
Filling in some more logic in `typeRel` that I came across when poking
the compiler in another PR. Some of the cases where `typeRel` returns an
"incorrect" result are actually common, but `sumGeneric` ends up
breaking the tie correctly. There isn't anything wrong with that
necessarily, but I assume that it's preferred these functions behave
just as well in isolation as they do when integrated.
I will be following up this description with specific examples.
(cherry picked from commit ccc7c45d71)
fixes#23002, fixes#22841, refs comments in #23097
When an identifier is ambiguous in scope (i.e. multiple imports contain
symbols with the same name), attempt resolving it through type inference
(by creating a symchoice). To do this efficiently, `qualifiedLookUp` had
to be broken up so that `semExpr` can access the ambiguous candidates
directly (now obtained directly via `lookUpCandidates`).
This fixes the linked issues, but an example like:
```nim
let on = 123
{.warning[ProveInit]: on.}
```
will still fail, since `on` is unambiguously the local `let` symbol here
(this is also true for `proc on` but `proc` symbols generate symchoices
anyway).
Type symbols are not considered to not confuse the type inference. This
includes the change in sigmatch, up to this point symchoices with
nonoverloadable symbols could be created, they just wouldn't be
considered during disambiguation. Now every proper symbol except types
are considered in disambiguation, so the correct symbols must be picked
during the creation of the symchoice node. I remember there being a
violating case of this in the compiler, but this was very likely fixed
by excluding type symbols as CI seems to have found no issues.
The pure enum ambiguity test was disabled because ambiguous pure enums
now behave like overloadable enums with this behavior, so we get a
longer error message for `echo amb` like `type mismatch: got <MyEnum |
OtherEnum> but expected T`
(cherry picked from commit b280100499)
