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.

Sign up for GitHub

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

chore(docs): correction of data type of loop index #3609

Closed
wants to merge 1 commit into from
+1 −1
Closed

chore(docs): correction of data type of loop index #3609

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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 docs/docs/language_concepts/02_control_flow.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ for i in 0..10 {
};
```

The index for loops is of type `u64`.
The index for loops is of type `field`.
Copy link
Contributor

Choose a reason for hiding this comment

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

We changed this to be u64 -- do you see something different when you run Noir programs?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kevaundray yes, even with 0.19.2, i get this error :

error: Comparisons are invalid on Field types. Try casting the operands to a sized integer type first

if i do something like this :

    for i in 0..8 {
        for j in 0..8 {                   
             if  (i + j < 8) {...}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kevaundray oh i see, 8 is of type field, this code works like this :
if (i+j < 8 as u64) {...}

my bad!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

well, actually, the behaviour is odd, this compiles as well :

  if  (i as u64 + j as u64 < 8) {...}

It seems one of the two operands needs to be converted to a uint

Copy link
Contributor

Choose a reason for hiding this comment

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

I think thats possibly a separate issue -- I would have expected 8 to be automatically casted to a u64 without the explicit cast

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It seems that 8 is automatically casted to a u64 only if we explicitly declare i and j as u64 in the comparison.

Copy link
Contributor

Choose a reason for hiding this comment

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

can you open an issue for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure. I'll do it tomorrow in the morning as soon as i am available.
Should i close this PR as it is finally irrelevant ?

Copy link
Contributor

Choose a reason for hiding this comment

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

This may be an ordering issue with two type constraints. IIRC i and j are not actually u64s, they just default to u64 if no other type is specified. The same is the case for the literal 8, although it defaults to Field instead.

The important part is that internally we have a TypeVariableKind::IntegerOrField (for 8) but not a corresponding kind for just integer / u64. The delayed_type_check hack is used instead which is why this defaults to Field and is not caught. The fix for this is to remove the hack and have a dedicated type variable kind - so yes this should be a separate issue. This PR can be closed.


## If Expressions

Expand Down