Skip to content

Ensure shape_id is never used on T_IMEMO#13347

Merged
byroot merged 1 commit intoruby:masterfrom
Shopify:no-imemo-shape-id
May 15, 2025
Merged

Ensure shape_id is never used on T_IMEMO#13347
byroot merged 1 commit intoruby:masterfrom
Shopify:no-imemo-shape-id

Conversation

@casperisfine
Copy link
Contributor

It doesn't make sense to set ivars or anything shape related on a T_IMEMO.

cc @jhawthorn

@launchable-app

This comment has been minimized.

It doesn't make sense to set ivars or anything shape
related on a T_IMEMO.

Co-Authored-By: John Hawthorn <john@hawthorn.email>
@byroot byroot enabled auto-merge (rebase) May 15, 2025 14:02
@byroot byroot merged commit 60ffb71 into ruby:master May 15, 2025
82 checks passed
ivoanjo added a commit to DataDog/dd-trace-rb that referenced this pull request May 16, 2025
**What does this PR do?**

This PR fixes three issues in the profiler when used with latest
ruby-head:

1. It's no longer possible to ask the object_id from a T_IMEMO object.
   This showed up as a Ruby VM crash with an error message
   "T_IMEMO can't have an object_id".
   (See ruby/ruby#13347 for the upstream change)

2. Creating new instances of a class is now inlined into the caller,
   and there is no longer a frame to represent the new.
   This broke some of our tests that expected the stack from
   allocating an object to have the `new` at the top.
   (See ruby/ruby#13080 for the upstream change)

3. Object ids now count towards the size of objects.
   This broke some of our tests that asserted on size of profiled
   objects.
   (See ruby/ruby#13159 for the upstream change)

**Motivation:**

Fix support for Ruby 3.5.

**Additional Notes:**

N/A

**How to test the change?**

I've updated our specs to cover these changes. Unfortunately, we don't
yet test with Ruby 3.5 in CI, so you'll have to test manually if you
want to see the fixes working with 3.5.

(Note that these changes showed up after 3.5.0-preview1, so testing
on -preview1 is not enough)
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