-
Notifications
You must be signed in to change notification settings - Fork 162
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
gccrs does not check if something is assignable #3287
Comments
I've purposely not tackled this because i have a feeling implementing a walk tree function like we do inside: https://github.com/Rust-GCC/gccrs/blob/master/gcc/rust/checks/lints/rust-lint-unused-var.cc and looking for the assignment expressions and checking the const qualifiers might be enough. But it probably wont be perfect it needs some though with someone with free time |
There is a better way to implement this by copying some of the code from the c front-end: We need a check in our assignment expression during code generation which is here: gccrs/gcc/rust/backend/rust-compile-expr.cc Line 988 in 55a9d8d
Where we need to port over this function from the c front-end: Line 5165 in 55a9d8d
This will tell us true or false if its a valid lvalue so then if its not we can emit a rust_error_at diagnostic. |
how complicated would that be compared to making a simple visitor on our side? I feel like there's two different things, checking if it's a proper lvalue and checking if it's mutable - both of which would be implemented super quickly with one of our custom visitors |
Yeah i think its two issues that need seperated |
part of the issue is the overflow traps return a var decl which is what we want so i think we need to use the lvalue check from c fropnmt-end and something on our HIR as well where yes GCC wise its a var devel but this HIR expr is an operator so its definetly not an lvalue |
I sort of think another tree walker could do the assignment check and look if the lvalue is TREE_READONLY but not totally sure it handles it properly for rust |
This ensures that we handle var decls readonly checks much better Addresses: #807 #3287 gcc/rust/ChangeLog: * checks/errors/rust-readonly-check.cc (check_decl): improve mut check (emit_error): helper (check_modify_expr): likewise (readonly_walk_fn): reuse helper (ReadonlyCheck::Lint): cleanup context each run gcc/testsuite/ChangeLog: * rust/execute/torture/builtin_macro_include_bytes.rs: missing mut Signed-off-by: Philip Herron <[email protected]>
this solves the mutability checks for this case: |
This ensures that we handle var decls readonly checks much better Addresses: #807 #3287 gcc/rust/ChangeLog: * checks/errors/rust-readonly-check.cc (check_decl): improve mut check (emit_error): helper (check_modify_expr): likewise (readonly_walk_fn): reuse helper (ReadonlyCheck::Lint): cleanup context each run gcc/testsuite/ChangeLog: * rust/execute/torture/builtin_macro_include_bytes.rs: needs mut * rust/compile/mutability_checks1.rs: New test. Signed-off-by: Philip Herron <[email protected]>
Summary
gccrs does not check if something is assignable, and gccrs accepts many un-assignable expressions.
Reproducer
I tried this code:
Does the code make use of any (1.49) nightly feature ?
Godbolt link
No response
Actual behavior
The current behavior is that gccrs accepts this kind of semantically invalid code and even successfully generates machine code for them (in this case, gccrs generates code to make the function return
42
).Expected behavior
I expected to see an error thrown like
rustc
does:GCC Version
fa93e28
The text was updated successfully, but these errors were encountered: