patch: add mangling for attribute enable_if
Richard Smith
metafoo at gmail.com
Thu Apr 24 20:35:07 PDT 2014
Mangling patch LGTM with s/saved/Saved/.
On Thu Apr 24 2014 at 7:46:10 PM, Nick Lewycky <nlewycky at google.com> wrote:
> On 9 March 2014 17:10, Richard Smith <richard at metafoo.co.uk> wrote:
>
>> Your lambda doesn't capture anything -- maybe use [] rather than [&]?
>> (Maybe we should warn on this.)
>>
>> Maybe use a reverse_iterator loop over FD->getAttrs, and skip the
>> non-EnableIfAttrs. That'd remove the need to make a copy of the vector and
>> to reverse it. (Do we have llvm::reversed yet? If so, 'for (Attr *A :
>> reversed(FD->getAttrs()))' seems like a nice way to write this.)
>>
>
> Done.
>
> Updated patches attached, please review.
>
> Nick
>
>
>> On Sun, Mar 9, 2014 at 5:02 PM, Nick Lewycky <nicholas at mxc.ca> wrote:
>>
>>> On 03/09/2014 11:01 AM, David Majnemer wrote:
>>>
>>>> Your std::remove_if would be more concise if it used a lambda instead
>>>> of IsNotEnableIfAttr.
>>>>
>>>
>>> Done. Also added another test case to the mangling tests.
>>>
>>> Nick
>>>
>>> On Sun Mar 09 2014 at 10:30:45 AM, Nick Lewycky <nicholas at mxc.ca
>>>> <mailto:nicholas at mxc.ca>> wrote:
>>>>
>>>> The attached mangle-enable_if-1.patch adds a mangling for
>>>> __attribute__((enable_if(expr, string-literal))) to clang.
>>>>
>>>> demangle-enable_if-1.patch adds support to libcxxabi's demangler.
>>>> This
>>>> patch I'm not very confident in. libcxxabi's cxa_demangle lacks
>>>> comments
>>>> and assertions, leaving its design criteria a mystery. I think the
>>>> functions return 'first' in case of error. I added a vector<>
>>>> inside the
>>>> demangler, I don't know whether that's OK because it means doing
>>>> allocation. I don't know what the two strings in the pair in
>>>> db.names
>>>> are for, but the first one appears to be the demangling so I put it
>>>> there. I don't understand what the members in 'db' are for, since
>>>> they
>>>> aren't commented (db.tag_templates?). The code is cargo culted from
>>>> parse_template_args and simplified down by making wild assumptions
>>>> (db.tag_templates is always false!) and constant folding.
>>>>
>>>> Also the demangler seems pretty buggy. It wraps expressions in extra
>>>> parentheses which don't correspond to pi .. E expressions (if you
>>>> were
>>>> to remangle it, you'd get the extra pi .. E). A mangling for
>>>> "&function"
>>>> ends up demangling "&(function())" which has different semantic
>>>> meaning.
>>>> I'm assuming these problems are pre-existing. While "_Z3foo"
>>>> demangles
>>>> to "foo" and "_Z3foov" demangles to "foo()", the same things with an
>>>> attribute demangle to include parens. For example, "_Z3fooUa3bar"
>>>> and
>>>> "_Z3fooUa3barv" both demangle to "foo() __attribute__((bar))", never
>>>> "foo __attribute__((bar))".
>>>>
>>>> Finally, I've never even compiled the change to the tests. I have
>>>> tested
>>>> those exact manglings and seen what cxa_demangle does to them, but
>>>> it
>>>> was easier to write a standalone tool than to deal with "testit".
>>>>
>>>> Please review!
>>>>
>>>> Nick
>>>> _________________________________________________
>>>> cfe-commits mailing list
>>>> cfe-commits at cs.uiuc.edu <mailto:cfe-commits at cs.uiuc.edu>
>>>> http://lists.cs.uiuc.edu/__mailman/listinfo/cfe-commits
>>>> <http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits>
>>>>
>>>>
>>>
>>> _______________________________________________
>>> cfe-commits mailing list
>>> cfe-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>>
>>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140425/d266a0f5/attachment.html>
More information about the cfe-commits
mailing list