From f9455fb91bebfeadd54050cb90c2061487d47c20 Mon Sep 17 00:00:00 2001 From: Philip Herron Date: Sun, 17 Sep 2023 20:11:23 +0100 Subject: [PATCH] gccrs: port over readonly_error from c-family for lvalue assignment checks Fixes #2391 gcc/rust/ChangeLog: * Make-lang.in: fixup formatting * resolve/rust-ast-resolve-expr.cc (ResolveExpr::visit): remove old check * rust-session-manager.cc (Session::compile_crate): call new lint * resolve/rust-ast-verify-assignee.h: Removed. * checks/errors/rust-readonly-check.cc: New file. * checks/errors/rust-readonly-check.h: New file. gcc/testsuite/ChangeLog: * rust/compile/wrong_lhs_assignment.rs: update error message * rust/compile/issue-2391.rs: New test. Signed-off-by: Philip Herron --- gcc/rust/Make-lang.in | 25 +-- gcc/rust/checks/errors/rust-readonly-check.cc | 164 ++++++++++++++++++ gcc/rust/checks/errors/rust-readonly-check.h | 36 ++++ gcc/rust/resolve/rust-ast-resolve-expr.cc | 7 - gcc/rust/resolve/rust-ast-verify-assignee.h | 84 --------- gcc/rust/rust-session-manager.cc | 4 +- gcc/testsuite/rust/compile/issue-2391.rs | 10 ++ .../rust/compile/wrong_lhs_assignment.rs | 4 +- 8 files changed, 227 insertions(+), 107 deletions(-) create mode 100644 gcc/rust/checks/errors/rust-readonly-check.cc create mode 100644 gcc/rust/checks/errors/rust-readonly-check.h delete mode 100644 gcc/rust/resolve/rust-ast-verify-assignee.h create mode 100644 gcc/testsuite/rust/compile/issue-2391.rs diff --git a/gcc/rust/Make-lang.in b/gcc/rust/Make-lang.in index 1534046d12b9..e6a2099f043c 100644 --- a/gcc/rust/Make-lang.in +++ b/gcc/rust/Make-lang.in @@ -86,12 +86,12 @@ GRS_OBJS = \ rust/rust-compile-resolve-path.o \ rust/rust-macro-expand.o \ rust/rust-cfg-strip.o \ - rust/rust-expand-visitor.o \ - rust/rust-ast-builder.o \ - rust/rust-derive.o \ - rust/rust-derive-clone.o \ - rust/rust-derive-copy.o \ - rust/rust-proc-macro.o \ + rust/rust-expand-visitor.o \ + rust/rust-ast-builder.o \ + rust/rust-derive.o \ + rust/rust-derive-clone.o \ + rust/rust-derive-copy.o \ + rust/rust-proc-macro.o \ rust/rust-macro-invoc-lexer.o \ rust/rust-proc-macro-invoc-lexer.o \ rust/rust-macro-substitute-ctx.o \ @@ -101,19 +101,19 @@ GRS_OBJS = \ rust/rust-attributes.o \ rust/rust-abi.o \ rust/rust-token-converter.o \ - rust/rust-macro.o \ + rust/rust-macro.o \ rust/rust-ast-lower.o \ rust/rust-ast-lower-base.o \ rust/rust-ast-lower-pattern.o \ rust/rust-ast-lower-item.o \ rust/rust-ast-lower-expr.o \ rust/rust-ast-lower-type.o \ - rust/rust-ast-lower-stmt.o \ - rust/rust-rib.o \ - rust/rust-name-resolution-context.o \ - rust/rust-default-resolver.o \ + rust/rust-ast-lower-stmt.o \ + rust/rust-rib.o \ + rust/rust-name-resolution-context.o \ + rust/rust-default-resolver.o \ rust/rust-toplevel-name-resolver-2.0.o \ - rust/rust-early-name-resolver-2.0.o \ + rust/rust-early-name-resolver-2.0.o \ rust/rust-early-name-resolver.o \ rust/rust-name-resolver.o \ rust/rust-ast-resolve.o \ @@ -160,6 +160,7 @@ GRS_OBJS = \ rust/rust-const-checker.o \ rust/rust-lint-marklive.o \ rust/rust-lint-unused-var.o \ + rust/rust-readonly-check.o \ rust/rust-hir-type-check-path.o \ rust/rust-unsafe-checker.o \ rust/rust-compile-intrinsic.o \ diff --git a/gcc/rust/checks/errors/rust-readonly-check.cc b/gcc/rust/checks/errors/rust-readonly-check.cc new file mode 100644 index 000000000000..474344fd638a --- /dev/null +++ b/gcc/rust/checks/errors/rust-readonly-check.cc @@ -0,0 +1,164 @@ +// Copyright (C) 2021-2023 Free Software Foundation, Inc. + +// This file is part of GCC. + +// GCC is free software; you can redistribute it and/or modify it under +// the terms of the GNU General Public License as published by the Free +// Software Foundation; either version 3, or (at your option) any later +// version. + +// GCC is distributed in the hope that it will be useful, but WITHOUT ANY +// WARRANTY; without even the implied warranty of MERCHANTABILITY or +// FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License +// for more details. + +// You should have received a copy of the GNU General Public License +// along with GCC; see the file COPYING3. If not see +// . + +#include "rust-readonly-check.h" +#include "rust-tree.h" +#include "rust-gcc.h" + +namespace Rust { +namespace Analysis { + +// ported over from c-family/c-warn.cc +void +readonly_error (location_t loc, tree arg, enum lvalue_use use) +{ + gcc_assert (use == lv_assign || use == lv_increment || use == lv_decrement + || use == lv_asm); + STRIP_ANY_LOCATION_WRAPPER (arg); + /* Using this macro rather than (for example) arrays of messages + ensures that all the format strings are checked at compile + time. */ +#define READONLY_MSG(A, I, D, AS) \ + (use == lv_assign \ + ? (A) \ + : (use == lv_increment ? (I) : (use == lv_decrement ? (D) : (AS)))) + if (TREE_CODE (arg) == COMPONENT_REF) + { + if (TYPE_READONLY (TREE_TYPE (TREE_OPERAND (arg, 0)))) + error_at (loc, + READONLY_MSG (G_ ("assignment of member " + "%qD in read-only object"), + G_ ("increment of member " + "%qD in read-only object"), + G_ ("decrement of member " + "%qD in read-only object"), + G_ ("member %qD in read-only object " + "used as % output")), + TREE_OPERAND (arg, 1)); + else + error_at ( + loc, + READONLY_MSG (G_ ("assignment of read-only member %qD"), + G_ ("increment of read-only member %qD"), + G_ ("decrement of read-only member %qD"), + G_ ("read-only member %qD used as % output")), + TREE_OPERAND (arg, 1)); + } + else if (VAR_P (arg)) + error_at (loc, + READONLY_MSG (G_ ("assignment of read-only variable %qD"), + G_ ("increment of read-only variable %qD"), + G_ ("decrement of read-only variable %qD"), + G_ ( + "read-only variable %qD used as % output")), + arg); + else if (TREE_CODE (arg) == PARM_DECL) + error_at (loc, + READONLY_MSG (G_ ("assignment of read-only parameter %qD"), + G_ ("increment of read-only parameter %qD"), + G_ ("decrement of read-only parameter %qD"), + G_ ( + "read-only parameter %qD use as % output")), + arg); + else if (TREE_CODE (arg) == RESULT_DECL) + { + error_at (loc, + READONLY_MSG (G_ ("assignment of " + "read-only named return value %qD"), + G_ ("increment of " + "read-only named return value %qD"), + G_ ("decrement of " + "read-only named return value %qD"), + G_ ("read-only named return value %qD " + "used as %output")), + arg); + } + else if (TREE_CODE (arg) == FUNCTION_DECL) + error_at (loc, + READONLY_MSG (G_ ("assignment of function %qD"), + G_ ("increment of function %qD"), + G_ ("decrement of function %qD"), + G_ ("function %qD used as % output")), + arg); + else + error_at (loc, + READONLY_MSG (G_ ("assignment of read-only location %qE"), + G_ ("increment of read-only location %qE"), + G_ ("decrement of read-only location %qE"), + G_ ( + "read-only location %qE used as % output")), + arg); +} + +static void +check_decl (tree *t) +{ + if (TREE_CODE (*t) == MODIFY_EXPR) + { + tree lhs = TREE_OPERAND (*t, 0); + if (TREE_READONLY (lhs) || TREE_CONSTANT (lhs)) + { + readonly_error (EXPR_LOCATION (*t), lhs, lv_assign); + TREE_OPERAND (*t, 0) = error_mark_node; + } + } +} + +static tree +readonly_walk_fn (tree *t, int *, void *) +{ + switch (TREE_CODE (*t)) + { + case MODIFY_EXPR: + check_decl (t); + break; + + default: + break; + } + return NULL_TREE; +} + +void +ReadonlyCheck::Lint (Compile::Context &ctx) +{ + for (auto &fndecl : ctx.get_func_decls ()) + { + for (tree p = DECL_ARGUMENTS (fndecl); p != NULL_TREE; p = DECL_CHAIN (p)) + { + check_decl (&p); + } + + walk_tree_without_duplicates (&DECL_SAVED_TREE (fndecl), + &readonly_walk_fn, &ctx); + } + + for (auto &var : ctx.get_var_decls ()) + { + tree decl = var->get_decl (); + check_decl (&decl); + } + + for (auto &const_decl : ctx.get_const_decls ()) + { + check_decl (&const_decl); + } +} + +} // namespace Analysis +} // namespace Rust diff --git a/gcc/rust/checks/errors/rust-readonly-check.h b/gcc/rust/checks/errors/rust-readonly-check.h new file mode 100644 index 000000000000..4fe78fb67f2d --- /dev/null +++ b/gcc/rust/checks/errors/rust-readonly-check.h @@ -0,0 +1,36 @@ +// Copyright (C) 2021-2023 Free Software Foundation, Inc. + +// This file is part of GCC. + +// GCC is free software; you can redistribute it and/or modify it under +// the terms of the GNU General Public License as published by the Free +// Software Foundation; either version 3, or (at your option) any later +// version. + +// GCC is distributed in the hope that it will be useful, but WITHOUT ANY +// WARRANTY; without even the implied warranty of MERCHANTABILITY or +// FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License +// for more details. + +// You should have received a copy of the GNU General Public License +// along with GCC; see the file COPYING3. If not see +// . + +#ifndef RUST_READONLY_CHECK +#define RUST_READONLY_CHECK + +#include "rust-compile-context.h" + +namespace Rust { +namespace Analysis { + +class ReadonlyCheck +{ +public: + static void Lint (Compile::Context &ctx); +}; + +} // namespace Analysis +} // namespace Rust + +#endif // RUST_READONLY_CHECK diff --git a/gcc/rust/resolve/rust-ast-resolve-expr.cc b/gcc/rust/resolve/rust-ast-resolve-expr.cc index 4242c8b1a472..92466ed35338 100644 --- a/gcc/rust/resolve/rust-ast-resolve-expr.cc +++ b/gcc/rust/resolve/rust-ast-resolve-expr.cc @@ -19,7 +19,6 @@ #include "rust-ast-resolve-expr.h" #include "rust-ast-resolve-stmt.h" #include "rust-ast-resolve-struct-expr-field.h" -#include "rust-ast-verify-assignee.h" #include "rust-ast-resolve-type.h" #include "rust-ast-resolve-pattern.h" #include "rust-ast-resolve-path.h" @@ -101,9 +100,6 @@ ResolveExpr::visit (AST::AssignmentExpr &expr) { ResolveExpr::go (expr.get_left_expr ().get (), prefix, canonical_prefix); ResolveExpr::go (expr.get_right_expr ().get (), prefix, canonical_prefix); - - // need to verify the assignee - VerifyAsignee::go (expr.get_left_expr ().get ()); } /* The "break rust" Easter egg. @@ -191,9 +187,6 @@ ResolveExpr::visit (AST::CompoundAssignmentExpr &expr) { ResolveExpr::go (expr.get_left_expr ().get (), prefix, canonical_prefix); ResolveExpr::go (expr.get_right_expr ().get (), prefix, canonical_prefix); - - // need to verify the assignee - VerifyAsignee::go (expr.get_left_expr ().get ()); } void diff --git a/gcc/rust/resolve/rust-ast-verify-assignee.h b/gcc/rust/resolve/rust-ast-verify-assignee.h deleted file mode 100644 index ffe7d3f45dc8..000000000000 --- a/gcc/rust/resolve/rust-ast-verify-assignee.h +++ /dev/null @@ -1,84 +0,0 @@ -// Copyright (C) 2020-2023 Free Software Foundation, Inc. - -// This file is part of GCC. - -// GCC is free software; you can redistribute it and/or modify it under -// the terms of the GNU General Public License as published by the Free -// Software Foundation; either version 3, or (at your option) any later -// version. - -// GCC is distributed in the hope that it will be useful, but WITHOUT ANY -// WARRANTY; without even the implied warranty of MERCHANTABILITY or -// FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License -// for more details. - -// You should have received a copy of the GNU General Public License -// along with GCC; see the file COPYING3. If not see -// . - -#ifndef RUST_AST_VERIFY_ASSIGNEE -#define RUST_AST_VERIFY_ASSIGNEE - -#include "rust-ast-resolve-base.h" -#include "rust-ast-full.h" - -namespace Rust { -namespace Resolver { - -class VerifyAsignee : public ResolverBase -{ - using Rust::Resolver::ResolverBase::visit; - -public: - static bool go (AST::Expr *assignee) - { - VerifyAsignee checker; - assignee->accept_vis (checker); - if (!checker.ok) - rust_error_at (assignee->get_locus (), ErrorCode::E0070, - "invalid left-hand side of assignment"); - return checker.ok; - } - - void visit (AST::ArrayIndexExpr &expr) override - { - expr.get_array_expr ()->accept_vis (*this); - } - - void visit (AST::FieldAccessExpr &expr) override - { - expr.get_receiver_expr ()->accept_vis (*this); - } - - void visit (AST::TupleIndexExpr &expr) override - { - expr.get_tuple_expr ()->accept_vis (*this); - } - - void visit (AST::IdentifierExpr &expr) override - { - if (!resolver->get_name_scope ().lookup ( - CanonicalPath::new_seg (expr.get_node_id (), expr.as_string ()), - &resolved_node)) - return; - - ok = true; - } - - void visit (AST::DereferenceExpr &expr) override - { - expr.get_dereferenced_expr ()->accept_vis (*this); - } - - void visit (AST::PathInExpression &) override { ok = true; } - -private: - VerifyAsignee () : ResolverBase (), ok (false) {} - - bool ok; -}; - -} // namespace Resolver -} // namespace Rust - -#endif // RUST_AST_VERIFY_ASSIGNEE diff --git a/gcc/rust/rust-session-manager.cc b/gcc/rust/rust-session-manager.cc index 346c471e63c6..8aeab4073877 100644 --- a/gcc/rust/rust-session-manager.cc +++ b/gcc/rust/rust-session-manager.cc @@ -32,9 +32,9 @@ #include "rust-cfg-parser.h" #include "rust-lint-scan-deadcode.h" #include "rust-lint-unused-var.h" +#include "rust-readonly-check.h" #include "rust-hir-dump.h" #include "rust-ast-dump.h" -#include "rust-ast-collector.h" #include "rust-export-metadata.h" #include "rust-imports.h" #include "rust-extern-crate.h" @@ -47,7 +47,6 @@ #include "rust-unicode.h" #include "rust-attribute-values.h" -#include "diagnostic.h" #include "input.h" #include "selftest.h" #include "tm.h" @@ -676,6 +675,7 @@ Session::compile_crate (const char *filename) // lints Analysis::ScanDeadcode::Scan (hir); Analysis::UnusedVariables::Lint (ctx); + Analysis::ReadonlyCheck::Lint (ctx); // metadata bool specified_emit_metadata diff --git a/gcc/testsuite/rust/compile/issue-2391.rs b/gcc/testsuite/rust/compile/issue-2391.rs new file mode 100644 index 000000000000..4904502c7c18 --- /dev/null +++ b/gcc/testsuite/rust/compile/issue-2391.rs @@ -0,0 +1,10 @@ +#![allow(unused)] +const SOME_CONST: i32 = 12; + +fn some_function() { + SOME_CONST = 15; // { dg-error "assignment of read-only location" } +} + +fn main() { + some_function(); +} diff --git a/gcc/testsuite/rust/compile/wrong_lhs_assignment.rs b/gcc/testsuite/rust/compile/wrong_lhs_assignment.rs index 0c638a901c3d..bc2ab5a91ba8 100644 --- a/gcc/testsuite/rust/compile/wrong_lhs_assignment.rs +++ b/gcc/testsuite/rust/compile/wrong_lhs_assignment.rs @@ -1,7 +1,7 @@ fn foo() { - 1 = 3; // { dg-error "invalid left-hand side of assignment" } + 1 = 3; // { dg-error "assignment of read-only location" } } fn main() { foo(); -} \ No newline at end of file +}