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)
fixes#23587
As explained in the issue, `getOrDefault` has a parameter named
`default` that can be a proc after generic instantiation. But the
parameter having a proc type [overrides all other
overloads](f73e03b132/compiler/semexprs.nim (L1203))
including the magic `system.default` overload and causes a compile error
if the proc doesn't match the normal use of `default`. To fix this, the
`result = default(B)` initializer call is removed because it's not
needed, `result` is always set in `getOrDefaultImpl` when a default
value is provided.
This is still a suspicious behavior of the compiler but `tables` working
has a higher priority.
(cherry picked from commit 67ea754b7f)
fixes#18649, refs #24183
Same as in #24183 for templates, we now process pragma nodes in generics
so that macro symbols are captured and the pragma arguments are checked,
but ignoring language pragma keywords.
A difference is that we cannot process call nodes as is, we have to
process their children individually so that the early untyped
macro/template instantiation in generics does not kick in.
(cherry picked from commit d72b848d17)
fixes#24233
Integer literals with type `int` can match `int64` with a generic match.
Normally this would generate an conversion via `isFromIntLit`, but when
it matches with a generic match (`isGeneric`) the node is left alone and
continues to have type `int` (related to #4858, but separate; since
`isFromIntLit > isGeneric` it doesn't propagate). This did not cause
problems on the C backend up to this point because either the compiler
generated a cast when generating the C code or it was implicitly casted
in the C code itself. On the JS backend however, we need to generate
`int64` and `int` values differently, so we copy the integer literal and
give it the matched type now instead.
This is somewhat risky even if CI passes but it's required to make the
times module work without [this
workaround](7dfadb8b4e/lib/pure/times.nim (L219-L238))
on `--jsbigint64:on` (the default).
CI exposed an issue: When matching an int literal to a generic parameter
in a generic instantiation, the literal is only treated like a value if
it has `int literal` type, but if it has the type `int`, it gets
transformed into literally the type `int` (#12664, #13906), which breaks
the tests t14193 and t12938. To deal with this, we don't give it the
type `int` if we are in a generic instantiation and preserve the `int
literal` type.
(cherry picked from commit c73eedfe6e)
fixes#24186
When encountering pragma nodes in templates, if it's a language pragma,
we don't process the name, and only any values if they exist. If it's
not a language pragma, we process the full node. Previously only the
values of colon expressions were processed.
To make this simpler, `whichPragma` is patched to consider bracketed
hint/warning etc pragmas like `{.hint[HintName]: off.}` as being a
pragma of kind `wHint` rather than an invalid pragma which would have to
be checked separately. From looking at the uses of `whichPragma` this
doesn't seem like it would cause problems.
Generics have [the same
problem](a27542195c/compiler/semgnrc.nim (L619))
(causing #18649), but to make it work we need to make sure the
templates/macros don't get evaluated or get evaluated correctly (i.e.
passing the proc node as the final argument), either with #23094 or by
completely disabling template/macro evaluation when processing the
pragma node, which would also cover `{.pragma.}` templates.
(cherry picked from commit 911cef1621)
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)
split from #24198
This is a required refactor for the only good solution I've been able to
think of for #4858 etc. Explanation:
---
`sigmatch` currently [disables
bindings](d6a71a1067/compiler/sigmatch.nim (L1956))
(except for binding to other generic parameters) when matching against
constraints of generic parameters. This is so when the constraint is a
general metatype like `seq`, the type matching will not treat all
following uses of `seq` as the type matched against that generic
parameter.
However to solve #4858 etc we need to bind `or` types with a conversion
match to the type they are supposed to be converted to (i.e. matching
`int literal(123)` against `int8 | int16` should bind `int8`[^1], not
`int`). The generic parameter constraint binding needs some way to keep
track of this so that matching `int literal(123)` against `T: int8 |
int16` also binds `T` to `int8`[^1].
The only good way to do this IMO is to generate a new "binding context"
when matching against constraints, then binding the generic param to
what the constraint was bound to in that context (in #24198 this is
restricted to just `or` types & concrete types with convertible matches,
it doesn't work in general).
---
`semtypinst` already does something similar for bindings of generic
invocations using `LayeredIdTable`, so `LayeredIdTable` is now split
into its own module and used in `sigmatch` for type bindings as well,
rather than a single-layer `TypeMapping`. Other modules which act on
`sigmatch`'s binding map are also updated to use this type instead.
The type is also made into an `object` type rather than a `ref object`
to reduce the pointer indirection when embedding it inside
`TCandidate`/`TReplTypeVars`, but only on arc/orc since there are some
weird aliasing bugs on refc/markAndSweep that cause a segfault when
setting a layer to its previous layer. If we want we can also just
remove the conditional compilation altogether and always use `ref
object` at the cost of some performance.
[^1]: `int8` binding here and not `int16` might seem weird, since they
match equally well. But we need to resolve the ambiguity here, in #24012
I tested disallowing ambiguities like this and it broke many packages
that tries to match int literals to things like `int16 | uint16` or
`int8 | int16`. Instead of making these packages stop working I think
it's better we resolve the ambiguity with a rule like "the earliest `or`
branch with the best match, matches". This is the rule used in #24198.
(cherry picked from commit cad8726907)
fixes#18396, fixes#20142
Set types with base types matching less than a generic match (so
subrange matches, conversion matches, int conversion matches) are now
considered mismatching, as their representation is different on the
backends (except VM and JS), causing codegen issues. An exception is
granted for set literal types, which now implicitly convert each element
to the matched base type, so things like `s == {'a', 'b'}` are still
possible where `s` is `set[range['a'..'z']]`. Also every conversion
match in this case is unified under the normal "conversion" match, so a
literal doesn't match one set type better than the other, unless it's
equal.
However `{'a', 'b'} == s` or `{'a', 'b'} - s` etc is now not possible.
when it used to work in the VM. So this is somewhat breaking, and needs
a changelog entry.
(cherry picked from commit 7dfadb8b4e)
Previously, the compiler never differentiated between `untyped`/`typed`
argument default values and other types, it considered any parameter
with a type as typed and called `semExprWithType`, which both
typechecked it and disallowed `void` expressions. Now, we perform no
typechecking at all on `untyped` template param default values, and call
`semExpr` instead for `typed` params, which allows expressions with
`void` type.
(cherry picked from commit 4eed341ba5)
fixes#23010, split from #24195
When resemming bracket nodes, the compiler currently unconditionally
makes a new node with an array type based on the node. However the VM
can generate bracket nodes with `seq` types, which this erases. To fix
this, if a bracket node already has a type, we still resem the bracket
node, but don't construct a new type for it, instead using the type of
the original node.
A version of this was rejected that didn't resem the node at all if it
was typed, but I can't find it. The difference with this one is that the
individual elements are still resemmed.
This should fix the break caused by #24184 so we could redo it after
this PR but it might still have issues, not to mention the related
pre-existing issues like #22793, #12559 etc.
(cherry picked from commit d98ef312f0)
fixes#24008
The old logic didn't check the contents of the output executables, when
it switched release->debug->release, it picked up the Json files used in
the first release building, the content of which didn't change. So it
mistook the executables which are built by the second debug building as
the functioning one.
`changeDetectedViaJsonBuildInstructions` needs a way to distinguish the
executables generated by different buildings.
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.
fixes#22523
There were 2 problems with the code in `sameType` for
`dcEqIgnoreDistinct`:
1. The code that skipped `{tyDistinct, tyGenericInst}` only ran if the
given types had different kinds. This is fixed by always performing this
skip.
2. The code block below that checks if `tyGenericInst`s have different
values still ran for `dcEqIgnoreDistinct` since it checks if the given
types are generic insts, not the skipped types (and also only the 1st
given type). This is fixed by only invoking this block for `dcEq`;
`dcEqOrDistinctOf` (which is unused) also skips the first given type.
Arguably there is another issue here that `skipGenericAlias` only ever
skips 1 type.
These combined fix the issue (`T` is `GenericInst(V, 1, distinct int)`
and `D[0]` is `GenericInst(D, 0, distinct int)`).
#24037 shouldn't be a dependency but the diff follows it.
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.
based on #24127
Needs some tweaks to replace the other `struct` type generations, e.g.
seqs, maybe by exposing `BaseTypeKind` as a parameter. C++ and
codegenDecl etc seem like they are going to need attention.
Also `Builder` should really be `distinct string` that one has to call
`extract` on, but for this to be optimal in the current codegen, we
would need something like:
```nim
template buildInto(s: var string, builderName: untyped, body) =
template `builderName`: untyped = Builder(s)
body
buildInto(result, builder):
builder.add ...
```
but this could be a separate PR since it might not work with the
compiler. The possibly-not-optimal alternative is to do:
```nim
template build(builderName: untyped, body): string =
var `builderName` = Builder("")
body
extract(`builderName`)
result = build(builder):
builder.add ...
```
where the compiler maybe copies the built string but shouldn't.
---------
Co-authored-by: ringabout <43030857+ringabout@users.noreply.github.com>
fixes#19010
`getType` for proc types generated an `nkProcTy` for iterator types
instead of `nkIteratorTy`, and didn't generate a calling convention
pragma unless it was in the proc AST. Iterator types now generate
`nkIteratorTy`, and a calling convention pragma is added if the calling
convention isn't `closure` or was explicitly provided.
fixes#11422, refs #8336/#8333, refs #20130
The compiler generates conversion nodes *after* evaluating the branches
of case statements as constants, the reasoning is that case branches
accept constants of different types, like arrays or sets. But this means
that conversion nodes that need to be evaluated like converter calls
don't get evaluated as a constant for codegen. #8336 fixed this by
re-evaluating the node if an `nkHiddenCallConv` was created, and in
#20130 this logic also had to be added for `nkHiddenStdConv` for
cstrings. This logic was only for single case elements, it has now been
added to range elements as well to fix#11422. Additionally, all
conversion nodes are now evaluated for simplicity, but maybe this won't
pass CI.
fixes#19866 given #23997
When searching for a module-qualified symbol, `qualifiedLookUp` tries to
obtain the raw identifier from the RHS of the dot field. However it only
does this when the RHS is either an `nkIdent` or an `nkAccQuoted` node,
not when the node is a resolved symbol or a symchoice, such as in
templates and generics when the module symbol can't be resolved yet.
Since the LHS is a module symbol when the compiler checks for this, any
resolved symbol information doesn't matter, since it has to be a member
of the module. So we now obtain the identifier from these nodes as well
as the unresolved identifier nodes.
The test is a bit niche and possibly not officially supported, but this
is likely a more general problem and I just couldn't think of another
test that would be more "proper". It's better than the error message
`'a' has no type` at least.
fixes#24179
The original fix made it so calls to `skError`/`skUnknown` (in this case
`->`, for some reason `sugar` couldn't be imported) returned an error
node, however this breaks tsug_accquote for some reason I don't
understand (it even parses as `tsug_accquote.discard`) so I've just
added a guard based on the stacktrace.
fixes#19277, refs #24169, refs #18124
When pragmas are pushed to a routine, if the routine symbol AST isn't
nil by the time the pushed pragmas are being processed, the pragmas are
implicitly added to the symbol AST. However this is done without
restriction on the pragma, if the pushed pragma isn't supposed to apply
to the routine, it's still added to the routine. This is why the symbol
AST for templates wasn't set before the pushed pragma processing in
#18124. Now, the pragmas added to the AST are restricted to ones that
apply to the given routine. This means we can set the template symbol
AST earlier so that the pragmas get added to the template AST.
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.
When importing from subdirectories, the line info used in `UnusedImport`
warning would be the `/` node and not the actual module node. More
obvious with grouped imports where all unused imports would show the
same column
Fix is to just use the last child node for infixes when getting the line
info
