M-Labs/nac3
M-Labs
/
nac3
8
0
Fork 10
Code Issues 99 Pull Requests 21 Releases Wiki Activity

core: improve binop and cmpop error messages #439

Merged
sb10q merged 1 commits from improve-error-432 into master 2024-08-17 17:37:21 +08:00
Conversation 6 Commits 1 Files Changed 5 +261 -105
lyken commented 2024-06-27 13:49:16 +08:00
Collaborator
Copy Link

NOTE: This PR depends on #437 (core: improve function call errors).

Fixes #436 (Improve type error messages for operator functions) (originally from #432 (type inferencer crash on artiq_sinara_tester)).

For #436 (Improve type error messages for operator functions), in particular, the error message is now:

self.core.delay(100*ms)

# Unsupported operand type(s) for *: 'int32' and 'float' (right operand should have type N) at /home/lyken/artiq/artiq/frontend/artiq_sinara_tester.py:469:36
# 
# Notes:
#     N ∈ {int32, ndarray[int32, ndarray_ndims]}
#     ndarray_ndims ∈ {uint32}

For ill-typed binary operations:

def run() -> int32:
    ok = 1.0 * 3
    return 0

# 1 error(s) occurred during top level analysis.
# =========== ERROR 1/1 ============
# Unsupported operand type(s) for +: 'float' and 'int32' (right operand should have type N) at src/test_operator_error.py:2:14
# 
# Notes:
#     N ∈ {float, ndarray[float, ndarray_ndims]}
#     ndarray_ndims ∈ {uint64}
# ==================================
# thread 'main' panicked at nac3standalone/src/main.rs:354:9:
# top level analysis failed

For ill-typed comparison operations:

def run() -> int32:
    ok = 1.0 <= 3
    return 0

# 1 error(s) occurred during top level analysis.
# =========== ERROR 1/1 ============
# '<=' not supported between instances of 'float' and 'int32' (right operand should have type N) at src/test_operator_error.py:2:14
# 
# Notes:
#     N ∈ {float, ndarray[float, ndarray_ndims]}
#     ndarray_ndims ∈ {uint64}
# ==================================
# thread 'main' panicked at nac3standalone/src/main.rs:354:9:
# top level analysis failed
NOTE: This PR depends on https://git.m-labs.hk/M-Labs/nac3/pulls/437 (core: improve function call errors). Fixes https://git.m-labs.hk/M-Labs/nac3/issues/436 (Improve type error messages for operator functions) (originally from https://git.m-labs.hk/M-Labs/nac3/issues/432 (type inferencer crash on artiq_sinara_tester)). For https://git.m-labs.hk/M-Labs/nac3/issues/436 (Improve type error messages for operator functions), in particular, the error message is now: ```python self.core.delay(100*ms) # Unsupported operand type(s) for *: 'int32' and 'float' (right operand should have type N) at /home/lyken/artiq/artiq/frontend/artiq_sinara_tester.py:469:36 # # Notes: # N ∈ {int32, ndarray[int32, ndarray_ndims]} # ndarray_ndims ∈ {uint32} ``` For ill-typed binary operations: ```python def run() -> int32: ok = 1.0 * 3 return 0 # 1 error(s) occurred during top level analysis. # =========== ERROR 1/1 ============ # Unsupported operand type(s) for +: 'float' and 'int32' (right operand should have type N) at src/test_operator_error.py:2:14 # # Notes: # N ∈ {float, ndarray[float, ndarray_ndims]} # ndarray_ndims ∈ {uint64} # ================================== # thread 'main' panicked at nac3standalone/src/main.rs:354:9: # top level analysis failed ``` For ill-typed comparison operations: ```python def run() -> int32: ok = 1.0 <= 3 return 0 # 1 error(s) occurred during top level analysis. # =========== ERROR 1/1 ============ # '<=' not supported between instances of 'float' and 'int32' (right operand should have type N) at src/test_operator_error.py:2:14 # # Notes: # N ∈ {float, ndarray[float, ndarray_ndims]} # ndarray_ndims ∈ {uint64} # ================================== # thread 'main' panicked at nac3standalone/src/main.rs:354:9: # top level analysis failed ```
lyken requested review from sb10q 2024-06-27 13:49:25 +08:00
lyken requested review from derppening 2024-06-27 13:49:25 +08:00
lyken added a new dependency 2024-06-27 13:58:50 +08:00
M-Labs/artiq-zynq#301 - subkernels: raise exceptions to kernel
lyken removed a dependency 2024-06-27 13:58:56 +08:00
M-Labs/artiq-zynq#301 - subkernels: raise exceptions to kernel
lyken added a new dependency 2024-06-27 13:59:03 +08:00
#437 core: improve function call errors
lyken force-pushed improve-error-432 from 8dfd7f2a7d to 889bb20844 2024-06-27 14:08:29 +08:00 Compare
lyken commented 2024-06-27 14:09:14 +08:00
Author
Collaborator
Copy Link

Force-pushed to rebase onto master since #437 has been merged.

Force-pushed to rebase onto master since https://git.m-labs.hk/M-Labs/nac3/pulls/437 has been merged.
lyken force-pushed improve-error-432 from 889bb20844 to 3087f77ed8 2024-06-27 14:55:51 +08:00 Compare
derppening requested changes 2024-06-27 15:06:47 +08:00
derppening left a comment
Collaborator
Copy Link

I'd prefer it to be called "invalid operand type(s) float and int32 for operator +"

I'd prefer it to be called "invalid operand type(s) `float` and `int32` for operator `+`"
nac3core/src/typecheck/magic_methods.rs Outdated
@ -73,1 +49,3 @@
_ => None,
impl OpInfo {
#[must_use]
pub fn from_binop(op: Operator, variant: BinOpVariant) -> Self {
derppening commented 2024-06-27 15:00:26 +08:00
Collaborator
Copy Link

Why not just implement these using the From trait?

Why not just implement these using the `From` trait?
lyken commented 2024-06-27 15:08:36 +08:00
Author
Collaborator
Copy Link

It is possible for Cmpop and Unaryop, but not for Operator (binary operator) since it takes an extra argument, although I could create:

struct BinaryOp {
  base_op: Operator,
  variant: BinOpVariant,
}
It is possible for `Cmpop` and `Unaryop`, but not for `Operator` (binary operator) since it takes an extra argument, although I could create: ```rust struct BinaryOp { base_op: Operator, variant: BinOpVariant, } ```
derppening commented 2024-06-27 15:13:06 +08:00
Collaborator
Copy Link

Just implement From<(Operator, BinOpVariant)> for OpInfo then.

Just implement `From<(Operator, BinOpVariant)> for OpInfo` then.
lyken commented 2024-06-27 15:13:18 +08:00
Author
Collaborator
Copy Link

Also I don't think From trait is meant to be used like that - it's purpose is for type conversions. Perhaps I will create a trait HasOpInfo.

Also I don't think `From` trait is meant to be used like that - it's purpose is for type conversions. Perhaps I will create a `trait HasOpInfo`.
nac3core/src/typecheck/typedef/mod.rs Outdated
@ -75,1 +77,4 @@
/// Extra details about how a [`Call`] was written by the user.
#[derive(Debug, Clone)]
pub enum CallInfo {
derppening commented 2024-06-27 15:02:17 +08:00
Collaborator
Copy Link

Call this struct OperatorInfo.

Call this struct `OperatorInfo`.
lyken commented 2024-06-27 15:10:20 +08:00
Author
Collaborator
Copy Link

CallInfo was intended to be general for any kind of function call. But to be honest I cannot think of anything other than normal function calls and operator calls (including is and is not and its friends). I will rename it.

`CallInfo` was intended to be general for any kind of function call. But to be honest I cannot think of anything other than normal function calls and operator calls (including `is` and `is not` and its friends). I will rename it.
derppening marked this conversation as resolved
nac3core/src/typecheck/typedef/mod.rs Outdated
@ -80,6 +108,7 @@ pub struct Call {
pub ret: Type,
pub fun: RefCell<Option<Type>>,
pub loc: Option<Location>,
pub info: CallInfo,
derppening commented 2024-06-27 15:02:46 +08:00
Collaborator
Copy Link

Make this Option<OperatorInfo>.

Make this `Option<OperatorInfo>`.
derppening marked this conversation as resolved
lyken commented 2024-06-27 15:23:16 +08:00
Author
Collaborator
Copy Link

I'd prefer it to be called invalid operand type(s) float and int32 for operator +

The current error message follows Python's:

>>> 3.0 + "a"
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: unsupported operand type(s) for +: 'float' and 'str'

Should we still make the change?

> I'd prefer it to be called `invalid operand type(s) float and int32 for operator +` The current error message follows Python's: ``` >>> 3.0 + "a" Traceback (most recent call last): File "<stdin>", line 1, in <module> TypeError: unsupported operand type(s) for +: 'float' and 'str' ``` Should we still make the change?
derppening commented 2024-06-27 15:33:52 +08:00
Collaborator
Copy Link

I'd prefer it to be called invalid operand type(s) float and int32 for operator +

The current error message follows Python's:

>>> 3.0 + "a"
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: unsupported operand type(s) for +: 'float' and 'str'

Should we still make the change?

Use Python's error message format then.

> > I'd prefer it to be called `invalid operand type(s) float and int32 for operator +` > > The current error message follows Python's: > ``` > >>> 3.0 + "a" > Traceback (most recent call last): > File "<stdin>", line 1, in <module> > TypeError: unsupported operand type(s) for +: 'float' and 'str' > ``` > > Should we still make the change? Use Python's error message format then.
lyken force-pushed improve-error-432 from d0ea69b7ae to c57e0936d5 2024-06-27 16:04:14 +08:00 Compare
lyken commented 2024-06-27 16:04:18 +08:00
Author
Collaborator
Copy Link

Revised

Revised
lyken requested review from derppening 2024-06-27 16:05:51 +08:00
lyken added a new dependency 2024-06-27 17:12:41 +08:00
#432 type inferencer crash on artiq_sinara_tester
lyken force-pushed improve-error-432 from c57e0936d5 to bc9a9b28c8 2024-07-04 16:33:58 +08:00 Compare
lyken force-pushed improve-error-432 from bc9a9b28c8 to 1e56109a52 2024-07-05 15:25:43 +08:00 Compare
lyken commented 2024-07-05 15:26:01 +08:00
Author
Collaborator
Copy Link

Rebased to resolve conflicts

Rebased to resolve conflicts
derppening approved these changes 2024-07-05 16:21:40 +08:00
derppening left a comment
Collaborator
Copy Link

One nit, otherwise LGTM.

One nit, otherwise LGTM.
nac3core/src/typecheck/magic_methods.rs Outdated
@ -62,0 +63,4 @@
/// Helper macro to conveniently build an [`OpInfo`].
///
/// Example usage: `make_info("add", "+")` generates `OpInfo { name: "__add__", symbol: "+" }`
macro_rules! make_info {
derppening commented 2024-07-05 16:19:10 +08:00
Collaborator
Copy Link

Call it make_op_info, since it is unclear what info you are actually producing.

Call it `make_op_info`, since it is unclear what info you are actually producing.
lyken force-pushed improve-error-432 from 1e56109a52 to f52086b706 2024-07-05 16:28:18 +08:00 Compare
lyken commented 2024-07-05 16:28:53 +08:00
Author
Collaborator
Copy Link

Revised by amending the commit that introduced OpInfo

Revised by amending the commit that introduced `OpInfo`
sb10q merged commit f52086b706 into master 2024-07-07 14:21:02 +08:00
sb10q deleted branch improve-error-432 2024-07-07 14:21:03 +08:00
Sign in to join this conversation.
Reviewers
No reviewers
sb10q
derppening
Labels
Clear labels   
future-extension

   
high-priority

   
low-priority

   
UROP0622
No Label
future-extension
high-priority
low-priority
UROP0622
Milestone
Clear milestone
No items
No Milestone
Assignees
Clear assignees
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Blocks
#432 type inferencer crash on artiq_sinara_tester
M-Labs/nac3
Depends on
#437 core: improve function call errors
M-Labs/nac3
Reference: M-Labs/nac3#439
No description provided.
Delete Branch "improve-error-432"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?