Release 8.0.1 Staging #38
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Eliminate numerous warnings caused by the deprecated presence of an 'll' qualifier in the "%p" format specifier. Syntax changes introduced by c++11 require that the string literals in a string concatentation (e.g, "abc" "def") be separated by a space (see the changes to VkRunTimes.h).
Format field lenght when parsing an 'unsigned int' should be '%u', not '%lu'.
GCC 6.3 (at least) no longer generates code that compares 'this' pointers to 'null'. While it's trivially easy to invoke a non-virtual member through using a 'null' pointer, the standard considers this to be 'undefined' behavior, so GCC 6.3 silently refuses to generate code for the test. While there may be clever ways to circumvent the compiler on the matter, the safest approach is to reorganize the code to give the compiler what it wants to see. This commit does that by relocated the required tests in the case of the implementation of 'VSymbolBinding::level ()' in the backend.
GCC 6.3 (at least) no longer generates code that compares 'this' pointers to 'null'. While it's trivially easy to invoke a non-virtual member through using a 'null' pointer, the standard considers this to be 'undefined' behavior, so GCC 6.3 silently refuses to generate code for the test. While there may be clever ways to circumvent the compiler on the matter, the safest approach is to reorganize the code to give the compiler what it wants to see. This commit does that by relocated the required tests in the case of the implementation of 'rtVECTOR_PMRDC::Append ()' in the backend.
Not only do recent c++ compilers (g++ 6.3) refuse to emit code to test if a 'this' pointer is null, in some cases, that refusal is not easily detectable by syntactic examination alone, as the changes submitted here attest.
Because of the number of code paths that pass through this particular null test, the changes commited here preserve the test by changing its form (never let it be said that even the most opinionated compiler can't be outsmarted :-)
And correct a field specifier in a 'printf' format string.
(Improving code encapsulation in the process).
No evidence that this is ever invoked via a null pointer.
No evidence that this is ever invoked via a null pointer.
In the process of adding appropriately placed guards in VkStreamAgent, this commit also removes long (#if 0) excluded code from an abandoned and unsupported implementation of OpenVision (which is also, for all practical purposes abandoned).
Code audit showed they weren't needed so simple removal was all that was needed.
This is probably the trickiest of the refactorings needed to eliminate null testing 'this' pointers. As is the case for all such refactorings, either the null test must be proven unnecessary or the required test must be moved to the callers of the routines from which the test must be eliminated. In the case of these changes, the test is needed and must be moved. Fortunately, the affected code paths in this case filtered through relatively few routines that all used an implicit 'Maybe' functional implementation pattern. Test deck and bootstrap tests passed; however, more in-the-wild testing of this particular set of changes is in order.
Add a file missing from commit d16fb4a.
Specifically, the incorporator, viewndf, viewseg, and vquery.
As much as can be built on Windows is built on Windows, including some very old and deprecated programs (e.g., vquery). Additionally, several projects that build Vca command line tools and samples (vcatool, vpassthru, vca_sample_*) were missing their required dependency on VcaMain. While surprising, build order differences probably masked that omission in earlier testing (as long as VcaMain had been built before any of the affected projects were linked, there was no error to report).
(Fixing a couple of format warnings not already caught by g++ on Linux).
Motivated by the fact that the newest Linux header files no longer define SIGxxx macros for signals that Linux does not support (e.g, SIGEMT, SIGBUS, SIGWINDOW, ...). As a added benefit, this is a better pattern for dealing with this on other platforms too.
To better detect compiler changes and developing issues, change our Linux g++ compiler options to warn all (-Wall) with selective exceptions. Eventually, the list of exceptions should be reduced as need, curiosity, and motivation require and/or permit.
As tempting as it was to have a 'no-comment' option in the build, it really was the easiest of all warnings to remove.
<editorial-comment> For no particularly good reason, </editorial-comment> there are Vision methods lurking in unit tests (and perhaps other places) that attempt to determine platform type by examining 'Utility hostId'. When they find a host id of zero, they take it to mean they are running on VMS. Unfortunately, 'hostid' is routinely zero on the Mac, where it can be explcitly set (unlikely) but is otherwise deprecated. The result is that those unit tests (IVR) do nothing (no errors, no problems, just plain nothing) on the Mac. To make those tests work again, make sure 'Utility hostId' NEVER returns zero on the Mac.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
A pull request to address changes in the current generation of c++ compilers (g++ versions 6 and 7, in particular). Because of the better code analysis done by these compilers (e.g., printf style format / argument type matching analysis), many of these compiler changes result in more and better warnings. Unfortunately, other changes result in code that no longer works. Specifically, as noted in issue #35, the newest versions of g++ (versions 6 and up) as well as potentially other c++ compilers silently refuse to emit code to test the nullness of this pointers. This pull request addresses the resulting issues and some of the opportunities for improvement in the context of the release 8.0.x code line.
The code submitted here passes basic database bootstrap and other unit tests; however, additional examination of the code and in-the-wild testing is still in order.