Remove warnings #2486
Merged
Remove warnings #2486
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This adds `override` statements, and removes an un-necessary
destructor declaration from status_printer.h in order to remove
annoying compiler warnings when building Ninja tests with CMake
with recent compilers. For example, clang 16.0 complains with:
```
/path/to/ninja/src/status_printer.h:29:16: warning: 'EdgeAddedToPlan' overrides a member function but is not marked 'override' [
-Winconsistent-missing-override]
virtual void EdgeAddedToPlan(const Edge* edge);
^
```
The root issue is that while libninja is compiled explicitly for C++11,
which disable the warning, the tests themselves are not, since they rely
on GTest which now requires C++14, and thus the standard being used is
determined by the compiler's default configuration.
This field is unused. This patch removes compiler warnings when building with recent compilers (e.g. Clang 16.0)
It was declared in an anonymous namespace, and nothing uses it anymore, removes a compiler warning.
Technically, these are not required since Ninja is built with -std=c++11, but using these directives is good practice to clarify the code and avoid simple human errors.
|
looks good I think it's useful to say whether whatever is removed was ever used before. I was asking myself "what happened to the destructors?" It might be nice to have removal of destructors as a separate commit with relevant info
as you know GetFrequency() was used in 72f1912 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Remove multiple C++ compiler warnings
A small series of patches to remove annoying warnings that appear with recent compilers, e.g. Clang 16.0
overridestatements to method declarations.Explanations::enabled_.GetFrequence()function.overridedirectives toCommandRunnersubclasses.