Page 4 of 5

Re: Require spaces around binary ops

PostPosted: Fri Jul 12, 2013 6:26 am
by hopscc
Probably !. would be the most fitting match given existing choices for deref (DOT) and non-nil coalesce.
Other langs use ?. and it fits with the nilability association ( nilTyping and nil-coalesce using ?)

I've made ticket:341 as a placeholder for enhancement of requiring spaces around binary ops.
I've got something working along these lines ( for ASSIGN(=), EQ(==) -
its reasonably straight forward to add others ((non)nil coalesce QUESTION(?) and BANG(!) and straight math ops.

With just the first two its not too onerous since mostly its natural to space wrap these anyway (except (for me at least) if they are enclosed in parentheses :) )
Interestingly with only those two implemented the compiler build gives 230 failures and the test suite has 213 failures ( tho most of those are wrt EQ).

I'll cleanup the code to a patch and drop it on that ticket for anyone elses experimentation/investigation..

Re: Require spaces around binary ops

PostPosted: Fri Jul 12, 2013 7:10 am
by hopscc
spacewrap.patch on ticket.
Apply in cobra Source dir not root of cobra source tree.

Re: Require spaces around binary ops

PostPosted: Sat Jul 20, 2013 4:21 am
by hopscc
Anyone looked at this at all ??

Re: Require spaces around binary ops

PostPosted: Sat Jul 20, 2013 12:54 pm
by Charles
No, but I will. Just been busy lately.

Re: Require spaces around binary ops

PostPosted: Sat Aug 03, 2013 2:14 am
by Charles
Sorry it took me awhile to get to this. Been busy, blah blah blah.

Regarding the patch at:
http://cobra-language.com/trac/cobra/at ... eref.patch

Re:
pro isCompilerMade from var as bool
1344 """ flag as internally created."""

... can you use the existing .isImplicit instead? We already have that and use it for that purpose.

I'm confused by what you're doing with the inverse coalesce (!) operator. In BinaryOpExpr.cobra, under on 'QUESTION_DOT', you're doing something with it and I see it being used in the test cases... but the cases also use "ns1?.co1?.name"

I think if I only saw it in the implementation, I would presume you were leveraging it for the implementation. But I see it in the test cases as well. Are you just making sure it works as expected?

The comments on the ticket with the patch don't mention it.

Maybe when I re-read it tommorow, it will click.

Re: Require spaces around binary ops

PostPosted: Sat Aug 03, 2013 1:22 pm
by nerdzero
Charles wrote:...
Re:
pro isCompilerMade from var as bool
1344 """ flag as internally created."""

... can you use the existing .isImplicit instead? We already have that and use it for that purpose.
...

That reminds me, I have a local hack that combines .isCompilerGenerated (yet another property for this purpose) and .isImplicit. Without it, a compiler generated member might be the last one in the list of members of a declaration and this throws off the .endToken for the container. It would be nice to get them all combined somehow.

For reference, here's what I have locally:
--- Members.cobra   (revision 3019)
+++ Members.cobra (working copy)
@@ -807,7 +807,11 @@

pro hasYieldStmt from var

- pro isCompilerGenerated from var
+ pro isCompilerGenerated as bool
+ get
+ return _isCompilerGenerated or .isImplicit
+ set
+ .isImplicit = _isCompilerGenerated = value

def bodyExclusion as String?
ensure result in [nil, 'abstract', 'interface', 'extern box', 'dll import']

Admittedly, it's ugly; but it works.

Re: Require spaces around binary ops

PostPosted: Tue Aug 13, 2013 4:13 am
by hopscc
Ok re .isimplicit. Didnt spot that.

The nilsafe deref is turned into sequence of non nil coalesce and tmp inferred vars as proceed thru the dot exprs.
As surmised the they use the existing handling for that and tests do both to check result and impl.

Theres a comment after 'on QWESTION_OP' thats supposed to show the expansion.
Obviosly needs extending/clarification.

I'm away fm home till Sept -will rework it then if noone else has.

Re: Require spaces around binary ops

PostPosted: Tue Sep 03, 2013 4:12 am
by hopscc
I'm back now
Will rework that patch next thing.

Should the use of .isCompilerGenerated also be changed to use .isImplicit instead
or is it separate for a purpose?

Re: Require spaces around binary ops

PostPosted: Tue Sep 03, 2013 11:12 pm
by Charles
hopscc wrote:I'm back now
Will rework that patch next thing.

Should the use of .isCompilerGenerated also be changed to use .isImplicit instead
or is it separate for a purpose?

I'm not sure. I haven't looked at all the uses yet.

Re: Require spaces around binary ops

PostPosted: Wed Sep 04, 2013 4:29 am
by hopscc
Reworked patch for nilsafe deref along lines suggested now on ticket:339