Implement type-level string append function #726
Conversation
|
Looks good! I'm a tad concerned that |
Yeah TApp looks to me like application of some kind. Why not just |
|
I went with
|
|
If we can decide on the name, then this can be merged. As far as consistency, note that the other type constructors are for numeric types. Another option for this one could be, say, Can you confirm my understanding: This feature came up because it can be used with the other PR for splitting structs, but that PR doesn't include these commits? I get that this feature is generally useful, but I just wanted to understand any relationship/dependency between the PRs. |
|
I was originally planning to do the computation of names for split fields completely at the type level. I forget the exact blocking problem that I ran into with this approach, but I ultimately ended up doing the field name computation in the evaluator with ordinary type class methods instead. So this feature is not currently used by anything, but it seems like a good thing to have, for completeness. Note that to make type-level string construction useful, we would probably also want to add a type-level primitive |
|
Meeting consensus: we'll call the primitives I'll add these when I get around to it, hopefully before our next sync meeting. |
|
I finally got around to finishing this - I renamed |
There was a problem hiding this comment.
Thanks! Just a few more nits. Should be minor, but let me know if you'd prefer me to do them.
Unrelated to your PR, I notice that there are issues in the existing documentation for numeric type functions and pseudo-functions (for example, it mixes up SizeOf and sizeOf which are not the same), and so I might want to go in an fix up that part of the documentation anyway.
| @@ -4154,7 +4167,7 @@ \subsubsection{Rules} | |||
|
|
|||
|
|
|||
| %================================================================ | |||
| \subsection{Operations on Numeric Types} | |||
| \subsection{Operations on Numeric and String Types} | |||
There was a problem hiding this comment.
Did you have thoughts on whether this should be one subsection versus two separate subsections ("Operations on Numeric Types" and "Operations on String Types")?
There was a problem hiding this comment.
I have no strong opinions here. I don't think there will be very many more string type operations added in the future, though.
| @@ -21,11 +21,11 @@ opNumT i [x, y] | i == idTMin = Just (min x y) | |||
| opNumT _ _ = Nothing | |||
|
|
|||
| numOpNames :: [Id] | |||
| numOpNames = [idTAdd, idTSub, idTMul, idTDiv, idTExp, idTLog, idTMax, idTMin] | |||
| numOpNames = [idTAdd, idTSub, idTMul, idTDiv, idTExp, idTLog, idTMax, idTMin, idTNumToStr] | |||
There was a problem hiding this comment.
I notice that this is used in Pred.expandSyn to decide whether to apply type functions. That code uses isTFun to decide what functions can be applied and apTFun to apply them. While apTFun has been extended to know how to apply opStrT (and not just opNumT), the isTFun is still only returning True for numOpNames -- it should be extended to also include strOpNames.
Although, if we could avoid having lists that need to be kept in sync with functions, that'd be nice. For example, I wonder if the function can be changed to return a Maybe (using Nothing for unrecognized TFun IDs) and then you wouldn't need the separate list. But anyway, that's not required for your PR; it's sufficient to just strOpNames to Pred.isTFun.
There was a problem hiding this comment.
I'm not sure that the isTFun guard is even needed now - opNumT and opStrT already return Maybes. The default for an unrecognized type function in apTFun is foldl TAp t as, which is the same as for exp. Unless this guard exists as a performance optimization?
This adds a
TApptype pseudo-constructor likeTAddand friends, for appending type-level strings.