Skip to content

Commit

Permalink
Add inlining strategy to compiler config (#1408)
Browse files Browse the repository at this point in the history
commit-id:65927daa

---

**Stack**:
- #1411
- #1410
- #1408⚠️ *Part of a stack created by [spr](https://github.com/ejoffe/spr). Do
not merge manually using the UI - doing so may have unexpected results.*
  • Loading branch information
maciektr authored Jul 4, 2024
1 parent b0a31a4 commit 5c074ec
Show file tree
Hide file tree
Showing 6 changed files with 79 additions and 4 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.

1 change: 1 addition & 0 deletions scarb/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ cairo-lang-defs.workspace = true
cairo-lang-diagnostics.workspace = true
cairo-lang-filesystem.workspace = true
cairo-lang-formatter.workspace = true
cairo-lang-lowering.workspace = true
cairo-lang-macro = { path = "../plugins/cairo-lang-macro" }
cairo-lang-macro-stable = { path = "../plugins/cairo-lang-macro-stable" }
cairo-lang-parser.workspace = true
Expand Down
22 changes: 21 additions & 1 deletion scarb/src/compiler/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use serde::Serialize;
use std::io::{BufWriter, Write};

use crate::compiler::{CairoCompilationUnit, CompilationUnitAttributes};
use crate::core::Workspace;
use crate::core::{InliningStrategy, Workspace};
use crate::flock::Filesystem;

pub fn build_compiler_config<'c>(
Expand Down Expand Up @@ -51,13 +51,33 @@ pub fn build_compiler_config<'c>(
diagnostics_reporter
},
replace_ids: unit.compiler_config.sierra_replace_ids,
inlining_strategy: unit.compiler_config.inlining_strategy.clone().into(),
add_statements_functions: unit
.compiler_config
.unstable_add_statements_functions_debug_info,
..CompilerConfig::default()
}
}

impl From<InliningStrategy> for cairo_lang_lowering::utils::InliningStrategy {
fn from(value: InliningStrategy) -> Self {
match value {
InliningStrategy::Default => cairo_lang_lowering::utils::InliningStrategy::Default,
InliningStrategy::Avoid => cairo_lang_lowering::utils::InliningStrategy::Avoid,
}
}
}

#[allow(unused)]
impl From<cairo_lang_lowering::utils::InliningStrategy> for InliningStrategy {
fn from(value: cairo_lang_lowering::utils::InliningStrategy) -> Self {
match value {
cairo_lang_lowering::utils::InliningStrategy::Default => InliningStrategy::Default,
cairo_lang_lowering::utils::InliningStrategy::Avoid => InliningStrategy::Avoid,
}
}
}

pub fn collect_main_crate_ids(unit: &CairoCompilationUnit, db: &RootDatabase) -> Vec<CrateId> {
vec![db.intern_crate(CrateLongId::Real(
unit.main_component().cairo_package_name(),
Expand Down
15 changes: 15 additions & 0 deletions scarb/src/core/manifest/compiler_config.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use serde::{Deserialize, Serialize};
use std::hash::Hash;

use crate::compiler::{DefaultForProfile, Profile};
use crate::core::TomlCairo;
Expand All @@ -25,6 +26,18 @@ pub struct ManifestCompilerConfig {
/// Used by [cairo-profiler](https://github.com/software-mansion/cairo-profiler).
/// This feature is unstable and is subject to change.
pub unstable_add_statements_functions_debug_info: bool,
// Inlining strategy.
pub inlining_strategy: InliningStrategy,
}

#[derive(Debug, Default, Deserialize, Serialize, Eq, PartialEq, Hash, Clone)]
#[serde(rename_all = "kebab-case")]
pub enum InliningStrategy {
/// Do not override inlining strategy.
#[default]
Default,
/// Inline only in the case of an `inline(always)` annotation.
Avoid,
}

impl DefaultForProfile for ManifestCompilerConfig {
Expand All @@ -34,6 +47,7 @@ impl DefaultForProfile for ManifestCompilerConfig {
allow_warnings: true,
enable_gas: true,
unstable_add_statements_functions_debug_info: false,
inlining_strategy: InliningStrategy::default(),
}
}
}
Expand All @@ -47,6 +61,7 @@ impl From<ManifestCompilerConfig> for TomlCairo {
unstable_add_statements_functions_debug_info: Some(
config.unstable_add_statements_functions_debug_info,
),
inlining_strategy: Some(config.inlining_strategy),
}
}
}
9 changes: 7 additions & 2 deletions scarb/src/core/manifest/toml_manifest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ use crate::core::manifest::{ManifestDependency, ManifestMetadata, Summary, Targe
use crate::core::package::PackageId;
use crate::core::source::{GitReference, SourceId};
use crate::core::{
DepKind, DependencyVersionReq, ManifestBuilder, ManifestCompilerConfig, PackageName,
TargetKind, TestTargetProps, TestTargetType,
DepKind, DependencyVersionReq, InliningStrategy, ManifestBuilder, ManifestCompilerConfig,
PackageName, TargetKind, TestTargetProps, TestTargetType,
};
use crate::internal::fsx;
use crate::internal::fsx::PathBufUtf8Ext;
Expand Down Expand Up @@ -320,6 +320,8 @@ pub struct TomlCairo {
/// Used by [cairo-profiler](https://github.com/software-mansion/cairo-profiler).
/// This feature is unstable and is subject to change.
pub unstable_add_statements_functions_debug_info: Option<bool>,
/// Inlining strategy.
pub inlining_strategy: Option<InliningStrategy>,
}

#[derive(Debug, Default, Deserialize, Serialize, Clone)]
Expand Down Expand Up @@ -835,6 +837,9 @@ impl TomlManifest {
if let Some(sierra_replace_ids) = cairo.sierra_replace_ids {
compiler_config.sierra_replace_ids = sierra_replace_ids;
}
if let Some(inlining_strategy) = cairo.inlining_strategy {
compiler_config.inlining_strategy = inlining_strategy;
}
if let Some(allow_warnings) = cairo.allow_warnings {
compiler_config.allow_warnings = allow_warnings;
}
Expand Down
35 changes: 34 additions & 1 deletion scarb/tests/profiles.rs
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,7 @@ fn can_use_shortcuts_in_scripts() {
}

#[test]
fn sierra_replace_ids_defaults_true_in_dev() {
fn compiler_config_defaults_in_dev() {
let t = TempDir::new().unwrap();
ProjectBuilder::start().name("hello").build(&t);

Expand All @@ -335,6 +335,14 @@ fn sierra_replace_ids_defaults_true_in_dev() {
.unwrap()
.as_bool()
.unwrap());
assert_eq!(
compiler_config
.get("inlining_strategy")
.unwrap()
.as_str()
.unwrap(),
"default"
);
}
}

Expand Down Expand Up @@ -376,6 +384,7 @@ fn compiler_config_set_for_all_profiles() {
r#"
[cairo]
sierra-replace-ids = true
inlining-strategy = "avoid"
[profile.some-profile]
"#,
Expand All @@ -396,6 +405,14 @@ fn compiler_config_set_for_all_profiles() {
.unwrap()
.as_bool()
.unwrap());
assert_eq!(
compiler_config
.get("inlining_strategy")
.unwrap()
.as_str()
.unwrap(),
"avoid"
);
}

let metadata = Scarb::quick_snapbox()
Expand All @@ -412,6 +429,14 @@ fn compiler_config_set_for_all_profiles() {
.unwrap()
.as_bool()
.unwrap());
assert_eq!(
compiler_config
.get("inlining_strategy")
.unwrap()
.as_str()
.unwrap(),
"avoid"
);
}

let metadata = Scarb::quick_snapbox()
Expand All @@ -435,6 +460,14 @@ fn compiler_config_set_for_all_profiles() {
.unwrap()
.as_bool()
.unwrap());
assert_eq!(
compiler_config
.get("inlining_strategy")
.unwrap()
.as_str()
.unwrap(),
"avoid"
);
}
}

Expand Down

0 comments on commit 5c074ec

Please sign in to comment.