On Dec 3, 2013, at 12:00 PM, Arthur O'Dwyer <[email protected]> wrote:
> On Tue, Dec 3, 2013 at 11:27 AM, jahanian <[email protected]> wrote: >> On Dec 3, 2013, at 10:31 AM, Arthur O'Dwyer <[email protected]> >> wrote: >>> On Tue, Dec 3, 2013 at 9:42 AM, jahanian <[email protected]> wrote: >>>> On Dec 3, 2013, at 9:35 AM, Jordan Rose <[email protected]> wrote: >>>>> On Dec 3, 2013, at 9:10 , Fariborz Jahanian <[email protected]> wrote: >>>>>> URL: http://llvm.org/viewvc/llvm-project?rev=196296&view=rev >>>>>> Issue diagnostic when constructor or destructor return void expression. >>>>>> rdar://15366494 pr17759. > [...] >>> 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. >> >> Problem I faced is that under an extension warning flag, one can return a >> non-void expression in a constructor/destructor. But returning a void >> expression is always an error. >> So, consolidating both cases into one diagnostic is not feasible. Please >> look at definition of ext_return_has_expr vs err_ctor_dtor_returns_void > > (Incidentally, Fariborz, please fix your quoting. I've fixed it for you > above.) I don’t understand (fixing my quoting :). > > I don't understand what you mean by "returning a void expression is > always an error.” Please see Richard’s initial feedback when I fixed the IRGen. > The only reason it's an error in Clang *now* is because you pushed the > change that > Jordan commented on. If you revert that change, then contrariwise, > returning a void > expression from a constructor will NEVER give a diagnostic. I > suggested that a better > behavior (different from both the old behavior and the new behavior) > would be to make > > C::C() { return foo(); } > > give a consistent diagnostic no matter what the decltype of foo is. There is a big difference here. returning a none-void expression in C::C() is allowed under an extension warning flag (ext_return_has_expr is ExtWarn<…>) while retuning a void expression in C::C() is always an error. So, there is no need to fix IRGen for ARM as this construct is not allowed. Unless, of course, I mis-read Rechard’s initial feedbeck. - Fariborz > > Incidentally, I am extremely skeptical that this patch could have 100% fixed > http://llvm.org/bugs/show_bug.cgi?id=17759 > If the problem there was that the ARM backend currently has an > invariant that a ctor's AST mustn't contain "return x" for any x, then > the fix should either have been to rewrite the AST to match that > invariant (e.g. rewrite "return x" to "x; return") or else to remove > the faulty assumption from the ARM backend. Tweaking one particular > diagnostic to attempt to lock out the special case that x happens to > be (cv-qualified?) 'void' seems like the wrong approach. > > –Arthur _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
