Headers #undef Windows-defined definitions which can be needed

[amokhuginnsson]

Headers #undef Windows-defined definitions which can be needed

[fnxweb]

Yes, you're right, I see now! Will test on Monday.

[AmokHuginnsson]

Hello.

I have put those #undefs explicitly because either replxx or some replxx client did not build on Windows.
You are right, #undef in replxx.h is unnecessary (and will be removed).

[fnxweb]

Sorry, that wasn't quite my point. The Windows SDK creates the following definitions:

WinNT.h

//  The following are masks for the predefined standard access types
#define DELETE                           (0x00010000L)

WinGDI.h

/* Region Flags */
#define ERROR               0

So the issue is that, yes, if replxx wants to use those tokens it can undefine them, but then they're not available to client code that's included those headers. That's why locally I've renamed those two entities in replxx. It's a breaking change to replxx, but I can't see a way for it to keep those entity names, and still let client code access those values from the Windows SDK.

I mean, it's up to you. I'm happy to keep local changes for those lines — I have another couple of local tweaks anyway¹, I just thought I ought to mention it.

¹ — which I may raise as enhancement reqs. a little later. One is to add declspec tags to allow use in a Windows DLL; the other is a shut-down problem that I have to investigate bit more, to see whether the issue I've fixed is unique to our codebase or more common. Actually, there's a third issue about use in a Windows GUI app that creates its own console window. I may raise the simple two shortly.

[AmokHuginnsson]

Do you actually use directly ERROR or DELETE from Windows headers?

[fnxweb]

Yes, that's how we found the issue.

That said, I think only ERROR caused a problem for us, then while looking for it I saw the DELETE one.

[fnxweb]

Honestly, I don't have an axe to grind, you can resolve this as you see fit — I've patched it locally, I'm just letting you know as it's technically wrong.

But then so is riddling system headers with #defines covering common keywords.

[AmokHuginnsson]

Thank you for your report.

Unfortunately I cannot in good conscience rename those conflicting identifiers
as the error is in my opinion in Windows headers.
With current approach replxx is at least usable on Windows without changes in the library code,
though as you report there are cases when client code breaks.

BTW

If I had encountered it myself I would probably try fix it by something like:

#ifdef ERROR
enum { ERROR_VAL = ERROR };
#undef ERROR
enum { ERROR = ERROR_VAL };
#endif 

Before including replxx.hxx header.

[AmokHuginnsson]

If the code above would fix it in your case I am ready to put this code into replxx.hxx header

[fnxweb]

It does look like that should resolve the ERROR issue, I'll try it later.
Not sure about DELETE (which, as I said, I don't think we've hit), as I suspect it has to have your specific value for key matching? I can't check the code ATM.
That macro does feel more esoteric, though.

[fnxweb]

Actually, no, don't do that: ERROR is defined in Windows to be 0, so the replxx ERROR/ERROR_VAL would then be zero and clash with BLACK.

[AmokHuginnsson]

Well I do not know what you mean.
I see no way for my workaround to clash with BLACK.
I want to place the workaround code at the very top of replxx.hxx header.
I mean before namespace replxx { line.

And by workaround code I mean code with enum trick.
Can you please check if adding this code (where I said) fixes the issue for you?

[fnxweb]

BLACK is zero.

#ifdef ERROR
enum { ERROR_VAL = ERROR };    // sets ERROR_VAL = 0
#undef ERROR
enum { ERROR = ERROR_VAL };    // sets ERROR = 0
#endif 

Now ERROR == BLACK == 0. Is that not going to cause problems?

[AmokHuginnsson]

With proposed fix only ERROR from global namespace is 0, Replxx::Color::ERROR is -2.

Can you please actually test if my solution fixes the problem for you?

[fnxweb]

(I'm not normally that dense, honest!)

[fnxweb]

I final got someone to test that fix against our SW (it's in a sub-system I'm not involved with). It does fix the problem, but with a caveat: I was wrong earlier in that it's DELETE that caused us a problem, not ERROR. So, I just replicated your fix above for that.

For completeness, my local copy of replxx.hxx now has the following, with the undefs removed, and it works (Linux [RHEL7] and Windows [VS2015]):

#ifdef ERROR
enum { ERROR_VAL = ERROR };    // sets ERROR_VAL = 0
#undef ERROR
enum { ERROR = ERROR_VAL };    // sets ERROR = 0
#endif
#ifdef DELETE
enum { DELETE_VAL = DELETE };  // sets DELETE_VAL = 0
#undef DELETE
enum { DELETE = DELETE_VAL };  // sets DELETE = 0
#endif