Skip to content
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

add new Rust 1.57 and Rust 1.58 targets #79

Merged
merged 1 commit into from
Feb 1, 2022

Conversation

sunshowers
Copy link
Contributor

There are some idiosyncratic targets in here -- include special code to
handle them.

@sunfishcode
Copy link
Member

Thinking about this in the context of #78, #63, and #76, we've been seeing more pressure towards special-case parsing, and it seems that Rust target names have been evolving away from being triples and toward just being arbitrary sequences of identifiers. This suggests that perhaps target-lexicon should start refocusing on supporting rustc json target identifiers, which are explicitly split into arch/os/vendor/env categories, and not try to support all the new rust target names.

I'm curious if you have a perspective on this. Do you need all these new targets? Do you use target-lexicon for parsing target names?

@sunshowers
Copy link
Contributor Author

Thanks! That's a great question. In my own uses in target-spec I use cfg-expr first. That library has a key-value map of builtin targets, similar to what you're suggesting.

I then fall back to target-lexicon if cfg-expr doesn't have a record of that target, typically if the target is new or somehow custom. The heuristic parsing target-lexicon does is still useful for some new targets (eg in this PR, m68k-unknown-linux-gnu, armv7-unknown-linux-uclibceabihf).

So I think the "key-value map queried from the latest rustc json targets" use case is covered well by cfg-expr. I can see target-lexicon still serving a few purposes:

  • historical record of old rustc targets
  • heuristic processing as a fallback to the key-value map approach

Let me know what you think!

@sunfishcode
Copy link
Member

Makes sense. So the thing about this PR that makes me the most nervous is TripleField. Would it be difficult to organize this code in a way that allows special-case rules to be factored out into helper functions, so that they can be applied one at a time and be clearly separated?

@sunshowers
Copy link
Contributor Author

Hi -- I completely apologize for dropping the ball here. I'm going to get to it now.

There are some idiosyncratic targets in here -- include special code to
handle them.
@sunshowers sunshowers changed the title add new Rust 1.57 targets add new Rust 1.57 and Rust 1.58 targets Jan 17, 2022
@sunshowers
Copy link
Contributor Author

This should address your concerns -- thanks @sunfishcode!

@sunfishcode sunfishcode merged commit 8a43a51 into bytecodealliance:main Feb 1, 2022
@sunfishcode
Copy link
Member

Looks good, thanks!

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.

2 participants