carbon-language / carbon-lang

Carbon Language's main repository: documents, design, implementation, and related tools. (NOTE: Carbon Language is experimental; see README)
http://docs.carbon-lang.dev/
Other
32.28k stars 1.48k forks source link

Name conflicts beyond inheritance #2745

Open josh11b opened 1 year ago

josh11b commented 1 year ago

Summary of issue:

2355 defines the behavior of name conflicts arising from inheritance. There are two other kinds of name conflicts that can arise in classes from using extend (of #995) to extend the API of a class:

We can have both of these issues, where there are members with the same name in the class and multiple things being extended. This is in fact a likely way people will want to resolve the ambiguity of having a name conflict in the things being extended.

What are the rules for resolving these name conflicts, and how do they interact with the restrictions from #2355?

chandlerc commented 1 year ago

My initial thoughts --

I'd be happy to generalize the rule we have for inheritance in #2355 for other cases of a class extending something and explicitly declaring a name -- the explicit name shouldn't be obviously incompatible but otherwise shadows. I don't have strong feelings on whether we want anything beyond "doesn't change the access level" for "obviously incompatible". While any restrictions here do present some evolution hazards, I feel like the balance we struck in #2355 is similarly reasonable elsewhere (to the extent it applies).

I'm much less clear on what we should do for extending two things and resolving a conflict between them.

Part of my hesitation with rejecting in the face of conflicts is that it doesn't seem like it leaves the user many good options at all.

Would be interested to hear other ideas for how we should or shouldn't handle these.

josh11b commented 1 year ago

To be concrete, here is an example:

// Both interface `I` and base class `B` define a method named `F`:
interface I { fn F[self: Self](); }
base class B { fn F[self: Self](x: i32) -> i32; }

// Name conflict between two things being `extend`ed
class C1 {
  extend base: B;
  extend impl as I;
}

// Resolve the name conflict of `C1` to use `B.F`
class C2 {
  extend base: B;
  extend impl as I;
  alias F = B.F;
}

// Resolve the name conflict of `C1` to use `I.F`
class C3 {
  extend base: B;
  extend impl as I;
  alias F = I.F;
}

// Name conflict between `I.F` and `C4.F`
class C4 {
  extend impl as I;
  fn F[me: Self]() -> bool;
}

// Name conflict between `B.F` and `C5.F`
class C5 {
  extend adapt B;
  fn F[me: Self]() -> bool;
}

To me, it seems like C1 should act like it doesn't have a member F, but it does have members B.F and I.F:

var c1: C1 = {};
c1.(B.F)(2);
c1.(I.F)();
// ❌  Error: `c1.F` is ambiguous, could mean either `B.F` or `I.F`
c1.F();

This is consistent with our approach to the & operator applied to interfaces, and other cases of member access, see #989 and https://github.com/carbon-language/carbon-lang/blob/trunk/docs/design/expressions/member_access.md#lookup-ambiguity . (At some point we will want to allow users to define C1.F to be an overload of B.F and I.F, since they take different parameter types.)

Another choice, specifically the one we chose before when implementing two interfaces internally with a name in common, was to report an error C1. This is a conservative choice, with the idea that Carbon will reject programs at compile time rather than doing something surprising. However it doesn't give many options for the user to work around problem, and seems like it may cause more programs than necessary to fail to compile in the face of evolution.

The cases of C2 and C3 I think are particularly well motivated: specifically using alias to disambiguate. We've talked about what criteria something in the class can shadow something it extends, and this use case motivates allowing an alias to shadow.

Case C4 seems the most dubious, since it seems like a conflict that could have been avoided by externally implementing I. On the other hand, it seems like something that could arise due to evolution. And it is similar to the case of C5 using an adapter, where we don't have as good other options for controlling the resulting API of the class.

It does seem like we'd like the policy to be consistent across extending an interface, a base class, and an adapter. This will make it easier to teach and remember, and make it more likely that the policy also applies when we want to answer this question for mixins in the future as well.

josh11b commented 1 year ago

My suggested resolution is:

Note that there is some extra complexity to this last point, since we may have two different things in extensions that are being "refined" by the new definition in the class, as in:

class C6 {
  extend base: B;
  extend impl as I;
  // Does this have to refine both `B.F` and `I.F`?
  fn F[me: Self]() -> bool;
}
josh11b commented 1 year ago

Another question is how extend in interfaces handles name conflicts. If the extending interface shadows the names, this could allow extending an incomplete interface. Not sure if that is an important use case.

officialhemant511 commented 12 months ago

@josh11b

To be concrete, here is an example:

// Both interface `I` and base class `B` define a method named `F`:
interface I { fn F[self: Self](); }
base class B { fn F[self: Self](x: i32) -> i32; }

// Name conflict between two things being `extend`ed
class C1 {
  extend base: B;
  extend impl as I;
}

// Resolve the name conflict of `C1` to use `B.F`
class C2 {
  extend base: B;
  extend impl as I;
  alias F = B.F;
}

// Resolve the name conflict of `C1` to use `I.F`
class C3 {
  extend base: B;
  extend impl as I;
  alias F = I.F;
}

// Name conflict between `I.F` and `C4.F`
class C4 {
  extend impl as I;
  fn F[me: Self]() -> bool;
}

// Name conflict between `B.F` and `C5.F`
class C5 {
  extend adapt B;
  fn F[me: Self]() -> bool;
}

To me, it seems like C1 should act like it doesn't have a member F, but it does have members B.F and I.F:

var c1: C1 = {};
c1.(B.F)(2);
c1.(I.F)();
// ❌  Error: `c1.F` is ambiguous, could mean either `B.F` or `I.F`
c1.F();

This is consistent with our approach to the & operator applied to interfaces, and other cases of member access, see #989 and https://github.com/carbon-language/carbon-lang/blob/trunk/docs/design/expressions/member_access.md#lookup-ambiguity . (At some point we will want to allow users to define C1.F to be an overload of B.F and I.F, since they take different parameter types.)

Another choice, specifically the one we chose before when implementing two interfaces internally with a name in common, was to report an error C1. This is a conservative choice, with the idea that Carbon will reject programs at compile time rather than doing something surprising. However it doesn't give many options for the user to work around problem, and seems like it may cause more programs than necessary to fail to compile in the face of evolution.

The cases of C2 and C3 I think are particularly well motivated: specifically using alias to disambiguate. We've talked about what criteria something in the class can shadow something it extends, and this use case motivates allowing an alias to shadow.

Case C4 seems the most dubious, since it seems like a conflict that could have been avoided by externally implementing I. On the other hand, it seems like something that could arise due to evolution. And it is similar to the case of C5 using an adapter, where we don't have as good other options for controlling the resulting API of the class.

It does seem like we'd like the policy to be consistent across extending an interface, a base class, and an adapter. This will make it easier to teach and remember, and make it more likely that the policy also applies when we want to answer this question for mixins in the future as well.

can we modify this example like this.... to resolve this name conflict error..

// Both interface `I` and base class `B` define a method named `F`:
interface I {
    fn F[self: Self]();
}

base class B {
    fn F[self: Self](x: i32) -> i32 {
        // Implementation of method F in base class B
        x * 2  // some computation
    }
}

// Name conflict between two things being `extend`ed
class C1 {
    extend base: B;
    extend impl as I;
}

// Resolve the name conflict of `C1` to use `B.F`
class C2 {
    extend base: B;
    extend impl as I;
    alias F = B.F;
}

// Resolve the name conflict of `C1` to use `I.F`
class C3 {
    extend base: B;
    extend impl as I;
    alias F = I.F;
}

// No name conflict, method F only exists in C4
class C4 {
    extend impl as I;
    fn F[me: Self]() -> bool {
        // Implementation of method F in class C4
        true
    }
}

// No name conflict, method F only exists in C5
class C5 {
    extend adapt B;
    fn F[me: Self]() -> bool {
        // Implementation of method F in class C5
        false
    }
}

ig there are many ways but we can do this also i hope this make it will help 😊....

geoffromer commented 12 months ago

@officialhemant511 FYI I've edited your comment to fix the formatting.

It looks like the only differences in your version are:

Are you suggesting that the name resolution behavior should change depending on whether the function body of F is inside the class/interface body (rather than being declared out-of-line)?

josh11b commented 12 months ago

@officialhemant511 I'm not sure if my post was clear: the goal is to show various examples where name conflicts can occur and decide what happens in each of those situations.