Skip to content

Commit ffd7830

Browse files
committed
improve warning of unused schema parameters
1 parent 8466767 commit ffd7830

File tree

2 files changed

+37
-12
lines changed

2 files changed

+37
-12
lines changed

godel-script/godel-frontend/src/error/error.cpp

+3-3
Original file line numberDiff line numberDiff line change
@@ -231,13 +231,13 @@ void error::warn_ignored_DO_schema(const std::unordered_set<std::string>& vec) {
231231
size_t ignored_count = 0;
232232
for(const auto& i : vec) {
233233
++ignored_count;
234-
if (ignored_count > 4) {
234+
if (ignored_count > 8) {
235235
break;
236236
}
237237
std::clog << reset << " " << i << "\n";
238238
}
239-
if (vec.size() > 4) {
240-
std::clog << reset << " ...(" << vec.size()-4 << ")\n";
239+
if (vec.size() > 8) {
240+
std::clog << reset << " ...(" << vec.size() - 8 << ")\n";
241241
}
242242
std::clog << std::endl;
243243
}

godel-script/godel-frontend/src/sema/ungrounded_checker.cpp

+34-9
Original file line numberDiff line numberDiff line change
@@ -293,7 +293,7 @@ void ungrounded_parameter_checker::report_unused_parameter(const report::span& s
293293
bool flag_self_ungrounded = false;
294294

295295
// start checking
296-
for(const auto& i : func->ordered_parameter_list) {
296+
for (const auto& i : func->ordered_parameter_list) {
297297
// if variable is not a parameter or already used, do not report
298298
if (!record.count(i) || record.at(i)) {
299299
continue;
@@ -315,25 +315,27 @@ void ungrounded_parameter_checker::report_unused_parameter(const report::span& s
315315
if (is_native_type(type) && !record_is_set_flag.at(i)) {
316316
ungrounded_params += ungrounded_params.length()? ", ":"";
317317
ungrounded_params += i;
318-
} else if (i!="self") {
318+
} else if (i != "self") {
319319
unused_params += unused_params.length()? ", ":"";
320320
unused_params += i;
321321
} else {
322+
// unused self, mark it as ungrounded
323+
// so self constraint will be generated to protect this parameter
322324
flag_self_ungrounded = true;
323325
}
324326
}
325327

326328
// unused warning report
327329
if (unused_params.length()) {
328330
err->warn(stmt_loc,
329-
"unused parameter \"" + unused_params + "\" in this branch."
331+
"\"" + unused_params + "\" is unused in this branch."
330332
);
331333
}
332334

333335
// ungrounded error report
334336
if (ungrounded_params.length()) {
335337
err->err(stmt_loc,
336-
"ungrounded parameter \"" + ungrounded_params + "\" in this branch."
338+
"\"" + ungrounded_params + "\" is ungrounded in this branch."
337339
);
338340
}
339341

@@ -381,6 +383,11 @@ bool ungrounded_parameter_checker::check_directly_call_identifier(expr* node) {
381383
!= ast_class::ac_identifier) {
382384
return false;
383385
}
386+
// schema instance getting primary key equals to directly using this instance
387+
// e.g. `a.id` in fact equals to `a` itself, so we see this as direct call id
388+
if (is_schema_get_primary_key(real)) {
389+
return true;
390+
}
384391
if (!real->get_call_chain().empty()) {
385392
return false;
386393
}
@@ -646,11 +653,13 @@ bool ungrounded_parameter_checker::visit_call_head(call_head* node) {
646653
}
647654

648655
bool ungrounded_parameter_checker::is_schema_get_primary_key(call_root* node) {
649-
if (node->get_call_head()->get_first_expression()->get_ast_class()!=ast_class::ac_identifier) {
656+
auto head = node->get_call_head();
657+
// should call a variable
658+
if (head->get_first_expression()->get_ast_class() != ast_class::ac_identifier) {
650659
return false;
651660
}
652661

653-
const auto& head_type = node->get_call_head()->get_resolve();
662+
const auto& head_type = head->get_resolve();
654663
// head type should not be global symbol or data-set type
655664
if (head_type.is_global || head_type.type.is_set) {
656665
return false;
@@ -663,16 +672,19 @@ bool ungrounded_parameter_checker::is_schema_get_primary_key(call_root* node) {
663672
if (node->get_call_chain().size()<1) {
664673
return false;
665674
}
675+
676+
// get type full path name
666677
const auto name = head_type.type.full_path_name_without_set();
667678
const auto index = ctx->global.get_index(name);
668-
if (index==global_symbol_table::npos) {
679+
if (index == global_symbol_table::npos) {
669680
return false;
670681
}
671-
if (ctx->global.get_kind(index)!=symbol_kind::schema) {
682+
if (ctx->global.get_kind(index) != symbol_kind::schema) {
672683
return false;
673684
}
685+
674686
const auto& sc = ctx->global.get_schema(index);
675-
if (node->get_call_chain()[0]->get_call_type()!=call_expr::type::get_field ||
687+
if (node->get_call_chain()[0]->get_call_type() != call_expr::type::get_field ||
676688
node->get_call_chain()[0]->has_func_call()) {
677689
return false;
678690
}
@@ -682,7 +694,20 @@ bool ungrounded_parameter_checker::is_schema_get_primary_key(call_root* node) {
682694
}
683695

684696
bool ungrounded_parameter_checker::visit_call_root(call_root* node) {
697+
// we see schema get primary key as call schema itself
698+
// because in generated souffle:
699+
// schema.primary_key = schema
700+
// if schema is not grounded, the primary key is not grounded too
701+
// but we add type constraint for each schema, so mark this as grounded
702+
// except this call is:
703+
// self.primary_key
704+
// self is not always constraint, if marked as grounded
705+
// self will be ungrounded in generated souffle
685706
if (is_schema_get_primary_key(node)) {
707+
auto first = node->get_call_head()->get_first_expression();
708+
if (reinterpret_cast<identifier*>(first)->get_name() != "self") {
709+
node->get_call_head()->accept(this);
710+
}
686711
return true;
687712
}
688713
for(auto i : node->get_call_chain()) {

0 commit comments

Comments
 (0)