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 +4 -1
1 changed files with 4 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit e75ec35687 - Show all commits

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

@ -354,7 +354,10 @@ impl TopLevelComposer {
if let Ok(v) = v.try_into() { if let Ok(v) = v.try_into() {
Ok(SymbolValue::I32(v)) Ok(SymbolValue::I32(v))
} else { } else {
Ok(SymbolValue::I64(v.try_into().unwrap())) Err(format!(
ychenfo marked this conversation as resolved Outdated
pca006132 commented 2021-11-19 15:38:48 +08:00
Outdated
Review
Copy Link

Should we just accept int32? int64 is handled in the call part, e.g. int64(1234).

Should we just accept int32? int64 is handled in the call part, e.g. `int64(1234)`.
ychenfo commented 2021-11-20 05:00:31 +08:00
Outdated
Review
Copy Link

this is handled in the new commit below

this is handled in the new commit below
"int64 default parameter should be specified explicitly by `int64()` at {}",
ychenfo marked this conversation as resolved Outdated
sb10q commented 2021-11-20 13:52:22 +08:00
Outdated
Review
Copy Link

Isn't that an internal compiler error if this code is executed, not a problem with the user code?
Using unreachable!() sounds more appropriate here.

Isn't that an internal compiler error if this code is executed, not a problem with the user code? Using ``unreachable!()`` sounds more appropriate here.
sb10q commented 2021-11-20 13:52:32 +08:00
Outdated
Review
Copy Link

Isn't that an internal compiler error if this code is executed, not a problem with the user code?
Using unreachable!() sounds more appropriate here.

Isn't that an internal compiler error if this code is executed, not a problem with the user code? Using ``unreachable!()`` sounds more appropriate here.
pca006132 commented 2021-11-20 14:39:07 +08:00
Outdated
Review
Copy Link

No I don't think so? It is possible for users to specify some default value outside the range of int64. I think a more appropriate error message would be value out of range and only say int64 if the value is really an int64?

No I don't think so? It is possible for users to specify some default value outside the range of int64. I think a more appropriate error message would be value out of range and only say `int64` if the value is really an int64?
sb10q commented 2021-11-20 14:39:43 +08:00
Outdated
Review
Copy Link

OK, yes.

OK, yes.
ychenfo commented 2021-11-21 22:06:50 +08:00
Outdated
Review
Copy Link

Thanks, this error message is updated in the new commit

Thanks, this error message is updated in the new commit
loc
))
} }
} }
Constant::Float(v) => Ok(SymbolValue::Double(*v)), Constant::Float(v) => Ok(SymbolValue::Double(*v)),