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

Windows VS 2017 compiler complains about constexpr construct #7357

Closed
wrowe opened this issue Jun 21, 2019 · 4 comments · Fixed by #7802
Closed

Windows VS 2017 compiler complains about constexpr construct #7357

wrowe opened this issue Jun 21, 2019 · 4 comments · Fixed by #7802

Comments

@wrowe
Copy link
Contributor

wrowe commented Jun 21, 2019

Commit b41ba59 introduces the following construct;

constexpr absl::string_view NotReadyReason{"TLS error: Secret is not supplied by SDS"};

// This SslSocket will be used when SSL secret is not fetched from SDS server.
class NotReadySslSocket : public Network::TransportSocket {
public:
  // Network::TransportSocket
  void setTransportSocketCallbacks(Network::TransportSocketCallbacks&) override {}
  std::string protocol() const override { return EMPTY_STRING; }
  absl::string_view failureReason() const override { return NotReadyReason; }

which results in the following error compiling with on cl.exe 19.16.27031.0;

source/extensions/transport_sockets/tls/ssl_socket.cc(25): error C2131: expression did not evaluate to a constant
C:\_eb\execroot\envoy\external\com_google_absl\absl/strings/string_view.h(187): note: failure was caused by call
of undefined function or one not declared 'constexpr'
C:\_eb\execroot\envoy\external\com_google_absl\absl/strings/string_view.h(187): note: see usage of 'strlen'

The following dirt stupid hack results in clean compilation. I'll profess my ignorance, and ask if there is a better workaround?

--- a/source/extensions/transport_sockets/tls/ssl_socket.cc
+++ b/source/extensions/transport_sockets/tls/ssl_socket.cc
@@ -22,7 +22,7 @@ namespace TransportSockets {

 namespace {
 
-constexpr absl::string_view NotReadyReason{"TLS error: Secret is not supplied by SDS"};
+const char* NotReadyReason = "TLS error: Secret is not supplied by SDS";

 // This SslSocket will be used when SSL secret is not fetched from SDS server.
 class NotReadySslSocket : public Network::TransportSocket {
@@ -30,7 +30,7 @@ public:
   // Network::TransportSocket
   void setTransportSocketCallbacks(Network::TransportSocketCallbacks&) override {}
   std::string protocol() const override { return EMPTY_STRING; }
-  absl::string_view failureReason() const override { return NotReadyReason; }
+  absl::string_view failureReason() const override { return constexpr absl::string_view(NotReadyReason); }
   bool canFlushClose() override { return true; }
   void closeSocket(Network::ConnectionEvent) override {}
   Network::IoResult doRead(Buffer::Instance&) override { return {PostIoAction::Close, 0, false}; }
@wrowe
Copy link
Contributor Author

wrowe commented Jun 25, 2019

Could this ticket be related? I note we are taking _repository_impl("com_google_absl") not github.com/abseil/ and the two implementations might be out of sync.

abseil/abseil-cpp#63 (comment)

@stale
Copy link

stale bot commented Jul 25, 2019

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged "help wanted" or other activity occurs. Thank you for your contributions.

@stale stale bot added the stale stalebot believes this issue/PR has not been touched recently label Jul 25, 2019
@wrowe
Copy link
Contributor Author

wrowe commented Jul 26, 2019

Appealed upstream.

@stale stale bot removed the stale stalebot believes this issue/PR has not been touched recently label Jul 26, 2019
@wrowe
Copy link
Contributor Author

wrowe commented Aug 1, 2019

We have confirmed that 14550beb3b7b97195e483fb74b5efb906395c31e commit to abseil does resolve this problem, whenever it becomes convenient to bump this on envoy master.

@wrowe wrowe closed this as completed Aug 1, 2019
mattklein123 pushed a commit that referenced this issue Aug 2, 2019
This fixes #7357 on windows Visual Studio 2017

Signed-off-by: William Rowe <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants