metaborg / nabl

Spoofax' Name Binding Language
Apache License 2.0
7 stars 12 forks source link

Intermittent empty query results #68

Closed MeAmAnUsername closed 2 years ago

MeAmAnUsername commented 2 years ago

Bug description Query result is sometimes empty even though there is clearly something in the relation.

Versions Eclipse: org.eclipse.platform.ide 4.16.0.I20200604-0540 Spoofax: org.metaborg.spoofax.eclipse 2.6.0.20210526-183432-master System: Mac OS X x86_64 10.14.6 (I got this info from Spoofax-meta > Report Issue, let me know if you really need it from the plugins view)

Statix setup: multi-file

Steps to reproduce the behavior Clone my fork of pie Checkout branch bug-statix-inconsistent-query (you should be on the commit named Add debugging info + example file for bug report with revision number cc52cb2ff43a8f9c63afd231af7e6c49aec4a511) Build the project Open example > example.pie Force reanalysis by renaming res until there is no error on Fuz<Fish>.

Renaming with keyboard is fine. I assume Spoofax > Refactor > Rename also works, but it takes far longer. The error normally disappears for me within 0 - 5 modify-analyze cycles.

Observed behavior In the problems view (or when hovering over Fuz of Fuz<Fish>) there is the following note: DEBUG - type args in "Fuz": [] -- s_data: #.-s_data_instance_42-7; occs: [] It shows the results of the query query generic_arg filter e in s_data |-> occs on line 119 in statics/type_common. The query returns empty. However, when opening the scope graph that scope does have a declaration for generic_arg:

  #.-s_data_instance_42-7 {
    relations {
      statics/base!root : #-s_1-1
      statics/base!generic_arg : ("T", DataType(#.-s_data_instance_232-19))
    }
    edges {
      statics/base!P : #example/example.pie-s_data_def_152-27
    }
  }

Happens inconsistently. Changes between analysis. Rebuilding does not affect it. Clean build does have an effect in the sense that some tests no longer fail, but this problem still happens. I haven't looked into exactly which tests stop (intermittently) failing after a clean build.

Expected behavior The query consistently returns the single generic argument declared in that scope.

Additional context This was introduced on the commit named [WIP] Return whether type arguments are correct, revision number 97100d3e92d50758c482e0beb83177792d5da41d. This is the second time I tried the change. This happened both times. The first time I made this change I assumed I had made some mistake and retried from scratch. This is not critical for me, it's for removing cascading errors on function calls with incorrect generic arguments. The next commit removed checking whether the arguments matched the parameters if the type arguments are incorrect. However, I would like to implement that at some point.

Some playing around brings up an interesting hypothesis: it seems like certain names for res will consistently succeed or fail to produce an error.

MeAmAnUsername commented 2 years ago

Log from running SPT tests on some of the commits showing how the tests fail intermittently

go to [WIP] Return whether type arguments are correct
build
run spt tests - 1464 passed
run spt tests - 1464 passed
clean build
run spt tests - 1464 passed
go to [WIP] Only check whether arguments match function parameters if type arguments are correct
build
run SPT tests - 1475 passed
run SPT tests - 1475 passed
build
run SPT tests - 1475 passed
clean build
run SPT tests - 1477 passed
run SPT tests - 1477 passed
go to [WIP] Return whether type arguments are correct
build
run SPT tests - 1469 passed
run SPT tests - 1470 passed
run SPT tests - 1470 passed
run SPT tests - 1469 passed - 1,6,29,27,1
run SPT tests - 1470 passed - 1,6,28,27,1
run SPT tests - 1469 passed - 1,6,29,27,1
run SPT tests - 1469 passed - 1,6,29,27,1
run SPT tests - 1470 passed - 1,6,28,27,1 - task supplier with two type arguments now succeeds
open expression.spt - task supplier with two type arguments succeeds
edit expression.spt to force re-analysis - task supplier with two type arguments fails
edit expression.spt to force re-analysis - task supplier with two type arguments succeeds
run SPT tests - 1470 passed - 1,6,28,27,1
build
run SPT tests - 1473 passed - 1,6,24,27,2
run SPT tests - 1473 passed - 1,6,24,27,2
Go to [WIP] change test because of bug
build
run SPT tests - 1480 passed - 1,6,24,27,1
AZWN commented 2 years ago

Thanks for the detailed report! On a first glance, this indeed looks like a scheduling bug, although such large examples are hard to analyze manually/debug. I created a new step-by-step guide to creating real minimal examples. If you have time to spare, I'd be happy when you could apply that on your example. That would also raise this issue a lot on my priority list.

Two remarks:

AZWN commented 2 years ago

Linking the test file: https://slde.slack.com/files/UHT4CDP99/F02AM21CDG9/statics.stxtest

MeAmAnUsername commented 2 years ago

Got the same/a similar bug again in this branch. It seems to (sometimes) not resolve the type arguments for nested Generic parameter references. It started in this commit, which brought the hypothesis that it might be caused by adding a return value to genericArgsOk, but that hypothesis is disproven by this commit where I removed the return value but the bug remained. Conclusion is that it is deterministic, but I don't have the slightest clue under what conditions this bug happens.

This is blocking me now, I need to declare the type arguments to generate the Java code and I have tried all feasible ways I could think of to work around it.

AZWN commented 2 years ago

I think this issue should be fixed in e67871d224d2fcb5fe08b5f5bdf48fdaa259757b. Can you confirm this solves the issue in your original project (PIE)?

MeAmAnUsername commented 2 years ago

I tried the new Statix on this problem today. Your change resolves the issue that I reported here, and a similar(?) issue I had later. I have another bug which is not resolved by this change, but I think that it is indeed another bug, so I'll open a new issue for it.

You can mark this issue as resolved as far as I am concerned, although you may want to wait what Arjan says.