Description
Proposal
Currently, we initialize rustc_target::spec::Target using Target::default and then update it. Then, elsewhere in the compiler, we make somewhat ad-hoc decisions on how we use the available data, directly accessing its internals.
Costs of the status quo
Every builtin target in rustc_target::spec has to specify its own overrides, instead of reusing patterns that other targets benefit from. Many properties on a target can be completely derived from specific parts of the logical tuple: the tuple after... "quirks" of spelling have been stripped away or elaborated into their meanings. Indeed, there are many places where it may be beneficial to precompute certain answers that are based on accessing multiple target fields and repeatedly used in the compiler, but we can't do that as part of rustc_target::spec::Target's constructor because it doesn't really have one.
- This makes it unnecessarily hard to maintain our target lists. e.g. in order to make most of our aarch64 targets use non-leaf frame pointers because Apple, Clang, and MSVC all default to that, we have shipped
- Add support for leaf function frame pointer elimination rust#86652 (touches 17 files in
specas part of initial impl) - Enable non-leaf Frame Pointers for Arm64 Windows rust#140828 (touches 1 file)
- aarch64-linux: Default to FramePointer::NonLeaf rust#140832 (touches 7 files in
spec) - compiler: set Apple frame pointers by architecture rust#141797 (16 files in
specto finish what #86652 started!) - Enable non-leaf Frame Pointers for mingw-w64 Arm64 Windows rust#141815 (touches 1 file)
- Add support for leaf function frame pointer elimination rust#86652 (touches 17 files in
- There is disunity between the constructor for the
target-spec.jsonand thedefault-then-override by the various targets, allowing those to drift in function or executed checks. I have repeatedly made the error, while preparing PRs, of making an update for just one of those two sets. Fortunately this tends to fall apart in testing when it comes to major changes, but this means that some drift is probably occurring in more subtle ways. - It makes it harder to consistently report errors for targets. I currently believe (but have not yet obtained proof that) there are probably ways you can set the
target-spec.jsonthat you can bypass, and which will not produce warnings or errors, when writing a builtin target in antarget_spec.rs! It is possible the reverse is also true, I'm not sure. - It means that the
target-spec.jsonandtarget_spec.rsboth have to repeat information, allowing possible disunity and requiring post-construction validation checks elsewhere, instead of simply constructing more information from what is given. - We tie the way we construct or deserialize the data tightly to the data that the rest of the compiler uses, allowing errors on invalid targets to be lazily issued only after starting compilation and not immediately after the target values are known, even if they are impossible to codegen with, and often making it so the rest of the compiler parses a string over and over again, e.g.
We already parse or elaborate the data from rustc_target::spec::Target in ad-hoc ways inside the compiler. I believe we should go the rest of the way and separate the constructor's arguments from the constructor per se: the Builder pattern. This would let us
- force more target values to be specified when there is possible ambiguity
- improve the coordination between
target_spec.rsandtarget-spec.json - improve our maintainability for the many targets we offer an extended support matrix for
- potentially even do previously hard-to-imagine things, like consider stabilizing an input format for the
target-spec.json(or any other input format) because we can keep the arguments consistent
Notably not changing
- For now, everyone will probably still have to have their own
tuple.rsfor now, it just will contain less code and especially less repeats between e.g.arch1_vendor1_os1_env1.rs,arch1_vendor1_os1_env2.rs,arch2_vendor1_os1_env1.rs, andarch2_vendor1_os1_env2.rs. - The
spec::basecode may mutate, but it will still be allowed to vary according to the unique needs of a given subset of targets. We will hopefully positioned to better use more of the same patterns between targets, that's all. - We still will accept kinda random strings for things in some cases, allowing people to still use the compiler in extended and experimental ways that are likely to backfire on them. We just will be able to better diagnose unusual setups and error out when we know we cannot pursue codegen in a consistent way.
Risks
- Moving to a more deliberately compositional construction may make it easier to accidentally change a target in a way that is inappropriate for it.
- I believe this is minimal risk and mostly-outweighed by our current ability to accidentally not change a target in a way that is appropriate for it.
- Churn in the way we define targets may annoy our target maintainership.
- People may hide one-off target overrides inside the Builder.
- I suggest we rap them on the knuckles for trying to prematurely abstract over a problem.
Mentors or Reviewers
- Myself?
- I'm hoping to get some reviews from target maintainers like @madsmtm, @mati865, @wesleywiser, etc.
Process
The main points of the Major Change Process are as follows:
- File an issue describing the proposal.
- A compiler team member or contributor who is knowledgeable in the area can second by writing
@rustbot second.- Finding a "second" suffices for internal changes. If however, you are proposing a new public-facing feature, such as a
-C flag, then full team check-off is required. - Compiler team members can initiate a check-off via
@rfcbot fcp mergeon either the MCP or the PR.
- Finding a "second" suffices for internal changes. If however, you are proposing a new public-facing feature, such as a
- Once an MCP is seconded, the Final Comment Period begins. If no objections are raised after 10 days, the MCP is considered approved.
You can read more about Major Change Proposals on forge.