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

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@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