Skip to content

types: share Function payload on copy - #255

Merged
rieske merged 2 commits into
masterfrom
types/share-function-payload
Sep 1, 2026
Merged

types: share Function payload on copy#255
rieske merged 2 commits into
masterfrom
types/share-function-payload

Conversation

@rieske

@rieske rieske commented Sep 1, 2026

Copy link
Copy Markdown
Owner

Summary

  • Function copies now share an immutable impl (shared_ptr) instead of deep-cloning the return type and every argument.
  • Pointer, array, and record payloads already shared. Function was the remaining expensive Type copy.
  • Assignment still replaces the impl, so later writes to one copy do not change the other. Function has no setters.

Isolated measure on this machine (Release, git -O0, 568 TUs): analyze 49s -> 40s, full user 224s -> 209s. diff.c analyze 96ms -> 80ms.

Test plan

  • ctest -R '^(typesTest|astTest|symbolsTest|semantic_analyzerTest)$'
  • ctest -L att -j24
  • CI

@rieske
rieske force-pushed the types/share-function-payload branch from aea1d63 to c542559 Compare September 1, 2026 15:35
@rieske

rieske commented Sep 1, 2026

Copy link
Copy Markdown
Owner Author

Review (cannot request-changes on our own PR)

The share belongs on Function. Pointer, array, and record payloads already share; this was the remaining expensive Type copy. No spaghetti, no file-size issue, no branching. The test locks the assignment-independence that sharing has to preserve.

The pimpl already bought something this patch does not use.

unique_ptr<Type> members existed because Function.h cannot see a complete Type (Type.h includes Function.h). Impl lives only in the .cpp, which already includes Type.h. Keeping unique_ptr inside Impl is the header-cycle workaround transplanted into the one place it is no longer needed.

Store values in Impl:

struct Function::Impl {
    Type returnType;
    std::vector<Type> arguments;
    bool variadic { false };
};

Then:

  • construction can be one make_shared<Impl>(...) instead of default-Impl plus three assignments
  • getReturnType() / getArguments() can return const Type& / const std::vector<Type>& instead of allocating a vector and cloning every argument on every inspect (sameShape and makeComposite both do this today)
  • a function type is one heap object (the impl), not impl + return + each argument

The public ctor can keep taking unique_ptr<Type> so the header cycle stays solved. Deref into Impl at the boundary.

That is the same change, with the incidental heap shape deleted. I would not merge the unique_ptr-in-Impl version.

@mergify

mergify Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Tick the box to add this pull request to the merge queue (same as @mergifyio queue).

  • Queue this pull request

@coveralls

coveralls commented Sep 1, 2026

Copy link
Copy Markdown

Coverage Status

coverage: 94.028% (-0.002%) from 94.03% — types/share-function-payload into master

Pointer, array, and record payloads already share. Function was the
odd one: every Type copy deep-cloned the return type and arguments.
Function is immutable after construction, so copies can share.

Analyze on a full git -O0 build went 49s -> 40s (user 224s -> 209s).
Impl lives in the .cpp, so it can hold Type by value. unique_ptr was
only needed in the header to break the Type include cycle. Accessors
now return const refs; the public ctor still takes unique_ptr.
@rieske
rieske force-pushed the types/share-function-payload branch from cbb7d6b to c88cf04 Compare September 1, 2026 15:48
@rieske
rieske merged commit 6a94429 into master Sep 1, 2026
3 of 4 checks passed
@rieske
rieske deleted the types/share-function-payload branch September 1, 2026 16:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants