Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Load prebuilt macros #1812

Closed
wants to merge 33 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
b35f947
Update `SharedLibraryProvider` to resolve prebuilt path
DelevoXDG Dec 5, 2024
908cc47
Add helper to determine whether the package is prebuilt
DelevoXDG Dec 5, 2024
7d399d3
Update `ProcMacroInstance` and `ProcMacroInstance` to allow loading p…
DelevoXDG Dec 5, 2024
a95a25d
Update `load_plugins` to load prebuilts
DelevoXDG Dec 5, 2024
2aa6ee8
Add `skip_prebuilt_proc_macros` flag and relevant logic
DelevoXDG Dec 5, 2024
bfeb684
Update `load_plugins` to print compilation message to ui
DelevoXDG Dec 5, 2024
e570a99
Update `proc_macro_server`
DelevoXDG Dec 5, 2024
c73c0f6
Add simple test
DelevoXDG Dec 5, 2024
87f87c5
misc
DelevoXDG Dec 5, 2024
a4a8d14
fix test
DelevoXDG Dec 6, 2024
c81d9ce
Merge `try_new` and `try_load_prebuilt`
DelevoXDG Dec 6, 2024
9cac622
Fix test
DelevoXDG Dec 6, 2024
eba6fe4
Partially fix name resolution
DelevoXDG Dec 6, 2024
57cc3d1
Add system-universal test
DelevoXDG Dec 6, 2024
6b342b1
Update `load_plugins` to try registering before compiling
DelevoXDG Dec 8, 2024
e76fcf2
Add `load_plugins` comments
DelevoXDG Dec 8, 2024
8424cb6
Fix false positive `is_prebuilt`
DelevoXDG Dec 8, 2024
be7b5c2
Fix false positive `is_prebuilt`
DelevoXDG Dec 8, 2024
1e2845a
misc
DelevoXDG Dec 8, 2024
8a468d3
misc
DelevoXDG Dec 8, 2024
394bf55
Use package name and version from Scarb manifest instead of Cargo man…
DelevoXDG Dec 8, 2024
7b0162c
misc: unused arg
DelevoXDG Dec 8, 2024
76118b0
Ensure `cargo fetch` is not called for prebuilt plugins
DelevoXDG Dec 8, 2024
05cf39d
Update test
DelevoXDG Dec 8, 2024
5a85ee2
Add test with invalid prebuilt plugins
DelevoXDG Dec 8, 2024
cce42b7
Fix comment
DelevoXDG Dec 8, 2024
06aab97
Format
DelevoXDG Dec 8, 2024
64f362e
Add `test-prebuilt-plugins` for all available targets
DelevoXDG Dec 10, 2024
e1873fd
Add `proc-macro-server` test
DelevoXDG Dec 10, 2024
034af9a
Preload prebuilts during compilation
DelevoXDG Dec 16, 2024
b8c4156
Adjust proc macro server to loading prebuilts on compilation
DelevoXDG Dec 18, 2024
3af99af
Format
DelevoXDG Dec 18, 2024
9eb6fc4
Reorder `load_prebuilts` in `generate_compilation_units`
DelevoXDG Dec 18, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -107,3 +107,29 @@ jobs:
cache-dependency-path: website/package-lock.json
- run: npm ci
- run: npm run fmt:check

test-prebuilt-plugins:
name: test prebuilt plugins ${{ matrix.platform.name }}
runs-on: ${{ matrix.platform.os }}
strategy:
fail-fast: false
matrix:
platform:
- name: linux x86-64
os: ubuntu-latest
- name: windows x86-64
os: windows-latest
- name: macos arm64
os: macos-latest
- name: macos x86-64
os: macos-13

steps:
- uses: actions/checkout@v4
- uses: dtolnay/rust-toolchain@stable
- uses: Swatinem/rust-cache@v2
- name: Run prebuilt plugin tests
run: |
cargo test -p scarb --test build_cairo_plugin compile_with_prebuilt_plugins -- --exact
cargo test -p scarb --test build_cairo_plugin compile_with_invalid_prebuilt_plugins -- --exact
cargo test -p scarb --test proc_macro_server load_prebuilt_proc_macros -- --exact
Comment on lines +133 to +135
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's use the same prefix for all this tests, and use the prefix to filter here, so we only run Cargo once.

8 changes: 8 additions & 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 Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ xshell = "0.2"
xxhash-rust = { version = "0.8", features = ["xxh3"] }
zip = { version = "0.6", default-features = false, features = ["deflate"] }
zstd = "0.13"
target-triple = "0.1"

[profile.release]
lto = true
Expand Down
1 change: 1 addition & 0 deletions scarb/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ windows-sys.workspace = true
zstd.workspace = true
cargo_metadata.workspace = true
flate2.workspace = true
target-triple.workspace = true

[target.'cfg(not(target_os = "linux"))'.dependencies]
reqwest = { workspace = true, default-features = true }
Expand Down
13 changes: 7 additions & 6 deletions scarb/src/bin/scarb/commands/proc_macro_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ pub fn run(config: &mut Config) -> Result<()> {
no_default_features: false,
},
true,
true,
&ws,
)?;

Expand Down Expand Up @@ -43,12 +44,12 @@ fn load_plugins(
ws: &Workspace<'_>,
proc_macros: &mut ProcMacroHost,
) -> Result<()> {
for plugin_info in unit
.cairo_plugins
.into_iter()
.filter(|plugin_info| !plugin_info.builtin)
{
proc_macros.register(plugin_info.package, ws.config())?;
for plugin_info in unit.cairo_plugins.into_iter().filter(|p| !p.builtin) {
if let Some(prebuilt) = plugin_info.prebuilt {
proc_macros.register_instance(prebuilt);
} else {
proc_macros.register_new(plugin_info.package, ws.config())?;
}
}

Ok(())
Expand Down
33 changes: 30 additions & 3 deletions scarb/src/compiler/compilation_unit.rs
Original file line number Diff line number Diff line change
@@ -1,14 +1,16 @@
use std::fmt::Write;
use std::hash::{Hash, Hasher};

use anyhow::{ensure, Result};
use cairo_lang_filesystem::cfg::CfgSet;
use cairo_lang_filesystem::db::CrateIdentifier;
use itertools::Itertools;
use serde::{Deserialize, Serialize};
use smol_str::SmolStr;
use std::fmt::Write;
use std::hash::{Hash, Hasher};
use std::sync::Arc;
use typed_builder::TypedBuilder;

use crate::compiler::plugin::proc_macro::compilation::SharedLibraryProvider;
use crate::compiler::plugin::proc_macro::ProcMacroInstance;
use crate::compiler::Profile;
use crate::core::{
ManifestCompilerConfig, Package, PackageId, PackageName, Target, TargetKind, Workspace,
Expand Down Expand Up @@ -72,6 +74,9 @@ pub struct ProcMacroCompilationUnit {

/// Rust compiler configuration parameters to use in this unit.
pub compiler_config: serde_json::Value,

/// Instance of the proc macro loaded from prebuilt library, if available.
pub prebuilt: Option<Arc<ProcMacroInstance>>,
}

/// Information about a single package that is part of a [`CompilationUnit`].
Expand All @@ -97,6 +102,9 @@ pub struct CompilationUnitCairoPlugin {
/// The Scarb plugin [`Package`] to load.
pub package: Package,
pub builtin: bool,

/// Instance of the proc macro loaded from prebuilt library, if available.
pub prebuilt: Option<Arc<ProcMacroInstance>>,
}

/// Unique identifier of the compilation unit component.
Expand Down Expand Up @@ -130,6 +138,8 @@ pub trait CompilationUnitAttributes {
fn components(&self) -> &[CompilationUnitComponent];
fn digest(&self) -> String;

fn is_prebuilt(&self) -> bool;

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think that's the right place to store this information. In this PR you should load all available pre-builts, and we will add a whitelist later on.

Copy link
Contributor Author

@DelevoXDG DelevoXDG Dec 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure I understand 😅

This PR does load all available prebuilts, but we can't load any binaries if none are present for the target triple, even if they're included in the whitelist.

This method merely indicated whether a proc macro has prebuilts for the current platform

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The trait CompilationUnitAttributes is shared between CompilationUnit, CairoCompilationUnit and ProcMacroCompilationUnit. Shouldn't this be defined only for the last one? I don't think it's used on the former two anyway.

fn main_component(&self) -> &CompilationUnitComponent {
// NOTE: This uses the order invariant of `component` field.
let component = &self.components()[0];
Expand Down Expand Up @@ -195,6 +205,12 @@ impl CompilationUnitAttributes for CompilationUnit {
Self::ProcMacro(unit) => unit.digest(),
}
}
fn is_prebuilt(&self) -> bool {
match self {
Self::Cairo(unit) => unit.is_prebuilt(),
Self::ProcMacro(unit) => unit.is_prebuilt(),
}
}
}

impl CompilationUnitAttributes for CairoCompilationUnit {
Expand All @@ -215,6 +231,10 @@ impl CompilationUnitAttributes for CairoCompilationUnit {
self.compiler_config.hash(&mut hasher);
hasher.finish_as_short_hash()
}

fn is_prebuilt(&self) -> bool {
false
}
}

impl CompilationUnitAttributes for ProcMacroCompilationUnit {
Expand All @@ -233,6 +253,13 @@ impl CompilationUnitAttributes for ProcMacroCompilationUnit {
}
hasher.finish_as_short_hash()
}

fn is_prebuilt(&self) -> bool {
self.components
.first()
.map(|c| c.package.is_prebuilt())
.unwrap_or(false)
}
}
Comment on lines +257 to +262
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be just self.prebuilt.is_some()?


impl CairoCompilationUnit {
Expand Down
13 changes: 7 additions & 6 deletions scarb/src/compiler/db.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
use crate::compiler::plugin::proc_macro::{ProcMacroHost, ProcMacroHostPlugin};
use crate::compiler::{CairoCompilationUnit, CompilationUnitAttributes, CompilationUnitComponent};
use crate::core::Workspace;
use crate::DEFAULT_MODULE_MAIN_FILE;
use anyhow::{anyhow, Result};
use cairo_lang_compiler::db::{RootDatabase, RootDatabaseBuilder};
use cairo_lang_compiler::project::{AllCratesConfig, ProjectConfig, ProjectConfigContent};
Expand All @@ -14,11 +18,6 @@ use std::path::PathBuf;
use std::sync::Arc;
use tracing::trace;

use crate::compiler::plugin::proc_macro::{ProcMacroHost, ProcMacroHostPlugin};
use crate::compiler::{CairoCompilationUnit, CompilationUnitAttributes, CompilationUnitComponent};
use crate::core::Workspace;
use crate::DEFAULT_MODULE_MAIN_FILE;

pub struct ScarbDatabase {
pub db: RootDatabase,
pub proc_macro_host: Arc<ProcMacroHostPlugin>,
Expand Down Expand Up @@ -59,8 +58,10 @@ fn load_plugins(
let plugin = ws.config().cairo_plugins().fetch(package_id)?;
let instance = plugin.instantiate()?;
builder.with_plugin_suite(instance.plugin_suite());
} else if let Some(prebuilt) = &plugin_info.prebuilt {
proc_macros.register_instance(prebuilt.clone());
} else {
proc_macros.register(plugin_info.package.clone(), ws.config())?;
proc_macros.register_new(plugin_info.package.clone(), ws.config())?;
}
}
let macro_host = Arc::new(proc_macros.into_plugin()?);
Expand Down
5 changes: 3 additions & 2 deletions scarb/src/compiler/plugin/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use itertools::Itertools;
use serde::{Deserialize, Serialize};

use crate::compiler::plugin::builtin::BuiltinCairoRunPlugin;
use crate::compiler::plugin::proc_macro::compilation::SharedLibraryProvider;
use crate::core::{Package, PackageId, TargetKind, Workspace};

use self::builtin::{BuiltinStarkNetPlugin, BuiltinTestPlugin};
Expand All @@ -29,8 +30,8 @@ pub fn fetch_cairo_plugin(package: &Package, ws: &Workspace<'_>) -> Result<()> {
assert!(package.is_cairo_plugin());
let target = package.fetch_target(&TargetKind::CAIRO_PLUGIN)?;
let props: CairoPluginProps = target.props()?;
// No need to fetch for buildin plugins.
if !props.builtin {
// No need to fetch for builtin or prebuilt plugins.
if !props.builtin && !package.is_prebuilt() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a comment saying, it will not run fetch for plugins with invalid prebuilts, even though they will need to compile with Cargo before being used.

proc_macro::fetch_crate(package, ws)?;
}
Ok(())
Expand Down
34 changes: 34 additions & 0 deletions scarb/src/compiler/plugin/proc_macro/compilation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,14 @@ use ra_ap_toolchain::Tool;
use scarb_ui::{Message, OutputFormat};
use serde::{Serialize, Serializer};
use serde_json::value::RawValue;
use std::env::consts::DLL_SUFFIX;
use std::fmt::Display;
use std::fs;
use std::io::{Seek, SeekFrom};
use std::ops::Deref;
use std::process::Command;
use tar::Archive;
use target_triple::target;
use tracing::trace_span;

pub const PROC_MACRO_BUILD_PROFILE: &str = "release";
Expand All @@ -31,6 +33,10 @@ pub trait SharedLibraryProvider {
fn target_path(&self, config: &Config) -> Filesystem;
/// Location of the shared library for the package.
fn shared_lib_path(&self, config: &Config) -> Result<Utf8PathBuf>;
/// Location of the prebuilt binary for the package.
fn prebuilt_lib_path(&self) -> Result<Utf8PathBuf>;
/// Returns true if the package contains a prebuilt binary.
fn is_prebuilt(&self) -> bool;
}

impl SharedLibraryProvider for Package {
Expand Down Expand Up @@ -61,6 +67,34 @@ impl SharedLibraryProvider for Package {
.path_unchecked()
.join(lib_name))
}

fn prebuilt_lib_path(&self) -> Result<Utf8PathBuf> {
let target_triple = target!();

let prebuilt_name = format!(
"{name}_v{version}_{target}{suffix}",
name = self.id.name,
version = self.id.version,
target = target_triple,
suffix = DLL_SUFFIX
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use libloading::library_filename instead, to be consistent with shared_lib_path?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've considered this as well. Unfortunately, it will lead to naming conflicts, as for example library with a name libsome_macro.so will be produced both for aarch64-unknown-linux-gnu and x86_64-unknown-linux-gnu.

Therefore I used format as described in the issue:

Within this directory, binaries for all platforms will follow the following naming pattern: ${PACKAGE_NAME}_v${PACKAGE_VERSION}_${TARGET}.${so|dylib|dll}

On the second look though

some_macro_v0.1.0_aarch64-apple-darwin.dylib
some_macro_v0.1.0_aarch64-unknown-linux-gnu.so
some_macro_v0.1.0_x86_64-apple-darwin.dylib
some_macro_v0.1.0_x86_64-unknown-linux-gnu.so

we may consider dropping ${PACKAGE_NAME}_v${PACKAGE_VERSION}_ to slightly simplify these.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just meant replace DLL_SUFFIX with call to libloading::library_filename which adds the extension for you

Copy link
Contributor Author

@DelevoXDG DelevoXDG Dec 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

libloading::library_filename

It also adds DLL_PREFIX, which can be either "" or lib, which seems more confusing than helpful, especially since users need to name the files manually.

What do you think about dropping the ${PACKAGE_NAME}_v${PACKAGE_VERSION}_ prefix though? It would simplify things and prevent naming issues caused by mismatched Cargo and Scarb names or versions. The downside is that users might accidentally reuse a prebuilt binary after a version update, but this seems like a minor issue to me.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It also adds DLL_PREFIX, which can be either "" or lib, which seems more confusing than helpful, especially since users need to name the files manually.

This is what Cargo compiles this packages into. Forcing macro authors to remove the prefix from compiled file is imho more confusing, that sticking to the conventional naming.

naming issues caused by mismatched Cargo and Scarb names or versions

What do you mean?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you mean?

I’m referring to the reason behind this warning.

When naming build artifacts, Cargo uses the package name specified in the Cargo manifest, which:

  1. May differ from the name specified in the Scarb manifest: Support packaging cairo-plugins #1605 (comment)
  2. Must be resolved manually via cargo_metadata: Support packaging cairo-plugins #1605 (comment)

The problem with 1. is that users will have to name packages manually, leading to confusion about whether the package name from the Cargo or Scarb manifest should be used - mistakes wills surely be made.
The problem with 2. is that using proc macros without Cargo is one of the motivations for introducing loading prebuilts. Relying on cargo_metadata would make that impossible.

This is what Cargo compiles this packages into. Forcing macro authors to remove the prefix from compiled file is imho more confusing, that sticking to the conventional naming.

While it’s true that Cargo compiles libraries with a prefix, the resulting libraries must be renamed anyway to avoid conflicts:

libsome_macro.dylib  
libsome_macro.so  
libsome_macro.dylib  
libsome_macro.so  
some_macro.dll  

Since renaming is inevitable, I argue that the simplest and most consistent naming convention - requiring minimal amount of documentation reading from the user - is:

aarch64-apple-darwin.dylib  
aarch64-unknown-linux-gnu.so  
x86_64-apple-darwin.dylib  
x86_64-unknown-linux-gnu.so  
x86_64-pc-windows-msvc.so  

This approach seems preferable to me alternatives like:

libsome_macro_v0.1.0_aarch64-apple-darwin.dylib  
libsome_macro_v0.1.0_aarch64-unknown-linux-gnu.so  
libsome_macro_v0.1.0_x86_64-apple-darwin.dylib  
libsome_macro_v0.1.0_x86_64-unknown-linux-gnu.so  
some_macro_v0.1.0_x86_64-pc-windows-msvc.so  

or:

libaarch64-apple-darwin.dylib  
libaarch64-unknown-linux-gnu.so  
libx86_64-apple-darwin.dylib  
libx86_64-unknown-linux-gnu.so  
x86_64-pc-windows-msvc.dll  

What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem with 1. is that users will have to name packages manually, leading to confusion about whether the package name from the Cargo or Scarb manifest should be used - mistakes wills surely be made.

True! Sounds like we should turn this warning into a hard error soon, to warn macro authors early on about the mismatch 😄

While it’s true that Cargo compiles libraries with a prefix, the resulting libraries must be renamed anyway to avoid conflicts:

Makes sense. So let's drop the prefix indeed. I would still keep the name + version, just to make sure.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So let's drop the prefix indeed.

👍🏻

I would still keep the name + version, just to make sure.

The current version does so, but I feel strongly that we should drop the name and version as well. They're redundant since this info is already available in the manifest, tarball name, and elsewhere, which potentially just makes things just a bit harder fot the macro developers. The only potential benefit I see is reducing the likelihood of outdated binaries being included. Unless you have any other reasons to keep it, I don't think this downside is really that important and justifies the trade-off.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It just makes it less likely to ship wrong version if some build script fails, so it's an advantage for me. I don't think adding this is really a problem.

);

let prebuilt_path = self
.root()
.join("target")
.join("scarb")
.join("cairo-plugin")
.join(prebuilt_name);

prebuilt_path
.exists()
.then_some(prebuilt_path)
.ok_or_else(|| anyhow!("prebuilt library not found"))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the path is expected to not be defined in some cases, this should be an Option instead of Result. Let's handle the error where path missing is an actual error.

}

fn is_prebuilt(&self) -> bool {
self.prebuilt_lib_path().is_ok()
}
Comment on lines +95 to +97
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally, I find this function more confusing than helpful, as all it does is check if path exist - it still doesn't mean that macro can be actually loaded, or that the compilation will not be run, but the name does not reflect that. Maybe we should just inline prebuilt_path().is_some() instead? Sounds short enough to me. 🤔

}

pub fn compile_unit(unit: ProcMacroCompilationUnit, ws: &Workspace<'_>) -> Result<()> {
Expand Down
21 changes: 15 additions & 6 deletions scarb/src/compiler/plugin/proc_macro/ffi.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::core::{Config, Package, PackageId};
use crate::core::{Package, PackageId};
use anyhow::{ensure, Context, Result};
use cairo_lang_defs::patcher::PatchBuilder;
use cairo_lang_macro::{
Expand Down Expand Up @@ -61,11 +61,20 @@ impl Debug for ProcMacroInstance {

impl ProcMacroInstance {
/// Load shared library
pub fn try_new(package: Package, config: &Config) -> Result<Self> {
let lib_path = package
.shared_lib_path(config)
.context("could not resolve shared library path")?;
let plugin = unsafe { Plugin::try_new(lib_path.to_path_buf())? };
pub fn try_new(package_id: PackageId, lib_path: Utf8PathBuf) -> Result<Self> {
let plugin = unsafe { Plugin::try_new(lib_path)? };
Ok(Self {
expansions: unsafe { Self::load_expansions(&plugin, package_id)? },
package_id,
plugin,
})
}

pub fn try_load_prebuilt(package: Package) -> Result<Self> {
let prebuilt_path = package
.prebuilt_lib_path()
.context("could not resolve prebuilt library path")?;
let plugin = unsafe { Plugin::try_new(prebuilt_path)? };
Ok(Self {
expansions: unsafe { Self::load_expansions(&plugin, package.id)? },
package_id: package.id,
Expand Down
16 changes: 12 additions & 4 deletions scarb/src/compiler/plugin/proc_macro/host.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
use crate::compiler::plugin::proc_macro::compilation::SharedLibraryProvider;
use crate::compiler::plugin::proc_macro::{
Expansion, ExpansionKind, FromSyntaxNode, ProcMacroInstance,
};
use crate::core::{Config, Package, PackageId};
use anyhow::{ensure, Result};
use anyhow::{ensure, Context, Result};
use cairo_lang_defs::ids::{ModuleItemId, TopLevelLanguageElementId};
use cairo_lang_defs::patcher::{PatchBuilder, RewriteNode};
use cairo_lang_defs::plugin::{
Expand Down Expand Up @@ -1150,9 +1151,16 @@ pub struct ProcMacroHost {
}

impl ProcMacroHost {
pub fn register(&mut self, package: Package, config: &Config) -> Result<()> {
let instance = ProcMacroInstance::try_new(package, config)?;
self.macros.push(Arc::new(instance));
pub fn register_instance(&mut self, instance: Arc<ProcMacroInstance>) {
self.macros.push(instance);
}

pub fn register_new(&mut self, package: Package, config: &Config) -> Result<()> {
let lib_path = package
.shared_lib_path(config)
.context("could not resolve shared library path")?;
let instance = ProcMacroInstance::try_new(package.id, lib_path)?;
self.register_instance(Arc::new(instance));
Ok(())
}

Expand Down
Loading
Loading