Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix/gcc9 cpp17 support #99

Merged
merged 10 commits into from
Apr 10, 2021
Merged

Fix/gcc9 cpp17 support #99

merged 10 commits into from
Apr 10, 2021

Conversation

sodero
Copy link
Contributor

@sodero sodero commented Feb 1, 2021

These changes in combination with an updated clib2 enables full C++11/17/2a support. It's of course not optimal to use a clib2 fork, but I figured it's better than to fork both adtools and clib2. The updated clib2 also paves the way for integrating the patchset for gcc10 that currently lives in my fork only.

This solves #96 #94 #93 #91

#define _GLIBCXX_USE_C99_STDINT_TR1 1
#define _GLIBCXX_USE_C99 1
-/* Temporary until newlib behaves properly */
-#undef __STRICT_ANSI__
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you remove the addition of these lines from the original patch? That is, adding these lines in the same patch series just to remove them seems to redudant. Also note that the #undef is needed also for newlib (your comment just talks about newlib). Generally, the #undef is only a temporary solution, that state should be reflected in the patch/comment.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I found out that I did not submit the review. Not sure if you saw it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll fix that. I wasn't sure how much 'paper trail' to produce. When looking at the original patch (0011) my guess is that we can skip it completely, the new newlib (!?) implements strtold and so does the proposed clib2.

Copy link
Contributor Author

@sodero sodero Feb 15, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I forgot; I think newlib behaves the way it should. It hides everything >C89 when __STRICT_ANSI__ is set, just like glibc The question is why __STRICT_ANSI__ is set during configuration. Either way, just like you say, the undef is temporary, but I don't think newer versions of newlib will solve the problem.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIRC __STRICT_ANSI__ "just" hides any gnu extensions of the C standard at the currently selected C level in the sense of the synax of the language. I'm not sure if shall have implications of the defined functions or at least it should not be the only parameter. This probably was a workaround because newlib defined some function in sope of __STRICT_ANSI__ that were present in c11, but c11 implies __STRICT_ANSI__ and so configure did not produce the desired results. That does not mean that the behaviour is the expected one but this is how I observed it.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll fix that. I wasn't sure how much 'paper trail' to produce. When looking at the original patch (0011) my guess is that we can skip it completely, the new newlib (!?) implements strtold and so does the proposed clib2.

At least for newlib, we have to support the older version for quite a while until new newlib and SDK is circulated.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIRC STRICT_ANSI "just" hides any gnu extensions of the C standard at the currently selected C level in the sense of the synax of the language. I'm not sure if shall have implications of the defined functions or at least it should not be the only parameter.

I don't think it's just about extensions. From the gcc documentation:

__STRICT_ANSI__
GCC defines this macro if and only if the -ansi switch, or a -std switch specifying strict conformance to some version of ISO C, was specified when GCC was invoked. It is defined to `1'. This macro exists primarily to direct GNU libc's header files to restrict their definitions to the minimal set found in the 1989 C standard.

There's always something hidden in there to cause confusion, 'primarily', what other use cases are there? :-)

At least for newlib, we have to support the older version for quite a while until new newlib and SDK is circulated.

Yes, bits and pieces from all over the place is not a good idea. I really don't understand why they don't open up newlib.

Copy link
Owner

@sba1 sba1 Feb 15, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIRC STRICT_ANSI "just" hides any gnu extensions of the C standard at the currently selected C level in the sense of the synax of the language. I'm not sure if shall have implications of the defined functions or at least it should not be the only parameter.

I don't think it's just about extensions. From the gcc documentation:

STRICT_ANSI
GCC defines this macro if and only if the -ansi switch, or a -std switch specifying strict conformance to some version of ISO C, was specified when GCC was invoked. It is defined to `1'. This macro exists primarily to direct GNU libc's header files to restrict their definitions to the minimal set found in the 1989 C standard.

There's always something hidden in there to cause confusion, 'primarily', what other use cases are there? :-)

Yes, I'm aware of this but following my observation I ignored it. I always had the feeling this was old documentation (especially the last part, the first part IMO holds true) because it does not reflect what gcc does at the moment. My bet was is that the usage of the macro was extended and the last part was just an example. One would have too look at the history of gcc and the gnu libc to see when this confusion started. My observation was based on:

 $ gcc -std=c++17 -x c++ -dM -E - < /dev/null | grep STRICT

or

 $ gcc -std=c11 -dM -E - < /dev/null | grep STRICT

in contrast to

 $ gcc -std=gnu++17 -x c++ -dM -E - < /dev/null | grep STRICT

or

 $ gcc -std=gnu11 -dM -E - < /dev/null | grep STRICT

(on an amd64 glibc-based linux system).

Note that apparently __STRICT_ANSI___ is also defined in C++. All in all I believe that the real fix (if you want to call it that way) should be done in the respective clib files, but temporarily it cal be addressed via patches.

Copy link
Contributor Author

@sodero sodero Feb 16, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting, thanks for the info.

Copy link
Contributor Author

@sodero sodero Feb 16, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I fixed the __STRICT_ANSI__ issue and bumped clib2 while I was at it. It contains a couple of fixes and some improvements in C99 compatibility, tgmath, iso646 and partial wchar support.

@sodero sodero force-pushed the fix/gcc9_cpp17_support branch from b943f16 to 2f10946 Compare February 16, 2021 17:28
@kas1e
Copy link

kas1e commented Apr 10, 2021

So, seems Sebastian has for real no more time for all that, if it takes months, and it means it better to works now on separate forks.

@sba1 sba1 merged commit 4628866 into sba1:master Apr 10, 2021
@sba1
Copy link
Owner

sba1 commented Apr 10, 2021

Sorry, I missed it. As gcc 9 support is already present and the merge significantly improves it, I merged it.

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.

5 participants