Skip to content

Add a bit of documentation for typed COBOL AST #576

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
GBertholon wants to merge 1 commit into
base: master
Choose a base branch
from
+14 −10
Draft

Add a bit of documentation for typed COBOL AST #576

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 6 additions & 5 deletions src/lsp/cobol_data/data_types.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,14 +83,14 @@ and item_definition =
and field_definition =
{
field_qualname: Cobol_ptree.qualname with_loc option;
field_redefines: Cobol_ptree.qualname with_loc option; (* redef only *)
field_leading_ranges: table_range list;
field_offset: Data_memory.offset; (** offset w.r.t record address *)
field_size: Data_memory.size;
field_layout: field_layout;
field_length_variability: length_variability;
field_conditions: condition_names;
field_redefinitions: item_redefinitions;
field_conditions: condition_names; (** Named conditions on the value of this field. *)
field_redefines: Cobol_ptree.qualname with_loc option; (** set iff this field is a redefinition. In that case this field must appear only inside item_redefinitions of the item it redefines. TODO: remove and extend item_redefintions type instead? *)
field_redefinitions: item_redefinitions; (** list alternative definitions for this field *)
Copy link
Copy Markdown
Collaborator

@nberth nberth Apr 17, 2026

Choose a reason for hiding this comment

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

I like the idea of extending item_redefinitions instead, probably with a record like { item_redefinitions_redefine: Cobol_ptree.qualname with_loc; item_redefinitions: item_definition wiith_loc nel; }, and then having field_redefinitions: item_redefinitions option; or something like that.

field_has_definition_issues: bool;
}

Expand All @@ -112,8 +112,8 @@ and table_definition =
table_size: Data_memory.size;
table_range: table_range;
table_init_values: Cobol_ptree.literal with_loc list; (* list for now *)
table_redefines: Cobol_ptree.qualname with_loc option; (* redef only *)
table_redefinitions: item_redefinitions;
table_redefines: Cobol_ptree.qualname with_loc option; (* same as field redefines for tables *)
table_redefinitions: item_redefinitions; (** list alternative definitions for the full table. Note that by default the typechecker generates a warning on table redefinition. *)
table_has_definition_issues: bool;
}
and table_range =
Expand Down Expand Up @@ -144,6 +144,7 @@ and dynamic_span =
occurs_dynamic_initialized: bool with_loc;
}

(** Named conditions a.k.a. level 88 items *)
and condition_names = condition_name with_loc list
and condition_name =
{
Expand Down
6 changes: 3 additions & 3 deletions src/lsp/cobol_typeck/typeck_outputs.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,9 @@ type artifacts =

type outputs =
{
ptree: Cobol_ptree.compilation_group;
group: Cobol_unit.Types.group;
artifacts: artifacts;
ptree: Cobol_ptree.compilation_group; (* Parse tree to access information missing in the typed AST. Is this field supposed to be stable? Should it only be used to access things that are inaccessible from cobol_unit ? *)
group: Cobol_unit.Types.group; (* The typed AST view as an unordered set of cobol_unit *)
artifacts: artifacts; (* Other typing artifacts *)
Copy link
Copy Markdown
Collaborator

@nberth nberth Apr 17, 2026

Choose a reason for hiding this comment

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

ptree is left here to ease its direct use by further passes in the analysis pipeline and queries in the LSP server (in which some position-based queries need to be performed on the parse-tree instead of the typed AST). Its presence here is not mandatory so it could be removed.

Copy link
Copy Markdown
Contributor Author

@GBertholon GBertholon Apr 17, 2026

Choose a reason for hiding this comment

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

Since it is for easing direct access with a pipeline setup it is obviously not mandatory but the question is more: do we want Cobol_typeck clients to see and use this or is it an internal quirk for LSP ?

}
type t = outputs

Expand Down
7 changes: 5 additions & 2 deletions src/lsp/cobol_unit/unit_types.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,12 @@ type unit_config =

(* data items *)

(** data_definitions exposes two views on the records in DATA DIVISION:
- [data_items] contains all the fields with direct access by name.
Copy link
Copy Markdown
Collaborator

@nberth nberth Apr 17, 2026

Choose a reason for hiding this comment

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

Note it's not "fields" but "items": an item is either a field or a table, and a table is always anonymous.
So data_items "contains all data items with direct access by name or ordered access."

- [data_records] is a list of record trees where each element represent a record root (typically at level 01) *)
Comment thread
GBertholon marked this conversation as resolved.
type data_definitions =
{
data_items: Cobol_data.Types.data_definition named_n_ordered;
data_items: Cobol_data.Types.data_definition named_n_ordered; (* FIXME: Is the ordered part useful here ? Isn't it already exposed in a more structured way by data_records... *)
data_records: Cobol_data.Types.record list;
Copy link
Copy Markdown
Collaborator

@nberth nberth Apr 17, 2026

Choose a reason for hiding this comment

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

Here it makes sense to keep the ordered alternative as some data items are anonymous (fillers) and therefore do not belong to the resolution map. Additionally the data_definition brings more info than item_definitions themselves, that are the types accessible from the record descriptions.

Copy link
Copy Markdown
Contributor Author

@GBertholon GBertholon Apr 17, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

Which information is only accessible through data_definition ? I cannot find any myself...

}

Expand Down Expand Up @@ -90,8 +93,8 @@ and arg_passing_style =

type procedure =
{
procedure_using: procedure_using with_loc option; (* PROCEDURE DIVISION USING ... *)
procedure_blocks: procedure_block named_n_ordered;
procedure_using: procedure_using with_loc option;
}

(* main *)
Expand Down
Loading