diff options
author | Marc Mutz <marc.mutz@qt.io> | 2022-09-22 11:39:44 +0200 |
---|---|---|
committer | Marc Mutz <marc.mutz@qt.io> | 2022-10-15 22:11:47 +0200 |
commit | fc7676769251a27cbbc6d40d68f04bfe38511a5b (patch) | |
tree | 93427f2f2ca5ec328ff0c84ff061ca507796337f /src/corelib/kernel | |
parent | 16dbbc8f8c93f28194b8b440b9616119ea2f7b45 (diff) |
Long live Q_UNREACHABLE_RETURN()!
This is a combination of Q_UNREACHABLE() with a return statement.
ATM, the return statement is unconditionally included. If we notice
that some compilers warn about return after __builtin_unreachable(),
then we can map Q_UNREACHABLE_RETURN(...) to Q_UNREACHABLE() without
having to touch all the code that uses explicit Q_UNREACHABLE() +
return.
The fact that Boost has BOOST_UNREACHABLE_RETURN() indicates that
there are compilers that complain about a lack of return after
Q_UNREACHABLE (we know that MSVC, ICC, and GHS are among them), as
well as compilers that complained about a return being present
(Coverity). Take this opportunity to properly adapt to Coverity, by
leaving out the return statement on this compiler.
Apply the macro around the code base, using a clang-tidy transformer
rule:
const std::string unr = "unr", val = "val", ret = "ret";
auto makeUnreachableReturn = cat("Q_UNREACHABLE_RETURN(",
ifBound(val, cat(node(val)), cat("")),
")");
auto ignoringSwitchCases = [](auto stmt) {
return anyOf(stmt, switchCase(subStmt(stmt)));
};
makeRule(
stmt(ignoringSwitchCases(stmt(isExpandedFromMacro("Q_UNREACHABLE")).bind(unr)),
nextStmt(returnStmt(optionally(hasReturnValue(expr().bind(val)))).bind(ret))),
{changeTo(node(unr), cat(makeUnreachableReturn,
";")), // TODO: why is the ; lost w/o this?
changeTo(node(ret), cat(""))},
cat("use ", makeUnreachableReturn))
);
where nextStmt() is copied from some upstream clang-tidy check's
private implementation and subStmt() is a private matcher that gives
access to SwitchCase's SubStmt.
A.k.a. qt-use-unreachable-return.
There were some false positives, suppressed them with NOLINTNEXTLINE.
They're not really false positiives, it's just that Clang sees the
world in one way and if conditonal compilation (#if) differs for other
compilers, Clang doesn't know better. This is an artifact of matching
two consecutive statements.
I haven't figured out how to remove the empty line left by the
deletion of the return statement, if it, indeed, was on a separate
line, so post-processed the patch to remove all the lines matching
^\+ *$ from the diff:
git commit -am meep
git reset --hard HEAD^
git diff HEAD..HEAD@{1} | sed '/^\+ *$/d' | recountdiff - | patch -p1
[ChangeLog][QtCore][QtAssert] Added Q_UNREACHABLE_RETURN() macro.
Change-Id: I9782939f16091c964f25b7826e1c0dbd13a71305
Reviewed-by: Marc Mutz <marc.mutz@qt.io>
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
Diffstat (limited to 'src/corelib/kernel')
-rw-r--r-- | src/corelib/kernel/qmetatype.cpp | 3 | ||||
-rw-r--r-- | src/corelib/kernel/qpropertyprivate.h | 3 | ||||
-rw-r--r-- | src/corelib/kernel/qvariant.cpp | 6 |
3 files changed, 4 insertions, 8 deletions
diff --git a/src/corelib/kernel/qmetatype.cpp b/src/corelib/kernel/qmetatype.cpp index 5ad7967605..1047c8c0aa 100644 --- a/src/corelib/kernel/qmetatype.cpp +++ b/src/corelib/kernel/qmetatype.cpp @@ -2006,8 +2006,7 @@ static bool convertToEnum(QMetaType fromType, const void *from, QMetaType toType *static_cast<qint64 *>(to) = value; return true; default: - Q_UNREACHABLE(); - return false; + Q_UNREACHABLE_RETURN(false); } } diff --git a/src/corelib/kernel/qpropertyprivate.h b/src/corelib/kernel/qpropertyprivate.h index 966821fa67..4387f0fbeb 100644 --- a/src/corelib/kernel/qpropertyprivate.h +++ b/src/corelib/kernel/qpropertyprivate.h @@ -194,8 +194,7 @@ struct BindingFunctionVTable return true; } else { // Our code will never instantiate this - Q_UNREACHABLE(); - return false; + Q_UNREACHABLE_RETURN(false); } }, /*destroy*/[](void *f){ static_cast<Callable *>(f)->~Callable(); }, diff --git a/src/corelib/kernel/qvariant.cpp b/src/corelib/kernel/qvariant.cpp index 5e949487db..3fb80f203e 100644 --- a/src/corelib/kernel/qvariant.cpp +++ b/src/corelib/kernel/qvariant.cpp @@ -2261,8 +2261,7 @@ static bool integralEquals(uint promotedType, const QVariant::Private *d1, const if (promotedType == QMetaType::ULongLong) return qulonglong(l1) == qulonglong(l2); - Q_UNREACHABLE(); - return 0; + Q_UNREACHABLE_RETURN(0); } namespace { @@ -2306,8 +2305,7 @@ static std::optional<int> integralCompare(uint promotedType, const QVariant::Pri if (promotedType == QMetaType::ULongLong) return spaceShip<qulonglong>(l1, l2); - Q_UNREACHABLE(); - return 0; + Q_UNREACHABLE_RETURN(0); } static std::optional<int> numericCompare(const QVariant::Private *d1, const QVariant::Private *d2) |