Skip to content

Commit

Permalink
refactor: make struct name index constructor private
Browse files Browse the repository at this point in the history
  • Loading branch information
georgemitenkov committed Jan 15, 2025
1 parent 3e5b63f commit f782aca
Show file tree
Hide file tree
Showing 14 changed files with 106 additions and 80 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,10 @@ use move_vm_runtime::{
};
use move_vm_test_utils::InMemoryStorage;
use move_vm_types::{
loaded_data::runtime_types::{AbilityInfo, StructIdentifier, StructNameIndex, TypeBuilder},
loaded_data::{
runtime_types::{AbilityInfo, StructIdentifier, TypeBuilder},
struct_name_indexing::StructNameIndex,
},
value_serde::FunctionValueExtension,
};
use std::str::FromStr;
Expand Down Expand Up @@ -74,7 +77,7 @@ fn test_function_value_extension() {
let foo_ty = types.pop().unwrap();
let name = module_storage
.runtime_environment()
.idx_to_struct_name_for_test(StructNameIndex(0))
.idx_to_struct_name_for_test(StructNameIndex::new(0))
.unwrap();
assert_eq!(name, StructIdentifier {
module: test_id.clone(),
Expand All @@ -84,8 +87,10 @@ fn test_function_value_extension() {
foo_ty,
ty_builder
.create_ref_ty(
&ty_builder
.create_struct_ty(StructNameIndex(0), AbilityInfo::struct_(AbilitySet::EMPTY)),
&ty_builder.create_struct_ty(
StructNameIndex::new(0),
AbilityInfo::struct_(AbilitySet::EMPTY)
),
false
)
.unwrap()
Expand Down Expand Up @@ -121,7 +126,7 @@ fn test_function_value_extension() {
let bar_ty = types.pop().unwrap();
let name = module_storage
.runtime_environment()
.idx_to_struct_name_for_test(StructNameIndex(1))
.idx_to_struct_name_for_test(StructNameIndex::new(1))
.unwrap();
assert_eq!(name, StructIdentifier {
module: other_test_id,
Expand All @@ -130,7 +135,7 @@ fn test_function_value_extension() {
assert_eq!(
bar_ty,
ty_builder.create_struct_ty(
StructNameIndex(1),
StructNameIndex::new(1),
AbilityInfo::struct_(AbilitySet::from_u8(Ability::Drop as u8).unwrap())
)
);
Expand Down
50 changes: 25 additions & 25 deletions third_party/move/move-vm/runtime/src/loader/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,19 @@
// SPDX-License-Identifier: Apache-2.0

use crate::{
config::VMConfig, data_cache::TransactionDataCache, logging::expect_no_verification_errors,
module_traversal::TraversalContext, storage::module_storage::ModuleStorage, CodeStorage,
config::VMConfig,
data_cache::TransactionDataCache,
loader::modules::{StructVariantInfo, VariantFieldInfo},
logging::expect_no_verification_errors,
module_traversal::TraversalContext,
native_functions::NativeFunctions,
storage::{
loader::LoaderV2,
module_storage::{FunctionValueExtensionAdapter, ModuleStorage},
ty_cache::StructInfoCache,
},
ty_tag_cache::TypeTagBuilder,
CodeStorage,
};
use hashbrown::Equivalent;
use lazy_static::lazy_static;
Expand All @@ -14,8 +25,9 @@ use move_binary_format::{
file_format::{
CompiledModule, CompiledScript, Constant, ConstantPoolIndex, FieldHandleIndex,
FieldInstantiationIndex, FunctionHandleIndex, FunctionInstantiationIndex, SignatureIndex,
StructDefInstantiationIndex, StructDefinitionIndex, StructFieldInformation, TableIndex,
TypeParameterIndex,
StructDefInstantiationIndex, StructDefinitionIndex, StructFieldInformation,
StructVariantHandleIndex, StructVariantInstantiationIndex, TableIndex, TypeParameterIndex,
VariantFieldHandleIndex, VariantFieldInstantiationIndex, VariantIndex,
},
IndexKind,
};
Expand All @@ -29,19 +41,26 @@ use move_core_types::{
value::{IdentifierMappingKind, MoveFieldLayout, MoveStructLayout, MoveTypeLayout},
vm_status::StatusCode,
};
use move_vm_metrics::{Timer, VM_TIMER};
use move_vm_types::{
gas::GasMeter,
loaded_data::runtime_types::{
AbilityInfo, DepthFormula, StructIdentifier, StructNameIndex, StructType, Type,
loaded_data::{
runtime_types::{
AbilityInfo, DepthFormula, StructIdentifier, StructLayout, StructType, Type,
TypeBuilder,
},
struct_name_indexing::{StructNameIndex, StructNameIndexMap},
},
sha3_256,
value_serde::FunctionValueExtension,
};
use parking_lot::{Mutex, RwLock};
use std::{
collections::{btree_map, BTreeMap, BTreeSet},
hash::Hash,
sync::Arc,
};
use type_loader::intern_type;
use typed_arena::Arena;

mod access_specifier_loader;
Expand All @@ -50,31 +69,12 @@ mod modules;
mod script;
mod type_loader;

use crate::{
loader::modules::{StructVariantInfo, VariantFieldInfo},
native_functions::NativeFunctions,
storage::{
loader::LoaderV2, module_storage::FunctionValueExtensionAdapter,
struct_name_index_map::StructNameIndexMap, ty_cache::StructInfoCache,
ty_tag_cache::TypeTagBuilder,
},
};
pub use function::{Function, LoadedFunction};
pub(crate) use function::{FunctionHandle, FunctionInstantiation, LoadedFunctionOwner};
pub use modules::Module;
pub(crate) use modules::{LegacyModuleCache, LegacyModuleStorage, LegacyModuleStorageAdapter};
use move_binary_format::file_format::{
StructVariantHandleIndex, StructVariantInstantiationIndex, VariantFieldHandleIndex,
VariantFieldInstantiationIndex, VariantIndex,
};
use move_vm_metrics::{Timer, VM_TIMER};
use move_vm_types::{
loaded_data::runtime_types::{StructLayout, TypeBuilder},
value_serde::FunctionValueExtension,
};
pub use script::Script;
pub(crate) use script::ScriptCache;
use type_loader::intern_type;

type ScriptHash = [u8; 32];

Expand Down
6 changes: 3 additions & 3 deletions third_party/move/move-vm/runtime/src/loader/modules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ use crate::{
BinaryCache,
},
native_functions::NativeFunctions,
storage::struct_name_index_map::StructNameIndexMap,
};
use move_binary_format::{
access::ModuleAccess,
Expand All @@ -29,8 +28,9 @@ use move_core_types::{
vm_status::StatusCode,
};
use move_vm_metrics::{Timer, VM_TIMER};
use move_vm_types::loaded_data::runtime_types::{
StructIdentifier, StructLayout, StructNameIndex, StructType, Type,
use move_vm_types::loaded_data::{
runtime_types::{StructIdentifier, StructLayout, StructType, Type},
struct_name_indexing::{StructNameIndex, StructNameIndexMap},
};
use parking_lot::RwLock;
use std::{
Expand Down
3 changes: 2 additions & 1 deletion third_party/move/move-vm/runtime/src/loader/script.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// SPDX-License-Identifier: Apache-2.0

use super::{intern_type, BinaryCache, Function, FunctionHandle, FunctionInstantiation};
use crate::{loader::ScriptHash, storage::struct_name_index_map::StructNameIndexMap};
use crate::loader::ScriptHash;
use move_binary_format::{
access::ScriptAccess,
binary_views::BinaryIndexedView,
Expand All @@ -13,6 +13,7 @@ use move_core_types::{identifier::Identifier, language_storage::ModuleId, vm_sta
use move_vm_types::loaded_data::{
runtime_access_specifier::AccessSpecifier,
runtime_types::{StructIdentifier, Type},
struct_name_indexing::StructNameIndexMap,
};
use std::{collections::BTreeMap, ops::Deref, sync::Arc};

Expand Down
5 changes: 4 additions & 1 deletion third_party/move/move-vm/runtime/src/loader/type_loader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,10 @@
use move_binary_format::{
binary_views::BinaryIndexedView, errors::PartialVMResult, file_format::SignatureToken,
};
use move_vm_types::loaded_data::runtime_types::{AbilityInfo, StructNameIndex, Type};
use move_vm_types::loaded_data::{
runtime_types::{AbilityInfo, Type},
struct_name_indexing::StructNameIndex,
};
use triomphe::Arc as TriompheArc;

/// Converts a signature token into the in memory type representation used by the MoveVM.
Expand Down
9 changes: 6 additions & 3 deletions third_party/move/move-vm/runtime/src/storage/environment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ use crate::{
loader::check_natives,
native_functions::{NativeFunction, NativeFunctions},
storage::{
struct_name_index_map::StructNameIndexMap, ty_cache::StructInfoCache,
ty_tag_cache::TypeTagCache, verified_module_cache::VERIFIED_MODULES_V2,
ty_cache::StructInfoCache, ty_tag_cache::TypeTagCache,
verified_module_cache::VERIFIED_MODULES_V2,
},
Module, Script,
};
Expand All @@ -26,8 +26,11 @@ use move_core_types::{
vm_status::{sub_status::unknown_invariant_violation::EPARANOID_FAILURE, StatusCode},
};
use move_vm_metrics::{Timer, VM_TIMER};
use move_vm_types::loaded_data::struct_name_indexing::StructNameIndexMap;
#[cfg(any(test, feature = "testing"))]
use move_vm_types::loaded_data::runtime_types::{StructIdentifier, StructNameIndex};
use move_vm_types::loaded_data::{
runtime_types::StructIdentifier, struct_name_indexing::StructNameIndex,
};
use std::sync::Arc;

/// [MoveVM] runtime environment encapsulating different configurations. Shared between the VM and
Expand Down
1 change: 0 additions & 1 deletion third_party/move/move-vm/runtime/src/storage/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
// SPDX-License-Identifier: Apache-2.0

pub(crate) mod loader;
pub(crate) mod struct_name_index_map;
pub(crate) mod ty_cache;
pub(crate) mod ty_tag_cache;
mod verified_module_cache;
Expand Down
6 changes: 4 additions & 2 deletions third_party/move/move-vm/runtime/src/storage/ty_cache.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
// Copyright (c) The Move Contributors
// SPDX-License-Identifier: Apache-2.0

use crate::storage::struct_name_index_map::StructNameIndexMap;
use move_binary_format::errors::{PartialVMError, PartialVMResult};
use move_core_types::{language_storage::StructTag, value::MoveTypeLayout, vm_status::StatusCode};
use move_vm_types::loaded_data::runtime_types::{DepthFormula, StructNameIndex, Type};
use move_vm_types::loaded_data::{
runtime_types::{DepthFormula, Type},
struct_name_indexing::{StructNameIndex, StructNameIndexMap},
};
use parking_lot::RwLock;

/// Layout information of a single struct instantiation.
Expand Down
1 change: 1 addition & 0 deletions third_party/move/move-vm/types/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ hashbrown = { workspace = true }
itertools = { workspace = true }
move-binary-format = { workspace = true }
move-core-types = { workspace = true }
parking_lot = { workspace = true }
proptest = { workspace = true, optional = true }
serde = { workspace = true, features = ["derive", "rc"] }
sha3 = { workspace = true }
Expand Down
11 changes: 0 additions & 11 deletions third_party/move/move-vm/types/src/code/errors.rs
Original file line number Diff line number Diff line change
@@ -1,17 +1,6 @@
// Copyright (c) The Move Contributors
// SPDX-License-Identifier: Apache-2.0

#[macro_export]
macro_rules! panic_error {
($msg:expr) => {{
println!("[Error] panic detected: {}", $msg);
move_binary_format::errors::PartialVMError::new(
move_core_types::vm_status::StatusCode::DELAYED_FIELD_OR_BLOCKSTM_CODE_INVARIANT_ERROR,
)
.with_message(format!("Panic detected: {:?}", $msg))
}};
}

#[macro_export]
macro_rules! module_storage_error {
($addr:expr, $name:expr, $err:ident) => {
Expand Down
1 change: 1 addition & 0 deletions third_party/move/move-vm/types/src/loaded_data/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,4 @@ pub mod runtime_access_specifier;
#[cfg(test)]
mod runtime_access_specifiers_prop_tests;
pub mod runtime_types;
pub mod struct_name_indexing;
16 changes: 7 additions & 9 deletions third_party/move/move-vm/types/src/loaded_data/runtime_types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#![allow(clippy::non_canonical_partial_ord_impl)]

use crate::loaded_data::struct_name_indexing::StructNameIndex;
use derivative::Derivative;
use itertools::Itertools;
use move_binary_format::{
Expand Down Expand Up @@ -245,7 +246,7 @@ impl StructType {
#[cfg(test)]
pub fn for_test() -> StructType {
Self {
idx: StructNameIndex(0),
idx: StructNameIndex::new(0),
layout: StructLayout::Single(vec![]),
phantom_ty_params_mask: SmallBitVec::new(),
abilities: AbilitySet::EMPTY,
Expand All @@ -254,9 +255,6 @@ impl StructType {
}
}

#[derive(Debug, Copy, Clone, Eq, Hash, Ord, PartialEq, PartialOrd)]
pub struct StructNameIndex(pub usize);

#[derive(Debug, Clone, Eq, Hash, Ord, PartialEq, PartialOrd)]
pub struct StructIdentifier {
pub module: ModuleId,
Expand Down Expand Up @@ -737,15 +735,15 @@ impl fmt::Display for Type {
Address => f.write_str("address"),
Signer => f.write_str("signer"),
Vector(et) => write!(f, "vector<{}>", et),
Struct { idx, ability: _ } => write!(f, "s#{}", idx.0),
Struct { idx, ability: _ } => write!(f, "s#{}", idx),
StructInstantiation {
idx,
ty_args,
ability: _,
} => write!(
f,
"s#{}<{}>",
idx.0,
idx,
ty_args.iter().map(|t| t.to_string()).join(",")
),
Reference(t) => write!(f, "&{}", t),
Expand Down Expand Up @@ -1207,15 +1205,15 @@ mod unit_tests {

fn struct_instantiation_ty_for_test(ty_args: Vec<Type>) -> Type {
Type::StructInstantiation {
idx: StructNameIndex(0),
idx: StructNameIndex::new(0),
ability: AbilityInfo::struct_(AbilitySet::EMPTY),
ty_args: TriompheArc::new(ty_args),
}
}

fn struct_ty_for_test() -> Type {
Type::Struct {
idx: StructNameIndex(0),
idx: StructNameIndex::new(0),
ability: AbilityInfo::struct_(AbilitySet::EMPTY),
}
}
Expand Down Expand Up @@ -1358,7 +1356,7 @@ mod unit_tests {

#[test]
fn test_create_struct_ty() {
let idx = StructNameIndex(0);
let idx = StructNameIndex::new(0);
let ability_info = AbilityInfo::struct_(AbilitySet::EMPTY);

// Limits are not relevant here.
Expand Down
Loading

0 comments on commit f782aca

Please sign in to comment.