Skip to content

[ssa-updater] Modernize style before adding support for guaranteed parameters #33347

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

Conversation

gottesmm
Copy link
Contributor

@gottesmm gottesmm commented Aug 6, 2020

Specifically:

  1. I made methods, variables camelCase.
  2. I expanded out variable names (e.x.: bb -> block, predBB -> predBlocks, U -> wrappedUse).
  3. I changed typedef -> using.
  4. I changed a few c style for loops into for each loops using llvm::enumerate.

NOTE: I left the parts needed for syncing to LLVM in the old style since LLVM
needs these to exist for CRTP to work correctly for the SILSSAUpdater.


Just getting this off my local branch so it can percolate a little.

…rameters.

Specifically:

1. I made methods, variables camelCase.
2. I expanded out variable names (e.x.: bb -> block, predBB -> predBlocks, U -> wrappedUse).
3. I changed typedef -> using.
4. I changed a few c style for loops into for each loops using llvm::enumerate.

NOTE: I left the parts needed for syncing to LLVM in the old style since LLVM
needs these to exist for CRTP to work correctly for the SILSSAUpdater.
@gottesmm gottesmm requested a review from atrick August 6, 2020 22:44
@gottesmm
Copy link
Contributor Author

gottesmm commented Aug 6, 2020

Oh I also fixed any/all instances of PHI -> Phi.

@gottesmm
Copy link
Contributor Author

gottesmm commented Aug 6, 2020

@swift-ci test

Copy link
Contributor

@atrick atrick left a comment

Choose a reason for hiding this comment

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

I'm fine with this but bb is generally an accepted abbreviation for basicBlock. I don't want to give that up everywhere. There are very few of these all-lower acronyms, like ii. They're pervasive and they directly mirror the typename so there's no possibility for confusion.

@atrick
Copy link
Contributor

atrick commented Aug 6, 2020

Thanks for fixing PHI -> Phi. It may not be important in the scheme of things but was a pet peeve of mine.

@gottesmm
Copy link
Contributor Author

gottesmm commented Aug 8, 2020

@swift-ci test windows platform

@gottesmm
Copy link
Contributor Author

gottesmm commented Aug 8, 2020

@atrick I am going to do a follow up that undoes those changes b/c I want to validate the bigger change was NFC here.

@gottesmm gottesmm merged commit 17eb463 into swiftlang:master Aug 8, 2020
@gottesmm gottesmm deleted the pr-0e907fa209a6d12c8e02117c55a3d0f8989fa231 branch August 8, 2020 18:38
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