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

"interface" concepts or something... I guess #24491

Draft
wants to merge 3 commits into
base: devel
Choose a base branch
from

Conversation

Graveflo
Copy link
Contributor

@Graveflo Graveflo commented Dec 2, 2024

This is going to be hard to explain. I've been trying to figure out how more complex examples of concepts should be implemented and that lead me to trying out a bunch of things that didn't work well. In the end, to me, it seems like a small conceptual adjustment should be made to make these work.

This PR is not fully implemented. I'm drafting it for visibility and because completing it is going to require a lot more work and I want to make sure I'm not wasting my time.

Consider the following two concepts:

type
  C1 = concept
    proc p(s: Self, a: string | int)
  C2[T: string | int] = concept
    proc p(s: Self, a: T)

Currently, they accept the same set of implementations (as well as the each form but I'm going to ignore that from now on since each is supposed to be the same as generic parameter on the concept minus the obvious small differences). If you think about what proc p(s: Self, a: string | int) could mean on the atomic concept it could mean one of the following:
1.) an implementation of C1 has to match p , a being either string or int parameter
2.) all implementations of C1 must match p and p (or overloads of p) must accept string | int

This question becomes especially relevant when dealing with concepts that reference other concepts in their body, but I'll get to that later. Again currently, concepts generally use 1 as their rule regardless. I'm going to try and make a case that 2 is better. First, 1 is already covered by C2 so there is no reason to choose one or the other in the first place. Second, it makes the expectations of C1 odd because there is no way to operate on C1 and know for sure what the implementation looks like which is a property I think is useful. It is also better for C2 to have this behavior exclusively because instantiations of C2 will have additional information about an implementation bound to it's generic parameters. Last, it makes resolving complex dependencies between concepts computationally expensive and confusing.

The below example is very contrived, but for now this is the example I'm going with:

type
  Writable = concept
    proc w(b: var Buffer; s: Self): int
  Buffer* = concept
    proc w(s: var Self; data: Writable): int
  SizedWritable* = concept
    proc size(x: Self): int
    proc w(b: var Buffer, x: Self): int
  BufferImpl = object

proc w(x: var BufferImpl, d: int): int = return 100
proc size(d: int): int = sizeof(int)

proc p(b: var Buffer, data: SizedWritable): int =
  b.w(data)

var b = BufferImpl()
echo p(b, 5)

Writable and Buffer are coupled to one another and SizedWritable is coupled to Buffer. Situations like this, some simpler, some more complex have been giving me grief with the current concept implementation. Either from recursion errors, abysmal performance or needing to auto excessively to get it to compile. Again, I know they are not finished yet, I'm just saying: I tried to get the old behaviors to handle this stuff and I couldn't figure it out.

Another problem with situations like this in implementations like 1 (above) is that only one example proc needs to match for the concept to call the binding valid. This is not really a problem when there is a single concept and one potential implementation, but if there are more, or a concept is being evaluated inside a concept the compiler needs to ask, for example, "is X a SizedWritable." If it proves this by observing a proc that implements w on a particular implementation of Buffer that is disjoint from references of Buffer (even within the same concept) it starts to become confusing what the compiler is even trying to prove and why.

It seems to me, like there are valid reasons to express "All implementations must handle some form X" as well as "An implementation must handle some form of X." I think that using container types (or each - pretty much the same thing) and generic parameters are the answer to the latter and something like this PR can answer the former.

There are more examples in the test cases. I will be adding more as I try and work through this. That is, unless I am made aware that I am being dumb :P

The reason I didn't make an RFC first is mostly because I need to understand what's going on and I'm only going to get there from messing around with the compiler. I will make an RFC is that is the guidance. Also, new-style concepts being new, have several things that still need to be implemented and a lot of the implementation falls into grey areas so I'm not exactly sure if any of this is even controversial. On that note of things that need to be done here is a short list of things that are not done yet / are incorrect in this PR. This is here just so that they are not confusing:

  • each is hacked into the compiler in a very lazy way. I'm unfamiliar with how to do this right for now and I didn't want that blocking me
  • generic parameters in the proc bodies inside concept bodies vs the generic parameters of the concept itself are not handled correctly
  • generic parameters of the concepts need some special cases
  • there are incorrect things that I haven't fixed yet (like concept subset comparison in the wrong place). I'll get to those through testing
  • there are comments and potentially dead code that I'll clean up once I get around to it. I'm trying not to forget what to do later.

@Araq
Copy link
Member

Araq commented Dec 2, 2024

The problems you encounter have a very simple solution you won't like:

  1. Disallow the | operator in concepts.
  2. Disallow using a concept within a concept in general. As a special case allow a generic concept to use an atomic concept so that "container of Sortable" remains to be possible.

on 1

type
  C1 = concept
    proc p(s: Self, a: string | int)

If you think about this, this is not a reasonable concept to begin with. Most likely what is meant is that "we need a p that can handle a string and we need a p that can handle an int":

type
  C1 = concept
    proc p(s: Self, a: string)
    proc p(s: Self, a: int)

If the concrete type happens to have only one p that used the | operator the concrete type needs to be patched so that it fulfils the concept's requirements. That is not a problem worth solving as in general it cannot be solved as the type could offer the feature under a different name entirely and then we're stuck anyway.

on 2

Likewise

type
  Writable = concept
    proc w(b: var Buffer; s: Self): int
  Buffer* = concept
    proc w(s: var Self; data: Writable): int
  SizedWritable* = concept
    proc size(x: Self): int
    proc w(b: var Buffer, x: Self): int
  BufferImpl = object

proc w(x: var BufferImpl, d: int): int = return 100
proc size(d: int): int = sizeof(int)

proc p(b: var Buffer, data: SizedWritable): int =
  b.w(data)

var b = BufferImpl()
echo p(b, 5)

is not a good design either. A concept is a concrete set of methods a type has to offer. In reality your SizedWritable must collapse into the following:

type
  SizedWritable*[Buffer] = concept
    proc size(x: Self): int
    proc w(b: var Buffer, x: Self): int

which is far easier to understand and likely works with today's compiler.

Because otherwise you effectively force the implementers of concrete implementations to use your concepts in their proc signatures which means the client-side code becomes much harder to understand (not to mention the resulting code bloat and compiler sweat).

@Araq
Copy link
Member

Araq commented Dec 2, 2024

You need to remember that a concept is not an interface, your Buffer "concept" cannot be used in a seq[Buffer] at runtime (which is pretty much the first issue that will pop up by you or your users ;-) ), it is rather worthless as an abstraction itself. Its entire purpose is to narrow down the list of requirements a generic routine has, eventually the goal is to typecheck generic routines.

As such a concept typically lists a few proc headers and should "inline" other concepts directly so that you don't end up in a recursive mess that requires the compiler and your readers to get a degree in computer science. ;-)

@Graveflo
Copy link
Contributor Author

Graveflo commented Dec 2, 2024

Lets see if I understand what you are saying. In the first bit tyOr is just an example, because composite typeclasses like A from A[T], tyAnything, typedesc, object etc have this same problem. Essentially, you are saying that atomic concepts should never touch anything that is not concrete.
I know my example wasn't good and don't have a good one, but just so we are clear, I am not trying to say that the concept should manifest polymorphism.

I am trying to understand and take into account how this would affect type checking generics but I can't say that I fully understand what your plan is for it. At least it doesn't seem like concepts that are not fully concrete would throw a wrench into anything since it can be checked if they are concrete or not. I think that is something that must be done on non-concepts too.

Would container concepts ever be constrained by other concepts like? :

type
  SizedWritable*[B: Buffer] = concept
    proc size(x: Self): int
    proc w(b: var B, x: Self): int

@Graveflo
Copy link
Contributor Author

Graveflo commented Dec 2, 2024

Also if I am getting the right feel for what you are trying to do instantiated container concepts should never have non concrete definitions when the type variable is substituted. Like:

type
  C[T] = concept
    proc w(a: A[T], b: object)

would be invalid or at least translated to it's valid form? :

type
  C[T;O] = concept
    proc w(a: A[T], b: O)

@Araq
Copy link
Member

Araq commented Dec 2, 2024

Would container concepts ever be constrained by other concepts like?

I don't know. But what I do know is that an "implementation that is restricted to the cases we know don't cause the compiler to solve an NP complete problem" is totally worth it, even if that means we're too cautious in the beginning and rule out combinations that later turn out to be useful and tame.

would be invalid or at least translated to it's valid form?

These translation forms are exactly what we need to arrive at a reasonable spec that can be analyzed properly and implemented.

@Graveflo
Copy link
Contributor Author

Graveflo commented Dec 2, 2024

I'm not sure I understand how unconstrained generic variables help with type checking. The way I'm thinking about it, the problem, (IE why essentially forward declaring generic proc signatures in the concept) is not good enough for type checking is that they may be "out-specialized" by some other binding that makes the expression ambiguous before instantiation. The return type being different for example. Am I right in that being the "main problem"?

I'm just going to use recursive terminology because it's easier for me. You want an expression (in a generic proc body or w/e) and it's generic dependencies to have concrete (bound or forward declared), non-overload-able leafs so that everything resolves to a concrete type before having the real symbols in scope. Right?

If that is the case, I totally get that. However, I can't fathom that being the "only way it works" because of the excessive boiler plate needed to roll generic code into atomic concepts. Again, not saying it's a bad idea, and I think smart people will take full advantage of those mechanics, but I also think that it's equally important to have power. I would argue that concepts would be missing a ton of QoL if they weren't at least capable of breaking those rules. It's simple check if the concept satisfies this. It would also be obvious at the time of type checking. I understand these new-style concepts are designed to be a vehicle that makes the nice cases work and perhaps teaches people to use them, but are you against the idea of them retaining more of their uglier powerful features that are not specifically designed to aide in type checking generics?

@Araq
Copy link
Member

Araq commented Dec 2, 2024

The major problems that I see are the ones you reported; slow compiles, obscure error messages, lots of gotchas. And that is particularly troubling for me because we had the same problems with the old-style concepts. The new ones were supposed to be much easier. Thus I stress to focus on restrictions that avoid the complexity and guide the users at the same time.

@Graveflo Graveflo changed the title Strict atomic concepts "interface" concepts or something... I guess Dec 2, 2024
@Graveflo
Copy link
Contributor Author

Graveflo commented Dec 2, 2024

When I was talking about the problematic behavior, that is to be expected since the concept code hardly checks for validation, but I was trying to say that this PR doesn't have many of those problems. It wasn't hard to do either. I don't think that the buggy behavior and issues that are associated with the old concepts are a given for any design similar to them. It seems clear now that this PR has pretty much nothing to do with new-style concepts.

What is also clear is that the new style concept code has a serious invalid code acceptable problem and that does not square with the air of rigidity that surrounds conversation of them and that needs to be fixed.

@juancarlospaco
Copy link
Collaborator

I do not see enough libs in the wild based on concepts to justify a big slowdown in the compiler.
Theres some libs that mimic Interfaces using macros, maybe can be added to stdlib, at lest do not require code in the core.

@Graveflo
Copy link
Contributor Author

Graveflo commented Dec 2, 2024

This being not relevant to new-style concepts I'm not anticipating it being taken seriously at this point, but I will say this: Old style concepts have issues with them that make them so broken I cannot use them in my projects. I'm unsure if new style concepts will be a viable substitute but I am more then willing to learn to use them and try it out. For now, this style of concept does currently work for me in all of my projects that use concepts and for that reason I'm still interested in it for now.

@Graveflo
Copy link
Contributor Author

Graveflo commented Dec 2, 2024

I do not see enough libs in the wild based on concepts to justify a big slowdown in the compiler. Theres some libs that mimic Interfaces using macros, maybe can be added to stdlib, at lest do not require code in the core.

... this doesn't slow down the compiler.

@Araq
Copy link
Member

Araq commented Dec 3, 2024

For now, this style of concept does currently work for me in all of my projects that use concepts and for that reason I'm still interested in it for now.

Don't be discouraged, your work is great! I want to ensure we're moving in the right direction though.

@Araq
Copy link
Member

Araq commented Dec 3, 2024

What is also clear is that the new style concept code has a serious invalid code acceptable problem and that does not square with the air of rigidity that surrounds conversation of them and that needs to be fixed.

Certainly! Please give an example for the "invalid code is accepted" problem as this is most concerning.

@Graveflo
Copy link
Contributor Author

Graveflo commented Dec 3, 2024

I want to ensure we're moving in the right direction though
Certainly! Please give an example for the "invalid code is accepted" problem as this is most concerning.

Sure, l'm planning on adding some error messages for this. Here is an example:

type
  C1 = concept
    proc p(a: Self, x: int | float)
  A = object
proc p(a: A, x: int)= discard
proc spring(c: C1)= discard
spring(A())

same goes for tyNot, tyAnything, tyGenericParam like proc p[T](a: Self, x: T) and anything else that is handled by matchType because it doesn't differentiate between containers and atomics as far as I know. and generally, I think there are going to be types that are acceptable as constraints on container type's generic variables that are not acceptable in atomic types. I'm also unsure if tyOr in container types either but that is less relevant.

of course this could be transformed to:

type
  C1[T: int | float] = concept
    proc p(a: Self, x: T)
  A = object
proc p(a: A, x: int)= discard
proc spring(c: C1)= discard
spring(A())

but it's not clear to me when this transformation isn't problematic or if we are even trying to offer that feature at this point. It seems like it would be more in line with what you want to make container concepts explicit and reject atomic types that have forms that are not atomic.

I also can't help but think that container types' variables should be allowed to be constrained by other concepts, but I guess that is a detail that doesn't matter much right now. Anyway, I'm just saying that if there are more "rules" as to what is acceptable in the concept body there should be error messages. I'll just guess what that is for now.

@Graveflo
Copy link
Contributor Author

Graveflo commented Dec 3, 2024

also, container types that do not resolve to concrete definitions when their parameters are substituted cause problems as far as I understand it. Like this:

type
  C1[T] = concept
    proc p(a: Self, x: T, h: auto)
  A = object
proc p(a: A, x: int, h: int)= discard
proc spring(c: C1)= discard
spring(A())

or maybe it's just generic return values that are a problem in this case? idk

type
  C1[T] = concept
    proc p[A](a: Self, x: T, h: auto): A
  A = object
proc p(a: A, x: int, h: int): int= discard
proc spring(c: C1)= discard
spring(A())

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

Successfully merging this pull request may close these issues.

3 participants