Name conflicts beyond inheritance

[josh11b]

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:

• A class can extend things other than a base class, and some member in the class can conflict with a member with the same name in the thing being extended: extending adapter, mixin, and internal impl.
• A class can extend more than one thing, and more than one of the things being extended can have a member of the same name.

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]

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]

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]

My suggested resolution is:

• Names defined in the class definition shadow names in an extension (something extended) when used without qualification.
• Otherwise a name used without qualification and found in multiple extensions is an ambiguity error.
• A definition in a class that isn't just an alias to one of the qualifications of that name may be disallowed, based on criteria like in Name conflicts between base and derived classes #2355.

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]

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]

@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]

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

It looks like the only differences in your version are:

• B.F, C4.F, and C5.F have definitions, not just declarations.
• In C4 and C5, the name conflict is resolved in favor of the declarations of F in those classes.
  Am I overlooking anything?

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]

@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.

[danakj]

My suggested resolution is:

• Names defined in the class definition shadow names in an extension (something extended) when used without qualification.

Since interfaces are public, if the shadowed name comes from an interface like I.F, should we require that the shadowing name is public as well, or else it would be an error to shadow? This would be consistent with the shadowing rules for extend base alone (#2355), so that we get the same behaviour if, say, a function moved from an interface to a base class.

This means that if an interface adds a new method that is shadowed by classes extending it, the compile does not break and existing code remains the same - calling the method in the class not the extension interface.

• Otherwise a name used without qualification and found in multiple extensions is an ambiguity error.

This feels consistent with a facet type that has multiple names that collide, which I think you also cited above. It does mean that if a class extends two interfaces, adding a method to one interface can break compile for users of that extending class. But it won't change runtime behaviour on them silently.

The other option here is I think to break compile of the class doing the extending of two colliding names, unless it shadows the new name being added to something it extends. This reduces the number of compiler errors to one in the class, instead of one-per-caller. It would require the fix to maintain behaviour to be adding alias F = Original.F in the class, whereas errors at the callers would likely push people to add qualifiers to all calls to F though the alias fix would also be available given the first bullet point here.

I think I would argue for doing the latter and breaking consistency with building a facet with &:
a) In the facet case, there's nowhere else to fix the errors, there is only the usage of the ambiguous name.
b) It's totally reasonable to combine facets together with a large number of methods and never want to use any of the colliding names.

Whereas with extend you are creating an API, which is more like a promise and feels like it should be fully self-consistent.
a) We can assume all of a class API will be used eventually, by tests or otherwise, unlike the case of an arbitrary combination of facets.
b) So then there will be a compiler error somewhere, the choice is between a single error that's brought up immediately or second-order errors all over a codebase that may come up at a much later time.

• A definition in a class that isn't just an alias to one of the qualifications of that name may be disallowed, based on criteria like in Name conflicts between base and derived classes #2355.

Were you specifically thinking of non-public visibility here, or other criteria? Or maybe leaving it open in case further criteria come in the future for base classes?

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;
}

Allowing this shadowing seems to match the semantics of #2355, which is both a simple rule to explain and consistent then for interfaces and inheritance. After all, it is hard to know what "refinement" means from the compiler.

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.

Giving a concrete example.

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

// Name conflict between two things being `extend`ed
interface M1 {
  extend impl as X {
    fn F[self:Self]() {}
  }
  extend impl as Y {
    fn F[self: Self](x: i32) -> i32 { return x; }
  }
}

Like in the class case above, I think I would argue that this should be an error inside M1 for creating two identical names F in its API with extend.

I did not understand what "allow extending an incomplete interface" means, is it this?

// The impl of `Y` extends the incomplete `X`?
interface M1 {
  extend impl as X;
  extend impl as Y {
    fn F[self: Self](x: i32) -> i32 { return x; }
  }
}

Or could you clarify what that point means?

[josh11b]

I did not understand what "allow extending an incomplete interface" means, is it this?

// The impl of `Y` extends the incomplete `X`?
interface M1 {
  extend impl as X;
  extend impl as Y {
    fn F[self: Self](x: i32) -> i32 { return x; }
  }
}

Or could you clarify what that point means?

(Edit: I was wrong, here is a better answer.)

I think it is talking about interfaces extending an interface for which we only have a forward declaration. In general, since we have decided to go with the "declare before use" approach, there is pressure to allow as many things as possible to support using an incomplete declaration instead of requiring that they be defined before use. This helps developers to break cycles and put their code in an order that they prefer. But we won't do it if it causes too much compiler complexity, for example having to re-evaluate a bunch of code once we finally see the definition.

Also note that current Carbon doesn't have the flexibility to extend impl as Foo { ... } in an interface. Only extend Foo; without a block.

So the case is:

interface X;

// The interface `Y` extends the incomplete `X`
interface Y {
  extend X;
  // Rules for name conflicts mean we don't need to
  // care whether `F` conflicts with something in `X`.
  fn F[self: Self](x: i32) -> i32 { return x; }
}

// Later definition of `X`.
interface X {
  // Can include `F` or not, fine either way.
}