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

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

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

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

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

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

clang/include/clang/AST/ASTContext.h

   /// \param Target The target
   void InitBuiltinTypes(const TargetInfo &Target,
                         const TargetInfo *AuxTarget = nullptr);
 
 private:
   void InitBuiltinType(CanQualType &R, BuiltinType::Kind K);
 
   // Return the Objective-C type encoding for a given type.
-  void getObjCEncodingForTypeImpl(QualType t, std::string &S,
-                                  bool ExpandPointedToStructures,
-                                  bool ExpandStructures,
+  enum {
+    OCET_ExpandPointedToStructures = 1 << 0,
+    OCET_ExpandStructures = 1 << 1,
+    OCET_IsOutermostType = 1 << 2,
+    OCET_EncodingProperty = 1 << 3,
+    OCET_IsStructField = 1 << 4,
+    OCET_EncodeBlockParameters = 1 << 5,
+    OCET_EncodeClassNames = 1 << 6,
+    OCET_EncodePointerToObjCTypedef = 1 << 7,
+  };
+  void getObjCEncodingForTypeImpl(QualType t, std::string &S, unsigned 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:

clang/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);
+  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
 /// 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,
+                             OCET_ExpandPointedToStructures |
+                                 OCET_ExpandStructures | OCET_IsOutermostType,
+                             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,
+                             OCET_ExpandPointedToStructures |
+                                 OCET_ExpandStructures |
+                                 OCET_IsOutermostType | OCET_EncodingProperty,
+                             /*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:

       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,
+                                            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()) {
   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, 0, /*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, 0, /*Field=*/nullptr);
     return;
   }
 
   // encoding for pointer or reference types.
   case Type::Pointer:
   case Type::LValueReference:
   case Type::RValueReference: {
     QualType PointeeTy;

     }
 
     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 & OCET_IsOutermostType) && T.isConstQualified()) {
         isReadOnly = true;
         S += 'r';
       }
-    } else if (OutermostType) {
+    } else if (Options & OCET_IsOutermostType) {
       QualType P = PointeeTy;
       while (P->getAs<PointerType>())
         P = P->getAs<PointerType>()->getPointeeType();
       if (P.isConstQualified()) {
         isReadOnly = true;
         S += 'r';
       }
     }

         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);
+    getObjCEncodingForTypeImpl(PointeeTy, S,
+                               (Options & OCET_ExpandPointedToStructures)
+                                   ? OCET_ExpandStructures
+                                   : 0,
+                               /*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 & OCET_IsStructField)) {
       // Incomplete arrays are encoded as a pointer to the array element.
       S += '^';
 
       getObjCEncodingForTypeImpl(AT->getElementType(), S,
-                                 /*ExpandPointedToStructures=*/false,
-                                 ExpandStructures, FD);
+                                 Options & OCET_ExpandStructures, 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);
+      getObjCEncodingForTypeImpl(AT->getElementType(), S,
+                                 Options & OCET_ExpandStructures, FD,
+                                 NotEncodedT);
       S += ']';
     }
     return;
   }
 
   case Type::FunctionNoProto:
   case Type::FunctionProto:
     S += '?';

         const TemplateArgumentList &TemplateArgs = Spec->getTemplateArgs();
         llvm::raw_string_ostream OS(S);
         printTemplateArgumentList(OS, TemplateArgs.asArray(),
                                   getPrintingPolicy());
       }
     } else {
       S += '?';
     }
-    if (ExpandStructures) {
+    if (Options & OCET_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);
+                                       OCET_ExpandStructures, 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, OCET_ExpandStructures | OCET_IsStructField, 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 & OCET_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 & ~(OCET_IsOutermostType |
+                                             OCET_IsStructField |
+                                             OCET_EncodePointerToObjCTypedef),
+                                 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);
+              I, S,
+              Options & ~(OCET_IsOutermostType | OCET_IsStructField |
+                          OCET_EncodePointerToObjCTypedef),
+              FD, NotEncodedT);
       }
       S += '>';
     }
     return;
   }
 
   case Type::ObjCObject: {
     // hack to match legacy encoding of *id and *Class
     QualType Ty = getObjCObjectPointerType(CT);
     if (Ty->isObjCIdType()) {
       S += "{objc_object=}";
       return;
     }
     else if (Ty->isObjCClassType()) {
       S += "{objc_class=}";
       return;
     }
     // TODO: Double check to make sure this intentionally falls through.
     LLVM_FALLTHROUGH;
   }
 
   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 & OCET_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);
+                                     OCET_ExpandStructures, Field);
         else
           getObjCEncodingForTypeImpl(
               Field->getType(), S,
-              /*ExpandPointedToStructures=*/false,
-              /*ExpandStructures=*/true, FD, /*OutermostType=*/false,
-              /*EncodingProperty=*/false,
-              /*StructField=*/false, /*EncodeBlockParameters=*/false,
-              /*EncodeClassNames=*/false, EncodePointerToObjCTypedef,
-              NotEncodedT);
+              OCET_ExpandStructures |
+                  (Options & OCET_EncodePointerToObjCTypedef),
+              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 & (OCET_ExpandPointedToStructures | OCET_ExpandStructures),
+          FD);
+      if (FD || (Options & (OCET_EncodingProperty | OCET_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 &&
-        isa<TypedefType>(PointeeTy.getTypePtr()) &&
-        !EncodePointerToObjCTypedef) {
+    if (!(Options &
+          (OCET_EncodingProperty | OCET_EncodePointerToObjCTypedef)) &&
+        isa<TypedefType>(PointeeTy.getTypePtr())) {
       // 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);
+          OCET_EncodePointerToObjCTypedef |
+              ((Options & OCET_ExpandPointedToStructures)
+                   ? OCET_ExpandStructures
+                   : 0),
+          /*Field=*/nullptr);
       return;
     }
 
     S += '@';
     if (OPT->getInterfaceDecl() &&
-        (FD || EncodingProperty || EncodeClassNames)) {
+        (FD || (Options & (OCET_EncodingProperty | OCET_EncodeClassNames)))) {
       S += '"';
       S += OPT->getInterfaceDecl()->getObjCRuntimeNameAsString();
       for (const auto *I : OPT->quals()) {
         S += '<';
         S += I->getObjCRuntimeNameAsString();
         S += '>';
       }
       S += '"';

       if (field->isBitField()) {
         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);
+        getObjCEncodingForTypeImpl(qt, S,
+                                   OCET_ExpandStructures | OCET_IsStructField,
+                                   FD, NotEncodedT);
 #ifndef NDEBUG
         CurOffs += getTypeSize(field->getType());
 #endif
       }
     }
   }
 }