anderkve / FYS3150

https://anderkve.github.io/FYS3150
26 stars 14 forks source link

Weird error when making wrapper class #66

Closed DavidSvejda2507 closed 1 year ago

DavidSvejda2507 commented 1 year ago

I'm having an issue writing a wrapper class for the Penning trap, I have reduced the code to this:

image

I get the following error when I try to compile:

IN: g++ Source/trapSim.cpp -o main.exe -larmadillo -I Include/

OUT:Source/trapSim.cpp: In constructor ‘TrapSim::TrapSim()’:
Source/trapSim.cpp:8:18: error: no matching function for call to ‘PenningTrap::PenningTrap()’
    8 | TrapSim::TrapSim(){
      |                  ^
In file included from Include/trapSim.hpp:6,
                 from Source/trapSim.cpp:1:
Include/penningTrap.hpp:22:9: note: candidate: ‘PenningTrap::PenningTrap(double, double, double)’
   22 |         PenningTrap(double B, double V, double d);
      |         ^~~~~~~~~~~
Include/penningTrap.hpp:22:9: note:   candidate expects 3 arguments, 0 provided
Include/penningTrap.hpp:9:7: note: candidate: ‘PenningTrap::PenningTrap(const PenningTrap&)’
    9 | class PenningTrap
      |       ^~~~~~~~~~~
Include/penningTrap.hpp:9:7: note:   candidate expects 1 argument, 0 provided
Include/penningTrap.hpp:9:7: note: candidate: ‘PenningTrap::PenningTrap(PenningTrap&&)’
Include/penningTrap.hpp:9:7: note:   candidate expects 1 argument, 0 provided

If I remove the member variable PenningTrap trap the error goes away If I add a constructor with the signature PenningTrap::PenningTrap() I get a different error:

/usr/bin/ld: /tmp/ccbXudHs.o: in function `TrapSim::TrapSim()':
trapSim.cpp:(.text+0x33c): undefined reference to `PenningTrap::PenningTrap()'
collect2: error: ld returned 1 exit status

For this it doesn't matter if the constructor does anything, in fact I get the same error whether I use just a declaration or a whole function definition.

If I replace the PenningTrap with a Particle I get the same error but with Particle::Particle() instead, if I replace it with int I get no error. I have no problems declaring a std::vector in PenningTrap.

anderkve commented 1 year ago

Hi @DavidSvejda2507!

For this it doesn't matter if the constructor does anything, in fact I get the same error whether I use just a declaration or a whole function definition.

When you added a full function definition for the PenningTrap::PenningTrap() constructor, did you add that directly in the PenningTrap.hpp header file or in the PenningTrap.cpp source file? If you added it in PenningTrap.cpp, then this build command

g++ Source/trapSim.cpp -o main.exe -larmadillo -I Include/

won't compile that constructor, and then I'm not surprised that you get an undefined reference error. The above command only compiles trapSim.cpp (+ the header file code trapSim.cpp includes via its #include statements), and then tries to build an executable main.exe from that. But then that won't contain the code from penningTrap.cpp...

What happens if you rather do

g++ Source/trapSim.cpp Source/penningTrap.cpp Source/particle.cpp  -I Include/ -larmadillo -o main.exe

(Also, where in this list of source files is the actual main function that should be run when you run main.exe? Or are there more source files missing from the build command?)

anderkve commented 1 year ago

If something is unclear about what to include and not in the single build command above, I'd recommend going back to doing compiling and linking as separate steps, since it makes it easier to track down the source of build problems. So e.g. something like this:

# First compile each cpp file:
g++ -c Source/trapSim.cpp -I Include
g++ -c Source/penningTrap.cpp -I Include
g++ -c Source/particle.cpp -I Include
g++ -c Source/main.cpp -I Include
# (etc. if you have more .cpp files.)

# Then link everything to an executable:
g++ Source/*.o -larmadillo -o main.exe
DavidSvejda2507 commented 1 year ago

Yes, Including the PenningTrap.cpp file in the compilation did fix the problem. Thanks.

I do still wonder why the PenningTrap::PenningTrap(){} constructor is necessary, because I never actually call it myself. My speculation would be that the PenningTrap field needs a default value, but I don't see why that couldn't be None,

anderkve commented 1 year ago

Yes, Including the PenningTrap.cpp file in the compilation did fix the problem. Thanks.

Good!

I do still wonder why the PenningTrap::PenningTrap(){} constructor is necessary, because I never actually call it myself. My speculation would be that the PenningTrap field needs a default value, but I don't see why that couldn't be None.

Yes, you're correct. When instances are created with the syntax PenningTrap trap, they are implicitly created using the so-called "default constructor", which is the PenningTrap::PenningTrap() constructor. If we had added no constructors to our class, then the compiler would automatically add a default constructor and you wouldn't have seen this problem. (This is the reason why there's no problem creating a vector<Particle> in the PenningTrap class, because all the Particle instances are created with their auto-generated default constructor.) But since we have explicitly added some constructors to PenningTrap, then the compiler will not automatically add a default PenningTrap() constructor, so we have to do it ourselves.

In C++ there is no None concept like there is in Python. You can achieve something similar by using pointers instead, because you can have a pointer of type PenningTrap* and set it to what's called the NULL pointer, and then only later set it to point to an actual PenningTrap instance. But we haven't discussed this in the lectures, and for the problem at hand it is much better to just create a default constructor. (Note that the default constructor doesn't have to have an empty function body. If you want your PenningTrap to have some default properties, you can set these inside the PenningTrap::PenningTrap() function body.)