Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Drop fake DW_TAG_expression #23154

Closed
dexonsmith mannequin opened this issue Mar 4, 2015 · 4 comments
Closed

Drop fake DW_TAG_expression #23154

dexonsmith mannequin opened this issue Mar 4, 2015 · 4 comments
Assignees
@dexonsmith
Labels
bugzilla Issues migrated from bugzilla debuginfo

Comments

@dexonsmith
Copy link
Mannequin

dexonsmith mannequin commented Mar 4, 2015

Bugzilla Link 22780
Resolution FIXED
Resolved on Aug 29, 2017 16:00
Version trunk
OS All
Blocks #21806
CC @adrian-prantl,@dwblaikie,@echristo,@fredriss,@rnk

Extended Description

Now that we have a type hierarchy (from bug 22464), we don't need DW_TAG_expression to discriminate DWARF expressions. We can use Metadata::getMetadataID() instead.

  1. Change MDExpression::getTag() to return 0u and fix any fallout.
  2. Remove dwarf::DW_TAG_expression.
  3. Change MDExpression to inherit directly from MDNode instead of DebugNode.
  4. Investigate having it inherit directly from Metadata -- expressions don't need/use metadata operands, they're not referenced from NamedMDNode or !attachments, and they have no need for distinct/temporary semantics.
@dexonsmith
Copy link
Mannequin Author

dexonsmith mannequin commented Mar 4, 2015

assigned to @dexonsmith

@dexonsmith
Copy link
Mannequin Author

dexonsmith mannequin commented Mar 17, 2015

r232550 does steps #​2 and #​3 in one commit (I skipped step #​1).

I'll leave this open to track #​4.

@dexonsmith
Copy link
Mannequin Author

dexonsmith mannequin commented Aug 19, 2015

There aren't very many MDExpressions -- I haven't seen MDExpression in any memory profile -- so step #​4 doesn't seem urgent. Since r232550 fixed the real problem, leaving the PR around is just noise at this point. Closing.

@rnk
Copy link
Collaborator

rnk commented Aug 29, 2017

I started implementing step #​4 of making DIExpression not be an MDNode in https://reviews.llvm.org/D37281, but I'm going to abandon it. It doesn't seem worth it.

@dexonsmith dexonsmith mannequin mentioned this issue Oct 31, 2014
Closed
@llvmbot llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 9, 2021
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@dexonsmith dexonsmith

Labels
bugzilla Issues migrated from bugzilla debuginfo
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@rnk