nr2.0: Add `ForeverStack` data structure. #2344

CohenArthur · 2023-06-28T14:00:24Z

This data structure replaces our existing scopes and allows for multiple
name resolution passes to insert definitions or look up names at different
stages of the pipeline. The documentation goes into detail about how the
data structure works, and how the source's hierarchy is preserved
throughout the pipeline.

The class is templated to enforce specific behavior based on the namespace
the scope map targets.

gcc/rust/ChangeLog:

* resolve/rust-forever-stack.h: New file.
* resolve/rust-forever-stack.hxx: New file.

Needs #2327 and #2317

P-E-P · 2023-06-29T08:48:26Z

gcc/rust/resolve/rust-forever-stack.h

+
+/**
+
+Let's look at our stack for resolving and traversing the following Rust code:


I think all those comments would make a good wiki page. That doesn't mean we should delete them from the code though.

gcc/rust/resolve/rust-forever-stack.h

gcc/rust/resolve/rust-forever-stack.hxx

P-E-P · 2023-06-29T09:49:00Z

gcc/rust/resolve/rust-forever-stack.hxx

+  // So what do we do here - if the Rib has already been pushed in an earlier
+  // pass, we might end up in a situation where it is okay to re-add new names.
+  // Do we just ignore that here? Do we keep track of if the Rib is new or not?
+  // should our cursor have info on the current node like "is it newly pushed"?


We could encode such information in the type system, mutating a YoungNode into an Grownode, this would prevent state in the cursor as well as state in the node, once grown there would be no way back.

Just throwing random ideas around, don't mind me 😆

gcc/rust/resolve/rust-forever-stack.hxx

philberty

I like it, just style nits. I think keeping logging in is a good idea. just need to change them to rust_debug lines

philberty · 2023-06-30T17:21:15Z

gcc/rust/resolve/rust-forever-stack.hxx

+
+// 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/>.


Why do we have two headers? Not sure i like the .hxx extension. Would prefer a different name or combine the header.

Tbh hxx for template only implementations is pretty standard. We've discussed them a bit in the past with @CohenArthur and agreed on not using them when there is a non templated implementation, no .hxx file should exist along a .cc file for a same given header file.

I believe that when handled correctly those might reduce compile time, do not quote me on that though as they're included anyway with the header ?

Let's talk about this tomorrow during the community call :)

By chance, I've just found that @rsandifo-arm's commit 73b7582 "Add rtl-ssa" did add *.inl files for what appears to be a similar purpose (from a quick look)?

gccrs/gcc/rtl-ssa/internals.inl

Line 1 in 73b7582

// Implementation of private inline member functions for RTL SSA -*- C++ -*-

gccrs/gcc/rtl-ssa/is-a.inl

Line 1 in 73b7582

// is_a<> support for RTL SSA classes -*- C++ -*-

gccrs/gcc/rtl-ssa/member-fns.inl

Lines 20 to 26 in 73b7582

// This file contains inline implementations of public member functions that

// are too large to be written in the class definition. It also contains

// some non-inline template definitions of public member functions.

// See the comments above the function declarations for details.

//

// The file also contains the bare minimum of private and protected inline

// member functions that are needed to make the public functions compile.

(I have no opinion on *.inl vs *.hxx.)

oh, interesting. I didn't know that was an option. I think *.inl looks a little less C++y than *.hxx but I'm not opposed to it

gcc/rust/resolve/rust-forever-stack.hxx

philberty · 2023-06-30T17:23:16Z

gcc/rust/resolve/rust-forever-stack.hxx

+  auto next = std::string (indentation + 4, ' ');
+  auto next_next = std::string (indentation + 8, ' ');
+
+  std::cerr << indent << "Node {\n"


I think logging is really useful but it should be rust_debug lines.

gcc/rust/resolve/rust-forever-stack.h

dkm · 2023-07-03T18:48:42Z

gcc/rust/resolve/rust-forever-stack.h

+   */
+  void pop ();
+
+  /**


There was #1638 , but it's getting killed for good reasons. But do we want to enforce some kind of comments for new comments? Using /** ... */ with doxygen style tags are fine. I was wondering how to properly document other changes.

This is the way I prefer, but I'm fine with other styles. As long as I can write @return and see it get highlighted in my editor, I'm happy haha

What do you use for an editor?

https://helix-editor.com/ :)

CohenArthur · 2023-07-09T13:03:38Z

For debugging, I went with an as_debug_string method which returns a string and uses a stringstream internally. We can dump it using rust_debug or write it to a file in the session manager

This data structure replaces our existing scopes and allows for multiple name resolution passes to insert definitions or look up names at different stages of the pipeline. The documentation goes into detail about how the data structure works, and how the source's hierarchy is preserved throughout the pipeline. The class is templated to enforce specific behavior based on the namespace the scope map targets. gcc/rust/ChangeLog: * resolve/rust-forever-stack.h: New file. * resolve/rust-forever-stack.hxx: New file.

CohenArthur added the enhancement label Jun 28, 2023

CohenArthur added this to the AST Pipeline for libcore 1.49 Complete milestone Jun 28, 2023

P-E-P reviewed Jun 29, 2023

View reviewed changes

philberty requested changes Jun 30, 2023

View reviewed changes

dkm reviewed Jul 3, 2023

View reviewed changes

CohenArthur force-pushed the nr2.0-forever-stack branch 2 times, most recently from 6824522 to 3c05c87 Compare July 9, 2023 13:02

CohenArthur requested a review from philberty July 9, 2023 13:04

CohenArthur force-pushed the nr2.0-forever-stack branch 3 times, most recently from a0aa199 to a3d0b29 Compare July 18, 2023 09:01

CohenArthur added this pull request to the merge queue Jul 18, 2023

github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Jul 18, 2023

CohenArthur added this pull request to the merge queue Jul 19, 2023

Merged via the queue into Rust-GCC:master with commit 011c867 Jul 19, 2023
9 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

nr2.0: Add `ForeverStack` data structure. #2344

nr2.0: Add `ForeverStack` data structure. #2344

CohenArthur commented Jun 28, 2023

P-E-P Jun 29, 2023

P-E-P Jun 29, 2023

philberty left a comment

philberty Jun 30, 2023

P-E-P Jul 4, 2023 •

edited

CohenArthur Jul 9, 2023

tschwinge Jul 18, 2023

CohenArthur Jul 18, 2023

philberty Jun 30, 2023

dkm Jul 3, 2023

CohenArthur Jul 9, 2023

powerboat9 Jul 9, 2023

CohenArthur Jul 9, 2023

CohenArthur commented Jul 9, 2023


		/**

		Let's look at our stack for resolving and traversing the following Rust code:

	// This file contains inline implementations of public member functions that
	// are too large to be written in the class definition. It also contains
	// some non-inline template definitions of public member functions.
	// See the comments above the function declarations for details.
	//
	// The file also contains the bare minimum of private and protected inline
	// member functions that are needed to make the public functions compile.

nr2.0: Add ForeverStack data structure. #2344

nr2.0: Add ForeverStack data structure. #2344

Conversation

CohenArthur commented Jun 28, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

philberty left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

P-E-P Jul 4, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CohenArthur commented Jul 9, 2023

nr2.0: Add `ForeverStack` data structure. #2344

nr2.0: Add `ForeverStack` data structure. #2344

P-E-P Jul 4, 2023 •

edited