Skip to content

Demangle explicitly named object parameters #299

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

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Demangle explicitly named object parameters #299

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
107 changes: 86 additions & 21 deletions src/ast.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -556,6 +556,8 @@ where
// argument pack.
is_template_argument_pack: bool,

is_explicit_obj_param: bool,

// Whether to show function parameters.
// This must be set to true before calling `demangle` on `Encoding`
// unless that call is via the toplevel call to `MangledName::demangle`.
Expand Down Expand Up @@ -615,6 +617,7 @@ where
is_template_prefix: false,
is_template_prefix_in_nested_name: false,
is_template_argument_pack: false,
is_explicit_obj_param: false,
show_params: !options.no_params,
show_return_type: !options.no_return_type,
show_expression_literal_types: !options.hide_expression_literal_types,
Expand Down Expand Up @@ -1037,6 +1040,12 @@ where
}

let mut need_comma = false;

// Only the first param should have `this`.
if ctx.is_explicit_obj_param {
write!(ctx, "this ")?;
}

for arg in self.iter() {
if need_comma {
write!(ctx, ", ")?;
Expand Down Expand Up @@ -1938,19 +1947,28 @@ impl<'a> GetLeafName<'a> for UnscopedTemplateName {
/// ```text
/// <nested-name> ::= N [<CV-qualifiers>] [<ref-qualifier>] <prefix> <unqualified-name> E
/// ::= N [<CV-qualifiers>] [<ref-qualifier>] <template-prefix> <template-args> E
/// ::= N H <prefix> <unqualified-name> E
/// ::= N H <template-prefix> <template-args> E
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since these variants don't admit the CvQualifiers or RefQualifier I think it would be better to create new NestedName variants for them.


/// ```
#[derive(Clone, Debug, PartialEq, Eq)]
pub enum NestedName {
/// A nested name.
Unqualified(
CvQualifiers,
Option<RefQualifier>,
Option<ExplicitObjectParameter>,
PrefixHandle,
UnqualifiedName,
),

/// A nested template name. The `<template-args>` are part of the `PrefixHandle`.
Template(CvQualifiers, Option<RefQualifier>, PrefixHandle),
Template(
CvQualifiers,
Option<RefQualifier>,
Option<ExplicitObjectParameter>,
PrefixHandle,
),
}

impl Parse for NestedName {
Expand All @@ -1963,19 +1981,28 @@ impl Parse for NestedName {

let tail = consume(b"N", input)?;

let (cv_qualifiers, tail) =
if let Ok((q, tail)) = try_recurse!(CvQualifiers::parse(ctx, subs, tail)) {
(q, tail)
} else {
(Default::default(), tail)
};
let (cv_qualifiers, ref_qualifier, explicit_obj_param, tail) = match tail.peek() {
Some(b'H') => {
let (explicit_obj_param, tail) = ExplicitObjectParameter::parse(ctx, subs, tail)?;
(Default::default(), None, Some(explicit_obj_param), tail)
}
_ => {
let (cv_qualifiers, tail) =
if let Ok((q, tail)) = try_recurse!(CvQualifiers::parse(ctx, subs, tail)) {
(q, tail)
} else {
(Default::default(), tail)
};

let (ref_qualifier, tail) =
if let Ok((r, tail)) = try_recurse!(RefQualifier::parse(ctx, subs, tail)) {
(Some(r), tail)
} else {
(None, tail)
};
let (ref_qualifier, tail) =
if let Ok((r, tail)) = try_recurse!(RefQualifier::parse(ctx, subs, tail)) {
(Some(r), tail)
} else {
(None, tail)
};
(cv_qualifiers, ref_qualifier, None, tail)
}
};

let (prefix, tail) = PrefixHandle::parse(ctx, subs, tail)?;
let tail = consume(b"E", tail)?;
Expand All @@ -1988,11 +2015,17 @@ impl Parse for NestedName {

match substitutable {
Some(&Substitutable::Prefix(Prefix::Nested(ref prefix, ref name))) => Ok((
NestedName::Unqualified(cv_qualifiers, ref_qualifier, prefix.clone(), name.clone()),
NestedName::Unqualified(
cv_qualifiers,
ref_qualifier,
explicit_obj_param,
prefix.clone(),
name.clone(),
),
tail,
)),
Some(&Substitutable::Prefix(Prefix::Template(..))) => Ok((
NestedName::Template(cv_qualifiers, ref_qualifier, prefix),
NestedName::Template(cv_qualifiers, ref_qualifier, explicit_obj_param, prefix),
tail,
)),
_ => Err(error::Error::UnexpectedText),
Expand All @@ -2017,12 +2050,20 @@ impl NestedName {
}
}

/// Get the ref-qualifier for this name, if one exists.
pub fn has_explicit_obj_param(&self) -> bool {
match *self {
NestedName::Unqualified(_, _, Some(ref _r), ..)
| NestedName::Template(_, _, Some(ref _r), ..) => true,
_ => false,
}
}
// Not public because the prefix means different things for different
// variants, and for `::Template` it actually contains part of what
// conceptually belongs to `<nested-name>`.
fn prefix(&self) -> &PrefixHandle {
match *self {
NestedName::Unqualified(_, _, ref p, _) | NestedName::Template(_, _, ref p) => p,
NestedName::Unqualified(_, _, _, ref p, _) | NestedName::Template(_, _, _, ref p) => p,
}
}
}
Expand All @@ -2039,7 +2080,7 @@ where
let ctx = try_begin_demangle!(self, ctx, scope);

match *self {
NestedName::Unqualified(_, _, ref p, ref name) => {
NestedName::Unqualified(_, _, _, ref p, ref name) => {
ctx.push_demangle_node(DemangleNodeType::NestedName);
p.demangle(ctx, scope)?;
if name.accepts_double_colon() {
Expand All @@ -2048,13 +2089,17 @@ where
name.demangle(ctx, scope)?;
ctx.pop_demangle_node();
}
NestedName::Template(_, _, ref p) => {
NestedName::Template(_, _, _, ref p) => {
ctx.is_template_prefix_in_nested_name = true;
p.demangle(ctx, scope)?;
ctx.is_template_prefix_in_nested_name = false;
}
}

if self.has_explicit_obj_param() {
ctx.is_explicit_obj_param = true;
Copy link
Collaborator

Choose a reason for hiding this comment

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

If you're going to track this state on the context it will have to be cleared somewhere, right?

}

if let Some(inner) = ctx.pop_inner() {
inner.demangle_as_inner(ctx, scope)?;
}
Expand All @@ -2075,7 +2120,7 @@ where
impl GetTemplateArgs for NestedName {
fn get_template_args<'a>(&'a self, subs: &'a SubstitutionTable) -> Option<&'a TemplateArgs> {
match *self {
NestedName::Template(_, _, ref prefix) => prefix.get_template_args(subs),
NestedName::Template(_, _, _, ref prefix) => prefix.get_template_args(subs),
_ => None,
}
}
Expand All @@ -2084,10 +2129,10 @@ impl GetTemplateArgs for NestedName {
impl<'a> GetLeafName<'a> for NestedName {
fn get_leaf_name(&'a self, subs: &'a SubstitutionTable) -> Option<LeafName<'a>> {
match *self {
NestedName::Unqualified(_, _, ref prefix, ref name) => name
NestedName::Unqualified(_, _, _, ref prefix, ref name) => name
.get_leaf_name(subs)
.or_else(|| prefix.get_leaf_name(subs)),
NestedName::Template(_, _, ref prefix) => prefix.get_leaf_name(subs),
NestedName::Template(_, _, _, ref prefix) => prefix.get_leaf_name(subs),
}
}
}
Expand Down Expand Up @@ -4018,6 +4063,19 @@ define_vocabulary! {
}
}

define_vocabulary! {
/// A <ref-qualifier> production.
///
/// ```text
/// <ref-qualifier> ::= R # & ref-qualifier
/// ::= O # && ref-qualifier
Copy link
Collaborator

Choose a reason for hiding this comment

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

The description here needs to be updated.

/// ```
#[derive(Clone, Debug, PartialEq, Eq)]
pub enum ExplicitObjectParameter {
ExplicitObjectParameter(b"H", "this")
}
}

define_vocabulary! {
/// A one of the standard variants of the <builtin-type> production.
///
Expand Down Expand Up @@ -8538,6 +8596,7 @@ mod tests {
Ok => {
b"NS0_3abcE..." => {
Name::Nested(NestedName::Unqualified(CvQualifiers::default(),
None,
None,
PrefixHandle::BackReference(1),
UnqualifiedName::Source(SourceName(Identifier {
Expand Down Expand Up @@ -8660,6 +8719,7 @@ mod tests {
const_: true,
},
Some(RefQualifier::RValueRef),
None,
PrefixHandle::BackReference(0),
UnqualifiedName::Source(
SourceName(Identifier {
Expand All @@ -8677,6 +8737,7 @@ mod tests {
const_: false,
},
Some(RefQualifier::RValueRef),
None,
PrefixHandle::BackReference(0),
UnqualifiedName::Source(
SourceName(Identifier {
Expand All @@ -8694,6 +8755,7 @@ mod tests {
const_: false,
},
None,
None,
PrefixHandle::BackReference(0),
UnqualifiedName::Source(
SourceName(Identifier {
Expand All @@ -8711,6 +8773,7 @@ mod tests {
const_: true,
},
Some(RefQualifier::RValueRef),
None,
PrefixHandle::NonSubstitution(NonSubstitution(0))),
b"...",
[
Expand All @@ -8732,6 +8795,7 @@ mod tests {
const_: false,
},
Some(RefQualifier::RValueRef),
None,
PrefixHandle::NonSubstitution(NonSubstitution(0))),
b"...",
[
Expand All @@ -8753,6 +8817,7 @@ mod tests {
const_: false,
},
None,
None,
PrefixHandle::NonSubstitution(NonSubstitution(0))),
b"...",
[
Expand Down
11 changes: 11 additions & 0 deletions tests/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -617,6 +617,17 @@ demangles!(
"deluge::gui::menu_item::MasterTranspose::MenuItem(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&)"
);

demangles!(_ZNH1S3fooES_, "S::foo(this S)");

demangles!(_ZNH1S3barILi5EiEEvS_T0_, "void S::bar<5, int>(this S, int)");

demangles!(_ZNH1S3bazERKS_, "S::baz(this S const&)");

demangles!(
_ZZNH2ns3Foo3fooES0_iENH4Foo24foo2EOKS1_,
"ns::Foo::foo(this ns::Foo, int)::Foo2::foo2(this Foo2 const&&)"
);

// This symbol previously ran into some mutual recursion and unbounded growth of the substitution table.
// See <https://github.com/gimli-rs/cpp_demangle/issues/277> and <https://github.com/getsentry/symbolic/issues/477>
#[test]
Expand Down