[cfe-commits] [PATCH] Warn on non-compatible returns from extern "C" functions

Eli Friedman eli.friedman at gmail.com
Wed Feb 8 16:12:15 PST 2012


On Tue, Feb 7, 2012 at 9:11 PM, Aaron Ballman <aaron at aaronballman.com> wrote:
> On Mon, Feb 6, 2012 at 4:47 PM, Eli Friedman <eli.friedman at gmail.com> wrote:
>> On Mon, Feb 6, 2012 at 2:40 PM, Aaron Ballman <aaron at aaronballman.com> wrote:
>>> On Mon, Feb 6, 2012 at 4:33 PM, Eli Friedman <eli.friedman at gmail.com> wrote:
>>>> On Sun, Feb 5, 2012 at 9:31 AM, Aaron Ballman <aaron at aaronballman.com> wrote:
>>>>> MSVC will warn the user when a declared return type of an extern "C"
>>>>> function is not C-compatible
>>>>> (http://msdn.microsoft.com/en-us/library/1e02627y(v=vs.100).aspx).
>>>>> This patch adds the same style of warning to clang, under the
>>>>> -Wreturn-type group, with test cases.
>>>>>
>>>>> Is there a better way for me to check whether a type is "C compatible?"
>>>>
>>>> CXXRecordDecl::isCLike was added recently... not sure if it's what you want.
>>>
>>> Thanks, I'll check it out.
>>
>> Thinking about it a bit more, isPOD is probably closer to what you
>> want... since you're actually worried about whether we guarantee C
>> compatibility, not whether the struct could be copy-pasted into a C
>> program.
>
> On reflection, I think you're right.  I stuck with isPOD instead of isCLike.
>
>>>> "UDT" is not a term we use anywhere in clang; please don't use it here.
>>>
>>> Suggestions on alternative wording?
>>
>> You could just expand out its definition.  MSVC's warning appears to
>> be much more aggressive than what you are implementing here.
>
> I've changed the wording to be:
>
> %0 has C-linkage specified, but returns user-defined type %1 which is
> incompatible with C
>
> So the only changes to this patch are the test cases and the wording
> for the warning itself.

+      if (!R.isPODType(Context) &&
+          !R->isScalarType() && !R->isVoidType())

The isScalarType() check appears to be redundant.


\ No newline at end of file

(Please fix.)


Otherwise, looks fine.

-Eli




More information about the cfe-commits mailing list