[Libreoffice-commits] core.git: cppcheck style fix for noExplicitConstructor in writerfilter

[j.nitschke at ok.de]

On 12/05/2016 10:58 AM, Stephan Bergmann wrote:
> On 12/03/2016 07:10 PM, Jochen Nitschke wrote:
>> commit 76936e787bd13fb1a747b7c716df3fba2d0d3fa9
>> Author: Jochen Nitschke <j.nitschke+logerrit at ok.de>
>> Date:   Sat Dec 3 13:39:44 2016 +0100
>>
>>     cppcheck style fix for noExplicitConstructor in writerfilter
>>
>>     make ctors with one parameter explicit
>
> Seeing this and similar commits mentioning "cppcheck" and
> "noExplicitConstructor" made me curious:
>
> * Is cppcheck reporting each and every ctor (with one parameter?
> callable with one argument?) that isn't declared as explicit?  Or does
> it use some heuristic that might be actually useful?
So far I saw only ctors with one parameter (none with one mandatory plus
x optional parameters). There seems to be no heuristic other than
excluding copy and move ctors.

from cppcheck source:
>             // We are looking for constructors, which are meeting
> following criteria:
>             //  1) Constructor is declared with a single parameter
>             //  2) Constructor is not declared as explicit
>             //  3) It is not a copy/move constructor of non-abstract class
>             //  4) Constructor is not marked as delete (programmer can
> mark the default constructor as deleted, which is ok)
>             if (!func->isConstructor() || func->isDelete() ||
> (!func->hasBody() && func->access == Private))
>                 continue;
>
>             if (!func->isExplicit() &&
>                 func->argCount() == 1 &&
>                 func->type != Function::eCopyConstructor &&
>                 func->type != Function::eMoveConstructor) {
> noExplicitConstructorError(func->tokenDef, scope->className,
> scope->type == Scope::eStruct);


> * Why that concentration on single-parameter ctors?  Is cppcheck a
> pre-C++11 tool?  That leads to an arbitrary-looking mix of explicit
> and non-explicit ctors in cases like
This is simply following guides like the C++ Core Guidelines. [1]
I think the reason for the rule didn't went away with C++11. But with {
initialisation } there is a new good use case now.

Sadly cppcheck results have a lot of noise, ~20% are such
noExplicitConstructor informations.
Maybe disable these messages for future reports. [2]

[1]
https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#Rc-explicit
[2] http://dev-builds.libreoffice.org/cppcheck_reports/master/

> [...]
>> diff --git a/writerfilter/source/rtftok/rtfvalue.hxx
>> b/writerfilter/source/rtftok/rtfvalue.hxx
>> index eeb9730..d113fbf 100644
>> --- a/writerfilter/source/rtftok/rtfvalue.hxx
>> +++ b/writerfilter/source/rtftok/rtfvalue.hxx
>> @@ -32,14 +32,14 @@ public:
>>               css::uno::Reference<css::embed::XEmbeddedObject> const&
>> xObject,
>>               bool bForceString, const RTFShape& aShape);
>>      RTFValue();
>> -    RTFValue(int nValue);
>> +    explicit RTFValue(int nValue);
>>      RTFValue(const OUString& sValue, bool bForce = false);
>> -    RTFValue(RTFSprms rAttributes);
>> +    explicit RTFValue(RTFSprms rAttributes);
>>      RTFValue(RTFSprms rAttributes, RTFSprms rSprms);
>> -    RTFValue(css::uno::Reference<css::drawing::XShape> const& xShape);
>> -    RTFValue(css::uno::Reference<css::io::XInputStream> const&
>> xStream);
>> -    RTFValue(css::uno::Reference<css::embed::XEmbeddedObject> const&
>> xObject);
>> -    RTFValue(const RTFShape& aShape);
>> +    explicit RTFValue(css::uno::Reference<css::drawing::XShape>
>> const& xShape);
>> +    explicit RTFValue(css::uno::Reference<css::io::XInputStream>
>> const& xStream);
>> +    explicit
>> RTFValue(css::uno::Reference<css::embed::XEmbeddedObject> const&
>> xObject);
>> +    explicit RTFValue(const RTFShape& aShape);
>>      virtual ~RTFValue() override;
>>      void setString(const OUString& sValue);
>>      virtual int getInt() const override;

PS: could someone look into the cppcheck warnings:
accessForwarded - Access of forwarded variable expression
accessMoved - Access of moved variable pReleasedFormat.
--
www.Ok.de - die kostenlose E-Mail Adresse