Re: [PATCH] Introducing compiler plugin support
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
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
> 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
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
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
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)