IIIM-IS / AERA

Other
12 stars 4 forks source link

Reduce compiler warnings #16

Open jefft0 opened 5 years ago

jefft0 commented 5 years ago

Compiling Replicode emits 974 warnings, some of which look a little scary. This task is to discuss possible low-hanging fruit to fix some of these warnings and to improve the experience of first-time users. I did a quick tally of the types of warnings:

#   message
472 possible loss of data in automatic numeric conversion
181 signed/unsigned mismatch
147 no suitable definition provided for explicit template instantiation request
122 You _will_ get either slowness or runtime errors
15  unreferenced formal parameter
13  C++ exception handler used, but unwind semantics are not enabled. Specify /EHsc
8   ignoring '/EDITANDCONTINUE' due to '/SAFESEH' specification:
7   not all control paths return a value
9   other
974 TOTAL

Update 2022-12-06:

#   message
198 possible loss of data in automatic numeric conversion 
99  unreferenced formal parameter
40  no suitable definition provided for explicit template instantiation request
30  C++ exception handler used, but unwind semantics are not enabled. Specify /EHsc
20  signed/unsigned mismatch 
19  local variable is initialized but not referenced
15  potentially uninitialized local variable 'write_index' used
11  declaration hides previous local declaration
7   not all control paths return a value
16  other
455 TOTAL
jefft0 commented 5 years ago

To me, the most off-putting are the 122 warnings for "You will get either slowness or runtime errors". These all come from two lines of code in CoreLibrary: https://github.com/IIIM-IS/CoreLibrary/blob/master/CoreLibrary/types.h#L138

#ifdef _MSC_VER
#if( _SECURE_SCL != 0 )
#pragma message( "Warning: _SECURE_SCL != 0. You _will_ get either slowness or runtime errors." )
#endif

#if( _HAS_ITERATOR_DEBUGGING != 0 )
#pragma message( "Warning: _HAS_ITERATOR_DEBUGGING != 0. You _will_ get either slowness or runtime errors." )
#endif
#endif

Any guess why this code is here? If not do you agree it is worth a little time to look into? (Can we ask Eric?)

thorisson commented 5 years ago

Hi Jeff happy holidays!

On 23 Dec 2018, at 11:56, Jeff Thompson notifications@github.com wrote:

To me, the most off-putting are the 122 warnings for "You will get either slowness or runtime errors". These all come from two lines of code in CoreLibrary: https://github.com/IIIM-IS/CoreLibrary/blob/master/CoreLibrary/types.h#L138 https://github.com/IIIM-IS/CoreLibrary/blob/master/CoreLibrary/types.h#L138

ifdef _MSC_VER

if( _SECURE_SCL != 0 )

pragma message( "Warning: _SECURE_SCL != 0. You will get either slowness or runtime errors." )

endif

if( _HAS_ITERATOR_DEBUGGING != 0 )

pragma message( "Warning: _HAS_ITERATOR_DEBUGGING != 0. You will get either slowness or runtime errors." )

endif

endif

hahaha! he’s such a code nazi - this could be a message to Thor List, who wrote (some of) the CoreLibrary

slowness is not that dangerous for us right now, and for the next 12-18 months, so better that than runtime errors. (Not sure why this would be an either/or situation though.)

Are we relying on CoreLibrary at all? I thought it got 100% replaced by MBrane...

Any guess why this code is here? If not do you agree it worth a little time to look into? (Can we ask Eric?)

Well, we could, but if the elf only grants us three wishes, should this be one of them?

=K

jefft0 commented 5 years ago

Happy holidays to you too!

Are we relying on CoreLibrary at all?

Replicode relies on it for some type definitions like core::float32 and the P smart pointer: https://github.com/IIIM-IS/CoreLibrary/blob/master/CoreLibrary/base.h#L94

It also uses its XML parser for settings.xml, and core::Time in CorrelatorTest: https://github.com/IIIM-IS/replicode/blob/master/Test/settings.h#L137 https://github.com/IIIM-IS/replicode/blob/master/CorrelatorTest/CorrelatorTest.cpp#L124

I would love to remove the dependency on CoreLIbrary (especially since there is some confusion about where it should be installed). Should I open an issue for removing the dependency?

But if the elf only grants us three wishes, should this be one of them?

I guess not. I'll post again on this issue about some other low-hanging fruit for removing warnings.

jefft0 commented 4 years ago

The warning for "You will get either slowness or runtime errors" was resolved with this commit: https://github.com/IIIM-IS/replicode/commit/deb15d3e21465ee958acf5270bfa1b342e88b6d2