Skip to content

Apply upstream Eigen commits e246f9cb and 43a01f06 - #152

Merged
eddelbuettel merged 2 commits into
RcppCore:fix/stanheadersfrom
SteveBronder:patch/stanheaders
Oct 7, 2026
Merged

eddelbuettel merged 2 commits into
RcppCore:fix/stanheadersfrom
SteveBronder:patch/stanheaders

Conversation

@SteveBronder

Copy link
Copy Markdown
Contributor
  • e246f9cb: Use memset in Fill.h only if !NumTraits::RequireInitialization
  • 43a01f06: Update AVX and AVX512 to support gcc < 10.1 and clang < 10

- e246f9cb: Use memset in Fill.h only if !NumTraits<Scalar>::RequireInitialization
- 43a01f06: Update AVX and AVX512 to support gcc < 10.1 and clang < 10
@eddelbuettel

eddelbuettel commented Oct 7, 2026 •

Copy link
Copy Markdown
Member

Thanks. These are the two you had mentioned.

RcppEigen already carries the first (ie e246f9cb for Fill.h) in the branch fix/stanheaders via its commit 4cf634e. I was a little late pushing the branch, but this (along with is what allowed me to avoid the segmentation fault we attribute to stan / StanHeaders eg RequireInitialization = 1 as I detailed here five days ago) is now present. I will merge that branch, and this will go to CRAN.

I did not apply the second part (ie 43a01f06) as not relevant system has gcc-10 or clang-10 as I feel that those are ancient. A quick check with Google however reveals that while e.g. Ubuntu 24.04 still "has" gcc-10, it defaults to gcc-13. Similarly, Ubuntu 22.04 defaults to gcc-11. So do you really feel we need this?

Edit: Ok, a little digging (as you had helpfully supplied the original commit sha1 in an earlier comment) reveals that this is your PR into Eigen so I guess I won't be able to argue you out of it :)

@eddelbuettel
eddelbuettel changed the base branch from master to fix/stanheaders October 7, 2026 12:15

@eddelbuettel eddelbuettel left a comment •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After aligning with my existing branch that already carries one of the two patches, we can take this. I am not convinced I feel all that strongly about support gcc-10 (ancient) and clang-10 (even way more ancient) but hey ... we can do it for one round.

I isolated your commit containing both patch (ie against Fill.h and the gcc-10/clang-10 one) into a clean diff I can carry forward if needed.

With that we can merge this against the target branch that already carried one of the two commits (after I also made a trivial edit to ChangeLog which had a trivial conflict).

@eddelbuettel
eddelbuettel merged commit 3959c8b into RcppCore:fix/stanheaders Oct 7, 2026
2 checks passed
eddelbuettel added a commit that referenced this pull request Oct 7, 2026
* Apply Eigen upstream patch for eigen_memset_helper

Source: https://gitlab.com/libeigen/eigen/-/commit/e246f9cb68d07e8f15c60a2404ac9a625c349223

* Add previous commit as diff in patches/

* Apply upstream Eigen commits e246f9cb and 43a01f06 (#152)

- [e246f9cb](https://gitlab.com/libeigen/eigen/-/commit/e246f9cb68d07e8f15c60a2404ac9a625c349223): Use memset in Fill.h only if !NumTraits<Scalar>::RequireInitialization (already present in branch)
- [43a01f06](https://gitlab.com/libeigen/eigen/-/commit/43a01f06ad52a2e07e8b522eb7106bdde5d8bb5c): Update AVX and AVX512 to support gcc < 10.1 and clang < 10

Co-authored-by: Dirk Eddelbuettel <edd@debian.org>

* Update local diff to contain both Eigen upstream commits

---------

Co-authored-by: Steve Bronder <Stevo15025@gmail.com>
@SteveBronder

Copy link
Copy Markdown
Contributor Author

I did not apply the second part (ie 43a01f06) as not relevant system has gcc-10 or clang-10 as I feel that those are ancient.

We use rocky linux at work and our default was gcc-8 until a few months ago.

Related to compiler versions I did a little writeup a bit ago for minimum compiler versions for C++20 that is in line with this convo you may find interesting. The tldr is that commercial license supported LTS OS versions have pretty old compilers so it is kind of unclear when we can fully not worry about old gcc / clang versions

https://gist.github.com/SteveBronder/166c1691c93fd84fe82e939de0d1bfcf

Edit: Ok, a little digging (as you had helpfully supplied the original commit sha1 in #143 (comment)) reveals that this is your PR into Eigen so I guess I won't be able to argue you out of it :)

To be very clear I do not want this!! 😆 . I use cpp23 for my personal projects and it is wonderful. The old compiler support is more of a "do be how it is" sort of thing

@eddelbuettel

eddelbuettel commented Oct 7, 2026 •

Copy link
Copy Markdown
Member

We use rocky linux at work and our default was gcc-8 until a few months ago.

Talk to your labour relations board, I believe this to be a war crime you can bring to The Hague. Seriously: WTF?

Related to compiler versions I did a little writeup a bit ago for minimum compiler versions for C++20

Will take a look. My most recent bug bear that R Core laid an egg with the combination of a) defaulting to C++20 (which came from the direction of Oxford, UK) and b) insisting that macOS supports an SDK / version from a while back (this one coming from Auckland, NZ). Together they create a real issue for more aggressive and modern use of C++20 because it is quite easy to say 'compiler supports standard XYZ' and then to eg forgot you also need a library up to snuff. I suspect your write-up goes there too. There are a bunch of packages failing because of that at the current CRAN set of machines. "Not great, Bob!"

To be very clear I do not want this!! 😆

I expressed myself poorly -- I meant to stress it as 'if it was important enough for you to bring it to Eigen you likely want it here too'. And it is there now. As you may have seen I aligned the different branches and PRs, merged yours into mine and that one into master. So RcppEigen has that in its main branch. R-universe runs a little slower these days so updates are not within the hour (one can always 'force' one) but I expect by end of day you should see binary packages. They will have version 0.4.9.9-5 as I had used -4 for some tests and wanted a clean step up.

So all good on my end. Now we're waiting on StanHeaders but I am sure we will hear from @bgoodri about it...

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.

2 participants