From 6cc70015d85844e643980529b9e79f84ef4da0f8 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Fri, 5 Aug 2022 00:20:33 +0200 Subject: [PATCH 1/7] remove all uses of `unsafe` --- compiler/ast/src/common/imported_modules.rs | 21 ++- compiler/span/src/lib.rs | 2 +- compiler/span/src/source_map.rs | 19 ++- compiler/span/src/symbol.rs | 176 +++++++++----------- 4 files changed, 102 insertions(+), 116 deletions(-) diff --git a/compiler/ast/src/common/imported_modules.rs b/compiler/ast/src/common/imported_modules.rs index 3fd55a4a75..213e8234b9 100644 --- a/compiler/ast/src/common/imported_modules.rs +++ b/compiler/ast/src/common/imported_modules.rs @@ -16,7 +16,7 @@ use crate::Program; -use leo_span::Symbol; +use leo_span::{symbol::with_session_globals, Symbol}; use indexmap::IndexMap; use serde::{Deserialize, Deserializer, Serialize, Serializer}; @@ -26,13 +26,18 @@ pub fn serialize( imported_modules: &IndexMap, Program>, serializer: S, ) -> Result { - let joined: IndexMap = imported_modules - .into_iter() - .map(|(package, program)| { - let package = package.iter().map(|x| x.as_str().to_string()).collect::>(); - (package.join("."), program.clone()) - }) - .collect(); + let joined: IndexMap = with_session_globals(|s| { + imported_modules + .into_iter() + .map(|(package, program)| { + let package = package + .iter() + .map(|x| x.as_str(s, |s| s.to_owned())) + .collect::>(); + (package.join("."), program.clone()) + }) + .collect() + }); joined.serialize(serializer) } diff --git a/compiler/span/src/lib.rs b/compiler/span/src/lib.rs index 114dc29ed2..aeb3c8d3ff 100644 --- a/compiler/span/src/lib.rs +++ b/compiler/span/src/lib.rs @@ -14,7 +14,7 @@ // You should have received a copy of the GNU General Public License // along with the Leo library. If not, see . -mod dropless; +// mod dropless; pub mod symbol; pub use symbol::{sym, Symbol}; diff --git a/compiler/span/src/source_map.rs b/compiler/span/src/source_map.rs index 15ee2b10d0..3544f49ee3 100644 --- a/compiler/span/src/source_map.rs +++ b/compiler/span/src/source_map.rs @@ -368,12 +368,16 @@ fn normalize_newlines(src: &mut String) { } // Account for removed `\r`. - // After `set_len`, `buf` is guaranteed to contain utf-8 again. + // After `buf.truncate(..)`, `buf` is guaranteed to contain utf-8 again. + // N.B., this is the safe version of the below. let new_len = buf.len() - gap_len; - unsafe { - buf.set_len(new_len); - *src = String::from_utf8_unchecked(buf); - } + buf.truncate(new_len); + *src = String::from_utf8(buf).unwrap(); + // // After `set_len`, `buf` is guaranteed to contain utf-8 again. + // unsafe { + // buf.set_len(new_len); + // *src = String::from_utf8_unchecked(buf); + // } fn find_crlf(src: &[u8]) -> Option { let mut search_idx = 0; @@ -414,9 +418,10 @@ fn analyze_source_file(src: &str, source_file_start_pos: BytePos) -> (Vec. -use crate::dropless::DroplessArena; use crate::source_map::SourceMap; +use core::borrow::Borrow; use core::cmp::PartialEq; -use core::convert::AsRef; +use core::hash::{Hash, Hasher}; use core::num::NonZeroU32; use core::ops::Deref; use core::{fmt, str}; @@ -26,8 +26,6 @@ use fxhash::FxBuildHasher; use indexmap::IndexSet; use serde::{Deserialize, Deserializer, Serialize, Serializer}; use std::cell::RefCell; -use std::intrinsics::transmute; -use std::marker::PhantomData; /// A helper for `symbols` defined below. /// The macro's job is to bind conveniently usable `const` items to the symbol names provided. @@ -240,8 +238,12 @@ impl Symbol { /// Returns the corresponding `Symbol` for the given `index`. pub const fn new(index: u32) -> Self { let index = index.saturating_add(1); - // SAFETY: per above addition, we know `index > 0` always applies. - Self(unsafe { NonZeroU32::new_unchecked(index) }) + Self(match NonZeroU32::new(index) { + None => unreachable!(), + Some(x) => x, + }) + // // SAFETY: per above addition, we know `index > 0` always applies. + // Self(unsafe { NonZeroU32::new_unchecked(index) }) } /// Maps a string to its interned representation. @@ -249,13 +251,9 @@ impl Symbol { with_session_globals(|session_globals| session_globals.symbol_interner.intern(string)) } - /// Convert to effectively a `&'static str`. - /// This is a slowish operation because it requires locking the symbol interner. - pub fn as_str(self) -> SymbolStr { - with_session_globals(|session_globals| { - let symbol_str = session_globals.symbol_interner.get(self); - SymbolStr::new(unsafe { std::mem::transmute::<&str, &str>(symbol_str) }) - }) + /// Convert to effectively a `&'static str` given the `SessionGlobals`. + pub fn as_str(self, s: &SessionGlobals, with: impl FnOnce(&str) -> R) -> R { + s.symbol_interner.get(self, with) } /// Converts this symbol to the raw index. @@ -268,82 +266,19 @@ impl Symbol { } fn serde_from_symbol(index: &NonZeroU32, ser: S) -> Result { - ser.serialize_str(&Self(*index).as_str()) + with_session_globals(|sg| Self(*index).as_str(sg, |s| ser.serialize_str(s))) } } impl fmt::Debug for Symbol { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - fmt::Debug::fmt(&self.as_str(), f) + with_session_globals(|s| self.as_str(s, |s| fmt::Debug::fmt(s, f))) } } impl fmt::Display for Symbol { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - fmt::Display::fmt(&self.as_str(), f) - } -} - -/// An alternative to [`Symbol`], useful when the chars within the symbol need to -/// be accessed. It deliberately has limited functionality and should only be -/// used for temporary values. -/// -/// Because the interner outlives any thread which uses this type, we can -/// safely treat `string` which points to interner data, as an immortal string, -/// as long as this type never crosses between threads. -#[derive(Clone, Eq, PartialOrd, Ord)] -pub struct SymbolStr { - string: &'static str, - /// Ensures the type is neither `Sync` nor `Send`, - /// so that we satisfy "never crosses between threads" per above. - not_sync_send: PhantomData<*mut ()>, -} - -impl SymbolStr { - /// Create a `SymbolStr` from a `&'static str`. - pub fn new(string: &'static str) -> Self { - Self { - string, - not_sync_send: PhantomData, - } - } -} - -// This impl allows a `SymbolStr` to be directly equated with a `String` or `&str`. -impl> PartialEq for SymbolStr { - fn eq(&self, other: &T) -> bool { - self.string == other.deref() - } -} - -/// This impl means that if `ss` is a `SymbolStr`: -/// - `*ss` is a `str`; -/// - `&*ss` is a `&str` (and `match &*ss { ... }` is a common pattern). -/// - `&ss as &str` is a `&str`, which means that `&ss` can be passed to a -/// function expecting a `&str`. -impl Deref for SymbolStr { - type Target = str; - #[inline] - fn deref(&self) -> &str { - self.string - } -} - -impl AsRef for SymbolStr { - fn as_ref(&self) -> &str { - self.string - } -} - -impl fmt::Debug for SymbolStr { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - fmt::Debug::fmt(self.string, f) - } -} - -impl fmt::Display for SymbolStr { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - fmt::Display::fmt(self.string, f) + with_session_globals(|s| self.as_str(s, |s| fmt::Display::fmt(s, f))) } } @@ -383,13 +318,53 @@ pub fn with_session_globals(f: impl FnOnce(&SessionGlobals) -> R) -> R { SESSION_GLOBALS.with(f) } +/// An interned string, +/// either prefilled "at compile time" (`Static`), +/// or created at runtime (`Owned`). +#[derive(Eq)] +enum InternedStr { + /// String is stored "at compile time", i.e. prefilled. + Static(&'static str), + /// String is constructed and stored during runtime. + Owned(Box), +} + +impl Borrow for InternedStr { + fn borrow(&self) -> &str { + self.deref() + } +} + +impl Deref for InternedStr { + type Target = str; + + fn deref(&self) -> &Self::Target { + match self { + Self::Static(s) => s, + Self::Owned(s) => &s, + } + } +} + +impl PartialEq for InternedStr { + fn eq(&self, other: &InternedStr) -> bool { + self.deref() == other.deref() + } +} + +impl Hash for InternedStr { + fn hash(&self, state: &mut H) { + self.deref().hash(state); + } +} + /// The inner interner. /// This construction is used to get interior mutability in `Interner`. struct InnerInterner { - /// Arena used to allocate the strings, giving us `&'static str`s from it. - arena: DroplessArena, + // /// Arena used to allocate the strings, giving us `&'static str`s from it. + // arena: DroplessArena, /// Registration of strings and symbol index allocation is done in this set. - set: IndexSet<&'static str, FxBuildHasher>, + set: IndexSet, } /// A symbol-to-string interner. @@ -406,8 +381,8 @@ impl Interner { /// Returns an interner prefilled with `init`. fn prefill(init: &[&'static str]) -> Self { let inner = InnerInterner { - arena: <_>::default(), - set: init.iter().copied().collect(), + // arena: <_>::default(), + set: init.iter().copied().map(InternedStr::Static).collect(), }; Self { inner: RefCell::new(inner), @@ -416,30 +391,31 @@ impl Interner { /// Interns `string`, returning a `Symbol` corresponding to it. fn intern(&self, string: &str) -> Symbol { - let InnerInterner { arena, set } = &mut *self.inner.borrow_mut(); + let InnerInterner { set } = &mut *self.inner.borrow_mut(); if let Some(sym) = set.get_index_of(string) { - // Already internet, return that symbol. + // Already interned, return that symbol. return Symbol::new(sym as u32); } - // SAFETY: `from_utf8_unchecked` is safe since we just allocated a `&str`, - // which is known to be UTF-8. - let bytes = arena.alloc_slice(string.as_bytes()); - let string: &str = unsafe { str::from_utf8_unchecked(bytes) }; + // // SAFETY: `from_utf8_unchecked` is safe since we just allocated a `&str`, + // // which is known to be UTF-8. + // let bytes = arena.alloc_slice(string.as_bytes()); + // let string: &str = unsafe { str::from_utf8_unchecked(bytes) }; + // + // unsafe fn transmute_lt<'a, 'b, T: ?Sized>(x: &'a T) -> &'b T { + // transmute(x) + // } + // + // // SAFETY: Extending to `'static` is fine. Accesses only happen while the arena is alive. + // let string: &'static _ = unsafe { transmute_lt(string) }; - unsafe fn transmute_lt<'a, 'b, T: ?Sized>(x: &'a T) -> &'b T { - transmute(x) - } - - // SAFETY: Extending to `'static` is fine. Accesses only happen while the arena is alive. - let string: &'static _ = unsafe { transmute_lt(string) }; - - Symbol::new(set.insert_full(string).0 as u32) + Symbol::new(set.insert_full(InternedStr::Owned(string.into())).0 as u32) } /// Returns the corresponding string for the given symbol. - fn get(&self, symbol: Symbol) -> &str { - self.inner.borrow().set.get_index(symbol.as_u32() as usize).unwrap() + fn get(&self, symbol: Symbol, with: impl FnOnce(&str) -> R) -> R { + let set = &self.inner.borrow().set; + with(set.get_index(symbol.as_u32() as usize).unwrap()) } } From 6dbc21484f4ee7407867da2cfc52b696dc474b90 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Fri, 5 Aug 2022 00:26:35 +0200 Subject: [PATCH 2/7] delete all unsafe instead of commenting out --- compiler/span/src/dropless.rs | 175 -------------------------------- compiler/span/src/lib.rs | 2 - compiler/span/src/source_map.rs | 8 -- compiler/span/src/symbol.rs | 14 --- 4 files changed, 199 deletions(-) delete mode 100644 compiler/span/src/dropless.rs diff --git a/compiler/span/src/dropless.rs b/compiler/span/src/dropless.rs deleted file mode 100644 index 376cb8f4da..0000000000 --- a/compiler/span/src/dropless.rs +++ /dev/null @@ -1,175 +0,0 @@ -// Copyright (C) 2019-2022 Aleo Systems Inc. -// This file is part of the Leo library. - -// The Leo library is free software: you can redistribute it and/or modify -// it under the terms of the GNU General Public License as published by -// the Free Software Foundation, either version 3 of the License, or -// (at your option) any later version. - -// The Leo library is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU General Public License for more details. - -// You should have received a copy of the GNU General Public License -// along with the Leo library. If not, see . - -// Copyright Rust project developers under MIT or APACHE-2.0. - -use core::alloc::Layout; -use core::cell::{Cell, RefCell}; -use core::mem::{self, MaybeUninit}; -use core::{cmp, ptr, slice}; -use std::iter; - -// The arenas start with PAGE-sized chunks, and then each new chunk is twice as -// big as its predecessor, up until we reach HUGE_PAGE-sized chunks, whereupon -// we stop growing. This scales well, from arenas that are barely used up to -// arenas that are used for 100s of MiBs. Note also that the chosen sizes match -// the usual sizes of pages and huge pages on Linux. -const PAGE: usize = 4096; -const HUGE_PAGE: usize = 2 * 1024 * 1024; - -pub struct DroplessArena { - /// A pointer to the start of the free space. - start: Cell<*mut u8>, - - /// A pointer to the end of free space. - /// - /// The allocation proceeds from the end of the chunk towards the start. - /// When this pointer crosses the start pointer, a new chunk is allocated. - end: Cell<*mut u8>, - - /// A vector of arena chunks. - chunks: RefCell>>, -} - -unsafe impl Send for DroplessArena {} - -impl Default for DroplessArena { - #[inline] - fn default() -> DroplessArena { - DroplessArena { - start: Cell::new(ptr::null_mut()), - end: Cell::new(ptr::null_mut()), - chunks: Default::default(), - } - } -} - -impl DroplessArena { - #[inline(never)] - #[cold] - fn grow(&self, additional: usize) { - unsafe { - let mut chunks = self.chunks.borrow_mut(); - - let new_cap = if let Some(last_chunk) = chunks.last_mut() { - // If the previous chunk's len is less than HUGE_PAGE bytes, - // then this chunk will be at least double the previous chunk's size. - last_chunk.storage.len().min(HUGE_PAGE / 2) * 2 - } else { - PAGE - }; - // Also ensure that this chunk can fit `additional`. - let new_cap = cmp::max(additional, new_cap); - - let mut chunk = TypedArenaChunk::::new(new_cap); - self.start.set(chunk.start()); - self.end.set(chunk.end()); - chunks.push(chunk); - } - } - - /// Allocates a byte slice with specified layout from the current memory chunk. - /// Returns `None` if there is no free space left to satisfy the request. - #[inline] - fn alloc_raw_without_grow(&self, layout: Layout) -> Option<*mut u8> { - let start = self.start.get() as usize; - let end = self.end.get() as usize; - - let align = layout.align(); - let bytes = layout.size(); - - let new_end = end.checked_sub(bytes)? & !(align - 1); - if start <= new_end { - let new_end = new_end as *mut u8; - self.end.set(new_end); - Some(new_end) - } else { - // There's no more space since we're growing towards the start. - None - } - } - - #[inline] - pub fn alloc_raw(&self, layout: Layout) -> *mut u8 { - assert!(layout.size() != 0); - loop { - if let Some(a) = self.alloc_raw_without_grow(layout) { - break a; - } - // No free space left. Allocate a new chunk to satisfy the request. - // On failure the grow will panic or abort. - self.grow(layout.size()); - } - } - - /// Allocates a slice of objects that are copied into the `DroplessArena`, - /// returning a mutable reference to it. - /// Will panic if passed a zero-sized type. - /// - /// Panics: - /// - /// - Zero-sized types - /// - Zero-length slices - #[inline] - #[allow(clippy::mut_from_ref)] - pub fn alloc_slice(&self, slice: &[T]) -> &mut [T] { - assert!(!mem::needs_drop::()); - assert!(mem::size_of::() != 0); - assert!(!slice.is_empty()); - - let mem = self.alloc_raw(Layout::for_value::<[T]>(slice)) as *mut T; - - unsafe { - mem.copy_from_nonoverlapping(slice.as_ptr(), slice.len()); - slice::from_raw_parts_mut(mem, slice.len()) - } - } -} - -struct TypedArenaChunk { - /// The raw storage for the arena chunk. - storage: Box<[MaybeUninit]>, -} - -impl TypedArenaChunk { - #[inline] - unsafe fn new(capacity: usize) -> TypedArenaChunk { - TypedArenaChunk { - // HACK(Centril) around `Box::new_uninit_slice` not being stable. - storage: iter::repeat_with(MaybeUninit::::uninit).take(capacity).collect(), - } - } - - // Returns a pointer to the first allocated object. - #[inline] - fn start(&mut self) -> *mut T { - // HACK(Centril) around `MaybeUninit::slice_as_mut_ptr` not being stable. - self.storage.as_mut_ptr() as *mut T - } - - // Returns a pointer to the end of the allocated space. - #[inline] - fn end(&mut self) -> *mut T { - unsafe { - if mem::size_of::() == 0 { - // A pointer as large as possible for zero-sized elements. - !0 as *mut T - } else { - self.start().add(self.storage.len()) - } - } - } -} diff --git a/compiler/span/src/lib.rs b/compiler/span/src/lib.rs index aeb3c8d3ff..2eaabf105c 100644 --- a/compiler/span/src/lib.rs +++ b/compiler/span/src/lib.rs @@ -14,8 +14,6 @@ // You should have received a copy of the GNU General Public License // along with the Leo library. If not, see . -// mod dropless; - pub mod symbol; pub use symbol::{sym, Symbol}; diff --git a/compiler/span/src/source_map.rs b/compiler/span/src/source_map.rs index 3544f49ee3..49cbf73a8e 100644 --- a/compiler/span/src/source_map.rs +++ b/compiler/span/src/source_map.rs @@ -369,15 +369,9 @@ fn normalize_newlines(src: &mut String) { // Account for removed `\r`. // After `buf.truncate(..)`, `buf` is guaranteed to contain utf-8 again. - // N.B., this is the safe version of the below. let new_len = buf.len() - gap_len; buf.truncate(new_len); *src = String::from_utf8(buf).unwrap(); - // // After `set_len`, `buf` is guaranteed to contain utf-8 again. - // unsafe { - // buf.set_len(new_len); - // *src = String::from_utf8_unchecked(buf); - // } fn find_crlf(src: &[u8]) -> Option { let mut search_idx = 0; @@ -420,8 +414,6 @@ fn analyze_source_file(src: &str, source_file_start_pos: BytePos) -> (Vec unreachable!(), Some(x) => x, }) - // // SAFETY: per above addition, we know `index > 0` always applies. - // Self(unsafe { NonZeroU32::new_unchecked(index) }) } /// Maps a string to its interned representation. @@ -398,18 +396,6 @@ impl Interner { return Symbol::new(sym as u32); } - // // SAFETY: `from_utf8_unchecked` is safe since we just allocated a `&str`, - // // which is known to be UTF-8. - // let bytes = arena.alloc_slice(string.as_bytes()); - // let string: &str = unsafe { str::from_utf8_unchecked(bytes) }; - // - // unsafe fn transmute_lt<'a, 'b, T: ?Sized>(x: &'a T) -> &'b T { - // transmute(x) - // } - // - // // SAFETY: Extending to `'static` is fine. Accesses only happen while the arena is alive. - // let string: &'static _ = unsafe { transmute_lt(string) }; - Symbol::new(set.insert_full(InternedStr::Owned(string.into())).0 as u32) } From e5819d790bf3323971c65c252a326546b1bdb66d Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Fri, 5 Aug 2022 00:28:54 +0200 Subject: [PATCH 3/7] cargo clippy --- compiler/span/src/symbol.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/span/src/symbol.rs b/compiler/span/src/symbol.rs index cf957f24b4..a1605dc9d9 100644 --- a/compiler/span/src/symbol.rs +++ b/compiler/span/src/symbol.rs @@ -339,7 +339,7 @@ impl Deref for InternedStr { fn deref(&self) -> &Self::Target { match self { Self::Static(s) => s, - Self::Owned(s) => &s, + Self::Owned(s) => s, } } } From a77d3b4092a862b1612fc8aa5ac5bbd1aff64354 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Fri, 5 Aug 2022 00:36:01 +0200 Subject: [PATCH 4/7] forbid unsafe_code in all crates --- compiler/ast/src/lib.rs | 1 + compiler/compiler/src/lib.rs | 1 + compiler/core/src/lib.rs | 1 + compiler/parser/examples/input_parser.rs | 2 ++ compiler/parser/examples/parser.rs | 2 ++ compiler/parser/src/lib.rs | 1 + compiler/passes/src/lib.rs | 1 + compiler/span/src/lib.rs | 2 ++ docs/grammar/src/main.rs | 2 ++ errors/src/lib.rs | 1 + leo/lib.rs | 1 + leo/package/src/lib.rs | 1 + tests/test-framework/src/lib.rs | 1 + 13 files changed, 17 insertions(+) diff --git a/compiler/ast/src/lib.rs b/compiler/ast/src/lib.rs index 09fa915f4d..3bad4767f4 100644 --- a/compiler/ast/src/lib.rs +++ b/compiler/ast/src/lib.rs @@ -20,6 +20,7 @@ //! The [`Ast`] type is intended to be parsed and modified by different passes //! of the Leo compiler. The Leo compiler can generate a set of R1CS constraints from any [`Ast`]. +#![forbid(unsafe_code)] #![doc = include_str!("../README.md")] pub mod access; pub use self::access::*; diff --git a/compiler/compiler/src/lib.rs b/compiler/compiler/src/lib.rs index ed7d5438f7..b45fae2d7e 100644 --- a/compiler/compiler/src/lib.rs +++ b/compiler/compiler/src/lib.rs @@ -14,6 +14,7 @@ // You should have received a copy of the GNU General Public License // along with the Leo library. If not, see . +#![forbid(unsafe_code)] #![allow(clippy::module_inception)] #![allow(clippy::upper_case_acronyms)] #![doc = include_str!("../README.md")] diff --git a/compiler/core/src/lib.rs b/compiler/core/src/lib.rs index 2200c359f7..4592d1da34 100644 --- a/compiler/core/src/lib.rs +++ b/compiler/core/src/lib.rs @@ -14,6 +14,7 @@ // You should have received a copy of the GNU General Public License // along with the Leo library. If not, see . +#![forbid(unsafe_code)] #![doc = include_str!("../README.md")] mod algorithms; diff --git a/compiler/parser/examples/input_parser.rs b/compiler/parser/examples/input_parser.rs index 83026a1c59..7b52e7706c 100644 --- a/compiler/parser/examples/input_parser.rs +++ b/compiler/parser/examples/input_parser.rs @@ -14,6 +14,8 @@ // You should have received a copy of the GNU General Public License // along with the Leo library. If not, see . +#![forbid(unsafe_code)] + use leo_errors::{emitter::Handler, Result}; use leo_span::symbol::create_session_if_not_set_then; diff --git a/compiler/parser/examples/parser.rs b/compiler/parser/examples/parser.rs index 30f0ec5471..4eb8c7ef0d 100644 --- a/compiler/parser/examples/parser.rs +++ b/compiler/parser/examples/parser.rs @@ -14,6 +14,8 @@ // You should have received a copy of the GNU General Public License // along with the Leo library. If not, see . +#![forbid(unsafe_code)] + use leo_ast::Ast; use leo_errors::emitter::Handler; use leo_span::symbol::create_session_if_not_set_then; diff --git a/compiler/parser/src/lib.rs b/compiler/parser/src/lib.rs index 1c7942b417..1bf2a264db 100644 --- a/compiler/parser/src/lib.rs +++ b/compiler/parser/src/lib.rs @@ -19,6 +19,7 @@ //! This module contains the [`parse_ast()`] method which calls the underlying [`parse()`] //! method to create a new program ast. +#![forbid(unsafe_code)] #![allow(clippy::vec_init_then_push)] #![doc = include_str!("../README.md")] diff --git a/compiler/passes/src/lib.rs b/compiler/passes/src/lib.rs index e8c15ff783..131688000d 100644 --- a/compiler/passes/src/lib.rs +++ b/compiler/passes/src/lib.rs @@ -14,6 +14,7 @@ // You should have received a copy of the GNU General Public License // along with the Leo library. If not, see . +#![forbid(unsafe_code)] #![doc = include_str!("../README.md")] pub mod code_generation; diff --git a/compiler/span/src/lib.rs b/compiler/span/src/lib.rs index 2eaabf105c..07b2e4f07f 100644 --- a/compiler/span/src/lib.rs +++ b/compiler/span/src/lib.rs @@ -14,6 +14,8 @@ // You should have received a copy of the GNU General Public License // along with the Leo library. If not, see . +#![forbid(unsafe_code)] + pub mod symbol; pub use symbol::{sym, Symbol}; diff --git a/docs/grammar/src/main.rs b/docs/grammar/src/main.rs index 3e88a3678f..2ff1f0cac8 100644 --- a/docs/grammar/src/main.rs +++ b/docs/grammar/src/main.rs @@ -39,6 +39,8 @@ // ``` // +#![forbid(unsafe_code)] + use abnf::types::{Node, Rule}; use anyhow::{anyhow, Result}; use std::collections::{HashMap, HashSet}; diff --git a/errors/src/lib.rs b/errors/src/lib.rs index 63fa017f41..6b48b8086c 100644 --- a/errors/src/lib.rs +++ b/errors/src/lib.rs @@ -14,6 +14,7 @@ // You should have received a copy of the GNU General Public License // along with the Leo library. If not, see . +#![forbid(unsafe_code)] #![deny(clippy::all, clippy::missing_docs_in_private_items)] #![doc = include_str!("../README.md")] diff --git a/leo/lib.rs b/leo/lib.rs index cb7a72bf73..a796dcf891 100644 --- a/leo/lib.rs +++ b/leo/lib.rs @@ -14,6 +14,7 @@ // You should have received a copy of the GNU General Public License // along with the Leo library. If not, see . +#![forbid(unsafe_code)] #![doc = include_str!("../README.md")] pub mod commands; diff --git a/leo/package/src/lib.rs b/leo/package/src/lib.rs index 750880a45e..14b3ba46fe 100644 --- a/leo/package/src/lib.rs +++ b/leo/package/src/lib.rs @@ -14,6 +14,7 @@ // You should have received a copy of the GNU General Public License // along with the Leo library. If not, see . +#![forbid(unsafe_code)] #![doc = include_str!("../README.md")] pub mod build; diff --git a/tests/test-framework/src/lib.rs b/tests/test-framework/src/lib.rs index c9e73a96d9..81b1c4310b 100644 --- a/tests/test-framework/src/lib.rs +++ b/tests/test-framework/src/lib.rs @@ -22,6 +22,7 @@ //! To regenerate the tests after a syntax change or failing test, delete the [`tests/expectations/`] //! directory and run the [`parser_tests()`] test in [`parser/src/test.rs`]. +#![forbid(unsafe_code)] #![cfg(not(doctest))] // Don't doctest the markdown. #![doc = include_str!("../README.md")] From d56bd3084d3dd339aac7bf13b4c47dadfc0317d2 Mon Sep 17 00:00:00 2001 From: Pranav Gaddamadugu Date: Fri, 5 Aug 2022 08:16:19 -0700 Subject: [PATCH 5/7] Fix order of folded ternary expressions when handling early return statements --- compiler/ast/src/expressions/ternary.rs | 2 +- .../static_single_assignment/rename_program.rs | 16 +++++++++------- .../static_single_assigner.rs | 1 + 3 files changed, 11 insertions(+), 8 deletions(-) diff --git a/compiler/ast/src/expressions/ternary.rs b/compiler/ast/src/expressions/ternary.rs index 43c32eb748..23125d6299 100644 --- a/compiler/ast/src/expressions/ternary.rs +++ b/compiler/ast/src/expressions/ternary.rs @@ -31,7 +31,7 @@ pub struct TernaryExpression { impl fmt::Display for TernaryExpression { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - write!(f, "if {} ? {} : {}", self.condition, self.if_true, self.if_false) + write!(f, "(if {} ? {} : {})", self.condition, self.if_true, self.if_false) } } diff --git a/compiler/passes/src/static_single_assignment/rename_program.rs b/compiler/passes/src/static_single_assignment/rename_program.rs index 6057650016..773d015209 100644 --- a/compiler/passes/src/static_single_assignment/rename_program.rs +++ b/compiler/passes/src/static_single_assignment/rename_program.rs @@ -56,14 +56,15 @@ impl ProgramReconstructor for StaticSingleAssigner<'_> { .fold(last_return_expression, |acc, (guard, expr)| match guard { None => unreachable!("All return statements except for the last one must have a guard."), // Note that type checking guarantees that all expressions in return statements in the function body have the same type. - Some(guard) => match (acc, expr) { + Some(guard) => match (expr, acc) { // If the function returns tuples, fold the return expressions into a tuple of ternary expressions. - (Expression::Tuple(acc_tuple), Expression::Tuple(expr_tuple)) => { + // Note that `expr` and `acc` are correspond to the `if` and `else` cases of the ternary expression respectively. + (Expression::Tuple(expr_tuple), Expression::Tuple(acc_tuple)) => { Expression::Tuple(TupleExpression { - elements: acc_tuple + elements: expr_tuple .elements .into_iter() - .zip_eq(expr_tuple.elements.into_iter()) + .zip_eq(acc_tuple.elements.into_iter()) .map(|(if_true, if_false)| { Expression::Ternary(TernaryExpression { condition: Box::new(guard.clone()), @@ -77,10 +78,11 @@ impl ProgramReconstructor for StaticSingleAssigner<'_> { }) } // Otherwise, fold the return expressions into a single ternary expression. - (acc, expr) => Expression::Ternary(TernaryExpression { + // Note that `expr` and `acc` are correspond to the `if` and `else` cases of the ternary expression respectively. + (expr, acc) => Expression::Ternary(TernaryExpression { condition: Box::new(guard), - if_true: Box::new(acc), - if_false: Box::new(expr), + if_true: Box::new(expr), + if_false: Box::new(acc), span: Default::default(), }), }, diff --git a/compiler/passes/src/static_single_assignment/static_single_assigner.rs b/compiler/passes/src/static_single_assignment/static_single_assigner.rs index b4649a92b3..6c199eb8f0 100644 --- a/compiler/passes/src/static_single_assignment/static_single_assigner.rs +++ b/compiler/passes/src/static_single_assignment/static_single_assigner.rs @@ -38,6 +38,7 @@ pub struct StaticSingleAssigner<'a> { /// A stack of condition `Expression`s visited up to the current point in the AST. pub(crate) condition_stack: Vec, /// A list containing tuples of guards and expressions associated with early `ReturnStatement`s. + /// Note that early returns are inserted in the order they are encountered during a pre-order traversal of the AST. pub(crate) early_returns: Vec<(Option, Expression)>, } From 08b9aa0170079dbf726b36a851c2ca7874eec6ba Mon Sep 17 00:00:00 2001 From: Pranav Gaddamadugu Date: Fri, 5 Aug 2022 08:43:01 -0700 Subject: [PATCH 6/7] Regen test expectations --- tests/expectations/compiler/function/conditional_return.out | 2 +- tests/expectations/compiler/statements/assign.out | 2 +- tests/expectations/compiler/statements/multiple_returns.out | 2 +- tests/expectations/compiler/statements/mutate.out | 2 +- tests/expectations/compiler/tuple/function_early_return.out | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/expectations/compiler/function/conditional_return.out b/tests/expectations/compiler/function/conditional_return.out index a57da13fd5..3ed267e322 100644 --- a/tests/expectations/compiler/function/conditional_return.out +++ b/tests/expectations/compiler/function/conditional_return.out @@ -6,4 +6,4 @@ outputs: - initial_input_ast: ae0703890dbea144e675f85228e958d6903df0d1ebd88f16a531624270205cc2 initial_ast: 3dac7cf725df154640f7ea5979ac102b14916dc88215a69f555752f1e8051eec unrolled_ast: 3dac7cf725df154640f7ea5979ac102b14916dc88215a69f555752f1e8051eec - ssa_ast: 4245cbfd9ba6af41aaffaa237c46d126d54f558b04bc7abb36751f47b0cb0c24 + ssa_ast: b92c41b537132f3b09078850939b343fb275836ce8f4890b74f3a4390b9a6c41 diff --git a/tests/expectations/compiler/statements/assign.out b/tests/expectations/compiler/statements/assign.out index a0676ca67d..00dd337d9a 100644 --- a/tests/expectations/compiler/statements/assign.out +++ b/tests/expectations/compiler/statements/assign.out @@ -6,4 +6,4 @@ outputs: - initial_input_ast: 9f519c84609aa8fc9c90d398fdb30fe75df106dc0347ab1e3d7e947b2ab1b724 initial_ast: 90d7b657319b7b4c7151826d74e891f1c2e9722ea96527002a5b541a8bf33107 unrolled_ast: 90d7b657319b7b4c7151826d74e891f1c2e9722ea96527002a5b541a8bf33107 - ssa_ast: 74be6b9acdcf67a3ac2f593b6437867c6661fe139bd40e48c26bb56fd0ddd732 + ssa_ast: abfbbb6c7005dc4102e1cacb6e8c1f8c8477896cbbeb9fa86c7c8d27a65a76e1 diff --git a/tests/expectations/compiler/statements/multiple_returns.out b/tests/expectations/compiler/statements/multiple_returns.out index 8598df7d86..ceae0280ec 100644 --- a/tests/expectations/compiler/statements/multiple_returns.out +++ b/tests/expectations/compiler/statements/multiple_returns.out @@ -7,4 +7,4 @@ outputs: - initial_input_ast: 05aec88bcd0cad7448814728a983f3ff8cb52f9dc5f9bd464e130f18c4ae1033 initial_ast: 72f2d64a55e6db776ee3af263fe200d8dd806e4f20e27696012e49f6987a8609 unrolled_ast: 72f2d64a55e6db776ee3af263fe200d8dd806e4f20e27696012e49f6987a8609 - ssa_ast: 1792d3a73c33221886da3e41cbf14dfe10b2d7181fba0c609f43db043643416d + ssa_ast: 073290d894ce45effb6c829253cf0e36a7efeeac4cef28b286d781f49e903277 diff --git a/tests/expectations/compiler/statements/mutate.out b/tests/expectations/compiler/statements/mutate.out index 805d641190..d1591beb4f 100644 --- a/tests/expectations/compiler/statements/mutate.out +++ b/tests/expectations/compiler/statements/mutate.out @@ -7,4 +7,4 @@ outputs: - initial_input_ast: 965f2de6d6d4b0d3b2bf32e29ca196835aec7ca5803f9c6a33b8987185a5c233 initial_ast: 97eb6d68a9c10827d6420dc9013f0c391291bfb52df42439f9fb9fa30abb6a93 unrolled_ast: 97eb6d68a9c10827d6420dc9013f0c391291bfb52df42439f9fb9fa30abb6a93 - ssa_ast: 44792534cd773c5072884b90803b2ddfa50041039f2459fa1e3e48a074a1413d + ssa_ast: e28d175aae45224d73f23adf2624c0e7873169c234528ba1b399f12977044129 diff --git a/tests/expectations/compiler/tuple/function_early_return.out b/tests/expectations/compiler/tuple/function_early_return.out index 2d4c79facb..5b7737dcf5 100644 --- a/tests/expectations/compiler/tuple/function_early_return.out +++ b/tests/expectations/compiler/tuple/function_early_return.out @@ -6,4 +6,4 @@ outputs: - initial_input_ast: 2839953e7e597fbba9a151db4849720b37a3b4394bf9de22a09410013a18c9f7 initial_ast: eb6dfd0a82152f03c56709e72e4a7403b1b7a8636268a3ac70e98349299e88ec unrolled_ast: eb6dfd0a82152f03c56709e72e4a7403b1b7a8636268a3ac70e98349299e88ec - ssa_ast: 5a016f84e71e78451062d81982bfb89a6a762f02529d515ed1acf27a94a4161f + ssa_ast: dde4e8447e34c663a78a96b7f1e603fbf44578b5c4abbad858c2b784d27e42e6 From e4db5511d619d222c1f5ffdef0cc186f8664dd16 Mon Sep 17 00:00:00 2001 From: d0cd Date: Fri, 5 Aug 2022 12:22:23 -0700 Subject: [PATCH 7/7] Update compiler/ast/src/expressions/ternary.rs Co-authored-by: Collin Chin <16715212+collinc97@users.noreply.github.com> --- compiler/ast/src/expressions/ternary.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/ast/src/expressions/ternary.rs b/compiler/ast/src/expressions/ternary.rs index 23125d6299..0cf6b0be5b 100644 --- a/compiler/ast/src/expressions/ternary.rs +++ b/compiler/ast/src/expressions/ternary.rs @@ -31,7 +31,7 @@ pub struct TernaryExpression { impl fmt::Display for TernaryExpression { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - write!(f, "(if {} ? {} : {})", self.condition, self.if_true, self.if_false) + write!(f, "({} ? {} : {})", self.condition, self.if_true, self.if_false) } }