[refactor] The KValue was stripped of public constructor and made abs…

…tract.

include/klee/Module/KCallable.h

@@ -10,8 +10,6 @@
 #ifndef KLEE_KCALLABLE_H
 #define KLEE_KCALLABLE_H
 
-#include <string>
-
 #include "klee/Module/KValue.h"
 #include "klee/Support/CompilerWarning.h"
 DISABLE_WARNING_PUSH
@@ -23,9 +21,12 @@ DISABLE_WARNING_DEPRECATED_DECLARATIONS
 #include "llvm/Support/Casting.h"
 DISABLE_WARNING_POP
 
+#include <functional>
+#include <string>
+
 namespace klee {
 /// Wrapper for callable objects passed in callExternalFunction
-class KCallable : public KValue {
+struct KCallable : public KValue {
 protected:
   KCallable(llvm::Value *value, KValue::Kind kind) : KValue(value, kind) {}
 
@@ -38,7 +39,7 @@ class KCallable : public KValue {
   }
 };
 
-class KInlineAsm : public KCallable {
+struct KInlineAsm : public KCallable {
 private:
   /* Prepared name of ASM code */
   std::string name;
@@ -66,6 +67,15 @@ class KInlineAsm : public KCallable {
   [[nodiscard]] llvm::InlineAsm *inlineAsm() const {
     return llvm::dyn_cast<llvm::InlineAsm>(value);
   }
+
+  [[nodiscard]] bool operator<(const KValue &rhs) const override {
+    return getKind() == rhs.getKind()
+               ? getName() < llvm::cast<KInlineAsm>(rhs).getName()
+               : getKind() < rhs.getKind();
+  }
+  [[nodiscard]] unsigned hash() const override {
+    return std::hash<std::string>{}(name);
+  }
 };
 
 } // namespace klee

include/klee/Module/KModule.h

@@ -89,9 +89,13 @@ struct KBlock : public KValue {
 
   /// Block number in function
   [[nodiscard]] uintptr_t getId() const;
+  [[nodiscard]] unsigned inModuleID() const;
+
+  [[nodiscard]] bool operator<(const KValue &rhs) const override;
+  [[nodiscard]] unsigned hash() const override;
 
   static bool classof(const KValue *rhs) {
-    return rhs->getKind() == Kind::BLOCK ? classof(cast<KBlock>(rhs)) : false;
+    return rhs->getKind() == Kind::BLOCK && classof(cast<KBlock>(rhs));
   }
 };
 
@@ -118,7 +122,7 @@ struct KCallBlock : KBlock {
   KFunction *getKFunction() const;
 
   static bool classof(const KValue *rhs) {
-    return rhs->getKind() == Kind::BLOCK ? classof(cast<KBlock>(rhs)) : false;
+    return rhs->getKind() == Kind::BLOCK && classof(cast<KBlock>(rhs));
   }
 };
 
@@ -135,7 +139,7 @@ struct KReturnBlock : KBlock {
   KBlockType getKBlockType() const override { return KBlockType::Return; };
 
   static bool classof(const KValue *rhs) {
-    return rhs->getKind() == Kind::BLOCK ? classof(cast<KBlock>(rhs)) : false;
+    return rhs->getKind() == Kind::BLOCK && classof(cast<KBlock>(rhs));
   }
 };
 
@@ -198,6 +202,9 @@ struct KFunction : public KCallable {
   [[nodiscard]] size_t getLine() const;
   [[nodiscard]] std::string getSourceFilepath() const;
 
+  [[nodiscard]] bool operator<(const KValue &rhs) const override;
+  [[nodiscard]] unsigned hash() const override;
+
   /// Unique index for KFunction and KInstruction inside KModule
   /// from 0 to [KFunction + KInstruction]
   [[nodiscard]] inline unsigned getGlobalIndex() const { return globalIndex; }
@@ -236,11 +243,17 @@ struct KConstant : public KValue {
   [[nodiscard]] static bool classof(const KValue *rhs) {
     return rhs->getKind() == KValue::Kind::CONSTANT;
   }
+
+  [[nodiscard]] bool operator<(const KValue &rhs) const override;
+  [[nodiscard]] unsigned hash() const override;
 };
 
 struct KGlobalVariable : public KValue {
 public:
-  KGlobalVariable(llvm::GlobalVariable *global);
+  // ID of the global variable
+  const unsigned id;
+
+  KGlobalVariable(llvm::GlobalVariable *global, unsigned id);
 
   [[nodiscard]] llvm::GlobalVariable *globalVariable() const {
     return llvm::dyn_cast_or_null<llvm::GlobalVariable>(value);
@@ -250,6 +263,9 @@ struct KGlobalVariable : public KValue {
     return rhs->getKind() == KValue::Kind::GLOBAL_VARIABLE;
   }
 
+  [[nodiscard]] bool operator<(const KValue &rhs) const override;
+  [[nodiscard]] unsigned hash() const override;
+
   // Filename where the global variable is defined
   [[nodiscard]] std::string getSourceFilepath() const;
   // Line number where the global variable is defined

include/klee/Module/KValue.h

@@ -28,25 +28,19 @@ struct KValue {
   /* Kind of KValue required for llvm rtti purposes */
   const Kind kind;
 
-  /* Inner constructor for setting kind */
+  /* Inner constructor for setting value and kind */
   KValue(llvm::Value *const value, const Kind kind)
       : value(value), kind(kind) {}
 
 public:
-  /* Construct KValue from raw llvm::Value. */
-  KValue(llvm::Value *const value) : KValue(value, Kind::VALUE) {
-    /* NB: heirs of this class should use inner constructor instead.
-       This constructor exposes convenient public interface. */
-  }
-
   /* Unwraps KValue to receive inner value. */
   [[nodiscard]] llvm::Value *unwrap() const;
 
   /* Returns name of inner value if so exists. */
   [[nodiscard]] virtual llvm::StringRef getName() const;
 
-  [[nodiscard]] virtual bool operator<(const KValue &rhs) const;
-  [[nodiscard]] virtual unsigned hash() const;
+  [[nodiscard]] virtual bool operator<(const KValue &rhs) const = 0;
+  [[nodiscard]] virtual unsigned hash() const = 0;
 
   /* Kind of value. Serves for llvm rtti purposes. */
   [[nodiscard]] Kind getKind() const { return kind; }

lib/Module/KModule.cpp

@@ -401,8 +401,9 @@ void KModule::manifest(InterpreterHandler *ih,
     functions.push_back(std::move(kf));
   }
 
+  unsigned globalID = 0;
   for (auto &global : module->globals()) {
-    globalMap.emplace(&global, new KGlobalVariable(&global));
+    globalMap.emplace(&global, new KGlobalVariable(&global, globalID++));
   }
 
   /* Compute various interesting properties */
@@ -557,15 +558,32 @@ KConstant::KConstant(llvm::Constant *_ct, unsigned _id, KInstruction *_ki)
   ki = _ki;
 }
 
-KGlobalVariable::KGlobalVariable(llvm::GlobalVariable *global)
-    : KValue(global, KValue::Kind::GLOBAL_VARIABLE) {}
+bool KConstant::operator<(const KValue &rhs) const {
+  return getKind() == rhs.getKind() ? id < cast<KConstant>(rhs).id
+                                    : getKind() < rhs.getKind();
+}
+
+unsigned KConstant::hash() const { return id; }
+
+KGlobalVariable::KGlobalVariable(llvm::GlobalVariable *global, unsigned id)
+    : KValue(global, KValue::Kind::GLOBAL_VARIABLE), id(id) {}
 
 std::string KGlobalVariable::getSourceFilepath() const {
-  getLocationInfo(globalVariable()).file;
+  return getLocationInfo(globalVariable()).file;
 }
 // Line number where the global variable is defined
 size_t KGlobalVariable::getLine() const {
-  getLocationInfo(globalVariable()).line;
+  return getLocationInfo(globalVariable()).line;
+}
+
+bool KGlobalVariable::operator<(const KValue &rhs) const {
+  return getKind() == rhs.getKind() ? id < cast<KGlobalVariable>(rhs).id
+                                    : getKind() < rhs.getKind();
+}
+unsigned KGlobalVariable::hash() const {
+  // It is good enough value to use it as hash as ID of globals
+  // different.
+  return id;
 }
 
 KFunction::KFunction(llvm::Function *_function, KModule *_km,
@@ -669,6 +687,23 @@ KBlock::KBlock(
   }
 }
 
+unsigned KBlock::inModuleID() const {
+  return parent->getGlobalIndex() + static_cast<unsigned>(getId());
+}
+
+bool KBlock::operator<(const KValue &rhs) const {
+  // Additional comparison on block types is redundant,
+  // as inModuleID defines the position of block.
+  return getKind() == rhs.getKind()
+             ? inModuleID() < cast<KBlock>(rhs).inModuleID()
+             : getKind() < rhs.getKind();
+}
+
+unsigned KBlock::hash() const {
+  // Use position of a block as a hash
+  return inModuleID();
+}
+
 KCallBlock::KCallBlock(
     KFunction *_kfunction, llvm::BasicBlock *block, KModule *km,
     const std::unordered_map<Instruction *, unsigned> &instructionToRegisterMap,
@@ -726,6 +761,19 @@ uintptr_t KBlock::getId() const { return instructions - parent->instructions; }
 KInstruction *KFunction::getInstructionByRegister(size_t reg) const {
   return instructions[reg - function()->arg_size()];
 }
+
+bool KFunction::operator<(const KValue &rhs) const {
+  return getKind() == rhs.getKind()
+             ? KFunctionCompare{}(this, cast<KFunction>(&rhs))
+             : getKind() < rhs.getKind();
+}
+
+unsigned KFunction::hash() const {
+  // It is good enough value to use it as
+  // index is unique.
+  return id;
+}
+
 size_t KFunction::getNumArgs() const { return function()->arg_size(); }
 size_t KFunction::getNumRegisters() const {
   return function()->arg_size() + numInstructions;

lib/Module/KValue.cpp

@@ -1,5 +1,5 @@
+#include "klee/Module/KValue.h"
 #include "klee/Support/CompilerWarning.h"
-#include <klee/Module/KValue.h>
 
 DISABLE_WARNING_PUSH
 DISABLE_WARNING_DEPRECATED_DECLARATIONS
@@ -17,14 +17,3 @@ llvm::Value *KValue::unwrap() const { return value; }
 llvm::StringRef KValue::getName() const {
   return value && value->hasName() ? value->getName() : "";
 }
-
-bool KValue::operator<(const KValue &rhs) const {
-  if (getName() != rhs.getName()) {
-    return getName() < rhs.getName();
-  }
-  return std::less<llvm::Value *>{}(unwrap(), rhs.unwrap());
-}
-
-[[nodiscard]] unsigned KValue::hash() const {
-  return std::hash<std::string>{}(getName().str());
-}