On Thu, Jul 19, 2012 at 11:22 PM, John McCall <[email protected]> wrote: > On Jul 19, 2012, at 4:44 AM, Timur Iskhodzhanov wrote: >> On Thu, Jul 19, 2012 at 3:37 PM, João Matos <[email protected]> wrote: >>> Not sure who is responsible for the MS mangler, the LLVM code owner >>> list doesn't list anyone directly responsible for non-codegen Windows >>> stuff. It would be nice to see someone assigned to this task, at the >>> moment the Windows patches take a lot of time to be reviewed or get >>> forgotten on the list. >> I totally agree here. > > It's me, and I'm afraid you'll just have to deal with me not being very > responsive. I have a lot of things to watch and not as much time to do so > as I might like. ok
João, Please prefer "I like the patch but wait for John's review" over "Looks good to me" then :) >>> By the way, I don't think we need to enable blocks support in this test >>> case. >> Good point! >> I've removed the unnecessary flags from the cc1 invocation, see the >> updated patch > > I don't really understand what's going on in this patch at all. If there's a > consistent rule here, it'd be nice to see a comment talking about it. Ooops - I wanted to update the comment but forgotten to. Will do tomorrow. > Otherwise it seems very, very ad-hoc, I do think this is more of a general rule as the same ABCD encoding should be used to fix http://llvm.org/PR13182 too. Maybe I should merge these two fixes into one patch and extract a "mangleCVQualifiers" function? WDYT? I was planning to have this as two separate patches. > which makes me worried that > this it only works for the test cases we've run through it already. That's TDD, right? :) I build Chromium tests to generate test cases - I get lots of link-time errors when the mangler is wrong. The suggested patch does fix mangling of return types needed to have a mixed cl/clang build of Chromium and I haven't observed any other/new return type mangling problems since applying the fix locally. My rationale of the TDD/a-bit-ad-hoc approach is that we constantly and incrementally move forward and don't regress on known manglings. I don't think it's immediately possible to come up with "one patch to fix them all" :) Suggestions are welcome! > John. _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
