Restore isa<Ty>(X) asserts inside cast<Ty>(X)

[reames]

Summary

PLEASE DO NOT REVERT without careful consideration, and preferably prior discussion.

cast<Ty>(X) is a "checked cast". Its entire purpose is explicitly documented (https://llvm.org/docs/ProgrammersManual.html#the-isa-cast-and-dyn-cast-templates) as catching bad casts by asserting that the cast is valid. Unfortunately, in a recent rewrite of our casting infrastructure about three months back, these asserts got dropped.

This is discussed in more detail on discourse in https://discourse.llvm.org/t/cast-x-is-broken-implications-and-proposal-to-address/63033.

[rriddle]

Really appreciate catching this and raising the discussion! (Approval for whenever you need/want to land)

[xbolva00]

Is it possible to create simple testcase?

[craig.topper]

llvm/include/llvm/Support/Casting.h

578

It doesn't look like there is an implementation of isa that takes From *. It fails to compile on a Casting unittest.

[craig.topper]

llvm/include/llvm/Support/Casting.h

578

llvm/include/llvm/Support/Casting.h: In instantiation of ‘static bool llvm::isa_impl<To, From, Enabler>::doit(const From&) [with To = llvm::bar*; From = llvm::bar; Enabler = void]’:
llvm/include/llvm/Support/Casting.h:110:36:   required from ‘static bool llvm::isa_impl_cl<To, const From*>::doit(const From*) [with To = llvm::bar*; From = llvm::bar]’
llvm/include/llvm/Support/Casting.h:137:41:   required from ‘static bool llvm::isa_impl_wrap<To, FromTy, FromTy>::doit(const FromTy&) [with To = llvm::bar*; FromTy = const llvm::bar*]’
llvm/include/llvm/Support/Casting.h:129:13:   required from ‘static bool llvm::isa_impl_wrap<To, From, SimpleFrom>::doit(const From&) [with To = llvm::bar*; From = const llvm::bar* const; SimpleFrom = const llvm::bar*]’
llvm/include/llvm/Support/Casting.h:263:62:   required from ‘static bool llvm::CastIsPossible<To, From, Enable>::isPossible(const From&) [with To = llvm::bar*; From = const llvm::bar*; Enable = void]’
llvm/include/llvm/Support/Casting.h:517:38:   required from ‘static bool llvm::CastInfo<To, From, typename std::enable_if<(! llvm::is_simple_type<From>::value), void>::type>::isPossible(From&) [with To = llvm::bar*; From = llvm::bar* const]’
llvm/include/llvm/Support/Casting.h:556:46:   required from ‘bool llvm::isa(const From&) [with To = llvm::bar*; From = llvm::bar*]’
llvm/include/llvm/Support/Casting.h:585:3:   required from ‘decltype(auto) llvm::cast(From*) [with To = llvm::bar*; From = llvm::bar]’
llvm/unittests/Support/Casting.cpp:181:27:   required from here
llvm/include/llvm/Support/Casting.h:64:64: error: ‘classof’ is not a member of ‘llvm::bar*’
   static inline bool doit(const From &Val) { return To::classof(&Val); }

[craig.topper]

llvm/include/llvm/Support/Casting.h

578

Nevermind. I think this is fixed by the reverting of 0809f63826d36c89574d6ac056ebf46a4b6f29ff that you mentioned in your email.

[reames]

llvm/include/llvm/Support/Casting.h

578

Yep, exactly. I just pushed that as 781de11f.

This got slightly delayed from when I'd expected as my build infrastructure went down for a bit. We should now be fully clean build (if not test) on ToT.

[MaskRay]

-DLLVM_ENABLE_PROJECTS='clang;clang-tools-extra;flang;lldb;lld;compiler-rt;openmp;mlir;polly;cross-project-tests seems fine.

[tschuett]

Maybe we can introduce an LLVM_ASSERT that supports severity levels and plays nice with GTEST?

llvm/include/llvm/Support/Casting.h

 /// casting operator asserts that the type is correct, so it does not return
 /// null on failure.  It does not allow a null argument (use cast_if_present for
 /// that). It is typically used like this:
 ///
 ///  cast<Instruction>(myVal)->getParent()
 
 template <typename To, typename From>
 LLVM_NODISCARD inline decltype(auto) cast(const From &Val) {
+  assert(isa<To>(Val) && "cast<Ty>() argument of incompatible type!");
   return CastInfo<To, const From>::doCast(Val);
 }
 
 template <typename To, typename From>
 LLVM_NODISCARD inline decltype(auto) cast(From &Val) {
+  assert(isa<To>(Val) && "cast<Ty>() argument of incompatible type!");
   return CastInfo<To, From>::doCast(Val);
 }
 
 template <typename To, typename From>
 LLVM_NODISCARD inline decltype(auto) cast(From *Val) {
+  assert(isa<To>(Val) && "cast<Ty>() argument of incompatible type!");
   return CastInfo<To, From *>::doCast(Val);
 }
 
 template <typename To, typename From>
 LLVM_NODISCARD inline decltype(auto) cast(std::unique_ptr<From> &&Val) {
+  assert(isa<To>(Val) && "cast<Ty>() argument of incompatible type!");
   return CastInfo<To, std::unique_ptr<From>>::doCast(std::move(Val));
 }
 
 /// dyn_cast<X> - Return the argument parameter cast to the specified type. This
 /// casting operator returns null if the argument is of the wrong type, so it
 /// can be used to test for a type as well as cast if successful. The value
 /// passed in must be present, if not, use dyn_cast_if_present. This should be
 /// used in the context of an if statement like this: