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

cascade set and reset operator #2066

Open
jodinathan opened this issue Jan 12, 2022 · 21 comments
Open

cascade set and reset operator #2066

jodinathan opened this issue Jan 12, 2022 · 21 comments
Labels
feature Proposed language feature that solves one or more problems

Comments

@jodinathan
Copy link

We were updating some code from AngularComponents:

overlayRef.state.visibility = visibility.Visibility.Hidden;
overlayRef.overlayElement.style
      ..display = ''
      ..visibility = 'hidden';

And we couldn't figure it out how to currently use cascade at it.
Can it?

If not, what about a cascade forward/reset operator?
The above code would be:

overlayRef
  ..state.visibility = visibility.Visibility.Hidden
  ..overlayElement.style
      >.display = '' // >. means that we are setting *style* as the new subject to be cascaded
      ..visibility = 'hidden';

Optionally you could reset it back to overlayRef:

overlayRef
  ..state.visibility = visibility.Visibility.Hidden
  ..overlayElement.style
      >.display = ''
      ..visibility = 'hidden'
  <.someOtherProp = true; // <. means we are reseting it one cascaded item before, which is *overlayRef* 

two dots:

overlayRef
  ..state.visibility = visibility.Visibility.Hidden
  ..overlayElement.style> // the set operator could be at this spot
      ..display = '' // when using two dots
      ..visibility = 'hidden'
  <..someOtherProp = true; // <.. instead of <.
@jodinathan jodinathan added the feature Proposed language feature that solves one or more problems label Jan 12, 2022
@jodinathan
Copy link
Author

another syntax:

overlayRef
  ..state.visibility = visibility.Visibility.Hidden
  ..overlayElement.>style // the operator is .> and this means *style* is the new 
                          // subject to be cascaded within this expression
      ..display = '' // the cascade itself keeps the same syntax, 
                     // just indentation obviously changes to reflect the cascaded subject
      ..visibility = 'hidden';

@munificent
Copy link
Member

Unfortunately, no, the cascade syntax doesn't handle cases like this well. I don't think we can reasonably extend it to support nesting better without leading to syntax that's even more confusing and hard for readers to grasp. It's a bummer because the original proposed cascade syntax would have handled this gracefully:

overlayRef.{
  state.visibility = visibility.Visibility.Hidden,
  overlayElement.style.{
    display = '',
    visibility = 'hidden'
  }
}.someOtherProp = true;

For reasons that aren't clear to me, when the initial proposal made its way to the old language team, what they came back with was the .. syntax we have now.

It's really hard to build on the current cascade syntax because it's already quite unfamiliar and pretty strange syntactically. The fact that it looks like .. but is all the way at the opposite end of the operator precedence hierarchy from . is a source of unending confusion for users.

In your case, I think the best solution is to simply not try to stuff that much code into a single cascade. Cascades are nice for some small scale uses, but they only go so far and that's OK.

@jodinathan
Copy link
Author

jodinathan commented Jan 12, 2022

@munificent that cascade syntax seems much better than the current. Apparently it could work well with the if in constants that we already have:

overlayRef.{
  state.visibility = visibility.Visibility.Hidden,
  overlayElement.style.{
    if (foo > 5)
      display = '',
    visibility = 'hidden'
  }
}.someOtherProp = true;

Just curious: why the dot?

*edit: Yeah, the current syntax seems easier to cascade method invocations. I am guessing that is the reason they ditched the first proposal. So a .> just to change the cascading subject would make a very good difference here without breaking anything

@jodinathan
Copy link
Author

Both forms have their uses. E.g. var sw=Stopwatch()..start(); But this syntax doesn't "scale". The idea of

it would scale with a .> to change the cascading subject

@jodinathan
Copy link
Author

it would scale with a .> to change the cascading subject

It can change it for the compiler, but not for a human reader. :-) The syntax with .{...} is a really good one. Take another look. You can't beat it.

I did.
It is nice but thinking twice it can be kinda confusing:

String foo() => 'hey';
var state = foo();

overlayRef.{
  state = 123, // is state the local variable?
  foo() // is foo the same function we just used to set state local var?
};

The above situation doesn't happen at all with ..
So for now I would be really happy with being able to change the cascading subject with something like .>

@lrhn
Copy link
Member

lrhn commented Jan 12, 2022

The brace syntax is interesting. It should probably not use commas as separators, it looks much more like statements.
So,

overlayRef.{
  state.visibility = visibility.Visibility.Hidden;
  overlayElement.style.{
    if (foo > 5) display = '';
    visibility = 'hidden';
  };
}.someOtherProp = true;

Using the "if element" syntax seems useful too.

It's still a little confusing that the initial selector, state/overlayElement here, look like raw identifiers, but are really lookups on an implicit receiver. And maybe also that the cascade sections look like statements (but no more confusing than looking like a set literal).

@jodinathan
Copy link
Author

foo() // is foo the same function we just used to set state local var?

No, it's a function overlayRef.foo(). It doesn't get shadowed.

yeah, I know the answer that is why I said confusing and not ambiguous.

I do like the brace syntax, I am just trying to figure it out why it was not chosen and this "confusing" is a quick guess only

@munificent
Copy link
Member

Both forms have their uses. E.g. var sw=Stopwatch()..start(); But this syntax doesn't "scale".

I don't know, it's only a single character longer:

var sw = Stopwatch()..start();
var sw = Stopwatch().{start()};

Sure, it was rejected by old management, but that was then, and this is now. Maybe now is the time to re-evaluate it again?

Path dependence rules everything. Like it or not, there are millions of and millions of uses of the current cascade syntax in the wild, so changing it now is dramatically more costly than it would have been back when the syntax was introduced. But the value of the .{ ... } syntax is no greater now than it was then, so the cost/benefit trade-off today just doesn't seem like it's worth the huge migration cost. :(

Just curious: why the dot?

Two main reasons:

  1. It makes it clear that you're doing method calls. You can think of .{ a(), b(), } as meaning sort of "take that . and apply it to a(), b(), and c().
  2. It avoids an ambiguity. foo() { bar(); } is already valid syntax for a local function declaration.

The brace syntax is interesting. It should probably not use commas as separators, it looks much more like statements.

Yeah, back when @jacob314 first proposed the syntax, I don't think we ever settled on whether it should use commas or semicolons to separate/terminate the clauses. I would lean towards comma because it means you don't need a trailing one (using ; as a separator would be weird), it's more consistent with collection literals, and I think it helps send a clearer signal that you aren't looking at a block.

@lrhn
Copy link
Member

lrhn commented Jan 13, 2022

What if expression.{ statements } was a statement which changes the this value for the { statements } block to the value of expression. (Similar in spirit to #328).

Then you can do:

  overlayRef.{
    state.visibility = visibility.Visibility.Hidden;
    overlayElement.style.{
      if (foo > 5) display = '';
      this.visibility = 'hidden'; // The `this` is needed because another `visibility` is in scope.
    }
  }

You can also do any other statement in there, and we'd still have .. for expression level cascades.

Maybe even allow .foo as shorthand for this.foo.

@lrhn
Copy link
Member

lrhn commented Jan 13, 2022

I deliberately avoided allowing statements to run inside an expression (because foo..{if(test) break;} can probably get very confusing), but if that's not a problem, then ..{ is also an option.

@eernstg
Copy link
Member

eernstg commented Jan 13, 2022

Here's another proposal which is actually even more similar to features discussed in this issue: #260 ('anonymous methods'). For example:

// Original code.
overlayRef.state.visibility = visibility.Visibility.Hidden;
overlayRef.overlayElement.style
    ..display = ''
    ..visibility = 'hidden';

// Same thing using anonymous methods.
overlayRef.{
  state.visibility = visibility.Visibility.Hidden;
  overlayElement.style.{
    display = '';
    visibility = 'hidden';
  }
};

@Levi-Lesches
Copy link

In any case, seems like @lrhn's earlier comment is identical to @eernstg's interpretation of anonymous methods. One clear benefit of that approach (besides for its cleanliness and intuitive meaning) is the lexical resolution. When reading through this thread, I was wondering how to differentiate between overlayRef.foo and a local or global variable foo in the above examples. But with an implicit this, the issue is easily resolved using current and familiar rules.

@Wdestroier
Copy link

Wdestroier commented Jan 14, 2022

This syntax looks a little strange...

overlayRef.{
  state.visibility = visibility.Visibility.Hidden;
  overlayElement.style.{
    display = '';
    visibility = 'hidden';
  }
};

What about adding trailing lambdas to Dart?
If a method call receives a function as the last parameter, then this function can be put outside the parentheses.

void main() {
  final strings = ['Hello world', '13245658767', 'Apple', 'Juice'];
  // strings.map((it) => it.upperCase());
  strings.map { it.upperCase(); };
}

(This is probably the most important kotlin feature. Another very important feature is optional/no semi-colons.)

The idea is to add this feature, make the call() method accept a void Function() by default and change the context of this like in extension methods.

Just to clarify, classes would have the call method overriden by default:

class OverlayRef {
  call(void Function()? lambda) {
    lambda?.call();
  }
}

And then the code could be rewritten as:

overlayRef {
  state.visibility = visibility.Visibility.Hidden;
  overlayElement.style {
    display = '';
    visibility = 'hidden';
  }
};

@jodinathan
Copy link
Author

overlayRef {
  state.visibility = visibility.Visibility.Hidden;
  overlayElement.style {
    display = '';
    visibility = 'hidden';
  }
};

how do you call a method with this syntax?

@Wdestroier
Copy link

how do you call a method with this syntax?

overlayRef {
  state.visibility = visibility.Visibility.Hidden;
  hide(); // equivalent to overlayRef.hide();
}

@Wdestroier
Copy link

Wdestroier commented Jan 15, 2022

It's impossible to answer these questions without going into an infinitely long series of further questions and answers.

I made an example that is valid Dart code to show how the scope of functions inside functions work and try to answer your question.

main() {
  // Define functions to mimic the future syntax
  overlayRef([void Function() lambda]) {
    lambda();
  }
  overlayElement_style([void Function() lambda]) {
    lambda();
  }
  
  // Code
  overlayRef(() {
    dynamic _this; // _this should be implicit and equal to overlayRef
    _this.state.visibility = _this.visibility.Visibility.Hidden;
    final emptyString = '';  // can I write it here? Of course yes! :)
    
    overlayElement_style(() {
      dynamic _this; // _this should be implicit and equal to overlayElement.style
      _this.display = emptyString; // can I see emptyString from here? Of course yes! :)
      // The scope of variables for functions inside functions is already defined
      // in the Dart specification

      _this.visibility = 'hidden';
    });
  });
}

@Wdestroier
Copy link

If I can see emptyString from that place, I must be able to see state.visibility, too, right?

No... You can see overlayRef.state. Too many name clashes would exist if you could see state.

We are going into infinite series now

Language engineers waking up tomorrow and seeing this discussion with 100 messages: My day is ruined

@Wdestroier
Copy link

Wdestroier commented Jan 15, 2022

I'm going to make a bunch of examples to explain this feature better.

About the changes in the call method:

class OverrideCall {
  call(int number) => print(number);
}
// This feature won't break any code that currently overrides the call() method.
void main() => OverrideCall(1);
// Compile time error, the function call doesn't accept a void Function()
void main() => OverrideCall { print("Hello world"); };

class NewCall {
  // This is the new call method, the function is nullable to avoid breaking any existing code.
  call([void Function()? function]) => function?.call();
}
// Fine
void main() => NewCall { print("Hello world"); };
void main() => NewCall(() => print("Hello world"));

class DynamicCall {
  String? message;
}

void main() {
  dynamic dynamicCall = DynamicCall();
  dynamicCall { message = 'Hi'; }; // Fine? The method's argument types must be checked during runtime
}

class ComplexExample {
  String? message;

  // People may want to override the call method.
  // Overriding the call method will "break" the cascade operator.
  // Unless, in the future, Dart allows union types, optional positional types or method overloading.
  // It's important to note that this feature isn't a cascade operator.
  // Being a cascade operator is one of the consequences.
  call(String message, void Function()? function) {
    message = message;
    function?.call();
  }
}

void main() {
  final complex = ComplexExample();
  // If a method call receives a function as the last parameter, then this function can be put outside the parentheses
  complex("Hi") { print(message); };
}

About the scope resolution:

class Foo {
  Bar bar(int i) => Bar();
}
class Bar {
  Baz baz() {
    return Baz();
  };
}
class Baz {}

void main() {
  var foo = Foo();

  // Label A
  foo().bar(1).baz() {
  // What's in the scope of this function:
  // all variables, properties, methods and functions accessible at Label A
  // and all public properties and methods from the Baz class.
  };
}

// --------------------------------

class AwkwardCase {
  int? timeout;

  void execute(void Function() task) {
    task();
  }
}

void main() {
  // This case looks awkward, because you're calling execute() and then call()
  final awkwardCase = AwkwardCase();
  awkwardCase.execute { print("Working"); } { timeout = 0; };
  // Similar example code:
  // awkwardCase.execute(() => print("Working"))(() => awkwardCase.timeout = 0);
}

// --------------------------------

class Foo {
  int x = 0;
  Bar bar() => Bar();
}
class Bar {
  int x = 1;
}

void main() {
  var variableScopeExample = Foo().bar() {
    // If bar is a top-level function, then nothing extra is visible.
	// If bar is declared inside a method, then it will probably be called locally or passed as argument, for this reason nothing extra should be visible.
	// In the statement below, x is the value present in the Bar class.
    x = 2;
  };
  
  // If the value of Foo needs to be accessed inside the trailing lambda, then
  // a variable must be defined before.
  var foo = Foo();
  var bar = Bar() {
    x = foo.x;
  };
}

@Wdestroier
Copy link

Wdestroier commented Jan 15, 2022

x {print(message);} {print(message);} may mean something different from x {print(message);}; {print(message);}

Yes, the first statement is valid, but the last statement must not compile.
The same rule applies to

callFunction() => print('Hello world');
void main() { callFunction; (); } // Error: Expected an identifier, but got ')'.

If semicolons were insignificant in the compilation process, then the second statement would be valid:

void main() {
  // First statement
  final number = 0..toString();
  // Second statement
  final number = 0..;toString(); // toString is not defined.
}

Someone could chain many calls with both syntaxes

class Chain {
  dynamic value;

  Chain call([void Function()? function]) => this;
}

void main() {
  Chain()()()()(); // Valid Dart code already
  Chain(); (); (); (); (); // Invalid code
  Chain() { value = 0; } { value = 1; } { value = 2; } { value = 3; }; // Valid
  Chain() {} {} {} {}; // Valid
  Chain() {}; {}; {}; {}; // Invalid
}

I don't know how the Dart compiler works, but it was made possible in Kotlin and everyone will probably use this feature at least once in their projects...

@ykmnkmi
Copy link

ykmnkmi commented Jan 15, 2022

expression { /* ... */ } looks like labeled blocks.

@Wdestroier
Copy link

@tatumizer oh, I didn't know about code blocks...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Proposed language feature that solves one or more problems
Projects
None yet
Development

No branches or pull requests

7 participants