Project

General

Profile

Actions

Bug #6066

closed

Fix "control may reach end of non-void function" warnings for clang

Added by drbrain (Eric Hodel) over 12 years ago. Updated over 12 years ago.

Status:
Closed
Target version:
ruby -v:
ruby 2.0.0dev (2012-02-23 trunk 34755) [x86_64-darwin11.3.0]
Backport:
[ruby-core:42815]

Description

=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


Files

fiddle.return.patch (404 Bytes) fiddle.return.patch ext/fiddle patch drbrain (Eric Hodel), 02/23/2012 09:39 AM
ripper.return.patch (391 Bytes) ripper.return.patch ext/ripper patch drbrain (Eric Hodel), 02/23/2012 09:39 AM
socket.return.patch (774 Bytes) socket.return.patch ext/socket patch drbrain (Eric Hodel), 02/23/2012 09:39 AM
tk.return.patch (1.46 KB) tk.return.patch ext/tk patch drbrain (Eric Hodel), 02/23/2012 09:39 AM
return.combined.patch (10.4 KB) return.combined.patch Combined patch for core and ext/ drbrain (Eric Hodel), 02/23/2012 09:39 AM
core.return.patch (5.75 KB) core.return.patch Patch for C files in the project root drbrain (Eric Hodel), 02/23/2012 09:39 AM
bigdecimal.return.patch (409 Bytes) bigdecimal.return.patch ext/bigdecimal patch drbrain (Eric Hodel), 02/23/2012 09:39 AM
digest.return.patch (904 Bytes) digest.return.patch ext/digest patch drbrain (Eric Hodel), 02/23/2012 09:39 AM
dl.return.patch (354 Bytes) dl.return.patch ext/dl patch drbrain (Eric Hodel), 02/23/2012 09:39 AM
return.UNREACHABLE.patch (10.9 KB) return.UNREACHABLE.patch Updated combined patch using UNREACHABLE drbrain (Eric Hodel), 02/26/2012 01:39 PM
return.not_reached.convert.patch (10.7 KB) return.not_reached.convert.patch update /* not reached */ to UNREACHABLE drbrain (Eric Hodel), 02/26/2012 02:08 PM

Updated by drbrain (Eric Hodel) over 12 years ago

Patches were not added in-order.

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

Updated by nobu (Nobuyoshi Nakada) over 12 years ago

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.

Updated by drbrain (Eric Hodel) over 12 years ago

=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

Updated by naruse (Yui NARUSE) over 12 years ago

  • Status changed from Open to Closed

r34784 uses __builtin_unreachable().

Updated by nobu (Nobuyoshi Nakada) over 12 years ago

  • Status changed from Closed to Open

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

Updated by drbrain (Eric Hodel) over 12 years ago

Thank you, I will post an updated patch using UNREACHABLE

Updated by drbrain (Eric Hodel) over 12 years ago

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

Updated by drbrain (Eric Hodel) over 12 years ago

=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

Actions #9

Updated by ko1 (Koichi Sasada) over 12 years ago

  • Assignee set to nobu (Nobuyoshi Nakada)
Actions #10

Updated by shyouhei (Shyouhei Urabe) over 12 years ago

  • Status changed from Open to Assigned

Updated by drbrain (Eric Hodel) over 12 years ago

Nobu, may I commit this?

Updated by nobu (Nobuyoshi Nakada) over 12 years ago

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

Feel free.

Actions #13

Updated by drbrain (Eric Hodel) over 12 years ago

  • 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.

Updated by drbrain (Eric Hodel) over 12 years ago

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

Updated by nobu (Nobuyoshi Nakada) over 12 years ago

  • 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.

Updated by nobu (Nobuyoshi Nakada) over 12 years ago

  • % Done changed from 100 to 50

Updated by nobu (Nobuyoshi Nakada) over 12 years ago

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

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0