Skip to content

Commit b2f5467

Browse files
committed
Clean up Wave preprocessor handling
1 parent ac822ad commit b2f5467

3 files changed

Lines changed: 94 additions & 72 deletions

File tree

src/nbl/asset/utils/CHLSLCompiler.cpp

Lines changed: 80 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,6 @@
2020
#include <combaseapi.h>
2121
#include <sstream>
2222
#include <dxc/dxcapi.h>
23-
#include <boost/algorithm/string/predicate.hpp>
24-
#include <boost/algorithm/string/trim.hpp>
2523

2624
using namespace nbl;
2725
using namespace nbl::asset;
@@ -363,6 +361,85 @@ namespace nbl::wave
363361
extern nbl::core::string preprocess(std::string& code, const IShaderCompiler::SPreprocessorOptions& preprocessOptions, bool withCaching, std::function<void(nbl::wave::context&)> post);
364362
}
365363

364+
static bool isHorizontalWhitespace(const char c)
365+
{
366+
return c == ' ' || c == '\t' || c == '\r' || c == '\v' || c == '\f';
367+
}
368+
369+
static bool consumeIdentifier(std::string_view line, size_t& pos, const std::string_view identifier)
370+
{
371+
if (line.substr(pos, identifier.size()) != identifier)
372+
return false;
373+
374+
const size_t end = pos + identifier.size();
375+
if (end < line.size())
376+
{
377+
const char next = line[end];
378+
if ((next >= 'a' && next <= 'z') || (next >= 'A' && next <= 'Z') || (next >= '0' && next <= '9') || next == '_')
379+
return false;
380+
}
381+
382+
pos = end;
383+
return true;
384+
}
385+
386+
static void normalizeLegacyShaderStagePragmas(std::string& code)
387+
{
388+
std::string normalized;
389+
normalized.reserve(code.size());
390+
391+
size_t lineStart = 0u;
392+
while (lineStart < code.size())
393+
{
394+
const size_t lineEnd = code.find('\n', lineStart);
395+
const bool hasNewline = lineEnd != std::string::npos;
396+
const size_t lineSize = hasNewline ? (lineEnd - lineStart) : (code.size() - lineStart);
397+
const std::string_view line(code.data() + lineStart, lineSize);
398+
399+
size_t pos = 0u;
400+
while (pos < line.size() && isHorizontalWhitespace(line[pos]))
401+
++pos;
402+
403+
bool normalizedLegacyPragma = false;
404+
if (pos < line.size() && line[pos] == '#')
405+
{
406+
++pos;
407+
while (pos < line.size() && isHorizontalWhitespace(line[pos]))
408+
++pos;
409+
410+
if (consumeIdentifier(line, pos, "pragma"))
411+
{
412+
while (pos < line.size() && isHorizontalWhitespace(line[pos]))
413+
++pos;
414+
415+
const size_t pragmaArgumentPos = pos;
416+
if (consumeIdentifier(line, pos, "shader_stage"))
417+
normalizedLegacyPragma = true;
418+
else
419+
pos = pragmaArgumentPos;
420+
}
421+
}
422+
423+
if (normalizedLegacyPragma)
424+
{
425+
normalized.append(line.substr(0u, pos - std::string_view("shader_stage").size()));
426+
normalized += "wave ";
427+
normalized.append(line.substr(pos - std::string_view("shader_stage").size()));
428+
}
429+
else
430+
{
431+
normalized.append(line);
432+
}
433+
434+
if (hasNewline)
435+
normalized.push_back('\n');
436+
437+
lineStart = hasNewline ? (lineEnd + 1u) : code.size();
438+
}
439+
440+
code = std::move(normalized);
441+
}
442+
366443
static std::string preprocessShaderImpl(
367444
std::string&& code,
368445
IShader::E_SHADER_STAGE& stage,
@@ -386,18 +463,7 @@ static std::string preprocessShaderImpl(
386463
if (depfileEnabled && !dependenciesOut)
387464
dependenciesOut = &localDependencies;
388465

389-
// HACK: we do a pre-pre-process here to add \n after every #pragma to neutralize boost::wave's actions
390-
// See https://github.com/Devsh-Graphics-Programming/Nabla/issues/746
391-
size_t line_index = 0;
392-
for (size_t i = 0; i < code.size(); i++) {
393-
if (code[i] == '\n') {
394-
auto line = code.substr(line_index, i - line_index);
395-
boost::trim(line);
396-
if (boost::starts_with(line, "#pragma"))
397-
code.insert(i++, 1, '\n');
398-
line_index = i;
399-
}
400-
}
466+
normalizeLegacyShaderStagePragmas(code);
401467

402468
// preprocess
403469
core::string resolvedString = nbl::wave::preprocess(code, preprocessOptions, bool(dependenciesOut),

src/nbl/asset/utils/CWaveStringResolver.cpp

Lines changed: 1 addition & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -134,48 +134,6 @@ std::string tokenValueToString(const TokenT& token)
134134
return std::string(value.data(), value.size());
135135
}
136136

137-
bool isBinaryLiteralTail(const std::string_view value)
138-
{
139-
if (value.size() < 2u)
140-
return false;
141-
142-
if (value.front() != 'b' && value.front() != 'B')
143-
return false;
144-
145-
bool hasBinaryDigit = false;
146-
size_t i = 1u;
147-
for (; i < value.size(); ++i)
148-
{
149-
const char c = value[i];
150-
if (c == '0' || c == '1')
151-
{
152-
hasBinaryDigit = true;
153-
continue;
154-
}
155-
if (c == '\'')
156-
continue;
157-
break;
158-
}
159-
160-
if (!hasBinaryDigit)
161-
return false;
162-
163-
for (; i < value.size(); ++i)
164-
{
165-
const char c = value[i];
166-
const bool isIntegerSuffix = c == 'u' || c == 'U' || c == 'l' || c == 'L' || c == 'z' || c == 'Z';
167-
if (!isIntegerSuffix)
168-
return false;
169-
}
170-
171-
return true;
172-
}
173-
174-
bool shouldConcatenateWithoutWhitespace(const std::string_view previousValue, const std::string_view currentValue)
175-
{
176-
return previousValue == "0" && isBinaryLiteralTail(currentValue);
177-
}
178-
179137
core::string renderPreprocessedOutput(nbl::wave::context& context)
180138
{
181139
using namespace boost::wave;
@@ -184,7 +142,6 @@ core::string renderPreprocessedOutput(nbl::wave::context& context)
184142
util::insert_whitespace_detection whitespace(true);
185143
std::optional<nbl::wave::context::position_type> previousPosition = std::nullopt;
186144
bool previousWasExplicitWhitespace = false;
187-
std::string previousTokenValue;
188145

189146
for (auto it = context.begin(); it != context.end(); ++it)
190147
{
@@ -211,7 +168,7 @@ core::string renderPreprocessedOutput(nbl::wave::context& context)
211168
whitespace.shift_tokens(T_NEWLINE);
212169
}
213170
}
214-
else if (!previousWasExplicitWhitespace && !shouldConcatenateWithoutWhitespace(previousTokenValue, value) && whitespace.must_insert(id, value))
171+
else if (!previousWasExplicitWhitespace && whitespace.must_insert(id, value))
215172
{
216173
if (output.empty() || (output.back() != ' ' && output.back() != '\n' && output.back() != '\r' && output.back() != '\t'))
217174
{
@@ -225,8 +182,6 @@ core::string renderPreprocessedOutput(nbl::wave::context& context)
225182
whitespace.shift_tokens(id);
226183
previousPosition = position;
227184
previousWasExplicitWhitespace = explicitWhitespace;
228-
if (!explicitWhitespace)
229-
previousTokenValue = value;
230185
}
231186

232187
return output;

src/nbl/asset/utils/waveContext.h

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,6 @@ struct preprocessing_hooks final : public boost::wave::context_policies::default
7272
typename ContextT::token_type const& act_token
7373
)
7474
{
75-
hash_token_occurences++;
7675
auto optionStr = option.get_value().c_str();
7776
if (strcmp(optionStr,"shader_stage")==0)
7877
{
@@ -118,12 +117,13 @@ struct preprocessing_hooks final : public boost::wave::context_policies::default
118117
std::string compiler_option_s = std::string(valueIter->get_value().c_str());
119118
// the compiler_option_s is a token thus can be only part of the actual argument, i.e. "-spirv" will be split into tokens [ "-", "spirv" ]
120119
// for dxc_compile_flags just join the strings until it finds a whitespace or end of args
121-
122-
if (compiler_option_s == " ")
120+
if (IS_CATEGORY(*valueIter, WhiteSpaceTokenType))
123121
{
124-
// push argument and reset
125-
m_dxc_compile_flags_override.push_back(arg);
126-
arg.clear();
122+
if (!arg.empty())
123+
{
124+
m_dxc_compile_flags_override.push_back(arg);
125+
arg.clear();
126+
}
127127
}
128128
else
129129
{
@@ -196,12 +196,13 @@ class context : private boost::noncopyable
196196
, current_relative_filename(fname)
197197
, macros(*this_())
198198
, language(language_support(
199-
support_cpp20
200-
| support_option_preserve_comments
201-
| support_option_emit_line_directives
202-
| support_option_emit_pragma_directives
203-
// | support_option_emit_contnewlines
204-
// | support_option_insert_whitespace
199+
support_cpp20 // C++20 lexer mode. https://github.com/Devsh-Graphics-Programming/wave/blob/e02cda69e4d070fd9b16a39282d6b5c717cb3da4/include/boost/wave/language_support.hpp#L56-L59
200+
| support_option_prefer_pp_numbers // Prefer pp-number lexing before retokenization. https://github.com/Devsh-Graphics-Programming/wave/blob/e02cda69e4d070fd9b16a39282d6b5c717cb3da4/include/boost/wave/language_support.hpp#L71
201+
| support_option_preserve_comments // Keep comments in the token stream. https://github.com/Devsh-Graphics-Programming/wave/blob/e02cda69e4d070fd9b16a39282d6b5c717cb3da4/include/boost/wave/language_support.hpp#L67
202+
| support_option_emit_line_directives // Emit #line directives in the output. https://github.com/Devsh-Graphics-Programming/wave/blob/e02cda69e4d070fd9b16a39282d6b5c717cb3da4/include/boost/wave/language_support.hpp#L72
203+
| support_option_emit_pragma_directives // Keep pragma directives in the output. https://github.com/Devsh-Graphics-Programming/wave/blob/e02cda69e4d070fd9b16a39282d6b5c717cb3da4/include/boost/wave/language_support.hpp#L74
204+
// | support_option_emit_contnewlines // Emit escaped line continuations. https://github.com/Devsh-Graphics-Programming/wave/blob/e02cda69e4d070fd9b16a39282d6b5c717cb3da4/include/boost/wave/language_support.hpp#L65
205+
// | support_option_insert_whitespace // Let Wave inject separator whitespace. https://github.com/Devsh-Graphics-Programming/wave/blob/e02cda69e4d070fd9b16a39282d6b5c717cb3da4/include/boost/wave/language_support.hpp#L66
205206
))
206207
, hooks(hooks_)
207208
{

0 commit comments

Comments
 (0)