-
Notifications
You must be signed in to change notification settings - Fork 38
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
Replace manual use of index newtypes bundles[bundle.index()]
with Index
impls and typed Vec
wrappers
#62
Comments
Could we just use |
That would likely require moving cranelift-entity out of the wasmtime repo and independently versioning it. Otherwise you are pretty much stuck with two copies in case you are compiling cranelift. |
Yeah I'd be hesitant to introduce a circular dependency in general between repositories. (Perhaps |
Or |
That's a much bigger discussion as well... at the time at least, we had good reasons to not do that: different (lighter-weight) CI here, historical precedent wrt |
Is this resolved by the upcoming Wasmtime 1.0 release? FWIW I'm already using |
If regalloc2 were to use cranelift-entity as is, you would get two copies of cranelift-entity. One used by regalloc2 from crates.io and one used by cranelift part of this repo. |
Not if cranelift-entity was 1.0, since the 2 uses would be semver-compatible and Cargo would satisfy both of them with a single dependency version. |
I don't think that works when one of the versions is from crates.io and the other is a path dependency. |
The switch to 1.0-series versions (bumping major number at each release) don't change anything here, I think; the plan is still for each release to be a semver bump wrt the last one. |
Right now, regalloc2 uses an entity-component-system sort of pattern with toplevel
Vec
s ofLiveBundle
,VRegData
, and the like, and newtype'd index wrappers likeLiveBundleIndex
,VRegIndex
, etc. We have a whole bunch of instances ofself.bundles[bundle.index()]...
.Ideally we would make
bundles
aVec
-wrapper type that has anIndex
implementation that natively takesLiveBundleIndex
, and then we could make all of these sites slightly less verbose.The text was updated successfully, but these errors were encountered: