Add self contained header tests and fix discovered bugs

Lastique commented 5 years ago

Added tests for self-contained headers.

The new set of tests iterates over Boost.Thread public headers and verifies that each header is self-contained, i.e. doesn't have any missing includes and contains syntactically correct content. On Windows and Cygwin, a second test per each header is generated, which includes the header after windows.h. This verifies that the header doesn't have conflicts with Windows SDK and that including windows.h does not break platform detection in Boost.Thread.

This set of tests would have detected the bug fixed by https://github.com/boostorg/thread/pull/263.

Fixed bugs discovered by the standalone header tests.

Added missing includes.
Added missing namespace qualification for Boost.Chrono casts.
Added typedefs renaming polymorphic lockable wrappers to a more consistent naming scheme. This fixes incorrect name references.
Fixed incorrect duration casts in polymorphic lockable wrappers.
Renamed upgrade_lockable_adapter to poly_upgrade_lockable_adapter, since upgrade_lockable_adapter already exists and has a different meaning.
Use BOOST_THREAD_FUTURE instead of future in executors and task region.
Disable serial_executor_cont when executors or future continuations are not enabled.
Marked sync_queue_is_closed exception as visible.

viboes commented 5 years ago

Thanks for providing those test.

viboes commented 5 years ago

Maybe it was not explicitly documented.

But see trying to use executors without version 3 seems to not work See https://api.travis-ci.org/v3/job/481848302/log.txt

viboes commented 5 years ago

To be complete and correct we need to do this check for all the Boost.Thread versions and add something like this on the generated file

#define BOOST_THREAD_VERSION N

In this sway we will see which files is not working for which version.

Lastique commented 5 years ago

If you compile all headers with all versions, the tests will fail and thus will be of no use. If you only want to compile headers with the version numbers they support then I'm not sure how to do that since the tests are generated automatically and have no knowledge of the supported versions. Therefore I think all headers must be compatible with all versions, even if it means the headers are empty for unsupported versions, and compiling for the default version (which should be the latest one) is enough.

Also note that the tests already take a long time to compile, which adds to the already long time to complete other tests. Compiling all headers for every version will increase this time N-fold.

viboes commented 5 years ago

I don't see why ant test should fails. If each file is protected with respect to the good enablers, non should fail as you have already done for one of the files. So yes, all headers must be compatible with all version. This is what I was asking. I know that the test will increase the time. I would like to have the test available on demand at least.

Lastique commented 5 years ago

Ok. But just to be clear, I don't know what header requires what version, so I'm not planning to add preprocessor checks. I will say though that I'm most interested for the tests to pass in the default version, as this is likely the version most users will use.

viboes commented 5 years ago

I'm saying the same as you: "Therefore I think all headers must be compatible with all versions, even if it means the headers are empty for unsupported versions."

However, compiling only for the default version will not check the files are self contained :(

I don't know which versions the people are using, but the default version is quite limited in features.

viboes commented 5 years ago

I'm preparing a fix for some of the issues still present on the self consistent files.

Lastique commented 5 years ago

I don't know which versions the people are using, but the default version is quite limited in features.

Well, we're using the default version in other Boost libraries. I'm not aware of anyone setting a non-default version.

If other versions are more useful, why not make it the default?

I'm preparing a fix for some of the issues still present on the self consistent files.

Ok, good, thanks.

viboes commented 5 years ago

If other versions are more useful, why not make it the default?

backward compatibility :(

Lastique commented 5 years ago

Those who want a specific API version could (and should) request it explicitly in their project. The default should be the version you recommend people to use in new projects or the projects that are willing to update to a new version. At least, that's how I see it.

viboes commented 5 years ago

I proposed long time ago a plan to move to new versions and the people rejected my plan. This is why we have yet so much versions in Boost.Thread.

I did something wrong and I'm tired now to make it better.

If someone wants to make a really new version on separated folders, please go ahead.

If someone want to propose to change the default version for Boost.Thread, please go ahead and propose it on the Boost development mailing list.

boostorg / thread

Add self contained header tests and fix discovered bugs #268