Skip to content

implement #sort aggregate - #650

Closed
MaxOstrowski wants to merge 1 commit into
potassco:wip-20from
MaxOstrowski:wip-20
Closed

MaxOstrowski wants to merge 1 commit into
potassco:wip-20from
MaxOstrowski:wip-20

Conversation

@MaxOstrowski

Copy link
Copy Markdown
Member

This implements a sorting aggregate according to the discussion in #469

Sorry for the large diff. It tries to cover all aspects of the new aggregate, including AST. There is a fast_track for domain input, otherwise the translation from the discussion is used.

As always I do not know what I'm doing, if anything is completely out of place or should have been done differently, please tell me.

Disclaimer: Most of the code changes are done with AI.

@rkaminsk

Copy link
Copy Markdown
Member

Can you also make your LLM describe what exactly it did and post it here?

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR adds support for a new #sort body literal/aggregate across clingo’s full stack (parser → input AST/rewrite → grounding → C/C++/Python APIs), including a strategy to keep #sort native for domain-friendly cases and to lower it to auxiliary rules when needed.

Changes:

  • Extend the language front-end to parse/print #sort body literals and represent them in the input AST (BdLitSort), including syntax/safety/dependency/rewriting support.
  • Add grounding support for #sort via a dedicated StateSort/LitSortStrat implementation and integrate it into the control layer.
  • Expose the new node through C/C++/Python AST APIs and add corresponding test coverage.

Reviewed changes

Copilot reviewed 42 out of 42 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
lib/python-api/tests/test_ast.py Adds Python AST tests for BodySort.
lib/python-api/stubs/ast.pyi Extends Python typing stubs/unions to include BodySort.
lib/python-api/src/ast.cc Implements Python bindings for BodySort and includes it in BodyLiteral.
lib/input/tests/safety.cc Adds safety-check expectations for #sort in rule bodies.
lib/input/tests/program.cc Adds rewrite-program expectations ensuring deferred lowering behavior.
lib/input/tests/parser.cc Adds parser roundtrip tests for #sort syntax.
lib/input/src/rewrite/visit.hh Updates generic visitor to traverse BdLitSort.
lib/input/src/rewrite/visit_variables.cc Adds variable visitation rules for BdLitSort.
lib/input/src/rewrite/unpool.cc Enables unpooling support for BdLitSort.
lib/input/src/rewrite/unpool_relations.cc Handles relation-unpooling transformations involving BdLitSort.
lib/input/src/rewrite/transform.hh Adds transformer support for rewriting BdLitSort.
lib/input/src/rewrite/simplify.cc Adds simplification logic for BdLitSort.
lib/input/src/rewrite/safety.cc Integrates BdLitSort into safety dependency graph construction.
lib/input/src/rewrite/lower_sort.cc Introduces lowering from #sort to auxiliary rules for non-domain scenarios.
lib/input/src/rewrite/dependency.cc Includes BdLitSort in dependency analysis.
lib/input/src/rewrite/compute_bounds.cc Computes bounds for sort elements similarly to aggregates.
lib/input/src/rewrite/check_syntax.cc Enforces #sort well-formedness (unnegated equality with a pair LHS, etc.).
lib/input/src/rewrite/analyze.cc Updates analysis helpers to classify BdLitSort.
lib/input/src/rewrite/add_sign.cc Extends sign adjustment logic to include BdLitSort.
lib/input/src/rewrite.cc Refactors relation-unpool handling flow (small structural change).
lib/input/src/program.cc Adds deferred sort-lowering decision logic based on component/domain analysis.
lib/input/src/print.cc Adds pretty-printing for BdLitSort.
lib/input/src/parse/parser_state.hh Adds #sort token type.
lib/input/src/parse/lexer_impl.xch Recognizes #sort keyword in the lexer.
lib/input/src/parse/body_literal.cc Parses #sort body literals and their elements.
lib/input/include/clingo/input/rewrite/lower_sort.hh Declares the sort-lowering rewrite entry point.
lib/input/include/clingo/input/program.hh Adds state for generating unique auxiliary predicate ids for lowering.
lib/input/include/clingo/input/print.hh Declares stream/buffer output operators for BdLitSort.
lib/input/include/clingo/input/body_literal.hh Adds BdLitSort to the input AST variant and defines its record layout.
lib/input/CMakeLists.txt Registers new rewrite header/source for build.
lib/ground/src/sort.cc Implements grounding state/matcher/strategy for #sort.
lib/ground/include/clingo/ground/sort.hh Declares the grounding API for #sort.
lib/ground/CMakeLists.txt Registers new ground header/source for build.
lib/cxx-api/tests/profile.cc Adds profiling test coverage for #sort grounding.
lib/cxx-api/tests/ast.cc Adds C++ AST tests for body_sort.
lib/cxx-api/include/clingo/detail/ast.hh Adds constructor metadata for body_sort AST nodes.
lib/cxx-api/include/clingo/ast.hh Adds outputs attribute and body_sort node type to C++ API enums.
lib/control/tests/text.cc Adds --text grounding output expectations for domain vs lowered sorts.
lib/control/src/statement.cc Builds ground representation for BdLitSort (domain/native path).
lib/c-api/src/ast.cc Adds C AST conversion/attributes/construct support for body_sort.
lib/c-api/src/ast_yaml.cc Documents body_sort in YAML AST type info.
lib/c-api/include/clingo/ast.h Extends C API enums with clingo_ast_type_body_sort and outputs attribute.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +121 to +122
auto elem_name = ctx.new_aux_predicate("#sort_elem_");
auto chain_name = ctx.new_aux_predicate("#sort_chain_");
Comment thread lib/ground/src/sort.cc
}
}
static void construct(auto &mbr, Assignment &ass, VariableVec const &global, GroupKey *&target) {
auto size = global.size() * sizeof(Symbol);
@MaxOstrowski

Copy link
Copy Markdown
Member Author

Do you still want my LLM's summary (will probably be the same as the summary above).
Otherwise I'll be on vacation and will respond once I'm back.

@rkaminsk

Copy link
Copy Markdown
Member

As far as I can see, this implementation adds actual rules with conditional literals, when the sort aggregate is non-domain. I am not sure I like that. It would probably be better to do the translation internally.

@MaxOstrowski

Copy link
Copy Markdown
Member Author

https://github.com/MaxOstrowski/clingo/tree/sort_direct_translation
There is an alternative implementation that does the translation internally without relying on conditionals. Not sure if the semantic change in terms of incremental programs and convex aggregate is worth it. No need to react.

@rkaminsk

Copy link
Copy Markdown
Member

I am going to close this in favor of #666.

@rkaminsk rkaminsk closed this Sep 14, 2026
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.

3 participants