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.

Sign up for GitHub

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

Jump to bottom

Setters should return their argument #54

Open
ChayimFriedman2 opened this issue Nov 6, 2020 · 2 comments · May be fixed by #83
Open

Setters should return their argument #54

ChayimFriedman2 opened this issue Nov 6, 2020 · 2 comments · May be fixed by #83

Comments

@ChayimFriedman2
Copy link

ChayimFriedman2 commented Nov 6, 2020 •
edited
Loading

In Wren, assignment returns its value, like in C:

var a
System.print(a = 5) // 5

When implementing setters, we need to maintain this behavior:

// Not good
class C {
  static a=(value) {
    __a = value
  }
}
System.print(C.a = 5) // null

// Good
class C {
  static a=(value) { __a = value }
}
System.print(C.a = 5) // 5

// Also good
class C {
  static a=(value) {
    __a = value
    return value
  }
}
System.print(C.a = 5) // 5

Generally, the CLI do that, except in one foreign setter - Stdin.isRaw=(_):

wren-cli/src/module/io.c

Lines 509 to 525 in b82cf5a

void stdinIsRawSet(WrenVM* vm)
{
initStdin();
isStdinRaw = wrenGetSlotBool(vm, 1);
if (uv_guess_handle(stdinDescriptor) == UV_TTY)
{
uv_tty_t* handle = (uv_tty_t*)stdinStream;
uv_tty_set_mode(handle, isStdinRaw ? UV_TTY_MODE_RAW : UV_TTY_MODE_NORMAL);
}
else
{
// Can't set raw mode when not talking to a TTY.
// TODO: Make this a runtime error?
}
}

Instead we should do:

void stdinIsRawSet(WrenVM* vm)
{
  initStdin();
  
  isStdinRaw = wrenGetSlotBool(vm, 1);
  
  if (uv_guess_handle(stdinDescriptor) == UV_TTY)
  {
    uv_tty_t* handle = (uv_tty_t*)stdinStream;
    uv_tty_set_mode(handle, isStdinRaw ? UV_TTY_MODE_RAW : UV_TTY_MODE_NORMAL);
  }
  else
  {
    // Can't set raw mode when not talking to a TTY.
    // TODO: Make this a runtime error?
  }

  wrenEnsureSlots(vm, 1); // Is that required? Probably not
  wrenSetSlotBool(vm, 0, isStdinRaw);
}
@ChayimFriedman2 ChayimFriedman2 mentioned this issue Nov 6, 2020
Open
@joshgoebel
Copy link
Contributor

While the other issue is being discussed (adding the API) would you like to go ahead and make a PR for this? It seems the agreement was reached that this is the correct behavior at least. :-)

joshgoebel added a commit to joshgoebel/wren-console that referenced this issue Apr 28, 2021
@joshgoebel
(chore) consistent setter behavior
cb381bf 
- Resolves wren-lang#54
@joshgoebel joshgoebel linked a pull request Apr 28, 2021 that will close this issue
Open
@aosenkidu
Copy link

aosenkidu commented Feb 24, 2023 •
edited
Loading

Interesting point of view. As we usually do not really "use" setters, but what other languages call "property access".
Aka, we pretend there is no setter and we could write to the field/attribute directly:
obj.property = value

hence we have the idea from C etc. that:
var x = obj.property = value
assigns value to the variabel x after calling the setter property on the object obj.

In my opinion the compiler should take care on the level of that expression that this is the case. And there is no reason that the setter is returning its argument.

In fact the setter should return "this". And that also should be the responsibility of the compiler. So you can write:
obj.propertyOne(value).propertyTwo(anotherValue)

Albeit, while I'm typing this: I wonder if you can call a setter in wren, or if a setter can only be accessed via "="?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

(chore) consistent setter behavior joshgoebel/wren-console
3 participants
@joshgoebel @ChayimFriedman2 @aosenkidu