Skip to content

Commit

Permalink
Fix lifetime issue: short ternary must also take its else property if…
Browse files Browse the repository at this point in the history
… not bottom.
  • Loading branch information
FeepingCreature committed Oct 12, 2023
1 parent 0304ee2 commit e852f37
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 9 deletions.
2 changes: 1 addition & 1 deletion build.sh
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ FLAGS="$FLAGS -file-id-output build/fileIdPins"
# see generation.md
NEXT=compiler$(($($NEAT -print-generation) + 1))
$NEAT $FLAGS -backend=c -macro-backend=c -next-generation -P$NEXT:src -j src/main.nt \
-o build/neat_stage1
-version=firstpass -o build/neat_stage1
cat build/fileIdPins -<<EOF > build/neat.ini
-syspackage compiler:src
-running-compiler-version=$TAG
Expand Down
11 changes: 7 additions & 4 deletions src/neat/statements.nt
Original file line number Diff line number Diff line change
Expand Up @@ -394,10 +394,13 @@ Statement sequence(Statement left, nullable Statement right)
}

Statement sequence2(nullable Statement first, Statement second) {
// TODO this has a lifetime bug
// return first?.(sequence(that, second)) else second;
if (auto first = first) return sequence(first, second);
return second;
// TODO remove
version (firstpass) {
if (auto first = first) return first.(sequence(that, second));
return second;
} else {
return first?.(sequence(that, second)) else second;
}
}

class StatementExpression : Expression
Expand Down
3 changes: 2 additions & 1 deletion src/neat/ternary.nt
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,10 @@ class ASTTernaryIf : ASTSymbol
* else { result = else_; }
*/
auto lifetimeThen = then if elseIsBottom else then.take(context, __RANGE__)?;
auto lifetimeElse = else_ if elseIsBottom else else_.take(context, __RANGE__)?;
auto thenConvert = expectImplicitConvertTo(
context, lifetimeThen, mergedType, this.locRange)?;
auto elseConvert = expectImplicitConvertTo(context, else_, mergedType, this.locRange)?;
auto elseConvert = expectImplicitConvertTo(context, lifetimeElse, mergedType, this.locRange)?;
// If else is bottom, we can just use the lifetime of `then` directly.
auto lifetime = then.info.lifetime if elseIsBottom else Lifetime.gifted;
auto result = new PairedTemporary(mergedType, lifetime, context.getUniqueId);
Expand Down
16 changes: 13 additions & 3 deletions test/runnable/lifetime.nt
Original file line number Diff line number Diff line change
Expand Up @@ -601,9 +601,9 @@ void test_either_replace()
assert(trace == "-S0 -T0 -U0");
}

void test_else_prop()
void test_short_ternary()
{
dbg("- else property");
dbg("- short ternary");
{
string trace;
{
Expand All @@ -619,6 +619,16 @@ void test_else_prop()
test? else new C(S("S", &trace, 0));
assert(trace == "+S1 -S0 -S1");
}
{
string trace;
{
(int | :else) test = :else;
auto c = new C(S("S", &trace, 0));
test? else c;
assert(trace == "+S1 -S0");
}
assert(trace == "+S1 -S0 -S1");
}
{
string trace;
(int | :else) test = 0;
Expand Down Expand Up @@ -1189,7 +1199,7 @@ void main()
test_class_member_ref;
test_either_member;
test_either_replace;
test_else_prop;
test_short_ternary;
test_error_prop;
test_array_cat;
test_statement_expr;
Expand Down

0 comments on commit e852f37

Please sign in to comment.