Skip to content

Conversation

mikeash
Copy link
Contributor

@mikeash mikeash commented Jun 24, 2019

No description provided.

@mikeash mikeash requested a review from gottesmm June 24, 2019 15:25
@mikeash
Copy link
Contributor Author

mikeash commented Jun 24, 2019

@swift-ci please smoke test

template <>
LLVM_ATTRIBUTE_USED
void TypeContextDescriptor::dump() const {
printf("TargetTypeContextDescriptor.\n");
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks awesome! My only question is why you are using this in only some of the places.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I banged these out while debugging a problem and figured I should add them in. We should definitely make more though.

Copy link
Contributor

Choose a reason for hiding this comment

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

No that isn't what I meant. I meant in some parts you are using "this->" and others you aren't.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, this! Hah. The compiler complained about not finding those fields without it. I don't really understand why.

Copy link
Contributor

Choose a reason for hiding this comment

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

From talking with JoeG it is due to dependent member lookup and c++ being what it is. This looks great to me!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks!

@mikeash mikeash merged commit 62cb20e into swiftlang:master Jun 24, 2019
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