Skip to content

Allow constructing RMatrix from a non-const buffer - #138

Merged
kevinushey merged 2 commits into
RcppCore:masterfrom
galexv:master
Dec 4, 2020
Merged

Allow constructing RMatrix from a non-const buffer#138
kevinushey merged 2 commits into
RcppCore:masterfrom
galexv:master

Conversation

@galexv

@galexv galexv commented Dec 3, 2020

Copy link
Copy Markdown
Contributor

No description provided.

@kevinushey
kevinushey merged commit c9af2a2 into RcppCore:master Dec 4, 2020
@kevinushey

Copy link
Copy Markdown
Contributor

Thanks!

@jwood000

jwood000 commented Feb 4, 2021

Copy link
Copy Markdown

I had the same issue, however my fix was to implement a similar pattern as the more common constructor:

template <typename Source>
inline explicit RMatrix(const Source& source)
: data_(const_cast<Source&>(source).begin()),
nrow_(source.nrow()),
ncol_(source.ncol())
{
}

Thus, I implemented the following:

   inline RMatrix(const T* data, std::size_t nrow, std::size_t ncol) 
      : data_(const_cast<T*>(data)), nrow_(nrow), ncol_(ncol) 
   {
   }

I had experimented with the current fix (removing the const altogether), however I ended up using the fix above.

I'm not sure which is better, but my gut feeling is that keeping const T* with const_cast is better.

@galexv

galexv commented Feb 5, 2021

Copy link
Copy Markdown
Contributor Author

This is a possibility, but my gut feeling is just the opposite: i do not like const_casts 🙂, and for a reason. IMHO, basically, the constructor above lies to me promising to create an object that cannot change my data (via the provided pointer). For example, i could have passed a pointer to an array in some kind of read-only memory, and the compiler would not say a word, resulting in undefined behavior (UB) (see, e.g., https://stackoverflow.com/q/4042626).

Case in point: a simplified code at https://godbolt.org/z/WP5qes uses const_cast<int*> from a const-data pointer at line 30, then tries to assign to the pointed-to location (line 31), which results in a crash when compiled without optimization ("-O0"). Without the forced const_cast the corresponding piece of code (lines 17-18) produces a compilation error (as it should happen when a user tries to do something grossly incorrect).

A side note: When the program is compiled with "-O1", the compiler deduces the result without actually running any memory access code, ignoring the incorrect assignment, so we get different results at different optimization levels, as is common when a UB is invoked.

[Sorry for double posting, accidentally posted it from a temporary account 😔 ]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants