[cfe-commits] r171263 - in /cfe/trunk: lib/Sema/SemaDecl.cpp test/SemaCXX/function-extern-c.cpp

[Hans Wennborg]

On Sun, Dec 30, 2012 at 8:40 PM, Rafael Espindola
<rafael.espindola at gmail.com> wrote:
> Author: rafael
> Date: Sun Dec 30 14:40:41 2012
> New Revision: 171263
>
> URL: http://llvm.org/viewvc/llvm-project?rev=171263&view=rev
> Log:
> Use hasCLanguageLinkage when warning about non C return types.
>
> Modified:
>     cfe/trunk/lib/Sema/SemaDecl.cpp
>     cfe/trunk/test/SemaCXX/function-extern-c.cpp
>
> Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=171263&r1=171262&r2=171263&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaDecl.cpp Sun Dec 30 14:40:41 2012
> @@ -6266,7 +6266,7 @@
>      // If this function is declared as being extern "C", then check to see if
>      // the function returns a UDT (class, struct, or union type) that is not C
>      // compatible, and if it does, warn the user.
> -    if (NewFD->isExternC()) {
> +    if (NewFD->hasCLanguageLinkage()) {
>        QualType R = NewFD->getResultType();
>        if (R->isIncompleteType() && !R->isVoidType())
>          Diag(NewFD->getLocation(), diag::warn_return_value_udt_incomplete)
>
> Modified: cfe/trunk/test/SemaCXX/function-extern-c.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/function-extern-c.cpp?rev=171263&r1=171262&r2=171263&view=diff
> ==============================================================================
> --- cfe/trunk/test/SemaCXX/function-extern-c.cpp (original)
> +++ cfe/trunk/test/SemaCXX/function-extern-c.cpp Sun Dec 30 14:40:41 2012
> @@ -38,3 +38,16 @@
>  extern "C" A *f10( void );
>
>  extern "C" struct mypodstruct f12(); // expected-warning {{'f12' has C-linkage specified, but returns incomplete type 'struct mypodstruct' which could be incompatible with C}}
> +
> +namespace test2 {
> +  // FIXME: we should probably suppress the first warning as the second one
> +  // is more precise.
> +  // For now this tests that a second 'extern "C"' is not necessary to trigger
> +  // the warning.
> +  struct A;
> +  extern "C" A f(void); // expected-warning {{'f' has C-linkage specified, but returns incomplete type 'test2::A' which could be incompatible with C}}
> +  struct A {
> +    A(const A&);
> +  };
> +  A f(void);  // expected-warning {{'f' has C-linkage specified, but returns user-defined type 'test2::A' which is incompatible with C}}
> +}

Hi Rafael,

As far as I can tell, your patch makes Clang warn about returning
non-POD types from functions with C-style linkage, even if those
functions have internal linkage. I'm not sure we want to warn in that
case?

We tripped over this in Chrome where we have some code that goes like this:

extern "C" {
  static NonPODType x() { ... }
  void foo() { ... code that uses x() }
}

Now we get a warning about x(). Does it make sense to warn here, given
that the function has internal linkage, and so will only be called
from C++ code?

It seems the test case you added passes already without your change to
SemaDecl.cpp, so I'm wondering if your change was necessary?

Thanks,
Hans