sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land.
I haven't really understood D65043 <https://reviews.llvm.org/D65043> yet, but I think this is a good change either way. ================ Comment at: lib/Format/TokenAnnotator.cpp:2862 return Right.is(TT_TemplateCloser) && Left.is(TT_TemplateCloser) && - (Style.Standard != FormatStyle::LS_Cpp11 || Style.SpacesInAngles); + (Style.Standard == FormatStyle::LS_Cpp03 || Style.SpacesInAngles); } ---------------- modocache wrote: > Just a note: I don't know what the original intent of > https://github.com/llvm/llvm-project/commit/dd978ae0e11 was, but in D65043 I > modified this condition to be `Style.Standard == FormatStyle::LS_Cpp03 || > Style.Standard == FormatStyle::LS_Auto`, because I believe that mirrors the > current behavior exactly. With this change, a user that specified a standard > of `FormatStyle::LS_Auto` will experience a change in behavior. If I'm reading the code right, `Formatter::analyze`calls `deriveLocalStyle` first, which replaces `LS_Auto` with a concrete style. Then later, it calls `TokenAnnotator::calculateFormattingInformation` which is what ultimately calls `spaceRequiredBefore`. So I think we never see `Auto` here, and the two forms are equivalent. ================ Comment at: lib/Format/TokenAnnotator.cpp:2862 return Right.is(TT_TemplateCloser) && Left.is(TT_TemplateCloser) && - (Style.Standard != FormatStyle::LS_Cpp11 || Style.SpacesInAngles); + (Style.Standard == FormatStyle::LS_Cpp03 || Style.SpacesInAngles); } ---------------- sammccall wrote: > modocache wrote: > > Just a note: I don't know what the original intent of > > https://github.com/llvm/llvm-project/commit/dd978ae0e11 was, but in D65043 > > I modified this condition to be `Style.Standard == FormatStyle::LS_Cpp03 || > > Style.Standard == FormatStyle::LS_Auto`, because I believe that mirrors the > > current behavior exactly. With this change, a user that specified a > > standard of `FormatStyle::LS_Auto` will experience a change in behavior. > If I'm reading the code right, `Formatter::analyze`calls `deriveLocalStyle` > first, which replaces `LS_Auto` with a concrete style. Then later, it calls > `TokenAnnotator::calculateFormattingInformation` which is what ultimately > calls `spaceRequiredBefore`. So I think we never see `Auto` here, and the two > forms are equivalent. nit: consider `< Cpp11`, or `<= Cpp03`? Fits in the spirit of this patch :-) Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65183/new/ https://reviews.llvm.org/D65183 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
