Skip to content

Commit

Permalink
Compiler: Rework options hashes
Browse files Browse the repository at this point in the history
- Switch from CRC32 to FNV1a32
- Add hash collision detection (if two options result in the same hash in the same AST, an error is triggered)
- Added OptionHash and HashOption to allow to switch in the future
  • Loading branch information
SirLynix committed Dec 17, 2023
1 parent 029d70a commit 10d3f5e
Show file tree
Hide file tree
Showing 5 changed files with 43 additions and 4 deletions.
11 changes: 10 additions & 1 deletion include/NZSL/Ast/SanitizeVisitor.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,15 @@

namespace nzsl::Ast
{
using OptionHash = std::uint32_t;

template<typename... Args> constexpr OptionHash HashOption(Args&&... args);

namespace Literals
{
constexpr OptionHash operator ""_opt(const char* str, std::size_t length);
}

class NZSL_API SanitizeVisitor final : Cloner
{
friend class AstTypeExpressionVisitor;
Expand All @@ -42,7 +51,7 @@ namespace nzsl::Ast
{
std::function<bool(std::string& identifier, IdentifierScope identifierScope)> identifierSanitizer; //< ignored when performing partial sanitization
std::shared_ptr<ModuleResolver> moduleResolver;
std::unordered_map<std::uint32_t, ConstantValue> optionValues;
std::unordered_map<OptionHash, ConstantValue> optionValues;
bool forceAutoBindingResolve = false;
bool makeVariableNameUnique = false;
bool partialSanitization = false;
Expand Down
15 changes: 15 additions & 0 deletions include/NZSL/Ast/SanitizeVisitor.inl
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,24 @@
// This file is part of the "Nazara Shading Language" project
// For conditions of distribution and use, see copyright notice in Config.hpp

#include <NazaraUtils/Hash.hpp>

namespace nzsl::Ast
{
template<typename... Args>
constexpr OptionHash HashOption(Args&&... args)
{
return Nz::FNV1a32(std::forward<Args>(args)...);
}

namespace Literals
{
constexpr OptionHash operator""_opt(const char* str, std::size_t length)
{
return HashOption(std::string_view(str, length));
}
}

inline ModulePtr SanitizeVisitor::Sanitize(const Module& module, std::string* error)
{
return Sanitize(module, {}, error);
Expand Down
1 change: 1 addition & 0 deletions include/NZSL/Lang/ErrorList.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ NZSL_SHADERLANG_COMPILER_ERROR(ModuleCompilationFailed, "module {} compilation f
NZSL_SHADERLANG_COMPILER_ERROR(ModuleFeatureMismatch, "module {} requires feature {}", std::string, Ast::ModuleFeature)
NZSL_SHADERLANG_COMPILER_ERROR(ModuleNotFound, "module {} not found", std::string)
NZSL_SHADERLANG_COMPILER_ERROR(NoModuleResolver, "import statement found but no module resolver has been set (and partial sanitization is not enabled)")
NZSL_SHADERLANG_COMPILER_ERROR(OptionHashCollision, "option {} has the same hash as option {}", std::string, std::string)
NZSL_SHADERLANG_COMPILER_ERROR(OptionDeclarationInsideFunction, "options must be declared outside of functions")
NZSL_SHADERLANG_COMPILER_ERROR(PartialTypeExpect, "expected a {} type at #{}", std::string, std::uint32_t)
NZSL_SHADERLANG_COMPILER_ERROR(PartialTypeTooFewParameters, "parameter count mismatch (expected at least {}, got {})", std::uint32_t, std::uint32_t)
Expand Down
12 changes: 11 additions & 1 deletion src/NZSL/Ast/SanitizeVisitor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,7 @@ namespace nzsl::Ast
std::unordered_map<std::string, std::size_t> moduleByName;
std::unordered_map<std::uint64_t, UsedExternalData> usedBindingIndexes;
std::unordered_map<std::string, UsedExternalData> declaredExternalVar;
std::unordered_map<OptionHash, std::string> declaredOptions;
std::shared_ptr<Environment> globalEnv;
std::shared_ptr<Environment> currentEnv;
std::shared_ptr<Environment> moduleEnv;
Expand Down Expand Up @@ -1750,7 +1751,16 @@ namespace nzsl::Ast

clone->optType = std::move(resolvedType);

std::uint32_t optionHash = Nz::CRC32(reinterpret_cast<const std::uint8_t*>(clone->optName.data()), clone->optName.size());
OptionHash optionHash = HashOption(clone->optName.data());

// Detect hash collisions
if (auto it = m_context->declaredOptions.find(optionHash); it != m_context->declaredOptions.end())
{
if (it->second != clone->optName)
throw CompilerOptionHashCollisionError{ node.sourceLocation, clone->optName, it->second };
}
else
m_context->declaredOptions.emplace(optionHash, clone->optName);

if (auto optionValueIt = m_context->options.optionValues.find(optionHash); optionValueIt != m_context->options.optionValues.end())
clone->optIndex = RegisterConstant(clone->optName, optionValueIt->second, node.optIndex, node.sourceLocation);
Expand Down
8 changes: 6 additions & 2 deletions tests/src/Tests/ConstTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,9 @@ fn main()

WHEN("Enabling option")
{
options.optionValues[Nz::CRC32("UseInt")] = true;
using namespace nzsl::Ast::Literals;

options.optionValues["UseInt"_opt] = true;

ExpectOutput(*shaderModule, options, R"(
struct inputStruct
Expand All @@ -123,7 +125,9 @@ fn main()

WHEN("Disabling option")
{
options.optionValues[Nz::CRC32("UseInt")] = false;
using namespace nzsl::Ast::Literals;

options.optionValues["UseInt"_opt] = false;

ExpectOutput(*shaderModule, options, R"(
struct inputStruct
Expand Down

0 comments on commit 10d3f5e

Please sign in to comment.