grasph / wrapit

Automatization of C++--Julia wrapper generation
MIT License
98 stars 12 forks source link

Fix enum scoping on Julia's side. #34

Closed termi-official closed 7 months ago

termi-official commented 7 months ago

On master the generated Julia names do not follow the C++ scope. This can lead to naming conflicts in codegen. MWE

enum class A {
   C
};

enum class B {
   C
};

would lead to a conflicting method C.

This PR fixes this issue by following the C++ scoping rule.

termi-official commented 7 months ago

If this change is acceptable I will add a regression test.

grasph commented 7 months ago

Thanks Dennis for this fix. The regression test is welcome, please add it to the PR. I will then merge it.

termi-official commented 7 months ago

The new tests are green for me locally.

However, part of them fail on this PR and on master. Is this reproducible on your side?

make: Entering directory '/home/dogiermann/Repos/wrapit-fork/test/TestInheritance'
rm -f libTestInheritance/libjlTestInheritance.so libTestInheritance/src/jlTestInheritance.cxx libTestInheritance/src/jlTestInheritance.h TestInheritance/src/TestInheritance.jl -r libTestInheritance/build jlTestInheritance-report.txt libTestInheritance/build/jlTestInheritance.o  libTestInheritance/build/jlTestInheritance.d
[ -d libTestInheritance/src ] && cd libTestInheritance/src && rm -f  dbg_msg.h  generated_cxx  Wrapper.h || true
rmdir -p libTestInheritance/src TestInheritance/src 2>/dev/null || true
make all && . ./setup.sh && julia runTestInheritance.jl
make[1]: Entering directory '/home/dogiermann/Repos/wrapit-fork/test/TestInheritance'
/home/dogiermann/Repos/wrapit/build/wrapit --force TestInheritance.wit
/usr/include/wchar.h:35:10: 'stddef.h' file not found

Generated wrapper statictics
  enums:            0
  classes/structs:  6
    templates:      0
    others:         6
  class methods:    11
  field accessors:  0 getters and 0 setters
  global variable accessors: 0 getters and 0 setters
  global functions: 1

[ -d libTestInheritance/build ] || mkdir libTestInheritance/build
/home/dogiermann/Repos/spack/opt/spack/linux-manjaro21-zen/gcc-11.2.0/llvm-13.0.1-ujjeg2kym5yi5vzurktjtknit5xexaow/bin/clang++ -I'/home/dogiermann/Tools/julia-1.9.4/include/julia' -fPIC -Wno-unused-variable -Wno-unused-but-set-variable -fmax-errors=3 -I. -MMD  -c -I /home/dogiermann/.julia/artifacts/8dbf3a116981de15f84137eda9d8c478f57d07a9/include -std=c++20 -o libTestInheritance/build/jlTestInheritance.o libTestInheritance/src/jlTestInheritance.cxx
clang-13: warning: argument unused during compilation: '-fmax-errors=3' [-Wunused-command-line-argument]
/home/dogiermann/Repos/spack/opt/spack/linux-manjaro21-zen/gcc-11.2.0/llvm-13.0.1-ujjeg2kym5yi5vzurktjtknit5xexaow/bin/clang++ -L'/home/dogiermann/Tools/julia-1.9.4/lib' -Wl,--export-dynamic  -o libTestInheritance/libjlTestInheritance.so --shared -fPIC libTestInheritance/build/jlTestInheritance.o
touch TestInheritance/src/TestInheritance.jl
make[1]: Leaving directory '/home/dogiermann/Repos/wrapit-fork/test/TestInheritance'
Inheritance test: Error During Test at /home/dogiermann/Repos/wrapit-fork/test/TestInheritance/runTestInheritance.jl:5
  Got exception outside of a @test
  MethodError: no method matching f(::String)

  Closest candidates are:
    f(::Union{TestInheritance.A, CxxRef{<:TestInheritance.A}})
     @ TestInheritance ~/.julia/packages/CxxWrap/5IZvn/src/CxxWrap.jl:624
    f(::Union{Ptr{Nothing}, CxxPtr{<:TestInheritance.A}})
     @ TestInheritance ~/.julia/packages/CxxWrap/5IZvn/src/CxxWrap.jl:624

  Stacktrace:
   [1] macro expansion
     @ ~/Repos/wrapit-fork/test/TestInheritance/runTestInheritance.jl:20 [inlined]
   [2] macro expansion
     @ ~/Tools/julia-1.9.4/share/julia/stdlib/v1.9/Test/src/Test.jl:1498 [inlined]
   [3] top-level scope
     @ ~/Repos/wrapit-fork/test/TestInheritance/runTestInheritance.jl:6
   [4] include(mod::Module, _path::String)
     @ Base ./Base.jl:457
   [5] exec_options(opts::Base.JLOptions)
     @ Base ./client.jl:307
   [6] _start()
     @ Base ./client.jl:522
Test Summary:    | Pass  Error  Total  Time
Inheritance test |    6      1      7  2.1s
grasph commented 7 months ago

However, part of them fail on this PR and on master. Is this reproducible on your side? ...

It runs fine on my side. Which OS are you running on?

termi-official commented 7 months ago

Lastet Manjaro with latest linux kernel. LLVM 13 custom build via spack. LLVM and clang test suite passed so far. Julia 1.9.4.

grasph commented 7 months ago

can you try this: https://github.com/grasph/wrapit/issues/30#issuecomment-1766304144 ?

termi-official commented 7 months ago

Interesting. Tests pass now locally. I guess this was then just some faulty linked stdlib.

Edit: The other issues remain, as I already have this included in the respective .wit files.