As noted in bug 96980, TypeTraits.h could use a little cleaning up.
Created attachment 165921 [details] Patch
Created attachment 166019 [details] Patch
Created attachment 166024 [details] Patch
Comment on attachment 166024 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=166024&action=review > Source/WTF/ChangeLog:3 > + Cleanup HasTrivialConstructor/Destructor The verb “clean up” is two words. > Source/WTF/wtf/TypeTraits.h:244 > // VC8 (VS2005) and later have built-in compiler support for HasTrivialConstructor / HasTrivialDestructor, I don’t think it’s right to say they have built-in compiler support for HasTrivialConstructor / HasTrivialDestructor. The comment should say: // VC8 (VS2005) and later has __has_trivial_constructor and __has_trivial_destructor, but the implementation // incorrectly returns false for built-in types. Work around that by explicitly checking IsPod. Also, if we wanted to cut down on copies of these templates, we could consider merging this case with the clang case. I see no harm in including the Visual Studio workaround even in the compilers that don’t need it. > Source/WTF/wtf/TypeTraits.h:245 > + // but for some unexplained reason it doesn't work on built-in types. We add the extra IsPod condition to I don’t think the phrase “for some unexplained reason” added anything to the old comment and adds nothing to the new either. It would be nice if the comment said something more specific than “it doesn’t work on” like “it returns false for”. > Source/WTF/wtf/TypeTraits.h:255 > + // For platforms that don't support HasTrivialConstructor/HasTrivialDestructor at all, we want to at least provide > + // some semblance of support. I think the comment should be more specific. I would have written something more like this: // For compilers that don't support detection of trivial constructors and destructors in classes, use a template // that returns true for any POD type but false for all class types. This will give false negatives, which can // hurt performance, but avoids false positives, which can result in incorrect behavior.
Committed r129780: <http://trac.webkit.org/changeset/129780>
(In reply to comment #4) > Also, if we wanted to cut down on copies of these templates, we could consider merging this case with the clang case. I see no harm in including the Visual Studio workaround even in the compilers that don’t need it. Oops, my suggestion probably broke the VC10 case. It’s supposed to use std::tr1::has_trivial_constructor, right?
> Oops, my suggestion probably broke the VC10 case. It’s supposed to use std::tr1::has_trivial_constructor, right? Argh, I believe so. Reopening...
Created attachment 166095 [details] Patch
Comment on attachment 166095 [details] Patch Clearing flags on attachment: 166095 Committed r129999: <http://trac.webkit.org/changeset/129999>
All reviewed patches have been landed. Closing bug.