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.

Sign up for GitHub

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

Jump to bottom

Finalize and update visitor interface #4809

Closed
nikomatsakis opened this issue Feb 6, 2013 · 10 comments
Closed

Finalize and update visitor interface #4809

nikomatsakis opened this issue Feb 6, 2013 · 10 comments
Labels
A-runtime Area: std's runtime and "pre-main" init for handling backtraces, unwinds, stack overflows C-cleanup Category: PRs that clean code up or issues documenting cleanup. metabug Issues about issues themselves ("bugs about bugs")

Comments

@nikomatsakis
Copy link
Contributor

The type visitor interface includes various outdated bits of cruft related to how types used to be, particularly around functions. For example, there is a ret_style, and bare fns and closures are not well distinguished. This should be updated.
@graydon
Copy link
Contributor

graydon commented Jun 12, 2013

still very much valid, properly tagged, 2013-06-11. see also #2595, #3727, #2594, #3475. pure cleanup.

@graydon graydon mentioned this issue Jun 12, 2013
Closed
@bblum
Copy link
Contributor

bblum commented Aug 9, 2013

is the visitor not something internal to rustc? I only know of the AST visitor. (if so this shouldn't be backwards-compatible milestone)

@thestinger
Copy link
Contributor

There's a TyVisitor implemented in librustc/middle/trans/reflect.rs, exposed in std::reflect and used in std::repr. The %? formatter used by fmt! is implemented with std::repr.

As far as I know, there are no other users.

@thestinger
Copy link
Contributor

I've done a bit of work on this in order to improve repr, but it's still in serious need of an overhaul.

@thestinger
Copy link
Contributor

I don't think we should ever make this a stable public interface, because it reveals too many internal compiler details. It would prevent us from adding new primitives in a backwards compatible way. Even a TyDesc should be an implementation detail, and it's exposed in this API.

@catamorphism
Copy link
Contributor

Clearing milestone, as per @thestinger 's argument

@brson
Copy link
Contributor

brson commented Oct 10, 2013

Agree that the visitor itself should not be a public interface. we should have a higher-level reflection api someday.

@flaper87
Copy link
Contributor

flaper87 commented May 5, 2014

Triage: It looks like there are still things to cleanup.

FWIW, I agree TyVisitor shouldn't be a public interface.

@thestinger
Copy link
Contributor

This interface has been removed.

@nikomatsakis
Copy link
Contributor Author

On Tue, Oct 21, 2014 at 03:57:17PM -0700, Daniel Micay wrote:

This interface has been removed.

Horray! (I'm late to the party; catching up on github notifications)

bors added a commit to rust-lang-ci/rust that referenced this issue May 2, 2020
@bors
Auto merge of rust-lang#4809 - iankronquist:patch-1, r=flip1995
779b6ae 
Typo in literal_representation.rs

Octal numbers can't have 8 in them ;)

changelog: none
bors added a commit to rust-lang-ci/rust that referenced this issue May 2, 2020
@bors
Auto merge of rust-lang#5190 - flip1995:nuke_traveyor, r=phansch,Mani…
06f0ab0 
…shearth,llogiq,flip1995

I like to move it, move it

GHA now runs in the background for 6 days (rust-lang#5088)

Since then ~~15~~ 19 PRs were successfully merged and Travis+Appveyor agreed on the status in every case. ([GitHub PR search query](https://github.com/rust-lang/rust-clippy/pulls?utf8=%E2%9C%93&q=is%3Apr+is%3Amerged+merged%3A%3E%3D2020-02-12T15%3A42%3A00+sort%3Aupdated-desc+NOT+%5Bgh-pages%5D+in%3Atitle))

Some PRs were:
- rust-lang#5163
- rust-lang#5170
- rust-lang#5168
- rust-lang#5173
- rust-lang#5171
- rust-lang#5156
- rust-lang#4809
- rust-lang#5177
- rust-lang#5182
- rust-lang#5183
- rust-lang#5184
- rust-lang#5185
- rust-lang#5186
- rust-lang#5181
- rust-lang#5189

Bug with GHA:
- When a rustc PR gets merged between the `integration_build` and the `integration` job, the `integration` job will fail. This happened once in rust-lang#5162, but not in the past 6 days. Even if it would happen every 4th PR we would save time, since splitting up the integration build and tests saves 5-7 minutes per run and a complete run takes 15-17 minutes
- Sometimes the MacOS build takes up to an hour to download the master toolchain. Until now, this happend 2 or 3 times and can be resolved by a `@bors r3try`+canceling the previous run (restarting single jobs is not supported yet)

## Before merging this, rust-lang/rust-central-station#578 has to get merged

This PR is for starting the discussion and to get consensus (@rust-lang/clippy) on a final move to GHA. If we're ready, I'll contact Pietro, to finalize the move.

changelog: Clippy completely runs on GHA now 🎉

---

BTW: The deployment already runs on GHA, instead of Travis.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-runtime Area: std's runtime and "pre-main" init for handling backtraces, unwinds, stack overflows C-cleanup Category: PRs that clean code up or issues documenting cleanup. metabug Issues about issues themselves ("bugs about bugs")
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
7 participants
@flaper87 @graydon @brson @nikomatsakis @catamorphism @thestinger @bblum