From c25281820f886891ec68192ff3eacf18d200ae40 Mon Sep 17 00:00:00 2001 From: Vincent Ambo Date: Fri, 22 May 2020 20:29:02 +0100 Subject: fix(3p/nix/libexpr): Do not allow duplicate attribute insertion This is closer to bug-for-bug compatibility with the previous version, which would put new elements at the end of the array and (due to the linear scan) return previous ones. --- third_party/nix/src/libexpr/attr-set.cc | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/third_party/nix/src/libexpr/attr-set.cc b/third_party/nix/src/libexpr/attr-set.cc index e961156487a9..55b1d0ccd540 100644 --- a/third_party/nix/src/libexpr/attr-set.cc +++ b/third_party/nix/src/libexpr/attr-set.cc @@ -4,15 +4,29 @@ #include #include +#include #include "eval-inline.hh" namespace nix { -// TODO: using insert_or_assign might break existing Nix code because -// of the weird ordering situation. Need to investigate. +// This function inherits its name from previous implementations, in +// which Bindings was backed by an array of elements which was scanned +// linearly. +// +// In that setup, inserting duplicate elements would always yield the +// first element (until the next sort, which wasn't stable, after +// which things are more or less undefined). +// +// This behaviour is mimicked by using .insert(), which will *not* +// override existing values. void Bindings::push_back(const Attr& attr) { - attributes_.insert_or_assign(attr.name, attr); + auto [_, inserted] = attributes_.insert_or_assign(attr.name, attr); + + if (!inserted) { + DLOG(WARNING) << "attempted to insert duplicate attribute for key '" + << attr.name << "'"; + } } size_t Bindings::size() { return attributes_.size(); } -- cgit 1.4.1