Collect functions with executable attributes into a debug info field

[maciektr]

This change is 

[maciektr]

crates/cairo-lang-syntax/src/attribute/consts.rs line 39 at r1 (raw file):

Previously, orizi wrote…

so rename to something longer and more cumbersome - for example: execution_entrypoint

if a test will create such a thing - won't compilation recreate a lot of functions?

I wonder whether it would make sense, to use it for marking a main function in cairo-run :thinking: In that case, user would need to use it directly.

[maciektr]

crates/cairo-lang-compiler/src/lib.rs line 150 at r2 (raw file):

Previously, orizi wrote…

i think maybe this function should just not exist. and similarly to how we call compile_contract_in_prepared_db which is contract aware, and compile_test_prepared_db in the test runner - we should call this with a list of the requested artifacts specifically - which would be in the executable list - being plugin based here feels like is going to be bad.

Sorry, what do you mean by list of the requested artifacts :thinking: ? The artifact here is just ProgramArtifact.

[maciektr]

Previously, orizi wrote…

the idea is - same as test plugin that knows what are the relevant functions (so it can fill the artifacts) and the starknet plugin, so can a run builtin - no need for new special attributes - as it logically per such build target in any case.

Well, we would like to push for a generalization of the executable concept here, so that multiple tools (like cairo-run, sncast-scripts, forge etc.) could rely on a single artifact format (sierra.json), without a need to define specialized targets for each tool in upstream (Scarb).

[maciektr]

Previously, orizi wrote…

but this means that things like tests would need to rewrite their code - we probably don't want that as well

Hmm, I am not saying we will push for cairo-test rewrite, but rather that we have tools that are bound to be rewritten already (forge test collection) and would benefit from this. With Scarb procedural macros + executable tag combo, we can run forge tests compiled through Scarb without any additional artefacts -> just sierra.json, thus fully decoupling Forge from the Cairo compilation level. Rewriting cairo-run / sncast scripts to this would be not a lot more than a one-liner, while also providing additional advantages (namely, ability to run code compiled in release mode, without debug ids).

[maciektr]

Ok, thanks so much for elaborating, it makes a lot more sense to me now!

i was talking about all plugins requiring rewriting their code for this to capture it

Well, in case of Forge, which would be the biggest beneficiary of this flow, we need to have a plugin that rewrites test attributes anyway, to move the compilation to Scarb (for full Cairo version <> Forge version decoupling). It would not be an issue there at all.

probably not - so there would be a compilation for testing, with its artifacts, and a different one for proof running, or starknet, or anything of that kind.

While it's of course true, that we should not push for a single artefact format for all use cases, I would still argue that there is a middle ground here. We would not want to have dedicated compilation targets, hardcoded into Scarb, for all possible tools that run Cairo (i.e. Forge/Cast/Dojo/etc) either. Some of them will naturally fall into +/- the same requirements category, i.e. finding a function in Sierra program and running it (which this proposition would enable).

[maciektr]

the artifacts should IMO be guided by the caller - instead of by annotation that will need to be added in all cases in the world

Do you think that we should have some per build input here, so we can only collect executables with specific keys? :thinking:

[maciektr]

crates/cairo-lang-sierra-generator/src/executables.rs line 61 at r1 (raw file):

Previously, mkaput (Marek Kaput) wrote…

Retracting because I have a better idea.

No longer relevant.

[maciektr]

crates/cairo-lang-sierra/src/debug_info.rs line 41 at r1 (raw file):

Previously, mkaput (Marek Kaput) wrote…

We're iterating the structure of this field internally/offline.

Done.

[maciektr]

crates/cairo-lang-defs/src/plugin.rs line 112 at r6 (raw file):

Previously, mkaput (Marek Kaput) wrote…

It doesn't make sense to return vec::IntoIter here. Return an owned Vec (you'll boil down to the same thing). You could also return impl Iterator<Item=String>, but this trait is using in dyn context so this is impossible.

Done.

[maciektr]

crates/cairo-lang-compiler/src/lib.rs line 163 at r8 (raw file):

Previously, orizi wrote…

does this actually make sense? isn't failing more logical?

Hmm, why? You can have a simple Cairo build without any plugins.

[maciektr]

crates/cairo-lang-compiler/src/lib.rs line 163 at r8 (raw file):

Previously, orizi wrote…

You can - but it is meaning less building without a particular target.

I am not sure if I would call it meaning less. It's what we have now with the scarb lib target - it compiles Cairo to Sierra. It can be used for instance to run functions by name, it's what some tooling uses already, or even simply for debugging purposes. I don't think we are ready to abandon it completely now.

[maciektr]

crates/cairo-lang-compiler/src/lib.rs line 163 at r8 (raw file):

Previously, orizi wrote…

so at the very least it makes no sense without --replace-ids.

and should probably have a todo to deprecate.

Yes, logically you are probably right. I don't think it's worth to ban this specific configuration though, especially since we would really have to verify with community if there is any use case we are yet not aware of.

[maciektr]

Previously, orizi wrote…

sounds great - just add a TODO here.

Great, added!