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

Add override to LogStreamBuf::overflow() #576

Merged
merged 1 commit into from
Sep 29, 2020

Conversation

santigl
Copy link
Contributor

@santigl santigl commented Sep 7, 2020

This quiets warnings such as:

glog/logging.h:1122:20: 
error: ‘virtual std::basic_streambuf<char>::int_type google::base_logging::LogStreamBuf::overflow(std::basic_streambuf<char>::int_type)’ 
can be marked override [-Werror=suggest-override]
   virtual int_type overflow(int_type ch) {
                    ^~~~~~~~

@google-cla google-cla bot added the cla: yes label Sep 7, 2020
@ukai
Copy link
Contributor

ukai commented Sep 14, 2020

@santigl santigl force-pushed the santigl/streambuf-overflow-override branch from 3fb7a75 to f79373f Compare September 14, 2020 09:05
@santigl
Copy link
Contributor Author

santigl commented Sep 14, 2020

Thank you, @ukai. I added the change for the Windows code.

@ukai
Copy link
Contributor

ukai commented Sep 23, 2020

why continuous-integration/travis-ci/pr doesn't report check status??

@sergiud
Copy link
Collaborator

sergiud commented Sep 24, 2020

Good question. Also, this change introduces a dependency to C++11 while glog still uses C++03. A possible workaround is to remove the virtual keyword instead of adding override.

Prevents warnings such as:
```
glog/logging.h:1122:20: error: ‘virtual
std::basic_streambuf<char>::int_type
google::base_logging::LogStreamBuf::overflow(std::basic_streambuf<char>::int_type)’
can be marked override [-Werror=suggest-override]
   virtual int_type overflow(int_type ch) {
                    ^~~~~~~~
```

Signed-off-by: Santiago Gil <[email protected]>
@santigl santigl force-pushed the santigl/streambuf-overflow-override branch from f79373f to 7258189 Compare September 24, 2020 09:48
@santigl
Copy link
Contributor Author

santigl commented Sep 24, 2020

why continuous-integration/travis-ci/pr doesn't report check status??

I hadn't notice it was missing a check before. The last push successfully triggered it.

@sergiud
Copy link
Collaborator

sergiud commented Sep 28, 2020

LGTM

@santigl
Copy link
Contributor Author

santigl commented Sep 28, 2020

Thanks, @sergiud. Feel free to merge this whenever you want.

@sergiud sergiud merged commit af4df08 into google:master Sep 29, 2020
@sergiud sergiud added this to the 0.5 milestone Mar 30, 2021
@sergiud sergiud mentioned this pull request May 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants