MyDeveloperDay accepted this revision.
MyDeveloperDay added a comment.
This revision is now accepted and ready to land.

Thank you for fixing this, I'm sorry my review probably wasn't thorough enough 
in the first place, this LGTM and thank you for the very thorough description 
as this really helps me see.

On a side rant, for me this only adds fuel to my feeling that we need D68554: 
[clang-format] Proposal for clang-format to give compiler style warnings 
<https://reviews.llvm.org/D68554>  and D29039: [clang-format] Proposal for 
clang-format -r option <https://reviews.llvm.org/D29039> in order to be able to 
run new clang-format binaries over a more substantial preformatted codebase 
quickly and easily, prior to commit of any changes.



================
Comment at: lib/Format/TokenAnnotator.cpp:1499
+    } else if (Current.isOneOf(tok::identifier, tok::kw_const,
+                               tok::kw_noexcept) &&
                Current.Previous &&
----------------
I slightly wonder if other training annotations like `override` and `final` 
might also suffer simular to what we saw with {D68481}


================
Comment at: unittests/Format/FormatTest.cpp:7093
                "  template <class T>\n"
-               "  int& foo(const std::string& str) const & noexcept {}\n"
+               "  int& foo(const std::string& str) const&& noexcept {}\n"
                "};",
----------------
krasimir wrote:
> note: the old test case was identical to the one above, I think the original 
> intent was to check both `&` and `&&` cases, so modified accordingly.
nice catch


Repository:
  rC Clang

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

https://reviews.llvm.org/D68695



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

Reply via email to