Skip to content

Deleted the as_string AST dump#2114

Merged
CohenArthur merged 1 commit into
Rust-GCC:masterfrom
mvvsmk:Issue_2022
Apr 21, 2023
Merged

Deleted the as_string AST dump#2114
CohenArthur merged 1 commit into
Rust-GCC:masterfrom
mvvsmk:Issue_2022

Conversation

@mvvsmk

@mvvsmk mvvsmk commented Apr 10, 2023

Copy link
Copy Markdown
Contributor

Fixes #2022
Removed the Session::dump_ast function, and all the elements associated with it.

gcc/rust/ChangeLog:

* rust-session-manager.cc (Session::enable_dump): Removed else if (arg == "parse").
(Session::compile_crate): Removed the statement that called the dump_ast.
(Session::dump_ast): Removed dump_ast function.
* rust-session-manager.h (struct CompileOptions): Removed the PARSER_AST_DUMP option.

Signed-off-by: M V V S Manoj Kumar mvvsmanojkumar@gmail.com

Do let me know if I need to change something or missed anything .

@mvvsmk mvvsmk force-pushed the Issue_2022 branch 2 times, most recently from 040ca5d to 7b5a7f6 Compare April 11, 2023 06:41

@philberty philberty left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You should also remove the function:

// Parses crate and dumps AST to stderr, recursively.
template <typename ManagedTokenSource>
void
Parser<ManagedTokenSource>::debug_dump_ast_output (AST::Crate &crate,
						   std::ostream &out)
{
  out << crate.as_string ();
}

From rust-parse-impl.h and it looks like clang-format is complaining

@philberty

Copy link
Copy Markdown
Member

Ah clang-format is complaining but its unreleated to your changes do a rebase ontop of master and see if that sorts it out

@mvvsmk mvvsmk force-pushed the Issue_2022 branch 4 times, most recently from 3b56561 to 8ccf283 Compare April 13, 2023 16:25
@mvvsmk

mvvsmk commented Apr 13, 2023

Copy link
Copy Markdown
Contributor Author

I made the changes you requested, and removed the debug_dump_ast_output function, I also had to remove the Session::dump_ast_expanded function too, and chaged the expanded dump to print only the ast_pretty. This also fixed #2021.

do tell me if I missed somthing. :)

I appologise for the large number of commits, for some reason clang format always raises an exception here, I looked at the log but still couldn't figure out what it wants it says :

--- gcc/rust/parse/rust-parse-impl.h	(original)
+++ gcc/rust/parse/rust-parse-impl.h	(reformatted)
@@ -10693,7 +10693,7 @@
     {
       lexer.skip_token ();
       alts.push_back (parse_pattern_no_alt ());
-    } 
+    }
   while (lexer.peek_token ()->get_id () == PIPE);
 
   /* alternates */

could you help me here I don't undustand what it wants me to do here
I looked at the master files:

do
{
lexer.skip_token ();
alts.push_back (parse_pattern_no_alt ());
}
while (lexer.peek_token ()->get_id () == PIPE);

and it looks the exact same T-T.

@CohenArthur

Copy link
Copy Markdown
Member

@mvvsmk the issue is that your original code has a whitespace after the curly bracket. The clang-format removes that extra whitespace.

I would really suggest running clang-format locally for things like this, which I agree are extremely annoying to fix haha. I have my clang-format set up to run whenever I save a file in my editor and it helps a lot. So the change you have to make is this:

-<space><space><space><space>}<space>
+<space><space><space><space>}

@mvvsmk mvvsmk force-pushed the Issue_2022 branch 2 times, most recently from 4d777ec to 1ca4c88 Compare April 14, 2023 13:05
@mvvsmk

mvvsmk commented Apr 14, 2023

Copy link
Copy Markdown
Contributor Author

Thank you @CohenArthur for helping me out. I did have clang-format to run when I saved the file, but idk why it has a problem with that perticular statement, as in my IDE auto formats it again for some reason even though I didn't touch while making my changes, I'll look into it and have it properly configured.

@mvvsmk mvvsmk requested a review from philberty April 14, 2023 15:07
@mvvsmk mvvsmk force-pushed the Issue_2022 branch 2 times, most recently from 2e9d994 to ab030e7 Compare April 17, 2023 10:55
@mvvsmk

mvvsmk commented Apr 17, 2023

Copy link
Copy Markdown
Contributor Author

You should also remove the function:

// Parses crate and dumps AST to stderr, recursively.
template <typename ManagedTokenSource>
void
Parser<ManagedTokenSource>::debug_dump_ast_output (AST::Crate &crate,
						   std::ostream &out)
{
  out << crate.as_string ();
}

From rust-parse-impl.h and it looks like clang-format is complaining

@philberty I had completed the changes you requested, could you please have a look at it again, I have also rebased the PR so that github dosen't show that the PR is outdated. :) this PR should also fixes #2021 , do let me know if I missed anything .

@CohenArthur CohenArthur left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM!

rust_error_at (
Location (),
"dump option was not given a name. choose %<lex%>, %<parse%>, "
"dump option was not given a name. choose %<lex%>, "

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

oh, I had never realized that we were parsing the string directly here instead of doing magic in gcc/rust/lang.opt like we do for the other enums options we've added. I think this is fine but we should eventually think about moving it here maybe? but this is not relevant to your PR so no worries

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We had another file for parsing command line args?, I will check that out. :D

Fixes Rust-GCC#2021, Rust-GCC#2022
Deleted Parser::debug_dump_ast_output, removed any functions that called
it i.e Session::dump_ast and Session::dump_ast_expanded, and any
associated items.

Made it so that when you use the dump option "expanded" it dumps the
pretty ast only.

gcc/rust/ChangeLog:

	* parse/rust-parse-impl.h (Parser::debug_dump_ast_output): Removed this funtion.
	* rust-session-manager.cc (Session::enable_dump): Removed else if (arg == "parse").
	(Session::compile_crate): Removed calls of dump_ast and dump_ast_expanded.
	(Session::dump_ast): Removed this function.
	(Session::dump_ast_expanded): Removed this function.
	* rust-session-manager.h (struct CompileOptions): Removed the PARSER_AST_DUMP option.

Signed-off-by: M V V S Manoj Kumar <mvvsmanojkumar@gmail.com>
@mvvsmk

mvvsmk commented Apr 21, 2023

Copy link
Copy Markdown
Contributor Author

update : just rebased the PR, so that it doesn't create any problems, my branch had became like 105 commits behind.

@CohenArthur CohenArthur added this pull request to the merge queue Apr 21, 2023
Merged via the queue into Rust-GCC:master with commit 2078d8d Apr 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Delete as_string AST dump

3 participants