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.
Because of the bug in `tools/parse_unicodedata.nim`, CJK Ideographs were
not considered letters in `isAlpha()`, even though they have category
Lo. This is because they are specified as range in `UnicodeData.txt`,
not as separate characters:
```
4E00;<CJK Ideograph, First>;Lo;0;L;;;;;N;;;;;
9FEF;<CJK Ideograph, Last>;Lo;0;L;;;;;N;;;;;
```
The parser was not prepared to parse such ranges and thus omitted almost
all CJK Ideographs from consideration.
To fix this, we need to consider ranges from `UnicodeData.txt` in
`tools/parse_unicodedata.nim`.
This adds a version of `almostEqual` (which was already available for
floats) thata works with `Complex[SomeFloat]`.
Proof that this is needed is that the first thing that the complex.nim
runnable examples block did before this commit was define (an
incomplete) `almostEqual` function that worked with complex values.
## Bug
Fixes https://github.com/nim-lang/Nim/issues/12381 - HttpClient socket
handle leak
To replicate the bug, run the following code in a loop:
```nim
import httpclient
while true:
echo "New loop"
var client = newHttpClient(timeout = 1000)
try:
let response = client.request("http://10.44.0.4/bla", httpMethod = HttpPost, body = "boo")
echo "HTTP " & $response.status
except CatchableError as e:
echo "Error sending logs: " & $e.msg
finally:
echo "Finally"
client.close()
```
Note the IP address as the hostname. I'm directly connecting to a
plausible local IP, but one that does not resolve, as I have everything
under 10.4.x.x.
The output looks like this to me:
```
New loop
Error sending logs: Operation timed out
Finally
New loop
Error sending logs: Operation timed out
Finally
New loop
...
```
In Nim 2.0.4, running the code above leaks the socket:
<img width="944" alt="Screenshot 2024-05-05 at 22 00 13"
src="https://github.com/nim-lang/Nim/assets/53387/ddac67db-d7df-45e6-b7a5-3d42f79775ea">
## Fix
With the added line of code, each old socket is cleanly removed:
<img width="938" alt="Screenshot 2024-05-05 at 21 54 18"
src="https://github.com/nim-lang/Nim/assets/53387/5b0b4b2d-d4f0-4e74-a9cf-74aec0c50d2e">
I believe the line below, `closeUnusedFds(ord(domain))` was supposed to
clean up the failed connection attempts, but it failed to do so for the
last one, assuming it succeeded. Yet it didn't. This fix makes sure
failed connections are closed immediately.
## Tests
I don't have a test with this PR. When testing locally, the
`connect(lastFd, ..)` call on line 2032 blocks for ~75 seconds, ignoring
the http timeout. I fear any test I could add would either 1) take way
too long, 2) one day run in an environment where my randomly chosen IP
is real, yielding in weird flakes.
The only bug i can imagine is if running `lastFd.close()` twice is a bad
idea. I tested by actually running it twice, and... no crash/op? So
seems safe? I'm hoping the CI run will be green, and this will be
enough. However I'm happy to take feedback on how I should test this,
and do the necessary changes.
~Edit: looks like a test does fail, so moving to a draft while I figure
this out.~ Attempt 2 fixed it.
`reset`, `wasMoved` and `move` doesn't support primitive types, which
generate `null` for these types. It is now produce `x = default(...)` in
the backend. Ideally it should be done by ast2ir in the future
fixes#4695
ref https://github.com/nim-lang/Nim/pull/15818
Since `nkState` is only for the main loop state labels and `nkGotoState`
is used only for dispatching the `:state` (since
https://github.com/nim-lang/Nim/pull/7770), it's feasible to rewrite the
loop body into a single case-based dispatcher, which enables support for
JS, VM backend. `nkState` Node is replaced by a label and Node pair and
`nkGotoState` is only used for intermediary processing. Backends only
need to implement `nkBreakState` and `closureIterSetupExc` to support
closure iterators.
pending https://github.com/nim-lang/Nim/pull/23484
<del> I also observed some performance boost for C backend in the
release mode (not in the danger mode though, I suppose the old
implementation is optimized into computed goto in the danger mode)
</del>
allPathsAsgnResult???
Because `isGitRepo()` call requires `/bin/sh` it will always fail when
building Nim in a Nix build sandbox, and the check doesn't even make
sense if Nix already provides Nimble source code.
Since for Nimble `allowBundled` is set to `true` this effectlvely does
not change behavior for normal builds, but does avoid ugly hacks when
building in Nix which lacks `/bin/sh` and fails to call `git`.
Reference:
*
https://github.com/status-im/nimbus-eth2/pull/6180#discussion_r1570237858
Signed-off-by: Jakub Sokołowski <jakub@status.im>
Added a second example inside the `typedthreads` file.
Also, add a more detailed introduction. When Nim is one's first
programming language, a short explanation of what a thread is might not
hurt.
For reference, the thread documentation of other languages looks like
this:
- https://en.cppreference.com/w/cpp/thread/thread
- https://doc.rust-lang.org/std/thread/
The documentation of a module is the first place one will look when
using a standard library feature, so I think it is important to have a
few usable examples for the main modules.
This is the example added
```nim
import locks
var l: Lock
proc threadFunc(obj: ptr seq[int]) {.thread.} =
withLock l:
for i in 0..<100:
obj[].add(obj[].len * obj[].len)
proc threadHandler() =
var thr: array[0..4, Thread[ptr seq[int]]]
var s = newSeq[int]()
for i in 0..high(thr):
createThread(thr[i], threadFunc, s.addr)
joinThreads(thr)
echo s
initLock(l)
threadHandler()
deinitLock(l)
```
Sharing memory between threads is very very common, so I think having an
example showcasing this is crucial.
---------
Co-authored-by: Andreas Rumpf <rumpf_a@web.de>
fixes#16771
follow up https://github.com/nim-lang/Nim/pull/16536
Ideally it should be handled in the IR part in the future
I have also checked the double evaluation of `swap` in the JS runtime
https://github.com/nim-lang/Nim/issues/16779, that might be solved by a
copy flag or something. Well, it should be best solved in the IR so that
it doesn't bother backends anymore.
## Reprodution
if on Windows:
```Nim
when defined(windows):
var file: File
let succ = file.open(<aFileHandle>)
```
then `succ` will be false.
If tested, it can be found to fail with errno `22` and message: `Invalid
argument`
## Problem
After some investigations and tests,
I found it's due to the `mode` argument for `fdopen`.
Currently `NoInheritFlag`(`'N'` in Windows) is added to `mode` arg
passed to `_fdopen`, but if referring to
[Windows `_fdopen`
doc](https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/fdopen-wfdopen?view=msvc-170),
you'll find there is no `'N'` describled. That's `'N'` is not accepted
by `_fdopen`.
Therefore, the demo above will fail.
## In Addition
To begin with, technologically speaking, when opening with a
`fileHandle`(or called `fd`), there is no concept of fd-inheritable as
`fd` is opened already.
In POSIX, `NoInheritFlag` is defined as `e`.
It's pointed out in [POSIX `open`
man-doc](https://www.man7.org/linux/man-pages/man3/fopen.3.html) that
`e` in mode is ignored for fdopen(),
which means `e` for `fdopen()` is not wanted, just allowed.
Therefore, better to also not pass `e` to `fdopen`
---
In all, that's this PR.
Fixes an issue that comes up when using strutils.`%` or any other
strutils/strformat feature that uses the unicode lookup tables behind
the scenes, on systems where ints are than 32-bit wide.
Tested with:
```bash
./koch test cat lib
```
Refer to the discussion in #23125.
The fix to the atomicArc looks to use `-1` as the check value from the
`SharedPtr` solution. However, it should be `-rcIncrement` since the
refcount is bit shifted in ARC/ORC.
I discovered this playing around doing atomic updates of refcounts in a
user library.
Related to https://github.com/nim-lang/Nim/issues/22711
@ringabout I believe you ported the sharedptr fix?
fix#23381
As for the read function, the original plan was to use lent for
annotation, but after my experiments, it still produced copies, so I had
to move it out.
Now the `read` function cannot be called repeatedly
When target is nodejs,
`isAbsolute` used to only check in the POSIX flavor,
i.e. for js backend on Windows,
```nim
isAbsolute(r"C:\Windows") == false
```
This fixes it.
For this
[proc](773c066634/lib/pure/browsers.nim (L83))
`proc openDefaultBrowser*() {.since: (1, 1).}`:
though it's documented to open default browser with `about:blank` page,
it behaves differently:
- On Windows, it failed and open no window
- On Linux(Debian with Kde), it opens not default browser but
`Konqueror`
I have paid much effort to implement this variant, but even the
implementation on Windows is considerably complex.
In short, it's not only hard but unworthy to fix this.
Just as Araq
[said](https://github.com/nim-lang/Nim/issues/22250#issuecomment-1631360617),
we shall remove the `proc openDefaultBrowser*() {.since: (1, 1).}`
variant
---------
Co-authored-by: ringabout <43030857+ringabout@users.noreply.github.com>
