Skip to content

build: update abseil & enable -Wrange-loop-analysis#13364

Merged
mattklein123 merged 11 commits intoenvoyproxy:masterfrom
rebello95:wrange
Oct 5, 2020
Merged

build: update abseil & enable -Wrange-loop-analysis#13364
mattklein123 merged 11 commits intoenvoyproxy:masterfrom
rebello95:wrange

Conversation

@rebello95
Copy link
Copy Markdown
Contributor

@rebello95 rebello95 commented Oct 1, 2020 •
edited
Loading

Resolves #13154.

Risk Level: Low
Testing: CI
Docs Changes: N/A

Signed-off-by: Michael Rebello me@michaelrebello.com

@rebello95
attempt fix
7d64861 
Signed-off-by: Michael Rebello <me@michaelrebello.com>
@rebello95
Merge remote-tracking branch 'origin/master' into wrange
75b5e65 
Signed-off-by: Michael Rebello <me@michaelrebello.com>
@repokitteh-read-only
Copy link
Copy Markdown

repokitteh-read-only bot commented Oct 1, 2020

CC @envoyproxy/dependency-watchers: FYI only for changes made to (bazel/repository_locations\.bzl)|(api/bazel/repository_locations\.bzl)|(.*/requirements\.txt).

🐱

Caused by: #13364 was opened by rebello95.

see: more, trace.

@mattklein123 mattklein123 self-assigned this Oct 1, 2020
@rebello95 rebello95 mentioned this pull request Oct 1, 2020
Closed
@rebello95
update abseil again
2a0e405 
Signed-off-by: Michael Rebello <me@michaelrebello.com>
@rebello95
fix violation
36c6595 
Signed-off-by: Michael Rebello <me@michaelrebello.com>
@rebello95
Merge remote-tracking branch 'origin/master' into wrange
4e62d71 
Signed-off-by: Michael Rebello <me@michaelrebello.com>
@rebello95
fixup
7342f37 
Signed-off-by: Michael Rebello <me@michaelrebello.com>
@rebello95
Merge remote-tracking branch 'origin/master' into wrange
4d7d88e 
Signed-off-by: Michael Rebello <me@michaelrebello.com>
@rebello95 rebello95 marked this pull request as ready for review October 2, 2020 13:30
@rebello95 rebello95 requested a review from dio as a code owner October 2, 2020 13:30
@rebello95
Copy link
Copy Markdown
Contributor Author

rebello95 commented Oct 2, 2020

Looks like this should pass CI now

@rebello95
Update repository_locations.bzl
98c5106 
Signed-off-by: Michael Rebello <me@michaelrebello.com>
@rebello95
Copy link
Copy Markdown
Contributor Author

rebello95 commented Oct 2, 2020

Hm looks like some platforms don't have this option:

Execution platform: @rbe_ubuntu_gcc//config:platform
gcc: error: unrecognized command line option '-Wrange-loop-analysis'

Is there a preferred way to enable this only for macOS within Envoy?

@keith
Copy link
Copy Markdown
Member

keith commented Oct 2, 2020

there's a place for clang specific options below, maybe that's the right place instead of darwin only?

@mattklein123
Copy link
Copy Markdown
Member

mattklein123 commented Oct 2, 2020

Yeah just add to clang below.

/wait

@rebello95
clang
c3f1d3e 
Signed-off-by: Michael Rebello <me@michaelrebello.com>
@ankane ankane mentioned this pull request Oct 2, 2020
Closed
5 tasks
mattklein123
mattklein123 reviewed Oct 2, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for doing this. LGTM with one question.

/wait-any

@rebello95
Merge remote-tracking branch 'origin/master' into wrange
dd5cf37 
Signed-off-by: Michael Rebello <me@michaelrebello.com>
@rebello95
CR
da02b37 
Signed-off-by: Michael Rebello <me@michaelrebello.com>
mattklein123
mattklein123 approved these changes Oct 5, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@mattklein123 mattklein123 merged commit fe7af9a into envoyproxy:master Oct 5, 2020
@rebello95 rebello95 deleted the wrange branch October 5, 2020 20:26
@rgs1
Copy link
Copy Markdown
Member

rgs1 commented Oct 6, 2020

@rebello95 i think some extensions were missed:

external/envoy/source/extensions/retry/priority/previous_priorities/previous_priorities.cc:54:17: error: loop variable 'excluded_priority' is always a copy because the range of type 'std::vector<bool>' does not return a reference [-Werror,-Wrange-loop-analysis]
16:16:36     for (auto&& excluded_priority : excluded_priorities_) {
16:16:36                 ^
16:16:36 external/envoy/source/extensions/retry/priority/previous_priorities/previous_priorities.cc:54:10: note: use non-reference type 'std::_Bit_reference'
16:16:36     for (auto&& excluded_priority : excluded_priorities_) {
16:16:36          ^~~~~~~~~~~~~~~~~~~~~~~~~~
16:16:36 1 error generated.

rgs1 pushed a commit to rgs1/envoy that referenced this pull request Oct 6, 2020
Fix missing case of -Wrange-loop-analysis
431e7f9 
Leftover from envoyproxy#13364.

Signed-off-by: Raul Gutierrez Segales <rgs@pinterest.com>
@rgs1 rgs1 mentioned this pull request Oct 6, 2020
Merged
@rgs1
Copy link
Copy Markdown
Member

rgs1 commented Oct 7, 2020

#13415 fixes it for us.

mattklein123 pushed a commit that referenced this pull request Oct 7, 2020
Fix missing case of -Wrange-loop-analysis (#13415)
221965d 
Leftover from #13364.

Signed-off-by: Raul Gutierrez Segales <rgs@pinterest.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mattklein123 mattklein123 mattklein123 approved these changes

@dio dio Awaiting requested review from dio

Assignees

@mattklein123 mattklein123

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

errors when building with -Wrange-loop-analysis option build: enable -Wrange-loop-analysis

4 participants

@rebello95 @keith @mattklein123 @rgs1