Skip to content

[NFC] Refactor parameter counting #10271

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

Closed
wants to merge 2 commits into from
Closed

Conversation

CodaFi
Copy link
Contributor

@CodaFi CodaFi commented Jun 15, 2017

As a first test of the new function representation, use it to count parameters in CSGen.

Depends on #10264

Pre-rebase the relevant code is in CSGen.cpp

CodaFi added 2 commits June 14, 2017 15:16
Prepares the AST for future work to eliminate `getInput()`
and perform function type argument matching in a less
ad-hoc manner.
Identifier Label;

/// Whether the parameter has a default argument. Not valid for arguments.
bool HasDefaultArgument = false;
Copy link
Member

Choose a reason for hiding this comment

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

HasDefaultArgument doesn't belong in the type. I think you will need to use something other than CallArgParam for your storage.

@@ -2295,7 +2323,8 @@ getSILFunctionLanguage(SILFunctionTypeRepresentation rep) {
/// be 'thin', indicating that a function value has no capture context and can be
/// represented at the binary level as a single function pointer.
class AnyFunctionType : public TypeBase {
const Type Input;
const ArrayRef<CallArgParam> Input;
Copy link
Member

Choose a reason for hiding this comment

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

I'd prefer to tail-allocate the parameters in the subclasses, so we don't pay for this extra pointer in such a common type.

RecursiveTypeProperties properties,
const ExtInfo &Info)
: TypeBase(Kind, CanTypeContext, properties),
Input((CanTypeContext ?: &RawInput->getASTContext())->AllocateCopy(Input)),
Copy link
Member

Choose a reason for hiding this comment

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

Hmm... I'm confused why we're doing the CanTypeContext null check dance here. If we do the tail allocation thing, then don't need to do the AllocateCopy dance here... we just copy over the data we get.

@CodaFi
Copy link
Contributor Author

CodaFi commented Jun 15, 2017

I destroyed any hope of a reasonable merge here. Going to reopen this elsewhere.

@CodaFi CodaFi closed this Jun 15, 2017
@CodaFi CodaFi deleted the parammore branch June 15, 2017 21:38
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