[PATCH] D61788: Make getObjCEncodingForTypeImpl() take a bitmask instead of 8 bools
Nico Weber via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Fri May 10 18:38:57 PDT 2019
thakis 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,
----------------
rjmccall wrote:
> 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.
Made them mutate this, except for keepOnly() which I renamed keepingOnly() and marked LLVM_NODISCARD, and clear which is now forComponentType() and also LLVM_NODISCARD.
Made all methods except the accessors camelCase. (Making the first letter lower case as-is is tricky with the xmacro, and isExpandPointedToStructures sounds strange. hasExpandPointedToStructures sounds better, but hasIsStructField is weird.)
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D61788/new/
https://reviews.llvm.org/D61788
More information about the cfe-commits
mailing list