Landed in r165764-r165770. -- Sean Silva
On Wed, Oct 10, 2012 at 6:10 PM, Chris Lattner <[email protected]> wrote: > > On Oct 10, 2012, at 2:53 PM, Sean Silva <[email protected]> wrote: > >> Ping. >> >> Chris, could you take a look at this? > > I don't think I have any special value to be added here. Some thoughts > though: I think it *does* make sense to allow cast<>'s that obviously > constant fold to true or false from static type information, this does help > generic code. Also, the bugs that disallowing it would find are pretty easy > to find through other means. > > The patches look fine to me. I'm not hugely thrilled with Casting.h pulling > in type_traits to get enable_if, but I don't have a better idea. :) Please > commit, > > -Chris > >> >> -- Sean Silva >> >> On Sun, Oct 7, 2012 at 5:04 PM, Sean Silva <[email protected]> wrote: >>> This patch set implements functionality in Casting.h that will >>> automatically detect upcasts and optimize away dynamic checks for >>> them. It also simplifies implementing LLVM-style RTTI since now one >>> fewer classof() is required. >>> >>> The core of the patch set is 0002, which introduces the specialization >>> of isa_impl<>. I would appreciate if someone doublechecks my use of >>> templates there. >>> >>> Patches 0001-0005 are for LLVM, 0006-0007 are for Clang. I've been >>> running `check` and `check-clang` throughout development, except where >>> the refactoring turned up existing bugs (that I fixed an made their >>> own patch and put before the refactoring). >>> >>> Chris, I did manage to find some classof() of the form: >>> static bool classof(const Foo *) { return true; } >>> where it is not done to optimize the upcast. In particular, there are >>> two in Operator.h which basically bless using cast<> to convert >>> Instruction's and ConstantExpr's to Operator (this seems to lead to UB >>> actually; I started a thread on LLVMdev about it). >>> >>> I also found quite a bit of cargo-cult'ing around classof. There were >>> many things like: >>> >>> class Base { >>> [...] >>> static bool classof(const Base *) { return true; } >>> static bool classof(const Derived *) { return true; } >>> static bool classof(const OtherDerived *) { return true; } >>> }; >>> >>> When the single Base one would suffice. After this patch set none of >>> these three are necessary anymore :) >>> >>> >>> Patch descriptions: >>> >>> 0001-Remove-buggy-classof.patch >>> This patch removes a buggy classof that has laid dormant since r55022. >>> >>> 0002-Casting.h-Automatically-handle-isa-Base-Derived.patch >>> This is the core of the patch set. It adds a specialization of >>> isa_impl<> that automatically allows upcasts. Includes two unittests >>> of the new functionality. The first checks that the upcast is >>> automatically inferred, and the second one checks that the mechanism >>> does not fall back to an explicit check when the cast is statically >>> known to be OK (i.e., that the dynamic check is optimized away). >>> >>> 0003-docs-Update-HowToSetUpLLVMStyleRTTI.patch >>> Documentation update to reflect changes in the previous patch. >>> >>> 0004-Remove-unnecessary-classof-s.patch >>> A mass removal of now-unnecessary classof()'s. >>> >>> 0005-docs-Improve-HowToSetUpLLVMStyleRTTI.patch >>> More documentation improvements regarding LLVM-style RTTI. >>> >>> 0006-Add-missing-classof.patch: >>> Bugfix. Adds a missing classof that manifested itself by triggering a >>> build failure when the next patch is applied. This bug is actually >>> kind of scary, since what was happening is that the check >>> To::classof(&Val) would use the parent class's classof(), yielding a >>> completely wrong dynamic check! I'm pretty much positive that there >>> are other instances of this bug somewhere in the codebase. I can't >>> think of a way to have the compiler enforce that a parent's classof() >>> isn't used though. >>> >>> 0007-Remove-pointless-classof-s.patch >>> Mass removal of useless classof()'s. >>> >>> Comments and review welcome. >>> >>> -- Sean Silva >> _______________________________________________ >> cfe-commits mailing list >> [email protected] >> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits > _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
