Skip to content

Commit 94f1984

Browse files
chqrliebvdberg
chqrlie
authored and
bvdberg
committed
Analyser: improve function flow analysis
* add flow analysis in module_analyser: statement analysers return `FlowBits` * remove ambiguous `hasReturn()`, use `FlowBits` to detect missing return * fix erroneous cases of 'missing return statement' * track more unreachable code cases: `if`, `while`, `for`, `switch`... * improve warning control: "no-" is an optional prefix to disable the warning * add $warning for unreachable-code: - disabled by default until bootstrap is regenerated - enabled by tester for tests * unknown warning is now a warning, not an error * fix tests for unreachable code
1 parent 825f018 commit 94f1984

20 files changed

+289
-150
lines changed

‎analyser/module_analyser_expr.c2‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -268,6 +268,7 @@ fn QualType Analyser.analyseConditionalOperator(Analyser* ma, Expr** e_ptr) {
268268
assert(lcanon.isValid());
269269
assert(rcanon.isValid());
270270

271+
// TODO: refine this by selecting branch depending on cond value
271272
if (cond.getCond().isCtv()) e.combineConstantFlags(cond.getLHS(), cond.getRHS());
272273

273274
u8 res = CondOpTable[lcanon.getKind()][rcanon.getKind()];

‎analyser/module_analyser_function.c2‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,7 @@ fn void Analyser.analyseFunction(Analyser* ma, FunctionDecl* fd) {
204204
ma.error(fd.asDecl().getLoc(), "pure functions cannot be variadic");
205205
return;
206206
}
207-
if (!fd.hasReturn()) {
207+
if (rtype.isVoid()) {
208208
ma.error(rtype.getLoc(), "pure functions must return a value");
209209
return;
210210
}
@@ -213,7 +213,6 @@ fn void Analyser.analyseFunction(Analyser* ma, FunctionDecl* fd) {
213213
ma.error(v.asDecl().getLoc(), "pure functions cannot have auto-arguments");
214214
return;
215215
}
216-
// must have body (even in libs)
217216
}
218217
}
219218

@@ -244,12 +243,12 @@ fn void Analyser.analyseFunctionBody(Analyser* ma, FunctionDecl* fd, scope.Scope
244243
ma.has_error = false;
245244
ma.labels.reset();
246245

247-
ma.analyseCompoundStmt(body);
246+
FlowBits flow = ma.analyseCompoundStmt(body);
248247

249248
// check for return stmt if function returns a value
250249
QualType rtype = fd.getRType();
251250
if (!rtype.isVoid()) {
252-
if (!hasReturn((Stmt*)body)) {
251+
if (flow & FlowNext) {
253252
// error loc is closing brace of the function body
254253
ma.error(body.getEndLoc() - 1, "control reaches end of non-void function");
255254
}

‎analyser/module_analyser_stmt.c2‎

Lines changed: 101 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -16,108 +16,104 @@
1616
module module_analyser;
1717

1818
import ast local;
19+
import ctv_analyser;
1920
import label_vector local;
2021
import scope;
2122

22-
fn bool hasReturn(const Stmt* s) {
23+
type Flow enum u8 {
24+
Next,
25+
Return,
26+
Break,
27+
Continue,
28+
Goto,
29+
NoReturn,
30+
Error,
31+
}
2332

24-
switch (s.getKind()) {
25-
case Return:
26-
return true;
27-
case If:
28-
const IfStmt* i = (IfStmt*)s;
29-
if (!hasReturn(i.getThen())) return false;
30-
const Stmt* e = i.getElse();
31-
if (e && hasReturn(e)) return true;
32-
break;
33-
case While:
34-
// TODO only allow while(true/1), no breaks and only return
35-
break;
36-
case For:
37-
// TODO
38-
break;
39-
case Switch:
40-
// TODO
41-
break;
42-
case Label:
43-
// TODO
44-
const LabelStmt* ls = (LabelStmt*)s;
45-
const Stmt* lss = ls.getStmt();
46-
if (!lss) return false;
47-
return hasReturn(lss);
48-
case Compound:
49-
const CompoundStmt* cs = (CompoundStmt*)s;
50-
const Stmt* last = cs.getLastStmt();
51-
if (!last) return false;
52-
return hasReturn(last);
53-
default:
54-
break;
55-
}
33+
type FlowBits u32;
34+
35+
const FlowBits FlowNext = 1 << Flow.Next;
36+
const FlowBits FlowReturn = 1 << Flow.Return;
37+
const FlowBits FlowBreak = 1 << Flow.Break;
38+
const FlowBits FlowContinue = 1 << Flow.Continue;
39+
const FlowBits FlowGoto = 1 << Flow.Goto;
40+
const FlowBits FlowNoReturn = 1 << Flow.NoReturn;
41+
const FlowBits FlowError = 1 << Flow.Error;
5642

57-
return false;
43+
fn Expr* getCondExpr(const Stmt* cond) {
44+
if (cond) {
45+
if (cond.isExpr()) return cast<Expr*>(cond);
46+
if (cond.isDecl()) return cast<DeclStmt*>(cond).getDecl(0).getInit();
47+
}
48+
return nil;
5849
}
5950

60-
fn void Analyser.analyseStmt(Analyser* ma, Stmt* s, bool checkEffect) {
61-
if (ma.scope.isUnreachable() && s.getKind() != StmtKind.Label) {
62-
ma.warn(s.getLoc(), "unreachable code");
51+
fn FlowBits Analyser.analyseStmt(Analyser* ma, Stmt* s, bool checkEffect) {
52+
FlowBits flow = 0;
53+
if (ma.scope.isUnreachable()) {
54+
if (s.getKind() != StmtKind.Label && !ma.warnings.no_unreachable_code) {
55+
ma.warn(s.getLoc(), "unreachable code");
56+
}
57+
ma.scope.setReachable(); // prevent multiple warnings
6358
}
6459
switch (s.getKind()) {
6560
case Return:
6661
ma.analyseReturnStmt(s);
67-
ma.scope.setUnreachable(); // Note: can have label after that is reachable!
68-
break;
62+
ma.scope.setUnreachable();
63+
return FlowReturn;
6964
case Expr:
7065
// TODO need a different set, since this one uses the Stack of Globals
7166
ma.analyseExpr((Expr**)&s, false, 0);
7267
Expr* e = (Expr*)s;
7368
if (checkEffect && !e.hasEffect()) ma.errorRange(e.getLoc(), e.getRange(), "expression without effect");
74-
break;
69+
return FlowNext; // TODO: check for call to noreturn function
7570
case If:
76-
ma.analyseIfStmt(s);
71+
flow = ma.analyseIfStmt(s);
7772
break;
7873
case While:
79-
ma.analyseWhileStmt(s);
74+
flow = ma.analyseWhileStmt(s);
8075
break;
8176
case For:
82-
ma.analyseForStmt(s);
77+
flow = ma.analyseForStmt(s);
8378
break;
8479
case Switch:
85-
ma.analyseSwitchStmt(s);
80+
flow = ma.analyseSwitchStmt(s);
8681
break;
8782
case Break:
8883
ma.analyseBreakStmt(s);
8984
ma.scope.setUnreachable();
90-
break;
85+
return FlowBreak;
9186
case Continue:
9287
ma.analyseContinueStmt(s);
9388
ma.scope.setUnreachable();
94-
break;
89+
return FlowContinue;
9590
case Fallthrough:
9691
ma.analyseFallthroughStmt(s);
97-
break;
92+
return FlowNext;
9893
case Label:
99-
ma.scope.setReachable();
100-
ma.analyseLabelStmt(s);
94+
flow = ma.analyseLabelStmt(s);
10195
break;
10296
case Goto:
10397
ma.analyseGotoStmt(s);
10498
ma.scope.setUnreachable();
105-
break;
99+
return FlowGoto;
106100
case Compound:
107101
ma.scope.enter(scope.Decl);
108-
ma.analyseCompoundStmt((CompoundStmt*)s);
102+
flow = ma.analyseCompoundStmt((CompoundStmt*)s);
109103
ma.scope.exit(ma.has_error);
110104
break;
111105
case Decl:
112106
ma.analyseDeclStmt(s);
113-
break;
107+
return FlowNext;
114108
case Asm:
115109
ma.analyseAsmStmt(s);
116-
break;
110+
return FlowNext;
117111
case Assert:
118112
ma.analyseAssertStmt(s);
119-
break;
113+
return FlowNext;
120114
}
115+
if (!(flow & FlowNext)) ma.scope.setUnreachable();
116+
return flow;
121117
}
122118

123119
fn void Analyser.analyseBreakStmt(Analyser* ma, Stmt* s) {
@@ -137,7 +133,7 @@ fn void Analyser.analyseFallthroughStmt(Analyser* ma, Stmt* s) {
137133
ma.error(s.getLoc(), "'fallthrough' statement cannot be used here");
138134
}
139135

140-
fn void Analyser.analyseLabelStmt(Analyser* ma, Stmt* s) {
136+
fn FlowBits Analyser.analyseLabelStmt(Analyser* ma, Stmt* s) {
141137
LabelStmt* ls = (LabelStmt*)s;
142138
u32 name = ls.getNameIdx();
143139

@@ -157,7 +153,8 @@ fn void Analyser.analyseLabelStmt(Analyser* ma, Stmt* s) {
157153
}
158154

159155
Stmt* lss = ls.getStmt();
160-
if (lss) ma.analyseStmt(lss, true);
156+
if (!lss) return FlowNext;
157+
return ma.analyseStmt(lss, true);
161158
}
162159

163160
fn void Analyser.analyseGotoStmt(Analyser* ma, Stmt* s) {
@@ -174,14 +171,18 @@ fn void Analyser.analyseGotoStmt(Analyser* ma, Stmt* s) {
174171
}
175172
}
176173

177-
fn void Analyser.analyseCompoundStmt(Analyser* ma, CompoundStmt* c) {
174+
fn FlowBits Analyser.analyseCompoundStmt(Analyser* ma, CompoundStmt* c) {
175+
FlowBits flow = 0;
176+
FlowBits flow2 = FlowNext;
178177
u32 count = c.getCount();
179178
Stmt** stmts = c.getStmts();
180179
for (u32 i=0; i<count; i++) {
181180
Stmt* s = stmts[i];
182-
ma.analyseStmt(s, true);
183-
if (ma.has_error) break;
181+
flow2 = ma.analyseStmt(s, true);
182+
flow |= flow2 & (FlowReturn | FlowBreak | FlowContinue | FlowGoto | FlowNoReturn | FlowError);
183+
if (ma.has_error) return flow;
184184
}
185+
return flow | flow2; // return FlowNext if last statement has FlowNext or empty block
185186
}
186187

187188
fn QualType Analyser.analyseCondition(Analyser* ma, Stmt** s_ptr, bool check_assign) {
@@ -212,28 +213,34 @@ fn QualType Analyser.analyseCondition(Analyser* ma, Stmt** s_ptr, bool check_ass
212213
return qt;
213214
}
214215

215-
fn void Analyser.analyseIfStmt(Analyser* ma, Stmt* s) {
216+
fn FlowBits Analyser.analyseIfStmt(Analyser* ma, Stmt* s) {
217+
FlowBits flow = FlowNext;
216218
IfStmt* i = (IfStmt*)s;
217219
ma.scope.enter(scope.Decl);
218220

219221
ma.analyseCondition(i.getCond2(), true);
220222
if (ma.has_error) goto done;
223+
// TODO: handle constant conditions
221224

222225
ma.scope.enter(scope.Decl);
223-
ma.analyseStmt(i.getThen(), true);
226+
flow = ma.analyseStmt(i.getThen(), true);
224227
ma.scope.exit(ma.has_error);
225228

229+
FlowBits flow2 = FlowNext;
226230
Stmt* else_ = i.getElse();
227231
if (else_) {
228232
ma.scope.enter(scope.Decl);
229-
ma.analyseStmt(else_, true);
233+
flow2 = ma.analyseStmt(else_, true);
230234
ma.scope.exit(ma.has_error);
231235
}
236+
flow |= flow2;
232237
done:
233238
ma.scope.exit(ma.has_error);
239+
return flow;
234240
}
235241

236-
fn void Analyser.analyseForStmt(Analyser* ma, Stmt* s) {
242+
fn FlowBits Analyser.analyseForStmt(Analyser* ma, Stmt* s) {
243+
FlowBits flow = FlowNext;
237244
ForStmt* f = (ForStmt*)s;
238245

239246
ma.scope.enter(scope.Break | scope.Continue | scope.Decl | scope.Control);
@@ -248,6 +255,16 @@ fn void Analyser.analyseForStmt(Analyser* ma, Stmt* s) {
248255
QualType qt = ma.analyseExpr(cond, true, RHS);
249256
if (qt.isInvalid()) goto done;
250257
ma.checker.check(builtins[BuiltinKind.Bool], qt, cond, (*cond).getLoc());
258+
if ((*cond).isCtv()) {
259+
Value v = ctv_analyser.get_value((*cond));
260+
if (v.isZero()) {
261+
// TODO: body is never reached
262+
} else {
263+
flow = 0;
264+
}
265+
}
266+
} else {
267+
flow = 0;
251268
}
252269

253270
Expr** cont = f.getCont2();
@@ -256,22 +273,40 @@ fn void Analyser.analyseForStmt(Analyser* ma, Stmt* s) {
256273
if (qt.isInvalid()) goto done;
257274
}
258275

259-
ma.analyseStmt(f.getBody(), true);
276+
FlowBits flow2 = ma.analyseStmt(f.getBody(), true);
277+
flow |= flow2 & (FlowReturn | FlowGoto | FlowNoReturn | FlowError);
278+
if (flow2 & FlowBreak) flow |= FlowNext;
260279
done:
261280
ma.scope.exit(ma.has_error);
281+
return flow;
262282
}
263283

264-
fn void Analyser.analyseWhileStmt(Analyser* ma, Stmt* s) {
284+
fn FlowBits Analyser.analyseWhileStmt(Analyser* ma, Stmt* s) {
285+
FlowBits flow = FlowNext;
265286
WhileStmt* w = (WhileStmt*)s;
287+
266288
ma.scope.enter(scope.Decl);
267289
ma.analyseCondition(w.getCond2(), true);
268290
if (ma.has_error) goto done;
269291

292+
Expr* cond = getCondExpr(w.getCond());
293+
if (cond.isCtv()) {
294+
Value v = ctv_analyser.get_value(cond);
295+
if (v.isZero()) {
296+
// TODO: body is never reached
297+
} else {
298+
flow = 0;
299+
}
300+
}
301+
270302
ma.scope.enter(scope.Break | scope.Continue | scope.Decl | scope.Control);
271-
ma.analyseStmt(w.getBody(), true);
303+
FlowBits flow2 = ma.analyseStmt(w.getBody(), true);
272304
ma.scope.exit(ma.has_error);
305+
flow |= flow2 & (FlowReturn | FlowGoto | FlowNoReturn | FlowError);
306+
if (flow2 & FlowBreak) flow |= FlowNext;
273307
done:
274308
ma.scope.exit(ma.has_error);
309+
return flow;
275310
}
276311

277312
fn QualType Analyser.analyseDeclStmt(Analyser* ma, Stmt* s) {

0 commit comments

Comments
 (0)