Skip to content

Commit

Permalink
[move] Consolidate source/dep logic (#17460)
Browse files Browse the repository at this point in the history
## Description 

- We have layered on some notions of dependencies/source targets because
of the package system.
- I have created an enum with 3 variants, but I kind of hate the name.
So open to suggestions here

## Test plan 

- Internal change 

---

## Release notes

Check each box that your changes affect. If none of the boxes relate to
your changes, release notes aren't required.

For each box you select, include information after the relevant heading
that describes the impact of your changes that a user might notice and
any actions they must take to implement updates.

- [ ] Protocol: 
- [ ] Nodes (Validators and Full nodes): 
- [ ] Indexer: 
- [ ] JSON-RPC: 
- [ ] GraphQL: 
- [ ] CLI: 
- [ ] Rust SDK:
  • Loading branch information
tnowacki authored May 3, 2024
1 parent 3eb9d4d commit 228d754
Show file tree
Hide file tree
Showing 15 changed files with 133 additions and 67 deletions.
20 changes: 12 additions & 8 deletions external-crates/move/crates/move-compiler/src/cfgir/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

use crate::{
diagnostics::WarningFilters,
expansion::ast::{Attributes, Friend, ModuleIdent, Mutability},
expansion::ast::{Attributes, Friend, ModuleIdent, Mutability, TargetKind},
hlir::ast::{
BaseType, Command, Command_, EnumDefinition, FunctionSignature, Label, SingleType,
StructDefinition, Var, Visibility,
Expand Down Expand Up @@ -38,7 +38,7 @@ pub struct ModuleDefinition {
// package name metadata from compiler arguments, not used for any language rules
pub package_name: Option<Symbol>,
pub attributes: Attributes,
pub is_source_module: bool,
pub target_kind: TargetKind,
/// `dependency_order` is the topological order/rank in the dependency graph.
pub dependency_order: usize,
pub friends: UniqueMap<ModuleIdent, Friend>,
Expand Down Expand Up @@ -203,7 +203,7 @@ impl AstDebug for ModuleDefinition {
warning_filter,
package_name,
attributes,
is_source_module,
target_kind,
dependency_order,
friends,
structs,
Expand All @@ -216,11 +216,15 @@ impl AstDebug for ModuleDefinition {
w.writeln(&format!("{}", n))
}
attributes.ast_debug(w);
if *is_source_module {
w.writeln("library module")
} else {
w.writeln("source module")
}
w.writeln(match target_kind {
TargetKind::Source {
is_root_package: true,
} => "root module",
TargetKind::Source {
is_root_package: false,
} => "dependency module",
TargetKind::External => "external module",
});
w.writeln(&format!("dependency order #{}", dependency_order));
for (mident, _loc) in friends.key_cloned_iter() {
w.write(&format!("friend {};", mident));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ fn module(
warning_filter,
package_name,
attributes,
is_source_module,
target_kind,
dependency_order,
friends,
structs,
Expand All @@ -200,7 +200,7 @@ fn module(
warning_filter,
package_name,
attributes,
is_source_module,
target_kind,
dependency_order,
friends,
structs,
Expand Down
27 changes: 21 additions & 6 deletions external-crates/move/crates/move-compiler/src/expansion/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,17 @@ pub type Attributes = UniqueMap<Spanned<KnownAttribute>, Attribute>;
// Modules
//**************************************************************************************************

#[derive(Debug, Clone, Copy)]
/// Specifies a source target or dependency
pub enum TargetKind {
/// A source module. If is_root_package is false, some warnings might be suppressed.
/// Bytecode/CompiledModules will be generated for any Source target
Source { is_root_package: bool },
/// A dependency only used for linking.
/// No bytecode or CompiledModules are generated
External,
}

#[derive(Clone, Copy)]
pub enum Address {
Numerical {
Expand All @@ -123,7 +134,7 @@ pub struct ModuleDefinition {
pub package_name: Option<Symbol>,
pub attributes: Attributes,
pub loc: Loc,
pub is_source_module: bool,
pub target_kind: TargetKind,
pub use_funs: UseFuns,
pub friends: UniqueMap<ModuleIdent, Friend>,
pub structs: UniqueMap<DatatypeName, StructDefinition>,
Expand Down Expand Up @@ -1106,7 +1117,7 @@ impl AstDebug for ModuleDefinition {
package_name,
attributes,
loc: _loc,
is_source_module,
target_kind,
use_funs,
friends,
structs,
Expand All @@ -1120,10 +1131,14 @@ impl AstDebug for ModuleDefinition {
w.writeln(&format!("{}", n))
}
attributes.ast_debug(w);
w.writeln(if *is_source_module {
"source module"
} else {
"library module"
w.writeln(match target_kind {
TargetKind::Source {
is_root_package: true,
} => "root module",
TargetKind::Source {
is_root_package: false,
} => "dependency module",
TargetKind::External => "external module",
});
use_funs.ast_debug(w);
for (mident, _loc) in friends.key_cloned_iter() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use crate::{
AliasEntry, AliasMapBuilder, ParserExplicitUseFun, UnnecessaryAlias, UseFunsBuilder,
},
aliases::AliasSet,
ast::{self as E, Address, Fields, ModuleIdent, ModuleIdent_},
ast::{self as E, Address, Fields, ModuleIdent, ModuleIdent_, TargetKind},
byte_string, hex_string,
path_expander::{
access_result, Access, LegacyPathExpander, ModuleAccessResult, Move2024PathExpander,
Expand Down Expand Up @@ -895,12 +895,18 @@ fn module_(

context.pop_alias_scope(Some(&mut use_funs));

let target_kind = if !context.is_source_definition {
TargetKind::External
} else {
let is_root_package = !context.env().package_config(package_name).is_dependency;
TargetKind::Source { is_root_package }
};
let def = E::ModuleDefinition {
package_name,
attributes,
loc,
use_funs,
is_source_module: context.is_source_definition,
target_kind,
friends,
structs,
enums,
Expand Down
19 changes: 12 additions & 7 deletions external-crates/move/crates/move-compiler/src/hlir/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use crate::{
diagnostics::WarningFilters,
expansion::ast::{
ability_modifiers_ast_debug, AbilitySet, Attributes, Friend, ModuleIdent, Mutability,
TargetKind,
},
naming::ast::{BuiltinTypeName, BuiltinTypeName_, DatatypeTypeParameter, TParam},
parser::ast::{
Expand Down Expand Up @@ -39,7 +40,7 @@ pub struct ModuleDefinition {
// package name metadata from compiler arguments, not used for any language rules
pub package_name: Option<Symbol>,
pub attributes: Attributes,
pub is_source_module: bool,
pub target_kind: TargetKind,
/// `dependency_order` is the topological order/rank in the dependency graph.
pub dependency_order: usize,
pub friends: UniqueMap<ModuleIdent, Friend>,
Expand Down Expand Up @@ -900,7 +901,7 @@ impl AstDebug for ModuleDefinition {
warning_filter,
package_name,
attributes,
is_source_module,
target_kind,
dependency_order,
friends,
structs,
Expand All @@ -913,11 +914,15 @@ impl AstDebug for ModuleDefinition {
w.writeln(&format!("{}", n))
}
attributes.ast_debug(w);
if *is_source_module {
w.writeln("library module")
} else {
w.writeln("source module")
}
w.writeln(match target_kind {
TargetKind::Source {
is_root_package: true,
} => "root module",
TargetKind::Source {
is_root_package: false,
} => "dependency module",
TargetKind::External => "external module",
});
w.writeln(&format!("dependency order #{}", dependency_order));
for (mident, _loc) in friends.key_cloned_iter() {
w.write(&format!("friend {};", mident));
Expand Down
17 changes: 11 additions & 6 deletions external-crates/move/crates/move-compiler/src/hlir/translate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
use crate::{
debug_display, debug_display_verbose, diag,
editions::{FeatureGate, Flavor},
expansion::ast::{self as E, Fields, ModuleIdent, Mutability},
expansion::ast::{self as E, Fields, ModuleIdent, Mutability, TargetKind},
hlir::{
ast::{self as H, Block, BlockLabel, MoveOpAnnotation, UnpackType},
detect_dead_code::program as detect_dead_code_analysis,
Expand Down Expand Up @@ -593,7 +593,7 @@ fn module(
warning_filter,
package_name,
attributes,
is_source_module,
target_kind,
dependency_order,
immediate_neighbors: _,
used_addresses: _,
Expand All @@ -619,7 +619,7 @@ fn module(
}
});

gen_unused_warnings(context, is_source_module, &structs);
gen_unused_warnings(context, target_kind, &structs);

context.current_package = None;
context.env.pop_warning_filter_scope();
Expand All @@ -629,7 +629,7 @@ fn module(
warning_filter,
package_name,
attributes,
is_source_module,
target_kind,
dependency_order,
friends,
structs,
Expand Down Expand Up @@ -3163,10 +3163,15 @@ fn freeze_single(sp!(sloc, s): H::SingleType) -> H::SingleType {

fn gen_unused_warnings(
context: &mut Context,
is_source_module: bool,
target_kind: TargetKind,
structs: &UniqueMap<DatatypeName, H::StructDefinition>,
) {
if !is_source_module {
if !matches!(
target_kind,
TargetKind::Source {
is_root_package: true
}
) {
// generate warnings only for modules compiled in this pass rather than for all modules
// including pre-compiled libraries for which we do not have source code available and
// cannot be analyzed in this pass
Expand Down
22 changes: 13 additions & 9 deletions external-crates/move/crates/move-compiler/src/naming/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ use crate::{
diagnostics::WarningFilters,
expansion::ast::{
ability_constraints_ast_debug, ability_modifiers_ast_debug, AbilitySet, Attributes,
DottedUsage, Fields, Friend, ImplicitUseFunCandidate, ModuleIdent, Mutability, Value,
Value_, Visibility,
DottedUsage, Fields, Friend, ImplicitUseFunCandidate, ModuleIdent, Mutability, TargetKind,
Value, Value_, Visibility,
},
parser::ast::{
self as P, Ability_, BinOp, ConstantName, DatatypeName, Field, FunctionName, UnaryOp,
Expand Down Expand Up @@ -141,7 +141,7 @@ pub struct ModuleDefinition {
// package name metadata from compiler arguments, not used for any language rules
pub package_name: Option<Symbol>,
pub attributes: Attributes,
pub is_source_module: bool,
pub target_kind: TargetKind,
pub use_funs: UseFuns,
pub syntax_methods: SyntaxMethods,
pub friends: UniqueMap<ModuleIdent, Friend>,
Expand Down Expand Up @@ -1193,7 +1193,7 @@ impl AstDebug for ModuleDefinition {
warning_filter,
package_name,
attributes,
is_source_module,
target_kind,
use_funs,
syntax_methods,
friends,
Expand All @@ -1207,11 +1207,15 @@ impl AstDebug for ModuleDefinition {
w.writeln(&format!("{}", n))
}
attributes.ast_debug(w);
if *is_source_module {
w.writeln("library module")
} else {
w.writeln("source module")
}
w.writeln(match target_kind {
TargetKind::Source {
is_root_package: true,
} => "root module",
TargetKind::Source {
is_root_package: false,
} => "dependency module",
TargetKind::External => "external module",
});
use_funs.ast_debug(w);
syntax_methods.ast_debug(w);
for (mident, _loc) in friends.key_cloned_iter() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1239,7 +1239,7 @@ fn module(
warning_filter,
package_name,
attributes,
is_source_module,
target_kind,
use_funs: euse_funs,
friends: efriends,
structs: estructs,
Expand Down Expand Up @@ -1310,7 +1310,7 @@ fn module(
warning_filter,
package_name,
attributes,
is_source_module,
target_kind,
use_funs,
syntax_methods,
friends,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use move_ir_types::location::Loc;
use move_symbol_pool::Symbol;

use crate::{
expansion::ast::{AbilitySet, Attributes, ModuleIdent, Visibility},
expansion::ast::{AbilitySet, Attributes, ModuleIdent, TargetKind, Visibility},
naming::ast::{
self as N, DatatypeTypeParameter, EnumDefinition, FunctionSignature, ResolvedUseFuns,
StructDefinition, SyntaxMethods, Type,
Expand Down Expand Up @@ -38,6 +38,7 @@ pub struct ConstantInfo {

#[derive(Debug, Clone)]
pub struct ModuleInfo {
pub target_kind: TargetKind,
pub attributes: Attributes,
pub package: Option<Symbol>,
pub use_funs: ResolvedUseFuns,
Expand Down Expand Up @@ -86,6 +87,7 @@ macro_rules! program_info {
.map(|module_use_funs| module_use_funs.remove(&mident).unwrap())
.unwrap_or_default();
let minfo = ModuleInfo {
target_kind: mdef.target_kind,
attributes: mdef.attributes.clone(),
package: mdef.package_name,
use_funs,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use crate::{
diag,
diagnostics::{Diagnostic, Diagnostics},
editions::Flavor,
expansion::ast::AbilitySet,
expansion::ast::{AbilitySet, TargetKind},
hlir::ast::{Exp, Label, ModuleCall, SingleType, Type, Type_, Var},
parser::ast::{Ability_, DatatypeName},
shared::{unique_map::UniqueMap, CompilationEnv, Identifier},
Expand Down Expand Up @@ -105,7 +105,12 @@ impl SimpleAbsIntConstructor for IDLeakVerifier {
// Skip if not sui
return None;
}
if config.is_dependency || !mdef.is_source_module {
if !matches!(
mdef.target_kind,
TargetKind::Source {
is_root_package: true
}
) {
// Skip non-source, dependency modules
return None;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use crate::{
diag,
diagnostics::{Diagnostic, WarningFilters},
editions::Flavor,
expansion::ast::{AbilitySet, Fields, ModuleIdent, Mutability, Visibility},
expansion::ast::{AbilitySet, Fields, ModuleIdent, Mutability, TargetKind, Visibility},
naming::ast::{
self as N, BuiltinTypeName_, FunctionSignature, StructFields, Type, TypeName_, Type_, Var,
},
Expand Down Expand Up @@ -114,7 +114,12 @@ impl<'a> TypingVisitorContext for Context<'a> {
// Skip if not sui
return true;
}
if config.is_dependency || !mdef.is_source_module {
if !matches!(
mdef.target_kind,
TargetKind::Source {
is_root_package: true
}
) {
// Skip non-source, dependency modules
return true;
}
Expand Down
Loading

0 comments on commit 228d754

Please sign in to comment.