Skip to content

Commit

Permalink
Reject exports duplicated in two .symtypes files
Browse files Browse the repository at this point in the history
  • Loading branch information
petrpavlu committed Jan 10, 2025
1 parent c2294d5 commit dc2b37e
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 14 deletions.
46 changes: 32 additions & 14 deletions src/sym/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -420,7 +420,8 @@ impl SymCorpus {
.insert(orig_variant_name.to_string(), variant_idx);
} else {
// Insert the record.
Self::insert_record(base_name, variant_idx, file_idx, &mut records, load_context)?;
records.insert(base_name.to_string(), variant_idx);
Self::try_insert_export(base_name, file_idx, line_idx, load_context)?;
}
}

Expand Down Expand Up @@ -472,7 +473,8 @@ impl SymCorpus {
})?;

// Insert the record.
Self::insert_record(base_name, variant_idx, file_idx, &mut records, load_context)?;
records.insert(base_name.to_string(), variant_idx);
Self::try_insert_export(base_name, file_idx, line_idx, load_context)?;
}

// Add implicit references, ones that were omitted by the F# declaration because only
Expand Down Expand Up @@ -579,24 +581,40 @@ impl SymCorpus {
}
}

/// Inserts the type specified by `base_name@variant_idx` in the file `records` and registers it
/// with its `file_idx` in the `load_context.exports` if it is an exported symbol.
fn insert_record(
base_name: &str,
variant_idx: usize,
/// Checks if a specified `type_name` is an export and, if so, registers it with its `file_idx`
/// in the `load_context.exports`.
fn try_insert_export(
type_name: &str,
file_idx: usize,
records: &mut FileRecords,
line_idx: usize,
load_context: &ParallelLoadContext,
) -> Result<(), crate::Error> {
records.insert(base_name.to_string(), variant_idx);
if !Self::is_export(type_name) {
return Ok(());
}

if Self::is_export(base_name) {
// TODO Diagnose duplicates.
// Try to add the export, return an error if it is a duplicate.
let other_file_idx = {
let mut exports = load_context.exports.lock().unwrap();
exports.insert(base_name.to_string(), file_idx);
}
match exports.entry(type_name.to_string()) {
Occupied(export_entry) => *export_entry.get(),
Vacant(export_entry) => {
export_entry.insert(file_idx);
return Ok(());
}
}
};

Ok(())
let files = load_context.files.lock().unwrap();
let path = &files[file_idx].path;
let other_path = &files[other_file_idx].path;
Err(crate::Error::new_parse(&format!(
"{}:{}: Export '{}' is duplicate. Previous occurrence found in '{}'.",
path.display(),
line_idx + 1,
type_name,
other_path.display()
)))
}

/// Processes a single symbol in some file originated from an `F#` record and enhances the
Expand Down
22 changes: 22 additions & 0 deletions src/sym/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,28 @@ fn read_invalid_file_record_ref3() {
assert_parse_err!(result, "file.symtypes:3: Type 'bar@1' is not known");
}

#[test]
fn read_duplicate_type_export() {
// Check that two exports with the same name in different files get rejected.
let mut syms = SymCorpus::new();
let result = syms.load_buffer(
Path::new("test.symtypes"),
concat!(
"foo int foo ( )\n" //
)
.as_bytes(),
);
assert_ok!(result);
let result = syms.load_buffer(
Path::new("test2.symtypes"),
concat!(
"foo int foo ( )" //
)
.as_bytes(),
);
assert_parse_err!(result, "test2.symtypes:1: Export 'foo' is duplicate. Previous occurrence found in 'test.symtypes'.");
}

#[test]
fn read_write_basic() {
// Check reading of a single file and writing the consolidated output.
Expand Down

0 comments on commit dc2b37e

Please sign in to comment.