r282255 - Fix for r280064 that added options for fp denormals and exceptions.

Renato Golin via cfe-commits cfe-commits at lists.llvm.org
Fri Sep 23 10:20:39 PDT 2016


On 23 September 2016 at 17:12, Sjoerd Meijer <Sjoerd.Meijer at arm.com> wrote:
> I don't think I am committing at will:  I thought it was okay to commit directly because it is a simple fix (or should be) for my own  previous half-working patch, but I don't mind going through phab.

A review would have caught many issues with this patch, for example,
the no default issue, and the fact that it has no tests at all.

As a rule of thumb, wait until you get several no-comments-LGTM
reviews in a row in the same area to start committing small patches
before review in that area. Just a few changes isn't enough.


> I am indeed suspecting it is missing default case (probably got confused because e.g. ThreadModel also doesn't have one, at least not there).

Please, also do a build on ARM, and at least try to run check-all, as
that will catch most bugs. And don't forget to add tests.

cheers,
--renato


More information about the cfe-commits mailing list