Commit Graph
6 Commits
Author SHA1 Message Date
odenixandDan Chao 52a86d3f32 Untangle external reader code (#776)
- move the following classes into package externalreader:
  - ExternalModuleResolver
  - ExternalResourceResolver
  - MessageTransportModuleResolver (renamed to ExternalModuleResolverImpl, made package-private)
  - MessageTransportResourceResolver (renamed to ExternalResourceResolverImpl, made package-private)
- replace interface ExternalModuleResolver.Spec with record ExternalModuleReaderSpec
- replace interface ExternalResourceResolver.Spec with record ExternalResourceReaderSpec
- translate between messaging.ResourceReaderSpec and ExternalResourceReaderSpec (eliminates dependency from messaging on higher layer)
- translate between messaging.ResourceResolverSpec and ExternalResourceResolverSpec (eliminates dependency from messaging on higher layer)
- add ServerMessages.ExternalReader and translate between this message component and the PklEvaluatorSettings.ExternalReader API
- add ServerMessages.Proxy and translate between this message component and the PklEvaluatorSettings.Proxy API
- change type of CreateEvaluatorRequest.allowedModules/allowedResources from List<Pattern>? to List<String>?
  - removes a lot of code
  - should not need to create a Pattern object to send a message
- deprecate method evaluatorSettings.PklEvaluatorSettings.Proxy.create()
  - only seems useful internally, inlined

Co-authored-by: Dan Chao <dan.chao@apple.com>
2025-02-20 22:38:51 -08:00
odenix 90df0662af Overhaul implementation of for-generators (#844)
Motivation:
* fix known bugs and limitations of for-generators
* improve code health by removing complex workarounds

Changes:
* simplify AstBuilder code related to for-generators
  * track for-generators via `SymbolTable.enterForGenerator()`
  * add `RestoreForBindingsNode` during initial AST construction
    instead of calling `MemberNode.replaceBody()` later on
  * simplify some unnecessarily complex code
* remove workarounds and band-aids such as:
  * `isInIterable`
  * `executeAndSetEagerly`
  * adding dummy slots in `AmendFunctionNode`
* overhaul implementation of for-generators
  * store keys and values of for-generator iterations in regular instead of auxiliary frame slots
    * set them via `TypeNode.executeAndSet()`
    * `ResolveVariableNode` no longer needs to search auxiliary slots
    * `Read(Enclosing)AuxiliarySlot` is no longer needed
  * at the start of each for-generator iteration, create a new `VirtualFrame`
    that is a copy of the current frame (arguments + slots)
    and stores the iteration key and value in additional slots.
  * execute for-generator iteration with the newly created frame
    * `childNode.execute(newFrame)`
    * Pkl objects created during the iteration will materialize this frame
  * store newly created frames in `owner.extraStorage`
    if their for-generator slots may be accessed when a generated member is executed
    * resolving variable names to for-generator variables at parse time would make this analysis more precise
  * when a generated member is executed,
	  * retrieve the corresponding frame stored in `owner.extraStorage`
	  * copy the retrieved frame's for-generator slots into slots of the current frame

Result:
* for-generators are implemented in a correct, reasonably simple, and reasonably efficient way
  * complexity is fully contained within package `generator` and `AstBuilder`
* for-generator keys and values can be accessed from all nested scopes:
  * key and value expressions of generated members
  * condition expressions of nested when-generators
  * iterable expressions of nested for-generators
* for-generator keys and values can be accessed from within objects created by the expressions listed above
* sibling for-generators can use the same key/value variable names
* parent/child for-generators can use the same key/value variable names
* fixes https://github.com/apple/pkl/issues/741
2025-01-28 14:06:42 -08:00
odenix 258eda8630 Update Kotlin to 2.0 (#900)
- update Kotlin from 1.7.10 to 2.0.21
  - Kotlin 1.6 dependencies in Gradle lock files are expected because kotlinc,
    which is also used by some tests, internally uses some 1.6 dependencies
    for backwards compatibility reasons.
- update kotlinx-html and kotlinx-serialization
- adapt Kotlin code where necessary
- use Kotlin stdlib Path APIs where possible
- fix IntelliJ Kotlin inspection warnings
- reformat code with `./gradlew spotlessApply`
  - ktfmt adds lots of trailing commas
- Add workaround to fix IntelliJ "unresolved reference" errors
2025-01-23 14:41:59 -08:00
odenix 17f431370a Fix exception handling of PklRootNode's (#837)
Motivation:
- Perform same exception handling for every implementation of PklRootNode.execute().
- Avoid code duplication.

Changes:
- Change PklRootNode.execute() to be a final method that performs exception handling
  and calls abstract method executeImpl(), which is implemented by subclasses.
- Remove executeBody() methods, which served a similar purpose but were more limited.
- Remove duplicate exception handling code.

Result:
- More reliable exception handling.
  This should fix known problems such as misclassifying stack overflows
  as internal errors and displaying errors without a stack trace.
- Less code duplication.
2024-12-19 09:58:44 -08:00
odenix 01bf844a96 codegen-java: Support not annotating constructor parameters (#792)
Motivation:
Spring Boot configuration classes neither require nor benefit from annotating constructor parameters with their name.
The same is true for pkl-config-java configuration classes compiled with `-parameter`.

Changes:
- Change CLI parameter `--params-annotation` to accept a `none` value.
  This is recommended in https://clig.dev/#arguments-and-flags and is how the `-F` parameter of the `ssh` command works.
- Change `paramsAnnotation` property in Gradle plugin, CliJavaCodeGeneratorOptions, and JavaCodegenOptions as follows:
  - Change meaning of `null` from "generate org.pkl.java.config.mapper.Named annotations" to "do not generate annotations".
    This is a breaking change (only) affecting users who explicitly set the property to `null` instead of omitting it.
  - Change property default from `null` to:
    `null` if `generateSpringBootConfig` is `true` and `org.pkl.java.config.mapper.Named` otherwise
- add tests
- update docs of this and other codegen options

Result:
Generated code does not contain unnecessary annotations.
2024-12-13 14:29:18 -08:00
odenix 1bc473ba54 Improve lazy type checking of listings and mappings (#789)
Motivation:
- simplify implementation of lazy type checking
- fix correctness issues of lazy type checking (#785)

Changes:
- implement listing/mapping type cast via amendment (`parent`) instead of delegation (`delegate`)
- handle type checking of *computed* elements/entries in the same way as type checking of computed properties
  - ElementOrEntryNode is the equivalent of TypeCheckedPropertyNode
- remove fields VmListingOrMapping.delegate/typeNodeFrame/cachedMembers/checkedMembers
- fix #785 by executing all type casts between a member's owner and receiver
- fix #823 by storing owner and receiver directly
  instead of storing the mutable frame containing them (typeNodeFrame)
- remove overrides of VmObject methods that are no longer required
  - good for Truffle partial evaluation and JVM inlining
- revert a85a173faa except for added tests
- move `VmUtils.setOwner` and `VmUtils.setReceiver` and make them private
  - these methods aren't generally safe to use

Result:
- simpler code with greater optimization potential
  - VmListingOrMapping can now have both a type node and new members
- fewer changes to surrounding code
- smaller memory footprint
- better performance in some cases
- fixes https://github.com/apple/pkl/issues/785
- fixes https://github.com/apple/pkl/issues/823

Potential future optimizations:
- avoid lazy type checking overhead for untyped listings/mappings
- improve efficiency of forcing a typed listing/mapping
  - currently, lazy type checking will traverse the parent chain once per member,
    reducing the performance benefit of shallow-forcing
	  a listing/mapping over evaluating each member individually
- avoid creating an intermediate untyped listing/mapping in the following cases:
  - `new Listing<X> {...}`
  - amendment of `property: Listing<X>`
2024-12-06 13:41:33 +01:00