[Bug default/30048] incorrect qualifiers within compound types

["dodji at seketeli dot org" <sourceware-bugzilla@sourceware.org>]

https://sourceware.org/bugzilla/show_bug.cgi?id=30048

--- Comment #8 from dodji at seketeli dot org ---
Hello Giuliano,

gprocida at google dot com via Libabigail <libabigail@sourceware.org> a
écrit:

> I don't feel qualified to review the code change

OK, no problem, I understand.

> and I haven't had the chance to patch and test it (but I'm sure you
> have!).

No problem.  And yes, I have tested it against the reproducer of this
problem report.

> However, I can offer some test case suggestions.
>
> Firstly, printing out C types is a hard problem!

Heh.  Yeah, I know. Libabigail's implementation is a bit different
from the standard C or C++ one, even if it's very close.  There are
things that are simplified so that it looks easier to understand.  Also,
by default, it could be look not too much "alien" for compilers of other
languages that might have produced the type info we are looking at.

> I worked on an implementation
> that seems to have mostly survived over a year or so of use:
>
> https://android.googlesource.com/platform/external/stg/+/refs/heads/master/naming.h
> https://android.googlesource.com/platform/external/stg/+/refs/heads/master/naming.cc
>
> The important code here is Name, which consolidates precedence handling, and
> the Describe function object, which works recursively on the type graph. I
> documented the original design approach here:
>
> https://android.googlesource.com/platform/external/stg/+/refs/heads/master/doc/NAMES.md
>
> I think the main takeaway is that qualifiers can be "tacked on" without
> disturbing the rest of the code too much but some contextual awareness is
> needed to make sure the qualifiers end up on the correct side with appropriate
> whitespace. It would have been significantly easier if the world (or C itself)
> just had "int const" and not "const int".

Thanks!

> In terms of testing... this is my torture test for naming and diffs. The
> comments are (or at least were at one point) valid input for cdecl(1).
>
> Please feel free to use or adapt this for libabigail. If you need copyright /
> licence attribution it's: Google Inc. 2020, LLVM licence (Apache + LLVM
> exception), same as libabigail.

[...]

Thanks!  I have added this test to the libabigail test suite.  After
(hopefully carefully) starring at the report of abidiff on it, it looks
like the output is what I would expect, with the patch I proposed.

I thus went ahead and applied the patch to the master branch at
https://sourceware.org/git/?p=libabigail.git;a=commit;h=03d8b52fc7a510edaf3182a3600cc54c0a180fe7.

Thanks!

-- 
You are receiving this mail because:
You are on the CC list for the bug.