Skip to content

Commit

Permalink
[GR-21322] Type C extensions' VALUE as unsigned long like MRI.
Browse files Browse the repository at this point in the history
PullRequest: truffleruby/1449
  • Loading branch information
eregon committed Mar 19, 2020
2 parents a15e7ab + d892f7d commit 0552ac7
Show file tree
Hide file tree
Showing 15 changed files with 61 additions and 128 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ Bug fixes:

Compatibility:

* The C API type `VALUE` is now defined as `unsigned long` as on MRI. This enables `switch (VALUE)` and other expressions which rely on `VALUE` being an integer type (#1409, #1541, #1675, #1917, #1954).
* Implemented `Float#{floor, ceil}` with `ndigits` argument.
* Implemented `Thread#fetch`.
* Implemented `Float#truncate` with `ndigits` argument.
Expand Down
16 changes: 6 additions & 10 deletions doc/user/compatibility.md
Original file line number Diff line number Diff line change
Expand Up @@ -231,26 +231,22 @@ experimental `--backtraces-omit-unused` option.

## C Extension Compatibility

#### `VALUE` is a pointer

In TruffleRuby `VALUE` is a pointer type (`void *`) rather than a
integer type (`long`). This means that `switch` statements cannot be
done using a raw `VALUE` as they can with MRI. You can normally
replace any `switch` statement with `if` statements with little
difficulty if required.

#### Identifiers may be macros or functions

Identifiers which are normally macros may be functions, functions may be macros,
and global variables may be macros. This may cause problems where they are used
in a context which relies on a particular implementation (e.g., taking the
address of it, assigning to a function pointer variable and using defined() to
address of it, assigning to a function pointer variable and using `defined()` to
check if a macro exists). These issues should all be considered bugs and be
fixed, please report these cases.

#### `rb_scan_args`

`rb_scan_args` only supports up to ten pointers.
`rb_scan_args` only supports up to 10 pointers.

#### `rb_funcall`

`rb_funcall` only supports up to 15 arguments.

#### mark functions of `RDATA` and `RTYPEDDATA`

Expand Down
16 changes: 4 additions & 12 deletions lib/cext/include/ruby/ruby.h
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ typedef char ruby_check_sizeof_voidp[SIZEOF_VOIDP == sizeof(void*) ? 1 : -1];
#define PRIxVALUE PRI_VALUE_PREFIX"x"
#define PRIXVALUE PRI_VALUE_PREFIX"X"
#ifdef TRUFFLERUBY
#define PRIsVALUE "Y"
#define PRIsVALUE "P"
#else
#define PRIsVALUE PRI_VALUE_PREFIX"i" RUBY_PRI_VALUE_MARK
#endif
Expand Down Expand Up @@ -267,7 +267,7 @@ typedef char ruby_check_sizeof_voidp[SIZEOF_VOIDP == sizeof(void*) ? 1 : -1];
#define FIXNUM_MAX RUBY_FIXNUM_MAX
#define FIXNUM_MIN RUBY_FIXNUM_MIN

#define RB_INT2FIX(i) (VALUE)((((unsigned long)(i)) << 1) | RUBY_FIXNUM_FLAG)
#define RB_INT2FIX(i) (((VALUE)(i))<<1 | RUBY_FIXNUM_FLAG)
#define INT2FIX(i) RB_INT2FIX(i)
#define RB_LONG2FIX(i) (VALUE)rb_tr_longwrap((long)(i))
#define LONG2FIX(i) RB_INT2FIX(i)
Expand Down Expand Up @@ -477,7 +477,7 @@ enum ruby_special_consts {
#endif
#define SYMBOL_FLAG RUBY_SYMBOL_FLAG

#define RB_TEST(v) !(((unsigned long)(v) & ~((unsigned long)RUBY_Qnil)) == 0)
#define RB_TEST(v) !(((VALUE)(v) & (VALUE)~RUBY_Qnil) == 0)
#define RB_NIL_P(v) !((VALUE)(v) != RUBY_Qnil)
#define RTEST(v) RB_TEST(v)
#define NIL_P(v) RB_NIL_P(v)
Expand Down Expand Up @@ -1163,11 +1163,7 @@ struct rb_data_type_struct {
const rb_data_type_t *parent;
void *data; /* This area can be used for any purpose
by a programmer who define the type. */
#ifdef TRUFFLERUBY
unsigned long flags;
#else
VALUE flags; /* RUBY_FL_WB_PROTECTED */
#endif
};

#define HAVE_TYPE_RB_DATA_TYPE_T 1
Expand All @@ -1177,11 +1173,7 @@ struct rb_data_type_struct {
struct RTypedData {
struct RBasic basic;
const rb_data_type_t *type;
#ifdef TRUFFLERUBY
int typed_flag; /* 1 or not */
#else
VALUE typed_flag; /* 1 or not */
#endif
void *data;
};

Expand Down Expand Up @@ -1539,7 +1531,7 @@ rb_obj_write(VALUE a, VALUE *slot, VALUE b, RB_UNUSED_VAR(const char *filename),
*slot = b;

#if USE_RGENGC
rb_obj_written(a, Qundef /* ignore `oldv' now */, b, filename, line);
rb_obj_written(a, RUBY_Qundef /* ignore `oldv' now */, b, filename, line);
#endif
return a;
}
Expand Down
9 changes: 6 additions & 3 deletions lib/cext/include/ruby/thread_native.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,12 @@ typedef union rb_thread_lock_union {

#elif defined(HAVE_PTHREAD_H)
#include <pthread.h>
typedef void *rb_nativethread_id_t;
typedef void *rb_nativethread_lock_t;

typedef pthread_t rb_nativethread_id_t;
#ifdef TRUFFLERUBY
typedef VALUE rb_nativethread_lock_t;
#else
typedef pthread_mutex_t rb_nativethread_lock_t;
#endif

#else
#error "unsupported thread type"
Expand Down
4 changes: 2 additions & 2 deletions lib/cext/include/truffleruby/truffleruby-pre.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ extern "C" {

// Value types

typedef void *VALUE;
typedef VALUE ID;
typedef unsigned long VALUE;
typedef unsigned long ID;

// Support

Expand Down
10 changes: 1 addition & 9 deletions lib/cext/include/truffleruby/truffleruby.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,11 @@ NORETURN(VALUE rb_f_notimplement(int args_count, const VALUE *args, VALUE object
NORETURN(void rb_tr_error(const char *message));
void rb_tr_log_warning(const char *message);
#define rb_tr_debug(args...) polyglot_invoke(RUBY_CEXT, "rb_tr_debug", args)
long rb_tr_obj_id(VALUE object);
void rb_tr_object_hidden_var_set(VALUE object, const char *name, VALUE value);
VALUE rb_tr_object_hidden_var_get(VALUE object, const char *name);
int rb_tr_obj_equal(VALUE first, VALUE second);
int rb_tr_flags(VALUE value);
void rb_tr_add_flags(VALUE value, int flags);
bool rb_tr_hidden_p(VALUE value);


// Managed Structs

Expand Down Expand Up @@ -66,11 +63,6 @@ bool rb_tr_obj_taintable_p(VALUE object);
bool rb_tr_obj_tainted_p(VALUE object);
void rb_tr_obj_infect(VALUE a, VALUE b);

#define Qfalse_int_const 0
#define Qtrue_int_const 2
#define Qnil_int_const 4
int rb_tr_to_int_const(VALUE value);

int rb_tr_readable(int mode);
int rb_tr_writable(int mode);

Expand All @@ -88,7 +80,7 @@ if (polyglot_as_boolean(polyglot_invoke(RUBY_CEXT, "warn?"))) { \
if (polyglot_as_boolean(polyglot_invoke(RUBY_CEXT, "warning?"))) { \
RUBY_INVOKE(rb_mKernel, "warn", rb_sprintf(FORMAT, ##__VA_ARGS__)); \
} \
})
})

#define rb_tr_scan_args_1(ARGC, ARGV, FORMAT, V1) rb_tr_scan_args(ARGC, ARGV, FORMAT, V1, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL)
#define rb_tr_scan_args_2(ARGC, ARGV, FORMAT, V1, V2) rb_tr_scan_args(ARGC, ARGV, FORMAT, V1, V2, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL)
Expand Down
11 changes: 0 additions & 11 deletions lib/truffle/truffle/cext.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1981,17 +1981,6 @@ def NATIVE_RSTRING_PTR(string)
Primitive.string_pointer_to_native(string)
end

def rb_tr_obj_id(object)
id = object.object_id

# This method specifically returns a long for everyday practicality - so return a sentinel value if it's out of range
if Truffle::Type.fits_into_long?(id) && id >= 0
id
else
0x0101010101010101
end
end

def rb_java_class_of(object)
Truffle::Debug.java_class_of(object)
end
Expand Down
13 changes: 0 additions & 13 deletions lib/truffle/truffle/patches/nokogiri_patches.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,6 @@

class NokogiriPatches

XML_NODE_SET_PATCH = <<-EOF
switch (rb_tr_to_int_const(rb_range_beg_len(arg, &beg, &len, (long)node_set->nodeNr, 0))) {
case Qfalse_int_const:
break;
case Qnil_int_const:
EOF

NOKOGIRI_DEALLOC_DECL_ORIG = <<-EOF
static int dealloc_node_i(xmlNodePtr key, xmlNodePtr node, xmlDocPtr doc)
{
Expand All @@ -34,19 +27,13 @@ class NokogiriPatches
gem: 'nokogiri',
patches: {
'xml_node_set.c' => [
{
match: /[[:blank:]]*?switch\s*?\(.*?Qnil:/m,
replacement: XML_NODE_SET_PATCH
},
{ # Nokogiri declares the function with more arguments than it
# is called with. This works on MRI but causes an error in
# TruffleRuby.
match: 'static VALUE to_array(VALUE self, VALUE rb_node)',
replacement: 'static VALUE to_array(VALUE self)'
},
],
'xml_io.c' => [
],
'xslt_stylesheet.c' => [
{ # It is not currently possible to pass var args from native
# functions to sulong, so we work round the issue here.
Expand Down
18 changes: 0 additions & 18 deletions lib/truffle/truffle/patches/pg_patches.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,6 @@
# Tested with pg version 0.21.0
class PgPatches

PG_BINARY_ENCODER_PATCH = <<-EOF
switch(rb_tr_to_int_const(value)){
case Qtrue_int_const : mybool = 1; break;
case Qfalse_int_const : mybool = 0; break;
EOF

PG_TEXT_DECODER_FREE_OLD = <<-EOF
static VALUE
pg_text_dec_bytea(t_pg_coder *conv, const char *val, int len, int tuple, int field, int enc_idx)
Expand All @@ -33,12 +27,6 @@ class PgPatches
PATCHES = {
gem: 'pg',
patches: {
'pg_binary_encoder.c' => [
{
match: /[[:blank:]]*?switch\s*?\(.*?Qfalse\s*?:.*?break;/m,
replacement: PG_BINARY_ENCODER_PATCH
}
],
'pg_result.c' => [
{
match: 'xmalloc(',
Expand Down Expand Up @@ -67,12 +55,6 @@ class PgPatches
replacement: 'if(TYPE(*intermediate) == T_FIXNUM && NUM2LL(*intermediate) != -(NUM2LL(*intermediate)))'
},
],
'pg_type_map_by_class.c' => [
{
match: '#define CACHE_LOOKUP(this, klass) ( &this->cache_row[(klass >> 8) & 0xff] )',
replacement: '#define CACHE_LOOKUP(this, klass) ( &this->cache_row[(unsigned long)(rb_tr_obj_id(klass)) & 0xff] )'
},
],
}
}
end
5 changes: 1 addition & 4 deletions spec/truffleruby.mspec
Original file line number Diff line number Diff line change
Expand Up @@ -75,10 +75,7 @@ class MSpecScript
set :library_cext, library_cext_specs

set :capi, [
"spec/ruby/optional/capi",

# switch (VALUE) is not yet supported
"^spec/ruby/optional/capi/language_spec.rb",
"spec/ruby/optional/capi"
]

set :truffle, [
Expand Down
30 changes: 5 additions & 25 deletions src/main/c/cext/ruby.c
Original file line number Diff line number Diff line change
Expand Up @@ -1299,10 +1299,6 @@ void rb_tr_add_flags(VALUE value, int flags) {
}
}

bool rb_tr_hidden_p(VALUE value) {
return false;
}

#undef rb_enc_str_new
VALUE rb_enc_str_new(const char *ptr, long len, rb_encoding *enc) {
return RUBY_INVOKE(rb_str_new(ptr, len), "force_encoding", rb_enc_from_encoding(enc));
Expand Down Expand Up @@ -1845,18 +1841,6 @@ void rb_bug(const char *fmt, ...) {
rb_tr_error("rb_bug not yet implemented");
}

int rb_tr_to_int_const(VALUE value) {
if (value == Qfalse) {
return Qfalse_int_const;
} else if (value == Qtrue) {
return Qtrue_int_const;
} else if (value == Qnil) {
return Qnil_int_const;
} else {
return 8;
}
}

VALUE rb_enumeratorize(VALUE obj, VALUE meth, int argc, const VALUE *argv) {
return RUBY_CEXT_INVOKE("rb_enumeratorize", obj, meth, rb_ary_new4(argc, argv));
}
Expand Down Expand Up @@ -2357,14 +2341,14 @@ VALUE rb_range_new(VALUE beg, VALUE end, int exclude_end) {
*/
int rb_range_values(VALUE range, VALUE *begp, VALUE *endp, int *exclp) {
if (!rb_obj_is_kind_of(range, rb_cRange)) {
if (!RTEST(RUBY_INVOKE(range, "respond_to?", rb_intern("begin")))) return Qfalse_int_const;
if (!RTEST(RUBY_INVOKE(range, "respond_to?", rb_intern("end")))) return Qfalse_int_const;
if (!RTEST(RUBY_INVOKE(range, "respond_to?", rb_intern("begin")))) return Qfalse;
if (!RTEST(RUBY_INVOKE(range, "respond_to?", rb_intern("end")))) return Qfalse;
}

*begp = RUBY_INVOKE(range, "begin");
*endp = RUBY_INVOKE(range, "end");
*exclp = (int) RTEST(RUBY_INVOKE(range, "exclude_end?"));
return Qtrue_int_const;
return Qtrue;
}

VALUE rb_range_beg_len(VALUE range, long *begp, long *lenp, long len, int err) {
Expand Down Expand Up @@ -2970,10 +2954,6 @@ void rb_tr_log_warning(const char *message) {
RUBY_CEXT_INVOKE_NO_WRAP("rb_tr_log_warning", rb_str_new_cstr(message));
}

long rb_tr_obj_id(VALUE object) {
return polyglot_as_i64(RUBY_CEXT_INVOKE_NO_WRAP("rb_tr_obj_id", object));
}

VALUE rb_java_class_of(VALUE obj) {
return RUBY_CEXT_INVOKE("rb_java_class_of", obj);
}
Expand Down Expand Up @@ -5005,8 +4985,8 @@ void rb_tr_init(void *ruby_cext) {

#ifdef __APPLE__
printf_domain = new_printf_domain();
register_printf_domain_function(printf_domain, 'Y', rb_tr_fprintf_value, rb_tr_fprintf_value_arginfo, NULL);
register_printf_domain_function(printf_domain, 'P', rb_tr_fprintf_value, rb_tr_fprintf_value_arginfo, NULL);
#else
register_printf_specifier('Y', rb_tr_fprintf_value, rb_tr_fprintf_value_arginfo);
register_printf_specifier('P', rb_tr_fprintf_value, rb_tr_fprintf_value_arginfo);
#endif
}
4 changes: 0 additions & 4 deletions src/main/c/openssl/ossl.c
Original file line number Diff line number Diff line change
Expand Up @@ -576,11 +576,7 @@ ossl_dyn_destroy_callback(struct CRYPTO_dynlock_value *l, const char *file, int
static void ossl_threadid_func(CRYPTO_THREADID *id)
{
/* register native thread id */
#ifdef TRUFFLERUBY
CRYPTO_THREADID_set_pointer(id, (void *)rb_tr_obj_id(rb_nativethread_self()));
#else
CRYPTO_THREADID_set_pointer(id, (void *)rb_nativethread_self());
#endif
}

static struct CRYPTO_dynlock_value *ossl_locks;
Expand Down
3 changes: 0 additions & 3 deletions src/main/c/syslog/syslog.c
Original file line number Diff line number Diff line change
Expand Up @@ -320,8 +320,6 @@ static VALUE mSyslog_log(int argc, VALUE *argv, VALUE self)
return self;
}

#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wformat"
/* Returns an inspect() string summarizing the object state.
*/
static VALUE mSyslog_inspect(VALUE self)
Expand All @@ -338,7 +336,6 @@ static VALUE mSyslog_inspect(VALUE self)
syslog_facility,
syslog_mask);
}
#pragma GCC diagnostic pop

/* Returns self, for backward compatibility.
*/
Expand Down
4 changes: 2 additions & 2 deletions src/main/c/zlib/zlib.c
Original file line number Diff line number Diff line change
Expand Up @@ -847,7 +847,7 @@ zstream_discard_input(struct zstream *z, long len)
{
if (NIL_P(z->input)) {
}
else if (rb_tr_hidden_p(z->input)) {
else if (RBASIC_CLASS(z->input) == 0) {
/* hidden, we created z->input and have complete control */
char *ptr;
long oldlen, newlen;
Expand Down Expand Up @@ -883,7 +883,7 @@ zstream_discard_input(struct zstream *z, long len)
static void
zstream_reset_input(struct zstream *z)
{
if (!NIL_P(z->input) && rb_tr_hidden_p(z->input)) {
if (!NIL_P(z->input) && RBASIC_CLASS(z->input) == 0) {
rb_str_resize(z->input, 0);
}
else {
Expand Down
Loading

0 comments on commit 0552ac7

Please sign in to comment.