[MSVC] Support of __unaligned qualifier for function types

[andreybokhanko]

Summary

This adds support of MS-specific "__unaligned" qualifier for function types and fixes errors described by David Majnemer in this thread: http://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20160509/thread.html#158098

Yours,

Andrey

Software Engineer
Intel Compiler Team

[majnemer]

include/clang/Sema/DeclSpec.h

1152–1169

Should __unaligned actually be carried here or does MSVC discard the qualifier here?

1414–1418

Ditto. Do we need storage for this bit or should we make sure it got discarded earlier?

[andreybokhanko]

Added a test for __unaligned arrays.

[andreybokhanko]

include/clang/Sema/DeclSpec.h

1152–1169

Yes, MSVC does carry __unaligned in arrays. I added a test to MicrosoftExtensions.cpp.

1414–1418

The same. A test already exists in MicrosoftExtensions.cpp (line 121).

[majnemer]

LGTM

lib/Sema/SemaExprCXX.cpp

937

Can you add a comment for this line.

[andreybokhanko]

lib/Sema/SemaExprCXX.cpp

937

Done.

Thank you for the review!

include/clang/AST/Type.h

public:
   }
 
   static Qualifiers fromCVRMask(unsigned CVR) {
     Qualifiers Qs;
     Qs.addCVRQualifiers(CVR);
     return Qs;
   }
 
+  static Qualifiers fromCVRUMask(unsigned CVRU) {
+    Qualifiers Qs;
+    Qs.addCVRUQualifiers(CVRU);
+    return Qs;
+  }
+
   // Deserialize qualifiers from an opaque representation.
   static Qualifiers fromOpaqueValue(unsigned opaque) {
     Qualifiers Qs;
     Qs.Mask = opaque;
     return Qs;
   }
 
   // Serialize these qualifiers into an opaque representation.
public:
   }
   void removeCVRQualifiers() {
     removeCVRQualifiers(CVRMask);
   }
   void addCVRQualifiers(unsigned mask) {
     assert(!(mask & ~CVRMask) && "bitmask contains non-CVR bits");
     Mask |= mask;
   }
+  void addCVRUQualifiers(unsigned mask) {
+    assert(!(mask & ~CVRMask & ~UMask) && "bitmask contains non-CVRU bits");
+    Mask |= mask;
+  }
 
   bool hasUnaligned() const { return Mask & UMask; }
   void setUnaligned(bool flag) {
     Mask = (Mask & ~UMask) | (flag ? UMask : 0);
   }
   void removeUnaligned() { Mask &= ~UMask; }
   void addUnaligned() { Mask |= UMask; }
 
class FunctionTypeBitfields {
     unsigned ExtInfo : 9;
 
     /// Used only by FunctionProtoType, put here to pack with the
     /// other bitfields.
     /// The qualifiers are part of FunctionProtoType because...
     ///
     /// C++ 8.3.5p4: The return type, the parameter type list and the
     /// cv-qualifier-seq, [...], are part of the function type.
-    unsigned TypeQuals : 3;
+    unsigned TypeQuals : 4;
 
     /// \brief The ref-qualifier associated with a \c FunctionProtoType.
     ///
     /// This is a value of type \c RefQualifierKind.
     unsigned RefQualifier : 2;
   };
 
   class ObjCObjectTypeBitfields {

include/clang/Sema/DeclSpec.h

#include "clang/Basic/OpenCLImageTypes.def"
   static const TST TST_error = clang::TST_error;
 
   // type-qualifiers
   enum TQ {   // NOTE: These flags must be kept in sync with Qualifiers::TQ.
     TQ_unspecified = 0,
     TQ_const       = 1,
     TQ_restrict    = 2,
     TQ_volatile    = 4,
+    TQ_unaligned   = 8,
     // This has no corresponding Qualifiers::TQ value, because it's not treated
     // as a qualifier in our type system.
-    TQ_atomic      = 8,
-    // There is no corresponding Qualifiers::TQ value, but it's kept separately
-    // in a dedicated Qualifiers::Mask bit.
-    TQ_unaligned   = 16
+    TQ_atomic      = 16
   };
 
   /// ParsedSpecifiers - Flags to query which specifiers were applied.  This is
   /// returned by getParsedSpecifiers.
   enum ParsedSpecifiers {
     PQ_None                  = 0,
     PQ_StorageClassSpecifier = 1,
     PQ_TypeSpecifier         = 2,
SourceRange getSourceRange() const {
     return SourceRange(Loc, EndLoc);
   }
 
   struct TypeInfoCommon {
     AttributeList *AttrList;
   };
 
   struct PointerTypeInfo : TypeInfoCommon {
-    /// The type qualifiers: const/volatile/restrict/atomic/unaligned.
+    /// The type qualifiers: const/volatile/restrict/unaligned/atomic.
     unsigned TypeQuals : 5;
 
     /// The location of the const-qualifier, if any.
     unsigned ConstQualLoc;
 
     /// The location of the volatile-qualifier, if any.
     unsigned VolatileQualLoc;
 
struct ReferenceTypeInfo : TypeInfoCommon {
     /// The type qualifier: restrict. [GNU] C++ extension
     bool HasRestrict : 1;
     /// True if this is an lvalue reference, false if it's an rvalue reference.
     bool LValueRef : 1;
     void destroy() {
     }
   };
 
   struct ArrayTypeInfo : TypeInfoCommon {
-    /// The type qualifiers for the array: const/volatile/restrict/_Atomic.
-    unsigned TypeQuals : 4;
+    /// The type qualifiers for the array:
+    /// const/volatile/restrict/__unaligned/_Atomic.
+    unsigned TypeQuals : 5;
 
     /// True if this dimension included the 'static' keyword.
     bool hasStatic : 1;
 
     /// True if this dimension was [*].  In this case, NumElts is null.
     bool isStar : 1;
 
     /// This is the size of the array, or null if [] or [*] was specified.
     /// Since the parser is multi-purpose, and we don't want to impose a root
     /// expression class on all clients, NumElts is untyped.
     Expr *NumElts;
 
     void destroy() {}
   };
 
   /// ParamInfo - An array of paraminfo objects is allocated whenever a function
   /// declarator is parsed.  There are two interesting styles of parameters
   /// here:
   /// K&R-style identifier lists and parameter type lists.  K&R-style identifier
   /// lists will have information about the identifier, but no type information.
   /// Parameter type lists will have type info (if the actions module provides
   /// it), but may have null identifier info: e.g. for 'void foo(int X, int)'.
struct FunctionTypeInfo : TypeInfoCommon {
 
     /// Can this declaration be a constructor-style initializer?
     unsigned isAmbiguous : 1;
 
     /// \brief Whether the ref-qualifier (if any) is an lvalue reference.
     /// Otherwise, it's an rvalue reference.
     unsigned RefQualifierIsLValueRef : 1;
 
-    /// The type qualifiers: const/volatile/restrict.
+    /// The type qualifiers: const/volatile/restrict/__unaligned
     /// The qualifier bitmask values are the same as in QualType.
-    unsigned TypeQuals : 3;
+    unsigned TypeQuals : 4;
 
     /// ExceptionSpecType - An ExceptionSpecificationType value.
     unsigned ExceptionSpecType : 4;
 
     /// DeleteParams - If this is true, we need to delete[] Params.
     unsigned DeleteParams : 1;
 
     /// HasTrailingReturnType - If this is true, a trailing return type was
struct FunctionTypeInfo : TypeInfoCommon {
     bool hasTrailingReturnType() const { return HasTrailingReturnType; }
 
     /// \brief Get the trailing-return-type for this function declarator.
     ParsedType getTrailingReturnType() const { return TrailingReturnType; }
   };
 
   struct BlockPointerTypeInfo : TypeInfoCommon {
     /// For now, sema will catch these as invalid.
-    /// The type qualifiers: const/volatile/restrict/_Atomic.
-    unsigned TypeQuals : 4;
+    /// The type qualifiers: const/volatile/restrict/__unaligned/_Atomic.
+    unsigned TypeQuals : 5;
 
     void destroy() {
     }
   };
 
   struct MemberPointerTypeInfo : TypeInfoCommon {
-    /// The type qualifiers: const/volatile/restrict/_Atomic.
-    unsigned TypeQuals : 4;
+    /// The type qualifiers: const/volatile/restrict/__unaligned/_Atomic.
+    unsigned TypeQuals : 5;
     // CXXScopeSpec has a constructor, so it can't be a direct member.
     // So we need some pointer-aligned storage and a bit of trickery.
     union {
       void *Aligner;
       char Mem[sizeof(CXXScopeSpec)];
     } ScopeMem;
     CXXScopeSpec &Scope() {
       return *reinterpret_cast<CXXScopeSpec*>(ScopeMem.Mem);
     }
     const CXXScopeSpec &Scope() const {

lib/AST/DeclCXX.cpp

QualType CXXMethodDecl::getThisType(ASTContext &C) const {
   // if the member function is declared volatile, the type of this is
   // volatile X*, and if the member function is declared const volatile,
   // the type of this is const volatile X*.
 
   assert(isInstance() && "No 'this' for static methods!");
 
   QualType ClassTy = C.getTypeDeclType(getParent());
   ClassTy = C.getQualifiedType(ClassTy,
-                               Qualifiers::fromCVRMask(getTypeQualifiers()));
+                               Qualifiers::fromCVRUMask(getTypeQualifiers()));
   return C.getPointerType(ClassTy);
 }
 
 bool CXXMethodDecl::hasInlineBody() const {
   // If this function is a template instantiation, look at the template from 
   // which it was instantiated.
   const FunctionDecl *CheckFn = getTemplateInstantiationPattern();
   if (!CheckFn)

lib/AST/MicrosoftMangle.cpp

void MicrosoftCXXNameMangler::manglePointerExtQualifiers(Qualifiers Quals,
   bool HasRestrict = Quals.hasRestrict();
   if (PointersAre64Bit &&
       (PointeeType.isNull() || !PointeeType->isFunctionType()))
     Out << 'E';
 
   if (HasRestrict)
     Out << 'I';
 
-  if (!PointeeType.isNull() && PointeeType.getLocalQualifiers().hasUnaligned())
+  if (Quals.hasUnaligned() ||
+      (!PointeeType.isNull() && PointeeType.getLocalQualifiers().hasUnaligned()))
     Out << 'F';
 }
 
 void MicrosoftCXXNameMangler::manglePointerCVQualifiers(Qualifiers Quals) {
   // <pointer-cv-qualifiers> ::= P  # no qualifiers
   //                         ::= Q  # const
   //                         ::= R  # volatile
   //                         ::= S  # const volatile
if (isa<CXXDestructorDecl>(MD)) {
         CC = getASTContext().getDefaultCallingConvention(
             /*IsVariadic=*/false, /*IsCXXMethod=*/true);
     }
   }
 
   // If this is a C++ instance method, mangle the CVR qualifiers for the
   // this pointer.
   if (HasThisQuals) {
-    Qualifiers Quals = Qualifiers::fromCVRMask(Proto->getTypeQuals());
+    Qualifiers Quals = Qualifiers::fromCVRUMask(Proto->getTypeQuals());
     manglePointerExtQualifiers(Quals, /*PointeeType=*/QualType());
     mangleRefQualifier(Proto->getRefQualifier());
     mangleQualifiers(Quals, /*IsMember=*/false);
   }
 
   mangleCallingConvention(CC);
 
   // <return-type> ::= <type>

lib/Sema/DeclSpec.cpp

bool DeclSpec::SetTypeQual(TQ T, SourceLocation Loc, const char *&PrevSpec,
   }
   TypeQualifiers |= T;
 
   switch (T) {
   case TQ_unspecified: break;
   case TQ_const:    TQ_constLoc = Loc; return false;
   case TQ_restrict: TQ_restrictLoc = Loc; return false;
   case TQ_volatile: TQ_volatileLoc = Loc; return false;
-  case TQ_atomic:   TQ_atomicLoc = Loc; return false;
   case TQ_unaligned: TQ_unalignedLoc = Loc; return false;
+  case TQ_atomic:   TQ_atomicLoc = Loc; return false;
   }
 
   llvm_unreachable("Unknown type qualifier!");
 }
 
 bool DeclSpec::setFunctionSpecInline(SourceLocation Loc, const char *&PrevSpec,
                                      unsigned &DiagID) {
   // 'inline inline' is ok.  However, since this is likely not what the user

lib/Sema/SemaExprCXX.cpp

if (!Enabled || !ContextDecl)
     return;
 
   CXXRecordDecl *Record = nullptr;
   if (ClassTemplateDecl *Template = dyn_cast<ClassTemplateDecl>(ContextDecl))
     Record = Template->getTemplatedDecl();
   else
     Record = cast<CXXRecordDecl>(ContextDecl);
     
+  CXXThisTypeQuals &= Qualifiers::FastMask;
   S.CXXThisTypeOverride
     = S.Context.getPointerType(
         S.Context.getRecordType(Record).withCVRQualifiers(CXXThisTypeQuals));
   
   this->Enabled = true;
 }
 
 

lib/Sema/SemaType.cpp

void Sema::diagnoseIgnoredQualifiers(unsigned DiagID, unsigned Quals,
   struct Qual {
     const char *Name;
     unsigned Mask;
     SourceLocation Loc;
   } const QualKinds[5] = {
     { "const", DeclSpec::TQ_const, ConstQualLoc },
     { "volatile", DeclSpec::TQ_volatile, VolatileQualLoc },
     { "restrict", DeclSpec::TQ_restrict, RestrictQualLoc },
-    { "_Atomic", DeclSpec::TQ_atomic, AtomicQualLoc },
-    { "__unaligned", DeclSpec::TQ_unaligned, UnalignedQualLoc }
+    { "__unaligned", DeclSpec::TQ_unaligned, UnalignedQualLoc },
+    { "_Atomic", DeclSpec::TQ_atomic, AtomicQualLoc }
   };
 
   SmallString<32> QualStr;
   unsigned NumQuals = 0;
   SourceLocation Loc;
   FixItHint FixIts[5];
 
   // Build a string naming the redundant qualifiers.

test/CodeGenCXX/mangle-ms-cxx11.cpp

 
 // CHECK-DAG: @"\01?unaligned_foo1@@YAPFAHXZ"
 // CHECK-DAG: @"\01?unaligned_foo2@@YAPFAPFAHXZ"
 // CHECK-DAG: @"\01?unaligned_foo3@@YAHXZ"
 // CHECK-DAG: @"\01?unaligned_foo4@@YAXPFAH@Z"
 // CHECK-DAG: @"\01?unaligned_foo5@@YAXPIFAH@Z"
 // CHECK-DAG: @"\01??$unaligned_foo6@PAH@@YAPAHPAH@Z"
 // CHECK-DAG: @"\01??$unaligned_foo6@PFAH@@YAPFAHPFAH@Z"
+
+// __unaligned qualifier for function types
+struct unaligned_foo8_S {
+    void unaligned_foo8() volatile __unaligned;
+};
+void unaligned_foo8_S::unaligned_foo8() volatile __unaligned {}
+
+// CHECK-DAG: @"\01?unaligned_foo8@unaligned_foo8_S@@QFCEXXZ"
+

test/Sema/MicrosoftExtensions.c

 
 // __unaligned handling
 void test_unaligned() {
   __unaligned int *p1 = 0;
   int *p2 = p1; // expected-warning {{initializing 'int *' with an expression of type '__unaligned int *' discards qualifiers}}
   __unaligned int *p3 = p2;
 }
 
+void test_unaligned2(int x[__unaligned 4]) {}
+

test/SemaCXX/MicrosoftExtensions.cpp

 
 struct aligned_type4 {
   int i;
 };
 
 __unaligned int aligned_type4::*p1_aligned_type4 = &aligned_type4::i;
 int aligned_type4::* __unaligned p2_aligned_type4 = &aligned_type4::i;
 __unaligned int aligned_type4::* __unaligned p3_aligned_type4 = &aligned_type4::i;
+void (aligned_type4::*__unaligned p4_aligned_type4)();
 
 // Check that __unaligned qualifier can be used for overloading
 void foo_unaligned(int *arg) {}
 void foo_unaligned(__unaligned int *arg) {}
 void foo_unaligned(int arg) {} // expected-note {{previous definition is here}}
 void foo_unaligned(__unaligned int arg) {} // expected-error {{redefinition of 'foo_unaligned'}}
 class A_unaligned {};
 class B_unaligned : public A_unaligned {};