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

[RFC] Consistent Pragma Syntax for Type Declarations #8514

Closed
awr1 opened this issue Aug 2, 2018 · 6 comments

Comments

Projects
None yet
6 participants
@awr1
Copy link
Contributor

commented Aug 2, 2018

Nim allows two placements for pragmas within a type declaration:

type A {.pragma.} = object
type B = object {.pragma.}

I realize that part of Nim's philosophy is to allow some degree of permissiveness wrt style, e.g. case insensitive identifiers, however I feel allowing both placements is not really necessary. For instance, the section on {.acyclic.} in the Nim manual uses both, inexplicably. In fact, from my reading the Nim manual doesn't specifically tell you where you can put pragmas (apart from the BNF grammar).

Generic type restraints already don't allow things like:

proc x[T: object {.union.}](a: ref T) = discard

so we should not have to worry about breaking something like that. (I can't imagine why someone would need this type of functionality, anyway.)

Personally, if previous usage were not a concern, I would prefer type B. However, the most common style in Nim, looking at the compiler, Arraymancer, NLVM, Mofuw, and numerous other repos, is overwhelmingly type A, so I would suggest deprecating type B. There also seems to be cases where type B is completely unallowed, e.g.

type Cardinal = enum {.pure.}
  north, east, south, west

does not compile.

Ofc a change like flat-out disallowing type B will inevitably break something for someone, somewhere, but I would recommend something like a compiler warning at the very least.

Let me know your thoughts.

@ghost

This comment has been minimized.

Copy link

commented Aug 2, 2018

@mratsim

This comment has been minimized.

Copy link
Collaborator

commented Aug 2, 2018

The shallow pragma works works on both sides:

type Foo = object {.shallow.}
  data: seq[int]


type Bar {.shallow.} = object
  data: seq[int]

But the acyclic pragma only works on the right hand side if it's a ref object

type Foo = ref object {.acyclic.}
  data: Foo


type Bar {.acyclic.} = ref object # Error invalid pragma
  data: Bar

But on both side if it's a object + ref object

type
  Baz {.acyclic.} = object
    data: BazRef
  BazRef = ref Baz
@Araq

This comment has been minimized.

Copy link
Member

commented Aug 2, 2018

acyclic applies to object and your Bar is a ref object, that's why. But I agree it's silly. :-)

@dom96

This comment has been minimized.

Copy link
Member

commented Aug 3, 2018

Yeah, let's deprecate type B.

@dom96 dom96 added the RFC: Accepted label Aug 3, 2018

@awr1

This comment has been minimized.

Copy link
Contributor Author

commented Aug 13, 2018

Something I noticed yesterday: {.inheritable.} pragma needs to be fixed, only currently works with type B format

@yglukhov

This comment has been minimized.

Copy link
Member

commented Aug 13, 2018

Some more dangerous form of it is that {.pure.} object pragma is silently ignored in one of the variants.

type Foo = ptr object {.pure, inheritable.} # Pure is ignored
  a: int

type Bar {.pure, inheritable.} = ptr object
  a: int

var f: Foo
var b: Bar

echo sizeof(f[]) # outputs 16
echo sizeof(b[]) # outputs 8

Given that pure objects are used for ABI compatibility this might be a challenge to debug.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.