[MS] Allow access to ambiguous, inaccessible direct bases

Summary:
Clang typically warns that in the following class hierarchy, 'A' is
inaccessible because there is no series of casts that the user can
write to access it unambiguously:
  struct A { };
  struct B : A { };
  struct C : A, B { };

MSVC allows the user to convert from C* to A*, though, and we've
encountered this issue in the latest Windows SDK headers.

This patch allows this conversion when -fms-compatibility is set and
adds a warning for it under -Wmicrosoft-inaccessible-base.

Reviewers: rsmith

Subscribers: cfe-commits

Differential Revision: https://reviews.llvm.org/D39389

llvm-svn: 316807

diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td
index 4cceaf6..c23183c 100644 (file)
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -890,6 +890,7 @@ def MicrosoftVoidPseudoDtor : DiagGroup<"microsoft-void-pseudo-dtor">;
 def MicrosoftAnonTag : DiagGroup<"microsoft-anon-tag">;
 def MicrosoftCommentPaste : DiagGroup<"microsoft-comment-paste">;
 def MicrosoftEndOfFile : DiagGroup<"microsoft-end-of-file">;
+def MicrosoftInaccessibleBase : DiagGroup<"microsoft-inaccessible-base">;
 // Aliases.
 def : DiagGroup<"msvc-include", [MicrosoftInclude]>;
                 // -Wmsvc-include = -Wmicrosoft-include

diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index b1cba06..b8538f9 100644 (file)
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -7565,6 +7565,9 @@ def err_ambiguous_derived_to_base_conv : Error<
 def err_ambiguous_memptr_conv : Error<
   "ambiguous conversion from pointer to member of %select{base|derived}0 "
   "class %1 to pointer to member of %select{derived|base}0 class %2:%3">;
+def ext_ms_ambiguous_direct_base : ExtWarn<
+  "accessing inaccessible direct base %0 of %1 is a Microsoft extension">,
+  InGroup<MicrosoftInaccessibleBase>;
 
 def err_memptr_conv_via_virtual : Error<
   "conversion from pointer to member of class %0 to pointer to member "

diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 750e062..fa9e9f3 100644 (file)
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -2503,13 +2503,8 @@ bool Sema::IsDerivedFrom(SourceLocation Loc, QualType Derived, QualType Base,
   return DerivedRD->isDerivedFrom(BaseRD, Paths);
 }
 
-void Sema::BuildBasePathArray(const CXXBasePaths &Paths,
-                              CXXCastPath &BasePathArray) {
-  assert(BasePathArray.empty() && "Base path array must be empty!");
-  assert(Paths.isRecordingPaths() && "Must record paths!");
-
-  const CXXBasePath &Path = Paths.front();
-
+static void BuildBasePathArray(const CXXBasePath &Path,
+                               CXXCastPath &BasePathArray) {
   // We first go backward and check if we have a virtual base.
   // FIXME: It would be better if CXXBasePath had the base specifier for
   // the nearest virtual base.
@@ -2526,6 +2521,13 @@ void Sema::BuildBasePathArray(const CXXBasePaths &Paths,
     BasePathArray.push_back(const_cast<CXXBaseSpecifier*>(Path[I].Base));
 }
 
+
+void Sema::BuildBasePathArray(const CXXBasePaths &Paths,
+                              CXXCastPath &BasePathArray) {
+  assert(BasePathArray.empty() && "Base path array must be empty!");
+  assert(Paths.isRecordingPaths() && "Must record paths!");
+  return ::BuildBasePathArray(Paths.front(), BasePathArray);
+}
 /// CheckDerivedToBaseConversion - Check whether the Derived-to-Base
 /// conversion (where Derived and Base are class types) is
 /// well-formed, meaning that the conversion is unambiguous (and
@@ -2557,23 +2559,42 @@ Sema::CheckDerivedToBaseConversion(QualType Derived, QualType Base,
          "Can only be used with a derived-to-base conversion");
   (void)DerivationOkay;
 
-  if (!Paths.isAmbiguous(Context.getCanonicalType(Base).getUnqualifiedType())) {
+  const CXXBasePath *Path = nullptr;
+  if (!Paths.isAmbiguous(Context.getCanonicalType(Base).getUnqualifiedType()))
+    Path = &Paths.front();
+
+  // For MSVC compatibility, check if Derived directly inherits from Base. Clang
+  // warns about this hierarchy under -Winaccessible-base, but MSVC allows the
+  // user to access such bases.
+  if (!Path && getLangOpts().MSVCCompat) {
+    for (const CXXBasePath &PossiblePath : Paths) {
+      if (PossiblePath.size() == 1) {
+        Path = &PossiblePath;
+        if (AmbigiousBaseConvID)
+          Diag(Loc, diag::ext_ms_ambiguous_direct_base)
+              << Base << Derived << Range;
+        break;
+      }
+    }
+  }
+
+  if (Path) {
     if (!IgnoreAccess) {
       // Check that the base class can be accessed.
-      switch (CheckBaseClassAccess(Loc, Base, Derived, Paths.front(),
-                                   InaccessibleBaseID)) {
-        case AR_inaccessible:
-          return true;
-        case AR_accessible:
-        case AR_dependent:
-        case AR_delayed:
-          break;
+      switch (
+          CheckBaseClassAccess(Loc, Base, Derived, *Path, InaccessibleBaseID)) {
+      case AR_inaccessible:
+        return true;
+      case AR_accessible:
+      case AR_dependent:
+      case AR_delayed:
+        break;
       }
     }
 
     // Build a base path if necessary.
     if (BasePath)
-      BuildBasePathArray(Paths, *BasePath);
+      ::BuildBasePathArray(*Path, *BasePath);
     return false;
   }
 

diff --git a/clang/test/CodeGenCXX/microsoft-inaccessible-base.cpp b/clang/test/CodeGenCXX/microsoft-inaccessible-base.cpp
new file mode 100644 (file)
index 0000000..2c0d124
--- /dev/null
+++ b/clang/test/CodeGenCXX/microsoft-inaccessible-base.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -fms-compatibility -triple x86_64-windows-msvc %s -emit-llvm -o - | FileCheck %s
+
+// Make sure we choose the *direct* base path when doing these conversions.
+
+// CHECK: %struct.C = type { %struct.A, %struct.B }
+// CHECK: %struct.D = type { %struct.B, %struct.A }
+
+struct A { int a; };
+struct B : A { int b; };
+
+struct C : A, B { };
+extern "C" A *a_from_c(C *p) { return p; }
+// CHECK-LABEL: define %struct.A* @a_from_c(%struct.C* %{{.*}})
+// CHECK: bitcast %struct.C* %{{.*}} to %struct.A*
+
+struct D : B, A { };
+extern "C" A *a_from_d(D *p) { return p; }
+// CHECK-LABEL: define %struct.A* @a_from_d(%struct.D* %{{.*}})
+// CHECK: %[[p_i8:[^ ]*]] = bitcast %struct.D* %{{.*}} to i8*
+// CHECK: getelementptr inbounds i8, i8* %[[p_i8]], i64 8

diff --git a/clang/test/SemaCXX/accessible-base.cpp b/clang/test/SemaCXX/accessible-base.cpp
index 6bf06ce..2796985 100644 (file)
--- a/clang/test/SemaCXX/accessible-base.cpp
+++ b/clang/test/SemaCXX/accessible-base.cpp
@@ -1,10 +1,11 @@
 // RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -DMS -fms-compatibility -Wmicrosoft-inaccessible-base -fsyntax-only -verify %s
 
 struct A {
   int a;
 };
 
-struct X1 : virtual A 
+struct X1 : virtual A
 {};
 
 struct Y1 : X1, virtual A
@@ -13,7 +14,7 @@ struct Y1 : X1, virtual A
 struct Y2 : X1, A // expected-warning{{direct base 'A' is inaccessible due to ambiguity:\n    struct Y2 -> struct X1 -> struct A\n    struct Y2 -> struct A}}
 {};
 
-struct X2 : A 
+struct X2 : A
 {};
 
 struct Z1 : X2, virtual A // expected-warning{{direct base 'A' is inaccessible due to ambiguity:\n    struct Z1 -> struct X2 -> struct A\n    struct Z1 -> struct A}}
@@ -21,3 +22,30 @@ struct Z1 : X2, virtual A // expected-warning{{direct base 'A' is inaccessible d
 
 struct Z2 : X2, A // expected-warning{{direct base 'A' is inaccessible due to ambiguity:\n    struct Z2 -> struct X2 -> struct A\n    struct Z2 -> struct A}}
 {};
+
+A *y2_to_a(Y2 *p) {
+#ifdef MS
+  // expected-warning@+4 {{accessing inaccessible direct base 'A' of 'Y2' is a Microsoft extension}}
+#else
+  // expected-error@+2 {{ambiguous conversion}}
+#endif
+  return p;
+}
+
+A *z1_to_a(Z1 *p) {
+#ifdef MS
+  // expected-warning@+4 {{accessing inaccessible direct base 'A' of 'Z1' is a Microsoft extension}}
+#else
+  // expected-error@+2 {{ambiguous conversion}}
+#endif
+  return p;
+}
+
+A *z1_to_a(Z2 *p) {
+#ifdef MS
+  // expected-warning@+4 {{accessing inaccessible direct base 'A' of 'Z2' is a Microsoft extension}}
+#else
+  // expected-error@+2 {{ambiguous conversion}}
+#endif
+  return p;
+}