Reduce the number of indirections in the attributes implementation.
authorBenjamin Kramer <benny.kra@googlemail.com>
Thu, 11 Jul 2013 12:13:16 +0000 (12:13 +0000)
committerBenjamin Kramer <benny.kra@googlemail.com>
Thu, 11 Jul 2013 12:13:16 +0000 (12:13 +0000)
- Coallocate entires for AttributeSetImpls and Nodes after the class itself.
- Remove mutable iterators from immutable classes.
- Remove unused context field from AttributeImpl.
- Derive Enum/Align/String attribute implementations from AttributeImpl instead
  of having a whole new inheritance tree for them.
- Derive AlignAttributeImpl from EnumAttributeImpl.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@186075 91177308-0d34-0410-b5e6-96231b3b80d8

lib/IR/AttributeImpl.h
lib/IR/Attributes.cpp

index 0b6228b331ebbca1a79c77915aca97bc68f59c0b..5a72c37505e1769e25ad4cbc91f579b385007430 100644 (file)
@@ -27,97 +27,30 @@ class LLVMContext;
 
 //===----------------------------------------------------------------------===//
 /// \class
-/// \brief A set of classes that contain the kind and (optional) value of the
-/// attribute object. There are three main categories: enum attribute entries,
-/// represented by Attribute::AttrKind; alignment attribute entries; and string
-/// attribute enties, which are for target-dependent attributes.
-class AttributeEntry {
-  unsigned char KindID;
+/// \brief This class represents a single, uniqued attribute. That attribute
+/// could be a single enum, a tuple, or a string.
+class AttributeImpl : public FoldingSetNode {
+  unsigned char KindID; ///< Holds the AttrEntryKind of the attribute
+
+  // AttributesImpl is uniqued, these should not be publicly available.
+  void operator=(const AttributeImpl &) LLVM_DELETED_FUNCTION;
+  AttributeImpl(const AttributeImpl &) LLVM_DELETED_FUNCTION;
+
 protected:
   enum AttrEntryKind {
     EnumAttrEntry,
     AlignAttrEntry,
     StringAttrEntry
   };
-public:
-  AttributeEntry(AttrEntryKind Kind)
-    : KindID(Kind) {}
-  virtual ~AttributeEntry() {}
-
-  unsigned getKindID() const { return KindID; }
-
-  static inline bool classof(const AttributeEntry *) { return true; }
-};
-
-class EnumAttributeEntry : public AttributeEntry {
-  Attribute::AttrKind Kind;
-public:
-  EnumAttributeEntry(Attribute::AttrKind Kind)
-    : AttributeEntry(EnumAttrEntry), Kind(Kind) {}
-
-  Attribute::AttrKind getEnumKind() const { return Kind; }
-
-  static inline bool classof(const AttributeEntry *AE) {
-    return AE->getKindID() == EnumAttrEntry;
-  }
-  static inline bool classof(const EnumAttributeEntry *) { return true; }
-};
-
-class AlignAttributeEntry : public AttributeEntry {
-  Attribute::AttrKind Kind;
-  unsigned Align;
-public:
-  AlignAttributeEntry(Attribute::AttrKind Kind, unsigned Align)
-    : AttributeEntry(AlignAttrEntry), Kind(Kind), Align(Align) {}
-
-  Attribute::AttrKind getEnumKind() const { return Kind; }
-  unsigned getAlignment() const { return Align; }
-
-  static inline bool classof(const AttributeEntry *AE) {
-    return AE->getKindID() == AlignAttrEntry;
-  }
-  static inline bool classof(const AlignAttributeEntry *) { return true; }
-};
-
-class StringAttributeEntry : public AttributeEntry {
-  std::string Kind;
-  std::string Val;
-public:
-  StringAttributeEntry(StringRef Kind, StringRef Val = StringRef())
-    : AttributeEntry(StringAttrEntry), Kind(Kind), Val(Val) {}
-
-  StringRef getStringKind() const { return Kind; }
-  StringRef getStringValue() const { return Val; }
-
-  static inline bool classof(const AttributeEntry *AE) {
-    return AE->getKindID() == StringAttrEntry;
-  }
-  static inline bool classof(const StringAttributeEntry *) { return true; }
-};
-
-//===----------------------------------------------------------------------===//
-/// \class
-/// \brief This class represents a single, uniqued attribute. That attribute
-/// could be a single enum, a tuple, or a string.
-class AttributeImpl : public FoldingSetNode {
-  LLVMContext &Context;  ///< Global context for uniquing objects
 
-  AttributeEntry *Entry; ///< Holds the kind and value of the attribute
+  AttributeImpl(AttrEntryKind KindID) : KindID(KindID) {}
 
-  // AttributesImpl is uniqued, these should not be publicly available.
-  void operator=(const AttributeImpl &) LLVM_DELETED_FUNCTION;
-  AttributeImpl(const AttributeImpl &) LLVM_DELETED_FUNCTION;
 public:
-  AttributeImpl(LLVMContext &C, Attribute::AttrKind Kind);
-  AttributeImpl(LLVMContext &C, Attribute::AttrKind Kind, unsigned Align);
-  AttributeImpl(LLVMContext &C, StringRef Kind, StringRef Val = StringRef());
-  ~AttributeImpl();
-
-  LLVMContext &getContext() { return Context; }
+  virtual ~AttributeImpl();
 
-  bool isEnumAttribute() const;
-  bool isAlignAttribute() const;
-  bool isStringAttribute() const;
+  bool isEnumAttribute() const { return KindID == EnumAttrEntry; }
+  bool isAlignAttribute() const { return KindID == AlignAttrEntry; }
+  bool isStringAttribute() const { return KindID == StringAttrEntry; }
 
   bool hasAttribute(Attribute::AttrKind A) const;
   bool hasAttribute(StringRef Kind) const;
@@ -153,15 +86,65 @@ public:
   static uint64_t getAttrMask(Attribute::AttrKind Val);
 };
 
+//===----------------------------------------------------------------------===//
+/// \class
+/// \brief A set of classes that contain the value of the
+/// attribute object. There are three main categories: enum attribute entries,
+/// represented by Attribute::AttrKind; alignment attribute entries; and string
+/// attribute enties, which are for target-dependent attributes.
+
+class EnumAttributeImpl : public AttributeImpl {
+  Attribute::AttrKind Kind;
+
+protected:
+  EnumAttributeImpl(AttrEntryKind ID, Attribute::AttrKind Kind)
+      : AttributeImpl(ID), Kind(Kind) {}
+
+public:
+  EnumAttributeImpl(Attribute::AttrKind Kind)
+      : AttributeImpl(EnumAttrEntry), Kind(Kind) {}
+
+  Attribute::AttrKind getEnumKind() const { return Kind; }
+};
+
+class AlignAttributeImpl : public EnumAttributeImpl {
+  unsigned Align;
+
+public:
+  AlignAttributeImpl(Attribute::AttrKind Kind, unsigned Align)
+      : EnumAttributeImpl(AlignAttrEntry, Kind), Align(Align) {
+    assert(
+        (Kind == Attribute::Alignment || Kind == Attribute::StackAlignment) &&
+        "Wrong kind for alignment attribute!");
+  }
+
+  unsigned getAlignment() const { return Align; }
+};
+
+class StringAttributeImpl : public AttributeImpl {
+  std::string Kind;
+  std::string Val;
+
+public:
+  StringAttributeImpl(StringRef Kind, StringRef Val = StringRef())
+      : AttributeImpl(StringAttrEntry), Kind(Kind), Val(Val) {}
+
+  StringRef getStringKind() const { return Kind; }
+  StringRef getStringValue() const { return Val; }
+};
+
 //===----------------------------------------------------------------------===//
 /// \class
 /// \brief This class represents a group of attributes that apply to one
 /// element: function, return type, or parameter.
 class AttributeSetNode : public FoldingSetNode {
-  SmallVector<Attribute, 4> AttrList;
+  unsigned NumAttrs; ///< Number of attributes in this node.
 
-  AttributeSetNode(ArrayRef<Attribute> Attrs)
-    : AttrList(Attrs.begin(), Attrs.end()) {}
+  AttributeSetNode(ArrayRef<Attribute> Attrs) : NumAttrs(Attrs.size()) {
+    // There's memory after the node where we can store the entries in.
+    std::copy(Attrs.begin(), Attrs.end(),
+              reinterpret_cast<Attribute *>(this + 1));
+  }
 
   // AttributesSetNode is uniqued, these should not be publicly available.
   void operator=(const AttributeSetNode &) LLVM_DELETED_FUNCTION;
@@ -171,7 +154,7 @@ public:
 
   bool hasAttribute(Attribute::AttrKind Kind) const;
   bool hasAttribute(StringRef Kind) const;
-  bool hasAttributes() const { return !AttrList.empty(); }
+  bool hasAttributes() const { return NumAttrs != 0; }
 
   Attribute getAttribute(Attribute::AttrKind Kind) const;
   Attribute getAttribute(StringRef Kind) const;
@@ -180,17 +163,12 @@ public:
   unsigned getStackAlignment() const;
   std::string getAsString(bool InAttrGrp) const;
 
-  typedef SmallVectorImpl<Attribute>::iterator       iterator;
-  typedef SmallVectorImpl<Attribute>::const_iterator const_iterator;
-
-  iterator begin() { return AttrList.begin(); }
-  iterator end()   { return AttrList.end(); }
-
-  const_iterator begin() const { return AttrList.begin(); }
-  const_iterator end() const   { return AttrList.end(); }
+  typedef const Attribute *iterator;
+  iterator begin() const { return reinterpret_cast<iterator>(this + 1); }
+  iterator end() const { return begin() + NumAttrs; }
 
   void Profile(FoldingSetNodeID &ID) const {
-    Profile(ID, AttrList);
+    Profile(ID, makeArrayRef(begin(), end()));
   }
   static void Profile(FoldingSetNodeID &ID, ArrayRef<Attribute> AttrList) {
     for (unsigned I = 0, E = AttrList.size(); I != E; ++I)
@@ -208,58 +186,58 @@ class AttributeSetImpl : public FoldingSetNode {
   LLVMContext &Context;
 
   typedef std::pair<unsigned, AttributeSetNode*> IndexAttrPair;
-  SmallVector<IndexAttrPair, 4> AttrNodes;
+  unsigned NumAttrs; ///< Number of entries in this set.
+
+  /// \brief Return a pointer to the IndexAttrPair for the specified slot.
+  const IndexAttrPair *getNode(unsigned Slot) const {
+    return reinterpret_cast<const IndexAttrPair *>(this + 1) + Slot;
+  }
 
   // AttributesSet is uniqued, these should not be publicly available.
   void operator=(const AttributeSetImpl &) LLVM_DELETED_FUNCTION;
   AttributeSetImpl(const AttributeSetImpl &) LLVM_DELETED_FUNCTION;
 public:
   AttributeSetImpl(LLVMContext &C,
-                   ArrayRef<std::pair<unsigned, AttributeSetNode*> > attrs)
-    : Context(C), AttrNodes(attrs.begin(), attrs.end()) {}
+                   ArrayRef<std::pair<unsigned, AttributeSetNode *> > Attrs)
+      : Context(C), NumAttrs(Attrs.size()) {
+    // There's memory after the node where we can store the entries in.
+    std::copy(Attrs.begin(), Attrs.end(),
+              reinterpret_cast<IndexAttrPair *>(this + 1));
+  }
 
   /// \brief Get the context that created this AttributeSetImpl.
   LLVMContext &getContext() { return Context; }
 
   /// \brief Return the number of attributes this AttributeSet contains.
-  unsigned getNumAttributes() const { return AttrNodes.size(); }
+  unsigned getNumAttributes() const { return NumAttrs; }
 
   /// \brief Get the index of the given "slot" in the AttrNodes list. This index
   /// is the index of the return, parameter, or function object that the
   /// attributes are applied to, not the index into the AttrNodes list where the
   /// attributes reside.
   unsigned getSlotIndex(unsigned Slot) const {
-    return AttrNodes[Slot].first;
+    return getNode(Slot)->first;
   }
 
   /// \brief Retrieve the attributes for the given "slot" in the AttrNode list.
   /// \p Slot is an index into the AttrNodes list, not the index of the return /
   /// parameter/ function which the attributes apply to.
   AttributeSet getSlotAttributes(unsigned Slot) const {
-    return AttributeSet::get(Context, AttrNodes[Slot]);
+    return AttributeSet::get(Context, *getNode(Slot));
   }
 
   /// \brief Retrieve the attribute set node for the given "slot" in the
   /// AttrNode list.
   AttributeSetNode *getSlotNode(unsigned Slot) const {
-    return AttrNodes[Slot].second;
+    return getNode(Slot)->second;
   }
 
-  typedef AttributeSetNode::iterator       iterator;
-  typedef AttributeSetNode::const_iterator const_iterator;
-
-  iterator begin(unsigned Slot)
-    { return AttrNodes[Slot].second->begin(); }
-  iterator end(unsigned Slot)
-    { return AttrNodes[Slot].second->end(); }
-
-  const_iterator begin(unsigned Slot) const
-    { return AttrNodes[Slot].second->begin(); }
-  const_iterator end(unsigned Slot) const
-    { return AttrNodes[Slot].second->end(); }
+  typedef AttributeSetNode::iterator iterator;
+  iterator begin(unsigned Slot) const { return getSlotNode(Slot)->begin(); }
+  iterator end(unsigned Slot) const { return getSlotNode(Slot)->end(); }
 
   void Profile(FoldingSetNodeID &ID) const {
-    Profile(ID, AttrNodes);
+    Profile(ID, makeArrayRef(getNode(0), getNumAttributes()));
   }
   static void Profile(FoldingSetNodeID &ID,
                       ArrayRef<std::pair<unsigned, AttributeSetNode*> > Nodes) {
index 59da815663dadc98cdf21605964754671c60e89e..c8e2f6be5c0e4803c3e5c2d6cf3f42314236d999 100644 (file)
@@ -43,9 +43,10 @@ Attribute Attribute::get(LLVMContext &Context, Attribute::AttrKind Kind,
   if (!PA) {
     // If we didn't find any existing attributes of the same shape then create a
     // new one and insert it.
-    PA = !Val ?
-      new AttributeImpl(Context, Kind) :
-      new AttributeImpl(Context, Kind, Val);
+    if (!Val)
+      PA = new EnumAttributeImpl(Kind);
+    else
+      PA = new AlignAttributeImpl(Kind, Val);
     pImpl->AttrsSet.InsertNode(PA, InsertPoint);
   }
 
@@ -65,7 +66,7 @@ Attribute Attribute::get(LLVMContext &Context, StringRef Kind, StringRef Val) {
   if (!PA) {
     // If we didn't find any existing attributes of the same shape then create a
     // new one and insert it.
-    PA = new AttributeImpl(Context, Kind, Val);
+    PA = new StringAttributeImpl(Kind, Val);
     pImpl->AttrsSet.InsertNode(PA, InsertPoint);
   }
 
@@ -279,35 +280,7 @@ bool Attribute::operator<(Attribute A) const {
 // AttributeImpl Definition
 //===----------------------------------------------------------------------===//
 
-AttributeImpl::AttributeImpl(LLVMContext &C, Attribute::AttrKind Kind)
-  : Context(C), Entry(new EnumAttributeEntry(Kind)) {}
-
-AttributeImpl::AttributeImpl(LLVMContext &C, Attribute::AttrKind Kind,
-                             unsigned Align)
-  : Context(C) {
-  assert((Kind == Attribute::Alignment || Kind == Attribute::StackAlignment) &&
-         "Wrong kind for alignment attribute!");
-  Entry = new AlignAttributeEntry(Kind, Align);
-}
-
-AttributeImpl::AttributeImpl(LLVMContext &C, StringRef Kind, StringRef Val)
-  : Context(C), Entry(new StringAttributeEntry(Kind, Val)) {}
-
-AttributeImpl::~AttributeImpl() {
-  delete Entry;
-}
-
-bool AttributeImpl::isEnumAttribute() const {
-  return isa<EnumAttributeEntry>(Entry);
-}
-
-bool AttributeImpl::isAlignAttribute() const {
-  return isa<AlignAttributeEntry>(Entry);
-}
-
-bool AttributeImpl::isStringAttribute() const {
-  return isa<StringAttributeEntry>(Entry);
-}
+AttributeImpl::~AttributeImpl() {}
 
 bool AttributeImpl::hasAttribute(Attribute::AttrKind A) const {
   if (isStringAttribute()) return false;
@@ -320,21 +293,23 @@ bool AttributeImpl::hasAttribute(StringRef Kind) const {
 }
 
 Attribute::AttrKind AttributeImpl::getKindAsEnum() const {
-  if (EnumAttributeEntry *E = dyn_cast<EnumAttributeEntry>(Entry))
-    return E->getEnumKind();
-  return cast<AlignAttributeEntry>(Entry)->getEnumKind();
+  assert(isEnumAttribute() || isAlignAttribute());
+  return static_cast<const EnumAttributeImpl *>(this)->getEnumKind();
 }
 
 uint64_t AttributeImpl::getValueAsInt() const {
-  return cast<AlignAttributeEntry>(Entry)->getAlignment();
+  assert(isAlignAttribute());
+  return static_cast<const AlignAttributeImpl *>(this)->getAlignment();
 }
 
 StringRef AttributeImpl::getKindAsString() const {
-  return cast<StringAttributeEntry>(Entry)->getStringKind();
+  assert(isStringAttribute());
+  return static_cast<const StringAttributeImpl *>(this)->getStringKind();
 }
 
 StringRef AttributeImpl::getValueAsString() const {
-  return cast<StringAttributeEntry>(Entry)->getStringValue();
+  assert(isStringAttribute());
+  return static_cast<const StringAttributeImpl *>(this)->getStringValue();
 }
 
 bool AttributeImpl::operator<(const AttributeImpl &AI) const {
@@ -433,7 +408,10 @@ AttributeSetNode *AttributeSetNode::get(LLVMContext &C,
   // If we didn't find any existing attributes of the same shape then create a
   // new one and insert it.
   if (!PA) {
-    PA = new AttributeSetNode(SortedAttrs);
+    // Coallocate entries after the AttributeSetNode itself.
+    void *Mem = ::operator new(sizeof(AttributeSetNode) +
+                               sizeof(Attribute) * SortedAttrs.size());
+    PA = new (Mem) AttributeSetNode(SortedAttrs);
     pImpl->AttrsSetNodes.InsertNode(PA, InsertPoint);
   }
 
@@ -442,48 +420,42 @@ AttributeSetNode *AttributeSetNode::get(LLVMContext &C,
 }
 
 bool AttributeSetNode::hasAttribute(Attribute::AttrKind Kind) const {
-  for (SmallVectorImpl<Attribute>::const_iterator I = AttrList.begin(),
-         E = AttrList.end(); I != E; ++I)
+  for (iterator I = begin(), E = end(); I != E; ++I)
     if (I->hasAttribute(Kind))
       return true;
   return false;
 }
 
 bool AttributeSetNode::hasAttribute(StringRef Kind) const {
-  for (SmallVectorImpl<Attribute>::const_iterator I = AttrList.begin(),
-         E = AttrList.end(); I != E; ++I)
+  for (iterator I = begin(), E = end(); I != E; ++I)
     if (I->hasAttribute(Kind))
       return true;
   return false;
 }
 
 Attribute AttributeSetNode::getAttribute(Attribute::AttrKind Kind) const {
-  for (SmallVectorImpl<Attribute>::const_iterator I = AttrList.begin(),
-         E = AttrList.end(); I != E; ++I)
+  for (iterator I = begin(), E = end(); I != E; ++I)
     if (I->hasAttribute(Kind))
       return *I;
   return Attribute();
 }
 
 Attribute AttributeSetNode::getAttribute(StringRef Kind) const {
-  for (SmallVectorImpl<Attribute>::const_iterator I = AttrList.begin(),
-         E = AttrList.end(); I != E; ++I)
+  for (iterator I = begin(), E = end(); I != E; ++I)
     if (I->hasAttribute(Kind))
       return *I;
   return Attribute();
 }
 
 unsigned AttributeSetNode::getAlignment() const {
-  for (SmallVectorImpl<Attribute>::const_iterator I = AttrList.begin(),
-         E = AttrList.end(); I != E; ++I)
+  for (iterator I = begin(), E = end(); I != E; ++I)
     if (I->hasAttribute(Attribute::Alignment))
       return I->getAlignment();
   return 0;
 }
 
 unsigned AttributeSetNode::getStackAlignment() const {
-  for (SmallVectorImpl<Attribute>::const_iterator I = AttrList.begin(),
-         E = AttrList.end(); I != E; ++I)
+  for (iterator I = begin(), E = end(); I != E; ++I)
     if (I->hasAttribute(Attribute::StackAlignment))
       return I->getStackAlignment();
   return 0;
@@ -491,9 +463,8 @@ unsigned AttributeSetNode::getStackAlignment() const {
 
 std::string AttributeSetNode::getAsString(bool InAttrGrp) const {
   std::string Str;
-  for (SmallVectorImpl<Attribute>::const_iterator I = AttrList.begin(),
-         E = AttrList.end(); I != E; ++I) {
-    if (I != AttrList.begin())
+  for (iterator I = begin(), E = end(); I != E; ++I) {
+    if (I != begin())
       Str += ' ';
     Str += I->getAsString(InAttrGrp);
   }
@@ -507,10 +478,10 @@ std::string AttributeSetNode::getAsString(bool InAttrGrp) const {
 uint64_t AttributeSetImpl::Raw(unsigned Index) const {
   for (unsigned I = 0, E = getNumAttributes(); I != E; ++I) {
     if (getSlotIndex(I) != Index) continue;
-    const AttributeSetNode *ASN = AttrNodes[I].second;
+    const AttributeSetNode *ASN = getSlotNode(I);
     uint64_t Mask = 0;
 
-    for (AttributeSetNode::const_iterator II = ASN->begin(),
+    for (AttributeSetNode::iterator II = ASN->begin(),
            IE = ASN->end(); II != IE; ++II) {
       Attribute Attr = *II;
 
@@ -550,7 +521,11 @@ AttributeSet::getImpl(LLVMContext &C,
   // If we didn't find any existing attributes of the same shape then
   // create a new one and insert it.
   if (!PA) {
-    PA = new AttributeSetImpl(C, Attrs);
+    // Coallocate entries after the AttributeSetImpl itself.
+    void *Mem = ::operator new(sizeof(AttributeSetImpl) +
+                               sizeof(std::pair<unsigned, AttributeSetNode *>) *
+                                   Attrs.size());
+    PA = new (Mem) AttributeSetImpl(C, Attrs);
     pImpl->AttrsLists.InsertNode(PA, InsertPoint);
   }
 
@@ -645,9 +620,9 @@ AttributeSet AttributeSet::get(LLVMContext &C, ArrayRef<AttributeSet> Attrs) {
 
   SmallVector<std::pair<unsigned, AttributeSetNode*>, 8> AttrNodeVec;
   for (unsigned I = 0, E = Attrs.size(); I != E; ++I) {
-    AttributeSet AS = Attrs[I];
-    if (!AS.pImpl) continue;
-    AttrNodeVec.append(AS.pImpl->AttrNodes.begin(), AS.pImpl->AttrNodes.end());
+    AttributeSetImpl *AS = Attrs[I].pImpl;
+    if (!AS) continue;
+    AttrNodeVec.append(AS->getNode(0), AS->getNode(AS->getNumAttributes()));
   }
 
   return getImpl(C, AttrNodeVec);
@@ -700,7 +675,7 @@ AttributeSet AttributeSet::addAttributes(LLVMContext &C, unsigned Index,
 
   for (unsigned I = 0, E = Attrs.pImpl->getNumAttributes(); I != E; ++I)
     if (Attrs.getSlotIndex(I) == Index) {
-      for (AttributeSetImpl::const_iterator II = Attrs.pImpl->begin(I),
+      for (AttributeSetImpl::iterator II = Attrs.pImpl->begin(I),
              IE = Attrs.pImpl->end(I); II != IE; ++II)
         B.addAttribute(*II);
       break;
@@ -821,7 +796,7 @@ bool AttributeSet::hasAttrSomewhere(Attribute::AttrKind Attr) const {
   if (pImpl == 0) return false;
 
   for (unsigned I = 0, E = pImpl->getNumAttributes(); I != E; ++I)
-    for (AttributeSetImpl::const_iterator II = pImpl->begin(I),
+    for (AttributeSetImpl::iterator II = pImpl->begin(I),
            IE = pImpl->end(I); II != IE; ++II)
       if (II->hasAttribute(Attr))
         return true;
@@ -937,7 +912,7 @@ AttrBuilder::AttrBuilder(AttributeSet AS, unsigned Index)
   for (unsigned I = 0, E = pImpl->getNumAttributes(); I != E; ++I) {
     if (pImpl->getSlotIndex(I) != Index) continue;
 
-    for (AttributeSetImpl::const_iterator II = pImpl->begin(I),
+    for (AttributeSetImpl::iterator II = pImpl->begin(I),
            IE = pImpl->end(I); II != IE; ++II)
       addAttribute(*II);