Can we stop using ALLOCA_N in C++?
okkez opened this issue · 3 comments
Background
I could not compile nmatrix with recent Ruby (2.6.0-preview1 and r63103)
I've tried to compile nmatrix with recent Ruby, but I could not compile it.
See also https://bugs.ruby-lang.org/issues/14668 (Japanese)
shyouhei fixes the issue in r63123 today.
What is the problem
Shyouhei said that we can stop using alloca
in C++ code in the comment.
How do you think about this?
@okkez Thank you so much for bringing this to our attention.
I read through a translation of the linked page, and wasn't totally clear: what is preferred in place of ALLOCA_N
in C++?
I'm not familiar with C++ specification and nmatrix code, so I could not describe the possibility that we can use C++ functionality more.
It is a general consideration.
VALEU *slice_argv = NM_ALLOCA_N(VALUE, dim);
can replace with
VALUE slice_argv[dim];
This is the simplest case in nmatrix.
For example in other C++ code,
diff --git a/ext/nmatrix/math.cpp b/ext/nmatrix/math.cpp
index 880c1cd..459a3fd 100644
--- a/ext/nmatrix/math.cpp
+++ b/ext/nmatrix/math.cpp
@@ -848,28 +848,29 @@ static VALUE nm_cblas_rot(VALUE self, VALUE n, VALUE x, VALUE incx, VALUE y, VAL
rb_raise(nm_eDataTypeError, "this operation undefined for integer vectors");
return Qfalse;
} else {
- void *pC, *pS;
-
// We need to ensure the cosine and sine arguments are the correct dtype -- which may differ from the actual dtype.
if (dtype == nm::COMPLEX64) {
- pC = NM_ALLOCA_N(float,1);
- pS = NM_ALLOCA_N(float,1);
+ float pC[1];
+ float pS[1];
rubyval_to_cval(c, nm::FLOAT32, pC);
rubyval_to_cval(s, nm::FLOAT32, pS);
+ ttable[dtype](FIX2INT(n), NM_STORAGE_DENSE(x)->elements, FIX2INT(incx), NM_STORAGE_DENSE(y)->elements, FIX2INT(incy), pC, pS);
} else if (dtype == nm::COMPLEX128) {
- pC = NM_ALLOCA_N(double,1);
- pS = NM_ALLOCA_N(double,1);
+ double pC[1];
+ double pS[1];
rubyval_to_cval(c, nm::FLOAT64, pC);
rubyval_to_cval(s, nm::FLOAT64, pS);
+ ttable[dtype](FIX2INT(n), NM_STORAGE_DENSE(x)->elements, FIX2INT(incx), NM_STORAGE_DENSE(y)->elements, FIX2INT(incy), pC, pS);
} else {
+ void *pC, *pS;
pC = NM_ALLOCA_N(char, DTYPE_SIZES[dtype]);
pS = NM_ALLOCA_N(char, DTYPE_SIZES[dtype]);
rubyval_to_cval(c, dtype, pC);
rubyval_to_cval(s, dtype, pS);
+ ttable[dtype](FIX2INT(n), NM_STORAGE_DENSE(x)->elements, FIX2INT(incx), NM_STORAGE_DENSE(y)->elements, FIX2INT(incy), pC, pS);
}
- ttable[dtype](FIX2INT(n), NM_STORAGE_DENSE(x)->elements, FIX2INT(incx), NM_STORAGE_DENSE(y)->elements, FIX2INT(incy), pC, pS);
return Qtrue;
}
From the linked page:
As an aside, in C ++, there is no necessity to use alloca. Usually VALUE slice_argv[dim]; should be okay. C is also so after C99. I am aware that Ruby still uses alloca because it sticks to the old C.
I think this not totally correct, variable length arrays are part of C99, but not part of any C++ spec, although they are supported by g++.