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

Inheritance #777

Merged
merged 37 commits into from
Sep 9, 2021
Merged

Inheritance #777

merged 37 commits into from
Sep 9, 2021

Conversation

josh11b
Copy link
Contributor

@josh11b josh11b commented Aug 23, 2021

This proposal adds inheritance to classes, following this syntax:

// Abstract classes may not be instantiated, but
// may have abstract methods and be extended.
abstract class AbstractBaseClass {
  virtual fn CanBeOverridden[me: Self]() -> i32 {
    return me.data;
  }
  abstract fn PureVirtual[me: Self]() -> i32;
  fn Create(data: i32) -> partial Self {
    return {.data = data};
  }
  protected var data: i32;
}

// Classes are final by default
class FinalClass extends AbstractBaseClass {
  impl fn PureVirtual[me: Self]() -> i32 {
    return me.x;
  }
  fn Create(data: i32) -> Self {
    return {.base = AbstractBaseClass.Create(data), .x = 2 * data};
  }
  private var x: i32;
}

// Can be instantiated and extended
base class ExtensibleClass {
  // May optionally use partial types in factory functions
  protected fn CreateAsBase(data: i32) -> partial Self { ... }
  fn Create(data: i32) -> Self { ... }
}

@josh11b josh11b added the proposal A proposal label Aug 23, 2021
@josh11b josh11b requested a review from a team August 23, 2021 22:50
@google-cla google-cla bot added the cla: yes PR meets CLA requirements according to bot. label Aug 23, 2021
@josh11b josh11b marked this pull request as ready for review August 25, 2021 00:07
@josh11b josh11b requested a review from a team as a code owner August 25, 2021 00:07
Copy link
Contributor

@chandlerc chandlerc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got a first pass over this. Mostly trying to clarify wording, generally very happy with the direction here. A few places with more substantial changes, but nothing that I think is changing the direction at all (and give a shout if I've said anything that gives a different impression).

docs/design/classes.md Outdated Show resolved Hide resolved
docs/design/classes.md Outdated Show resolved Hide resolved
docs/design/classes.md Outdated Show resolved Hide resolved
docs/design/classes.md Outdated Show resolved Hide resolved
docs/design/classes.md Outdated Show resolved Hide resolved
docs/design/classes.md Outdated Show resolved Hide resolved
docs/design/classes.md Outdated Show resolved Hide resolved
docs/design/classes.md Outdated Show resolved Hide resolved
docs/design/classes.md Outdated Show resolved Hide resolved
proposals/p0777.md Show resolved Hide resolved
@github-actions github-actions bot added the proposal rfc Proposal with request-for-comment sent out label Aug 25, 2021
proposals/p0777.md Outdated Show resolved Hide resolved
josh11b and others added 2 commits August 27, 2021 14:55
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
docs/design/classes.md Outdated Show resolved Hide resolved
docs/design/classes.md Outdated Show resolved Hide resolved
docs/design/classes.md Outdated Show resolved Hide resolved
Copy link
Contributor

@chandlerc chandlerc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is basically good to go. I'd like to double check that others aren't interested in digging into it before it lands, and I'd like to officially close the blocking issues, but not seeing anything major left.

I suspect we may have some follow-up changes to tweak things, but I don't think that should hold up this version landing.

Couple of minor wording suggestions below, but none of these are really big issues, just me attempting to improve clarity.

Copy link
Contributor

@chandlerc chandlerc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor tweak below. LGTM with that, and 🎉🎉🎉 for inheritance landing!

docs/design/classes.md Outdated Show resolved Hide resolved
josh11b and others added 2 commits September 8, 2021 17:22
Co-authored-by: Chandler Carruth <chandlerc@gmail.com>
@josh11b josh11b merged commit d4e4c17 into carbon-language:trunk Sep 9, 2021
@josh11b josh11b deleted the inheritance branch September 9, 2021 00:37
@github-actions github-actions bot added proposal accepted Decision made, proposal accepted and removed proposal rfc Proposal with request-for-comment sent out labels Sep 9, 2021
chandlerc added a commit that referenced this pull request Jun 28, 2022
This proposal adds inheritance to classes, following this syntax:
```
// Abstract classes may not be instantiated, but
// may have abstract methods and be extended.
abstract class AbstractBaseClass {
  virtual fn CanBeOverridden[me: Self]() -> i32 {
    return me.data;
  }
  abstract fn PureVirtual[me: Self]() -> i32;
  fn Create(data: i32) -> partial Self {
    return {.data = data};
  }
  protected var data: i32;
}

// Classes are final by default
class FinalClass extends AbstractBaseClass {
  impl fn PureVirtual[me: Self]() -> i32 {
    return me.x;
  }
  fn Create(data: i32) -> Self {
    return {.base = AbstractBaseClass.Create(data), .x = 2 * data};
  }
  private var x: i32;
}

// Can be instantiated and extended
base class ExtensibleClass {
  // May optionally use partial types in factory functions
  protected fn CreateAsBase(data: i32) -> partial Self { ... }
  fn Create(data: i32) -> Self { ... }
}
```

Co-authored-by: Chandler Carruth <chandlerc@gmail.com>
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes PR meets CLA requirements according to bot. proposal accepted Decision made, proposal accepted proposal A proposal
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants