Skip to content

Fix shadowing of C <complex.h> by c10 include directory #8442

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 14, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -373,7 +373,7 @@ if(NOT "${_repo_dir_name}" STREQUAL "executorch")
"fix for this restriction."
)
endif()
set(_common_include_directories ${CMAKE_CURRENT_SOURCE_DIR}/.. ${CMAKE_CURRENT_SOURCE_DIR}/runtime/core/portable_type)
set(_common_include_directories ${CMAKE_CURRENT_SOURCE_DIR}/.. ${CMAKE_CURRENT_SOURCE_DIR}/runtime/core/portable_type/c10)

#
# The `_<target>_srcs` lists are defined by including ${EXECUTORCH_SRCS_FILE}.
Expand Down
2 changes: 1 addition & 1 deletion backends/apple/coreml/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ target_include_directories(
coremldelegate PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/runtime/util
)
target_include_directories(coremldelegate PRIVATE ${EXECUTORCH_ROOT}/..)
target_include_directories(coremldelegate PRIVATE ${EXECUTORCH_ROOT}/runtime/core/portable_type)
target_include_directories(coremldelegate PRIVATE ${EXECUTORCH_ROOT}/runtime/core/portable_type/c10)
target_compile_definitions(coremldelegate PRIVATE C10_USING_CUSTOM_GENERATED_MACROS)
target_link_libraries(coremldelegate PRIVATE executorch_core)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -922,7 +922,7 @@
"$(SRCROOT)/../kvstore",
"$(SRCROOT)/../inmemoryfs",
"$(SRCROOT)/../include",
"$(SRCROOT)/../include/executorch/runtime/core/portable_type",
"$(SRCROOT)/../include/executorch/runtime/core/portable_type/c10",
"$(SRCROOT)/../sdk",
"$(SRCROOT)/../util",
"$(SRCROOT)/../../third-party/nlohmann_json/single_include",
Expand Down Expand Up @@ -954,7 +954,7 @@
"$(SRCROOT)/../kvstore",
"$(SRCROOT)/../inmemoryfs",
"$(SRCROOT)/../include",
"$(SRCROOT)/../include/executorch/runtime/core/portable_type",
"$(SRCROOT)/../include/executorch/runtime/core/portable_type/c10",
"$(SRCROOT)/../sdk",
"$(SRCROOT)/../util",
"$(SRCROOT)/../../third-party/nlohmann_json/single_include",
Expand Down
2 changes: 1 addition & 1 deletion backends/arm/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ endif()

include(${EXECUTORCH_ROOT}/build/Utils.cmake)

set(_common_include_directories ${EXECUTORCH_ROOT}/.. ${EXECUTORCH_ROOT}/runtime/core/portable_type)
set(_common_include_directories ${EXECUTORCH_ROOT}/.. ${EXECUTORCH_ROOT}/runtime/core/portable_type/c10)
add_compile_definitions(C10_USING_CUSTOM_GENERATED_MACROS)

# Third-party folder and Ethos-U driver inclued
Expand Down
2 changes: 1 addition & 1 deletion backends/qualcomm/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ endif()
include_directories(
BEFORE ${_common_include_directories} ${QNN_SDK_ROOT}/include/QNN
${EXECUTORCH_SOURCE_DIR}/third-party/flatbuffers/include
${EXECUTORCH_SOURCE_DIR}/runtime/core/portable_type
${EXECUTORCH_SOURCE_DIR}/runtime/core/portable_type/c10
)

set(_qnn_schema__srcs
Expand Down
6 changes: 3 additions & 3 deletions build/build_apple_frameworks.sh
Original file line number Diff line number Diff line change
Expand Up @@ -207,11 +207,11 @@ check_command "$BUCK2"
# So, just patch our generated framework to do that.
sed -i '' '1i\
#define C10_USING_CUSTOM_GENERATED_MACROS
' $HEADERS_PATH/executorch/runtime/core/portable_type/c10/macros/Macros.h
' $HEADERS_PATH/executorch/runtime/core/portable_type/c10/c10/macros/Macros.h
sed -i '' '1i\
#define C10_USING_CUSTOM_GENERATED_MACROS
' $HEADERS_PATH/executorch/runtime/core/portable_type/c10/macros/Export.h
cp -r $HEADERS_PATH/executorch/runtime/core/portable_type/c10 "$HEADERS_PATH/"
' $HEADERS_PATH/executorch/runtime/core/portable_type/c10/c10/macros/Export.h
cp -r $HEADERS_PATH/executorch/runtime/core/portable_type/c10/c10 "$HEADERS_PATH/"


cp "$SOURCE_ROOT_DIR/extension/apple/ExecuTorch/Exported/"*.h "$HEADERS_PATH/executorch"
Expand Down
6 changes: 3 additions & 3 deletions build/executorch-config.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ cmake_minimum_required(VERSION 3.19)
set(_root "${CMAKE_CURRENT_LIST_DIR}/../../..")
set(required_lib_list executorch executorch_core portable_kernels)
set(EXECUTORCH_LIBRARIES)
set(EXECUTORCH_INCLUDE_DIRS ${_root}/include ${_root}/include/executorch/runtime/core/portable_type ${_root}/lib)
set(EXECUTORCH_INCLUDE_DIRS ${_root}/include ${_root}/include/executorch/runtime/core/portable_type/c10 ${_root}/lib)
foreach(lib ${required_lib_list})
set(lib_var "LIB_${lib}")
add_library(${lib} STATIC IMPORTED)
Expand All @@ -40,7 +40,7 @@ foreach(lib ${required_lib_list})
)
set_target_properties(${lib} PROPERTIES IMPORTED_LOCATION "${${lib_var}}")
target_compile_definitions(${lib} INTERFACE C10_USING_CUSTOM_GENERATED_MACROS)
target_include_directories(${lib} INTERFACE ${_root}/include ${_root}/include/executorch/runtime/core/portable_type ${_root}/lib)
target_include_directories(${lib} INTERFACE ${_root}/include ${_root}/include/executorch/runtime/core/portable_type/c10 ${_root}/lib)
list(APPEND EXECUTORCH_LIBRARIES ${lib})
endforeach()

Expand Down Expand Up @@ -110,7 +110,7 @@ foreach(lib ${lib_list})
add_library(${lib} STATIC IMPORTED)
endif()
set_target_properties(${lib} PROPERTIES IMPORTED_LOCATION "${${lib_var}}")
target_include_directories(${lib} INTERFACE ${_root}/include ${_root}/include/executorch/runtime/core/portable_type ${_root}/lib)
target_include_directories(${lib} INTERFACE ${_root}/include ${_root}/include/executorch/runtime/core/portable_type/c10 ${_root}/lib)
list(APPEND EXECUTORCH_LIBRARIES ${lib})
endif()
endforeach()
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,7 @@
ENABLE_HARDENED_RUNTIME = YES;
HEADER_SEARCH_PATHS = (
"$(SRCROOT)/include",
"$(SRCROOT)/include/executorch/runtime/core/portable_type",
"$(SRCROOT)/include/executorch/runtime/core/portable_type/c10",
);
IPHONEOS_DEPLOYMENT_TARGET = 16.0;
LIBRARY_SEARCH_PATHS = (
Expand All @@ -320,7 +320,7 @@
ENABLE_HARDENED_RUNTIME = YES;
HEADER_SEARCH_PATHS = (
"$(SRCROOT)/include",
"$(SRCROOT)/include/executorch/runtime/core/portable_type",
"$(SRCROOT)/include/executorch/runtime/core/portable_type/c10",
);
IPHONEOS_DEPLOYMENT_TARGET = 16.0;
LIBRARY_SEARCH_PATHS = (
Expand Down
2 changes: 1 addition & 1 deletion examples/arm/executor_runner/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -384,7 +384,7 @@ target_link_options( arm_executor_runner PUBLIC LINKER:-Map=arm_executor_runner.

# ET headers and generated headers includes
target_include_directories(
arm_executor_runner PRIVATE ${ET_INCLUDE_PATH} ${ET_DIR_PATH}/runtime/core/portable_type ${CMAKE_CURRENT_BINARY_DIR}
arm_executor_runner PRIVATE ${ET_INCLUDE_PATH} ${ET_DIR_PATH}/runtime/core/portable_type/c10 ${CMAKE_CURRENT_BINARY_DIR}
)
target_compile_definitions(arm_executor_runner PRIVATE C10_USING_CUSTOM_GENERATED_MACROS)

Expand Down
2 changes: 1 addition & 1 deletion kernels/optimized/cpu/targets.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ _OPTIMIZED_ATEN_OPS = (
name = "op_gelu",
deps = [
"//executorch/kernels/portable/cpu/util:activation_ops_util",
"//executorch/runtime/core/portable_type/c10:aten_headers_for_executorch",
"//executorch/runtime/core/portable_type/c10/c10:aten_headers_for_executorch",
],
),
op_target(
Expand Down
7 changes: 7 additions & 0 deletions runtime/core/portable_type/c10/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
We added an extra c10 directory so that runtime/core/portable_type/c10
can be the directory to put on your include path, rather than
runtime/core/portable_type, because using runtime/core/portable_type
would cause all headers in that directory to be includeable with
`#include <foo.h>`. In particular, that includes
runtime/core/portable_type/complex.h, which would shadow the C99
complex.h standard header.
2 changes: 1 addition & 1 deletion runtime/core/portable_type/targets.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ def define_common_targets():
"bits_types.h",
],
exported_deps = [
"//executorch/runtime/core/portable_type/c10:c10",
"//executorch/runtime/core/portable_type/c10/c10:c10",
],
visibility = [
"//executorch/extension/...",
Expand Down
10 changes: 8 additions & 2 deletions runtime/core/portable_type/test/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,14 @@ set(EXECUTORCH_ROOT ${CMAKE_CURRENT_SOURCE_DIR}/../../../..)

include(${EXECUTORCH_ROOT}/build/Test.cmake)

set(_test_srcs optional_test.cpp tensor_test.cpp half_test.cpp scalar_test.cpp
tensor_impl_test.cpp bfloat16_test.cpp
set(_test_srcs
bfloat16_test.cpp
dont_shadow_complex_test.c
half_test.cpp
optional_test.cpp
scalar_test.cpp
tensor_impl_test.cpp
tensor_test.cpp
)

et_cxx_test(runtime_core_portable_type_test SOURCES ${_test_srcs} EXTRA_LIBS)
9 changes: 9 additions & 0 deletions runtime/core/portable_type/test/dont_shadow_complex_test.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
// This include statement should get the C99 standard header
// complex.h. At one point we messed up our c10 include setup such
// that it instead included runtime/core/portable_type/complex.h. This
// is a regression test for that issue.
#include <complex.h>

#ifndef complex
#warning "complex.h does not define complex"
#endif
7 changes: 4 additions & 3 deletions test/utils/OSSTestConfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -61,12 +61,13 @@
{
"directory": "runtime/core/portable_type/test",
"sources": [
"optional_test.cpp",
"tensor_test.cpp",
"bfloat16_test.cpp",
"dont_shadow_complex_test.c",
"half_test.cpp",
"optional_test.cpp",
"scalar_test.cpp",
"tensor_impl_test.cpp",
"bfloat16_test.cpp"
"tensor_test.cpp"
]
},
{
Expand Down
Loading