Skip to content

Add enumeration values for DwarfVirtuality, DIFlags, and DIEmissionKind#180

Draft
kquick wants to merge 1 commit intomasterfrom
dgb_1765144881-0
Draft

Add enumeration values for DwarfVirtuality, DIFlags, and DIEmissionKind#180
kquick wants to merge 1 commit intomasterfrom
dgb_1765144881-0

Conversation

@kquick
Copy link
Member

@kquick kquick commented Dec 7, 2025

No description provided.

@kquick kquick requested a review from RyanGlScott December 7, 2025 22:01
@kquick kquick marked this pull request as draft December 8, 2025 12:44
@kquick kquick removed the request for review from RyanGlScott December 8, 2025 12:44
, pure ("encoding:" <+> integral (dibtEncoding bt))
, (("flags:" <+>) . integral)
<$> dibtFlags bt
, (\f -> "flags:" <+> (commas (text . show <$> f))) <$> dibtFlags bt
Copy link
Collaborator

Choose a reason for hiding this comment

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

To my knowledge, this is the first time that we are relying on the output of a derived Show instance in the pretty-printing output. I think that this is fine, but we should add some comments to the relevant data type(s) saying that the names of the data constructors must match what is in LLVM exactly, as the pretty-printing code depends on this property.

-- llvm-project/llvm/include/llvm/IR/{DebugInfoMetadata.h,DebugInfoFlags.def}
type DISPFlags = [DISPFlag]
data DISPFlag
= DISPFlagNonvirtual -- DISPFlagZero -- == 0
Copy link
Collaborator

Choose a reason for hiding this comment

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

Where is DISPFlagNonvirtual defined on the LLVM side? If I'm reading this correctly, shouldn't it be named DISPFlagZero?

, pure ("isDefinition:" <+> ppBool (dispIsDefinition sp))
, pure ("scopeLine:" <+> integral (dispScopeLine sp))
, (("containingType:" <+>) . ppValMd' pp) <$> (dispContainingType sp)
, pure ("virtuality:" <+> integral (dispVirtuality sp))
Copy link
Collaborator

Choose a reason for hiding this comment

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

The virtuality field is now being displayed as spflags. Is this a change that was introduced in a particular LLVM version? Does the pretty-printing code need to do different things depending on the LLVM version?

, pure ("align:" <+> integral (dictAlign ct))
, (("offset:" <+>) . ppSizeOrOffsetValMd' pp) <$> dictOffset ct
, pure ("flags:" <+> integral (dictFlags ct))
, pure ("flags:" <+> (commas (text . show <$> (dictFlags ct))))
Copy link
Collaborator

Choose a reason for hiding this comment

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

These parentheses are redundant:

Suggested change
, pure ("flags:" <+> (commas (text . show <$> (dictFlags ct))))
, pure ("flags:" <+> (commas (text . show <$> dictFlags ct)))

Similar comments apply to other uses of text . show <$> (...) introduced in this PR.

@RyanGlScott
Copy link
Collaborator

NB: This PR would fix #32.

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