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

Nested tuple access can't parse #2

Closed
andymcn opened this issue Sep 1, 2014 · 5 comments
Closed

Nested tuple access can't parse #2

andymcn opened this issue Sep 1, 2014 · 5 comments

Comments

@andymcn
Copy link
Contributor

andymcn commented Sep 1, 2014

Using "dot int literal" to access tuple elements cannot parse correctly for nested elements, because it looks like dot float literal.

Example, access element 2 in element 1 of tuple x:
x.1.2

This is intended to be the lexer tokens:
id x, dot, int literal 1, dot, int literal 2

However it is actually:
id x, dot, float literal 1.2

Possible solutions:

  1. This can be worked around by putting a space anywhere between the 1 and the 2, eg:
    x.1 .2
    but that's pretty ugly.
  2. Handle the dot float access when parsing. This will be really nasty.
  3. Use a different syntax for tuple element access.
  4. Do not allow nested tuples.
@sylvanc
Copy link
Contributor

sylvanc commented Oct 5, 2014

Sebastian suggests using array-like notation, eg:

var w = (1, 2, 3)
var x = w(0) // x == 1
var y = w(1) // y == 2
var z = w(2) // z == 3

That means basically getting rid of accepting a TK_INT as the right side of a TK_DOT, and in the type checker for TK_CALL, if the receiver is a tuple, we accept a single literal integer positional argument, which must be in-bounds.

Nested tuples just get nested calls:

var w = (1, 2, (3, 4))
var x = w(2)(1) // x == 4

@andymcn
Copy link
Contributor Author

andymcn commented Oct 5, 2014

The only other sensible option I've come up with for this is to use a name that is not just an integer, a leading underscore would be the obvious choice:

var x = w._0
var x = w._2._1

but that's a bit ugly.

One thing, for the array style syntax, would the index have an be an actual literal, or would a constant expression be allowed? Eg:

var z = w(1 + 2)

@sylvanc
Copy link
Contributor

sylvanc commented Oct 6, 2014

The issue with a constant expression is we would then be lifting elements of code generation into the type checking phase, since we need the final value in order to make sure the index is in bounds and to get a type for the expression. That can be done, of course, but it would be a significant change, and could get complicated if we wanted to allow constant expressions that themselves required type checking (a function call that's actually a constant expression, etc).

Leading underscores are possible, that's what scala does:

http://stackoverflow.com/questions/6884298/why-is-scalas-syntax-for-tuples-so-unusual

And it's sort of what's done in LaTeX, since _ is subscript, so x_1 is x1.

@andymcn
Copy link
Contributor Author

andymcn commented Oct 6, 2014

Agreed, an actual literal int is required.

@sylvanc
Copy link
Contributor

sylvanc commented Oct 14, 2014

We went with leading underscores, and a 1 based index.

@sylvanc sylvanc closed this as completed Oct 14, 2014
slfritchie added a commit that referenced this issue Oct 18, 2018
There are a couple places where we can be notified that a TCP
socket peer has closed the connection on us:

1. _event_notify(), via an event from the ASIO thread.
2. _pending_reads(), where we try to read from the socket
   (via the @pony_os_recv()? FFI call which is partial!)
   and get an error.

If we discover the remote peer's close by path #2, then sometime
later the ASIO thread may notify us of the close via path #1.

Both paths will call `@pony_asio_event_unsubscribe(event)`.  There's
a sanity check + `pony_assert(0)` inside of the Pony runtime to
prevent double calls to `@pony_asio_event_unsubscribe(event)`.
However, `pony_assert()` will only act if using a Pony "debug" build
of the runtime.  So, in non-debug use (i.e., normal day-to-day use),
nobody notices the problem: it's only visible when using a debug
build (probably because you're debugging some *other* problem).

This commit adds a comments + check to avoid double-calls to
`@pony_asio_event_unsubscribe(event)`.
slfritchie added a commit that referenced this issue Oct 19, 2018
There are a couple places where we can be notified that a TCP
socket peer has closed the connection on us:

1. _event_notify(), via an event from the ASIO thread.
2. _pending_reads(), where we try to read from the socket
   (via the @pony_os_recv()? FFI call which is partial!)
   and get an error.

If we discover the remote peer's close by path #2, then sometime
later the ASIO thread may notify us of the close via path #1.

Both paths will call `@pony_asio_event_unsubscribe(event)`.  There's
a sanity check + `pony_assert(0)` inside of the Pony runtime to
prevent double calls to `@pony_asio_event_unsubscribe(event)`.
However, `pony_assert()` will only act if using a Pony "debug" build
of the runtime.  So, in non-debug use (i.e., normal day-to-day use),
nobody notices the problem: it's only visible when using a debug
build (probably because you're debugging some *other* problem).

This commit adds a comment + check to avoid double-calls to
`@pony_asio_event_unsubscribe(event)`.  It's done by adding a new
Pony API FFI call, `@pony_asio_event_get_disposable`, that peeks
into the event's flag status (and *not* the `flags` argument to
`_event_notify()`!) to check if it's safe to unsubscribe.

Many thanks to @dipinhora for discussing the merits of how the
heck to fix this and to @nisanharamati for being curious about,
"Why is this thing sometimes crashing, but only when using a
debug build of Pony?"
slfritchie added a commit that referenced this issue Oct 19, 2018
There are a couple places where we can be notified that a TCP
socket peer has closed the connection on us:

1. _event_notify(), via an event from the ASIO thread.
2. _pending_reads(), where we try to read from the socket
   (via the @pony_os_recv()? FFI call which is partial!)
   and get an error.

If we discover the remote peer's close by path #2, then sometime
later the ASIO thread may notify us of the close via path #1.

Both paths will call `@pony_asio_event_unsubscribe(event)`.  There's
a sanity check + `pony_assert(0)` inside of the Pony runtime to
prevent double calls to `@pony_asio_event_unsubscribe(event)`.
However, `pony_assert()` will only act if using a Pony "debug" build
of the runtime.  So, in non-debug use (i.e., normal day-to-day use),
nobody notices the problem: it's only visible when using a debug
build (probably because you're debugging some *other* problem).

This commit adds a comment + check to avoid double-calls to
`@pony_asio_event_unsubscribe(event)`.  It's done by adding a new
Pony API FFI call, `@pony_asio_event_get_disposable`, that peeks
into the event's flag status (and *not* the `flags` argument to
`_event_notify()`!) to check if it's safe to unsubscribe.

Many thanks to @dipinhora for discussing the merits of how the
heck to fix this and to @nisanharamati for being curious about,
"Why is this thing sometimes crashing, but only when using a
debug build of Pony?"
slfritchie added a commit to slfritchie/ponyc that referenced this issue Oct 27, 2018
There are a couple places where we can be notified that a TCP
socket peer has closed the connection on us:

1. _event_notify(), via an event from the ASIO thread.
2. _pending_reads(), where we try to read from the socket
   (via the @pony_os_recv()? FFI call which is partial!)
   and get an error.

If we discover the remote peer's close by path ponylang#2, then sometime
later the ASIO thread may notify us of the close via path ponylang#1.

Both paths will call `@pony_asio_event_unsubscribe(event)`.  There's
a sanity check + `pony_assert(0)` inside of the Pony runtime to
prevent double calls to `@pony_asio_event_unsubscribe(event)`.
However, `pony_assert()` will only act if using a Pony "debug" build
of the runtime.  So, in non-debug use (i.e., normal day-to-day use),
nobody notices the problem: it's only visible when using a debug
build (probably because you're debugging some *other* problem).

This commit adds a comment + check to avoid double-calls to
`@pony_asio_event_unsubscribe(event)`.  It's done by adding a new
Pony API FFI call, `@pony_asio_event_get_disposable`, that peeks
into the event's flag status (and *not* the `flags` argument to
`_event_notify()`!) to check if it's safe to unsubscribe.

Many thanks to @dipinhora for discussing the merits of how the
heck to fix this and to @nisanharamati for being curious about,
"Why is this thing sometimes crashing, but only when using a
debug build of Pony?"
aturley pushed a commit that referenced this issue Nov 6, 2018
* Fix race condition with socket close event from peer

There are a couple places where we can be notified that a TCP
socket peer has closed the connection on us:

1. _event_notify(), via an event from the ASIO thread.
2. _pending_reads(), where we try to read from the socket
   (via the @pony_os_recv()? FFI call which is partial!)
   and get an error.

If we discover the remote peer's close by path #2, then sometime
later the ASIO thread may notify us of the close via path #1.

Both paths will call `@pony_asio_event_unsubscribe(event)`.  There's
a sanity check + `pony_assert(0)` inside of the Pony runtime to
prevent double calls to `@pony_asio_event_unsubscribe(event)`.
However, `pony_assert()` will only act if using a Pony "debug" build
of the runtime.  So, in non-debug use (i.e., normal day-to-day use),
nobody notices the problem: it's only visible when using a debug
build (probably because you're debugging some *other* problem).

This commit adds a comment + check to avoid double-calls to
`@pony_asio_event_unsubscribe(event)`.  It's done by adding a new
Pony API FFI call, `@pony_asio_event_get_disposable`, that peeks
into the event's flag status (and *not* the `flags` argument to
`_event_notify()`!) to check if it's safe to unsubscribe.

Many thanks to @dipinhora for discussing the merits of how the
heck to fix this and to @nisanharamati for being curious about,
"Why is this thing sometimes crashing, but only when using a
debug build of Pony?"

* Code review fix: bit masking is not appropriate for ev->flags

* Add prototype for pony_asio_event_get_disposable in event.h

Fix Windows linking problem
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

No branches or pull requests

2 participants