[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