[cfe-commits] r150128 - in /cfe/trunk: include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/SemaDecl.cpp test/SemaCXX/function-extern-c.cpp

[David Blaikie]

How would we detect the calls from c? The extern "c" stuff is ifdef'd out
when compiling as c, isn't it?
------------------------------
From: Chandler Carruth
Sent: 2/16/2012 2:56 PM
To: Aaron Ballman
Cc: cfe-commits at cs.uiuc.edu
Subject: Re: [cfe-commits] r150128 - in /cfe/trunk:
include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/SemaDecl.cpp
test/SemaCXX/function-extern-c.cpp

On Thu, Feb 9, 2012 at 3:18 PM, Aaron Ballman <aaron at aaronballman.com>wrote:

> On Thu, Feb 9, 2012 at 5:04 PM, Chandler Carruth <chandlerc at google.com>
> wrote:
> > On Thu, Feb 9, 2012 at 12:42 PM, Matt Beaumont-Gay <matthewbg at google.com
> >
> > wrote:
> >>
> >> On a different note, what do you think about not warning when the
> >> return type is incomplete? It's a little surprising that we warn on
> >> this:
> >>
> >> struct foo;
> >> extern "C" { struct foo get_foo(void); }
> >> struct foo { };
> >
> >
> > I think that's a bug. Incomplete struct types are relatively common as
> > "opaque" return types.
>
> Oddly enough, MSVC does complain on that code.
>
> error C2526: 'get_foo' : C linkage function cannot return C++ class 'foo'
>
> It's hard to really say one way or the other, since you can make a
> case either way.  Yes, it may be perfectly fine as an opaque type, but
> it may also contain non-C-compatible fields which would justify the
> warning.


This warning is causing massive pain for our users. It has thus far found
zero real bugs. I would like to disable it until you can change the logic
to make it more accurate and have fewer false positives. Let me know if you
have any strong objections to this, or if you already have the fixes in
flight.

The specific problem is that warning on the declaration is fundamentally
not sound -- there are clearly many reasons to declare a function which
uses C++ return types but with C linkage. We've seen quite a few, some even
within LLVM: the mangled name is much easier to deal with.

The *bug* is calling such a function from a C context. I think you should
re-cast this warning to fire on *calls* rather than on *declarations* as
that will be much less prone to false positives. Does this make sense?
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120216/84671d71/attachment.html>