getsentry / pdb

A parser for Microsoft PDB (Program Database) debugging information
https://docs.rs/pdb/
Apache License 2.0
379 stars 67 forks source link

Consider adding separate structs for the variants of SymbolData and TypeData #1

Closed philipc closed 6 years ago

philipc commented 7 years ago

I think this is a bit more ergonomic for variants with large numbers of fields, so that you don't have to type them all out in the match. It also lets you pass the variant data to another function without needing to either copy the fields or do another match in that function.

Also, thanks a lot of creating this crate, it's saved me a lot of work :)

willglynn commented 7 years ago

I figured the extra fields were easy enough to ignore ({ foo, bar, .. }) if the consuming code didn't care, and the way my match-heavy code seems to flow (e.g. pdb2hpp), every parse() gets match{}ed immediately and doesn't need to escape the local scope. I find myself passing specific fields from the matched TypeData around (usually TypeIndexes), but I haven't yet felt a need to pass all of the parsed data directly.

My hesitation mostly stems from a desire to avoid adding 20-30 structs. Although, hmm… that would also allow some of those individual structs to have useful variant-specific helpers. For example, I don't think anyone will ever want to use a pdb::TypeData::FieldList as anything except an iterator, but iterating can require following into continuation FieldLists, which means constructing an iterator needs a FieldList and a &TypeFinder. That kind of helper doesn't and shouldn't exist on TypeData, but it would definitely make sense on a separate struct.

All right, you've sold me 😄