add -std=c++98 to compiler #9029

WalterBright · 2018-12-02T10:29:53Z

The end result is as specified in cpp98.dd. However, all this PR does is add support for the -version=cpp98 switch, and the setting of the global.params.cpp98. It will be up to later PRs to change the name mangling in dmd and change the wchar_t and WCHAR aliases in druntime.

dlang-bot · 2018-12-02T10:29:54Z

Thanks for your pull request, @WalterBright!

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub fetch digger
dub run digger -- build "master + dmd#9029"

src/dmd/cond.d

thewilsonator · 2018-12-02T10:36:44Z

src/dmd/globals.h

@@ -123,7 +123,8 @@ struct Param
    bool fix16997;      // fix integral promotions for unary + - ~ operators
                        // https://issues.dlang.org/show_bug.cgi?id=16997
    bool vsafe;         // use enhanced @safe checking
-    bool ehnogc;        // use @nogc exception handling
+    bool cpp98;             // follow C++98 type system issues rather than C++11


type system issues
type mangling rules?

It's a little more than just mangling.

changelog/cpp98.dd

thewilsonator · 2018-12-02T10:39:02Z

Looks otherwise OK, cc @ibuclaw

Geod24

This looks like a precedent. We don't use -version to enable some features so far.
E.g. all of those flags could possibly be enabled the same way, but we have a predefined switch each time.

Also this only adds a version identifier, but doesn't actually changes the mangling as the changelog advertises.

ibuclaw · 2018-12-02T15:15:26Z

As noted, I see a discrepancy between changelog and patch as well.

Shouldn't the new field be an enum to be future-proof? It was raised in the other PR that C++-20 will add char8_t, so we'll likely be adding -version=Cpp11 (or Cpp17) when the next standards bump comes around.

WalterBright · 2018-12-02T22:06:05Z

I see a discrepancy between changelog and patch as well.

I explained this in the top comment.

WalterBright · 2018-12-02T22:12:55Z

Shouldn't the new field be an enum to be future-proof? It was raised in the other PR that C++-20 will add char8_t, so we'll likely be adding -version=Cpp11 (or Cpp17) when the next standards bump comes around.

It would be trivial to do that in the future if/when it becomes necessary. My experience with future proofing in the past is that future inevitably turns out different than anticipated, making ones attempts look silly. For example, look at the future proofing Microsoft did in windows.h. It was all done completely wrong.

WalterBright · 2018-12-02T22:18:05Z

This looks like a precedent. We don't use -version to enable some features so far.
E.g. all of those flags could possibly be enabled the same way, but we have a predefined switch each time.

This was @andralex 's idea. I thought I'd run it up the flagpole and see if people like it. It provides a nice way to connect the two concepts, reduces the documentation necessary, and provides a way to add such without needing to continually invent more switches.

WalterBright · 2018-12-02T23:26:41Z

Blocking: dlang/druntime#2390

CyberShadow · 2018-12-02T23:37:04Z

This was @andralex 's idea. I thought I'd run it up the flagpole and see if people like it. It provides a nice way to connect the two concepts, reduces the documentation necessary, and provides a way to add such without needing to continually invent more switches.

I don't like it either. It mixes two different things under one lid (-version=Foo switches do nothing but enable version(Foo) blocks, except for this one Foo, which also has other side effects).

I suggest using a convention identical or similar to what C++ compilers use.

gcc and clang use -std=c++98. We can do the same (or use -cstd= in case we'll ever want to use -std= for D, but the c++ part is unambiguous enough already).

WalterBright · 2018-12-02T23:40:25Z

@CyberShadow you don't like it because you'd prefer to be compatible with the gcc/clang switches, or because you don't like the concept of using -version to set characteristics? If the latter, why?

thewilsonator · 2018-12-02T23:46:17Z

Compatibility (well, more familiarity) with gcc/clang is a bonus,.

If the latter, why?

c.f. the switches used to control the C standard library version.

TurkeyMan · 2018-12-03T02:04:09Z

changelog/cpp98.dd

+        wchar   char16_t        wchar_t         char16_t
+        dchar   char32_t        unsigned        char32_t
+        wchar_t wchar_t         wchar           wchar_t
+        WCHAR   --              wchar           wchar_t


What is wchar in C++? There's no C++ type called wchar that I'm aware of... Is it just a DMC thing?
Otherwise, this looks correct to me. And I completely endorse using WCHAR in Win32 API's.

TurkeyMan · 2018-12-03T02:14:56Z

changelog/cpp98.dd

+        wchar   unsigned short  wchar_t         wchar_t
+        dchar   wchar_t         unsigned        unsigned
+        wchar_t wchar_t         wchar_t         wchar_t
+        WCHAR   --              wchar_t         wchar_t


I think "old behaviour" is a better term to use than "Cpp98" behaviour, which feels misleading...
If we were implementing C++98 behaviour, shouldn't it look like this:

D Posix DMC++ Windows VC++ Windows wchar ** ERROR: CAN'T MANGLE ** dchar ** ERROR: CAN'T MANGLE ** wchar_t wchar_t wchar_t wchar_t WCHAR -- wchar_t wchar_t

?
There's one additional detail though; VC++ has a compiler option where you can choose to NOT mangle wchar_t and instead it is a typedef for unsigned short... I've seen code compiled that way, but not for over a decade. I recommend not making any concession for that deprecated case.

I guess the reason for that hack dates back to when people used C compilers with no mangling distinction, and in C, WCHAR is a typedef for unsigned short, so some C programmers used unsigned short directly in their code, and hence the option in VC++ appeared to mangle for compatibility with those old programs.

ERROR: CAN'T MANGLE

But it can and does mangle them for C++98, per the chart.

I agree with not supporting the old VC++ switch.

I think Manu is saying that D's wchar and dchar do not exist in C++98, which is why strictly speaking the table does not describe C++98 behavior, just the way DMD approached it.

But it can and does mangle them for C++98, per the chart.

But it's wrong, and it's always been wrong. There is no compatible mangling for wchar or dchar in C++. We've just been using an arbitrary (albeit practical) mapping.
I'm just saying that calling it "C++98" is misleading.

wchar_t is a keyword in C++98, and a distinct type, and has its own mangling. This is reflected in the chart, where wchar uses the wchar_t mangling for Windows, and dchar uses the wchar_t mangling for Posix.

where wchar uses the wchar_t mangling for Windows, and dchar uses the wchar_t mangling for Posix.

wchar and dchar have no equivalent type in C++98. I think it's improper to map them to wchar_t. I understand it's for backwards compatibility with legacy DMD behaviour, but what I'm saying is, I don't think it's correct to call that mapping "Cpp98", because that's not what it is... it's just a mapping that DMD implemented.

I would expect a mode called "C++98 compatibility" to issue a compile error on account of no counterpart type in the C++98 language. Same way we give compile errors for T[], etc.

wchar and dchar have no equivalent type in C++98.

Neither do any of the D basic types, not even int. We just look for the most pragmatic mapping.

TurkeyMan · 2018-12-03T02:16:45Z

@WalterBright Thank you for getting involved on this issue. It will be nice to finally move on it!

jacob-carlborg · 2018-12-05T19:00:55Z

So does every Phobos addition, if the added symbol happens to have a compatible signature with a function in your modules. I think code breakage due to collisions with user identifiers is unavoidable in general, and we shouldn't worry about it.

It less likely to happened with symbols in Phobos because of modules are creating a namespace. If you don't import the module with the new symbol there's no problem. This identifier is global.

WalterBright · 2018-12-05T21:42:02Z

This identifier is global.

Version identifiers are in a separate symbol table from all other symbols.

CyberShadow · 2018-12-06T00:33:39Z

It less likely to happened with symbols in Phobos because of modules are creating a namespace.

Using static / renamed imports is not really common practice in D. The problem still exists when an added symbol causes an ambiguous overload.

This identifier is global.

Probably, reserving a namespace for compiler-declared or internal version identifiers would have made sense, but it is probably too late now.

Actually, we can still follow the convention of D_Coverage etc., and name the identifier D_Cpp98 or such.

WalterBright · 2018-12-06T01:12:40Z

Probably, reserving a namespace for compiler-declared or internal version identifiers would have made sense, but it is probably too late now.

They've been in their own namespace since Day 1:

https://github.com/dlang/dmd/blob/master/src/dmd/globals.d#L213

Debug identifiers are in their own namespace as well.

CyberShadow · 2018-12-06T05:50:59Z

They've been in their own namespace since Day 1:

I meant, a separate namespace from user-declared version= or -version= identifiers, like the __ prefix for regular identifiers.

ibuclaw · 2018-12-06T09:31:09Z

Lastly, the Cpp98 is purposely looking backwards, not forwards. C++98 is not going to be changing, by definition. Cpp98 means "be compatible with the old version which will not ever change", not "be compatible with the latest version whatever that may be".

What else does this switch control other than what ABI to conform with?

(I will assume the answer is nothing then).

My concern is that ABI is only ever forward looking, not backward. This doesn't really concern user code, but internally we should treat it this way.
I read the given version example as:

version (Cpp98)
   ... C++ 98 ABI compatible ...
else
   ... Older than or newer than C++ 98 ...

My gripe with -std= is that it means "what language standard/dialect do we implement?". But we don't implement any part of C++ in the compiler, we only have an ABI compatibility layer.
C++ ABI is independent of the standards version it is supporting - each new ABI increase will make it closer to the standard it is implementing, however.

jacob-carlborg · 2018-12-06T10:33:24Z

Version identifiers are in a separate symbol table from all other symbols.

Yes. But there are no namespaces for version identifiers. If I use the Cpp98 version identifier in my code it will stop compiling after this PR. Both the version identifiers I set and those predefined by the compiler live in the same namespace.

12345swordy · 2018-12-07T16:54:53Z

We can add -std=c++11 to the compiler and have the flag be the default when compiling code.
My 2 cents.

TurkeyMan · 2018-12-07T19:16:28Z

and have the flag be the default

I think it's a mistake for the default to not match the C++ compiler... otherwise you compile a .cpp and .d file with no special arguments, and get surprise link errors.

I tried to point this out above...

12345swordy · 2018-12-07T20:03:44Z

I think it's a mistake for the default to not match the C++ compiler... otherwise you compile a .cpp and .d file with no special arguments, and get surprise link errors.

Which c++ compiler are we talking about here? What you are asking for is practically is impossible!
gcc default flag is -std=gnu++98 for version 5.X.X or earlier which later they bump it up to -std=gnu++14. You can't know that the user will use the latest compiler of gcc, which for all intent and purposes, might be using an outdated one for reasons unknown. The miss match of std flags when linking is not an issue that dmd should strive to solve.

My main concern on this PR is the lack of clarification codewise when introducing the -std=c++98 flag and not a flag for 11, 14, 17 and the upcoming 20.

If we are going to mangle via a flag then there needs to be one for every official version of cpp.

ibuclaw · 2018-12-07T20:32:26Z

Which c++ compiler are we talking about here? What you are asking for is practically is impossible!

Unless your compiler is called gdc. :-)

This is another point. Whatever proposed solution shouldn't make so that gdc is out of sync with the g++ compiler it is shipped with.

I am very much aware of dmd's problem though, and that they tend to be 2 versions behind gcc because what is current compiler and what is the compiler installed on people's systems (Redhat, Ubuntu LTS) are two different things.

12345swordy · 2018-12-07T21:43:49Z

Unless your compiler is called gdc. :-)

You know I was referring to dmd here. :p
Although I didn't know that gdc could do that already.
That is pretty impressive there.

CyberShadow · 2018-12-07T22:02:39Z

You can't know that the user will use the latest compiler of gcc, which for all intent and purposes, might be using an outdated one for reasons unknown.

I'm not following your logic.

We should match the behavior of older versions of gcc, because..?

As more time passes, there will be fewer and fewer users of old gcc, and more and more users of new versions.

The only thing that makes sense is to match the default behavior of current compiler versions. Then, assuming that the C++ and D compiler a user has installed were released around the same time, they have the highest chance that everything will work by default.

12345swordy · 2018-12-07T22:25:58Z

I'm not following your logic.

I am talking std flags here and why matching default flags of dmd with other c++ compilers is practically impossible, and not an issue that dmd needs to solve.

CyberShadow · 2018-12-07T23:32:45Z

I understood what you are talking about. I don't see how your arguments back up your suggestion, and you haven't replied to my arguments why I think your suggestion doesn't make sense.

12345swordy · 2018-12-08T02:31:24Z

I don't see how your arguments back up your suggestion,

I have encounter programmers who are stuck with using old c++ compilers in their work place. Why they are stuck with it does not matter. What matter is that they exist and they NEED to be accountable, not assume that they will use the latest compiler.

We should match the behavior of older versions of gcc, because..?

As I said before:

You can't know that the user will use the latest compiler of gcc, which for all intent and purposes, might be using an outdated one for reasons unknown.

Unless W&A write down the list of C++ compilers that dmd will support with regards to mangling , they still need to be taken into account.

As more time passes, there will be fewer and fewer users of old gcc, and more and more users of new versions.

The Latest Boost library still support certain old compilers.

CyberShadow · 2018-12-08T02:33:37Z

If the default is C++11 mode, then linking against code built with old compilers will not work unless you specify -std=c++98.

If the default is C++98 mode, then linking against code built with new compilers will not work unless you specify -std=c++11.

Why do you think we should care only about old compilers, and not new ones?

12345swordy · 2018-12-08T02:46:12Z

Why do you think we should care only about old compilers, and not new ones?

How the heck do you reach that conclusion!?
We are talking about default flags here!

CyberShadow · 2018-12-08T02:47:06Z

Right. So why do you think the default should be to support old compilers, when the number of their users will only shrink?

12345swordy · 2018-12-08T03:29:55Z

My suggestion is to introduce the c++11 flag and have it be default for dmd.
Manu thinks the default should match the c++ compiler, because he worries that user may get surprise link error.
I respond that what he is asking is impossible, as not all c++ compilers have the same default flag and the surprise linking error when running default flags is not an issue that dmd should be solving.
Even if we took your suggestion and set the default std flag to say -std=c++14 based on the recent new c++ compilers, his "surprise linking error" scenario is still plausible with old c++ compilers.
That the summary of it.

TurkeyMan · 2018-12-08T21:06:33Z

Regarding the defaults, I think sensible options exist; for GDC it should be the same as the GCC compiler of the same version. For LDC, some marriages make sense, like MSVC LDC can match MSVC's defaults sensibly.
DMD is kinda platform specific. For Win-COFF, it should match the MSVC default (which is 11), for OSX, the C++ compiler is very well-controlled, and can easily match that.
Linux is just tricky, because distros...

Or just throw it all into the wind!

Either way, it's still a level, not a set of boolean flags.

jacob-carlborg · 2018-12-09T11:49:58Z

for OSX, the C++ compiler is very well-controlled, and can easily match that.

You still don't know which version of Xcode (the compiler) the user is running. For the latest version of Xcode, Apple only supports the two latest versions of macOS. If you're running an older version of macOS you cannot run the latest version of the compiler.

DMD could in theory query the version of the compiler and adopt for that.

12345swordy · 2018-12-28T15:28:30Z

Ok, the alternative has been merged. Is there any reason why this should remain open?

PetarKirov · 2018-12-28T21:46:43Z

I believe no, thanks for the reminder!

WalterBright requested review from Geod24, ibuclaw, dnadlinger, MartinNowak and rainers as code owners December 2, 2018 10:29

This was referenced Dec 2, 2018

Fix Issue 18958 - extern(C++) wchar, dchar mangling not correct #8342

Merged

Use __c_wchar_t. dlang/druntime#2349

Closed

thewilsonator reviewed Dec 2, 2018

View reviewed changes

src/dmd/cond.d Show resolved Hide resolved

thewilsonator reviewed Dec 2, 2018

View reviewed changes

changelog/cpp98.dd Outdated Show resolved Hide resolved

WalterBright force-pushed the cpp98 branch from e2a2ae1 to a080511 Compare December 2, 2018 10:52

wilzbach added the Review:Needs Spec PR A PR updating the language specification needs to be submitted to dlang.org label Dec 2, 2018

Geod24 requested changes Dec 2, 2018

View reviewed changes

WalterBright force-pushed the cpp98 branch 2 times, most recently from a521909 to 17166c1 Compare December 2, 2018 23:06

WalterBright mentioned this pull request Dec 2, 2018

set WCHAR to wchar_t for future dlang/druntime#2390

Open

This comment has been minimized.

Sign in to view

TurkeyMan reviewed Dec 3, 2018

View reviewed changes

WalterBright force-pushed the cpp98 branch from 84c55aa to f5838d4 Compare December 18, 2018 08:16

thewilsonator mentioned this pull request Dec 24, 2018

add -std-c++= #9130

Merged

WalterBright force-pushed the cpp98 branch from f5838d4 to 3f16e11 Compare December 24, 2018 11:02

add -std=c++98 to compiler

aaadbc5

WalterBright force-pushed the cpp98 branch from 3f16e11 to aaadbc5 Compare December 24, 2018 11:07

PetarKirov closed this Dec 28, 2018

add -std=c++98 to compiler #9029

add -std=c++98 to compiler #9029

Conversation

WalterBright commented Dec 2, 2018

dlang-bot commented Dec 2, 2018

Bugzilla references

Testing this PR locally

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thewilsonator commented Dec 2, 2018

Geod24 left a comment • edited Loading

Choose a reason for hiding this comment

ibuclaw commented Dec 2, 2018 • edited Loading

WalterBright commented Dec 2, 2018

WalterBright commented Dec 2, 2018

WalterBright commented Dec 2, 2018

WalterBright commented Dec 2, 2018 • edited Loading

CyberShadow commented Dec 2, 2018 • edited Loading

WalterBright commented Dec 2, 2018

This comment has been minimized.

thewilsonator commented Dec 2, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TurkeyMan Dec 3, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TurkeyMan Dec 4, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TurkeyMan commented Dec 3, 2018

jacob-carlborg commented Dec 5, 2018 • edited Loading

WalterBright commented Dec 5, 2018

CyberShadow commented Dec 6, 2018

WalterBright commented Dec 6, 2018

CyberShadow commented Dec 6, 2018

ibuclaw commented Dec 6, 2018

jacob-carlborg commented Dec 6, 2018

12345swordy commented Dec 7, 2018

TurkeyMan commented Dec 7, 2018

12345swordy commented Dec 7, 2018

ibuclaw commented Dec 7, 2018

12345swordy commented Dec 7, 2018 • edited Loading

CyberShadow commented Dec 7, 2018

12345swordy commented Dec 7, 2018

CyberShadow commented Dec 7, 2018

12345swordy commented Dec 8, 2018

CyberShadow commented Dec 8, 2018

12345swordy commented Dec 8, 2018

CyberShadow commented Dec 8, 2018

12345swordy commented Dec 8, 2018

TurkeyMan commented Dec 8, 2018

jacob-carlborg commented Dec 9, 2018 • edited Loading

12345swordy commented Dec 28, 2018

PetarKirov commented Dec 28, 2018

Geod24 left a comment •

edited

Loading

ibuclaw commented Dec 2, 2018 •

edited

Loading

WalterBright commented Dec 2, 2018 •

edited

Loading

CyberShadow commented Dec 2, 2018 •

edited

Loading

TurkeyMan Dec 3, 2018 •

edited

Loading

TurkeyMan Dec 4, 2018 •

edited

Loading

jacob-carlborg commented Dec 5, 2018 •

edited

Loading

12345swordy commented Dec 7, 2018 •

edited

Loading

jacob-carlborg commented Dec 9, 2018 •

edited

Loading