[clang][Sema] Better support for ObjC++ in Sema::LookupName

[bulbazord]

Summary

Sema::LookupName essentially has two possible code paths based on what
language is currently being used. The predicate selecting the code path
is whether or not C++ is being used. In this scenario, the Scope
argument that is being passed needs to be not nullptr in order to
succeed. In the C/ObjC case, the Scope argument can potentially be
nullptr and still succeed. In the case where the Scope argument is
nullptr and you are dealing with Objective-C++ while looking up an
Objective-C name, you will fail where you otherwise should succeed.

This was surfaced when working on Swift/C++ interop. Swift passes
nullptr for the Scope when looking up some Objective-C name with the
experimental C++ interop option enabled resulting in a failure.

[bulbazord]

I'm not quite sure the best way to test this. Looks like Sema::LookupName isn't currently directly tested. I've run the test suite and did not find any new failing tests.

[arphaman]

Is there a Swift-based test case you have that demonstrates the original problem from Swift's clang importer side? I'm curious to see where it manifests in Swift.

[bulbazord]

In D121748#3384357, @arphaman wrote:

Is there a Swift-based test case you have that demonstrates the original problem from Swift's clang importer side? I'm curious to see where it manifests in Swift.

% cat Test.h
@protocol Foo
@end
@interface Foo <Foo>
@end

% cat test.swift
import Test

public protocol Thing: FooProtocol {
}

% cat module.modulemap
module Test [extern_c] {
  header "Test.h"
}

% $SWIFT_BUILD_DIR/bin/swiftc -frontend -c -enable-cxx-interop -enable-objc-interop -sdk /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/ -I. -o /tmp/foo.o test.swift
test.swift:3:24: error: cannot find type 'FooProtocol' in scope
public protocol Thing: FooProtocol {
                       ^~~~~~~~~~~

There's a call to Sema::LookupName that fails for the reason I outlined in the description when building the SwiftLookupTable during PCM creation time. The renaming of @protocol Foo to protocol FooProtocol doesn't occur because it thinks there is no naming conflict.

[arphaman]

I see, thanks! Let me think a bit more about this change and test it on our codebase to see if this is a viable Sema change.

[arphaman]

I don't think this patch is sound. I found this problem with this change when the following file is compiled in Objective-C++ mode:

template <class T, bool umax = true>
T umax(T a, T b) {
  return a;
}

produces this error:

test.mm:3:3: error: declaration of 'umax' shadows template parameter
T umax(T a, T b) {
  ^
test.mm:2:25: note: template parameter is declared here
template <class T, bool umax = true>
                        ^

which is unexpected and isn't produced in C++ mode.

[bulbazord]

In D121748#3397715, @arphaman wrote:

I don't think this patch is sound. I found this problem with this change when the following file is compiled in Objective-C++ mode:

template <class T, bool umax = true>
T umax(T a, T b) {
  return a;
}

produces this error:

test.mm:3:3: error: declaration of 'umax' shadows template parameter
T umax(T a, T b) {
  ^
test.mm:2:25: note: template parameter is declared here
template <class T, bool umax = true>
                        ^

which is unexpected and isn't produced in C++ mode.

Could you tell me what flags you gave to clang to compile the test? I'd like to test a few things.

[arphaman]

clang -x objective-c++ -Weverything -std=c++14 -c test.mm

clang/lib/Sema/SemaLookup.cpp

 ///
 /// @returns \c true if lookup succeeded and false otherwise.
 bool Sema::LookupName(LookupResult &R, Scope *S, bool AllowBuiltinCreation) {
   DeclarationName Name = R.getLookupName();
   if (!Name) return false;
 
   LookupNameKind NameKind = R.getLookupKind();
 
-  if (!getLangOpts().CPlusPlus) {
+  if (!getLangOpts().CPlusPlus || getLangOpts().ObjC) {
+    // In the case of Objective-C++, try C++ unqualified name lookup first.
+    if (getLangOpts().CPlusPlus && CppLookupName(R, S))
+      return true;
     // Unqualified name lookup in C/Objective-C is purely lexical, so
     // search in the declarations attached to the name.
     if (NameKind == Sema::LookupRedeclarationWithLinkage) {
       // Find the nearest non-transparent declaration scope.
       while (!(S->getFlags() & Scope::DeclScope) ||
              (S->getEntity() && S->getEntity()->isTransparentContext()))
         S = S->getParent();
     }