Linker code cleanups

Turn `Linker::Link()` into free functions

  As very little information was kept in the Linker class, we can get rid
  of the whole class and have the `Link()` as free functions instead; the
  environment target as well as the consumer are passed along through an
  `spv_context` object.
  The resulting linked_binary is passed as a pointer rather than a
  reference to follow the Google C++ Style guidelines.

  Addresses remaining comments from
  https://github.com/KhronosGroup/SPIRV-Tools/pull/693 about the SPIR-V
  linker.

Fix variable naming in the linker

  Some of the variables were using mixed case, which did not follow the
  Google C++ Style guidelines.

Linker: Use EXPECT_EQ when possible and update some test

* Replace occurrences of ASSERT_EQ by EXPECT_EQ when possible;
* Reformulated some of the error messages;
* Added the symbol name in the error message when there is a type or
  decoration mismatch between the imported and exported declarations.

Opt: List all duplicates removed by RemoveDuplicatePass in the header

Opt: Make the const version of GetLabelInst() return a pointer

  For consistency with the non-const version, as well as other similar
  functions.

Opt: Rename function_end to EndInst()

  As pointed out by dneto0 the previous name was quite confusing and could
  be mistaken with a function returning an end iterator.
  Also change the return type of the const version to a pointer rather
  than a reference, for consistency.

Opt: Add performance comment to RemoveDuplicateTypes and decorations

  This comment was requested during the review of
  https://github.com/KhronosGroup/SPIRV-Tools/pull/693.

Opt: Add comments and fix variable naming in RemoveDuplicatePass

* Add missing comments to private functions;
* Rename variables that were using mixed case;
* Add TODO for moving AreTypesEqual out.

Linker: Remove commented out code and add TODOs

Linker: Merged together strings that were too much splitted

Implement a C++ RAII wrapper around spv_context

3 files changed, 68 insertions, 59 deletions

diff --git a/include/spirv-tools/libspirv.hpp b/include/spirv-tools/libspirv.hpp
index 9dbcf16e..6b9ad279 100644
--- a/include/spirv-tools/libspirv.hpp
+++ b/include/spirv-tools/libspirv.hpp
@@ -31,6 +31,39 @@ using MessageConsumer = std::function<void(
     const spv_position_t& /* position */, const char* /* message */
     )>;
 
+// C++ RAII wrapper around the C context object spv_context.
+class Context {
+ public:
+  // Constructs a context targeting the given environment |env|.
+  //
+  // The constructed instance will have an empty message consumer, which just
+  // ignores all messages from the library. Use SetMessageConsumer() to supply
+  // one if messages are of concern.
+  explicit Context(spv_target_env env);
+
+  // Enables move constructor/assignment operations.
+  Context(Context&& other);
+  Context& operator=(Context&& other);
+
+  // Disables copy constructor/assignment operations.
+  Context(const Context&) = delete;
+  Context& operator=(const Context&) = delete;
+
+  // Destructs this instance.
+  ~Context();
+
+  // Sets the message consumer to the given |consumer|. The |consumer| will be
+  // invoked once for each message communicated from the library.
+  void SetMessageConsumer(MessageConsumer consumer);
+
+  // Returns the underlying spv_context.
+  spv_context& CContext();
+  const spv_context& CContext() const;
+
+ private:
+  spv_context context_;
+};
+
 // A RAII wrapper around a validator options object.
 class ValidatorOptions {
  public:
diff --git a/include/spirv-tools/linker.hpp b/include/spirv-tools/linker.hpp
index a36aa75f..df56251f 100644
--- a/include/spirv-tools/linker.hpp
+++ b/include/spirv-tools/linker.hpp
@@ -26,9 +26,7 @@ namespace spvtools {
 
 class LinkerOptions {
  public:
-  LinkerOptions()
-      : createLibrary_(false),
-        verifyIds_(false) {}
+  LinkerOptions() : create_library_(false), verify_ids_(false) {}
 
   // Returns whether a library or an executable should be produced by the
   // linking phase.
@@ -37,75 +35,48 @@ class LinkerOptions {
   // be removed when creating an executable.
   // The returned value will be true if creating a library, and false if
   // creating an executable.
-  bool GetCreateLibrary() const { return createLibrary_; }
+  bool GetCreateLibrary() const { return create_library_; }
 
   // Sets whether a library or an executable should be produced.
   void SetCreateLibrary(bool create_library) {
-    createLibrary_ = create_library;
+    create_library_ = create_library;
   }
 
   // Returns whether to verify the uniqueness of the unique ids in the merged
   // context.
-  bool GetVerifyIds() const { return verifyIds_; }
+  bool GetVerifyIds() const { return verify_ids_; }
 
   // Sets whether to verify the uniqueness of the unique ids in the merged
   // context.
-  void SetVerifyIds(bool verifyIds) {
-    verifyIds_ = verifyIds;
-  }
+  void SetVerifyIds(bool verify_ids) { verify_ids_ = verify_ids; }
 
  private:
-  bool createLibrary_;
-  bool verifyIds_;
+  bool create_library_;
+  bool verify_ids_;
 };
 
-class Linker {
- public:
-  // Constructs an instance targeting the given environment |env|.
-  //
-  // The constructed instance will have an empty message consumer, which just
-  // ignores all messages from the library. Use SetMessageConsumer() to supply
-  // one if messages are of concern.
-  explicit Linker(spv_target_env env);
-
-  // Disables copy/move constructor/assignment operations.
-  Linker(const Linker&) = delete;
-  Linker(Linker&&) = delete;
-  Linker& operator=(const Linker&) = delete;
-  Linker& operator=(Linker&&) = delete;
-
-  // Destructs this instance.
-  ~Linker();
-
-  // Sets the message consumer to the given |consumer|. The |consumer| will be
-  // invoked once for each message communicated from the library.
-  void SetMessageConsumer(MessageConsumer consumer);
-
-  // Links one or more SPIR-V modules into a new SPIR-V module. That is,
-  // combine several SPIR-V modules into one, resolving link dependencies
-  // between them.
-  //
-  // At least one binary has to be provided in |binaries|. Those binaries do
-  // not have to be valid, but they should be at least parseable.
-  // The functions can fail due to the following:
-  // * No input modules were given;
-  // * One or more of those modules were not parseable;
-  // * The input modules used different addressing or memory models;
-  // * The ID or global variable number limit were exceeded;
-  // * Some entry points were defined multiple times;
-  // * Some imported symbols did not have an exported counterpart;
-  // * Possibly other reasons.
-  spv_result_t Link(const std::vector<std::vector<uint32_t>>& binaries,
-                    std::vector<uint32_t>& linked_binary,
-                    const LinkerOptions& options = LinkerOptions()) const;
-  spv_result_t Link(const uint32_t* const* binaries, const size_t* binary_sizes,
-                    size_t num_binaries, std::vector<uint32_t>& linked_binary,
-                    const LinkerOptions& options = LinkerOptions()) const;
-
- private:
-  struct Impl;  // Opaque struct for holding the data fields used by this class.
-  std::unique_ptr<Impl> impl_;  // Unique pointer to implementation data.
-};
+// Links one or more SPIR-V modules into a new SPIR-V module. That is, combine
+// several SPIR-V modules into one, resolving link dependencies between them.
+//
+// At least one binary has to be provided in |binaries|. Those binaries do not
+// have to be valid, but they should be at least parseable.
+// The functions can fail due to the following:
+// * The given context was not initialised using `spvContextCreate()`;
+// * No input modules were given;
+// * One or more of those modules were not parseable;
+// * The input modules used different addressing or memory models;
+// * The ID or global variable number limit were exceeded;
+// * Some entry points were defined multiple times;
+// * Some imported symbols did not have an exported counterpart;
+// * Possibly other reasons.
+spv_result_t Link(const Context& context,
+                  const std::vector<std::vector<uint32_t>>& binaries,
+                  std::vector<uint32_t>* linked_binary,
+                  const LinkerOptions& options = LinkerOptions());
+spv_result_t Link(const Context& context, const uint32_t* const* binaries,
+                  const size_t* binary_sizes, size_t num_binaries,
+                  std::vector<uint32_t>* linked_binary,
+                  const LinkerOptions& options = LinkerOptions());
 
 }  // namespace spvtools
 
diff --git a/include/spirv-tools/optimizer.hpp b/include/spirv-tools/optimizer.hpp
index 2141bda4..b9096481 100644
--- a/include/spirv-tools/optimizer.hpp
+++ b/include/spirv-tools/optimizer.hpp
@@ -410,7 +410,12 @@ Optimizer::PassToken CreateAggressiveDCEPass();
 // The pass remaps result ids to a compact and gapless range starting from %1.
 Optimizer::PassToken CreateCompactIdsPass();
 
-// Creates a remove duplicate capabilities pass.
+// Creates a remove duplicate pass.
+// This pass removes various duplicates:
+// * duplicate capabilities;
+// * duplicate extended instruction imports;
+// * duplicate types;
+// * duplicate decorations.
 Optimizer::PassToken CreateRemoveDuplicatesPass();
 
 // Creates a CFG cleanup pass.