Add TH support for pattern synonyms (fixes #8761)
ClosedPublic

Authored by bollmann on Feb 22 2016, 11:14 AM.

Details

Summary

This commit adds Template Haskell support for pattern synonyms as
requested by trac ticket Trac #8761.

Test Plan

./validate

Diff Detail

Repository
rGHC Glasgow Haskell Compiler
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
There are a very large number of changes, so older changes are hidden. Show Older Changes
bollmann marked an inline comment as done.Mar 24 2016, 6:49 AM

rename hsPatSynBinders to hsPatSynSelectors.

mpickering added inline comments.Mar 24 2016, 8:54 AM
compiler/hsSyn/HsUtils.hs
940

Was this function not actually used anywhere?

bollmann added inline comments.Mar 24 2016, 2:38 PM
compiler/hsSyn/HsUtils.hs
940

surprisingly, it wasn't, so I adjusted it to my needs :-)

bollmann updated this revision to Diff 7038.Mar 24 2016, 3:19 PM

propagate name change for hsPatSynSelectors correctly.

Ack. I've run out of time. Will continue to do a thorough review, possibly later today. But see notes below!

compiler/deSugar/DsMeta.hs
719

This call to repHsSigType works for the strange shape of a pattern synonym? It's quite possible that it does. But it's probably worth adding a comment somewhere to repType that, say, dropping empty contexts, etc., would break pattern synonyms.

1452

The arguments aren't in scope for the direction, are they? The direction is either quite simple (<-, =) or binds its own arguments (<- ... where ...). I doubt this matters, but this line should probably not have the ss in scope.

1470

OK. I wanted to understand this better. Now I think I do.

The sels are the global names used as accessor functions. They do not strictly need to interact with genSym because they are global. And genSym is all about making local names that don't conflict with other names.

The pats are the local variables that connect the body of the pattern synonym with its left-hand side. Using genSym is appropriate here. Though I don't believe it's necessary. genSym is meant to avoid name capture, but since pattern synonym declarations are always top-level, I don't think we need to worry about capture.

The problem is that we really want the sels and pats to be the same in the TH syntax that DsMeta spits out. So, instead of genSyming here, let's abuse the genSym mechanism to do our dirty work.

A GenSymBind is just a (Name, Id) pair that maps a Name to an Id. Let's be concrete. Suppose we have a GenSymBind mapping a Name "foo". mkGenSyms will produce a new Id (we'll call it x) of type TH.Name, and return ("foo", x) (the double-quotes there are to remind us that foo is just a name). In the scope of addBinds [(foo, x)], every occurrence of something named foo will be replaced by x in the produced Core code. The wrapGenSyms call does something like newName "foo" >>= \x -> ... such that x is bound to the uncapturing name. So when we replace foo with x (say, when desugaring HsVar), x refers back to the result of newName "foo".

We don't wish to do any of that. Instead, we want each pat (taken from pats) to map to an Id that stores the appropriate TH.Name from sels. This happens in two stages.

  1. We need to create appropriate pairs to send to addBinds. mkGenSyms actually works for our purposes. Despite its name, it doesn't actually do anything about generating new names that will appear in the TH output -- it just creates a local variable that will appear in the GHC Core code that generates the TH. So we can say mkGenSyms pats to get the pairs.
  1. We need to bind the GHC local variables appropriately. wrapGenSyms gets this all wrong (for our purposes). Instead, we want the Ids mapped in the GenSymBinds to be bound to the selector names, not freshly generated names. So we need to generate things like let x = sel_name in ... by a new wrapSelBinds that maps the pats to their respective sels.

I think that should do it. My guess is that, with what you have now, any selectors attached to a record synonym will not be available as selector functions.

libraries/template-haskell/Language/Haskell/TH.hs
76

Also export PatSynType.

libraries/template-haskell/Language/Haskell/TH/Ppr.hs
359

This won't work with explicitly bidirectional synonyms.

libraries/template-haskell/Language/Haskell/TH/Syntax.hs
1558

Have you looked at the Haddock output? Still seems suspicious.

It would be good to test TH's pretty-printer a bit. (Admittedly, not all improvements to TH test the pretty-printer. But that's a poor reason not to.)

It also might not be bad to have a test or two that works with the raw TH AST. It's possible that you have a nice round-trip property but that the TH representation is somehow broken.

This is certainly getting closer!

compiler/hsSyn/Convert.hs
1327

You could omit this line and nothing would change.

compiler/hsSyn/HsUtils.hs
786–787

Try it. If it breaks, it will do so quite loudly.

testsuite/tests/th/T8761.hs
30

Given the difficulty around the names of selectors, make sure that x1 and y1 are in scope outside (after) the splice. My guess is that, with your current implementation, they won't be.

Also, test to make sure an error is issued if x1 and y1 are previously in scope.

bollmann marked 3 inline comments as done.Mar 25 2016, 2:55 PM

@goldfire: Thanks for your feedback! My responses are below. A new diff adding your feedback is on the way. :-)

compiler/deSugar/DsMeta.hs
719

yes, repHsSigType (or rather repTy) works well with the strange pattern synonym types. The reason is repForall, which also creates a TH ForallT type when there are no variable binders and/or constraint contexts and this is exactly what we need to roundtrip pattern synonym type signatures faithfully. I've now added a comment to repForall to indicate this.

1452

well, I did it that way since the argument binders appear to the left of the dir in the concrete surface syntax of pattern synonyms and thus seem to be in scope when dir is processed. But yea, it doesn't matter if the ss are in scope here or not. So I can move it outside of the addBinds ss if you'd prefer it.

1470

I think that what your proposing in the two stages is exactly what I've done with functions mkGenArgSyms and replaceNames, respectively. I also used an additional wrapGenArgSyms function to gensym the local binders of prefix and infix pattern synonyms. A record pattern synonym's local binders don't have to be gensymed anymore because its right hand sides (i.e., the pattern only) names are mapped to the corresponding selectors (so the local names will not exist anymore).

Finally, keep in mind that since a record pattern synonym's selector functions are top-level functions, they're brought into scope in repTopDs by hsPatSynSelectors. This has the effect that a record pattern synonym's selector functions are available everywhere in the file (and not just when representing the PatSynD as a Core expression; see also the test on record pattern synonyms in T8761 for this).

So the overall conflating record pattern synonym names story is this:

  1. in repTopDs we bring into scope a record pattern synonym's selectors, (which we also gensym at the very end).
  2. When desugaring a record PatSynD in rep_bind, we let its right hand side pattern names point to the corresponding selectors (this happens in mkGenArgSyms, which uses replaceNames)
  3. We further send these additional pattern name to selector id mappings, ss, to addBinds, so that the lookups of a record pattern synonym's right hand side names get correctly mapped to the corresponding selectors.
  4. Now the entire record pattern synonym representation in core consists only of its selector names, which are already in scope and which will be gensymed by repTopDs later. So we don't have to do anything anymore and hence wrapGenArgSyms is a no-op for record pattern synonyms.

In contrast, the story for prefix and infix pattern synonyms goes the normal mkGenSyms -> addBinds -> wrapGenSyms way, as expected. This is also achieved by our mkGenArgSyms and wrapGenArgSyms functions.

libraries/template-haskell/Language/Haskell/TH.hs
76

ok.

libraries/template-haskell/Language/Haskell/TH/Ppr.hs
359

ahhh, it's been to long since I added the PatSynDir data type that I don't even remember its different forms. Sorry about that. :/. I've just accounted for this as well, see most recent diff.

libraries/template-haskell/Language/Haskell/TH/Syntax.hs
1558

sorry, I haven't looked at this yet. How do I see it? :)

mpickering added inline comments.Mar 25 2016, 3:20 PM
compiler/rename/RnNames.hs
694 ↗(On Diff #7038)

I don't understand how this is right at all. The first argument of FieldOcc is name of the pattern variable, it is only in scope inside the pattern synonym definition. The second argument is the name of the selector.

If this is right it needs a much better comment. At the moment it isn't clear at all why if the field isExact we just don't do anything.

mpickering added inline comments.Mar 25 2016, 3:32 PM
compiler/hsSyn/Convert.hs
366

I think this is wrong as well.

Look at the code which deals with normal records.

cvt_id_arg :: (TH.Name, TH.Bang, TH.Type) -> CvtM (LConDeclField RdrName)           
cvt_id_arg (i, str, ty)                                                             
  = do  { L li i' <- vNameL i                                                       
        ; ty' <- cvt_arg (str,ty)                                                   
        ; return $ noLoc (ConDeclField                                              
                          { cd_fld_names                                            
                              = [L li $ FieldOcc (L li i') PlaceHolder]             
                          , cd_fld_type =  ty'                                      
                          , cd_fld_doc = Nothing}) }

The second argument is PlaceHolder. Why are we going something different and complicated here?

bollmann marked 2 inline comments as done.Mar 26 2016, 9:21 AM

see some more comments below.

compiler/hsSyn/Convert.hs
366

Note that in both code snippets (your quoted code as well as the cvtArgs we're doing the same thing for the selectors sels and i, respectively, namely calling vNameL on them.

And concerning the PlaceHolder, I would like to use it, but RecordPatSynField is not using the PostRn type family, and so I have to make up an unqualified RdrName here, which is exactly what the vNameL . mkNameS . nameBase does. The reason for making up an Unqual RdrName for the record pattern synonym's right hand side-only names is that @goldfire wants to conflate a record pattern synonym's selector names and their pattern-only RHS names for TH to have a nicer API. However, when implementing this approach we don't know anything about the pattern-only RHS names anymore. So we make them up and rely on the renaming pass afterwards to rename them accordingly (which happens in newLocalBndrRn I think).

Does this make sense? Or did I not understand you correctly?

1327

you mean the otherwise is redundant? I'm afraid I don't see it. The otherwise case kicks in iff either both univs and reqs are non-empty or univs is empty and reqs is non-empty. However, due to round-tripping from HsSyn -> DeSugar -> Convert non-empty reqs also imply non-empty univs. Hence, the otherwise case happens iff both univs and reqs are non-empty, in which case we want to print all binders and contexts.

Or am I missing anything?

compiler/rename/RnNames.hs
694 ↗(On Diff #7038)

note that Exact RdrNames behave like already renamer-resolved Names. And Template Haskell uses Exact RdrNames in its generated code, see Note [Binders in Template Haskell]. Hence, after splicing in a TH bracket like

[d| pattern R {x, y} <- ((x, _), [y])
      getX = x ((1, 2), [3])
      getY  = y((1, 2), [3]) |]

we want to maintain the connection between the selector binding sites of x and y and their corresponding use-sites in getX and getY. But the original code in newRecordSelector unconditionally created a mkRdrUnqual name from just the selector occurences x and y hereby breaking the connection between binders and use-sites...

I think the original implementation of newRecordSelector was correct for renaming parsed-only HsSyn (since there we only deal with OccNames anyways). However, it is incorrect when renaming already preprocessed, namely Template Haskell generated code, where some scope resolution has already been done. For the latter (i.e., when dealing with Exact RdrNames), we want to keep them as are in order to not destroy any connection between the x and y binders and their corresponding usage sites.

And this is the reason why I added the check isExact fld, in which case we don't want do do anything. See Note [Looking up Exact RdrNames] for more information.

testsuite/tests/th/T8761.hs
30

both tests yield the results they should :-).

The reason for this is that TH newNames end up as Exact (System) RdrNames in Converts thRdrName. And on these Exact RdrNames Note [Looking up Exact RdrNames] from RnEnv then applies. This ensures that even outside of a TH bracket, occurrences of x1 and y1 are bound to the selectors x1 and y1 from inside the TH bracket. Moreover, this causes name errors to be issued if there are other conflicting top level binders named x1 or y1.

Responded to two comments. My main question is *why* are these two things necessary for record pattern synonym selectors when they are not for normal selectors.

compiler/hsSyn/Convert.hs
366

There is no reason why it isn't a PostRn field. The value is only filled in after renaming. For some reason I never made it into one whilst I was working on it.

compiler/rename/RnNames.hs
694 ↗(On Diff #7038)

This doesn't really answer my question. The ordering seems like it is the wrong way around?

Another question, this code path is used for normal record selectors as well and I assume there is template haskell support for normal datatypes with record data constructors? If so, why is the behaviour for record pattern synonym selectors different from normal selectors?

bollmann marked 5 inline comments as done.Mar 31 2016, 6:39 AM

@mpickering: Thanks for hinting me at normal record selectors, for which the code path to newRecordSelector is used as well via RnSource's getLocalNonValBinders function. And now a surprise: splicing in normal data constructors with records doesn't work either in HEAD! (@goldfire: Do you know why?) Consider the following code snippet:

{- Test normal data constructors with records -}
[d|
 data D a = MkD { unD :: a }

 someD = MkD "Hello"
 getD  = unD someD
 |]

getD' = unD someD

Running this wit current HEAD fails with the same error as do record pattern synonyms:

DataRecordTest.hs:(5,1)-(10,3): Splicing declarations
    [d| someD_ap5 = MkD_ap3 "Hello"
        getD_ap6 = unD someD_ap5
        
        data D_ap2 a_ap7 = MkD_ap3 {unD :: a_ap7} |]
  ======>
    data D_a4QQ a_a4QT = MkD_a4QR {unD_a4QS :: a_a4QT}
    someD_a4QO = MkD_a4QR "Hello"
    getD_a4QP = unD_a4QS someD_a4QO

DataRecordTest.hs:5:1: error:
    The exact Name ‘unD_a4QS’ is not in scope
      Probable cause: you used a unique Template Haskell name (NameU), 
      perhaps via newName, but did not bind it
      If that's it, then -ddump-splices might be useful

My guess is that this bug was introduced due to the new DuplicateRecordFields extension. But I'm not sure. All I can tell is that it works in 7.10.3 and it fails in HEAD.

So I think this makes my change to newRecordSelector even more reasonable.

compiler/hsSyn/Convert.hs
366

I'm happy to change RecordPatSynField to use a PostRn type family. However, I think this is best done in a different patch.

compiler/rename/RnNames.hs
694 ↗(On Diff #7038)

As was done beforehand, we're taking fld, the first argument of FieldOcc, as input. The only change I made is that instead of unconditionally building an Unqual RdrName from a fields occurrence name (i.e., the field + enclosing data constructor occurrence name), we're taking given Exact RdrNames as they are. And we are not doing anything with the second argument of FieldOcc as we're still in the middle of renaming (i.e., dealing with RdrNames) and thus don't have the FieldOccs selector name yet (which is PostRn).

bollmann updated this revision to Diff 7156.Mar 31 2016, 7:46 AM
bollmann marked an inline comment as done.

improve comments; incorporate last feedback.

bollmann marked 12 inline comments as done.Mar 31 2016, 7:54 AM

So does this patch also fix the problem with normal record selectors?

So does this patch also fix the problem with normal record selectors?

yes, it does.

Thanks, glad that my confusion helped a little. The comment is better now as well.

For the purposes of others following along, is this patch now done? What is left to get working?

Yes, I think this patch is done.

The only thing I haven't accounted for is to also test the TH pretty printer wrt pattern synonyms and to test whether that one Haddock comment is correct, that goldfire remarked. I'll get to that in a calm minute (shouldn't take longer than 20 minutes to do) .Other than that I think the patch is done.

@goldfire: Or is there anything else you think should be done?

Modulo the tiny wibbles I've suggested below, this looks good.

@bollmann, @mpickering: Is there a beautifully long Note somewhere explaining all the shenanigans around the naming issues that you've been debating? I've not kept up with it at all. I'm satisfied that the code works if @mpickering is. But it would -- to invoke Simon -- jolly good to have a Note to explain it to the rest of us.

Thanks!

compiler/deSugar/DsMeta.hs
1452

Yes, I prefer that you make this change. Otherwise, I'm worried that keeping the repPatSynDir within the addBinds will be confusing to future readers. Thanks.

compiler/hsSyn/Convert.hs
1327

Or am I missing anything?

I think you are: the next line of code, which is an effective fall-through for the first case. Unless I'm missing something. :)

libraries/template-haskell/Language/Haskell/TH/Ppr.hs
359

This still looks wrong for explicitly bidirectional synonyms. The where bit will be printed out twice, no?

To test this code, just call pprint on a PatSynD.

libraries/template-haskell/Language/Haskell/TH/Syntax.hs
1558
This comment has been deleted.
1558

Just a friendly reminder to check this.

bollmann marked 5 inline comments as done.Apr 5 2016, 9:56 AM

@goldfire: thanks for your new feedback. The naming story for pattern synonym names is just the usual one, explained in the notes in Note [Binders in Template Haskell] in Convert.hs as well as in Note [Looking up Exact RdrNames] in RnEnv. However, I noticed a bug with this naming story when implementing record pattern synonym support in Template Haskell, where I had to change newRecordSelector in RnNames slightly to be compatible with THs naming. I think the note that I put in newRecordSelector explains the change well.

Then, however, @mpickering wondered why selectors for record pattern synonyms had to be treated specially, considering that there should already be TH support for normal record selector functions. And thanks to this, I found that without this patch's fix for newRecordSelector normal selectors don't get correctly renamed either in GHC 8.0! In 7.10.3 they do, but something broke them after that. I suspect the culprit is the new extension DuplicateRecordFields, but I'm not sure.

See my previous comment quoted below for more information, but the bottom line is: I think that in GHC 8, Template Haskell support for splicing in data types with record selectors doesn't work correctly. So we might consider to either merge this patch in its entirety or to just fix the emminent bug in newRecordSelector before the release of GHC 8.0.

Thoughts?

@mpickering: Thanks for hinting me at normal record selectors, for which the code path to newRecordSelector is used as well via RnSource's getLocalNonValBinders function. And now a surprise: splicing in normal data constructors with records doesn't work either in HEAD! (@goldfire: Do you know why?) Consider the following code snippet:

{- Test normal data constructors with records -}
[d|
 data D a = MkD { unD :: a }

 someD = MkD "Hello"
 getD  = unD someD
 |]

getD' = unD someD

Running this wit current HEAD fails with the same error as do record pattern synonyms:

DataRecordTest.hs:(5,1)-(10,3): Splicing declarations
    [d| someD_ap5 = MkD_ap3 "Hello"
        getD_ap6 = unD someD_ap5
        
        data D_ap2 a_ap7 = MkD_ap3 {unD :: a_ap7} |]
  ======>
    data D_a4QQ a_a4QT = MkD_a4QR {unD_a4QS :: a_a4QT}
    someD_a4QO = MkD_a4QR "Hello"
    getD_a4QP = unD_a4QS someD_a4QO

DataRecordTest.hs:5:1: error:
    The exact Name ‘unD_a4QS’ is not in scope
      Probable cause: you used a unique Template Haskell name (NameU), 
      perhaps via newName, but did not bind it
      If that's it, then -ddump-splices might be useful

My guess is that this bug was introduced due to the new DuplicateRecordFields extension. But I'm not sure. All I can tell is that it works in 7.10.3 and it fails in HEAD.

So I think this makes my change to newRecordSelector even more reasonable.

compiler/deSugar/DsMeta.hs
1452

ok, I changed it accordingly. See last diff.

compiler/hsSyn/Convert.hs
1327

Ah, right, I understand what you're getting at now. :-). Yes, of course, we could get rid of the otherwise guard without loosing anything. However, I kind of prefer the current format to deleting the otherwise guard, so I'm inclined to leaving it as is.

libraries/template-haskell/Language/Haskell/TH/Ppr.hs
359

I'm very much embarrassed that I've gotten this wrong for so many times now. Sorry! I fixed it and now I also wrote some end-to-end test cases.

bollmann updated this revision to Diff 7190.Apr 5 2016, 2:18 PM
bollmann marked 2 inline comments as done.

add last round of feedback.

goldfire accepted this revision.Apr 5 2016, 2:36 PM

I think that in GHC 8, Template Haskell support for splicing in data types with record selectors doesn't work correctly. So we might consider to either merge this patch in its entirety or to just fix the emminent bug in newRecordSelector before the release of GHC 8.0.

Thoughts?

I don't think we should merge this patch for 8.0. Template Haskell already has a ton of churn, and I don't like to make any changes after the first RC is out. It's just too disruptive. That said, you've identified (and fixed) a real bug here. I'd prefer that you post a new bug report with the precise bug and then a new patch just to fix that. It still may not make it for 8.0, given that the activity I'm seeing looks like the final RC is imminent. If it misses 8.0.1, then the bugfix patch can go in for 8.0.2.

Otherwise, looks great!

This revision is now accepted and ready to land.Apr 5 2016, 2:36 PM

... and I forgot to say: Thanks!!

... and I forgot to say: Thanks!!

@goldfire: Thanks to you for all the thorough feedback which improved this patch a lot!

Ok, I just created a ticket for the TH splicing-then-renaming issue of record selectors; see ticket Trac #11809.

@bgamari Should I merge this into master before it needs rebasing?

@bgamari Should I merge this into master before it needs rebasing?

Too late - but I'll tackle this now.

I've resolved the conflicts locally and will push shortly.

bgamari requested changes to this revision.May 4 2016, 10:33 AM

Sadly I must have gotten the rebase wrong as this doesn't quite validate. My branch can be found here; I'll try to sort this out eventually if no one else gets to it.

This revision now requires changes to proceed.May 4 2016, 10:33 AM

Sadly I must have gotten the rebase wrong as this doesn't quite validate. My branch can be found here; I'll try to sort this out eventually if no one else gets to it.

@bgamari: It seems that you have given ids up to 532 in THNames.hs. This, however, has to be reflected in PrelNames.hs as well, since right now PrelNames.hs assumes the range 200-499 for IdUniques. Might that be it?

I'll try to get to it later today; would you prefer me to submit another diff here or to submit a pull request to your github branch?

Sadly I must have gotten the rebase wrong as this doesn't quite validate. My branch can be found here; I'll try to sort this out eventually if no one else gets to it.

@bgamari: It seems that you have given ids up to 532 in THNames.hs. This, however, has to be reflected in PrelNames.hs as well, since right now PrelNames.hs assumes the range 200-499 for IdUniques. Might that be it?

Sadly not. I indeed did update PrelNames.hs, but did so in a different commit (https://github.com/bgamari/ghc/commit/fe49b570f5859fc59e876725dfed96b56be973c5).

I'll try to get to it later today; would you prefer me to submit another diff here or to submit a pull request to your github branch?

I think it makes sense to keep the PrelNames.hs change as a separate commit which is harder than necessary in Phab, so perhaps just submit a pull request.

bollmann updated this revision to Diff 7486.May 7 2016, 9:47 AM
  • correct desuagaring of pattern synonym signatures.

While doing the rebase and then running ./validate again, I noticed that the past problem (a) (quoted below), which I thought was fixed (4 weeks ago the very same tests passed), has reappeared.

remaining Problems

(a) Apparently something like

[d| pattern Pe :: () => forall a. a -> Ex
     pattern Pe    x   <-  MkEx x |]

cannot be spliced and results in a type error:

T8761.hs:37:1: error:
     • Couldn't match expected type ‘a0’ with actual type ‘a’
       ‘a’ is a rigid type variable bound by
         a pattern with constructor: MkEx :: forall a. a -> Ex,
         in a pattern synonym declaration
         at T8761.hs:37:1
       ‘a0’ is a rigid type variable bound by
         the type signature for pattern synonym ‘Pe’:
           forall a0. a0 -> Ex
         at T8761.hs:37:1
     • In the declaration for pattern synonym ‘Pe’
     • Relevant bindings include x_a6li :: a (bound at T8761.hs:37:1)

The same happens when splicing the pattern Pep. It seems as if inside quotes an empty required constraint context could not be handled? I haven't really investigated why this might be. Anyone any ideas?

I'm not sure what was different before this rebase four weeks ago when ./validate passed. However, looking at function repHsSigType in DsMeta now, I believe it is wrong as it "forgets" a pattern synonym's explicitly given, but empty required constraint context, i.e. an initial () =>. To faithfully desugar pattern synonym type signatures, even such initial empty constraint contexts must be preserved. I've added this to the most recent diff along with a note NOTE [Pattern synonym type signatures and Template Haskell] in Convert.hs explaining Template Haskell's interactions with pattern synonym type signatures.

With this last update ./validate passes again without any failures on my machine. (Interestingly though, reifying pattern synonym type signatures in T8761.hs now returns somewhat more clusmily named type variables. See the updated expected *.stderr output for T8761.hs for more details. If anyone (@goldfire?) knows why this is, let me know as I haven't been able to figure it out myself.)

bollmann updated this revision to Diff 7487.EditedMay 7 2016, 10:32 AM

add references to the new note NOTE [Pattern synonym type signatures and Template Haskell].

bgamari accepted this revision.May 11 2016, 6:34 AM

Looks like all of the major points have been covered. Thanks again for tackling this!

compiler/hsSyn/Convert.hs
366

It would be great if you could do this!

This revision is now accepted and ready to land.May 11 2016, 6:34 AM
This revision was automatically updated to reflect the committed changes.