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!

~Aaron

Attachment: CatchHandlerChecker_v5.patch
Description: Binary data

_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to