diff --git a/FIXME_TODO.txt b/FIXME_TODO.txt new file mode 100644 index 0000000..ce96d19 --- /dev/null +++ b/FIXME_TODO.txt @@ -0,0 +1,143 @@ +FIXME triage and plan +====================== +Source: every `# FIXME` comment in the tree, grouped and prioritized by blast +radius -- items that reshape the most other items come first. + +Status key: [ ] todo [~] in progress [x] done [-] wont-do/moot + +------------------------------------------------------------------------------ +STATUS (as of end of T1.3) + DONE: T1.1, T1.2, T1.3 (all of Tier 1) + T4.3. + NEXT: >>> T2.1 (shadowing policy) <<< -- needs a decision, see below. + Open FIXMEs remaining in tree: 21. + Baseline: full suite 1511 passed, 3 skipped, 1 xpassed; black + spec-links clean. + Line numbers below are CURRENT (re-grep `# FIXME` if they drift after edits). +------------------------------------------------------------------------------ + + +============================================================================== +TIER 1 -- structural, reshapes everything downstream [ALL DONE] +============================================================================== + +[x] T1.1 Collapse the 3 parallel symbol tables (type/callable/value) into ONE + symbols.Scope holding 3 name-group dicts. Stop passing 3 tables around. + - global_/base_/enclosing_ *_scope -> main_scope / base_scope / + enclosing_scope (single Scope objects); SequenceContext.{callable, + value}_scope -> one scope. + - Left SymbolTable as the flat ModuleSymbol (Tier 3 formalizes it). + - flags_var moved into base_scope; its main-frame slot reserved + explicitly in CalculateFrameSizes.run. + Removed FIXMEs: base/global naming x2, weird docstring, unused-callable- + scope, store-scopes-in-enclosing. + +[x] T1.2 Rebuild the compilation unit cleanly + centralize scope creation. + Step1 killed the body.stmts "switcheroo" (fresh library root + main + block; codegen entry points dropped the `body` param); renamed + program_block -> main_block. + Step2 renamed global_scope -> main_scope. + Step3 deleted sequence_root_scopes; is_root == `scope is main_scope`. + Step4 made CreateScopes the SOLE scope creator (only special case: the + root owns base_scope). + Removed FIXMEs: switcheroo, state.root-misleading, splice, rename, + confusing comment, container special-case, sequence_root_scopes. + +[x] T1.3 Remove container_sequence + sequence_of (and own_definitions/ + get_definition). + - SequenceContext now holds `block` (not `scope`); scope derived via + state.enclosing_scope[block]. + - BindImports resolves importer/target Scopes from blocks; passes + Scopes into _bind_symbol/_bind_module. own_definitions/get_definition + DELETED -> one Scope.own_symbols(). + - star_underscore_names moved onto Scope; underscore warning walks the + scope chain to the root scope (parent is base_scope). sequence_of + DELETED. + - _build_compilation_unit reuses `program` as main_block; dropped the + library SequenceContext. + - state.root -> state.root_block. + Removed FIXMEs: 7. + + +============================================================================== +TIER 2 -- behavioral policy (touches spec + tests; needs a decision) +============================================================================== + +>>> NEXT UP <<< +[ ] T2.1 Settle the shadowing / name-collision rule across all THREE sites, + consistently. DECISION NEEDED before implementing (it changes SPEC.md + + tests, not just code). + + The three FIXME sites: + - semantics.py:297 DefineFunctions.visit_AstDef -- function + redefinition. + - semantics.py:364 DefineVariables.define_variable -- variable + redeclaration. + - imports.py:552 BindImports._bind_module -- import binding over an + existing name. + + Current (INCONSISTENT) behavior: + - Functions: redefinition is an ERROR, checked up the WHOLE chain + (cannot even shadow a dictionary/builtin callable). + - Variables: same-scope redeclaration is an ERROR; but shadowing an + OUTER/global name from an inner block is SILENTLY ALLOWED; at the + sequence root, cannot shadow base (dictionary) names. + - Imports: binding over an existing name is an ERROR. + + The decision to make: pick ONE uniform policy -- e.g. (a) all ERROR + (no shadowing anywhere), (b) all WARN (allow, but warn), or (c) a + defined split (e.g. inner-block shadowing warns; same-scope + base/dict + collisions error). Then apply it to all three sites + update SPEC.md + + tests. + + >> When resuming: ASK THE USER which policy before coding. << + + +============================================================================== +TIER 3 -- module modeling +============================================================================== + +[ ] T3.1 symbols.py:131 -- replace the is_sequence_module bool on SymbolTable + with a real Module class. Then, built on it: + - imports.py:379 simplify BindImports._bind (imagine an explicit IR: + "from file import symbol as path"; delete helpers). + - semantics.py:1623 explain / stop using parent_map in the + module-used-as-a-value check; point it at a test. + + +============================================================================== +TIER 4 -- localized correctness / investigations (independent, low ripple) +============================================================================== + +[ ] T4.1 desugaring.py:748 -- _make_func_call should look up in the enclosing + callable scope (correctness). Currently main_scope.lookup, which is + fine for the builtins it targets; revisit for correctness. +[ ] T4.2 test_imports.py:2163 -- verify/add a test that relative imports INSIDE + an imported lib resolve relative to THAT lib, not the importer + (possible bug). +[x] T4.3 delete sequence_of() entirely [done in T1.3] +[ ] T4.4 imports.py:275 -- is the `i != 0` sequence()-metadata check already + caught by another semantic pass? if so, dedupe. +[ ] T4.5 imports.py:313 + imports.py:315 -- do we actually forbid duplicate + search dirs? + do a brief TOCTOU analysis of file resolution. +[ ] T4.6 imports.py:143 -- assign node ids at parse time instead of in the + semantics passes (would remove _ensure_id). +[ ] T4.7 imports.py:100 -- split InlineImports into separate resolve vs inline + passes (deferred from T1.2). +[ ] T4.8 imports.py:126 -- confirm it's safe to early-return from _inline_stmts + on state.errors without signalling further (deferred from T1.2). + + +============================================================================== +TIER 5 -- cosmetic / cleanup (last, near-zero ripple) +============================================================================== + +[ ] T5.1 codegen_fpybc.py:192 -- explain the main-block fresh-frame logic in + CalculateFrameSizes.visit_AstBlock (seems weird / wants a comment). +[ ] T5.2 compiler.py:153 -- _build_compilation_unit docstring is too long/verbose; + trim it. +[ ] T5.3 imports.py:162 -- add a type annotation for import_stack. +[ ] T5.4 imports.py:164 -- better circular-import error msg (show the cycle). +[ ] T5.5 imports.py:262 -- fix the misleading "not top-level statements" wording + (AstDef/AstImport/AstSequenceMetadata ARE top-level statements). +[ ] T5.6 imports.py:305 -- use pathlib instead of os.path.dirname? (safety). +[ ] T5.7 test_imports.py:2107 -- are the TestImportModuleMerging tests duplicates + of existing coverage? diff --git a/SEQ_ARGS.md b/SEQ_ARGS.md new file mode 100644 index 0000000..83b39c5 --- /dev/null +++ b/SEQ_ARGS.md @@ -0,0 +1,212 @@ +> **WARNING:** The Fpy specification is a work-in-progress + +# Fpy Sequence Arguments and Sequence Calling + +This document specifies how an Fpy sequence declares arguments, how argument values are bound when a sequence starts, and how a running sequence starts another sequence. It is a companion to the main [Fpy specification](SPEC.md) and follows its conventions. Terms such as [variable](SPEC.md#variables), [type](SPEC.md#types), [coercion](SPEC.md#type-conversion), [name group](SPEC.md#name-groups) and [command](SPEC.md#commands) are defined there. + +The last section, [Design notes](#design-notes), is non-normative. + +# Sequences and the environment + +A **sequence** is a single Fpy program, compiled as a unit. + +The **environment** is whatever starts a sequence running: a ground operator, an F-Prime component, or another sequence via a [sequence-run command](#sequence-run-commands). + +Each sequence has an **argument specification**: an ordered list of name-and-[type](SPEC.md#types) pairs, declared by its [sequence statement](#the-sequence-statement). A sequence with no sequence statement, or with an empty one, has an empty argument specification. + +The compiled form of a sequence records its argument specification as an ordered list of triples (name, fully-qualified type name, size), where **size** is the length in bytes of the binary form of the type. + +> The compiled form is otherwise left unspecified. Recording the argument specification in the compiled sequence is what allows type-checked calls between separately compiled sequences. + +# Sequence arguments + +A **sequence argument** is a [variable](SPEC.md#variables) implicitly defined by the sequence statement, whose initial value is supplied by the environment when the sequence starts. + +## The sequence statement + +A **sequence statement** declares the argument specification of the sequence. + +### Syntax + +Rule: + +``` +sequence_stmt: "sequence" "(" [sequence_parameters] ")" +sequence_parameters: sequence_parameter ("," sequence_parameter)* [","] +sequence_parameter: name ":" qualified_name +``` + +Name: + +``` +sequence_stmt: "sequence" "(" parameters ")" +sequence_parameter: parameter_name ":" parameter_type +``` + +A sequence statement is only valid outside an indentation block. + +*Tests:* [1](test/fpy/test_sequence_metadata.py#L301 "test/fpy/test_sequence_metadata.py::test_defining_sequence_in_function"), [2](test/fpy/test_sequence_metadata.py#L310 "test/fpy/test_sequence_metadata.py::test_defining_sequence_in_loop"), [3](test/fpy/test_sequence_metadata.py#L319 "test/fpy/test_sequence_metadata.py::test_defining_sequence_in_if_stmt") + +If a sequence statement is present, it must be the first statement of the sequence. + +> This implies there is at most one sequence statement per sequence. + +*Tests:* [1](test/fpy/test_sequence_metadata.py#L140 "test/fpy/test_sequence_metadata.py::test_sequence_after_statement"), [2](test/fpy/test_sequence_metadata.py#L105 "test/fpy/test_sequence_metadata.py::test_duplicate_sequence_statement"), [3](test/fpy/test_sequence_metadata.py#L24 "test/fpy/test_sequence_metadata.py::test_empty_sequence"), [4](test/fpy/test_sequence_metadata.py#L48 "test/fpy/test_sequence_metadata.py::test_sequence_with_trailing_comma") + +Each `parameter_name` is resolved in the value name group. Each `parameter_type` is resolved in the type name group. + +*Tests:* [1](test/fpy/test_sequence_metadata.py#L132 "test/fpy/test_sequence_metadata.py::test_sequence_with_invalid_type"), [2](test/fpy/test_sequence_metadata.py#L227 "test/fpy/test_sequence_metadata.py::test_sequence_literal_as_type"), [3](test/fpy/test_sequence_metadata.py#L235 "test/fpy/test_sequence_metadata.py::test_sequence_bool_as_type") + +### Semantics + +Each parameter defines a variable named `parameter_name` with type `parameter_type` in the global scope, exactly as if by a [variable definition statement](SPEC.md#variable-definition), except: +* the variable is considered defined starting from the first statement of the sequence, and +* its initial value is supplied by the environment, per [argument binding](#argument-binding). + +> In all other respects, sequence arguments are ordinary variables: they may be read, reassigned, passed to functions, and shadowed in inner scopes, and they occupy only the value name group, so they never conflict with types or callables. + +*Tests:* [1](test/fpy/test_sequence_metadata.py#L56 "test/fpy/test_sequence_metadata.py::test_sequence_parameter_as_variable"), [2](test/fpy/test_sequence_metadata.py#L556 "test/fpy/test_sequence_metadata.py::test_modify_arg"), [3](test/fpy/test_sequence_metadata.py#L170 "test/fpy/test_sequence_metadata.py::test_sequence_parameter_in_function"), [4](test/fpy/test_sequence_metadata.py#L269 "test/fpy/test_sequence_metadata.py::test_sequence_param_same_name_as_func"), [5](test/fpy/test_sequence_metadata.py#L280 "test/fpy/test_sequence_metadata.py::test_sequence_param_shadowed_by_loop_var"), [6](test/fpy/test_sequence_metadata.py#L290 "test/fpy/test_sequence_metadata.py::test_sequence_param_shadowed_by_func_param") + +Because each parameter is a variable definition in the global scope, no two parameters may share a name, and no other global variable definition may share a name with a parameter. + +*Tests:* [1](test/fpy/test_sequence_metadata.py#L114 "test/fpy/test_sequence_metadata.py::test_duplicate_parameter_names"), [2](test/fpy/test_sequence_metadata.py#L122 "test/fpy/test_sequence_metadata.py::test_sequence_parameter_conflicts_with_variable") + +If `parameter_type` is not [constant-sized](SPEC.md#types), an error is raised. + +*Tests:* [1](test/fpy/test_sequence_metadata.py#L243 "test/fpy/test_sequence_metadata.py::test_sequence_string_type_parameter"), [2](test/fpy/test_sequence_metadata.py#L251 "test/fpy/test_sequence_metadata.py::test_sequence_struct_with_string_member"), [3](test/fpy/test_sequence_metadata.py#L87 "test/fpy/test_sequence_metadata.py::test_sequence_with_struct_type"), [4](test/fpy/test_sequence_metadata.py#L96 "test/fpy/test_sequence_metadata.py::test_sequence_with_array_type"), [5](test/fpy/test_sequence_metadata.py#L183 "test/fpy/test_sequence_metadata.py::test_sequence_with_enum_type") + +A sequence argument is not a [constant expression](SPEC.md#expressions). + +> For example, a sequence argument cannot be the default value of a function parameter. + +*Tests:* [1](test/fpy/test_sequence_metadata.py#L259 "test/fpy/test_sequence_metadata.py::test_sequence_param_as_default_arg") + +If a sequence statement has more than 255 parameters, an error is raised. + +If a parameter's name, or the fully-qualified name of a parameter's type, is longer than 255 UTF-8 bytes, an error is raised. + +> These limits let the compiled argument specification store the count in one byte and each name with a one-byte length prefix. + +*Tests:* [1](test/fpy/test_sequence_metadata.py#L584 "test/fpy/test_sequence_metadata.py::test_too_many_parameters"), [2](test/fpy/test_seq_calling.py#L649 "test/fpy/test_seq_calling.py::TestSeqArgLimits::test_arg_name_too_long"), [3](test/fpy/test_seq_calling.py#L658 "test/fpy/test_seq_calling.py::TestSeqArgLimits::test_arg_name_exactly_255_bytes") + +The argument specification of the sequence is the ordered list of (`parameter_name`, `parameter_type`) pairs. + +If the argument specification of a sequence is non-empty, that sequence cannot be [imported](SPEC.md#imports). + +## Argument binding + +To start a sequence, the environment supplies an **argument buffer**: the binary forms of one value per entry of the argument specification, in order, concatenated. + +Before the first statement of the sequence executes: +1. If the length of the supplied argument buffer is not equal to the sum of the sizes of the argument specification, the sequence fails to start, and no statement executes. +2. Otherwise, each sequence argument's initial value is the value of its declared type whose binary form is the corresponding slice of the argument buffer. + +> The buffer is validated only by total length. The environment is trusted to supply well-formed values of the declared types; there is no per-value check. + +*Tests:* [1](test/fpy/test_sequence_metadata.py#L384 "test/fpy/test_sequence_metadata.py::test_arg_value_u32"), [2](test/fpy/test_sequence_metadata.py#L448 "test/fpy/test_sequence_metadata.py::test_multiple_args_correct_offsets"), [3](test/fpy/test_sequence_metadata.py#L354 "test/fpy/test_sequence_metadata.py::test_run_sequence_no_args_expected_none_provided"), [4](test/fpy/test_sequence_metadata.py#L363 "test/fpy/test_sequence_metadata.py::test_run_sequence_args_wrong_size"), [5](test/fpy/test_sequence_metadata.py#L373 "test/fpy/test_sequence_metadata.py::test_run_sequence_args_expected_but_missing") + +# Sequence calling + +A sequence starts another sequence by calling a sequence-run command. The called sequence is the **target**; its compiled form is the **target binary**. + +## The Svc.SeqArgs type + +The dictionary must define the type `Svc.SeqArgs` as a struct with exactly two members, in order: +1. `size`: an unsigned integer type +2. `buffer`: an array of `U8` with positive length + +The **argument buffer capacity** is the length of `buffer`. + +> `Svc.SeqArgs` carries an argument buffer inside a command. Its capacity is set per-deployment in the dictionary; the compiler adopts whatever capacity the dictionary declares. + +*Tests:* [1](test/fpy/test_seq_calling.py#L700 "test/fpy/test_seq_calling.py::TestSeqArgsBufferSizeFromDictionary::test_non_default_buffer_size_loads_cleanly") + +## Sequence-run commands + +A **sequence-run command** is a command whose F-Prime parameters are, in order, exactly: +1. a string type +2. `Svc.BlockState` +3. `Svc.SeqArgs` + +> In the reference FpySequencer this is the `RUN_ARGS` command, with parameters `fileName`, `block`, `buffer`. Any command matching the shape is treated as a sequence-run command; commands that do not match (such as `RUN` or `VALIDATE_ARGS`) are ordinary commands. + +*Tests:* [1](test/fpy/test_seq_calling.py#L52 "test/fpy/test_seq_calling.py::TestSeqRunDetection::test_run_args_detected_as_seq_run"), [2](test/fpy/test_seq_calling.py#L70 "test/fpy/test_seq_calling.py::TestSeqRunDetection::test_regular_run_not_seq_run") + +The Fpy [callable](SPEC.md#callables) corresponding to a sequence-run command does not take the `Svc.SeqArgs` parameter. Its parameters are the first two F-Prime parameters (name it `file_name` and `block`), followed by the parameters of the target's argument specification, in order and under their declared names. + +> The caller passes the target's arguments directly, by position or by name, as if calling a function with the target's signature: `Ref.seqDisp.RUN_ARGS("child.bin", Svc.BlockState.BLOCK, x=42)`. + +*Tests:* [1](test/fpy/test_seq_calling.py#L87 "test/fpy/test_seq_calling.py::TestSeqCallingNoArgs::test_call_child_no_args"), [2](test/fpy/test_seq_calling.py#L105 "test/fpy/test_seq_calling.py::TestSeqCallingWithArgs::test_call_child_one_u32_arg"), [3](test/fpy/test_seq_calling.py#L120 "test/fpy/test_seq_calling.py::TestSeqCallingWithArgs::test_call_child_multiple_args"), [4](test/fpy/test_seq_calling.py#L446 "test/fpy/test_seq_calling.py::TestSeqCallingNamedArgs::test_single_named_arg"), [5](test/fpy/test_seq_calling.py#L481 "test/fpy/test_seq_calling.py::TestSeqCallingNamedArgs::test_named_args_reordered"), [6](test/fpy/test_seq_calling.py#L499 "test/fpy/test_seq_calling.py::TestSeqCallingNamedArgs::test_mixed_positional_and_named") + +## Target resolution + +The `file_name` argument of a sequence-run command call must be a string literal; otherwise an error is raised. Its value is both the path at which the running F-Prime system will load the target binary, and the key by which the compiler locates a copy of the target binary. + +The **ground binary directory** is a directory provided by the environment in which the compiler is invoked. + +> In the command-line compiler, this is the `-g`/`--ground-binary-dir` option, defaulting to the directory containing the input sequence. + +For each call to a sequence-run command: +1. If no ground binary directory was provided, an error is raised. +2. The `file_name` value is joined to the ground binary directory. If no file exists at the resulting path, an error is raised. +3. The argument specification recorded in that file is read. If the file cannot be read as a compiled sequence, an error is raised. +4. Each type name in the argument specification is resolved to a type. If a type name does not name a known type, or the recorded size differs from the size of the resolved type's binary form, an error is raised. + +*Tests:* [1](test/fpy/test_seq_calling.py#L275 "test/fpy/test_seq_calling.py::TestSeqCallingErrors::test_missing_bin_file"), [2](test/fpy/test_seq_calling.py#L286 "test/fpy/test_seq_calling.py::TestSeqCallingErrors::test_no_binary_dir") + +> The compiler checks the call against a ground copy of the target binary; the running system loads its own copy by the same name. Nothing verifies that the two copies are identical. If they disagree, the mismatch is caught at run time only if the total argument size differs (see [argument binding](#argument-binding)). + +## Call checking + +The call is checked as an ordinary [function call expression](SPEC.md#function-call-expression) against the parameter list defined in [sequence-run commands](#sequence-run-commands): arguments may be positional or named, every parameter must be supplied exactly once, and no unknown names may be supplied. Each argument for a target parameter must be [coercible](SPEC.md#type-conversion) to that parameter's declared type; otherwise an error is raised. + +*Tests:* [1](test/fpy/test_seq_calling.py#L238 "test/fpy/test_seq_calling.py::TestSeqCallingErrors::test_wrong_arg_count"), [2](test/fpy/test_seq_calling.py#L258 "test/fpy/test_seq_calling.py::TestSeqCallingErrors::test_wrong_arg_type"), [3](test/fpy/test_seq_calling.py#L557 "test/fpy/test_seq_calling.py::TestSeqCallingNamedArgErrors::test_unknown_named_arg"), [4](test/fpy/test_seq_calling.py#L579 "test/fpy/test_seq_calling.py::TestSeqCallingNamedArgErrors::test_duplicate_named_arg"), [5](test/fpy/test_seq_calling.py#L601 "test/fpy/test_seq_calling.py::TestSeqCallingNamedArgErrors::test_positional_and_named_conflict"), [6](test/fpy/test_seq_calling.py#L623 "test/fpy/test_seq_calling.py::TestSeqCallingNamedArgErrors::test_missing_named_arg") + +If the sum of the sizes of the target's argument specification exceeds the argument buffer capacity, an error is raised. + +*Tests:* [1](test/fpy/test_seq_calling.py#L712 "test/fpy/test_seq_calling.py::TestSeqArgsBufferSizeFromDictionary::test_oversized_args_use_dictionary_capacity"), [2](test/fpy/test_seq_calling.py#L731 "test/fpy/test_seq_calling.py::TestSeqArgsBufferSizeFromDictionary::test_args_still_bounded_by_dictionary_capacity") + +## Evaluation + +A sequence-run command call is evaluated per [command evaluation](SPEC.md#command-evaluation), with the underlying F-Prime command's third argument constructed as the `Svc.SeqArgs` value whose: +* `size` is the sum of the sizes of the target's argument specification, and +* `buffer` is the argument values, coerced to their declared types and serialized in argument specification order, followed by zero bytes up to the argument buffer capacity. + +> The target receives exactly the argument buffer described in [argument binding](#argument-binding); the zero padding is not part of it. + +The target executes as its own program: it does not share variables, functions, or any other state with the caller, and the caller observes only the command response. + +The expression evaluates to the command response: +* If `block` is `Svc.BlockState.BLOCK`, the response arrives when the target finishes: `Fw.CmdResponse.OK` if it ran to completion successfully, `Fw.CmdResponse.EXECUTION_ERROR` if it failed to load, failed [argument binding](#argument-binding), or halted with an error. +* If `block` is `Svc.BlockState.NO_BLOCK`, the response is `Fw.CmdResponse.OK` as soon as the command is accepted, before the target's outcome is known. +* In either case, the response is `Fw.CmdResponse.EXECUTION_ERROR` if the command cannot be accepted (for example, the receiving sequencer is busy). + +> Because command evaluation blocks until the response arrives, `BLOCK` runs the target synchronously, and `NO_BLOCK` runs it concurrently with the rest of the calling sequence. + +> Per the semantics of bare command calls, a `BLOCK` call whose response is discarded halts the calling sequence if the target fails, unless `flags.assert_cmd_success` is `False`. Saving the response in a variable, or using the response in any way, suppresses this. + +*Tests:* [1](test/fpy/test_seq_calling.py#L364 "test/fpy/test_seq_calling.py::TestSeqCallingReturnStatus::test_branch_on_success"), [2](test/fpy/test_seq_calling.py#L383 "test/fpy/test_seq_calling.py::TestSeqCallingReturnStatus::test_branch_on_child_failure"), [3](test/fpy/test_seq_calling.py#L214 "test/fpy/test_seq_calling.py::TestSeqCallingWithArgs::test_wrong_value_causes_failure") + +A target may itself call sequence-run commands, to any depth supported by the running system. + +*Tests:* [1](test/fpy/test_seq_calling.py#L307 "test/fpy/test_seq_calling.py::TestSeqCallingNested::test_nested_two_levels"), [2](test/fpy/test_seq_calling.py#L335 "test/fpy/test_seq_calling.py::TestSeqCallingNested::test_nested_pass_through_arg") + +# Design notes + +This section is non-normative. It records trade-offs in the current design and possible improvements. The original design rationale is in [issue #39](https://github.com/fprime-community/fpy/issues/39); the notes below stay within the decisions made there. + +**The call syntax is settled.** Issue #39 chose command-call syntax over the alternatives. Import-style calling (`import some_seq from "path.bin"` then `some_seq(1, 2, 3)`) was the initial choice but was rejected: it costs two lines per call, gives no way to name the sequencer instance that runs the target (the command receiver expresses this for free), and borrows a Python mental model for something fundamentally different. That last point is stronger now than when it was written: Fpy `import` means compile-time inlining, while a sequence call is runtime dispatch to a separately compiled program. A custom statement (`run "seq.bin" no_block args(1, 2)`) was rejected as new syntax for operators to learn. The notes below therefore keep the command-call design and address its edges. + +**Structural command detection is fragile.** A sequence-run command is recognized purely by its parameter shape (string, `Svc.BlockState`, `Svc.SeqArgs`). This keeps argument types and counts out of the flight software, which is a requirement of #39, but shape-matching is a heuristic that can misfire in both directions: an unrelated command that happens to match the shape gets its signature silently rewritten, and a sequencer command that renames or reorders parameters silently loses checking. An explicit designation -- an FPP annotation on the command, or a compiler configuration entry naming the commands -- would keep the FSW-independence of the argument types while making the treatment intentional. + +**The file name does double duty.** One string literal is both the onboard load path and the ground lookup key, forcing the ground directory layout to mirror the onboard one. Without changing the call syntax, the compiler could resolve the ground copy through a search path or a configurable path mapping (as imports resolve sources) while still recording the onboard path verbatim in the command. + +**Interface identity is name-plus-size only.** Target resolution accepts a type if its name and byte size match. Two dictionary versions can disagree about a struct's field layout while agreeing on both, and the call still compiles. #39 deliberately limited runtime validation to total-size matching to avoid fragile string comparisons; recording a structural hash of each type in the argument specification would strengthen the compile-time check, and would give the sequencer a fixed-width comparison to perform onboard -- satisfying the issue's runtime-type-validation nice-to-have without comparing strings. + +**No staleness protection.** The compiler checks the ground copy of the target binary, but nothing ties the call to the onboard copy; only the total argument size is validated at run time. Recording the target binary's CRC in the caller and having the sequencer compare it (perhaps optionally) would close this gap, again as a fixed-width comparison in the same spirit as the size check. + +**`Svc.SeqArgs` is always full capacity.** The struct serializes at fixed size, so every sequence-run command carries the whole buffer, padding included, regardless of how many argument bytes are used. This bloats uplinked commands and couples the max argument size to the command buffer size. A variable-length encoding would remove the waste, at the cost of departing from plain FPP struct serialization. + +**No default values.** Function parameters may have constant defaults, but sequence parameters may not. Since the argument specification already carries per-argument metadata, constant defaults could be recorded there and filled in by callers that omit the argument. + +**`RUN_ARGS` is a stopgap.** #39 describes the separate `RUN_ARGS` opcode as a temporary workaround for GDS compatibility, so `RUN` and `RUN_ARGS` may eventually merge. Until then the split has rough edges: `RUN` on an argument-taking sequence fails only at run time (size mismatch), and `VALIDATE_ARGS` does not match the sequence-run shape, so calling it from Fpy requires constructing a raw `Svc.SeqArgs`, which is impractical. Explicit designation (above) would let `VALIDATE_ARGS` receive the same vararg treatment, and the compiler could warn when `RUN` targets a sequence with a non-empty argument specification. diff --git a/SPEC.md b/SPEC.md index 12c1e4c..5ddaaa7 100644 --- a/SPEC.md +++ b/SPEC.md @@ -57,7 +57,7 @@ The list of reserved words is: A **symbol** is a language construct that can be referred to by a name in the program. The following language constructs may be symbols: -* [namespaces](#namespaces) +* [modules](#modules) * [variables](#variables) * [callables](#callables) * [types](#types) @@ -91,7 +91,7 @@ The list of name groups is: * The **[type](#types) name group** * The **[callable](#callables) name group** -Each name group only contains names which map to their particular language construct, or namespaces with the same property, recursively. +Each name group only contains names which map to their particular language construct, or modules with the same property, recursively. TODO Maybe could remove this line TODO explain why we need this @@ -112,9 +112,9 @@ TODO Does it refer to something that has a scope, or does it refer to something The **resolving name group** is the name group that a name should be resolved in, based on its syntactic context. -## Namespaces +## Modules -A **namespace** is a mapping of names to symbols, associated with a name. +A **module** is a mapping of names to symbols, associated with a name. ## Qualified names @@ -131,22 +131,22 @@ To resolve a qualified name in a name group: 2. Otherwise: 1. Resolve the qualifier. 2. If the qualifier is an expression, resolution is handled by the rules of [member access](#member-access-expression). - 3. If the qualifier is not a namespace, an error is raised. - 4. Resolve the name in the qualifier namespace. + 3. If the qualifier is not a module, an error is raised. + 4. Resolve the name in the qualifier module. If at any point a name fails to be resolved, an error is raised, unless otherwise specified. A **fully-qualified name** is a qualified name which is not itself a qualifier. TODO names are semantic, ident is syntactic TODO you can't actually tell at syntax level what is a fqn -If a fully-qualified name resolves to a namespace, an error is raised. +If a fully-qualified name resolves to a module, an error is raised. -*Tests:* [1](test/fpy/test_imports.py#L343 "test/fpy/test_imports.py::TestImportNamespaceIsolation::test_module_name_not_usable_as_value") +*Tests:* [1](test/fpy/test_imports.py#L530 "test/fpy/test_imports.py::TestImportModuleIsolation::test_module_name_not_usable_as_value") TODO you can think of the dict as "importing definitions" -> Namespace symbols cannot be used anywhere, so this forces names to resolve to something "useful" +> Module symbols cannot be used anywhere, so this forces names to resolve to something "useful" TODO I'm not sure this is clear what this means. The idea here is that the full qualified name should always reference SOMETHING--cannot just put Svc in place of a type, even though Svc does resolve in type name group and global scope. @@ -630,107 +630,173 @@ If at any point during execution, two times which are [incomparable](todo) are a # Imports -> **Note:** The import statement is not yet implemented. - -An **import statement** compiles another Fpy source file, called a **module**, and makes the module's [definitions](#definitions) available in the importing file under a [namespace](#namespaces). +An **import statement** compiles another Fpy sequence, and makes the sequence's [definitions](#definitions) available in the importing sequence. ## Syntax Rule: -`import_stmt: "import" name ("." name)*` +`import_stmt: import_seq | import_from` +`import_seq: "import" "."* name ("." name)* ["as" name]` +`import_from: "from" "."* name ("." name)* "import" ("*" | import_members | "(" import_members [","] ")")` +`import_members: name ["as" name] ("," name ["as" name])*` Name: -`import_stmt: "import" module_path` +`import_seq: "import" [dots] import_path ["as" alias]` +`import_from: "from" [dots] sequence_path "import" ("*" | members | "(" members [","] ")")` +`members: member ["as" alias] ("," member ["as" alias])*` + +The **import path** is the dotted chain of names, excluding any leading dots. Its first name is its **root segment**, and its last is its **leaf segment**. The **alias** is the name introduced by an `as` clause. + +An import statement with one or more leading dots is **relative**; one with none is **absolute**. The leading dots must be followed by an import path of at least one name. + +*Tests:* [1](test/fpy/test_imports.py#L2067 "test/fpy/test_imports.py::TestImportRelative::test_bare_dot_from_is_error") -The **module path** is the entire dotted sequence of names. The first name of a module path is its **root segment**, and the last is its **leaf segment**. +> Unlike Python, `import .util` is valid and `from . import util` is not. The members of a `from` statement are always definitions, never sequences; to import a sibling sequence whole, write `import .util`. + +In the parenthesized form, the member list may span multiple lines. An import statement is only valid outside an indentation block. -*Tests:* [1](test/fpy/test_imports.py#L535 "test/fpy/test_imports.py::TestImportOnlyAtTopLevel::test_import_inside_if_block_fails"), [2](test/fpy/test_imports.py#L552 "test/fpy/test_imports.py::TestImportOnlyAtTopLevel::test_import_inside_function_fails") +*Tests:* [1](test/fpy/test_imports.py#L843 "test/fpy/test_imports.py::TestImportOnlyAtTopLevel::test_import_inside_if_block_fails"), [2](test/fpy/test_imports.py#L860 "test/fpy/test_imports.py::TestImportOnlyAtTopLevel::test_import_inside_function_fails") + +## Semantics -## Module resolution +### Sequence resolution -The **import search path** is an ordered list of directories provided by the environment in which the compiler is invoked. +The **base import search path** is a list of directories provided by the environment in which the compiler is invoked. It is the same for every sequence in a compilation. -> In the command-line compiler, the import search path is the directory containing the file being compiled, followed by each directory passed with `-i`/`--include`, in order. +> In the command-line compiler, the base import search path is each directory passed with `-i`/`--include`; exact duplicates (after path resolution) are dropped, so a repeated `-i` flag cannot manufacture an ambiguity error. The main sequence's own directory is not implicitly added: a sequence reaches its own siblings with relative imports. -A module path `s_0.s_1. ... .s_n` **resolves** in a directory `dir` if the file `dir/s_0/s_1/.../s_n.fpy` exists. +An import statement is resolved against **candidate directories**: +* The candidate directories of an absolute import are the base import search path. The location of the importing sequence plays no role, so an absolute import path names the same file in every sequence of a compilation. +* A relative import has a single candidate directory, its **anchor**: one leading dot anchors at the directory containing the importing sequence, and each additional leading dot moves the anchor to its parent directory. The base import search path plays no role. -*Tests:* [1](test/fpy/test_imports.py#L705 "test/fpy/test_imports.py::TestImportDottedPaths::test_single_dotted_import"), [2](test/fpy/test_imports.py#L725 "test/fpy/test_imports.py::TestImportDottedPaths::test_deeply_nested_dotted_import"), [3](test/fpy/test_imports.py#L801 "test/fpy/test_imports.py::TestImportPackagePrecedence::test_module_file_beats_namespace_directory"), [4](test/fpy/test_imports.py#L815 "test/fpy/test_imports.py::TestImportPackagePrecedence::test_package_dir_used_for_dotted_descent") +*Tests:* [1](test/fpy/test_imports.py#L1889 "test/fpy/test_imports.py::TestImportRelative::test_relative_import_of_sibling"), [2](test/fpy/test_imports.py#L1912 "test/fpy/test_imports.py::TestImportRelative::test_absolute_import_ignores_importer_directory"), [3](test/fpy/test_imports.py#L1938 "test/fpy/test_imports.py::TestImportRelative::test_relative_import_does_not_search_base_path"), [4](test/fpy/test_imports.py#L1981 "test/fpy/test_imports.py::TestImportRelative::test_parent_relative_import") -The module path of an import statement is resolved in each directory of the import search path, in order. The file from the first directory in which it resolves is the imported module. +If a relative import appears in a sequence that has no containing directory (such as one compiled from a stream), an error is raised. -*Tests:* [1](test/fpy/test_imports.py#L856 "test/fpy/test_imports.py::TestImportSearchDirs::test_module_found_in_later_search_dir"), [2](test/fpy/test_imports.py#L871 "test/fpy/test_imports.py::TestImportSearchDirs::test_first_search_dir_shadows_later"), [3](test/fpy/test_imports.py#L887 "test/fpy/test_imports.py::TestImportSearchDirs::test_search_order_respects_dir_order"), [4](test/fpy/test_imports.py#L903 "test/fpy/test_imports.py::TestImportSearchDirs::test_dotted_module_resolved_across_search_dirs") +*Tests:* [1](test/fpy/test_imports.py#L2097 "test/fpy/test_imports.py::TestImportRelative::test_relative_import_without_location_is_error") -If the module path resolves in no directory of the import search path, an error is raised. +A sequence path `s_0.s_1. ... .s_n` **resolves** in a directory `dir` if the file `dir/s_0/s_1/.../s_n.fpy` exists. -*Tests:* [1](test/fpy/test_imports.py#L246 "test/fpy/test_imports.py::TestImportErrors::test_missing_module_is_an_error"), [2](test/fpy/test_imports.py#L919 "test/fpy/test_imports.py::TestImportSearchDirs::test_no_search_dirs_cannot_resolve"), [3](test/fpy/test_imports.py#L763 "test/fpy/test_imports.py::TestImportDottedPaths::test_missing_leaf_in_existing_package_is_error"), [4](test/fpy/test_imports.py#L828 "test/fpy/test_imports.py::TestImportPackagePrecedence::test_bare_package_import_is_error"), [5](test/fpy/test_imports.py#L841 "test/fpy/test_imports.py::TestImportPackagePrecedence::test_dotted_leaf_package_import_is_error"), [6](test/fpy/test_imports.py#L297 "test/fpy/test_imports.py::TestImportFileErrors::test_import_path_is_a_directory_fails") +*Tests:* [1](test/fpy/test_imports.py#L1013 "test/fpy/test_imports.py::TestImportDottedPaths::test_single_dotted_import"), [2](test/fpy/test_imports.py#L1033 "test/fpy/test_imports.py::TestImportDottedPaths::test_deeply_nested_dotted_import"), [3](test/fpy/test_imports.py#L1111 "test/fpy/test_imports.py::TestImportPackagePrecedence::test_sequence_file_beats_directory"), [4](test/fpy/test_imports.py#L1125 "test/fpy/test_imports.py::TestImportPackagePrecedence::test_package_dir_used_for_dotted_descent") -> Every segment except the leaf names a plain directory; no `__init__.fpy`-style marker file is required. Because only the leaf's `.fpy` file satisfies an import, a file `foo.fpy` always takes precedence over a sibling directory `foo/` for `import foo`, while `import foo.bar` descends into the directory `foo/` regardless of whether `foo.fpy` exists. Importing a name that resolves only to a directory is an error: a directory has no code to import. +An import statement names its imported sequence and an optional member as follows. Let its import path be `s_0. ... .s_n`. In each candidate directory: +1. If `s_0. ... .s_n` resolves in the directory, that file is the directory's **split**; the whole path is the **sequence path**, and there is no member. +2. Otherwise, if `n > 0` and `s_0. ... .s_{n-1}` resolves in the directory, that file is the directory's split; `s_0. ... .s_{n-1}` is the sequence path, and the final name `s_n` is the **member**. +3. Otherwise, the directory has no split. -## Semantics +If exactly one candidate directory has a split, it names the imported sequence and the member. If no candidate directory has one, an error is raised. If more than one candidate directory has one, an error is raised, even if the splits name the same file. -If the imported module fails to parse or compile, an error is raised. +A `from` statement's path splits by step 1 only: it is always the whole sequence path, never a sequence path plus member. Each name in its import list is a member. -*Tests:* [1](test/fpy/test_imports.py#L279 "test/fpy/test_imports.py::TestImportFileErrors::test_parse_error_in_imported_file_fails") +*Tests:* [1](test/fpy/test_imports.py#L1169 "test/fpy/test_imports.py::TestImportSearchDirs::test_sequence_found_in_later_search_dir"), [2](test/fpy/test_imports.py#L1185 "test/fpy/test_imports.py::TestImportSearchDirs::test_same_name_in_two_dirs_is_ambiguous"), [3](test/fpy/test_imports.py#L1206 "test/fpy/test_imports.py::TestImportSearchDirs::test_whole_vs_member_across_dirs_is_ambiguous"), [4](test/fpy/test_imports.py#L1228 "test/fpy/test_imports.py::TestImportSearchDirs::test_dotted_sequence_resolved_across_search_dirs"), [5](test/fpy/test_imports.py#L350 "test/fpy/test_imports.py::TestImportErrors::test_missing_sequence_is_an_error"), [6](test/fpy/test_imports.py#L1246 "test/fpy/test_imports.py::TestImportSearchDirs::test_no_search_dirs_cannot_resolve"), [7](test/fpy/test_imports.py#L1071 "test/fpy/test_imports.py::TestImportDottedPaths::test_missing_leaf_in_existing_package_is_error"), [8](test/fpy/test_imports.py#L1138 "test/fpy/test_imports.py::TestImportPackagePrecedence::test_bare_package_import_is_error"), [9](test/fpy/test_imports.py#L1151 "test/fpy/test_imports.py::TestImportPackagePrecedence::test_dotted_leaf_package_import_is_error"), [10](test/fpy/test_imports.py#L484 "test/fpy/test_imports.py::TestImportFileErrors::test_import_path_is_a_directory_fails"), [11](test/fpy/test_imports.py#L2003 "test/fpy/test_imports.py::TestImportRelative::test_relative_member_import") + +> A file `foo.fpy` always takes precedence over a sibling directory `foo/` for `import foo`, while `import foo.bar` descends into the directory `foo/` regardless of whether `foo.fpy` exists. + +> Splitting `import a.b.c` against a directory: +> * `a/b/c.fpy` exists: sequence path `a.b.c`, no member -- the whole sequence is imported. +> * `a/b/c.fpy` is missing but `a/b.fpy` exists: sequence path `a.b`, member `c`. +> * neither exists: the directory has no split. +> +> The whole path is preferred over a member within a single directory: if both `a/b/c.fpy` and `a/b.fpy` exist in one directory, `import a.b.c` imports `a/b/c.fpy` whole. Nothing is preferred across directories: two base directories that both contain a `util.fpy` make `import util` ambiguous. Ambiguity is an error rather than a shadowing rule so that adding a file to a search directory can never silently change which file another import names. + +> The intended layout for a reusable library is a directory of sequences that reference one another with relative imports. A consumer adds the directory *containing* the library directory to the base import search path, and writes `import .`. The library then works unmodified wherever it is checked out or copied to, and every consumer names its sequences identically. + +### Importing a sequence + +If the imported sequence fails to parse or compile, an error is raised. + +*Tests:* [1](test/fpy/test_imports.py#L466 "test/fpy/test_imports.py::TestImportFileErrors::test_parse_error_in_imported_file_fails") > The diagnostic should point into the imported file, not at the import statement. -If the imported module declares one or more [sequence arguments](todo), an error is raised. +If the imported sequence declares one or more [sequence arguments](todo), an error is raised. -*Tests:* [1](test/fpy/test_imports.py#L228 "test/fpy/test_imports.py::TestImportErrors::test_cannot_import_sequence_with_arguments") +*Tests:* [1](test/fpy/test_imports.py#L332 "test/fpy/test_imports.py::TestImportErrors::test_cannot_import_sequence_with_arguments") > A `sequence()` directive with no arguments does not prevent a file from being imported. -*Tests:* [1](test/fpy/test_imports.py#L254 "test/fpy/test_imports.py::TestImportErrors::test_no_arg_sequence_is_importable") +*Tests:* [1](test/fpy/test_imports.py#L358 "test/fpy/test_imports.py::TestImportErrors::test_no_arg_sequence_is_importable") + +The imported sequence is compiled as a sequence in its own right: names in it resolve in its own, new global scope, and the semantics of this section apply recursively to its own import statements, with the imported sequence in the role of the importing sequence. + +*Tests:* [1](test/fpy/test_imports.py#L882 "test/fpy/test_imports.py::TestImportTransitive::test_transitive_import_works"), [2](test/fpy/test_imports.py#L579 "test/fpy/test_imports.py::TestImportModuleIsolation::test_imported_function_cannot_see_importer_globals"), [3](test/fpy/test_imports.py#L910 "test/fpy/test_imports.py::TestImportTransitive::test_transitive_dependency_is_private") + +> Isolation is by construction. The importing sequence's symbols live in the importing sequence's global scope, so they are not visible in the imported sequence; a sequence imported by the imported sequence binds names in the imported sequence's global scope, so it is not visible in the importing sequence. Every rule below that binds a name says which global scope receives it. + +If a sequence transitively imports itself, an error is raised. + +*Tests:* [1](test/fpy/test_imports.py#L943 "test/fpy/test_imports.py::TestImportCycles::test_self_import_is_cycle_error"), [2](test/fpy/test_imports.py#L964 "test/fpy/test_imports.py::TestImportCycles::test_mutual_import_is_cycle_error"), [3](test/fpy/test_imports.py#L995 "test/fpy/test_imports.py::TestImportCycles::test_three_way_cycle_error") + +To introduce a sequence path `s_0. ... .s_k` as a **module chain** is to add `s_0` as a [module](#modules) to the importing sequence's global scope, and each following `s_i` as a module to `s_{i-1}`; the last module, `s_k`, is the **leaf module**. + +An import statement with no member and no alias introduces its sequence path as a module chain, and adds each [definition](#definitions) in the imported sequence's global scope to the leaf module, under its own name. + +*Tests:* [1](test/fpy/test_imports.py#L103 "test/fpy/test_imports.py::TestImportInlining::test_call_imported_function"), [2](test/fpy/test_imports.py#L144 "test/fpy/test_imports.py::TestImportInlining::test_local_and_imported_names_coexist"), [3](test/fpy/test_imports.py#L511 "test/fpy/test_imports.py::TestImportModuleIsolation::test_imported_symbol_requires_module_prefix"), [4](test/fpy/test_imports.py#L549 "test/fpy/test_imports.py::TestImportModuleIsolation::test_same_function_name_in_two_modules_no_collision"), [5](test/fpy/test_imports.py#L1051 "test/fpy/test_imports.py::TestImportDottedPaths::test_dotted_symbol_requires_full_path") + +> After `import a.b.c` of a sequence that defines `x`, the symbol is named `a.b.c.x`, and is available under no shorter name. + +The sequence path of a relative import excludes its leading dots; the dots affect resolution only. + +*Tests:* [1](test/fpy/test_imports.py#L1959 "test/fpy/test_imports.py::TestImportRelative::test_relative_binds_path_after_dots"), [2](test/fpy/test_imports.py#L1981 "test/fpy/test_imports.py::TestImportRelative::test_parent_relative_import") + +> `import .sub.mod` introduces the module chain `sub.mod`, and `import ..util` introduces the module `util`. So the depth of the importer never leaks into names: `lib/a.fpy` (via `import .util`) and `lib/sub/b.fpy` (via `import ..util`) both bind `lib/util.fpy` as module `util`. + +A module that holds an imported sequence's definitions -- the leaf module of a module chain, or a module bound by an alias -- is a **sequence module**. A module introduced as a non-leaf segment of a module chain is a **package module**. + +A name introduced by an import is an ordinary symbol: mapping one name to two symbols in the same [name group](#name-groups) and the same scope or [module](#modules) is a collision, and an error is raised. The exception is two modules, which instead merge into one module whose members are those of both -- unless both are sequence modules, which collide like any other symbols. + +*Tests:* [1](test/fpy/test_imports.py#L608 "test/fpy/test_imports.py::TestImportNameCollisions::test_import_collides_with_local_function"), [2](test/fpy/test_imports.py#L629 "test/fpy/test_imports.py::TestImportNameCollisions::test_import_collides_with_local_variable"), [3](test/fpy/test_imports.py#L648 "test/fpy/test_imports.py::TestImportNameCollisions::test_import_coexists_with_local_variable"), [4](test/fpy/test_imports.py#L1469 "test/fpy/test_imports.py::TestImportAlias::test_alias_collides_with_local"), [5](test/fpy/test_imports.py#L1719 "test/fpy/test_imports.py::TestImportFrom::test_from_import_collides_with_local"), [6](test/fpy/test_imports.py#L1740 "test/fpy/test_imports.py::TestImportFrom::test_from_star_collides_across_sequences"), [7](test/fpy/test_imports.py#L1663 "test/fpy/test_imports.py::TestImportFrom::test_from_import_duplicate_member_is_error"), [8](test/fpy/test_imports.py#L1088 "test/fpy/test_imports.py::TestImportDottedPaths::test_two_sequences_in_same_package_no_collision"), [9](test/fpy/test_imports.py#L2132 "test/fpy/test_imports.py::TestImportModuleMerging::test_sequence_and_package_module_merge"), [10](test/fpy/test_imports.py#L2148 "test/fpy/test_imports.py::TestImportModuleMerging::test_two_aliases_same_name_collide"), [11](test/fpy/test_imports.py#L2161 "test/fpy/test_imports.py::TestImportModuleMerging::test_relative_and_absolute_leaf_collision") + +> So after `import pkg.a` and `import pkg.b`, package module `pkg` contains both `a` and `b`. After `import pkg` and `import pkg.mod`, module `pkg` holds sequence `pkg.fpy`'s definitions alongside module `mod`. And a variable `lib` coexists with an imported module `lib` that defines only functions, because a value and a callable never share a name group. + +> Sequence modules never merge, so two different files' definitions cannot silently pool under one name: an absolute `import util` and a relative `import .util` that name different files collide. Rename one with `as`. (Two imports of the same file are already rejected by the [duplicate-import rule](#importing-a-sequence).) -An imported module may itself contain import statements. The semantics of this section apply to them recursively, with the module in the role of the importing file. All import statements in a compilation resolve against the same import search path. +An import statement with a member and no alias introduces its sequence path as a module chain, and adds the [definition](#definitions) named by the member in the imported sequence's global scope to the leaf module, under the member's name. If the imported sequence's global scope has no such definition, an error is raised. -*Tests:* [1](test/fpy/test_imports.py#L574 "test/fpy/test_imports.py::TestImportTransitive::test_transitive_import_works") +*Tests:* [1](test/fpy/test_imports.py#L1290 "test/fpy/test_imports.py::TestImportMember::test_import_member_function"), [2](test/fpy/test_imports.py#L1309 "test/fpy/test_imports.py::TestImportMember::test_import_member_of_dotted_sequence"), [3](test/fpy/test_imports.py#L1328 "test/fpy/test_imports.py::TestImportMember::test_member_requires_full_path"), [4](test/fpy/test_imports.py#L1348 "test/fpy/test_imports.py::TestImportMember::test_member_import_hides_other_symbols"), [5](test/fpy/test_imports.py#L1371 "test/fpy/test_imports.py::TestImportMember::test_missing_member_is_error") -If a module transitively imports itself, an error is raised. +> `import a.b.c.foo` adds only `foo`, named `a.b.c.foo`. -*Tests:* [1](test/fpy/test_imports.py#L635 "test/fpy/test_imports.py::TestImportCycles::test_self_import_is_cycle_error"), [2](test/fpy/test_imports.py#L656 "test/fpy/test_imports.py::TestImportCycles::test_mutual_import_is_cycle_error"), [3](test/fpy/test_imports.py#L687 "test/fpy/test_imports.py::TestImportCycles::test_three_way_cycle_error") +An import statement with an alias introduces no module chain. It binds the alias, in the importing sequence's global scope, to the imported symbol if the import has a member, or otherwise to a [module](#modules) holding each [definition](#definitions) in the imported sequence's global scope. The alias occupies the [name groups](#name-groups) of what it is bound to. -The import statement introduces the module path as a chain of namespaces in the global scope. Each [definition](#definitions) at the top level of the module adds its symbol to the leaf namespace: a symbol `x` defined in module `a.b.c` is named `a.b.c.x` in the importing file, and is not accessible under any shorter name. +*Tests:* [1](test/fpy/test_imports.py#L1394 "test/fpy/test_imports.py::TestImportAlias::test_import_sequence_as_alias"), [2](test/fpy/test_imports.py#L1413 "test/fpy/test_imports.py::TestImportAlias::test_dotted_import_as_alias"), [3](test/fpy/test_imports.py#L1431 "test/fpy/test_imports.py::TestImportAlias::test_import_member_as_alias"), [4](test/fpy/test_imports.py#L1449 "test/fpy/test_imports.py::TestImportAlias::test_alias_hides_chain") -*Tests:* [1](test/fpy/test_imports.py#L85 "test/fpy/test_imports.py::TestImportInlining::test_call_imported_function"), [2](test/fpy/test_imports.py#L123 "test/fpy/test_imports.py::TestImportInlining::test_local_and_imported_names_coexist"), [3](test/fpy/test_imports.py#L324 "test/fpy/test_imports.py::TestImportNamespaceIsolation::test_imported_symbol_requires_module_prefix"), [4](test/fpy/test_imports.py#L362 "test/fpy/test_imports.py::TestImportNamespaceIsolation::test_same_function_name_in_two_modules_no_collision"), [5](test/fpy/test_imports.py#L743 "test/fpy/test_imports.py::TestImportDottedPaths::test_dotted_symbol_requires_full_path") +> `import a.b.c as x` binds `x` to a module of `a.b.c`'s definitions; `import a.b.c.foo as x` binds `x` to the symbol `foo`. -Per the [name group](#name-groups) rules, these namespaces exist only in the name groups of the symbols they contain. +A `from` statement introduces no module chain. It binds, in the importing sequence's global scope, the [definition](#definitions) named by each member in the imported sequence's global scope, under the member's name or its alias if one is given. `from p import *` binds every definition in the imported sequence's global scope, each under its own name. If a member names no definition in the imported sequence's global scope, an error is raised. -*Tests:* [1](test/fpy/test_imports.py#L461 "test/fpy/test_imports.py::TestImportNameCollisions::test_import_coexists_with_local_variable") +*Tests:* [1](test/fpy/test_imports.py#L1495 "test/fpy/test_imports.py::TestImportFrom::test_from_import_function"), [2](test/fpy/test_imports.py#L1513 "test/fpy/test_imports.py::TestImportFrom::test_from_dotted_sequence"), [3](test/fpy/test_imports.py#L1531 "test/fpy/test_imports.py::TestImportFrom::test_from_import_as_alias"), [4](test/fpy/test_imports.py#L1549 "test/fpy/test_imports.py::TestImportFrom::test_from_import_star"), [5](test/fpy/test_imports.py#L1680 "test/fpy/test_imports.py::TestImportFrom::test_from_import_does_not_introduce_sequence_name"), [6](test/fpy/test_imports.py#L1702 "test/fpy/test_imports.py::TestImportFrom::test_from_import_missing_member_is_error"), [7](test/fpy/test_imports.py#L1571 "test/fpy/test_imports.py::TestImportFrom::test_from_import_multiple_members"), [8](test/fpy/test_imports.py#L1592 "test/fpy/test_imports.py::TestImportFrom::test_from_import_multiple_with_aliases"), [9](test/fpy/test_imports.py#L1613 "test/fpy/test_imports.py::TestImportFrom::test_from_import_parenthesized_single_line"), [10](test/fpy/test_imports.py#L1634 "test/fpy/test_imports.py::TestImportFrom::test_from_import_parenthesized_multiline") -If a name introduced by an import statement is already mapped, in the same name group and the same scope or namespace, to anything other than a namespace introduced by another import statement, an error is raised. +Two import statements import the same sequence if they resolve to the same file, whatever paths named it. -*Tests:* [1](test/fpy/test_imports.py#L421 "test/fpy/test_imports.py::TestImportNameCollisions::test_import_collides_with_local_function"), [2](test/fpy/test_imports.py#L442 "test/fpy/test_imports.py::TestImportNameCollisions::test_import_collides_with_local_variable") +A sequence is compiled once, however many import statements name it: in one importing sequence or across several, and whether named by one path or by different paths that resolve to the same file. Its definitions are shared, never duplicated. -> Namespaces introduced by imports merge: after `import pkg.a` and `import pkg.b`, the namespace `pkg` contains both `a` and `b`. Because name groups do not intersect, an imported module conflicts only with names in the groups it occupies: a variable `lib` may coexist with an imported module `lib` that defines only functions. +> So a sequence and a sequence it imports may both import a third, and it is compiled once for the whole program. -*Tests:* [1](test/fpy/test_imports.py#L780 "test/fpy/test_imports.py::TestImportDottedPaths::test_two_modules_in_same_package_no_collision") +*Tests:* [1](test/fpy/test_imports.py#L813 "test/fpy/test_imports.py::TestImportDuplicates::test_duplicate_across_files_is_allowed") -Names within the module are resolved in the module's own global scope. Symbols of the importing file are not visible in the module, and modules imported by the module are not visible in the importing file. +Importing the same sequence more than once in one sequence is governed by the ordinary collision rule above: the names the imports bind must not collide. -*Tests:* [1](test/fpy/test_imports.py#L392 "test/fpy/test_imports.py::TestImportNamespaceIsolation::test_imported_function_cannot_see_importer_globals"), [2](test/fpy/test_imports.py#L602 "test/fpy/test_imports.py::TestImportTransitive::test_transitive_dependency_is_private") +> To use several of a sequence's symbols, import it whole and qualify them, use one `from seq import a, b`, or use `from seq import *`. -If the same module path is imported more than once in the same file, an error is raised. +*Tests:* [1](test/fpy/test_imports.py#L1784 "test/fpy/test_imports.py::TestImportDuplicateSequence::test_import_and_from_same_sequence_coexist"), [2](test/fpy/test_imports.py#L1856 "test/fpy/test_imports.py::TestImportDuplicateSequence::test_two_from_same_sequence_coexist"), [3](test/fpy/test_imports.py#L793 "test/fpy/test_imports.py::TestImportDuplicates::test_import_same_sequence_twice_collides"), [4](test/fpy/test_imports.py#L1809 "test/fpy/test_imports.py::TestImportDuplicateSequence::test_two_members_same_sequence_collides"), [5](test/fpy/test_imports.py#L1834 "test/fpy/test_imports.py::TestImportDuplicateSequence::test_whole_and_member_same_sequence_collides"), [6](test/fpy/test_imports.py#L2108 "test/fpy/test_imports.py::TestImportRelative::test_relative_and_absolute_same_file_collides") -*Tests:* [1](test/fpy/test_imports.py#L489 "test/fpy/test_imports.py::TestImportDuplicates::test_duplicate_import_is_error") +An imported sequence may contain only function definitions and import statements. If it contains any other top-level statement, an error is raised. Such a statement is a side effect that would have to execute at the position of the import; because an imported sequence is compiled as a sibling scope block that does not run inline, it has no place to execute. -> This rule is per-file: a file and a module it imports may each import the same module. +*Tests:* [1](test/fpy/test_imports.py#L179 "test/fpy/test_imports.py::TestImportSideEffects::test_side_effecting_import_is_error"), [2](test/fpy/test_imports.py#L193 "test/fpy/test_imports.py::TestImportSideEffects::test_functions_only_sequence_compiles"), [3](test/fpy/test_imports.py#L1260 "test/fpy/test_imports.py::TestImportVariables::test_top_level_variable_is_error"), [4](test/fpy/test_imports.py#L1753 "test/fpy/test_imports.py::TestImportFrom::test_from_import_side_effects_is_error"), [5](test/fpy/test_imports.py#L734 "test/fpy/test_imports.py::TestImportedSequenceCannotShadowBuiltins::test_imported_sequence_cannot_declare_top_level_flags"), [6](test/fpy/test_imports.py#L495 "test/fpy/test_imports.py::TestImportFileErrors::test_empty_sequence_compiles_without_warning") -*Tests:* [1](test/fpy/test_imports.py#L506 "test/fpy/test_imports.py::TestImportDuplicates::test_duplicate_across_files_is_allowed") +If an importing sequence names a [definition](#definitions) of an imported sequence by a name that begins with an underscore, the `import-underscore` warning is emitted. -If the imported module contains top-level statements other than function definitions and import statements, the `import-side-effects` warning is emitted. +*Tests:* [1](test/fpy/test_imports.py#L229 "test/fpy/test_imports.py::TestImportUnderscore::test_underscore_module_access_warns"), [2](test/fpy/test_imports.py#L245 "test/fpy/test_imports.py::TestImportUnderscore::test_underscore_member_import_warns"), [3](test/fpy/test_imports.py#L260 "test/fpy/test_imports.py::TestImportUnderscore::test_underscore_from_import_warns"), [4](test/fpy/test_imports.py#L275 "test/fpy/test_imports.py::TestImportUnderscore::test_star_import_underscore_use_warns"), [5](test/fpy/test_imports.py#L291 "test/fpy/test_imports.py::TestImportUnderscore::test_library_internal_use_does_not_warn"), [6](test/fpy/test_imports.py#L305 "test/fpy/test_imports.py::TestImportUnderscore::test_underscore_alias_statement_warns_but_uses_do_not") -*Tests:* [1](test/fpy/test_imports.py#L158 "test/fpy/test_imports.py::TestImportSideEffects::test_side_effecting_import_warns"), [2](test/fpy/test_imports.py#L172 "test/fpy/test_imports.py::TestImportSideEffects::test_side_effect_warning_can_be_ignored"), [3](test/fpy/test_imports.py#L187 "test/fpy/test_imports.py::TestImportSideEffects::test_side_effect_warning_can_be_escalated"), [4](test/fpy/test_imports.py#L202 "test/fpy/test_imports.py::TestImportSideEffects::test_functions_only_module_does_not_warn"), [5](test/fpy/test_imports.py#L308 "test/fpy/test_imports.py::TestImportFileErrors::test_empty_module_compiles_without_warning") +> A leading underscore marks a definition as internal to its sequence. The warning is on the importer's mention of the name -- `lib._helper()`, `import lib._helper`, `from lib import _helper` -- never on the imported sequence's own references to it. An alias renames: after `from lib import _helper as helper`, uses of `helper` do not warn, though the `from` statement itself already did. -At execution, the imported module's top-level statements execute as part of the importing sequence, at the position of the import statement, in order. -*Tests:* [1](test/fpy/test_imports.py#L106 "test/fpy/test_imports.py::TestImportInlining::test_imported_function_runs"), [2](test/fpy/test_imports.py#L932 "test/fpy/test_imports.py::TestImportVariables::test_top_level_variable_is_side_effect_and_namespaced") -> Importing is compile-time source inlining: the imported module does not exist in the compiled output, and no files are resolved or loaded at run time. This is what motivates the `import-side-effects` warning: a file written to run as a standalone sequence typically has top-level commands, and importing it splices that code into the importing sequence, which is rarely intended. A file meant to be imported should contain only definitions. Note that a top-level variable definition is such a side effect (its initialization executes at the import site), while still defining a namespaced symbol: `counter: U32 = 5` in module `m` warns, and is thereafter accessible as `m.counter`. # Callables @@ -1119,7 +1185,7 @@ Name: If `parent` is not an expression, an error is raised. -> Namespaces, types names, and function names are valid expressions syntactically, but not semantically. Thus, trying to access a member of either of these symbols will raise an error. +> Modules, type names, and function names are valid expressions syntactically, but not semantically. Thus, trying to access a member of either of these symbols will raise an error. If the type of `parent` is not a [struct](#structs), an error is raised. diff --git a/src/fpy/codegen_fpybc.py b/src/fpy/codegen_fpybc.py index 258dae1..9a09e81 100644 --- a/src/fpy/codegen_fpybc.py +++ b/src/fpy/codegen_fpybc.py @@ -43,6 +43,7 @@ CommandSymbol, FieldAccess, FunctionSymbol, + NameGroup, TypeCtorSymbol, VariableSymbol, ) @@ -167,21 +168,37 @@ class CalculateFrameSizes(TopDownVisitor): def __init__(self): super().__init__() self.offset = 0 + self._frame_root = None def run(self, start: Ast, state: CompileState): - # For the global frame, lay out sequence args (already on stack) first, - # then start body variables after them. - if start is state.root: + self._frame_root = start + # For the global (main) frame -- the program block -- lay out sequence + # args (already on stack) first, then start body variables after them. + if start is state.main_block: for name, arg_type in state.this_seq_arg_specs: - arg_var = state.global_value_scope[name] + arg_var = state.main_scope.group(NameGroup.VALUE)[name] arg_var.frame_offset = self.offset self.offset += arg_type.max_size + # The flags struct lives in the shared base scope (so every sequence + # sees it), but it occupies a slot in the main frame right after the + # sequence args. Reserve it here; the walk below lays out the user's + # own locals after it. + state.flags_var.frame_offset = self.offset + self.offset += state.flags_var.type.max_size super().run(start, state) state.frame_sizes[start] = self.offset def visit_AstBlock(self, node: AstBlock, state: CompileState): - scope = state.enclosing_value_scope[node] - for _name, sym in scope.items(): + # FIXME I'd like an explanation of this, it seems weird + # The program block is its own (global/main) frame. When we reach it + # while walking the library root, lay it out in a fresh frame and stop -- + # the library and imported blocks around it hold only definitions, whose + # frames are handled per-function by visit_AstDef. + if node is state.main_block and node is not self._frame_root: + CalculateFrameSizes().run(node, state) + return STOP_DESCENT + values = state.enclosing_scope[node].group(NameGroup.VALUE) + for _name, sym in values.items(): if is_instance_compat(sym, VariableSymbol) and sym.frame_offset is None: sym.frame_offset = self.offset self.offset += sym.type.max_size @@ -193,7 +210,9 @@ def visit_AstDef(self, node: AstDef, state: CompileState): arg_offset = -STACK_FRAME_HEADER_SIZE for arg in reversed(func.args): arg_name, arg_type, _ = arg - arg_var = state.enclosing_value_scope[node.body][arg_name] + arg_var = state.enclosing_scope[node.body].group(NameGroup.VALUE)[ + arg_name + ] arg_offset -= arg_type.max_size arg_var.frame_offset = arg_offset # Assign body variable offsets in a fresh frame @@ -642,6 +661,11 @@ def _should_lower_stmt(self, stmt: Ast, state: CompileState) -> bool: def emit_AstBlock(self, node: AstBlock, state: CompileState): dirs = [] for stmt in node.stmts: + if is_instance_compat(stmt, AstBlock): + # a sub block. this is only possible if it is an imported sequence + # emit its statements inline in this frame + dirs.extend(self.emit(stmt, state)) + continue if not self._should_lower_stmt(stmt, state): continue dirs.extend(self.emit(stmt, state)) @@ -1306,7 +1330,7 @@ def emit_AstAssert(self, node: AstAssert, state: CompileState): class GenerateModule(Emitter): def emit_AstBlock(self, node: AstBlock, state: CompileState): - if node is not state.root: + if node is not state.main_block: return [] main_body = [] diff --git a/src/fpy/codegen_llvm.py b/src/fpy/codegen_llvm.py index 6dd83f7..6468ff8 100644 --- a/src/fpy/codegen_llvm.py +++ b/src/fpy/codegen_llvm.py @@ -486,8 +486,15 @@ class GenerateLlvmModule: its storage, then lowers the root block's statements with EmitLlvmStmt. """ - def emit(self, body: AstBlock, state: CompileState) -> ir.Module: - assert body is state.root, "module generator must be run on the root block" + def emit(self, root_block: AstBlock, state: CompileState) -> ir.Module: + assert ( + root_block is state.root_block + ), "module generator must be run on the root block" + # The entry function is the main sequence -- the main block -- not the + # library root (which also holds the builtin library and the imported + # sequences, all definition-only). Functions are lowered on demand at + # their call sites, so they need no separate walk here. + program = state.main_block module = ir.Module(name="seq") module.triple = LLVM_TRIPLE @@ -499,11 +506,11 @@ def emit(self, body: AstBlock, state: CompileState) -> ir.Module: self._declare_flags(module, state) # Declare storage for every variable in this frame up front. collector = CollectFrameVariables() - collector.run(body, state) + collector.run(program, state) for sym in collector.symbols: self._declare_variable(module, builder, sym) - EmitLlvmStmt(builder).emit(body, state) + EmitLlvmStmt(builder).emit(program, state) # Fell off the end of the sequence without failing: success. if not builder.block.is_terminated: diff --git a/src/fpy/compiler.py b/src/fpy/compiler.py index 1643577..4719540 100644 --- a/src/fpy/compiler.py +++ b/src/fpy/compiler.py @@ -1,4 +1,5 @@ from __future__ import annotations +import copy from pathlib import Path from lark import Lark, LarkError from llvmlite import ir @@ -52,6 +53,11 @@ UpdateStateWithTypes, WarnRangesAreNotEmpty, ) +from fpy.imports import ( + BindImports, + InlineImports, + WarnImportUnderscore, +) from fpy.syntax import AstBlock, FpyTransformer, PythonIndenter from fpy.types import ( DEFAULT_MAX_DIRECTIVE_SIZE, @@ -93,6 +99,29 @@ maybe_placeholders=True, ) + +def _parse_fpy(text: str, **kwargs): + """Parse fpy source into a Lark tree, tolerating a missing final newline. + + The indenter (PythonIndenter) derives INDENT/DEDENT tokens from the + whitespace each _NEWLINE token carries. When the source does not end in a + newline, the final line's leading whitespace is never followed by a newline, + so at end-of-input it is misread as a brand-new indentation level: a trailing + tab or an over-indented comment on the last line then emits a spurious INDENT + (or a bogus dedent) and an otherwise-valid sequence fails to compile + (https://github.com/fprime-community/fpy/issues/61). + + Appending a newline when one is absent makes the last line's indentation + resolve to column 0 -- closing any open blocks cleanly -- exactly as + CPython's tokenizer supplies an implicit NEWLINE before end-of-input. The + newline is added only at the very end, so it shifts no positions and leaves + error line/column reporting (and text.splitlines()) unchanged. + """ + if not text.endswith("\n"): + text = text + "\n" + return _fpy_parser.parse(text, **kwargs) + + # Load builtin time.fpy functions at module level _builtin_time_path = Path(__file__).parent / "builtin" / "time.fpy" _builtin_time_text = _builtin_time_path.read_text(encoding="utf-8") @@ -112,7 +141,7 @@ def _get_builtin_library_ast(): fpy.error.input_text = _builtin_time_text fpy.error.input_lines = _builtin_time_text.splitlines() - tree = _fpy_parser.parse(_builtin_time_text) + tree = _parse_fpy(_builtin_time_text) _builtin_library_ast = FpyTransformer().transform(tree) # Restore error state @@ -123,13 +152,49 @@ def _get_builtin_library_ast(): return _builtin_library_ast +def _build_compilation_unit(program: AstBlock, state: CompileState): + """Assemble the compilation unit: a fresh *library root* block wrapping the + whole program, and store it as state.root_block. + + On entry `program` holds the main program's statements; InlineImports has + already removed each import and collected its target as a sibling block in + state.imported_blocks. We build: + + library root (state.root_block) scope = base + |- builtin library defs (time_add, time_cmp, ...; registered in base) + |- main block (state.main_block) scope = main (child of base) + |- imported sequence block scope = child of base + |- ... + + Every sequence block (the main block and each import) is a direct child of + the library root, so its scope is a child of base by lexical nesting: syntax + and scope agree, imported sequences are siblings of the main block (isolated + from it and each other), and the builtin library functions resolve up the + parent chain for all of them. Only the main block executes; the library and + imported blocks hold definitions, emitted once each by the dead-function pass. + + # FIXME i think this docstring is too long and verbose + The program block itself becomes state.main_block.""" + library_ast = _get_builtin_library_ast() + + # The program block, as parsed, is the main block; the main sequence's + # context points at it so BindImports can reach its scope. + state.main_block = program + state.main_sequence.block = program + + state.root_block = AstBlock( + program.meta, + copy.deepcopy(library_ast.stmts) + [program] + state.imported_blocks, + ) + + def text_to_ast(text: str): from lark.exceptions import VisitError fpy.error.input_text = text fpy.error.input_lines = text.splitlines() try: - tree = _fpy_parser.parse(text, on_error=handle_lark_error) + tree = _parse_fpy(text, on_error=handle_lark_error) except LarkError as e: handle_lark_error(e) return None @@ -157,30 +222,25 @@ def analyze_ast(body: AstBlock, state: CompileState) -> CompileState: Returns the populated CompileState. Raises the first CompileError encountered. """ - state.root = body - - # we want to run this past first, because the next - # stage will add statements to the start of the file - # which would mess with this pass - pre_builtin_lib_include_passes = [ - CheckSequenceMetadataDefinedAtTop(), - ] - - for compile_pass in pre_builtin_lib_include_passes: - compile_pass.run(body, state) - if len(state.errors) != 0: - raise state.errors[0] - - # Now prepend builtin library functions to user code - always available. - # will be elided if unused - import copy + # Resolve and inline every import first, on the raw program AST: this removes + # each import statement and collects its target sequence as a sibling block + # in state.imported_blocks, recording an ImportBinding for BindImports. + InlineImports().run(body, state) + if len(state.errors) != 0: + raise state.errors[0] - builtin_library_ast = _get_builtin_library_ast() - body.stmts = copy.deepcopy(builtin_library_ast.stmts) + body.stmts + # Assemble the compilation unit: a fresh library root block wrapping the + # builtin library, the main program (state.main_block), and every imported + # sequence as sibling children. All later passes run on state.root_block. + _build_compilation_unit(body, state) pre_semantic_desugaring_passes = [DesugarCheckStatements()] semantics_passes: list[Visitor] = [ + # sequence() metadata, if present, must be the first statement of the + # main block (the builtin library prepend cannot displace it because it + # goes into the library root, not the main block). + CheckSequenceMetadataDefinedAtTop(), # assign each node a unique id for indexing/hashing AssignIds(), # based on position of node in tree, figure out which scope it is in @@ -188,16 +248,22 @@ def analyze_ast(body: AstBlock, state: CompileState) -> CompileState: # check that assignment targets are valid CheckAssignSyntax(), # register all user-defined functions in the global callable scope + # (and the builtin library functions in the shared base callable scope) DefineFunctions(), # register all variable declarations in their enclosing scopes. # Function bodies are deferred so that globals declared later in # the source are visible inside functions. DefineVariables(), + # now that every sequence's definitions are registered, bind the + # modules / names each import introduces into the importer's scopes + BindImports(), # check that break/continue are in loops, and store which loop they're in CheckBreakAndContinueInLoop(), CheckReturnInFunc(), ResolveQualifiedIdentifiers(), CheckAllUnqualifiedIdentifiersResolved(), + # warn when the importer uses an underscore-prefixed imported definition + WarnImportUnderscore(), CheckAllTypesAndCallablesResolved(), CheckForConstantSizeTypes(), UpdateStateWithTypes(), @@ -237,17 +303,17 @@ def analyze_ast(body: AstBlock, state: CompileState) -> CompileState: ] for compile_pass in pre_semantic_desugaring_passes: - compile_pass.run(body, state) + compile_pass.run(state.root_block, state) if len(state.errors) != 0: raise state.errors[0] for compile_pass in semantics_passes: - compile_pass.run(body, state) + compile_pass.run(state.root_block, state) if len(state.errors) != 0: raise state.errors[0] for compile_pass in desugaring_passes: - compile_pass.run(body, state) + compile_pass.run(state.root_block, state) if len(state.errors) != 0: raise state.errors[0] @@ -255,7 +321,7 @@ def analyze_ast(body: AstBlock, state: CompileState) -> CompileState: def analysis_to_fpybc_directives( - body: AstBlock, state: CompileState + state: CompileState, ) -> tuple[list[Directive], list[FpyType]]: """Runs fpybc codegen passes on analysis results, returning fpybc directives. @@ -271,11 +337,11 @@ def analysis_to_fpybc_directives( GenerateFunctions(), ] for compile_pass in codegen_passes: - compile_pass.run(body, state) + compile_pass.run(state.root_block, state) if len(state.errors) != 0: raise state.errors[0] - ir = GenerateModule().emit(body, state) + ir = GenerateModule().emit(state.main_block, state) ir_passes: list[IrPass] = [ResolveLabels(), FinalChecks()] for compile_pass in ir_passes: @@ -293,18 +359,13 @@ def analysis_to_fpybc_directives( def analysis_to_llvm_module( - body: AstBlock, state: CompileState + state: CompileState, ) -> tuple[ir.Module, list[FpyType]]: """Runs LLVM codegen passes on analysis results, returning an llvmlite ir.Module (the LLVM backend). Raises BackendError on failure.""" - for compile_pass in []: - compile_pass.run(body, state) - if len(state.errors) != 0: - raise state.errors[0] - - module = GenerateLlvmModule().emit(body, state) + module = GenerateLlvmModule().emit(state.root_block, state) # print out warnings for warning in state.warnings: @@ -314,24 +375,22 @@ def analysis_to_llvm_module( def analysis_to_wasm( - body: AstBlock, state: CompileState, ) -> tuple[bytes, list[FpyType]]: """Runs the LLVM backend and lowers the result to a runnable wasm module. Raises BackendError on failure.""" - module, seq_arg_types = analysis_to_llvm_module(body, state) + module, seq_arg_types = analysis_to_llvm_module(state) return llvm_module_to_wasm(module), seq_arg_types def analysis_to_wat( - body: AstBlock, state: CompileState, ) -> tuple[str, list[FpyType]]: """Runs the LLVM backend and lowers the result to WebAssembly text. Raises BackendError on failure.""" - module, seq_arg_types = analysis_to_llvm_module(body, state) + module, seq_arg_types = analysis_to_llvm_module(state) return llvm_module_to_wasm_text(module), seq_arg_types @@ -343,33 +402,32 @@ def ast_to_dependencies(body: AstBlock, state: CompileState) -> list[str]: Raises CompileError on failure. """ - state.root = body - - pre_builtin_passes = [CheckSequenceMetadataDefinedAtTop()] - for compile_pass in pre_builtin_passes: - compile_pass.run(body, state) - if state.errors: - raise state.errors[0] - - import copy + # Inline imports first so sequence-run dependencies in imported sequences + # are discovered too. + InlineImports().run(body, state) + if state.errors: + raise state.errors[0] - body.stmts = copy.deepcopy(_get_builtin_library_ast().stmts) + body.stmts + # Assemble the compilation unit (sets state.root_block and state.main_block). + _build_compilation_unit(body, state) discovery_passes: list[Visitor] = [ + CheckSequenceMetadataDefinedAtTop(), DesugarCheckStatements(), AssignIds(), CreateScopes(), DefineFunctions(), DefineVariables(), + BindImports(), ResolveQualifiedIdentifiers(), ] for compile_pass in discovery_passes: - compile_pass.run(body, state) + compile_pass.run(state.root_block, state) if state.errors: raise state.errors[0] discover = CollectSequenceDependencies() - discover.run(body, state) + discover.run(state.root_block, state) if state.errors: raise state.errors[0] diff --git a/src/fpy/desugaring.py b/src/fpy/desugaring.py index 1a8bec3..5c041ae 100644 --- a/src/fpy/desugaring.py +++ b/src/fpy/desugaring.py @@ -36,6 +36,7 @@ ) from fpy.symbols import ( FieldAccess, + NameGroup, Symbol, ) from fpy.visitors import Transformer @@ -740,7 +741,12 @@ def _make_func_call( state: CompileState, ) -> AstFuncCall: """Create a function call AST node with proper state.""" - func_symbol = state.global_callable_scope.get(func_name) + # lookup(), not get(): the builtin library functions this desugars to + # (time_add, time_sub, time_cmp, ...) live in the base scope's callable + # group, the parent of main_scope, not in the main sequence's own scope. + + # FIXME i think for correctness sake we should lookup in enclosing callable scope + func_symbol = state.main_scope.lookup(NameGroup.CALLABLE, func_name) assert ( func_symbol is not None ), f"Function {func_name} not found in callable scope" diff --git a/src/fpy/error.py b/src/fpy/error.py index c80bd43..ab4422c 100644 --- a/src/fpy/error.py +++ b/src/fpy/error.py @@ -196,7 +196,7 @@ class WarningType(str, Enum): """The set of diagnostics the compiler may warn about.""" EMPTY_RANGE = "empty-range" - IMPORT_SIDE_EFFECTS = "import-side-effects" + IMPORT_UNDERSCORE = "import-underscore" @classmethod def from_value(cls, value: str) -> "WarningType": diff --git a/src/fpy/grammar.lark b/src/fpy/grammar.lark index 313f9cf..fc79c94 100644 --- a/src/fpy/grammar.lark +++ b/src/fpy/grammar.lark @@ -3,9 +3,7 @@ # input is the top-level rule, consisting of zero or more newlines or top level statements input: (_NEWLINE | _toplevel_stmt)* -# Top-level statements: function definitions OR regular statements -# Function definitions can ONLY appear here, not nested in any control structure -_toplevel_stmt: def_stmt | _stmt | meta_stmt +_toplevel_stmt: def_stmt | _stmt | meta_stmt | import_stmt # Regular statements (no function definitions allowed) # Small statements must be followed by a newline; compound statements end with their block's DEDENT @@ -21,6 +19,24 @@ sequence_stmt_parameters: sequence_stmt_parameter ("," sequence_stmt_parameter)* sequence_stmt_parameter: name ":" expr +# the import statement. imports another Fpy sequence and makes its definitions +# available in the importing sequence. Two forms: +# import [dots] a.b.c [as alias] +# from [dots] a.b.c import (* | m1 [as x], m2, ...) +# The leading dots (DOTS) make an import relative; absence makes it absolute. +import_stmt: "import" [import_dots] dotted_name ["as" name] -> import_stmt + | "from" [import_dots] dotted_name "import" import_star -> import_from_stmt + | "from" [import_dots] dotted_name "import" import_members -> import_from_stmt + | "from" [import_dots] dotted_name "import" "(" import_members ")" -> import_from_stmt +import_dots: DOTS +dotted_name: name ("." name)* +import_star: "*" +import_members: import_member ("," import_member)* ","? +import_member: name ["as" name] +# one or more literal dots, used as the relative-import prefix +DOTS: /\.+/ + + # an indented list of statements block: _NEWLINE _INDENT _stmt+ _DEDENT # -> AstBlock(stmts) diff --git a/src/fpy/imports.py b/src/fpy/imports.py new file mode 100644 index 0000000..4c84477 --- /dev/null +++ b/src/fpy/imports.py @@ -0,0 +1,626 @@ +"""Import statement support. + +Importing sequence S imports sequence T by compiling T's definitions alongside +S's and exposing them to S under a module (or, for `from` imports, directly). An +imported sequence may contain only function definitions and imports -- no +top-level statements, so importing runs no code and order does not matter. + +Isolation is by construction. Each imported sequence is compiled as its own +block, a sibling of the main program under the shared library root; its callable +and value scopes are children of the base/universe scopes the library root owns. +A sequence's own functions live in its own scopes, so they are invisible to any +other sequence except through the modules an import binds, while dictionary and +builtin names still resolve up the parent chain. + +The work is split into two passes: + +* `InlineImports` runs first, on the raw AST. It resolves each import to a file, + recursively parses it, collects its statements as a sibling block in + state.imported_blocks, and records an `ImportBinding` for each import. + _build_compilation_unit then installs those blocks under the library root. + +* `BindImports` runs after `DefineFunctions`/`DefineVariables` have registered + every sequence's definitions. It creates the module chains / direct bindings + each recorded import calls for, handling module merging and name collisions. +""" + +from __future__ import annotations + +from dataclasses import dataclass, field +import os + +import fpy.error +from fpy.error import WarningType +from fpy.symbols import ( + CallableSymbol, + ModuleSymbol, + NameGroup, + Scope, + SymbolTable, + VariableSymbol, +) +from fpy.syntax import ( + AstBlock, + AstDef, + AstGetAttr, + AstIdent, + AstImport, + AstSequenceMetadata, +) +from fpy.state import CompileState +from fpy.types import is_instance_compat +from fpy.visitors import Visitor + + +@dataclass +class SequenceContext: + """A single sequence taking part in a compilation. + + Holds only what an import needs: where the sequence came from (for resolving + its own relative imports) and the AST block it was compiled into..""" + + file_path: str | None + """the sequence's resolved file path (realpath), or None for the main + sequence when it was compiled from a stream.""" + dir_path: str | None + """the directory the sequence lives in, anchoring its relative imports. + None when the sequence has no location.""" + block: AstBlock = None + """the sequence's AST block (the main block, or an imported sequence's + sibling block). Set once the block exists (_build_compilation_unit for the + main sequence, _handle_import for an imported one).""" + + +@dataclass +class ImportBinding: + """A resolved import, recorded for `BindImports` to bind once every + sequence's definitions exist.""" + + node: AstImport + importer: SequenceContext + target: SequenceContext + seq_path: list[str] + """the sequence path segments (leading dots already stripped) that name the + imported file; the module chain an `import` introduces.""" + member: str | None + """the trailing member name for `import a.b.c.foo`, else None.""" + + +def _make_module(is_sequence_module: bool) -> ModuleSymbol: + m = SymbolTable() + m.is_sequence_module = is_sequence_module + return m + + +def _is_sequence_module(m) -> bool: + return m.is_sequence_module + + +class InlineImports: + # FIXME can we split resolving and inlining into separate passes? + """Resolve and inline every import into the main sequence's AST.""" + + def run(self, body: AstBlock, state): + main_ctx = SequenceContext( + file_path=None, + dir_path=state.main_file_dir, + ) + state.main_sequence = main_ctx + + body.stmts = self._inline_stmts( + body.stmts, main_ctx, state, import_stack=[], is_main=True + ) + + def _inline_stmts( + self, stmts, ctx: SequenceContext, state, import_stack, is_main: bool + ): + """Return *stmts* with each import removed. Each import's target sequence + is collected as a sibling block in state.imported_blocks rather than + spliced in at the import's position.""" + result = [] + for stmt in stmts: + if is_instance_compat(stmt, AstImport): + self._ensure_id(stmt, state) + inlined = self._handle_import(stmt, ctx, state, import_stack) + if state.errors: + # FIXME how do we know that it's okay to not return some + # error condition here? + return result + result.extend(inlined) + elif is_instance_compat(stmt, AstSequenceMetadata): + # The main sequence keeps its metadata (handled by the normal + # passes). Imported sequences' metadata was already validated + # in _load_sequence and is dropped. + if is_main: + result.append(stmt) + else: + result.append(stmt) + return result + + def _ensure_id(self, node, state): + # Import nodes are removed before AssignIds runs, but we still use them + # for diagnostics, so give them a unique id up front. + # FIXME can we consider assigning ids at parse time instead of in semantics passes? + if node.id is None: + node.id = state.next_node_id + state.next_node_id += 1 + + def _handle_import( + self, + node: AstImport, + importer: SequenceContext, + state: CompileState, + import_stack, + ): + resolved = self._resolve(node, importer, state) + if resolved is None: + return [] + seq_path, member, file_path = resolved + + # A file currently on the import stack is mid-load: importing it now is a + # cycle. + # FIXME type annotation for import stack? + if file_path in import_stack: + # FIXME would it be easy to make a better err msg for circular imports? showing the cycle? + # If not we can ignore this for now. + state.err( + f"Circular import detected: '{os.path.basename(file_path)}'", + node, + ) + return [] + + # A sequence is compiled once. The first import of a file loads it and + # installs its block; later imports of the same file (in this sequence or + # any other) reuse that one SequenceContext, so its definitions are shared, + # never duplicated. Whether importing the same sequence more than once is + # allowed then falls to the ordinary name-collision rule in BindImports. + target = state.loaded_sequences.get(file_path) + if target is None: + target, target_stmts = self._load_sequence( + file_path, state, import_stack + [file_path] + ) + if state.errors: + return [] + state.loaded_sequences[file_path] = target + + # Collect the imported sequence's statements as a sibling block of the + # main program (installed under the library root by + # _build_compilation_unit). CreateScopes gives this block its own + # isolated scope; the sequence's context points at it via .block. The + # import statement itself contributes nothing at its position -- an + # imported sequence has only definitions (no side effects), so it + # never executes inline. + block = AstBlock(node.meta, target_stmts) + target.block = block + state.imported_blocks.append(block) + + state.import_bindings.append( + ImportBinding(node, importer, target, seq_path, member) + ) + return [] + + def _load_sequence(self, file_path: str, state, import_stack): + """Parse and recursively inline the sequence at *file_path*. + + Returns (SequenceContext, inlined_statements).""" + from fpy.compiler import text_to_ast + + try: + with open(file_path, "r", encoding="utf-8") as f: + text = f.read() + except OSError as e: + state.err(f"Cannot read imported sequence '{file_path}': {e}", None) + return None, [] + + ctx = SequenceContext( + file_path=file_path, + dir_path=os.path.dirname(file_path), + ) + + # Parse the imported file with its own diagnostic context so parse + # errors point into it, then restore the caller's context. + old_file, old_text, old_lines = ( + fpy.error.file_name, + fpy.error.input_text, + fpy.error.input_lines, + ) + fpy.error.file_name = file_path + try: + parsed = text_to_ast(text) + finally: + fpy.error.file_name = old_file + fpy.error.input_text = old_text + fpy.error.input_lines = old_lines + + if parsed is None: + state.err(f"Failed to parse imported sequence '{file_path}'", None) + return ctx, [] + + self._check_metadata(parsed.stmts, state) + if state.errors: + return ctx, [] + + self._check_no_side_effects(parsed.stmts, state) + if state.errors: + return ctx, [] + + stmts = self._inline_stmts( + parsed.stmts, ctx, state, import_stack, is_main=False + ) + return ctx, stmts + + def _check_no_side_effects(self, stmts, state): + """An imported sequence may contain only function definitions and import + statements. Any other top-level statement is a side effect that would + have to execute at the importer's position; since an imported sequence is + a sibling scope block (it does not run inline), such a statement has no + place to execute and is an error.""" + for stmt in stmts: + if not is_instance_compat(stmt, (AstDef, AstImport, AstSequenceMetadata)): + state.err( + "An imported sequence may contain only function definitions " + # FIXME error msg misleading, astdef and astimport and astsequencemeta are top level statements + "and imports, not top-level statements", + stmt, + ) + return + + def _check_metadata(self, stmts, state): + """An imported sequence may declare `sequence()` (no args) as its first + statement; sequence arguments make it un-importable.""" + for i, stmt in enumerate(stmts): + if not is_instance_compat(stmt, AstSequenceMetadata): + continue + if i != 0: + # FIXME won't this already be caught in other semantic passes? + # should we move that pass up? should we have it run as a sub pass of this? + # should this pass handle "importing" the main sequence? i.e. just treat it + # as yet another block? + state.err( + "sequence() definition must be the first statement in the file", + stmt, + ) + return + if stmt.parameters: + state.err( + "Cannot import a sequence that declares sequence arguments", + stmt, + ) + return + + # -- resolution ----------------------------------------------------------- + + def _resolve(self, node: AstImport, importer: SequenceContext, state): + """Resolve an import to (seq_path, member, file_path), or None on error.""" + path = node.path + if node.num_dots > 0: + anchor = importer.dir_path + if anchor is None: + state.err( + "Relative import in a sequence with no containing directory", + node, + ) + return None + for _ in range(node.num_dots - 1): + # FIXME should we use the Path api? is it safer? + anchor = os.path.dirname(anchor) + candidate_dirs = [anchor] + else: + candidate_dirs = list(state.import_search_dirs) + + # Drop duplicate directories (after resolution) so a repeated search + # dir cannot manufacture an ambiguity. + # FIXME I thought that we don't allow duplicate directories? I thought + # that was a compile error? or i guess its just ambiguous files that we dont allow + # FIXME we should do a brief TOCTOU analysis just to find out if that's + # something we need to consider a bit more + seen = set() + deduped = [] + for d in candidate_dirs: + rp = os.path.realpath(d) + if rp not in seen: + seen.add(rp) + deduped.append(d) + candidate_dirs = deduped + + splits = [] + for d in candidate_dirs: + s = self._split_in_dir(d, path, node.is_from) + if s is not None: + splits.append(s) + + if len(splits) == 0: + state.err( + f"Cannot resolve import '{'.'.join(path)}'; no matching sequence found", + node, + ) + return None + if len(splits) > 1: + state.err( + f"Ambiguous import '{'.'.join(path)}': it resolves in more than " + f"one search directory", + node, + ) + return None + return splits[0] + + def _split_in_dir(self, d, path, is_from): + """Return (seq_path, member, file_path) for how *path* splits in *d*, + or None if it does not resolve there.""" + whole = self._file_for(d, path) + if whole is not None: + return (list(path), None, whole) + # `from` paths are always the whole sequence path; only plain/alias + # imports may split a trailing member off. + if not is_from and len(path) > 1: + f = self._file_for(d, path[:-1]) + if f is not None: + return (list(path[:-1]), path[-1], f) + return None + + def _file_for(self, d, segments): + p = os.path.join(d, *segments) + ".fpy" + if os.path.isfile(p): + return os.path.realpath(p) + return None + + +class BindImports: + """Bind each recorded import into its importer's scopes.""" + + def run(self, body, state): + for binding in state.import_bindings: + self._bind(binding, state) + if state.errors: + return + + def _bind(self, binding: ImportBinding, state): + node = binding.node + # FIXME i wonder if there's a way we can simplify this code. + # ideally, I'd like to make it very clear just from looking at + # the code that it's doing the right thing. maybe try to imagine + # an IR that is much more explicit. so transform everything into: + # from file import symbol as sym.path.or.alias + # and then just handle from there? you don't litearlly have to use an + # ir, could just be tuples e.g. + # in general i want to delete functions and helpers and code as much as + # possible, and make everything correspond to the spec, or change the spec + # if it's contrived. + if node.is_from: + self._bind_from(binding, state) + elif node.alias is not None: + self._bind_alias(binding, state) + else: + self._bind_plain(binding, state) + + # -- import forms --------------------------------------------------------- + + def _bind_plain(self, binding: ImportBinding, state): + node = binding.node + importer_scope = state.enclosing_scope[binding.importer.block] + target_scope = state.enclosing_scope[binding.target.block] + if binding.member is not None: + sym = self._lookup_definition(target_scope, binding.member, node, state) + if sym is None: + return + self._maybe_underscore_warn(binding.member, node, state) + leaf = _make_module(is_sequence_module=True) + leaf[binding.member] = sym + else: + leaf = _make_module(is_sequence_module=True) + for name, sym in target_scope.own_symbols().items(): + leaf[name] = sym + + root_name, root_module = self._build_chain(binding.seq_path, leaf) + self._bind_module(importer_scope, root_name, root_module, node, state) + + def _bind_alias(self, binding: ImportBinding, state): + node = binding.node + importer_scope = state.enclosing_scope[binding.importer.block] + target_scope = state.enclosing_scope[binding.target.block] + if binding.member is not None: + sym = self._lookup_definition(target_scope, binding.member, node, state) + if sym is None: + return + self._maybe_underscore_warn(binding.member, node, state) + self._bind_symbol(importer_scope, node.alias, sym, node, state) + else: + module = _make_module(is_sequence_module=True) + for name, sym in target_scope.own_symbols().items(): + module[name] = sym + self._bind_module(importer_scope, node.alias, module, node, state) + + def _bind_from(self, binding: ImportBinding, state): + node = binding.node + importer_scope = state.enclosing_scope[binding.importer.block] + target_scope = state.enclosing_scope[binding.target.block] + if node.is_star: + for name, sym in target_scope.own_symbols().items(): + self._bind_symbol(importer_scope, name, sym, node, state) + if state.errors: + return + if name.startswith("_"): + importer_scope.star_underscore_names.add(name) + return + + for member_name, alias in node.members: + sym = self._lookup_definition(target_scope, member_name, node, state) + if sym is None: + return + self._maybe_underscore_warn(member_name, node, state) + self._bind_symbol(importer_scope, alias or member_name, sym, node, state) + if state.errors: + return + + # -- helpers -------------------------------------------------------------- + + def _lookup_definition(self, target_scope: Scope, name: str, node, state): + sym = target_scope.own_symbols().get(name) + if sym is None: + state.err( + f"Imported sequence has no definition named '{name}'", + node, + ) + return sym + + def _maybe_underscore_warn(self, name: str, node, state): + if name.startswith("_"): + state.warn( + WarningType.IMPORT_UNDERSCORE, + f"'{name}' is a library-internal definition (its name begins " + f"with an underscore)", + node, + ) + + def _build_chain(self, seq_path, leaf): + """Wrap *leaf* in package modules for a dotted sequence path, returning + (root_name, root_module).""" + current = leaf + for i in range(len(seq_path) - 2, -1, -1): + parent = _make_module(is_sequence_module=False) + parent[seq_path[i + 1]] = current + current = parent + return seq_path[0], current + + def _symbol_groups(self, sym) -> set: + if is_instance_compat(sym, ModuleSymbol): + return self._module_groups(sym) + if is_instance_compat(sym, CallableSymbol): + return {NameGroup.CALLABLE} + if is_instance_compat(sym, VariableSymbol): + return {NameGroup.VALUE} + # Any other value-like definition resolves in the value name group. + return {NameGroup.VALUE} + + def _module_groups(self, module) -> set: + groups = set() + for sym in module.values(): + groups |= self._symbol_groups(sym) + return groups + + def _bind_symbol(self, scope: Scope, name, sym, node, state): + """Bind a plain (non-module) definition symbol under *name* into *scope* + (the importer's), erroring on a collision in any of its name groups.""" + + groups = self._symbol_groups(sym) + # lookup(), not get(): the importer's scope is a child of the shared + # base, so a name already taken by a dictionary/builtin definition must + # count as a collision even though it is not in the importer's own scope. + for ng in groups: + if scope.lookup(ng, name) is not None: + state.err( + f"Import of '{name}' collides with an existing definition", + node, + ) + return + for ng in groups: + scope.define(ng, name, sym) + + def _bind_module(self, scope: Scope, name, module, node, state): + """Bind *module* under *name* into *scope* (the importer's), merging with + an existing package module or erroring on a collision.""" + groups = self._module_groups(module) + if not groups: + # An empty sequence's module holds nothing and belongs to no name + # group; there is nothing to bind. + return + + # get(), not lookup(): we only merge with a package module THIS sequence + # already built by import, never with a base (dictionary) namespace. + existing = None + for ng in (NameGroup.CALLABLE, NameGroup.VALUE): + candidate = scope.get(ng, name) + if is_instance_compat(candidate, ModuleSymbol): + existing = candidate + break + + if existing is not None: + if _is_sequence_module(existing) and _is_sequence_module(module): + state.err( + f"Import of '{name}' collides with an existing imported " + f"sequence of the same name", + node, + ) + return + self._merge_modules(existing, module, node, state) + if state.errors: + return + module = existing + groups = self._module_groups(module) + + for ng in groups: + # FIXME: should this be a warning? like other shadowing instances? + # lookup(), not get(): binding a module over a name already taken by + # a base (dictionary) definition is a collision, even though it is + # not in the importer's own scope. The merge above already made an + # own package module the occupant, so lookup returns it (own-scope + # first) and `is not module` stays False for the legitimate merge case. + occupant = scope.lookup(ng, name) + if occupant is not None and occupant is not module: + state.err( + f"Import of '{name}' collides with an existing definition", + node, + ) + return + scope.define(ng, name, module) + + def _merge_modules(self, existing, incoming, node, state): + """Merge *incoming* module's members into *existing*.""" + for key, sym in incoming.items(): + if key in existing: + ex = existing[key] + if ( + is_instance_compat(ex, ModuleSymbol) + and is_instance_compat(sym, ModuleSymbol) + and not (_is_sequence_module(ex) and _is_sequence_module(sym)) + ): + self._merge_modules(ex, sym, node, state) + if state.errors: + return + else: + state.err( + f"Imported module member '{key}' collides on merge", + node, + ) + return + else: + existing[key] = sym + if _is_sequence_module(incoming): + existing.is_sequence_module = True + + +class WarnImportUnderscore(Visitor): + """Warn when the importer *uses* an underscore-prefixed imported definition. + + Covers `module._helper` member access and a bare use of a name that a + `from ... import *` brought in under an underscore name. Import statements + that name an underscore member warn at bind time in `BindImports`.""" + + def visit_AstGetAttr(self, node: AstGetAttr, state): + if not node.attr.startswith("_"): + return + parent_sym = state.resolved_symbols.get(node.parent) + if is_instance_compat(parent_sym, ModuleSymbol): + state.warn( + WarningType.IMPORT_UNDERSCORE, + f"'{node.attr}' is a library-internal definition (its name " + f"begins with an underscore)", + node, + ) + + def visit_AstIdent(self, node: AstIdent, state): + if not node.name.startswith("_"): + return + # star_underscore_names lives on the sequence's root scope -- the scope + # in this node's chain whose parent is the shared base scope. Walk up to + # it (a node under no sequence, e.g. a builtin lib def, finds none). + scope = state.enclosing_scope[node] + while scope is not None and scope.parent is not state.base_scope: + scope = scope.parent + if scope is not None and node.name in scope.star_underscore_names: + state.warn( + WarningType.IMPORT_UNDERSCORE, + f"'{node.name}' is a library-internal definition (its name " + f"begins with an underscore)", + node, + ) diff --git a/src/fpy/main.py b/src/fpy/main.py index ce088c1..aadcc59 100644 --- a/src/fpy/main.py +++ b/src/fpy/main.py @@ -116,16 +116,16 @@ def compile_main(args: list[str] = None): metavar="DIR", dest="include", help=( - "Directory to search when resolving `import` statements (repeatable). " - "The importing file's own directory is always searched first; each " - "--include directory is searched afterwards, in the order given" + "Directory to search when resolving absolute `import` statements " + "(repeatable). An import that resolves in more than one --include " + "directory is ambiguous and an error." ), ) arg_parser.add_argument( "--ignore", type=str, default="", - help="Comma-separated warning types to silence (e.g. 'import-side-effects,empty-range'), or 'all'", + help="Comma-separated warning types to silence (e.g. 'import-underscore,empty-range'), or 'all'", ) arg_parser.add_argument( "--error", @@ -165,11 +165,14 @@ def compile_main(args: list[str] = None): if ground_binary_dir is None: ground_binary_dir = parsed_args.input.parent - # imports resolve against the importing file's own directory first, then - # each -i/--include directory in the order given - import_search_dirs = [str(parsed_args.input.parent.resolve())] + [ - str(d.resolve()) for d in parsed_args.include - ] + # absolute imports resolve against the -i/--include directories only; the + # input file's own directory anchors its relative imports but is not on + # the base search path. Exact duplicate directories are dropped: a + # repeated -i flag carries no information and must not manufacture an + # ambiguity error. + import_search_dirs = list( + dict.fromkeys(str(d.resolve()) for d in parsed_args.include) + ) # reading dictionary try: @@ -179,6 +182,7 @@ def compile_main(args: list[str] = None): ignored_warnings=ignored_warnings, error_warnings=error_warnings, import_search_dirs=import_search_dirs, + main_file_dir=str(parsed_args.input.parent.resolve()), ) except fpy.error.DictionaryError as e: print(e, file=sys.stderr) @@ -207,13 +211,13 @@ def compile_main(args: list[str] = None): # codegen try: if parsed_args.emit == "llvm-ir": - output, seq_arg_types = analysis_to_llvm_module(body, state) + output, seq_arg_types = analysis_to_llvm_module(state) elif parsed_args.emit == "wasm": - output, seq_arg_types = analysis_to_wasm(body, state) + output, seq_arg_types = analysis_to_wasm(state) elif parsed_args.emit == "wat": - output, seq_arg_types = analysis_to_wat(body, state) + output, seq_arg_types = analysis_to_wat(state) elif parsed_args.emit in ["fpybin", "fpyasm"]: - output, seq_arg_types = analysis_to_fpybc_directives(body, state) + output, seq_arg_types = analysis_to_fpybc_directives(state) else: assert False, parsed_args.emit except fpy.error.BackendError as e: @@ -538,7 +542,7 @@ def cmd_main(args: list[str] = None): try: state = analyze_ast(body, state) - directives, _ = analysis_to_fpybc_directives(body, state) + directives, _ = analysis_to_fpybc_directives(state) except RecursionError: print("Recursion limit exceeded in compiling", file=sys.stderr) sys.exit(1) @@ -620,6 +624,16 @@ def depend_main(args: list[str] = None): default=None, help="Local directory to resolve .bin file paths for sequence calls (default: input file directory)", ) + arg_parser.add_argument( + "-i", + "--include", + type=Path, + action="append", + default=[], + metavar="DIR", + dest="include", + help="Directory to search when resolving absolute `import` statements (repeatable)", + ) if args is not None: parsed_args = arg_parser.parse_args(args) else: @@ -634,10 +648,16 @@ def depend_main(args: list[str] = None): if ground_binary_dir is None: ground_binary_dir = parsed_args.input.parent + import_search_dirs = list( + dict.fromkeys(str(d.resolve()) for d in parsed_args.include) + ) + try: state = get_base_compile_state( str(parsed_args.dictionary.resolve()), str(ground_binary_dir.resolve()), + import_search_dirs=import_search_dirs, + main_file_dir=str(parsed_args.input.parent.resolve()), ) except fpy.error.DictionaryError as e: print(e, file=sys.stderr) diff --git a/src/fpy/semantics.py b/src/fpy/semantics.py index 4b24b95..0c05150 100644 --- a/src/fpy/semantics.py +++ b/src/fpy/semantics.py @@ -59,8 +59,8 @@ FunctionSymbol, NameGroup, ModuleSymbol, + Scope, Symbol, - SymbolTable, TypeCtorSymbol, VariableSymbol, is_symbol_an_expr, @@ -165,39 +165,53 @@ def visit_default(self, node, state: CompileState): class CreateScopes(TopDownVisitor): - """Creates block-level scopes for all AstBlocks. + """Creates the Scope for every AstBlock - Each AstBlock creates a new scope that is a child of the enclosing scope. - Non-block nodes inherit the scope of their enclosing block. + Every block gets a fresh Scope that is a child of its enclosing block's + scope, so scope nesting exactly follows syntactic nesting. The one exception + is the library root, which has no enclosing block: it owns the pre-built base + scope (dictionary/builtin symbols, created before any AST existed). + + Non-block nodes inherit the scope from their enclosing block. """ def visit_default(self, node: Ast, state: CompileState): parent = state.parent_map.get(node) - if parent is not None: - state.enclosing_value_scope[node] = state.enclosing_value_scope[parent] + if parent is None: + return + state.enclosing_scope[node] = state.enclosing_scope[parent] def visit_AstBlock(self, node: AstBlock, state: CompileState): - if node is state.root: - state.enclosing_value_scope[node] = state.global_value_scope - else: - parent = state.parent_map[node] - parent_scope = state.enclosing_value_scope[parent] - block_scope = SymbolTable(parent=parent_scope) - if isinstance(parent, AstDef): - block_scope.in_function = True - state.enclosing_value_scope[node] = block_scope + parent = state.parent_map.get(node) + + if parent is None: + # The base block has no enclosing block, so it cannot make a child + # scope. It owns the pre-built base scope (dictionary/builtin symbols, + # created before any AST existed) instead. + state.enclosing_scope[node] = state.base_scope + return + + parent_scope = state.enclosing_scope[parent] + in_function = parent_scope.in_function or isinstance(parent, AstDef) + scope = Scope(parent=parent_scope, in_function=in_function) + state.enclosing_scope[node] = scope + + # The main block's scope is the main sequence's scope + if node is state.main_block: + state.main_scope = scope class CheckSequenceMetadataDefinedAtTop(TopDownVisitor): """ Ensure that sequence() statement is at the top of the user's sequence. - This pass runs BEFORE builtin functions are inserted, so we check the raw user code. + We check only the main block, whose statements are the raw user code. This avoids + taking into account the automatically prepended compile time library. If a sequence() definition exists, it must be the very first statement. """ def visit_AstBlock(self, node: AstBlock, state: CompileState): - # Only check the root block (top-level sequence) - if node is not state.root: + # Only check the main block (the user's top-level sequence) + if node is not state.main_block: return # Walk through statements in order @@ -273,8 +287,16 @@ def visit_AstSequenceMetadata(self, node: AstSequenceMetadata, state: CompileSta class DefineFunctions(TopDownVisitor): def visit_AstDef(self, node: AstDef, state: CompileState): - # Functions always go in the global callable scope - existing_func = state.global_callable_scope.get(node.name.name) + # Functions go in their node's enclosing scope's callable group. + scope = state.enclosing_scope[node] + # lookup(), not get(): a sequence's scope is a child of the shared base, + # so a name already taken by a dictionary command, cast or type + # constructor is not free just because this sequence's own callable group + # is empty. Redefinition must be rejected up the parent chain too. + + # FIXME so basically we're disallowing shadowing here? should we disallow shadowing + # for variables too? maybe we either make both a warning or both an error. + existing_func = scope.lookup(NameGroup.CALLABLE, node.name.name) if existing_func is not None: state.err( f"Function '{node.name.name}' has already been defined", node.name @@ -291,7 +313,7 @@ def visit_AstDef(self, node: AstDef, state: CompileState): definition=node, ) - state.global_callable_scope[func.name] = func + scope.define(NameGroup.CALLABLE, func.name, func) class DefineVariables(TopDownVisitor): @@ -320,13 +342,33 @@ def run(self, start: Ast, state: CompileState): def define_variable( self, sym: VariableSymbol, - scope: SymbolTable, + scope: Scope, state: CompileState, variable_kind: str, assert_undeclared: bool = False, ): - # make sure it isn't defined in this scope (shadowing parent scopes is ok) - existing_local = scope.get(sym.name) + # A variable is global if it is declared directly in the main sequence's + # root scope, not in a nested block or function scope. Only the main + # sequence can declare top-level variables: imported sequences may hold + # only definitions and imports (no top-level statements) and may not take + # sequence arguments, so their root scope never gains a variable. + is_root = scope is state.main_scope + + # In a block scope, shadowing an outer or global name is allowed, so only + # the block's own value group blocks a redeclaration. In the main + # sequence's root scope, the parent is the shared base (dictionary + # channels, params, `flags`, ...); a top-level name must not shadow those, + # so consult the whole chain -- matching how DefineFunctions checks + # callable names. + + # FIXME no, I think either it should allow shadowing with warnings, or + # error. + + existing_local = ( + scope.lookup(NameGroup.VALUE, sym.name) + if is_root + else scope.get(NameGroup.VALUE, sym.name) + ) if existing_local is not None: if assert_undeclared: @@ -338,10 +380,10 @@ def define_variable( ) return - sym.is_global = scope is state.global_value_scope + sym.is_global = is_root # new var. put it in the scope - scope[sym.name] = sym + scope.define(NameGroup.VALUE, sym.name, sym) def visit_AstAssign(self, node: AstAssign, state: CompileState): @@ -349,15 +391,14 @@ def visit_AstAssign(self, node: AstAssign, state: CompileState): # not a variable definition return - scope = state.enclosing_value_scope[node] + scope = state.enclosing_scope[node] # yes a variable definition - self.define_variable( - VariableSymbol(node.lhs.name, node.type_ann, node), scope, state, "Variable" - ) + sym = VariableSymbol(node.lhs.name, node.type_ann, node) + self.define_variable(sym, scope, state, "Variable") def visit_AstFor(self, node: AstFor, state: CompileState): # The loop variable is always a new declaration in the loop body's scope. - body_scope = state.enclosing_value_scope[node.body] + body_scope = state.enclosing_scope[node.body] self.define_variable( loop_var := VariableSymbol(node.loop_var.name, None, node, LoopVarType), @@ -385,7 +426,7 @@ def visit_AstFor(self, node: AstFor, state: CompileState): def visit_AstDef(self, node: AstDef, state: CompileState): # Parameters go in the function's enclosing scope - body_scope = state.enclosing_value_scope[node.body] + body_scope = state.enclosing_scope[node.body] for arg in node.parameters or []: arg_name_var, arg_type_name, _ = arg @@ -399,7 +440,7 @@ def visit_AstDef(self, node: AstDef, state: CompileState): return STOP_DESCENT def visit_AstSequenceMetadata(self, node: AstSequenceMetadata, state: CompileState): - scope = state.enclosing_value_scope[node] + scope = state.enclosing_scope[node] for arg in node.parameters or []: arg_name_var, arg_type_name = arg @@ -486,23 +527,12 @@ def try_resolve_ident( if not is_instance_compat(node, AstIdent): return True - # Resolve the root identifier in the appropriate scope - if ng == NameGroup.CALLABLE: - scope = state.global_callable_scope - elif ng == NameGroup.TYPE: - scope = state.global_type_scope - else: - scope = state.enclosing_value_scope[node] - - resolved = None - while scope is not None: - resolved = scope.get(node.name) - if resolved is not None: - break - scope = scope.parent + # Resolve the root identifier in the node's enclosing scope, in the + # requested name group, walking the parent chain. + resolved = state.enclosing_scope[node].lookup(ng, node.name) if resolved is None: - state.err(f"Unknown {ng} '{node.name}'", node) + state.err(f"Unknown {ng.value} '{node.name}'", node) return False # root identifier was successfully resolved @@ -537,9 +567,9 @@ def visit_AstDef(self, node: AstDef, state: CompileState): if node.parameters is not None: # Params are defined in the function body's scope by DefineVariables - body_scope = state.enclosing_value_scope[node.body] + body_values = state.enclosing_scope[node.body].group(NameGroup.VALUE) for arg_name_var, arg_type_name, default_value in node.parameters: - state.resolved_symbols[arg_name_var] = body_scope[arg_name_var.name] + state.resolved_symbols[arg_name_var] = body_values[arg_name_var.name] if not self.try_resolve_ident(arg_type_name, NameGroup.TYPE, state): return if default_value is not None: @@ -563,9 +593,9 @@ def visit_AstSequenceMetadata(self, node: AstSequenceMetadata, state: CompileSta if node.parameters is None: return - scope = state.enclosing_value_scope[node] + values = state.enclosing_scope[node].group(NameGroup.VALUE) for arg_name_var, arg_type_name in node.parameters: - state.resolved_symbols[arg_name_var] = scope[arg_name_var.name] + state.resolved_symbols[arg_name_var] = values[arg_name_var.name] if not self.try_resolve_ident(arg_type_name, NameGroup.TYPE, state): return @@ -601,8 +631,8 @@ def visit_AstUnaryOp(self, node: AstUnaryOp, state: CompileState): def visit_AstFor(self, node: AstFor, state: CompileState): # loop_var is defined in the body's scope by DefineVariables - body_scope = state.enclosing_value_scope[node.body] - state.resolved_symbols[node.loop_var] = body_scope[node.loop_var.name] + body_values = state.enclosing_scope[node.body].group(NameGroup.VALUE) + state.resolved_symbols[node.loop_var] = body_values[node.loop_var.name] # this really shouldn't be possible to be a var right now # but this is future proof @@ -697,13 +727,13 @@ class CheckAllTypesAndCallablesResolved(Visitor): def check_resolved(self, node: AstExpr, ng: NameGroup, state: CompileState) -> bool: sym = state.resolved_symbols.get(node) if sym is None: - state.err(f"Unknown {ng}", node) + state.err(f"Unknown {ng.value}", node) return False if ng == NameGroup.CALLABLE and not is_instance_compat(sym, CallableSymbol): - state.err(f"Expected a {ng}", node) + state.err(f"Expected a {ng.value}", node) return False if ng == NameGroup.TYPE and not is_instance_compat(sym, FpyType): - state.err(f"Expected a {ng}", node) + state.err(f"Expected a {ng.value}", node) return False return True @@ -925,7 +955,7 @@ def visit_AstIdent(self, node: AstIdent, state: CompileState): # Global variables referenced from inside a function are always # accessible — they are allocated and zero-initialized at sequence # start, regardless of textual ordering. - if sym.is_global and state.enclosing_value_scope[node].in_function: + if sym.is_global and state.enclosing_scope[node].in_function: return state.err(f"'{node.name}' used before defined", node) return @@ -1028,7 +1058,7 @@ def __init__(self): self.defined: list[VariableSymbol] = [] def visit_AstFuncCall(self, node: AstFuncCall, state: CompileState): - if state.enclosing_value_scope[node].in_function: + if state.enclosing_scope[node].in_function: # checked transitively at the top-level call that reaches this one return sym = state.resolved_symbols.get(node.func) @@ -1590,6 +1620,18 @@ def visit_AstIdent(self, node: AstIdent, state: CompileState): if sym is None: return if not is_symbol_an_expr(sym): + # FIXME explain this, why do we need this? point me to a test. we really should not be using parent + # map. + + # A module used directly as a value is invalid, unless it is the + # qualifier of a member access (that getattr resolves it). + if is_instance_compat(sym, ModuleSymbol): + parent = state.parent_map.get(node) + is_qualifier = ( + is_instance_compat(parent, AstGetAttr) and parent.parent is node + ) + if not is_qualifier: + state.err("A module cannot be used as a value", node) return sym_type = self.get_type_of_symbol(sym) diff --git a/src/fpy/state.py b/src/fpy/state.py index aa0be9c..1b03237 100644 --- a/src/fpy/state.py +++ b/src/fpy/state.py @@ -11,8 +11,9 @@ CallableSymbol, CastSymbol, CommandSymbol, + NameGroup, + Scope, Symbol, - SymbolTable, TypeCtorSymbol, VariableSymbol, create_symbol_table, @@ -66,13 +67,9 @@ class ForLoopAnalysis: class CompileState: """a collection of input, internal and output state variables and maps""" - global_type_scope: SymbolTable - """The global type scope: a symbol table whose leaf nodes are FpyType instances.""" - global_callable_scope: SymbolTable - """The global callable scope: a symbol table whose leaf nodes are CallableSymbol instances.""" - global_value_scope: SymbolTable - """The global value scope: a symbol table whose leaf nodes are runtime values - (telemetry channels, parameters, enum constants, variables).""" + main_scope: Scope = None + """The main sequence's scope: a child of `base_scope` holding the main + sequence's own top-level definitions. None until CreateScopes runs""" type_defs: dict = field(default_factory=dict) """Flat map of fully-qualified type name to FpyType, for resolving types at compile time.""" @@ -83,13 +80,23 @@ class CompileState: max_directive_size: int = DEFAULT_MAX_DIRECTIVE_SIZE next_node_id: int = 0 - root: AstBlock = None + root_block: AstBlock = None + """the outermost block: the library root, which owns the base scope and holds + the builtin library functions, the main block, and every imported sequence + block as its children. Built by _build_compilation_unit.""" + main_block: AstBlock = None + """the main sequence's block (the executable body), a child of `root`. The + frame layout and directive stream are generated from this block, not `root` + (which also contains the builtin library and the imported sequences).""" + imported_blocks: list = field(default_factory=list, repr=False) + """every imported sequence's block, collected by InlineImports. They are + installed as children of the library `root` (siblings of `main_block`), so + each imported sequence is an isolated scope block that does not execute inline.""" parent_map: dict[Ast, Ast] = field(default_factory=dict, repr=False) """map of each node to its parent node in the AST""" - enclosing_value_scope: dict[Ast, SymbolTable] = field( - default_factory=dict, repr=False - ) - """map of node to its enclosing value scope (block scope, function scope, or global_value_scope)""" + enclosing_scope: dict[Ast, Scope] = field(default_factory=dict, repr=False) + """map of node to its enclosing Scope (block scope, function scope, a + sequence's root scope, or the base scope).""" for_loops: dict[AstFor, ForLoopAnalysis] = field(default_factory=dict) """map of for loops to a ForLoopAnalysis struct, which contains additional info about the loops""" enclosing_loops: dict[Union[AstBreak, AstContinue], Union[AstFor, AstWhile]] = ( @@ -183,9 +190,31 @@ class CompileState: """warning types to promote to hard compile errors (`--error`)""" import_search_dirs: list[str] = field(default_factory=list) - """ordered list of directories searched to resolve `import` statements to - source files (first match wins). Populated from `-i/--include` in the CLI (in addition to the - importing file's own directory).""" + """the base import search path: directories searched to resolve absolute + `import` statements to source files. Resolving in more than one of them is + an ambiguity error. Populated from `-i/--include` in the CLI.""" + + main_file_dir: str | None = None + """directory containing the main sequence, used to anchor its relative + imports. None when compiling from a stream (a relative import in the main + sequence is then an error).""" + + base_scope: Scope = None + """the dictionary/builtin scope: dictionary types, commands/casts/type + constructors/macros (callable group), and dictionary channels, params, enum + constants, constants and `flags` (value group), plus the builtin library + functions (builtin/time.fpy) that DefineFunctions registers via the library + root block.""" + + main_sequence: object = None + """the SequenceContext for the main (top-level) sequence being compiled.""" + import_bindings: list = field(default_factory=list, repr=False) + """recorded imports (ImportBinding) to bind into scopes once every sequence's + definitions have been registered (by DefineFunctions / DefineVariables).""" + loaded_sequences: dict = field(default_factory=dict, repr=False) + """maps a resolved sequence file (realpath) -> its SequenceContext. A sequence + is loaded and compiled once; every import that resolves to the same file + reuses this context, so its definitions are shared, never duplicated.""" next_anon_var_id: int = 0 @@ -584,6 +613,7 @@ def get_base_compile_state( ignored_warnings: set[WarningType] | None = None, error_warnings: set[WarningType] | None = None, import_search_dirs: list[str] | None = None, + main_file_dir: str | None = None, ) -> CompileState: """return the initial state of the compiler, based on the given dict path""" type_scope, callable_scope, values_scope, type_defs = _build_global_scopes( @@ -601,14 +631,23 @@ def _const_int(key: str, default: int) -> int: ), f"Expected int for constant {key}, got {type(val.val)}" return val.val - # Make copies of the scopes since we'll mutate them during compilation - # (e.g., adding user-defined functions to callable_scope, variables to values_scope) - # if we don't make copies, then the lru cache will return the modified versions, causing - # two runs of the compiler to conflict + # The base scope holds the dictionary/builtin names, split by name group: + # types in the type group; commands, casts, type constructors and macros in + # the callable group (plus the builtin library functions once compiled); and + # dictionary channels, params, enum constants, constants and `flags` in the + # value group. It is the parent of every sequence's scope -- the main + # sequence's main_scope and every imported sequence's -- so those names + # resolve up the parent chain for all of them, while each sequence's own + # definitions live in its own child scope, isolated from the rest. + # + # The group dicts are copied out of the (lru-cached) module tables so that + # mutating the base scope during a compile does not corrupt the cache. + base_scope = Scope() + base_scope.group(NameGroup.TYPE).update(type_scope) + base_scope.group(NameGroup.CALLABLE).update(callable_scope) + base_scope.group(NameGroup.VALUE).update(values_scope) + state = CompileState( - global_type_scope=type_scope, # types are not mutated - global_callable_scope=callable_scope.copy(), - global_value_scope=values_scope.copy(), type_defs=type_defs, ground_binary_dir=ground_binary_dir, max_directives_count=_const_int( @@ -620,12 +659,15 @@ def _const_int(key: str, default: int) -> int: ignored_warnings=set(ignored_warnings) if ignored_warnings else set(), error_warnings=set(error_warnings) if error_warnings else set(), import_search_dirs=list(import_search_dirs) if import_search_dirs else [], + main_file_dir=main_file_dir, ) + state.base_scope = base_scope - # Create the built-in 'flags' variable ($Flags struct). - # declaration=None marks it as a built-in that is always defined. + # Create the built-in 'flags' variable ($Flags struct). declaration=None + # marks it as a built-in that is always defined. It lives in the base scope's + # value group, so it is shared across all sequences. flags_var = VariableSymbol("flags", None, None, FLAGS_TYPE, is_global=True) - state.global_value_scope["flags"] = flags_var + base_scope.define(NameGroup.VALUE, "flags", flags_var) state.flags_var = flags_var return state diff --git a/src/fpy/symbols.py b/src/fpy/symbols.py index 25e4ba1..8487da9 100644 --- a/src/fpy/symbols.py +++ b/src/fpy/symbols.py @@ -128,6 +128,12 @@ def __init__(self, parent=None): next_symbol_table_id += 1 self.parent = parent self.in_function = parent.in_function if parent is not None else False + # FIXME confusing... should be a module class outright + self.is_sequence_module = False + """when this table is used as a ModuleSymbol, True marks it as a sequence + module (the leaf holding an imported file's definitions, which never + merges) rather than a package module (a chain intermediate, which does). + Meaningless (and left False) for non-module symbol tables.""" def __getitem__(self, key: str) -> "Symbol": return super().__getitem__(key) @@ -154,10 +160,64 @@ def copy(self): """Return a shallow copy that preserves SymbolTable metadata.""" new = SymbolTable(parent=self.parent) new.in_function = self.in_function + new.is_sequence_module = self.is_sequence_module new.update(self) return new +class Scope: + """A lexical scope. + + Name resolution is name-group-directed: a use site knows whether it wants a + type, a callable, or a value, and the same identifier text may name a + different symbol in each group. A Scope therefore holds three independent + namespaces (one per NameGroup) plus a parent link; a lookup consults the + requested group, walking the parent chain.""" + + def __init__(self, parent: "Scope | None" = None, in_function: bool | None = None): + self.parent = parent + if in_function is None: + in_function = parent.in_function if parent is not None else False + self.in_function = in_function + self._groups: dict[NameGroup, dict[str, "Symbol"]] = { + ng: {} for ng in NameGroup + } + self.star_underscore_names: set[str] = set() + """underscore-prefixed names bound into this scope via `from ... import *`; + a later bare use of one warns. Only a sequence's root scope carries any.""" + + def group(self, ng: NameGroup) -> dict[str, "Symbol"]: + """The name->symbol dict for one name group (mutable).""" + return self._groups[ng] + + def own_symbols(self) -> dict[str, "Symbol"]: + """The symbols defined directly in this scope (no parent walk), flattened + across name groups. Used to enumerate a sequence's own top-level + definitions -- functions, globals, and re-exported modules -- when an + import binds the whole sequence.""" + merged = dict(self._groups[NameGroup.CALLABLE]) + merged.update(self._groups[NameGroup.VALUE]) + return merged + + def define(self, ng: NameGroup, name: str, sym: "Symbol"): + """Bind *name* to *sym* in this scope's *ng* group.""" + self._groups[ng][name] = sym + + def get(self, ng: NameGroup, name: str): + """Look up *name* in this scope's *ng* group only (no parent walk).""" + return self._groups[ng].get(name) + + def lookup(self, ng: NameGroup, name: str): + """Look up *name* in the *ng* group of this scope and its ancestors.""" + scope = self + while scope is not None: + sym = scope._groups[ng].get(name) + if sym is not None: + return sym + scope = scope.parent + return None + + def create_symbol_table(symbols: dict[str, "Symbol"]) -> SymbolTable: """from a flat dict of strs to symbols, creates a hierarchical symbol table. no two leaf nodes may have the same name""" diff --git a/src/fpy/syntax.py b/src/fpy/syntax.py index 72a3654..5cc27cd 100644 --- a/src/fpy/syntax.py +++ b/src/fpy/syntax.py @@ -329,6 +329,29 @@ class AstSequenceMetadata(Ast): parameters: Union[list[tuple[AstIdent, AstExpr]], None] +@dataclass +class AstImport(Ast): + """An import statement. Only valid as a top-level statement. + + `import [dots] a.b.c [as alias]` and + `from [dots] a.b.c import (* | m1 [as x], ...)`. + """ + + is_from: bool + """True for a `from` import, False for a plain `import`.""" + num_dots: int + """Number of leading dots. 0 means absolute; >0 means relative.""" + path: list[str] + """The dotted path segments after any leading dots (at least one).""" + alias: Union[str, None] + """The `as` alias for a plain `import ... as alias`, else None.""" + members: Union[list[tuple[str, Union[str, None]]], None] + """For a `from` import: list of (member_name, alias_or_None). None for a + plain import. Empty/None when `is_star` is True.""" + is_star: bool + """True for `from ... import *`.""" + + AstStmt = Union[ AstExpr, AstAssign, @@ -525,6 +548,67 @@ def handle_sequence_argument(meta, args): return (name, type_expr) +# Sentinel marking a `from ... import *` target. +_IMPORT_STAR = object() + + +def handle_import_dots(meta, children): + """Count the leading dots of a relative import (the DOTS token).""" + return len(children[0]) + + +def handle_dotted_name(meta, children): + """Collect a dotted path's segment names (the `name` children are AstIdent).""" + return [str(c.name) for c in children] + + +def handle_import_member(meta, children): + """Parse a `from` import member: (name, alias_or_None).""" + name = children[0] + alias = children[1] if len(children) > 1 else None + return (str(name.name), str(alias.name) if alias is not None else None) + + +def handle_import_stmt(meta, children): + """Build an AstImport for `import [dots] a.b.c [as alias]`.""" + dots, path, alias = children + num_dots = dots if dots is not None else 0 + return AstImport( + meta, + is_from=False, + num_dots=num_dots, + path=path, + alias=(str(alias.name) if alias is not None else None), + members=None, + is_star=False, + ) + + +def handle_import_from_stmt(meta, children): + """Build an AstImport for `from [dots] a.b.c import (* | members)`.""" + dots, path, targets = children + num_dots = dots if dots is not None else 0 + if targets is _IMPORT_STAR: + return AstImport( + meta, + is_from=True, + num_dots=num_dots, + path=path, + alias=None, + members=None, + is_star=True, + ) + return AstImport( + meta, + is_from=True, + num_dots=num_dots, + path=path, + alias=None, + members=targets, + is_star=False, + ) + + @v_args(meta=True, inline=True) class FpyTransformer(Transformer): input = no_inline(AstBlock) @@ -596,6 +680,17 @@ def positional_argument(self, meta, value): sequence_stmt_parameters = no_inline_or_meta(list) sequence_stmt_parameter = no_inline(handle_sequence_argument) + import_stmt = no_inline(handle_import_stmt) + import_from_stmt = no_inline(handle_import_from_stmt) + import_dots = no_inline(handle_import_dots) + dotted_name = no_inline(handle_dotted_name) + import_member = no_inline(handle_import_member) + import_members = no_inline_or_meta(list) + + @v_args(meta=True, inline=True) + def import_star(self, meta): + return _IMPORT_STAR + NAME = lambda self, token: token[1:] if token.startswith("$") else token DEC_NUMBER = int FLOAT_NUMBER = Decimal diff --git a/src/fpy/test_helpers.py b/src/fpy/test_helpers.py index 5afebb5..0e82a00 100644 --- a/src/fpy/test_helpers.py +++ b/src/fpy/test_helpers.py @@ -47,12 +47,9 @@ def compile_seq( ignored_warnings=None, error_warnings=None, import_search_dirs: list[str] | None = None, + main_file_dir: str | None = None, ) -> tuple[CompileState, list[Directive], list[tuple[str, FpyType]]]: - """Compile a sequence string and return (state, directives, arg_types). - - Exposes the CompileState so tests can inspect collected warnings. - Raises CompilationFailed on a compile/backend error (which is also how an - `error_warnings`-escalated warning surfaces).""" + """Compile a sequence string and return (state, directives, arg_types).""" fpy.error.file_name = "" state = get_base_compile_state( @@ -61,12 +58,13 @@ def compile_seq( ignored_warnings=ignored_warnings, error_warnings=error_warnings, import_search_dirs=import_search_dirs, + main_file_dir=main_file_dir, ) try: body = text_to_ast(seq) state = analyze_ast(body, state) - directives, arg_types = analysis_to_fpybc_directives(body, state) + directives, arg_types = analysis_to_fpybc_directives(state) except (fpy.error.CompileError, fpy.error.BackendError) as e: raise CompilationFailed(f"Compilation failed:\n{e}") @@ -79,6 +77,7 @@ def compile_seq_wasm( import_search_dirs: list[str] | None = None, ignored_warnings=None, error_warnings=None, + main_file_dir: str | None = None, ) -> bytes: """Compile a sequence string to a runnable wasm binary (the LLVM backend).""" fpy.error.file_name = "" @@ -89,12 +88,13 @@ def compile_seq_wasm( ignored_warnings=ignored_warnings, error_warnings=error_warnings, import_search_dirs=import_search_dirs, + main_file_dir=main_file_dir, ) try: body = text_to_ast(seq) state = analyze_ast(body, state) - wasm, _ = analysis_to_wasm(body, state) + wasm, _ = analysis_to_wasm(state) except (fpy.error.CompileError, fpy.error.BackendError) as e: raise CompilationFailed(f"Compilation failed:\n{e}") @@ -102,7 +102,10 @@ def compile_seq_wasm( def run_seq_wasm( - seq: str, ground_binary_dir: str = None, import_search_dirs: list[str] | None = None + seq: str, + ground_binary_dir: str = None, + import_search_dirs: list[str] | None = None, + main_file_dir: str | None = None, ) -> int: """Compile *seq* to wasm and run it, returning fpy_main's error code. @@ -115,7 +118,10 @@ def run_seq_wasm( ), "SPACEWASM_RUNNER not set; run pytest with --wasm" wasm = compile_seq_wasm( - seq, ground_binary_dir, import_search_dirs=import_search_dirs + seq, + ground_binary_dir, + import_search_dirs=import_search_dirs, + main_file_dir=main_file_dir, ) wasm_file = tempfile.NamedTemporaryFile(suffix=".wasm", delete=False) wasm_file.write(wasm) @@ -285,18 +291,23 @@ def assert_run_success( ground_binary_dir: str = None, seq_run_opcodes: set[int] = None, import_search_dirs: list[str] | None = None, + main_file_dir: str | None = None, ): if USE_WASM: code = run_seq_wasm( seq, ground_binary_dir=ground_binary_dir, import_search_dirs=import_search_dirs, + main_file_dir=main_file_dir, ) if code != DirectiveErrorCode.NO_ERROR.value: raise RuntimeError(f"wasm sequence returned error code {code}") return _, directives, arg_name_types = compile_seq( - seq, ground_binary_dir=ground_binary_dir, import_search_dirs=import_search_dirs + seq, + ground_binary_dir=ground_binary_dir, + import_search_dirs=import_search_dirs, + main_file_dir=main_file_dir, ) arg_types = [t for _, t in arg_name_types] args_bytes = None @@ -330,6 +341,7 @@ def assert_compile_failure( import_search_dirs: list[str] | None = None, ignored_warnings=None, error_warnings=None, + main_file_dir: str | None = None, ): try: if USE_WASM: @@ -339,6 +351,7 @@ def assert_compile_failure( import_search_dirs=import_search_dirs, ignored_warnings=ignored_warnings, error_warnings=error_warnings, + main_file_dir=main_file_dir, ) else: compile_seq( @@ -347,6 +360,7 @@ def assert_compile_failure( import_search_dirs=import_search_dirs, ignored_warnings=ignored_warnings, error_warnings=error_warnings, + main_file_dir=main_file_dir, ) except (SystemExit, CompilationFailed) as e: if match is not None: diff --git a/test/fpy/test_compiler_config.py b/test/fpy/test_compiler_config.py index 91c90ca..28f7164 100644 --- a/test/fpy/test_compiler_config.py +++ b/test/fpy/test_compiler_config.py @@ -233,7 +233,7 @@ def test_too_many_directives_with_custom_limit(): # Should fail because we exceed the custom limit with pytest.raises(fpy.error.BackendError) as exc_info: state = analyze_ast(body, state) - analysis_to_fpybc_directives(body, state) + analysis_to_fpybc_directives(state) assert "Too many directives" in str(exc_info.value) finally: Path(dict_path).unlink() @@ -269,7 +269,7 @@ def test_within_custom_limit_succeeds(): # Should succeed state = analyze_ast(body, state) - analysis_to_fpybc_directives(body, state) + analysis_to_fpybc_directives(state) finally: Path(dict_path).unlink() _clear_caches() @@ -449,4 +449,4 @@ def test_timebase_additional_constants_available(): assert body is not None state = analyze_ast(body, state) - analysis_to_fpybc_directives(body, state) + analysis_to_fpybc_directives(state) diff --git a/test/fpy/test_depend.py b/test/fpy/test_depend.py index 0905208..79b7f40 100644 --- a/test/fpy/test_depend.py +++ b/test/fpy/test_depend.py @@ -17,10 +17,20 @@ from fpy.test_helpers import default_dictionary -def _collect(seq: str, ground_binary_dir: str = None) -> list[str]: +def _collect( + seq: str, + ground_binary_dir: str = None, + import_search_dirs: list[str] = None, + main_file_dir: str = None, +) -> list[str]: """Run ast_to_dependencies on a sequence string; return the dependency list.""" fpy.error.file_name = "" - state = get_base_compile_state(default_dictionary, ground_binary_dir) + state = get_base_compile_state( + default_dictionary, + ground_binary_dir, + import_search_dirs=import_search_dirs, + main_file_dir=main_file_dir, + ) body = text_to_ast(seq) assert body is not None return ast_to_dependencies(body, state) @@ -143,9 +153,57 @@ def test_non_string_literal_filename_is_compile_error(self): assert "string literal" in str(exc_info.value) +class TestImports: + """The depend tool must inline imports, so a sequence-run dependency inside + an imported sequence is discovered (and an import never breaks discovery).""" + + def test_import_of_functions_only_sequence_does_not_break_discovery(self, tmp_path): + (tmp_path / "lib.fpy").write_text( + "def add_one(x: U32) -> U32:\n return U32(x + 1)\n" + ) + seq = "import lib\n\nresult: U32 = lib.add_one(41)\n" + deps = _collect(seq, import_search_dirs=[str(tmp_path)]) + assert deps == [] + + def test_seq_run_dep_inside_imported_sequence_is_discovered(self, tmp_path): + # The RUN_ARGS lives only in the imported sequence; discovery must follow + # the import to find it. + (tmp_path / "lib.fpy").write_text( + "def run_child():\n" + ' Ref.seqDisp.RUN_ARGS("child.bin", Svc.BlockState.BLOCK)\n' + ) + seq = "import lib\n\nlib.run_child()\n" + deps = _collect( + seq, ground_binary_dir="/tmp/bins", import_search_dirs=[str(tmp_path)] + ) + assert deps == ["/tmp/bins/child.bin"] + + class TestDependMainCLI: """End-to-end CLI tests that write real .fpy files and call depend_main.""" + def test_import_dependency_discovered_via_cli(self, tmp_path, capsys): + from fpy.main import depend_main + + (tmp_path / "lib.fpy").write_text( + "def run_child():\n" + ' Ref.seqDisp.RUN_ARGS("child.bin", Svc.BlockState.BLOCK)\n' + ) + fpy_path = tmp_path / "seq.fpy" + fpy_path.write_text("import lib\n\nlib.run_child()\n") + depend_main( + [ + str(fpy_path), + "-d", + default_dictionary, + "-g", + str(tmp_path), + "-i", + str(tmp_path), + ] + ) + assert capsys.readouterr().out.strip() == str(tmp_path / "child.bin") + def test_no_deps_produces_no_output(self, tmp_path, capsys): from fpy.main import depend_main diff --git a/test/fpy/test_golden.py b/test/fpy/test_golden.py index 17607e2..1bdcdd0 100644 --- a/test/fpy/test_golden.py +++ b/test/fpy/test_golden.py @@ -35,7 +35,7 @@ def compile_to_fpybc(source: str) -> str: assert body is not None, "Parsing failed" state = analyze_ast(body, state) - directives, _ = analysis_to_fpybc_directives(body, state) + directives, _ = analysis_to_fpybc_directives(state) return fpybc_directives_to_fpyasm(directives) diff --git a/test/fpy/test_imports.py b/test/fpy/test_imports.py index 6ad812e..f95a5ec 100644 --- a/test/fpy/test_imports.py +++ b/test/fpy/test_imports.py @@ -1,74 +1,91 @@ """Tests for the `import` statement (spec / TDD). -`import foo` inlines the definitions of a sibling `foo.fpy` source file into -the importing sequence and sections its symbols off under the name `foo`, so a -function `bar` defined in `foo.fpy` is called as `foo.bar()`. +`import foo` inlines the definitions of a sibling `foo.fpy` sequence into the +importing sequence and sections its symbols off under a module named `foo`, so +a function `bar` defined in `foo.fpy` is called as `foo.bar()`. + +Terminology: a *sequence* is an importable `.fpy` file; a *module* is the +name-to-symbol mapping an import introduces to hold a sequence's symbols. Design decisions encoded here: - * Import paths resolve against `state.import_search_dirs`, an ordered list of - directories searched first-match-wins. In the CLI this is the importing - file's own directory followed by each `-i/--include` directory. This is - DISTINCT from `ground_binary_dir`, which roots runtime - sequence-binary (.bin) paths -- imports are a compile-time-only source - inlining and never survive into the emitted bytecode. - * A bare name resolves to a sibling file: `import foo` -> `foo.fpy`. Dotted - module paths resolve through package directories, Pythonically: `import - a.b.c` -> `a/b/c.fpy`, searched against each `import_search_dirs` entry - first-match-wins. Package directories are plain namespace packages -- no - `__init__.fpy` marker is required. The name bound is the top segment, and - members are reached by the full path: `import a.b.c` makes `a.b.c.bar()` - callable (and `import foo` makes `foo.bar()` callable). - * File/package precedence follows Python's namespace-package rules. Because - no `__init__.fpy` marker is required, directories are namespace portions, - which rank BELOW modules: at an import's leaf segment a module file - `foo.fpy` outranks a same-named `foo/` directory. Non-leaf segments must be - package directories -- `import a.b` descends into `a/` to reach `a/b.fpy` - regardless of any sibling `a.fpy` module. Importing a leaf that resolves - only to a package directory (no module file to inline) is an error: a - namespace package has nothing to inline. + * Imports come in two disjoint resolution styles (PEP-328-like). An + ABSOLUTE import (`import foo`, `import a.b.c`) resolves only against the + shared base search path `state.import_search_dirs` (the `-i/--include` + directories); the importing file's own location plays no role, so an + absolute path names the same file in every sequence of a compilation. A + RELATIVE import (leading dots: `import .foo`, `import ..util`, + `from .foo import f`) resolves only against its anchor -- one dot is the + importing sequence's own directory, each extra dot one parent up -- and + never consults the base search path. So a library directory's sequences + reference one another relatively and work unmodified wherever the library + is mounted, while consumers name the library absolutely from anywhere. + The search dirs are DISTINCT from `ground_binary_dir`, which roots + runtime sequence-binary (.bin) paths -- imports are a compile-time-only + source inlining and never survive into the emitted bytecode. + * An absolute import that resolves in more than one base search dir is + AMBIGUOUS -- a hard error, even if the candidates agree -- so adding a + file to one search dir can never silently rebind another sequence's + import. There is no shadowing order among the search dirs. + * Dotted sequence paths resolve through package directories, Pythonically: + `import a.b.c` -> `a/b/c.fpy`. Package directories need no + `__init__.fpy` marker. The imported symbols live under a module chain, + reached by the full path: `import a.b.c` makes `a.b.c.bar()` callable + (and `import foo` makes `foo.bar()` callable). A relative import binds + the path after its dots: `import .sub.mod` -> `sub.mod.f()`, and + `import ..util` -> `util.f()`. + * Unlike Python, `import .foo` is legal (binding module `foo`) and + `from . import foo` is not: `from` members are always definitions, never + sequences. + * File/directory precedence: at an import's leaf segment a sequence file + `foo.fpy` outranks a same-named `foo/` directory. Non-leaf segments must + be directories -- `import a.b` descends into `a/` to reach `a/b.fpy` + regardless of any sibling `a.fpy`. Importing a leaf that resolves only to + a directory (no sequence file to inline) is an error. * `import` is only valid as a top-level statement (not nested in a block), - but an imported module MAY itself import other modules (transitive imports - are supported, with cycle detection). - * Inlining an imported file that does more than define functions emits the - `import-side-effects` warning (its top-level code runs as part of the + but an imported sequence MAY itself import other sequences (transitive + imports are supported, with cycle detection). + * Inlining an imported sequence that does more than define functions emits + the `import-side-effects` warning (its top-level code runs as part of the importing sequence). - * Importing a file that declares sequence arguments (`sequence(x: U32)`) is a - hard error. - * The module namespace obeys name groups: it exists only in the name groups - of the symbols the module defines. So `import lib` of a functions-only - module collides with a local function `lib` (callable name group) but - coexists with a local variable `lib` (value name group). - * Importing the same module path more than once in one file is a hard - error. The rule is per-file: a file and a module it imports may each - import the same module. - -Every sequence that is expected to compile is also *run* (via -`assert_run_success`), so the asserts embedded in the sequences actually -execute. Every sequence that is expected to fail compilation uses -`assert_compile_failure`. These tests are `xfail` until the import passes are -implemented; they define the target behavior. Remove the `pytestmark` once -`import` is implemented. + * A leading underscore marks a definition as library-internal: an importer + naming it (`lib._helper()`, `import lib._helper`, `from lib import + _helper`) emits the `import-underscore` warning. The library's own + references to it never warn, and an alias silences later uses. + * Importing a sequence that declares sequence arguments (`sequence(x: U32)`) + is a hard error. + * A module obeys name groups: it exists only in the name groups of the + symbols the sequence defines. So `import lib` of a functions-only sequence + collides with a local function `lib` (callable name group) but coexists + with a local variable `lib` (value name group). + * Package modules (non-leaf chain segments) merge when paths overlap + (`import pkg.a` + `import pkg.b` share `pkg`), but two SEQUENCE modules + (a module holding a file's definitions, whether chain leaf or alias) + never merge: binding two different files' modules to one name is a + collision. + * Importing the same sequence more than once in one file is a hard error, + across every form (`import seq`, `import seq.foo`, `from seq import bar`). + The rule is per-file: a file and a sequence it imports may each import the + same sequence. + +Beyond the plain `import seq` form, this file also covers member imports +(`import seq.func`), aliases (`import seq as x`, `... as y`), and `from` imports +(`from seq import a, b`, `from seq import *`). + """ from pathlib import Path -import pytest - from fpy.test_helpers import ( assert_compile_failure, assert_run_success, compile_seq, ) from fpy.error import WarningType - -# The whole module targets not-yet-implemented behavior. -pytestmark = pytest.mark.xfail( - reason="import statement not yet implemented", strict=False -) +from fpy.types import U32, FpyValue -def _write_module(search_dir: Path, dotted_name: str, src: str) -> None: - """Write an importable module for `import ` into *search_dir*. +def _write_sequence(search_dir: Path, dotted_name: str, src: str) -> None: + """Write an importable sequence for `import ` into *search_dir*. A bare name `foo` becomes `/foo.fpy`; a dotted name `a.b.c` becomes `/a/b/c.fpy`, creating the intervening package @@ -80,15 +97,16 @@ def _write_module(search_dir: Path, dotted_name: str, src: str) -> None: class TestImportInlining: - """An imported function is inlined and callable under the module name.""" + """An imported function is inlined and callable under the sequence's + module name.""" def test_call_imported_function(self, fprime_test_api, tmp_path): - _write_module( + _write_sequence( tmp_path, "lib", """\ def add_one(x: U32) -> U32: - return x + 1 + return U32(x + 1) """, ) main = """\ @@ -97,33 +115,36 @@ def add_one(x: U32) -> U32: result: U32 = lib.add_one(41) assert result == 42 """ - # Funcs-only module: compiles cleanly with no side-effect warning... + # Funcs-only sequence: compiles cleanly with no side-effect warning... state, _, _ = compile_seq(main, import_search_dirs=[str(tmp_path)]) assert state.warnings == [] # ...and the embedded assert holds at run time. assert_run_success(fprime_test_api, main, import_search_dirs=[str(tmp_path)]) - def test_imported_function_runs(self, fprime_test_api, tmp_path): - _write_module( + def test_imported_sequence_without_trailing_newline( + self, fprime_test_api, tmp_path + ): + """An imported sequence file whose last line is a lone tab with no final + newline must still parse. Imported files flow through the same parser + entry point as the main sequence, so the fix for issue #61 (trailing + whitespace at end of file) covers them too.""" + _write_sequence( tmp_path, "lib", - """\ -def double(x: U32) -> U32: - return x * 2 -""", + "def add_one(x: U32) -> U32:\n return U32(x + 1)\n\t", ) main = """\ import lib -v: U32 = lib.double(21) -assert v == 42 +result: U32 = lib.add_one(41) +assert result == 42 """ assert_run_success(fprime_test_api, main, import_search_dirs=[str(tmp_path)]) def test_local_and_imported_names_coexist(self, fprime_test_api, tmp_path): """A local `helper` and an imported `lib.helper` must not collide -- the imported symbols are sectioned off under `lib`.""" - _write_module( + _write_sequence( tmp_path, "lib", """\ @@ -146,46 +167,17 @@ def helper() -> U32: class TestImportSideEffects: - """Importing a file with top-level, non-def code warns.""" + """An imported sequence may contain only function definitions and imports.""" - SIDE_EFFECT_MODULE = """\ + SIDE_EFFECT_SEQUENCE = """\ CdhCore.cmdDisp.CMD_NO_OP() def noop_wrapper(): CdhCore.cmdDisp.CMD_NO_OP() """ - def test_side_effecting_import_warns(self, fprime_test_api, tmp_path): - _write_module(tmp_path, "side_effects", self.SIDE_EFFECT_MODULE) - main = """\ -import side_effects - -side_effects.noop_wrapper() -""" - state, _, _ = compile_seq(main, import_search_dirs=[str(tmp_path)]) - assert any( - w.type == WarningType.IMPORT_SIDE_EFFECTS for w in state.warnings - ), f"expected an import-side-effects warning, got {state.warnings}" - # The warning is non-fatal: the sequence still compiles and runs. - assert_run_success(fprime_test_api, main, import_search_dirs=[str(tmp_path)]) - - def test_side_effect_warning_can_be_ignored(self, fprime_test_api, tmp_path): - _write_module(tmp_path, "side_effects", self.SIDE_EFFECT_MODULE) - main = """\ -import side_effects - -side_effects.noop_wrapper() -""" - state, _, _ = compile_seq( - main, - import_search_dirs=[str(tmp_path)], - ignored_warnings={WarningType.IMPORT_SIDE_EFFECTS}, - ) - assert state.warnings == [] - assert_run_success(fprime_test_api, main, import_search_dirs=[str(tmp_path)]) - - def test_side_effect_warning_can_be_escalated(self, fprime_test_api, tmp_path): - _write_module(tmp_path, "side_effects", self.SIDE_EFFECT_MODULE) + def test_side_effecting_import_is_error(self, fprime_test_api, tmp_path): + _write_sequence(tmp_path, "side_effects", self.SIDE_EFFECT_SEQUENCE) main = """\ import side_effects @@ -194,13 +186,12 @@ def test_side_effect_warning_can_be_escalated(self, fprime_test_api, tmp_path): assert_compile_failure( fprime_test_api, main, - match="import-side-effects", + match="only function definitions and imports", import_search_dirs=[str(tmp_path)], - error_warnings={WarningType.IMPORT_SIDE_EFFECTS}, ) - def test_functions_only_module_does_not_warn(self, fprime_test_api, tmp_path): - _write_module( + def test_functions_only_sequence_compiles(self, fprime_test_api, tmp_path): + _write_sequence( tmp_path, "clean", """\ @@ -214,7 +205,7 @@ def b() -> U32: main = """\ import clean -x: U32 = clean.a() + clean.b() +x: U32 = U32(clean.a() + clean.b()) assert x == 3 """ state, _, _ = compile_seq(main, import_search_dirs=[str(tmp_path)]) @@ -222,11 +213,124 @@ def b() -> U32: assert_run_success(fprime_test_api, main, import_search_dirs=[str(tmp_path)]) +class TestImportUnderscore: + """A leading underscore marks a definition as internal to its sequence: + the importer naming it emits the `import-underscore` warning; the + library's own references to it do not.""" + + LIB = """\ +def _helper() -> U32: + return 7 + +def public() -> U32: + return _helper() +""" + + def test_underscore_module_access_warns(self, fprime_test_api, tmp_path): + """`lib._helper()` names an imported underscore definition: warn.""" + _write_sequence(tmp_path, "lib", self.LIB) + main = """\ +import lib + +x: U32 = lib._helper() +assert x == 7 +""" + state, _, _ = compile_seq(main, import_search_dirs=[str(tmp_path)]) + assert any( + w.type == WarningType.IMPORT_UNDERSCORE for w in state.warnings + ), f"expected an import-underscore warning, got {state.warnings}" + # The warning is non-fatal: the sequence still compiles and runs. + assert_run_success(fprime_test_api, main, import_search_dirs=[str(tmp_path)]) + + def test_underscore_member_import_warns(self, fprime_test_api, tmp_path): + """`import lib._helper` names the underscore definition as a member.""" + _write_sequence(tmp_path, "lib", self.LIB) + main = """\ +import lib._helper + +x: U32 = lib._helper() +assert x == 7 +""" + state, _, _ = compile_seq(main, import_search_dirs=[str(tmp_path)]) + assert any( + w.type == WarningType.IMPORT_UNDERSCORE for w in state.warnings + ), f"expected an import-underscore warning, got {state.warnings}" + assert_run_success(fprime_test_api, main, import_search_dirs=[str(tmp_path)]) + + def test_underscore_from_import_warns(self, fprime_test_api, tmp_path): + """`from lib import _helper` names the underscore definition.""" + _write_sequence(tmp_path, "lib", self.LIB) + main = """\ +from lib import _helper + +x: U32 = _helper() +assert x == 7 +""" + state, _, _ = compile_seq(main, import_search_dirs=[str(tmp_path)]) + assert any( + w.type == WarningType.IMPORT_UNDERSCORE for w in state.warnings + ), f"expected an import-underscore warning, got {state.warnings}" + assert_run_success(fprime_test_api, main, import_search_dirs=[str(tmp_path)]) + + def test_star_import_underscore_use_warns(self, fprime_test_api, tmp_path): + """`from lib import *` binds `_helper` without naming it; the later + bare `_helper()` use is what warns.""" + _write_sequence(tmp_path, "lib", self.LIB) + main = """\ +from lib import * + +x: U32 = _helper() +assert x == 7 +""" + state, _, _ = compile_seq(main, import_search_dirs=[str(tmp_path)]) + assert any( + w.type == WarningType.IMPORT_UNDERSCORE for w in state.warnings + ), f"expected an import-underscore warning, got {state.warnings}" + assert_run_success(fprime_test_api, main, import_search_dirs=[str(tmp_path)]) + + def test_library_internal_use_does_not_warn(self, fprime_test_api, tmp_path): + """`lib.public()` internally calls `_helper`; the importer never names + an underscore definition, so nothing warns.""" + _write_sequence(tmp_path, "lib", self.LIB) + main = """\ +import lib + +x: U32 = lib.public() +assert x == 7 +""" + state, _, _ = compile_seq(main, import_search_dirs=[str(tmp_path)]) + assert state.warnings == [] + assert_run_success(fprime_test_api, main, import_search_dirs=[str(tmp_path)]) + + def test_underscore_alias_statement_warns_but_uses_do_not( + self, fprime_test_api, tmp_path + ): + """`from lib import _helper as helper` warns once, at the statement; + uses of the alias `helper` add nothing.""" + _write_sequence(tmp_path, "lib", self.LIB) + main = """\ +from lib import _helper as helper + +x: U32 = helper() +y: U32 = helper() +assert x == 7 +assert y == 7 +""" + state, _, _ = compile_seq(main, import_search_dirs=[str(tmp_path)]) + underscore_warnings = [ + w for w in state.warnings if w.type == WarningType.IMPORT_UNDERSCORE + ] + assert ( + len(underscore_warnings) == 1 + ), f"expected exactly one import-underscore warning, got {state.warnings}" + assert_run_success(fprime_test_api, main, import_search_dirs=[str(tmp_path)]) + + class TestImportErrors: """Error cases for import.""" def test_cannot_import_sequence_with_arguments(self, fprime_test_api, tmp_path): - _write_module( + _write_sequence( tmp_path, "with_arguments", """\ @@ -243,7 +347,7 @@ def f() -> U32: fprime_test_api, main, match="argument", import_search_dirs=[str(tmp_path)] ) - def test_missing_module_is_an_error(self, fprime_test_api, tmp_path): + def test_missing_sequence_is_an_error(self, fprime_test_api, tmp_path): main = """\ import does_not_exist """ @@ -254,7 +358,7 @@ def test_missing_module_is_an_error(self, fprime_test_api, tmp_path): def test_no_arg_sequence_is_importable(self, fprime_test_api, tmp_path): """A bare `sequence()` with no arguments is importable -- only sequences *with arguments* are rejected (per the feature's wording).""" - _write_module( + _write_sequence( tmp_path, "no_argument_sequence", """\ @@ -273,13 +377,96 @@ def f() -> U32: assert_run_success(fprime_test_api, main, import_search_dirs=[str(tmp_path)]) +class TestImporterWithArguments: + """Importing a sequence WITH arguments is rejected, but the *importer* + declaring its own sequence arguments is fine: it may both take arguments and + import another (argument-less) sequence, and use both together.""" + + def test_importer_declares_args_and_imports(self, fprime_test_api, tmp_path): + _write_sequence( + tmp_path, + "lib", + """\ +def add_one(x: U32) -> U32: + return U32(x + 1) +""", + ) + main = """\ +sequence(n: U32) + +import lib + +result: U32 = lib.add_one(n) +assert result == 42 +""" + state, _, _ = compile_seq(main, import_search_dirs=[str(tmp_path)]) + assert state.warnings == [] + assert_run_success( + fprime_test_api, + main, + args=[FpyValue(U32, 41)], + import_search_dirs=[str(tmp_path)], + ) + + +class TestImportBuiltinLibrary: + """The builtin library functions (`time_add`, `time_sub`, + `time_interval_cmp`, ...) register into the shared base callable scope that + every sequence's scope descends from, so an imported sequence may reference + them too -- whether written explicitly or produced by desugaring a + `check`/timeout statement. + + Found by a one-time import-inlining sweep.""" + + def test_imported_sequence_can_use_time_builtin(self, fprime_test_api, tmp_path): + """A `check ... timeout` in an imported sequence desugars into a + `time_add`/`time_interval_cmp` call that must resolve.""" + _write_sequence( + tmp_path, + "lib", + """\ +def wait_ok() -> bool: + check True timeout Fw.TimeIntervalValue(1, 0) persist Fw.TimeIntervalValue(0, 0) period Fw.TimeIntervalValue(0, 100000): + return True + timeout: + return False + return False +""", + ) + main = """\ +import lib + +ok: bool = lib.wait_ok() +assert ok +""" + assert_run_success(fprime_test_api, main, import_search_dirs=[str(tmp_path)]) + + def test_imported_sequence_can_use_time_operators(self, fprime_test_api, tmp_path): + """Time operators inside an imported sequence desugar to builtin calls + (`time_sub`, `time_interval_cmp`) that must resolve there too.""" + _write_sequence( + tmp_path, + "lib", + """\ +def is_past(deadline: Fw.Time) -> bool: + return (now() - deadline) > Fw.TimeIntervalValue(0, 0) +""", + ) + main = """\ +import lib + +past: bool = lib.is_past(Fw.Time(TimeBase.TB_NONE, 0, 0, 0)) +""" + assert_run_success(fprime_test_api, main, import_search_dirs=[str(tmp_path)]) + + class TestImportFileErrors: """Failure modes rooted in the imported file itself.""" def test_parse_error_in_imported_file_fails(self, fprime_test_api, tmp_path): """A syntax error inside the imported file is a hard compile error. Ideally the diagnostic points into the imported file, not the importer.""" - _write_module( + _write_sequence( tmp_path, "broken", """\ @@ -305,9 +492,9 @@ def test_import_path_is_a_directory_fails(self, fprime_test_api, tmp_path): fprime_test_api, main, import_search_dirs=[str(tmp_path)] ) - def test_empty_module_compiles_without_warning(self, fprime_test_api, tmp_path): + def test_empty_sequence_compiles_without_warning(self, fprime_test_api, tmp_path): """An empty module has no definitions and no side effects.""" - _write_module(tmp_path, "empty", "") + _write_sequence(tmp_path, "empty", "") main = """\ import empty @@ -318,17 +505,17 @@ def test_empty_module_compiles_without_warning(self, fprime_test_api, tmp_path): assert_run_success(fprime_test_api, main, import_search_dirs=[str(tmp_path)]) -class TestImportNamespaceIsolation: +class TestImportModuleIsolation: """Imported symbols are sectioned under the module name and isolated.""" def test_imported_symbol_requires_module_prefix(self, fprime_test_api, tmp_path): """`add_one` is only reachable as `lib.add_one`, never bare.""" - _write_module( + _write_sequence( tmp_path, "lib", """\ def add_one(x: U32) -> U32: - return x + 1 + return U32(x + 1) """, ) main = """\ @@ -341,13 +528,13 @@ def add_one(x: U32) -> U32: ) def test_module_name_not_usable_as_value(self, fprime_test_api, tmp_path): - """The module name is a namespace, not an expression.""" - _write_module( + """A module name resolves to a module, not a value, so it is not usable as an expression.""" + _write_sequence( tmp_path, "lib", """\ def add_one(x: U32) -> U32: - return x + 1 + return U32(x + 1) """, ) main = """\ @@ -362,7 +549,7 @@ def add_one(x: U32) -> U32: def test_same_function_name_in_two_modules_no_collision( self, fprime_test_api, tmp_path ): - _write_module( + _write_sequence( tmp_path, "lib_a", """\ @@ -370,7 +557,7 @@ def helper() -> U32: return 1 """, ) - _write_module( + _write_sequence( tmp_path, "lib_b", """\ @@ -394,7 +581,7 @@ def test_imported_function_cannot_see_importer_globals( ): """An imported function is analyzed in its own module scope: a name defined only in the importing sequence must NOT resolve inside it.""" - _write_module( + _write_sequence( tmp_path, "iso", """\ @@ -414,14 +601,14 @@ def uses_outside() -> U32: class TestImportNameCollisions: - """The module namespace collides with existing top-level names per name - group: the namespace exists only in the name groups of the symbols the - module defines.""" + """An imported module collides with existing top-level names per name + group: the module exists only in the name groups of the symbols the + sequence defines.""" def test_import_collides_with_local_function(self, fprime_test_api, tmp_path): """A functions-only module occupies the callable name group, so a local function with the module's name collides.""" - _write_module( + _write_sequence( tmp_path, "dup", """\ @@ -442,7 +629,7 @@ def dup() -> U32: def test_import_collides_with_local_variable(self, fprime_test_api, tmp_path): """A module with a top-level variable occupies the value name group, so a local variable with the module's name collides.""" - _write_module( + _write_sequence( tmp_path, "dup", """\ @@ -462,7 +649,7 @@ def test_import_coexists_with_local_variable(self, fprime_test_api, tmp_path): """A functions-only module does NOT occupy the value name group, so a local variable with the module's name is legal, and both remain usable in their respective name groups.""" - _write_module( + _write_sequence( tmp_path, "dup", """\ @@ -481,18 +668,135 @@ def f() -> U32: assert_run_success(fprime_test_api, main, import_search_dirs=[str(tmp_path)]) +class TestImportedSequenceCannotShadowBuiltins: + """An imported sequence's own scope layers on top of the shared dictionary / + builtin base scope, but must never shadow a name that already lives in that + base (a cast like `U32`, a type constructor, a dictionary namespace). This + holds regardless of how the sequence's scope is implemented (a copy of base, + or a child of it), so these guard the copy->child-of-base refactor: a child + scope resolves base names only up its parent chain, so a naive own-dict-only + collision check would silently let an import shadow them.""" + + def test_imported_sequence_cannot_define_function_shadowing_builtin_cast( + self, fprime_test_api, tmp_path + ): + """Defining a function named after a builtin cast (`U32`) inside an + imported sequence is rejected -- the name is already taken by the + shared base scope, not free just because the sequence's own dict is + empty.""" + _write_sequence( + tmp_path, + "lib", + """\ +def U32() -> U32: + return U32(0) +""", + ) + main = "import lib\n" + assert_compile_failure( + fprime_test_api, + main, + match="already been defined", + import_search_dirs=[str(tmp_path)], + ) + + def test_nested_import_alias_colliding_with_builtin_cast_is_rejected( + self, fprime_test_api, tmp_path + ): + """When an IMPORTED sequence is itself the importer, binding a name + (here an alias) that collides with a builtin base name (`U32`) is a + collision, exactly as it is for the main sequence. The importer's scope + is a child of base, so the collision must be detected up the parent + chain, not just in the importer's own dict.""" + _write_sequence( + tmp_path, + "inner", + """\ +def helper() -> U32: + return U32(5) +""", + ) + _write_sequence( + tmp_path, + "outer", + """\ +from inner import helper as U32 +""", + ) + main = "import outer\n" + assert_compile_failure( + fprime_test_api, + main, + match="collides with an existing definition", + import_search_dirs=[str(tmp_path)], + ) + + def test_imported_sequence_cannot_declare_top_level_flags( + self, fprime_test_api, tmp_path + ): + """An imported sequence can not declare a top-level `flags` (or any + other top-level variable): a top-level assignment is a side effect, so it + is rejected outright, before it could shadow the shared $Flags builtin.""" + _write_sequence( + tmp_path, + "lib", + """\ +flags: U32 = 5 +""", + ) + main = "import lib\n" + assert_compile_failure( + fprime_test_api, + main, + match="only function definitions and imports", + import_search_dirs=[str(tmp_path)], + ) + + def test_nested_import_of_module_named_after_dictionary_namespace_is_rejected( + self, fprime_test_api, tmp_path + ): + """An imported sequence that itself imports a module whose name matches + a dictionary namespace (`Ref`) collides with that base namespace rather + than silently shadowing it (which would break `Ref.*` commands in that + sequence). Bound through `_bind_module`, so it must also consult the + parent chain.""" + _write_sequence( + tmp_path, + "Ref", + """\ +def f() -> U32: + return U32(1) +""", + ) + _write_sequence( + tmp_path, + "outer", + """\ +import Ref +""", + ) + main = "import outer\n" + assert_compile_failure( + fprime_test_api, + main, + match="collides with an existing definition", + import_search_dirs=[str(tmp_path)], + ) + + class TestImportDuplicates: - """Importing the same module path twice in one file is an error; the rule - is per-file, so a file and a module it imports may each import the same - module.""" + """A sequence is compiled once however many import statements name it, and + importing it more than once in one file is governed by the ordinary + collision rule: two `import lib` statements bind the module `lib` twice and + collide.""" - def test_duplicate_import_is_error(self, fprime_test_api, tmp_path): - _write_module( + def test_import_same_sequence_twice_collides(self, fprime_test_api, tmp_path): + _write_sequence( tmp_path, "lib", """\ def add_one(x: U32) -> U32: - return x + 1 + return U32(x + 1) """, ) main = """\ @@ -500,14 +804,18 @@ def add_one(x: U32) -> U32: import lib """ assert_compile_failure( - fprime_test_api, main, import_search_dirs=[str(tmp_path)] + fprime_test_api, + main, + match="collides with an existing imported sequence", + import_search_dirs=[str(tmp_path)], ) def test_duplicate_across_files_is_allowed(self, fprime_test_api, tmp_path): - """main imports both `a` and `c`; `a` also imports `c` internally. - Each file imports `c` only once, so no duplicate error.""" - _write_module(tmp_path, "c", "def g() -> U32:\n return 7\n") - _write_module( + """main imports both `a` and `c`; `a` also imports `c` internally. `c` + is imported from two places but compiled once and shared, not + duplicated.""" + _write_sequence(tmp_path, "c", "def g() -> U32:\n return 7\n") + _write_sequence( tmp_path, "a", """\ @@ -521,7 +829,7 @@ def f() -> U32: import a import c -x: U32 = a.f() + c.g() +x: U32 = U32(a.f() + c.g()) assert x == 14 """ assert_run_success(fprime_test_api, main, import_search_dirs=[str(tmp_path)]) @@ -529,11 +837,11 @@ def f() -> U32: class TestImportOnlyAtTopLevel: """`import` is only valid as a top-level statement (never nested in a - block). Note: an imported *module* may still contain its own top-level + block). Note: an imported *sequence* may still contain its own top-level imports -- see TestImportTransitive.""" def test_import_inside_if_block_fails(self, fprime_test_api, tmp_path): - _write_module( + _write_sequence( tmp_path, "lib", """\ @@ -550,7 +858,7 @@ def f() -> U32: ) def test_import_inside_function_fails(self, fprime_test_api, tmp_path): - _write_module( + _write_sequence( tmp_path, "lib", """\ @@ -569,11 +877,11 @@ def wrapper() -> U32: class TestImportTransitive: - """An imported module may itself import other modules.""" + """An imported sequence may itself import other sequences.""" def test_transitive_import_works(self, fprime_test_api, tmp_path): """main -> a -> b: `a` uses `b` internally, and main runs `a.f()`.""" - _write_module( + _write_sequence( tmp_path, "b", """\ @@ -581,7 +889,7 @@ def g() -> U32: return 7 """, ) - _write_module( + _write_sequence( tmp_path, "a", """\ @@ -601,7 +909,7 @@ def f() -> U32: def test_transitive_dependency_is_private(self, fprime_test_api, tmp_path): - _write_module( + _write_sequence( tmp_path, "b", """\ @@ -609,7 +917,7 @@ def g() -> U32: return 7 """, ) - _write_module( + _write_sequence( tmp_path, "a", """\ @@ -633,7 +941,7 @@ class TestImportCycles: """Import cycles are detected and rejected.""" def test_self_import_is_cycle_error(self, fprime_test_api, tmp_path): - _write_module( + _write_sequence( tmp_path, "self_import", """\ @@ -654,7 +962,7 @@ def f() -> U32: ) def test_mutual_import_is_cycle_error(self, fprime_test_api, tmp_path): - _write_module( + _write_sequence( tmp_path, "mod_a", """\ @@ -664,7 +972,7 @@ def a() -> U32: return mod_b.b() """, ) - _write_module( + _write_sequence( tmp_path, "mod_b", """\ @@ -685,9 +993,9 @@ def b() -> U32: ) def test_three_way_cycle_error(self, fprime_test_api, tmp_path): - _write_module(tmp_path, "c1", "import c2\n\ndef f() -> U32:\n return 1\n") - _write_module(tmp_path, "c2", "import c3\n\ndef f() -> U32:\n return 1\n") - _write_module(tmp_path, "c3", "import c1\n\ndef f() -> U32:\n return 1\n") + _write_sequence(tmp_path, "c1", "import c2\n\ndef f() -> U32:\n return 1\n") + _write_sequence(tmp_path, "c2", "import c3\n\ndef f() -> U32:\n return 1\n") + _write_sequence(tmp_path, "c3", "import c1\n\ndef f() -> U32:\n return 1\n") main = """\ import c1 """ @@ -700,16 +1008,16 @@ def test_three_way_cycle_error(self, fprime_test_api, tmp_path): class TestImportDottedPaths: - """Dotted module paths resolve through package directories, Pythonically.""" + """Dotted sequence paths resolve through package directories, Pythonically.""" def test_single_dotted_import(self, fprime_test_api, tmp_path): """`import pkg.mod` resolves `pkg/mod.fpy` and binds `pkg.mod`.""" - _write_module( + _write_sequence( tmp_path, "pkg.mod", """\ def add_one(x: U32) -> U32: - return x + 1 + return U32(x + 1) """, ) main = """\ @@ -724,7 +1032,7 @@ def add_one(x: U32) -> U32: def test_deeply_nested_dotted_import(self, fprime_test_api, tmp_path): """`import a.b.c` resolves `a/b/c.fpy` (arbitrary nesting depth).""" - _write_module( + _write_sequence( tmp_path, "a.b.c", """\ @@ -743,7 +1051,7 @@ def val() -> U32: def test_dotted_symbol_requires_full_path(self, fprime_test_api, tmp_path): """A member of `pkg.mod` is only reachable as `pkg.mod.f`, never as a bare `f` nor via a truncated `mod.f`.""" - _write_module( + _write_sequence( tmp_path, "pkg.mod", """\ @@ -761,8 +1069,8 @@ def f() -> U32: ) def test_missing_leaf_in_existing_package_is_error(self, fprime_test_api, tmp_path): - """The package dir exists but the leaf module file does not.""" - _write_module( + """The package dir exists but the leaf sequence file does not.""" + _write_sequence( tmp_path, "pkg.other", """\ @@ -777,10 +1085,12 @@ def f() -> U32: fprime_test_api, main, import_search_dirs=[str(tmp_path)] ) - def test_two_modules_in_same_package_no_collision(self, fprime_test_api, tmp_path): - """Sibling modules under one package are independently namespaced.""" - _write_module(tmp_path, "pkg.a", "def f() -> U32:\n return 1\n") - _write_module(tmp_path, "pkg.b", "def f() -> U32:\n return 2\n") + def test_two_sequences_in_same_package_no_collision( + self, fprime_test_api, tmp_path + ): + """Sibling sequences under one package get independent modules.""" + _write_sequence(tmp_path, "pkg.a", "def f() -> U32:\n return 1\n") + _write_sequence(tmp_path, "pkg.b", "def f() -> U32:\n return 2\n") main = """\ import pkg.a import pkg.b @@ -794,16 +1104,16 @@ def test_two_modules_in_same_package_no_collision(self, fprime_test_api, tmp_pat class TestImportPackagePrecedence: - """File-vs-directory precedence follows Python's namespace-package rules: - a module file outranks a same-named (init-less) package directory at a - leaf, but non-leaf segments always descend into the directory.""" + """File-vs-directory precedence: a sequence file outranks a same-named + (init-less) directory at a leaf, but non-leaf segments always descend into + the directory.""" - def test_module_file_beats_namespace_directory(self, fprime_test_api, tmp_path): + def test_sequence_file_beats_directory(self, fprime_test_api, tmp_path): """`foo.fpy` and a `foo/` directory both exist; `import foo` resolves - the module file (namespace dirs rank below modules).""" - _write_module(tmp_path, "foo", "def f() -> U32:\n return 1\n") + the sequence file (a directory ranks below a sequence file).""" + _write_sequence(tmp_path, "foo", "def f() -> U32:\n return 1\n") # This also creates the sibling `foo/` directory: - _write_module(tmp_path, "foo.inner", "def g() -> U32:\n return 2\n") + _write_sequence(tmp_path, "foo.inner", "def g() -> U32:\n return 2\n") main = """\ import foo @@ -815,8 +1125,8 @@ def test_module_file_beats_namespace_directory(self, fprime_test_api, tmp_path): def test_package_dir_used_for_dotted_descent(self, fprime_test_api, tmp_path): """A `pkg.fpy` module does not block `import pkg.mod` from descending into the `pkg/` directory to reach `pkg/mod.fpy`.""" - _write_module(tmp_path, "pkg", "def top() -> U32:\n return 1\n") - _write_module(tmp_path, "pkg.mod", "def f() -> U32:\n return 5\n") + _write_sequence(tmp_path, "pkg", "def top() -> U32:\n return 1\n") + _write_sequence(tmp_path, "pkg.mod", "def f() -> U32:\n return 5\n") main = """\ import pkg.mod @@ -827,10 +1137,10 @@ def test_package_dir_used_for_dotted_descent(self, fprime_test_api, tmp_path): def test_bare_package_import_is_error(self, fprime_test_api, tmp_path): """`import pkg` where only a `pkg/` directory exists (no `pkg.fpy`) is - an error -- a namespace package has nothing to inline.""" + an error -- a directory has no sequence file to inline.""" # Creates `pkg/mod.fpy`, so `pkg/` exists as a directory but `pkg.fpy` # does not. - _write_module(tmp_path, "pkg.mod", "def f() -> U32:\n return 1\n") + _write_sequence(tmp_path, "pkg.mod", "def f() -> U32:\n return 1\n") main = """\ import pkg """ @@ -841,7 +1151,7 @@ def test_bare_package_import_is_error(self, fprime_test_api, tmp_path): def test_dotted_leaf_package_import_is_error(self, fprime_test_api, tmp_path): """`import a.b` where `a/b/` is a directory but `a/b.fpy` does not exist is likewise an error at the dotted leaf.""" - _write_module(tmp_path, "a.b.c", "def f() -> U32:\n return 1\n") + _write_sequence(tmp_path, "a.b.c", "def f() -> U32:\n return 1\n") main = """\ import a.b """ @@ -851,15 +1161,19 @@ def test_dotted_leaf_package_import_is_error(self, fprime_test_api, tmp_path): class TestImportSearchDirs: - """`import_search_dirs` is an ordered search path: first match wins.""" - - def test_module_found_in_later_search_dir(self, fprime_test_api, tmp_path): - """A module present only in the second search dir is still found.""" + """`import_search_dirs` holds the candidate directories for absolute + imports: an import must resolve in exactly one of them. Resolving in + none is a missing-sequence error; resolving in more than one is an + ambiguity error. There is no shadowing order.""" + + def test_sequence_found_in_later_search_dir(self, fprime_test_api, tmp_path): + """A module present in only one search dir is found, wherever that + dir sits in the list.""" d1 = tmp_path / "d1" d2 = tmp_path / "d2" d1.mkdir() d2.mkdir() - _write_module(d2, "lib", "def f() -> U32:\n return 9\n") + _write_sequence(d2, "lib", "def f() -> U32:\n return 9\n") main = """\ import lib @@ -868,46 +1182,59 @@ def test_module_found_in_later_search_dir(self, fprime_test_api, tmp_path): """ assert_run_success(fprime_test_api, main, import_search_dirs=[str(d1), str(d2)]) - def test_first_search_dir_shadows_later(self, fprime_test_api, tmp_path): - """When a module name exists in two search dirs, the earlier dir wins.""" + def test_same_name_in_two_dirs_is_ambiguous(self, fprime_test_api, tmp_path): + """A module name that resolves in two search dirs is ambiguous and + fails, even though either candidate alone would compile.""" d1 = tmp_path / "d1" d2 = tmp_path / "d2" d1.mkdir() d2.mkdir() - _write_module(d1, "lib", "def f() -> U32:\n return 1\n") - _write_module(d2, "lib", "def f() -> U32:\n return 2\n") + _write_sequence(d1, "lib", "def f() -> U32:\n return 1\n") + _write_sequence(d2, "lib", "def f() -> U32:\n return 2\n") main = """\ import lib x: U32 = lib.f() -assert x == 1 """ - assert_run_success(fprime_test_api, main, import_search_dirs=[str(d1), str(d2)]) + assert_compile_failure( + fprime_test_api, + main, + match="(?i)ambig", + import_search_dirs=[str(d1), str(d2)], + ) - def test_search_order_respects_dir_order(self, fprime_test_api, tmp_path): - """Reversing the search-dir order flips which module wins.""" + def test_whole_vs_member_across_dirs_is_ambiguous(self, fprime_test_api, tmp_path): + """Whole-path preference applies only within a single directory. A + member split in one dir and a whole-path split in another are two + candidate directories with splits: ambiguous.""" d1 = tmp_path / "d1" d2 = tmp_path / "d2" d1.mkdir() d2.mkdir() - _write_module(d1, "lib", "def f() -> U32:\n return 1\n") - _write_module(d2, "lib", "def f() -> U32:\n return 2\n") + # d1 can split `import a.b` as sequence `a` plus member `b`... + _write_sequence(d1, "a", "def b() -> U32:\n return 1\n") + # ...while d2 resolves it whole as the sequence `a.b`. + _write_sequence(d2, "a.b", "def f() -> U32:\n return 2\n") main = """\ -import lib - -x: U32 = lib.f() -assert x == 2 +import a.b """ - assert_run_success(fprime_test_api, main, import_search_dirs=[str(d2), str(d1)]) + assert_compile_failure( + fprime_test_api, + main, + match="(?i)ambig", + import_search_dirs=[str(d1), str(d2)], + ) - def test_dotted_module_resolved_across_search_dirs(self, fprime_test_api, tmp_path): - """Dotted resolution honors the search path: `pkg/mod.fpy` lives only in - the second dir.""" + def test_dotted_sequence_resolved_across_search_dirs( + self, fprime_test_api, tmp_path + ): + """Dotted resolution tries every search dir: `pkg/mod.fpy` lives only + in the second dir.""" d1 = tmp_path / "d1" d2 = tmp_path / "d2" d1.mkdir() d2.mkdir() - _write_module(d2, "pkg.mod", "def f() -> U32:\n return 5\n") + _write_sequence(d2, "pkg.mod", "def f() -> U32:\n return 5\n") main = """\ import pkg.mod @@ -918,7 +1245,7 @@ def test_dotted_module_resolved_across_search_dirs(self, fprime_test_api, tmp_pa def test_no_search_dirs_cannot_resolve(self, fprime_test_api, tmp_path): """With an empty search path, no import can resolve.""" - _write_module(tmp_path, "lib", "def f() -> U32:\n return 1\n") + _write_sequence(tmp_path, "lib", "def f() -> U32:\n return 1\n") main = """\ import lib """ @@ -926,13 +1253,12 @@ def test_no_search_dirs_cannot_resolve(self, fprime_test_api, tmp_path): class TestImportVariables: - """A module's top-level variable is both a side effect and a namespaced - symbol.""" + """A sequence's top-level variable is a side effect (its assignment runs at + sequence start), so an imported sequence may not declare one -- there is no + module-level variable, only functions.""" - def test_top_level_variable_is_side_effect_and_namespaced( - self, fprime_test_api, tmp_path - ): - _write_module( + def test_top_level_variable_is_error(self, fprime_test_api, tmp_path): + _write_sequence( tmp_path, "with_variable", """\ @@ -945,15 +1271,914 @@ def get() -> U32: main = """\ import with_variable -x: U32 = with_variable.counter +x: U32 = with_variable.get() +""" + assert_compile_failure( + fprime_test_api, + main, + match="only function definitions and imports", + import_search_dirs=[str(tmp_path)], + ) + + +class TestImportMember: + """`import seq.member` imports a single symbol of sequence `seq`, reachable + under the full dotted name `seq.member` and no shorter name. The sequence + path is the longest prefix that resolves to a file; the remaining suffix is + the member.""" + + def test_import_member_function(self, fprime_test_api, tmp_path): + """`import lib.add_one` exposes only `lib.add_one`, callable under its + full path.""" + _write_sequence( + tmp_path, + "lib", + """\ +def add_one(x: U32) -> U32: + return U32(x + 1) +""", + ) + main = """\ +import lib.add_one + +result: U32 = lib.add_one(41) +assert result == 42 +""" + assert_run_success(fprime_test_api, main, import_search_dirs=[str(tmp_path)]) + + def test_import_member_of_dotted_sequence(self, fprime_test_api, tmp_path): + """The sequence path may itself be dotted: `import pkg.mod.f` splits + into sequence `pkg.mod` and member `f`.""" + _write_sequence( + tmp_path, + "pkg.mod", + """\ +def f() -> U32: + return 5 +""", + ) + main = """\ +import pkg.mod.f + +x: U32 = pkg.mod.f() assert x == 5 -assert with_variable.counter == 5 -assert with_variable.get() == 5 """ - # The top-level assignment runs at sequence start -> side effect warning, - # but `with_variable.counter` still resolves as a namespaced symbol. - state, _, _ = compile_seq(main, import_search_dirs=[str(tmp_path)]) - assert any( - w.type == WarningType.IMPORT_SIDE_EFFECTS for w in state.warnings - ), f"expected an import-side-effects warning, got {state.warnings}" assert_run_success(fprime_test_api, main, import_search_dirs=[str(tmp_path)]) + + def test_member_requires_full_path(self, fprime_test_api, tmp_path): + """A member import is reachable only under its full path, never as a + bare name.""" + _write_sequence( + tmp_path, + "lib", + """\ +def add_one(x: U32) -> U32: + return U32(x + 1) +""", + ) + main = """\ +import lib.add_one + +result: U32 = add_one(41) +""" + assert_compile_failure( + fprime_test_api, main, import_search_dirs=[str(tmp_path)] + ) + + def test_member_import_hides_other_symbols(self, fprime_test_api, tmp_path): + """`import lib.a` imports only `a`; the sibling `b` is not bound, even + though the whole sequence is inlined at execution.""" + _write_sequence( + tmp_path, + "lib", + """\ +def a() -> U32: + return 1 + +def b() -> U32: + return 2 +""", + ) + main = """\ +import lib.a + +y: U32 = lib.b() +""" + assert_compile_failure( + fprime_test_api, main, import_search_dirs=[str(tmp_path)] + ) + + def test_missing_member_is_error(self, fprime_test_api, tmp_path): + """`import lib.nope` where `lib` has no symbol `nope` is an error.""" + _write_sequence( + tmp_path, + "lib", + """\ +def a() -> U32: + return 1 +""", + ) + main = """\ +import lib.nope +""" + assert_compile_failure( + fprime_test_api, main, import_search_dirs=[str(tmp_path)] + ) + + +class TestImportAlias: + """An `as` clause introduces no module chain: the alias is bound to the + leaf of the import -- the imported sequence's module (empty member path) or + the imported symbol (non-empty member path).""" + + def test_import_sequence_as_alias(self, fprime_test_api, tmp_path): + """`import lib as L` binds `L` to `lib`'s module; members are reached as + `L.member`.""" + _write_sequence( + tmp_path, + "lib", + """\ +def add_one(x: U32) -> U32: + return U32(x + 1) +""", + ) + main = """\ +import lib as L + +result: U32 = L.add_one(41) +assert result == 42 +""" + assert_run_success(fprime_test_api, main, import_search_dirs=[str(tmp_path)]) + + def test_dotted_import_as_alias(self, fprime_test_api, tmp_path): + """`import pkg.mod as m` binds `m` to the leaf module `mod`.""" + _write_sequence( + tmp_path, + "pkg.mod", + """\ +def f() -> U32: + return 5 +""", + ) + main = """\ +import pkg.mod as m + +x: U32 = m.f() +assert x == 5 +""" + assert_run_success(fprime_test_api, main, import_search_dirs=[str(tmp_path)]) + + def test_import_member_as_alias(self, fprime_test_api, tmp_path): + """`import lib.add_one as inc` binds `inc` directly to the symbol.""" + _write_sequence( + tmp_path, + "lib", + """\ +def add_one(x: U32) -> U32: + return U32(x + 1) +""", + ) + main = """\ +import lib.add_one as inc + +result: U32 = inc(41) +assert result == 42 +""" + assert_run_success(fprime_test_api, main, import_search_dirs=[str(tmp_path)]) + + def test_alias_hides_chain(self, fprime_test_api, tmp_path): + """With `import pkg.mod as m`, the chain `pkg.mod` is not introduced; + only `m` is bound.""" + _write_sequence( + tmp_path, + "pkg.mod", + """\ +def f() -> U32: + return 5 +""", + ) + main = """\ +import pkg.mod as m + +x: U32 = pkg.mod.f() +""" + assert_compile_failure( + fprime_test_api, main, import_search_dirs=[str(tmp_path)] + ) + + def test_alias_collides_with_local(self, fprime_test_api, tmp_path): + """An alias is an ordinary symbol: it collides with a local symbol in + the same name group.""" + _write_sequence( + tmp_path, + "lib", + """\ +def f() -> U32: + return 1 +""", + ) + main = """\ +import lib as dup + +def dup() -> U32: + return 2 +""" + assert_compile_failure( + fprime_test_api, main, import_search_dirs=[str(tmp_path)] + ) + + +class TestImportFrom: + """A `from` statement binds its imported symbols directly in the importing + sequence's global scope, introducing no module chain.""" + + def test_from_import_function(self, fprime_test_api, tmp_path): + """`from lib import add_one` binds the bare name `add_one`.""" + _write_sequence( + tmp_path, + "lib", + """\ +def add_one(x: U32) -> U32: + return U32(x + 1) +""", + ) + main = """\ +from lib import add_one + +result: U32 = add_one(41) +assert result == 42 +""" + assert_run_success(fprime_test_api, main, import_search_dirs=[str(tmp_path)]) + + def test_from_dotted_sequence(self, fprime_test_api, tmp_path): + """The sequence path of a `from` may be dotted.""" + _write_sequence( + tmp_path, + "pkg.mod", + """\ +def f() -> U32: + return 5 +""", + ) + main = """\ +from pkg.mod import f + +x: U32 = f() +assert x == 5 +""" + assert_run_success(fprime_test_api, main, import_search_dirs=[str(tmp_path)]) + + def test_from_import_as_alias(self, fprime_test_api, tmp_path): + """`from lib import add_one as inc` binds `inc`.""" + _write_sequence( + tmp_path, + "lib", + """\ +def add_one(x: U32) -> U32: + return U32(x + 1) +""", + ) + main = """\ +from lib import add_one as inc + +result: U32 = inc(41) +assert result == 42 +""" + assert_run_success(fprime_test_api, main, import_search_dirs=[str(tmp_path)]) + + def test_from_import_star(self, fprime_test_api, tmp_path): + """`from lib import *` binds every top-level symbol of `lib` under its + own name.""" + _write_sequence( + tmp_path, + "lib", + """\ +def a() -> U32: + return 1 + +def b() -> U32: + return 2 +""", + ) + main = """\ +from lib import * + +x: U32 = U32(a() + b()) +assert x == 3 +""" + assert_run_success(fprime_test_api, main, import_search_dirs=[str(tmp_path)]) + + def test_from_import_multiple_members(self, fprime_test_api, tmp_path): + """`from lib import a, b` binds each member under its own name.""" + _write_sequence( + tmp_path, + "lib", + """\ +def a() -> U32: + return 1 + +def b() -> U32: + return 2 +""", + ) + main = """\ +from lib import a, b + +x: U32 = U32(a() + b()) +assert x == 3 +""" + assert_run_success(fprime_test_api, main, import_search_dirs=[str(tmp_path)]) + + def test_from_import_multiple_with_aliases(self, fprime_test_api, tmp_path): + """Each member in a list may carry its own `as` alias.""" + _write_sequence( + tmp_path, + "lib", + """\ +def a() -> U32: + return 1 + +def b() -> U32: + return 2 +""", + ) + main = """\ +from lib import a as first, b as second + +x: U32 = U32(first() + second()) +assert x == 3 +""" + assert_run_success(fprime_test_api, main, import_search_dirs=[str(tmp_path)]) + + def test_from_import_parenthesized_single_line(self, fprime_test_api, tmp_path): + """The member list may be parenthesized.""" + _write_sequence( + tmp_path, + "lib", + """\ +def a() -> U32: + return 1 + +def b() -> U32: + return 2 +""", + ) + main = """\ +from lib import (a, b) + +x: U32 = U32(a() + b()) +assert x == 3 +""" + assert_run_success(fprime_test_api, main, import_search_dirs=[str(tmp_path)]) + + def test_from_import_parenthesized_multiline(self, fprime_test_api, tmp_path): + """The parenthesized member list may span multiple lines and end with a + trailing comma.""" + _write_sequence( + tmp_path, + "lib", + """\ +def a() -> U32: + return 1 + +def b() -> U32: + return 2 + +def c() -> U32: + return 3 +""", + ) + main = """\ +from lib import ( + a, + b as bb, + c, +) + +x: U32 = U32(a() + bb() + c()) +assert x == 6 +""" + assert_run_success(fprime_test_api, main, import_search_dirs=[str(tmp_path)]) + + def test_from_import_duplicate_member_is_error(self, fprime_test_api, tmp_path): + """Binding the same name twice in one member list is a collision.""" + _write_sequence( + tmp_path, + "lib", + """\ +def a() -> U32: + return 1 +""", + ) + main = """\ +from lib import a, a +""" + assert_compile_failure( + fprime_test_api, main, import_search_dirs=[str(tmp_path)] + ) + + def test_from_import_does_not_introduce_sequence_name( + self, fprime_test_api, tmp_path + ): + """`from lib import add_one` introduces no module chain, so `lib.add_one` + is not reachable.""" + _write_sequence( + tmp_path, + "lib", + """\ +def add_one(x: U32) -> U32: + return U32(x + 1) +""", + ) + main = """\ +from lib import add_one + +result: U32 = lib.add_one(41) +""" + assert_compile_failure( + fprime_test_api, main, import_search_dirs=[str(tmp_path)] + ) + + def test_from_import_missing_member_is_error(self, fprime_test_api, tmp_path): + """`from lib import nope` where `lib` has no `nope` is an error.""" + _write_sequence( + tmp_path, + "lib", + """\ +def a() -> U32: + return 1 +""", + ) + main = """\ +from lib import nope +""" + assert_compile_failure( + fprime_test_api, main, import_search_dirs=[str(tmp_path)] + ) + + def test_from_import_collides_with_local(self, fprime_test_api, tmp_path): + """A `from`-imported name is ordinary and collides with a local symbol + in the same name group.""" + _write_sequence( + tmp_path, + "lib", + """\ +def f() -> U32: + return 1 +""", + ) + main = """\ +from lib import f + +def f() -> U32: + return 2 +""" + assert_compile_failure( + fprime_test_api, main, import_search_dirs=[str(tmp_path)] + ) + + def test_from_star_collides_across_sequences(self, fprime_test_api, tmp_path): + """Two `from ... import *` statements that both define `f` collide; + unlike module chains, these names do not merge.""" + _write_sequence(tmp_path, "a", "def f() -> U32:\n return 1\n") + _write_sequence(tmp_path, "b", "def f() -> U32:\n return 2\n") + main = """\ +from a import * +from b import * +""" + assert_compile_failure( + fprime_test_api, main, import_search_dirs=[str(tmp_path)] + ) + + def test_from_import_side_effects_is_error(self, fprime_test_api, tmp_path): + """A `from` import brings in the whole sequence, so a side-effecting + imported sequence is an error just as a plain import of it would be.""" + _write_sequence( + tmp_path, + "side_effects", + """\ +CdhCore.cmdDisp.CMD_NO_OP() + +def noop_wrapper(): + CdhCore.cmdDisp.CMD_NO_OP() +""", + ) + main = """\ +from side_effects import noop_wrapper + +noop_wrapper() +""" + assert_compile_failure( + fprime_test_api, + main, + match="only function definitions and imports", + import_search_dirs=[str(tmp_path)], + ) + + +class TestImportDuplicateSequence: + """A sequence may be imported more than once in one file. Whether that is + allowed follows from the collision rule on the names each import binds, and + the sequence is compiled just once regardless.""" + + def test_import_and_from_same_sequence_coexist(self, fprime_test_api, tmp_path): + """`import lib` binds the module `lib`; `from lib import b` binds `b`. + The names differ, so they coexist.""" + _write_sequence( + tmp_path, + "lib", + """\ +def a() -> U32: + return 1 + +def b() -> U32: + return 2 +""", + ) + main = """\ +import lib +from lib import b + +x: U32 = U32(lib.a() + b()) +assert x == 3 +""" + state, _, _ = compile_seq(main, import_search_dirs=[str(tmp_path)]) + assert state.warnings == [] + assert_run_success(fprime_test_api, main, import_search_dirs=[str(tmp_path)]) + + def test_two_members_same_sequence_collides(self, fprime_test_api, tmp_path): + """`import lib.a` and `import lib.b` both bind the sequence module `lib`, + which collides.""" + _write_sequence( + tmp_path, + "lib", + """\ +def a() -> U32: + return 1 + +def b() -> U32: + return 2 +""", + ) + main = """\ +import lib.a +import lib.b +""" + assert_compile_failure( + fprime_test_api, + main, + match="collides with an existing imported sequence", + import_search_dirs=[str(tmp_path)], + ) + + def test_whole_and_member_same_sequence_collides(self, fprime_test_api, tmp_path): + """`import lib` and `import lib.a` both bind the sequence module `lib`, + which collides.""" + _write_sequence( + tmp_path, + "lib", + """\ +def a() -> U32: + return 1 +""", + ) + main = """\ +import lib +import lib.a +""" + assert_compile_failure( + fprime_test_api, + main, + match="collides with an existing imported sequence", + import_search_dirs=[str(tmp_path)], + ) + + def test_two_from_same_sequence_coexist(self, fprime_test_api, tmp_path): + """Two `from lib import ...` statements that name distinct members bind + distinct names, so they coexist (and `lib` is compiled once).""" + _write_sequence( + tmp_path, + "lib", + """\ +def a() -> U32: + return 1 + +def b() -> U32: + return 2 +""", + ) + main = """\ +from lib import a +from lib import b + +x: U32 = U32(a() + b()) +assert x == 3 +""" + state, _, _ = compile_seq(main, import_search_dirs=[str(tmp_path)]) + assert state.warnings == [] + assert_run_success(fprime_test_api, main, import_search_dirs=[str(tmp_path)]) + + +class TestImportRelative: + """Leading dots make an import relative: one dot anchors at the importing + sequence's own directory, each extra dot one parent up, and the base + search path is never consulted. Absolute imports, conversely, never see + the importing sequence's own directory. The dots affect resolution only; + the bound module chain is the path after them.""" + + def test_relative_import_of_sibling(self, fprime_test_api, tmp_path): + """`pkg/helper.fpy` imports its sibling `pkg/sibling.fpy` with + `import .sibling`; `pkg/` is not on the base search path, only the + anchor makes it reachable.""" + _write_sequence(tmp_path, "pkg.sibling", "def thing() -> U32:\n return 9\n") + _write_sequence( + tmp_path, + "pkg.helper", + """\ +import .sibling + +def f() -> U32: + return sibling.thing() +""", + ) + main = """\ +import pkg.helper + +x: U32 = pkg.helper.f() +assert x == 9 +""" + assert_run_success(fprime_test_api, main, import_search_dirs=[str(tmp_path)]) + + def test_absolute_import_ignores_importer_directory( + self, fprime_test_api, tmp_path + ): + """An absolute `import util` sees only the base search path: a + same-named file sitting next to the importing sequence must not + shadow it (no Python-2-style implicit relative imports).""" + _write_sequence(tmp_path, "util", "def v() -> U32:\n return 1\n") + _write_sequence(tmp_path, "pkg.util", "def v() -> U32:\n return 2\n") + _write_sequence( + tmp_path, + "pkg.helper", + """\ +import util + +def f() -> U32: + return util.v() +""", + ) + main = """\ +import pkg.helper + +x: U32 = pkg.helper.f() +assert x == 1 +""" + assert_run_success(fprime_test_api, main, import_search_dirs=[str(tmp_path)]) + + def test_relative_import_does_not_search_base_path(self, fprime_test_api, tmp_path): + """`import .nope` must not fall back to the base search path, even + when a `nope.fpy` exists there.""" + _write_sequence(tmp_path, "nope", "def f() -> U32:\n return 1\n") + _write_sequence( + tmp_path, + "pkg.helper", + """\ +import .nope + +def f() -> U32: + return nope.f() +""", + ) + main = """\ +import pkg.helper +""" + assert_compile_failure( + fprime_test_api, main, import_search_dirs=[str(tmp_path)] + ) + + def test_relative_binds_path_after_dots(self, fprime_test_api, tmp_path): + """`import .sub.mod` resolves `/sub/mod.fpy` and binds the + chain `sub.mod`: the dots are not part of the name.""" + _write_sequence(tmp_path, "lib.sub.mod", "def f() -> U32:\n return 3\n") + _write_sequence( + tmp_path, + "lib.helper", + """\ +import .sub.mod + +def f() -> U32: + return sub.mod.f() +""", + ) + main = """\ +import lib.helper + +x: U32 = lib.helper.f() +assert x == 3 +""" + assert_run_success(fprime_test_api, main, import_search_dirs=[str(tmp_path)]) + + def test_parent_relative_import(self, fprime_test_api, tmp_path): + """`import ..util` in `lib/sub/inner.fpy` anchors one parent up, + resolving `lib/util.fpy` and binding module `util`.""" + _write_sequence(tmp_path, "lib.util", "def v() -> U32:\n return 4\n") + _write_sequence( + tmp_path, + "lib.sub.inner", + """\ +import ..util + +def f() -> U32: + return util.v() +""", + ) + main = """\ +import lib.sub.inner + +x: U32 = lib.sub.inner.f() +assert x == 4 +""" + assert_run_success(fprime_test_api, main, import_search_dirs=[str(tmp_path)]) + + def test_relative_member_import(self, fprime_test_api, tmp_path): + """Member splitting applies to relative imports too: `import + .sibling.thing` splits into sequence `.sibling` and member `thing`.""" + _write_sequence(tmp_path, "pkg.sibling", "def thing() -> U32:\n return 9\n") + _write_sequence( + tmp_path, + "pkg.helper", + """\ +import .sibling.thing + +def f() -> U32: + return sibling.thing() +""", + ) + main = """\ +import pkg.helper + +x: U32 = pkg.helper.f() +assert x == 9 +""" + assert_run_success(fprime_test_api, main, import_search_dirs=[str(tmp_path)]) + + def test_relative_alias(self, fprime_test_api, tmp_path): + """`import .sibling as s` binds only `s`, as with absolute aliases.""" + _write_sequence(tmp_path, "pkg.sibling", "def thing() -> U32:\n return 9\n") + _write_sequence( + tmp_path, + "pkg.helper", + """\ +import .sibling as s + +def f() -> U32: + return s.thing() +""", + ) + main = """\ +import pkg.helper + +x: U32 = pkg.helper.f() +assert x == 9 +""" + assert_run_success(fprime_test_api, main, import_search_dirs=[str(tmp_path)]) + + def test_relative_from_import(self, fprime_test_api, tmp_path): + """`from .sibling import thing` binds the bare member name.""" + _write_sequence(tmp_path, "pkg.sibling", "def thing() -> U32:\n return 9\n") + _write_sequence( + tmp_path, + "pkg.helper", + """\ +from .sibling import thing + +def f() -> U32: + return thing() +""", + ) + main = """\ +import pkg.helper + +x: U32 = pkg.helper.f() +assert x == 9 +""" + assert_run_success(fprime_test_api, main, import_search_dirs=[str(tmp_path)]) + + def test_bare_dot_from_is_error(self, fprime_test_api, tmp_path): + """`from . import sibling` is invalid: the leading dots must be + followed by at least one name (`from` members are definitions, not + sequences). Write `import .sibling` instead.""" + _write_sequence(tmp_path, "sibling", "def f() -> U32:\n return 1\n") + main = """\ +from . import sibling +""" + assert_compile_failure( + fprime_test_api, + main, + import_search_dirs=[str(tmp_path)], + main_file_dir=str(tmp_path), + ) + + def test_relative_import_in_main(self, fprime_test_api, tmp_path): + """The main sequence's own relative imports anchor at its directory + (`main_file_dir` here; the input file's directory in the CLI). An + empty base search path proves it plays no role.""" + _write_sequence(tmp_path, "util", "def v() -> U32:\n return 6\n") + main = """\ +import .util + +x: U32 = util.v() +assert x == 6 +""" + assert_run_success( + fprime_test_api, main, import_search_dirs=[], main_file_dir=str(tmp_path) + ) + + def test_relative_import_without_location_is_error(self, fprime_test_api, tmp_path): + """A relative import in a sequence with no containing directory (a + stream; `main_file_dir=None` here) has no anchor and is an error.""" + _write_sequence(tmp_path, "util", "def v() -> U32:\n return 6\n") + main = """\ +import .util +""" + assert_compile_failure( + fprime_test_api, main, import_search_dirs=[str(tmp_path)] + ) + + def test_relative_and_absolute_same_file_collides(self, fprime_test_api, tmp_path): + """An absolute and a relative import that resolve to the same file import + the same sequence; both bind the sequence module `util`, which collides + (the shared file is still compiled only once).""" + _write_sequence(tmp_path, "util", "def v() -> U32:\n return 6\n") + main = """\ +import util +import .util +""" + assert_compile_failure( + fprime_test_api, + main, + match="collides with an existing imported sequence", + import_search_dirs=[str(tmp_path)], + main_file_dir=str(tmp_path), + ) + + +class TestImportModuleMerging: + # FIXME i think these might be duplicates? + """Package modules (non-leaf chain segments) merge freely; sequence + modules (a module holding a file's definitions -- chain leaf or alias) + never merge with each other.""" + + def test_sequence_and_package_module_merge(self, fprime_test_api, tmp_path): + """`import pkg` (the file `pkg.fpy`) and `import pkg.mod` (the file + `pkg/mod.fpy`) share the name `pkg`: the sequence module and the + package module merge, holding `pkg.fpy`'s definitions alongside + module `mod`.""" + _write_sequence(tmp_path, "pkg", "def top() -> U32:\n return 1\n") + _write_sequence(tmp_path, "pkg.mod", "def f() -> U32:\n return 5\n") + main = """\ +import pkg +import pkg.mod + +x: U32 = U32(pkg.top() + pkg.mod.f()) +assert x == 6 +""" + assert_run_success(fprime_test_api, main, import_search_dirs=[str(tmp_path)]) + + def test_two_aliases_same_name_collide(self, fprime_test_api, tmp_path): + """Two different sequences aliased to one name are two sequence + modules on the same name: a collision, not a merge.""" + _write_sequence(tmp_path, "a", "def f() -> U32:\n return 1\n") + _write_sequence(tmp_path, "b", "def g() -> U32:\n return 2\n") + main = """\ +import a as m +import b as m +""" + assert_compile_failure( + fprime_test_api, main, import_search_dirs=[str(tmp_path)] + ) + + def test_relative_and_absolute_leaf_collision(self, fprime_test_api, tmp_path): + """An absolute `import util` and a relative `import .util` naming two + DIFFERENT files both bind a sequence module `util`: a collision, even + though the two files' definitions do not overlap.""" + d_base = tmp_path / "base" + d_main = tmp_path / "main" + d_base.mkdir() + d_main.mkdir() + _write_sequence(d_base, "util", "def f() -> U32:\n return 1\n") + _write_sequence(d_main, "util", "def g() -> U32:\n return 2\n") + main = """\ +import util +import .util +""" + assert_compile_failure( + fprime_test_api, + main, + import_search_dirs=[str(d_base)], + main_file_dir=str(d_main), + ) + + +# FIXME what happens if an import has relative imports? do we test this? the relative imports from the imported lib should +# resolve relative to the imported lib, not the original importing file diff --git a/test/fpy/test_main.py b/test/fpy/test_main.py index 40be627..4d47405 100644 --- a/test/fpy/test_main.py +++ b/test/fpy/test_main.py @@ -42,7 +42,7 @@ def fake_get_base_compile_state(dictionary, ground_binary_dir=None, **kwargs): monkeypatch.setattr( fpy_main, "analysis_to_fpybc_directives", - lambda body, state: (["directive"], []), + lambda state: (["directive"], []), ) monkeypatch.setattr( fpy_main, "serialize_directives", lambda directives, arg_specs: (b"\x01", 0x1) @@ -83,7 +83,7 @@ def fake_get_base_compile_state(dictionary, ground_binary_dir=None, **kwargs): monkeypatch.setattr( fpy_main, "analysis_to_fpybc_directives", - lambda body, state: (["directive"], []), + lambda state: (["directive"], []), ) monkeypatch.setattr( fpy_main, "serialize_directives", lambda directives, arg_specs: (b"\x01", 0x1) @@ -116,7 +116,7 @@ def fake_get_base_compile_state(dictionary, ground_binary_dir=None, **kwargs): monkeypatch.setattr( fpy_main, "analysis_to_fpybc_directives", - lambda body, state: (["directive"], []), + lambda state: (["directive"], []), ) monkeypatch.setattr( fpy_main, "serialize_directives", lambda directives, arg_specs: (b"\x01", 0x1) @@ -126,9 +126,10 @@ def fake_get_base_compile_state(dictionary, ground_binary_dir=None, **kwargs): return captured_kwargs -def test_compile_main_include_dirs_after_input_parent(monkeypatch, tmp_path): - """-i/--include dirs are appended, resolved, after the input file's own dir, - which is always searched first.""" +def test_compile_main_include_dirs_are_base_search_path(monkeypatch, tmp_path): + """The base search path is exactly the resolved -i/--include dirs; the + input file's own dir is NOT on it (it anchors relative imports instead, + via main_file_dir).""" input_path = tmp_path / "sub" / "seq.fpy" input_path.parent.mkdir() input_path.write_text("content") @@ -153,14 +154,42 @@ def test_compile_main_include_dirs_after_input_parent(monkeypatch, tmp_path): ) assert captured_kwargs["import_search_dirs"] == [ - str(input_path.parent.resolve()), str(inc_a.resolve()), str(inc_b.resolve()), ] + assert captured_kwargs["main_file_dir"] == str(input_path.parent.resolve()) -def test_compile_main_include_defaults_to_input_parent_only(monkeypatch, tmp_path): - """With no -i flags, the search path is just the input file's own dir.""" +def test_compile_main_duplicate_includes_are_deduped(monkeypatch, tmp_path): + """A repeated -i directory (even spelled differently) collapses to one + search-path entry after resolution, so it cannot manufacture an + ambiguity error.""" + input_path = tmp_path / "seq.fpy" + input_path.write_text("content") + dict_path = tmp_path / "dict.json" + dict_path.write_text("{}") + inc = tmp_path / "inc" + inc.mkdir() + + captured_kwargs = _run_compile_capturing_kwargs( + monkeypatch, + [ + str(input_path), + "--dictionary", + str(dict_path), + "-i", + str(inc), + "-i", + str(tmp_path / "inc" / ".." / "inc"), + ], + ) + + assert captured_kwargs["import_search_dirs"] == [str(inc.resolve())] + + +def test_compile_main_no_includes_means_empty_search_path(monkeypatch, tmp_path): + """With no -i flags, the base search path is empty; the input file's own + dir still anchors its relative imports.""" input_path = tmp_path / "seq.fpy" input_path.write_text("content") dict_path = tmp_path / "dict.json" @@ -175,7 +204,8 @@ def test_compile_main_include_defaults_to_input_parent_only(monkeypatch, tmp_pat ], ) - assert captured_kwargs["import_search_dirs"] == [str(input_path.parent.resolve())] + assert captured_kwargs["import_search_dirs"] == [] + assert captured_kwargs["main_file_dir"] == str(input_path.parent.resolve()) def test_compile_main_missing_input(tmp_path, capsys): @@ -210,8 +240,7 @@ def test_compile_main_fpyasm_output(monkeypatch, tmp_path, capsys): ) monkeypatch.setattr(fpy_main, "analyze_ast", lambda body, state: state) - def fake_analysis_to_fpybc_directives(body, state): - assert body == "AST" + def fake_analysis_to_fpybc_directives(state): assert state == "STATE" return ["directive"], [] @@ -258,8 +287,7 @@ def test_compile_main_wat_output(monkeypatch, tmp_path, capsys): ) monkeypatch.setattr(fpy_main, "analyze_ast", lambda body, state: state) - def fake_analysis_to_wat(body, state): - assert body == "AST" + def fake_analysis_to_wat(state): assert state == "STATE" return "WAT_TEXT", [] @@ -295,7 +323,7 @@ def test_compile_main_binary_output(monkeypatch, tmp_path, capsys): monkeypatch.setattr( fpy_main, "analysis_to_fpybc_directives", - lambda body, state: (["directive"], []), + lambda state: (["directive"], []), ) monkeypatch.setattr( fpy_main, @@ -441,7 +469,7 @@ def fake_text_to_ast(text): directive = ConstCmdDirective(cmd_opcode=0x10006001, args=b"\xab\xcd") monkeypatch.setattr( - fpy_main, "analysis_to_fpybc_directives", lambda body, state: ([directive], []) + fpy_main, "analysis_to_fpybc_directives", lambda state: ([directive], []) ) sent = {} @@ -477,7 +505,7 @@ def test_cmd_main_compile_error(monkeypatch, capsys): ) monkeypatch.setattr(fpy_main, "analyze_ast", lambda body, state: state) - def raise_compile_error(body, state): + def raise_compile_error(state): raise fpy_error.CompileError("bad arg", None) monkeypatch.setattr(fpy_main, "analysis_to_fpybc_directives", raise_compile_error) @@ -508,7 +536,7 @@ def test_cmd_main_non_const_arg(monkeypatch, capsys): monkeypatch.setattr( fpy_main, "analysis_to_fpybc_directives", - lambda body, state: ([StackCmdDirective(args_size=10)], []), + lambda state: ([StackCmdDirective(args_size=10)], []), ) with pytest.raises(SystemExit) as exc: @@ -538,7 +566,7 @@ def test_cmd_main_send_failure(monkeypatch, capsys): monkeypatch.setattr( fpy_main, "analysis_to_fpybc_directives", - lambda body, state: ([directive], []), + lambda state: ([directive], []), ) def fail_send(*a): @@ -574,7 +602,7 @@ def fake_get_base_compile_state(dictionary, ground_binary_dir=None, **kwargs): monkeypatch.setattr( fpy_main, "analysis_to_fpybc_directives", - lambda body, state: ([ConstCmdDirective(cmd_opcode=0x10006001, args=b"")], []), + lambda state: ([ConstCmdDirective(cmd_opcode=0x10006001, args=b"")], []), ) monkeypatch.setattr(fpy_main, "send_command_zmq", lambda *a: None) @@ -608,7 +636,7 @@ def test_cmd_main_zmq_addr(monkeypatch, capsys): monkeypatch.setattr( fpy_main, "analysis_to_fpybc_directives", - lambda body, state: ([directive], []), + lambda state: ([directive], []), ) sent = {} diff --git a/test/fpy/test_parsing.py b/test/fpy/test_parsing.py index a37b15a..858e7ff 100644 --- a/test/fpy/test_parsing.py +++ b/test/fpy/test_parsing.py @@ -663,3 +663,174 @@ def test_multiline_assert(self, fprime_test_api): ) """ assert_run_success(fprime_test_api, seq) + + +class TestTrailingWhitespaceAtEndOfFile: + """A file that ends *without* a final newline must still compile, even when + the last line is a lone tab, trailing spaces, or an (over/under-indented) + comment. + + Regression tests for https://github.com/fprime-community/fpy/issues/61 + ("Tabbed New Line does not compile at the end of a if/else statement"). The + indenter derives INDENT/DEDENT from the whitespace each _NEWLINE token + carries; with no trailing newline the last line's indentation used to be + misread as a fresh indentation level at end-of-file, emitting a spurious + INDENT (trailing tab / over-indented comment), a bogus dedent + (under-indented comment), or a stray blank logical line inside a block. + Python ignores such trailing blank/comment lines, and now so does fpy. + + The sequences are built with explicit ``\\n``/``\\t`` escapes (not + triple-quoted literals) so the significant trailing whitespace survives + editors and formatters. + """ + + def test_trailing_tab_after_statement(self, fprime_test_api): + # A lone tab on the final line, no trailing newline. + seq = "x: U32 = 1\nassert x == 1\n\t" + assert_run_success(fprime_test_api, seq) + + def test_trailing_spaces_after_statement(self, fprime_test_api): + seq = "x: U32 = 1\nassert x == 1\n " + assert_run_success(fprime_test_api, seq) + + def test_trailing_over_indented_comment(self, fprime_test_api): + seq = "x: U32 = 1\nassert x == 1\n # trailing comment" + assert_run_success(fprime_test_api, seq) + + def test_trailing_tab_then_comment(self, fprime_test_api): + # The exact shape called out in the issue: a tab, then a comment, at EOF. + seq = "x: U32 = 1\nassert x == 1\n\t# there is a tab here" + assert_run_success(fprime_test_api, seq) + + def test_trailing_comment_at_body_indent(self, fprime_test_api): + # Comment aligned with the block body (used to leave a stray blank + # logical line inside the block, breaking it). + seq = "if True:\n x: U32 = 1\n # aligned comment" + assert_run_success(fprime_test_api, seq) + + def test_trailing_under_indented_comment(self, fprime_test_api): + # Comment indented between column 0 and the body (used to raise a + # DedentError for dedenting to an unknown column). + seq = "if True:\n x: U32 = 1\n # under-indented comment" + assert_run_success(fprime_test_api, seq) + + def test_trailing_tab_after_if_elif(self, fprime_test_api): + seq = "x: U32 = 0\nif True:\n x = 1\nelif False:\n x = 2\n\t" + assert_run_success(fprime_test_api, seq) + + def test_trailing_tab_after_if_else(self, fprime_test_api): + seq = "x: U32 = 0\nif True:\n x = 1\nelse:\n x = 2\n\t" + assert_run_success(fprime_test_api, seq) + + def test_trailing_tab_after_for_body(self, fprime_test_api): + seq = "s: I64 = 0\nfor i in 0 .. 3:\n s = s + i\n\t" + assert_run_success(fprime_test_api, seq) + + def test_trailing_tab_after_while_body(self, fprime_test_api): + seq = "i: U64 = 0\nwhile i < 3:\n i = i + 1\n\t" + assert_run_success(fprime_test_api, seq) + + def test_trailing_tab_after_def_body(self, fprime_test_api): + seq = "def f() -> U32:\n return 1\n\t" + assert_run_success(fprime_test_api, seq) + + def test_trailing_tab_after_check_body(self, fprime_test_api): + seq = "x: bool = False\ncheck True:\n x = True\n\t" + assert_run_success(fprime_test_api, seq) + + def test_trailing_tabs_after_nested_blocks(self, fprime_test_api): + # Several dedent levels to close at once, with an over-indented last line. + seq = "x: U32 = 0\nif True:\n if True:\n x = 1\n\t\t\t" + assert_run_success(fprime_test_api, seq) + + def test_trailing_crlf_then_tab(self, fprime_test_api): + # CRLF line endings, trailing tab, no final newline. + seq = "x: U32 = 1\r\nassert x == 1\r\n\t" + assert_run_success(fprime_test_api, seq) + + def test_only_whitespace_file(self, fprime_test_api): + # A file that is nothing but a tab must compile to an empty sequence. + seq = "\t" + assert_run_success(fprime_test_api, seq) + + def test_only_indented_comment_file(self, fprime_test_api): + seq = " # just a comment, indented, no newline" + assert_run_success(fprime_test_api, seq) + + def test_issue_61_reported_example(self, fprime_test_api): + # Faithful adaptation of the snippet in the issue: an if/elif whose final + # branch is followed by a tab + comment at end of file, with a multiline + # log() call (no backslash) inside a branch body. + seq = ( + "temp: I64 = -1\n" + "if temp < 0:\n" + ' log("temperature sensor invalid reading",\n' + " Fw.LogSeverity.WARNING_HI)\n" + "elif temp > 100:\n" + ' log("temp high", Fw.LogSeverity.WARNING_HI)\n' + "\t# there is a tab here" + ) + assert_run_success(fprime_test_api, seq) + + +class TestMultilineConstructorsWithoutBackslash: + """Multiline function calls / type constructors must not require a backslash + at the end of each line: inside ``()``, ``[]`` or ``{}`` the expression + continues implicitly, exactly like Python. + + Regression tests for https://github.com/fprime-community/fpy/issues/68 + ("Require fewer backslashes in multiline exprs"). Existing backslash + continuation must keep working too. + """ + + def test_constructor_args_on_separate_lines(self, fprime_test_api): + # The issue's shape: constructor arguments on separate lines, aligned + # under the open paren, with no backslashes. + seq = ( + "v: Fw.TimeIntervalValue = Fw.TimeIntervalValue(10,\n" + " 500)\n" + "assert v.seconds == 10\n" + "assert v.useconds == 500\n" + ) + assert_run_success(fprime_test_api, seq) + + def test_constructor_multiline_is_last_statement_no_newline(self, fprime_test_api): + # Multiline constructor as the final statement, file ends with no + # trailing newline (exercises issue #61 and #68 together). + seq = "v: Fw.TimeIntervalValue = Fw.TimeIntervalValue(\n 10,\n 500\n)" + assert_run_success(fprime_test_api, seq) + + def test_constructor_multiline_then_trailing_tab(self, fprime_test_api): + seq = "v: Fw.TimeIntervalValue = Fw.TimeIntervalValue(\n 10,\n 500\n)\n\t" + assert_run_success(fprime_test_api, seq) + + def test_constructor_multiline_with_backslash_still_works(self, fprime_test_api): + # Redundant backslashes inside the parens must not break. + seq = ( + "v: Fw.TimeIntervalValue = Fw.TimeIntervalValue(10, \\\n" + " 500)\n" + "assert v.useconds == 500\n" + ) + assert_run_success(fprime_test_api, seq) + + def test_constructor_multiline_backslash_on_every_line(self, fprime_test_api): + # The full "backslash at the end of each line" style users reached for; + # it must remain valid alongside the now-unnecessary implicit form. + seq = ( + "v: Fw.TimeIntervalValue = Fw.TimeIntervalValue( \\\n" + " 10, \\\n" + " 500 \\\n" + ")\n" + "assert v.seconds == 10\n" + ) + assert_run_success(fprime_test_api, seq) + + def test_nested_multiline_constructor(self, fprime_test_api): + seq = ( + "v: Fw.TimeIntervalValue = Fw.TimeIntervalValue(\n" + " [10, 20, 30][0],\n" + " 500,\n" + ")\n" + "assert v.seconds == 10\n" + ) + assert_run_success(fprime_test_api, seq) diff --git a/test/fpy/test_seq_calling.py b/test/fpy/test_seq_calling.py index e65d8c7..2188bc2 100644 --- a/test/fpy/test_seq_calling.py +++ b/test/fpy/test_seq_calling.py @@ -39,7 +39,7 @@ def _compile_to_bin(seq_text: str, out_path: Path, ground_binary_dir: str = None body = text_to_ast(seq_text) assert body is not None, "Failed to parse child sequence" state = analyze_ast(body, state) - directives, arg_types = analysis_to_fpybc_directives(body, state) + directives, arg_types = analysis_to_fpybc_directives(state) arg_specs = [(name, t.name, t.max_size) for name, t in arg_types] data, _ = serialize_directives(directives, arg_specs=arg_specs) out_path.write_bytes(data) @@ -726,7 +726,7 @@ def test_oversized_args_use_dictionary_capacity(self, fprime_test_api): assert body is not None state = analyze_ast(body, state) # should compile without error (fits in the 1024-byte buffer) - analysis_to_fpybc_directives(body, state) + analysis_to_fpybc_directives(state) def test_args_still_bounded_by_dictionary_capacity(self, fprime_test_api): """Args larger than the dictionary's buffer must still be rejected, @@ -744,7 +744,7 @@ def test_args_still_bounded_by_dictionary_capacity(self, fprime_test_api): body = text_to_ast(child_seq) assert body is not None state = analyze_ast(body, state) - directives, arg_types = analysis_to_fpybc_directives(body, state) + directives, arg_types = analysis_to_fpybc_directives(state) arg_specs = [(name, t.name, t.max_size) for name, t in arg_types] data, _ = serialize_directives(directives, arg_specs=arg_specs) Path(child_path).write_bytes(data) @@ -758,7 +758,7 @@ def test_args_still_bounded_by_dictionary_capacity(self, fprime_test_api): assert body is not None with pytest.raises(fpy.error.CompileError) as exc_info: state = analyze_ast(body, state) - analysis_to_fpybc_directives(body, state) + analysis_to_fpybc_directives(state) assert "exceed" in str(exc_info.value) and "64 bytes" in str( exc_info.value ), f"Diagnostic should mention the 64-byte capacity, got: {exc_info.value}" diff --git a/test/fpy/test_warnings.py b/test/fpy/test_warnings.py index e3c8de4..5fc8ffd 100644 --- a/test/fpy/test_warnings.py +++ b/test/fpy/test_warnings.py @@ -31,15 +31,15 @@ def test_single_type(self): assert parse_warning_set("empty-range") == {WarningType.EMPTY_RANGE} def test_comma_separated(self): - assert parse_warning_set("empty-range,import-side-effects") == { + assert parse_warning_set("empty-range,import-underscore") == { WarningType.EMPTY_RANGE, - WarningType.IMPORT_SIDE_EFFECTS, + WarningType.IMPORT_UNDERSCORE, } def test_whitespace_tolerated(self): - assert parse_warning_set(" empty-range , import-side-effects ") == { + assert parse_warning_set(" empty-range , import-underscore ") == { WarningType.EMPTY_RANGE, - WarningType.IMPORT_SIDE_EFFECTS, + WarningType.IMPORT_UNDERSCORE, } def test_empty_spec_is_empty_set(self): @@ -87,7 +87,7 @@ def test_ignore_all_suppresses_warning(self): def test_ignore_unrelated_type_keeps_warning(self): state, _, _ = compile_seq( - EMPTY_RANGE_SEQ, ignored_warnings={WarningType.IMPORT_SIDE_EFFECTS} + EMPTY_RANGE_SEQ, ignored_warnings={WarningType.IMPORT_UNDERSCORE} ) assert any(w.type == WarningType.EMPTY_RANGE for w in state.warnings) @@ -106,7 +106,7 @@ def test_escalated_message_mentions_type(self): def test_unrelated_error_type_does_not_fail(self): # Escalating a different warning type must not affect the empty-range warning. state, _, _ = compile_seq( - EMPTY_RANGE_SEQ, error_warnings={WarningType.IMPORT_SIDE_EFFECTS} + EMPTY_RANGE_SEQ, error_warnings={WarningType.IMPORT_UNDERSCORE} ) assert any(w.type == WarningType.EMPTY_RANGE for w in state.warnings) diff --git a/test/fpy/test_wasm.py b/test/fpy/test_wasm.py index a57b0bb..35dc41b 100644 --- a/test/fpy/test_wasm.py +++ b/test/fpy/test_wasm.py @@ -43,7 +43,7 @@ def _seq_to_llvm_module(seq: str): state = get_base_compile_state(default_dictionary, None) body = text_to_ast(seq) state = analyze_ast(body, state) - return GenerateLlvmModule().emit(body, state) + return GenerateLlvmModule().emit(state.root_block, state) def _emit_wasm_asm(seq: str, cpu: str) -> str: