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

should Chapel allow pure virtual methods? #8566

Open
mppf opened this issue Feb 23, 2018 · 17 comments
Open

should Chapel allow pure virtual methods? #8566

mppf opened this issue Feb 23, 2018 · 17 comments

Comments

@mppf
Copy link
Member

mppf commented Feb 23, 2018

Other languages, e.g. C++ and Java, allow pure virtual methods. Currently some Chapel internal module code mocks this up with methods that halt. But, the compiler could check that a pure virtual method can never be called.

Some syntax inspirations from other languages:

  • C++ virtual int myMethod() = 0;
  • D abstract int myMethod();
  • Java abstract int myMethod()

It seems Swift does not support this mechanism and rather expects use of protocols (which are like concepts/interfaces a-la CHIP 2).

@bradcray
Copy link
Member

I'm generally in favor of adding support for pure virtual methods unless our eventual implementation of constrained generics would make them pointless.

@BryantLam
Copy link

Syntax suggestion: proc myMethod() = override, where = override will appear after all return intents, types, and where clauses, kind of like how C++ uses = 0 or = default (for "insert compiler default implementation") to replace the braced body.

@mppf
Copy link
Member Author

mppf commented Sep 20, 2019

proc myMethod() { override; } as another possibility for that, in case we don't like = here.

@dlongnecke-cray
Copy link
Contributor

dlongnecke-cray commented Feb 17, 2024

Would an attribute such as @abstractMethod be sufficient as a stepping stone here? We could add this now and perhaps deprecate it in favor of syntax in the future.

Edit: I realize that we probably can't freely deprecate attributes either 😢. Well, I'd also be fine with having both ways for the forseeable future, but I understand that may not be preferable.

@bradcray
Copy link
Member

bradcray commented Feb 17, 2024

I think syntax would be a better choice than attributes since the features is very intrinsic to how the language functions and isn't something that could be ignored. My mental model for attributes currently is "Is this part of a conversation between the programmer and the compiler/tools?" and this doesn't fit that for me.

Thinking about the expression of this for the first time in awhile:

A keyword-less approach would be to simply interpret the lack of a procedure body as an indication of a p.v.m. Currently, we disallow this for anything other than extern procedures, but we could extend it to class methods as well, so:

class C {
  proc myMethod(...args...) ...<ret-signature>..;  // where `<ret-signature>` is meant to include whatever return type signature, intent, where-clause, etc.
}

I'm not opposed to introducing a keyword as well to make it less subtle, where I'd probably pick abstract if introducing a new one:

class C {
  abstract proc myMethod(...args...) ...<ret-signature>...;
}

If we wanted to use a keyword but reuse an existing one, we could say:

class C {
  override proc myMethod(...args...) ...<ret-signature>...;
}

The downside is that normally this says "I override my parent" and here I'm abusing it to say "I must be overridden" which seems potentially too confusing. The only reason I suggest it is that it doesn't introduce a new keyword.\

I like the Bryant and Michael ideas to put the override into more of the body of the method as well. Bryant's reads very cleanly to me, but is a bit odd in that it's the only place = would be permitted after a procedure signature. Michael's is nearly as clean, but also is pretty special in that I don't think we'd want to permit override; to appear as a statement anywhere other than as a singleton body like this.

Oh, now that we have single-statement procedures, we could require Michael's to be written:

class C {
  proc myMethod(...args...) ...<ret-signature>... do override;
}

which I think feels less funny somehow—probably because it doesn't suggest other statements could also be in the body and could be pattern-matched directly in the parser.

FWIW, I'd love to resolve this and make it happen. It feels overdue.

@dlongnecke-cray
Copy link
Contributor

dlongnecke-cray commented Feb 17, 2024

I like do override; or = override;. Regardless of the form we can also introduce others such as = delete; or do delete; to indicate a particular method is deleted, see #15271.

I'm not sure if = override or do override feels stranger. When I read do override; it feels like override is a statement. In which case I'd be asking "well, why can't you just write { override; }? Whereas = override; is something I haven't seen anywhere else.

And you can totally make the argument that = override; looks like I'm assigning override to a procedure 😄.

@bradcray
Copy link
Member

Maybe it's time to put our 4-6 favorite approaches in front of the team for a post-meeting vote.

@dlongnecke-cray
Copy link
Contributor

dlongnecke-cray commented Feb 17, 2024

How about we narrow down to the following?

  • proc foo(): bar = none;
  • proc foo(): bar = 0;
  • proc foo(): bar = override;
  • abstract proc foo(): bar;
  • proc foo(): bar do override;
  • proc foo(): bar { override; }
  • override proc foo(): bar;
  • proc foo(): bar;

I've opted to drop proc foo(): bar; as I agree it's too subtle (edit: we'll keep it just in case). I'm not going to bother including virtual as I have a suspicion it won't be generally liked 😄.

Here I'd conceptually group the options into some different categories:

  • Forms that introduces a new = ... where the RHS of the = indicates what's happening to the method, such as it being made abstract.
  • A form that uses abstract, a new keyword we would add, which opens up discussion about adding keywords post-2.0.
  • Forms that try to use override as a single statement in the body.
  • Forms that try to generalize the notion of declaring a function w/o a body (as is done for extern).

@bradcray
Copy link
Member

I'd probably leave proc foo(): bar; in just in case it gets surprise love, but otherwise like the list.

@DanilaFe
Copy link
Contributor

DanilaFe commented Feb 20, 2024

I think that syntax with override is confusing. In languages where override is used for, well, overriding methods, it's usually used on child (non-abstract) methods to ensure that the parent has an appropriate method. We, on the other hand, would be using it on abstract base classes, which flips this convention.

I'm in favor of abstract, but does abstract become a keyword? Currently, it is not. I don't think we can reserve new keywords at this point. I will not champion this, but a keyword-free variant inspired by C++ is the following:

proc foo(): bar = 0;

@mppf
Copy link
Member Author

mppf commented Feb 20, 2024

FWIW, my favorite is something along the lines of proc foo(): bar = override; or proc foo(): bar = 0;. I like that this generalizes to proc foo(): bar = delete; or proc foo(): bar = none; (etc) for #15271.

I personally find these = forms less mind-bending than the alternatives.

@vasslitvinov
Copy link
Member

Of all the words mentioned above, = none communicates most clearly that the method is not defined.

@mppf
Copy link
Member Author

mppf commented Feb 20, 2024

@vasslitvinov - do you think we need to have a different way of writing a "pure virtual method" vs "method not provided"? The "method not provided" case is what I was imagining = none would do, as a way of opting out of something that is normally compiler generated (#15271). But, IMO it should not suggest that subclasses can provide it. (It might be moot for init= but is it moot for everything? What about proc serialize née proc writeThis ?)

@dlongnecke-cray
Copy link
Contributor

dlongnecke-cray commented Feb 20, 2024

I tend to agree with Michael here that we should probably reserve = none; as a way to avoid generating a method at all (personally I was imagining we might use delete, 😄), and use = override; instead. I will add = none; to the list if you want to champion it, Vass?

@vasslitvinov
Copy link
Member

The difference between the case "this method should not be provided implicitly" (15271) and the case of a pure virtual method is pretty subtle. OK, I am open to =none and =override, respectively. =delete sounds less fitting for the former... and perhaps =none is less fitting for the latter, if I think about it in just the right way. We probably want to direct users to thinking the right way by providing a mnemonic rule in the spec, like "when calling this method, its override will be dispatched to."

@dlongnecke-cray
Copy link
Contributor

Great, I've added = none to the list and heapified the list based on suggestions.

@bradcray
Copy link
Member

I'm in favor of abstract, but does abstract become a keyword? Currently, it is not. I don't think we can reserve new keywords at this point.

I think you're right that it would need to be a keyword, Daniel. In some past discussions about the Chapel language and breaking changes, users have admitted that we'd likely need to continue reserving new keywords after 2.0, and I think there are other cases on-deck that will likely require new keywords as well. But there's obviously a discussion to be had there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants