On Fri, Nov 14, 2014 at 7:55 AM, Aaron Ballman <[email protected]> wrote:
> On Thu, Nov 13, 2014 at 10:06 PM, Richard Smith <[email protected]> > wrote: > > + std::list<QualType> BaseClassTypes; > > > > This will allocate memory for every node; it would be better to use a > > SmallVector here (and use a depth-first search rather than a > breadth-first > > one so you're only modifying one end of your list). > > > > + for (auto &CurType : BaseClassTypes) { > > > > It's a bit scary to be iterating over your container while modifying it > > (even though it's actually correct in this case). Alternative idiom: > > > > SmallVector<QualType, 8> BaseClassTypes; > > BaseClassTypes.push_back(...); > > while (!BaseClassTypes.empty()) { > > QualType T = BaseClassTypes.pop_back_val(); > > // ... maybe push back some values. > > } > > I have a strong preference for your idiom. ;-) > > > > > + auto I = std::find_if( > > + HandledTypes.begin(), HandledTypes.end(), > > > > This is still quadratic-time; maybe use a DenseMap with the canonical > > QualType plus a "was it a pointer" flag as a key? > > I've given this a shot in my latest patch. Thank you for the feedback! + unsigned IsReference : 1; I think we don't need / want an IsReference flag. Consider: try { throw Derived(); } catch (Base) {} catch (Derived &d) {} The second catch handler is unreachable even though we have a by reference/by value mismatch. + QualType Underlying = CurType.underlying(); + if (auto *RD = Underlying->getAsCXXRecordDecl()) { + for (const auto &B : RD->bases()) + BaseClassTypes.push_back(QualTypeExt(B.getType(), CurType.isPointer(), + CurType.isReference())); Per [except.handle]p1, you should only consider public, unambiguous base classes here. Rather than walk the base classes yourself, you could use CXXRecordDecl::lookupInBases to enumerate the base classes and paths, and then check the public unambiguous ones.
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
