Minor code fixes

[jarmonik]

• Removed 'register' keyword.
• fixed link warnings.
• Replaced prt() function with '&' macro.
• Set default standard to C++17

[dimitry-ishenko]

This is not related to C++20. Taking address for a temporary is invalid C++ code. It fails with GCC and Clang even with C++11 standard: https://godbolt.org/z/e1jjobY9G

MSVC simply allowed this code up until C++20. So if you want portability and being able to use other compilers, this should be reverted.

[dimitry-ishenko]

It's not faster. With /O2 flag MSVC produces identical code for both versions:

https://godbolt.org/z/jnqvMq4G3

Except this is a valid C++ code, as long as you use the address within the same expression.

[jarmonik]

Ok, I change it back. But this is just an other example of language falling apart. Need a hack to solve a very basic issue that's been working fine before. Not my fault.

[dimitry-ishenko]

@jarmonik I understand how you feel. 😏 The problem is not really the language, but rather Microsoft allowing and promoting non-standard code for many years. People latched on and got used to it.

[schnepe2]

Am I the only one that gets VERY scared when one uses this?

#define ptr &

It was defined as a macro, so its use would be clear (as you "invoke" it like a function call like auto foo = ptr(bar);)
but now every occurance of ptr will be replaced by &, which is silly - or even dangerous!
Imagine I would like to write code like this:
bool uptrend = true; ...
Sure , this example would lead to a syntax error, but imagine this:

bool u = false; // some flag with a good name 'u'
bool end = false;; // flag indicating some end-condition
bool foo = uptrend; // store current uptrend value for later use

this would give valid code

bool u = false; // some flag with a good name 'u'
bool end = false; // flag indicating some end-condition
bool foo = u&end; // !!!!

This should never be accepted. By no reason!

[jarmonik]

I'll suspend my current work in progress and start restoring that fine function immediately.

[schnepe2]

What would be the right way to adress this?
Change the signture(s) of the method(s) that are using this?
Like change

void vObject::RenderAxisVector(D3D9Pad *pSkp, const D3DXCOLOR *pColor, /*...*/);
void vObject::RenderAxisLabel(D3D9Pad *pSkp, const D3DXCOLOR *clr, /*...*/);

to use const references?

void vObject::RenderAxisVector(D3D9Pad *pSkp, const D3DXCOLOR &Color, /*...*/);
void vObject::RenderAxisLabel(D3D9Pad *pSkp, const D3DXCOLOR &clr, /*...*/);

Still code like this:

FX->SetVector(eColor, ptr(D3DXVECTOR4(0, 0, 0, 0)));

needs to be addressed as well :( hmmm

[jarmonik]

I have thought about that. But there are a lot of optional parameters and passing a NULL pointer is a common way to indicate (no data available). Which doesn't work with reference. I have googled about that many times. So, using a reference would require more complex solution to a very simple problem. I looked at the ptr() function and didn't quite follow what was it doing exactly. I don't like relaying on a hack solution but I guess there's no other choice. So, I was planning the use the ptr() anyway, a lesser bad.

Do you have any info about what's going on with the unions ? I heard that they are obsolete since C++11

[schnepe2]

Yeah, I also think the ptr() macro is currently the "least-worse" of our options.
Maybe there is a way to limit the exception to the [-fpermissive] rule to specific code-units...
Some #pragma(...) on some few files.

Do you have any info about what's going on with the unions ? I heard that they are obsolete since C++11

As far as I remember the changes will not break any existing code since they only relax current union rules.
https://en.wikipedia.org/wiki/C%2B%2B11#Unrestricted_unions

[jarmonik]

Thanks about the info. Yes, I tried the [-permissive] compile flag and it works on MSVC but GCC won't support it. Since some of us are aiming on multi-platform, we must set coding standards according to the worst compiler on markets.

[jarmonik]

There is plenty of union discussion here: #368

[schnepe2]

Regarding @dimitry-ishenko s comment further up, we could introduce some "common-often-used" VECTORS and use them.
And for those few places we need special VECTORS we still can use temporaries:
Here's my proof-of-concept (based on comment further up)

https://godbolt.org/z/hsarT8588

...just an idea 😉

[dimitry-ishenko]

I have thought about that. But there are a lot of optional parameters and passing a NULL pointer is a common way to indicate (no data available).

@jarmonik C++17 introduced std::optional for these situations, so you could do this:

#include <iostream>
#include <optional>

struct vector { double x, y, z; };

auto foo(const std::optional<vector>& v)
{
    if (v == std::nullopt)
        std::cout << "no vector" << std::endl;
    else std::cout << v->x << ' ' << v->y << ' ' << v->z << std::endl;
}

int main()
{
    foo({ });
    foo(vector{5, 6, 7});
    return 0;
}

Live example: https://godbolt.org/z/3Ma7Yofv8

Do you have any info about what's going on with the unions ? I heard that they are obsolete since C++11

There are not obsolete per se. It's just that people have been abusing them for years. Many, like you, without even realizing it. 😺

[schnepe2]

Just another idea...
Would something like this "wrapper template" be a way to go? The changes in the "original" code would be mainly deleting of the '&'...

struct vector { double x, y, z; };

struct ClassFX { // Some DirectX Class
    void MethodFoo(const vector * pV);
    void MethodBar(const vector * pV, bool b = true);
    void MethodBaz(const vector * pV, float f = 0.0);
};

template <class T>
struct ClassFXwrapper {
    T *FX = new T{};
    inline void MethodFoo(const vector V) { FX->MethodFoo(&V); }
    inline void MethodBar(const vector V, bool b = true) { FX->MethodBar(&V, b); }
    inline void MethodBaz(const vector V, float f = 0.0) { FX->MethodBaz(&V, f); }
};

int main()
{
    // --- "NEW" code style
    auto *FX = new ClassFXwrapper<ClassFX>{};
    FX->MethodFoo(vector{});
    FX->MethodBar({1.0, 1.0, 1.0}, false);
    FX->MethodBaz({0.5, 0.5, 0.5}, 12.34);

    // --- "OLD"/"current" code style
    // ClassFX *FX = new ClassFX{};
    // FX->MethodFoo(&vector{});
    // FX->MethodBar(&vector{1.0, 1.0, 1.0}, false);
    // FX->MethodBaz(&vector{0.5, 0.5, 0.5}, 12.34);
}

https://godbolt.org/z/79bGo4Go6

[jarmonik]

Just another idea... Would something like this "wrapper template" be a way to go? The changes in the "original" code would be mainly deleting of the '&'...

Maybe a working solution but it looks over complicated due to wrapper class.

[dimitry-ishenko]

I thought of different solutions, but in the end came up with the ptr() as the simplest. It's also expressive in that it tells you that you are extracting a pointer from an expression.

[schnepe2]

Fine with me 👍 If the other compilers do like it (when being "pushed to accept it" 😁)
I didn't mean to distract you two too much from the original PRs intention.

[jarmonik]

something like this could work too: But wouldn't make much difference to std::optional<> approach

#include <cmath>

struct vector3 {
	float x, y, z;
	static vector3 null;

        inline bool notNull() { return x != null.x; }
	inline bool operator!= (const vector3& f) const
	{
		return x != f.x || y != f.y || z != f.z;
	}
};

void func(double a, vector3& v)
{
	if (v != vector3::null) {
		//use v
	}
        if (v.notNull()) {
		//use v
	}
}

void testfunc()
{
	// glabally initialize null
	vector3::null = { nanf("0"), nanf("0"), nanf("0") };

	func(1.0, vector3::null);
}

[dimitry-ishenko]

something like this could work too

@jarmonik hehe, it doesn't actually... https://godbolt.org/z/Er1Mc17h8

EDIT: Basically, in C++ NaN is never equal to any value, including itself. In other words, nanf("0") != nanf("0") will always evaluate to true.

So, when you compare v != vector3::null, it will always evaluate to true, even if v is also null.

See here: https://godbolt.org/z/G9Yjsrjr3

[jarmonik]

Yeah, it looks like it fails to compare them. It works if comparing the bit-fields instead.

[dimitry-ishenko]

@jarmonik however, we can use your approach if you don't want to use std::optional. But, I would recommend using templates instead of member variables/functions to avoid code duplication. Eg, something like this:

https://godbolt.org/z/xxYsrdTjz