Windows compilation of RetryStateImpl::shouldHedgeRetryPerTryTimeout impl fails

[wrowe]

Windows compilation using MSBuild 15.9, we cannot get this simple line of code, introduced two months ago, to compile;

envoy/source/common/router/retry_state_impl.cc

Line 228 in afb2370

return shouldRetry([]() -> bool { return true; }, callback);

source/common/router/retry_state_impl.cc(228): error C2664: 'Envoy::Router::RetryStatus Envoy::Router::RetryStateImpl::shouldRetry(bool,Envoy::Router::RetryState::DoRetryCallback)': cannot convert argument 1 from 'Envoy::Router::RetryStateImpl::shouldHedgeRetryPerTryTimeout::<lambda_f5b81d16bf43f4b112188693c10b6098>' to 'bool'
source/common/router/retry_state_impl.cc(228): note: Ambiguous user-defined-conversion

[mattklein123]

@wrowe FWIW, I would just submit PRs to fix any Windows compile issues as I'm pretty sure you are the only person compiling on Windows. :)

[wrowe]

Thanks @mattklein123, those that we can resolve we will escalate strait to a PR. Why this compilation is failing is a mystery to me, so far. Looking for extra C++ savvy eyeballs.

[wrowe]

Ping; still looking for a c++ guru to suggest how Win32 CL.exe fails to interpret this correctly.

[zuercher]

Given

RetryStatus RetryStateImpl::shouldRetry(bool would_retry, DoRetryCallback callback)

It seems that we're passing a lamba to a function that takes a bool. And I guess gcc and clang implicitly convert it and MSVC doesn't? Seems like the line should just be shouldRetry(true, callback).

[mergeconflict]

I agree with @zuercher, we should just be passing true to shouldRetry, which raises the question of why other compilers accept this code. I think it's because the lambda expression is converted to a std::function<bool()>, which has an explicit operator bool that, for some reason I can't explain, is being used. Try this experiment:

#include <iostream>

void foo(bool b) {
  std::cout << b << std::endl;
}

int main(int argc, char** argv) {
  foo([]() -> bool { return false; });
}

With clang, this prints 1 regardless of the false in the lambda.

[zuercher]

I'm going to go ahead and submit a PR to fix this, since it makes me anxious, even if it works.