From 52f739a3b0d506634527bc6d132adf1e8c0dc7dd Mon Sep 17 00:00:00 2001 From: Daniel Burgener Date: Fri, 29 Sep 2023 14:19:28 -0400 Subject: [PATCH 1/2] Clean up alias_map.rs * Remove unneeded dead_code warning silencer * Drop inaccurate comment --- src/alias_map.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/alias_map.rs b/src/alias_map.rs index ffb0b169..b5a73106 100644 --- a/src/alias_map.rs +++ b/src/alias_map.rs @@ -12,7 +12,6 @@ use crate::util::append_set_map; #[derive(Clone, Debug)] pub struct AliasMap { declarations: BTreeMap, - #[allow(dead_code)] aliases: BTreeMap, // Secondary_indices allow efficient lookups based on some other key. This is useful to // efficiently get a subset of the map based on a prepopulated key, such as all functions for a @@ -156,8 +155,6 @@ impl AliasMap { errors.into_result(()) } - // The need for alias_files is a little awkward. Without it, we can't report errors on the - // alias declarations. Post-0.1 we'll include file info in CascadeStrings and this can go away pub fn set_aliases(&mut self, aliases: BTreeMap) { self.update_alias_secondary_indices(&aliases); self.aliases = aliases; From 60d7c109e3230bf1801f94d3a11eaabff8fa39fc Mon Sep 17 00:00:00 2001 From: Daniel Burgener Date: Fri, 29 Sep 2023 15:28:43 -0400 Subject: [PATCH 2/2] Return an error when an alias matches another alias We previously covered this for aliases matching real values, but the existing aliases are already discarded by the time of that check. --- data/error_policies/duplicate_alias.cas | 7 ++++ src/compile.rs | 43 ++++++++++++++++++++----- src/lib.rs | 9 ++++-- 3 files changed, 49 insertions(+), 10 deletions(-) create mode 100644 data/error_policies/duplicate_alias.cas diff --git a/data/error_policies/duplicate_alias.cas b/data/error_policies/duplicate_alias.cas new file mode 100644 index 00000000..c0c492f5 --- /dev/null +++ b/data/error_policies/duplicate_alias.cas @@ -0,0 +1,7 @@ +@alias(some_alias) +domain foo { + allow(this, resource, file, read); +} + +@alias(some_alias) +domain bar {} diff --git a/src/compile.rs b/src/compile.rs index 1e1a9687..f6d58335 100644 --- a/src/compile.rs +++ b/src/compile.rs @@ -1259,7 +1259,7 @@ pub fn get_reduced_infos( mark_non_virtual_children(&mut new_type_map); // Generate type aliases for the new reduced type map - let (new_t_aliases, alias_files) = collect_aliases(new_type_map.iter()); + let (new_t_aliases, alias_files) = collect_aliases(new_type_map.iter())?; new_type_map.validate_aliases(&new_t_aliases, &alias_files)?; new_type_map.set_aliases(new_t_aliases); @@ -1403,7 +1403,7 @@ pub fn get_funcs<'a>( // Stops if something went wrong for this major step. ret = ret.into_result_self()?; // Get function aliases - let (f_aliases, alias_files) = collect_aliases(reduced_func_map.iter()); + let (f_aliases, alias_files) = collect_aliases(reduced_func_map.iter())?; reduced_func_map.validate_aliases(&f_aliases, &alias_files)?; reduced_func_map.set_aliases(f_aliases); ret.into_result(reduced_func_map) @@ -2084,12 +2084,18 @@ fn organize_type_map(types: &TypeMap) -> Result, CascadeErrors> { // in the maps // We gather files, because we aren't storing file info in CascadeStrings yet. That can go away // once we store file info in CascadeStrings +// Silence the clippy warning about the return type. It's not *that* complex, and it will get +// simpler naturally once we do the above +#[allow(clippy::type_complexity)] pub fn collect_aliases<'a, I, T>( aliasable_map: I, -) -> ( - BTreeMap, - BTreeMap>, -) +) -> Result< + ( + BTreeMap, + BTreeMap>, + ), + CascadeErrors, +> where I: Iterator, T: Declared + 'a, @@ -2097,10 +2103,31 @@ where { let mut aliases = BTreeMap::new(); let mut alias_files = BTreeMap::new(); + let mut errors = CascadeErrors::new(); for (k, v) in aliasable_map { for a in v.get_annotations() { if let AnnotationInfo::Alias(a) = a { - aliases.insert(a.clone(), k.clone()); + if aliases.insert(a.clone(), k.clone()).is_some() { + errors.append( + ErrorItem::make_compile_or_internal_error( + "Alias name conflicts with an existing alias", + v.get_file().as_ref(), + a.get_range(), + "", + ) + .maybe_add_additional_message( + alias_files.get(a), + // This is the range of the *existing* key. Insert updates the + // value, but not the key when we overwrite. Since our PartialEq + // isn't identical (it just compares the strings, not the ranges), + // we still have the old range in the key + // https://doc.rust-lang.org/std/collections/struct.BTreeMap.html#method.insert + aliases.get_key_value(a).and_then(|(a, _)| a.get_range()), + "Existing alias found here", + ) + .into(), + ); + } if let Some(file) = v.get_file().clone() { alias_files.insert(a.clone(), file); } @@ -2108,7 +2135,7 @@ where } } - (aliases, alias_files) + errors.into_result((aliases, alias_files)) } pub fn call_associated_calls<'a>( diff --git a/src/lib.rs b/src/lib.rs index 256a2dc4..fdcb1ff1 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -146,7 +146,7 @@ fn compile_machine_policies_internal( } // Generate type aliases - let (t_aliases, alias_files) = compile::collect_aliases(type_map.iter()); + let (t_aliases, alias_files) = compile::collect_aliases(type_map.iter())?; type_map.validate_aliases(&t_aliases, &alias_files)?; type_map.set_aliases(t_aliases); @@ -233,7 +233,7 @@ fn compile_machine_policies_internal( compile::validate_modules(&policies, &type_map, &mut module_map)?; // Generate module aliases - let (m_aliases, alias_files) = compile::collect_aliases(module_map.iter()); + let (m_aliases, alias_files) = compile::collect_aliases(module_map.iter())?; module_map.validate_aliases(&m_aliases, &alias_files)?; module_map.set_aliases(m_aliases); @@ -1672,6 +1672,11 @@ mod tests { error_policy_test!("derive_noderive.cas", 1, ErrorItem::Compile(_)); } + #[test] + fn duplicate_alias_test() { + error_policy_test!("duplicate_alias.cas", 1, ErrorItem::Compile(_)); + } + #[test] fn valid_nested_alias() { valid_policy_test(