[PATCH] D38767: Make Twine's copy constructor private
Zachary Turner via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Oct 10 15:04:09 PDT 2017
zturner created this revision.
Herald added subscribers: hiraditya, klimek.
I was running across a lot of abuse of `Twine`, some of which probably worked, and some of which came very close to skirting the line of safety. Examples of this include:
1. Accepting `Twine` by value instead of by const reference.
2. Returning `Twine`s by value from functions.
3. Storing `Twine`s by value in classes.
and others. The 3rd of those mentioned was particularly onerous because it can happen without you even knowing it, for example if you have:
template<class T>
struct storage {
T t;
};
and then you use a function such as `make_storage` to perfectly forward args through, and one of those happens to be a twine.
Ultimately, `Twine` really should not be copyable. The header file even hints at this by saying "functions should only accept `Twine` by const reference". But since this rule is not enforced, many violations of this were occurring.
I've made the copy constructor private here (since `Twine` internally has a few valid use cases for a copy constructor), and fixed up all uses.
In testing this, I noticed that there were no unit tests for `Twine`'s operator helpers, so I added some there (which incidentally exposed a bug in the implementation of `printOneRepr`, which is also now fixed)
https://reviews.llvm.org/D38767
Files:
clang/include/clang/Tooling/CompilationDatabase.h
clang/lib/CodeGen/CGObjCMac.cpp
clang/lib/CodeGen/CodeGenAction.cpp
clang/lib/Tooling/CompilationDatabase.cpp
clang/unittests/Tooling/TestVisitor.h
llvm/include/llvm/ADT/Twine.h
llvm/include/llvm/IR/DiagnosticInfo.h
llvm/include/llvm/Object/Error.h
llvm/include/llvm/Object/WindowsResource.h
llvm/include/llvm/Support/Error.h
llvm/include/llvm/Support/FormatVariadicDetails.h
llvm/lib/Object/Error.cpp
llvm/lib/Support/Error.cpp
llvm/lib/Support/Twine.cpp
llvm/lib/Transforms/Scalar/SROA.cpp
llvm/tools/llvm-nm/llvm-nm.cpp
llvm/tools/llvm-objcopy/Object.cpp
llvm/tools/llvm-objcopy/Object.h
llvm/tools/llvm-objcopy/llvm-objcopy.cpp
llvm/tools/llvm-objcopy/llvm-objcopy.h
llvm/unittests/ADT/TwineTest.cpp
llvm/utils/TableGen/RegisterBankEmitter.cpp
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D38767.118492.patch
Type: text/x-patch
Size: 32072 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20171010/a31fa841/attachment.bin>
More information about the llvm-commits
mailing list