On 18 Mar 2019, at 15:38, Don Hinton wrote:
Hi John:
I found this investigating the cast assert noted here:
http://lists.llvm.org/pipermail/cfe-dev/2019-March/061685.html
I subsequently did quick grep and found a number of cases in
clang+llvm
(didn't find any in other projects) . I'm happy to fix these in mass,
i.e., s/cast/dyn_cast/, but as you mentioned, some might require
additional
refactoring, e.g., removal of the conditional.
They probably all ought to be considered individually. Tagging Richard
in case he has opinions about the AST ones.
In any case, here's what I've found -- perhaps a new llvm clang-tidy
checker would be useful:
Yeah, it would be great if something like this got run automatically.
John.
$ find ./clang ./llvm -type f \( -name "*.h" -o -name "*.cpp" \) -exec
grep
-Hn "if *(auto.* *= *cast *<" \{\} \;
./clang/lib/Sema/SemaOpenMP.cpp:10904: else if (auto *DRD =
cast<OMPDeclareReductionDecl>(D))
./clang/lib/CodeGen/CGExprConstant.cpp:1701: if (auto destPtrTy =
cast<llvm::PointerType>(destTy)) {
./clang/lib/CodeGen/MicrosoftCXXABI.cpp:738: if (auto *Fn =
cast<llvm::Function>(Throw.getCallee()))
./clang/lib/AST/ASTImporter.cpp:8463: if (auto *FromDC =
cast<DeclContext>(From)) {
./clang/lib/AST/DeclBase.cpp:1182: if (auto *Def =
cast<ObjCInterfaceDecl>(this)->getDefinition())
./clang/lib/AST/DeclBase.cpp:1187: if (auto *Def =
cast<ObjCProtocolDecl>(this)->getDefinition())
./llvm/tools/llvm-objdump/llvm-objdump.cpp:802: if (auto *Elf64BEObj
=
cast<ELF64BEObjectFile>(Obj))
./llvm/tools/llvm-objdump/llvm-objdump.cpp:846: else if (auto
*Elf64BEObj
= cast<ELF64BEObjectFile>(Obj))
./llvm/lib/Target/X86/AsmParser/X86Operand.h:102: if (auto Imm
=
cast<MCConstantExpr>(Val)->getValue())
./llvm/lib/Transforms/IPO/CalledValuePropagation.cpp:139: } else
if
(auto *F = cast<Function>(Key.getPointer()))
./llvm/lib/Transforms/Coroutines/CoroEarly.cpp:183: if (auto
*CII =
cast<CoroIdInst>(&I)) {
thanks...
don
On Mon, Mar 18, 2019 at 12:07 PM John McCall <jmcc...@apple.com>
wrote:
On 18 Mar 2019, at 14:39, Don Hinton wrote:
It looks like this change introduced a small bug; Specifically, the
following cast test:
- if (auto PT = dyn_cast<llvm::PointerType>(DestTy)) {
...
+ // If we're producing a pointer, this is easy.
+ if (auto destPtrTy = cast<llvm::PointerType>(destTy)) {
Since the cast can fail, shouldn't you prefer dyn_cast<>(), which
can
return nullptr, over cast<>(), which will assert?
Yes, although if it hasn't caused a problem in the last year and a
half,
maybe we should just change the code to be non-conditional.
John.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
