I have not been able to come up with a testcase which does better than what test/Tooling/multi-jobs.cpp already does.
The current test status: 1. no remove_if (and thus no erase) : multi-jobs fails 2. remove_if : multi-jobs pass 3. remove_if + erase : multi-jobs pass Invoking clang-check with '-v' shows that in cases 2 & 3 the -no-integrated-as has been dropped. I feel a bit like hunting a red herring there : depending on the argument position, it gets dropped or not. Jordan's hypothesis is probably right. Cheers, -- Arnaud On Wed, Apr 30, 2014 at 11:30 PM, Jordan Rose <[email protected]> wrote: > My hypothesis for how this was working: std::remove_if reorders elements > using std::move, so we were getting lucky that -no-integrated-as wasn't the > last argument and something else got moved on top of it. We'd have > duplicates of the last few arguments (because they got moved earlier), but > that wouldn't show -no-integrated-as. > > Jordan > > > On Apr 30, 2014, at 14:09, Richard Smith <[email protected]> wrote: > > This needs a test case. This code has already been committed, incorrectly, > twice before this revision; the existing test case clearly doesn't actually > cover it properly. > > > On Wed, Apr 30, 2014 at 12:59 PM, Arnaud A. de Grandmaison < > [email protected]> wrote: > >> Author: aadg >> Date: Wed Apr 30 14:59:22 2014 >> New Revision: 207696 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=207696&view=rev >> Log: >> PR19601: std::remove_if does not really remove the elements. >> >> It moves them at the end of the range instead, so an extra erase is >> needed. >> >> It is strange that this code works without the erase. On the other hand, >> removing the remove_if will make fail some tests. >> >> Modified: >> cfe/trunk/lib/Tooling/CompilationDatabase.cpp >> >> Modified: cfe/trunk/lib/Tooling/CompilationDatabase.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/CompilationDatabase.cpp?rev=207696&r1=207695&r2=207696&view=diff >> >> ============================================================================== >> --- cfe/trunk/lib/Tooling/CompilationDatabase.cpp (original) >> +++ cfe/trunk/lib/Tooling/CompilationDatabase.cpp Wed Apr 30 14:59:22 2014 >> @@ -238,8 +238,9 @@ static bool stripPositionalArgs(std::vec >> >> // Remove -no-integrated-as; it's not used for syntax checking, >> // and it confuses targets which don't support this option. >> - std::remove_if(Args.begin(), Args.end(), >> - MatchesAny(std::string("-no-integrated-as"))); >> + Args.erase(std::remove_if(Args.begin(), Args.end(), >> + >> MatchesAny(std::string("-no-integrated-as"))), >> + Args.end()); >> >> const std::unique_ptr<driver::Compilation> Compilation( >> NewDriver->BuildCompilation(Args)); >> >> >> _______________________________________________ >> 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
