On Jul 25, 2012, at 9:15 AM, Timur Iskhodzhanov wrote: > On Mon, Jul 23, 2012 at 10:00 PM, John McCall <[email protected]> wrote: >> On Jul 20, 2012, at 7:16 AM, Timur Iskhodzhanov wrote: >>> See attached: >>> I've generated some more test cases for more coverage - all are OK >>> with the bug_13389_2 patch. >>> Also slightly re-factored the code to be ... well, a bit more readable. >>> >>> Some thoughts: >>> I've noticed there are a few places in the code like this: >>> ------------- >>> if (!PointeeTy.hasQualifiers()) >>> // Lack of qualifiers is mangled as 'A'. >>> Out << 'A'; >>> ------------- >>> which cover Pointer and Reference types. >>> >>> That said, when there are no qualifiers, >>> a) Pointer and Reference types add 'A' (already OK) >>> b) built-ins shouldn't have extra 'A' (already OK) >>> c) the others seem to be needing 'A'. >>> >>> So there are two ways to fix (c): >>> 1) Add an extra 'A' when mangling return types (see patch; works OK) >>> This roughly follows the logic of Pointer/Reference return type mangling. >>> 2) Always add extra 'A' when mangling >>> non-builtin-non-pointer-non-reference type (e.g. in mangleQualifiers). >>> I'm pretty sure (2) will break something else. >>> I've tried a few things (like refactoring) here and there but the >>> deeper I went the more broken it was, so I've currently abandoned it. >> >> Okay, so it sounds like the rule we have so far is this: when we're >> mangling a type as the pointee type of a pointer or reference, or as a >> return type, and the type is unqualified, and it isn't a builtin type, then >> we should add 'A'. > Yes > >> There are two things to investigate here that will help illuminate how >> to design the solution. >> >> First, is the rule really "not a builtin type", or is it "is a class type"? >> For >> example, if you have a const char **, is "const char *" mangled with an A? >> It doesn't look like this is tested right now. > Added a test for this ...
Okay. So it does look like the query really is "is a fundamental type"; a pointer to non-const pointer does not get the 'A'. >> It should be pretty easy to >> explore this with MSVC by checking out the manglings of references to >> every possible type kind. Maybe we've already checked this for a few >> types, but it's easy to flesh the list out. You should literally be scanning >> down TypeNodes.td and testing every thing you can. :) > ... as well as a few tests for enums, structs and function pointers. > Only "const function_pointer* foo()" fails right now, > most likely the same cause as > http://llvm.org/bugs/show_bug.cgi?id=13444 which I wanted to address > separately. > > See the attached patch [no code change, only new tests] Looks fine. >> Second, this seems like a pretty long list of places where we need an 'A'. >> It seems like it's more likely to be the general rule, and there are maybe >> some exceptions (parameter types?). I would suggest adding a required >> parameter for whether to use it to the qualified-type-mangling routine; >> then, as you fix the ensuing compiler errors, you can come up with >> a declaration that would send an unqualified type through that path. > I think this is what I've tried before but found myself digging way too deep. > If possible, I'd like to postpone this until it is really needed (see > also PR13182 where something like this is needed) or at least work on > this as a separate patch. Okay. John. _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
