[PATCH] D52933: Fix incorrect Twine usage in CFGPrinter

[Marcin Copik via Phabricator via llvm-commits]

mcopik added a comment.

@grosser @kristina The superfluous comment is removed.



================
Comment at: include/llvm/Analysis/CFGPrinter.h:176
+    // Evaluate in a single statement to avoid dangling pointers in a Twine.
+    return ("label=\"W:" + Twine(Weight->getZExtValue()) + "\"").str();
   }
----------------
JonasToth wrote:
> lebedev.ri wrote:
> > JonasToth wrote:
> > > lebedev.ri wrote:
> > > > Reading the http://llvm.org/docs/ProgrammersManual.html#dss-twine, i *do* think the fix is correct.
> > > > The old code would call `str()` on a `Twine` stored on a stack,
> > > > which is clearly stated as broken in the manual.
> > > The pattern for Twine would be `Twine("label=\"W:") + Weight->getZExtValue() + "\"").str()`, why use `Twine` for the second value instead of the first one?
> > > The fix itself should still be correct.
> > That doesn't compile https://godbolt.org/z/7UhuUJ
> Fair point :D
@JonasToth I didn't want to change the original code and only remove the dangling pointer to a stack temporary object. I see that @lebedev.ri already answered your question; I think the problem is that addition operators for Twine are defined only for Twine and StringRef objects. I think there's a conversion to StringRef via ctor for c-string but not for uint64_t.


https://reviews.llvm.org/D52933