Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Remove error-prone source_id from TypeEngine::insert() and make inserting API more robust and less verbose #5991

Open
3 tasks
ironcev opened this issue May 12, 2024 · 0 comments
Assignees
Labels
compiler: frontend Everything to do with type checking, control flow analysis, and everything between parsing and IRgen compiler General compiler. Should eventually become more specific as the issue is triaged

Comments

@ironcev
Copy link
Member

ironcev commented May 12, 2024

In the TypeEngine::insert() method, the source_id parameter makes sense and should be provided only for the types that can actually be defined in source files, e.g., TypeInfo::Enum or TypeInfo::Struct. Actually, for such types it must be provided in order for garbage collection to work properly.

For types like e.g. TypeInfo::Array the source_id makes no sense as well as for all the built in types. E.g. calling te.insert(engines, TypeInfo::Numeric, some_span.source_id()) makes no sense.

The current signature of insert() is quite error-prone in that regard and verbose in many cases (e.g. inserting built-in types). The caller has to choose the valid source_id (or None) although the choice is actually determined by the type being inserted. E.g. if we are inserting an enum the source_id must always come from the enum decl span. Currently, since we are inserting same types at various places, it is only the convention that makes the source_id always the same.

Also, we have places in code where source_id should be None (inserting TypeInfo::Array for example) but it is set to a value (source of the array element span in that particular case, although it makes no sense).

Therefore, the proposal is to make the insert() API secure in regard to source_id and less verbose at the same time by:

  • removing source_id it from the insert() method and infer it based on the inserted type.
  • provide robust and secure support for the autogenerated source files (engines.se().get_autogenerated_source_id(module_id))
  • provide convenience methods for inserting common types like Unknown or built-in types.
@ironcev ironcev self-assigned this May 12, 2024
@ironcev ironcev added compiler General compiler. Should eventually become more specific as the issue is triaged compiler: frontend Everything to do with type checking, control flow analysis, and everything between parsing and IRgen labels May 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler: frontend Everything to do with type checking, control flow analysis, and everything between parsing and IRgen compiler General compiler. Should eventually become more specific as the issue is triaged
Projects
None yet
Development

No branches or pull requests

1 participant