Skip to content

Conversation

@kendalharland
Copy link
Member

@kendalharland kendalharland commented Mar 29, 2024

This should fix an error in the windows CI builds:

MSBuild version 17.9.8+b34f75857 for .NET Framework
2024-03-29T16:16:13.8972368Z 
2024-03-29T16:16:14.1874711Z   1>Checking Build System
2024-03-29T16:16:14.3859793Z   Building Custom Rule D:/a/firebase-cpp-sdk/firebase-cpp-sdk/BinaryCache/firebase/external/src/flatbuffers/CMakeLists.txt
2024-03-29T16:16:14.5018507Z   idl_parser.cpp
2024-03-29T16:16:15.3000069Z C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.38.33130\include\vector(1041,13): error C2220: the following warning is treated as an error [D:\a\firebase-cpp-sdk\firebase-cpp-sdk\BinaryCache\firebase\external\src\flatbuffers-build\flatbuffers.vcxproj]
2024-03-29T16:16:15.3018139Z   (compiling source file '../flatbuffers/src/idl_parser.cpp')
2024-03-29T16:16:15.3018873Z   
2024-03-29T16:16:15.3405081Z C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.38.33130\include\vector(1041,13): warning C4530: C++ exception handler used, but unwind semantics are not enabled. Specify /EHsc [D:\a\firebase-cpp-sdk\firebase-cpp-sdk\BinaryCache\firebase\external\src\flatbuffers-build\flatbuffers.vcxproj]

Copy link
Collaborator

@compnerd compnerd left a comment

Choose a reason for hiding this comment

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

Hmm, I wonder if this is going to cause some trouble. We do not enable exceptions when importing into Swift, and so this can cause ABI differences here. I think that we should disable exceptions: /EHsc- and then tell the C++ runtime to sod off (there is a macro for disabling exceptions).

@compnerd
Copy link
Collaborator

/D_HAS_EXCEPTIONS=0 should do the trick for that.

@kendalharland kendalharland changed the title Enable C++ standard exception handling for windows build Disable C++ exception handling for windows builds Mar 29, 2024
@kendalharland
Copy link
Member Author

Updated to disable exception handling. Thanks for the flag.

Copy link
Collaborator

@compnerd compnerd left a comment

Choose a reason for hiding this comment

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

If it builds, LGTM!

@hjyamauchi
Copy link

I'm curious why this wasn't an issue before. LGTM.

@compnerd
Copy link
Collaborator

Likely due to bad build system setup by Google, I suspect that they are modifying CMAKE_C[XX]_FLAGS manually which is not permitted, and that would silence the issue previously.

@kendalharland kendalharland merged commit b6b3a9f into compnerd/swift Mar 29, 2024
@kendalharland kendalharland deleted the kendal/cxx-exception-handling branch March 29, 2024 21:29
kendalharland added a commit that referenced this pull request May 8, 2024
* Enable C++ standard exception handling for windows build

* Disable exception handling altogether

---------

Co-authored-by: kendal <kendal@thebrowser.company>
github-actions bot pushed a commit that referenced this pull request May 16, 2024
* Enable C++ standard exception handling for windows build

* Disable exception handling altogether

---------

Co-authored-by: kendal <kendal@thebrowser.company>
bcny-fork-syncer bot pushed a commit that referenced this pull request May 25, 2024
* Enable C++ standard exception handling for windows build

* Disable exception handling altogether

---------

Co-authored-by: kendal <kendal@thebrowser.company>
bcny-fork-syncer bot pushed a commit that referenced this pull request May 31, 2024
* Enable C++ standard exception handling for windows build

* Disable exception handling altogether

---------

Co-authored-by: kendal <kendal@thebrowser.company>
bcny-fork-syncer bot pushed a commit that referenced this pull request Jun 1, 2024
* Enable C++ standard exception handling for windows build

* Disable exception handling altogether

---------

Co-authored-by: kendal <kendal@thebrowser.company>
bcny-fork-syncer bot pushed a commit that referenced this pull request Jun 7, 2024
* Enable C++ standard exception handling for windows build

* Disable exception handling altogether

---------

Co-authored-by: kendal <kendal@thebrowser.company>
bcny-fork-syncer bot pushed a commit that referenced this pull request Jul 16, 2024
* Enable C++ standard exception handling for windows build

* Disable exception handling altogether

---------

Co-authored-by: kendal <kendal@thebrowser.company>
bcny-fork-syncer bot pushed a commit that referenced this pull request Jul 18, 2024
* Enable C++ standard exception handling for windows build

* Disable exception handling altogether

---------

Co-authored-by: kendal <kendal@thebrowser.company>
bcny-fork-syncer bot pushed a commit that referenced this pull request Aug 3, 2024
* Enable C++ standard exception handling for windows build

* Disable exception handling altogether

---------

Co-authored-by: kendal <kendal@thebrowser.company>
bcny-fork-syncer bot pushed a commit that referenced this pull request Aug 6, 2024
* Enable C++ standard exception handling for windows build

* Disable exception handling altogether

---------

Co-authored-by: kendal <kendal@thebrowser.company>
bcny-fork-syncer bot pushed a commit that referenced this pull request Aug 21, 2024
* Enable C++ standard exception handling for windows build

* Disable exception handling altogether

---------

Co-authored-by: kendal <kendal@thebrowser.company>
bcny-fork-syncer bot pushed a commit that referenced this pull request Sep 7, 2024
* Enable C++ standard exception handling for windows build

* Disable exception handling altogether

---------

Co-authored-by: kendal <kendal@thebrowser.company>
bcny-fork-syncer bot pushed a commit that referenced this pull request Sep 11, 2024
* Enable C++ standard exception handling for windows build

* Disable exception handling altogether

---------

Co-authored-by: kendal <kendal@thebrowser.company>
bcny-fork-syncer bot pushed a commit that referenced this pull request Sep 12, 2024
* Enable C++ standard exception handling for windows build

* Disable exception handling altogether

---------

Co-authored-by: kendal <kendal@thebrowser.company>
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.

4 participants