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

Enabling recursion in Visitor.visit_ty makes the compiler reject fixed-length arrays #10894

Closed
alexcrichton opened this issue Dec 10, 2013 · 7 comments · Fixed by #22943
Closed

Enabling recursion in Visitor.visit_ty makes the compiler reject fixed-length arrays #10894

alexcrichton opened this issue Dec 10, 2013 · 7 comments · Fixed by #22943

Comments

@alexcrichton
Copy link
Member

It turns out that the compiler has a blank implementation of visit_ty (which does not recurse). The only relevant recursing is the visit_expr on the length of a fixed-length vector. If recursion is enabled (which I'm doing), the compiler dies all over the place because the expression doesn't have a type listed in the tcx.

For now I'm disabling recursion manually (to restore the old behavior), and tagging all relevant locations with this issue.

cc @nikomatsakis
cc @pnkfelix
@pnkfelix
Copy link
Member

see also #10108

@alexcrichton
Copy link
Member Author

Hm, that is a far better explanation. I'm currently landing this as part of #10862, so it sounds like everything is working as intended. Perhaps both bugs should be closed after that lands?

@pnkfelix
Copy link
Member

@alexcrichton no objections here.

@nikomatsakis
Copy link
Contributor

I agree recursion should be enabled by default. Can you point at the locations where you disabled recursion? it's quite possible that this is just the right thing to do in that instance.

@alexcrichton
Copy link
Member Author

All of the relevant locations are tagged with a FIXME to this bug now that #10862 has landed.

I'm going to close the other bug for now, but leave this one open so it can track the blank visit_ty methods.

@alexcrichton alexcrichton mentioned this issue Dec 10, 2013
Closed
@steveklabnik
Copy link
Member

rust/src/libsyntax/visit.rs

Line 76 in a0f86de

fn visit_ty(&mut self, t: &'v Ty) { walk_ty(self, t) }
<- I think this was fixed. And non of the visit_ty methods are blank now.

@ipetkov
Copy link
Contributor

ipetkov commented Feb 23, 2015

There is still a reference to this bug present in the source, namely the lint visitor: https://github.com/rust-lang/rust/blob/master/src/librustc/lint/context.rs#L645

@ipetkov ipetkov mentioned this issue Mar 1, 2015
Merged
Manishearth added a commit to Manishearth/rust that referenced this issue Mar 3, 2015
@Manishearth
Rollup merge of rust-lang#22943 - ipetkov:lint-recursion, r=alexcrichton
3b30b74 
 * The lint visitor's visit_ty method did not recurse, and had a
  reference to the now closed rust-lang#10894
* The newly enabled recursion has only affected the `deprectated` lint
  which now detects uses of deprecated items in trait impls and
  function return types
* Renamed some references to `CowString` and `CowVec` to `Cow<str>` and
  `Cow<[T]>`, respectively, which appear outside of the crate which
  defines them
* Replaced a few instances of `InvariantType<T>` with
  `PhantomData<Cell<T>>`
* Disabled the `deprecated` lint in several places that
  reference/implement traits on deprecated items which will get cleaned
  up in the future
* Unfortunately, this means that if a library declares
  `#![deny(deprecated)]` and marks anything as deprecated, it will have
  to disable the lint for any uses of said item, e.g. any impl the now
  deprecated item

For any library that denies deprecated items but has deprecated items
of its own, this is a [breaking-change]

I had originally intended for the lint to ignore uses of deprecated items that are declared in the same crate, but this goes against some previous test cases that expect the lint to capture *all* uses of deprecated items, so I maintained the previous approach to avoid changing the expected behavior of the lint.

Tested locally on OS X, so hopefully there aren't any deprecated item uses behind a `cfg` that I may have missed.
flip1995 pushed a commit to flip1995/rust that referenced this issue Jun 30, 2023
@bors
Auto merge of rust-lang#10894 - Centri3:type_repetition_in_bounds, r=…
903fe3b 
…blyxyas,xFrednet

[`type_repetition_in_bounds`]: Don't lint on derived code

fixes rust-lang#10504.

changelog: [`type_repetition_in_bounds`]: Don't lint on derived code
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Enable recursion for visit_ty in lint visitor
5 participants
@steveklabnik @alexcrichton @nikomatsakis @pnkfelix @ipetkov