Make getObjCEncodingForTypeImpl() take a bitmask instead of 8 bools

[thakis]

Summary

Slightly easier to read, uses slightly less stack space, and makes it
impossible to mix up the order of all those bools.

No behavior change.

[rjmccall]

clang/include/clang/AST/ASTContext.h

2877 ↗

(On Diff #199021)

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

[thakis]

Now with more boilerplate!

[thakis]

clang/include/clang/AST/ASTContext.h

2877 ↗

(On Diff #199021)

Done. It got pretty wordy (+30 lines instead of -30 before), so I x-macro'd it a bit.

[rjmccall]

clang/include/clang/AST/ASTContext.h

2877 ↗

(On Diff #199021)

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.

clang/lib/AST/ASTContext.cpp

6885 ↗

(On Diff #199056)

These two uses can probably be hoisted up as a common operation, basically "do this for a component of the original type". forComponentType(), maybe?

[thakis]

comments

clang/include/clang/AST/ASTContext.h

2877 ↗

(On Diff #199021)

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.)

[thakis]

minor tweaks for landing

cfe/trunk/include/clang/AST/ASTContext.h

public:
   ///
   /// \param Target The target
   void InitBuiltinTypes(const TargetInfo &Target,
                         const TargetInfo *AuxTarget = nullptr);
 
 private:
   void InitBuiltinType(CanQualType &R, BuiltinType::Kind K);
 
+  class ObjCEncOptions {
+    unsigned Bits;
+
+    ObjCEncOptions(unsigned Bits) : Bits(Bits) {}
+
+  public:
+    ObjCEncOptions() : Bits(0) {}
+    ObjCEncOptions(const ObjCEncOptions &RHS) : 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 &S,
-                                  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 &S,
                                        const FieldDecl *Field,
                                        bool includeVBases = true,
                                        QualType *NotEncodedT=nullptr) const;
 
 public:

cfe/trunk/lib/AST/ASTContext.cpp

 /// method parameter or return type. If Extended, include class names and
 /// block object types.
 void ASTContext::getObjCEncodingForMethodParameter(Decl::ObjCDeclQualifier QT,
                                                    QualType T, std::string& S,
                                                    bool Extended) const {
   // 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
 /// declaration.
 std::string ASTContext::getObjCEncodingForMethodDecl(const ObjCMethodDecl *Decl,
                                                      bool Extended) const {
   // FIXME: This is not very efficient.
   // Encode return type.

 
 void ASTContext::getObjCEncodingForType(QualType T, std::string& S,
                                         const FieldDecl *Field,
                                         QualType *NotEncodedT) const {
   // We follow the behavior of gcc, expanding structures which are
   // 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,
                                                 std::string& S) const {
   // 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,
                                             BuiltinType::Kind kind) {
     switch (kind) {
     case BuiltinType::Void:       return 'v';
     case BuiltinType::Bool:       return 'B';
     case BuiltinType::Char8:
else {
       const auto *BT = T->castAs<BuiltinType>();
       S += getObjCEncodingForPrimitiveKind(Ctx, BT->getKind());
     }
   }
   S += llvm::utostr(FD->getBitWidthValue(*Ctx));
 }
 
 // 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 &S,
+                                            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()) {
   case Type::Builtin:
   case Type::Enum:
     if (FD && FD->isBitField())
       return EncodeBitField(this, S, T, FD);
     if (const auto *BT = dyn_cast<BuiltinType>(CT))
       S += getObjCEncodingForPrimitiveKind(this, BT->getKind());
     else
       S += ObjCEncodingForEnumType(this, cast<EnumType>(CT));
     return;
 
   case Type::Complex: {
     const auto *CT = T->castAs<ComplexType>();
     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<AtomicType>();
     S += 'A';
-    getObjCEncodingForTypeImpl(AT->getValueType(), S,
-                               /*ExpandPointedToStructures=*/false,
-                               /*ExpandStructures=*/false, /*Field=*/nullptr);
+    getObjCEncodingForTypeImpl(AT->getValueType(), S, ObjCEncOptions(),
+                               /*Field=*/nullptr);
     return;
   }
 
   // encoding for pointer or reference types.
   case Type::Pointer:
   case Type::LValueReference:
   case Type::RValueReference: {
     QualType PointeeTy;
case Type::RValueReference: {
     }
 
     bool isReadOnly = false;
     // For historical/compatibility reasons, the read-only qualifier of the
     // pointee gets emitted _before_ the '^'.  The read-only qualifier of
     // the pointer itself gets ignored, _unless_ we are looking at a typedef!
     // Also, do not emit the 'r' for anything but the outermost type!
     if (isa<TypedefType>(T.getTypePtr())) {
-      if (OutermostType && T.isConstQualified()) {
+      if (Options.IsOutermostType() && T.isConstQualified()) {
         isReadOnly = true;
         S += 'r';
       }
-    } else if (OutermostType) {
+    } else if (Options.IsOutermostType()) {
       QualType P = PointeeTy;
       while (P->getAs<PointerType>())
         P = P->getAs<PointerType>()->getPointeeType();
       if (P.isConstQualified()) {
         isReadOnly = true;
         S += 'r';
       }
     }
if (PointeeTy->isCharType()) {
         S += '@';
         return;
       }
       // fall through...
     }
     S += '^';
     getLegacyIntegralTypeEncoding(PointeeTy);
 
-    getObjCEncodingForTypeImpl(
-        PointeeTy, S, /*ExpandPointedToStructures=*/false,
-        /*ExpandStructures=*/ExpandPointedToStructures, /*Field=*/nullptr,
-        /*OutermostType=*/false, /*EncodingProperty=*/false,
-        /*StructField=*/false, /*EncodeBlockParameters=*/false,
-        /*EncodeClassNames=*/false, /*EncodePointerToObjCTypedef=*/false,
-        NotEncodedT);
+    ObjCEncOptions NewOptions;
+    if (Options.ExpandPointedToStructures())
+      NewOptions.setExpandStructures();
+    getObjCEncodingForTypeImpl(PointeeTy, S, NewOptions,
+                               /*Field=*/nullptr, NotEncodedT);
     return;
   }
 
   case Type::ConstantArray:
   case Type::IncompleteArray:
   case Type::VariableArray: {
     const auto *AT = cast<ArrayType>(CT);
 
-    if (isa<IncompleteArrayType>(AT) && !StructField) {
+    if (isa<IncompleteArrayType>(AT) && !Options.IsStructField()) {
       // Incomplete arrays are encoded as a pointer to the array element.
       S += '^';
 
-      getObjCEncodingForTypeImpl(AT->getElementType(), S,
-                                 /*ExpandPointedToStructures=*/false,
-                                 ExpandStructures, FD);
+      getObjCEncodingForTypeImpl(
+          AT->getElementType(), S,
+          Options.keepingOnly(ObjCEncOptions().setExpandStructures()), FD);
     } else {
       S += '[';
 
       if (const auto *CAT = dyn_cast<ConstantArrayType>(AT))
         S += llvm::utostr(CAT->getSize().getZExtValue());
       else {
         //Variable length arrays are encoded as a regular array with 0 elements.
         assert((isa<VariableArrayType>(AT) || isa<IncompleteArrayType>(AT)) &&
                "Unknown array type!");
         S += '0';
       }
 
       getObjCEncodingForTypeImpl(
           AT->getElementType(), S,
-          /*ExpandPointedToStructures=*/false, ExpandStructures, FD,
-          /*OutermostType=*/false,
-          /*EncodingProperty=*/false, /*StructField=*/false,
-          /*EncodeBlockParameters=*/false, /*EncodeClassNames=*/false,
-          /*EncodePointerToObjCTypedef=*/false, NotEncodedT);
+          Options.keepingOnly(ObjCEncOptions().setExpandStructures()), FD,
+          NotEncodedT);
       S += ']';
     }
     return;
   }
 
   case Type::FunctionNoProto:
   case Type::FunctionProto:
     S += '?';
if (const IdentifierInfo *II = RDecl->getIdentifier()) {
         const TemplateArgumentList &TemplateArgs = Spec->getTemplateArgs();
         llvm::raw_string_ostream OS(S);
         printTemplateArgumentList(OS, TemplateArgs.asArray(),
                                   getPrintingPolicy());
       }
     } else {
       S += '?';
     }
-    if (ExpandStructures) {
+    if (Options.ExpandStructures()) {
       S += '=';
       if (!RDecl->isUnion()) {
         getObjCEncodingForStructureImpl(RDecl, S, FD, true, NotEncodedT);
       } else {
         for (const auto *Field : RDecl->fields()) {
           if (FD) {
             S += '"';
             S += Field->getNameAsString();
             S += '"';
           }
 
           // Special case bit-fields.
           if (Field->isBitField()) {
             getObjCEncodingForTypeImpl(Field->getType(), S,
-                                       /*ExpandPointedToStructures=*/false,
-                                       /*ExpandStructures=*/true, Field);
+                                       ObjCEncOptions().setExpandStructures(),
+                                       Field);
           } else {
             QualType qt = Field->getType();
             getLegacyIntegralTypeEncoding(qt);
             getObjCEncodingForTypeImpl(
-                qt, S, /*ExpandPointedToStructures=*/false,
-                /*ExpandStructures=*/true, FD, /*OutermostType=*/false,
-                /*EncodingProperty=*/false,
-                /*StructField=*/true, /*EncodeBlockParameters=*/false,
-                /*EncodeClassNames=*/false,
-                /*EncodePointerToObjCTypedef=*/false, NotEncodedT);
+                qt, S,
+                ObjCEncOptions().setExpandStructures().setIsStructField(), FD,
+                NotEncodedT);
           }
         }
       }
     }
     S += RDecl->isUnion() ? ')' : '}';
     return;
   }
 
   case Type::BlockPointer: {
     const auto *BT = T->castAs<BlockPointerType>();
     S += "@?"; // Unlike a pointer-to-function, which is "^?".
-    if (EncodeBlockParameters) {
+    if (Options.EncodeBlockParameters()) {
       const auto *FT = BT->getPointeeType()->castAs<FunctionType>();
 
       S += '<';
       // Block return type
-      getObjCEncodingForTypeImpl(
-          FT->getReturnType(), S, ExpandPointedToStructures, ExpandStructures,
-          FD, /*OutermostType=*/false, EncodingProperty,
-          /*StructField=*/false, EncodeBlockParameters, EncodeClassNames,
-          /*EncodePointerToObjCTypedef=*/false, NotEncodedT);
+      getObjCEncodingForTypeImpl(FT->getReturnType(), S,
+                                 Options.forComponentType(), FD, NotEncodedT);
       // Block self
       S += "@?";
       // Block parameters
       if (const auto *FPT = dyn_cast<FunctionProtoType>(FT)) {
         for (const auto &I : FPT->param_types())
-          getObjCEncodingForTypeImpl(
-              I, S, ExpandPointedToStructures, ExpandStructures, FD,
-              /*OutermostType=*/false, EncodingProperty,
-              /*StructField=*/false, EncodeBlockParameters, EncodeClassNames,
-              /*EncodePointerToObjCTypedef=*/false, NotEncodedT);
+          getObjCEncodingForTypeImpl(I, S, Options.forComponentType(), FD,
+                                     NotEncodedT);
       }
       S += '>';
     }
     return;
   }
 
   case Type::ObjCObject: {
     // hack to match legacy encoding of *id and *Class
void ASTContext::getObjCEncodingForTypeImpl(QualType T, std::string &S,
   }
 
   case Type::ObjCInterface: {
     // Ignore protocol qualifiers when mangling at this level.
     // @encode(class_name)
     ObjCInterfaceDecl *OI = T->castAs<ObjCObjectType>()->getInterface();
     S += '{';
     S += OI->getObjCRuntimeNameAsString();
-    if (ExpandStructures) {
+    if (Options.ExpandStructures()) {
       S += '=';
       SmallVector<const ObjCIvarDecl*, 32> Ivars;
       DeepCollectObjCIvars(OI, true, Ivars);
       for (unsigned i = 0, e = Ivars.size(); i != e; ++i) {
         const FieldDecl *Field = Ivars[i];
         if (Field->isBitField())
           getObjCEncodingForTypeImpl(Field->getType(), S,
-                                     /*ExpandPointedToStructures=*/false,
-                                     /*ExpandStructures=*/true, Field);
-        else
-          getObjCEncodingForTypeImpl(
-              Field->getType(), S,
-              /*ExpandPointedToStructures=*/false,
-              /*ExpandStructures=*/true, FD, /*OutermostType=*/false,
-              /*EncodingProperty=*/false,
-              /*StructField=*/false, /*EncodeBlockParameters=*/false,
-              /*EncodeClassNames=*/false, EncodePointerToObjCTypedef,
+                                     ObjCEncOptions().setExpandStructures(),
+                                     Field);
+        else {
+          ObjCEncOptions NewOptions = ObjCEncOptions().setExpandStructures();
+          if (Options.EncodePointerToObjCTypedef())
+            NewOptions.setEncodePointerToObjCTypedef();
+          getObjCEncodingForTypeImpl(Field->getType(), S, NewOptions, FD,
                                      NotEncodedT);
         }
       }
+    }
     S += '}';
     return;
   }
 
   case Type::ObjCObjectPointer: {
     const auto *OPT = T->castAs<ObjCObjectPointerType>();
     if (OPT->isObjCIdType()) {
       S += '@';
       return;
     }
 
     if (OPT->isObjCClassType() || OPT->isObjCQualifiedClassType()) {
       // FIXME: Consider if we need to output qualifiers for 'Class<p>'.
       // Since this is a binary compatibility issue, need to consult with
       // runtime folks. Fortunately, this is a *very* obscure construct.
       S += '#';
       return;
     }
 
     if (OPT->isObjCQualifiedIdType()) {
-      getObjCEncodingForTypeImpl(getObjCIdType(), S,
-                                 ExpandPointedToStructures,
-                                 ExpandStructures, FD);
-      if (FD || EncodingProperty || EncodeClassNames) {
+      getObjCEncodingForTypeImpl(
+          getObjCIdType(), S,
+          Options.keepingOnly(ObjCEncOptions()
+                                  .setExpandPointedToStructures()
+                                  .setExpandStructures()),
+          FD);
+      if (FD || Options.EncodingProperty() || Options.EncodeClassNames()) {
         // Note that we do extended encoding of protocol qualifer list
         // Only when doing ivar or property encoding.
         S += '"';
         for (const auto *I : OPT->quals()) {
           S += '<';
           S += I->getObjCRuntimeNameAsString();
           S += '>';
         }
         S += '"';
       }
       return;
     }
 
     QualType PointeeTy = OPT->getPointeeType();
-    if (!EncodingProperty &&
+    if (!Options.EncodingProperty() &&
         isa<TypedefType>(PointeeTy.getTypePtr()) &&
-        !EncodePointerToObjCTypedef) {
+        !Options.EncodePointerToObjCTypedef()) {
       // Another historical/compatibility reason.
       // We encode the underlying type which comes out as
       // {...};
       S += '^';
       if (FD && OPT->getInterfaceDecl()) {
         // Prevent recursive encoding of fields in some rare cases.
         ObjCInterfaceDecl *OI = OPT->getInterfaceDecl();
         SmallVector<const ObjCIvarDecl*, 32> Ivars;
         DeepCollectObjCIvars(OI, true, Ivars);
         for (unsigned i = 0, e = Ivars.size(); i != e; ++i) {
           if (Ivars[i] == FD) {
             S += '{';
             S += OI->getObjCRuntimeNameAsString();
             S += '}';
             return;
           }
         }
       }
-      getObjCEncodingForTypeImpl(
-          PointeeTy, S,
-          /*ExpandPointedToStructures=*/false,
-          /*ExpandStructures=*/ExpandPointedToStructures,
-          /*Field=*/nullptr,
-          /*OutermostType=*/false, /*EncodingProperty=*/false,
-          /*StructField=*/false, /*EncodeBlockParameters=*/false,
-          /*EncodeClassNames=*/false,
-          /*EncodePointerToObjCTypedef=*/true);
+      ObjCEncOptions NewOptions =
+          ObjCEncOptions().setEncodePointerToObjCTypedef();
+      if (Options.ExpandPointedToStructures())
+        NewOptions.setExpandStructures();
+      getObjCEncodingForTypeImpl(PointeeTy, S, NewOptions, /*Field=*/nullptr);
       return;
     }
 
     S += '@';
     if (OPT->getInterfaceDecl() &&
-        (FD || EncodingProperty || EncodeClassNames)) {
+        (FD || Options.EncodingProperty() || Options.EncodeClassNames())) {
       S += '"';
       S += OPT->getInterfaceDecl()->getObjCRuntimeNameAsString();
       for (const auto *I : OPT->quals()) {
         S += '<';
         S += I->getObjCRuntimeNameAsString();
         S += '>';
       }
       S += '"';
     }
     return;
   }
 
   // gcc just blithely ignores member pointers.
-  // FIXME: we shoul do better than that.  'M' is available.
+  // FIXME: we should do better than that.  'M' is available.
   case Type::MemberPointer:
   // This matches gcc's encoding, even though technically it is insufficient.
   //FIXME. We should do a better job than gcc.
   case Type::Vector:
   case Type::ExtVector:
   // Until we have a coherent encoding of these three types, issue warning.
     if (NotEncodedT)
       *NotEncodedT = T;
#endif
         EncodeBitField(this, S, field->getType(), field);
 #ifndef NDEBUG
         CurOffs += field->getBitWidthValue(*this);
 #endif
       } else {
         QualType qt = field->getType();
         getLegacyIntegralTypeEncoding(qt);
         getObjCEncodingForTypeImpl(
-            qt, S, /*ExpandPointedToStructures=*/false,
-            /*ExpandStructures=*/true, FD,
-            /*OutermostType=*/false,
-            /*EncodingProperty=*/false,
-            /*StructField=*/true, /*EncodeBlockParameters=*/false,
-            /*EncodeClassNames=*/false, /*EncodePointerToObjCTypedef=*/false,
-            NotEncodedT);
+            qt, S, ObjCEncOptions().setExpandStructures().setIsStructField(),
+            FD, NotEncodedT);
 #ifndef NDEBUG
         CurOffs += getTypeSize(field->getType());
 #endif
       }
     }
   }
 }