Page MenuHomePhabricator

[AST] Make NullStmt final and give it factory functions
AcceptedPublic

Authored by hamzasood on Aug 15 2018, 4:24 AM.

Details

Summary

I've submitted a patch for contracts (review linked to this) that adds trailing objects to NullStmt. This patch contains the changes to make that possible.

Diff Detail

Event Timeline

hamzasood created this revision.Aug 15 2018, 4:24 AM
steveire accepted this revision.EditedAug 15 2018, 1:02 PM

This looks like a NFC change.

Given that the next patch moves these methods out of line, you might consider introducing them out of line here (and moving the constructors out of line). That would make the next patch easier to review because the addition of assertion handling would be move visible.

You could also state in the commit message why this is necessary to make your contracts possible. From looking at the follow-up patch, it seems that you want to centralize creation so that it doesn't have to be duplicated in the follow-up patch.

LGTM, but I'm new here, so you may want to wait for another reviewer if you wish.

This revision is now accepted and ready to land.Aug 15 2018, 1:02 PM

Thanks. I'm not planning to move forward with this until the contracts patch is ready (it might change enough to not even require this); that should leave plenty of time for others to take a look.