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.

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

Add rudimentary bindings to std::unordered_map #662

Merged
merged 1 commit into from
Mar 26, 2020

Conversation

AndrejMitrovic
Copy link
Contributor

@Geod24 there seems to be a linking issue when I change cpp_unordered_map_create to return a proper type rather than just a void*. See all the // todo comments.

.dub/build/unittest-unittest-linux.posix-x86_64-dmd_2090-B745E6F9C1062E73969E8A9A1EE52EEC/agora-unittests.o: In function `_D4scpd3Cpp__T13unordered_mapTiTiZQu6createFNaNbNiNeZSQCaQBy__TQBxTiTiZQCf':
/mnt/c/dev/agora/source/scpd/Cpp.d:158: undefined reference to `std::unordered_map<int, int>* cpp_unordered_map_create<int, int>()'

@AndrejMitrovic AndrejMitrovic added the type-feature An addition to the system introducing new functionalities label Mar 16, 2020
@codecov
Copy link

codecov bot commented Mar 16, 2020

Codecov Report

Merging #662 into v0.x.x will increase coverage by 0.03%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           v0.x.x     #662      +/-   ##
==========================================
+ Coverage   90.97%   91.01%   +0.03%     
==========================================
  Files          63       63              
  Lines        4754     4761       +7     
==========================================
+ Hits         4325     4333       +8     
+ Misses        429      428       -1
Flag Coverage Δ
#integration 54.2% <ø> (+0.05%) ⬆️
#unittests 89.75% <100%> (+0.03%) ⬆️
Impacted Files Coverage Δ
source/scpd/Cpp.d 93.97% <100%> (+0.55%) ⬆️
source/agora/test/Base.d 89.39% <0%> (+0.5%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3d3c49c...9c0db4b. Read the comment docs.

@AndrejMitrovic AndrejMitrovic linked an issue Mar 16, 2020 that may be closed by this pull request
6 tasks
AndrejMitrovic added a commit to AndrejMitrovic/agora that referenced this pull request Mar 16, 2020
AndrejMitrovic added a commit to AndrejMitrovic/agora that referenced this pull request Mar 16, 2020
AndrejMitrovic added a commit to AndrejMitrovic/agora that referenced this pull request Mar 16, 2020
@Geod24
Copy link
Collaborator

Geod24 commented Mar 17, 2020

Can you paste the linking error ? Probably some bad mangling of the return value.

EDIT: 🤦‍♂

@AndrejMitrovic
Copy link
Contributor Author

It's in the PR description no?

@AndrejMitrovic
Copy link
Contributor Author

AndrejMitrovic commented Mar 17, 2020

/mnt/c/dev/agora unordered-map-bindings * $ nm .dub/build/unittest-unittest-linux.posix-x86_64-dmd_2090-B745E6F9C1062E73969E8A9A1EE52EEC/agora-unittests.o | grep unordered_map
0000000000000000 V _D44TypeInfo_S4scpd3Cpp__T13unordered_mapTiTiZQu6__initZ
0000000000000000 W _D4scpd3Cpp__T13unordered_mapTiTiZQu13opIndexAssignMFNbNiNexixiZv
0000000000000000 W _D4scpd3Cpp__T13unordered_mapTiTiZQu6createFNaNbNiNeZSQCaQBy__TQBxTiTiZQCf
0000000000000000 V _D4scpd3Cpp__T13unordered_mapTiTiZQu6__initZ
0000000000000000 W _D4scpd3Cpp__T13unordered_mapTiTiZQu6lengthMFNaNbNiNfZm
                 U _Z24cpp_unordered_map_assignIiiEvPvPKvS2_
                 U _Z24cpp_unordered_map_createIiiEPSt13unordered_mapIT_T0_Ev
                 U _Z24cpp_unordered_map_lengthIiiEmPKv

/mnt/c/dev/agora unordered-map-bindings * $ nm source/scpp/build/DUtils.o | grep unordered
0000000000000000 W _Z24cpp_unordered_map_assignIiiEvPvPKvS2_
0000000000000000 W _Z24cpp_unordered_map_assignIN7stellar9PublicKeyESt10shared_ptrINS0_12SCPQuorumSetEEEvPvPKvS7_
0000000000000000 W _Z24cpp_unordered_map_createIiiEPSt13unordered_mapIT_T0_St4hashIS1_ESt8equal_toIS1_ESaISt4pairIKS1_S2_EEEv
0000000000000000 W _Z24cpp_unordered_map_createIN7stellar9PublicKeyESt10shared_ptrINS0_12SCPQuorumSetEEEPSt13unordered_mapIT_T0_St4hashIS6_ESt8equal_toIS6_ESaISt4pairIKS6_S7_EEEv
0000000000000000 W _Z24cpp_unordered_map_lengthIiiEmPKv
0000000000000000 W _Z24cpp_unordered_map_lengthIN7stellar9PublicKeyESt10shared_ptrINS0_12SCPQuorumSetEEEmPKv
0000000000000000 W _ZNKSt13unordered_mapIiiSt4hashIiESt8equal_toIiESaISt4pairIKiiEEE4sizeEv
0000000000000000 W _ZNKSt13unordered_mapIN7stellar9PublicKeyESt10shared_ptrINS0_12SCPQuorumSetEESt4hashIS1_ESt8equal_toIS1_ESaISt4pairIKS1_S4_EEE4sizeEv
0000000000000000 W _ZNSt13unordered_mapIiiSt4hashIiESt8equal_toIiESaISt4pairIKiiEEEC1Ev
0000000000000000 W _ZNSt13unordered_mapIiiSt4hashIiESt8equal_toIiESaISt4pairIKiiEEEC2Ev
0000000000000000 n _ZNSt13unordered_mapIiiSt4hashIiESt8equal_toIiESaISt4pairIKiiEEEC5Ev
0000000000000000 W _ZNSt13unordered_mapIiiSt4hashIiESt8equal_toIiESaISt4pairIKiiEEEixERS5_
0000000000000000 W _ZNSt13unordered_mapIN7stellar9PublicKeyESt10shared_ptrINS0_12SCPQuorumSetEESt4hashIS1_ESt8equal_toIS1_ESaISt4pairIKS1_S4_EEEC1Ev
0000000000000000 W _ZNSt13unordered_mapIN7stellar9PublicKeyESt10shared_ptrINS0_12SCPQuorumSetEESt4hashIS1_ESt8equal_toIS1_ESaISt4pairIKS1_S4_EEEC2Ev
0000000000000000 n _ZNSt13unordered_mapIN7stellar9PublicKeyESt10shared_ptrINS0_12SCPQuorumSetEESt4hashIS1_ESt8equal_toIS1_ESaISt4pairIKS1_S4_EEEC5Ev
0000000000000000 W _ZNSt13unordered_mapIN7stellar9PublicKeyESt10shared_ptrINS0_12SCPQuorumSetEESt4hashIS1_ESt8equal_toIS1_ESaISt4pairIKS1_S4_EEEixERSA_

@AndrejMitrovic
Copy link
Contributor Author

I added @bug remarks about the substitution failure. Should I file an issue to https://issues.dlang.org?

@Geod24
Copy link
Collaborator

Geod24 commented Mar 17, 2020

@AndrejMitrovic : Yes please. The bug is very simple once you look at the mangling:

/// D
_Z24cpp_unordered_map_createIiiEPSt13unordered_mapIT_T0_St4hashIi**ESt8equal_toIi**ESaISt4pairIKi**i**EEEv
/// C++
_Z24cpp_unordered_map_createIiiEPSt13unordered_mapIT_T0_St4hashIS1_ESt8equal_toIS1_ESaISt4pairIKS1_S2_EEEv

@AndrejMitrovic
Copy link
Contributor Author

Filed https://issues.dlang.org/show_bug.cgi?id=20679 and referenced it in the comments.

AndrejMitrovic added a commit to AndrejMitrovic/agora that referenced this pull request Mar 18, 2020
AndrejMitrovic added a commit to AndrejMitrovic/agora that referenced this pull request Mar 18, 2020
@AndrejMitrovic
Copy link
Contributor Author

Had to continue using const void* in place of actual types due to the mangling issues which were just added to #561.

Otherwise it's ready.

@AndrejMitrovic
Copy link
Contributor Author

Rebased against master, green.

AndrejMitrovic added a commit to AndrejMitrovic/agora that referenced this pull request Mar 20, 2020
AndrejMitrovic added a commit to AndrejMitrovic/agora that referenced this pull request Mar 20, 2020
AndrejMitrovic added a commit to AndrejMitrovic/agora that referenced this pull request Mar 24, 2020
@Geod24
Copy link
Collaborator

Geod24 commented Mar 25, 2020

  1. Why the ocean update ?
  2. Can we use ref instead of pointers where possible ?

@AndrejMitrovic
Copy link
Contributor Author

Why the ocean update ?

Bad rebase, fixed.

Can we use ref instead of pointers where possible ?

Sure, I'll fix it up tomorrow.

It's used heavily in the quorum intersection
checker code which will be binded to soon.
@AndrejMitrovic
Copy link
Contributor Author

Replaced some pointers with const ref.

@Geod24 Geod24 merged commit 3a605c0 into bosagora:v0.x.x Mar 26, 2020
AndrejMitrovic added a commit to AndrejMitrovic/agora that referenced this pull request Mar 30, 2020
AndrejMitrovic added a commit to AndrejMitrovic/agora that referenced this pull request Mar 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-feature An addition to the system introducing new functionalities
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants