Merge "PDL: Change the scoping of packet and group declarations" am: 64f59bab31 am: 4ab486883e (15b2d57a) · Commits · e / os / android_packages_modules_Bluetooth

tools/pdl/src/lint.rs

+73 −45

Original line number	Diff line number	Diff line
		@@ -27,15 +27,7 @@ struct FieldPath<'d>(Vec<&'d Field>);

		/// Gather information about the full grammar declaration.
		struct Scope<'d> {
		// Collection of Group declarations.
		groups: HashMap<String, &'d Decl>,

		// Collection of Packet declarations.
		packets: HashMap<String, &'d Decl>,

		// Collection of Enum, Struct, Checksum, and CustomField declarations.
		// Packet and Group can not be referenced in a Typedef field and thus
		// do not share the same namespace.
		// Collection of Group, Packet, Enum, Struct, Checksum, and CustomField declarations.
		typedef: HashMap<String, &'d Decl>,

		// Collection of Packet, Struct, and Group scope declarations.
		@@ -500,10 +492,11 @@ impl<'d> Scope<'d> {
		_ => (),
		}

		let (parent_id, parent_namespace, fields) = match decl {
		Decl::Packet { parent_id, fields, .. } => (parent_id, &scope.packets, fields),
		Decl::Struct { parent_id, fields, .. } => (parent_id, &scope.typedef, fields),
		Decl::Group { fields, .. } => (&None, &scope.groups, fields),
		let (parent_id, fields) = match decl {
		Decl::Packet { parent_id, fields, .. } \| Decl::Struct { parent_id, fields, .. } => {
		(parent_id.as_ref(), fields)
		}
		Decl::Group { fields, .. } => (None, fields),
		_ => return None,
		};

		@@ -514,7 +507,7 @@ impl<'d> Scope<'d> {
		for f in fields {
		match f {
		Field::Group { group_id, constraints, .. } => {
		match scope.groups.get(group_id) {
		match scope.typedef.get(group_id) {
		None => result.push(
		Diagnostic::error()
		.with_message(format!(
		@@ -523,7 +516,7 @@ impl<'d> Scope<'d> {
		))
		.with_labels(vec![f.loc().primary()]),
		),
		Some(group_decl) => {
		Some(group_decl @ Decl::Group { .. }) => {
		// Recurse to flatten the inserted group.
		if let Some(rscope) = bfs(group_decl, context, scope, result) {
		// Inline the group fields and constraints into
		@@ -531,6 +524,15 @@ impl<'d> Scope<'d> {
		lscope.inline(scope, rscope, f, constraints.iter(), result)
		}
		}
		Some(_) => result.push(
		Diagnostic::error()
		.with_message(format!(
		"invalid group field identifier `{}`",
		group_id
		))
		.with_labels(vec![f.loc().primary()])
		.with_notes(vec!["hint: expected group identifier".to_owned()]),
		),
		}
		}
		Field::Typedef { type_id, .. } => {
		@@ -555,21 +557,32 @@ impl<'d> Scope<'d> {
		}

		// Iterate over parent declaration.
		for id in parent_id {
		match parent_namespace.get(id) {
		None => result.push(
		let parent = parent_id.and_then(\|id\| scope.typedef.get(id));
		match (decl, parent) {
		(Decl::Packet { parent_id: Some(_), .. }, None)
		\| (Decl::Struct { parent_id: Some(_), .. }, None) => result.push(
		Diagnostic::error()
		.with_message(format!(
		"undeclared parent identifier `{}`",
		parent_id.unwrap()
		))
		.with_labels(vec![decl.loc().primary()])
		.with_notes(vec![format!("hint: expected {} parent", decl.kind())]),
		),
		(Decl::Packet { .. }, Some(Decl::Struct { .. }))
		\| (Decl::Struct { .. }, Some(Decl::Packet { .. })) => result.push(
		Diagnostic::error()
		.with_message(format!("undeclared parent identifier `{}`", id))
		.with_message(format!("invalid parent identifier `{}`", parent_id.unwrap()))
		.with_labels(vec![decl.loc().primary()])
		.with_notes(vec![format!("hint: expected {} parent", decl.kind())]),
		),
		Some(parent_decl) => {
		(_, Some(parent_decl)) => {
		if let Some(rscope) = bfs(parent_decl, context, scope, result) {
		// Import the parent fields and constraints into the current scope.
		lscope.inherit(scope, rscope, decl.constraints(), result)
		}
		}
		}
		_ => (),
		}

		lscope.finalize(result);
		@@ -582,7 +595,7 @@ impl<'d> Scope<'d> {
		let mut context =
		Context::<'d> { list: vec![], visited: HashMap::new(), scopes: HashMap::new() };

		for decl in self.packets.values().chain(self.typedef.values()).chain(self.groups.values()) {
		for decl in self.typedef.values() {
		bfs(decl, &mut context, self, result);
		}

		@@ -1216,28 +1229,15 @@ impl Decl {

		impl Grammar {
		fn scope<'d>(&'d self, result: &mut LintDiagnostics) -> Scope<'d> {
		let mut scope = Scope {
		groups: HashMap::new(),
		packets: HashMap::new(),
		typedef: HashMap::new(),
		scopes: HashMap::new(),
		};
		let mut scope = Scope { typedef: HashMap::new(), scopes: HashMap::new() };

		// Gather top-level declarations.
		// Validate the top-level scopes (Group, Packet, Typedef).
		//
		// TODO: switch to try_insert when stable
		for decl in &self.declarations {
		if let Some((id, namespace)) = match decl {
		Decl::Checksum { id, .. }
		\| Decl::CustomField { id, .. }
		\| Decl::Struct { id, .. }
		\| Decl::Enum { id, .. } => Some((id, &mut scope.typedef)),
		Decl::Group { id, .. } => Some((id, &mut scope.groups)),
		Decl::Packet { id, .. } => Some((id, &mut scope.packets)),
		_ => None,
		} {
		if let Some(prev) = namespace.insert(id.clone(), decl) {
		if let Some(id) = decl.id() {
		if let Some(prev) = scope.typedef.insert(id.clone(), decl) {
		result.err_redeclared(id, decl.kind(), decl.loc(), prev.loc())
		}
		}
		@@ -1264,3 +1264,31 @@ impl Lintable for Grammar {
		result
		}
		}

		#[cfg(test)]
		mod test {
		use crate::ast::*;
		use crate::lint::Lintable;
		use crate::parser::parse_inline;

		macro_rules! grammar {
		($db:expr, $text:literal) => {
		parse_inline($db, "stdin".to_owned(), $text.to_owned()).expect("parsing failure")
		};
		}

		#[test]
		fn test_packet_redeclared() {
		let mut db = SourceDatabase::new();
		let grammar = grammar!(
		&mut db,
		r#"
		little_endian_packets
		struct Name { }
		packet Name { }
		"#
		);
		let result = grammar.lint();
		assert!(!result.diagnostics.is_empty());
		}
		}

tools/pdl/src/parser.rs

+19 −8

Original line number	Diff line number	Diff line
		@@ -506,17 +506,14 @@ fn parse_grammar(root: Node<'_>, context: &Context) -> Result<ast::Grammar, Stri
		Ok(grammar)
		}

		/// Parse a new source file.
		/// The source file is fully read and added to the compilation database.
		/// Returns the constructed AST, or a descriptive error message in case
		/// of syntax error.
		pub fn parse_file(
		/// Parse a PDL grammar text.
		/// The grammar is added to the compilation database under the
		/// provided name.
		pub fn parse_inline(
		sources: &mut ast::SourceDatabase,
		name: String,
		source: String,
		) -> Result<ast::Grammar, Diagnostic<ast::FileId>> {
		let source = std::fs::read_to_string(&name).map_err(\|e\| {
		Diagnostic::error().with_message(format!("failed to read input file '{}': {}", &name, e))
		})?;
		let root = PDLParser::parse(Rule::grammar, &source)
		.map_err(\|e\| {
		Diagnostic::error()
		@@ -528,3 +525,17 @@ pub fn parse_file(
		let file = sources.add(name, source.clone());
		parse_grammar(root, &(file, &line_starts)).map_err(\|e\| Diagnostic::error().with_message(e))
		}

		/// Parse a new source file.
		/// The source file is fully read and added to the compilation database.
		/// Returns the constructed AST, or a descriptive error message in case
		/// of syntax error.
		pub fn parse_file(
		sources: &mut ast::SourceDatabase,
		name: String,
		) -> Result<ast::Grammar, Diagnostic<ast::FileId>> {
		let source = std::fs::read_to_string(&name).map_err(\|e\| {
		Diagnostic::error().with_message(format!("failed to read input file '{}': {}", &name, e))
		})?;
		parse_inline(sources, name, source)
		}