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

old keyword syntax should accept a space (Was: "old" keyword syntax: Should not be like a function but an instance) #15

Closed
nhatminhle opened this issue Aug 17, 2014 · 20 comments
Assignees
Labels

Comments

@nhatminhle
Copy link
Owner

@nhatminhle nhatminhle commented Aug 17, 2014

From ckkohl79 on February 06, 2011 03:09:22

The "old" keyword allows you to refer to the instance's state prior to calling the corresponding method. It is, however, specified as a special function/method "old(...)":

@ensures can use the old keyword to refer to the state at the start of the method, e.g.
@ensures("size() == old(size()) + 1")

This has two disadvantages:

  1. You cannot use old() as a method name.
  2. It is counter-intuitive. It could be understood as "take the result of size() and perform 'old' on that value".

Suggestion (to solve both problems): Use "old." as an instance reference.
This is semantically much cleaner and in fact correct Java syntax.

For example:
@ensures("size() == old.size() + 1")

This even works for methods called "old()":
@ensures("old() == old.old() + 1")

As an additional benefit, we save one pair of parentheses.

Original issue: http://code.google.com/p/cofoja/issues/detail?id=8

@nhatminhle nhatminhle added bug labels Aug 17, 2014
@nhatminhle nhatminhle self-assigned this Aug 17, 2014
@nhatminhle
Copy link
Owner Author

@nhatminhle nhatminhle commented Aug 17, 2014

From ckkohl79 on February 06, 2011 03:37:29

PS: To be 100% correct, we should prefix "old" by a special character that is not a valid variable name, e.g. @ => @old. I'll file another issue for that as it is also related to the special "result" keyword.

The example would then read:
@ensures("size() == @old.size() + 1")

@nhatminhle
Copy link
Owner Author

@nhatminhle nhatminhle commented Aug 17, 2014

From nhat.min...@gmail.com on February 06, 2011 06:32:03

In Cofoja, old() is strictly more powerful than in JASS. You can refer to any pre-state expression, not just the current instance. E.g. you can take the value of a parameter before the call:

@ensures("old(x.f()) == x.f()")
void foo(T x)

Hence, any expression can be used with old(), not just the pre-state. And no you cannot use it as a method name. This has been thoroughly debated when we designed the language and we settled with this syntax on the ground that it was a minor inconvenience (not to be able to name a method old()) and that it was more consistent with overall syntax (as if we had added a keyword to the language).

I know you are complaining about the fact that this effectively adds a keyword to the language, but that is what we intended, so unfortunately, it won't be "fixed". We simply see this as a naming convention to honor, and no a priori collision was found on the whole Google code base when we tried to grep for it, so it was considered safe.

Status: WontFix
Cc: andreasl...@google.com

@nhatminhle
Copy link
Owner Author

@nhatminhle nhatminhle commented Aug 17, 2014

From ckkohl79 on February 06, 2011 07:02:49

Thanks for your fast reply.

Wouldn't
@ensures("old.x.f() == x.f()")
still be semantically valid?

Hence, any expression can be used with old(), not just the pre-state
I don't understand this. I thought "old" would refer to the state prior to calling the method, which of course may refer to the whole state of the VM.

@nhatminhle
Copy link
Owner Author

@nhatminhle nhatminhle commented Aug 17, 2014

From nhat.min...@gmail.com on February 06, 2011 08:18:42

I meant "not just the current instance in its pre-state". So your understanding is correct: it is the pre-state of the VM. Old value expressions are computed on entry of the method and the result is stored for further use.

old.size() makes a lot of sense because you can see old as the old version of this, so old acts as an object. old.x would make a lot less sense because there is no object with an x property in the current context. Also, with the function syntax you can write old(x+y), which would have to become old.x + old.y with the new syntax. Overall, we settled for old() because it has a simpler semantic associated (old() evaluates the given expression in the pre-state context) vs an object-like old, whose semantic would be something like "old.field evaluates the given identifier in the pre-state context". It would be less general, since there is no reason to limit ourselves to identifiers.

@nhatminhle
Copy link
Owner Author

@nhatminhle nhatminhle commented Aug 17, 2014

From ckkohl79 on February 06, 2011 09:48:51

The parentheses could mislead the unaware programmer that "old" is a function.
You claim "old()" has a simpler semantic, but in fact it has a /wrong/ semantic because "old()" is clearly not a function per definition.

Old is a reference to a specific state (or "context in time"), and thus it should be written in a different way.

Suggestion: Why not use curly brackets?
e.g.

old{x+y}

This would also logically fit (In Eiffel, "old" is a keyword similar to "do"; and in Java, we use curly brackets for "do" as well)

@nhatminhle
Copy link
Owner Author

@nhatminhle nhatminhle commented Aug 17, 2014

From andreasl...@google.com on February 06, 2011 10:30:07

IMO, Eiffel, where "old" is a keyword, has it right. But IIRC we went with the function because it was much simpler to implement.

@nhatminhle
Copy link
Owner Author

@nhatminhle nhatminhle commented Aug 17, 2014

From ckkohl79 on February 06, 2011 10:46:08

I wouldn't take this as a valid argument, Andreas.

What cofoja attempts to do actually is an extension to the Java programming language, not some quick-fix implementation, and thus implementors-ease should be only play a secondary role if you desire a large-scale adoption in the Java world.

Keeping Eiffel syntax aside, would "old{...}" instead of "old(...)" be somehow harder to implement?

@nhatminhle
Copy link
Owner Author

@nhatminhle nhatminhle commented Aug 17, 2014

From andreasl...@google.com on February 06, 2011 10:52:01

I wouldn't require any kind of brackets. I'd do it just like in Eiffel, where there are also none. You can always add brackets because expressions allow it. (e.g. "(a + b)" is a valid expression hence "old (a + b)" is one too.

As for ease of implementation: I let Nhat speak for that, since he knows best how hard it would be (he did all the work really :) .

@nhatminhle
Copy link
Owner Author

@nhatminhle nhatminhle commented Aug 17, 2014

From nhat.min...@gmail.com on February 06, 2011 11:18:58

I must disagree. do is actually a block keyword. Curly brackets are traditionally associated with blocks or collections. Expression-level keywords would use parentheses, e.g. in C, you have sizeof, and non-standard extensions such as alignof, typeof, etc.

Ideally, the parentheses should be optional (i.e. old x, instead of old(x); same as sizeof in C), but due to limitations in the way we handle expressions, this is not possible with the current implementation. If you care about the difference, you could write old (x) instead of old(x) as you do for methods, the same way that some people (including me) write sizeof (x+y) in C.

While I can understand that "different things should have different syntaxes" is a valid point, I rather doubt it'll really be confusing to readers in this particular case, since unary keyword operators have existed for a long time in similar languages, using a parenthesis syntax. Also, switching to the object-like syntax does not solve this issue either since there is no such object that contains all the symbols of the pre-state.

Finally, C-like syntax is known for its massive reuse of syntactic constructs, and Java is no exception. E.g. you already have the OuterClass.this syntax, which looks exactly the same as a static access, yet is very much an instance-specific thing. So I disagree that this is stranger than other things that exist in Java and related languages.

@nhatminhle
Copy link
Owner Author

@nhatminhle nhatminhle commented Aug 17, 2014

From nhat.min...@gmail.com on February 06, 2011 11:31:38

(My comment above was directed to ck@.)

I agree with Andreas on principle, as stated in my comment, but implementation-wise, it is tricky to add a unary operator, with the proper precedence, without parsing the full expression (which we do not wish to do). I suggest you use old (x) for now if you so wish.

@nhatminhle
Copy link
Owner Author

@nhatminhle nhatminhle commented Aug 17, 2014

From ckkohl79 on February 06, 2011 11:42:21

The more we are discussing about this, the more I am convinced that my initial proposal to use "old." as a context identifier is the way to go.

The Java syntax [1] makes of course use of enforced parentheses (ParExpression) and optional curly brackets (Statement) for non-functions like "for", "do", "while" etc. But in those cases, we always combine Statements with an ParExpression. "old" only takes an Expression, no Statement.

If "old" would be defined like "this" and "super" (as a "Primary" in "Expression3"; see [1]), the extension would be semantically coherent.

We'd simply need to extend "Expression3" by the following line:
{old.} Primary {Selector} {PostfixOp}
to allow statements like this:

old.x: refers to a local variable x at pre-state, or is identical to old.this.x if no local variable x exists
old.this.x: refers to an instance attribute or static class attribute, at pre-state
old.super.x: refers to an instance attribute or static class attribute of the super class, at pre-state.
old.org.example.package.SomeClazz.A: refers to some static field in SomeClazz, at pre-state

[1] http://java.sun.com/docs/books/jls/third_edition/html/syntax.html

@nhatminhle
Copy link
Owner Author

@nhatminhle nhatminhle commented Aug 17, 2014

From ckkohl79 on February 06, 2011 11:44:57

"old (x)" (with a space) is a good compromise!

@nhatminhle
Copy link
Owner Author

@nhatminhle nhatminhle commented Aug 17, 2014

From nhat.min...@gmail.com on February 06, 2011 12:15:03

Good! I'll put that on my TODO list.

Supporting arbitrary whitespace is actually not a simple matter because of the way we report errors, which relies on matching positions in the underlying compiler's error message with positions from the original string. This is the price to pay in order to piggy back on the compiler infrastructure.

Summary: old keyword syntax should accept a space (Was: "old" keyword syntax: Should not be like a function but an instance)
Status: Accepted
Owner: nhat.minh.le

@nhatminhle
Copy link
Owner Author

@nhatminhle nhatminhle commented Aug 17, 2014

From ckkohl79 on February 06, 2011 12:27:15

Great!

You might consider switching to a JavaCC-based compiler (or something similar) to avoid ending up in the expressiveness vs. complexity trap.

@nhatminhle
Copy link
Owner Author

@nhatminhle nhatminhle commented Aug 17, 2014

From nhat.min...@gmail.com on February 06, 2011 12:37:03

Actually, we've considered various parser solutions, and, mind you, I'd be happy to have a real expression parser --- as someone who wants to specialize in languages, it kind of kills me to have to hack that by hand. Also, it'd be really great for error reporting!

The big advantage of having a fuzzy parser is that it allows us to pass on Java code verbatim to the compiler, hence everything the compiler supports, we do as well. Whereas we'd have to maintain our grammar and/or keep the external parser package up-to-date in order to keep up with Java evolutions or dialectal distortions. Since we're a pretty small team, it's not something we want to invest in right now, but we're still considering such possibilities.

If we do get a real parser, I'm inclined to make it an add-on to the current set-up, for the purpose of error reporting, so that it could be disabled in order to do compile-only passes if needed (a bit like a lint tool or something). As it is now, our poor error reporting is our biggest flaw.

@nhatminhle
Copy link
Owner Author

@nhatminhle nhatminhle commented Aug 17, 2014

From nhat.min...@gmail.com on February 06, 2011 12:59:54

This patch should make old() accept spaces before the parentheses while preserving error reporting capabilities. Andreas, can you review that since David is away?

Status: Started

Attachment: 0001-Add-support-for-arbitrary-whitespace-after-old-keywo.patch

@nhatminhle
Copy link
Owner Author

@nhatminhle nhatminhle commented Aug 17, 2014

From nhat.min...@gmail.com on February 06, 2011 18:56:07

Hum, a bug slipped in the old patch, I've regenerated a new one; please ignore the previous version. Andreas, can you review this one instead? Thanks.

Attachment: 0001-Add-support-for-arbitrary-whitespace-after-old-keywo.patch

@nhatminhle
Copy link
Owner Author

@nhatminhle nhatminhle commented Aug 17, 2014

From ckkohl79 on February 07, 2011 00:33:44

This part of your patch doesn't look good:

if (!tokenizer.hasNext()) {
syntaxValid = false;
}
Token afterOld = tokenizer.next();

If syntaxValid == false, you should fail quickly (i.e., return or throw). Right now, you just continue with next(), which does not work if tokener.hasNext() was false.

@nhatminhle
Copy link
Owner Author

@nhatminhle nhatminhle commented Aug 17, 2014

From nhat.min...@gmail.com on February 07, 2011 03:25:34

Er... yeah, sorry. I'll remember that hurrying patches late at night on a post-release day is a bad idea. :) Patch doesn't even honor our own coding style, actually.

Anyway, thanks for your review, here's a new (hopefully good) one. It also fixes an offset problem in error reporting related to old().

Attachment: 0001-Add-support-for-arbitrary-whitespace-after-old-keywo.patch

@nhatminhle
Copy link
Owner Author

@nhatminhle nhatminhle commented Aug 17, 2014

From nhat.min...@gmail.com on February 28, 2011 11:45:20

old() should now accept spaces before the parentheses in r84 .

Status: Fixed

@nhatminhle nhatminhle closed this Aug 17, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
1 participant
You can’t perform that action at this time.