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

const cstring incorrectly cgen'd (pointer is copied, pointing to garbage) #12334

Closed
yglukhov opened this issue Oct 2, 2019 · 2 comments · Fixed by #23216
Closed

const cstring incorrectly cgen'd (pointer is copied, pointing to garbage) #12334

yglukhov opened this issue Oct 2, 2019 · 2 comments · Fixed by #23216
Labels
const `const x=expr` or `static: stmt` Regression

Comments

@yglukhov
Copy link
Member

yglukhov commented Oct 2, 2019

type O = object
  a: cstring

proc test(o: var O) =
  const foo = "Hello"
  const fooc: cstring = foo
  o.a = fooc

var o: O
test(o)
echo o.a

Actual output: empty
Expected output:

Hello

Offending commit: 85db42a
The issue can be worked around with the following pattern: o.a = static(cstring(foo))

@Clyybber
Copy link
Contributor

Clyybber commented Oct 2, 2019

I think this may be related to #12282 (EDIT(timotheecour): probably not, this doesn't involve distinct), since with 85db42a there should now be an implicit conversion in const fooc: cstring = foo.

@timotheecour
Copy link
Member

timotheecour commented Oct 24, 2020

reduced:

when true: # D20201024T083352
  const a = "foo"
  const b: cstring = a
  # const b: cstring = "foo" # would work
  var c = b
  echo (a.repr, b.repr, c.repr, a, b, $b, c, c.len, cast[int](c), cast[int](b))

prints:
("0x105df7f98"foo"", "0x105df7f98"foo"", "0x105df7f88"\3"\n", "foo", "\x03", "foo", "\x03", 1, 4393500552, 4393500552)

so the title is wrong, the cstring is not empty, it's just that the cstring is being copied incorrectly (the pointer is copied directly instead of copying the content it points to) when crossing the VM => RT boundary

note

another unrelated issue I found is:

when true: # D20201024T085220:here
  const a = "foo"
  var b: cstring = a
  echo (a.repr, b.repr)

("0x10d7dff98"foo"", "0x10d7df518"foo"\n")
=> repr gives different results for VM cstring vs RT cstring; this is likely due to how cstring's are represented in VM so might not be a real bug, but it's still an inconsistency

@timotheecour timotheecour changed the title Const cstrings are silently empty const cstring incorrectly cgen'd (pointer is copied, pointing to garbage) Oct 24, 2020
@timotheecour timotheecour added VM see also `const` label const `const x=expr` or `static: stmt` and removed VM see also `const` label labels Oct 24, 2020
Araq pushed a commit that referenced this issue Jan 18, 2024
fixes #12334

`nkHiddenStdConv` shouldn't be removed if the sources aren't literals,
viz. constant symbols.
narimiran pushed a commit that referenced this issue Apr 19, 2024
fixes #12334

`nkHiddenStdConv` shouldn't be removed if the sources aren't literals,
viz. constant symbols.

(cherry picked from commit 3fb46fa)
narimiran pushed a commit that referenced this issue Apr 19, 2024
fixes #12334

`nkHiddenStdConv` shouldn't be removed if the sources aren't literals,
viz. constant symbols.

(cherry picked from commit 3fb46fa)
narimiran pushed a commit that referenced this issue Apr 20, 2024
fixes #12334

`nkHiddenStdConv` shouldn't be removed if the sources aren't literals,
viz. constant symbols.

(cherry picked from commit 3fb46fa)
narimiran pushed a commit that referenced this issue Apr 20, 2024
fixes #12334

`nkHiddenStdConv` shouldn't be removed if the sources aren't literals,
viz. constant symbols.

(cherry picked from commit 3fb46fa)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
const `const x=expr` or `static: stmt` Regression
Projects
None yet
3 participants