Skip to content

Conversation

guoyu-wang
Copy link
Contributor

Description: Mitigate pybind11 build break using Xcode 12 on macOS

Motivation and Context

  • Apple released Xcode 12 in September, 2020, the compiler comes with it is AppleClang 12, it will produce warning if you take reference of a temporary object,...
  • Pybind11 if being built with appleclang 12 will throw the above warning and cause build break on macOS, this is fixed here fix: warning on latest AppleClang pybind/pybind11#2522, but the fix will be in pybind11 2.6.0 which is not yet released
  • The work around is changing build script to disable dev_mode when we are using Xcode 12 on macOS and are building python package, we need to remove this when we update to pybind11 2.6.0+

@guoyu-wang guoyu-wang requested review from skottmckay and snnn October 6, 2020 00:10
@guoyu-wang guoyu-wang requested a review from a team as a code owner October 6, 2020 00:10
@guoyu-wang guoyu-wang requested a review from wenbingl October 6, 2020 00:10
# https://github.com/pybind/pybind11/pull/2522
# TODO, remove this after switch to pybind11 2.6.0+
return 'OFF'
return 'ON'
Copy link
Contributor

Choose a reason for hiding this comment

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

Disabling dev mode feels like a fairly severe hack vs. either fixing warnings where possible or disabling specific warnings on specific targets. Whilst this is consistent with the existing approach, it would be good to have a more refined way to handle warnings on iOS/macOS builds than this. Not sure why ACL or ARMNN should need this either. Could you maybe add a task to the ORT Core backlog for someone to investigate improving this?

Copy link
Contributor Author

@guoyu-wang guoyu-wang Oct 6, 2020

Choose a reason for hiding this comment

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

Create 3 PBI
Mobile

  • Product Backlog Item 920207: Enable dev mode for ORT iOS

Core

  • Product Backlog Item 920213: Investigate enabling dev mode for ACL/ARMNN EP
  • Product Backlog Item 920214: Remove '-Wno-range-loop-analysis/-Wno-unused-value' flags building on macOS after updating pybind11 to 2.6.0+

skottmckay
skottmckay previously approved these changes Oct 6, 2020
Copy link
Contributor

@skottmckay skottmckay left a comment

Choose a reason for hiding this comment

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

:shipit:

Comment on lines 77 to 85
# Disable pybind11 warning on macOS
# TODO, remove this after switch to pybind11 2.6.0+
if(APPLE)
# https://github.com/pybind/pybind11/pull/2522
target_compile_options(onnxruntime_pybind11_state PRIVATE "-Wno-range-loop-analysis")
# https://github.com/pybind/pybind11/pull/2294
target_compile_options(onnxruntime_pybind11_state PRIVATE "-Wno-unused-value")
endif()

Copy link
Contributor Author

@guoyu-wang guoyu-wang Oct 6, 2020

Choose a reason for hiding this comment

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

Moved disable dev mode to disable certain warning flags here, however we cannot check xcode version here since it is possible we are not using xcode as Cmake generator

Decide to still keep the use_dev_mode() in build.py since it will make it clearer to see what disables the dev mode instead of burying it in the generate_build_tree()

[update] added check for AppleClang and versions

Copy link
Contributor

@skottmckay skottmckay left a comment

Choose a reason for hiding this comment

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

:shipit:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants