HazardyKnusperkeks added reviewers: MyDeveloperDay, HazardyKnusperkeks.
HazardyKnusperkeks requested changes to this revision.
HazardyKnusperkeks added inline comments.
This revision now requires changes to proceed.


================
Comment at: clang/docs/ReleaseNotes.rst:168
 
+- Option ``SpaceBeforeForLoopSemiColon`` has been added to control whether
+  spaces will be added before the semi-colons in for loops.
----------------
No need for change, but in the future I would prefer to add changes to the end 
of the list. :)


================
Comment at: clang/include/clang/Format/Format.h:2841
+  ///    true:                                  false:
+  ///    for (i = 0 ; i < 1 ; ++i) {}   vs.     for (i = 0; i < 1; ++i) {}
+  /// \endcode
----------------
Please also show the range based for here. Otherwise it may be surprising, it 
would be for me.


================
Comment at: clang/unittests/Format/FormatTest.cpp:12712
+  verifyFormat("for (i = 0 ; i < 10 ; ++i) {\n}", Space);
+  verifyFormat("for (int i = 0 ; auto a : b) {\n}", Space);
+}
----------------
Okay that was unexpected for me, I thought the space would only apply to the 
old `for`s.

In that case please add `while` and maybe `if` with initializer. What should be 
discussed is if `for` and the other control statements with initializer should 
behave differently with regard to their initializer semicolon.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98429/new/

https://reviews.llvm.org/D98429

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to