Skip to content

Commit

Permalink
Address warnings on Windows with new more robust warning flags
Browse files Browse the repository at this point in the history
  • Loading branch information
@lefticus
lefticus committed Feb 10, 2021
1 parent 924c138 commit 8b1935b
Show file tree
Hide file tree
Showing 6 changed files with 70 additions and 40 deletions.
2 changes: 1 addition & 1 deletion src/EnergyPlus/HighTempRadiantSystem.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1005,7 +1005,7 @@ namespace HighTempRadiantSystem {
// SUBROUTINE ARGUMENT DEFINITIONS:

// SUBROUTINE PARAMETER DEFINITIONS:
float const TempConvToler(0.1); // Temperature controller tries to converge to within 0.1C
float const TempConvToler(0.1f); // Temperature controller tries to converge to within 0.1C
int const MaxIterations(10); // Maximum number of iterations to achieve temperature control
// (10 interval halvings achieves control to 0.1% of capacity)
// These two parameters are intended to achieve reasonable control
Expand Down
19 changes: 19 additions & 0 deletions src/EnergyPlus/Psychrometrics.hh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -701,6 +701,17 @@ namespace Psychrometrics {
std::string const &CalledFrom = blank_string // routine this function was called from (error messages)
);


// we are disabling these warnings on Windows because the cache value lookups are using 64bit integers,
// but the () and [] operator overloads for Array1D (which stores the cache) only uses 32bit lookups
// this seems ... very bad. This problem will be fixed when we get rid of Array1D
// at which time this warning disable should be removed.
#ifdef _MSC_VER
#pragma warning(push)
#pragma warning(disable : 4244)
#endif


inline Real64 PsyPsatFnTemp(EnergyPlusData &state,
Real64 const T, // dry-bulb temperature {C}
std::string const &CalledFrom = blank_string // routine this function was called from (error messages)
Expand Down Expand Up @@ -730,6 +741,8 @@ namespace Psychrometrics {

// FUNCTION LOCAL VARIABLE DECLARATIONS:



Int64 const Tdb_tag(bit_shift(bit_transfer(T, Grid_Shift), -Grid_Shift)); // Note that 2nd arg to TRANSFER is not used: Only type matters
// Int64 const hash( bit::bit_and( Tdb_tag, psatcache_mask ) ); //Tuned Replaced by below
Int64 const hash(Tdb_tag & psatcache_mask);
Expand Down Expand Up @@ -1197,6 +1210,12 @@ namespace Psychrometrics {
return cTsat.Tsat; // saturation temperature
}


#ifdef _MSC_VER
#pragma warning(pop)
#endif


#else
Real64 PsyTsatFnPb(EnergyPlusData &state,
Real64 const Press, // barometric pressure {Pascals}
Expand Down
75 changes: 43 additions & 32 deletions third_party/CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,53 @@
# we link the project_options in here for every library in this
# subdirectory

# SAM Is included up here to make sure it doesn't get the C++17 flags

# SAM Simulation Core (SSC)
mark_as_advanced(FORCE JSONCPP_USE_SECURE_MEMORY)
mark_as_advanced(FORCE JSONCPP_WITH_CMAKE_PACKAGE)
mark_as_advanced(FORCE JSONCPP_WITH_EXAMPLE)
mark_as_advanced(FORCE JSONCPP_WITH_PKGCONFIG_SUPPORT)
mark_as_advanced(FORCE JSONCPP_WITH_STRICT_ISO)
mark_as_advanced(FORCE JSONCPP_WITH_TESTS)
set(JSONCPP_WITH_TESTS OFF CACHE BOOL "Compile and (for jsoncpp_check) run JsonCpp test executables" FORCE)
mark_as_advanced(FORCE JSONCPP_WITH_WARNING_AS_ERROR)
mark_as_advanced(FORCE DEBUG_LIBNAME_SUFFIX)
mark_as_advanced(FORCE JSONCPP_WITH_POST_BUILD_UNITTEST)
set(JSONCPP_WITH_POST_BUILD_UNITTEST OFF CACHE BOOL "Automatically run unit-tests as a post build step" FORCE)
mark_as_advanced(FORCE CCACHE_FOUND)
set(SAM_SKIP_TOOLS ON CACHE BOOL "Skips the sdktool and tcsconsole builds" FORCE)
mark_as_advanced(FORCE SAM_SKIP_TOOLS)
set(SAM_SKIP_TESTS ON CACHE BOOL "Skips building tests" FORCE)
mark_as_advanced(FORCE SAM_SKIP_TESTS)
set(SAMAPI_EXPORT OFF CACHE BOOL "Export of ssc binaries to the SAM_api directory; for Unix, compile ssc libraries for SAM_api" FORCE)
mark_as_advanced(FORCE SAMAPI_EXPORT)
ADD_SUBDIRECTORY(ssc)
set_target_properties(ssc PROPERTIES FOLDER ThirdParty/ssc)
set_target_properties(shared PROPERTIES FOLDER ThirdParty/ssc)
set_target_properties(splinter PROPERTIES FOLDER ThirdParty/ssc)

IF ( CMAKE_COMPILER_IS_GNUCXX OR "${CMAKE_CXX_COMPILER_ID}" STREQUAL "Clang" OR "${CMAKE_CXX_COMPILER_ID}" STREQUAL "AppleClang" ) # g++/Clang
target_compile_options(splinter PRIVATE -Wno-error -w)
target_compile_options(shared PRIVATE -Wno-error -w)
target_compile_options(ssc PRIVATE -Wno-error -w)
ELSEIF( MSVC ) # VisualStudio
target_compile_options(splinter PRIVATE /W0)
target_compile_options(shared PRIVATE /W0 /wd4244)
target_compile_options(ssc PRIVATE /W0)
ENDIF()
target_compile_features(ssc PRIVATE cxx_std_11)

# C++17 flag is intentionally after ssd
set(CMAKE_CXX_STANDARD 17)

#Note: the Tarcog files are improperly initializing their virtual base classes, and this 4589 warning
# on MSVC is legitimate, but silencing the warning because it's a third-party library

if (NOT MSVC)
add_compile_options(-Wno-pedantic -Wno-unused-parameter -Wno-unknown-pragmas)
else()
add_compile_options(/wd4267 /wd4996 /wd4068 /wd4244 )
add_compile_options(/wd4267 /wd4996 /wd4068 /wd4244 /wd4589 )
endif()

set(INSTALL_GTEST CACHE BOOL OFF)
Expand Down Expand Up @@ -88,6 +129,7 @@ endif()
add_subdirectory(Windows-CalcEngine)
set_target_properties(Windows-CalcEngine PROPERTIES FOLDER ThirdParty/Windows-CalcEngine)


# Btwxt
include_directories(${gtest_SOURCE_DIR}/include/ SYSTEM)
set(BUILD_BTWXT_TESTING
Expand All @@ -107,34 +149,3 @@ endif()



# SAM Simulation Core (SSC)
mark_as_advanced(FORCE JSONCPP_USE_SECURE_MEMORY)
mark_as_advanced(FORCE JSONCPP_WITH_CMAKE_PACKAGE)
mark_as_advanced(FORCE JSONCPP_WITH_EXAMPLE)
mark_as_advanced(FORCE JSONCPP_WITH_PKGCONFIG_SUPPORT)
mark_as_advanced(FORCE JSONCPP_WITH_STRICT_ISO)
mark_as_advanced(FORCE JSONCPP_WITH_TESTS)
set(JSONCPP_WITH_TESTS OFF CACHE BOOL "Compile and (for jsoncpp_check) run JsonCpp test executables" FORCE)
mark_as_advanced(FORCE JSONCPP_WITH_WARNING_AS_ERROR)
mark_as_advanced(FORCE DEBUG_LIBNAME_SUFFIX)
mark_as_advanced(FORCE JSONCPP_WITH_POST_BUILD_UNITTEST)
set(JSONCPP_WITH_POST_BUILD_UNITTEST OFF CACHE BOOL "Automatically run unit-tests as a post build step" FORCE)
mark_as_advanced(FORCE CCACHE_FOUND)
set(SAM_SKIP_TOOLS ON CACHE BOOL "Skips the sdktool and tcsconsole builds" FORCE)
mark_as_advanced(FORCE SAM_SKIP_TOOLS)
set(SAM_SKIP_TESTS ON CACHE BOOL "Skips building tests" FORCE)
mark_as_advanced(FORCE SAM_SKIP_TESTS)
set(SAMAPI_EXPORT OFF CACHE BOOL "Export of ssc binaries to the SAM_api directory; for Unix, compile ssc libraries for SAM_api" FORCE)
mark_as_advanced(FORCE SAMAPI_EXPORT)
ADD_SUBDIRECTORY(ssc)
set_target_properties(ssc PROPERTIES FOLDER ThirdParty/ssc)
IF ( CMAKE_COMPILER_IS_GNUCXX OR "${CMAKE_CXX_COMPILER_ID}" STREQUAL "Clang" OR "${CMAKE_CXX_COMPILER_ID}" STREQUAL "AppleClang" ) # g++/Clang
target_compile_options(splinter PUBLIC -Wno-error -w)
target_compile_options(shared PUBLIC -Wno-error -w)
target_compile_options(ssc PUBLIC -Wno-error -w)
ELSEIF( MSVC ) # VisualStudio
target_compile_options(splinter PUBLIC /W0)
target_compile_options(shared PUBLIC /W0)
target_compile_options(ssc PUBLIC /W0)
ENDIF()

2 changes: 1 addition & 1 deletion third_party/ObjexxFCL/src/ObjexxFCL/DimensionSlice.hh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ public: // Creation
assert( range.bounded() );
assert( multiplier <= size_type( std::numeric_limits< std::int64_t >::max() ) );
m_ = multiplier;
k_ = ( range.l() - 1 ) * multiplier;
k_ = ( static_cast<std::int64_t>(range.l()) - 1 ) * multiplier;
u_ = range.isize();
}

Expand Down
8 changes: 4 additions & 4 deletions tst/EnergyPlus/unit/EconomicTariff.unit.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -316,7 +316,7 @@ TEST_F(EnergyPlusFixture, EconomicTariff_Water_CCF_Test)

// Check conversion choice
EXPECT_EQ(iEconConv::CCF, state->dataEconTariff->tariff(1).convChoice);
ASSERT_FLOAT_EQ(0.35314666721488586, state->dataEconTariff->tariff(1).energyConv);
ASSERT_DOUBLE_EQ(0.35314666721488586, state->dataEconTariff->tariff(1).energyConv);
}

/** Test that if a meter is a gas meter, and CCF is used, it uses the right conversion (not the water one) **/
Expand Down Expand Up @@ -357,7 +357,7 @@ TEST_F(EnergyPlusFixture, EconomicTariff_Gas_CCF_Test)
// Check conversion choice

EXPECT_EQ(iEconConv::CCF, state->dataEconTariff->tariff(1).convChoice);
ASSERT_FLOAT_EQ(9.4781712e-9, state->dataEconTariff->tariff(1).energyConv);
ASSERT_DOUBLE_EQ(9.4781712e-9, state->dataEconTariff->tariff(1).energyConv);
}

/** Test that if a meter is an Electric meter, and CCF is used, it still defaults to kWh (not allowed) **/
Expand Down Expand Up @@ -398,8 +398,8 @@ TEST_F(EnergyPlusFixture, EconomicTariff_Electric_CCF_Test)

// Check conversion choice, should force back to kWh
EXPECT_EQ(iEconConv::KWH, state->dataEconTariff->tariff(1).convChoice);
ASSERT_FLOAT_EQ(0.0000002778, state->dataEconTariff->tariff(1).energyConv);
ASSERT_FLOAT_EQ(0.001, state->dataEconTariff->tariff(1).demandConv);
ASSERT_DOUBLE_EQ(0.0000002778, state->dataEconTariff->tariff(1).energyConv);
ASSERT_DOUBLE_EQ(0.001, state->dataEconTariff->tariff(1).demandConv);
}

TEST_F(EnergyPlusFixture, EconomicTariff_LEEDtariffReporting_Test)
Expand Down
4 changes: 2 additions & 2 deletions tst/EnergyPlus/unit/StringUtilities.unit.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,14 +81,14 @@ TEST_F(EnergyPlusFixture, readList)
// with commas
EXPECT_TRUE(EnergyPlus::readList("1,3.4,a,hello", i, f, c, s));
EXPECT_EQ(i, 1);
EXPECT_FLOAT_EQ(f, 3.4);
EXPECT_FLOAT_EQ(f, 3.4f);
EXPECT_EQ(c, 'a');
EXPECT_EQ(s,"hello");

// without
EXPECT_TRUE(EnergyPlus::readList("bob q 1.5 10", s, c, f, i));
EXPECT_EQ(i, 10);
EXPECT_FLOAT_EQ(f, 1.5);
EXPECT_FLOAT_EQ(f, 1.5f);
EXPECT_EQ(c, 'q');
EXPECT_EQ(s, "bob");

Expand Down

4 comments on commit 8b1935b

@nrel-bot-3
Copy link

Choose a reason for hiding this comment

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

cmake_modernize (lefticus) - x86_64-MacOS-10.15-clang-11.0.0: OK (3004 of 3004 tests passed, 0 test warnings)

Build Badge Test Badge

@nrel-bot-2c
Copy link

Choose a reason for hiding this comment

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

cmake_modernize (lefticus) - x86_64-Linux-Ubuntu-18.04-gcc-7.5: OK (3044 of 3044 tests passed, 0 test warnings)

Build Badge Test Badge

@nrel-bot-2c
Copy link

Choose a reason for hiding this comment

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

cmake_modernize (lefticus) - x86_64-Linux-Ubuntu-18.04-gcc-7.5-UnitTestsCoverage-Debug: OK (1568 of 1568 tests passed, 0 test warnings)

Build Badge Test Badge Coverage Badge

@nrel-bot-2c
Copy link

Choose a reason for hiding this comment

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

cmake_modernize (lefticus) - x86_64-Linux-Ubuntu-18.04-gcc-7.5-IntegrationCoverage-Debug: OK (722 of 722 tests passed, 0 test warnings)

Build Badge Test Badge Coverage Badge

Please sign in to comment.