public inbox for gcc-cvs@sourceware.org
help / color / mirror / Atom feed
* [gcc/devel/rust/master] privacy: Add base for resolving SimplePaths to Modules
@ 2022-06-08 12:44 Thomas Schwinge
  0 siblings, 0 replies; 2+ messages in thread
From: Thomas Schwinge @ 2022-06-08 12:44 UTC (permalink / raw)
  To: gcc-cvs

https://gcc.gnu.org/g:22966211a8d0d5ffed82ebeec2bdc985d7390269

commit 22966211a8d0d5ffed82ebeec2bdc985d7390269
Author: Arthur Cohen <arthur.cohen@embecosm.com>
Date:   Fri Apr 29 13:48:34 2022 +0200

    privacy: Add base for resolving SimplePaths to Modules

Diff:
---
 gcc/rust/privacy/rust-visibility-resolver.cc | 1 +
 1 file changed, 1 insertion(+)

diff --git a/gcc/rust/privacy/rust-visibility-resolver.cc b/gcc/rust/privacy/rust-visibility-resolver.cc
index d2e75c257cb..4dde9b7b195 100644
--- a/gcc/rust/privacy/rust-visibility-resolver.cc
+++ b/gcc/rust/privacy/rust-visibility-resolver.cc
@@ -110,6 +110,7 @@ VisibilityResolver::resolve_visibility (const HIR::Visibility &visibility,
       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 ());


^ permalink raw reply	[flat|nested] 2+ messages in thread

* [gcc/devel/rust/master] privacy: Add base for resolving SimplePaths to Modules
@ 2022-06-08 12:43 Thomas Schwinge
  0 siblings, 0 replies; 2+ messages in thread
From: Thomas Schwinge @ 2022-06-08 12:43 UTC (permalink / raw)
  To: gcc-cvs

https://gcc.gnu.org/g:88c20f5e1d59b691b8e8cfbd550206a39bf9727b

commit 88c20f5e1d59b691b8e8cfbd550206a39bf9727b
Author: Arthur Cohen <arthur.cohen@embecosm.com>
Date:   Fri Apr 29 13:48:34 2022 +0200

    privacy: Add base for resolving SimplePaths to Modules
    
    This also fixes the missing kind in HIR::Visibility.
    Having both `pub` and `pub restricted` as a single variant in the
    HIR::Visibility::Kind enum was a mistake, as it cause the path resolver to
    be triggered for `pub` visibilities which do not refer to any paths.
    
    Finally, erroneous calls to Visibility::create_error() are removed.
    This caused a lot of ICEs now that the Visibility lowering logic is
    fixed

Diff:
---
 gcc/rust/hir/rust-ast-lower.cc                 |  2 +-
 gcc/rust/hir/tree/rust-hir-item.h              | 14 +++++-
 gcc/rust/hir/tree/rust-hir-path.h              |  2 +-
 gcc/rust/parse/rust-parse-impl.h               | 14 +++---
 gcc/rust/privacy/rust-privacy-check.cc         |  6 ++-
 gcc/rust/privacy/rust-privacy-common.h         |  1 +
 gcc/rust/privacy/rust-visibility-resolver.cc   | 70 +++++++++++++++++++++-----
 gcc/rust/privacy/rust-visibility-resolver.h    | 16 ++++--
 gcc/testsuite/rust/compile/pub_restricted_1.rs |  5 +-
 9 files changed, 97 insertions(+), 33 deletions(-)

diff --git a/gcc/rust/hir/rust-ast-lower.cc b/gcc/rust/hir/rust-ast-lower.cc
index a3850514b09..d861207ebe8 100644
--- a/gcc/rust/hir/rust-ast-lower.cc
+++ b/gcc/rust/hir/rust-ast-lower.cc
@@ -46,7 +46,7 @@ translate_visibility (const AST::Visibility &vis)
     case AST::Visibility::PUB_CRATE:
     case AST::Visibility::PUB_SUPER:
     case AST::Visibility::PUB_IN_PATH:
-      return Visibility (Visibility::VisType::PUBLIC,
+      return Visibility (Visibility::VisType::RESTRICTED,
 			 ASTLoweringSimplePath::translate (vis.get_path ()));
       break;
     }
diff --git a/gcc/rust/hir/tree/rust-hir-item.h b/gcc/rust/hir/tree/rust-hir-item.h
index 5f664af17eb..109a5c238f2 100644
--- a/gcc/rust/hir/tree/rust-hir-item.h
+++ b/gcc/rust/hir/tree/rust-hir-item.h
@@ -557,6 +557,7 @@ public:
   {
     PRIVATE,
     PUBLIC,
+    RESTRICTED,
     ERROR,
   };
 
@@ -568,7 +569,7 @@ private:
 
 public:
   Visibility (VisType vis_type,
-	      HIR::SimplePath path = HIR::SimplePath::create_error ())
+	      HIR::SimplePath path = HIR::SimplePath::create_empty ())
     : vis_type (vis_type), path (std::move (path))
   {}
 
@@ -578,14 +579,23 @@ public:
   // Does the current visibility refer to a simple `pub <item>` entirely public
   bool is_public () const { return vis_type == PUBLIC; }
 
+  // Is the current visibility public restricted to a certain path
+  bool is_restricted () const { return vis_type == RESTRICTED; }
+
   // Creates an error visibility.
   static Visibility create_error ()
   {
-    return Visibility (ERROR, HIR::SimplePath::create_error ());
+    return Visibility (ERROR, HIR::SimplePath::create_empty ());
   }
 
   VisType get_vis_type () const { return vis_type; }
 
+  const HIR::SimplePath &get_path () const
+  {
+    rust_assert (!is_error ());
+    return path;
+  }
+
   std::string as_string () const;
 };
 
diff --git a/gcc/rust/hir/tree/rust-hir-path.h b/gcc/rust/hir/tree/rust-hir-path.h
index d153eae1c61..b18bf1dde22 100644
--- a/gcc/rust/hir/tree/rust-hir-path.h
+++ b/gcc/rust/hir/tree/rust-hir-path.h
@@ -969,7 +969,7 @@ public:
     : segments (std::move (segments)), mappings (mappings), locus (locus)
   {}
 
-  static HIR::SimplePath create_error ()
+  static HIR::SimplePath create_empty ()
   {
     return HIR::SimplePath ({}, Analysis::NodeMapping::get_error (),
 			    Location ());
diff --git a/gcc/rust/parse/rust-parse-impl.h b/gcc/rust/parse/rust-parse-impl.h
index 23ab32c832e..e76bdd8b524 100644
--- a/gcc/rust/parse/rust-parse-impl.h
+++ b/gcc/rust/parse/rust-parse-impl.h
@@ -5402,7 +5402,7 @@ Parser<ManagedTokenSource>::parse_inherent_impl_item ()
     case FN_TOK:
       // function or method
       return parse_inherent_impl_function_or_method (
-	AST::Visibility::create_error (), std::move (outer_attrs));
+	AST::Visibility::create_private (), std::move (outer_attrs));
     case CONST:
       /* lookahead to resolve production - could be function/method or const
        * item */
@@ -5412,13 +5412,13 @@ Parser<ManagedTokenSource>::parse_inherent_impl_item ()
 	{
 	case IDENTIFIER:
 	case UNDERSCORE:
-	  return parse_const_item (AST::Visibility::create_error (),
+	  return parse_const_item (AST::Visibility::create_private (),
 				   std::move (outer_attrs));
 	case UNSAFE:
 	case EXTERN_TOK:
 	case FN_TOK:
 	  return parse_inherent_impl_function_or_method (
-	    AST::Visibility::create_error (), std::move (outer_attrs));
+	    AST::Visibility::create_private (), std::move (outer_attrs));
 	default:
 	  add_error (Error (t->get_locus (),
 			    "unexpected token %qs in some sort of const item "
@@ -5572,7 +5572,7 @@ Parser<ManagedTokenSource>::parse_trait_impl_item ()
       // these seem to be SimplePath tokens, so this is a macro invocation semi
       return parse_macro_invocation_semi (std::move (outer_attrs));
     case TYPE:
-      return parse_type_alias (AST::Visibility::create_error (),
+      return parse_type_alias (AST::Visibility::create_private (),
 			       std::move (outer_attrs));
       case PUB: {
 	// visibility, so not a macro invocation semi - must be constant,
@@ -5631,7 +5631,7 @@ Parser<ManagedTokenSource>::parse_trait_impl_item ()
     case FN_TOK:
       // function or method
       return parse_trait_impl_function_or_method (
-	AST::Visibility::create_error (), std::move (outer_attrs));
+	AST::Visibility::create_private (), std::move (outer_attrs));
     case CONST:
       // lookahead to resolve production - could be function/method or const
       // item
@@ -5641,13 +5641,13 @@ Parser<ManagedTokenSource>::parse_trait_impl_item ()
 	{
 	case IDENTIFIER:
 	case UNDERSCORE:
-	  return parse_const_item (AST::Visibility::create_error (),
+	  return parse_const_item (AST::Visibility::create_private (),
 				   std::move (outer_attrs));
 	case UNSAFE:
 	case EXTERN_TOK:
 	case FN_TOK:
 	  return parse_trait_impl_function_or_method (
-	    AST::Visibility::create_error (), std::move (outer_attrs));
+	    AST::Visibility::create_private (), std::move (outer_attrs));
 	default:
 	  add_error (Error (
 	    t->get_locus (),
diff --git a/gcc/rust/privacy/rust-privacy-check.cc b/gcc/rust/privacy/rust-privacy-check.cc
index ccfed2c1a0f..ed90c7c38d3 100644
--- a/gcc/rust/privacy/rust-privacy-check.cc
+++ b/gcc/rust/privacy/rust-privacy-check.cc
@@ -20,6 +20,7 @@
 #include "rust-reachability.h"
 #include "rust-hir-type-check.h"
 #include "rust-hir-map.h"
+#include "rust-name-resolver.h"
 #include "rust-visibility-resolver.h"
 
 extern bool
@@ -32,9 +33,10 @@ Resolver::resolve (HIR::Crate &crate)
 {
   PrivacyContext ctx;
   auto mappings = Analysis::Mappings::get ();
+  auto resolver = Rust::Resolver::Resolver::get ();
 
-  auto resolver = VisibilityResolver (*mappings);
-  resolver.go (crate);
+  auto visibility_resolver = VisibilityResolver (*mappings, *resolver);
+  visibility_resolver.go (crate);
 
   auto ty_ctx = ::Rust::Resolver::TypeCheckContext::get ();
   auto visitor = ReachabilityVisitor (ctx, *ty_ctx);
diff --git a/gcc/rust/privacy/rust-privacy-common.h b/gcc/rust/privacy/rust-privacy-common.h
index 145eac73ac9..a47992f778c 100644
--- a/gcc/rust/privacy/rust-privacy-common.h
+++ b/gcc/rust/privacy/rust-privacy-common.h
@@ -51,6 +51,7 @@ public:
   Type get_kind () const { return kind; }
 
   const DefId &get_module_id () const { return module_id; }
+  DefId &get_module_id () { return module_id; }
 
 private:
   ModuleVisibility (Type kind, DefId module_id)
diff --git a/gcc/rust/privacy/rust-visibility-resolver.cc b/gcc/rust/privacy/rust-visibility-resolver.cc
index 3ab6085ce61..d2e75c257cb 100644
--- a/gcc/rust/privacy/rust-visibility-resolver.cc
+++ b/gcc/rust/privacy/rust-visibility-resolver.cc
@@ -24,8 +24,9 @@
 namespace Rust {
 namespace Privacy {
 
-VisibilityResolver::VisibilityResolver (Analysis::Mappings &mappings)
-  : mappings (mappings)
+VisibilityResolver::VisibilityResolver (Analysis::Mappings &mappings,
+					Resolver::Resolver &resolver)
+  : mappings (mappings), resolver (resolver)
 {}
 
 void
@@ -45,16 +46,56 @@ VisibilityResolver::go (HIR::Crate &crate)
     }
 }
 
-// FIXME: At this point in the pipeline, we should not be dealing with
-// `AST::SimplePath`s anymore! We need to be dealing with their "resolved
-// counterpart", so probably a NodeId/HirId/DefId.
+bool
+VisibilityResolver::resolve_module_path (const HIR::SimplePath &restriction,
+					 DefId &id)
+{
+  // We need, from the restriction, to figure out the actual Module it
+  // belongs to.
+
+  NodeId ast_node_id = restriction.get_mappings ().get_nodeid ();
+
+  auto invalid_path
+    = Error (restriction.get_locus (),
+	     "cannot use non-module path as privacy restrictor");
+
+  NodeId ref_node_id = UNKNOWN_NODEID;
+  if (!resolver.lookup_resolved_name (ast_node_id, &ref_node_id))
+    {
+      invalid_path.emit_error ();
+      return false;
+    }
+  // FIXME: Add a hint here if we can find the path in another scope, such as
+  // a type or something else
+  // TODO: For the hint, can we point to the original item's definition if
+  // present?
+
+  Resolver::Definition def;
+  rust_assert (resolver.lookup_definition (ref_node_id, &def));
+
+  // FIXME: Is that what we want?
+  ref_node_id = def.parent;
 
-// static bool
-// resolve_module_path (std::vector<HIR::Module> &module_stack,
-// 		     const AST::SimplePath &restriction, DefId &id)
-// {
-//   return false;
-// }
+  HirId ref;
+  rust_assert (
+    mappings.lookup_node_to_hir (restriction.get_mappings ().get_crate_num (),
+				 ref_node_id, &ref));
+
+  auto module
+    = mappings.lookup_module (restriction.get_mappings ().get_crate_num (),
+			      ref);
+
+  if (!module)
+    {
+      invalid_path.emit_error ();
+      return false;
+    }
+
+  // Fill in the resolved `DefId`
+  id = module->get_mappings ().get_defid ();
+
+  return true;
+}
 
 bool
 VisibilityResolver::resolve_visibility (const HIR::Visibility &visibility,
@@ -66,11 +107,14 @@ VisibilityResolver::resolve_visibility (const HIR::Visibility &visibility,
       to_resolve = ModuleVisibility::create_restricted (peek_module ());
       return true;
     case HIR::Visibility::PUBLIC:
-      // FIXME: We need to handle the restricted path here
-      // FIXME: We also need to handle 2015 vs 2018 edition conflicts
       to_resolve = ModuleVisibility::create_public ();
       return true;
+    case HIR::Visibility::RESTRICTED:
+      to_resolve = ModuleVisibility::create_public ();
+      return resolve_module_path (visibility.get_path (),
+				  to_resolve.get_module_id ());
     default:
+      gcc_unreachable ();
       return false;
     }
 }
diff --git a/gcc/rust/privacy/rust-visibility-resolver.h b/gcc/rust/privacy/rust-visibility-resolver.h
index 16fb42fa0b4..89e6e2bdc8a 100644
--- a/gcc/rust/privacy/rust-visibility-resolver.h
+++ b/gcc/rust/privacy/rust-visibility-resolver.h
@@ -24,6 +24,7 @@
 #include "rust-hir-stmt.h"
 #include "rust-hir-item.h"
 #include "rust-hir-map.h"
+#include "rust-name-resolver.h"
 #include "rust-hir-visitor.h"
 
 namespace Rust {
@@ -32,13 +33,20 @@ namespace Privacy {
 class VisibilityResolver : public HIR::HIRVisItemVisitor
 {
 public:
-  VisibilityResolver (Analysis::Mappings &mappings);
+  VisibilityResolver (Analysis::Mappings &mappings,
+		      Rust::Resolver::Resolver &resolver);
 
   /**
    * Perform visibility resolving on an entire crate
    */
   void go (HIR::Crate &crate);
 
+  /**
+   * Resolve a path to the module it refers
+   */
+  bool resolve_module_path (const HIR::SimplePath &restriction,
+			    DefId &to_resolve);
+
   /**
    * Resolve the visibility of an item to its ModuleVisibility. This function
    * emits errors if necessary. The contents of the to_resolve parameter will be
@@ -84,11 +92,11 @@ public:
   virtual void visit (HIR::ExternBlock &block);
 
 private:
-  /* Mappings to insert visibilities into */
-  Analysis::Mappings &mappings;
-
   /* Stack of modules visited by this visitor */
   std::vector<DefId> module_stack;
+
+  Analysis::Mappings &mappings;
+  Rust::Resolver::Resolver &resolver;
 };
 
 } // namespace Privacy
diff --git a/gcc/testsuite/rust/compile/pub_restricted_1.rs b/gcc/testsuite/rust/compile/pub_restricted_1.rs
index 01fb65ea610..9bda9682403 100644
--- a/gcc/testsuite/rust/compile/pub_restricted_1.rs
+++ b/gcc/testsuite/rust/compile/pub_restricted_1.rs
@@ -1,12 +1,11 @@
 pub mod foo {
     pub mod bar {
         pub fn baz() {}
+
+        pub(in foo::bar) struct A0;
     }
 }
 
-// this is invalid Rust: We just want to make sure the paths get resolved properly
-pub(in foo::bar::baz) struct A0;
-
 pub(in foo::fah::baz) struct A1; // { dg-error "cannot find simple path segment .fah." }
 pub(in fro::bulator::saindoux) struct A2; // { dg-error "cannot find simple path segment .fro." }
 pub(in foo::bar::saindoux) struct A3; // { dg-error "cannot find simple path segment .saindoux." }


^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2022-06-08 12:44 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-08 12:44 [gcc/devel/rust/master] privacy: Add base for resolving SimplePaths to Modules Thomas Schwinge
  -- strict thread matches above, loose matches on Subject: below --
2022-06-08 12:43 Thomas Schwinge

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).