Skip to content

Commit

Permalink
robustify the debug and error reports (#2495)
Browse files Browse the repository at this point in the history
* fix crash in error report on empty source location

* robustify the type-analysis debug report

The type-analysis debug report used to crash on various assertions
because it assumed a well-typed program. However the first type-analysis
runs before the semantics checker, hence the program may be ill-typed at
that time.

With this change the type-analysis debug report printer makes no
assumptions about the expected types, instead it prints the infered
types.
  • Loading branch information
quentin authored Jun 26, 2024
1 parent 99399f3 commit 49f511d
Show file tree
Hide file tree
Showing 14 changed files with 85 additions and 111 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/CI-Tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ jobs:
if: ${{ matrix.domain == '32bit' }}
uses: ./.github/actions/cmake-test
with:
cmake-flags: -DSOUFFLE_CODE_COVERAGE=ON
cmake-flags: -DSOUFFLE_CODE_COVERAGE=ON -DSOUFFLE_TEST_DEBUG_REPORT=ON
n-chunks: ${{ needs.Test-Setup.outputs.n-chunks }}
chunk: ${{ matrix.chunk }}

Expand Down
1 change: 1 addition & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ option(SOUFFLE_CODE_COVERAGE "Enable coverage reporting" OFF)
option(SOUFFLE_BASH_COMPLETION "Enable/Disable bash completion" OFF)
option(SOUFFLE_USE_LIBFFI "Enable/Disable use of libffi" ON)
option(SOUFFLE_CUSTOM_GETOPTLONG "Enable/Disable custom getopt_long implementation" OFF)
option(SOUFFLE_TEST_DEBUG_REPORT "Enable/Disable generating debug-report for all tests" OFF)

cmake_dependent_option(SOUFFLE_USE_LIBCPP "Link to libc++ instead of libstdc++" ON
"CMAKE_CXX_COMPILER_ID STREQUAL Clang" OFF)
Expand Down
7 changes: 6 additions & 1 deletion cmake/SouffleTests.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -159,9 +159,10 @@ function(SOUFFLE_RUN_TEST_HELPER)
#and input paths
#PARAM_FACTS_DIR_NAME - the name of the "facts" subdirectory in each test.
#Usually just "facts" but can be different when running multi - tests
#PARAM_DEBUG_REPORT - dump a debug report
cmake_parse_arguments(
PARAM
"COMPILED;COMPILED_SPLITTED;FUNCTORS;NEGATIVE;MULTI_TEST;NO_PREPROCESSOR;OUTPUT_STDOUT" # Options
"COMPILED;COMPILED_SPLITTED;FUNCTORS;NEGATIVE;MULTI_TEST;NO_PREPROCESSOR;OUTPUT_STDOUT;DEBUG_REPORT" # Options
"TEST_NAME;CATEGORY;FACTS_DIR_NAME;EXTRA_DATA" #Single valued options
"INCLUDE_DIRS" # Multi-valued options
${ARGV}
Expand All @@ -182,6 +183,10 @@ function(SOUFFLE_RUN_TEST_HELPER)
set(SHORT_EXEC_STYLE "")
endif()

if (PARAM_DEBUG_REPORT OR SOUFFLE_TEST_DEBUG_REPORT)
list(APPEND EXTRA_FLAGS "--debug-report=dbg.html")
endif()

if (PARAM_NO_PREPROCESSOR)
list(APPEND EXTRA_FLAGS "--no-preprocessor")
else ()
Expand Down
122 changes: 39 additions & 83 deletions src/ast/analysis/typesystem/Type.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include "ast/Aggregator.h"
#include "ast/Atom.h"
#include "ast/BinaryConstraint.h"
#include "ast/BooleanConstraint.h"
#include "ast/BranchInit.h"
#include "ast/Clause.h"
#include "ast/IntrinsicAggregator.h"
Expand Down Expand Up @@ -557,11 +558,10 @@ void TypeAnalysis::run(const TranslationUnit& translationUnit) {
this->translationUnit = &translationUnit;
}

void TypeAnnotationPrinter::branchOnArgument(const Argument* cur, const Type& type) {
void TypeAnnotationPrinter::branchOnArgument(const Argument* cur) {
if (isA<ast::Variable>(*cur)) {
auto var = as<ast::Variable>(cur);
auto inferedTypeName = argumentTypes.find(var)->second;
os << *as<ast::Variable>(cur) << "" << inferedTypeName;
auto inferedTypeName = argumentTypes.find(cur)->second;
os << *cur << "" << inferedTypeName;
} else if (isA<UnnamedVariable>(*cur)) {
os << "_";
} else if (isA<NumericConstant>(*cur)) {
Expand All @@ -571,7 +571,7 @@ void TypeAnnotationPrinter::branchOnArgument(const Argument* cur, const Type& ty
} else if (isA<NilConstant>(*cur)) {
print_(type_identity<NilConstant>(), *as<NilConstant>(cur));
} else if (isA<RecordInit>(*cur)) {
print_(type_identity<RecordInit>(), *as<RecordInit>(cur), *as<RecordType>(getBaseType(&type)));
print_(type_identity<RecordInit>(), *as<RecordInit>(cur));
} else if (isA<BranchInit>(*cur)) {
print_(type_identity<BranchInit>(), *as<BranchInit>(cur));
} else if (isA<IntrinsicFunctor>(*cur)) {
Expand All @@ -595,18 +595,10 @@ void TypeAnnotationPrinter::print_(type_identity<Atom>, const Atom& atom) {
auto name = atom.getQualifiedName();
os << name << "(";
auto args = atom.getArguments();
auto rel = program.getRelation(atom);
auto atts = rel->getAttributes();

std::size_t i = 0;
for (auto cur : args) {
const auto& declaredTypeName = atts[i]->getTypeName();
const auto& declaredType = typeEnv.getType(declaredTypeName);
assert(typeEnv.isType(declaredTypeName));
branchOnArgument(cur, declaredType);
if (isA<RecordInit>(cur) || isA<UnnamedVariable>(cur) || isA<TypeCast>(cur)) {
os << "∈{" << declaredTypeName << "}";
}
branchOnArgument(cur);
if (i + 1 < args.size()) {
os << ",";
}
Expand Down Expand Up @@ -644,76 +636,52 @@ void TypeAnnotationPrinter::print_(type_identity<NumericConstant>, const Numeric
}
}

void TypeAnnotationPrinter::print_(type_identity<BinaryConstraint>, const BinaryConstraint& rel) {
auto lhs = rel.getLHS();
auto lTySet = argumentTypes.find(lhs)->second;
assert(lTySet.size() == 1);
auto lTy = lTySet.begin();
branchOnArgument(lhs, *lTy);
if (isA<RecordInit>(lhs) || isA<UnnamedVariable>(lhs)) {
os << "∈{" << lTy->getName() << "}";
}

os << " " << rel.getBaseOperator() << " ";
void TypeAnnotationPrinter::print_(type_identity<BinaryConstraint>, const BinaryConstraint& bin) {
auto lhs = bin.getLHS();
auto rhs = bin.getRHS();
branchOnArgument(lhs);
os << " " << bin.getBaseOperator() << " ";
branchOnArgument(rhs);
}

auto rhs = rel.getRHS();
auto rTySet = argumentTypes.find(rhs)->second;
assert(rTySet.size() == 1);
auto rTy = rTySet.begin();
branchOnArgument(rhs, *rTy);
if (isA<RecordInit>(rhs) || isA<UnnamedVariable>(rhs)) {
os << "∈{" << rTy->getName() << "}";
}
void TypeAnnotationPrinter::print_(type_identity<BooleanConstraint>, const BooleanConstraint& c) {
os << c;
}

void TypeAnnotationPrinter::print_(type_identity<IntrinsicFunctor>, const IntrinsicFunctor& fun) {
auto arguments = fun.getArguments();
if (arguments.size() == 2) { // binary
auto tySet = argumentTypes.find(arguments[0])->second;
assert(tySet.size() == 1);
auto ty = tySet.begin();
os << "(";
branchOnArgument(arguments[0], *ty);
if (isA<RecordInit>(arguments[0]) || isA<UnnamedVariable>(arguments[0])) {
os << "∈{" << ty->getName() << "}";
}

branchOnArgument(arguments[0]);
os << " " << fun.getBaseFunctionOp() << " ";

auto tySet2 = argumentTypes.find(arguments[1])->second;
assert(tySet2.size() == 1);
auto ty2 = tySet2.begin();
branchOnArgument(arguments[1], *ty2);
if (isA<RecordInit>(arguments[1]) || isA<UnnamedVariable>(arguments[1])) {
os << "∈{" << ty2->getName() << "}";
}

branchOnArgument(arguments[1]);
os << ")";
} else {
os << fun.getBaseFunctionOp() << "(";
for (std::size_t i = 0; i < arguments.size(); ++i) {
TypeAttribute argType = typeAnalysis.getFunctorParamTypeAttribute(fun, i);
auto& ty = typeEnv.getConstantType(argType);
branchOnArgument(arguments[i], ty);
branchOnArgument(arguments[i]);
if (i + 1 < arguments.size()) {
os << ",";
}
}
os << ")";
}
const TypeSet& ts = argumentTypes.at(&fun);
os << "" << ts;
}

void TypeAnnotationPrinter::print_(type_identity<UserDefinedFunctor>, const UserDefinedFunctor& fun) {
auto arguments = fun.getArguments();
os << "@" << fun.getName() << "(";
for (std::size_t i = 0; i < arguments.size(); ++i) {
const auto& ty = typeAnalysis.getFunctorParamType(fun, i);
branchOnArgument(arguments[i], ty);
branchOnArgument(arguments[i]);
if (i + 1 < arguments.size()) {
os << ",";
}
}
os << ")";
const TypeSet& ts = argumentTypes.at(&fun);
os << "" << ts;
}

void TypeAnnotationPrinter::print_(type_identity<Counter>, [[maybe_unused]] const Counter& counter) {
Expand All @@ -727,52 +695,39 @@ void TypeAnnotationPrinter::print_(

void TypeAnnotationPrinter::print_(type_identity<TypeCast>, const ast::TypeCast& typeCast) {
os << "as(";
auto& ty = typeEnv.getType(typeCast.getType());
branchOnArgument(typeCast.getValue(), ty);
os << "," << ty.getName() << ")";
branchOnArgument(typeCast.getValue());
os << "," << typeCast.getType() << ")";
const TypeSet& ts = argumentTypes.at(&typeCast);
os << "" << ts;
}

void TypeAnnotationPrinter::print_(
type_identity<RecordInit>, const RecordInit& record, const RecordType& type) {
void TypeAnnotationPrinter::print_(type_identity<RecordInit>, const RecordInit& record) {
auto arguments = record.getArguments();
const auto& ftypes = type.getFields();
os << "[";
for (std::size_t i = 0; i < arguments.size(); ++i) {
branchOnArgument(arguments[i], *ftypes[i]);
if (isA<RecordInit>(arguments[i]) || isA<UnnamedVariable>(arguments[i])) {
os << "∈{" << (*ftypes[i]).getName() << "}";
}
branchOnArgument(arguments[i]);
if (i + 1 < arguments.size()) {
os << ",";
}
}
os << "]";
const TypeSet& ts = argumentTypes.at(&record);
os << "" << ts;
}

void TypeAnnotationPrinter::print_(type_identity<BranchInit>, const BranchInit& adt) {
auto* correspondingType = sumTypesBranches.getType(adt.getBranchName());

assert(correspondingType != nullptr);
assert(isA<AlgebraicDataType>(correspondingType));

auto branchTypes = as<AlgebraicDataType>(correspondingType)->getBranchTypes(adt.getBranchName());
auto branchArgs = adt.getArguments();

assert(branchTypes.size() == branchArgs.size());

os << "$" << adt.getBranchName() << "(";
for (std::size_t i = 0; i < branchArgs.size(); i++) {
auto arg = branchArgs[i];
auto argTy = branchTypes[i];
branchOnArgument(arg, *argTy);
if (isA<RecordInit>(arg) || isA<UnnamedVariable>(arg)) {
os << "∈{" << argTy->getName() << "}";
}
branchOnArgument(branchArgs[i]);
if (i + 1 < branchArgs.size()) {
os << ", ";
}
}
os << ")";
const TypeSet& ts = argumentTypes.at(&adt);
os << "" << ts;
}

void TypeAnnotationPrinter::print_(type_identity<Aggregator>, const Aggregator& agg) {
Expand All @@ -781,14 +736,13 @@ void TypeAnnotationPrinter::print_(type_identity<Aggregator>, const Aggregator&
os << baseOperator << " ";
auto targetExpr = agg.getTargetExpression();
if (targetExpr /* the target expression can be null */) {
auto tySet = argumentTypes.find(targetExpr)->second;
assert(tySet.size() == 1);
auto ty = tySet.begin();
branchOnArgument(targetExpr, *ty);
branchOnArgument(targetExpr);
}
os << " : { ";
printBodyLiterals(bodyLiterals, " ");
os << " }";
const TypeSet& ts = argumentTypes.at(&agg);
os << "" << ts;
}

void TypeAnnotationPrinter::printBodyLiterals(std::vector<Literal*> bodyLiterals, const std::string& spc) {
Expand All @@ -800,6 +754,8 @@ void TypeAnnotationPrinter::printBodyLiterals(std::vector<Literal*> bodyLiterals
print_(type_identity<Negation>(), *as<Negation>(*cur));
} else if (isA<BinaryConstraint>(*cur)) {
print_(type_identity<BinaryConstraint>(), *as<BinaryConstraint>(*cur));
} else if (isA<BooleanConstraint>(*cur)) {
print_(type_identity<BooleanConstraint>(), *as<BooleanConstraint>(*cur));
} else {
os << "(?)";
}
Expand Down
12 changes: 7 additions & 5 deletions src/ast/analysis/typesystem/Type.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

#include "AggregateOp.h"
#include "FunctorOps.h"
#include "ast/BooleanConstraint.h"
#include "ast/Clause.h"
#include "ast/NumericConstant.h"
#include "ast/TranslationUnit.h"
Expand Down Expand Up @@ -163,7 +164,7 @@ class TypeAnalysis : public Analysis {
*/
class TypeAnnotationPrinter {
public:
TypeAnnotationPrinter(const TranslationUnit* tu, const std::map<const Argument*, TypeSet> argumentTypes,
TypeAnnotationPrinter(const TranslationUnit* tu, const std::map<const Argument*, TypeSet>& argumentTypes,
std::ostream& os)
: tu(tu), argumentTypes(argumentTypes), os(os) {}

Expand All @@ -177,24 +178,25 @@ class TypeAnnotationPrinter {
const SumTypeBranchesAnalysis& sumTypesBranches = tu->getAnalysis<SumTypeBranchesAnalysis>();
const TypeAnalysis& typeAnalysis = tu->getAnalysis<TypeAnalysis>();

std::map<const Argument*, TypeSet> argumentTypes;
const std::map<const Argument*, TypeSet>& argumentTypes;
std::ostream& os;

void branchOnArgument(const Argument*, const Type&);
void branchOnArgument(const Argument*);
void print_(type_identity<Atom>, const Atom& atom);
void print_(type_identity<Negation>, const Negation& cur);
void print_(type_identity<StringConstant>, const StringConstant& cnst);
void print_(type_identity<NumericConstant>, const NumericConstant& constant);
void print_(type_identity<NilConstant>, const NilConstant& constant);
void print_(type_identity<BinaryConstraint>, const BinaryConstraint& rel);
void print_(type_identity<BinaryConstraint>, const BinaryConstraint& bin);
void print_(type_identity<IntrinsicFunctor>, const IntrinsicFunctor& fun);
void print_(type_identity<UserDefinedFunctor>, const UserDefinedFunctor& fun);
void print_(type_identity<Counter>, const Counter& counter);
void print_(type_identity<IterationCounter>, const IterationCounter& counter);
void print_(type_identity<TypeCast>, const ast::TypeCast& typeCast);
void print_(type_identity<RecordInit>, const RecordInit& record, const RecordType&);
void print_(type_identity<RecordInit>, const RecordInit& record);
void print_(type_identity<BranchInit>, const BranchInit& adt);
void print_(type_identity<Aggregator>, const Aggregator& agg);
void print_(type_identity<BooleanConstraint>, const BooleanConstraint& c);
void printBodyLiterals(std::vector<Literal*> literals, const std::string& spc);
};

Expand Down
8 changes: 5 additions & 3 deletions src/ast/analysis/typesystem/TypeSystem.h
Original file line number Diff line number Diff line change
Expand Up @@ -364,12 +364,12 @@ class AlgebraicDataType : public Type {
[](const Branch& left, const Branch& right) { return left.name.lexicalLess(right.name); });
}

const std::vector<const Type*>& getBranchTypes(const QualifiedName& name) const {
std::vector<const Type*> getBranchTypes(const QualifiedName& name) const {
for (auto& branch : branches) {
if (branch.name == name) return branch.types;
}
// Branch doesn't exist.
throw std::out_of_range("Trying to access non-existing branch.");
return {};
}

/** Return the branches as a sorted vector */
Expand Down Expand Up @@ -523,7 +523,9 @@ struct TypeSet {
/** Print type set */
void print(std::ostream& out) const {
if (all) {
out << "{ - all types - }";
out << "";
} else if (types.empty()) {
out << "";
} else {
out << "{"
<< join(types, ",", [](std::ostream& out, const Type* type) { out << type->getName(); })
Expand Down
16 changes: 8 additions & 8 deletions src/ast/tests/type_system_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -173,20 +173,20 @@ TEST(TypeSystem, GreatestCommonSubtype) {
EXPECT_EQ("{B}", toString(getGreatestCommonSubtypes(B, B)));
EXPECT_EQ("{C}", toString(getGreatestCommonSubtypes(C, C)));

EXPECT_EQ("{}", toString(getGreatestCommonSubtypes(A, B)));
EXPECT_EQ("{}", toString(getGreatestCommonSubtypes(A, C)));
EXPECT_EQ("{}", toString(getGreatestCommonSubtypes(B, C)));
EXPECT_EQ("", toString(getGreatestCommonSubtypes(A, B)));
EXPECT_EQ("", toString(getGreatestCommonSubtypes(A, C)));
EXPECT_EQ("", toString(getGreatestCommonSubtypes(B, C)));

EXPECT_EQ("{}", toString(getGreatestCommonSubtypes(A, B, C)));
EXPECT_EQ("", toString(getGreatestCommonSubtypes(A, B, C)));

EXPECT_EQ("{A}", toString(getGreatestCommonSubtypes(A, N)));
EXPECT_EQ("{A}", toString(getGreatestCommonSubtypes(N, A)));

EXPECT_EQ("{B}", toString(getGreatestCommonSubtypes(B, N)));
EXPECT_EQ("{B}", toString(getGreatestCommonSubtypes(N, B)));

EXPECT_EQ("{}", toString(getGreatestCommonSubtypes(C, N)));
EXPECT_EQ("{}", toString(getGreatestCommonSubtypes(N, C)));
EXPECT_EQ("", toString(getGreatestCommonSubtypes(C, N)));
EXPECT_EQ("", toString(getGreatestCommonSubtypes(N, C)));

// // bring in unions

Expand Down Expand Up @@ -242,8 +242,8 @@ TEST(TypeSystem, complexSubsetTypes) {

EXPECT_EQ("{B}", toString(getGreatestCommonSubtypes(A, BfromA)));
EXPECT_EQ("{C}", toString(getGreatestCommonSubtypes(A, CfromA)));
EXPECT_EQ("{}", toString(getGreatestCommonSubtypes(A, BfromA, CfromA)));
EXPECT_EQ("{}", toString(getGreatestCommonSubtypes(BfromA, CfromA)));
EXPECT_EQ("", toString(getGreatestCommonSubtypes(A, BfromA, CfromA)));
EXPECT_EQ("", toString(getGreatestCommonSubtypes(BfromA, CfromA)));

auto* base = &env.createType<SubsetType>(qn("B0"), BfromA);
for (std::size_t i = 1; i <= 10; ++i) {
Expand Down
2 changes: 1 addition & 1 deletion src/ast/transform/TypeChecker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -495,7 +495,7 @@ void TypeCheckerImpl::visit_(type_identity<BranchInit>, const BranchInit& adt) {
// We know now that the set "types" is a singleton
auto& sumType = *as<analysis::AlgebraicDataType>(*types.begin());

auto& argsDeclaredTypes = sumType.getBranchTypes(adt.getBranchName());
auto argsDeclaredTypes = sumType.getBranchTypes(adt.getBranchName());

auto args = adt.getArguments();

Expand Down
Loading

0 comments on commit 49f511d

Please sign in to comment.