aaron.ballman added inline comments.
================ Comment at: clang/lib/ASTMatchers/Dynamic/Registry.cpp:282 REGISTER_MATCHER(hasInitStatement); + REGISTER_MATCHER(hasInitializer); REGISTER_MATCHER(hasKeywordSelector); ---------------- air20 wrote: > aaron.ballman wrote: > > air20 wrote: > > > aaron.ballman wrote: > > > > Why did this one move? Please keep the list sorted alphabetically. > > > I think this wasn’t sorted correctly before. > > I think it was sorted properly before (`i` comes before `S` even if you > > ignore capitalization), but regardless, it's a change unrelated to the > > patch, which we usually ask to be a separate patch. > I trust my Vim for this :) I think uppercase letters have a smaller ASCII > value so they come first. > > Line 354-355 are sorted correctly, for example. > ``` > REGISTER_MATCHER(isConstQualified); > REGISTER_MATCHER(isConstexpr); > ``` > > I'd rather not upload another patch for this though. > I trust my Vim for this :) I think uppercase letters have a smaller ASCII > value so they come first. I think it depends on your sort criteria and whether you are sorting this list for humans or computers. I sort it for humans where `i` comes before `s` alphabetically because the goal of alphabetizing this list is for a human to quickly scan it visually. (Computer sort order is unimportant in this case -- these are all being stored into a hash map where the order does not matter.) > I'd rather not upload another patch for this though. This is a change unrelated to the patch at hand, so it should be removed from this patch. If you feel strongly about changing this, please start a separate patch. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72233/new/ https://reviews.llvm.org/D72233 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
