Skip to content

Comments

#449 add explicit to node constructor#484

Merged
ZigRazor merged 3 commits intoZigRazor:masterfrom
JustCallMeRay:#449-add-explicit-to-Node-constructor
Feb 3, 2025
Merged

#449 add explicit to node constructor#484
ZigRazor merged 3 commits intoZigRazor:masterfrom
JustCallMeRay:#449-add-explicit-to-Node-constructor

Conversation

@JustCallMeRay
Copy link
Contributor

@JustCallMeRay JustCallMeRay commented Jan 4, 2025

Fixes #449 by adding explicit keyword to constructor

In an ideal scenario I would write a fails to compile test. I looked for a nice way to do it with SNIAFE but it's been flaky with me in the past and I've forgotten how to.

I have considered adding try_compile to the CMake, something like:

try_compile(TEST_NAME, "TEST_NAME_File.cpp")
# ... more tests ...
configure_file("CompileTests.cpp.in")

where CompileTests.cpp.in contains something like:

constexpr bool TEST_NAMECompiles = @TEST_NAME@;
// ... and so on for all tests ... 
TEST(CompileTests, TEST_NAME) 
{
    ASSERT_FALSE(TEST_NAMECompiles);
   // Personally I wouldn't add any `ASSERT_TRUE` tests like this
}

I thought GDAL did something similar but I had a look at the source code and can't see it so I assume I've confused it with something (I had a look at geos too but nothing obvious).

@ZigRazor ZigRazor merged commit 1e6b0ce into ZigRazor:master Feb 3, 2025
3 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Node creation bug where implicit conversion happens between type std::string and double

3 participants