Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

invalid C++ codegen in --mm:refc with Thread/Atomic/createThread #24159

Open
tersec opened this issue Sep 23, 2024 · 1 comment · May be fixed by #24204
Open

invalid C++ codegen in --mm:refc with Thread/Atomic/createThread #24159

tersec opened this issue Sep 23, 2024 · 1 comment · May be fixed by #24204

Comments

@tersec
Copy link
Contributor

tersec commented Sep 23, 2024

Description

import std/atomics
type N = object
  u: ptr Atomic[int]
proc w(args: N) = discard
var e: Thread[N]
createThread(e, w, default(N))

compile with nim cpp --mm:refc r.nim

Nim Version

v2.0.8:

Nim Compiler Version 2.0.8 [Linux: amd64]
Compiled at 2024-09-23
Copyright (c) 2006-2023 by Andreas Rumpf

git hash: 5935c3bfa9fec6505394867b23510eb5cbab3dbf
active boot switches: -d:release

version-2-0:

Nim Compiler Version 2.0.9 [Linux: amd64]
Compiled at 2024-09-23
Copyright (c) 2006-2023 by Andreas Rumpf

git hash: 27381cc60213e19aa34664176bd358ca5e45bd5a
active boot switches: -d:release

version-2-2:

Nim Compiler Version 2.1.99 [Linux: amd64]
Compiled at 2024-09-23
Copyright (c) 2006-2024 by Andreas Rumpf

git hash: 755307be61e4ee7b32c8354b2c303d04bdfc3a3e
active boot switches: -d:release

devel:

Nim Compiler Version 2.1.99 [Linux: amd64]
Compiled at 2024-09-23
Copyright (c) 2006-2024 by Andreas Rumpf

git hash: a55c15c651b805c5eca4475f9845a57adf6cddef
active boot switches: -d:release

Current Output

In file included from /usr/include/strings.h:23,
                 from /usr/include/string.h:462,
                 from .cache/nim/r_d/@mr.nim.cpp:10:
.cache/nim/r_d/@mr.nim.cpp: In function ‘void atmrdotnim_DatInit000()’:
.cache/nim/r_d/@mr.nim.cpp:207:82: error: ‘TY__HdySGEQ1I0yaP9bnrEvY9bpQ’ {aka ‘struct std::atomic<long int>’} has no member named ‘raw’
  207 | TM__O9ck8QE34G7SWmidGMuOitg_0[1].offset = offsetof(TY__HdySGEQ1I0yaP9bnrEvY9bpQ, raw);
      |                                                                                  ^~~
Error: execution of an external compiler program 'g++ -c -std=gnu++17 -funsigned-char  -w -fmax-errors=3 -fpermissive -pthread   -I/home/user/nimdevel/lib -I/tmp -o .cache/nim/r_d/@mr.nim.cpp.o .cache/nim/r_d/@mr.nim.cpp' failed with exit code: 1

Expected Output

No invalid C++ codegen

Known Workarounds

No response

Additional Information

No response

@metagn
Copy link
Collaborator

metagn commented Sep 23, 2024

This raw field has caused so much grief. If the only thing that needed it was #12726 size: sizeof(T) should have been made to work instead.

For this issue though the problem is that we're generating typeinfo when this is an imported C++ type here.

Edit: Issue was predicted here

Yeah size: sizeof(T) should be implemented but the compiler structure isn't ready for it yet. I'll PR a "fix" for this and work on that separately.

metagn added a commit to metagn/Nim that referenced this issue Sep 30, 2024
metagn added a commit to metagn/Nim that referenced this issue Sep 30, 2024
metagn added a commit to metagn/Nim that referenced this issue Sep 30, 2024
Araq pushed a commit that referenced this issue Sep 30, 2024
refs #24200 (comment)

Workaround for C++ Atomic[T] issues that doesn't require a compiler
change. Not tested or documented in case it's not meant to be officially
supported, locally tested `tatomics` and #24159 to work with it though,
can add these as tests if required.
narimiran pushed a commit that referenced this issue Sep 30, 2024
refs #24200 (comment)

Workaround for C++ Atomic[T] issues that doesn't require a compiler
change. Not tested or documented in case it's not meant to be officially
supported, locally tested `tatomics` and #24159 to work with it though,
can add these as tests if required.

(cherry picked from commit febc58e)
metagn added a commit to metagn/Nim that referenced this issue Oct 13, 2024
metagn added a commit to metagn/Nim that referenced this issue Oct 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants