On Wed, Jan 23, 2013 at 12:13 PM, Hans Wennborg <[email protected]> wrote: > On Sun, Dec 30, 2012 at 8:40 PM, Rafael Espindola > <[email protected]> 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?
Ping? _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
