<div dir="ltr">On 30 January 2013 22:43, Richard Smith <span dir="ltr"><<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote">
<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">SortUndefinedButUsed is not a strict weak ordering. Consider:<br>
<br>
A = {1, invalid}<br>
B = {2, 1}<br>
C = {0, 2}<br>
<br>
We have A < B (comparing first because A.second is invalid), B < C<br>
(comparing second), and C < A (comparing first because A.second is<br>
invalid).<br></blockquote><div><br></div><div style>Fixed. Thanks for catching that.</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">
@@ -8428,12 +8436,12 @@<br>
if (FD) {<br>
FD->setBody(Body);<br>
<br>
- // The only way to be included in UndefinedInternals is if there is an<br>
+ // The only way to be included in UndefinedButUsed is if there is an<br>
// ODR-use before the definition. Avoid the expensive map lookup if this<br>
// is the first declaration.<br>
- if (FD->getPreviousDecl() != 0 && FD->getPreviousDecl()->isUsed() &&<br>
- FD->getLinkage() != ExternalLinkage)<br>
- UndefinedInternals.erase(FD);<br>
+ if (FD->getPreviousDecl() != 0 && FD->getPreviousDecl()->isUsed())<br>
+ if (FD->getLinkage() != ExternalLinkage && !FD->isInlined())<br>
+ UndefinedButUsed.erase(FD);<br>
<br>
Shouldn't this be (FD->getLinkage() != ExternalLinkage ||<br>
FD->getPreviousDecl()->isInlined()) ?<br></blockquote><div><br></div><div style>Right.</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">
+namespace test7 {<br>
+ // It is invalid to redeclare a function inline after its definition, but we<br>
+ // do not diagnose this yet.<br>
+ void f(); // expected-warning{{inline function 'test7::f' is not defined}}<br>
+ void test() { f(); } // no used-here note.<br>
+ inline void f();<br>
+}<br>
<br>
This test doesn't redeclare a function inline after its definition,<br>
just after its use.<br></blockquote><div><br></div><div style>Erp, of course. Removed the stale comment.</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 class="im">On Wed, Jan 30, 2013 at 10:19 PM, Nick Lewycky <<a href="mailto:nlewycky@google.com">nlewycky@google.com</a>> wrote:<br>
> This patch extends the tracking for used but undefined functions and<br>
> variables with internal linkage to also apply to inline functions.<br>
> Sema::UndefinedInternals is renamed Sema::UndefinedButUsed.<br>
><br>
> Fixes PR14993. Please review! Please pay particular attention to the<br>
> behaviour of the error under C99 and GNU89 modes.<br>
<br>
</div>I've not looked at this from the perspective of C yet, but it looks<br>
correct for C++ (modulo the above comments).<br></blockquote><div><br></div><div style>C99 has a similar rule:</div><div style><br></div><div> "For a function with external linkage, the following restrictions apply: If a function is declared with an inline function specifier, then it shall also be defined in the same translation unit."</div>
<div style> - C99 6.7.4p6</div><div><br></div><div>Of course, a function with internal linkage and isn't covered by the above, but it doesn't matter since that's invalid for other reasons. Good news, in C99 mode GCC warns on declaration without definition of 'inline', 'extern inline' functions, and used but undefined 'static inline' functions.<br>
</div><div><br></div><div style>Bad news, GNU89 mode seems to permit it. 'static inline' functions get the same warning as in C99 mode, but in practice you can define a function of the same name in another TU, without 'static inline', and it links fine. With 'extern inline' I think it's even deliberate.</div>
<div style><br></div><div style>I've changed the patch to simply not warn on used but not defined gnu89-inline functions.</div><div style><br></div><div style>Nick</div><div style><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 class="im">
> Also, there's a comment that these should be errors. That turned out to not<br>
> be entirely trivial due to some codegen tests, so it's left out of this<br>
> patch.<br>
><br>
> Nick<br>
><br>
><br>
</div>> _______________________________________________<br>
> cfe-commits mailing list<br>
> <a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
><br>
</blockquote></div><br></div></div>