Re: [PATCH] Introducing compiler plugin support

2012-10-12 Thread Lubos Lunak
On Tuesday 09 of October 2012, Lubos Lunak wrote:
> On Friday 05 of October 2012, Lubos Lunak wrote:
> >  Hello,
> >
> >  attached is my implementation of basic build support for using Clang
> > compiler plugin during building LO. I'm posting it here first in case our
> > build system people have different ideas to how I have integrated it in
> > the build system, if there are no comments, I will push it.
>
>  Pushed. There is a README included, but some highlights:

 One thing I forgot:

 If you use this together with ccache, you need ccache git checkout 
(specifically [1], not even 3.1.8 is new enough), otherwise ccache will 
misinterpret '-Xclang loplugin' as a second source file to compile and will 
not cache anything. For those using packages from home:llunak:ccache in OBS, 
you need to update too.

[1] 
http://gitweb.samba.org/?p=ccache.git;a=blobdiff;f=compopt.c;h=77b57f592b13f0c3cf8216ad2760ade421d0cb44;hp=35d51ad43d5e5a94ccf0d8a6c4e9ddaedccab631;hb=8e005b067d8c2423e24ee14ffdee8343f650f1e8;hpb=806b511643ec830f50d6d6975b2ecfd1876a6f17

-- 
 Lubos Lunak
 l.lu...@suse.cz
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice


Re: [PATCH] Introducing compiler plugin support

2012-10-09 Thread Caolán McNamara
On Tue, 2012-10-09 at 17:26 +0200, Lubos Lunak wrote:
> - there is a warning about unused variables of types marked SAL_WARN_UNUSED 
> plus some C++ std classes ; quite a number of hits. Feel free to tag all 
> those Point, Rectangle, etc. classes with it too, as long as it's certain for 
> the class that a mere presence of a variable of that type does nothing in 
> practice.

That's cool, very cool.

C.

___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice


Re: [PATCH] Introducing compiler plugin support

2012-10-09 Thread Michael Stahl
> On Monday 08 of October 2012, Michael Stahl wrote:
>> On 05/10/12 18:22, Lubos Lunak wrote:
>>> +$(CLANGOUTDIR)/compileplugin.so: $(2)
>>> +$(CLANGOUTDIR)/compileplugin.so: CLANGOBJS += $(2)
>>> +
>>> +$(CLANGOUTDIR)/compileplugin.so: $(CLANGOBJS)
>>
>> there's a bit of redundancy there.
> 
>  That actually seems to be needed for whatever reason.

d'oh, now i remember: target local variables only work in a rule or in a
target local variable definition; specifically that
"$(CLANGOUTDIR)/compileplugin.so: $(CLANGOBJS)" dependency evaluates
global $(CLANGOBJS) and it will be empty.

sorry for not noticing this common pitfall earlier.

___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice


Re: [PATCH] Introducing compiler plugin support

2012-10-09 Thread Lubos Lunak
On Friday 05 of October 2012, Lubos Lunak wrote:
>  Hello,
>
>  attached is my implementation of basic build support for using Clang
> compiler plugin during building LO. I'm posting it here first in case our
> build system people have different ideas to how I have integrated it in the
> build system, if there are no comments, I will push it.

 Pushed. There is a README included, but some highlights:

- is enabled automatically by --enable-dbgutil if Clang headers are found or 
manually using --enable-compiler-plugins
- I'll enable it for the Clang tinderbox soon
- for the time being, -Werror does not affect these warnings, just in case 
somebody builds with --enable-dbgutil --enable-werror, for obvious reasons 
below
- there is a warning about unused variables of types marked SAL_WARN_UNUSED 
plus some C++ std classes ; quite a number of hits. Feel free to tag all 
those Point, Rectangle, etc. classes with it too, as long as it's certain for 
the class that a mere presence of a variable of that type does nothing in 
practice.
- there is a warning about

if( a != 0 )
b = 2;
c = 3;

 which triggers a lot of warnings, many of them about code like

if( a == 1 )
foo();
else
if( a == 2 )
bar();
whatever();

 This is a valid warning, because it may appear that the whatever() call is 
part of the else block. I consider this to be poorly formatted code and find 
moving the second if up to the else line a much better solution than trying 
to detect this case in the compiler check.

 There may be possibly some problems that I haven't noticed, so feel free to 
point out whatever you thing the plugin detects wrong. I'll eventually 
make -Werror have an effect on these too for the --enable-werror crowd.

On Monday 08 of October 2012, Michael Stahl wrote:
> On 05/10/12 18:22, Lubos Lunak wrote:
> > +$(CLANGOUTDIR)/compileplugin.so: $(2)
> > +$(CLANGOUTDIR)/compileplugin.so: CLANGOBJS += $(2)
> > +
> > +$(CLANGOUTDIR)/compileplugin.so: $(CLANGOBJS)
>
> there's a bit of redundancy there.

 That actually seems to be needed for whatever reason.

-- 
 Lubos Lunak
 l.lu...@suse.cz
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice


Re: [PATCH] Introducing compiler plugin support

2012-10-08 Thread Michael Stahl
On 05/10/12 18:22, Lubos Lunak wrote:
> 
>  Hello,
> 
>  attached is my implementation of basic build support for using Clang 
> compiler 
> plugin during building LO. I'm posting it here first in case our build system 
> people have different ideas to how I have integrated it in the build system, 
> if there are no comments, I will push it.

hmm... so that effectively needs to be built as a "bootstrap" step and
the build system basically assumes that it's been done already.  which
seems appropriate since probably ~everything would need to have a
dependency on it otherwise.

the stuff used for bootstrap seems to live in the top-level directory
and solenv/ (and dmake/) currently... but i don't really have an opinion
on how that should look...

only problem is that this seems to generate files below the new
compilerplugins/ dir, in the source tree...  which doesn't make it any
worse than other parts of the bootstrapping.

> +$(CLANGOUTDIR)/compileplugin.so: $(2)
> +$(CLANGOUTDIR)/compileplugin.so: CLANGOBJS += $(2)
> +
> +$(CLANGOUTDIR)/compileplugin.so: $(CLANGOBJS)

there's a bit of redundancy there.
hmm actually the .so only has .o dependecies so could just use $^ in the
command.

> +$(CLANGOUTDIR):
> + mkdir -p $(CLANGOUTDIR)

i vaguely remember problems with directories in make 3.81 but that was
probably with pattern rules... ah right in a trivial test this seems to
work with 3.81.

___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice


[PATCH] Introducing compiler plugin support

2012-10-05 Thread Lubos Lunak

 Hello,

 attached is my implementation of basic build support for using Clang compiler 
plugin during building LO. I'm posting it here first in case our build system 
people have different ideas to how I have integrated it in the build system, 
if there are no comments, I will push it.

-- 
 Lubos Lunak
 l.lu...@suse.cz
From 6c63f9809ecaa63e2db6fb606b573ab12f09752f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Lubo=C5=A1=20Lu=C5=88=C3=A1k?= 
Date: Fri, 5 Oct 2012 18:17:13 +0200
Subject: [PATCH] initial support for clang compiler plugins

The plugin is intentionally built using a custom Makefile,
because it's used by gbuild, so I don't want to build the plugin
using gbuild too. It is also intentionally not placed under workdir/,
as that is cleaned by 'make clean', the plugin is cleaned only
by 'make distclean', so that cleaning it doesn't cause ccache misses.
Right now there's a relatively trivial plugin that just checks
for unused rtl::OUString variables.

Change-Id: Ic05eba8d6260eec123c9e699eb5385abfe1b832f
---
 Makefile.top  |6 ++-
 compilerplugins/.gitignore|1 +
 compilerplugins/Makefile  |   23 +
 compilerplugins/Makefile-clang.mk |   47 +
 compilerplugins/Makefile.mk   |   29 +++
 compilerplugins/clang/compileplugin.cxx   |   66 
 compilerplugins/clang/compileplugin.hxx   |   14 ++
 compilerplugins/clang/unusedvariablecheck.cxx |   67 +
 compilerplugins/clang/unusedvariablecheck.hxx |   36 +
 config_host.mk.in |1 +
 configure.in  |   39 ++
 solenv/gbuild/platform/com_GCC_class.mk   |2 +
 solenv/gbuild/platform/com_GCC_defs.mk|6 +++
 13 files changed, 335 insertions(+), 2 deletions(-)
 create mode 100644 compilerplugins/.gitignore
 create mode 100644 compilerplugins/Makefile
 create mode 100644 compilerplugins/Makefile-clang.mk
 create mode 100644 compilerplugins/Makefile.mk
 create mode 100644 compilerplugins/clang/compileplugin.cxx
 create mode 100644 compilerplugins/clang/compileplugin.hxx
 create mode 100644 compilerplugins/clang/unusedvariablecheck.cxx
 create mode 100644 compilerplugins/clang/unusedvariablecheck.hxx

diff --git a/Makefile.top b/Makefile.top
index bb632a2..bd85703 100644
--- a/Makefile.top
+++ b/Makefile.top
@@ -350,10 +350,12 @@ ifeq ($(CROSS_COMPILING),YES)
 	rm -rf $(SRCDIR)/*/$(INPATH_FOR_BUILD)
 endif
 
+include $(SRCDIR)/compilerplugins/Makefile.mk
+
 #
 # Distclean
 #
-distclean : clean
+distclean : clean compilerplugins-clean
 ifeq ($(BUILD_DMAKE),YES)
 	(if [ -f dmake/Makefile ] ; then $(GNUMAKE) -j $(GMAKE_PARALLELISM) -C dmake distclean; fi) && \
 	rm -f solenv/*/bin/dmake*
@@ -391,7 +393,7 @@ endif
 #
 bootstrap: $(WORKDIR)/bootstrap
 
-$(WORKDIR)/bootstrap: solenv/bin/concat-deps.c
+$(WORKDIR)/bootstrap: solenv/bin/concat-deps.c compilerplugins
 	@cd $(SRCDIR) && ./bootstrap
 	@mkdir -p $(dir $@) && touch $@
 
diff --git a/compilerplugins/.gitignore b/compilerplugins/.gitignore
new file mode 100644
index 000..b672fde
--- /dev/null
+++ b/compilerplugins/.gitignore
@@ -0,0 +1 @@
+obj
diff --git a/compilerplugins/Makefile b/compilerplugins/Makefile
new file mode 100644
index 000..4281c12
--- /dev/null
+++ b/compilerplugins/Makefile
@@ -0,0 +1,23 @@
+# -*- Mode: makefile-gmake; tab-width: 4; indent-tabs-mode: t -*-
+#
+# This file is part of the LibreOffice project.
+#
+# This Source Code Form is subject to the terms of the Mozilla Public
+# License, v. 2.0. If a copy of the MPL was not distributed with this
+# file, You can obtain one at http://mozilla.org/MPL/2.0/.
+#
+
+ifeq ($(SOLARENV),)
+ifeq ($(gb_Side),)
+gb_Side:=host
+endif
+include $(dir $(realpath $(lastword $(MAKEFILE_LIST../config_$(gb_Side).mk
+endif
+
+include $(SRCDIR)/compilerplugins/Makefile.mk
+
+all: build
+build: compilerplugins
+clean: compilerplugins-clean
+
+# vim: set noet sw=4 ts=4:
diff --git a/compilerplugins/Makefile-clang.mk b/compilerplugins/Makefile-clang.mk
new file mode 100644
index 000..9c93931
--- /dev/null
+++ b/compilerplugins/Makefile-clang.mk
@@ -0,0 +1,47 @@
+#
+# This file is part of the LibreOffice project.
+#
+# This Source Code Form is subject to the terms of the Mozilla Public
+# License, v. 2.0. If a copy of the MPL was not distributed with this
+# file, You can obtain one at http://mozilla.org/MPL/2.0/.
+#
+
+# Make sure variables in this Makefile do not conflict with other variables (e.g. from gbuild).
+
+CLANGSRC=compileplugin.cxx unusedvariablecheck.cxx
+
+CLANGDIR=/usr
+CLANGBUILD=/usr
+CLANGDEFS=-D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -fno-rtti
+CLANGINCLUDES=-I$(CLANGDIR)/include -I$(CLANGDIR)/tools/clang/include -I$(CLANGBUILD)/include -I$(CLANGBUILD)/tools/clang/include 
+CLANGCXXFLAGS=-O2 -Wall -g
+
+CLANGINDIR=$(SRCDIR)