Each plugin currently uses its own recursive AST run, which adds up.
This patch adds another shared plugin which internally contains all
(suitable) plugins and dispatches to them from the same one recursive
run. This patch converts ~25 plugins and for starmath's accessibility.cxx
reduces clang build time from 5.43s to 5.14s (and it's 4.39s without any
plugins). As there are almost 50 more plugins to go, this can theoretically
result in 4.56s final time, although probably not all plugins can be
that easily converted, if at all.
This mostly requires very little change in many plugins (see e.g.
BadStatics), some even work without any functionality change (e.g.
CharRightShift). Traverse* calls require some changes but are often
not that difficult. WalkUp* probably can't be supported, although some
plugins can(?) possibly be adjusted to not rely on them. And of course
some plugins can be left as they are, using their own recursive run.
See description at the top of generator.cxx for description of how to
convert a plugin.
The sharedvisitor.cxx source is generated based on scanning relevant
plugin sources using a clang-based scanner/generator. The generated
source is intentionally included instead of getting always generated,
as the generating currently takes some time, so it should get updated
in git whenever a change in a plugin triggers a source change in it.
Change-Id: Ia0d2e3a5a464659503dbb4ed6c20b6cc89b4de01
Reviewed-on: https://gerrit.libreoffice.org/68026
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
Reviewed-by: Luboš Luňák <l.lunak@collabora.com>
I seem to have missed quite a few in
commit 9f4d23c151
filter out some of the AST in the plugins
This nets me another 14% improvement
Change-Id: I39b980b49ced560f768045dbedd3ddfef29306c1
Reviewed-on: https://gerrit.libreoffice.org/59501
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
...which first added alternative names to and then deprecated getLocBegin/End
Change-Id: Iaefb8ce259057abfa6cd20f0b63c0ef2949a96b2
Reviewed-on: https://gerrit.libreoffice.org/58820
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
when we have two definitions of the same method in a straight
inheritance path up our hierarchy, we were missing a warning
Change-Id: Ibc0f11644c7321a0d5618024860c2503665d8835
Reviewed-on: https://gerrit.libreoffice.org/58354
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
There are some problems here, this should fix one of them: the
getFilename function returns "<stdin>" for spelling locations, because
the input to clang is sort of preprocessed via -frewrite-includes if
icecream is used and the file is built on a remote host (whereas it's
apparently not preprocessed if the file is compiled locally by icecream).
Using getPresumedLoc() uses the #line directives in the preprocessed
input, which avoids the problem but is more expensive, so try to use it
only when necessary.
The getFileEntry(getMainFileID())->getName() pattern will also result
in "<stdin>", but fortunately icecream passes -main-file-name,
which oddly enough isn't used by the SourceManager's spelling locations,
but is available separately via CodeGenOptions.
This builds everything successfully with clang version 6.0.0:
ICECC_PREFERRED_HOST=myremote make check gb_SUPPRESS_TESTS=t
Change-Id: Ic121511683e5302d7b9d85186c8b9c4a5443fa1b
Reviewed-on: https://gerrit.libreoffice.org/54993
Tested-by: Jenkins
Reviewed-by: Thorsten Behrens <Thorsten.Behrens@CIB.de>
...instead of blacklisting such cases. Reuses the
checkIdenticalDefaultArguments code that was originally in
loplugin:overrideparam (and appears to work reasonably well for the default
arguments that actually happen in practice).
Change-Id: I9cf2db17101beb135b2039a9b7ed335bd2af2c08
Reviewed-on: https://gerrit.libreoffice.org/44594
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
This now hides one of the ScVbaFormatCondition_BASE::Operator overloads, but
that doesn't get called directly at ScVbaFormatCondition anyway. (And if it
were, we could add an appropriate using declaration to ScVbaFormatCondition.)
Change-Id: I8440b76a5745c6874f7a3bfcbb4bc4ce5618a4c4
...when a class derives from multiple (non-virtual) instances of one base class,
and the override disambiguates which of those instances' member to call.
That was the case with SwXTextDocument::queryAdapter
(sw/source/uibase/uno/unotxdoc.cxx), where SwXTextDocument derives from
cppu::OWeakObject through both SwXTextDocumentBaseClass and SfxBaseModel, but
calling queryAdapter through a pointer to SwXTextDocumentBaseClass apparently
needs to call OWeakObject::queryAdapter on the second, SfxBaseModel-inherited
OWeakObject base instance, or else CppunitTest_sw_macros_test fails.
Who knows what other instances of similar non-unnecessary overrides have been
removed with the help of broken loplugin:unnecessaryoverride, for which there
were no tests that started to fail...
Turns out .clang-format lacked "ReflowComments: false" to not break the special
"// expected-error {{...}}" etc. comments in compilerplugins/clang/test/.
Also, use a better location to report loplugin:unnecessaryoverride, to keep
clang-format and loplugin:unnecessaryoverride from fighting over how to split
lines and where to put the comment in
compilerplugins/clang/test/unnecessaryoverride.cxx.
Change-Id: I3b24df24369db12f8ec1080d6c9f7b70ff561a16
Reviewed-on: https://gerrit.libreoffice.org/44418
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
...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>
Most of its virtual member functions are (though insertRow is already a curious
exception), even though they are defined, so smells like a copy/paste error
rather than deliberate design. And appears to have been the only reason why
loplugin:unnecessaryoverride filtered out such overriding of pure base
functions.
Change-Id: Ib2a40af9cd3cd3dbb26c4147f7d01de4e11f5f6e
<sberg> thorsten, remember what that "TODO" in
SvxAccessibleTextPropertySet::getSupportedServiceNames was to be about exactly,
in a909acb700 ?
<thorsten> sberg: that's a nonsense, prolly copy'n'pasted, or a 'please review
me'
<sberg> thorsten, OK, thanks (that override will eventually go away with
loplugin:unnecessaryoverride, and the TODO comment be lost)
Change-Id: Iba964c61768459aac4067bbd4e1f7d4f78f6adac
Reviewed-on: https://gerrit.libreoffice.org/27232
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
Tested-by: Stephan Bergmann <sbergman@redhat.com>
