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

Refactor TyStruct/TyEnum/TyUnion into TyAdt #36331

Merged
merged 2 commits into from
Sep 9, 2016
Merged

Conversation

petrochenkov
Copy link
Contributor

r? @eddyb

@petrochenkov
Copy link
Contributor Author

The diff looks much better with whitespaces ignored https://github.com/rust-lang/rust/pull/36331/files?w=1

@@ -555,7 +555,7 @@ impl<'a, 'gcx, 'tcx> Struct {
}

// Is this the NonZero lang item wrapping a pointer or integer type?
(&Univariant { non_zero: true, .. }, &ty::TyStruct(def, substs)) => {
(&Univariant { non_zero: true, .. }, &ty::TyAdt(def, substs)) if def.is_struct() => {
Copy link
Member

Choose a reason for hiding this comment

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

The guard is unnecessary (because non_zero means it's the NonZero lang item).

@eddyb
Copy link
Member

eddyb commented Sep 7, 2016

LGTM modulo somewhat minor ty::layout / trans::adt concerns.

@@ -133,23 +128,27 @@ impl<'a, 'tcx> MarkSymbolVisitor<'a, 'tcx> {

fn handle_field_access(&mut self, lhs: &hir::Expr, name: ast::Name) {
match self.tcx.expr_ty_adjusted(lhs).sty {
ty::TyStruct(def, _) | ty::TyUnion(def, _) => {
ty::TyAdt(def, _) => {
self.insert_def_id(def.struct_variant().field_named(name).did);
Copy link
Member

Choose a reason for hiding this comment

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

What happens here if sty here was a TyEnum previously?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

struct_variant() reports another span_bug!

@petrochenkov
Copy link
Contributor Author

Updated, comments addressed.

@eddyb
Copy link
Member

eddyb commented Sep 9, 2016

@bors r+

@bors
Copy link
Contributor

bors commented Sep 9, 2016

📌 Commit 553d5f0 has been approved by eddyb

@bors
Copy link
Contributor

bors commented Sep 9, 2016

⌛ Testing commit 553d5f0 with merge f1f40f8...

bors added a commit that referenced this pull request Sep 9, 2016
Refactor `TyStruct`/`TyEnum`/`TyUnion` into `TyAdt`

r? @eddyb
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.

4 participants