[PATCH] D103641: [scudo] Rework Vector/String

[Vitaly Buka via Phabricator via llvm-commits]

vitalybuka added a comment.

Local buffer is LGTM, but CantGrow looks like unrelated change and a good candidate for a separate patch.



================
Comment at: compiler-rt/lib/scudo/standalone/string_utils.h:21
 public:
-  explicit ScopedString(uptr MaxLength) : String(MaxLength), Length(0) {
+  explicit ScopedString(uptr InitialSize = 0) : String(InitialSize) {
     String[0] = '\0';
----------------
Interface will be cleaner without InitialSize, we removed it sanitizer_common some time ago.

maybe for a separate patch:
uptr length() const { return buffer_.size() - 1; } can retire Length 


================
Comment at: compiler-rt/lib/scudo/standalone/vector.h:20
 // small vectors. The current implementation supports only POD types.
-template <typename T> class VectorNoCtor {
+template <typename T, bool CanGrow> class VectorNoCtor {
 public:
----------------
As is seems all users of VectorNoCtor will need to re-think consequences of failed push_back.
Interface mimics std::vector, but it's unexpected that after N push_back() container may be smaller than N.

Maybe we can we rather set max grow size for Trusty and crash if it's reached. Which also a problem, but at least easy to detect.

another alternative is to assume that Trusty will can handle Grow in most cases and when it cant, we can handle that explicitly on caller side by checking container size before push_back


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103641/new/

https://reviews.llvm.org/D103641