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

Depreciation of . by :: in paths #1694

Merged
merged 30 commits into from
Aug 23, 2024
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
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
2 changes: 1 addition & 1 deletion airgen/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -341,7 +341,7 @@ impl<'a> ASMPILConverter<'a> {
.find(|o| o.name == callable)
.unwrap_or_else(|| {
panic!(
"function/operation not found: {}.{}",
"function/operation not found: {}::{}",
gzanitti marked this conversation as resolved.
Show resolved Hide resolved
&instance.name, callable
)
});
Expand Down
30 changes: 15 additions & 15 deletions analysis/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,9 @@ machine Main with degree: 16 {
reg Y[<=];
reg A;

instr identity X -> Y = sub.identity;
instr one -> Y = sub.one;
instr nothing = sub.nothing;
instr identity X -> Y = sub::identity;
gzanitti marked this conversation as resolved.
Show resolved Hide resolved
instr one -> Y = sub::one;
instr nothing = sub::nothing;

function main {
start:
Expand Down Expand Up @@ -226,9 +226,9 @@ The diff for our example program is as follows:
- reg A;
+ operation main<2>;
// external instructions are removed and replaced with links, see down below
- instr identity X -> Y = sub.identity;
- instr one -> Y = sub.one;
- instr nothing = sub.nothing;
- instr identity X -> Y = sub::identity;
- instr one -> Y = sub::one;
- instr nothing = sub::nothing;
-
- function main {
- start:
Expand Down Expand Up @@ -295,9 +295,9 @@ The diff for our example program is as follows:
+
+
// we use links to encode cross-machine calls
+ link instr_identity => sub.identity X -> Y;
+ link instr_one => sub.one -> Y;
+ link instr_nothing => sub.nothing;
+ link instr_identity => sub::identity X -> Y;
+ link instr_one => sub::one -> Y;
+ link instr_nothing => sub::nothing;
```

### Block enforcer
Expand Down Expand Up @@ -440,9 +440,9 @@ machine Main with degree: 16, latch: instr_return, operation_id: _operation_id {
}


link instr_identity X -> Y => sub.identity;
link instr_one -> Y => sub.one;
link instr_nothing => sub.nothing;
link instr_identity X -> Y => sub::identity;
link instr_one -> Y => sub::one;
link instr_nothing => sub::nothing;

}
```
Expand Down Expand Up @@ -616,9 +616,9 @@ pol constant _block_enforcer_last_step = [0]* + [1];
pol commit _operation_id_no_change;
_operation_id_no_change = ((1 - _block_enforcer_last_step) * (1 - instr_return));
(_operation_id_no_change * (_operation_id' - _operation_id)) = 0;
instr_identity $ [ 2, X, Y ] in main_sub.instr_return $ [ main_sub._operation_id, main_sub._input_0, main_sub._output_0 ];
instr_one $ [ 4, Y ] in main_sub.instr_return $ [ main_sub._operation_id, main_sub._output_0 ];
instr_nothing $ [ 3 ] in main_sub.instr_return $ [ main_sub._operation_id ];
instr_identity $ [ 2, X, Y ] in main_sub::instr_return $ [ main_sub::_operation_id, main_sub::_input_0, main_sub::_output_0 ];
instr_one $ [ 4, Y ] in main_sub::instr_return $ [ main_sub::_operation_id, main_sub::_output_0 ];
instr_nothing $ [ 3 ] in main_sub::instr_return $ [ main_sub::_operation_id ];
pol constant _linker_first_step = [1] + [0]*;
(_linker_first_step * (_operation_id - 2)) = 0;
namespace main_sub(16);
Expand Down
11 changes: 1 addition & 10 deletions ast/src/analyzed/display.rs
Original file line number Diff line number Diff line change
Expand Up @@ -454,16 +454,7 @@ impl Display for PolynomialReference {
fn fmt(&self, f: &mut Formatter<'_>) -> Result {
if let Some(type_args) = &self.type_args {
gzanitti marked this conversation as resolved.
Show resolved Hide resolved
if !type_args.is_empty() {
// We need to add a `::`-component, so the name should not contain a `.`.
// NOTE: This special handling can be removed once we remove
// the `to_dotted_string` function.
let name = if self.name.contains('.') {
// Re-format the name with ``::`-separators.
SymbolPath::from_str(&self.name).unwrap().to_string()
} else {
self.name.clone()
};
write!(f, "{name}::{}", format_type_args(type_args))?;
write!(f, "{}::{}", self.name, format_type_args(type_args))?;
return Ok(());
}
}
Expand Down
20 changes: 6 additions & 14 deletions ast/src/parsed/asm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -162,11 +162,9 @@ impl SymbolPath {
self
}

/// Formats the path and uses `.` as separator if
/// there are at most two components.
/// Formats the path using `::` as separator
gzanitti marked this conversation as resolved.
Show resolved Hide resolved
pub fn to_dotted_string(&self) -> String {
let separator = if self.parts.len() <= 2 { "." } else { "::" };
self.parts.iter().format(separator).to_string()
self.parts.iter().format("::").to_string()
}

pub fn try_to_identifier(&self) -> Option<&String> {
Expand Down Expand Up @@ -207,14 +205,10 @@ impl SymbolPath {
impl FromStr for SymbolPath {
type Err = String;

/// Parses a symbol path both in the "a.b" and the "a::b" notation.
/// Parses a symbol path using the "::" notation.
fn from_str(s: &str) -> Result<Self, String> {
let (dots, double_colons) = (s.matches('.').count(), s.matches("::").count());
if dots != 0 && double_colons != 0 {
Err(format!("Path mixes \"::\" and \".\" separators: {s}"))?
}
let parts = s
.split(if double_colons > 0 { "::" } else { "." })
.split("::")
.map(|s| {
if s == "super" {
Part::Super
Expand Down Expand Up @@ -362,11 +356,9 @@ impl AbsoluteSymbolPath {
Self { parts }
}

/// Formats the path without leading `::` and uses `.` as separator if
/// there are at most two components.
/// Formats the path without leading `::` and uses `::` as separator
pub fn to_dotted_string(&self) -> String {
let separator = if self.parts.len() <= 2 { "." } else { "::" };
self.parts.join(separator)
self.parts.join("::")
gzanitti marked this conversation as resolved.
Show resolved Hide resolved
}
}

Expand Down
10 changes: 5 additions & 5 deletions ast/src/parsed/display.rs
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ impl Display for CallableRef {
fn fmt(&self, f: &mut Formatter<'_>) -> Result {
write!(
f,
"{}{}.{}({})",
"{}{}::{}({})",
gzanitti marked this conversation as resolved.
Show resolved Hide resolved
match &self.params.outputs[..] {
[] => "".to_string(),
[output] => format!("{output} = "),
Expand Down Expand Up @@ -1216,12 +1216,12 @@ mod tests {
"a | b * (c << d + e) & (f ^ g) = h * (i + g);",
),
(
"instr_or $ [0, X, Y, Z] is (main_bin.latch * main_bin.sel[0]) $ [main_bin.operation_id, main_bin.A, main_bin.B, main_bin.C];",
"instr_or $ [0, X, Y, Z] is main_bin.latch * main_bin.sel[0] $ [main_bin.operation_id, main_bin.A, main_bin.B, main_bin.C];",
"instr_or $ [0, X, Y, Z] is (main_bin::latch * main_bin::sel[0]) $ [main_bin::operation_id, main_bin::A, main_bin::B, main_bin::C];",
"instr_or $ [0, X, Y, Z] is main_bin::latch * main_bin::sel[0] $ [main_bin::operation_id, main_bin::A, main_bin::B, main_bin::C];",
),
(
"instr_or $ [0, X, Y, Z] is main_bin.latch * main_bin.sel[0] $ [main_bin.operation_id, main_bin.A, main_bin.B, main_bin.C];",
"instr_or $ [0, X, Y, Z] is main_bin.latch * main_bin.sel[0] $ [main_bin.operation_id, main_bin.A, main_bin.B, main_bin.C];",
"instr_or $ [0, X, Y, Z] is main_bin::latch * main_bin::sel[0] $ [main_bin::operation_id, main_bin::A, main_bin::B, main_bin::C];",
"instr_or $ [0, X, Y, Z] is main_bin::latch * main_bin::sel[0] $ [main_bin::operation_id, main_bin::A, main_bin::B, main_bin::C];",
),
(
"pc' = (1 - first_step') * ((((instr__jump_to_operation * _operation_id) + (instr__loop * pc)) + (instr_return * 0)) + ((1 - ((instr__jump_to_operation + instr__loop) + instr_return)) * (pc + 1)));",
Expand Down
2 changes: 1 addition & 1 deletion backend/src/composite/split.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ pub(crate) fn machine_witness_columns<F: FieldElement>(
panic!("Machine {machine_name} has witness columns of different sizes")
}
});
let dummy_column_name = format!("{machine_name}.{DUMMY_COLUMN_NAME}");
let dummy_column_name = format!("{machine_name}::{DUMMY_COLUMN_NAME}");
chriseth marked this conversation as resolved.
Show resolved Hide resolved
let dummy_column = vec![F::zero(); size];
iter::once((dummy_column_name, dummy_column))
.chain(machine_columns.into_iter().cloned())
Expand Down
2 changes: 1 addition & 1 deletion backend/src/estark/json_exporter/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ fn fixup_name(name: &str) -> String {
if name.contains('.') {
Copy link
Member

Choose a reason for hiding this comment

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

This cannot be the case any more, but I think the part you changed should remain as it is, shouldn't it? It replaces the last :: by a ..

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It was not being replaced correctly. With the original code, a::b::c becomes a::b.:c, with this change, it now returns a::b.c which I understand is the correct version.

name.to_string()
} else if let Some(last) = name.rfind("::") {
format!("{}.{}", &name[..last], &name[last + 1..])
format!("{}.{}", &name[..last], &name[last + 2..])
} else {
panic!("Witness or intermediate column is not inside a namespace: {name}");
}
Expand Down
2 changes: 1 addition & 1 deletion backend/src/estark/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ struct EStarkFilesCommon<F: FieldElement> {
degree: DegreeType,
pil: PIL,
/// If this field is present, it means the constants were patched with
/// "main.first_step" column and must be written again to a file.
/// "main::first_step" column and must be written again to a file.
constants: Arc<Fixed<F>>,
output_dir: Option<PathBuf>,
proof_type: ProofType,
Expand Down
2 changes: 1 addition & 1 deletion backend/src/estark/starky_wrapper.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ fn create_stark_setup(
const_pols,
&mut pil,
params,
Some("main.first_step".to_string()),
Some("main::first_step".to_string()),
)
.unwrap()
}
Expand Down
Loading
Loading