Ah, I understand now. Thanks. > -----Original Message----- > From: [email protected] [mailto:[email protected]] On > Behalf Of Aaron Ballman > Sent: 17 July 2014 15:51 > To: Daniel Sanders > Cc: Alp Toker; llvm cfe > Subject: Re: r213265 - Using a std::string instead of a StringRef because the > Default case synthesizes a temporary std::string from a Twine. Assigning that > into a StringRef causes the StringRef to refer to a temporary, and bad things > happen. > > On Thu, Jul 17, 2014 at 10:39 AM, Daniel Sanders > <[email protected]> wrote: > > Thanks for fixing this. It also caused some other buildbot failures which > > I'm > in the middle of debugging. > > > > It's my understanding that the Args.MakeArgString() call a few lines further > down is supposed to convert the temporary string into a string with the same > lifetime as the argument list. That's clearly not happening so I'm wondering > if > there's a possible bug there. Have I misunderstood MakeArgString()? > > No, MarkArgString() turns out to be immaterial. The problem is that ("+" + > ABIName).str() results in a std::string temporary object whose lifetime is > that > of the full expression containing it. That resulting StringRef (from the > StringSwitch call chain) is referencing data which is then destroyed after the > assignment takes place. > > ~Aaron > > > > >> -----Original Message----- > >> From: [email protected] [mailto:cfe-commits- > >> [email protected]] On Behalf Of Aaron Ballman > >> Sent: 17 July 2014 15:09 > >> To: Alp Toker > >> Cc: llvm cfe > >> Subject: Re: r213265 - Using a std::string instead of a StringRef > >> because the Default case synthesizes a temporary std::string from a > >> Twine. Assigning that into a StringRef causes the StringRef to refer > >> to a temporary, and bad things happen. > >> > >> On Thu, Jul 17, 2014 at 10:03 AM, Alp Toker <[email protected]> wrote: > >> > > >> > On 17/07/2014 16:28, Aaron Ballman wrote: > >> >> > >> >> Author: aaronballman > >> >> Date: Thu Jul 17 08:28:50 2014 > >> >> New Revision: 213265 > >> >> > >> >> URL: http://llvm.org/viewvc/llvm-project?rev=213265&view=rev > >> >> Log: > >> >> Using a std::string instead of a StringRef because the Default > >> >> case synthesizes a temporary std::string from a Twine. Assigning > >> >> that into a StringRef causes the StringRef to refer to a > >> >> temporary, and bad things happen. > >> >> > >> >> This fixes a failing test case on Windows. > >> >> > >> >> Modified: > >> >> cfe/trunk/lib/Driver/Tools.cpp > >> >> > >> >> Modified: cfe/trunk/lib/Driver/Tools.cpp > >> >> URL: > >> >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Tools.cpp > >> >> ?re v=213265&r1=213264&r2=213265&view=diff > >> >> > >> >> > >> > ========================================================== > >> =========== > >> >> ========= > >> >> --- cfe/trunk/lib/Driver/Tools.cpp (original) > >> >> +++ cfe/trunk/lib/Driver/Tools.cpp Thu Jul 17 08:28:50 2014 > >> >> @@ -1037,12 +1037,12 @@ static void getMIPSTargetFeatures(const > >> >> ABIName = getGnuCompatibleMipsABIName(ABIName); > >> >> // Always override the backend's default ABI. > >> >> - StringRef ABIFeature = llvm::StringSwitch<StringRef>(ABIName) > >> >> - .Case("32", "+o32") > >> >> - .Case("n32", "+n32") > >> >> - .Case("64", "+n64") > >> >> - .Case("eabi", "+eabi") > >> >> - .Default(("+" + ABIName).str()); > >> >> + std::string ABIFeature = llvm::StringSwitch<StringRef>(ABIName) > >> >> + .Case("32", "+o32") > >> >> + .Case("n32", "+n32") > >> >> + .Case("64", "+n64") > >> >> + .Case("eabi", "+eabi") > >> >> + .Default(("+" + ABIName).str()); > >> > > >> > > >> > Is the optimizer smart enough to eliminate the evaluation of that > >> > str() call or will the string be built and discarded every time the > >> > StringSwitch is run? > >> > >> I'm not certain (I've not tested it), but I would guess the str() > >> call is evaluated each time because it is used as an argument in a > >> function call which is evaluated. > >> > >> ~Aaron > >> _______________________________________________ > >> 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
