From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 1643) id C7A893810AD4; Wed, 8 Jun 2022 12:48:17 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org C7A893810AD4 Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: Thomas Schwinge To: gcc-cvs@gcc.gnu.org Subject: [gcc/devel/rust/master] privacy: PrivacyReporter: Add type privacy checking on explicit types X-Act-Checkin: gcc X-Git-Author: Arthur Cohen X-Git-Refname: refs/heads/devel/rust/master X-Git-Oldrev: 64a41cce91795561c2b007cb25ad4e43b53d126a X-Git-Newrev: 3c31c11393b630e59d189e815af0fe7ea47fdd31 Message-Id: <20220608124817.C7A893810AD4@sourceware.org> Date: Wed, 8 Jun 2022 12:48:17 +0000 (GMT) X-BeenThere: gcc-cvs@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-cvs mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 08 Jun 2022 12:48:17 -0000 https://gcc.gnu.org/g:3c31c11393b630e59d189e815af0fe7ea47fdd31 commit 3c31c11393b630e59d189e815af0fe7ea47fdd31 Author: Arthur Cohen Date: Wed May 18 11:13:56 2022 +0200 privacy: PrivacyReporter: Add type privacy checking on explicit types privacy: Circumvent weird behavior about inference types for now The issue we're facing is detailed in #1260. It's not necessary to fix now to have a good type privacy base privacy: PrivacyReporter: Handle projections and placeholders Diff: --- gcc/rust/hir/tree/rust-hir.h | 2 + gcc/rust/privacy/rust-privacy-check.cc | 2 +- gcc/rust/privacy/rust-privacy-reporter.cc | 120 ++++++++++++++++++++++++++++-- gcc/rust/privacy/rust-privacy-reporter.h | 24 +++++- gcc/testsuite/rust/compile/privacy5.rs | 18 +++++ gcc/testsuite/rust/compile/privacy6.rs | 39 ++++++++++ 6 files changed, 198 insertions(+), 7 deletions(-) diff --git a/gcc/rust/hir/tree/rust-hir.h b/gcc/rust/hir/tree/rust-hir.h index 67cfb3ae423..cab46b27b5d 100644 --- a/gcc/rust/hir/tree/rust-hir.h +++ b/gcc/rust/hir/tree/rust-hir.h @@ -486,6 +486,8 @@ protected: virtual Type *clone_type_impl () const = 0; Analysis::NodeMapping mappings; + + // FIXME: How do we get the location here for each type? }; // A type without parentheses? - abstract diff --git a/gcc/rust/privacy/rust-privacy-check.cc b/gcc/rust/privacy/rust-privacy-check.cc index dca5235806f..9664d62f65c 100644 --- a/gcc/rust/privacy/rust-privacy-check.cc +++ b/gcc/rust/privacy/rust-privacy-check.cc @@ -41,7 +41,7 @@ Resolver::resolve (HIR::Crate &crate) VisibilityResolver (*mappings, *resolver).go (crate); PubRestrictedVisitor (*mappings).go (crate); - PrivacyReporter (*mappings, *resolver).go (crate); + PrivacyReporter (*mappings, *resolver, *ty_ctx).go (crate); auto visitor = ReachabilityVisitor (ctx, *ty_ctx); diff --git a/gcc/rust/privacy/rust-privacy-reporter.cc b/gcc/rust/privacy/rust-privacy-reporter.cc index 377e8616624..1685a969d45 100644 --- a/gcc/rust/privacy/rust-privacy-reporter.cc +++ b/gcc/rust/privacy/rust-privacy-reporter.cc @@ -6,9 +6,10 @@ namespace Rust { namespace Privacy { -PrivacyReporter::PrivacyReporter (Analysis::Mappings &mappings, - Resolver::Resolver &resolver) - : mappings (mappings), resolver (resolver), +PrivacyReporter::PrivacyReporter ( + Analysis::Mappings &mappings, Resolver::Resolver &resolver, + const Rust::Resolver::TypeCheckContext &ty_ctx) + : mappings (mappings), resolver (resolver), ty_ctx (ty_ctx), current_module (Optional::none ()) {} @@ -52,7 +53,11 @@ PrivacyReporter::check_for_privacy_violation (const NodeId &use_id, if (!resolver.lookup_resolved_name (use_id, &ref_node_id)) resolver.lookup_resolved_type (use_id, &ref_node_id); - rust_assert (ref_node_id != UNKNOWN_NODEID); + // FIXME: Assert here. For now, we return since this causes issues when + // checking inferred types (#1260) + // rust_assert (ref_node_id != UNKNOWN_NODEID); + if (ref_node_id == UNKNOWN_NODEID) + return; ModuleVisibility vis; @@ -97,6 +102,102 @@ PrivacyReporter::check_for_privacy_violation (const NodeId &use_id, rust_error_at (locus, "definition is private in this context"); } +void +PrivacyReporter::check_base_type_privacy (Analysis::NodeMapping &node_mappings, + const TyTy::BaseType *ty, + const Location &locus) +{ + // Avoids repeating commong argument such as `use_id` or `locus` since we're + // doing a lot of recursive calls here + auto recursive_check + = [this, &node_mappings, &locus] (const TyTy::BaseType *ty) { + return check_base_type_privacy (node_mappings, ty, locus); + }; + + switch (ty->get_kind ()) + { + // These "simple" types are our stop condition + case TyTy::BOOL: + case TyTy::CHAR: + case TyTy::INT: + case TyTy::UINT: + case TyTy::FLOAT: + case TyTy::USIZE: + case TyTy::ISIZE: + case TyTy::ADT: + case TyTy::STR: { + auto ref_id = ty->get_ref (); + NodeId lookup_id; + + mappings.lookup_hir_to_node (node_mappings.get_crate_num (), ref_id, + &lookup_id); + + return check_for_privacy_violation (lookup_id, locus); + } + case TyTy::REF: + return recursive_check ( + static_cast (ty)->get_base ()); + case TyTy::POINTER: + return recursive_check ( + static_cast (ty)->get_base ()); + case TyTy::ARRAY: + return recursive_check ( + static_cast (ty)->get_element_type ()); + case TyTy::SLICE: + return recursive_check ( + static_cast (ty)->get_element_type ()); + case TyTy::FNPTR: + for (auto ¶m : static_cast (ty)->get_params ()) + recursive_check (param.get_tyty ()); + return recursive_check ( + static_cast (ty)->get_return_type ()); + case TyTy::TUPLE: + for (auto ¶m : + static_cast (ty)->get_fields ()) + recursive_check (param.get_tyty ()); + return; + case TyTy::PLACEHOLDER: + return recursive_check ( + // FIXME: Can we use `resolve` here? Is that what we should do? + static_cast (ty)->resolve ()); + case TyTy::PROJECTION: + return recursive_check ( + static_cast (ty)->get ()); + case TyTy::NEVER: + case TyTy::CLOSURE: + case TyTy::ERROR: + case TyTy::INFER: + rust_unreachable (); + break; + + // If we're dealing with a generic param, there's nothing we should be + // doing here + case TyTy::PARAM: + // We are dealing with a function definition that has been assigned + // somewhere else. Nothing to resolve privacy-wise other than the actual + // function, which is resolved as an expression + case TyTy::FNDEF: + // FIXME: Can we really not resolve Dynamic types here? Shouldn't we have + // a look at the path and perform proper privacy analysis? + case TyTy::DYNAMIC: + return; + } +} + +void +PrivacyReporter::check_type_privacy (const HIR::Type *type, + const Location &locus) +{ + rust_assert (type); + + TyTy::BaseType *lookup = nullptr; + rust_assert ( + ty_ctx.lookup_type (type->get_mappings ().get_hirid (), &lookup)); + + auto node_mappings = type->get_mappings (); + return check_base_type_privacy (node_mappings, lookup, locus); +} + void PrivacyReporter::visit (HIR::IdentifierExpr &ident_expr) {} @@ -529,6 +630,11 @@ PrivacyReporter::visit (HIR::UseDeclaration &use_decl) void PrivacyReporter::visit (HIR::Function &function) { + for (auto ¶m : function.get_function_params ()) + check_type_privacy (param.get_type (), param.get_locus ()); + + // FIXME: It would be better if it was specifically the type's locus (#1256) + function.get_definition ()->accept_vis (*this); } @@ -626,7 +732,11 @@ PrivacyReporter::visit (HIR::EmptyStmt &stmt) void PrivacyReporter::visit (HIR::LetStmt &stmt) { - // FIXME: We probably have to check the type as well + auto type = stmt.get_type (); + if (type) + check_type_privacy (type, stmt.get_locus ()); + // FIXME: #1256 + auto init_expr = stmt.get_init_expr (); if (init_expr) init_expr->accept_vis (*this); diff --git a/gcc/rust/privacy/rust-privacy-reporter.h b/gcc/rust/privacy/rust-privacy-reporter.h index 868428a7c98..234bea718dc 100644 --- a/gcc/rust/privacy/rust-privacy-reporter.h +++ b/gcc/rust/privacy/rust-privacy-reporter.h @@ -37,7 +37,8 @@ class PrivacyReporter : public HIR::HIRExpressionVisitor, { public: PrivacyReporter (Analysis::Mappings &mappings, - Rust::Resolver::Resolver &resolver); + Rust::Resolver::Resolver &resolver, + const Rust::Resolver::TypeCheckContext &ty_ctx); /** * Perform privacy error reporting on an entire crate @@ -57,6 +58,26 @@ private: void check_for_privacy_violation (const NodeId &use_id, const Location &locus); + /** + * Internal function used by `check_type_privacy` when dealing with complex +types + * such as references or arrays + */ + void check_base_type_privacy (Analysis::NodeMapping &node_mappings, + const TyTy::BaseType *ty, + const Location &locus); + + /** + * Check the privacy of an explicit type. + * + * This function reports the errors it finds. + * + * @param type Reference to an explicit type used in a statement, expression + * or parameter + * @param locus Location of said type + */ + void check_type_privacy (const HIR::Type *type, const Location &locus); + virtual void visit (HIR::StructExprFieldIdentifier &field); virtual void visit (HIR::StructExprFieldIdentifierValue &field); virtual void visit (HIR::StructExprFieldIndexValue &field); @@ -142,6 +163,7 @@ private: Analysis::Mappings &mappings; Rust::Resolver::Resolver &resolver; + const Rust::Resolver::TypeCheckContext &ty_ctx; // `None` means we're in the root module - the crate Optional current_module; diff --git a/gcc/testsuite/rust/compile/privacy5.rs b/gcc/testsuite/rust/compile/privacy5.rs new file mode 100644 index 00000000000..ad552c73abe --- /dev/null +++ b/gcc/testsuite/rust/compile/privacy5.rs @@ -0,0 +1,18 @@ +mod orange { + mod green { + struct Foo; + pub(in orange) struct Bar; + pub struct Baz; + } + + fn brown() { + let _ = green::Foo; // { dg-error "definition is private in this context" } + let _ = green::Bar; + let _ = green::Baz; + + let _: green::Foo; // { dg-error "definition is private in this context" } + + fn any(a0: green::Foo, a1: green::Bar) {} + // { dg-error "definition is private in this context" "" { target *-*-* } .-1 } + } +} diff --git a/gcc/testsuite/rust/compile/privacy6.rs b/gcc/testsuite/rust/compile/privacy6.rs new file mode 100644 index 00000000000..487ed024209 --- /dev/null +++ b/gcc/testsuite/rust/compile/privacy6.rs @@ -0,0 +1,39 @@ +// { dg-additional-options "-w" } + +struct Adt; +enum EAdt { + V0, + V1, +} +struct Registers { + r0: i64, + r1: i64, + r2: i64, + r3: i64, +} +trait Foo {} + +fn foo1(value: bool) {} +fn foo2(value: char) {} +fn foo3(value: i32) {} +fn foo4(value: u16) {} +fn foo5(value: f64) {} +fn foo6(value: usize) {} +fn foo7(value: isize) {} +fn foo8(value: Adt) {} +fn foo9(value: EAdt) {} +fn foo10(value: &str) {} +fn foo11(value: *const i8) {} +fn foo12(value: T) {} +fn foo13(value: [i32; 5]) {} +fn foo14(value: [Adt]) {} +fn foo15(value: fn(i32) -> i32) {} +fn foo16(value: (i32, Adt)) {} +fn foo17(value: (i32, [f64; 5])) {} +fn foo18(value: Registers) {} +fn foo19(value: &dyn Foo) {} +fn foo20(value: &[Adt]) {} +// FIXME: Uncomment once #1257 is fixed +// fn foo21(value: fn(i32)) {} +// fn foo22(value: fn()) {} +fn foo23(value: fn() -> i32) {}