<div dir="ltr"><div class="gmail_extra"><div class="gmail_extra"> unsigned FunctionChunkIndex) {</div><div class="gmail_extra">+</div><div class="gmail_extra">+ // C99 6.9.1/3: The return type of a function shall be void or</div>
<div class="gmail_extra">+ // an object type other than array type.</div><div class="gmail_extra">+ // A return type of void cannot be qualified.</div><div><br></div></div><div class="gmail_extra"><br></div><div class="gmail_extra">
Micro-nit: no blank line at the start of a function body.</div><div class="gmail_extra"><br></div><div class="gmail_extra">This should only apply in C; 'const void' is explicitly a valid return type in C++ (see for instance 6.6.3/2, "a function with the return type cv void").</div>
<div class="gmail_extra"><br></div><div class="gmail_extra">Please make this an ExtWarn rather than an error, since GCC accepts it by default (and doesn't even diagnose it if it's on a non-definition function declaration), and it seems thoroughly harmless.</div>
<div class="gmail_extra"><br><div class="gmail_quote">On Mon, Jul 7, 2014 at 9:22 AM, Zach Davis <span dir="ltr"><<a href="mailto:zdavkeos@gmail.com" target="_blank">zdavkeos@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
Thanks for the comments. I have:<br>
<br>
- Cleaned up the code<br>
- Made the warning an error<br>
- Moved the check into diagnoseIgnoredFunctionQualifiers()<br>
- Added 3 test cases to test/Sema/function.c<br>
<div class=""><div class="h5"><br>
<br>
<br>
On Thu, Jul 3, 2014 at 4:41 PM, Alp Toker <<a href="mailto:alp@nuanti.com">alp@nuanti.com</a>> wrote:<br>
><br>
> On 03/07/2014 22:08, Zach Davis wrote:<br>
>><br>
>> As reported in bug 20146, a function cannot have a return type of<br>
>> 'void' with qualifiers.<br>
>><br>
>> Clang does emit a warning that the qualifiers are ignored<br>
>> [-Wignored-qualifiers] (off by default), but according to [1] this<br>
>> code is non-conforming.<br>
>><br>
>> The attached patch makes Clang issue a more specific warning like so:<br>
>><br>
>> test3.c:8:18: warning: return type of void cannot be qualified<br>
>> 'volatile void'<br>
>> volatile void baz(void) { return; }<br>
>> ^<br>
>><br>
>> [1]<a href="http://www.open-std.org/jtc1/sc22/wg14/docs/rr/dr_113.html" target="_blank">http://www.open-std.org/jtc1/sc22/wg14/docs/rr/dr_113.html</a><br>
><br>
><br>
> It seems fine to make this a hard error instead of a warning for C, and<br>
> probably C++ too. Richard?<br>
><br>
>><br>
>> 20146_return_qual_void.patch<br>
>><br>
>><br>
>> Index: lib/Sema/SemaType.cpp<br>
>> ===================================================================<br>
>> --- lib/Sema/SemaType.cpp (revision 212275)<br>
>> +++ lib/Sema/SemaType.cpp (working copy)<br>
>> @@ -2741,6 +2741,15 @@<br>
>> D.setInvalidType(true);<br>
>> }<br>
>> + // C99 6.9.1/3: The return type of a function shall be void or<br>
>> + // an object type other than array type.<br>
>> + // A return type of void cannot be qualified.<br>
>> + if (T->isVoidType() && T.getCVRQualifiers()) {<br>
>> + unsigned diagID = diag::warn_func_returning_qualified_void;<br>
><br>
><br>
> Just pass the ID directly to Diag().<br>
><br>
>> + S.Diag(DeclType.Loc, diagID) << T;<br>
>> + D.setInvalidType(true);<br>
>> + }<br>
>> +<br>
><br>
><br>
> How about placing this check with an early return at the top of<br>
> diagnoseIgnoredFunctionQualifiers()?<br>
><br>
>> // Do not allow returning half FP value.<br>
>> // FIXME: This really should be in BuildFunctionType.<br>
>> if (T->isHalfType()) {<br>
>> Index: include/clang/Basic/DiagnosticSemaKinds.td<br>
>> ===================================================================<br>
>> --- include/clang/Basic/DiagnosticSemaKinds.td (revision 212275)<br>
>> +++ include/clang/Basic/DiagnosticSemaKinds.td (working copy)<br>
>> @@ -4160,6 +4160,8 @@<br>
>> def err_func_returning_array_function : Error<<br>
>> "function cannot return %select{array|function}0 type %1">;<br>
>> +def warn_func_returning_qualified_void : Warning<<br>
>> + "return type of void cannot be qualified %0">;<br>
><br>
><br>
> (Warnings need to have a diagnostic group / -W flag, though it doesn't<br>
> matter if you go ahead and make it an error.)<br>
><br>
>> def err_field_declared_as_function : Error<"field %0 declared as a<br>
>> function">;<br>
>> def err_field_incomplete : Error<"field has incomplete type %0">;<br>
>> def ext_variable_sized_type_in_struct : ExtWarn<<br>
><br>
><br>
> Test case?<br>
><br>
> Alp.<br>
><br>
>><br>
>><br>
>> _______________________________________________<br>
>> cfe-commits mailing list<br>
>> <a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
>> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
><br>
><br>
> --<br>
> <a href="http://www.nuanti.com" target="_blank">http://www.nuanti.com</a><br>
> the browser experts<br>
><br>
</div></div></blockquote></div><br></div></div>