Bug #6066: Fix "control may reach end of non-void function" warnings for clang

• Status: Closed
• Priority: Normal
• Assignee: drbrain (Eric Hodel)
• Target version: 2.0.0
• ruby -v: ruby 2.0.0dev (2012-02-23 trunk 34755) [x86_64-darwin11.3.0]
• Backport: 2.3: UNKNOWN, 2.4: UNKNOWN

[drbrain (Eric Hodel)]

=begin
clang 3.1 is more picky about "control may reach end of non-void function"

The attached patches contain add (({return Qnil; /* not reached */})) or the equivalent where the warnings occurred.
=end

[drbrain (Eric Hodel)]

Patches were not added in-order.

Note that return.combined.patch contains all the other patches grouped together

[nobu (Nobuyoshi Nakada)]

clang 3.1 is more picky about "control may reach end of non-void function"

or doesn't deal with noreturn attribute properly?

Anyway, (ID)NULL seems odd.

[drbrain (Eric Hodel)]

=begin
Yes, (ID)NULL seems odd to me too.

clang supports __builtin_unreachable():

http://clang.llvm.org/docs/LanguageExtensions.html#__builtin_unreachable

It looks like replacing (({return Qnil; /* NOT REACHED */})) lines with __builtin_unreachable() may improve optimizations with clang.

Adding configure support to detect __builltin_unreachable() looks like it is beyond my knowledge based on the __builtin_setjmp() detection, can you help me?
=end

[naruse (Yui NARUSE)]

• Status changed from Open to Closed

r34784 uses __builtin_unreachable().

[nobu (Nobuyoshi Nakada)]

• Status changed from Closed to Open

No, the commit just checks the function, and defines UNREACHABLE macro.

[drbrain (Eric Hodel)]

Thank you, I will post an updated patch using UNREACHABLE

[drbrain (Eric Hodel)]

• File return.UNREACHABLE.patch return.UNREACHABLE.patch added

This patch updates all -Wreturn-type warning locations to use the UNREACHABLE macro.

[drbrain (Eric Hodel)]

• File return.not_reached.convert.patch return.not_reached.convert.patch added

=begin
This patch updates existing (({return Qnil; /* not reached */})) with (({UNREACHABLE;}))

I did not change (({trace_en()})) in variable.c as it seems to return, I think the (({/* not reached */})) should be removed, but can you check it?
=end

[ko1 (Koichi Sasada)]

• Assignee set to nobu (Nobuyoshi Nakada)

[shyouhei (Shyouhei Urabe)]

• Status changed from Open to Assigned

[drbrain (Eric Hodel)]

Nobu, may I commit this?

[nobu (Nobuyoshi Nakada)]

• Assignee changed from nobu (Nobuyoshi Nakada) to drbrain (Eric Hodel)

Feel free.

[drbrain (Eric Hodel)]

• Status changed from Assigned to Closed
• % Done changed from 0 to 100

This issue was solved with changeset r35321.
Eric, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.

---

• encoding.c (rb_enc_codepoint_len): Use UNREACHABLE to avoid "control reaches end of non-void function" warnings. [ruby-trunk - Bug #6066]
• re.c (name_to_backref_number): ditto.
• object.c (rb_Float): ditto.
• io.c (io_readpartial): ditto.
• io.c (io_read_nonblock): ditto.
• pack.c (rb_uv_to_utf8): ditto.
• proc.c (rb_method_entry_arity): ditto.
• vm_method.c (rb_f_notimplement): ditto.
• struct.c (rb_struct_aset_id): ditto.
• class.c (rb_scan_args): ditto.
• process.c (rlimit_resource_type): ditto.
• process.c (rlimit_resource_value): ditto.
• process.c (p_uid_switch): ditto.
• process.c (p_gid_switch): ditto.
• ext/digest/digest.c (rb_digest_instance_update): ditto.
• ext/digest/digest.c (rb_digest_instance_finish): ditto.
• ext/digest/digest.c (rb_digest_instance_reset): ditto.
• ext/digest/digest.c (rb_digest_instance_block_length): ditto.
• ext/bigdecimal/bigdecimal.c (BigDecimalCmp): ditto.
• ext/dl/handle.c (rb_dlhandle_close): ditto.
• ext/tk/tcltklib.c (pending_exception_check0): ditto.
• ext/tk/tcltklib.c (pending_exception_check1): ditto.
• ext/tk/tcltklib.c (ip_cancel_eval_core): ditto.
• ext/tk/tcltklib.c (lib_get_reltype_name): ditto.
• ext/tk/tcltklib.c (create_dummy_encoding_for_tk_core): ditto.
• ext/tk/tkutil/tkutil.c (tk_hash_kv): ditto.
• ext/openssl/ossl_ssl.c (ossl_ssl_session_reused): ditto.
• ext/openssl/ossl_pkey_ec.c (ossl_ec_key_dsa_verify_asn1): ditto.
• ext/openssl/ossl_pkey_ec.c (ossl_ec_point_is_at_infinit): ditto.
• ext/openssl/ossl_pkey_ec.c (ossl_ec_point_is_on_curve): ditto.
• ext/fiddle/conversions.c (generic_to_value): ditto.
• ext/socket/raddrinfo.c (rsock_io_socket_addrinfo): ditto.
• ext/socket/socket.c (sock_s_getnameinfo): ditto.
• ext/ripper/eventids2.c (ripper_token2eventid): ditto.
• cont.c (return_fiber): ditto.
• dmydln.c (dln_load): ditto.
• vm_insnhelper.c (vm_search_normal_superclass): ditto.
• bignum.c (big_fdiv): ditto.
• marshal.c (r_symlink): ditto.
• marshal.c (r_symbol): ditto.

[drbrain (Eric Hodel)]

r35322 is also part of this bug, but I forgot to mention it in my commit message.

[nobu (Nobuyoshi Nakada)]

• Category set to misc
• Status changed from Closed to Assigned
• Target version set to 2.0.0

Thank you for the commit, and found r35322 let older compilers and non-gcc compilers warn unreachable code.
UNREACHABLE should not replace those `return's, but be appended, I think.

[nobu (Nobuyoshi Nakada)]

• % Done changed from 100 to 50

[nobu (Nobuyoshi Nakada)]

• Status changed from Assigned to Closed
• % Done changed from 50 to 100