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.

[Daniel Sanders]

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()?

> -----Original Message-----
> From: cfe-commits-bounces at cs.uiuc.edu [mailto:cfe-commits-
> bounces at cs.uiuc.edu] 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 <alp at nuanti.com> 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
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits