Merge #1215

1215: Report `pub restricted` violations r=CohenArthur a=CohenArthur

Needs #1202. The only commit worth reviewing in the meantime is [c229c7f](https://github.com/Rust-GCC/gccrs/pull/1215/commits/c229c7f891f02c3ecd1c7b743b35edb3fc2c5539)

This visitor allows us to report invalid restrictions in `pub(restricted)` visibilities. I've added documentation and an example to the visitor to explain the expected behavior.

Co-authored-by: Arthur Cohen <arthur.cohen@embecosm.com>

8 files changed, 325 insertions, 22 deletions

diff --git a/gcc/rust/Make-lang.in b/gcc/rust/Make-lang.in
index 2aec8a9..aa125a1 100644
--- a/gcc/rust/Make-lang.in
+++ b/gcc/rust/Make-lang.in
@@ -99,6 +99,7 @@ GRS_OBJS = \
     rust/rust-privacy-ctx.o \
     rust/rust-reachability.o \
     rust/rust-visibility-resolver.o \
+    rust/rust-pub-restricted-visitor.o \
     rust/rust-tyty.o \
     rust/rust-tyctx.o \
     rust/rust-tyty-bounds.o \
diff --git a/gcc/rust/hir/tree/rust-hir-full-test.cc b/gcc/rust/hir/tree/rust-hir-full-test.cc
index 8ef1194..b328a7f 100644
--- a/gcc/rust/hir/tree/rust-hir-full-test.cc
+++ b/gcc/rust/hir/tree/rust-hir-full-test.cc
@@ -123,6 +123,8 @@ Visibility::as_string () const
     case PRIVATE:
       return std::string ("private");
     case PUBLIC:
+      return std::string ("pub");
+    case RESTRICTED:
       return std::string ("pub(in ") + path.get_mappings ().as_string ()
 	     + std::string (")");
     default:
diff --git a/gcc/rust/privacy/rust-privacy-check.cc b/gcc/rust/privacy/rust-privacy-check.cc
index ed90c7c..b2e33c0 100644
--- a/gcc/rust/privacy/rust-privacy-check.cc
+++ b/gcc/rust/privacy/rust-privacy-check.cc
@@ -22,23 +22,25 @@
 #include "rust-hir-map.h"
 #include "rust-name-resolver.h"
 #include "rust-visibility-resolver.h"
+#include "rust-pub-restricted-visitor.h"
 
 extern bool
 saw_errors (void);
 
 namespace Rust {
 namespace Privacy {
+
 void
 Resolver::resolve (HIR::Crate &crate)
 {
   PrivacyContext ctx;
   auto mappings = Analysis::Mappings::get ();
   auto resolver = Rust::Resolver::Resolver::get ();
+  auto ty_ctx = ::Rust::Resolver::TypeCheckContext::get ();
 
-  auto visibility_resolver = VisibilityResolver (*mappings, *resolver);
-  visibility_resolver.go (crate);
+  VisibilityResolver (*mappings, *resolver).go (crate);
+  PubRestrictedVisitor (*mappings).go (crate);
 
-  auto ty_ctx = ::Rust::Resolver::TypeCheckContext::get ();
   auto visitor = ReachabilityVisitor (ctx, *ty_ctx);
 
   const auto &items = crate.items;
diff --git a/gcc/rust/privacy/rust-pub-restricted-visitor.cc b/gcc/rust/privacy/rust-pub-restricted-visitor.cc
new file mode 100644
index 0000000..ca50fe3
--- /dev/null
+++ b/gcc/rust/privacy/rust-pub-restricted-visitor.cc
@@ -0,0 +1,178 @@
+// Copyright (C) 2020-2022 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
+// <http://www.gnu.org/licenses/>.
+
+#include "rust-pub-restricted-visitor.h"
+#include "rust-hir.h"
+#include "rust-hir-item.h"
+
+namespace Rust {
+namespace Privacy {
+
+bool
+PubRestrictedVisitor::is_restriction_valid (DefId item_id,
+					    const Location &locus)
+{
+  ModuleVisibility visibility;
+
+  // If there is no visibility in the mappings, then the item is private and
+  // does not contain any restriction
+  if (!mappings.lookup_visibility (item_id, &visibility))
+    return true;
+
+  for (auto mod = module_stack.rbegin (); mod != module_stack.rend (); mod++)
+    if (*mod == visibility.get_module_id ())
+      return true;
+
+  rust_error_at (locus, "restricted path is not an ancestor of the "
+			"current module");
+  return false;
+}
+
+PubRestrictedVisitor::PubRestrictedVisitor (Analysis::Mappings &mappings)
+  : mappings (mappings)
+{}
+
+void
+PubRestrictedVisitor::go (HIR::Crate &crate)
+{
+  // The `crate` module will always be present
+  module_stack.emplace_back (crate.get_mappings ().get_defid ());
+
+  // FIXME: When do we insert `super`? `self`?
+  // We need wrapper function for these
+
+  for (auto &item : crate.items)
+    {
+      if (item->get_hir_kind () == HIR::Node::VIS_ITEM)
+	{
+	  auto vis_item = static_cast<HIR::VisItem *> (item.get ());
+	  vis_item->accept_vis (*this);
+	}
+    }
+}
+
+void
+PubRestrictedVisitor::visit (HIR::Module &mod)
+{
+  // FIXME: We need to update `super` and `self` here
+  module_stack.push_back (mod.get_mappings ().get_defid ());
+
+  is_restriction_valid (mod.get_mappings ().get_defid (), mod.get_locus ());
+
+  for (auto &item : mod.get_items ())
+    {
+      if (item->get_hir_kind () == HIR::Node::VIS_ITEM)
+	{
+	  auto vis_item = static_cast<HIR::VisItem *> (item.get ());
+	  vis_item->accept_vis (*this);
+	}
+    }
+
+  module_stack.pop_back ();
+}
+
+void
+PubRestrictedVisitor::visit (HIR::ExternCrate &crate)
+{
+  is_restriction_valid (crate.get_mappings ().get_defid (), crate.get_locus ());
+}
+
+void
+PubRestrictedVisitor::visit (HIR::UseDeclaration &use_decl)
+{
+  is_restriction_valid (use_decl.get_mappings ().get_defid (),
+			use_decl.get_locus ());
+}
+
+void
+PubRestrictedVisitor::visit (HIR::Function &func)
+{
+  is_restriction_valid (func.get_mappings ().get_defid (), func.get_locus ());
+}
+
+void
+PubRestrictedVisitor::visit (HIR::TypeAlias &type_alias)
+{
+  is_restriction_valid (type_alias.get_mappings ().get_defid (),
+			type_alias.get_locus ());
+}
+
+void
+PubRestrictedVisitor::visit (HIR::StructStruct &struct_item)
+{
+  is_restriction_valid (struct_item.get_mappings ().get_defid (),
+			struct_item.get_locus ());
+  // FIXME: Check fields here as well
+}
+
+void
+PubRestrictedVisitor::visit (HIR::TupleStruct &tuple_struct)
+{
+  is_restriction_valid (tuple_struct.get_mappings ().get_defid (),
+			tuple_struct.get_locus ());
+  // FIXME: Check fields here as well
+}
+
+void
+PubRestrictedVisitor::visit (HIR::Enum &enum_item)
+{
+  is_restriction_valid (enum_item.get_mappings ().get_defid (),
+			enum_item.get_locus ());
+}
+
+void
+PubRestrictedVisitor::visit (HIR::Union &union_item)
+{
+  is_restriction_valid (union_item.get_mappings ().get_defid (),
+			union_item.get_locus ());
+}
+
+void
+PubRestrictedVisitor::visit (HIR::ConstantItem &const_item)
+{
+  is_restriction_valid (const_item.get_mappings ().get_defid (),
+			const_item.get_locus ());
+}
+
+void
+PubRestrictedVisitor::visit (HIR::StaticItem &static_item)
+{
+  is_restriction_valid (static_item.get_mappings ().get_defid (),
+			static_item.get_locus ());
+}
+
+void
+PubRestrictedVisitor::visit (HIR::Trait &trait)
+{
+  is_restriction_valid (trait.get_mappings ().get_defid (), trait.get_locus ());
+}
+
+void
+PubRestrictedVisitor::visit (HIR::ImplBlock &impl)
+{
+  is_restriction_valid (impl.get_mappings ().get_defid (), impl.get_locus ());
+}
+
+void
+PubRestrictedVisitor::visit (HIR::ExternBlock &block)
+{
+  is_restriction_valid (block.get_mappings ().get_defid (), block.get_locus ());
+}
+
+} // namespace Privacy
+} // namespace Rust
diff --git a/gcc/rust/privacy/rust-pub-restricted-visitor.h b/gcc/rust/privacy/rust-pub-restricted-visitor.h
new file mode 100644
index 0000000..dc725e1
--- /dev/null
+++ b/gcc/rust/privacy/rust-pub-restricted-visitor.h
@@ -0,0 +1,120 @@
+// Copyright (C) 2020-2022 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
+// <http://www.gnu.org/licenses/>.
+
+#ifndef RUST_PUB_RESTRICTED_VISITOR_H
+#define RUST_PUB_RESTRICTED_VISITOR_H
+
+#include "rust-hir-visitor.h"
+#include "rust-hir.h"
+#include "rust-hir-expr.h"
+#include "rust-hir-stmt.h"
+#include "rust-hir-item.h"
+#include "rust-hir-map.h"
+
+namespace Rust {
+namespace Privacy {
+
+/**
+ * This visitor takes care of reporting `pub(restricted)` violations:
+ * A `pub(restricted)` violation is defined as the usage of a path restriction
+ * on an item which does not restrict the item's visibility to one of its parent
+ * modules. What this means is that an user is allowed to specify that an item
+ * should be public for any of its parent modules, going all the way to the
+ * `crate` module, but not for any of its children module.
+ *
+ * ```rust
+ * mod a {
+ * 	mod b {
+ * 		pub (in a) struct A0;
+ *
+ * 		mod c {
+ * 			mod d {
+ * 				pub (in a) struct A1;
+ * 			}
+ * 		}
+ *
+ * 		pub (in c::d) struct A2;
+ * 	}
+ * }
+ * ```
+ *
+ * The above `A0`'s visibility is valid: It is restricted to a path, `a`,
+ * which is a parent of the current module, `b`.
+ * Likewise, `A1` is also defined properly: `a` is a parent of `d`, albeit
+ * a great-great-great-grandparant of it.
+ *
+ * `A2` visibility, however, is invalid: Where the struct is defined, the
+ * current module is `b`. `c::d` (which refers to the `d` module) is a child of
+ * `b`, and not one of its ancestors.
+ *
+ * Note that these resolution rules are also the ones of the 2015 rust edition:
+ * All the `pub(restricted)` visibilities above would be invalid in the 2018
+ * edition, as the paths there must be absolute and not relative (`c::d` would
+ * become `crate::a::b::c::d` etc). Nonetheless, the logic stays the same.
+ */
+class PubRestrictedVisitor : public HIR::HIRVisItemVisitor
+{
+public:
+  PubRestrictedVisitor (Analysis::Mappings &mappings);
+
+  void go (HIR::Crate &crate);
+
+  /**
+   * Check if an item's restricted visibility (`pub (crate)`, `pub (self)`,
+   * `pub(super)`, `pub (in <path>)`) is valid in the current context.
+   * `pub restricted` visibilities are only allowed when the restriction path
+   * is a parent module of the item being visited.
+   *
+   * In case of error, this function will emit the errors and return.
+   *
+   * @param item_id DefId of the item to check the restriction of
+   * @param locus Location of the item being checked
+   *
+   * @return true if the visibility restriction is valid, false otherwise.
+   */
+  bool is_restriction_valid (DefId item_id, const Location &locus);
+
+  virtual void visit (HIR::Module &mod);
+  virtual void visit (HIR::ExternCrate &crate);
+  virtual void visit (HIR::UseDeclaration &use_decl);
+  virtual void visit (HIR::Function &func);
+  virtual void visit (HIR::TypeAlias &type_alias);
+  virtual void visit (HIR::StructStruct &struct_item);
+  virtual void visit (HIR::TupleStruct &tuple_struct);
+  virtual void visit (HIR::Enum &enum_item);
+  virtual void visit (HIR::Union &union_item);
+  virtual void visit (HIR::ConstantItem &const_item);
+  virtual void visit (HIR::StaticItem &static_item);
+  virtual void visit (HIR::Trait &trait);
+  virtual void visit (HIR::ImplBlock &impl);
+  virtual void visit (HIR::ExternBlock &block);
+
+private:
+  /* Stack of ancestor modules visited by this visitor */
+  std::vector<DefId> module_stack;
+
+  // FIXME: Do we have to handle `self` and `super` as part of the name
+  // resolution pass?
+
+  Analysis::Mappings &mappings;
+};
+
+} // namespace Privacy
+} // namespace Rust
+
+#endif // !RUST_PUB_RESTRICTED_VISITOR_H
diff --git a/gcc/rust/privacy/rust-visibility-resolver.cc b/gcc/rust/privacy/rust-visibility-resolver.cc
index d2e75c2..38d6403 100644
--- a/gcc/rust/privacy/rust-visibility-resolver.cc
+++ b/gcc/rust/privacy/rust-visibility-resolver.cc
@@ -32,7 +32,6 @@ VisibilityResolver::VisibilityResolver (Analysis::Mappings &mappings,
 void
 VisibilityResolver::go (HIR::Crate &crate)
 {
-  module_stack.push_back (crate.get_mappings ().get_defid ());
   mappings.insert_visibility (crate.get_mappings ().get_defid (),
 			      ModuleVisibility::create_public ());
 
@@ -104,12 +103,12 @@ VisibilityResolver::resolve_visibility (const HIR::Visibility &visibility,
   switch (visibility.get_vis_type ())
     {
     case HIR::Visibility::PRIVATE:
-      to_resolve = ModuleVisibility::create_restricted (peek_module ());
       return true;
     case HIR::Visibility::PUBLIC:
       to_resolve = ModuleVisibility::create_public ();
       return true;
     case HIR::Visibility::RESTRICTED:
+      // FIXME: We also need to handle 2015 vs 2018 edition conflicts
       to_resolve = ModuleVisibility::create_public ();
       return resolve_module_path (visibility.get_path (),
 				  to_resolve.get_module_id ());
@@ -129,21 +128,9 @@ VisibilityResolver::resolve_and_update (const HIR::VisItem *item)
   mappings.insert_visibility (item->get_mappings ().get_defid (), module_vis);
 }
 
-DefId
-VisibilityResolver::peek_module ()
-{
-  // We're always inserting a top module - the crate
-  // But we have to check otherwise `.back()` is UB
-  rust_assert (!module_stack.empty ());
-
-  return module_stack.back ();
-}
-
 void
 VisibilityResolver::visit (HIR::Module &mod)
 {
-  module_stack.push_back (mod.get_mappings ().get_defid ());
-
   for (auto &item : mod.get_items ())
     {
       if (item->get_hir_kind () == HIR::Node::VIS_ITEM)
@@ -152,8 +139,6 @@ VisibilityResolver::visit (HIR::Module &mod)
 	  vis_item->accept_vis (*this);
 	}
     }
-
-  module_stack.pop_back ();
 }
 
 void
diff --git a/gcc/rust/privacy/rust-visibility-resolver.h b/gcc/rust/privacy/rust-visibility-resolver.h
index 89e6e2b..c57d518 100644
--- a/gcc/rust/privacy/rust-visibility-resolver.h
+++ b/gcc/rust/privacy/rust-visibility-resolver.h
@@ -92,9 +92,6 @@ public:
   virtual void visit (HIR::ExternBlock &block);
 
 private:
-  /* Stack of modules visited by this visitor */
-  std::vector<DefId> module_stack;
-
   Analysis::Mappings &mappings;
   Rust::Resolver::Resolver &resolver;
 };
diff --git a/gcc/testsuite/rust/compile/pub_restricted_2.rs b/gcc/testsuite/rust/compile/pub_restricted_2.rs
new file mode 100644
index 0000000..8588f27
--- /dev/null
+++ b/gcc/testsuite/rust/compile/pub_restricted_2.rs
@@ -0,0 +1,18 @@
+// { dg-additional-options "-w" }
+
+mod foo {
+    mod bar {
+        mod baz {
+            pub(in baz) struct A0;
+            pub(in bar::baz) struct A1;
+            pub(in foo::bar::baz) struct A2;
+
+            mod sain {
+                mod doux {}
+            }
+
+            pub(in sain) struct A3; // { dg-error "restricted path is not an ancestor of the current module" }
+            pub(in sain::doux) struct A4; // { dg-error "restricted path is not an ancestor of the current module" }
+        }
+    }
+}