[PATCH] D69764: [clang-format] Add Left/Right Const (East/West , Before/After) fixer capability

Manuel Klimek via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Dec 2 02:23:31 PST 2019


klimek added a comment.

I'm not generally opposed to this, given that
a) clang-format already changes code; I think by now we're not fixing double semicolon mainly for workflow reasons, would be fine to add
b) the implementation is very self contained



================
Comment at: clang/docs/ClangFormatStyleOptions.rst:1378
 
+**ConstStyle** (``ConstAlignmentStyle``)
+  Different ways to arrange const.
----------------
Personally, I'm somewhat against having 3 different aliases for the options. I'd chose one, even though it doesn't make everybody happy, and move on. I'm fine with East/West as long as the documentation makes it clear what it is.


================
Comment at: clang/lib/Format/EastWestConstFixer.cpp:143
+                            SmallVectorImpl<AnnotatedLine *> &AnnotatedLines,
+                            FormatTokenLexer &Tokens) {
+  const AdditionalKeywords &Keywords = Tokens.getKeywords();
----------------
This function is super large - can we split it up?


================
Comment at: clang/unittests/Format/FormatTest.cpp:30-32
+#define VERIFYFORMAT(expect, style) verifyFormat(expect, style, __LINE__)
+#define VERIFYFORMAT2(expect, actual, style)                                   \
+  verifyFormat(expect, actual, style, __LINE__)
----------------
I'm somewhat opposed to introducing these macros in this patch. If we want them, we should create an extra patch, and figure out ho to use them in all format tests.


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

https://reviews.llvm.org/D69764





More information about the cfe-commits mailing list