[PATCH] D153305: [MemoryEffects][NFCI] Make the MemoryEffects class reusable

[Yevgeny Rouban via Phabricator via llvm-commits]

yrouban added inline comments.


================
Comment at: llvm/include/llvm/Support/ModRef.h:65
+  /// Any other memory.
+  Other = 2,
+
----------------
yrouban wrote:
> These unscoped enumerators expose very generic identifiers //Other//, //First//, //Last// in the namespace llvm.
> So all sources using llvm namespace may get a compilation error if they use these identifiers for their own purposes.
> These unscoped enumerators expose very generic identifiers //Other//, //First//, //Last// in the namespace llvm.
> So all sources using llvm namespace may get a compilation error if they use these identifiers for their own purposes.




================
Comment at: llvm/include/llvm/Support/ModRef.h:65
+  /// Any other memory.
+  Other = 2,
+
----------------
yrouban wrote:
> yrouban wrote:
> > These unscoped enumerators expose very generic identifiers //Other//, //First//, //Last// in the namespace llvm.
> > So all sources using llvm namespace may get a compilation error if they use these identifiers for their own purposes.
> > These unscoped enumerators expose very generic identifiers //Other//, //First//, //Last// in the namespace llvm.
> > So all sources using llvm namespace may get a compilation error if they use these identifiers for their own purposes.
> 
> 
I suggest that we use a scoped enumerators:
```
enum class IRMemLocation { ... }
```


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D153305/new/

https://reviews.llvm.org/D153305