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

[DO NOT MERGE] experiment with -d:nimImplicitCompleteStruct showing we need completeStruct instead of incompleteStruct #97

Draft
wants to merge 25 commits into
base: devel
Choose a base branch
from

Conversation

timotheecour
Copy link
Owner

As I explained in proposal 2 from nim-lang/RFCs#205, {.incompleteStruct.} (introduced in 2012) should be deprecated, as it's assuming the wrong default (which is implicit assumption that importc types would be complete unless marked with {.incompleteStruct.}). Which is why the correct pragma should be {.completeStruct.} (as introduced in nim-lang#13926) and makes the opposite assumption that importc types are incomplete unless marked with {.completeStruct.}

this experiment-only PR introduces -d:nimImplicitCompleteStruct, which when enabled, allows CT sizeof; it then compares with c_sizeof. It shows that most importc types are in fact incomplete, and the CT sizeof differs from c_sizeof.
Only very few types are annotated with {.incompleteStruct.} so most code out there would be wrong if that flag had any meaning.
It would be pointless to have to annotate all those types with {.incompleteStruct.}, when in fact the sane thing to do is to opt-in CT sizeof instead of opt-out.

nim c -r --stacktrace:off -d:nimImplicitCompleteStruct -f $timn_D/src/timn/exp/abichecks.nim

prints:

Dirent struct dirent => 272 vs 1048
Tflock struct flock => 32 vs 24
Glob glob_t => 24 vs 88
Group struct group => 24 vs 32
Ipc_perm struct ipc_perm => 20 vs 24
Stat struct stat => 104 vs 144
Statvfs struct statvfs => 88 vs 64
{.emit:"""
#define c_astToStrImpl(T) #T
"""
.}

import macros
import strutils
import unittest

import posix

template ensureCgen(T: typedesc) =
  ## ensures cgen
  var a {.volatile.}: T

template c_astToStr(T: typedesc): string =
  block:
    # proc needed pending https://github.com/nim-lang/Nim/issues/13943 D20200409T215527
    proc fun2(): string =
      var s: cstring
      {.emit:[s, "= c_astToStrImpl(", T, ");"].}
      $s
    fun2()

template c_sizeof(T: typedesc): int =
  block:
    # proc needed pending https://github.com/nim-lang/Nim/issues/13943 D20200409T215527
    proc fun2(): int =
      {.emit:[result," = sizeof(", T, ");"].}
    fun2()

macro test(body: untyped): untyped =
  result = newStmtList()
  for T in body:
    result.add quote do:
      let s1 = `T`.sizeof
      let s2 = `T`.c_sizeof
      if s1 != s2:
        echo "$1 $2 => $3 vs $4" % [$astToStr(`T`), c_astToStr(`T`), $s1, $s2]
      # check s1 == s2

test:
  Dirent
  Tflock
  Glob
  Group
  Time
  Utsname
  Ipc_perm
  Stat
  Statvfs
  SigEvent
  # Posix_typed_mem_info # BUG? # error: invalid application of 'sizeof' to an incomplete type 'struct posix_typed_mem_info'

timotheecour and others added 9 commits April 8, 2020 09:56
…row (nim-lang#13907) [backport:1.2]

* fix nim-lang#13902 distinct uint64 type corruption on 32-bit with borrow

Co-authored-by: Timothee Cour <timothee.cour2+lightsail@gmail.com>
Co-authored-by: cooldome <ariabushenko@bk.ru>
Co-authored-by: cooldome <ariabushenko@bk.ru>
* test important packages on Linux

* enable chronos
…-lang#13919) [backport]

This fixes at least a couple of issues:

* Procs loaded from the DLL being used even when the pointer is nil.
* The actual issue (nim-lang#13903) which appeared to cause stack corruption on
  Android 7.1.1 with OpenSSL 1.1.1f. The change that fixed this was the
  move to loading the procs in `sslSym`.
Co-authored-by: cooldome <ariabushenko@bk.ru>
* posix: add full Haiku support

This commit provides a posix_haiku derived from posix_other, with types
following Haiku's definition. This fixes cases where the compiler
generates type check for the wrong types (ie. checks where generated for
an int-derived type but it's actually implemented as an uint instead).

* tools/kochdocs: welcome posix_haiku to the blacklist
Also modified tosprocterminate to verify waitForExit implementations.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants