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

Implement support for IPI types #51

Merged
merged 6 commits into from
Sep 5, 2019
Merged

Implement support for IPI types #51

merged 6 commits into from
Sep 5, 2019

Conversation

jan-auer
Copy link
Member

@jan-auer jan-auer commented Sep 2, 2019

This PR exposes the ID stream (IPI).

The TPI and IPI streams share the same structure, but they contain different records. Most importantly, the type of index used indicates whether a type lives in the IPI or the TPI. In microsoft-pdb this is either CV_typ_t for types, or CV_ItemId for ids. Of course, both are u32.

To create a clear interface that avoids confusion, identifiers are clearly separated into TypeIndex(u32) and IdIndex(u32). Since now the id stream needs the same root struct, iterator and finder, this is abstracted into ItemInformation<I>, ItemIter<I> and ItemFinder<I>, where I is the index being used. On top of that, there are type definitions to simplify the handling of such types: type Type<'t> = Item<'t, TypeIndex>, etc.

Apart from some renaming for consistency, usage stays mostly the same.

Unrelated, this PR contains two more fixes. They can be moved into separate PRs if desired:

  • Flags and modes of pointer types have been implemented (and fixed)
  • Some types have a "unique name", i.e. the partially mangled type name. It is exposed now.

@jan-auer
Copy link
Member Author

jan-auer commented Sep 3, 2019

@willglynn Before I finish this up, wondering if you have reservations against the general approach of splitting the streams up into an explicit TPI and IPI stream rather than merging them magically internally.

Unforunately, it's not possible to know which stream to use by just looking at the numbers. That means, that it would be necessary to look in both streams and then prefer one result over the other. However, at the declaration side, it is always clear which stream the index refers to, hence the differentiation.

Copy link
Collaborator

@willglynn willglynn left a comment

Choose a reason for hiding this comment

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

Great work!

I favor distinguishing TypeIndex(u32) and IdIndex(u32), mostly because I favor having fewer u32s to mix up. And as you note, each field containing such a reference is strongly typed already, and the TPI and IPI streams are separate at the MSF level anyway. Modeling them separately seems appropriate despite their internal similarities.

No worries about the little changes mixed in here. We'll need to cut a release once your current body of work lands, and there aren't any other projects currently in flight, so separate vs combined is all the same to me.

src/pdb.rs Outdated Show resolved Hide resolved
src/symbol/annotations.rs Outdated Show resolved Hide resolved
src/symbol/annotations.rs Outdated Show resolved Hide resolved
src/symbol/annotations.rs Outdated Show resolved Hide resolved
src/symbol/mod.rs Outdated Show resolved Hide resolved
src/tpi/data.rs Outdated Show resolved Hide resolved
@jan-auer
Copy link
Member Author

jan-auer commented Sep 5, 2019

@willglynn All feedback should be addressed now.
I hope that the type defs don't make it odd to use TypeIndex now.

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.

None yet

2 participants