Skip to content

Commit cfe32f6

Browse files
Conlessfacebook-github-bot
authored andcommitted
Fix include in namespace libkineto (#1056)
Summary: Thank you for maintaining Kineto and for all the work that goes into making these tools available! I’ve been using Kineto APIs and encountered an issue in ApproximateClock.h that could lead to unexpected compile errors. https://github.com/pytorch/kineto/blob/08fcb941769f6afb6ad5792c29e1c0db891d07ab/libkineto/src/ApproximateClock.h#L21-L43 Here `#include <x86intrin.h>` is placed inside the namespace, which I think is generally discouraged in C/C++ because it causes the contents of the header—and any headers it indirectly includes—to be placed within that namespace. For example, since `ApproximateClock.h` indirectly includes `cstdlib` in the `libkineto` namespace, standard symbols like abs will be defined in `namespace libkineto` instead of the global namespace, resulting in errors such as: ``` /usr/include/c++/14.2.1/bits/std_abs.h:52:11: error: ‘abs’ has not been declared in ‘::’ 52 | using ::abs; ``` To adhere to best practices and avoid these issues, I have moved the include directives outside of the namespace. Pull Request resolved: #1056 Reviewed By: aaronenyeshi Differential Revision: D71082945 Pulled By: sraikund16 fbshipit-source-id: 85ad341e4bf18a603d8b2d9217c26f8eaccebf41
1 parent 2859721 commit cfe32f6

File tree

1 file changed

+2
-2
lines changed

1 file changed

+2
-2
lines changed

libkineto/src/ApproximateClock.h

+2-2
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,6 @@
1818
#include <functional>
1919
#include <type_traits>
2020

21-
namespace libkineto {
22-
2321
#if defined(__i386__) || defined(__x86_64__) || defined(__amd64__)
2422
#define KINETO_RDTSC
2523
#if defined(_MSC_VER)
@@ -42,6 +40,8 @@ namespace libkineto {
4240
#define KINETO_UNUSED __attribute__((__unused__))
4341
#endif //_MSC_VER
4442

43+
namespace libkineto {
44+
4545
using time_t = int64_t;
4646
using steady_clock_t = std::conditional_t<
4747
std::chrono::high_resolution_clock::is_steady,

0 commit comments

Comments
 (0)