Skip to content

Include <array> in flatbuffer headers#7376

Closed
bdero wants to merge 1 commit intogoogle:masterfrom
bdero:bdero/include-array
Closed

Include <array> in flatbuffer headers#7376
bdero wants to merge 1 commit intogoogle:masterfrom
bdero:bdero/include-array

Conversation

@bdero
Copy link

@bdero bdero commented Jul 6, 2022

This fixes a small inconsistency when using flatbuffers headers with different cpp runtimes.

With the --cpp-std c++17 and --cpp-static-reflection options, the C++ generator emits std::array fields as part of the reflection data. When using LLVM's STL, flatbuffers.h happens to implicitly include array via span, but the MS STL forward declares array in span, and so it needs to be included separately.

(Resolves one of the issues in #6898)

@CasperN
Copy link
Collaborator

CasperN commented Jul 12, 2022

so I can merge this but I'm not familiar with the MS STL... can you add something to our CI too to prevent regression? I would have thought this should have been caught by our "CI / Build Windows 2019" test some time ago. Perhaps the CI needs to also be invoked with the flags you mention?

@CasperN
Copy link
Collaborator

CasperN commented Jul 12, 2022

Also did you consider the solution mentioned in #6898 (comment)?

@bdero
Copy link
Author

bdero commented Jul 18, 2022

This just fixes a minor inconvenience with VS 2022 where flatbuffers implicitly assumes that <span> imports <array>. The work around is to include <array> above all flatbuffer header imports if generated with the --cpp-static-reflection flag.

I don't have the cycles to look into the CI at the moment, so closing this out.

@bdero bdero closed this Jul 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants