Conversation
Signed-off-by: Isaev, Ilya <[email protected]>
isaevil
requested review from
akukanov,
aleksei-fedotov,
dnmokhov,
kboyarinov and
vossmjp
|
|
||
| ### CMake integration | ||
|
|
||
| The library must remain at C++11 as the minimum standard, hence the module source cannot be |
There was a problem hiding this comment.
"must remain at C++11" is because of a project policy, not a technical reason. We may for example later, change to C++17 as the minimum standard as a policy change. As written, its unclear if you are stating a design constraint or a technical constraint.
There was a problem hiding this comment.
Added more details.
Co-authored-by: Mike Voss <[email protected]>
Signed-off-by: Isaev, Ilya <[email protected]>
|
|
||
| ## Proposal | ||
|
|
||
| ### ABI non-breaking style with using-declaration |
There was a problem hiding this comment.
I would also add a link to LLVM manual somewhere in this paragraph
| that uses `import tbb;` instead of `#include`, or running the existing test suite | ||
| with module imports. Should whitebox tests be covered in the latter scenario? | ||
|
|
||
| 6. How to provide preview functionality with modules? Should it be a separate `tbb.preview` module |
There was a problem hiding this comment.
Also, the important details about the preview features and feature-test macros are missed from the document.
For the preview feature, the only way to enable them is to compile the source with -DTBB_PREVIEW..., defining before import does not work.
For feature-test macros, they are simply not available from the module. Initially, they can be available by #include <tbb/version.h> header. Core functionality is available using import tbb.
We can consider providing exported functions replacing these macros in the future.
There was a problem hiding this comment.
For the preview feature, the only way to enable them is to compile the source with -DTBB_PREVIEW...
That is what I meant by "should the tbb module be compiled with defined TBB_PREVIEW_* macros as needed?" in line below.
For feature-test macros, they are simply not available from the module. Initially, they can be available by #include <tbb/version.h> header. Core functionality is available using import tbb.
We can consider providing exported functions replacing these macros in the future.
I think I mostly cover this in 7th question. But I'll add a little more details as well as for question 6.
Signed-off-by: Isaev, Ilya <[email protected]>
…and 7 Signed-off-by: Isaev, Ilya <[email protected]>
| after `import tbb;`. Some options: | ||
| - Replace them with inline variables where possible or provide exported functions. | ||
| - Require the consumer to `#include <tbb/version.h>` alongside the import. | ||
|
|
There was a problem hiding this comment.
I would add a question about plans to handle transitive inclusion of the STL.
Since modules tend to break down when some project use the STL through #include directives and other through import std this should be considered.
I would recommend either toggling all #include into import when using TBB as a module, or at least provide a define to toggle between the two options.
4 participants
Description
Add a comprehensive description of proposed changes
Fixes # - issue number(s) if exists
Type of change
Choose one or multiple, leave empty if none of the other choices apply
Add a respective label(s) to PR if you have permissions
Tests
Documentation
Breaks backward compatibility
Notify the following users
List users with
@to send notificationsOther information