Commit Graph

48 Commits

Author SHA1 Message Date
Noel Grandin
31f04378db loplugin:passstuffbyref
Change-Id: Icb7c22cf4ac95eab54d04e79312fb471ca27bceb
Reviewed-on: https://gerrit.libreoffice.org/74246
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
2019-06-18 20:10:57 +02:00
Noel Grandin
9f4d23c151 filter out some of the AST in the plugins
by checking if the current namespace decl is in our code, so we have to
scan less stuff, which results in a 10% perf improvement for me

Change-Id: Idf0e30d57b6d0dcd13daa9ed679c28b9d233d387
Reviewed-on: https://gerrit.libreoffice.org/58942
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
2018-08-14 13:02:14 +02:00
Noel Grandin
ff3bdde252 loplugin:passstuffbyref
Change-Id: I785e96599bbda029adf4698d11d7f981750dec07
Reviewed-on: https://gerrit.libreoffice.org/54802
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
2018-05-25 21:46:32 +02:00
Noel Grandin
acdba3c2ee loplugin:passstuffbyref more return improvements
slightly less restrictive check when calling functions

Change-Id: I35e268ac611797b1daa83777cda02288a635aa32
Reviewed-on: https://gerrit.libreoffice.org/47259
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
2018-01-03 08:07:41 +01:00
Noel Grandin
a0e136d2cb loplugin:passstuffbyref improved return in sd,various
Change-Id: I4b6ea89ae2072f4389a696ea3c96d8f7a5731e7a
Reviewed-on: https://gerrit.libreoffice.org/47246
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
2018-01-02 13:15:33 +01:00
Noel Grandin
4855257e52 loplugin:passstuffbyref improved return in sw
Change-Id: I4484ac461761e4c46364b4f473c7e62f8ec72103
Reviewed-on: https://gerrit.libreoffice.org/47243
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
2018-01-02 12:19:10 +01:00
Noel Grandin
17ee20b161 loplugin:passstuffbyref improved return in emfio,writerfilter
Change-Id: I237936d62d0f1b17574dd88b5c9de932dc03238e
Reviewed-on: https://gerrit.libreoffice.org/47214
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
2017-12-31 16:03:43 +01:00
Noel Grandin
a967c8f684 loplugin:passstuffbyref improved return in xmloff,sfx2
Change-Id: I7161dfca77f944027bd20614616e22d6acfa27cd
Reviewed-on: https://gerrit.libreoffice.org/47081
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
2017-12-27 15:39:29 +01:00
Noel Grandin
c54d34f708 loplugin:passstuffbyref improved return in canvas and svtools
and for now, ignore methods with params so we don't fall into the trap
of thinking that calls to methods like:
   Bar& foo(Bar &p) { return p; }
can be converted from
   Bar f() { return foo(Bar()); }
to
Bar const & f() { return foo(Bar()); }

Change-Id: Ia3795eb2baf353cb6bec4ebf40451f2789d66ad7
Reviewed-on: https://gerrit.libreoffice.org/47034
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
2017-12-26 07:16:07 +01:00
Noel Grandin
6028b64e27 loplugin:passstuffbyref even more return improvements
Change-Id: I2a752025cd429e4d271626402dce5d8a8b0c76d2
Reviewed-on: https://gerrit.libreoffice.org/47021
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
2017-12-24 07:01:47 +01:00
Stephan Bergmann
c926a1e346 Fix compilerplugins/clang/test/passstuffbyref.cxx
...after 2a1fb4401d "loplugin:passstuffbyref
improved returns", where compiling as C++17 causes a false positive for
S2::set1, whose return statement consists of

> `-ReturnStmt 0x9ef8d78 <col:23, col:44>
>   `-ExprWithCleanups 0x9ef8d60 <col:30, col:44> 'class rtl::OUString'
>     `-CXXFunctionalCastExpr 0x9ef8d38 <col:30, col:44> 'class rtl::OUString' functional cast to class rtl::OUString <ConstructorConversion>
>       `-CXXBindTemporaryExpr 0x9ef8d18 <col:30, col:44> 'class rtl::OUString' (CXXTemporary 0x9ef8d10)
>         `-CXXConstructExpr 0x9ef8cd0 <col:30, col:44> 'class rtl::OUString' 'void (char const &[4], typename libreoffice_internal::ConstCharArrayDetector<char const[4], libreoffice_internal::Dummy>::Type)'
>           |-StringLiteral 0x9ef7160 <col:39> 'const char [4]' lvalue "xxx"
>           `-CXXDefaultArgExpr 0x9ef8cb0 <<invalid sloc>> 'libreoffice_internal::Dummy':'struct rtl::libreoffice_internal::Dummy'

Change-Id: I7b9de7ce6b5604c7d686c8a4a7034019cd1d75c4
Reviewed-on: https://gerrit.libreoffice.org/47029
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
2017-12-24 00:19:20 +01:00
Noel Grandin
2a1fb4401d loplugin:passstuffbyref improved returns
improve the detection of stuff we can return by const &, instead of by
copying

Change-Id: I479ae89d0413125a8295cc3cddbc0017ed61ed69
Reviewed-on: https://gerrit.libreoffice.org/46915
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
2017-12-23 08:04:54 +01:00
Stephan Bergmann
9663341f92 Bump --enable-compiler-plugins to Clang 3.8.0
<https://lists.freedesktop.org/archives/libreoffice/2017-December/079107.html>
"Clang baseline bump"

Change-Id: I18fca8794ea34118fc6308458064d0c28cf5caf7
Reviewed-on: https://gerrit.libreoffice.org/46557
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
2017-12-19 22:08:38 +01:00
Stephan Bergmann
37fe0f729c Better heuristic to only look through implicit copy/move ctors
At least recent libc++ has a std::string ctor overload without a (defaulted)
Allocator argument (which otherwise causes creation of a temporary Allocator
object and thus a ExprWithCleanups), so in C++17 mode (i.e., with no implicit
move CXXConstructExpr -> MaterializeTemporaryExpr -> CXXBindTemporaryExpr chain
in the way) CellInfo::toString (sw/source/filter/ww8/WW8TableInfo.cxx) has a
ReturnStmt of just

> ReturnStmt
> `-ImplicitCastExpr 'std::string':'class std::__1::basic_string<char>' <ConstructorConversion>
>   `-CXXConstructExpr 'std::string':'class std::__1::basic_string<char>' 'void (const char *)'
>     `-ImplicitCastExpr 'const char *' <NoOp>
>       `-ImplicitCastExpr 'char *' <ArrayToPointerDecay>
>         `-DeclRefExpr 'char [256]' lvalue Var 'sBuffer' 'char [256]'

that erroneously triggered loplugin:passstuffbyref.

Change-Id: I53c8911cb1356560692c003808280a103c399e25
Reviewed-on: https://gerrit.libreoffice.org/45916
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
2017-12-06 11:37:06 +01:00
Stephan Bergmann
b35bb38f18 Clean away temporarily added using declarations
Change-Id: I26734c13515394162d88351a1cbe2b20abdac865
2017-11-07 11:50:47 +01:00
Stephan Bergmann
419d664fb7 Adapt loplugins to clang-cl's (implicit) -fdelayed-template-parsing
...which is there for MSVC compatibility, but can cause getBody() to return null
even when doesThisDeclarationHaveABody() is true.

And in staticmethods.cxx we need to check doesThisDeclarationHaveABody() instead
of hasBody():  For some class template member functions that are only defined
outside their class definition, as is the case for
OSequenceIterator::hasMoreElements in include/comphelper/sequence.hxx, hasBody()
may be true for the original member function declaration inside the class (as
there is some later definition that does have a body), but
isLateTemplateParsed() is not (it is only true for the later definition).  So
just skip any such declarations that are not definitions (which is sane anyway,
as otherwise such functions could pointlessly be inspected multiple times).

Change-Id: I724f652a8f060a931f8b5fc3e4feb5f307a922bf
Reviewed-on: https://gerrit.libreoffice.org/42914
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
Tested-by: Stephan Bergmann <sbergman@redhat.com>
2017-09-29 13:31:23 +02:00
Noel Grandin
d21b119df3 loplugin:passstuffbyref ignore params that are assigned to
makes writing nice code awkward sometimes.

Also split plugin into two different plugins, the logic was getting
tangled up.

Change-Id: I232e314d29c766c160c29373988dc37a466505be
2017-08-17 13:18:34 +02:00
Noel Grandin
d615af618c create SfxInterfaceId o3tl::strong_int
Change-Id: Ie52f63382a9fb36f9a472801be012b140bfb51f6
Reviewed-on: https://gerrit.libreoffice.org/35722
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
2017-03-27 09:43:58 +00:00
Andrea Gelmini
5262883672 Fix typos
Change-Id: If92860597a44ee79b513d255ce3f21112485a97e
Reviewed-on: https://gerrit.libreoffice.org/35617
Reviewed-by: Julien Nabet <serval2412@yahoo.fr>
Tested-by: Julien Nabet <serval2412@yahoo.fr>
2017-03-25 08:43:01 +00:00
Andrea Gelmini
6fab286b2a Fix typos
Change-Id: I4f16ba5fc32cbfd6a5b01e495f3ad905da193524
Reviewed-on: https://gerrit.libreoffice.org/34808
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
Tested-by: Noel Grandin <noel.grandin@collabora.co.uk>
2017-03-03 07:42:39 +00:00
Stephan Bergmann
9d7802e7fc Make move detection in loplugin::passstuffbyref work for parenthesized cases
Change-Id: I56754a718af9433c0fa654ccb8eb34da00e75420
2016-12-16 15:16:37 +01:00
Stephan Bergmann
b998313d9d Make move detection in loplugin:passstuffbyref work with MSVCRT
...where an ImplicitCastExpr happens to appear between CXXConstructExpr and
CallExpr

Change-Id: I62226cc89d87bd3d9c03743b650f10c32c18f9be
2016-12-16 15:14:23 +01:00
Noel Grandin
8cf59c6743 make passstuffbyref plugin ignore std::move'd params
request from vmiklos

Change-Id: If263beb0623d725e406003bb1660df10fe4b4e35
Reviewed-on: https://gerrit.libreoffice.org/31555
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
2016-12-05 05:53:25 +00:00
Noel Grandin
40fd53a238 loplugins: extract some common functionality
Change-Id: If470e1d9b481c9eda0829aa985152baf8fb46d7a
2016-10-18 08:51:07 +02:00
Noel Grandin
508c95f1b6 improve passstuffbyref return analysis
Change-Id: I4258bcc97273d8bb7a8c4879fac02a427f76e18c
Reviewed-on: https://gerrit.libreoffice.org/27317
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Noel Grandin <noelgrandin@gmail.com>
2016-07-27 06:48:25 +00:00
Stephan Bergmann
eba4f69a32 loplugin:passstuffbyref also for {css::uno,rtl}::Reference
Change-Id: I76cfdcd031430dc10c464896af2e79221e9da0c3
2016-07-07 18:59:55 +02:00
Stephan Bergmann
ac265f6210 Further clean-up
Change-Id: I16b8bfe2c4a337acf188ec8ffa2ed084ca437faa
2016-06-29 11:38:22 +02:00
Stephan Bergmann
5d88bf766c Further clean-up
Change-Id: Ice5fcb8f598b079afde3346f569d9619f1383506
2016-06-29 11:31:13 +02:00
Stephan Bergmann
1294013bf5 sc/source/core/tool/scmatrix.cxx no longer triggers this?
Change-Id: Ie8d18d66e89621f0cb4762ed5abfe2ec39788f72
2016-06-29 09:25:52 +02:00
Stephan Bergmann
0d3738a258 More Clang 3.4 "(anonymous namespace)" fixes
Change-Id: I7cb43f915565dadd611b90ee30373e472f97efb5
Reviewed-on: https://gerrit.libreoffice.org/26748
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
Tested-by: Stephan Bergmann <sbergman@redhat.com>
2016-06-28 20:33:58 +00:00
Stephan Bergmann
9308f35318 Adapt to Clang 3.4 (in preparation of a buildbot on CentOS 7)
Change-Id: Ie2859f03b31c57deb7fd0deba3285f782e33b239
2016-06-28 16:26:33 +02:00
Stephan Bergmann
a94a0b148c Could swear I'd seen this produce a bogus compiler error
...but now it apparently works

Change-Id: Iac1b4e49788ac620ed55dec7a52c839ba2937f5b
2016-06-14 08:43:03 +02:00
Stephan Bergmann
8110dd24d1 Fix loplugin:passstuffbyref to not warn when ref param is bound to ref
cf. d150eab88e "loplugin:passstuffbyref: For now
disable 'pass parm by value' warnings".  At least all the other changes in
4d49c9601c "Let loplugin:passstuffbyref also look
at fn defn not preceded by any decl" were OK but the one reverted with
b3e939971f "coverity#1362680 Pointer to local
outside scope".

Change-Id: I022125fbcb592e7da3c288c0fd09079dd2e87928
2016-06-13 19:26:56 +02:00
Stephan Bergmann
d150eab88e loplugin:passstuffbyref: For now disable "pass parm by value" warnings
That needs fixing, to check that the parm is not bound to a reference, cf.
<https://gerrit.libreoffice.org/#/c/26189/> "coverity#1362680 Pointer to local
outside scope".

Change-Id: I3656354ccd10affafa006c9e46cf1db608b5b2a7
2016-06-13 09:22:13 +02:00
Stephan Bergmann
4d49c9601c Let loplugin:passstuffbyref also look at fn defn not preceded by any decl
Change-Id: I752bc96d2d521d790e919283cabb14b6526626f4
2016-06-08 17:14:34 +02:00
Noel Grandin
f3d9aab841 teach passstuffbyref plugin to check for..
unnecessarily passing primitives by const ref.

Suggested by Tor Lillqvist

Change-Id: I445e220542969ca3e252581e5953fb01cb2b2be6
Reviewed-on: https://gerrit.libreoffice.org/24672
Reviewed-by: Tor Lillqvist <tml@collabora.com>
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Noel Grandin <noelgrandin@gmail.com>
2016-05-06 06:48:38 +00:00
Jochen Nitschke
1520d0d6f3 cppcheck: silence warnings in compilerplugins
mostly missing explicit before ctors and
uninitialized member vars

one odd use of std::find
> compilerplugins/clang/implicitboolconversion.cxx
> 800 stlIfFind warning	Suspicious condition.
> The result of find() is an iterator, but it is not properly checked.

Change-Id: Iade53494cd7fe8ddb0e110e431449ae5a517fe3b
Reviewed-on: https://gerrit.libreoffice.org/24398
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
Tested-by: Stephan Bergmann <sbergman@redhat.com>
2016-04-27 12:03:57 +00:00
Noel Grandin
b0c435f88b turn on the passstuffbyref plugin again
now that I have committed all of the changes to return types it
found

Change-Id: Iaee121037ce83e94001e4591d232b075dfeade7c
2016-04-14 12:00:10 +02:00
Noel Grandin
7573abfdef update loplugin passstuffbyref to check return types
of methods like
   Foo getFoo() const { return m_foo; }
where we can rather do
   const Foo& getFoo() const { return m_foo; }
and let the client code decide if it wants copy Foo.

Inspired by a performance problem where we were unwittingly
copy constructing a large struct repeatedly just so client code
could interrogate the members of the struct.

When all of the changes this plugin finds are applied, I find
that 'perf stat make check' shows on average a 1.7% reduction
in CPU cycles.

Change-Id: Ic27b4f817aa98f2a2a009f2d4e4a962cbe9c613e
2016-04-13 13:27:50 +02:00
Stephan Bergmann
bb24492c76 More loplugin::TypeCheck use
Change-Id: I2f4a26a918134568f541cd45bdcf5a12b1f1d2ee
2015-12-08 22:56:02 +01:00
Jan Holesovsky
4c4999d944 tdf#96042: 'std::string::find("something") == 0' means "startsWith()".
This should fix a regression from 3bdd176731,
apparently the cppcheck's advice is misleading.

Change-Id: I427ecaa1eb3c9841cb6112997b9b51feda4583d0
2015-12-08 12:12:53 +01:00
Noel Grandin
3bdd176731 cppcheck:stlIfStrFind
"Inefficient usage of string::find() in condition; string::compare() would be faster."

Change-Id: I90403b1d05eff6499c10be33068e5fd4fed30b62
Reviewed-on: https://gerrit.libreoffice.org/19966
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Noel Grandin <noelgrandin@gmail.com>
2015-11-15 06:46:07 +00:00
Stephan Bergmann
24a89b2772 Improve loplugin:passstuffbyref
Change-Id: I88ab4c51ff59312127681d3087d22b9c79192b94
2015-03-03 08:56:01 +01:00
Stephan Bergmann
92db2f17b3 Adapt loplugin:passstuffbyref to Clang 3.2
Change-Id: I24d0b7531feba32f86f761daf18170397cfe5d2f
2015-02-09 10:40:19 +01:00
Stephan Bergmann
0a1c6d4554 Extend loplugin:passstuffbyref to handle lambdas
...even if it is known to be dangerous

Change-Id: Ied96284e33b966bf072d0961054479ec7f891dea
2015-02-05 16:54:10 +01:00
Noel Grandin
c03efa7c6c loplugin-passbyref: ignore non-base declarations
Only consider base declarations, not overriden ones, or we warn on methods that
are overriding stuff from external libraries.

Change-Id: I08791c96f7adba5997ad237a98e7c08a759042ad
2014-05-22 07:54:54 +02:00
Noel Grandin
8d54796bf1 enhance pass-by-ref plugin to detect large arguments
Detect arguments larger than 64 chars passed by value.

Change-Id: I9b0ea9ccb99d115984a26eab67c9cf6afd5f6cae
Signed-off-by: Stephan Bergmann <sbergman@redhat.com>
2014-05-20 11:17:22 +02:00
Noel Grandin
97d50c425f combine the pass-by-ref clang plugins
Change-Id: Idac24afb7cb67fa2d539553fb9fa049c2d61ecf0
2014-05-16 11:18:45 +02:00