<div dir="ltr">On Tue, Mar 10, 2015 at 7:16 PM, Joerg Sonnenberger <span dir="ltr"><<a href="mailto:joerg@britannica.bec.de" target="_blank">joerg@britannica.bec.de</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On Tue, Mar 10, 2015 at 07:03:16PM -0700, John McCall wrote:<br>
> On Tue, Mar 10, 2015 at 6:37 PM, Joerg Sonnenberger <<a href="mailto:joerg@britannica.bec.de">joerg@britannica.bec.de</a><br>
> > wrote:<br>
><br>
> > On Tue, Mar 10, 2015 at 05:44:00PM -0700, John McCall wrote:<br>
> > > I'm sorry, I missed your early request, and your response to my review.<br>
> > > I'm much more likely to respond quickly if you keep me as a recipient.<br>
> > ><br>
> > > I really would like you to diagnose this in Sema, please.<br>
> > Target-specific<br>
> > > restrictions are not new, especially on builtin functions.  But if you do<br>
> > > that, it's approved for merge.<br>
> ><br>
> > But Sema is too early, it breaks valid use cases that are never going to<br>
> > hit the backend at all. Consider clang --analyze or clang-modernize.<br>
> > Especially the latter is completely target independent, so it shouldn't<br>
> > get fail on code that is valid on one platform and only fails on another<br>
> > because of LLVM bugs.<br>
> ><br>
><br>
> It's target-independent except for the thousands of ways that C code is not<br>
> target-independent.  Headers need to be in place and provide the right<br>
> declarations, hordes of warnings turn out differently based on type size,<br>
> printf format checking has target-specific logic, etc.<br>
<br>
</span>A refactoring tool knows how to deal with many of those variations... </blockquote><div><br></div><div>A refactoring tool generally needs to be smart enough to not accidentally</div><div>produce unportable code.  That is a different problem from actually being</div><div>able to function on a file that doesn't compile because it wasn't designed</div><div>to be portable in the first place.  And __builtin_setjmp / __builtin_longjmp</div><div>are not portable.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">
> The way we (don't) implement them, __builtin_setjmp and __builtin_longjmp<br>
> are target-specific builtin functions, and they should be diagnosed along<br>
> with the rest of them.<br>
<br>
</span>But that we don't implement them is a choice of CodeGen. Ideally, that<br>
wouldn't be needed if the IR intrinsics were cleanly separated, so this<br>
is really an implementation detail of the contract between Clang and<br>
LLVM IR and nothing about the semantic analysis of the source code. Why<br>
do you insist on violating the layering?</blockquote><div><br></div><div>The contract between Clang and LLVM IR is completely unimportant</div><div>compared to the contract between the user and their compilation results.</div><div>Semantic analysis is supposed to be sufficient to identify all the problems</div><div>in a user's code.  We sometimes fall short of that, but we usually consider</div><div>those bugs, and we like to at least have good reasons for it.</div><div><br></div><div>There are many good reasons to raise this to the semantic-analysis layer.</div><div>There should probably be has a __has_feature check for this.  The size</div><div>of the buffer is effectively guaranteed, and a really good semantic analysis</div><div>layer would be checking that the buffer is at least that size.  We should</div><div>also be communicating that size to the user in some way instead of</div><div>forcing them to use a jmp_buf (which can be an order of magnitude</div><div>larger) for no particular reason.</div><div><br></div><div>If you want to keep arguing with me about this, go ahead, but I'm not</div><div>actually going to change my mind, so if you want this in either the</div><div>branch or trunk, you're going to need to move it to Sema.</div><div><br></div><div>John.</div></div>
</div></div>