r196296 - Issue diagnostic when constructor or destructor

[Arthur O'Dwyer]

On Tue, Dec 3, 2013 at 9:42 AM, jahanian <fjahanian at apple.com> wrote:
> On Dec 3, 2013, at 9:35 AM, Jordan Rose <jordan_rose at apple.com> wrote:
>> On Dec 3, 2013, at 9:10 , Fariborz Jahanian <fjahanian at apple.com> wrote:
>
> Author: fjahanian
> Date: Tue Dec  3 11:10:08 2013
> New Revision: 196296
>
> URL: http://llvm.org/viewvc/llvm-project?rev=196296&view=rev
> Log:
> Issue diagnostic when constructor or destructor
> return void expression. // rdar://15366494
> pr17759.
[...]
> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Tue Dec  3 11:10:08
> 2013
> @@ -6426,6 +6426,8 @@ def ext_return_has_void_expr : Extension
> def err_return_init_list : Error<
>  "%select{void function|void method|constructor|destructor}1 %0 "
>  "must not return a value">;
> +def err_ctor_dtor_returns_void : Error<
> +  "%select{constructor|destructor}1 %0 must not return void expression">;


IMO, the diagnostic's wording should be the same for

    int foo();
    struct C1 { C1() { return foo(); } };  // error: constructor 'C'
should not return a value [-Wreturn-type]

and

    void bar();
    struct C2 { C2() { return bar(); } };  // error: constructor 'C'
must not return void expression [-Wreturn-type]

I don't see any significant difference between the two cases, and it
doesn't seem like they should have two different diagnostics issued
along two different (and convoluted and difficult-to-maintain)
codepaths. A compromise would be to %select a user-friendly
clarification in the case that we're about to issue the diagnostic and
we notice that the returned expression is of type (cv-qualified) void:

    void bar();
    struct C2 { C2() { return bar(); } };  // error: constructor 'C'
should not return a value (not even of type 'void') [-Wreturn-type]

my $.02,
–Arthur