Skip to content

Commit

Permalink
Update to C++17 and make Config::kControllerIntervalMsecs in… (#791)
Browse files Browse the repository at this point in the history
Summary:
PyTorch is on C++17 now, so Kineto should be too.

The specific issue was with Config::kControllerIntervalMsecs. This is a static constexpr member - which in pre-C++17 needed a separate namespace definition (i.e. both a declaration `static constexpr chronos::milliseconds kControllerIntervalMsecs` in the class definition and a definition `constexpr chronos::milliseconds Config::kControllerIntervalMsecs`); but after C++17 static constexpr members don't need a separate definition.

Updating to C++17 seems to fix the issue. We could probably remove the `#if __cplusplus < 201703L` handling entirely, but I'm not 100% sure every user of kineto is on C++17

Pull Request resolved: #791

Reviewed By: aaronenyeshi

Differential Revision: D47743393

Pulled By: davidberard98

fbshipit-source-id: a38a8f105344808f239ec3d4da6626b3ffc7c356
  • Loading branch information
davidberard98 authored and facebook-github-bot committed Jul 25, 2023
1 parent 9af4ea4 commit a94f97b
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 4 deletions.
6 changes: 3 additions & 3 deletions libkineto/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -82,17 +82,17 @@ add_custom_target(libkineto_defs.bzl DEPENDS libkineto_defs.bzl)
add_dependencies(kineto_base libkineto_defs.bzl)

set_target_properties(kineto_base kineto_api PROPERTIES
CXX_STANDARD 14
CXX_STANDARD 17
CXX_STANDARD_REQUIRED YES
CXX_EXTENSIONS NO)

set(KINETO_COMPILE_OPTIONS "-DKINETO_NAMESPACE=libkineto")
list(APPEND KINETO_COMPILE_OPTIONS "-DFMT_HEADER_ONLY")
list(APPEND KINETO_COMPILE_OPTIONS "-DENABLE_IPC_FABRIC")
if(NOT MSVC)
list(APPEND KINETO_COMPILE_OPTIONS "-std=c++14")
list(APPEND KINETO_COMPILE_OPTIONS "-std=c++17")
else()
list(APPEND KINETO_COMPILE_OPTIONS "/std:c++14")
list(APPEND KINETO_COMPILE_OPTIONS "/std:c++17")
list(APPEND KINETO_COMPILE_OPTIONS "-DWIN32_LEAN_AND_MEAN")
list(APPEND KINETO_COMPILE_OPTIONS "-DNOGDI")
endif()
Expand Down
2 changes: 1 addition & 1 deletion libkineto/src/Config.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ using std::vector;

namespace KINETO_NAMESPACE {

#if __clang__ && __cplusplus < 201703L
#if __cplusplus < 201703L
constexpr std::chrono::milliseconds Config::kControllerIntervalMsecs;
#endif

Expand Down

0 comments on commit a94f97b

Please sign in to comment.