...following up on b6d0ca0458 "The Clang
RecursiveASTVisitor change is already in Clang 11"; no longer sure why I
originally wrote the checks using <= rather than < in
5d546de67b "Adapt to Clang 12 trunk
RecursiveASTVisitor change"
Change-Id: I79877e21823334c939ecdf9c64e4efe5e0b1571b
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/104349
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
...see 5d546de67b "Adapt to Clang 12 trunk
RecursiveASTVisitor change"; no longer sure why I thought <https://github.com/
llvm/llvm-project/commit/5689b38c6a4220cc5f6ba68a56486229b10071bf> "Removed a
RecursiveASTVisitor feature to visit operator kinds with different methods"
would not be included in the release/11.x branch
Change-Id: I1e206368c447b74cc6adec4c1d4790c80f0cb744
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/104309
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
...from which an OUString can cheaply be instantiated. This is the OUString
equivalent of 4b9e440c51 "Turn OStringLiteral into
a consteval'ed, static-refcound rtl_String". Most remarks about that commit
apply here too (this commit is just substantially bigger and a bit more
complicated because there were so much more uses of OUStringLiteral than of
OStringLiteral):
The one downside is that OUStringLiteral now needs to be a template abstracting
over the string length. But any uses for which that is a problem (e.g., as the
element type of a container that would no longer be homogeneous, or in the
signature of a function that shall not be turned into a template for one reason
or another) can be replaced with std::u16string_view, without loss of efficiency
compared to the original OUStringLiteral, and without loss of expressivity.
The new OUStringLiteral ctor code would probably not be very efficient if it
were ever executed at runtime, but it is intended to be only executed at compile
time. Where available, C++20 "consteval" is used to statically ensure that.
The intended use of the new OUStringLiteral is in all cases where an
object that shall itself not be an OUString (e.g., because it shall be a
global static variable for which the OUString ctor/dtor would be detrimental at
library load/unload) must be converted to an OUString instance in at least one
place. Other string literal abstractions could use std::u16string_view (or just
plain char16_t const[N]), but interestingly OUStringLiteral might be more
efficient than constexpr std::u16string_view even for such cases, as it should
not need any relocations at library load time. For now, no existing uses of
OUStringLiteral have been changed to some other abstraction (unless technically
necessary as discussed above), and no additional places that would benefit from
OUStringLiteral have been changed to use it.
Global constexpr OUStringLiteral variables defined in an included file would be
somewhat suboptimal, as each translation unit that uses them would create its
own, unshared instance. The envisioned solution is to turn them into static
data members of some class (and there may be a loplugin coming to find and fix
affected places). Another approach that has been taken here in a few cases
where such variables were only used in one .cxx anyway is to move their
definitions from the .hxx into that one .cxx (in turn causing some files to
become empty and get removed completely)---which also silenced some GCC
-Werror=unused-variable if a variable from a .hxx was not used in some .cxx
including it.
To keep individual commits reasonably manageable, some consumers of
OUStringLiteral in rtl/ustrbuf.hxx and rtl/ustring.hxx are left in a somewhat
odd state for now, where they don't take advantage of OUStringLiteral's
equivalence to rtl_uString, but just keep extracting its contents and copy it
elsewhere. In follow-up commits, those consumers should be changed
appropriately, making them treat OUStringLiteral like an rtl_uString or
dropping the OUStringLiteral overload in favor of an existing (and cheap to use
now) OUString overload, etc.
In a similar vein, comparison operators between OUString and std::u16string_view
have been added to the existing plethora of comparison operator overloads. It
would be nice to eventually consolidate them, esp. with the overloads taking
OUStringLiteral and/or char16_t const[N] string literals, but that appears
tricky to get right without introducing new ambiguities. Also, a handful of
places across the code base use comparisons between OUString and OUStringNumber,
which are now ambiguous (converting the OUStringNumber to either OUString or
std::u16string_view). For simplicity, those few places have manually been fixed
for now by adding explicit conversion to std::u16string_view.
Also some compilerplugins code needed to be adapted, and some of the
compilerplugins/test cases have become irrelevant (and have been removed), as
the tested code would no longer compile in the first place.
sal/qa/rtl/strings/test_oustring_concat.cxx documents a workaround for GCC bug
<https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96878> "Failed class template
argument deduction in unevaluated, parenthesized context". That place, as well
as uses of OUStringLiteral in extensions/source/abpilot/fieldmappingimpl.cxx and
i18npool/source/localedata/localedata.cxx, which have been replaced with
OUString::Concat (and which is arguably a better choice, anyway), also caused
failures with at least Clang 5.0.2 (but would not have caused failures with at
least recent Clang 12 trunk, so appear to be bugs in Clang that have meanwhile
been fixed).
Change-Id: I34174462a28f2000cfeb2d219ffd533a767920b8
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/102222
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
<https://github.com/llvm/llvm-project/commit/
5689b38c6a4220cc5f6ba68a56486229b10071bf> "Removed a RecursiveASTVisitor feature
to visit operator kinds with different methods".
That change is incompatible in that before the change individual TraverseUnary*
and TraverseBin* functions were called, while now TraverseUnaryOperator and
TraverseBinaryOperator/TraverseCompoundAssignOperator are called for all the
different operators. Fixed that with a few #if for the non-shared plugins, but
that doesn't work for the shared plugin. So made the two affected plugins non-
shared for now and left a better fix as a TODO.
Change-Id: I5b87d329ae2c4c93bf605bb1ecc9641039f014a3
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/99000
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
These are triggered when using llvm-config --cxxflags for building,
and sometimes there's -Werror. The warnings were mostly unused
variables because of being used only in assert(), or default case
in switch that covers all enums (it's better to not handle default
to get warning if a case is not handled).
Change-Id: I0ecdd1f27390aadf033852b0d1ee0ca424ae3c37
Reviewed-on: https://gerrit.libreoffice.org/80317
Tested-by: Jenkins
Reviewed-by: Luboš Luňák <l.lunak@collabora.com>
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>
...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>
After f82dc45bdb "Use the canonical TemplateDecl",
builds on macOS (at least those using C++17 and recent trunk libc++) started to
emit false warnings for that
std::pair< Reference<XConnection>,sal_Bool> aRet;
aRet.second = false;
code in dbaccess/source/ui/dlg/DbAdminImpl.cxx. There's a declaration of
std::pair in type_traits and a definition in utility, and for some reason the
declaration in type_traits was deemed the canonical one, while the
SubstTemplateTypeParmType pointed at the definition in utility. So just check
both, the original and the canonical TemplateDecl.
Change-Id: I2fb9d5172c031e6ad4989b215f19d11a4b17f743
Reviewed-on: https://gerrit.libreoffice.org/46474
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
...in case there are multiple, as is the case at least with recent (towards
GCC 8) libstdc++, where std::pair is forward-declared also in
include/c++/8.0.0/bits/stl_iterator.h, so that in
dbaccess/source/ui/dlg/DbAdminImpl.cxx
std::pair< Reference<XConnection>,sal_Bool> aRet;
aRet.second = false;
failed to reconstruct the sal_Bool template argument and issued a
loplugin:implicitboolconversion warning.
Change-Id: I0054f2596d3f8837b857f1dca2f25952828b12cc
Reviewed-on: https://gerrit.libreoffice.org/45254
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
...see a2d814ac1d
"loplugin:implicitboolconversion" and 24eeb4d286
"loplugin:implicitboolconversion" for things previously only found in C++17. As
expected, no further occurrences were found.
Change-Id: Id0ab621b82dc3c40c8b5801413fceb73ade1408a
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>
needed adapting loplugin:implicitboolconversion to not warn about
Sequence<sal_Bool> arBool({true, false, true});
Change-Id: I971918aab7c958ef8f1e4e0548a84314e95f8325
...(but not Objective-C++ code) where BOOL (aka unsigned char) expressions are
routinely implicitly converted to int per the C rules, e.g., as operands of &&.
Change-Id: I17e5dae9f065aaa814850196b1ef31f8fb07c99b
...so that
... ? isdigit(...) : true
will not trigger a warning (where isdigit is the standard C function returning
int). (Odd code like that will fall out of the improved
loplugin:literaltoboolconversion rewriter shortly.)
Change-Id: If51402bd5f4b3f8b0630e874988f4b836ae246f8
...while avoiding warnings about conversions to bool-like typedefs (sal_Bool
etc.), also in cases where those typedefs are used as type arguments of
template specializations (which is no little feat, and the current code is only
an approximation of it, one that appears to cover the relevant cases in our code
base though).
Change-Id: I0ed3801aec7787bf38b429b66e25244ec00cac9b
...to be able to find problems like 6e0bdf04ad
"sal_Bool arg of SetUseImagesInMenus was abused to squeeze '2' through it"
earlier when converting occurrences of sal_Bool to bool.
Restricting this check to function call arguments avoids too much noise while
hopefully still catching all the relevant problems.
(This check partially overlaps the pointertobool check, so implicit conversions
from pointers to bool call arguments will now generate two loplugin warnings,
but that's harmless.)
Change-Id: I0b03b1d1615aaf8bc18e7a84c56fff3ef9903508
...as found in Mac OS X' assert macro definition,
__builtin_expect(!(e), 0) ? ... : ...
with type
long __builtin_expect(long, long)
The code in literaltoboolconversion.cxx is needed for
assert(false);
Change-Id: I42f87482c56986af74b2ec849db9852f74c7c938
...which can act as either a rewriter or a non-rewriter that emits warnings.
Also added COMPILER_PLUGIN_WARNINGS_ONLY=X to demote warnings from plugin X from
errors to warnings, even under --enable-werror.
Change-Id: I05361936240a890515c6bba2459565417c1746b7
...as MSVC would warn about those anyway (at least about cases that do not
compare against literal sal_True/sal_False, but warning even about those helped
clean up lots of redundant
if (foo == true)
instead of just
if (foo)
etc. across the code base).
Change-Id: Iad4b335c35c5411070f04f87f974db7942c288d4
