[cfe-commits] Casting.h: Automatically detect isa<Base>(Derived).

Sean Silva silvas at purdue.edu
Sun Oct 7 14:04:57 PDT 2012


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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Remove-buggy-classof.patch
Type: application/octet-stream
Size: 5321 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20121007/ea6f05d1/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-Casting.h-Automatically-handle-isa-Base-Derived.patch
Type: application/octet-stream
Size: 3993 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20121007/ea6f05d1/attachment-0001.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-docs-Update-HowToSetUpLLVMStyleRTTI.patch
Type: application/octet-stream
Size: 4765 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20121007/ea6f05d1/attachment-0002.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0004-Remove-unnecessary-classof-s.patch
Type: application/octet-stream
Size: 72106 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20121007/ea6f05d1/attachment-0003.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0005-docs-Improve-HowToSetUpLLVMStyleRTTI.patch
Type: application/octet-stream
Size: 6313 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20121007/ea6f05d1/attachment-0004.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0006-Add-missing-classof.patch
Type: application/octet-stream
Size: 2004 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20121007/ea6f05d1/attachment-0005.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0007-Remove-pointless-classof-s.patch
Type: application/octet-stream
Size: 113142 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20121007/ea6f05d1/attachment-0006.obj>


More information about the cfe-commits mailing list