fixes#24863, refs #23787 and #24316
Working off the minimized example, my understanding of the issue is: `n`
captures `r` as `:envP.r1` where `:envP` is the environment of `b`, then
`proc () = n()` does the lambda lifting of `n` again (which isn't done
if the `proc ()` is marked `{.closure.}`, hence the workaround) which
then captures the `:envP` as another field inside the `:envP`, so it
generates `:envP.:envP_2.r1` but the `.:envP_2` field is `nil`, so it
causes a segfault.
The problem is that the capture of `r` in `n` is done inside
`detectCapturedVars` for the surrounding closure iterator: inner procs
are not special cased and traversed as regular nodes, so it thinks it's
inside the iterator and generates a field access of `:envP` freely. The
lambda lifting version of `detectCapturedVars` ignores inner procs and
works off of symbol uses (anonymous iterator and lambda declarations
pretend their symbol is used).
As a naive solution, closure iterators now also ignore inner proc
declarations same as `lambdalifting.detectCapturedVars`, but unlike it
they also don't do anything for the inner proc symbols. Lambdalifting
seems to properly handle the lifted variables but in the worst case we
can also make sure `closureiters.detectCapturedVars` traverses inner
procs by marking every local of the closure iter used in them as needing
lifting (but not doing the lifting). This does not seem necessary for
now so it's not done (was done and reverted in [this
commit](9bb39a9259)),
but regressions are still possible
(cherry picked from commit c06bb6cc03)
refs https://forum.nim-lang.org/t/12785, refs #4711
The code was already there that when `propertyWriteAccess` returns `nil`
(i.e. cannot find a setter), `semAsgn` turns the [LHS into a call and
semchecks
it](1ef9a656d2/compiler/semexprs.nim (L1941-L1948)),
meaning if a setter cannot be found a getter will be assigned to
instead. However `propertyWriteAccess` never returned nil, because
`semOverloadedCallAnalyseEffects` was not called with `efNoUndeclared`
and so produced an error directly. So `efNoUndeclared` is passed to this
call so this code works as intended.
This fixes the issue described in #4711 which was closed because
subscripts do not have the same behavior implemented. However we can
implement this for subscripts as well (I have an implementation ready),
it just changes the error message from the failed overloads of `[]=` to
the failed overloads of `[]` for the LHS, which might be misleading but
is consistent with the error messages for any other assignment. I can do
this in this PR or another one.
(cherry picked from commit 4d9e5e8b6d)
split from #24204, closes#7674
The `{.size.}` pragma no longer restricts the given size to 1, 2, 4 or 8
if it is used for an imported type. This is not tested very thoroughly
but there's no obvious reason to disallow it.
(cherry picked from commit 1ef9a656d2)
fixes#4554, fixes#10900, fixes#13843, fixes#19471, fixes#19517
Instead of matching generic converters to their arguments using the full
call match bindings, a new match is created for them (from which the
bindings are used to instantiate the converter return type). Then when
instantiating generic converters, they are matched to their argument
again to get their bindings again instead of using the call bindings.
This prevents generic converters which match more than once from
interfering with each other's bindings.
(cherry picked from commit 334f96c05a)
Implements #24569
Adds `--stdinfile` flag for specifying the file to use in place of
`stdinfile.nim` in error messages. Will enable easier integration of
tooling with nim check
(cherry picked from commit 0cba752c8a)
fixes#24484, fixes#24672
When an array, set or tuple constructor has an element that resolves to
`tyFromExpr`, the type of the entire literal is now set to `tyFromExpr`
and the subsequent elements are not matched to any type.
The remaining expressions are still typed (a version of the PR before
this called `semGenericStmt` on them instead), however elements with int
literal types have their types set to `nil`, since generic instantiation
removes int literal types and the int literal type is required for
implicitly converting the int literal element to the set type. Tuples
should not really need this but it is done for them anyway in case it
messes up some type inference
---------
Co-authored-by: Andreas Rumpf <rumpf_a@web.de>
(cherry picked from commit 897126a711)
Combining two small PRs in one here. The test case explains what was
wrong with the concepts and for naitivesockets, it's typical to adjust
`ai_flags` so I opened that up.
(cherry picked from commit d4098e6ca0)
Previously it would try to parse the semicolon as its own statement and
produce an `nkEmpty` node
Also more than 1 semicolon in an expression list i.e. `(a;; b)` gives an
"expression expected" error in `semiStmtList` when multiple semicolons
are allowed in normal statements, this could be fixed by changing the
`if tok.kind == tokSemicolon` check to a `while` but it does not match
the grammar so not done here.
(cherry picked from commit 918f972369)
fixes#24847
Object constructors call `fillObjectFields` when a field inside the
constructor does not have a location, however when the field is from a
base type this does not process it. Now `fillObjectFields` also calls
itself for the base type to fix this but not sure if this is a good
solution as `fillObjectFields` is used in other places too.
(cherry picked from commit a625fab098)
implements https://github.com/nim-lang/RFCs/issues/557
It inserts defect handing into a bare except branch
```nim
try:
raiseAssert "test"
except:
echo "nope"
```
=>
```nim
try:
raiseAssert "test"
except:
# New behaviov, now well-defined: **never** catches the assert, regardless of panic mode
raiseDefect()
echo "nope"
```
In this way, `except` still catches foreign exceptions, but panics on
`Defect`. Probably when Nim has `except {.foreign.}`, we can extend
`raiseDefect` to foreign exceptions as well. That's supposed to be a
small use case anyway.
`--legacy:noPanicOnExcept` is provided for a transition period.
(cherry picked from commit 26b86c8f4d)
fixes#24806
Blocks don't merge symbols that are used before destruction to the
parent scope, which causes `wasMoved; destroy` to elide incorrectly
(cherry picked from commit 73aeac81d1)
fixes#24801
Because distinct `seq` types match `proc `=destroy`*[T](x: var T)
{.inline, magic: "Destroy".}`. But the Nim compiler generates lifted seq
types for corresponding distinct types. So we skip the address for
distinct types.
Related to https://github.com/nim-lang/Nim/pull/22207 I had a hard time
finding the other place where generic destructors get replaced by
attachedDestructors
(cherry picked from commit 4352fa2ef0)
```nim
proc foo =
var x = "1234"
var y = x
when nimvm:
discard
else:
var s = x
doAssert s == "1234"
doAssert y == "1234"
static: foo()
foo()
```
`dfa` chooses the `nimvm` branch, `x` is misread as a last read and
`wasMoved`.
`injectDestructor` is used for codegen and is not used for vmgen. It's
reasonable to choose the codegen path instead of the `nimvm` path so the
code works for codegen. Though the problem is often hidden by
`cursorinference` or `optimizer`.
found in https://github.com/nim-lang/Nim/pull/24831
(cherry picked from commit 3617d2e077)
Script wasn't working on my machine with GDB 16.2
Main issues
- `gdb.types` wasn't imported, leading to import error on initial load
- dollar function didn't work with the new mangling scheme
Fixes them, also updates the test script to work with some new mangling
changes.
Test evidence
(cherry picked from commit e0a4876981)
Multiple replacements based on character sets in a single pass. Useful
for string sanitation. Follows existing `multiReplace` semantics.
Note: initially copied the substring version logic with a `while` and a
named block break, but Godbolt showed it had produced slightly larger
assembly using higher registers than the final version.
- [x] Tests
- [x] changelog.md
(cherry picked from commit 909f3b8b79)
closes#24780
This proc `genStmt` is only called to run the VM in `vm.evalStmt`,
otherwise it's not used in vmgen. Now it acts the same as `proc
gen(PCtx, PNode)`, used by `discard` statements, which just calls
`freeTemp` on the dest if it was set rather than erroring.
(cherry picked from commit fcba14707a)
fixes#24773
`c.inheritancePenalty` is supposed to be used for the entire match, but
in these places the inheritance penalty of a single argument overrides
the entire match penalty. The `+ ord(c.inheritancePenalty < 0)` is
copied from other places that use the same idiom, the intent is that the
existing penalty changes from -1 to 0 first to mark that it participates
in inheritance before adding the inheritance depth.
---------
Co-authored-by: Andreas Rumpf <araq4k@proton.me>
(cherry picked from commit fb93295344)
Otherwise, `sink T` is kept as it is. This PR treats sink types as its
base types for the arguments. So the concept would match both cases
Required by https://github.com/nim-lang/Nim/pull/24724
(cherry picked from commit 9ebfa7973a)
refs #24766
Detect when we track a call to a forward declaration without explicit
`raises` effects, then when the `raises` check fails for the proc, give
a hint that this forward declaration was tracked as potentially raising
any exception.
(cherry picked from commit 82891e6850)
motivating example:
```nim
iterator p(a: openArray[char]): int =
if a.len != 0:
if a[0] != '/':
discard
for t in p(""): discard
```
The compiler wants to evaluate `a[0]` at compile time even though it is
protected by the if statement above it. Similarly expressions like
`a.len != 0 and a[0] == '/'` have problems. It seems like the logic in
semfold needs to be more aware of branches to positively identify when
it is okay to fail compilation in these scenarios. It's a bit tough
though because it may be the case that non-constant expressions in
branching logic can properly protect some constant expressions.
(cherry picked from commit 850f327713)
fixes#24755, refs #24710
Instead of using the node from `indexTypesMatch` which inserts a hidden
conv node, just change the type of the node back to the old type
directly
(cherry picked from commit 38ad336c69)
fixes#24751
`typeof` leaves the object constructor as a call node for some reason,
in this case it tries to access the first child of the type node but the
object has no fields so the type field is empty. Alternatively the
optimizer can stop looking into `typeof`
(cherry picked from commit e2e7790779)
Without this fix, trying to use `scanTuple` in a generic proc imported
from a different module fails to compile (`undeclared identifier:
'scanf'`):
```nim
# module.nim
import std/strscans
proc scan*[T](s: string): (bool, string) =
s.scanTuple("$+")
```
```nim
# main.nim
import ./module
echo scan[int]("foo")
```
Workaround is to `export scanf` in `module.nim` or `import std/strscans`
in `main.nim`.
(cherry picked from commit f8294ce06e)
Yet another one of these. Multiple changes piled up in this one. I've
only minimally cleaned it for now (debug code is still here etc). Just
want to start putting this up so I might get feedback. I know this is a
lot and you all are busy with bigger things. As per my last PR, this
might just contain changes that are not ready.
### concept instantiation uniqueness
It has already been said that concepts like `ArrayLike[int]` is not
unique for each matching type of that concept. Likewise the compiler
needs to instantiate a new proc for each unique *bound* type not each
unique invocation of `ArrayLike`
### generic parameter bindings
Couple of things here. The code in sigmatch has to give it's bindings to
the code in concepts, else the information is lost in that step. The
code that prepares the generic variables bound in concepts was also
changed slightly. Net effect is that it works better.
I did choose to use the `LayedIdTable` instead of the `seq`s in
`concepts.nim`. This was mostly to avoid confusing myself. It also
avoids some unnecessary movings around. I wouldn't doubt this is
slightly less performant, but not much in the grand scheme of things and
I would prefer to keep things as easy to understand as possible for as
long as possible because this stuff can get confusing.
### various fixes in the matching logic
Certain forms of modifiers like `var` and generic types like
`tyGenericInst` and `tyGenericInvocation` have logic adjustments based
on my testing and usage
### signature matching method adjustment
This is the weird one, like my last PR. I thought a lot about the
feedback from my last attempt and this is what I came up with. Perhaps
unfortunately I am preoccupied with a slight grey area. consider the
follwing:
```nim
type
C1 = concept
proc p[T](s: Self; x: T)
C2[T] = concept
proc p(s: Self; x: T)
```
It would be temping to say that these are the same, but I don't think
they are. `C2` makes each invocation distinct, and this has important
implications in the type system. eg `C2[int]` is not the same type as
`C2[string]` and this means that signatures are meant to accept a type
that only matches `p` for a single type per unique binding. For `C1` all
are the same and the binding `p` accepts multiple types. There are
multiple variations of this type classes, `tyAnything` and the like.
The make things more complicated, an implementation might match:
```nim
type
A = object
C3 = concept
proc p(s: Self; x: A)
```
if the implementation defines:
```nim
proc p(x: Impl; y: object)
```
while a concept that fits `C2` may be satisfied by something like:
```nim
proc p(x: Impl; y: int)
proc spring[T](x: C2[T])
```
it just depends. None of this is really a problem, it just seems to
provoke some more logic in `concepts.nim` that makes all of this (appear
to?) work. The logic checks for both kinds of matches with a couple of
caveats. The fist is that some unbind-able arrangements may be matched
during overload resolution. I don't think this is avoidable and I
actually think this is a good way to get a failed compilation. So, first
note imo is that failing during binding is preferred to forcing the
programming to write annoying stub procs and putting insane gymnastics
in the compiler. Second thing is: I think this logic is way to accepting
for some parts of overload resolutions. Particularly in `checkGeneric`
when disambiguation is happening. Things get hard to understand for me
here. ~~I made it so the implicit bindings to not count during
disambiguation~~. I still need to test this more, but the thought is
that it would help curb excessive ambiguity errors.
Again, I'm sorry for this being so many changes. It's probably
inconvenient.
---------
Co-authored-by: Andreas Rumpf <rumpf_a@web.de>
(cherry picked from commit dfab30734b)
fixes#24743, refs #24718
We cannot do this in general for any expression with generic type
because the `typeof` logic is called for things like `type Foo` in:
```nim
type Foo[T] = object
proc init(_: type Foo) = discard
```
We also cannot use `containsUnresolvedType` to work around this specific
case because the base type of `static[auto]` is not unresolved, it is a
typeclass that isn't lifted to a parameter. The behavior of generating
`tyFromExpr` is also consistent with pre-2.0, so we do this in this
special case of `static`.
(cherry picked from commit 569d02e212)
fixes#24736
The VM can produce integer nodes with no types as set elements, which
are later reannotated in `semmacrosanity.annotateType`. However the case
of ranges was not handled properly. Not sure why this is a regression,
probably unrelated but will have to see the bisect result to make sure.
Note. Originally tried to fix this in `opcInclRange`, generated for and
only for range expressions in set constructors, this seems to add the
range node directly to the set node without checking if it has overlap
with the existing elements by calling `nimsets` so an expression like
`{cctNone, cctNone..cctHeader}` can produce `{0, 0..5}`. Doesn't seem to
cause problems but `opcIncl` for single elements does check for overlap.
Something else to note is that integer nodes produced by `nimsets` have
proper types, so another option instead of relying on semmacrosanity to
fix this would be to make `opcIncl` and `opcInclRange` call `nimsets` to
add to the set node, but this might lose performance.
(cherry picked from commit e39d152b89)
fixes#24705
```nim
proc xxx(v: static int) =
echo v
xxx(10)
xxx(20)
```
They are mangled as `_ZN14titaniummangle7xxx_s10E` and
`_ZN14titaniummangle7xxx_s20E` with `--debugger:native`. Static
parameters are prefixed with `_s` to distinguish simple cases like
`xxx(10, 15)` and `xxx(101, 5)` if `xxx` supports two `static[int]`
parameters
(cherry picked from commit c452275e29)
fixes#24698
The same aim as #24224 but for tuple constructors. The difference here
is that the type of a tuple constructor is always going to be valid
unlike array constructors which can have `seq` etc types, so we can just
generate a conversion again. If the conversion fails, it is ignored
similar to #24611, this is to protect against modified typed nodes in
macros.
Also #24611 was only adapted to `semTupleFieldsConstr` and not
`semTuplePositionsConstr`, this is now fixed.
(cherry picked from commit 49dfc3a0d4)
fixes#24715
In generic typechecking, unresolved static param symbols (i.e.
`skGenericParam`) have [the static type
itself](1f8da3835f/compiler/semexprs.nim (L1483-L1485))
as their type when used in an expression. This is not the case when the
static param is resolved (the type is wrapped in static when necessary),
but semchecking of types and generic typechecking expects the type of
the value to be wrapped in `static` (at least `array[N, int]` breaks).
So for now, to solve the issue, `typeof` just skips static types.
(cherry picked from commit 514a25c9a2)
fixes#24725
`lacksMTypeField` doesn't take the base types into consideration. And
for ` {.inheritable, pure.}`, it shouldn't generate a `m_type` field.
(cherry picked from commit e449813c61)
