On Jul 25, 2012, at 10:19 AM, Timur Iskhodzhanov wrote: > On Wed, Jul 25, 2012 at 8:15 PM, Timur Iskhodzhanov <[email protected]> > 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 ... >> >>> 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] > Are there any important entries in TypeNodes.td which I've skipped? > I looked only at TYPE lines, skipped a few things intentionally: > - BlockPointer's - we can mangle them the way we want as it's a Clang > extension > - MemberPointer - not supported now anyways > - *Array's -> PR13182 > - ObjC* This is also a Clang extension. > - Atomic Don't know how. Does VC even support C11 _Atomic? > > ... and a few I don't fully undersand: > - Vector - ?? Can't for the general case. For certain cases (i.e. the ones corresponding to the __mXXX types from the mmintrin headers), we can mangle it. > - FunctionNoProto - ?? Should never ever happen, because all functions have a prototype in C++ (if you omit the parameter list, 'void' is assumed).
Chip > > >>> 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. >> >>> John. > _______________________________________________ > cfe-commits mailing list > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
