Skip to content

Preview typefunctions#12101

Closed
UplinkCoder wants to merge 97 commits intodlang:masterfrom
UplinkCoder:preview_typefunctions
Closed

Preview typefunctions#12101
UplinkCoder wants to merge 97 commits intodlang:masterfrom
UplinkCoder:preview_typefunctions

Conversation

@UplinkCoder
Copy link
Member

@UplinkCoder UplinkCoder commented Jan 5, 2021

Preview switch for typefunction functionality.

@dlang-bot
Copy link
Contributor

dlang-bot commented Jan 5, 2021

Thanks for your pull request, @UplinkCoder!

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "master + dmd#12101"

@UplinkCoder UplinkCoder force-pushed the preview_typefunctions branch 3 times, most recently from b82b7fc to 1a983d7 Compare January 5, 2021 17:35
WalterBright
WalterBright requested changes Jan 6, 2021
View reviewed changes
Copy link
Member

@WalterBright WalterBright left a comment

Choose a reason for hiding this comment

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

Fair warning: I'm very predisposed in favor of the far simpler approach in #12029

That said, what this is missing is:

  1. document or DIP describing what type functions are, how to use them, grammar, etc.
  2. a description of how the implementation is designed. There's a thousand lines of code with very little in the way of any description. Like what's a Ttype?

RazvanN7
RazvanN7 requested changes Jan 6, 2021
View reviewed changes
Copy link
Contributor

@RazvanN7 RazvanN7 left a comment

Choose a reason for hiding this comment

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

Besides Walter's comments, this needs a changelog entry, a spec PR and all lines of code that were added need to be covered by tests (currently there are a lot of lines that are not covered).

@RazvanN7 RazvanN7 added Review:Needs Changelog A changelog entry needs to be added to /changelog Review:Pending DIP Approval Review:Needs Spec PR A PR updating the language specification needs to be submitted to dlang.org Review:Needs Tests labels Jan 6, 2021
@UplinkCoder
Copy link
Member Author

UplinkCoder commented Jan 8, 2021 •
edited
Loading

@WalterBright the performance characteristics and ease of use of #12029
are not the same as the ones in this PR.
You have gotten aliasAssign merged without a DIP.
On the basis that it is purely additive.
I don't see why the same reasoning shouldn't apply to this PR once I've done all the changed to astbase.d to make it pass CI.

This PR is also additive as it provides a syntax and semantic extension that was not available previously and is behind a preview flag.

Ttype is the type of types.
is(typeof(int) == __type__)
iff -preview=typefunctions is given

And that is incidentally the only change which might affect working code.
Though I have found only once place in Phobos,
the CommonType template where it actually does.
And CommonType can be easily fixed to behave the same regardless of whether typefunctions are enabled. (dlang/phobos#7663)

@UplinkCoder
Copy link
Member Author

UplinkCoder commented Jan 9, 2021

Who put needs tests there?
it has tests.

@RazvanN7
Copy link
Contributor

RazvanN7 commented Jan 10, 2021

I did because there are a lot of uncovered lines in this PR. Just from the top of my head you still need tests for:

  • nested type functions
  • aliases to type functions
  • overloaded type functions
  • introducing a type function into an overload set

@UplinkCoder
Copy link
Member Author

UplinkCoder commented Jan 10, 2021

I did because there are a lot of uncovered lines in this PR. Just from the top of my head you still need tests for:

  • nested type functions
  • aliases to type functions
  • overloaded type functions
  • introducing a type function into an overload set

Ah yes.
Very good points!

I shall add them.

@nordlow
Copy link
Contributor

nordlow commented Jan 10, 2021 •
edited
Loading

Would be interesting to find relevant benchmarks, using common patterns such as staticMap, that highlight drops in compilation time and memory usage when using type functions compared to traditional template-solutions including new alias-assign.

@UplinkCoder UplinkCoder force-pushed the preview_typefunctions branch 5 times, most recently from 34cd6a5 to 90fc39b Compare January 27, 2021 16:31
@maxhaton
Copy link
Member

maxhaton commented Jan 27, 2021

Would be interesting to find relevant benchmarks, using common patterns such as staticMap, that highlight drops in compilation time and memory usage when using type functions compared to traditional template-solutions including new alias-assign.

@nordlow There should be a blog post soon-ish on what typefunctions are and aren't, some background on why they could be useful, and what their ethos implies for the rest of D

UplinkCoder and others added 18 commits January 30, 2021 18:45
@UplinkCoder
Resolve typeof(type) to __type
c04ae88
@UplinkCoder
Rename TOK.empty_ to TOK.emptyType_
67e779e
@UplinkCoder
Allow ? : expressions with types
03af0c5
@UplinkCoder
Error if __type escapes into the backend!
f584283
@UplinkCoder
Rename __type to __type__ to avoid clashes with the in.h header
b57db64
@UplinkCoder
Disable typeof(type) == __type__ if -sktf is not given
41d91b9 
this is a temporary meassure to pass phobos tests which are not expecting a type of types to exist
@UplinkCoder
Add empty mangle for Tempty
a3445f1
@UplinkCoder
Introduce mangle for empty type, cuz otherwise it'll merge, what a bo…
ba3789b 
…ther ... we should fix that type mergeing
@UplinkCoder
Introduce __traits(getSuper) for type functions
5352abe
@UplinkCoder
Add TypeExpression to the visited Types
d9b2699
@UplinkCoder
Fix is(type-variable[index-exp])
9b3ef41 
An is evaluated quite early in the process.
Therefore when resolving the type or what is within an is expression,
to determinte whether it has to deferred to CTFE for type function purposes
the information is not enough to know that you are working on a type varible
therefore we need to special case the type resolving process to treat type variables
(a TypeExpression with a VarExp in it) specially.
@UplinkCoder
Adjust tests to new __type__ type
503430c
@UplinkCoder
double underscore identifiers don't show up in traits(allMembers) ...…
2d73e88 
… yay. well done guys!
@UplinkCoder
fix incompatible type error
dda6128
@UplinkCoder
Accept typeof(type) only when typefunctions are enabled
29d3587
@UplinkCoder
Add test for basic type type invariants
87121f6
@UplinkCoder
Make sk_typefunctions a preview swtich
7af952f
@UplinkCoder
Add __type__ and __emptyType to lexer unittest
5b4a45b
@UplinkCoder UplinkCoder force-pushed the preview_typefunctions branch from 673a96e to 2f87630 Compare January 30, 2021 17:49
@UplinkCoder UplinkCoder force-pushed the preview_typefunctions branch from 2f87630 to bab70b1 Compare January 30, 2021 18:04
@UplinkCoder
Copy link
Member Author

UplinkCoder commented Mar 22, 2021

On Hold while more fundamental issues are being addressed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@WalterBright WalterBright WalterBright requested changes

@RazvanN7 RazvanN7 RazvanN7 requested changes

@Geod24 Geod24 Awaiting requested review from Geod24 Geod24 is a code owner

@ibuclaw ibuclaw Awaiting requested review from ibuclaw ibuclaw is a code owner

@kinke kinke Awaiting requested review from kinke

@MartinNowak MartinNowak Awaiting requested review from MartinNowak MartinNowak is a code owner

@rainers rainers Awaiting requested review from rainers rainers is a code owner

Assignees

No one assigned

Labels

Review:Needs Changelog A changelog entry needs to be added to /changelog Review:Needs Spec PR A PR updating the language specification needs to be submitted to dlang.org Review:Needs Tests Review:Pending DIP Approval

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@UplinkCoder @dlang-bot @RazvanN7 @nordlow @maxhaton @WalterBright

Comments