public inbox for gcc-cvs@sourceware.org help / color / mirror / Atom feed
From: Thomas Schwinge <tschwinge@gcc.gnu.org> To: gcc-cvs@gcc.gnu.org Subject: [gcc/devel/rust/master] privacy: Add base for resolving SimplePaths to Modules Date: Wed, 8 Jun 2022 12:43:33 +0000 (GMT) [thread overview] Message-ID: <20220608124333.4146E38936C0@sourceware.org> (raw) 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." }
next reply other threads:[~2022-06-08 12:43 UTC|newest] Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-06-08 12:43 Thomas Schwinge [this message] 2022-06-08 12:44 Thomas Schwinge
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20220608124333.4146E38936C0@sourceware.org \ --to=tschwinge@gcc.gnu.org \ --cc=gcc-cvs@gcc.gnu.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
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).