Date: prev next · Thread: first prev next last
2017 Archives by date, by thread · List index


On 04/28/2017 10:40 AM, Chris Sherlock wrote:
On 27 Apr 2017, at 6:07 pm, Stephan Bergmann <sbergman@redhat.com> wrote:
On 04/27/2017 10:04 AM, Stephan Bergmann wrote:
commit 5a7123d6869c9248a7ce3a22fffd84f76ceb360f
Author: Stephan Bergmann <sbergman@redhat.com>
Date:   Thu Apr 27 10:03:03 2017 +0200
     Fix build after 7d71451e8e0226d3f3f523611f55132eda6ec10f
          "vcl: change pImpl class names to fit with existing convention"

Please think twice (or even more often) before doing cosmetic changes like that 
<https://cgit.freedesktop.org/libreoffice/core/commit/?id=7d71451e8e0226d3f3f523611f55132eda6ec10f> 
"vcl: change pImpl class names to fit with existing convention".  They easily cause more pain (for 
others) than they're worth the trouble.

     Change-Id: Ic02ca5b71a96b852951ac1b14b966b1ba2f006e9
diff --git a/compilerplugins/clang/vclwidgets.cxx b/compilerplugins/clang/vclwidgets.cxx
index 32d471240de1..2d5dd6333bc9 100644
--- a/compilerplugins/clang/vclwidgets.cxx
+++ b/compilerplugins/clang/vclwidgets.cxx
@@ -428,7 +428,7 @@ bool VCLWidgets::VisitFieldDecl(const FieldDecl * fieldDecl) {
      if (containsVclReferenceBaseSubclass(fieldDecl->getType())) {
          // have to ignore this for now, nasty reverse dependency from tools->vcl
          auto check = loplugin::DeclCheck(pParentRecordDecl);
-        if (!(check.Struct("ErrorContextImpl").GlobalNamespace()
+        if (!(check.Struct("ImplErrorContext").GlobalNamespace()
                || check.Class("ScHFEditPage").GlobalNamespace()))
          {
              report(

What exactly did it break? Genuinely curious.

Just look at the Jenkins builds for <https://gerrit.libreoffice.org/#/c/36861/> "vcl: change pImpl class names to fit with existing convention", namely <https://ci.libreoffice.org/job/lo_gerrit/10639/Config=linux_clang_dbgutil_64/console>:

In --enable-compiler-plugins --enable-werror Clang builds, compiling vcl/source/window/errinf.cxx now failed with a loplugin:vclwidgets warning/error about the changed

-struct ErrorContextImpl
+struct ImplErrorContext
 {
     vcl::Window *pWin; // FIXME: should be VclPtr for strong lifecycle
 };

as the whitelisting of that struct in compilerplugins/clang/vclwidgets.cxx still used the old name.

Context


Privacy Policy | Impressum (Legal Info) | Copyright information: Unless otherwise specified, all text and images on this website are licensed under the Creative Commons Attribution-Share Alike 3.0 License. This does not include the source code of LibreOffice, which is licensed under the Mozilla Public License (MPLv2). "LibreOffice" and "The Document Foundation" are registered trademarks of their corresponding registered owners or are in actual use as trademarks in one or more countries. Their respective logos and icons are also subject to international copyright laws. Use thereof is explained in our trademark policy.