Add ForeverStackStore
#3251
Add ForeverStackStore
#3251
Conversation
powerboat9
force-pushed
the
impl-store
branch
from
d3836c3 to
3c27f41
Compare
|
@CohenArthur @P-E-P @philberty thoughts? |
I'm mostly fine with this idea, but I've not yet finished reviewing it entirely. |
There was a problem hiding this comment.
It looks good, although I'd like @CohenArthur to review this PR because he's the one who brought ForeverStack in the first place and should have a better insight.
There was a problem hiding this comment.
I have style nits but up to @CohenArthur on that one or @tschwinge or @dkm to make it GCC Style
| { | ||
| Node *tmp = this; | ||
|
|
||
| while (true) |
There was a problem hiding this comment.
while true is something we should be wary of doing i think it would be btter to do something like:
bool keep_going = lambda ();
while (keep_going == KeepGoing::Yes) {
}
I just think its better to avoid just flat out while (true) but thats my style preference
There was a problem hiding this comment.
Yeah, lambda () == KeepGoing::Yes should probably just be the loop condition
There was a problem hiding this comment.
I've tweaked it to use a for loop now -- the condition is a bit odd, but it should be succinct
There was a problem hiding this comment.
I'm not against the PR, I think the code looks good but I am having a hard time wrapping my head around how this could be used. @powerboat9 could you do one big PR containing the ForeverStackStore and the usage of it by ForeverStack? I feel like it would help me a lot.
|
Actually nevermind my comment I just saw you had submitted #3258 as well - I'll review it and then come back to this PR. apologies for the hasty review |
powerboat9
force-pushed
the
impl-store
branch
from
3c27f41 to
868947a
Compare
There was a problem hiding this comment.
LGTM! if everything @philberty had to say is resolved, then I'm happy to merge this
powerboat9
force-pushed
the
impl-store
branch
from
868947a to
9c9ba56
Compare
powerboat9
force-pushed
the
impl-store
branch
from
9c9ba56 to
275ce4b
Compare
ForeverStackStore is meant to partially unify the internal states of per-namespace ForeverStack instances. This commit does not contain modifications to ForeverStack which would allow it to rely on a ForeverStackStore to store nodes, but a future commit should address this. gcc/rust/ChangeLog: * Make-lang.in: Handle rust-forever-stack.cc. * resolve/rust-forever-stack.h (class ForeverStackStore): Add. * resolve/rust-forever-stack.cc: New file, based on rust-forever-stack.hxx. Signed-off-by: Owen Avery <[email protected]>
powerboat9
force-pushed
the
impl-store
branch
from
275ce4b to
2adcc43
Compare
ForeverStackStoreis meant to partially unify the internal states of per-namespaceForeverStackinstances. This PR doesn't contain modifications toForeverStackwhich would allow it to rely on aForeverStackStoreto store nodes, but a future PR should address this.