Fix for crash due to g++.old-deja/g++.other/using3.C

[dinesh.d]

Summary

Following patch fixes crash in clang with attached test case [taken from g++.old-deja/g++.other/using3.C].

Issue:

Test is trying to make using declaration for a non-existing member in base class in derived class and expect error
while compiling. Current clang code handles normal cases properly.

Problem arises when base class is typedefed to a type constructed in-place. As using declaration is referring to a
non-existing member, normal lookup fails and while trying typo-correction, clang returns typedef declaration as
possible alternative, which is not a record type and can not be type casted to EnumDecl/ RecordDecl/ CXXRecordDecl.

But Sema::CheckUsingShadowDecl() assumes otherwise and tries to typecast it to CXXRecordDecl without any check
while checking if referred type is from base class or not. This results in crash in clang.

Solution:

I looked for ways to get CXXRecordDecl from TypedefDecl but did not find any. I found function like getUnderlyingDecl()
but they just work with UsingDecls and ObjCCompatibleAliasDecls. Moreover, for any existing member, clang find proper
declaration and everything works fine.

So I have added a check in Sema::CheckUsingShadowDecl() to bypass base class check if referred type in using declaration
is not in a record type. A proper fix may be if we can get proper RecordDecl from TypedefDecl and use it for this check
or handle TypedefDecl case separately.

[dinesh.d]

Submitted bug report [http://llvm.org/bugs/show_bug.cgi?id=19171]

Should we update typo-correction code, to suggest only base class member if using declaration is inside derivied class.

[rsmith]

• {F53369, layout=link}

[dinesh.d]

Hi Richard,

Thanks for reviewing.

I have updated patch to handle this issue in typo-correction. This can be further improved is we can check if suggested typo belong to base class or not.

I have merged test case in using-decl-1.cpp as suggested.

Please have a look.

[rsmith]

Thanks, this looks good. Please also add test cases (with FIXMEs) for the
two cases mentioned in your FIXME comment, then this looks fine to commit.

[dinesh.d]

Added test cases with fix-me as per review comments.

[dinesh.d]

gentle ping

[rsmith]

LGTM

lib/Sema/SemaDeclCXX.cpp

7335–7337

Is this still necessary?

[dinesh.d]

Updated test-cases

updated test case fails if we comment lines 7335:7337.

using declaration for class member can not use templates id, scoped enumerators etc.

[dinesh.d]

lib/Sema/SemaDeclCXX.cpp

7335–7337

yes, I have added one test case which fails if we comment these lines. I have kept these lines so that my change just filter out non class member if RequiredMember is true but should not add any other type of class member which it was not considering before my changes.

[rsmith]

Please commit the patch as-is, I'd like to get the crash fix in first before worrying about getting better typo correction. Let me know if you need me to commit this for you.

lib/Sema/SemaDeclCXX.cpp

7335–7337

I think your testcase shows that these lines are in fact incorrect -- if the function template were in a base class, rather than in an unrelated class, typo correction *should* find it.

[dinesh.d]

lib/Sema/SemaDeclCXX.cpp

7335–7337

No, in added test case, function template is not in base class.

[dinesh.d]

It will be great if you can commit this for me.

[dinesh.d]

Thanks for committing this patch (r207677).

[rsmith]

Committed as r207677.

FIXME for members of base classes fixed in r207680.
Support for typo-correcting to member templates added in r207681.

lib/Sema/SemaDeclCXX.cpp

7335–7337

Right, that's my point -- the reason we shouldn't find the function template is because it's not in a base class, *not* because it's a template. If it were in a base class but mistyped, typo correction *should* find it, but won't, because of this incorrect check.

lib/Sema/SemaDeclCXX.cpp

public:
 
   bool ValidateCandidate(const TypoCorrection &Candidate) override {
     NamedDecl *ND = Candidate.getCorrectionDecl();
 
     // Keywords are not valid here.
     if (!ND || isa<NamespaceDecl>(ND))
       return false;
 
+    // FIXME: We should check if ND is member of base class of class having
+    // using declaration and direct base class in case using declaration names
+    // a constructor.
+    if (RequireMember && !ND->isCXXClassMember())
+      return false;
+
     if (RequireMember && !isa<FieldDecl>(ND) && !isa<CXXMethodDecl>(ND) &&
         !isa<TypeDecl>(ND))
       return false;
 
     // Completely unqualified names are invalid for a 'using' declaration.
     if (Candidate.WillReplaceSpecifier() && !Candidate.getCorrectionSpecifier())
       return false;
 
     if (isa<TypeDecl>(ND))
       return HasTypenameKeyword || !IsInstantiation;
 

test/SemaCXX/using-decl-1.cpp

struct S {
       }
     };
 
     using A::HiddenTag2; // expected-error {{conflicts with declaration already in scope}}
     using A::HiddenFn2; // expected-error {{conflicts with declaration already in scope}}
     using A::HiddenLocalExtern2;
   }
 }
+
+struct Z {
+  Z();
+};
+
+typedef struct {
+  Z i;
+} S;
+
+struct Y : S {
+  using S::S; // expected-error {{no member named 'S' in 'S'}}
+};
+
+// [namespace.udecl] Para3: In a using-declaration used as a member-declaration,
+// the nested-name-specifier shall name a base class of the class being defined.
+// If such a using-declaration names a constructor, the nested-name-specifier
+// shall name a direct base class of the class being defined;
+
+// FIXME: For c++11, Typo correction should only consider constructor of direct base class
+struct PR19171_B { }; // expected-note {{'PR19171_B' declared here}}
+struct PR19171_C : PR19171_B { };
+struct PR19171_D : PR19171_C {
+  using PR19171_B::PR19171_C; // expected-error{{no member named 'PR19171_C' in 'PR19171_B'; did you mean 'PR19171_B'?}}
+};
+
+// FIXME: Typo correction should only consider member of base classes
+struct PR19171_E { };
+struct PR19171_EE { int EE; }; // expected-note {{'PR19171_EE::EE' declared here}} \
+                                // expected-note {{target of using declaration}}
+struct PR19171_F : PR19171_E {
+  using PR19171_E::EE; // expected-error{{no member named 'EE' in 'PR19171_E'; did you mean 'PR19171_EE::EE'?}} \
+                          // expected-error{{using declaration refers into 'PR19171_E::', which is not a base class of 'PR19171_F'}}
+};