Add replacement for symbol table
authorPaul Bauer <paul.bauer.q@gmail.com>
Thu, 21 Mar 2019 13:27:39 +0000 (14:27 +0100)
committerMark Abraham <mark.j.abraham@gmail.com>
Wed, 29 Jan 2020 17:11:36 +0000 (18:11 +0100)
Refs #2833, #2916

Change-Id: Ibd771444933ef4b01f5a3eca9db3efbbbff2926f

14 files changed:
src/gromacs/topology/symtab.cpp
src/gromacs/topology/symtab.h
src/gromacs/topology/tests/refdata/LegacySymtabTest_AddLargeNumberOfEntries.xml [new file with mode: 0644]
src/gromacs/topology/tests/refdata/LegacySymtabTest_NoDuplicatesInLargeTable.xml [new file with mode: 0644]
src/gromacs/topology/tests/refdata/StringTableTest_AddLargeNumberOfEntries.xml [new file with mode: 0644]
src/gromacs/topology/tests/refdata/StringTableTest_AddSingleEntry.xml [new file with mode: 0644]
src/gromacs/topology/tests/refdata/StringTableTest_AddTwoDistinctEntries.xml [new file with mode: 0644]
src/gromacs/topology/tests/refdata/StringTableTest_CanAccessWithAt.xml [new file with mode: 0644]
src/gromacs/topology/tests/refdata/StringTableTest_CanAccessWithBracket.xml [new file with mode: 0644]
src/gromacs/topology/tests/refdata/StringTableTest_NoDuplicatesInLargeTable.xml [new file with mode: 0644]
src/gromacs/topology/tests/refdata/StringTableTest_StringCompareIsCorrect.xml [new file with mode: 0644]
src/gromacs/topology/tests/refdata/StringTableTest_ThrowsOutOfRange.xml [new file with mode: 0644]
src/gromacs/topology/tests/refdata/StringTableTest_TryToAddDuplicates.xml [new file with mode: 0644]
src/gromacs/topology/tests/symtab.cpp

index deaec5e175097b37ef14e8b9a6ca87432c84c010..ea075e411b4696ab90d318a034981bc5a446680d 100644 (file)
  * To help us fund GROMACS development, we humbly ask that you cite
  * the research papers on the package. Check out http://www.gromacs.org.
  */
+/*! \internal \file
+ * \brief
+ * Implements new and legacy symbol table routines.
+ *
+ * \author David van der Spoel <david.vanderspoel@icm.uu.se>
+ * \author Paul Bauer <paul.bauer.q@gmail.com>
+ * \ingroup module_topology
+ */
 #include "gmxpre.h"
 
 #include "symtab.h"
 
 #include "gromacs/utility/basedefinitions.h"
 #include "gromacs/utility/cstringutil.h"
+#include "gromacs/utility/exceptions.h"
 #include "gromacs/utility/fatalerror.h"
+#include "gromacs/utility/iserializer.h"
 #include "gromacs/utility/smalloc.h"
+#include "gromacs/utility/stringutil.h"
 #include "gromacs/utility/txtdump.h"
 
-constexpr int c_trimSize   = 1024;
+StringTableEntry StringTableBuilder::addString(const std::string& theString)
+{
+    int         size     = map_.size();
+    std::string stripped = gmx::stripString(theString);
+
+    const auto foundEntry = map_.insert(StringTablePair(stripped, size));
+    return StringTableEntry(foundEntry.first->first, foundEntry.first->second);
+}
+
+int StringTableBuilder::findEntryByName(const std::string& name) const
+{
+    auto foundEntry = map_.find(name);
+    if (foundEntry != map_.end())
+    {
+        return foundEntry->second;
+    }
+    else
+    {
+        GMX_THROW(gmx::InternalError(
+                gmx::formatString("Could not find string \"%s\" in SymbolTable", name.c_str())));
+    }
+}
+
+StringTable StringTableBuilder::build()
+{
+    std::vector<std::string> table(map_.size());
+    for (const auto& entry : map_)
+    {
+        table[entry.second] = entry.first;
+    }
+    map_.clear();
+    return StringTable(table);
+}
+
+void StringTable::printStringTableStorageToFile(FILE* fp, int indent, const char* title) const
+{
+    indent = pr_title_n(fp, indent, title, table_.size());
+    int i  = 0;
+    for (const auto& entry : table_)
+    {
+        pr_indent(fp, indent);
+        fprintf(fp, "%s[%d]=\"%s\"\n", title, i++, entry.c_str());
+    }
+}
+
+StringTable::StringTable(gmx::ISerializer* serializer)
+{
+    GMX_RELEASE_ASSERT(serializer->reading(),
+                       "Can not use writing serializer to read string table");
+    int nr = 0;
+    serializer->doInt(&nr);
+    table_.resize(nr);
+    for (auto& entry : table_)
+    {
+        serializer->doString(&entry);
+    }
+}
+
+void StringTable::serializeStringTable(gmx::ISerializer* serializer)
+{
+    GMX_RELEASE_ASSERT(!serializer->reading(),
+                       "Can not use reading serializer to write string table");
+    int nr = table_.size();
+    serializer->doInt(&nr);
+    for (auto& entry : table_)
+    {
+        serializer->doString(&entry);
+    }
+}
+
+StringTableEntry StringTable::at(gmx::index index) const
+{
+    if (index >= gmx::ssize(table_))
+    {
+        GMX_THROW(gmx::InternalError("Can't read beyond last entry"));
+    }
+    return StringTableEntry(table_[index], index);
+}
+
+StringTableEntry StringTable::operator[](gmx::index index) const
+{
+    GMX_ASSERT(index < gmx::ssize(table_), "Can't read beyond last entry");
+    return StringTableEntry(table_[index], index);
+}
+
+void StringTableEntry::serialize(gmx::ISerializer* serializer) const
+{
+    GMX_RELEASE_ASSERT(!serializer->reading(),
+                       "Can not use reading serializer to write string index");
+    int entry = tableIndex_;
+    serializer->doInt(&entry);
+}
+
+StringTableEntry readStringTableEntry(gmx::ISerializer* serializer, const StringTable& table)
+{
+    GMX_RELEASE_ASSERT(serializer->reading(),
+                       "Can not use writing serializer to read string index");
+    int entry = 0;
+    serializer->doInt(&entry);
+    return table.at(entry);
+}
+
+// Old code for legacy data structure starts below.
+//! Maximum size of character string in table.
+constexpr int c_trimSize = 1024;
+//! Maximum number of entries in each element of the linked list.
 constexpr int c_maxBufSize = 5;
 
-static char* trim_string(const char* s, char* out, int maxlen)
-/*
- * Returns a pointer to a static area which contains a copy
- * of s without leading or trailing spaces. Strings are
- * truncated to c_trimSize positions.
+/*! \brief
+ * Remove leading and trailing whitespace from string and enforce maximum length.
  *
- * TODO This partially duplicates code in trim(), but perhaps
- * replacing symtab with a std::map is a better fix.
+ * \param[in]    s      String to trim.
+ * \param[inout] out    String to return.
+ * \param[in]    maxlen Maximum string length to use.
+ * \returns New pruned string.
  */
+static char* trim_string(const char* s, char* out, int maxlen)
 {
     int len, i;
 
@@ -133,6 +249,7 @@ char** get_symtab_handle(t_symtab* symtab, int name)
     gmx_fatal(FARGS, "symtab get_symtab_handle %d not found", name);
 }
 
+//! Returns a new initialized entry into the symtab linked list.
 static t_symbuf* new_symbuf()
 {
     t_symbuf* symbuf;
@@ -145,6 +262,13 @@ static t_symbuf* new_symbuf()
     return symbuf;
 }
 
+/*! \brief
+ * Low level function to enter new string into legacy symtab.
+ *
+ * \param[inout] symtab Symbol table to add entry to.
+ * \param[in]    name   New string to add to symtab.
+ * \returns Pointer to new entry in the legacy symbol table, or to existing entry if it already existed.
+ */
 static char** enter_buf(t_symtab* symtab, char* name)
 {
     int       i;
index cfb1678e4fd8cf10a7e0ed8efdb5db81d8fc094e..b75b32d0c9cb8e3b6b9e60e9ca57a9132f4a091c 100644 (file)
@@ -3,7 +3,7 @@
  *
  * Copyright (c) 1991-2000, University of Groningen, The Netherlands.
  * Copyright (c) 2001-2004, The GROMACS development team.
- * Copyright (c) 2010,2014,2017,2018,2019, by the GROMACS development team, led by
+ * Copyright (c) 2010,2014,2017,2018,2019,2020, by the GROMACS development team, led by
  * Mark Abraham, David van der Spoel, Berk Hess, and Erik Lindahl,
  * and including many others, as listed in the AUTHORS file in the
  * top-level source directory and at http://www.gromacs.org.
  * To help us fund GROMACS development, we humbly ask that you cite
  * the research papers on the package. Check out http://www.gromacs.org.
  */
+/*! \file
+ * \brief
+ * Declares modern and legacy symbol table used to store strings of characters.
+ *
+ * \author David van der Spoel <david.vanderspoel@icm.uu.se>
+ * \author Paul Bauer <paul.bauer.q@gmail.com>
+ *
+ * \ingroup module_topology
+ * \inlibraryapi
+ */
 #ifndef GMX_TOPOLOGY_SYMTAB_H
 #define GMX_TOPOLOGY_SYMTAB_H
 
 #include <stdio.h>
 
-typedef struct t_symbuf
+#include <functional>
+#include <string>
+#include <unordered_map>
+#include <vector>
+
+#include "gromacs/utility/gmxassert.h"
+
+struct t_commrec;
+struct t_fileio;
+
+namespace gmx
+{
+class ISerializer;
+namespace test
+{
+class StringTableTest;
+} // namespace test
+} // namespace gmx
+
+//! Convenience typedef for pair stored in map.
+using StringTablePair = std::pair<std::string, int>;
+//! Convenience typedef for string reference wrapper.
+using StringReference = std::reference_wrapper<const std::string>;
+
+class StringTableBuilder;
+class StringTableEntry;
+/*! \brief
+ * A class to store strings for lookup.
+ *
+ * We store the strings in a dedicated object to avoid
+ * wrong usage of the flat string vector, and forcing people
+ * to use an object that can only be constructed from the transitional
+ * StringTableBuilder or filled during file IO.
+ *
+ * Note that strings are stripped of trailing and leading whitespace.
+ */
+class StringTable
+{
+public:
+    //! Constructor used to generate table object from file reading.
+    StringTable(gmx::ISerializer* serializer);
+    //! Can move construct.
+    StringTable(StringTable&&) = default;
+    //! Can move assign.
+    StringTable& operator=(StringTable&&) = default;
+    //! No copy constructor.
+    StringTable(const StringTable&) = delete;
+    //! No copy assign.
+    StringTable& operator=(const StringTable&) = delete;
+    /*! \brief
+     *  Access string at \p index.
+     *
+     *  \returns Entry type that constains both the string and the index,
+     *           with the index needed during (de-)serialization.
+     *  \throws  On index being out of range.
+     */
+    StringTableEntry at(gmx::index index) const;
+    //! Bracket operator.
+    StringTableEntry operator[](gmx::index index) const;
+    //! Handle file IO.
+    void serializeStringTable(gmx::ISerializer* serializer);
+
+    //! Print human readable format of storage.
+    void printStringTableStorageToFile(FILE* fp, int indent, const char* title) const;
+
+    friend class StringTableBuilder;
+
+private:
+    /*! \brief
+     * Private constructor so that only builder can create the final table.
+     *
+     * \param[in] table A vector of strings to be stored in the table.
+     */
+    StringTable(const std::vector<std::string>& table) : table_(table) {}
+
+    //! The table is stored as a vector of strings.
+    std::vector<std::string> table_;
+};
+
+/*! \brief
+ * Helper class to access members in StringTable.
+ *
+ * This class is a wrapper around a string reference to access
+ * the actual entry in the table, as well as an index used for
+ * serializing the datastructure.
+ *
+ * This also provides efficient comparison calls between different entries.
+ */
+class StringTableEntry
+{
+public:
+    //! Copy construct.
+    StringTableEntry(const StringTableEntry&) = default;
+    //! Move construct.
+    StringTableEntry(StringTableEntry&&) noexcept = default;
+    //! Copy assign.
+    StringTableEntry& operator=(const StringTableEntry&) = default;
+    //! Move assign.
+    StringTableEntry& operator=(StringTableEntry&&) = default;
+
+    //! Compare entries by indices. Same string should always have same index.
+    bool operator==(const StringTableEntry& o) const { return tableIndex_ == o.tableIndex_; }
+    //! Unequal comparisson.
+    bool operator!=(const StringTableEntry& o) const { return !(*this == o); }
+    //! Access to underlying view.
+    const std::string& operator*() const { return entry_; }
+    //! Access to underlying view.
+    const std::string* operator->() const { return &entry_.get(); }
+    //! Serialize index.
+    void serialize(gmx::ISerializer* serializer) const;
+
+    // We only allow construction from the places that are known to create
+    // valid objects for us.
+    friend StringTableEntry readStringTableEntry(gmx::ISerializer* serializer, const StringTable& table);
+    friend class StringTableBuilder;
+    friend class StringTable;
+
+private:
+    //! Only allow construct with all information present.
+    StringTableEntry(StringReference entry, int tableIndex) : entry_(entry), tableIndex_(tableIndex)
+    {
+    }
+    //! The actual string reference that is stored.
+    StringReference entry_;
+    //! The index into the table.
+    int tableIndex_ = -1;
+};
+
+/*! \brief
+ * De-serialize StringTableEntry using the index into the \p table.
+ *
+ * \param[in] serializer  The object containing the serialized index.
+ * \param[in] table       The storage object holding all strings.
+ * \returns The entry into the Table as StringTableEntry.
+ */
+StringTableEntry readStringTableEntry(gmx::ISerializer* serializer, const StringTable& table);
+
+/*! \libinternal \brief
+ * Builds a memory efficient storage for strings of characters.
+ *
+ * Allows storing strings of characters with unique entries.
+ */
+class StringTableBuilder
+{
+public:
+    /*! \brief
+     * Place new unique string in storage object.
+     *
+     * Enters new string into the underlying storage or recovers existing entry.
+     * \param[in] theString New string to enter.
+     * \returns New entry object with reference to string and index into storage.
+     *          The reference is only valid while the builder is in use, and becomes
+     *          invalidated when generating the StringTable.
+     */
+    StringTableEntry addString(const std::string& theString);
+    //! Find matching entry in storage by name as string.
+    int findEntryByName(const std::string& name) const;
+    /*! \brief
+     * Build the StringTable from the internal map of strings.
+     *
+     * The unique indices returned from addString() can be used
+     * to index into the returned StringTable. Clears the
+     * temporary storage so that the StringTableBuilder can be re-used to
+     * build a distinct StringTable.
+     */
+    StringTable build();
+
+private:
+    //! Storage object for entries.
+    std::unordered_map<std::string, int> map_;
+};
+
+// Below this is the legacy code for the old symbol table, only used in
+// deprecated datastructures.
+/*! \libinternal \brief
+ * Legacy symbol table entry as linked list.
+ */
+struct t_symbuf
 {
-    int              bufsize;
-    char**           buf;
+    //! Number of entries in this item
+    int bufsize;
+    //! Storage for strings in this item.
+    char** buf;
+    //! Next item in linked list.
     struct t_symbuf* next;
-} t_symbuf;
+};
 
-typedef struct t_symtab
+/* \libinternal \brief
+ * Legacy symbol table.
+ */
+struct t_symtab
 {
-    int       nr;
+    //! Total number of entries stored.
+    int nr;
+    //! First item in linked list of storage elements.
     t_symbuf* symbuf;
-} t_symtab;
+};
 
 /*
  * This module handles symbol table manipulation. All text strings
@@ -63,64 +258,74 @@ typedef struct t_symtab
  * back to a text string handle by get_symtab_handle().
  */
 
+//! Initialises the symbol table symtab.
 void open_symtab(t_symtab* symtab);
-/* Initialises the symbol table symtab.
- */
 
-void close_symtab(t_symtab* symtab);
-/* Undoes the effect of open_symtab(), after invoking this function,
- * no value can be added to the symbol table, only values can be
- * retrieved using get_symtab().
+/*! \brief
+ * Undoes the effect of open_symtab()
+ *
+ * After invoking this function, no value can be added to the
+ * symbol table, only values can be retrieved using get_symtab_handle().
  *
  * Note that this does no work.
+ * \param[inout] symtab Symbol table to close.
  */
+void close_symtab(t_symtab* symtab);
 
 /*! \brief Returns a deep copy of \c symtab. */
 t_symtab* duplicateSymtab(const t_symtab* symtab);
 
+//! Frees the space allocated by the symbol table itself.
 void free_symtab(t_symtab* symtab);
-/* Frees the space allocated by the symbol table itself */
 
+//! Frees the space allocated by the symbol table, including all entries in it.
 void done_symtab(t_symtab* symtab);
-/* Frees the space allocated by the symbol table, including all
- * entries in it */
-
-char** put_symtab(t_symtab* symtab, const char* name);
-/* Enters a string into the symbol table symtab, if it was not
- * available, a reference to a copy is returned else a reference
- * to the earlier entered value is returned. Strings are trimmed
- * of spaces.
- */
 
-int lookup_symtab(t_symtab* symtab, char** name);
-/* Returns a unique handle for **name, without a memory reference.
- * It is a failure when name cannot be found in the symbol table,
- * it should be entered before with put_symtab().
+/*! \brief
+ * Enters a string into the symbol table.
+ *
+ * If the string \p name was not present before, a reference to a copy is returned,
+ * else a reference to the earlier entered value is returned. Strings are trimmed of spaces.
+ *
+ * \param[inout] symtab Symbol table to add string to.
+ * \param[in] name String to add.
+ * \returns Pointer to entry of string in symtab.
  */
+char** put_symtab(t_symtab* symtab, const char* name);
 
-char** get_symtab_handle(t_symtab* symtab, int name);
-/* Returns a text string handle for name. Name should be a value
- * returned from lookup_symtab(). So get_symtab_handle() and
- * lookup_symtab() are inverse functions.
+/*! \brief
+ * Returns unique handle for \p name.
+ *
+ * Looks up the string pointer \p name in the symbol table and returns the
+ * index in it to the matching entry. Gives fatal error if \p name is
+ * not found. \p name has to be entered first using put_symtab().
+ *
+ * \param[in] symtab Symbol table to search.
+ * \param[in] name String pointer into \p symtab.
+ * \returns Unique index to position in symbol table.
  */
+int lookup_symtab(t_symtab* symtab, char** name);
 
-long wr_symtab(FILE* fp, t_symtab* symtab);
-/* Writes the symbol table symtab to the file, specified by fp.
- * The function returns the number of bytes written.
+/*! \brief
+ * Returns text string corresponding to \p index.
+ *
+ * \p index needs to be value obtained from call to lookup_symtab().
+ * get_symtab_handle() and lookup_symtab() are inverse functions.
+ *
+ * \param[in] symtab Symbol table to search.
+ * \param[in] index  Entry to find in table.
+ * \returns String pointer into \p symtab corresponding to the entry.
  */
+char** get_symtab_handle(t_symtab* symtab, int index);
 
-long rd_symtab(FILE* fp, t_symtab* symtab);
-/* Reads the symbol table symtab from the file, specified by fp.
- * This will include allocating the needed space. The function
- * returns the number of bytes read. The symtab is in the closed
- * state afterwards, so no strings can be added to it.
+/*! \brief
+ * Prints human readable form of \p symtab.
+ *
+ * \param[in] fp File to print to.
+ * \param[in] indent Number of spaces to use for indentation.
+ * \param[in] title Name for header text.
+ * \param[in] symtab Symbol table to print out.
  */
-
 void pr_symtab(FILE* fp, int indent, const char* title, t_symtab* symtab);
-/* This routine prints out a (human) readable representation of
- * the symbol table symtab to the file fp. Ident specifies the
- * number of spaces the text should be indented. Title is used
- * to print a header text.
- */
 
 #endif
diff --git a/src/gromacs/topology/tests/refdata/LegacySymtabTest_AddLargeNumberOfEntries.xml b/src/gromacs/topology/tests/refdata/LegacySymtabTest_AddLargeNumberOfEntries.xml
new file mode 100644 (file)
index 0000000..9ad22db
--- /dev/null
@@ -0,0 +1,15 @@
+<?xml version="1.0"?>
+<?xml-stylesheet type="text/xsl" href="referencedata.xsl"?>
+<ReferenceData>
+  <Sequence Name="Complete dump of SymbolTable">
+    <Int Name="Length">8</Int>
+    <String>Symtab[0]="0"</String>
+    <String>Symtab[1]="1"</String>
+    <String>Symtab[2]="2"</String>
+    <String>Symtab[3]="3"</String>
+    <String>Symtab[4]="4"</String>
+    <String>Symtab[5]="5"</String>
+    <String>Symtab[6]="6"</String>
+    <String>Symtab[7]="foobar"</String>
+  </Sequence>
+</ReferenceData>
diff --git a/src/gromacs/topology/tests/refdata/LegacySymtabTest_NoDuplicatesInLargeTable.xml b/src/gromacs/topology/tests/refdata/LegacySymtabTest_NoDuplicatesInLargeTable.xml
new file mode 100644 (file)
index 0000000..b57efc4
--- /dev/null
@@ -0,0 +1,22 @@
+<?xml version="1.0"?>
+<?xml-stylesheet type="text/xsl" href="referencedata.xsl"?>
+<ReferenceData>
+  <Sequence Name="Complete dump of SymbolTable">
+    <Int Name="Length">15</Int>
+    <String>Symtab[0]="0"</String>
+    <String>Symtab[1]="1"</String>
+    <String>Symtab[2]="2"</String>
+    <String>Symtab[3]="3"</String>
+    <String>Symtab[4]="4"</String>
+    <String>Symtab[5]="5"</String>
+    <String>Symtab[6]="6"</String>
+    <String>Symtab[7]="baz"</String>
+    <String>Symtab[8]="7"</String>
+    <String>Symtab[9]="8"</String>
+    <String>Symtab[10]="9"</String>
+    <String>Symtab[11]="10"</String>
+    <String>Symtab[12]="11"</String>
+    <String>Symtab[13]="12"</String>
+    <String>Symtab[14]="13"</String>
+  </Sequence>
+</ReferenceData>
diff --git a/src/gromacs/topology/tests/refdata/StringTableTest_AddLargeNumberOfEntries.xml b/src/gromacs/topology/tests/refdata/StringTableTest_AddLargeNumberOfEntries.xml
new file mode 100644 (file)
index 0000000..1d1bfb5
--- /dev/null
@@ -0,0 +1,15 @@
+<?xml version="1.0"?>
+<?xml-stylesheet type="text/xsl" href="referencedata.xsl"?>
+<ReferenceData>
+  <String Name="Output"><![CDATA[
+    Test title (8):
+       Test title[0]="0"
+       Test title[1]="1"
+       Test title[2]="2"
+       Test title[3]="3"
+       Test title[4]="4"
+       Test title[5]="5"
+       Test title[6]="6"
+       Test title[7]="foobar"
+]]></String>
+</ReferenceData>
diff --git a/src/gromacs/topology/tests/refdata/StringTableTest_AddSingleEntry.xml b/src/gromacs/topology/tests/refdata/StringTableTest_AddSingleEntry.xml
new file mode 100644 (file)
index 0000000..fb12c9a
--- /dev/null
@@ -0,0 +1,8 @@
+<?xml version="1.0"?>
+<?xml-stylesheet type="text/xsl" href="referencedata.xsl"?>
+<ReferenceData>
+  <String Name="Output"><![CDATA[
+    Test title (1):
+       Test title[0]="foo"
+]]></String>
+</ReferenceData>
diff --git a/src/gromacs/topology/tests/refdata/StringTableTest_AddTwoDistinctEntries.xml b/src/gromacs/topology/tests/refdata/StringTableTest_AddTwoDistinctEntries.xml
new file mode 100644 (file)
index 0000000..34a8fee
--- /dev/null
@@ -0,0 +1,9 @@
+<?xml version="1.0"?>
+<?xml-stylesheet type="text/xsl" href="referencedata.xsl"?>
+<ReferenceData>
+  <String Name="Output"><![CDATA[
+    Test title (2):
+       Test title[0]="foo"
+       Test title[1]="Bar"
+]]></String>
+</ReferenceData>
diff --git a/src/gromacs/topology/tests/refdata/StringTableTest_CanAccessWithAt.xml b/src/gromacs/topology/tests/refdata/StringTableTest_CanAccessWithAt.xml
new file mode 100644 (file)
index 0000000..fb12c9a
--- /dev/null
@@ -0,0 +1,8 @@
+<?xml version="1.0"?>
+<?xml-stylesheet type="text/xsl" href="referencedata.xsl"?>
+<ReferenceData>
+  <String Name="Output"><![CDATA[
+    Test title (1):
+       Test title[0]="foo"
+]]></String>
+</ReferenceData>
diff --git a/src/gromacs/topology/tests/refdata/StringTableTest_CanAccessWithBracket.xml b/src/gromacs/topology/tests/refdata/StringTableTest_CanAccessWithBracket.xml
new file mode 100644 (file)
index 0000000..fb12c9a
--- /dev/null
@@ -0,0 +1,8 @@
+<?xml version="1.0"?>
+<?xml-stylesheet type="text/xsl" href="referencedata.xsl"?>
+<ReferenceData>
+  <String Name="Output"><![CDATA[
+    Test title (1):
+       Test title[0]="foo"
+]]></String>
+</ReferenceData>
diff --git a/src/gromacs/topology/tests/refdata/StringTableTest_NoDuplicatesInLargeTable.xml b/src/gromacs/topology/tests/refdata/StringTableTest_NoDuplicatesInLargeTable.xml
new file mode 100644 (file)
index 0000000..d054c47
--- /dev/null
@@ -0,0 +1,22 @@
+<?xml version="1.0"?>
+<?xml-stylesheet type="text/xsl" href="referencedata.xsl"?>
+<ReferenceData>
+  <String Name="Output"><![CDATA[
+    Test title (15):
+       Test title[0]="0"
+       Test title[1]="1"
+       Test title[2]="2"
+       Test title[3]="3"
+       Test title[4]="4"
+       Test title[5]="5"
+       Test title[6]="6"
+       Test title[7]="baz"
+       Test title[8]="7"
+       Test title[9]="8"
+       Test title[10]="9"
+       Test title[11]="10"
+       Test title[12]="11"
+       Test title[13]="12"
+       Test title[14]="13"
+]]></String>
+</ReferenceData>
diff --git a/src/gromacs/topology/tests/refdata/StringTableTest_StringCompareIsCorrect.xml b/src/gromacs/topology/tests/refdata/StringTableTest_StringCompareIsCorrect.xml
new file mode 100644 (file)
index 0000000..fb12c9a
--- /dev/null
@@ -0,0 +1,8 @@
+<?xml version="1.0"?>
+<?xml-stylesheet type="text/xsl" href="referencedata.xsl"?>
+<ReferenceData>
+  <String Name="Output"><![CDATA[
+    Test title (1):
+       Test title[0]="foo"
+]]></String>
+</ReferenceData>
diff --git a/src/gromacs/topology/tests/refdata/StringTableTest_ThrowsOutOfRange.xml b/src/gromacs/topology/tests/refdata/StringTableTest_ThrowsOutOfRange.xml
new file mode 100644 (file)
index 0000000..fb12c9a
--- /dev/null
@@ -0,0 +1,8 @@
+<?xml version="1.0"?>
+<?xml-stylesheet type="text/xsl" href="referencedata.xsl"?>
+<ReferenceData>
+  <String Name="Output"><![CDATA[
+    Test title (1):
+       Test title[0]="foo"
+]]></String>
+</ReferenceData>
diff --git a/src/gromacs/topology/tests/refdata/StringTableTest_TryToAddDuplicates.xml b/src/gromacs/topology/tests/refdata/StringTableTest_TryToAddDuplicates.xml
new file mode 100644 (file)
index 0000000..5889ed6
--- /dev/null
@@ -0,0 +1,10 @@
+<?xml version="1.0"?>
+<?xml-stylesheet type="text/xsl" href="referencedata.xsl"?>
+<ReferenceData>
+  <String Name="Output"><![CDATA[
+    Test title (3):
+       Test title[0]="foo"
+       Test title[1]="Bar"
+       Test title[2]="Foo"
+]]></String>
+</ReferenceData>
index 301685fdcf5d72113b5559abae0f4db4875f1b19..f52e058f33be25169372775c8400bfb7bbbe9f2b 100644 (file)
@@ -1,7 +1,7 @@
 /*
  * This file is part of the GROMACS molecular simulation package.
  *
- * Copyright (c) 2019, by the GROMACS development team, led by
+ * Copyright (c) 2019,2020, by the GROMACS development team, led by
  * Mark Abraham, David van der Spoel, Berk Hess, and Erik Lindahl,
  * and including many others, as listed in the AUTHORS file in the
  * top-level source directory and at http://www.gromacs.org.
 
 #include <gtest/gtest.h>
 
+#include "gromacs/utility/arrayref.h"
+#include "gromacs/utility/exceptions.h"
+#include "gromacs/utility/inmemoryserializer.h"
 #include "gromacs/utility/strconvert.h"
 #include "gromacs/utility/stringutil.h"
+#include "gromacs/utility/textreader.h"
 
 #include "testutils/refdata.h"
+#include "testutils/testfilemanager.h"
 
 namespace gmx
 {
@@ -57,14 +62,338 @@ namespace gmx
 namespace test
 {
 
+class StringTableTest : public ::testing::Test
+{
+public:
+    StringTableTest() {}
+    //! Get handle to symbol table.
+    StringTableBuilder& builder() { return stringTableBuilder_; }
+    /* \brief
+     * Check human readable format of the table.
+     * \todo change when table writing to users is done also with serializer.
+     * \parm[in] table The string table to check the serialization.
+     */
+    void checkTable(const StringTable& table);
+
+private:
+    //! Get reference checker using lazy initialization
+    TestReferenceChecker* checker()
+    {
+        if (!checker_)
+        {
+            checker_ = std::make_unique<TestReferenceChecker>(data_.rootChecker());
+        }
+        return checker_.get();
+    }
+    //! Symbol table for testing purposes.
+    StringTableBuilder stringTableBuilder_;
+    //! Handler for reference data.
+    TestReferenceData data_;
+    //! Handler for checking reference data.
+    std::unique_ptr<TestReferenceChecker> checker_;
+};
+
+void StringTableTest::checkTable(const StringTable& table)
+{
+    TestFileManager files;
+    std::string     filename(files.getTemporaryFilePath("table.txt"));
+    FILE*           fp = fopen(filename.c_str(), "w");
+    table.printStringTableStorageToFile(fp, 4, "Test title");
+    fclose(fp);
+    const std::string text = TextReader::readFileToString(filename);
+    checker()->checkTextBlock(text, "Output");
+}
+
+/*! \brief
+ *  Check that symbols obtained from symtab compare correctly.
+ *
+ *  Helper function to find out if two entries obtained by a symtab lookup
+ *  are equivalent or not, according to testing criteria.
+ *  Checks that the indices match before finalizing storage.
+ *
+ *  \param[in] builder     StringTableBuilder table that contains the entries to validate.
+ *  \param[in] firstSymbol Handle into \p builder obtained from placing string in \p builder.
+ *  \param[in] otherSymbol Other handle from obtained from separate string deposit.
+ *  \param[in] expectedOutcome If the handles should result in equal entries or not.
+ */
+static void compareDifferentIndices(const StringTableBuilder& builder,
+                                    const StringTableEntry&   firstSymbol,
+                                    const StringTableEntry&   otherSymbol,
+                                    bool                      expectedOutcome)
+{
+    EXPECT_EQ(expectedOutcome, (firstSymbol == otherSymbol));
+    auto firstIndex = builder.findEntryByName(*firstSymbol);
+    auto otherIndex = builder.findEntryByName(*otherSymbol);
+    EXPECT_EQ(expectedOutcome, (firstIndex == otherIndex))
+            << "Expected was " << expectedOutcome << " firstIndex is " << firstIndex
+            << " otherIndex is " << otherIndex;
+}
+
+/*! \brief
+ * Helper to obtain the integer index from an entry.
+ *
+ * As the index is only used during (de-) serialization, use this machinery
+ * to obtain it.
+ *
+ * \param[in] symbol Single StringTableEntry to obtain the index of.
+ * \returns Integer index to be used to obtain value from StringTable.
+ */
+static int readIndexFromSerializer(const StringTableEntry& symbol)
+{
+    gmx::InMemorySerializer writer;
+    symbol.serialize(&writer);
+    auto                      buffer = writer.finishAndGetBuffer();
+    gmx::InMemoryDeserializer reader(buffer, false);
+    int                       index = 0;
+    reader.doInt(&index);
+    return index;
+}
+
+/*! \brief
+ * Helper function to check that a string in matches when looked up in non finalized table.
+ *
+ * Checks that a string looked up by using the index in the symbol table matches
+ * the string stored in the wrapper object obtained by entering a string.
+ *
+ * \param[in] symtab Symbol table that contains the entries.
+ * \param[in] symbol The entry obtained from placing a string in the symbol table.
+ * \param[in] string The string the entry should match.
+ */
+static void stringMatches(const StringTable& symtab, const StringTableEntry& symbol, const char* string)
+{
+    int  index          = readIndexFromSerializer(symbol);
+    auto entryFromIndex = symtab.at(index);
+
+    EXPECT_EQ(*entryFromIndex, string)
+            << "Index is " << index << " Entry from index is " << entryFromIndex->c_str();
+}
+
+
+TEST_F(StringTableTest, AddSingleEntry)
+{
+    builder().addString("foo");
+    StringTable table = builder().build();
+    checkTable(table);
+}
+
+TEST_F(StringTableTest, CanAccessWithAt)
+{
+    builder().addString("foo");
+    StringTable table = builder().build();
+    EXPECT_NO_THROW(table.at(0));
+    checkTable(table);
+}
+
+TEST_F(StringTableTest, CanAccessWithBracket)
+{
+    builder().addString("foo");
+    StringTable table = builder().build();
+    checkTable(table);
+    auto entry = table[0];
+    EXPECT_EQ(*entry, "foo");
+}
+
+TEST_F(StringTableTest, ThrowsOutOfRange)
+{
+    builder().addString("foo");
+    StringTable table = builder().build();
+    EXPECT_THROW(table.at(1), InternalError);
+    checkTable(table);
+}
+
+TEST_F(StringTableTest, StringCompareIsCorrect)
+{
+    auto        fooSymbol = builder().addString("foo");
+    StringTable table     = builder().build();
+    stringMatches(table, fooSymbol, "foo");
+    checkTable(table);
+}
+
+TEST_F(StringTableTest, AddTwoDistinctEntries)
+{
+    auto fooSymbol = builder().addString("foo");
+    auto barSymbol = builder().addString("Bar");
+
+    EXPECT_FALSE(fooSymbol == barSymbol);
+    compareDifferentIndices(builder(), fooSymbol, barSymbol, false);
+    EXPECT_TRUE("foo" == *fooSymbol);
+    EXPECT_TRUE("Bar" == *barSymbol);
+    auto table = builder().build();
+    stringMatches(table, fooSymbol, "foo");
+    stringMatches(table, barSymbol, "Bar");
+    checkTable(table);
+}
+
+TEST_F(StringTableTest, TryToAddDuplicates)
+{
+    auto fooSymbol = builder().addString("foo");
+    auto barSymbol = builder().addString("Bar");
+
+    EXPECT_FALSE(fooSymbol == barSymbol);
+    EXPECT_FALSE(fooSymbol->empty());
+    compareDifferentIndices(builder(), fooSymbol, barSymbol, false);
+    EXPECT_TRUE("foo" == *fooSymbol);
+    EXPECT_TRUE("Bar" == *barSymbol);
+
+    // Insert a duplicate element
+    auto anotherFooSymbol = builder().addString("foo");
+    // Insert element with different case
+    auto capitalFooSymbol = builder().addString("Foo");
+
+    // Check that no duplicate is made
+    EXPECT_TRUE(fooSymbol == anotherFooSymbol);
+    // Check case sensitivity
+    EXPECT_FALSE(fooSymbol == capitalFooSymbol);
+
+    // Check that underlying representation is same
+    EXPECT_TRUE("foo" == *anotherFooSymbol);
+    EXPECT_TRUE("foo" == *fooSymbol);
+    EXPECT_FALSE(*fooSymbol == *capitalFooSymbol);
+    EXPECT_TRUE("Bar" == *barSymbol);
+
+    // Check for correct behaviours with new and old symbols
+    compareDifferentIndices(builder(), fooSymbol, anotherFooSymbol, true);
+    compareDifferentIndices(builder(), barSymbol, anotherFooSymbol, false);
+    compareDifferentIndices(builder(), fooSymbol, barSymbol, false);
+    compareDifferentIndices(builder(), fooSymbol, capitalFooSymbol, false);
+    auto table = builder().build();
+    checkTable(table);
+}
+
+TEST_F(StringTableTest, AddLargeNumberOfEntries)
+{
+    int                           numStringsToAdd = 7; // Random number of strings.
+    std::vector<StringTableEntry> symbolsAdded;
+    symbolsAdded.reserve(numStringsToAdd);
+    for (int i = 0; i < numStringsToAdd; ++i)
+    {
+        symbolsAdded.push_back(builder().addString(toString(i)));
+    }
+    for (int i = 0; i < numStringsToAdd; ++i)
+    {
+        EXPECT_TRUE(toString(i) == *symbolsAdded[i]) << "index is " << i;
+    }
+    // Add something unrelated and check that indices still work afterward.
+    builder().addString("foobar");
+    for (int i = 0; i < numStringsToAdd; ++i)
+    {
+        EXPECT_TRUE(toString(i) == *symbolsAdded[i]) << "index is " << i;
+    }
+    auto table = builder().build();
+    for (int i = 0; i < numStringsToAdd; ++i)
+    {
+        stringMatches(table, symbolsAdded[i], toString(i).c_str());
+    }
+    checkTable(table);
+}
+
+TEST_F(StringTableTest, NoDuplicatesInLargeTable)
+{
+    int                           halfOfStringsToAdd   = 7; // Random number of strings.
+    int                           totalNumStringsToAdd = 2 * halfOfStringsToAdd;
+    std::vector<StringTableEntry> symbolsAdded;
+    symbolsAdded.reserve(halfOfStringsToAdd);
+    for (int i = 0; i < halfOfStringsToAdd; ++i)
+    {
+        symbolsAdded.push_back(builder().addString(toString(i)));
+    }
+
+    // We now try to mess around in the symtab.
+    auto bazSymbol = builder().addString("baz");
+
+    // Now try to add more symbols, also including those that are already there.
+    for (int i = 0; i < totalNumStringsToAdd; i++)
+    {
+        symbolsAdded.push_back(builder().addString(toString(i)));
+    }
+
+    //! Check that entries that should be equal are, and new ones are not.
+    for (int i = 0; i < halfOfStringsToAdd; i++)
+    {
+        compareDifferentIndices(builder(), symbolsAdded[i], symbolsAdded[halfOfStringsToAdd + i], true);
+        compareDifferentIndices(builder(), symbolsAdded[i], symbolsAdded[2 * halfOfStringsToAdd + i], false);
+        compareDifferentIndices(builder(), symbolsAdded[i], bazSymbol, false);
+    }
+    EXPECT_TRUE("baz" == *bazSymbol);
+    symbolsAdded.emplace_back(bazSymbol);
+    auto table = builder().build();
+    checkTable(table);
+}
+
+TEST_F(StringTableTest, CanWriteToBuffer)
+{
+    builder().addString("foo");
+    builder().addString("bar");
+    builder().addString("baz");
+    auto               finalTable = builder().build();
+    InMemorySerializer writer;
+    finalTable.serializeStringTable(&writer);
+
+    auto buffer = writer.finishAndGetBuffer();
+    EXPECT_EQ(buffer.size(), 37); // 4 (size) + 3*(8 (string size) + 3*1 (char size) )
+}
+
+TEST_F(StringTableTest, Roundtrip)
+{
+    // First generate a buffer from a string table
+    builder().addString("foo");
+    builder().addString("bar");
+    builder().addString("baz");
+    auto               finalTable = builder().build();
+    InMemorySerializer writer;
+    finalTable.serializeStringTable(&writer);
+
+    auto buffer = writer.finishAndGetBuffer();
+    EXPECT_EQ(buffer.size(), 37); // 4 (size) + 3*(8 (string size) + 3*1 (char size) )
+
+    // Now try to make a new table from it.
+    InMemoryDeserializer reader(buffer, false);
+    StringTable          readInTable(&reader);
+    EXPECT_EQ(*(finalTable.at(0)), *(readInTable.at(0)));
+    EXPECT_EQ(*(finalTable.at(1)), *(readInTable.at(1)));
+    EXPECT_EQ(*(finalTable.at(2)), *(readInTable.at(2)));
+}
+
+TEST_F(StringTableTest, RoundtripWithCorrectStringIndices)
+{
+    std::vector<StringTableEntry> testEntries;
+    // First generate a buffer from a string table
+    testEntries.emplace_back(builder().addString("foo"));
+    testEntries.emplace_back(builder().addString("bar"));
+    testEntries.emplace_back(builder().addString("baz"));
+    auto               finalTable = builder().build();
+    InMemorySerializer writer;
+    finalTable.serializeStringTable(&writer);
+    for (const auto& stringEntry : testEntries)
+    {
+        stringEntry.serialize(&writer);
+    }
+
+    auto buffer = writer.finishAndGetBuffer();
+    EXPECT_EQ(buffer.size(), 49); // 4 (size) + 3*(8 (string size) + 3*1 (char size) + 3*4 (int size))
+
+    // Now try to make a new table from it.
+    InMemoryDeserializer          reader(buffer, false);
+    StringTable                   readInTable(&reader);
+    std::vector<StringTableEntry> deserializedEntries;
+    for (index gmx_unused i = 0; i < gmx::ssize(testEntries); i++)
+    {
+        deserializedEntries.emplace_back(readStringTableEntry(&reader, readInTable));
+    }
+    EXPECT_EQ(*(finalTable.at(0)), *(deserializedEntries[0]));
+    EXPECT_EQ(*(finalTable.at(1)), *(deserializedEntries[1]));
+    EXPECT_EQ(*(finalTable.at(2)), *(deserializedEntries[2]));
+}
+
+
 namespace
 {
 
-class SymtabTest : public ::testing::Test
+class LegacySymtabTest : public ::testing::Test
 {
 public:
-    SymtabTest() { open_symtab(&symtab_); }
-    ~SymtabTest() override
+    LegacySymtabTest() { open_symtab(&symtab_); }
+    ~LegacySymtabTest() override
     {
         done_symtab(&symtab_);
         EXPECT_EQ(symtab_.nr, 0);
@@ -94,7 +423,7 @@ private:
     std::unique_ptr<TestReferenceChecker> checker_;
 };
 
-void SymtabTest::dumpSymtab()
+void LegacySymtabTest::dumpSymtab()
 {
     int                      nr     = symtab_.nr;
     t_symbuf*                symbuf = symtab_.symbuf;
@@ -161,13 +490,13 @@ void compareDifferentHandles(t_symtab* symtab, char** firstSymbol, char** otherS
     EXPECT_EQ(expectedOutcome, entriesAreEqual(symtab, otherSymbol, firstIndex));
 }
 
-TEST_F(SymtabTest, EmptyOnOpen)
+TEST_F(LegacySymtabTest, EmptyOnOpen)
 {
     ASSERT_EQ(0, symtab()->nr);
     ASSERT_EQ(nullptr, symtab()->symbuf);
 }
 
-TEST_F(SymtabTest, AddSingleEntry)
+TEST_F(LegacySymtabTest, AddSingleEntry)
 {
     auto fooSymbol = put_symtab(symtab(), "Foo");
     ASSERT_EQ(1, symtab()->nr);
@@ -175,7 +504,7 @@ TEST_F(SymtabTest, AddSingleEntry)
     EXPECT_STREQ("Foo", *fooSymbol);
 }
 
-TEST_F(SymtabTest, AddTwoDistinctEntries)
+TEST_F(LegacySymtabTest, AddTwoDistinctEntries)
 {
     auto fooSymbol = put_symtab(symtab(), "Foo");
     auto barSymbol = put_symtab(symtab(), "Bar");
@@ -190,7 +519,7 @@ TEST_F(SymtabTest, AddTwoDistinctEntries)
     EXPECT_STREQ("Bar", *barSymbol);
 }
 
-TEST_F(SymtabTest, TryToAddDuplicates)
+TEST_F(LegacySymtabTest, TryToAddDuplicates)
 {
     auto fooSymbol = put_symtab(symtab(), "Foo");
     auto barSymbol = put_symtab(symtab(), "Bar");
@@ -220,7 +549,7 @@ TEST_F(SymtabTest, TryToAddDuplicates)
     compareDifferentHandles(symtab(), fooSymbol, barSymbol, false);
 }
 
-TEST_F(SymtabTest, AddLargeNumberOfEntries)
+TEST_F(LegacySymtabTest, AddLargeNumberOfEntries)
 {
     int                 numStringsToAdd = 7; // Larger than c_maxBufSize limit for size of symbuf.
     std::vector<char**> symbolsAdded;
@@ -249,7 +578,7 @@ TEST_F(SymtabTest, AddLargeNumberOfEntries)
     dumpSymtab();
 }
 
-TEST_F(SymtabTest, NoDuplicatesInLargeTable)
+TEST_F(LegacySymtabTest, NoDuplicatesInLargeTable)
 {
     int halfOfStringsToAdd   = 7; // Larger than c_maxBufSize limit for size of symbuf.
     int totalNumStringsToAdd = 2 * halfOfStringsToAdd;