On Dec 3, 2013, at 2:05 PM, Richard Smith <[email protected]> wrote:
> On Tue, Dec 3, 2013 at 1:46 PM, jahanian <[email protected]> wrote: > > On Dec 3, 2013, at 1:19 PM, Arthur O'Dwyer <[email protected]> wrote: > > > Richard Smith originally wrote: > >> This is a frontend bug, not an IRGen bug; the test case is ill-formed. > >> "return;" > >> can be used in a constructor or destructor, but "return > >> <expression-with-type-void>;" > >> cannot. > > > > Fariborz Jahanian wrote in response to my comments: > >> There is a big difference here. returning a non-void expression in C::C() > >> is allowed > >> under an extension warning flag (ext_return_has_expr is ExtWarn<…>) while > >> returning > >> a void expression in C::C() is always an error. > > > > Returning a non-void expression in C::C() is NOT allowed, not in any > > dialect of C++, as far as I can tell. > > The codepath under ext_return_has_expr is actually dealing with a GNU > > C (not C++) extension that allows "return void_expr();" (not > > "non_void_expr") in a function returning void. Here's how to trigger > > that diagnostic: > > > > $ clang test.c -Wpedantic -c > > test.c:1:26: warning: void function 'bar' should not return void > > expression [-Wpedantic] > > void foo(); void bar() { return foo(); } > > ^ ~~~~~ > > > > My point stands, as far as I can tell. And the reason you've had so > > much trouble understanding this code is that it's really convoluted > > and confusing! We should be trying to *simplify* it, not complicate it > > by adding EVEN MORE codepaths and inconsistent behavior. > > Code was simple enough to understand. I based my change on Richard’s comment > that this should not be allowed (-Wpedantic or not). > > void foo(); C::C() { return foo(); } > > This cannot be consolidated with a diagnostic which allows -Wpedantic warning. > So, another code path added to check for this condition. Feel free to change > it provided > unconditional error remains for above test case. > > What happens if you build this: > > int foo(); C::C() { return foo(); } > > ... for ARM, with -Wno-return-type? Does that crash too? No. this works. Problem was trying to returning a ‘void’ expression in IR. > > I was not aware that we had an extension allowing returning a (non-void) > value from a function with a 'void' return type, but it appears we do. Does > anyone know the history here? > > Since we do have this extension (and assuming we don't intend to remove it), > I agree that the return-void-expression and return-non-void-expression cases > should behave the same. Within a ctor or dtor, either neither should be > allowed as an extension or both should. > > I think we need two changes here: 1) an IRGen fix, similar to Fariborz's > original patch, but checking whether we're in a ctor/dtor rather than > checking if the returned expression has type 'void', and 2) a tweak to this > change, to treat the above case as an extension (to match our existing > behavior). sgtm. Consolidation of the warning/errors is now trivial. I will take a look when I get the chance. - Thanks, Fariborz
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
