This is an archive of the discontinued LLVM Phabricator instance.

[MLIR] Continue renaming of "SideEffects"
ClosedPublic

Authored by stephenneuendorffer on May 13 2020, 10:36 AM.

Details

Summary

MLIRSideEffects -> MLIRSideEffectInterfaces
SideEffects.h -> SideEffectInterfaces.h
SideEffects.cpp -> SideEffectInterface.cpp

Note that I haven't renamed TableGen/SideEffects.h or TableGen/SideEffects.cpp

find -name "*.h" -exec sed -i "s/SideEffects.h/SideEffectInterfaces.h/" "{}" \;
find -name "CMakeLists.txt" -exec sed -i "s/MLIRSideEffects/MLIRSideEffectInterfaces/" "{}" \;

Diff Detail

Event Timeline

stephenneuendorffer edited the summary of this revision. (Show Details)
mehdi_amini added inline comments.May 14 2020, 9:53 AM
mlir/lib/Interfaces/SideEffectInterfaces.cpp
1

Please fix :)

stephenneuendorffer marked an inline comment as done.May 14 2020, 2:49 PM
mehdi_amini accepted this revision.May 14 2020, 7:18 PM
This revision was not accepted when it landed; it landed in state Needs Review.May 15 2020, 2:43 PM
This revision was automatically updated to reflect the committed changes.

I would have really preferred that we went in the opposite direction.

I would have really preferred that we went in the opposite direction.

+1, I mentioned it on a previous revision: duplicating "Interface" in the path isn't useful.
I understood it would be coming later but first this was just aligning it all. @stephenneuendorffer can you clarify?

@mehdi_amini @rriddle There were multiple commenters who thought it was better to be consistent even if there was further mass renaming involved:
see https://reviews.llvm.org/D79517

mehdi_amini added a comment.EditedMay 27 2020, 11:11 PM

@mehdi_amini @rriddle There were multiple commenters who thought it was better to be consistent even if there was further mass renaming involved:
see https://reviews.llvm.org/D79517

Sure but my assumption is that the mass renaming is coming... Otherwise I'd have push back here.
(But I actually didn't understand why "being consistent" was preventing from renaming in the other direction in the first place....)