[PATCH] [RFC] PR20146 - Cannot return void with qualifiers

Zach Davis zdavkeos at gmail.com
Mon Aug 4 09:52:35 PDT 2014


{ including cfe-commits }

Thanks for the word-smithing, and for forcing me to do it the right
way by taking advantage of Sema::
diagnoseIgnoredQualifiers...

- Changed the wording of the warning
- Parameterized Sema::diagnoseIgnoredQualifiers so we can use our new
diagnostic text

On Wed, Jul 16, 2014 at 5:35 PM, Richard Smith <richard at metafoo.co.uk> wrote:
> It's a little confusing to put "X cannot have Y" in an ExtWarn; we usually
> phrase such things as "X has Y" or "ISO C does not allow X to have Y" to
> make it clear that we may be allowing this despite the standard saying
> otherwise. Also, the trailing %0 in the diagnostic text doesn't seem to fit
> into the sentence very well. Suggestion:
>
>   ISO C does not allow %0 qualifier%plural{1:|:s} on void return type
>
> ... and use Sema::diagnoseIgnoredQualifiers to produce a diagnostic with the
> relevant qualifiers listed.
>
> Other than the diagnostic wording, this looks good to me, thanks.
>
> On Thu, Jul 10, 2014 at 7:57 AM, Zach Davis <zdavkeos at gmail.com> wrote:
>>
>> Thanks for the suggestions. In this patch:
>>
>> - The error is now a warning with a "return-qualified-void" group
>> - The warning only applies to C code
>> - Re-formatted the code
>>
>> The warning is currently on by default.
>>
>>
>> On Tue, Jul 8, 2014 at 7:29 PM, Richard Smith <richard at metafoo.co.uk>
>> wrote:
>> >                                                unsigned
>> > FunctionChunkIndex)
>> > {
>> > +
>> > +  // C99 6.9.1/3: The return type of a function shall be void or
>> > +  // an object type other than array type.
>> > +  // A return type of void cannot be qualified.
>> >
>> >
>> > Micro-nit: no blank line at the start of a function body.
>> >
>> > 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").
>> >
>> > 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.
>> >
>> > On Mon, Jul 7, 2014 at 9:22 AM, Zach Davis <zdavkeos at gmail.com> wrote:
>> >>
>> >> Thanks for the comments.  I have:
>> >>
>> >> - Cleaned up the code
>> >> - Made the warning an error
>> >> - Moved the check into diagnoseIgnoredFunctionQualifiers()
>> >> - Added 3 test cases to test/Sema/function.c
>> >>
>> >>
>> >>
>> >> On Thu, Jul 3, 2014 at 4:41 PM, Alp Toker <alp at nuanti.com> wrote:
>> >> >
>> >> > On 03/07/2014 22:08, Zach Davis wrote:
>> >> >>
>> >> >> As reported in bug 20146, a function cannot have a return type of
>> >> >> 'void' with qualifiers.
>> >> >>
>> >> >> Clang does emit a warning that the qualifiers are ignored
>> >> >> [-Wignored-qualifiers] (off by default), but according to [1] this
>> >> >> code is non-conforming.
>> >> >>
>> >> >> The attached patch makes Clang issue a more specific warning like
>> >> >> so:
>> >> >>
>> >> >>      test3.c:8:18: warning: return type of void cannot be qualified
>> >> >> 'volatile void'
>> >> >>      volatile void baz(void) { return; }
>> >> >>                    ^
>> >> >>
>> >> >> [1]http://www.open-std.org/jtc1/sc22/wg14/docs/rr/dr_113.html
>> >> >
>> >> >
>> >> > It seems fine to make this a hard error instead of a warning for C,
>> >> > and
>> >> > probably C++ too. Richard?
>> >> >
>> >> >>
>> >> >> 20146_return_qual_void.patch
>> >> >>
>> >> >>
>> >> >> Index: lib/Sema/SemaType.cpp
>> >> >> ===================================================================
>> >> >> --- lib/Sema/SemaType.cpp       (revision 212275)
>> >> >> +++ lib/Sema/SemaType.cpp       (working copy)
>> >> >> @@ -2741,6 +2741,15 @@
>> >> >>           D.setInvalidType(true);
>> >> >>         }
>> >> >>   +      // C99 6.9.1/3: The return type of a function shall be void
>> >> >> or
>> >> >> +      // an object type other than array type.
>> >> >> +      // A return type of void cannot be qualified.
>> >> >> +      if (T->isVoidType() && T.getCVRQualifiers()) {
>> >> >> +          unsigned diagID =
>> >> >> diag::warn_func_returning_qualified_void;
>> >> >
>> >> >
>> >> > Just pass the ID directly to Diag().
>> >> >
>> >> >> +          S.Diag(DeclType.Loc, diagID) << T;
>> >> >> +          D.setInvalidType(true);
>> >> >> +      }
>> >> >> +
>> >> >
>> >> >
>> >> > How about placing this check with an early return at the top of
>> >> > diagnoseIgnoredFunctionQualifiers()?
>> >> >
>> >> >>         // Do not allow returning half FP value.
>> >> >>         // FIXME: This really should be in BuildFunctionType.
>> >> >>         if (T->isHalfType()) {
>> >> >> Index: include/clang/Basic/DiagnosticSemaKinds.td
>> >> >> ===================================================================
>> >> >> --- include/clang/Basic/DiagnosticSemaKinds.td  (revision 212275)
>> >> >> +++ include/clang/Basic/DiagnosticSemaKinds.td  (working copy)
>> >> >> @@ -4160,6 +4160,8 @@
>> >> >>     def err_func_returning_array_function : Error<
>> >> >>     "function cannot return %select{array|function}0 type %1">;
>> >> >> +def warn_func_returning_qualified_void : Warning<
>> >> >> +  "return type of void cannot be qualified %0">;
>> >> >
>> >> >
>> >> > (Warnings need to have a diagnostic group / -W flag, though it
>> >> > doesn't
>> >> > matter if you go ahead and make it an error.)
>> >> >
>> >> >>   def err_field_declared_as_function : Error<"field %0 declared as a
>> >> >> function">;
>> >> >>   def err_field_incomplete : Error<"field has incomplete type %0">;
>> >> >>   def ext_variable_sized_type_in_struct : ExtWarn<
>> >> >
>> >> >
>> >> > Test case?
>> >> >
>> >> > Alp.
>> >> >
>> >> >>
>> >> >>
>> >> >> _______________________________________________
>> >> >> cfe-commits mailing list
>> >> >> cfe-commits at cs.uiuc.edu
>> >> >> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>> >> >
>> >> >
>> >> > --
>> >> > http://www.nuanti.com
>> >> > the browser experts
>> >> >
>> >
>> >
>
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 20146_return_qual_void_4.patch
Type: text/x-patch
Size: 4665 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140804/0b007a10/attachment.bin>


More information about the cfe-commits mailing list