Skip to content

THRIFT-5659: lib: cpp: protocol: declare when methods override - #2687

Merged
Jens-G merged 1 commit into
apache:masterfrom
cfriedt:add-missing-overrides
Oct 26, 2022
Merged

THRIFT-5659: lib: cpp: protocol: declare when methods override#2687
Jens-G merged 1 commit into
apache:masterfrom
cfriedt:add-missing-overrides

Conversation

@cfriedt

@cfriedt cfriedt commented Oct 1, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor

This avoids compiler warnings about inconsistent use of override like the one below.

In file included from /Users/cfriedt/workspace/thrift/test/cpp/src/TestServer.cpp:29:
/Users/cfriedt/workspace/thrift/lib/cpp/src/thrift/protocol/TJSONProtocol.h:255:8: warning: 'checkReadBytesAvailable' overrides a member function but is not marked 'override' [-Winconsistent-missing-override]
  void checkReadBytesAvailable(TList& list)
       ^
/Users/cfriedt/workspace/thrift/lib/cpp/src/thrift/protocol/TProtocol.h:563:16: note: overridden virtual function is here
  virtual void checkReadBytesAvailable(TList& list)
  • Did you create an Apache Jira ticket? (not required for trivial changes)
  • If a ticket exists: Does your pull request title follow the pattern "THRIFT-NNNN: describe my issue"?
  • Did you squash your changes to a single commit? (not required, but preferred)
  • Did you do your best to avoid breaking changes? If one was needed, did you label the Jira ticket with "Breaking-Change"?

@cfriedt cfriedt mentioned this pull request Oct 1, 2022
Closed
24 of 25 tasks
cfriedt pushed a commit to zephyrproject-rtos/gsoc-2022-thrift that referenced this pull request Oct 1, 2022
Christopher Friedt
upstream: cherry-pick commits upstream and from upstream PRs
53db0b8 
apache/thrift#2687
apache/thrift#2681
apache/thrift#2680
apache/thrift#2630
apache/thrift#2682

Signed-off-by: Christopher Friedt <cfriedt@meta.com>
cfriedt pushed a commit to zephyrproject-rtos/gsoc-2022-thrift that referenced this pull request Oct 1, 2022
Christopher Friedt
upstream: cherry-pick commits upstream and from upstream PRs
e4eb875 
With this, we're starting to clean up the commit history a bit.
See #147  for details.

apache/thrift#2687
apache/thrift#2681
apache/thrift#2680
apache/thrift#2630
apache/thrift#2682

Signed-off-by: Christopher Friedt <cfriedt@meta.com>
@cfriedt cfriedt mentioned this pull request Oct 1, 2022
Merged
cfriedt pushed a commit to zephyrproject-rtos/gsoc-2022-thrift that referenced this pull request Oct 1, 2022
Christopher Friedt Christopher Friedt
upstream: cherry-pick commits upstream and from upstream PRs
e66f65d 
With this, we're starting to clean up the commit history a bit.
See #147  for details.

apache/thrift#2687
apache/thrift#2681
apache/thrift#2680
apache/thrift#2630
apache/thrift#2682

Signed-off-by: Christopher Friedt <cfriedt@meta.com>
cfriedt pushed a commit to zephyrproject-rtos/gsoc-2022-thrift that referenced this pull request Oct 1, 2022
Christopher Friedt Christopher Friedt
upstream: cherry-pick commits upstream and from upstream PRs
1c1baa5 
With this, we're starting to clean up the commit history a bit.
See #147  for details.

apache/thrift#2687
apache/thrift#2681
apache/thrift#2680
apache/thrift#2630
apache/thrift#2682
apache/thrift#2689

Signed-off-by: Christopher Friedt <cfriedt@meta.com>
cfriedt pushed a commit to zephyrproject-rtos/gsoc-2022-thrift that referenced this pull request Oct 2, 2022
Christopher Friedt @cfriedt
upstream: cherry-pick commits upstream and from upstream PRs
c17a036 
With this, we're starting to clean up the commit history a bit.
See #147  for details.

apache/thrift#2687
apache/thrift#2681
apache/thrift#2680
apache/thrift#2630
apache/thrift#2682
apache/thrift#2689

Signed-off-by: Christopher Friedt <cfriedt@meta.com>
@cfriedt

cfriedt commented Oct 22, 2022

Copy link
Copy Markdown
Contributor Author

rebased

@cfriedt
cfriedt force-pushed the add-missing-overrides branch from c85442e to aae5324 Compare October 22, 2022 16:10
@cfriedt cfriedt changed the title lib: cpp: protocol: declare when methods override THRIFT-5659: lib: cpp: protocol: declare when methods override Oct 25, 2022
@Jens-G

Jens-G commented Oct 25, 2022

Copy link
Copy Markdown
Member

The patch seems incomplete. TBinaryProtocol seems also affected by this.

@Jens-G Jens-G added the c++ Pull requests that update C++ code label Oct 25, 2022
@cfriedt

cfriedt commented Oct 25, 2022

Copy link
Copy Markdown
Contributor Author

The patch seems incomplete. TBinaryProtocol seems also affected by this.

Thanks for pointing that out - I'll update shortly.

@cfriedt
cfriedt force-pushed the add-missing-overrides branch from aae5324 to 7f19224 Compare October 26, 2022 11:29
@cfriedt Chris Friedt
lib: cpp: protocol: declare when methods override
7dec123 
This avoids compiler warnings about inconsistent use
of `override`.

Signed-off-by: Chris Friedt <cfriedt@meta.com>
@cfriedt
cfriedt force-pushed the add-missing-overrides branch from 7f19224 to 7dec123 Compare October 26, 2022 11:30
@cfriedt

cfriedt commented Oct 26, 2022

Copy link
Copy Markdown
Contributor Author

@Jens-G - should be ready

cfriedt pushed a commit to zephyrproject-rtos/gsoc-2022-thrift that referenced this pull request Oct 26, 2022
Chris Friedt
upstream: bump upstream to include additional override annotations
84e9a24 
Some of the protocol implementations did not include the
`override` annotation on certain virtual methods, which would
cause a warning in gcc.

Fixed upstream in the PR below and cherry-picked into
`cfriedt/zephyr`.

apache/thrift#2687

Signed-off-by: Chris Friedt <cfriedt@meta.com>
@cfriedt cfriedt mentioned this pull request Oct 26, 2022
Closed
@Jens-G
Jens-G merged commit cea5559 into apache:master Oct 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

c++ Pull requests that update C++ code

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@cfriedt @Jens-G