[gcc/devel/rust/master] privacy: PrivacyReporter: Add type privacy checking on explicit types

[gcc/devel/rust/master] privacy: PrivacyReporter: Add type privacy checking on explicit types

[Thomas Schwinge]

https://gcc.gnu.org/g:3c31c11393b630e59d189e815af0fe7ea47fdd31

commit 3c31c11393b630e59d189e815af0fe7ea47fdd31
Author: Arthur Cohen <arthur.cohen@embecosm.com>
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<NodeId>::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<const TyTy::ReferenceType *> (ty)->get_base ());
+    case TyTy::POINTER:
+      return recursive_check (
+	static_cast<const TyTy::PointerType *> (ty)->get_base ());
+    case TyTy::ARRAY:
+      return recursive_check (
+	static_cast<const TyTy::ArrayType *> (ty)->get_element_type ());
+    case TyTy::SLICE:
+      return recursive_check (
+	static_cast<const TyTy::SliceType *> (ty)->get_element_type ());
+    case TyTy::FNPTR:
+      for (auto &param : static_cast<const TyTy::FnPtr *> (ty)->get_params ())
+	recursive_check (param.get_tyty ());
+      return recursive_check (
+	static_cast<const TyTy::FnPtr *> (ty)->get_return_type ());
+    case TyTy::TUPLE:
+      for (auto &param :
+	   static_cast<const TyTy::TupleType *> (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<const TyTy::PlaceholderType *> (ty)->resolve ());
+    case TyTy::PROJECTION:
+      return recursive_check (
+	static_cast<const TyTy::ProjectionType *> (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 &param : 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<NodeId> 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<T>(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) {}