pashapm / cofoja

Automatically exported from code.google.com/p/cofoja
GNU Lesser General Public License v3.0
0 stars 0 forks source link

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

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
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 reported on code.google.com by ckkohl79 on 6 Feb 2011 at 11:09

GoogleCodeExporter commented 9 years ago
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")

Original comment by ckkohl79 on 6 Feb 2011 at 11:37

GoogleCodeExporter commented 9 years ago
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.

Original comment by nhat.min...@gmail.com on 6 Feb 2011 at 2:32

GoogleCodeExporter commented 9 years ago
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.

Original comment by ckkohl79 on 6 Feb 2011 at 3:02

GoogleCodeExporter commented 9 years ago
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.

Original comment by nhat.min...@gmail.com on 6 Feb 2011 at 4:18

GoogleCodeExporter commented 9 years ago
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)

Original comment by ckkohl79 on 6 Feb 2011 at 5:48

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

Original comment by andreasl...@google.com on 6 Feb 2011 at 6:30

GoogleCodeExporter commented 9 years ago
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?

Original comment by ckkohl79 on 6 Feb 2011 at 6:46

GoogleCodeExporter commented 9 years ago
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 :) . 

Original comment by andreasl...@google.com on 6 Feb 2011 at 6:52

GoogleCodeExporter commented 9 years ago
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.

Original comment by nhat.min...@gmail.com on 6 Feb 2011 at 7:18

GoogleCodeExporter commented 9 years ago
(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.

Original comment by nhat.min...@gmail.com on 6 Feb 2011 at 7:31

GoogleCodeExporter commented 9 years ago
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

Original comment by ckkohl79 on 6 Feb 2011 at 7:42

GoogleCodeExporter commented 9 years ago
"old (x)"   (with a space) is a good compromise!

Original comment by ckkohl79 on 6 Feb 2011 at 7:44

GoogleCodeExporter commented 9 years ago
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.

Original comment by nhat.min...@gmail.com on 6 Feb 2011 at 8:15

GoogleCodeExporter commented 9 years ago
Great!

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

Original comment by ckkohl79 on 6 Feb 2011 at 8:27

GoogleCodeExporter commented 9 years ago
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.

Original comment by nhat.min...@gmail.com on 6 Feb 2011 at 8:37

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

Original comment by nhat.min...@gmail.com on 6 Feb 2011 at 8:59

Attachments:

GoogleCodeExporter commented 9 years ago
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.

Original comment by nhat.min...@gmail.com on 7 Feb 2011 at 2:56

Attachments:

GoogleCodeExporter commented 9 years ago
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.

Original comment by ckkohl79 on 7 Feb 2011 at 8:33

GoogleCodeExporter commented 9 years ago
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().

Original comment by nhat.min...@gmail.com on 7 Feb 2011 at 11:25

Attachments:

GoogleCodeExporter commented 9 years ago
old() should now accept spaces before the parentheses in r84.

Original comment by nhat.min...@gmail.com on 28 Feb 2011 at 7:45