On Fri, 2012-01-20 at 18:25 +0100, Stephan Bergmann wrote:
Hi Dave, all,
Attached is a cppadditions.patch (against recent git head of
gcc-python-plugin) that adds a number of attributes to various gcc.Tree
classes, mainly relevant for C++. It is surely far from polished and
there's probably some issues with it. Anyway, let me know what you
think about it.
Thanks - this looks very promising. Unfortunately I'm a bit doomed
right now dealing with a security vulnerability in another piece of
software, so I may take a few days to look at this properly.
The most obvious issue with the patch (based on a quick visual review)
is that it adds lots of new functionality to the plugin, but is missing
unit tests for each new part of the functionality. For example, I see
you added a "values" attribute to gcc.EnumeralType. That looks useful,
but I'm left wondering how I'd programatically access both the names and
the numeric values. Ideally such a change also ought to add a test e.g.
a directory "tests/plugin/enums" or somesuch, with an:
input.c or (input.cc for C++)
script.py (exercising the relevant code)
stdout.txt: containing the expected output from the script.
Or put it below "tests/examples"; these can be included by reference
from the docs, so that we have documentation that's automatically
verified by run-test-suite.py, and users can use this to see the
relationship between source-code constructs and the corresponding Python
objects.
More information can be seen in run-test-suite.py
You can generate the "gold" stdout.txt by hacking up this line in
run-test-suite.py:
out.check_for_diff(out.actual, err.actual, p, args, 'stdout', 0)
so that the final 0 is a 1 (the "writeback" argument to check_for_diff).
There may need to be a non-empty stdout.txt file in the directory for
this to take effect though.
(I'd write the tests myself, but need to focus on the aforementioned
security issue; sorry).
BTW, looking at gcc.EnumeralType.values, returning a list of
gcc.TreeList instances may be the wrong thing to do: it's probably
better to convert it directly to just a python list of gcc.Tree
instances, which ought to make for a more usable API (though I haven't
actually run the code yet, so I could be misreading this. A test case
should make this clear: does the final API seem pleasing?).
* I added c_string_quote because of problems with quotes (") in
docu
strings (where I shamelessly reused the documentation present in GCC's
tree.h and cp-tree.h, and that already contained those quotes). The
implementation of that function is surely evidence that I am no native
Python programmer... ;)
:)
* I needed to add cu.add_include("cp/cp-tree.h") to
generate-tree-c.py
-- not sure if that is acceptable (it would fail for GCC installations
that do not include C++ support).
We're already using it in gcc-python-tree.c:
#include "cp/cp-tree.h" /* for TFF_* for use by
gcc_FunctionDecl_get_fullname */
I made those additions to be able to write the attached
unusedparameters.py, which I used to find unused function parameters in
the LibreOffice code base: When we made that code base warning-clean a
number of years ago, many warnings about unused parameters were silenced
by removing the parameter name, instead of actually fixing the code by
dropping the unused parameter. Of course, dropping an unused parameter
is not always the right fix, e.g., in virtual functions (where some
overrides require the parameter and others don't, so it consistently
needs to be present in all overrides). The script looks for unused,
unnamed parameters (so that they can be revisited now, deciding on a
proper fix), but takes into account patterns where the parameter is
likely legitimately unused (like in the case of virtual functions).
However, there are still a number of legitimately unused function
parameters that the script does not catch (think about functions that
need to adhere to a certain signature, so their addresses can be
assigned to function pointers of given type). So I started to annotate
those cases with GCC's __attribute__ ((unused)) in the LibreOffice
source code (and let unusedparameters.py ignore those). One problem I
found with that is that for constructors (and only for constructors),
the annotation needs to go into the constructor's declaration instead of
the definition, for unusedparameters.py to find it within
arg.attributes. That is,
class C { C(int); };
C::C(__attribute__ ((unused)) int) {}
causes unusedparameters.py to erroneously warn about the int parameter,
while
class C { C(__attribute__ ((unused)) int); };
C::C(int) {}
has the desired effect.
Does anybody happen to know whether that is a problem of GCC itself (I
observed it with Fedora 16's gcc-c++-4.6.2-1.fc16.x86_64) or of
gcc-python-plugin?
I don't know; sorry.
Hope the above is helpful
Dave