Runtime stack overflow prevention for Twines

Will Dietz w at wdtz.org
Tue May 27 15:19:37 PDT 2014


LGTM.

@Code owners: Ping!

This patch builds cleanly and passes lit tests locally, FWIW.

Quick scan through the commit logs shows there have been multiple
Twine bugs fixed that this change would have made a compile-time error
(instead of being fixed later, presumably after some head-scratching):

r206804
r205107
r205105
r200457

Which helps motivate the change.  It's a trade-off-- in theory a
programmer who "knows what they were doing"
could employ the assignment operator correctly, but on the other hand
all the above bugs (and more) might have
been avoided if this patch were applied at the time.  Furthermore,
since it seems the assignment operator is unused
in all of LLVM/Clang/lldb/other-projects-I-have-locally (and
historically seems to only be used in ways that
are later reverted due to causing bugs) this seems like a good step
for helping prevent future misuse.

Thoughts/comments/review welcome! :)

~Will

On Fri, May 16, 2014 at 8:34 AM, Marco Alesiani <marco.diiga at gmail.com> wrote:
> Compile-time prevention of runtime stack overflow when concatenating Twines
> (infinite recursion). Attached patch, test case and a plain English
> explanation of the problem.
>
> --
>
> __________________________
>
> Marco Alesiani
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>



More information about the llvm-commits mailing list