aaron.ballman added inline comments.
================ Comment at: clang/lib/Format/EastWestConstFixer.cpp:139 + return (Tok->isSimpleTypeSpecifier() || + Tok->isOneOf(tok::kw_volatile, tok::kw_auto)); +} ---------------- Do you need to look for `restrict` here as well as `volatile`? ================ Comment at: clang/lib/Format/EastWestConstFixer.cpp:150 + } + // don't concern youself if nothing follows const + if (!Tok->Next) { ---------------- Comments should start with a capital letter and end with punctuation per the coding standard (same applies to other comments in the patch). ================ Comment at: clang/lib/Format/EastWestConstFixer.cpp:157-158 + IsCVQualifierOrType(Tok->Next->Next)) { + // The unsigned/signed case `const unsigned int` -> `unsigned int + // const` + swapFirstThreeTokens(SourceMgr, Fixes, Tok, Tok->Next, Tok->Next->Next, ---------------- What about something like `const unsigned long long` or the less-common-but-equally-valid `long const long unsigned`? Or multiple qualifiers like `unsigned volatile long const long * restrict` ================ Comment at: clang/lib/Format/Format.cpp:2547 + if (Style.isCpp() || Style.Language == FormatStyle::LK_ObjC) { + if (Style.ConstStyle != FormatStyle::CS_Leave) ---------------- MyDeveloperDay wrote: > aaron.ballman wrote: > > This prevents us from using this in C code despite C having qualifiers that > > can go to the left or right of the base type but still allows you to use if > > from Objective-C. That seems incorrect. > clang-format's isCpp() covers C and C++ (and ObjectiveC and ObjectiveC++) > > but you did highlight that I don't need the extra LK_ObjC check > clang-format's isCpp() covers C and C++ (and ObjectiveC and ObjectiveC++) Wow, that's a really poorly named function then! Thank you for the clarification. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69764/new/ https://reviews.llvm.org/D69764 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits