[BUG] Invalid operation Precedence #367

Ph0enixKM · 2024-07-28T19:48:15Z

This will be a hefty one since it requires overhaul of an Expr syntax module and all operators.

What has changed:

The operators are now parsed with the following precedence:

Level	Direction	Precedence Group
	N/A	Unary operators: `!`, `-`, `nameof`
	Left to Right	Parentheses group: `(x)`
1	N/A	Literals: Bool, Number, Text, Array, Null, Status
	Left to Right	Callables: FunctionInvocation, Command
	N/A	Variable access
2	Left to Right	Advanced: `is`, `as`
3	Left to Right	Multiplicative: `*`, `/`
	Left to Right	Modulo: `%`
4	Left to Right	Additive: `+`, `-`
5	Left to Right	Relational: `>`, `<`, `>=`, `<=`
6	Left to Right	Equality: `==`, `!=`
7	Left to Right	Logical `and`
8	Left to Right	Logical `or`
9	Left to Right	Range: `..`, `..=`
10	Right to Left	Ternary `then..else`

Previously everything was parsed from right to left in the same precedence.
We should add this information to the documentation btw.

The error messages related to expressions now highlight the entire expressions they're related to.

The whole parsing couldn't be done in the Heraclitus way to parse everything contained in it's syntax module. This however could influence the development of Heraclitus to create some macro rules that would create an easy way to define precedence. For now as this is a rather important fix - I want to merge what I came up with so far.

Mte90 · 2024-07-29T09:18:29Z

The CI fails ;-)

…o 364-bug-invalid-operation-precedence

KrosFire · 2024-07-29T15:40:37Z

Equality shouldn't have the same precedence as inequality?

Ph0enixKM · 2024-07-29T15:53:46Z

The CI fails because I haven't handled the unary operators yet. Hold on :)

KrosFire

Why did you add AlreadyParsedExpr and AlreadyParsedType? I don't see that we check them anywhere. Expression is an expression - it's parsed by definition, as it is not a token. It's part of the parsing result.

src/modules/expression/precedence.rs

src/modules/expression/binop/add.rs

…easier

Ph0enixKM · 2024-08-01T22:19:13Z

Todo:

handle_binop to handle error messages as well.
Document the work (only once the current state of PR is approved)

This reverts commit 88fc6b0.

Ph0enixKM · 2024-08-01T22:30:33Z

I reverted the format commit because too much conflicts showed up. I'll do the format when we can merge this

Ph0enixKM · 2024-08-01T22:48:59Z

Everyone is welcome to review this PR. The main changes:

expr/macro.rs now holds all of the repeating mechanism that expr/precedence.rs used to hold.
Some traits created such as BinOp, TernOp, UnOp and TypeOp to help me create special types of precedence groups for the macros.

Here is the usage of the new macro:

It reads like this:
A list of groups that are defined like this:

<group_name> @ <group_type> => [ ...<group_modules> ]

If you need any questions, ask me. If you approve this PR, let me know (or just approve). I'll write some guide to aid these changes.

Ph0enixKM · 2024-08-05T15:00:19Z

@KrosFire This PR is now ready for final look.

The wiki has been added: https://github.com/amber-lang/amber/wiki
It's pretty simple, but we can iterate on it as we go

KrosFire

Looks beautiful

src/modules/expression/binop/eq.rs

src/modules/expression/binop/neq.rs

src/modules/expression/binop/or.rs

src/modules/expression/binop/sub.rs

src/modules/expression/binop/and.rs

src/modules/expression/unop/not.rs

src/modules/shorthand/mul.rs

Co-authored-by: Hubert Jabłoński <[email protected]>

Ph0enixKM · 2024-08-07T12:33:08Z

@KrosFire can you reiterate on the changes?

KrosFire

Just small stuff. Looks good overall. Does someone have exp in macros? I've reached my limits XD

src/modules/expression/binop/and.rs

src/modules/expression/binop/range.rs

src/modules/expression/ternop/ternary.rs

…oks diffrent now)

Co-authored-by: Hubert Jabłoński <[email protected]>

KrosFire

LGTM

* feat(expr): overhaul of expression syntax module * feat: adapt binary operator modules to the new structure * merge(precedence): resolve path to command * fix(refactor): Added macros to make writing operator precedence much easier * fix(expr): Attach operators to work with precedence macro * feat(precedence): remove precedence file * fix(format): Cargo clippy and fmt * revert(format): Cargo clippy and fmt This reverts commit 88fc6b0. * fix(lint): resolve clippy errors * Update src/modules/expression/macros.rs Co-authored-by: Hubert Jabłoński <[email protected]> * feat(tests): add validity tests * fix(macro): improved handle binop macro * fix(expr): remove pub from fields * feat(macro): change prev to next * feat(macro): add edge case for one group * Update src/modules/expression/binop/sub.rs Co-authored-by: Hubert Jabłoński <[email protected]> * Update src/modules/expression/binop/eq.rs Co-authored-by: Hubert Jabłoński <[email protected]> * Update src/modules/expression/binop/neq.rs Co-authored-by: Hubert Jabłoński <[email protected]> * Update src/modules/expression/binop/or.rs * Update src/modules/expression/binop/and.rs * feat(error-macro): simplify code in a macro to keep DRY principle * feat(test): Add test for multiple cast * fix(and): remove comment * fix(range): Fix range and remove import from code eval (the stdlib looks diffrent now) * Update src/modules/expression/ternop/ternary.rs Co-authored-by: Hubert Jabłoński <[email protected]> * Update src/modules/expression/ternop/ternary.rs Co-authored-by: Hubert Jabłoński <[email protected]> --------- Co-authored-by: Hubert Jabłoński <[email protected]>

feat(expr): overhaul of expression syntax module

1b5ca42

Ph0enixKM self-assigned this Jul 28, 2024

Ph0enixKM linked an issue Jul 28, 2024 that may be closed by this pull request

[BUG] Invalid operation Precedence #364

Closed

Ph0enixKM changed the title ~~feat(expr): overhaul of expression syntax module~~ [BUG] Invalid operation Precedence Jul 28, 2024

feat: adapt binary operator modules to the new structure

7d60ba9

Ph0enixKM marked this pull request as ready for review July 29, 2024 14:46

This comment was marked as outdated.

Sign in to view

Ph0enixKM added 2 commits July 29, 2024 17:19

Merge branch 'master' of https://github.com/Ph0enixKM/AmberNative int…

89c0bbd

…o 364-bug-invalid-operation-precedence

merge(precedence): resolve path to command

8e5d627

Ph0enixKM marked this pull request as draft July 29, 2024 15:31

KrosFire requested changes Jul 29, 2024

View reviewed changes

src/modules/expression/precedence.rs Outdated Show resolved Hide resolved

src/modules/expression/binop/add.rs Outdated Show resolved Hide resolved

This comment was marked as outdated.

Sign in to view

Ph0enixKM added 3 commits August 1, 2024 18:25

fix(refactor): Added macros to make writing operator precedence much …

6277bfd

…easier

fix(expr): Attach operators to work with precedence macro

0c4c6f5

feat(precedence): remove precedence file

ffa1afb

Ph0enixKM added 2 commits August 1, 2024 22:24

fix(format): Cargo clippy and fmt

88fc6b0

revert(format): Cargo clippy and fmt

6bd14e5

This reverts commit 88fc6b0.

Ph0enixKM added 2 commits August 1, 2024 22:33

merge(master): resolve conflicts

0faf4fc

fix(lint): resolve clippy errors

6ded64f

Ph0enixKM marked this pull request as ready for review August 1, 2024 22:44

Mte90 requested a review from mks-h August 2, 2024 10:53

Ph0enixKM added 5 commits August 3, 2024 16:39

feat(tests): add validity tests

17f4e5c

fix(macro): improved handle binop macro

bb9e214

fix(expr): remove pub from fields

b7dc7cb

feat(macro): change prev to next

685887e

feat(macro): add edge case for one group

afa14a9

Mte90 requested a review from KrosFire August 5, 2024 15:11

KrosFire reviewed Aug 5, 2024

View reviewed changes

Update src/modules/expression/binop/sub.rs

ceda405

Co-authored-by: Hubert Jabłoński <[email protected]>

mks-h removed their request for review August 5, 2024 22:10

Mte90 requested a review from mks-h August 6, 2024 09:06

Ph0enixKM and others added 7 commits August 6, 2024 20:03

Update src/modules/expression/binop/eq.rs

05515cc

Co-authored-by: Hubert Jabłoński <[email protected]>

Update src/modules/expression/binop/neq.rs

f4302a6

Co-authored-by: Hubert Jabłoński <[email protected]>

Update src/modules/expression/binop/or.rs

45586bf

Update src/modules/expression/binop/and.rs

819333e

feat(error-macro): simplify code in a macro to keep DRY principle

10d87d0

merge(binop): resolve conflicts in binop modules

d21eecd

feat(test): Add test for multiple cast

08b3927

KrosFire requested changes Aug 7, 2024

View reviewed changes

Ph0enixKM and others added 4 commits August 7, 2024 18:45

fix(and): remove comment

48219ae

fix(range): Fix range and remove import from code eval (the stdlib lo…

2410018

…oks diffrent now)

Update src/modules/expression/ternop/ternary.rs

1c7c1a4

Co-authored-by: Hubert Jabłoński <[email protected]>

Update src/modules/expression/ternop/ternary.rs

4a62cf3

Co-authored-by: Hubert Jabłoński <[email protected]>

KrosFire approved these changes Aug 7, 2024

View reviewed changes

Mte90 approved these changes Aug 8, 2024

View reviewed changes

Mte90 merged commit 2851990 into master Aug 8, 2024
1 check passed

Mte90 deleted the 364-bug-invalid-operation-precedence branch August 8, 2024 10:06

Mte90 mentioned this pull request Aug 8, 2024

[Feature/Bug] No stdlib comments in bash generated #393

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[BUG] Invalid operation Precedence #367

[BUG] Invalid operation Precedence #367

Ph0enixKM commented Jul 28, 2024 •

edited

Loading

Mte90 commented Jul 29, 2024

This comment was marked as outdated.

KrosFire commented Jul 29, 2024

Ph0enixKM commented Jul 29, 2024

KrosFire left a comment

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

Ph0enixKM commented Aug 1, 2024 •

edited

Loading

Ph0enixKM commented Aug 1, 2024 •

edited

Loading

Ph0enixKM commented Aug 1, 2024

Ph0enixKM commented Aug 5, 2024 •

edited

Loading

KrosFire left a comment

Ph0enixKM commented Aug 7, 2024

KrosFire left a comment

KrosFire left a comment

[BUG] Invalid operation Precedence #367

[BUG] Invalid operation Precedence #367

Conversation

Ph0enixKM commented Jul 28, 2024 • edited Loading

Mte90 commented Jul 29, 2024

This comment was marked as outdated.

KrosFire commented Jul 29, 2024

Ph0enixKM commented Jul 29, 2024

KrosFire left a comment

Choose a reason for hiding this comment

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

Ph0enixKM commented Aug 1, 2024 • edited Loading

Ph0enixKM commented Aug 1, 2024 • edited Loading

Ph0enixKM commented Aug 1, 2024

Ph0enixKM commented Aug 5, 2024 • edited Loading

KrosFire left a comment

Choose a reason for hiding this comment

Ph0enixKM commented Aug 7, 2024

KrosFire left a comment

Choose a reason for hiding this comment

KrosFire left a comment

Choose a reason for hiding this comment

Ph0enixKM commented Jul 28, 2024 •

edited

Loading

Ph0enixKM commented Aug 1, 2024 •

edited

Loading

Ph0enixKM commented Aug 1, 2024 •

edited

Loading

Ph0enixKM commented Aug 5, 2024 •

edited

Loading