M-Labs/nac3
M-Labs/nac3
8
1
Fork 12
Code Issues 98 Pull Requests 19 Releases Wiki Activity

Constant Default Parameter Support #98

Merged
sb10q merged 7 commits from default_parameter into master 2021-11-23 07:32:09 +08:00
Conversation 3 Commits 7 Files Changed 10 +22 -23
2 changed files with 22 additions and 23 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 6b059fa288 - Show all commits

39
nac3core/src/toplevel/composer.rs
View File

@ -1066,7 +1066,7 @@ impl TopLevelComposer {
.into()); .into());
} }
let mut arg_with_defualt: Vec<(&ast::Located<ast::ArgData<()>>, Option<&ast::Expr>)> = args let arg_with_default: Vec<(&ast::Located<ast::ArgData<()>>, Option<&ast::Expr>)> = args
.args .args
.iter() .iter()
.rev() .rev()
@ -1077,10 +1077,10 @@ impl TopLevelComposer {
.map(|x| -> Option<&ast::Expr> { Some(x) }) .map(|x| -> Option<&ast::Expr> { Some(x) })
.chain(std::iter::repeat(None)) .chain(std::iter::repeat(None))
).collect_vec(); ).collect_vec();
arg_with_defualt.reverse();
arg_with_defualt arg_with_default
.iter() .iter()
.rev()
.map(|(x, default)| -> Result<FuncArg, String> { .map(|(x, default)| -> Result<FuncArg, String> {
let annotation = x let annotation = x
.node .node
@ -1133,12 +1133,11 @@ impl TopLevelComposer {
Ok(FuncArg { Ok(FuncArg {
name: x.node.arg, name: x.node.arg,
ty, ty,
default_value: default.map(|default| { default_value: match default {
match Self::parse_parameter_default_value(default) { None => None,
Ok(res) => res, Some(default) =>
Err(err) => panic!("{}", err), Some(Self::parse_parameter_default_value(default)?)
} }
}),
}) })
}) })
.collect::<Result<Vec<_>, _>>()? .collect::<Result<Vec<_>, _>>()?
@ -1291,7 +1290,7 @@ impl TopLevelComposer {
let mut result = Vec::new(); let mut result = Vec::new();
let mut arg_with_defualt: Vec<(&ast::Located<ast::ArgData<()>>, Option<&ast::Expr>)> = args let arg_with_default: Vec<(&ast::Located<ast::ArgData<()>>, Option<&ast::Expr>)> = args
.args .args
.iter() .iter()
.rev() .rev()
@ -1302,8 +1301,8 @@ impl TopLevelComposer {
.map(|x| -> Option<&ast::Expr> { Some(x) }) .map(|x| -> Option<&ast::Expr> { Some(x) })
.chain(std::iter::repeat(None)) .chain(std::iter::repeat(None))
).collect_vec(); ).collect_vec();
arg_with_defualt.reverse();
for (x, default) in arg_with_defualt { for (x, default) in arg_with_default.into_iter().rev() {
let name = x.node.arg; let name = x.node.arg;
if name != zelf { if name != zelf {
let type_ann = { let type_ann = {
@ -1348,15 +1347,15 @@ impl TopLevelComposer {
let dummy_func_arg = FuncArg { let dummy_func_arg = FuncArg {
name, name,
ty: unifier.get_fresh_var().0, ty: unifier.get_fresh_var().0,
default_value: default.map(|default| { default_value: match default {
if name == "self".into() { None => None,
panic!("`self` parameter cannot take default value at {}", x.location) Some(default) => {
if name == "self".into() {
sb10q commented 2021-11-21 11:41:22 +08:00
Outdated
Review
Copy Link

Technically CPython lets you name self whatever you want, it's only a convention.
So this test should be "the function is a method and we are looking at its first parameter".

Technically CPython lets you name ``self`` whatever you want, it's only a convention. So this test should be "the function is a method and we are looking at its first parameter".
ychenfo commented 2021-11-21 22:32:52 +08:00
Outdated
Review
Copy Link

Would it be acceptable that in nac3, we just enforce and document that the self should always be self and should not be named arbitrarily? As I think this would not cause very big problems and our existing nac3 codebase is already doing this (e.g., here, here, and here)

Would it be acceptable that in nac3, we just enforce and document that the `self` should always be `self` and should not be named arbitrarily? As I think this would not cause very big problems and our existing nac3 codebase is already doing this (e.g., [here](https://git.m-labs.hk/M-Labs/nac3/src/branch/master/nac3core/src/codegen/expr.rs#L293), [here](https://git.m-labs.hk/M-Labs/nac3/src/branch/master/nac3core/src/codegen/expr.rs#L348), and [here](https://github.com/m-labs/artiq/blob/nac3/artiq/language/core.py#L56))
sb10q commented 2021-11-22 11:13:04 +08:00
Outdated
Review
Copy Link

It is acceptable but not super clean - is it difficult to use the condition I proposed instead?

https://github.com/m-labs/artiq/blob/nac3/artiq/language/core.py#L56 is unavoidable without a lot of code (AFAICT) because CPython does not tell you if something is a method until it is bound, but inside NAC3 we have more information.

It is acceptable but not super clean - is it difficult to use the condition I proposed instead? https://github.com/m-labs/artiq/blob/nac3/artiq/language/core.py#L56 is unavoidable without a lot of code (AFAICT) because CPython does not tell you if something is a method until it is bound, but inside NAC3 we have more information.
return Err(format!("`self` parameter cannot take default value at {}", x.location));
}
Some(Self::parse_parameter_default_value(default)?)
} }
match Self::parse_parameter_default_value(default) { }
Ok(res) => res,
Err(err) => panic!("{}", err),
}
})
}; };
// push the dummy type and the type annotation // push the dummy type and the type annotation
// into the list for later unification // into the list for later unification

6
nac3core/src/toplevel/helper.rs
View File

@ -362,7 +362,7 @@ impl TopLevelComposer {
Constant::Tuple(tuple) => Ok(SymbolValue::Tuple( Constant::Tuple(tuple) => Ok(SymbolValue::Tuple(
tuple.iter().map(|x| handle_constant(x, loc)).collect::<Result<Vec<_>, _>>()? tuple.iter().map(|x| handle_constant(x, loc)).collect::<Result<Vec<_>, _>>()?
)), )),
_ => unimplemented!("this constant is not supported now at {}", loc), _ => unimplemented!("this constant is not supported at {}", loc),
} }
} }
match &default.node { match &default.node {
@ -377,10 +377,10 @@ impl TopLevelComposer {
match &args[0].node { match &args[0].node {
ast::ExprKind::Constant { value: Constant::Int(v), .. } => ast::ExprKind::Constant { value: Constant::Int(v), .. } =>
Ok(SymbolValue::I64(v.try_into().unwrap())), Ok(SymbolValue::I64(v.try_into().unwrap())),
_ => panic!("only allow constant integer here at {}", default.location) _ => Err(format!("only allow constant integer here at {}", default.location))
} }
} else { } else {
panic!("only allow constant integer here at {}", default.location) Err(format!("only allow constant integer here at {}", default.location))
} }
} }
ast::ExprKind::Tuple { elts, .. } => Ok(SymbolValue::Tuple(elts ast::ExprKind::Tuple { elts, .. } => Ok(SymbolValue::Tuple(elts