erik.pilkington added inline comments.

================
Comment at: clang/include/clang/AST/DeclObjCCommon.h:21
+/// Keep this list in sync with LLVM's Dwarf.h ApplePropertyAttributes.
+enum ObjCPropertyAttributeKind {
+  OBJC_PR_noattr = 0x00,
----------------
plotfi wrote:
> plotfi wrote:
> > compnerd wrote:
> > > It seems that you are touching all the sites that use this, so perhaps 
> > > this is the time to change this to `enum class` and drop the `OBJC_PR_` 
> > > prefixes and explicitly inherit from `uint16_t`.  This should be named 
> > > `ObjCPropertyAttribute` I think.
> > Ah yeah, that sounds pretty good. Will do. 
> Talked offline: update is that its not so easy to change these enums to enum 
> classes because of how they are constantly used with unsigned ints. I could 
> try and implement lots of operator overloads but I think that could be 
> potentially buggy and not so NFC-like. @compnerd you wanted to leave the 
> OBJC_PR_  and dropped the DQ prefixes right? Other than that, ping (for 
> others)? I think this could be a nice cleanup. 
If you just want the scoping benefits of the enum class you can simulate it 
with a `namespace`:

```
namespace ObjCPropertyAttribute {
enum {
  noattr,
  readonly,
  ...
};
}
```

Which would make the users of this enum look a bit nicer. I agree that adding 
overloading operators is over-engineering this. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77233



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to