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

smart_pointer_traits.py bug #85

Closed
RomanYakovenko opened this issue Jul 18, 2017 · 9 comments
Closed

smart_pointer_traits.py bug #85

RomanYakovenko opened this issue Jul 18, 2017 · 9 comments
Labels
bug
Milestone
1.9.1

Comments

@RomanYakovenko
Copy link

Hello. Method "get_by_name" contains a small typo, which is very problematic:
"class_declaration_traits" are used twice. I believe the first time, there was intention to use "class_traits"
patch.txt

Regards,
Roman
@RomanYakovenko
Copy link
Author

It looks like I fired the issue too early, as the file contains few more. So the summary:

  1. It contains the type
  2. wrong function call, instead of class_traits.declaration_class it should call class_traits.get_declaration
  3. Modern smart pointers refer to the "pointer type" as element_type, instead of "value_type". So I changed it too.

The patch is attached
patch.txt

Regards,
Roman

@iMichka iMichka added this to the 2.0.0 milestone Aug 5, 2017
@iMichka iMichka added the bug label Aug 5, 2017
@iMichka iMichka modified the milestones: 1.9.0, 2.0.0, 1.9.1 Aug 5, 2017
@iMichka
Copy link
Contributor

iMichka commented Aug 5, 2017

I confirm that there is definitively a typo there. Happened probably during one of the last refactorings.

The bug is in a part of the code that has no code coverage. Funnily all the bigger bugs I introduced during the last years happened in places that were untested. Code coverage is pretty good, around 92%, but would need to be pushed a little bit more. I'll add a test for that function while fixing this, and make a fix for the 1.9.1 release.

@RomanYakovenko
Copy link
Author

Sorry, but this bug was not fixed. It looks like you applied my first patch and not the second one.
! Modern smart pointers refer to the "pointer type" as element_type, instead of "value_type". So I changed it too.

Thank you.

@iMichka
Copy link
Contributor

iMichka commented Aug 6, 2017

I didn't fix the bug, had no time to write the code for it on the 1.9.1 branch. I hope I can do this in the next days.

@iMichka
Copy link
Contributor

iMichka commented Aug 13, 2017

Okay I pushed the changes to the 1.9.1 branch (https://github.com/gccxml/pygccxml/tree/hotfix/v1.9.1). I added also some tests for all the methods.

Interestingly they work locally for me on mac, and on the mac builds. But the Linux and windows builds all failed:
https://travis-ci.org/gccxml/pygccxml/builds/264160493

I think this has to do with the value_type / element_type change. I will need to investigate the produced xml files.

======================================================================
ERROR: test_smart_pointer_value_type (unittests.test_smart_pointer.Test)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/travis/build/gccxml/pygccxml/unittests/test_smart_pointer.py", line 90, in test_smart_pointer_value_type
    vt = declarations.smart_pointer_traits.value_type(decls[0].decl_type)
  File "/home/travis/build/gccxml/pygccxml/pygccxml/declarations/smart_pointer_traits.py", line 75, in value_type
    return internal_type_traits.get_by_name(type_, "element_type")
  File "/home/travis/build/gccxml/pygccxml/pygccxml/declarations/smart_pointer_traits.py", line 23, in get_by_name
    cls.typedef(name, recursive=False).decl_type)
  File "/home/travis/build/gccxml/pygccxml/pygccxml/declarations/scopedef.py", line 1184, in typedef
    recursive=recursive)
  File "/home/travis/build/gccxml/pygccxml/pygccxml/declarations/scopedef.py", line 484, in _find_single
    found = matcher.get_single(decl_matcher, decls, False)
  File "/home/travis/build/gccxml/pygccxml/pygccxml/declarations/scopedef.py", line 87, in get_single
    raise runtime_errors.declaration_not_found_t(decl_matcher)
pygccxml.declarations.runtime_errors.declaration_not_found_t: Unable to find declaration. Matcher: [(decl type==typedef_t) and (name==element_type)]

@RomanYakovenko
Copy link
Author

RomanYakovenko commented Aug 14, 2017 via email

iMichka added a commit that referenced this issue Aug 14, 2017
@iMichka
Look for element_type typedef in base classes (#85)
76bf57c 
For the yes1 variable from the test_smart_pointer test, we get

- with llvm/clang on mac:

<Variable id="_1012" name="yes1" type="_1554" init="new int{6}" context="_1" location="f118:11" file="f118" line="11" mangled="_Z4yes1"/>
<Class id="_1554" name="shared_ptr&lt;int&gt;" context="_7" location="f46:3868" file="f46" line="3868" members="_7239 _7240 _7241 _7242 _7243 _7244 _7245 _7246 _7247 _7248 _7249 _7250 _7251 _7252 _7253 _7254 _7255 _7256 _7257 _7258 _7259" befriending="" size="128" align="64"/>
<Typedef id="_7239" name="element_type" type="_2541" context="_1554" access="public" location="f46:3871" file="f46" line="3871"/>

- with gcc on Linux:

<Variable id="_977" name="yes1" type="_1024" init="new int{6}" context="_1" location="f44:11" file="f44" line="11" mangled="_Z4yes1"/>
<Class id="_1024" name="shared_ptr&lt;int&gt;" context="_7" location="f51:93" file="f51" line="93" members="_3644 _3645 _3646 _3647 _3648 _3649 _3650" bases="_1039" befriending="_1025" size="128" align="64">
    <Base type="_1039" access="public" virtual="0" offset="0"/>
</Class>
<Class id="_1039" name="__shared_ptr&lt;int, __gnu_cxx::_Lock_policy::_S_atomic&gt;" context="_7" location="f42:867" file="f42" line="867" members="_3711 _3712 _3713 _3714 _3715 _3716 _3717 _3718 _3719 _3720 _3721 _3722 _3723 _3724 _3725 _3726 _3727 _3728 _3729 _3730 _3731" befriending="_1040" size="128" align="64"/>
<Typedef id="_3711" name="element_type" type="_3121" context="_1039" access="public" location="f42:874" file="f42" line="874"/>

In the second case, we need to loop over the base class
and check the members of that class. The member _3711
is the one we are looking for. Without this change the value_type
methods were not able to find the typedef.
@iMichka
Copy link
Contributor

iMichka commented Aug 14, 2017

In fact I had to loop over the base class to find the "element_type" typedef. I made a patch for that: 76bf57c

There is some xml in the commit message which explains why this is like this.

If this is fine for you, I'll make a new release tomorrow with these changes.

@RomanYakovenko
Copy link
Author

RomanYakovenko commented Aug 15, 2017 via email •
edited
Loading

@iMichka
Copy link
Contributor

iMichka commented Aug 15, 2017

I answered in the other issue for the "incomplete declarations tree" bug.

I just merged the 1.9.1 branch to master. I will push the 1.9.1 tag once the unit tests are done running on Travis. Of course if you want to work with the bleeding edge version, you can use the develop branch, which also contains the fixes.

@iMichka iMichka closed this as completed Aug 15, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
1.9.1
Development

No branches or pull requests
2 participants
@iMichka @RomanYakovenko