[clang-tidy] cppcoreguidelines-virtual-class-destructor: Fix crash when "virtual" keyword is expanded from a macro

Check llvm::Optional before dereferencing it.

Compute VirtualEndLoc differently to

[clang-tidy] cppcoreguidelines-virtual-class-destructor: Fix crash when "virtual" keyword is expanded from a macro

Check llvm::Optional before dereferencing it.

Compute VirtualEndLoc differently to avoid an assertion failure
in clang::SourceManager::getFileIDLoaded:

Assertion `0 && "Invalid SLocOffset or bad function choice"' failed

show more ...

[clang-tidy] cppcoreguidelines-virtual-class-destructor should ignore final classes

The `cppcoreguidelines-virtual-class-destructor` supposed to enforce
http://isocpp.github.io/CppCoreGuidelines/Cpp

[clang-tidy] cppcoreguidelines-virtual-class-destructor should ignore final classes

The `cppcoreguidelines-virtual-class-destructor` supposed to enforce
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c35-a-base-class-destructor-should-be-either-public-and-virtual-or-protected-and-non-virtual

Quote:
> A **base** class destructor should be either public and virtual, or
> protected and non-virtual
[emphasis mine]

However, this check still rules the following case:

class MostDerived final : public Base {
public:
MostDerived() = default;
~MostDerived() = default;
void func() final;
};

Even though `MostDerived` class is marked `final`, thus it should not be
considered as a **base** class. Consequently, the rule is satisfied, yet
the check still flags this code.

In this patch, I'm proposing to ignore `final` classes since they cannot
be //base// classes.

Reviewed By: whisperity

Differential Revision: https://reviews.llvm.org/D126891

show more ...

Don't use Optional::getValue (NFC)

Fix bugprone argument comments.

Identified by bugprone-argument-comment.

Fix cppcoreguidelines-virtual-base-class-destructor in macros

The `cppcoreguidelines-virtual-base-class-destructor` checker crashed on
this example:

#define DECLARE(CLASS) \
class CLASS {

Fix cppcoreguidelines-virtual-base-class-destructor in macros

The `cppcoreguidelines-virtual-base-class-destructor` checker crashed on
this example:

#define DECLARE(CLASS) \
class CLASS { \
protected: \
virtual ~CLASS(); \
}
DECLARE(Foo); // no-crash

The checker will hit the following assertion:

clang-tidy: llvm/include/llvm/ADT/Optional.h:196: T &llvm::optional_detail::OptionalStorage<clang::Token, true>::getValue() & [T = clang::Token]: Assertion `hasVal' failed."

It turns out, `Lexer::findNextToken()` returned `llvm::None` within the
`getVirtualKeywordRange()` function when the `VirtualEndLoc`
SourceLocation represents a macro expansion.
To prevent this from happening, I decided to propagate the `llvm::None`
further up and only create the removal of `virtual` if the
`getVirtualKeywordRange()` succeeds.

I considered an alternative fix for this issue:
I could have checked the `Destructor.getLocation().isMacroID()` before
doing any Fixit calculation inside the `check()` function.
In contrast to this approach my patch will preserve the diagnostics and
drop the fixits only if it would have crashed.

Reviewed By: whisperity

Differential Revision: https://reviews.llvm.org/D113558

show more ...

[clang-tidy] Fix false positive in cppcoreguidelines-virtual-class-destructor

Incorrectly triggers for template classes that inherit
from a base class that has virtual destructor.

Any class inherit

[clang-tidy] Fix false positive in cppcoreguidelines-virtual-class-destructor

Incorrectly triggers for template classes that inherit
from a base class that has virtual destructor.

Any class inheriting from a base that has a virtual destructor
will have their destructor also virtual, as per the Standard:

https://timsong-cpp.github.io/cppwp/n4140/class.dtor#9

> If a class has a base class with a virtual destructor,
> its destructor (whether user- or implicitly-declared) is virtual.

Added unit tests to prevent regression.

Fixes bug https://bugs.llvm.org/show_bug.cgi?id=51912

Differential Revision: https://reviews.llvm.org/D110614

show more ...

[clang-tidy] cppcoreguidelines-virtual-base-class-destructor: a new check

Finds base classes and structs whose destructor is neither public and
virtual nor protected and non-virtual.
A base class's

[clang-tidy] cppcoreguidelines-virtual-base-class-destructor: a new check

Finds base classes and structs whose destructor is neither public and
virtual nor protected and non-virtual.
A base class's destructor should be specified in one of these ways to
prevent undefined behaviour.

Fixes are available for user-declared and implicit destructors that are
either public and non-virtual or protected and virtual.

This check implements C.35 [1] from the CppCoreGuidelines.

Reviewed By: aaron.ballman, njames93

Differential Revision: http://reviews.llvm.org/D102325

[1]: http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rc-dtor-virtual

show more ...