-
Notifications
You must be signed in to change notification settings - Fork 389
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.
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
feat: gno type check #1426
Merged
jaekwon
merged 202 commits into
gnolang:master
from
ltzmaxwell:ltzmaxwell/fix/interface_comparison
Jun 19, 2024
Merged
feat: gno type check #1426
jaekwon
merged 202 commits into
gnolang:master
from
ltzmaxwell:ltzmaxwell/fix/interface_comparison
Jun 19, 2024
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…terface_comparison
jaekwon
reviewed
Jun 16, 2024
jaekwon
reviewed
Jun 16, 2024
jaekwon
reviewed
Jun 16, 2024
jaekwon
reviewed
Jun 16, 2024
jaekwon
reviewed
Jun 16, 2024
jaekwon
reviewed
Jun 17, 2024
jaekwon
force-pushed
the
ltzmaxwell/fix/interface_comparison
branch
from
June 19, 2024 03:31
49e6030
to
2f5b613
Compare
jaekwon
approved these changes
Jun 19, 2024
7 tasks
mvertes
added a commit
that referenced
this pull request
Oct 15, 2024
[shift operator where first operand is an untyped bigint always results in a bigint](#1462) is not resolved by #1426, it's fixed by this one. ================================================================= 1. This is a fix to /issues/1462; 3. **NOTE**: This PR should be reviewed following the potential merger of #1426, from which it is both decoupled and dependent. #1426 serves as base branch of this one. 4. **NOTE**: Currently, this PR displays all code including that from #1426, because it is being compared to the master branch instead of differing against #1426 directly. --------- Co-authored-by: Morgan <[email protected]> Co-authored-by: Marc Vertes <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
📦 🌐 tendermint v2
Issues or PRs tm2 related
📦 ⛰️ gno.land
Issues or PRs gno.land package related
📦 🤖 gnovm
Issues or PRs gnovm related
🧾 package/realm
Tag used for new Realms or Packages.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Pinned Update:
The original #1426 is now divided into 4 parts, with the dependency relationship being: #1426 < #1775, #1426 <- #1890<- #1891.
Among these, the main part, #1426, has been supplemented and optimized for the missing parts in the type checks of the original implementation, specifically as follows:
A new layer for type check is added(type_check.go). during the preprocess stage, the compatibility of operators and operands in expressions is checked, such as 1 - "a". This part used to be implemented as a runtime error, but now it is checked in type_check.go;
Modifications have been made to checkOrConvertType to add conversion checks for constants, such as int(1) + int64(1), which previously would not trigger a compile-time error;
Refined and improved several aspects of the handling logic for BinaryExpr during the preprocessing stage.
The existing checkType has been renamed to assertAssignableTo.
==========================update complete=======================
Problem Definition
Please proceed to #1424.
======update:
fix #1462 , tests located in
gnovm/tests/files/type2
.this issue is fixed since they share the same contexts of type check and conversion.
briefly for #1462, type of shift expression (or any composed expression involved shift expression) will be determined in the context they are used if they are untyped, also can be mutated by explicitly conversion with a
type call
.==========================================================================================
Overview of Solution
checkOperandWithOp function:
Purpose: Newly introduced to evaluate operand compatibility before deep type analysis.
Functionality: Employs predefined rules to quickly identify incompatible patterns (e.g., "a" << 1 is flagged as incompatible).
Advantage: Prevents unnecessary processing by checkOrConvertType for clear mismatches.
checkOrConvertType function:
Role: Engages after checkOperandWithOp's clearance. It's the hub for core type checking and conversion.
Key Improvement: Enhanced handling of const conversions by limiting it within a certain range.
Example: In cases like int(1) + int(8), the issue of unregulated const conversion is addressed.
Constraints: Mandatory const conversion is now limited to specific scenarios (e.g., explicit conversion, operand in array/slice index, RHS of a shift expression).
Specific Problems Solved
This code should output "something else". the root cause for this is Error(0) is assignable to errCmp since it satisfies the interface of error, and result in inequality since the have different concrete type in runtime.
Thanks @jaekwon for pointing out my mistake and give an improved version of this.
Conducted during preprocessing, not runtime.
Example:
3. Implicit Conversion:(this is split out)Focus: Ensuring accurate conversions, particularly unnamed to named types.Example:go~~ ~~package main~~ ~~type word uint~~ ~~type nat []word~~ ~~func (n nat) add() bool {~~ ~~ return true~~ ~~} ~~func Gen() nat {~~ ~~n := []word{0}~~ ~~return n~~ ~~}~~ ~~func main() {~~ ~~r := Gen()~~ ~~switch r.(type) {~~ ~~ case nat:~~ ~~println("nat")~~ ~~println(r.add())~~ ~~default:~~ ~~println("should not happen")~~ ~~ }~~ ~~}~~ ~~
4. Type of Shift Expressions:Context: Determines the type based on usage context and explicit conversions.Implementation: Additional checks in assignStmt, callExpr for potential untyped shift expressions (or else expressions with untyped shift expression embedded)~~~~appear, e.g. uint64(1 << x). This will trigger a potentially recursive check&convert until the shift expr got its final type.Conclusion:
This PR enhances the type check workflow and addresses previously overlooked aspects, resolving a variety of type-related issues.