What Ron said, plus if I recall Coverity has a lint for misleading indentation, so I don't think there are any current instances of this in the code base. In fact, GCC even has -Wmisleading-indentation that was added to -Wall in GCC 6, so we should currently be ok. That being said, these warnings are only heuristics (there's no precise definition of what "misleading" is supposed to be), so I don't think they should be relied upon over using braces, which avoids this problem entirely.
Jacob
On Thu, Jun 20, 2019 at 08:26:49AM -0700, ron minnich wrote:
clang-format is not a textual preprocessor. It is basically the ast builder of followed by output.
So in your case, I just tried it main() {
if (foo) bar(); baz(); }
and got the right thing, i.e. braces around bar but not baz.
The history of reviewers looking at code is they miss this kind of error. Constantly. I'm in favor of as much automation as we can get.
ron
On Thu, Jun 20, 2019 at 5:25 AM Nico Huber nico.h@gmx.de wrote:
On 20.06.19 06:01, Jacob Garber wrote:
On Wed, Jun 19, 2019 at 08:38:14PM -0700, ron minnich wrote:
Given the number of serious problems that lack of braces causes, I like this proposal. It's indicative that both Rust and Go require the {}, for reasons of safety.
There was a famous vulnerability in Apple's SSL code several years ago because of lack of braces. clang-format can also reformat old code to have mandatory braces if I'm not mistaken.
What will clang-format do if it encounters?
if (foo) bar(); baz();a) if (foo) { bar(); } baz();
or b) if (foo) { bar(); baz(); }
Will it spit out a warning? If not, this shows how dangerous automatic formatting can be. Because after the formatter run, it's much less ob- vious for the reviewer that something is wrong.
Nico