-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
base: devel
Are you sure you want to change the base?
Conversation
The problems you encounter have a very simple solution you won't like:
on 1type
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 on 2Likewise 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 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). |
You need to remember that a concept is not an interface, your 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. ;-) |
Lets see if I understand what you are saying. In the first bit 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 |
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) |
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.
These translation forms are exactly what we need to arrive at a reasonable spec that can be analyzed properly and implemented. |
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? |
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. |
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. |
I do not see enough libs in the wild based on concepts to justify a big slowdown in the compiler. |
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. |
... this doesn't slow down the compiler. |
Don't be discouraged, your work is great! 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 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. |
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()) |
022ccc3
to
1d64528
Compare
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:
Currently, they accept the same set of implementations (as well as the
each
form but I'm going to ignore that from now on sinceeach
is supposed to be the same as generic parameter on the concept minus the obvious small differences). If you think about whatproc 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 matchp
,a
being eitherstring
orint
parameter2.) all implementations of
C1
must matchp
andp
(or overloads ofp
) must acceptstring | 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 ofC1
odd because there is no way to operate onC1
and know for sure what the implementation looks like which is a property I think is useful. It is also better forC2
to have this behavior exclusively because instantiations ofC2
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:
Writable
andBuffer
are coupled to one another andSizedWritable
is coupled toBuffer
. 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 toauto
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 aproc
that implementsw
on a particular implementation ofBuffer
that is disjoint from references ofBuffer
(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 meproc
bodies inside concept bodies vs the generic parameters of the concept itself are not handled correctly