Location and Range unittests for FriendDecl

[nikola]

Summary

Any other interesting scenarios to test while am at it?

[klimek]

This looks like a positive step, so LG.
(I assume you have checked that at least of them fails without your patch ;)

Richard might have more ideas for corner cases (he's a master of corner cases)

[klimek]

+richard for corner case generation

[nikola]

Yep, the one with friend class fails without my patch.

[nikola]

Committed these tests in r210306 but I'll leave this review request open in case the following code produces something in the next week or so :)

richard_smith_engine e(beer()); // not sure if this is a proper way to seed the engine
obscure_frienddecl_distribution d(e);
d();

[nikola]

Turns out we didn't have correct location and range for class template friend. The same was true for case where friend is not the first token as in Richard's inline example.

Manuel:
Are my constructor/destructor matchers good?

Richard:

1. What's the TypeSourceInfo case in FriendDecl::getSourceRange?
2. I couldn't find a way to hit the first call to FriendDecl::Create inside Sema::ActOnTemplatedFriendTag (some kind of explicit specialization case). I tried issuing a random diagnostic from there hoping it would break something in the test suite but it seems we don't have any coverage for this?

[rsmith]

One comment, the rest looks fine.

include/clang/AST/DeclFriend.h

148 ↗

(On Diff #10374)

This use of < is not correct (it'll do the wrong thing in the presence of macro expansion). SourceManager::isBeforeInTranslationUnit is the right way, but it's hard to get at from here. What does a friend function declaration think its LocStart is? Does that include the friend token already? Maybe you can just use that.

[nikola]

include/clang/AST/DeclFriend.h

148 ↗

(On Diff #10374)

This is very unfortunate but getStartLoc depends on the range, it's whatever beginning of the range is :(

Note that we need this only for that contrived case of yours ;) where friend isn't the first token in the declaration. What's worse, having wrong range in that case or in the presence of macro expansion? Who uses macros anyway :P

I tried this but friend keyword and start of NamedDecl point to same location. Could we already be wrong in this case?

#define DECL void f();
struct A{
  friend DECL
};

[nikola]

Ping.

[nikola]

This covers all test cases.

[nikola]

Committed in r213220.

unittests/AST/SourceLocationTest.cpp

       unresolvedUsingValueDecl()));
 }
 
+TEST(FriendDecl, FriendFunctionLocation) {
+  LocationVerifier<FriendDecl> Verifier;
+  Verifier.expectLocation(2, 13);
+  EXPECT_TRUE(Verifier.match("struct A {\n"
+                             "friend void f();\n"
+                             "};\n",
+                             friendDecl()));
+}
+
+TEST(FriendDecl, FriendFunctionRange) {
+  RangeVerifier<FriendDecl> Verifier;
+  Verifier.expectRange(2, 1, 2, 15);
+  EXPECT_TRUE(Verifier.match("struct A {\n"
+                             "friend void f();\n"
+                             "};\n",
+                             friendDecl()));
+}
+
+TEST(FriendDecl, FriendClassLocation) {
+  LocationVerifier<FriendDecl> Verifier;
+  Verifier.expectLocation(2, 8);
+  EXPECT_TRUE(Verifier.match("struct A {\n"
+                             "friend class B;\n"
+                             "};\n",
+                             friendDecl()));
+}
+
+TEST(FriendDecl, FriendClassRange) {
+  RangeVerifier<FriendDecl> Verifier;
+  Verifier.expectRange(2, 1, 2, 14);
+  EXPECT_TRUE(Verifier.match("struct A {\n"
+                             "friend class B;\n"
+                             "};\n",
+                             friendDecl()));
+}
+
+TEST(FriendDecl, FriendTemplateParameterLocation) {
+  LocationVerifier<FriendDecl> Verifier;
+  Verifier.expectLocation(3, 8);
+  EXPECT_TRUE(Verifier.match("template <typename T>\n"
+                             "struct A {\n"
+                             "friend T;\n"
+                             "};\n",
+                             friendDecl(), Lang_CXX11));
+}
+
+TEST(FriendDecl, FriendTemplateParameterRange) {
+  RangeVerifier<FriendDecl> Verifier;
+  Verifier.expectRange(3, 1, 3, 8);
+  EXPECT_TRUE(Verifier.match("template <typename T>\n"
+                             "struct A {\n"
+                             "friend T;\n"
+                             "};\n",
+                             friendDecl(), Lang_CXX11));
+}
+
+TEST(FriendDecl, FriendDecltypeLocation) {
+  LocationVerifier<FriendDecl> Verifier;
+  Verifier.expectLocation(4, 8);
+  EXPECT_TRUE(Verifier.match("struct A;\n"
+                             "A foo();\n"
+                             "struct A {\n"
+                             "friend decltype(foo());\n"
+                             "};\n",
+                             friendDecl(), Lang_CXX11));
+}
+
+TEST(FriendDecl, FriendDecltypeRange) {
+  RangeVerifier<FriendDecl> Verifier;
+  Verifier.expectRange(4, 1, 4, 8);
+  EXPECT_TRUE(Verifier.match("struct A;\n"
+                             "A foo();\n"
+                             "struct A {\n"
+                             "friend decltype(foo());\n"
+                             "};\n",
+                             friendDecl(), Lang_CXX11));
+}
+
 TEST(FriendDecl, InstantiationSourceRange) {
   RangeVerifier<FriendDecl> Verifier;
   Verifier.expectRange(4, 3, 4, 35);