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 manual size calculations to use totalSizeToAlloc #33256

Merged
merged 1 commit into from Aug 4, 2020

Conversation

mateioprea
Copy link
Contributor

Refactor some of the compiler methods which are using manual calculations for allocating memory. Since some of the compiler classes do not extend the TrailingObjects class as a friend class, I assumed that this requires more than a "StarterBug" experience.

Resolves SR-13246.

@mateioprea
Copy link
Contributor Author

@varungandhi-apple can you look over this? thanks

@varungandhi-apple
Copy link
Contributor

Since some of the compiler classes do not extend the TrailingObjects class as a friend class, I assumed that this requires more than a "StarterBug" experience.

Yeah, I intended the fix to be only for places sub-classing from TrailingObjects, since the totalSizeToAlloc method is a method on TrailingObjects.

Copy link
Contributor

@varungandhi-apple varungandhi-apple left a comment

Choose a reason for hiding this comment

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

LGTM. Let me run the CI first. If it passes, we can squash and merge.

@varungandhi-apple
Copy link
Contributor

@swift-ci please test

Refactor manual size calculations in getImportSet

Refactor manual size calculations in Enum Declaration::create

Refactor manual size calculations in memory allocation on SequenceExpr::create

Refactor manual size calculations in memory allocation

Refactor manual size calculations in memory allocation
@mateioprea
Copy link
Contributor Author

@varungandhi-apple since all tests passed here, i squashed the commit and should be good to go. thank you!

@varungandhi-apple
Copy link
Contributor

@swift-ci please smoke test and merge

1 similar comment
@varungandhi-apple
Copy link
Contributor

@swift-ci please smoke test and merge

@varungandhi-apple
Copy link
Contributor

@swift-ci please smoke test macOS

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.

None yet

2 participants