patch: new warning on ODR-use of undefined inline functions

Richard Smith richard at metafoo.co.uk
Wed Jan 30 22:43:58 PST 2013


SortUndefinedButUsed is not a strict weak ordering. Consider:

  A = {1, invalid}
  B = {2, 1}
  C = {0, 2}

We have A < B (comparing first because A.second is invalid), B < C
(comparing second), and C < A (comparing first because A.second is
invalid).


@@ -8428,12 +8436,12 @@
   if (FD) {
     FD->setBody(Body);

-    // The only way to be included in UndefinedInternals is if there is an
+    // The only way to be included in UndefinedButUsed is if there is an
     // ODR-use before the definition. Avoid the expensive map lookup if this
     // is the first declaration.
-    if (FD->getPreviousDecl() != 0 && FD->getPreviousDecl()->isUsed() &&
-        FD->getLinkage() != ExternalLinkage)
-      UndefinedInternals.erase(FD);
+    if (FD->getPreviousDecl() != 0 && FD->getPreviousDecl()->isUsed())
+      if (FD->getLinkage() != ExternalLinkage && !FD->isInlined())
+        UndefinedButUsed.erase(FD);

Shouldn't this be (FD->getLinkage() != ExternalLinkage ||
FD->getPreviousDecl()->isInlined()) ?

+namespace test7 {
+  // It is invalid to redeclare a function inline after its definition, but we
+  // do not diagnose this yet.
+  void f();  // expected-warning{{inline function 'test7::f' is not defined}}
+  void test() { f(); } // no used-here note.
+  inline void f();
+}

This test doesn't redeclare a function inline after its definition,
just after its use.

On Wed, Jan 30, 2013 at 10:19 PM, Nick Lewycky <nlewycky at google.com> wrote:
> This patch extends the tracking for used but undefined functions and
> variables with internal linkage to also apply to inline functions.
> Sema::UndefinedInternals is renamed Sema::UndefinedButUsed.
>
> Fixes PR14993. Please review! Please pay particular attention to the
> behaviour of the error under C99 and GNU89 modes.

I've not looked at this from the perspective of C yet, but it looks
correct for C++ (modulo the above comments).

> Also, there's a comment that these should be errors. That turned out to not
> be entirely trivial due to some codegen tests, so it's left out of this
> patch.
>
> Nick
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>



More information about the cfe-commits mailing list