[PATCH] D74466: [mlir] StdToLLVM: Add error when the sourceMemRef of a subview is not a llvm type.

[Alex Zinenko via Phabricator via llvm-commits]

ftynse added inline comments.


================
Comment at: mlir/lib/Conversion/StandardToLLVM/ConvertStandardToLLVM.cpp:389
+  structType = value.getType().dyn_cast<LLVM::LLVMType>();
+  assert(structType && "expected llvm type");
 }
----------------
mehdi_amini wrote:
> Something does not line up here: you are using dyn_cast which is supposed to be expecting a possible nullptr as a result, but then you add adding an assert, which implies this is an impossible case...
> 
> If you are using the assert just to get a “nicer” crash, then the pattern should be asserting on “isa<>” and using cast<>
It is indeed for a more explicative assert message.

What you say sort of contradicts http://llvm.org/docs/ProgrammersManual.html#the-isa-cast-and-dyn-cast-templates which says "you should not use an isa<> test followed by a cast<>".


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74466/new/

https://reviews.llvm.org/D74466