[PATCH] D61788: Make getObjCEncodingForTypeImpl() take a bitmask instead of 8 bools
This revision was not accepted when it landed; it landed in state "Needs
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rL360668: Make getObjCEncodingForTypeImpl() take a bitmask
instead of 8 bools (authored by nico, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D61788?vs=199408=199409#toc
Repository:
rL LLVM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D61788/new/
https://reviews.llvm.org/D61788
Files:
cfe/trunk/include/clang/AST/ASTContext.h
cfe/trunk/lib/AST/ASTContext.cpp
Index: cfe/trunk/include/clang/AST/ASTContext.h
===
--- cfe/trunk/include/clang/AST/ASTContext.h
+++ cfe/trunk/include/clang/AST/ASTContext.h
@@ -2864,18 +2864,53 @@
private:
void InitBuiltinType(CanQualType , BuiltinType::Kind K);
+ class ObjCEncOptions {
+unsigned Bits;
+
+ObjCEncOptions(unsigned Bits) : Bits(Bits) {}
+
+ public:
+ObjCEncOptions() : Bits(0) {}
+ObjCEncOptions(const ObjCEncOptions ) : Bits(RHS.Bits) {}
+
+#define OPT_LIST(V)\
+ V(ExpandPointedToStructures, 0) \
+ V(ExpandStructures, 1) \
+ V(IsOutermostType, 2)\
+ V(EncodingProperty, 3) \
+ V(IsStructField, 4) \
+ V(EncodeBlockParameters, 5) \
+ V(EncodeClassNames, 6) \
+ V(EncodePointerToObjCTypedef, 7)
+
+#define V(N,I) ObjCEncOptions& set##N() { Bits |= 1 << I; return *this; }
+OPT_LIST(V)
+#undef V
+
+#define V(N,I) bool N() const { return Bits & 1 << I; }
+OPT_LIST(V)
+#undef V
+
+#undef OPT_LIST
+
+LLVM_NODISCARD ObjCEncOptions keepingOnly(ObjCEncOptions Mask) const {
+ return Bits & Mask.Bits;
+}
+
+LLVM_NODISCARD ObjCEncOptions forComponentType() const {
+ ObjCEncOptions Mask = ObjCEncOptions()
+.setIsOutermostType()
+.setIsStructField()
+.setEncodePointerToObjCTypedef();
+ return Bits & ~Mask.Bits;
+}
+ };
+
// Return the Objective-C type encoding for a given type.
void getObjCEncodingForTypeImpl(QualType t, std::string ,
- bool ExpandPointedToStructures,
- bool ExpandStructures,
+ ObjCEncOptions Options,
const FieldDecl *Field,
- bool OutermostType = false,
- bool EncodingProperty = false,
- bool StructField = false,
- bool EncodeBlockParameters = false,
- bool EncodeClassNames = false,
- bool EncodePointerToObjCTypedef = false,
- QualType *NotEncodedT=nullptr) const;
+ QualType *NotEncodedT = nullptr) const;
// Adds the encoding of the structure's members.
void getObjCEncodingForStructureImpl(RecordDecl *RD, std::string ,
Index: cfe/trunk/lib/AST/ASTContext.cpp
===
--- cfe/trunk/lib/AST/ASTContext.cpp
+++ cfe/trunk/lib/AST/ASTContext.cpp
@@ -6311,13 +6311,13 @@
// Encode type qualifer, 'in', 'inout', etc. for the parameter.
getObjCEncodingForTypeQualifier(QT, S);
// Encode parameter type.
- getObjCEncodingForTypeImpl(T, S, /*ExpandPointedToStructures=*/true,
- /*ExpandStructures=*/true, /*Field=*/nullptr,
- /*OutermostType=*/true,
- /*EncodingProperty=*/false,
- /*StructField=*/false,
- /*EncodeBlockParameters=*/Extended,
- /*EncodeClassNames=*/Extended);
+ ObjCEncOptions Options = ObjCEncOptions()
+ .setExpandPointedToStructures()
+ .setExpandStructures()
+ .setIsOutermostType();
+ if (Extended)
+Options.setEncodeBlockParameters().setEncodeClassNames();
+ getObjCEncodingForTypeImpl(T, S, Options, /*Field=*/nullptr);
}
/// getObjCEncodingForMethodDecl - Return the encoded type for this method
@@ -6509,13 +6509,12 @@
// directly pointed to, and expanding embedded structures. Note that
// these rules are sufficient to prevent recursive encoding of the
// same type.
- getObjCEncodingForTypeImpl(T, S,
[PATCH] D61788: Make getObjCEncodingForTypeImpl() take a bitmask instead of 8 bools
thakis updated this revision to Diff 199408.
thakis added a comment.
minor tweaks for landing
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D61788/new/
https://reviews.llvm.org/D61788
Files:
clang/include/clang/AST/ASTContext.h
clang/lib/AST/ASTContext.cpp
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -6311,13 +6311,13 @@
// Encode type qualifer, 'in', 'inout', etc. for the parameter.
getObjCEncodingForTypeQualifier(QT, S);
// Encode parameter type.
- getObjCEncodingForTypeImpl(T, S, /*ExpandPointedToStructures=*/true,
- /*ExpandStructures=*/true, /*Field=*/nullptr,
- /*OutermostType=*/true,
- /*EncodingProperty=*/false,
- /*StructField=*/false,
- /*EncodeBlockParameters=*/Extended,
- /*EncodeClassNames=*/Extended);
+ ObjCEncOptions Options = ObjCEncOptions()
+ .setExpandPointedToStructures()
+ .setExpandStructures()
+ .setIsOutermostType();
+ if (Extended)
+Options.setEncodeBlockParameters().setEncodeClassNames();
+ getObjCEncodingForTypeImpl(T, S, Options, /*Field=*/nullptr);
}
/// getObjCEncodingForMethodDecl - Return the encoded type for this method
@@ -6509,13 +6509,12 @@
// directly pointed to, and expanding embedded structures. Note that
// these rules are sufficient to prevent recursive encoding of the
// same type.
- getObjCEncodingForTypeImpl(T, S, /*ExpandPointedToStructures=*/true,
- /*ExpandStructures=*/true, Field,
- /*OutermostType=*/true, /*EncodingProperty=*/false,
- /*StructField=*/false,
- /*EncodeBlockParameters=*/false,
- /*EncodeClassNames=*/false,
- /*EncodePointerToObjCTypedef=*/false, NotEncodedT);
+ getObjCEncodingForTypeImpl(T, S,
+ ObjCEncOptions()
+ .setExpandPointedToStructures()
+ .setExpandStructures()
+ .setIsOutermostType(),
+ Field, NotEncodedT);
}
void ASTContext::getObjCEncodingForPropertyType(QualType T,
@@ -6523,9 +6522,13 @@
// Encode result type.
// GCC has some special rules regarding encoding of properties which
// closely resembles encoding of ivars.
- getObjCEncodingForTypeImpl(
- T, S, /*ExpandPointedToStructures=*/true, /*ExpandStructures=*/true,
- /*Field=*/nullptr, /*OutermostType=*/true, /*EncodingProperty=*/true);
+ getObjCEncodingForTypeImpl(T, S,
+ ObjCEncOptions()
+ .setExpandPointedToStructures()
+ .setExpandStructures()
+ .setIsOutermostType()
+ .setEncodingProperty(),
+ /*Field=*/nullptr);
}
static char getObjCEncodingForPrimitiveKind(const ASTContext *C,
@@ -6672,16 +6675,9 @@
}
// FIXME: Use SmallString for accumulating string.
-void ASTContext::getObjCEncodingForTypeImpl(QualType T, std::string& S,
-bool ExpandPointedToStructures,
-bool ExpandStructures,
+void ASTContext::getObjCEncodingForTypeImpl(QualType T, std::string ,
+const ObjCEncOptions Options,
const FieldDecl *FD,
-bool OutermostType,
-bool EncodingProperty,
-bool StructField,
-bool EncodeBlockParameters,
-bool EncodeClassNames,
-bool EncodePointerToObjCTypedef,
QualType *NotEncodedT) const {
CanQualType CT = getCanonicalType(T);
switch (CT->getTypeClass()) {
@@ -6698,18 +6694,16 @@
case Type::Complex: {
const auto *CT = T->castAs();
S += 'j';
-getObjCEncodingForTypeImpl(CT->getElementType(), S,
- /*ExpandPointedToStructures=*/false,
- /*ExpandStructures=*/false, /*Field=*/nullptr);
+getObjCEncodingForTypeImpl(CT->getElementType(), S, ObjCEncOptions(),
+ /*Field=*/nullptr);
return;
}
case Type::Atomic: {
const auto *AT = T->castAs();
S += 'A';
-getObjCEncodingForTypeImpl(AT->getValueType(), S,
-
[PATCH] D61788: Make getObjCEncodingForTypeImpl() take a bitmask instead of 8 bools
thakis added inline comments. Comment at: clang/include/clang/AST/ASTContext.h:2877 +OCET_EncodePointerToObjCTypedef = 1 << 7, + }; + void getObjCEncodingForTypeImpl(QualType t, std::string , 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 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D61788: Make getObjCEncodingForTypeImpl() take a bitmask instead of 8 bools
thakis updated this revision to Diff 199116.
thakis marked 4 inline comments as done.
thakis added a comment.
comments
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D61788/new/
https://reviews.llvm.org/D61788
Files:
clang/include/clang/AST/ASTContext.h
clang/lib/AST/ASTContext.cpp
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -6303,13 +6303,13 @@
// Encode type qualifer, 'in', 'inout', etc. for the parameter.
getObjCEncodingForTypeQualifier(QT, S);
// Encode parameter type.
- getObjCEncodingForTypeImpl(T, S, /*ExpandPointedToStructures=*/true,
- /*ExpandStructures=*/true, /*Field=*/nullptr,
- /*OutermostType=*/true,
- /*EncodingProperty=*/false,
- /*StructField=*/false,
- /*EncodeBlockParameters=*/Extended,
- /*EncodeClassNames=*/Extended);
+ ObjCEncOptions Options = ObjCEncOptions()
+ .setExpandPointedToStructures()
+ .setExpandStructures()
+ .setIsOutermostType();
+ if (Extended)
+Options.setEncodeBlockParameters().setEncodeClassNames();
+ getObjCEncodingForTypeImpl(T, S, Options, /*Field=*/nullptr);
}
/// getObjCEncodingForMethodDecl - Return the encoded type for this method
@@ -6501,13 +6501,12 @@
// directly pointed to, and expanding embedded structures. Note that
// these rules are sufficient to prevent recursive encoding of the
// same type.
- getObjCEncodingForTypeImpl(T, S, /*ExpandPointedToStructures=*/true,
- /*ExpandStructures=*/true, Field,
- /*OutermostType=*/true, /*EncodingProperty=*/false,
- /*StructField=*/false,
- /*EncodeBlockParameters=*/false,
- /*EncodeClassNames=*/false,
- /*EncodePointerToObjCTypedef=*/false, NotEncodedT);
+ getObjCEncodingForTypeImpl(T, S,
+ ObjCEncOptions()
+ .setExpandPointedToStructures()
+ .setExpandStructures()
+ .setIsOutermostType(),
+ Field, NotEncodedT);
}
void ASTContext::getObjCEncodingForPropertyType(QualType T,
@@ -6515,9 +6514,13 @@
// Encode result type.
// GCC has some special rules regarding encoding of properties which
// closely resembles encoding of ivars.
- getObjCEncodingForTypeImpl(
- T, S, /*ExpandPointedToStructures=*/true, /*ExpandStructures=*/true,
- /*Field=*/nullptr, /*OutermostType=*/true, /*EncodingProperty=*/true);
+ getObjCEncodingForTypeImpl(T, S,
+ ObjCEncOptions()
+ .setExpandPointedToStructures()
+ .setExpandStructures()
+ .setIsOutermostType()
+ .setEncodingProperty(),
+ /*Field=*/nullptr);
}
static char getObjCEncodingForPrimitiveKind(const ASTContext *C,
@@ -6664,16 +6667,9 @@
}
// FIXME: Use SmallString for accumulating string.
-void ASTContext::getObjCEncodingForTypeImpl(QualType T, std::string& S,
-bool ExpandPointedToStructures,
-bool ExpandStructures,
+void ASTContext::getObjCEncodingForTypeImpl(QualType T, std::string ,
+ObjCEncOptions Options,
const FieldDecl *FD,
-bool OutermostType,
-bool EncodingProperty,
-bool StructField,
-bool EncodeBlockParameters,
-bool EncodeClassNames,
-bool EncodePointerToObjCTypedef,
QualType *NotEncodedT) const {
CanQualType CT = getCanonicalType(T);
switch (CT->getTypeClass()) {
@@ -6690,18 +6686,16 @@
case Type::Complex: {
const auto *CT = T->castAs();
S += 'j';
-getObjCEncodingForTypeImpl(CT->getElementType(), S,
- /*ExpandPointedToStructures=*/false,
- /*ExpandStructures=*/false, /*Field=*/nullptr);
+getObjCEncodingForTypeImpl(CT->getElementType(), S, ObjCEncOptions(),
+ /*Field=*/nullptr);
return;
}
case Type::Atomic: {
const auto *AT = T->castAs();
S += 'A';
-getObjCEncodingForTypeImpl(AT->getValueType(), S,
-
[PATCH] D61788: Make getObjCEncodingForTypeImpl() take a bitmask instead of 8 bools
rjmccall added inline comments. Comment at: clang/include/clang/AST/ASTContext.h:2877 +OCET_EncodePointerToObjCTypedef = 1 << 7, + }; + void getObjCEncodingForTypeImpl(QualType t, std::string , 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 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D61788: Make getObjCEncodingForTypeImpl() take a bitmask instead of 8 bools
thakis marked 2 inline comments as done. thakis added inline comments. Comment at: clang/include/clang/AST/ASTContext.h:2877 +OCET_EncodePointerToObjCTypedef = 1 << 7, + }; + void getObjCEncodingForTypeImpl(QualType t, std::string , unsigned Options, 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. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61788/new/ https://reviews.llvm.org/D61788 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D61788: Make getObjCEncodingForTypeImpl() take a bitmask instead of 8 bools
rjmccall added inline comments. Comment at: clang/include/clang/AST/ASTContext.h:2877 +OCET_EncodePointerToObjCTypedef = 1 << 7, + }; + void getObjCEncodingForTypeImpl(QualType t, std::string , unsigned Options, 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 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61788/new/ https://reviews.llvm.org/D61788 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D61788: Make getObjCEncodingForTypeImpl() take a bitmask instead of 8 bools
thakis created this revision.
thakis added a reviewer: rjmccall.
Slightly less code, uses slightly less stack space, and makes it
impossible to mix up the order of all those bools.
No behavior change.
https://reviews.llvm.org/D61788
Files:
clang/include/clang/AST/ASTContext.h
clang/lib/AST/ASTContext.cpp
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -6303,13 +6303,11 @@
// Encode type qualifer, 'in', 'inout', etc. for the parameter.
getObjCEncodingForTypeQualifier(QT, S);
// Encode parameter type.
- getObjCEncodingForTypeImpl(T, S, /*ExpandPointedToStructures=*/true,
- /*ExpandStructures=*/true, /*Field=*/nullptr,
- /*OutermostType=*/true,
- /*EncodingProperty=*/false,
- /*StructField=*/false,
- /*EncodeBlockParameters=*/Extended,
- /*EncodeClassNames=*/Extended);
+ unsigned Options = OCET_ExpandPointedToStructures | OCET_ExpandStructures |
+ OCET_IsOutermostType;
+ if (Extended)
+Options |= OCET_EncodeBlockParameters | OCET_EncodeClassNames;
+ getObjCEncodingForTypeImpl(T, S, Options, /*Field=*/nullptr);
}
/// getObjCEncodingForMethodDecl - Return the encoded type for this method
@@ -6501,13 +6499,10 @@
// directly pointed to, and expanding embedded structures. Note that
// these rules are sufficient to prevent recursive encoding of the
// same type.
- getObjCEncodingForTypeImpl(T, S, /*ExpandPointedToStructures=*/true,
- /*ExpandStructures=*/true, Field,
- /*OutermostType=*/true, /*EncodingProperty=*/false,
- /*StructField=*/false,
- /*EncodeBlockParameters=*/false,
- /*EncodeClassNames=*/false,
- /*EncodePointerToObjCTypedef=*/false, NotEncodedT);
+ getObjCEncodingForTypeImpl(T, S,
+ OCET_ExpandPointedToStructures |
+ OCET_ExpandStructures | OCET_IsOutermostType,
+ Field, NotEncodedT);
}
void ASTContext::getObjCEncodingForPropertyType(QualType T,
@@ -6515,9 +6510,11 @@
// Encode result type.
// GCC has some special rules regarding encoding of properties which
// closely resembles encoding of ivars.
- getObjCEncodingForTypeImpl(
- T, S, /*ExpandPointedToStructures=*/true, /*ExpandStructures=*/true,
- /*Field=*/nullptr, /*OutermostType=*/true, /*EncodingProperty=*/true);
+ getObjCEncodingForTypeImpl(T, S,
+ OCET_ExpandPointedToStructures |
+ OCET_ExpandStructures |
+ OCET_IsOutermostType | OCET_EncodingProperty,
+ /*Field=*/nullptr);
}
static char getObjCEncodingForPrimitiveKind(const ASTContext *C,
@@ -6664,16 +6661,9 @@
}
// FIXME: Use SmallString for accumulating string.
-void ASTContext::getObjCEncodingForTypeImpl(QualType T, std::string& S,
-bool ExpandPointedToStructures,
-bool ExpandStructures,
+void ASTContext::getObjCEncodingForTypeImpl(QualType T, std::string ,
+unsigned Options,
const FieldDecl *FD,
-bool OutermostType,
-bool EncodingProperty,
-bool StructField,
-bool EncodeBlockParameters,
-bool EncodeClassNames,
-bool EncodePointerToObjCTypedef,
QualType *NotEncodedT) const {
CanQualType CT = getCanonicalType(T);
switch (CT->getTypeClass()) {
@@ -6690,18 +6680,14 @@
case Type::Complex: {
const auto *CT = T->castAs();
S += 'j';
-getObjCEncodingForTypeImpl(CT->getElementType(), S,
- /*ExpandPointedToStructures=*/false,
- /*ExpandStructures=*/false, /*Field=*/nullptr);
+getObjCEncodingForTypeImpl(CT->getElementType(), S, 0, /*Field=*/nullptr);
return;
}
case Type::Atomic: {
const auto *AT = T->castAs();
S += 'A';
-getObjCEncodingForTypeImpl(AT->getValueType(), S,
- /*ExpandPointedToStructures=*/false,
- /*ExpandStructures=*/false, /*Field=*/nullptr);
+getObjCEncodingForTypeImpl(AT->getValueType(), S, 0, /*Field=*/nullptr);
return;
}
@@ -6727,11 +6713,11 @@
// the pointer itself
