Skip to content

rust: track inline module scopes for module file resolution #785

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 3, 2021

Conversation

mathstuf
Copy link
Contributor

@mathstuf mathstuf commented Nov 1, 2021

The set of inline modules is required to find the expected location of a
module file. Track this information with an RAII object
(InlineModuleStackScope) and pass it down to any out-of-line modules
so that, when requested, the set of inline modules can be added to the
search path.

Signed-off-by: Ben Boeckel [email protected]


Note that this does not include a test case because I have no idea how to mark up this for all the warnings that come out (#676):

diff --git a/gcc/testsuite/rust/compile/missing_middle/sub/mod.rs b/gcc/testsuite/rust/compile/missing_middle/sub/mod.rs
new file mode 100644
index 00000000000..f099d61e04a
--- /dev/null
+++ b/gcc/testsuite/rust/compile/missing_middle/sub/mod.rs
@@ -0,0 +1,3 @@
+pub fn f() -> u32 {
+    1
+}
diff --git a/gcc/testsuite/rust/compile/mod_missing_middle.rs b/gcc/testsuite/rust/compile/mod_missing_middle.rs
new file mode 100644
index 00000000000..d9cdf0a54f1
--- /dev/null
+++ b/gcc/testsuite/rust/compile/mod_missing_middle.rs
@@ -0,0 +1,11 @@
+pub mod missing_middle {
+    pub mod sub;
+}

Observed warnings:

/home/boeckb/code/depot/group-compilers/gcc/src-gccrs/gcc/testsuite/rust/compile/missing_middle/sub/mod.rs:1:5: warning: unused name 'f'
/home/boeckb/code/depot/group-compilers/gcc/src-gccrs/gcc/testsuite/rust/compile/mod_missing_middle.rs:2:9: warning: unused name 'sub'
/home/boeckb/code/depot/group-compilers/gcc/src-gccrs/gcc/testsuite/rust/compile/missing_middle/sub/mod.rs:1:5: warning: unused name 'sub::f'
/home/boeckb/code/depot/group-compilers/gcc/src-gccrs/gcc/testsuite/rust/compile/mod_missing_middle.rs:2:9: warning: unused name 'missing_middle::sub'
/home/boeckb/code/depot/group-compilers/gcc/src-gccrs/gcc/testsuite/rust/compile/mod_missing_middle.rs:1:5: warning: unused name 'missing_middle'
/home/boeckb/code/depot/group-compilers/gcc/src-gccrs/gcc/testsuite/rust/compile/missing_middle/sub/mod.rs:1:5: warning: unused name 'missing_middle::sub::f'

Fixes: #645

Here is a checklist to help you with your PR.

  • GCC development requires copyright assignment or the Developer's Certificate of Origin sign-off, see https://gcc.gnu.org/contribute.html or https://gcc.gnu.org/dco.html
  • Read contributing guidlines
  • make check-rust passes locally
  • Run clang-format (leaving to CI)
  • Added any relevant test cases to gcc/testsuite/rust/ (see above)

@mathstuf mathstuf force-pushed the rust-track-inline-module-stack branch 3 times, most recently from c93f104 to aa12e9d Compare November 1, 2021 00:21
Copy link
Member

@CohenArthur CohenArthur left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks :D I think the PR needs some tests before it gets merged, I can take a look at the warning thingies

@CohenArthur
Copy link
Member

Ah, @mathstuf: You can add the following to the test cases for now to remove (unused) warnings:

// { dg-additional-options "-w" }
mod modules;

fn main() {
    let twelve = modules::return_12();
}

the issues you are seeing are because of name resolution. I faced them too when implementing the little bits for module inclusion

@mathstuf mathstuf force-pushed the rust-track-inline-module-stack branch from aa12e9d to 455f282 Compare November 1, 2021 01:01
@mathstuf
Copy link
Contributor Author

mathstuf commented Nov 1, 2021

OK, so something isn't quite right with #[path] yet, but I'll look at that later.

@mathstuf mathstuf force-pushed the rust-track-inline-module-stack branch 2 times, most recently from ba67c52 to 4b33a83 Compare November 1, 2021 22:23
@mathstuf
Copy link
Contributor Author

mathstuf commented Nov 1, 2021

Gah. Mutable local variables bite again. #[path] issues resolved.

@philberty philberty added this to the Imports and visibility milestone Nov 2, 2021
@philberty
Copy link
Member

I don't think this PR relates to the #635 I think its this one you mean #645

Copy link
Member

@philberty philberty left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work, I think it looks good but I would like to see any new classes have private fields if possible its not always the case so let me know.

@mathstuf mathstuf force-pushed the rust-track-inline-module-stack branch 2 times, most recently from 034f3c4 to c46eb66 Compare November 2, 2021 13:25
@mathstuf
Copy link
Contributor Author

mathstuf commented Nov 2, 2021

OK, so I've updated to support #[path = "…"] mod inline {} as well. The test suite needed an update.

I'll also note that it seems that inline modules don't support inner attributes.

@mathstuf mathstuf force-pushed the rust-track-inline-module-stack branch from c46eb66 to 44fe4f2 Compare November 2, 2021 13:46
Copy link
Member

@philberty philberty left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Member

@CohenArthur CohenArthur left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apart from the copied function I think this is all good :D Thanks a lot!

@mathstuf mathstuf force-pushed the rust-track-inline-module-stack branch from 92bde08 to 2a88ef0 Compare November 2, 2021 22:24
The set of inline modules is required to find the expected location of a
module file. Track this information with an RAII object
(`InlineModuleStackScope`) and pass it down to any out-of-line modules
so that, when requested, the set of inline modules can be added to the
search path.

Signed-off-by: Ben Boeckel <[email protected]>
@mathstuf mathstuf force-pushed the rust-track-inline-module-stack branch from 2a88ef0 to 1657ee5 Compare November 3, 2021 11:10
Copy link
Member

@CohenArthur CohenArthur left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! LGTM!

@CohenArthur
Copy link
Member

bors r+

@bors
Copy link
Contributor

bors bot commented Nov 3, 2021

Build succeeded:

@bors bors bot merged commit 8e992e3 into Rust-GCC:master Nov 3, 2021
@mathstuf mathstuf deleted the rust-track-inline-module-stack branch November 3, 2021 18:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Modules without module files
3 participants