Skip to content

Add frust-extern option#2134

Merged
CohenArthur merged 1 commit into
Rust-GCC:masterfrom
P-E-P:extern_option
Apr 28, 2023
Merged

Add frust-extern option#2134
CohenArthur merged 1 commit into
Rust-GCC:masterfrom
P-E-P:extern_option

Conversation

@P-E-P

@P-E-P P-E-P commented Apr 18, 2023

Copy link
Copy Markdown
Member

Add a new option to gather extern crates location.

gcc/rust/ChangeLog:

* lang.opt: Add frust-extern option.
* rust-session-manager.cc (Session::handle_extern_option): Add frust-extern option handler.
* rust-session-manager.h (struct Session): Add an unordered map to store library name and locations.

@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.

I think this is a good start, but it does not yet cover the entirety of what we want from the option (which is fine). we'll also need to add tests later

Comment thread gcc/rust/rust-session-manager.cc Outdated
Comment on lines +261 to +264
if (std::string::npos == pos)
{
return false;
}

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.

Suggested change
if (std::string::npos == pos)
{
return false;
}
if (std::string::npos == pos)
return false;

nit, also shouldn't we error out here? not sure why rustc accepts rustc --extern <something_without_an_equal_sign> and I haven't looked into it, so maybe we should not error out here

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

No we shouldn't. At least rustc does not, it simply ignores the dependency.

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.

mh, I wonder why that is the case and why there isn't at least a warning or error. @bjorn3 @flip1995 any idea?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I think this is due to the unstable feature with options here. But still, no option and no path seems to raise no error.

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.

I have no idea how the argument parsing and logic in rustc works, so I can't really help here. I just know that last time I looked, rustc uses https://docs.rs/getargs/latest/getargs/ to parse arguments.

Comment thread gcc/rust/rust-session-manager.cc
Comment thread gcc/rust/rust-session-manager.h Outdated
Add a new option to gather extern crates location.

gcc/rust/ChangeLog:

	* lang.opt: Add frust-extern option.
	* rust-session-manager.cc (Session::handle_extern_option): Add
	frust-extern option handler.
	* rust-session-manager.h (struct Session): Add an unordered map
	to store library name and locations.

Signed-off-by: Pierre-Emmanuel Patry <pierre-emmanuel.patry@embecosm.com>
@P-E-P P-E-P requested a review from CohenArthur April 28, 2023 15:29
@CohenArthur CohenArthur enabled auto-merge April 28, 2023 15:36
@CohenArthur CohenArthur added this pull request to the merge queue Apr 28, 2023
Merged via the queue into Rust-GCC:master with commit 1fb32d7 Apr 28, 2023
@P-E-P P-E-P deleted the extern_option branch November 21, 2023 11:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants