Skip to content

Commit 4871e24

Browse files
AnastaZIukAnastaZIuk
committed
Clean up preprocess failure plumbing
1 parent c86f8e1 commit 4871e24

3 files changed

Lines changed: 97 additions & 73 deletions

File tree

‎include/nbl/asset/utils/IShaderCompiler.h‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616

1717
#include "nbl/builtin/hlsl/enums.hlsl"
1818

19+
#include <functional>
20+
1921
namespace nbl::asset
2022
{
2123

@@ -136,7 +138,7 @@ class NBL_API2 IShaderCompiler : public core::IReferenceCounted
136138
E_SPIRV_VERSION targetSpirvVersion = E_SPIRV_VERSION::ESV_1_6;
137139
bool depfile = false;
138140
system::path depfilePath = {};
139-
std::string* partialOutputOnFailure = nullptr;
141+
std::function<void(std::string_view)> onPartialOutputOnFailure = {};
140142
};
141143

142144
// https://github.com/microsoft/DirectXShaderCompiler/blob/main/docs/SPIR-V.rst#debugging

‎src/nbl/asset/utils/CWaveStringResolver.cpp‎

Lines changed: 17 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
#include "nabla.h"
4141
#include <boost/exception/diagnostic_information.hpp>
4242
#include <boost/wave/util/insert_whitespace_detection.hpp>
43+
#include <algorithm>
4344
#include <optional>
4445

4546
using namespace nbl;
@@ -49,9 +50,9 @@ using namespace nbl::asset;
4950

5051
namespace
5152
{
52-
constexpr size_t kWavePartialOutputTailMaxChars = 4096ull;
53-
constexpr size_t kWavePartialOutputTailMaxLines = 16ull;
54-
constexpr size_t kWaveTokenPreviewMaxChars = 160ull;
53+
constexpr size_t kWaveFailureLogOutputTailMaxChars = 4096ull;
54+
constexpr size_t kWaveFailureLogOutputTailMaxLines = 16ull;
55+
constexpr size_t kWaveFailureLogTokenPreviewMaxChars = 160ull;
5556

5657
struct WaveRenderProgress
5758
{
@@ -106,13 +107,7 @@ size_t countLogicalLines(const std::string_view text)
106107
if (text.empty())
107108
return 0ull;
108109

109-
size_t lines = 0ull;
110-
for (const auto ch : text)
111-
{
112-
if (ch == '\n')
113-
++lines;
114-
}
115-
110+
size_t lines = static_cast<size_t>(std::count(text.begin(), text.end(), '\n'));
116111
if (text.back() != '\n')
117112
++lines;
118113
return lines;
@@ -158,7 +153,7 @@ std::string indentMultiline(std::string_view text, std::string_view indent)
158153
return out;
159154
}
160155

161-
std::string makeOutputTail(std::string_view text)
156+
std::string makeFailureLogOutputTail(std::string_view text)
162157
{
163158
if (text.empty())
164159
return {};
@@ -173,14 +168,14 @@ std::string makeOutputTail(std::string_view text)
173168
if (text[start] == '\n')
174169
{
175170
++newlines;
176-
if (newlines > kWavePartialOutputTailMaxLines)
171+
if (newlines > kWaveFailureLogOutputTailMaxLines)
177172
{
178173
++start;
179174
break;
180175
}
181176
}
182177

183-
if (chars >= kWavePartialOutputTailMaxChars)
178+
if (chars >= kWaveFailureLogOutputTailMaxChars)
184179
break;
185180
}
186181

@@ -226,7 +221,7 @@ std::string makeWaveFailureContext(
226221
if (!renderProgress.lastTokenFile.empty())
227222
stream << "\n last_emitted_token_location: " << nbl::wave::detail::escape_control_chars(renderProgress.lastTokenFile) << ':' << renderProgress.lastTokenLine << ':' << renderProgress.lastTokenColumn;
228223
if (!renderProgress.lastTokenValue.empty())
229-
stream << "\n last_emitted_token_value: " << truncateEscapedPreview(nbl::wave::detail::escape_control_chars(renderProgress.lastTokenValue), kWaveTokenPreviewMaxChars);
224+
stream << "\n last_emitted_token_value: " << truncateEscapedPreview(nbl::wave::detail::escape_control_chars(renderProgress.lastTokenValue), kWaveFailureLogTokenPreviewMaxChars);
230225

231226
const auto snippet = getLineSnippet(code, lineNo);
232227
if (!snippet.empty() && fileName && preprocessOptions.sourceIdentifier == fileName)
@@ -237,7 +232,7 @@ std::string makeWaveFailureContext(
237232
stream << "\n " << caret;
238233
}
239234

240-
const auto outputTail = makeOutputTail(renderProgress.output);
235+
const auto outputTail = makeFailureLogOutputTail(renderProgress.output);
241236
if (!outputTail.empty())
242237
stream << "\n partial_output_tail:\n" << indentMultiline(outputTail, " ");
243238

@@ -327,14 +322,12 @@ std::string preprocessImpl(
327322
context.add_macro_definition("__SPIRV_MINOR_VERSION__=" + std::to_string(IShaderCompiler::getSpirvMinor(preprocessOptions.targetSpirvVersion)));
328323

329324
WaveRenderProgress renderProgress;
330-
if (preprocessOptions.partialOutputOnFailure)
331-
preprocessOptions.partialOutputOnFailure->clear();
332325
const char* phase = "registering built-in macros";
333326
std::string activeMacroDefinition;
334-
const auto storePartialOutputOnFailure = [&]()
327+
const auto reportPartialOutputOnFailure = [&]()
335328
{
336-
if (preprocessOptions.partialOutputOnFailure)
337-
*preprocessOptions.partialOutputOnFailure = renderProgress.output;
329+
if (preprocessOptions.onPartialOutputOnFailure)
330+
preprocessOptions.onPartialOutputOnFailure(renderProgress.output);
338331
};
339332
const auto makeFailureContext = [&](const char* const fileName, const int lineNo, const int columnNo)
340333
{
@@ -357,7 +350,7 @@ std::string preprocessImpl(
357350
}
358351
catch (boost::wave::preprocess_exception& e)
359352
{
360-
storePartialOutputOnFailure();
353+
reportPartialOutputOnFailure();
361354
const auto escapedDescription = nbl::wave::detail::escape_control_chars(e.description());
362355
const auto escapedFileName = nbl::wave::detail::escape_control_chars(e.file_name());
363356
const auto failureContext = makeFailureContext(e.file_name(), e.line_no(), e.column_no());
@@ -367,22 +360,22 @@ std::string preprocessImpl(
367360
}
368361
catch (const boost::exception& e)
369362
{
370-
storePartialOutputOnFailure();
363+
reportPartialOutputOnFailure();
371364
const auto failureContext = makeFailureContext(preprocessOptions.sourceIdentifier.data(), 0, 0);
372365
preprocessOptions.logger.log("Boost exception caught during Wave preprocessing.\n%s", system::ILogger::ELL_ERROR, failureContext.c_str());
373366
preprocessOptions.logger.log("Boost diagnostic information:\n%s", system::ILogger::ELL_ERROR, boost::diagnostic_information(e).c_str());
374367
return {};
375368
}
376369
catch (const std::exception& e)
377370
{
378-
storePartialOutputOnFailure();
371+
reportPartialOutputOnFailure();
379372
const auto failureContext = makeFailureContext(preprocessOptions.sourceIdentifier.data(), 0, 0);
380373
preprocessOptions.logger.log("std::exception caught during Wave preprocessing. %s\n%s", system::ILogger::ELL_ERROR, e.what(), failureContext.c_str());
381374
return {};
382375
}
383376
catch (...)
384377
{
385-
storePartialOutputOnFailure();
378+
reportPartialOutputOnFailure();
386379
const auto failureContext = makeFailureContext(preprocessOptions.sourceIdentifier.data(), 0, 0);
387380
preprocessOptions.logger.log("Unknown exception caught during Wave preprocessing.\n%s", system::ILogger::ELL_ERROR, failureContext.c_str());
388381
preprocessOptions.logger.log("Current exception diagnostic information:\n%s", system::ILogger::ELL_ERROR, boost::current_exception_diagnostic_information().c_str());

‎tools/nsc/main.cpp‎

Lines changed: 77 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,6 @@
88
#include <stdexcept>
99
#include <thread>
1010
#include <chrono>
11-
#include <filesystem>
12-
#include <fstream>
1311
#include <cstring>
1412
#include <cstdarg>
1513
#include <argparse/argparse.hpp>
@@ -81,9 +79,9 @@ class ShaderLogger final : public IThreadsafeLogger
8179
if (!m_system || m_logPath.empty())
8280
return;
8381

84-
const auto parent = std::filesystem::path(m_logPath).parent_path();
85-
if (!parent.empty() && !std::filesystem::exists(parent))
86-
std::filesystem::create_directories(parent);
82+
const auto parent = m_logPath.parent_path();
83+
if (!parent.empty() && !m_system->isDirectory(parent))
84+
m_system->createDirectory(parent);
8785

8886
for (auto attempt = 0u; attempt < kDeleteRetries; ++attempt)
8987
{
@@ -152,6 +150,61 @@ class ShaderLogger final : public IThreadsafeLogger
152150
bool m_noLog = false;
153151
};
154152

153+
static bool writeFileWithSystem(ISystem* system, const path& outputPath, const std::string_view content)
154+
{
155+
if (!system || outputPath.empty())
156+
return false;
157+
158+
const auto parent = outputPath.parent_path();
159+
if (!parent.empty() && !system->isDirectory(parent))
160+
{
161+
if (!system->createDirectory(parent))
162+
return false;
163+
}
164+
165+
auto tempPath = outputPath;
166+
tempPath += ".tmp";
167+
system->deleteFile(tempPath);
168+
169+
smart_refctd_ptr<IFile> outputFile;
170+
{
171+
ISystem::future_t<smart_refctd_ptr<IFile>> future;
172+
system->createFile(future, tempPath, IFileBase::ECF_WRITE);
173+
if (!future.wait())
174+
return false;
175+
176+
auto lock = future.acquire();
177+
if (!lock)
178+
return false;
179+
lock.move_into(outputFile);
180+
}
181+
if (!outputFile)
182+
return false;
183+
184+
if (!content.empty())
185+
{
186+
IFile::success_t success;
187+
outputFile->write(success, content.data(), 0ull, content.size());
188+
if (!success)
189+
{
190+
outputFile = nullptr;
191+
system->deleteFile(tempPath);
192+
return false;
193+
}
194+
}
195+
outputFile = nullptr;
196+
197+
system->deleteFile(outputPath);
198+
const auto moveError = system->moveFileOrDirectory(tempPath, outputPath);
199+
if (moveError)
200+
{
201+
system->deleteFile(tempPath);
202+
return false;
203+
}
204+
205+
return true;
206+
}
207+
155208
class ShaderCompiler final : public IApplicationFramework
156209
{
157210
using base_t = IApplicationFramework;
@@ -192,10 +245,13 @@ class ShaderCompiler final : public IApplicationFramework
192245
return false;
193246
}
194247

248+
m_system = system ? std::move(system) : IApplicationFramework::createSystem();
249+
if (!m_system)
250+
return false;
251+
195252
if (program.get<bool>("--dump-build-info"))
196253
{
197-
dumpBuildInfo(program);
198-
std::exit(0);
254+
return dumpBuildInfo(program);
199255
}
200256

201257
if (!isAPILoaded())
@@ -204,10 +260,6 @@ class ShaderCompiler final : public IApplicationFramework
204260
return false;
205261
}
206262

207-
m_system = system ? std::move(system) : IApplicationFramework::createSystem();
208-
if (!m_system)
209-
return false;
210-
211263
if (program.get<bool>("--self-test-unmount-builtins"))
212264
{
213265
const auto mountedBuiltinArchiveCount = m_system->getMountedBuiltinArchiveCount();
@@ -284,7 +336,7 @@ class ShaderCompiler final : public IApplicationFramework
284336
return false;
285337
}
286338

287-
const auto logPath = logPathOverride.empty() ? std::filesystem::path(outputFilepath).concat(".log") : std::filesystem::path(logPathOverride);
339+
const auto logPath = logPathOverride.empty() ? path(outputFilepath).concat(".log") : path(logPathOverride);
288340
const auto fileMask = bitflag(ILogger::ELL_ALL);
289341
const auto consoleMask = bitflag(ILogger::ELL_WARNING) | ILogger::ELL_ERROR;
290342

@@ -427,7 +479,7 @@ class ShaderCompiler final : public IApplicationFramework
427479
return out;
428480
}
429481

430-
static void dumpBuildInfo(const argparse::ArgumentParser& program)
482+
bool dumpBuildInfo(const argparse::ArgumentParser& program)
431483
{
432484
::json j;
433485
auto& modules = j["modules"];
@@ -456,57 +508,29 @@ class ShaderCompiler final : public IApplicationFramework
456508
const auto pretty = j.dump(4);
457509
std::cout << pretty << std::endl;
458510

459-
std::filesystem::path oPath = "build-info.json";
511+
path oPath = "build-info.json";
460512
if (program.is_used("--file"))
461513
{
462514
const auto filePath = program.get<std::string>("--file");
463515
if (!filePath.empty())
464516
oPath = filePath;
465517
}
466518

467-
std::ofstream outFile(oPath);
468-
if (!outFile.is_open())
519+
if (!writeFileWithSystem(m_system.get(), oPath, pretty))
469520
{
470-
std::printf("Failed to open \"%s\" for writing\n", oPath.string().c_str());
471-
std::exit(-1);
521+
std::printf("Failed to write \"%s\"\n", oPath.string().c_str());
522+
return false;
472523
}
473524

474-
outFile << pretty;
475525
std::printf("Saved \"%s\"\n", oPath.string().c_str());
526+
return true;
476527
}
477528

478529
bool writeOutputFile(const std::string& outputFilepath, const std::string_view content, const char* const description)
479530
{
480-
const auto outParent = std::filesystem::path(outputFilepath).parent_path();
481-
if (!outParent.empty() && !std::filesystem::exists(outParent))
482-
{
483-
if (!std::filesystem::create_directories(outParent))
484-
{
485-
m_logger->log("Failed to create parent directory for %s %s.", ILogger::ELL_ERROR, description, outputFilepath.c_str());
486-
return false;
487-
}
488-
}
489-
490-
std::fstream out(outputFilepath, std::ios::out | std::ios::binary | std::ios::trunc);
491-
if (!out.is_open())
492-
{
493-
m_logger->log("Failed to open %s file: %s", ILogger::ELL_ERROR, description, outputFilepath.c_str());
494-
return false;
495-
}
496-
497-
if (!content.empty())
498-
out.write(content.data(), content.size());
499-
if (out.fail())
531+
if (!writeFileWithSystem(m_system.get(), outputFilepath, content))
500532
{
501533
m_logger->log("Failed to write %s file: %s", ILogger::ELL_ERROR, description, outputFilepath.c_str());
502-
out.close();
503-
return false;
504-
}
505-
506-
out.close();
507-
if (out.fail())
508-
{
509-
m_logger->log("Failed to close %s file: %s", ILogger::ELL_ERROR, description, outputFilepath.c_str());
510534
return false;
511535
}
512536

@@ -578,7 +602,12 @@ class ShaderCompiler final : public IApplicationFramework
578602
std::string_view code(codePtr, std::strlen(codePtr));
579603
std::string partialOutputOnFailure;
580604
if (dumpPreprocessedOnFailure)
581-
opt.partialOutputOnFailure = &partialOutputOnFailure;
605+
{
606+
opt.onPartialOutputOnFailure = [&](const std::string_view partialOutput)
607+
{
608+
partialOutputOnFailure.assign(partialOutput);
609+
};
610+
}
582611

583612
r.text = hlslcompiler->preprocessShader(std::string(code), shaderStage, opt, nullptr);
584613
r.ok = !r.text.empty();

0 commit comments

Comments
 (0)