Skip to content

Commit 5390bba

Browse files
xokdviumxokdvium
authored
Merge pull request #14314 from Radvendii/parser-cpp-variant
libexpr: parser.y: use api.value.type variant
2 parents 7e8db2e + 96c8cc5 commit 5390bba

File tree

7 files changed

+147
-128
lines changed

7 files changed

+147
-128
lines changed

‎src/libexpr/eval.cc‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2050,10 +2050,10 @@ void ExprConcatStrings::eval(EvalState & state, Env & env, Value & v)
20502050
};
20512051

20522052
// List of returned strings. References to these Values must NOT be persisted.
2053-
SmallTemporaryValueVector<conservativeStackReservation> values(es->size());
2053+
SmallTemporaryValueVector<conservativeStackReservation> values(es.size());
20542054
Value * vTmpP = values.data();
20552055

2056-
for (auto & [i_pos, i] : *es) {
2056+
for (auto & [i_pos, i] : es) {
20572057
Value & vTmp = *vTmpP++;
20582058
i->eval(state, env, vTmp);
20592059

@@ -2097,7 +2097,7 @@ void ExprConcatStrings::eval(EvalState & state, Env & env, Value & v)
20972097
.debugThrow();
20982098
} else {
20992099
if (s.empty())
2100-
s.reserve(es->size());
2100+
s.reserve(es.size());
21012101
/* skip canonization of first path, which would only be not
21022102
canonized in the first place if it's coming from a ./${foo} type
21032103
path */

‎src/libexpr/include/nix/expr/nixexpr.hh‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -695,11 +695,11 @@ struct ExprConcatStrings : Expr
695695
{
696696
PosIdx pos;
697697
bool forceString;
698-
std::vector<std::pair<PosIdx, Expr *>> * es;
699-
ExprConcatStrings(const PosIdx & pos, bool forceString, std::vector<std::pair<PosIdx, Expr *>> * es)
698+
std::vector<std::pair<PosIdx, Expr *>> es;
699+
ExprConcatStrings(const PosIdx & pos, bool forceString, std::vector<std::pair<PosIdx, Expr *>> && es)
700700
: pos(pos)
701701
, forceString(forceString)
702-
, es(es) {};
702+
, es(std::move(es)) {};
703703

704704
PosIdx getPos() const override
705705
{

‎src/libexpr/include/nix/expr/parser-state.hh‎

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -282,15 +282,15 @@ ParserState::stripIndentation(const PosIdx pos, std::vector<std::pair<PosIdx, st
282282
}
283283

284284
/* Strip spaces from each line. */
285-
auto * es2 = new std::vector<std::pair<PosIdx, Expr *>>;
285+
std::vector<std::pair<PosIdx, Expr *>> es2{};
286286
atStartOfLine = true;
287287
size_t curDropped = 0;
288288
size_t n = es.size();
289289
auto i = es.begin();
290290
const auto trimExpr = [&](Expr * e) {
291291
atStartOfLine = false;
292292
curDropped = 0;
293-
es2->emplace_back(i->first, e);
293+
es2.emplace_back(i->first, e);
294294
};
295295
const auto trimString = [&](const StringToken & t) {
296296
std::string s2;
@@ -324,7 +324,7 @@ ParserState::stripIndentation(const PosIdx pos, std::vector<std::pair<PosIdx, st
324324

325325
// Ignore empty strings for a minor optimisation and AST simplification
326326
if (s2 != "") {
327-
es2->emplace_back(i->first, new ExprString(alloc, s2));
327+
es2.emplace_back(i->first, new ExprString(alloc, s2));
328328
}
329329
};
330330
for (; i != es.end(); ++i, --n) {
@@ -333,19 +333,17 @@ ParserState::stripIndentation(const PosIdx pos, std::vector<std::pair<PosIdx, st
333333

334334
// If there is nothing at all, return the empty string directly.
335335
// This also ensures that equivalent empty strings result in the same ast, which is helpful when testing formatters.
336-
if (es2->size() == 0) {
336+
if (es2.size() == 0) {
337337
auto * const result = new ExprString("");
338-
delete es2;
339338
return result;
340339
}
341340

342341
/* If this is a single string, then don't do a concatenation. */
343-
if (es2->size() == 1 && dynamic_cast<ExprString *>((*es2)[0].second)) {
344-
auto * const result = (*es2)[0].second;
345-
delete es2;
342+
if (es2.size() == 1 && dynamic_cast<ExprString *>((es2)[0].second)) {
343+
auto * const result = (es2)[0].second;
346344
return result;
347345
}
348-
return new ExprConcatStrings(pos, true, es2);
346+
return new ExprConcatStrings(pos, true, std::move(es2));
349347
}
350348

351349
inline PosIdx LexerState::at(const ParserLocation & loc)

‎src/libexpr/lexer.l‎

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -142,11 +142,11 @@ or { return OR_KW; }
142142
return PIPE_INTO;
143143
}
144144

145-
{ID} { yylval->id = {yytext, (size_t) yyleng}; return ID; }
145+
{ID} { yylval->emplace<StringToken>(yytext, (size_t) yyleng); return ID; }
146146
{INT} { errno = 0;
147147
std::optional<int64_t> numMay = string2Int<int64_t>(yytext);
148148
if (numMay.has_value()) {
149-
yylval->n = NixInt{*numMay};
149+
yylval->emplace<NixInt>(*numMay);
150150
} else {
151151
throw ParseError(ErrorInfo{
152152
.msg = HintFmt("invalid integer '%1%'", yytext),
@@ -156,7 +156,7 @@ or { return OR_KW; }
156156
return INT_LIT;
157157
}
158158
{FLOAT} { errno = 0;
159-
yylval->nf = strtod(yytext, 0);
159+
yylval->emplace<NixFloat>(strtod(yytext, 0));
160160
if (errno != 0)
161161
throw ParseError(ErrorInfo{
162162
.msg = HintFmt("invalid float '%1%'", yytext),
@@ -183,7 +183,7 @@ or { return OR_KW; }
183183
/* It is impossible to match strings ending with '$' with one
184184
regex because trailing contexts are only valid at the end
185185
of a rule. (A sane but undocumented limitation.) */
186-
yylval->str = unescapeStr(yytext, yyleng, [&]() { return state->positions[CUR_POS]; });
186+
yylval->emplace<StringToken>(unescapeStr(yytext, yyleng, [&]() { return state->positions[CUR_POS]; }));
187187
return STR;
188188
}
189189
<STRING>\$\{ { PUSH_STATE(DEFAULT); return DOLLAR_CURLY; }
@@ -198,27 +198,27 @@ or { return OR_KW; }
198198

199199
\'\'(\ *\n)? { PUSH_STATE(IND_STRING); return IND_STRING_OPEN; }
200200
<IND_STRING>([^\$\']|\$[^\{\']|\'[^\'\$])+ {
201-
yylval->str = {yytext, (size_t) yyleng, true};
202-
forceNoNullByte(yylval->str, [&]() { return state->positions[CUR_POS]; });
201+
yylval->emplace<StringToken>(yytext, (size_t) yyleng, true);
202+
forceNoNullByte(yylval->as<StringToken>(), [&]() { return state->positions[CUR_POS]; });
203203
return IND_STR;
204204
}
205205
<IND_STRING>\'\'\$ |
206206
<IND_STRING>\$ {
207-
yylval->str = {"$", 1};
207+
yylval->emplace<StringToken>("$", 1);
208208
return IND_STR;
209209
}
210210
<IND_STRING>\'\'\' {
211-
yylval->str = {"''", 2};
211+
yylval->emplace<StringToken>("''", 2);
212212
return IND_STR;
213213
}
214214
<IND_STRING>\'\'\\{ANY} {
215-
yylval->str = unescapeStr(yytext + 2, yyleng - 2, [&]() { return state->positions[CUR_POS]; });
215+
yylval->emplace<StringToken>(unescapeStr(yytext + 2, yyleng - 2, [&]() { return state->positions[CUR_POS]; }));
216216
return IND_STR;
217217
}
218218
<IND_STRING>\$\{ { PUSH_STATE(DEFAULT); return DOLLAR_CURLY; }
219219
<IND_STRING>\'\' { POP_STATE(); return IND_STRING_CLOSE; }
220220
<IND_STRING>\' {
221-
yylval->str = {"'", 1};
221+
yylval->emplace<StringToken>("'", 1);
222222
return IND_STR;
223223
}
224224

@@ -232,14 +232,14 @@ or { return OR_KW; }
232232
<PATH_START>{PATH_SEG} {
233233
POP_STATE();
234234
PUSH_STATE(INPATH_SLASH);
235-
yylval->path = {yytext, (size_t) yyleng};
235+
yylval->emplace<StringToken>(yytext, (size_t) yyleng);
236236
return PATH;
237237
}
238238

239239
<PATH_START>{HPATH_START} {
240240
POP_STATE();
241241
PUSH_STATE(INPATH_SLASH);
242-
yylval->path = {yytext, (size_t) yyleng};
242+
yylval->emplace<StringToken>(yytext, (size_t) yyleng);
243243
return HPATH;
244244
}
245245

@@ -248,15 +248,15 @@ or { return OR_KW; }
248248
PUSH_STATE(INPATH_SLASH);
249249
else
250250
PUSH_STATE(INPATH);
251-
yylval->path = {yytext, (size_t) yyleng};
251+
yylval->emplace<StringToken>(yytext, (size_t) yyleng);
252252
return PATH;
253253
}
254254
{HPATH} {
255255
if (yytext[yyleng-1] == '/')
256256
PUSH_STATE(INPATH_SLASH);
257257
else
258258
PUSH_STATE(INPATH);
259-
yylval->path = {yytext, (size_t) yyleng};
259+
yylval->emplace<StringToken>(yytext, (size_t) yyleng);
260260
return HPATH;
261261
}
262262

@@ -272,7 +272,7 @@ or { return OR_KW; }
272272
PUSH_STATE(INPATH_SLASH);
273273
else
274274
PUSH_STATE(INPATH);
275-
yylval->str = {yytext, (size_t) yyleng};
275+
yylval->emplace<StringToken>(yytext, (size_t) yyleng);
276276
return STR;
277277
}
278278
<INPATH>{ANY} |
@@ -294,8 +294,8 @@ or { return OR_KW; }
294294
});
295295
}
296296

297-
{SPATH} { yylval->path = {yytext, (size_t) yyleng}; return SPATH; }
298-
{URI} { yylval->uri = {yytext, (size_t) yyleng}; return URI; }
297+
{SPATH} { yylval->emplace<StringToken>(yytext, (size_t) yyleng); return SPATH; }
298+
{URI} { yylval->emplace<StringToken>(yytext, (size_t) yyleng); return URI; }
299299

300300
%{
301301
// Doc comment rule

‎src/libexpr/meson.build‎

Lines changed: 47 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -183,17 +183,62 @@ subdir('primops')
183183
subdir('nix-meson-build-support/export-all-symbols')
184184
subdir('nix-meson-build-support/windows-version')
185185

186+
# Turns out that Bison/Flex are particularly sensitive to compilers
187+
# failing to inline functions. For that reason we crank up the inlining
188+
# threshold manually for optimized builds. Yes, this can be considered 'ricing'
189+
# the compiler, but it does pay off.
190+
#
191+
# NOTE: missed inlining can be spotted (for Clang) using -Rpass-missed=inline
192+
# and -fdump-ipa-inline-missed (for GCC).
193+
parser_library_cpp_args = []
194+
195+
if not get_option('debug')
196+
if cxx.get_id() == 'clang'
197+
# The default as of LLVM 21 is 225:
198+
# llc --help-hidden | grep inline-threshold
199+
parser_library_cpp_args += [
200+
'-mllvm',
201+
'-inline-threshold=5000',
202+
]
203+
elif cxx.get_id() == 'gcc'
204+
parser_library_cpp_args += [
205+
'--param=max-inline-insns-single=1000',
206+
'--param=max-inline-insns-auto=1000',
207+
'--param=inline-unit-growth=400',
208+
]
209+
endif
210+
endif
211+
212+
# Working around https://github.com/mesonbuild/meson/issues/1367.
213+
parser_library = static_library(
214+
'nixexpr-parser',
215+
parser_tab,
216+
lexer_tab,
217+
cpp_args : parser_library_cpp_args,
218+
dependencies : deps_public + deps_private + deps_other,
219+
include_directories : include_dirs,
220+
# 1. Stdlib and regular assertions regress parser performance significantly, so build without
221+
# them for this one library when building in a release configuration.
222+
# 2. Disable LTO for GCC because then inlining flags won't apply, since LTO in GCC is done
223+
# by plonking down GIMPLE in the archive.
224+
override_options : [
225+
'b_ndebug=@0@'.format(not get_option('debug')),
226+
'b_lto=@0@'.format(cxx.get_id() != 'gcc'),
227+
],
228+
)
229+
186230
this_library = library(
187231
'nixexpr',
188232
sources,
189233
config_priv_h,
190-
parser_tab,
191-
lexer_tab,
234+
parser_tab[1],
235+
lexer_tab[1],
192236
generated_headers,
193237
soversion : nix_soversion,
194238
dependencies : deps_public + deps_private + deps_other,
195239
include_directories : include_dirs,
196240
link_args : linker_export_flags,
241+
link_whole : [ parser_library ],
197242
prelink : true, # For C++ static initializers
198243
install : true,
199244
cpp_pch : do_pch ? [ 'pch/precompiled-headers.hh' ] : [],

‎src/libexpr/nixexpr.cc‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -246,7 +246,7 @@ void ExprConcatStrings::show(const SymbolTable & symbols, std::ostream & str) co
246246
{
247247
bool first = true;
248248
str << "(";
249-
for (auto & i : *es) {
249+
for (auto & i : es) {
250250
if (first)
251251
first = false;
252252
else
@@ -564,7 +564,7 @@ void ExprConcatStrings::bindVars(EvalState & es, const std::shared_ptr<const Sta
564564
if (es.debugRepl)
565565
es.exprEnvs.insert(std::make_pair(this, env));
566566

567-
for (auto & i : *this->es)
567+
for (auto & i : this->es)
568568
i.second->bindVars(es, env);
569569
}
570570

0 commit comments

Comments
 (0)