[Sema][Windows] Don't special-case void* in __unaligned conversions.

[efriedma]

Summary

As far as I can tell, MSVC allows the relevant conversions for all pointer types. Found compiling a Windows SDK header.

I've verified that the updated errors in MicrosoftExtensions.cpp match the ones that MSVC actually emits, except for the one with a FIXME. (Not sure why this wasn't done for the patch that added the tests.)

[erichkeane]

I don't particularly understand this qualifier, is this something that perhaps @rnk could take a look at?

clang/test/SemaCXX/MicrosoftExtensions.cpp

88–89

Hmm... this one was quite suspicious. In this case, it was choosing the 'int' returning overload, MSVC doesn't do that here, it seems to always choose the best match without the __unaligned (as you're doing now).

91–92

MSVC is warning on this one, its a shame we don't have something similar.

[rnk]

I remember writing up feedback on this patch, but it's gone now, oh well.

I wasn't able to find any history for why void* was special here.

I see that MSVC doesn't warn when converting from __unaligned int* to int*, but that seems dangerous. Our team has an active feature request to tighten up clang warnings to make it harder to form unaligned pointers, so I'd like to insist that we warn when the __unaligned qualifier gets lost.

clang/test/SemaCXX/MicrosoftExtensions.cpp

91–92

I agree this is important, and I think it's a blocking issue. We don't want to allow new constructs, introduce a warning for them later, and then ask people to clean it up.

[efriedma]

Added warning. This roughly matches the corresponding MSVC warning, as far as I can tell.

[efriedma]

In D120936#3368566, @rnk wrote:

I wasn't able to find any history for why void* was special here.

As far as I can tell, nobody ever thought void* was special. The check was an attempt to restrict dubious conversions while still remaining compatible with existing code.

[erichkeane]

This looks fine to me, though we should wait for @rnk to decide whether the warning is sufficient for him.

[rnk]

lgtm, thanks!

clang/include/clang/Basic/DiagnosticSemaKinds.td

 
 def err_openclcxx_placement_new : Error<
   "use of placement new requires explicit declaration">;
 
 // MIG routine annotations.
 def warn_mig_server_routine_does_not_return_kern_return_t : Warning<
   "'mig_server_routine' attribute only applies to routines that return a kern_return_t">,
   InGroup<IgnoredAttributes>;
+
+def warn_imp_cast_drops_unaligned : Warning<
+  "implicit cast from type %0 to type %1 drops __unaligned qualifier">,
+  InGroup<DiagGroup<"unaligned-qualifier-implicit-cast">>;
+
 } // end of sema category
 
 let CategoryName = "OpenMP Issue" in {
 // OpenMP support.
 def err_omp_expected_var_arg : Error<
   "%0 is not a global variable, static local variable or static data member">;
 def err_omp_expected_var_arg_suggest : Error<
   "%0 is not a global variable, static local variable or static data member; "

clang/lib/Sema/SemaExprCXX.cpp

if (ToType->isReferenceType() &&
             From->getType().getAddressSpace())
       CK = CK_AddressSpaceConversion;
 
     if (ToType->isPointerType() &&
         ToType->getPointeeType().getAddressSpace() !=
             From->getType()->getPointeeType().getAddressSpace())
       CK = CK_AddressSpaceConversion;
 
+    if (!isCast(CCK) &&
+        !ToType->getPointeeType().getQualifiers().hasUnaligned() &&
+        From->getType()->getPointeeType().getQualifiers().hasUnaligned()) {
+      Diag(From->getBeginLoc(), diag::warn_imp_cast_drops_unaligned)
+          << InitialFromType << ToType;
+    }
+
     From = ImpCastExprToType(From, ToType.getNonLValueExprType(Context), CK, VK,
                              /*BasePath=*/nullptr, CCK)
                .get();
 
     if (SCS.DeprecatedStringLiteralToCharPtr &&
         !getLangOpts().WritableStrings) {
       Diag(From->getBeginLoc(),
            getLangOpts().CPlusPlus11

clang/lib/Sema/SemaOverload.cpp

 /// outer layer is const-qualified.
 static bool isQualificationConversionStep(QualType FromType, QualType ToType,
                                           bool CStyle, bool IsTopLevel,
                                           bool &PreviousToQualsIncludeConst,
                                           bool &ObjCLifetimeConversion) {
   Qualifiers FromQuals = FromType.getQualifiers();
   Qualifiers ToQuals = ToType.getQualifiers();
 
-  // Ignore __unaligned qualifier if this type is void.
-  if (ToType.getUnqualifiedType()->isVoidType())
-    FromQuals.removeUnaligned();
+  // Ignore __unaligned qualifier.
 
   // Objective-C ARC:
   //   Check Objective-C lifetime conversions.
   if (FromQuals.getObjCLifetime() != ToQuals.getObjCLifetime()) {
     if (ToQuals.compatiblyIncludesObjCLifetime(FromQuals)) {
       if (isNonTrivialObjCLifetimeConversion(FromQuals, ToQuals))
         ObjCLifetimeConversion = true;
       FromQuals.removeObjCLifetime();

clang/test/SemaCXX/MicrosoftExtensions.cpp

 void test_unaligned() {
   int *p1 = 0;
   foo_unaligned(p1);
 
   __unaligned int *p2 = 0;
   foo_unaligned(p2);
 
   __unaligned B_unaligned *p3 = 0;
-  int p4 = foo_unaligned(p3);
+  int p4 = foo_unaligned(p3); // expected-error {{cannot initialize a variable of type 'int' with an rvalue of type 'void *'}}
+  // expected-warning@-1 {{implicit cast from type '__unaligned B_unaligned *' to type 'B_unaligned *' drops __unaligned qualifier}}
 
-  B_unaligned *p5 = p3; // expected-error {{cannot initialize a variable of type 'B_unaligned *' with an lvalue of type '__unaligned B_unaligned *'}}
+  B_unaligned *p5 = p3;
+  // expected-warning@-1 {{implicit cast from type '__unaligned B_unaligned *' to type 'B_unaligned *' drops __unaligned qualifier}}
 
   __unaligned B_unaligned *p6 = p3;
 
   p1_aligned_type4 = p2_aligned_type4;
-  p2_aligned_type4 = p1_aligned_type4; // expected-error {{assigning to 'int aligned_type4::*' from incompatible type '__unaligned int aligned_type4::*'}}
+  p2_aligned_type4 = p1_aligned_type4;
+  // expected-warning@-1 {{implicit cast from type '__unaligned int aligned_type4::*' to type 'int aligned_type4::*' drops __unaligned qualifier}}
   p3_aligned_type4 = p1_aligned_type4;
 
   __unaligned int a[10];
-  int *b = a; // expected-error {{cannot initialize a variable of type 'int *' with an lvalue of type '__unaligned int[10]'}}
+  int *b = a;
+  // expected-warning@-1 {{implicit cast from type '__unaligned int[10]' to type 'int *' drops __unaligned qualifier}}
 }
 
 // Test from PR27367
 // We should accept assignment of an __unaligned pointer to a non-__unaligned
 // pointer to void
 typedef struct _ITEMIDLIST { int i; } ITEMIDLIST;
 typedef ITEMIDLIST __unaligned *LPITEMIDLIST;
 extern "C" __declspec(dllimport) void __stdcall CoTaskMemFree(void* pv);
 __inline void FreeIDListArray(LPITEMIDLIST *ppidls) {
   CoTaskMemFree(*ppidls);
   __unaligned int *x = 0;
   void *y = x;
 }
 
 // Test from PR27666
 // We should accept type conversion of __unaligned to non-__unaligned references
 typedef struct in_addr {
 public:
-  in_addr(in_addr &a) {} // expected-note {{candidate constructor not viable: no known conversion from '__unaligned IN_ADDR *' (aka '__unaligned in_addr *') to 'in_addr &' for 1st argument; dereference the argument with *}}
-  in_addr(in_addr *a) {} // expected-note {{candidate constructor not viable: 1st argument ('__unaligned IN_ADDR *' (aka '__unaligned in_addr *')) would lose __unaligned qualifier}}
+  in_addr(in_addr &a) {} // expected-note {{candidate constructor not viable: expects an lvalue for 1st argument}}
+  in_addr(in_addr *a) {} // expected-note {{candidate constructor not viable: no known conversion from 'IN_ADDR' (aka 'in_addr') to 'in_addr *' for 1st argument}}
 } IN_ADDR;
 
 void f(IN_ADDR __unaligned *a) {
   IN_ADDR local_addr = *a;
-  IN_ADDR local_addr2 = a; // expected-error {{no viable conversion from '__unaligned IN_ADDR *' (aka '__unaligned in_addr *') to 'IN_ADDR' (aka 'in_addr')}}
+  // FIXME: MSVC accepts the following; not sure why clang tries to
+  // copy-construct an in_addr.
+  IN_ADDR local_addr2 = a; // expected-error {{no viable constructor copying variable of type 'IN_ADDR' (aka 'in_addr')}}
+  // expected-warning@-1 {{implicit cast from type '__unaligned IN_ADDR *' (aka '__unaligned in_addr *') to type 'in_addr *' drops __unaligned qualifier}}
+  IN_ADDR local_addr3(a);
+  // expected-warning@-1 {{implicit cast from type '__unaligned IN_ADDR *' (aka '__unaligned in_addr *') to type 'in_addr *' drops __unaligned qualifier}}
 }
 
 template<typename T> void h1(T (__stdcall M::* const )()) { }
 
 void m1() {
   h1<int>(&M::addP);
   h1(&M::subtractP);
 }