Clang plugin FindBadConstructsConsumer should count templated class correctly |
|||
Issue description
This plugin counts non-trivial class members to decide whether implicit inline ctor/dtor would be too expensive, however it currently can't handle templated class. Any class member that is of templated type would be conunted as non-trivial, making the plugin to warn about non-trivial ctor/dtor.
This behavior can be counter productive, as it forces developers to add unneeded empty out-of-line destructors.
Here is the minimal code snippet to reproduce this issue:
template <typename T>
class Foo {};
class Bar {
Foo<Bar> ham;
};
,
Jun 30 2016
I think FindBadConstructsConsumer::CountType can be simplified and handle even more cases. I can make a CL in ~30mins, but I'm not familiar with clang plugin build process. Any hint how can I build it and run unit tests locally?
,
Jul 1 2016
I did some research to the issue today. I think our heuristic is not very accurate, and can cause many false positives.
For example, fields that are primitive or have trivial default constructor contributes no complexity to the default constructor of the class, but we still issue warnings for them. Same logic applies to destructors.
struct Foo {};
struct Bar {
Foo one;
Foo two;
Foo three;
Foo four;
};
Should not issue any warnings.
I think the rule should be that the destructor should either contain:
1. No more than 1 inlined non-trivial destructor, or
2. No more than 3 non-inlined non-trivial destructor from embedded objects.
Default constructor and other constructors follow similar rule. For copy constructor and move constructor plus 1 weight for each trivial member in addition.
How does that sound?
----
Additional rant:
I spent hours today figuring out why std::vector<std::string> could ever have trivial destructor... Takeaway: don't name a mock class after real-world system class.
,
Mar 9 2018
Un-cc-ing me from all bugs on my final day.
,
Aug 3
This bug has an owner, thus, it's been triaged. Changing status to "assigned". |
|||
►
Sign in to add a comment |
|||
Comment 1 by vmp...@chromium.org
, Jun 30 2016