Mangling patch LGTM with s/saved/Saved/. On Thu Apr 24 2014 at 7:46:10 PM, Nick Lewycky <[email protected]> wrote:
> On 9 March 2014 17:10, Richard Smith <[email protected]> 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 <[email protected]> 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 <[email protected] >>>> <mailto:[email protected]>> 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 >>>> [email protected] <mailto:[email protected]> >>>> http://lists.cs.uiuc.edu/__mailman/listinfo/cfe-commits >>>> <http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits> >>>> >>>> >>> >>> _______________________________________________ >>> 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 >> >>
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
