<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN">
<html>
<head>
<meta http-equiv="Content-Type" content="text/xhtml; charset=utf-8">
</head>
<body>
<div style="font-family:sans-serif"><div style="white-space:normal">
<p dir="auto">On 18 Mar 2019, at 18:05, John McCall wrote:</p>
</div>
<div style="white-space:normal"><blockquote style="border-left:2px solid #777; color:#777; margin:0 0 5px; padding-left:5px"><p dir="auto">On 18 Mar 2019, at 15:38, Don Hinton wrote:</p>
<blockquote style="border-left:2px solid #777; color:#999; margin:0 0 5px; padding-left:5px; border-left-color:#999"><p dir="auto">Hi John:<br>
<br>
I found this investigating the cast assert noted here:<br>
<a href="http://lists.llvm.org/pipermail/cfe-dev/2019-March/061685.html" style="color:#999">http://lists.llvm.org/pipermail/cfe-dev/2019-March/061685.html</a><br>
<br>
I subsequently did quick grep and found a number of cases in clang+llvm<br>
(didn't find any in other projects) . I'm happy to fix these in mass,<br>
i.e., s/cast/dyn_cast/, but as you mentioned, some might require additional<br>
refactoring, e.g., removal of the conditional.</p>
</blockquote><p dir="auto">They probably all ought to be considered individually. Tagging Richard in case he has opinions about the AST ones.</p>
</blockquote></div>
<div style="white-space:normal">
<p dir="auto">To get just the Clang ones:</p>
</div>
<div style="white-space:normal"><blockquote style="border-left:2px solid #777; color:#777; margin:0 0 5px; padding-left:5px"><blockquote style="border-left:2px solid #777; color:#999; margin:0 0 5px; padding-left:5px; border-left-color:#999"><p dir="auto">./clang/lib/CodeGen/CGExprConstant.cpp:1701: if (auto destPtrTy = cast<llvm::PointerType>(destTy)) {</p>
</blockquote></blockquote></div>
<div style="white-space:normal">
<p dir="auto">As discussed, I think this should just be a <code style="background-color:#F7F7F7; border-radius:3px; margin:0; padding:0 0.4em" bgcolor="#F7F7F7">cast<></code> and<br>
we should drop the dead code following the <code style="background-color:#F7F7F7; border-radius:3px; margin:0; padding:0 0.4em" bgcolor="#F7F7F7">if</code>.</p>
</div>
<div style="white-space:normal"><blockquote style="border-left:2px solid #777; color:#777; margin:0 0 5px; padding-left:5px"><blockquote style="border-left:2px solid #777; color:#999; margin:0 0 5px; padding-left:5px; border-left-color:#999"><p dir="auto">./clang/lib/CodeGen/MicrosoftCXXABI.cpp:738: if (auto *Fn = cast<llvm::Function>(Throw.getCallee()))</p>
</blockquote></blockquote></div>
<div style="white-space:normal">
<p dir="auto">This should be a <code style="background-color:#F7F7F7; border-radius:3px; margin:0; padding:0 0.4em" bgcolor="#F7F7F7">dyn_cast</code> in case there's an existing declaration.<br>
Well, really <code style="background-color:#F7F7F7; border-radius:3px; margin:0; padding:0 0.4em" bgcolor="#F7F7F7">CreateRuntimeFunction</code> should take a CC as an optional<br>
argument, but that's not something I can ask you to do.</p>
</div>
<div style="white-space:normal"><blockquote style="border-left:2px solid #777; color:#777; margin:0 0 5px; padding-left:5px"><blockquote style="border-left:2px solid #777; color:#999; margin:0 0 5px; padding-left:5px; border-left-color:#999"><p dir="auto">./clang/lib/Sema/SemaOpenMP.cpp:10904: else if (auto *DRD = cast<OMPDeclareReductionDecl>(D))</p>
</blockquote></blockquote></div>
<div style="white-space:normal">
<p dir="auto">This should be a <code style="background-color:#F7F7F7; border-radius:3px; margin:0; padding:0 0.4em" bgcolor="#F7F7F7">dyn_cast</code>, I think, although again arguably there<br>
should be a more intrusive change to this code that would ensure that<br>
the lookup can only contain these declarations.</p>
</div>
<div style="white-space:normal"><blockquote style="border-left:2px solid #777; color:#777; margin:0 0 5px; padding-left:5px"><blockquote style="border-left:2px solid #777; color:#999; margin:0 0 5px; padding-left:5px; border-left-color:#999"><p dir="auto">./clang/lib/AST/ASTImporter.cpp:8463: if (auto *FromDC = cast<DeclContext>(From)) {</p>
</blockquote></blockquote></div>
<div style="white-space:normal">
<p dir="auto">It does actually seem to be a general rule that this is only used with<br>
declarations that are DCs, so this can be a <code style="background-color:#F7F7F7; border-radius:3px; margin:0; padding:0 0.4em" bgcolor="#F7F7F7">cast<></code>, and honestly maybe<br>
it could be updated so that the caller has to pass in a <code style="background-color:#F7F7F7; border-radius:3px; margin:0; padding:0 0.4em" bgcolor="#F7F7F7">DeclContext*</code>.</p>
</div>
<div style="white-space:normal"><blockquote style="border-left:2px solid #777; color:#777; margin:0 0 5px; padding-left:5px"><blockquote style="border-left:2px solid #777; color:#999; margin:0 0 5px; padding-left:5px; border-left-color:#999"><p dir="auto">./clang/lib/AST/DeclBase.cpp:1182: if (auto *Def = cast<ObjCInterfaceDecl>(this)->getDefinition())<br>
./clang/lib/AST/DeclBase.cpp:1187: if (auto *Def = cast<ObjCProtocolDecl>(this)->getDefinition())</p>
</blockquote></blockquote></div>
<div style="white-space:normal">
<p dir="auto">I think <code style="background-color:#F7F7F7; border-radius:3px; margin:0; padding:0 0.4em" bgcolor="#F7F7F7">getPrimaryContext</code> is probably only ever used on declarations<br>
that have members, but <code style="background-color:#F7F7F7; border-radius:3px; margin:0; padding:0 0.4em" bgcolor="#F7F7F7">dyn_cast</code> would be the safe thing to do for<br>
both of these.</p>
<p dir="auto">John.</p>
</div>
</div>
</body>
</html>