<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Wed, May 21, 2014 at 4:43 PM, jahanian <span dir="ltr"><<a href="mailto:fjahanian@apple.com" target="_blank">fjahanian@apple.com</a>></span> wrote:<br>

<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div style="word-wrap:break-word"><br><div><div><div>On May 21, 2014, at 4:29 PM, Richard Smith <<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>> wrote:</div>

<br><blockquote type="cite"><div dir="ltr">It's not OK to set a declaration invalid having only produced a warning.</div></blockquote><div><br></div></div><div>Yes.  But this is a unique situation:</div></div><div><br>

</div><div>Both <span style="font-family:Menlo;font-size:11px">Old-></span><span style="color:rgb(49,89,93);font-family:Menlo;font-size:11px">getBuiltinID</span><span style="font-family:Menlo;font-size:11px">() and </span><span style="font-family:Menlo;font-size:11px">New-></span> <span style="color:rgb(49,89,93);font-family:Menlo;font-size:11px">getBuiltinID() refer to the same identifier</span></div>

<div><span style="font-family:Menlo;font-size:11px">New-></span><span style="color:rgb(49,89,93);font-family:Menlo;font-size:11px">getIdentifier</span><span style="font-family:Menlo;font-size:11px">()-></span><span style="color:rgb(49,89,93);font-family:Menlo;font-size:11px">setBuiltinID</span><span style="font-family:Menlo;font-size:11px">(</span><span style="font-family:Menlo;font-size:11px;color:rgb(79,129,135)">Builtin</span><span style="font-family:Menlo;font-size:11px">::</span><span style="color:rgb(49,89,93);font-family:Menlo;font-size:11px">NotBuiltin</span><span style="font-family:Menlo;font-size:11px">)  (which we must do because it is user declared)</span></div>

<div><font face="Menlo"><span style="font-size:11px">makes “Old” as non-builtin as well. </span></font><span style="font-size:11px;font-family:Menlo">Not invalidating it we end up with a declaration of a built-in function</span></div>

<div><font face="Menlo"><span style="font-size:11px">which has no built-in ID!</span></font></div></div></blockquote><div><br></div><div>With the patch, I'd expect that calls to the builtin function that don't find the local extern decl will (sometimes) be silently discarded. It's far too dangerous to allow a valid AST to contain invalid decls.</div>

<div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div style="word-wrap:break-word"><div><font face="Menlo"><span style="font-size:11px">Is there an alternative? As things stand, we don’t keep source fidelity for local re-declaration</span></font></div>

<div><font face="Menlo"><span style="font-size:11px">of a built-in function  (and IRGen doesn’t like that :).</span></font></div></div></blockquote><div><br></div><div>How about this: drop the setInvalidDecl() call. In its place, when we get a type mismatch between two function declarations, if Old is implicit then check whether it used to be a builtin. If so, check the type against the most recent declaration instead.</div>
</div></div></div>