Skip to content

fixed/aligned some compiler warnings #3065

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.

Sign up for GitHub

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

Merged
danmar merged 1 commit into from
Jan 21, 2021
Merged

fixed/aligned some compiler warnings #3065

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions cmake/compileroptions.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ elseif (CMAKE_CXX_COMPILER_ID MATCHES "Clang")
add_compile_options_safe(-Wdocumentation)
add_compile_options_safe(-Wdocumentation-pedantic)
add_compile_options_safe(-Wno-documentation-unknown-command)
add_compile_options_safe(-Wimplicit-fallthrough)

if(ENABLE_COVERAGE OR ENABLE_COVERAGE_XML)
message(FATAL_ERROR "Do not use clang for generate code coverage. Use gcc.")
Expand Down
5 changes: 5 additions & 0 deletions gui/projectfile.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,7 @@ void ProjectFile::readBuildDir(QXmlStreamReader &reader)
switch (type) {
case QXmlStreamReader::Characters:
mBuildDir = reader.text().toString();
FALLTHROUGH;
case QXmlStreamReader::EndElement:
return;
// Not handled
Expand All @@ -273,6 +274,7 @@ void ProjectFile::readImportProject(QXmlStreamReader &reader)
switch (type) {
case QXmlStreamReader::Characters:
mImportProject = reader.text().toString();
FALLTHROUGH;
case QXmlStreamReader::EndElement:
return;
// Not handled
Expand All @@ -298,6 +300,7 @@ bool ProjectFile::readBool(QXmlStreamReader &reader)
switch (type) {
case QXmlStreamReader::Characters:
ret = (reader.text().toString() == "true");
FALLTHROUGH;
case QXmlStreamReader::EndElement:
return ret;
// Not handled
Expand All @@ -323,6 +326,7 @@ int ProjectFile::readInt(QXmlStreamReader &reader, int defaultValue)
switch (type) {
case QXmlStreamReader::Characters:
ret = reader.text().toString().toInt();
FALLTHROUGH;
case QXmlStreamReader::EndElement:
return ret;
// Not handled
Expand Down Expand Up @@ -617,6 +621,7 @@ void ProjectFile::readPlatform(QXmlStreamReader &reader)
switch (type) {
case QXmlStreamReader::Characters:
mPlatform = reader.text().toString();
FALLTHROUGH;
case QXmlStreamReader::EndElement:
return;
// Not handled
Expand Down
2 changes: 1 addition & 1 deletion lib/analyzer.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ struct Analyzer {
/// If the analysis is unsure whether to update a scope, this will return true if the analysis should bifurcate the scope
virtual bool updateScope(const Token* endBlock, bool modified) const = 0;
/// Called when a scope will be forked
virtual void forkScope(const Token* endBlock) {}
virtual void forkScope(const Token* /*endBlock*/) {}
/// If the value is conditional
virtual bool isConditional() const = 0;
/// The condition that will be assumed during analysis
Expand Down
4 changes: 2 additions & 2 deletions lib/checkio.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -929,7 +929,7 @@ void CheckIO::checkFormatString(const Token * const tok,
case 'l':
if (i+1 != formatString.end() && *(i+1) == *i)
specifier += *i++;
// fallthrough
FALLTHROUGH;
case 'j':
case 'q':
case 't':
Expand Down Expand Up @@ -1255,7 +1255,7 @@ void CheckIO::checkFormatString(const Token * const tok,
specifier += *i++;
specifier += *i++;
}
// fallthrough
FALLTHROUGH;
case 'j': // intmax_t or uintmax_t
case 'z': // size_t
case 't': // ptrdiff_t
Expand Down
2 changes: 1 addition & 1 deletion lib/checkstl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1105,7 +1105,7 @@ void CheckStl::invalidContainerLoopError(const Token *tok, const Token * loopTok
reportError(errorPath, Severity::error, "invalidContainerLoop", msg, CWE664, false);
}

void CheckStl::invalidContainerError(const Token *tok, const Token * contTok, const ValueFlow::Value *val, ErrorPath errorPath)
void CheckStl::invalidContainerError(const Token *tok, const Token * /*contTok*/, const ValueFlow::Value *val, ErrorPath errorPath)
Copy link
Copy Markdown
Contributor

@rikardfalkeborn rikardfalkeborn Jan 20, 2021

Choose a reason for hiding this comment

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

Can't it be removed instead?

Copy link
Copy Markdown
Collaborator

@danmar danmar Jan 21, 2021

Choose a reason for hiding this comment

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

I actually write this often myself.

{
const bool inconclusive = val ? val->isInconclusive() : false;
if (val)
Expand Down
9 changes: 9 additions & 0 deletions lib/config.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,15 @@
# define NORETURN
#endif

// fallthrough
Copy link
Copy Markdown
Collaborator

@amai2012 amai2012 Jan 19, 2021

Choose a reason for hiding this comment

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

Shouldn't https://en.cppreference.com/w/cpp/language/attributes/fallthrough be preferred if available?

Copy link
Copy Markdown
Collaborator Author

@firewave firewave Jan 19, 2021

Choose a reason for hiding this comment

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

That is a C++-17 feature.

Copy link
Copy Markdown
Collaborator

@amai2012 amai2012 Jan 19, 2021

Choose a reason for hiding this comment

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

Yes.
Anyway we may check if the current compiler support C++17 oder beyond...

Copy link
Copy Markdown
Collaborator Author

@firewave firewave Jan 19, 2021

Choose a reason for hiding this comment

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

We not compile Cppcheck as C++17 at all so there's not really a point in adding that. Not sure if we even want to support that. @danmar What do you think?

Copy link
Copy Markdown
Collaborator

@danmar danmar Jan 21, 2021

Choose a reason for hiding this comment

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

I would probably prefer to use [[fallthrough]] but as this is a define we can easily select the best fit and I am not against your code. I think it's ok to use this c++17 in development builds as long as cppcheck releases can be built with an old compiler. It's not crucial that developers use c++11 as long as we have CI that do use it and test it.

Copy link
Copy Markdown
Collaborator

@danmar danmar Jan 21, 2021

Choose a reason for hiding this comment

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

feel free to change it later. I feel I will merge this as it is..

#if defined(__clang__)
# define FALLTHROUGH [[clang::fallthrough]]
#elif (defined(__GNUC__) && (__GNUC__ >= 7))
# define FALLTHROUGH __attribute__((fallthrough))
#else
# define FALLTHROUGH
#endif

#define REQUIRES(msg, ...) class=typename std::enable_if<__VA_ARGS__::value>::type

#include <string>
Expand Down