Skip to content

Purge sort depending functions in favor of parametric functions#3773

Open
Drodt wants to merge 44 commits intomainfrom
purge-sort-depending-fn
Open

Purge sort depending functions in favor of parametric functions#3773
Drodt wants to merge 44 commits intomainfrom
purge-sort-depending-fn

Conversation

@Drodt
Copy link
Copy Markdown
Member

@Drodt Drodt commented Mar 16, 2026

Intended Change

Replace all sort depending functions from KeY and replace them with parametric functions, e.g., int::select -> select<[int]>.

This PR does not affect behavior. In particular, sequences are not (yet) made parametric and the grammar is not really modified.

SortDependingFunction is removed.

Plan

  • The grammar cannot be improved, because the sort depending function syntax is also used for fields, especially java.lang.Object::<created>
  • Fix all the tests that are sure to break

Type of pull request

  • Breaking change (fix or feature that would cause existing functionality to change)
  • There are changes to the (Java) code
  • There are changes to the taclet rule base

Ensuring quality

  • I made sure that introduced/changed code is well documented (javadoc and inline comments).
  • I made sure that new/changed end-user features are well documented (https://github.com/KeYProject/key-docs).
  • I added new test case(s) for new functionality.
  • I have tested the feature as follows: ...
  • I have checked that runtime performance has not deteriorated.
  • For new Gradle modules: I added the Gradle module to the test matrix in
    .github/workflows/tests.yml

Additional information and contact(s)

The contributions within this pull request are licensed under GPLv2 (only) for inclusion in KeY.

@Drodt Drodt force-pushed the purge-sort-depending-fn branch from 8256cc5 to 46212b5 Compare March 17, 2026 15:34
@Drodt Drodt mentioned this pull request Mar 18, 2026
2 tasks
@Drodt Drodt marked this pull request as ready for review March 18, 2026 17:21
@Drodt Drodt self-assigned this Mar 18, 2026
@wadoon
Copy link
Copy Markdown
Member

wadoon commented Mar 18, 2026

This java.lang.Object::<created> is not a sort-independent function. I am also not aware, way the <..> are back again. Should the new syntax not ``java.lang.Object::#$created`?

@Drodt
Copy link
Copy Markdown
Member Author

Drodt commented Mar 19, 2026

This java.lang.Object::<created> is not a sort-independent function.

It is a constant, not a SortDependingFunction.

I am also not aware, way the <..> are back again. Should the new syntax not ``java.lang.Object::#$created`?

This branch is not yet based on Java Parser. I thought, JP would take a bit longer than this PR.

@wadoon wadoon added this to the v3.0.0 milestone Mar 22, 2026
@Drodt
Copy link
Copy Markdown
Member Author

Drodt commented Mar 27, 2026

Delayed until #3120 is merged and invariants are changed from <int> to $inv

@unp1 unp1 self-requested a review April 10, 2026 13:23
@Drodt
Copy link
Copy Markdown
Member Author

Drodt commented Apr 10, 2026

Per the KeY meeting: This PR is truly ready. Eventual syntax improvements are left for future PR (but should be discussed soon!)

Copy link
Copy Markdown
Member

@WolframPfeifer WolframPfeifer left a comment

Choose a reason for hiding this comment

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

I reviewed the code and it looks mostly fine, except for a few minor complaints (mostly about comments).

However, when trying to test the PR the GUI, already at start of KeY I get the following exception (even when removing the .key directory):

[16:35:05.483] ERROR XMLResources - Cannot not load help messages in info viewjava.util.InvalidPropertiesFormatException: jdk.internal.org.xml.sax.SAXParseException; 
	at java.base/jdk.internal.util.xml.PropertiesDefaultHandler.load(PropertiesDefaultHandler.java:85)
	at java.base/java.util.Properties.loadFromXML(Properties.java:1003)
	at de.uka.ilkd.key.util.XMLResources.getResource(XMLResources.java:58)
	at de.uka.ilkd.key.util.XMLResources.<init>(XMLResources.java:36)
	at de.uka.ilkd.key.gui.InfoView.<init>(InfoView.java:55)
	at de.uka.ilkd.key.gui.InfoView.<init>(InfoView.java:149)
	at de.uka.ilkd.key.gui.MainWindow.<init>(MainWindow.java:329)
	at de.uka.ilkd.key.gui.MainWindow.getInstance(MainWindow.java:417)

I am not sure if the reason lies really in the PR, also KeY does not crash.

Apart from that and the minor comments, it seems to work fine.

Comment thread key.core/src/main/java/de/uka/ilkd/key/pp/SelectPrinter.java Outdated
Comment thread key.core/src/main/java/de/uka/ilkd/key/pp/FieldPrinter.java Outdated

WS: [ \t\n\r\u00a0]+ -> channel(HIDDEN); //U+00A0 = non breakable whitespace
STRING_LITERAL:'"' ('\\' . | ~( '"' | '\\') )* '"' ;
LESS: '<';
LESSEQUAL: '<' '=' | '\u2264';
LGUILLEMETS: '<' '<' | '«' | '‹';
RGUILLEMETS: '>''>' | '»' | '›';
IMPLICIT_IDENT: '<' '$'? (LETTER)+ '>' ('$lmtd')? -> type(IDENT);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Until doing this review, I was not aware that we have 11 ANTLR .g4 files now (3 * lexer, 8 * parser), instead of 4 which I remember from a while ago. How many different parsers are there, and how do they work together? Is this documented or at least outlined anywhere?

In my opinion, this makes it more difficult to find token/rule declarations, and also opens possibilities for inconsistencies: Here, IMPLICIT_IDENT was removed because it was already in the ncore KeYLexer, right?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It was removed because it is no longer needed. We have no more of these <id> identifiers with this PR

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

For the rest, let's discuss at the next meeting

@@ -178,6 +179,11 @@ public record SplitFieldName(String className, String attributeName) {
* @return the split field name
*/
public static @Nullable SplitFieldName trySplitFieldName(Named symbol) {
if (symbol instanceof ParametricFunctionInstance pfi) {
// e.g., $classErroneous<[A]>
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is this not one of our implicit field names? Why is it used as an example for a parametric function here?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

You weren't there at the KeY meeting when I explained just how amazing this implementation is and was: Yes, it is an implicit field but on the logic side, these fields where always sort depending functions, e.g., A::<classErroneous>. It just happened to look like the field syntax.

Now that sort depending functions are removed, we use parametric functions and cannot just split.

Comment thread key.core/src/main/java/de/uka/ilkd/key/ldt/HeapLDT.java Outdated
Comment on lines +127 to +128
// TODO(DD): Extend to more complex parametric functions; ask RB what is expected
// here
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Did you want to fix or remove this TODO?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants