Skip to content
This repository has been archived by the owner on Dec 9, 2020. It is now read-only.

Add support for parsing attributes and other improvements #23

Merged
merged 54 commits into from Nov 26, 2019

Conversation

kennykerr
Copy link
Contributor

@kennykerr kennykerr commented Nov 25, 2019

This update adds the ability to parse basic built-in and custom WinRT attributes, the last remaining feature required by WinRT. It isn't complete, but it does cover the bases. What remains is substantially more testing, which I have started addressing here with some more testing infrastructure that will need to be fleshed out considerably. Notables:

  • Got rid of the "not_used" marker variants. The result is much simpler to read and doesn't confuse IntelliSense.
  • Removed a lot of dead metadata tables not used by WinRT.
  • Simplified error handling
  • Renamed Database -> File to avoid giving the wrong impression as the Database on its own not a complete representation and needs the Reader
  • The row types can now follow the breadcrumbs back to the Reader. This is key to supporting more complex queries that require type lookup (e.g. attributes)

Finally, I now include a test IDL file along with a checked-in winmd file for testing. The latter is checked in to avoid requiring MIDL as part of the build. Once the winmd parser can also produce winmd files, this will no longer be required, but until then it helps to test various edge cases without relying on recent OS metadata.

Fixes #22

@kennykerr kennykerr requested a review from rylev November 25, 2019 20:35
i8::from_le_bytes(value_bytes.try_into().unwrap())
}

fn read_u8(bytes: &mut &[u8]) -> u8 {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not too happy with these - seems very repetitive, but couldn't figure out how to write this as a generic function - much like a function template in C++.

Copy link
Contributor

Choose a reason for hiding this comment

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

You could use a macro to write these.

@kennykerr kennykerr merged commit 4159210 into master Nov 26, 2019
@kennykerr kennykerr deleted the kennykerr branch November 26, 2019 14:15
Copy link
Contributor

@rylev rylev left a comment

Choose a reason for hiding this comment

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

Looking great!

let snake = syn::Ident::new(&to_snake(&camel_name), camel.span());
let index = index as u32;
if let Some((_, value)) = &variant.discriminant {
if let syn::Expr::Lit(value) = value {
Copy link
Contributor

Choose a reason for hiding this comment

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

You can inline this if let into the one above:

if let Some((_, syn::Expr::Lit(value))) = &variant.discriminant {


pub fn split_type_name(name: &str) -> ParseResult<(&str, &str)> {
let index = name.rfind('.').ok_or_else(|| ParseError::InvalidTypeName)?;
Ok((name.get(0..index).unwrap(), name.get(index + 1..).unwrap()))
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: if you're just going to unwrap no need to use get. You can do name[0..index]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, forgot about that. 👍

use crate::tables::*;

pub struct Reader {
files: std::vec::Vec<File>,
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Vec doesn't need to be namespaced

types: &'a NamespaceData,
impl<'a> Iterator for NamespaceIterator<'a> {
type Item = Namespace<'a>;
fn next(&mut self) -> Option<Self::Item> {
Copy link
Contributor

Choose a reason for hiding this comment

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

This iterator seems like it can be accomplished using map on iterator. Is there a need for this custom iterator?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll check.

i8::from_le_bytes(value_bytes.try_into().unwrap())
}

fn read_u8(bytes: &mut &[u8]) -> u8 {
Copy link
Contributor

Choose a reason for hiding this comment

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

You could use a macro to write these.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Finish attribute value parsing
2 participants