Skip to content

Support for comparison of functions#2085

Merged
bettio merged 1 commit intoatomvm:mainfrom
mat-hek:mf/upstream-compare-funs
Feb 6, 2026
Merged

Support for comparison of functions#2085
bettio merged 1 commit intoatomvm:mainfrom
mat-hek:mf/upstream-compare-funs

Conversation

@mat-hek
Copy link
Contributor

@mat-hek mat-hek commented Feb 5, 2026

These changes are made under both the "Apache 2.0" and the "GNU Lesser General
Public License 2.1 or later" license terms (dual license).

SPDX-License-Identifier: Apache-2.0 OR LGPL-2.1-or-later

@mat-hek mat-hek force-pushed the mf/upstream-compare-funs branch 6 times, most recently from c30e345 to 395f800 Compare February 5, 2026 14:40
Copy link
Collaborator

@bettio bettio 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 it is mostly ok, just a few minor changes:

  • Few more tests should be added so we test all cases.
  • I suggest moving the function block just after TERM_TYPE_INDEX_REFERENCE block

}
}
default:
case TERM_TYPE_INDEX_FUNCTION:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's move this entire block of code between TERM_TYPE_INDEX_REFERENCE and TERM_TYPE_INDEX_PORT, so we follow TermTypeIndex ordering:

[...]
    TERM_TYPE_INDEX_ATOM = 3,
    TERM_TYPE_INDEX_REFERENCE = 4,
    TERM_TYPE_INDEX_FUNCTION = 5,
    TERM_TYPE_INDEX_PORT = 6,
    TERM_TYPE_INDEX_PID = 7,
[...]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I moved it just after TERM_TYPE_INDEX_REFERENCE, but the order is not followed for other indices

@mat-hek mat-hek requested a review from bettio February 6, 2026 12:06
@mat-hek mat-hek force-pushed the mf/upstream-compare-funs branch from 75dfd2d to de2f176 Compare February 6, 2026 15:18
@bettio
Copy link
Collaborator

bettio commented Feb 6, 2026

Awesome, let's rebase & fixup everything into a single commit and it can be merged. I'll do a last CI run in the meanwhile.

@bettio bettio added this to the v0.7.0 milestone Feb 6, 2026
Signed-off-by: Mateusz Front <mateusz.front@swmansion.com>
@mat-hek mat-hek force-pushed the mf/upstream-compare-funs branch from de2f176 to c4978bf Compare February 6, 2026 17:26
@mat-hek
Copy link
Contributor Author

mat-hek commented Feb 6, 2026

Squashed into a single commit. BTW, Github supports squash&merge, maybe worth using that instead of squashing manually each time

@bettio
Copy link
Collaborator

bettio commented Feb 6, 2026

BTW, Github supports squash&merge, maybe worth using that instead of squashing manually each time

Quite yes, works well unless you sign your commits.

@bettio bettio merged commit d75b9f1 into atomvm:main Feb 6, 2026
189 of 192 checks passed
@mat-hek
Copy link
Contributor Author

mat-hek commented Feb 6, 2026

eh okay...

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.

3 participants