[PATCH] D61788: Make getObjCEncodingForTypeImpl() take a bitmask instead of 8 bools

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.


================
Comment at: clang/include/clang/AST/ASTContext.h:2877
+    OCET_EncodePointerToObjCTypedef = 1 << 7,
+  };
+  void getObjCEncodingForTypeImpl(QualType t, std::string &S, unsigned Options,
----------------
thakis wrote:
> rjmccall wrote:
> > I like the idea of doing this, but can you add some boilerplate? :)  I think it'd be better if this were a struct with some nice accessors, factories, transformations, and so on.
> > 
> > This example isn't from Clang, but something like this (without the templating, of course): https://github.com/apple/swift/blob/14a20eea03e9115e2c5cf91bccc86e6cd5334df9/include/swift/ABI/MetadataValues.h#L118
> Done. It got pretty wordy (+30 lines instead of -30 before), so I x-macro'd it a bit.
That looks good, but please `camelCase` the method names.  Also, the method names sound like they mutate `this` rather than returning a value with the mutation in effect.


================
Comment at: clang/lib/AST/ASTContext.cpp:6885
+                                .SetIsStructField()
+                                .SetEncodePointerToObjCTypedef()),
+              FD, NotEncodedT);
----------------
These two uses can probably be hoisted up as a common operation, basically "do this for a component of the original type".  `forComponentType()`, maybe?


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

https://reviews.llvm.org/D61788