This is an archive of the discontinued LLVM Phabricator instance.

[ModRefInfo] Make enum ModRefInfo an enum class [NFC].
ClosedPublic

Authored by asbirlea on Dec 6 2017, 5:12 PM.

Details

Summary

Make enum ModRefInfo an enum class. Changes to ModRefInfo values should be done using inline wrappers.
This should prevent future bit-wise operations from being added, which can be more error-prone.
FunctionModRefLocation and FunctionModRefBehavior remain enums.

Changes needed to migrate code (out of tree, branches etc):

  • Replace all bit-wise operations with the inline methods declared in "include/llvm/Analysis/AliasAnalysis.h" (example replacements in D40749).
  • Replace all occurrences of "MRI_*" with "ModRefInfo::*".

Diff Detail

Repository
rL LLVM

Event Timeline

asbirlea created this revision.Dec 6 2017, 5:12 PM
asbirlea updated this revision to Diff 125853.Dec 6 2017, 5:17 PM

Update description/commit message.

Note: FunctionModRefLocation and FunctionModRefBehavior remain enums.

asbirlea retitled this revision from [ModRefInfo] Make enum ModRefInfo and enum class. to [ModRefInfo] Make enum ModRefInfo an enum class [NFC]..Dec 6 2017, 5:19 PM
asbirlea edited the summary of this revision. (Show Details)
sanjoy edited edge metadata.Dec 6 2017, 5:26 PM

This is just a find-replace right? I didn't look at the textual changes carefully assuming that but please let me know if there are pieces of this CL that are *not* find-replace and I'll review those more carefully.

include/llvm/Analysis/AliasAnalysis.h
101 ↗(On Diff #125853)

Now that we have an enum class, the MRI_ prefix should be removed (hopefully just a find-replace).

asbirlea updated this revision to Diff 125884.Dec 6 2017, 10:10 PM

Remove MRI_ prefix.

Yes. Just added static casts in the header where we do all the bit-wise operations, and find-replaced all occurrences to include ModRefInfo:: prefix.

sanjoy accepted this revision.Dec 7 2017, 2:07 PM

lgtm

Do you think it will be useful to add some migration instructions to the commit message for folks with out of tree changes to LLVM?

This revision is now accepted and ready to land.Dec 7 2017, 2:07 PM
asbirlea edited the summary of this revision. (Show Details)Dec 7 2017, 2:14 PM

Yes, I think that will be useful, added.

This revision was automatically updated to reflect the committed changes.