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

Start item tree #3720

Closed
wants to merge 13 commits into from
Closed

Start item tree #3720

wants to merge 13 commits into from

Conversation

matklad
Copy link
Member

@matklad matklad commented Mar 25, 2020

No description provided.

@matklad matklad mentioned this pull request Mar 25, 2020
@matklad
Copy link
Member Author

matklad commented Mar 26, 2020

Looks like this can subsume our AstIdMap and HasChildSource as well, using a nice ECS approach
2745a45

@Veetaha
Copy link
Contributor

Veetaha commented Mar 26, 2020

Data-driven design)

}

pub struct Static {
/// const _: () = ();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/const/static

@matklad matklad force-pushed the item-tree branch 2 times, most recently from 6a7fdc1 to 9cbae00 Compare March 27, 2020 13:56
@matklad
Copy link
Member Author

matklad commented Mar 27, 2020

One thing I am unsure about is how to add handle children. The first way is

struct Tree {
  structs: Arena<Struct>,
}
struct Struct {
  fields: Arena<Field>
}

The second way is

struct Tree {
  structs: Arena<Struct>,
  fields: Arena<Field>,
}

struct Struct {
  fields: Range<Idx<Field>>
}

The advantage of the first approach is that the id of field is local to struct. Adding a new field does not shift other structs. The second scheme is the opposite.

However, the second scheme seems to be simpler and more efficient (if we discount potential incrementality benefits). In particular, handling sources would be easier. 9cbae00 implements the first version, but I think I'll switch it to the second one.

@matklad
Copy link
Member Author

matklad commented Mar 27, 2020

@flodiebold do you have any thoughts on #3720 (comment) ?

@flodiebold
Copy link
Member

Hmm I'd have thought the incrementality benefits are kind of important... But I guess it really only makes a difference in the moment where one adds a new field somewhere, so it probably doesn't matter that much. And I like the uniformity of the second approach.

@matklad
Copy link
Member Author

matklad commented Mar 27, 2020

Hmm I'd have thought the incrementality benefits are kind of important...

Yeah, that's why I've started with a nested approach... OTOH, I think incrementality should not really matter in this case: for crate def map, we'll need projection query anyway, for individual items the recompute should not be too bad.

Also, super-long term I actually want to see only single global vector for all entities, and using interning to achieve stable indexes ... somehow.

pub is_macro_use: bool,
}

pub struct Function {
Copy link
Contributor

@Veetaha Veetaha Mar 27, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we store unsafety and asynchrony modifiers for Function and unsafety for Trait and Impl?

Comment on lines +154 to +155
pub export: bool,
pub builtin: bool,
Copy link
Contributor

@Veetaha Veetaha Mar 27, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd suggest unifying the booleans naming, e.g. in Import we use is_*, but here we don't


fn desugar_future_path(orig: TypeRef) -> Path {
let path = path![std::future::Future];
let mut generic_args: Vec<_> = std::iter::repeat(None).take(path.segments.len() - 1).collect();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
let mut generic_args: Vec<_> = std::iter::repeat(None).take(path.segments.len() - 1).collect();
let mut generic_args = vec![None; path.segments.len() - 1];

@matklad matklad force-pushed the item-tree branch 2 times, most recently from 1a1cf63 to d4908d5 Compare March 31, 2020 13:27
@matklad
Copy link
Member Author

matklad commented Apr 9, 2020

Status update: got sidetracked :(

@matklad
Copy link
Member Author

matklad commented Jun 2, 2020

Closing due to inactivity, although this is something we absolutely have to do eventually ;-(

My current plan is to nerd-snipe @jonas-schievink into taking over this :-)

@matklad matklad closed this Jun 2, 2020
@matklad matklad deleted the item-tree branch August 13, 2020 09:03
lnicola pushed a commit to lnicola/rust-analyzer that referenced this pull request Jul 16, 2024
Fix libc::read shim: make it write to a buffer correct amount of bytes. Add tests for new behavior

libc::read shim had a bug: if underlying real call libc::read(fd, buf, N) returns M, then
libc::read shim writes N bytes to buf instead of M. Remaining N - M bytes are filled with zeros.
This commit fixes this bug and adds tests for new behavior
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.

4 participants