tahonermann added a comment.
> Ping for the other reviewers in case they have thoughts.
I took a gander and it all looks good to me. I added one comment regarding a
test change.
================
Comment at: clang/unittests/AST/SourceLocationTest.cpp:135-145
TEST(ParmVarDecl, KNRLocation) {
LocationVerifier<ParmVarDecl> Verifier;
- Verifier.expectLocation(1, 8);
- EXPECT_TRUE(Verifier.match("void f(i) {}", varDecl(), Lang_C99));
+ Verifier.expectLocation(1, 15);
+ EXPECT_TRUE(Verifier.match("void f(i) int i; {}", varDecl(), Lang_C99));
}
TEST(ParmVarDecl, KNRRange) {
----------------
Perhaps preserve the prior tests (switched to `Lang_C89`) and then add these
tests? That would ensure locations stay correct for both declaration forms.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D124258/new/
https://reviews.llvm.org/D124258
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
