Commit Graph

197 Commits

Author SHA1 Message Date
metagn
821d0806fe Revert "make default values typed in proc AST same as param sym AST" (#24191)
Reverts #24184, reopens #12942, reopens #19118

#24184 seems to have caused a regression in
https://github.com/c-blake/thes and
https://github.com/c-blake/bu/blob/main/rp.nim#L84 reproducible with
`git clone https://github.com/c-blake/cligen; git clone
https://github.com/c-blake/thes; cd thes; nim c -p=../cligen thes`.
Changing the `const` to `let` makes it compile.

A minimization that is probably the same issue is:

```nim
const a: seq[string] = @[]

proc foo(x = a) =
  echo typeof(x)
  echo x

import macros

macro resemFoo() =
  result = getImpl(bindSym"foo")

block:
  resemFoo() # Error: cannot infer the type of parameter 'x'
```

This should be a regression test in a future reimplementation of #24184.
2024-09-27 15:34:09 +02:00
metagn
c21bf7f41b make default values typed in proc AST same as param sym AST (#24184)
fixes #12942, fixes #19118

This is the exact same as #20735 but maybe the situation has improved
after #24065.
2024-09-27 09:33:40 +02:00
metagn
fd379c2f94 fix nimsuggest crash with arrow type sugar (#24185)
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.
2024-09-27 06:23:29 +02:00
metagn
ff005ad7dc fix segfault in generic param mismatch error, skip typedesc (#24140)
refs #24010, refs
https://github.com/nim-lang/Nim/issues/24125#issuecomment-2358377076

The generic mismatch errors added in #24010 made it possible for `nArg`
to be `nil` in the error reporting since it checked the call argument
list, not the generic parameter list for the mismatching argument node,
which causes a segfault. This is fixed by checking the generic parameter
list immediately on any generic mismatch error.

Also the `typedesc` type is skipped for the value of the generic params
since it's redundant and the generic parameter constraints don't have
it.
2024-09-19 07:19:07 +02:00
metagn
1660ddf98a make var/pointer types not match if base type has to be converted (#24130)
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.
2024-09-18 17:37:18 +02:00
metagn
f223f016f3 show symchoices as ambiguous in overload type mismatches (#24077)
fixes #23397

All ambiguous symbols generate symchoices for call arguments since
#23123. So, if a type mismatch receives a symchoice node for an
argument, we now treat it as an ambiguous identifier and list the
ambiguous symbols in the error message.
2024-09-09 09:50:45 +02:00
metagn
4a548deb08 proper errors for subscript overloads (#24068)
The magic `mArrGet`/`mArrPut` subscript overloads always match, so if a
subscript doesn't match any other subscript overloads and isn't a
regular language-handled subscript, it creates a fake overload mismatch
error in `semArrGet` that doesn't have any information (gives stuff like
"first mismatch at index: 0" for every single mismatch). Instead of
generating the fake mismatches, we only generate the fake mismatch for
`mArrGet`/`mArrPut`, and process every overload except them as a real
call and get the errors from there.
2024-09-06 11:25:51 +02:00
metagn
71de7fca9e handle explicit generic routine instantiations in sigmatch (#24010)
fixes #16376

The way the compiler handled generic proc instantiations in calls (like
`foo[int](...)`) up to this point was to instantiate `foo[int]`, create
a symbol for the instantiated proc (or a symchoice for multiple procs
excluding ones with mismatching generic param counts), then perform
overload resolution on this symbol/symchoice. The exception to this was
when the called symbol was already a symchoice node, in which case it
wasn't instantiated and overloading was called directly ([these
lines](b7b1313d21/compiler/semexprs.nim (L3366-L3371))).

This has several problems:

* Templates and macros can't create instantiated symbols, so they
couldn't participate in overloaded explicit generic instantiations,
causing the issue #16376.
* Every single proc that can be instantiated with the given generic
params is fully instantiated including the body. #9997 is about this but
isn't fixed here since the instantiation isn't in a call.

The way overload resolution handles explicit instantiations by itself is
also buggy:

* It doesn't check constraints.
* It allows only partially providing the generic parameters, which makes
sense for implicit generics, but can cause ambiguity in overloading.

Here is how this PR deals with these problems:

* Overload resolution now always handles explicit generic instantiations
in calls, in `initCandidate`, as long as the symbol resolves to a
routine symbol.
* Overload resolution now checks the generic params for constraints and
correct parameter count (ignoring implicit params). If these don't
match, the entire overload is considered as not matching and not
instantiated.
* Special error messages are added for mismatching/missing/extra generic
params. This is almost all of the diff in `semcall`.
* Procs with matching generic parameters now instantiate only the type
of the signature in overload resolution, not the proc itself, which also
works for templates and macros.

Unfortunately we can't entirely remove instantiations because overload
resolution can't handle some cases with uninstantiated types even though
it's resolved in the binding (see the last 2 blocks in
`texplicitgenerics`). There are also some instantiation issues with
default params that #24005 didn't fix but I didn't want this to become
the 3rd huge generics PR in a row so I didn't dive too deep into trying
to fix them. There is still a minor instantiation fix in `semtypinst`
though for subscripts in calls.

Additional changes:

* Overloading of `[]` wasn't documented properly, it somewhat is now
because we need to mention the limitation that it can't be done for
generic procs/types.
* Tests can now enable the new type mismatch errors with just
`-d:testsConciseTypeMismatch` in the command.

Package PRs:

- using fork for now:
[combparser](https://github.com/PMunch/combparser/pull/7) (partial
generic instantiation)
- merged: [cligen](https://github.com/c-blake/cligen/pull/233) (partial
generic instantiation but non-overloaded + template)
- merged: [neo](https://github.com/andreaferretti/neo/pull/56) (trying
to instantiate template with no generic param)
2024-09-02 18:22:20 +02:00
metagn
34719cad9d allow untyped arguments to fail to compile in overload mismatch error (#23984)
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.
2024-08-20 11:43:11 +02:00
metagn
cd946084ab make routine implicitly gensym when other gensym symbol exists again (#23842)
fixes #23813, partially reverts #23392

Before #23392, if a `gensym` symbol was defined before a proc with the
same name in a template even with an `inject` annotation, the proc would
be `gensym`. After #23392 the proc was instead changed to be `inject` as
long as no `gensym` annotation was given. Now, to keep compatibility
with the old behavior, the behavior is changed back to infer the proc as
`gensym` when no `inject` annotation is given, however an explicit
`inject` annotation will still inject the proc. This is also documented
in the manual as the old behavior was undocumented and the new behavior
is slightly different.
2024-07-16 08:47:06 +02:00
ringabout
173b8a8c58 fixes #3011; handles meta fields defined in the ref object (#23818)
fixes #3011

In https://github.com/nim-lang/Nim/pull/23532, meta fields that defined
in the object are handled.

In this PR, RefObjectTy is handled as well:
```nim
type
  Type = ref object 
    context: ref object
```
Ref alias won't trigger mata fields checking so there won't have
cascaded errors on `TypeBase`.

```nim
type
  TypeBase = object 
    context: ref object
  Type = ref TypeBase 
    context: ref object
```
2024-07-11 15:39:44 +02:00
ringabout
8f5ae28fab fixes #22672; Destructor not called for result when exception is thrown (#23267)
fixes #22672
2024-06-06 11:51:41 +02:00
ringabout
185e06c923 fixes #23419; internal error with void in generic array instantiation (#23550)
fixes #23419

`void` is only supported as fields of objects/tuples. It shouldn't allow
void in the array.

I didn't merge it with taField because that flag is also used for
tyLent, which is allowed in the fields of other types.
2024-05-01 09:02:43 +02:00
ringabout
f682dabf71 fixes #23531; fixes invalid meta type accepted in the object fields (#23532)
fixes #23531
fixes #19546
fixes #6982
2024-04-26 16:05:03 +02:00
ringabout
0b0f185bd1 fixes #23536; Stack trace with wrong line number when the proc called inside for loop (#23540)
fixes #23536
2024-04-26 16:02:02 +02:00
ringabout
229c125d2f workaround #23435; real fix pending #23279 (#23436)
workaround #23435

related to https://github.com/nim-lang/Nim/issues/22852

see also #23279
2024-04-18 21:55:26 +02:00
metagn
73b0b0d31c stop gensym identifiers hijacking routine decl names in templates (#23392)
fixes #23326

In a routine declaration node in a template, if the routine is marked as
`gensym`, the compiler adds it as a new symbol to a preliminary scope of
the template. If it's not marked as gensym, then it searches the
preliminary scope of the template for the name of the routine, then when
it matches a template parameter or a gensym identifier, the compiler
replaces the name node with a symbol node of the found symbol.

This makes sense for the template parameter since it has to be replaced
later, but not really for the gensym identifier, as it doesn't allow us
to inject a routine with the same name as an identifier previously
declared as gensym (the problem in #23326 is when this is in another
`when` branch).

However this is the only channel to reuse a gensym symbol in a
declaration, so maybe removing it has side effects. For example if we
have:

```nim
proc foo(x: int) {.gensym.} = discard
proc foo(x: float) {.gensym.} = discard
```

it will not behave the same as

```nim
proc foo(x: int) {.gensym.} = discard
proc foo(x: float) = discard
```

behaved previously, which maybe allowed overloading over the gensym'd
symbols.

A note to the "undeclared identifier" error message has also been added
for a potential error code that implicitly depended on the old behavior
might give, namely ``undeclared identifier: 'abc`gensym123'``, which
happens when in a template an identifier is first declared gensym in
code that doesn't compile, then as a routine which injects by default,
then the identifier is used.
2024-04-09 14:37:34 +02:00
ringabout
320311182c fixes #22284; fixes #22282; don't override original parameters of inferred lambdas (#23368)
fixes #22284
fixes #22282


```
Error: j(uRef, proc (config: F; sources: auto) {.raises: [].} = discard ) can raise an unlisted exception: Exception
```


The problem is that `n.typ.n` contains the effectList which shouldn't
appear in the parameter of a function defintion. We could not simply use
`n.typ.n` as `n[paramsPos]`. The effect lists should be stripped away
anyway.
2024-03-09 11:42:15 +01:00
metagn
f46f26e79a don't use previous bindings of auto for routine return types (#23207)
fixes #23200, fixes #18866

#21065 made it so `auto` proc return types remained as `tyAnything` and
not turned to `tyUntyped`. This had the side effect that anything
previously bound to `tyAnything` in the proc type match was then bound
to the proc return type, which is wrong since we don't know the proc
return type even if we know the expected parameter types (`tyUntyped`
also [does not care about its previous bindings in
`typeRel`](ab4278d217/compiler/sigmatch.nim (L1059-L1061))
maybe for this reason).

Now we mark `tyAnything` return types for routines as `tfRetType` [as
done for other meta return
types](18b5fb256d/compiler/semtypes.nim (L1451)),
and ignore bindings to `tyAnything` + `tfRetType` types in `semtypinst`.
On top of this, we reset the type relation in `paramTypesMatch` only
after creating the instantiation (instead of trusting
`isInferred`/`isInferredConvertible` before creating the instantiation),
using the same mechanism that `isBothMetaConvertible` uses.

This fixes the issues as well as making the disabled t15386_2 test
introduced in #21065 work. As seen in the changes for the other tests,
the error messages give an obscure `proc (a: GenericParam): auto` now,
but it does give the correct error that the overload doesn't match
instead of matching the overload pre-emptively and expecting a specific
return type.

tsugar had to be changed due to #16906, which is the problem where
`void` is not inferred in the case where `result` was never touched.
2024-01-17 11:59:54 +01:00
ringabout
ab4278d217 fixes #23180; fixes #19805; prohibits invalid tuple unpacking code in for loop (#23185)
fixes #23180
fixes #19805
2024-01-13 14:09:34 +01:00
metagn
e8092a5470 delay resolved procvar check for proc params + acknowledge unresolved statics (#23188)
fixes #23186

As explained in #23186, generics can transform `genericProc[int]` into a
call `` `[]`(genericProc, int) `` which causes a problem when
`genericProc` is resemmed, since it is not a resolved generic proc. `[]`
needs unresolved generic procs since `mArrGet` also handles explicit
generic instantiations, so delay the resolved generic proc check to
`semFinishOperands` which is intentionally not called for `mArrGet`.

The root issue for
[t6137](https://github.com/nim-lang/Nim/blob/devel/tests/generics/t6137.nim)
is also fixed (because this change breaks it otherwise), the compiler
doesn't consider the possibility that an assigned generic param can be
an unresolved static value (note the line `if t.kind == tyStatic: s.ast
= t.n` below the change in sigmatch), now it properly errors that it
couldn't instantiate it as it would for a type param. ~~The change in
semtypinst is just for symmetry with the code above it which also gives
a `cannot instantiate` error, it may or may not be necessary/correct.~~
Now removed, I don't think it was correct.

Still possible that this has unintended consequences.
2024-01-11 07:45:11 +01:00
metagn
b280100499 ambiguous identifier resolution (#23123)
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`
2024-01-01 12:21:19 +01:00
ringabout
7e1ea50bc3 fixes #23060; editDistance wrongly compare the length of rune strings (#23062)
fixes #23060
2023-12-13 10:34:41 +01:00
ringabout
795aad4f2a fixes #22996; typeAllowedCheck for default fields (#22998)
fixes #22996
2023-11-29 10:35:50 +01:00
ringabout
642ac0c1c3 fixes #22753; Nimsuggest segfault with invalid assignment to table (#22781)
fixes #22753

## Future work
We should turn all the error nodes into nodes of a nkError kind, which
could be a industrious task. But perhaps we can add a special treatment
for error nodes to make the transition smooth.
2023-10-02 14:45:04 -04:00
ringabout
a7a0cfe8eb fixes #10542; suppresses varargs conversion warnings (#22757)
fixes #10542
revives and close #20169
2023-09-26 14:05:18 +02:00
metagn
5f9038a5d7 make expressions opt in to symchoices (#22716)
refs #22605

Sym choice nodes are now only allowed to pass through semchecking if
contexts ask for them to (with `efAllowSymChoice`). Otherwise they are
resolved or treated as ambiguous. The contexts that can receive
symchoices in this PR are:

* Call operands and addresses and emulations of such, which will subject
them to overload resolution which will resolve them or fail.
* Type conversion operands only for routine symchoices for type
disambiguation syntax (like `(proc (x: int): int)(foo)`), which will
resolve them or fail.
* Proc parameter default values both at the declaration and during
generic instantiation, which undergo type narrowing and so will resolve
them or fail.

This means unless these contexts mess up sym choice nodes should never
leave the semchecking stage. This serves as a blueprint for future
improvements to intermediate symbol resolution.

Some tangential changes are also in this PR:

1. The `AmbiguousEnum` hint is removed, it was always disabled by
default and since #22606 it only started getting emitted after the
symchoice was soundly resolved.
2. Proc setter syntax (`a.b = c` becoming `` `b=`(a, c) ``) used to
fully type check the RHS before passing the transformed call node to
proc overloading. Now it just passes the original node directly so proc
overloading can deal with its typechecking.
2023-09-18 06:39:22 +02:00
metagn
ba158d73dc type annotations for variable tuple unpacking, better error messages (#22611)
* type annotations for variable tuple unpacking, better error messages

closes #17989, closes https://github.com/nim-lang/RFCs/issues/339

* update grammar

* fix test
2023-09-01 06:26:53 +02:00
metagn
53d43e9671 round out tuple unpacking assignment, support underscores (#22537)
* round out tuple unpacking assignment, support underscores

fixes #18710

* fix test messages

* use discard instead of continue

Co-authored-by: Andreas Rumpf <rumpf_a@web.de>

---------

Co-authored-by: Andreas Rumpf <rumpf_a@web.de>
2023-08-24 06:11:48 +02:00
ringabout
1aff402998 fixes #6499; disallow built-in procs used as procvars (#22291) 2023-07-19 09:45:28 +02:00
ringabout
77beb15214 fixes #22049; fixes #22054; implicit conversion keeps varness (#22097)
* fixes #22054; codegen for var tuples conv

* rethink fixes

* add test cases

* templates only

* fixes var tuples

* keep varness no matter what

* fixes typ.isNil

* make it work for generics

* restore isSubrange

* add a test case as requested
2023-06-16 12:06:50 +02:00
metagn
b97d603cd0 some test cleanups & category reorganization (#22010)
* clean up some test categories

* mention exact slice issue

* magics into system

* move trangechecks into overflow

* move tmemory to system

* try fix CI

* try fix CI

* final CI fix
2023-06-06 06:54:07 +02:00
metagn
0a212f97a5 properly disallow unresolved generic proc values (#22005)
* properly disallow unresolved generic proc values

* mirrors semoperand

* shallow efTypeAllowed, add back special case
2023-06-05 10:53:40 +02:00
metagn
02be212dae clean up SOME pending/xxx/issue link comments (#21826)
* clean up SOME pending/xxx/issue link comments

* great
2023-05-11 10:23:52 +02:00
metagn
c2bcfd8cd9 cheap fix for #10853 + better tuple subscript error message (#21767)
* cheap fix for #10853

* also better tuple subscript error message

* weird
2023-05-02 11:13:38 +02:00
metagn
63d29ddd69 alias syntax fixes, improvements and tests (#21671)
* alias syntax fixes, improvements and tests

* even better, cannot use alias syntax with generics

* more type tests, improve comment

* fix again

* consistent error message + make t5167_5 work

* more comments, remove {.noalias.}
2023-04-22 09:11:56 +02:00
Andreas Rumpf
d5719c47dc make --exceptions:quirky work with C++ (#21581)
* make --exceptions:quirky work with C++

* make tests green again
2023-03-31 04:16:09 +02:00
Andreas Rumpf
72ca444122 hopefully easier to understand error message (#21585) 2023-03-30 15:35:00 +02:00
ringabout
4d76725299 closes #16654; add a test case (#21478) 2023-03-06 14:12:47 +08:00
Anna
070938720a tests/errmsgs/tcall_with_default_arg.nim: sync (#21237) 2023-03-01 21:29:23 +01:00
Anna
cf083af262 tests: explicitly enable stack traces where needed (#21236)
* tests/assert/tassert_c.nim: explicitly enable stack traces

* tests/errmsgs: explicitly enable stack traces
2023-03-01 18:01:58 +01:00
ringabout
38f876dd48 fixes #19795; fixes #11852; fixes #19974; remove parsing pipeline, Nim now parses the whole module at one time (#21379)
* fixes #19795; remove parse pipeline

* isScript

* fixes nimscriptapi

* don't touch reorder

* check script

* fixes tests

* it seems implicit imports of system cause troubles

* access the first child of `nkStmtList`

* ignore comments

* minor messages

* perhaps increases hloLoopDetector

* the module is a stmtList, which changes the errors

* fixes nimdoc

* fixes tlinter

* fixes nim  secret tests

* fixes arc_misc

* fixes nim secret tests again

* safe; fixes one more test

* GlobalError is the root cause too

* fixes parsing errors

* put emit types to the cfsForwardTypes section

* fixes #11852; `{.push checks:off}` now works in procs

* disable navigator

* fixes nimdoc

* add tests for JS

* fixes nimsuggest
2023-02-22 20:34:20 +01:00
metagn
ac982d8a96 close #21257 (#21275)
* close #21257

* fix generics
2023-01-18 21:40:00 +01:00
ringabout
ebd1c678be fixes #21195; std/assertions continue to use sysFatal when nimPreviewSlimSystem is not defined (#21196)
* fixes #21195; `std/assertions` continue to use `sysFatal`

* try includes

* make `std/assertions` self-contained

* fixes tests

* fixes tests
2022-12-29 14:15:08 -05:00
ringabout
705da9d452 fixes #14444; add genLineDir before assignment (#21201)
* fixes #14444; add `genLineDir` before raises

* add a test case

* fixes differently
2022-12-29 17:06:57 +01:00
ringabout
761c5a0830 less verbose type mismatch messages (#21191)
* less verbose type mismatch messages

* Update compiler/types.nim

* fixes i386

* fixes i386
2022-12-28 20:54:15 +01:00
metagn
5684093135 fix for bad error message with const in case statement (#21182)
* preliminary fix for bad error message with const

* add test case

* fix tmatrixconcept and tmatrixlib
2022-12-27 21:24:00 +01:00
metagn
4ca2dcb404 Named arguments in commands + many grammar fixes (#20994)
* Breaking parser changes, implement https://github.com/nim-lang/RFCs/issues/442

Types are separated from expressions and better reflected in the grammar.

* add test

* more accurate grammar

* fix keyword typedescs

* accept expressions in proc argument lists

* CI "fixes"

* fixes

* allow full ref expressions again, adapt old tests

* cleanup, fix some tests

* improve grammar, try and revert semtypes change

* restrict sigil binding to identOrLiteral

* fix, should have caught this immediately

* add changelog entry, fix double not nil bug

* correct grammar

* change section

* fix

* real fix hopefully

* fix test

* support LL(1) for tuples

* make grammar.txt too
2022-12-06 13:11:56 +01:00
ringabout
12a20b9fb6 revert #20719; relieve std/assertions of the sysFatal dep (#20743)
* Revert "make `system/fatal` importable (#20718)"

This reverts commit d735c447d3.

* relieve `std/assertions` of the sysFatal dep
2022-11-04 09:53:19 +01:00
Bung
eec1543baf fix semcase on tySequence and tyObject #20283 #19682 (#20339)
* fix semcase on tySequence and tyObject #20283 #19682

* use better arg name

* avoiding returns nil use errorNode instead, clean code

* use efNoDiagnostics flag

* remove tests/errmsgs/t19682.nim

* combine 2 test cases to one file
2022-11-01 10:19:37 +01:00