Skip to content

Enforce CompilerOptionName serialization exhaustiveness (classifyCommandLineOption) #12780

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
nv-slang-bot wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Enforce CompilerOptionName serialization exhaustiveness (classifyCommandLineOption) #12780

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
213 changes: 213 additions & 0 deletions source/slang/slang-command-line-option-class.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,213 @@
// slang-command-line-option-class.cpp
//
// Kept in its own translation unit, depending only on the `CompilerOptionName` enum, so the same
// source compiles into both the slang library and the unit-test module (the classification is not
// part of the exported ABI). See tools/CMakeLists.txt.

#include "slang-compiler-options.h"

namespace Slang
{
// Single source of truth for how `writeCommandLineArgs` treats each option. The `default` arm
// returns `Unclassified` so a newly-added enumerator is caught by the exhaustiveness unit test
// rather than silently omitted from the reproduction command line.
CommandLineOptionClass classifyCommandLineOption(CompilerOptionName name)
{
switch (name)
{
// Descriptive options worth recording for reproduction fidelity: they either influence the
// generated artifact (codegen, layout, capabilities, language rules, debug info, optimization,
// floating-point behavior) or document how it was produced (e.g. `SkipSPIRVValidation`,
// `DisableNonEssentialValidations`, `DumpIntermediates` do not change the emitted bytes but
// belong on a faithful reproduction command line), and all have a stable CLI spelling.
case CompilerOptionName::MacroDefine:
case CompilerOptionName::Include:
case CompilerOptionName::Language:
case CompilerOptionName::MatrixLayoutColumn:
case CompilerOptionName::MatrixLayoutRow:
case CompilerOptionName::ZeroInitialize:
case CompilerOptionName::IgnoreCapabilities:
case CompilerOptionName::RestrictiveCapabilityCheck:
case CompilerOptionName::Profile:
case CompilerOptionName::SkipSPIRVValidation:
case CompilerOptionName::DisableShortCircuit:
case CompilerOptionName::MinimumSlangOptimization:
case CompilerOptionName::DisableNonEssentialValidations:
case CompilerOptionName::DisableSourceMap:
case CompilerOptionName::UnscopedEnum:
case CompilerOptionName::PreserveParameters:
case CompilerOptionName::Capability:
case CompilerOptionName::DefaultImageFormatUnknown:
case CompilerOptionName::DisableDynamicDispatch:
case CompilerOptionName::DisableSpecialization:
case CompilerOptionName::FloatingPointMode:
case CompilerOptionName::DebugInformation:
case CompilerOptionName::DebugInformationFormat:
case CompilerOptionName::LineDirectiveMode:
case CompilerOptionName::Optimization:
case CompilerOptionName::Obfuscate:
case CompilerOptionName::VulkanBindShift:
case CompilerOptionName::VulkanBindShiftAll:
case CompilerOptionName::VulkanBindGlobals:
case CompilerOptionName::VulkanInvertY:
case CompilerOptionName::VulkanUseDxPositionW:
case CompilerOptionName::VulkanUseEntryPointName:
case CompilerOptionName::VulkanUseGLLayout:
case CompilerOptionName::VulkanEmitReflection:
case CompilerOptionName::GLSLForceScalarLayout:
case CompilerOptionName::EnableEffectAnnotations:
case CompilerOptionName::IncompleteLibrary:
case CompilerOptionName::DownstreamArgs:
case CompilerOptionName::BindlessSpaceIndex:
case CompilerOptionName::SPIRVResourceHeapStride:
case CompilerOptionName::SPIRVSamplerHeapStride:
case CompilerOptionName::LanguageVersion:
case CompilerOptionName::TypeConformance:
case CompilerOptionName::EnableExperimentalDynamicDispatch:
case CompilerOptionName::GenerateWholeProgram:
case CompilerOptionName::ForceDXLayout:
case CompilerOptionName::DenormalModeFp16:
case CompilerOptionName::DenormalModeFp32:
case CompilerOptionName::DenormalModeFp64:
case CompilerOptionName::UseMSVCStyleBitfieldPacking:
case CompilerOptionName::ForceCLayout:
case CompilerOptionName::ExperimentalFeature:
case CompilerOptionName::EmitSeparateDebug:
case CompilerOptionName::TraceCoverage:
case CompilerOptionName::TraceCoverageBinding:
case CompilerOptionName::TraceCoverageReservedSpace:
case CompilerOptionName::TraceFunctionCoverage:
case CompilerOptionName::TraceBranchCoverage:
case CompilerOptionName::TraceCoverageCounterByteWidth:
case CompilerOptionName::TraceCoverageBoolean:
// `TraceCoverageBindlessIndex` is a compile-time constant baked into the artifact (it selects
// the descriptor-array index the synthesized coverage buffer is indexed at), so it describes
// the compile and is serialized like the other coverage-binding options.
case CompilerOptionName::TraceCoverageBindlessIndex:
case CompilerOptionName::SPIRVUnifiedDescriptorHeapStride:
case CompilerOptionName::DebugInfoIncludeSource:
case CompilerOptionName::DumpIntermediates:
case CompilerOptionName::EmitSpirvMethod:
case CompilerOptionName::EmitCPUMethod:
case CompilerOptionName::EmbedDownstreamIR:
case CompilerOptionName::NoMangle:
case CompilerOptionName::NoHLSLBinding:
case CompilerOptionName::NoHLSLPackConstantBufferElements:
case CompilerOptionName::EnableExperimentalPasses:
case CompilerOptionName::TrackLiveness:
case CompilerOptionName::LoopInversion:
case CompilerOptionName::LLVMTargetTriple:
case CompilerOptionName::LLVMCPU:
case CompilerOptionName::LLVMFeatures:
case CompilerOptionName::AllowGLSL:
case CompilerOptionName::PassThrough:
return CommandLineOptionClass::Serialize;

// The source flags for the emit-method options. The command-line parser folds each into the
// corresponding serialized method key (`-emit-spirv-directly`/`-emit-spirv-via-glsl` ->
// `EmitSpirvMethod`, `-emit-cpu-via-cpp`/`-emit-cpu-via-llvm` -> `EmitCPUMethod`), so emitting
// the source flag as well would double-count the choice the method key already carries.
case CompilerOptionName::EmitSpirvViaGLSL:
case CompilerOptionName::EmitSpirvDirectly:
case CompilerOptionName::EmitCPUViaCPP:
case CompilerOptionName::EmitCPUViaLLVM:
return CommandLineOptionClass::RepresentedElsewhere;

// Options intentionally excluded from the reproduction command line. Most describe context or
// side channels rather than the artifact's contents: the entry-point context contributed by the
// caller (target/stage/entry are appended by the SPIR-V emit site itself), input/output paths
// and module identity, output-policy sidecar paths, repro tooling, dump/introspection,
// diagnostics routing and reporting, downstream toolchain selection/paths, and
// deprecated/removed/sentinel values. A few (e.g. the API-only `SkipDownstreamLinking`) can
// affect emission but have no command-line spelling to reconstruct, so they are omitted here
// rather than misrepresented.
case CompilerOptionName::DepFile:
case CompilerOptionName::EntryPointName:
case CompilerOptionName::Specialize:
case CompilerOptionName::Help:
case CompilerOptionName::HelpStyle:
case CompilerOptionName::ModuleName:
case CompilerOptionName::Output:
case CompilerOptionName::Stage:
case CompilerOptionName::Target:
case CompilerOptionName::Version:
case CompilerOptionName::WarningsAsErrors:
case CompilerOptionName::DisableWarnings:
case CompilerOptionName::EnableWarning:
case CompilerOptionName::DisableWarning:
case CompilerOptionName::WarningLevel:
case CompilerOptionName::DumpWarningDiagnostics:
case CompilerOptionName::InputFilesRemain:
case CompilerOptionName::EmitIr:
case CompilerOptionName::ReportDownstreamTime:
case CompilerOptionName::ReportPerfBenchmark:
case CompilerOptionName::ReportCheckpointIntermediates:
case CompilerOptionName::SourceEmbedStyle:
case CompilerOptionName::SourceEmbedName:
case CompilerOptionName::SourceEmbedLanguage:
case CompilerOptionName::SPIRVCoreGrammarJSON:
case CompilerOptionName::CompilerPath:
case CompilerOptionName::DefaultDownstreamCompiler:
case CompilerOptionName::DumpRepro:
case CompilerOptionName::DumpReproOnError:
case CompilerOptionName::ExtractRepro:
case CompilerOptionName::LoadRepro:
case CompilerOptionName::LoadReproDirectory:
case CompilerOptionName::ReproFallbackDirectory:
case CompilerOptionName::DumpAst:
case CompilerOptionName::DumpIntermediatePrefix:
case CompilerOptionName::DumpIr:
case CompilerOptionName::DumpIrIds:
case CompilerOptionName::PreprocessorOutput:
case CompilerOptionName::OutputIncludes:
case CompilerOptionName::ReproFileSystem:
case CompilerOptionName::REMOVED_SerialIR:
case CompilerOptionName::SkipCodeGen:
case CompilerOptionName::ValidateIr:
case CompilerOptionName::VerbosePaths:
case CompilerOptionName::VerifyDebugSerialIr:
case CompilerOptionName::NoCodeGen:
case CompilerOptionName::FileSystem:
case CompilerOptionName::Heterogeneous:
case CompilerOptionName::ValidateUniformity:
case CompilerOptionName::ArchiveType:
case CompilerOptionName::CompileCoreModule:
case CompilerOptionName::Doc:
case CompilerOptionName::IrCompression:
case CompilerOptionName::LoadCoreModule:
case CompilerOptionName::ReferenceModule:
case CompilerOptionName::SaveCoreModule:
case CompilerOptionName::SaveCoreModuleBinSource:
case CompilerOptionName::ParameterBlocksUseRegisterSpaces:
case CompilerOptionName::EmitReflectionJSON:
case CompilerOptionName::CountOfParsableOptions:
case CompilerOptionName::UseUpToDateBinaryModule:
case CompilerOptionName::SaveGLSLModuleBinSource:
case CompilerOptionName::SkipDownstreamLinking:
case CompilerOptionName::DumpModule:
case CompilerOptionName::GetModuleInfo:
case CompilerOptionName::GetSupportedModuleVersions:
case CompilerOptionName::ReportDetailedPerfBenchmark:
case CompilerOptionName::ValidateIRDetailed:
case CompilerOptionName::DumpIRBefore:
case CompilerOptionName::DumpIRAfter:
case CompilerOptionName::EnableRichDiagnostics:
case CompilerOptionName::ReportDynamicDispatchSites:
case CompilerOptionName::EnableMachineReadableDiagnostics:
case CompilerOptionName::DiagnosticColor:
case CompilerOptionName::CompilerVersion:
case CompilerOptionName::CoverageManifestOutput:
case CompilerOptionName::SeparateDebugInfoOutput:
// `CountOf` is the terminal sentinel, not a real option; `writeCommandLineArgs` never passes it
// and the exhaustiveness test iterates only `[0, CountOf)`. It is listed here purely so the
// switch is visibly total over the enum (and thus classifies as a benign `Omit` rather than
// reaching the `Unclassified` default) should a caller ever pass it.
case CompilerOptionName::CountOf:
return CommandLineOptionClass::Omit;

default:
return CommandLineOptionClass::Unclassified;
}
}

} // namespace Slang
17 changes: 15 additions & 2 deletions source/slang/slang-compiler-options.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,12 @@ void CompilerOptionSet::writeCommandLineArgs(Session* globalSession, StringBuild
{
for (auto& option : options)
{
// `classifyCommandLineOption` is the single source of truth for what this reconstruction
// emits; routing every option through it (rather than an open-coded set here) is what lets
// the exhaustiveness unit test force a deliberate classification for each new enumerator.
if (classifyCommandLineOption(option.key) != CommandLineOptionClass::Serialize)
continue;

// Most emitted options resolve their flag name from the command-option catalog; a few keys
// that are not registered as their own command option (e.g. DebugInformationFormat,
// EmitSpirvMethod) build the flag inline in their case below, so a missing registration is
Expand Down Expand Up @@ -365,8 +371,15 @@ void CompilerOptionSet::writeCommandLineArgs(Session* globalSession, StringBuild
sb << " " << name;
break;
default:
// Other option kinds are currently omitted.
break;
// Unreachable for the current enum: the gate above admits only `Serialize` options and
// every one has an emit case here (the classification and this switch are kept in
// lockstep). A `Serialize` option with no case is a bug — it would emit nothing — so
// fail loudly. Note this catches such a mistake only at run time and only where
// debug-only asserts fire: under `SLANG_ASSERT=release-assert-only` it is skipped, so a
// future unpaired `Serialize` option would silently drop in that mode. The
// exhaustiveness unit test guards classification completeness but not this pairing; a
// reviewer adding a `Serialize` option must add a matching emit arm to this switch.
SLANG_ASSERT_FAILURE("serialized CompilerOptionName has no writeCommandLineArgs case");
}
}
}
Expand Down
23 changes: 21 additions & 2 deletions source/slang/slang-compiler-options.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,24 @@ enum class OptimizationLevel : SlangOptimizationLevelIntegral;
enum class DebugInfoLevel : SlangDebugInfoLevelIntegral;
enum class CodeGenTarget : SlangCompileTargetIntegral;

/// How `CompilerOptionSet::writeCommandLineArgs` treats a given option when reconstructing the
/// descriptive command line embedded in debug info (e.g. the SPIR-V `DebugEntryPoint` `OpString`).
enum class CommandLineOptionClass
{
Serialize, ///< Emitted: a descriptive option worth recording for reproduction
///< fidelity (influences the artifact, or documents how it was produced).
RepresentedElsewhere, ///< Not emitted directly: derived from / represented through another
///< option.
Omit, ///< Not emitted: excluded from the reconstruction (context/I/O/tooling/
///< diagnostics, or an API-only knob with no CLI spelling to reproduce).
Unclassified, ///< No decision recorded — a bug caught by the exhaustiveness unit test.
};

/// Classify how the reproduction command line should treat `name`. This is the single source of
/// truth for that decision; `classifyCommandLineOption` must return a non-`Unclassified` value for
/// every `CompilerOptionName`, which the unit test enforces (see the enum doc above).
CommandLineOptionClass classifyCommandLineOption(CompilerOptionName name);

struct CompilerOptionValue
{
CompilerOptionValueKind kind = CompilerOptionValueKind::Int;
Expand Down Expand Up @@ -96,9 +114,10 @@ struct CompilerOptionSet
static bool allowDuplicate(CompilerOptionName name);

/// Append a CLI-like reconstruction of the stored options to `sb`, for the descriptive command
/// line embedded in debug info. Only the option kinds it explicitly handles are emitted; it
/// line embedded in debug info. It emits the options present in this set that are classified
/// `Serialize` by `classifyCommandLineOption`; every other kind is omitted by design. It
/// reports what is stored (which for some options is a default materialized during option
/// resolution) and does not add implicit defaults for absent options.
/// resolution), and does not itself add implicit defaults for absent options.
void writeCommandLineArgs(Session* globalSession, StringBuilder& sb);

OrderedDictionary<CompilerOptionName, List<CompilerOptionValue>> options;
Expand Down
4 changes: 4 additions & 0 deletions tools/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -416,10 +416,14 @@ if(SLANG_ENABLE_TESTS AND SLANG_ENABLE_SLANG_RHI)
# export annotation, so it is not visible from outside the DLL. The unit tests
# call it directly, so compile the .cpp again into this module without
# publishing an internal validator as part of the stable public ABI.
# slang-command-line-option-class.cpp is the same situation: classifyCommandLineOption()
# is an unexported free function the classification unit test calls directly, so the
# self-contained .cpp is recompiled here rather than exported from the DLL.
target_sources(
slang-unit-test
PRIVATE
${slang_SOURCE_DIR}/source/slang/slang-repro-validator.cpp
${slang_SOURCE_DIR}/source/slang/slang-command-line-option-class.cpp
${slang_SOURCE_DIR}/tools/slang-test/test-output-path-util.cpp
${slang_SOURCE_DIR}/tools/slang-test/test-reporter.cpp
)
Expand Down
36 changes: 36 additions & 0 deletions tools/slang-unit-test/unit-test-command-line-option-classification.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
// unit-test-command-line-option-classification.cpp
//
// Enforces that `classifyCommandLineOption` classifies every `CompilerOptionName`. This has to be a
// test rather than a compiler-checked exhaustive switch because the project builds with
// `-Wno-switch`, so a missing case produces no warning.

#include "slang/slang-compiler-options.h"
#include "unit-test/slang-unit-test.h"

using namespace Slang;

SLANG_UNIT_TEST(commandLineOptionClassificationIsExhaustive)
{
// `CompilerOptionName` is a densely, contiguously numbered enum (0 .. CountOf-1), so iterating
// the half-open integer range visits every enumerator exactly once; the assumption is checked
// implicitly, since a hole would classify as `Unclassified` and fail below.
const int countOf = (int)CompilerOptionName::CountOf;

// Guard against a vacuous pass: if the range were empty the loop below would assert nothing.
SLANG_CHECK(countOf > 0);

for (int i = 0; i < countOf; ++i)
{
const bool classified = classifyCommandLineOption((CompilerOptionName)i) !=
CommandLineOptionClass::Unclassified;
// Report a single result carrying the offending option index. `SLANG_CHECK_MSG` only
// accepts a string literal (it concatenates it with the stringized condition), so call the
// reporter directly to include the dynamic value.
StringBuilder message;
message << "CompilerOptionName value " << i
<< " is not classified in classifyCommandLineOption; add it to the appropriate "
"group.";
getTestReporter()
->addResultWithLocation(classified, message.getBuffer(), __FILE__, __LINE__);
}
}
Loading