Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make -D- option for compiled datalog behave same as if interpreted #2401

Merged
merged 3 commits into from
Feb 27, 2023

Conversation

Gueckmooh
Copy link
Contributor

@Gueckmooh Gueckmooh commented Feb 22, 2023

Makes compiled datalog handle -D- option as interpreted datalog. Fixes #2348
It is a second and less inclusive attempt after #2400.

Change slightly the generated code so that, if the output directory is - the io config is stdout.

As I said in #2400:
I don't know what is the best solution, to change slightly the generated code so that it puts "IO" to "stdout" when the output file is - or to change method IOSystem::getWriter to do the change before getting the appropriate factory.

In my opinion IOSystem should not have this intelligence and it should just expect the caller to know where it wants to put the output.. I'll try to do the change in the generated code, doing it in the getWriter method would be a piece of cake if we want to go for this solution afterwards.

.decl foo(a:number)
.output foo
foo(1).
$souffle test.dl -D-
---------------
foo
a
===============
1
===============

Before:

$ souffle test.dl -o test && ./test -D-
Output directory - does not exists!

After:

$souffle test.dl -o test && ./test -D-
---------------
foo
a
===============
1
===============

I still don't know how to test that, if you have any pointer to do that I would add a test to the PR.

@codecov
Copy link

codecov bot commented Feb 22, 2023

Codecov Report

Merging #2401 (519c438) into master (0334b07) will increase coverage by 0.09%.
The diff coverage is 85.75%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2401      +/-   ##
==========================================
+ Coverage   77.56%   77.65%   +0.09%     
==========================================
  Files         467      472       +5     
  Lines       30785    31045     +260     
==========================================
+ Hits        23877    24107     +230     
- Misses       6908     6938      +30     
Impacted Files Coverage Δ
src/parser/ParserDriver.h 100.00% <ø> (ø)
src/synthesiser/Synthesiser.h 100.00% <ø> (ø)
.../ast/transform/SimplifyConstantBinaryConstraints.h 50.00% <50.00%> (ø)
src/MainDriver.cpp 70.39% <70.27%> (+0.15%) ⬆️
src/parser/scanner.ll 78.01% <76.92%> (+5.49%) ⬆️
tests/libsouffle_interface/vfs_overlay_test.cpp 85.93% <85.93%> (ø)
src/parser/SrcLocation.cpp 91.86% <87.50%> (+7.24%) ⬆️
src/parser/ParserDriver.cpp 89.44% <88.33%> (-0.39%) ⬇️
src/parser/VirtualFileSystem.cpp 88.37% <88.37%> (ø)
...st/transform/SimplifyConstantBinaryConstraints.cpp 100.00% <100.00%> (ø)
... and 12 more

@quentin
Copy link
Member

quentin commented Feb 24, 2023

That's a nice solution!

About testing it, you probably want to pass -D-, for that you need to modify the test harness here and look how it's done to customize the facts directory:

set(SOUFFLE_PARAMS "-D" "." "-F" "${FACTS_DIR}")

In the CMakeLists.txt you would pass the new option as in:

souffle_run_test_helper(TEST_NAME pragma2 FUNCTORS CATEGORY semantic)

Then just put the expected output in your test's .out file.

@Gueckmooh
Copy link
Contributor Author

I've added a test in a new directory "behavior", because I did not find appropriate directory to put my test. Let me know if there is another better place for it.

@quentin
Copy link
Member

quentin commented Feb 27, 2023

Great, could you just move the test to the existing semantic directory? Most tests about I/Os are located there.
Thanks.

@quentin quentin merged commit 63a5996 into souffle-lang:master Feb 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Souffle compiled programs do not accept '-D-' output option
2 participants