On Tue, Apr 17, 2012 at 10:28 PM, David Blaikie <[email protected]> wrote: > On Tue, Apr 17, 2012 at 11:19 AM, Timur Iskhodzhanov > <[email protected]> wrote: >> Hi John, >> >> Can you please review the attached patch and suggest how to make it >> better if needed? >> See also http://llvm.org/bugs/show_bug.cgi?id=12574 >> >> Questions: >> 1) It was tested only on simple class constructor/destructor, without >> inheritance etc. >> However, since the code was broken before, supporting at least a >> simple case sounds like a win anyways. >> I'm not sure how to make a more general solution at the moment. >> [FTR: the idea of the extra conditions was taken from Dmitry's patch >> for Microsoft C++ ABI support] >> >> 2) How should I test that apps built with "-cxx-abi microsoft" actually work? >> I've written a short "smoke" test that runs the resultant .exe but >> it won't work on Linux. >> For me, it has required a few hacks to work on Windows too [e.g. set >> LIB env in the lit.cfg]. >> I don't think the test I wrote is a good solution... >> Alternative suggestions are welcome! >> For example, I can re-make the test as a compile-only test just to >> ensure we don't have infinite loops anymore. > > We don't generally have execution tests in the regression suite as far > as I know. The Clang regression test should simply verify that the > emitted LLVM bitcode is correct (using filecheck - see other codegen > tests for examples, I expect). You could check for particular features > that indicate the infinite loop bug that previously existed. I see, thanks! Will update the test tomorrow.
[Still would be nice to have the code change reviewed.] > - David > >> >> Thanks! >> Timur Iskhodzhanov, >> Google Russia >> >> _______________________________________________ >> 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
