On 18 Mar 2019, at 18:05, John McCall wrote:

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.

To get just the Clang ones:

./clang/lib/CodeGen/CGExprConstant.cpp:1701: if (auto destPtrTy = cast<llvm::PointerType>(destTy)) {

As discussed, I think this should just be a `cast<>` and
we should drop the dead code following the `if`.

./clang/lib/CodeGen/MicrosoftCXXABI.cpp:738: if (auto *Fn = cast<llvm::Function>(Throw.getCallee()))

This should be a `dyn_cast` in case there's an existing declaration.
Well, really `CreateRuntimeFunction` should take a CC as an optional
argument, but that's not something I can ask you to do.

./clang/lib/Sema/SemaOpenMP.cpp:10904: else if (auto *DRD = cast<OMPDeclareReductionDecl>(D))

This should be a `dyn_cast`, I think, although again arguably there
should be a more intrusive change to this code that would ensure that
the lookup can only contain these declarations.

./clang/lib/AST/ASTImporter.cpp:8463: if (auto *FromDC = cast<DeclContext>(From)) {

It does actually seem to be a general rule that this is only used with
declarations that are DCs, so this can be a `cast<>`, and honestly maybe
it could be updated so that the caller has to pass in a `DeclContext*`.

./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())

I think `getPrimaryContext` is probably only ever used on declarations
that have members, but `dyn_cast` would be the safe thing to do for
both of these.

John.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to