Skip to content

Improve field comparison error message#499

Merged
jfecher merged 1 commit intomasterfrom
jf/field-comparisons
Nov 17, 2022
Merged

Improve field comparison error message#499
jfecher merged 1 commit intomasterfrom
jf/field-comparisons

Conversation

@jfecher
Copy link
Contributor

@jfecher jfecher commented Nov 17, 2022

Related issue(s)

Related to #498

Description

The type system used to allow field comparisons, but since these operations are invalid we would eventually fail in SSA with a compiler crash. This fixes the check in the type system to disallow comparison for fields, but allow eq and neq operations.

Checklist

  • I have tested the changes locally.
  • I have formatted the changes with Prettier and/or cargo fmt with default settings.
  • I have linked this PR to the issue(s) that it resolves.
  • I have reviewed the changes on GitHub, line by line.
  • I have ensured all changes are covered in the description.

@jfecher
Copy link
Contributor Author

jfecher commented Nov 17, 2022

Before:

thread 'main' panicked at 'not implemented: Field comparison is not implemented yet, try to cast arguments to integer type', crates/noirc_evaluator/src/ssa/acir_gen.rs:339:29
stack backtrace:
   0: rust_begin_unwind
             at /rustc/e7119a0300b87a3d670408ee8e847c6821b3ae80/library/std/src/panicking.rs:556:5
   1: core::panicking::panic_fmt
             at /rustc/e7119a0300b87a3d670408ee8e847c6821b3ae80/library/core/src/panicking.rs:142:14
   [... omitted ...]
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
exit 101

After:

error: Fields cannot be compared, try casting to an integer first
  ┌─ /Users/jakefecher/Code/Noir/noir/lessthan/src/main.nr:2:15
  │
2 │     constrain x < y;
  │               -----

error: aborting due to 1 previous errors
exit 1

Copy link
Contributor

@guipublic guipublic left a comment

Choose a reason for hiding this comment

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

I think we should enable field comparison, but until then it is better to report the error. We can easily enable it in the type checker when we will do field comparison.

@jfecher jfecher merged commit 06f3852 into master Nov 17, 2022
@jfecher jfecher deleted the jf/field-comparisons branch November 17, 2022 14:09
TomAFrench added a commit to TomAFrench/noir that referenced this pull request Nov 19, 2022
* master:
  Array sort (noir-lang#477)
  Add `--allow-warnings` flag to treat certain errors as warnings (noir-lang#481)
  Improve field comparison error message (noir-lang#499)
  Disable debug output during integration tests (noir-lang#494)
  Aligns build to (noir-lang#443) Refactor stdlib into a separate noir library (noir-lang#496)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants