Skip to content

Conversation

@jmarrec
Copy link
Collaborator

@jmarrec jmarrec commented Apr 25, 2025

Pull request overview

Pull Request Author

  • Model API Changes / Additions
  • Any new or modified fields have been implemented in the EnergyPlus ForwardTranslator (and ReverseTranslator as appropriate)
  • Model API methods are tested (in src/model/test)
  • EnergyPlus ForwardTranslator Tests (in src/energyplus/Test)
  • If a new object or method, added a test in NREL/OpenStudio-resources: Add Link
  • If needed, added VersionTranslation rules for the objects (src/osversion/VersionTranslator.cpp)
  • Verified that C# bindings built fine on Windows, partial classes used as needed, etc.
  • All new and existing tests passes
  • If methods have been deprecated, update rest of code to use the new methods

Labels:

  • If change to an IDD file, add the label IDDChange
  • If breaking existing API, add the label APIChange
  • If deemed ready, add label Pull Request - Ready for CI so that CI builds your PR

Review Checklist

This will not be exhaustively relevant to every PR.

  • Perform a Code Review on GitHub
  • Code Style, strip trailing whitespace, etc.
  • All related changes have been implemented: model changes, model tests, FT changes, FT tests, VersionTranslation, OS App
  • Labeling is ok
  • If defect, verify by running develop branch and reproducing defect, then running PR and reproducing fix
  • If feature, test running new feature, try creative ways to break it
  • CI status: all green or justified

jmarrec added 5 commits April 25, 2025 10:58
Clang has a new warning that requires a template argument list after using the template keyword.

See llvm/llvm-project#94194 for the upstream
clang changes

```
/Users/julien/Software/Others/OpenStudio3/src/nano/nano_signal_slot.hpp:40:41: error: a template argument list is expected after a name prefixed by the template keyword [-Wmissing-template-arg-list-after-template-kw]
   40 |     Observer::insert(Delegate::template bind(instance), this);
      |                                         ^
/Users/julien/Software/Others/OpenStudio3/src/nano/nano_signal_slot.hpp:74:41: error: a template argument list is expected after a name prefixed by the template keyword [-Wmissing-template-arg-list-after-template-kw]
   74 |     Observer::remove(Delegate::template bind(instance), this);
      |                                         ^
2 errors generated.
```
```
/Users/julien/.conan2/p/b/boost1b0853b234640/p/include/boost/mpl/aux_/integral_wrapper.hpp:73:31: error: integer value -1 is outside the valid range of values [0, 3] for the enumeration type 'lock_access_mode' [-Wenum-constexpr-conversion]
   73 |     typedef AUX_WRAPPER_INST( BOOST_MPL_AUX_STATIC_CAST(AUX_WRAPPER_VALUE_TYPE, (value - 1)) ) prior;
      |                               ^
/Users/julien/.conan2/p/b/boost1b0853b234640/p/include/boost/mpl/aux_/static_cast.hpp:24:47: note: expanded from macro 'BOOST_MPL_AUX_STATIC_CAST'
   24 | #   define BOOST_MPL_AUX_STATIC_CAST(T, expr) static_cast<T>(expr)
```
```
In file included from /Users/julien/.conan2/p/b/boost1b0853b234640/p/include/boost/numeric/ublas/traits.hpp:21:
/Users/julien/.conan2/p/b/boost1b0853b234640/p/include/boost/numeric/ublas/detail/iterator.hpp:204:21: error: 'iterator<boost::numeric::ublas::dense_random_access_iterator_tag, double>' is deprecated [-Werror,-Wdeprecated-declarations]
  204 |         public std::iterator<IC, T> {
      |                     ^
/Users/julien/.conan2/p/b/boost1b0853b234640/p/include/boost/numeric/ublas/vector_expression.hpp:1333:20: note: in instantiation of template class 'boost::numeric::ublas::random_access_iterator_base<boost::numeric::ublas::dense_random_access_iterator_tag, boost::numeric::ublas::vector_binary_scalar2<boost::numeric::ublas::vector<double>, const double, boost::numeric::ublas::scalar_multiplies<double, double>>::const_iterator, double>' requested here
 1333 |             public iterator_base_traits<typename E1::const_iterator::iterator_category>::template
```
@jmarrec jmarrec added Developer Issue Pull Request - Ready for CI This pull request if finalized and is ready for continuous integration verification prior to merge. component - Conan Dependency / Package manager problems labels Apr 25, 2025
@jmarrec jmarrec self-assigned this Apr 25, 2025
@jmarrec jmarrec requested a review from kbenne April 25, 2025 09:11
Comment on lines +1 to +14
#ifndef CPPRESTSDK_CHAR_TRAITS_WORKAROUND
#define CPPRESTSDK_CHAR_TRAITS_WORKAROUND

// https://github.com/microsoft/cpprestsdk/issues/1812

#include <cstdint>
#include <cstring>
#include <string>

namespace std {
template <>
struct char_traits<uint8_t>
{
using char_type = uint8_t;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm reimplementing std::char_traits<uint8_t>...

Comment on lines +24 to 28
#if __APPLE__
# include "../../dependencies/cpprestsdk_char_traits_workaround.hpp"
#endif
#define _TURN_OFF_PLATFORM_STRING // cpprestsdk has an ugly macro U() that makes fmt break...
#include <cpprest/http_listener.h>
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

And I include it where needed..

Comment on lines +17 to +19
#if __APPLE__
# include "../../../dependencies/cpprestsdk_char_traits_workaround.hpp"
#endif
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Here too

template <typename L>
void disconnect(L* instance) {
Observer::remove(Delegate::template bind(instance), this);
Observer::remove(Delegate::bind(instance), this);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Clang has a new warning -Wmissing-template-arg-list-after-template-kw that requires a template argument list after using the template keyword.

See llvm/llvm-project#94194 for the upstream clang changes.

I'm using CTAD.

CMakeLists.txt Outdated
Comment on lines 547 to 548
# TODO: workaround for #5398, remove when updating boost
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-enum-constexpr-conversion -Wno-deprecated-declarations")
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

New flags for boost 1.79.0

@ci-commercialbuildings
Copy link
Collaborator

ci-commercialbuildings commented Apr 25, 2025

CI Results for 2956fd9:

@kbenne kbenne merged commit ce69a31 into develop Apr 30, 2025
3 of 6 checks passed
@kbenne kbenne deleted the 5398-RecentClang branch April 30, 2025 20:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component - Conan Dependency / Package manager problems Developer Issue Pull Request - Ready for CI This pull request if finalized and is ready for continuous integration verification prior to merge. severity - Blocker

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants