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

Hide non-exported stuff from other libraries #6905

Closed
wants to merge 43 commits into from

Conversation

JaroslavTulach
Copy link
Member

@JaroslavTulach JaroslavTulach commented May 31, 2023

Goal

This is a proof of concept prototype for improvements in import/export encapsulation. It has following goals:

  • import Namespace.Name should use only Main and its re-exports
  • import project.some.Item should always use physical paths inside of the same project/package

The Plan

A new test is written to check that non-rexported type Impl isn't available for other libraries.

Additional compiler pass ImportApiAnalysis is created that verifies the "logical imports". If there is a Namespace.Name.Main in a package, then only modules (transitively) exported via Main are allowed. For others an IR.Error.ImportExport is inserted into the IR.

With dfd116a the following is working:

sbt> runtime/test
sbt> runEngineDistribution --run test/Tests

Checklist

Please ensure that the following checklist has been satisfied before submitting the PR:

  • The documentation has been updated, if necessary.
  • All code follows the
    Scala,
    Java,
    style guides.
  • All code has been tested:
    • Unit tests have been written where possible.

@JaroslavTulach JaroslavTulach force-pushed the wip/jtulach/ImportExportWithMain branch from ed0ec62 to 42118ac Compare June 2, 2023 16:54
@JaroslavTulach
Copy link
Member Author

With additional tweaks of import and export in standard library done in dfd116a the following command passes OK:

sbt:enso> runEngineDistribution --run test/Tests
1863 tests succeeded.
0 tests failed.
21 tests skipped.

@JaroslavTulach JaroslavTulach force-pushed the wip/jtulach/ImportExportWithMain branch from 3fbf872 to e8ef79d Compare June 15, 2023 16:31
@JaroslavTulach
Copy link
Member Author

We have following failures:

@JaroslavTulach
Copy link
Member Author

JaroslavTulach commented Jun 16, 2023

There was 14 FAILED tests in test/Table_Tests:

  - [FAILED] should allow equality joins for custom objects [22ms]
      Reason: An unexpected panic was thrown: (No_Such_Conversion.Error Comparable 1 UnresolvedConversion)
      at <enso> My_Type_Comparator.compare(/runner/_work/enso/enso/test/Table_Tests/src/Common_Table_Operations/Join/Join_Spec.enso:25:16-37)
      at <enso> Any.<<arg-1>(/runner/_work/enso/enso/built-distribution/enso-engine-2023.2.1-dev-linux-amd64/enso-2023.2.1-dev/lib/Standard/Base/2023.2.1-dev/src/Any.enso:212:18-57)
      at <enso> case_branch(/runner/_work/enso/enso/built-distribution/enso-engine-2023.2.1-dev-linux-amd64/enso-2023.2.1-dev/lib/Standard/Base/2023.2.1-dev/src/Any.enso:424:17-22)
      at <enso> Any.assert_same_comparators(/runner/_work/enso/enso/built-distribution/enso-engine-2023.2.1-dev-linux-amd64/enso-2023.2.1-dev/lib/Standard/Base/2023.2.1-dev/src/Any.enso:423-425)

  - [FAILED] should allow range-based joins (using Between) for custom objects [6ms]
      Reason: An unexpected panic was thrown: (No_Such_Conversion.Error Comparable 20 UnresolvedConversion)
      at <enso> My_Type_Comparator.compare(/runner/_work/enso/enso/test/Table_Tests/src/Common_Table_Operations/Join/Join_Spec.enso:25:16-37)
      at <enso> Any.<<arg-1>(/runner/_work/enso/enso/built-distribution/enso-engine-2023.2.1-dev-linux-amd64/enso-2023.2.1-dev/lib/Standard/Base/2023.2.1-dev/src/Any.enso:212:18-57)
      at <enso> case_branch(/runner/_work/enso/enso/built-distribution/enso-engine-2023.2.1-dev-linux-amd64/enso-2023.2.1-dev/lib/Standard/Base/2023.2.1-dev/src/Any.enso:424:17-22)
      at <enso> Any.assert_same_comparators(/runner/_work/enso/enso/built-distribution/enso-engine-2023.2.1-dev-linux-amd64/enso-2023.2.1-dev/lib/Standard/Base/2023.2.1-dev/src/Any.enso:423-425)

  - [FAILED] should respect the right row limit [1ms]
      Reason: An unexpected panic was thrown: (No_Such_Method.Error 0 UnresolvedSymbol<up_to>)
      at <enso> Cross_Join_Spec.spec<arg-0>(/runner/_work/enso/enso/test/Table_Tests/src/Common_Table_Operations/Join/Cross_Join_Spec.enso:69:41-51)
      at <enso> Cross_Join_Spec.spec<arg-0>(/runner/_work/enso/enso/test/Table_Tests/src/Common_Table_Operations/Join/Cross_Join_Spec.enso:69:41-63)
      at <enso> Cross_Join_Spec.spec<arg-1>(/runner/_work/enso/enso/test/Table_Tests/src/Common_Table_Operations/Join/Cross_Join_Spec.enso:69:20-65)
      at <enso> Test.run_spec<arg-1>(/runner/_work/enso/enso/built-distribution/enso-engine-2023.2.1-dev-linux-amd64/enso-2023.2.1-dev/lib/Standard/Test/2023.2.1-dev/src/Test.enso:196:18-25)

  - [FAILED] should efficiently compute equality joins [0ms]
      Reason: An unexpected panic was thrown: (No_Such_Method.Error 0 UnresolvedSymbol<up_to>)
      at <enso> Join_Performance_Spec.spec<arg-0>(/runner/_work/enso/enso/test/Table_Tests/src/In_Memory/Join_Performance_Spec.enso:18:19-27)
      at <enso> Join_Performance_Spec.spec<arg-1>(/runner/_work/enso/enso/test/Table_Tests/src/In_Memory/Join_Performance_Spec.enso:18:19-39)
      at <enso> Test.run_spec<arg-1>(/runner/_work/enso/enso/built-distribution/enso-engine-2023.2.1-dev-linux-amd64/enso-2023.2.1-dev/lib/Standard/Test/2023.2.1-dev/src/Test.enso:196:18-25)
      at <enso> Panic.catch(Internal)

  - [FAILED] should efficiently compute equality joins mixed with other secondary conditions [0ms]
      Reason: An unexpected panic was thrown: (No_Such_Method.Error 0 UnresolvedSymbol<up_to>)
      at <enso> Join_Performance_Spec.spec<arg-0>(/runner/_work/enso/enso/test/Table_Tests/src/In_Memory/Join_Performance_Spec.enso:40:19-27)
      at <enso> Join_Performance_Spec.spec<arg-1>(/runner/_work/enso/enso/test/Table_Tests/src/In_Memory/Join_Performance_Spec.enso:40:19-39)
      at <enso> Test.run_spec<arg-1>(/runner/_work/enso/enso/built-distribution/enso-engine-2023.2.1-dev-linux-amd64/enso-2023.2.1-dev/lib/Standard/Test/2023.2.1-dev/src/Test.enso:196:18-25)
      at <enso> Panic.catch(Internal)

  - [FAILED] should efficiently compute case-insensitive equality joins [0ms]
      Reason: An unexpected panic was thrown: (No_Such_Method.Error 0 UnresolvedSymbol<up_to>)
      at <enso> Join_Performance_Spec.spec<arg-0>(/runner/_work/enso/enso/test/Table_Tests/src/In_Memory/Join_Performance_Spec.enso:67:22-30)
      at <enso> Join_Performance_Spec.spec<arg-1>(/runner/_work/enso/enso/test/Table_Tests/src/In_Memory/Join_Performance_Spec.enso:67:22-65)
      at <enso> Test.run_spec<arg-1>(/runner/_work/enso/enso/built-distribution/enso-engine-2023.2.1-dev-linux-amd64/enso-2023.2.1-dev/lib/Standard/Test/2023.2.1-dev/src/Test.enso:196:18-25)
      at <enso> Panic.catch(Internal)

grep:   - [FAILED] should fail to append to JSON non-arrays [1ms]
/tmp/log: binary file matches
      Reason: An unexpected panic was thrown: (No_Such_Method.Error '1' UnresolvedSymbol<write>)
      at <enso> Formats_Spec.spec<arg-1>(/runner/_work/enso/enso/test/Table_Tests/src/IO/Formats_Spec.enso:60:9-21)
      at <enso> Test.run_spec<arg-1>(/runner/_work/enso/enso/built-distribution/enso-engine-2023.2.1-dev-linux-amd64/enso-2023.2.1-dev/lib/Standard/Test/2023.2.1-dev/src/Test.enso:196:18-25)
      at <enso> Panic.catch(Internal)
      at <enso> <anonymous>(/runner/_work/enso/enso/built-distribution/enso-engine-2023.2.1-dev-linux-amd64/enso-2023.2.1-dev/lib/Standard/Base/2023.2.1-dev/src/Panic.enso:186-188)

  - [FAILED] should write to a temporary CSV file part of the data if context disabled [0ms]
      Reason: An unexpected panic was thrown: (No_Such_Method.Error 1 UnresolvedSymbol<up_to>)
      at <enso> write_test<arg-0>(/runner/_work/enso/enso/test/Table_Tests/src/IO/Formats_Spec.enso:95:38-49)
      at <enso> write_test<arg-1>(/runner/_work/enso/enso/test/Table_Tests/src/IO/Formats_Spec.enso:95:38-61)
      at <enso> write_test(/runner/_work/enso/enso/test/Table_Tests/src/IO/Formats_Spec.enso:95:21-63)
      at <enso> Formats_Spec.spec<arg-1>(/runner/_work/enso/enso/test/Table_Tests/src/IO/Formats_Spec.enso:120:9-24)

  - [FAILED] should write to a temporary JSON file part of the data if context disabled [0ms]
      Reason: An unexpected panic was thrown: (No_Such_Method.Error 1 UnresolvedSymbol<up_to>)
      at <enso> write_test<arg-0>(/runner/_work/enso/enso/test/Table_Tests/src/IO/Formats_Spec.enso:95:38-49)
      at <enso> write_test<arg-1>(/runner/_work/enso/enso/test/Table_Tests/src/IO/Formats_Spec.enso:95:38-61)
      at <enso> write_test(/runner/_work/enso/enso/test/Table_Tests/src/IO/Formats_Spec.enso:95:21-63)
      at <enso> Formats_Spec.spec<arg-1>(/runner/_work/enso/enso/test/Table_Tests/src/IO/Formats_Spec.enso:123:9-25)

  - [FAILED] should make a best-effort attempt at returning a reasonable error for the short-hand [21ms]
      Reason: An unexpected panic was thrown: (No_Such_Method.Error 'The name NONEXISTENT-TABLE was treated as a query, but the query failed with the following error: There was an SQL error: [SQLITE_ERROR] SQL error or missing database (near "NONEXISTENT": syntax error). [Query was: NONEXISTENT-TABLE]; if you want to force to use that as a table name, wrap it in `SQL_Query.Table_Name`.' UnresolvedSymbol<starts_with>)
      at <enso> Common_Spec.run_tests<arg-0>(/runner/_work/enso/enso/test/Table_Tests/src/Database/Common_Spec.enso:131:17-112)
      at <enso> Common_Spec.run_tests<arg-1>(/runner/_work/enso/enso/test/Table_Tests/src/Database/Common_Spec.enso:131:17-129)
      at <enso> Test.with_clue(/runner/_work/enso/enso/built-distribution/enso-engine-2023.2.1-dev-linux-amd64/enso-2023.2.1-dev/lib/Standard/Test/2023.2.1-dev/src/Test.enso:183:18-25)
      at <enso> Common_Spec.run_tests<arg-1>(/runner/_work/enso/enso/test/Table_Tests/src/Database/Common_Spec.enso:130-132)

  - [FAILED] should not create any table if upload fails [0ms]
      Reason: An unexpected panic was thrown: (No_Such_Method.Error 100 UnresolvedSymbol<up_to>)
      at <enso> Upload_Spec.spec<arg-0>(/runner/_work/enso/enso/test/Table_Tests/src/Database/Upload_Spec.enso:207:53-76)
      at <enso> Upload_Spec.spec<arg-2>(/runner/_work/enso/enso/test/Table_Tests/src/Database/Upload_Spec.enso:207:52-87)
      at <enso> Upload_Spec.spec<arg-1>(/runner/_work/enso/enso/test/Table_Tests/src/Database/Upload_Spec.enso:207:29-87)
      at <enso> Test.run_spec<arg-1>(/runner/_work/enso/enso/built-distribution/enso-engine-2023.2.1-dev-linux-amd64/enso-2023.2.1-dev/lib/Standard/Test/2023.2.1-dev/src/Test.enso:196:18-25)

  - [FAILED] should be able to persist a complex query with generated columns, joins etc. [21ms]
      Reason: An unexpected panic was thrown: (No_Such_Method.Error 'SELECT "copied-table-9dc8cf00-0d9c-47c5-ba81-eefcb6638313"."X" AS "X", "copied-table-9dc8cf00-0d9c-47c5-ba81-eefcb6638313"."Y" AS "Y", "copied-table-9dc8cf00-0d9c-47c5-ba81-eefcb6638313"."C1" AS "C1", "copied-table-9dc8cf00-0d9c-47c5-ba81-eefcb6638313"."C2" AS "C2", "copied-table-9dc8cf00-0d9c-47c5-ba81-eefcb6638313"."Right X" AS "Right X", "copied-table-9dc8cf00-0d9c-47c5-ba81-eefcb6638313"."C3" AS "C3" FROM "copied-table-9dc8cf00-0d9c-47c5-ba81-eefcb6638313" AS "copied-table-9dc8cf00-0d9c-47c5-ba81-eefcb6638313"' UnresolvedSymbol<contains>)
      at <enso> Upload_Spec.spec<arg-0>(/runner/_work/enso/enso/test/Table_Tests/src/Database/Upload_Spec.enso:310:17-36)
      at <enso> Upload_Spec.spec<arg-1>(/runner/_work/enso/enso/test/Table_Tests/src/Database/Upload_Spec.enso:310:17-54)
      at <enso> Test.with_clue(/runner/_work/enso/enso/built-distribution/enso-engine-2023.2.1-dev-linux-amd64/enso-2023.2.1-dev/lib/Standard/Test/2023.2.1-dev/src/Test.enso:183:18-25)
      at <enso> Upload_Spec.spec<arg-1>(/runner/_work/enso/enso/test/Table_Tests/src/Database/Upload_Spec.enso:309-312)

Let's see what's the status after deab523

@JaroslavTulach JaroslavTulach added the CI: Clean build required CI runners will be cleaned before and after this PR is built. label Jun 16, 2023
@JaroslavTulach
Copy link
Member Author

  • Method select_into_database_table not found here.
  • Method split of Text not found here

@JaroslavTulach
Copy link
Member Author

JaroslavTulach commented Jun 17, 2023

This draft PR proof of concept is green. However its core - the ImportAPIAnalysis pass is a "throw away".

The general preference seems to be a private method keyword (or priv or anything that is inside of the module and next to the element which is not supposed to be accessible). Something like:

private

## This adds two numbers
private
add a b = a + b

or

private

## This adds two numbers
private add a b = a + b

To implement the private keyword approach, I'd like us to stick with static info as much as possible and avoid runtime checking until necessary. Can we have two ModuleScopes? One public one spiced with private elements? If the call site is in the library the (one time) lookup uses private ModuleScope. Otherwise it uses public ModuleScope. When the lookup is done, the result is cached. Fast path execution speed shall remain the same.

@farmaazon farmaazon deleted the wip/jtulach/ImportExportWithMain branch May 9, 2024 07:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI: Clean build required CI runners will be cleaned before and after this PR is built.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants