Subject: Porting Envoy to C++17 by aimless404 · Pull Request #11570 · envoyproxy/envoy

aimless404 · 2020-06-12T14:45:30Z

Signed-off-by: Yifan Yang needyyang@google.com

For an explanation of how to fill out the fields, please see the relevant section
in PULL_REQUESTS.md

Commit Message: This is to continue the process of porting Envoy to C++17. It aims to set the foundation for the improved readability when Envoy codebase can start to utilize a various of features of C++17.
Additional Description:
Risk Level:
Testing:
Docs Changes:
Release Notes:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Deprecated:]

junr03 · 2020-06-16T21:33:10Z

@stedsome I see you have this as draft. Let me know when you are ready for a review

/wait

aimless404 · 2020-06-16T22:55:44Z

@stedsome I see you have this as draft. Let me know when you are ready for a review

/wait

Thanks. It is currently blocked on fixing windows CI.

repokitteh-read-only · 2020-06-26T15:18:51Z

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to (api/envoy[\w/]*/(v1alpha\d?|v1|v2alpha\d?|v2))|(api/envoy/type/(matcher/)?\w+.proto).
CC @envoyproxy/api-shepherds: Your approval is needed for changes made to api/envoy/.
CC @envoyproxy/api-watchers: FYI only for changes made to api/envoy/.

🐱

Caused by: #11570 was synchronize by stedsome.

see: more, trace.

Signed-off-by: Yifan Yang <needyyang@google.com>

wrowe · 2020-06-26T20:39:24Z

Without a chance to review the specific submission, appreciate your efforts! Are you familiar with PR #9654? This subject interests @sunjayBhatia and I, and @lizan as well, so it would be great to make progress and perhaps coordinate some resources to accomplishing this rev 17 bump.

aimless404 · 2020-06-26T20:52:17Z

Yes, this is something I am aware of and prior to starting this, we have talked with lizan and asked for his permission to pick up the work.

…

On Fri, Jun 26, 2020 at 4:39 PM William A. Rowe Jr. < ***@***.***> wrote: Without a chance to review the specific submission, appreciate your efforts! Are you familiar with PR #9654 <#9654>? This subject interests @sunjayBhatia <https://github.com/sunjayBhatia> and I, and @lizan <https://github.com/lizan> as well, so it would be great to make progress and perhaps coordinate some resources to accomplishing this rev 17 bump. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#11570 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AHAYQQRTYN3CVRERTNHGJSDRYUBQVANCNFSM4N4MQHJQ> .

Signed-off-by: Yifan Yang <needyyang@google.com>

lizan · 2020-06-29T22:22:55Z

.bazelrc

 build --host_javabase=@bazel_tools//tools/jdk:remote_jdk11
 build --javabase=@bazel_tools//tools/jdk:remote_jdk11
 build --enable_platform_specific_config
+build --cxxopt=-std=c++17


build:linux?

lizan · 2020-06-29T22:24:29Z

bazel/envoy_internal.bzl

        "-Wformat-security",
        "-Wvla",
-        "-std=c++14",
+        "-std=c++17",


I think we can just remove this line if .bazelrc specify it.

.bazelrc is specifying cxxopt's and not copt's?

This is got rid of. But -std=c++17 is something not applicable to C, so it is in cxxopt rather than copt

lizan · 2020-06-29T22:24:35Z

bazel/envoy_internal.bzl

        "-WX",
        "-Zc:__cplusplus",
-        "-std:c++14",
+        "-std:c++17",


lizan · 2020-06-29T22:24:56Z

bazel/cel-cpp.patch

+         "statusor.h",
+         "statusor_internals.h",
+     ],
+-    copts = ["-std=c++14"],


Need to verify that .bazelrc cxxopts propagate to cell-cpp dependency. But eliminating the duplicates would be fantastic.

We expect to also pick up c++17, the msvc / clang-cl syntax is just slightly different, as illustrated in bazel/envoy_internal.bzl

Unfortunately the flag doesn't propagate to cel-cpp dependency.

Signed-off-by: Yifan Yang <needyyang@google.com>

…the windows and macOS build Signed-off-by: Yifan Yang <needyyang@google.com>

Signed-off-by: Yifan Yang <needyyang@google.com>

…_iter Signed-off-by: Yifan Yang <needyyang@google.com>

Signed-off-by: Yifan Yang <needyyang@google.com>

This reverts commit 711507a. Signed-off-by: Yifan Yang <needyyang@google.com>

This is the PR that does the setup work needed to fix some incompatiblity issue with the current codebase and C++17. This is a part of the draft PR #11570 that intends to port Envoy to C++17, sans all the changes to build configurations. Signed-off-by: Yifan Yang needyyang@google.com

Signed-off-by: Yifan Yang <needyyang@google.com>

Signed-off-by: Yifan Yang <needyyang@google.com> Signed-off-by: Kevin Baichoo <kbaichoo@google.com>

This is the PR that does the setup work needed to fix some incompatiblity issue with the current codebase and C++17. This is a part of the draft PR envoyproxy#11570 that intends to port Envoy to C++17, sans all the changes to build configurations. Signed-off-by: Yifan Yang needyyang@google.com Signed-off-by: scheler <santosh.cheler@appdynamics.com>

Signed-off-by: Yifan Yang <needyyang@google.com> Signed-off-by: scheler <santosh.cheler@appdynamics.com>

aimless404 requested review from mattklein123, snowp and zuercher as code owners June 12, 2020 14:45

aimless404 requested a review from jmarantz as a code owner June 12, 2020 17:25

aimless404 marked this pull request as draft June 12, 2020 19:42

mattklein123 assigned junr03 Jun 14, 2020

repokitteh-read-only bot added the waiting label Jun 16, 2020

repokitteh-read-only bot added v2-freeze api and removed waiting labels Jun 22, 2020

Yifan Yang added 7 commits June 26, 2020 15:24

Subject: porting Envoy to C++17

d5644c4

Signed-off-by: Yifan Yang <needyyang@google.com>

Subject: fixing the CI builds

b8462a2

Signed-off-by: Yifan Yang <needyyang@google.com>

fix of a bug (loss of precision when doing implicit conversion)

b38c8d9

Signed-off-by: Yifan Yang <needyyang@google.com>

added compiler flag to silence incorrect MSVC warning

1b35238

Signed-off-by: Yifan Yang <needyyang@google.com>

trying to see if adding external dep in build solves linker issue

93991ef

Signed-off-by: Yifan Yang <needyyang@google.com>

another attempt to fix msvc linkage problems

2707547

Signed-off-by: Yifan Yang <needyyang@google.com>

trying one last time

09d2ed6

Signed-off-by: Yifan Yang <needyyang@google.com>

aimless404 force-pushed the PR2 branch from ec9bfbe to 09d2ed6 Compare June 26, 2020 15:27

Yifan Yang added 4 commits June 26, 2020 17:30

fixing the windows ci and get rid of the warnings

07a5d44

Signed-off-by: Yifan Yang <needyyang@google.com>

api builds fix

f477318

Signed-off-by: Yifan Yang <needyyang@google.com>

trying to fix api ci

807d4c3

Signed-off-by: Yifan Yang <needyyang@google.com>

does this fix ci problem

f7726fd

Signed-off-by: Yifan Yang <needyyang@google.com>

htuch removed api v2-freeze labels Jun 29, 2020

get rid of the separate c++14 flag added to clang_tools.bzl

713e306

Signed-off-by: Yifan Yang <needyyang@google.com>

aimless404 mentioned this pull request Jun 29, 2020

Subject: preliminary PR to Porting Envoy to C++17 #11807

Closed

lizan reviewed Jun 29, 2020

View reviewed changes

Yifan Yang added 2 commits June 30, 2020 14:29

getting rid of unnecessary c++ flag

da4c5cc

Signed-off-by: Yifan Yang <needyyang@google.com>

getting rid of unnecessary c++ flag

c9c2799

Signed-off-by: Yifan Yang <needyyang@google.com>

aimless404 mentioned this pull request Jul 1, 2020

preliminary PR to Porting Envoy to C++17 #11840

Merged

Yifan Yang added 3 commits July 1, 2020 16:00

getting ready for submission

6a7a254

Signed-off-by: Yifan Yang <needyyang@google.com>

add macos flag

16f981f

Signed-off-by: Yifan Yang <needyyang@google.com>

add the flag to macos build

89e1872

Signed-off-by: Yifan Yang <needyyang@google.com>

aimless404 marked this pull request as ready for review July 1, 2020 17:48

Yifan Yang added 6 commits July 1, 2020 18:53

fixing macos c++17 flag

9423af5

Signed-off-by: Yifan Yang <needyyang@google.com>

adding the flags in envoy_internal.bzl back because they are failing …

5c0b94f

…the windows and macOS build Signed-off-by: Yifan Yang <needyyang@google.com>

to force the rerun of CI

711507a

Signed-off-by: Yifan Yang <needyyang@google.com>

applying a patch to quiche to fix a weird MSVC problem in absl::const…

449856c

…_iter Signed-off-by: Yifan Yang <needyyang@google.com>

actually making sure the patch applies correctly

243de00

Signed-off-by: Yifan Yang <needyyang@google.com>

Revert "to force the rerun of CI"

2aa9ada

This reverts commit 711507a. Signed-off-by: Yifan Yang <needyyang@google.com>

aimless404 force-pushed the PR2 branch from 60ad031 to 2aa9ada Compare July 7, 2020 21:50

aimless404 closed this Jul 7, 2020

aimless404 mentioned this pull request Jul 7, 2020

Subject: Porting Envoy to C++17 #11570 #11941

Merged

mattklein123 pushed a commit that referenced this pull request Jul 15, 2020

Subject: Porting Envoy to C++17 #11570 (#11941)

42a4b9f

Signed-off-by: Yifan Yang <needyyang@google.com>

KBaichoo pushed a commit to KBaichoo/envoy that referenced this pull request Jul 30, 2020

Subject: Porting Envoy to C++17 envoyproxy#11570 (envoyproxy#11941)

fc1b062

Signed-off-by: Yifan Yang <needyyang@google.com> Signed-off-by: Kevin Baichoo <kbaichoo@google.com>

scheler pushed a commit to scheler/envoy that referenced this pull request Aug 4, 2020

Subject: Porting Envoy to C++17 envoyproxy#11570 (envoyproxy#11941)

68b7ff8

Signed-off-by: Yifan Yang <needyyang@google.com> Signed-off-by: scheler <santosh.cheler@appdynamics.com>

Conversation

aimless404 commented Jun 12, 2020

Uh oh!

junr03 commented Jun 16, 2020

Uh oh!

aimless404 commented Jun 16, 2020

Uh oh!

repokitteh-read-only bot commented Jun 26, 2020

Uh oh!

wrowe commented Jun 26, 2020

Uh oh!

aimless404 commented Jun 26, 2020 via email

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants