search

Brewtarget / brewtarget

Main brewtarget source code repository.
GNU General Public License v3.0
313 stars 134 forks source link

Error in ObjectStoreTyped.cpp when Compiling on Windows 10 under Visual Studio 2022 #742

Open Lubart opened 1 year ago

Lubart commented 1 year ago

Error: 4>D:\Git\brewtarget\src\database\ObjectStoreTyped.cpp(585,5): error C2908: explicit specialization; '`anonymous-namespace'::PRIMARY_TABLE' has already been instantiated

matty0ung commented 1 year ago

I'm guessing the Microsoft compiler doesn't like us referring to the address of PRIMARY_TABLE<Recipe> inside the constructor to PRIMARY_TABLE<Recipe>. Need to have a think about an elegant way to solve this. Open to suggestions!

Lubart commented 1 year ago

What is the purpose of ancestor_id? Maybe that linkage should be stored in separate table?

matty0ung commented 1 year ago

What is the purpose of ancestor_id? Maybe that linkage should be stored in separate table?

It's for versioning of recipes, a rather neat feature written by @mikfire.

I'd be a little reluctant to change the DB schema just to get things working on the MS compiler. It seems like a sledgehammer to crack a nut, especially as it's a C++ compile error rather than an inherent problem with the DB schema.

What I don't know, because I'm not a C++ language lawyer, is whether the MS compiler is, in this instance, being "strictly correct" or "unnecessarily inflexible". It felt to me like the code is well-defined / unambiguous, and the two compilers we do nightly builds on (GCC and Clang) both seem to understand it, so I was leaning towards MS compiler being "unnecessarily inflexible". But I'm always happy to be corrected. Still learning things about C++!

If the MS compiler really is being "strictly correct" here, then my first thought had been to use some sort of special value that means "this table". We can't use std::nullptr for that, because it already means "no foreign key", but I would probably start looking to see if std::variant could help us.

Realistically, given that there is a working build on Windows (using GCC and MSYS2), any non-trivial changes to get things also building with MS are going to wait until after BeerJSON is done. (Unless someone else wants to make a PR.) Don't get me wrong, I am in favour of building with the MS compiler too, because it will help find errors and warnings that the other compilers might miss and should, generally, make the code better. It's just a question of prioritising what's on the "to do" list.

Lubart commented 1 year ago

Yes. I agree with you. No need to change schema of program or DB just for specific compiler. I just don't know yet the full structure of program. But it looks very promising to me as for home brewer. I'm also looking for some of MSVC compiler "options", it is strange why it tries to build it differently.