Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support Visual Studio 2017 #446

Closed
qingfengxia opened this issue Nov 10, 2022 · 10 comments · May be fixed by #449
Closed

Add support Visual Studio 2017 #446

qingfengxia opened this issue Nov 10, 2022 · 10 comments · May be fixed by #449

Comments

@qingfengxia
Copy link

qingfengxia commented Nov 10, 2022

Some example can not be compiled, like using API named_async()

p.object.set_value(cancel ? std::nullopt : std::make_optional(f(args...)));

I wonder if it is possible to replace the std::make_optional(f(args...)) with conditonal compiling

can not correlate with visual studio release version with taskflow requirement of "Microsoft Visual Studio at least v19.27 with /std:c++17"

@tsung-wei-huang
Copy link
Member

Hi @qingfengxia , could you please paste the compilation error from your side? Thanks!

@qingfengxia
Copy link
Author

cancel_async exmple has error

template <typename F, typename... ArgsT> auto Executor::named_async(const std::string& name, F&& f, ArgsT&&... args)

Severity Code Description File Line Suppression State
Error C2672 'make_optional': no matching overloaded function found executor.hpp 871
Error C2660 'std::promise::set_value': function does not take 1 arguments executor.hpp 871
Error C2893 Failed to specialize function template 'std::optional<decay<_Ty>::type> std::make_optional(_Ty &&)' executor.hpp 847

I had some moderate trial (like make es5 code work in older browser), it does not work, it seems the compiler lack the feature to support make_optional()

@tsung-wei-huang
Copy link
Member

tsung-wei-huang commented Nov 11, 2022

Yes, I suspect it's an issue with MS compiler to detect the return value (type of the optional) that should be done correctly with auto-deduction. Could you please do the following change and report to me if the problem is still there?

// original code
else {
  p.object.set_value(cancel ? std::nullopt : std::make_optional(f(args...)));
}

// change to the following
else {
  if(cancel) {
    p.object.set_value(std::nullopt);
  }
  else {
    p.object.set_value(f(args...));
  }
}

// or you can do the following
else {
  std::optional<T> res;
  if(!cancel) {
    res = f(args...);
  }
  p.object.set_value(std::move(res));
 }

@qingfengxia
Copy link
Author

using your first solution, the std::opitonal error has gone, now get into second error message.

Severity Code Description File Line Suppression State
Error C2660 'std::promise::set_value': function does not take 1 arguments executor.hpp 876
Error C2661 'std::promise::set_value': no overloaded function takes 0 arguments executor.hpp 868

I think visual c++ 14.x, does not support if constexpr(std::is_same_v<R, void>) instead of that inside function body, would function template specialization help? with std::enable_if<>

==============================
There may be too many errors to solve

// Procedure: _on_pipe
template <typename... Ps>
void DataPipeline<Ps...>::_on_pipe(Pipeflow& pf, Runtime&) {
  }, _pipes, pf._pipe);

FAILED: examples/CMakeFiles/parallel_data_pipeline.dir/parallel_data_pipeline.cpp
Error C1004 unexpected end-of-file found data_pipeline.hpp 534

@tsung-wei-huang
Copy link
Member

Do you have a C++17 compiler in your MSVC version? This might be the best way to get rid of constexpr error.

@qingfengxia
Copy link
Author

we have no chance to upgrade visual studio, I have solved the problem. named_async(), now the last error : unexpected end-of-file.

@qingfengxia
Copy link
Author

qingfengxia commented Nov 15, 2022

Reason: visual studio 2017 (16.35 latest) still does not support if-constexpr, inside lambda

broken parsing of greater-than comparison operator inside if-constexpr, inside lambda, inside std::visit

Solution from hints https://stackoverflow.com/questions/26421104/how-do-i-enable-if-a-class-with-variadic-template-arguments,

Here is my template function specialization for template ellipsis

// Function: named_async
template <typename F, typename... ArgsT>
auto Executor::named_async(const std::string& name, F&& f, ArgsT&&... args)
{
    return named_async_T<F, std::is_same_v<std::invoke_result_t<F, ArgsT...>, void>, ArgsT...>
        (name, std::forward<F>(f), std::forward<ArgsT>(args)...);
}


// add declaration
template <typename F, bool IsVoidRet, typename... ArgsT>
auto Executor::named_async_T(const std::string& name, F&& f, ArgsT&&... args)

// add impl, it is almost same as your original, but
           if constexpr (IsVoidRet) {
// instead of ,  it seems type_traits will fail in lambda.
           if constexpr (std::is_same_v<...> {

similarly problem and solution for: executor.hpp 26xx auto Subflow::_named_async

I can not make PR, so please fix this by change several lines.

Although I have not all examples compiled, but seems I have most features, so I can get started.
Thank you.

@qingfengxia
Copy link
Author

except for data_pipeline , all other example and unittest can be compiled.

@tsung-wei-huang
Copy link
Member

@qingfengxia thanks! would you be willing to create a pull request? I guess you can skip the data pipeline compilation errors by not including its header in your application.

@tsung-wei-huang
Copy link
Member

The newest version has removed this optional support in async task - should be no problem. Feel free to open another issue if the problem is still there.

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 a pull request may close this issue.

2 participants