arichardson added inline comments.
================
Comment at: clang/lib/Format/TokenAnnotator.cpp:2427
+ // inside a function this should always be treated as a variable.
+ return CouldBeTypeList && Line.Level == 0;
}
----------------
MyDeveloperDay wrote:
> how hard would it be to do the TODO?
I am not that familiar with the clang-format codebase yet, but it appears to me
that this information is not tracked at all. It should be quite easy to add
another `Scope` (or similar) member that is an enum for
global/function/namespace/class/other and then update that in all cases that
also change the level member.
================
Comment at: clang/unittests/Format/FormatTest.cpp:5507
+ Google);
verifyGoogleFormat(
"bool aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa GUARDED_BY(aaaaaaaaaaaa) =\n"
----------------
MyDeveloperDay wrote:
> I want to approve this change, but I HATE changing unit tests (Beyonce rule),
> I'm struggling to see if we are changing anything here? or if you are just
> qualifying it a little better because the usage is different depending on
> where its used (as a function,as a variable)
>
Before a non-empty `()` with a single indentifier inside was always treated as
a variable, with this change it's (in this case incorrectly) parsed as function.
================
Comment at: clang/unittests/Format/FormatTest.cpp:6762
+ "} // namespace namspace_scope\n",
+ Style);
verifyFormat("class E {\n"
----------------
MyDeveloperDay wrote:
> Nit that is a mother of an assert, but when it fails.. how easy is it going
> to be to debug where it goes wrong exactly.
>
> Could we break it up a little?
`I agree this is too long. I was just trying to avoid repeating the `namespace
{}/void fn()` prefix. Will try to break it up for next version of this patch.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D87028/new/
https://reviews.llvm.org/D87028
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
