Skip to content

Commit 659a1a4

Browse files
authored
Merge pull request #443 from Kmeakin/smaller-span
Reduce size of `ByteRange` and `Span`
2 parents 9cbfe12 + b73d111 commit 659a1a4

File tree

12 files changed

+191
-27
lines changed

12 files changed

+191
-27
lines changed

fathom/src/core.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -713,4 +713,10 @@ mod tests {
713713
assert!(!std::mem::needs_drop::<Term<'_>>());
714714
assert!(!std::mem::needs_drop::<Term<'_>>());
715715
}
716+
717+
#[test]
718+
#[cfg(target_pointer_width = "64")]
719+
fn term_size() {
720+
assert_eq!(std::mem::size_of::<Term>(), 56);
721+
}
716722
}

fathom/src/core/semantics.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1310,4 +1310,10 @@ mod tests {
13101310
Elim::ConstMatch(..) => {}
13111311
}
13121312
}
1313+
1314+
#[test]
1315+
#[cfg(target_pointer_width = "64")]
1316+
fn value_size() {
1317+
assert_eq!(std::mem::size_of::<Value>(), 72);
1318+
}
13131319
}

fathom/src/driver.rs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,8 @@ use std::path::Path;
77

88
use crate::core;
99
use crate::core::binary::{self, BufferError, ReadError};
10-
use crate::source::{ByteRange, FileId, Span, StringInterner};
10+
use crate::files::{FileId, Files};
11+
use crate::source::{ByteRange, Span, StringInterner};
1112
use crate::surface::{self, elaboration};
1213
use crate::BUG_REPORT_URL;
1314

@@ -27,7 +28,7 @@ impl Status {
2728
}
2829

2930
pub struct Driver<'surface, 'core> {
30-
files: SimpleFiles<String, String>,
31+
files: Files<String, String>,
3132
interner: RefCell<StringInterner>,
3233
surface_scope: scoped_arena::Scope<'surface>,
3334
core_scope: scoped_arena::Scope<'core>,
@@ -47,7 +48,7 @@ impl<'surface, 'core> Driver<'surface, 'core> {
4748
interner: RefCell::new(StringInterner::new()),
4849
surface_scope: scoped_arena::Scope::new(),
4950
core_scope: scoped_arena::Scope::new(),
50-
files: SimpleFiles::new(),
51+
files: Files::new(),
5152

5253
allow_errors: false,
5354
seen_errors: RefCell::new(false),

fathom/src/files.rs

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
//! A reimplementation of `codespan-reporting::files::SimpleFiles` that uses
2+
//! `FileId` as the file id, instead of `usize`.
3+
4+
use codespan_reporting::files::{Error, SimpleFile};
5+
use std::{num::NonZeroU32, ops::Range};
6+
7+
/// File id.
8+
// 4 billion files should be enough for anyone, and `u16` doesn't save any size in `ByteRange` or `Span`.
9+
// `NonZeroU32` saves 4 bytes on the size of `Span` compared to `u32`.
10+
#[derive(Debug, Copy, Clone, PartialEq, Eq)]
11+
pub struct FileId(NonZeroU32);
12+
13+
impl TryFrom<u32> for FileId {
14+
type Error = <NonZeroU32 as TryFrom<u32>>::Error;
15+
16+
fn try_from(value: u32) -> Result<Self, Self::Error> {
17+
let id = NonZeroU32::try_from(value)?;
18+
Ok(Self(id))
19+
}
20+
}
21+
22+
impl From<FileId> for NonZeroU32 {
23+
fn from(value: FileId) -> Self {
24+
value.0
25+
}
26+
}
27+
28+
impl From<FileId> for u32 {
29+
fn from(value: FileId) -> Self {
30+
value.0.get()
31+
}
32+
}
33+
34+
impl From<FileId> for usize {
35+
fn from(value: FileId) -> Self {
36+
value.0.get() as usize
37+
}
38+
}
39+
40+
pub struct Files<Name, Source> {
41+
files: Vec<SimpleFile<Name, Source>>,
42+
}
43+
44+
impl<Name, Source> Files<Name, Source>
45+
where
46+
Name: std::fmt::Display,
47+
Source: AsRef<str>,
48+
{
49+
/// Create a new files database.
50+
pub fn new() -> Files<Name, Source> {
51+
Files { files: Vec::new() }
52+
}
53+
54+
/// Add a file to the database, returning the handle that can be used to
55+
/// refer to it again.
56+
pub fn add(&mut self, name: Name, source: Source) -> FileId {
57+
self.files.push(SimpleFile::new(name, source));
58+
let len = u32::try_from(self.files.len())
59+
.expect("Too many files (maximum amount of files is `u32::MAX`)");
60+
FileId::try_from(len).unwrap()
61+
}
62+
63+
/// Get the file corresponding to the given id.
64+
pub fn get(&self, file_id: FileId) -> Result<&SimpleFile<Name, Source>, Error> {
65+
let index = usize::from(file_id) - 1;
66+
self.files.get(index).ok_or(Error::FileMissing)
67+
}
68+
}
69+
70+
impl<'a, Name, Source> codespan_reporting::files::Files<'a> for Files<Name, Source>
71+
where
72+
Name: 'a + std::fmt::Display + Clone,
73+
Source: 'a + AsRef<str>,
74+
{
75+
type FileId = FileId;
76+
type Name = Name;
77+
type Source = &'a str;
78+
79+
fn name(&self, file_id: FileId) -> Result<Name, Error> {
80+
Ok(self.get(file_id)?.name().clone())
81+
}
82+
83+
fn source(&self, file_id: FileId) -> Result<&str, Error> {
84+
Ok(self.get(file_id)?.source().as_ref())
85+
}
86+
87+
fn line_index(&self, file_id: FileId, byte_index: usize) -> Result<usize, Error> {
88+
self.get(file_id)?.line_index((), byte_index)
89+
}
90+
91+
fn line_range(&self, file_id: FileId, line_index: usize) -> Result<Range<usize>, Error> {
92+
self.get(file_id)?.line_range((), line_index)
93+
}
94+
}

fathom/src/lib.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
// Supporting modules
77
mod alloc;
88
pub mod env;
9+
pub mod files;
910
pub mod source;
1011

1112
// Intermediate languages

fathom/src/main.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ fn unwrap_or_exit<T>(option: Option<T>) -> T {
115115
option.unwrap_or_else(|| std::process::exit(fathom::Status::Error.exit_code()))
116116
}
117117

118-
fn load_file_or_exit(driver: &mut fathom::Driver, file: PathOrStdin) -> fathom::source::FileId {
118+
fn load_file_or_exit(driver: &mut fathom::Driver, file: PathOrStdin) -> fathom::files::FileId {
119119
unwrap_or_exit(match file {
120120
PathOrStdin::StdIn => driver.load_source("<stdin>".to_owned(), std::io::stdin()),
121121
PathOrStdin::Path(path) => driver.load_source_path(&path),
@@ -124,7 +124,7 @@ fn load_file_or_exit(driver: &mut fathom::Driver, file: PathOrStdin) -> fathom::
124124

125125
fn read_bytes_or_exit(driver: &mut fathom::Driver, file: PathOrStdin) -> Vec<u8> {
126126
unwrap_or_exit(match file {
127-
PathOrStdin::StdIn => driver.read_bytes("<stdio>".to_owned(), std::io::stdin()),
127+
PathOrStdin::StdIn => driver.read_bytes("<stdin>".to_owned(), std::io::stdin()),
128128
PathOrStdin::Path(path) => driver.read_bytes_path(&path),
129129
})
130130
}

fathom/src/source.rs

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22
33
use std::ops::{Deref, DerefMut, Range};
44

5+
use crate::files::FileId;
6+
57
// Interned strings.
68
pub type StringId = string_interner::symbol::SymbolU16;
79

@@ -76,9 +78,6 @@ impl StringInterner {
7678
}
7779
}
7880

79-
/// File id.
80-
pub type FileId = usize; // TODO: use wrapper struct
81-
8281
#[derive(Debug, Clone)]
8382
pub struct Spanned<T> {
8483
span: Span,
@@ -162,7 +161,7 @@ impl From<Option<ByteRange>> for Span {
162161
}
163162

164163
/// Byte offsets into source files.
165-
pub type BytePos = usize;
164+
pub type BytePos = u32;
166165

167166
/// Byte ranges in source files.
168167
#[derive(Debug, Copy, Clone)]
@@ -208,6 +207,23 @@ impl ByteRange {
208207

209208
impl From<ByteRange> for Range<usize> {
210209
fn from(range: ByteRange) -> Self {
211-
range.start..range.end
210+
(range.start as usize)..(range.end as usize)
211+
}
212+
}
213+
214+
#[cfg(test)]
215+
mod tests {
216+
use super::*;
217+
218+
#[test]
219+
/// `ByteRange` is used a lot. Ensure it doesn't grow accidentally.
220+
fn byte_range_size() {
221+
assert_eq!(std::mem::size_of::<ByteRange>(), 12);
222+
}
223+
224+
#[test]
225+
/// `Span` is used a lot. Ensure it doesn't grow accidentally.
226+
fn span_size() {
227+
assert_eq!(std::mem::size_of::<Span>(), 12);
212228
}
213229
}

fathom/src/surface.rs

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,10 @@ use codespan_reporting::diagnostic::{Diagnostic, Label};
77
use lalrpop_util::lalrpop_mod;
88
use scoped_arena::Scope;
99

10-
use crate::source::{ByteRange, FileId, StringId, StringInterner};
10+
use crate::{
11+
files::FileId,
12+
source::{BytePos, ByteRange, StringId, StringInterner},
13+
};
1114

1215
lalrpop_mod!(
1316
#[allow(clippy::all)]
@@ -467,10 +470,10 @@ impl ParseMessage {
467470
}
468471

469472
type LalrpopParseError<'source> =
470-
lalrpop_util::ParseError<usize, lexer::Token<'source>, lexer::Error>;
473+
lalrpop_util::ParseError<BytePos, lexer::Token<'source>, lexer::Error>;
471474

472475
type LalrpopErrorRecovery<'source> =
473-
lalrpop_util::ErrorRecovery<usize, lexer::Token<'source>, lexer::Error>;
476+
lalrpop_util::ErrorRecovery<BytePos, lexer::Token<'source>, lexer::Error>;
474477

475478
fn format_expected(expected: &[impl std::fmt::Display]) -> Option<String> {
476479
use itertools::Itertools;
@@ -491,4 +494,18 @@ mod tests {
491494
assert!(!std::mem::needs_drop::<Term<'_, StringId>>());
492495
assert!(!std::mem::needs_drop::<Pattern<StringId>>());
493496
}
497+
498+
#[test]
499+
#[cfg(target_pointer_width = "64")]
500+
fn term_size() {
501+
assert_eq!(std::mem::size_of::<Term<()>>(), 32);
502+
assert_eq!(std::mem::size_of::<Term<ByteRange>>(), 56);
503+
}
504+
505+
#[test]
506+
#[cfg(target_pointer_width = "64")]
507+
fn pattern_size() {
508+
assert_eq!(std::mem::size_of::<Pattern<()>>(), 4);
509+
assert_eq!(std::mem::size_of::<Pattern<ByteRange>>(), 16);
510+
}
494511
}

fathom/src/surface/elaboration.rs

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ use crate::alloc::SliceVec;
2929
use crate::core::semantics::{self, ArcValue, Head, Telescope, Value};
3030
use crate::core::{self, prim, Const, Prim, UIntStyle};
3131
use crate::env::{self, EnvLen, Level, SharedEnv, UniqueEnv};
32-
use crate::source::{ByteRange, Span, Spanned, StringId, StringInterner};
32+
use crate::source::{BytePos, ByteRange, Span, Spanned, StringId, StringInterner};
3333
use crate::surface::elaboration::reporting::Message;
3434
use crate::surface::{distillation, pretty, BinOp, FormatField, Item, Module, Pattern, Term};
3535

@@ -484,8 +484,8 @@ impl<'interner, 'arena> Context<'interner, 'arena> {
484484

485485
for (offset, ch) in source.char_indices() {
486486
if !ch.is_ascii() {
487-
let ch_start = range.start() + 1 + offset;
488-
let ch_end = ch_start + ch.len_utf8();
487+
let ch_start = range.start() + 1 + offset as BytePos;
488+
let ch_end = ch_start + ch.len_utf8() as BytePos;
489489

490490
self.push_message(Message::NonAsciiStringLiteral {
491491
invalid_range: ByteRange::new(range.file_id(), ch_start, ch_end),
@@ -2218,3 +2218,14 @@ struct MatchInfo<'arena> {
22182218
/// The expected type of the match arms
22192219
expected_type: ArcValue<'arena>,
22202220
}
2221+
2222+
#[cfg(test)]
2223+
mod tests {
2224+
use super::*;
2225+
2226+
#[test]
2227+
#[cfg(target_pointer_width = "64")]
2228+
fn checked_pattern_size() {
2229+
assert_eq!(std::mem::size_of::<CheckedPattern>(), 32);
2230+
}
2231+
}

fathom/src/surface/elaboration/reporting.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,8 @@ use codespan_reporting::diagnostic::{Diagnostic, Label};
22
use itertools::Itertools;
33
use std::cell::RefCell;
44

5-
use crate::source::{ByteRange, FileId, StringId, StringInterner};
5+
use crate::files::FileId;
6+
use crate::source::{ByteRange, StringId, StringInterner};
67
use crate::surface::elaboration::{unification, MetaSource};
78
use crate::surface::BinOp;
89
use crate::BUG_REPORT_URL;

0 commit comments

Comments
 (0)