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

separate symbol table interface from implementation #2200

Merged
merged 3 commits into from Mar 15, 2022

Conversation

quentin
Copy link
Member

@quentin quentin commented Mar 7, 2022

A common issue with c++ functors is that they must be compiled with the same OpenMP support as the datalog program that will call them. This change removes this constraint.

It separates interface from implementation, so that c++ functors only have to include the symbol table interface header and don't know about the actual implementation.

Interfaces of SymbolTable and RecordTable are respectively in souffle/SymbolTable.h and souffle/RecordTable.h. The implementation are respectively in souffle/datastructure/SymbolTableImpl.h and souffle/datastructure/RecordTableImpl.h.

Added souffle/SouffleFunctor.h, the minimal header that c++ functors must include.

Any code that used to instantiate SouffleTable objects must now include souffle/datastructure/SymbolTableImpl.h and instantiate the SouffleTableImpl class instead

Relates to #2195

@quentin quentin marked this pull request as ready for review March 7, 2022 13:14
@codecov
Copy link

codecov bot commented Mar 7, 2022

Codecov Report

Merging #2200 (16aeba1) into master (fcd024b) will decrease coverage by 0.26%.
The diff coverage is 66.71%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2200      +/-   ##
==========================================
- Coverage   75.51%   75.25%   -0.27%     
==========================================
  Files         452      454       +2     
  Lines       27394    27578     +184     
==========================================
+ Hits        20687    20753      +66     
- Misses       6707     6825     +118     
Impacted Files Coverage Δ
src/ast/ExecutionOrder.h 100.00% <ø> (ø)
src/ast/Program.h 100.00% <ø> (ø)
src/ast/transform/ComponentInstantiation.cpp 91.98% <ø> (ø)
src/ast/transform/MinimiseProgram.h 100.00% <ø> (ø)
src/ast/transform/ResolveAliases.cpp 98.93% <ø> (ø)
src/ast/utility/SipsMetric.h 18.18% <ø> (ø)
src/ast/utility/Utils.h 100.00% <ø> (ø)
src/ast/utility/Visitor.h 57.33% <0.00%> (ø)
src/ast2ram/provenance/ClauseTranslator.h 100.00% <ø> (ø)
src/ast2ram/provenance/UnitTranslator.h 100.00% <ø> (ø)
... and 99 more

@b-scholz
Copy link
Member

There seems to be a conflict now.

@b-scholz
Copy link
Member

There is now merge conflict.

@quentin
Copy link
Member Author

quentin commented Mar 11, 2022

@b-scholz I resolved the conflicts.

Copy link
Member

@b-scholz b-scholz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution!

@b-scholz b-scholz merged commit d1522e0 into souffle-lang:master Mar 15, 2022
@quentin quentin deleted the symboltable branch March 15, 2022 16:26
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.

None yet

2 participants