-
Notifications
You must be signed in to change notification settings - Fork 375
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
fix(gnovm): check const type and values expr #2651
fix(gnovm): check const type and values expr #2651
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2651 +/- ##
=======================================
Coverage 60.15% 60.15%
=======================================
Files 560 560
Lines 74738 74738
=======================================
+ Hits 44955 44958 +3
+ Misses 26397 26394 -3
Partials 3386 3386
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
thank you for your work! can you add a test for the fix? |
case *CallExpr: | ||
checkValConstValue(x.Func) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see this case:
package main
type f func()
func foo() {
println("foo")
}
const ff = foo
func main() {
println("ok")
}
it should be excluded, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for comment, In fact, I'm having a bit of trouble figuring out how to check that a name expr is a const or a cast of a basic type, for example const x = uint(8)
, do you have any tips to give me on this? In the meantime, I'll keep working on the code and keep looking, thanks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems there may be a misunderstanding about the use of nameExpr as the right-hand side (RHS) of a const declaration. For example:
const a = 1
// 'a' qualifies as a ConstExpr
const b = a
In this case, using a
as the RHS in the declaration of b is perfectly valid since a is evaluated to be a constant expression.
Regarding the expression uint(8), this is a CallExpr with the function type(TypeType) of uint8. I'm not certain if this directly addresses your question.
However, I recommend we ensure a shared understanding of the related concepts by reviewing the section on constants in the Go language specification: Go Constants.
Tips, you can try to conduct checkValConstValue
in trans_leave *ValueDecl
to see if it works.
case *BinaryExpr: | ||
checkValConstValue(x.Left) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also this one:
package main
var x = 1
const ff = 1 << x
func main() {
println("ok")
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello @ltzmaxwell,
I understand better thank you, however not all RHS are constExpr by default and this code is then executed to transform them at the last moment.
if n.Const {
// NOTE: may or may not be a *ConstExpr,
// but if not, make one now.
for i, vx := range n.Values {
n.Values[i] = evalConst(store, last, vx)
}
}
So i tried to just change this code by adding a if !isConst(vx) -> panic
but some expr are not yet evaluted as const.
I'm wondering whether I should try to evaluate all constant expressions as constExpr
before reaching the snippet of code above or continue along the path of checking each type of expression, which can be complicated by the fact that special cases exist in many cases?
I also noticed that this code evaluates the const Minute
as a constExpr
.
package main
import (
"fmt"
)
type Duration int64
const (
Nanosecond Duration = 1
Microsecond = 1000 * Nanosecond
Millisecond = 1000 * Microsecond
Second = 1000 * Millisecond
Minute = 60 * Second
Hour = 60 * Minute
)
const df = Minute
func main() {
fmt.Printf("df: %v %T\n", df, df)
}
but this code where i import the pkg time does not evalue the RHS time.Duration
of the const decl. as a constExpr before the final loop, which transforms the RHS into constExpr regardless of its value.
package main
import (
"fmt"
"time"
)
const df = time.Minute
func main() {
fmt.Printf("df: %v %T\n", df, df)
}
It's one of my problems right now.
I closed the PR for now since i think i need more vision and knowledge about the whole architecture to fix the problem, i would be happy to peer-code the fix or see a proposal for this issue.
case *CallExpr: | ||
checkValConstValue(x.Func) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems there may be a misunderstanding about the use of nameExpr as the right-hand side (RHS) of a const declaration. For example:
const a = 1
// 'a' qualifies as a ConstExpr
const b = a
In this case, using a
as the RHS in the declaration of b is perfectly valid since a is evaluated to be a constant expression.
Regarding the expression uint(8), this is a CallExpr with the function type(TypeType) of uint8. I'm not certain if this directly addresses your question.
However, I recommend we ensure a shared understanding of the related concepts by reviewing the section on constants in the Go language specification: Go Constants.
Tips, you can try to conduct checkValConstValue
in trans_leave *ValueDecl
to see if it works.
- Revert partially "feat(stdlibs): add math/rand (gnolang#2455) (f547d7d) - Update p/demo/rand and rename to p/demo/entropy --------- Signed-off-by: moul <94029+moul@users.noreply.github.com> Co-authored-by: Marc Vertes <mvertes@free.fr>
This PR has different purposes: * adding Gnofaucet to Goreleaser task (closes gnolang#2557) * moving Goreleaser from v 1.26 to v 2.1 * reducing the verbosity of tasks by merging the different release types (latest, nightly, master) into a single goreleaser file * adding a fallbacked env variable called `TAG_VERSION`, which label the various docker images * adding the `nightly` subtask, which is ignored for latest releases * skipping publish for master release (NOTE: this part is the one TBC, because `master` release is using nightly too, but the variable `tag_name` unfortunately cannot be templated, let's see if it can make sense or if there is another workaround) <details><summary>Contributors' checklist...</summary> - [x] Added new tests, or not needed, or not feasible - [x] Provided an example (e.g. screenshot) to aid review or the PR is self-explanatory - [x] Updated the official documentation or not needed - [x] No breaking changes were made, or a `BREAKING CHANGE: xxx` message was included in the description - [x] Added references to related issues and PRs - [x] Provided any useful hints for running manual tests - [x] Added new benchmarks to [generated graphs](https://gnoland.github.io/benchmarks), if any. More info [here](https://github.com/gnolang/gno/blob/master/.benchmarks/README.md). </details>
Providing a tuned command to trigger goreleaser on merges in master branch
## Description This PR fixates the `golangci-lint` CI version to `1.59`, since this is the version we utilize for `misc/devdeps` that powers the `make lint` directive. <details><summary>Contributors' checklist...</summary> - [x] Added new tests, or not needed, or not feasible - [x] Provided an example (e.g. screenshot) to aid review or the PR is self-explanatory - [x] Updated the official documentation or not needed - [x] No breaking changes were made, or a `BREAKING CHANGE: xxx` message was included in the description - [x] Added references to related issues and PRs - [ ] Provided any useful hints for running manual tests - [ ] Added new benchmarks to [generated graphs](https://gnoland.github.io/benchmarks), if any. More info [here](https://github.com/gnolang/gno/blob/master/.benchmarks/README.md). </details>
Introducing templating for gorelease when running in master branch
<!-- please provide a detailed description of the changes made in this pull request. --> ## Description This PR adds the r/gnoland/events realm. It handles events gno.land is attending dynamically. An admin and modetators can add new events, delete old ones, or make changes to existing ones. <details><summary>Contributors' checklist...</summary> - [x] Added new tests, or not needed, or not feasible - [x] Provided an example (e.g. screenshot) to aid review or the PR is self-explanatory - [x] Updated the official documentation or not needed - [x] No breaking changes were made, or a `BREAKING CHANGE: xxx` message was included in the description - [x] Added references to related issues and PRs - [x] Provided any useful hints for running manual tests - [x] Added new benchmarks to [generated graphs](https://gnoland.github.io/benchmarks), if any. More info [here](https://github.com/gnolang/gno/blob/master/.benchmarks/README.md). </details> --------- Co-authored-by: Manfred Touron <94029+moul@users.noreply.github.com>
…#2697) ## Description This PR updates the test4 item on the home page, and fixes a small rendering bug in the r/events realm, and adds tests to it. <details><summary>Contributors' checklist...</summary> - [x] Added new tests, or not needed, or not feasible - [x] Provided an example (e.g. screenshot) to aid review or the PR is self-explanatory - [x] Updated the official documentation or not needed - [x] No breaking changes were made, or a `BREAKING CHANGE: xxx` message was included in the description - [x] Added references to related issues and PRs - [x] Provided any useful hints for running manual tests - [x] Added new benchmarks to [generated graphs](https://gnoland.github.io/benchmarks), if any. More info [here](https://github.com/gnolang/gno/blob/master/.benchmarks/README.md). </details>
## Description This PR bumps the `gnolang/faucet` version to `v0.3.2`, which fixes a bug where the middlewares would block a standard endpoints like `/health`. <details><summary>Contributors' checklist...</summary> - [x] Added new tests, or not needed, or not feasible - [x] Provided an example (e.g. screenshot) to aid review or the PR is self-explanatory - [x] Updated the official documentation or not needed - [x] No breaking changes were made, or a `BREAKING CHANGE: xxx` message was included in the description - [x] Added references to related issues and PRs - [ ] Provided any useful hints for running manual tests - [ ] Added new benchmarks to [generated graphs](https://gnoland.github.io/benchmarks), if any. More info [here](https://github.com/gnolang/gno/blob/master/.benchmarks/README.md). </details>
<!-- please provide a detailed description of the changes made in this pull request. --> <details><summary>Contributors' checklist...</summary> - [x] Added new tests, or not needed, or not feasible - [x] Provided an example (e.g. screenshot) to aid review or the PR is self-explanatory - [x] Updated the official documentation or not needed - [x] No breaking changes were made, or a `BREAKING CHANGE: xxx` message was included in the description - [ ] Added references to related issues and PRs - [ ] Provided any useful hints for running manual tests - [ ] Added new benchmarks to [generated graphs](https://gnoland.github.io/benchmarks), if any. More info [here](https://github.com/gnolang/gno/blob/master/.benchmarks/README.md). </details> Co-authored-by: Guilhem Fanton <8671905+gfanton@users.noreply.github.com>
<!-- please provide a detailed description of the changes made in this pull request. --> This PR aims to solves gnolang#2494 in order to parse `markdown` files from packages templates. It currently uses the file extension (from `window.location`) to detect if the content is a `md` file in a package template. _Although keeping the extension in the url is not a good practice for SEO & UX (because we are using pretty URLs for some other contents and are not consistent throughout gnoweb), This should do the work for now as short term solution since we are gonna parse `.md` from `go` soon with the new gnoweb design/codebase refactor (cc @gfanton)._ It also improves the package file template by removing global JS and pushing the `source` content into the main rendering flow from `renderer.js`.
) Address gnolang#2008. In this pull request, we're implementing a special handling for type declarations to check cynic dependency. Due to their unique nature, we must meticulously search through their fields to identify potential cyclic reference. <details><summary>Contributors' checklist...</summary> - [*] Added new tests, or not needed, or not feasible - [ ] Provided an example (e.g. screenshot) to aid review or the PR is self-explanatory - [ ] Updated the official documentation or not needed - [*] No breaking changes were made, or a `BREAKING CHANGE: xxx` message was included in the description - [ ] Added references to related issues and PRs - [ ] Provided any useful hints for running manual tests - [ ] Added new benchmarks to [generated graphs](https://gnoland.github.io/benchmarks), if any. More info [here](https://github.com/gnolang/gno/blob/master/.benchmarks/README.md). </details> --------- Co-authored-by: deelawn <dboltz03@gmail.com>
Fixes gnolang#1376, fixes gnolang#2386. --------- Co-authored-by: deelawn <dboltz03@gmail.com>
Hello, To keep up to date with progress, I'm experiencing difficulties managing all cases, since some constant expressions, such as the import of a constant are not evaluated as I've tried to add constExpr evaluation to the expressions that were missing, but I'm running into several problems. I'm going to work sideways on the issue, and if anyone who's interested in the issue decides to work on it, I'll be happy to share my exp on it in more detail. 👍 |
closes #2628
this code should not work following the issue above since the type is not valid for const
The proposal avoid any expression that is not a basic litteral or a name expr fixing the issue above.
Ideally, we'd need to check that NameExpr have good underlying type & value to fix the underlying code
Contributors' checklist...
BREAKING CHANGE: xxx
message was included in the description