leonardchan created this revision.
leonardchan added reviewers: rsmith, rjmccall, ebevhan, mcgrathr, phosek.
leonardchan added a project: clang.

If an `address_space` attribute is defined in a macro, print the macro instead 
when diagnosing a warning or error for incompatible pointers with different 
address_spaces.

Originally started from 
http://lists.llvm.org/pipermail/cfe-dev/2018-August/058702.html where the 
original proposal was a new pragma, but instead just changing the way 
`address_space` gets printed in a diagnostic.


Repository:
  rC Clang

https://reviews.llvm.org/D51329

Files:
  include/clang/AST/ASTContext.h
  include/clang/AST/Type.h
  include/clang/Lex/Preprocessor.h
  lib/AST/ASTContext.cpp
  lib/AST/TypePrinter.cpp
  lib/Lex/PPDirectives.cpp
  lib/Sema/SemaType.cpp
  test/Sema/address_space_print_macro.c
  test/Sema/address_spaces.c

Index: test/Sema/address_spaces.c
===================================================================
--- test/Sema/address_spaces.c
+++ test/Sema/address_spaces.c
@@ -71,5 +71,5 @@
 
 // Clang extension doesn't forbid operations on pointers to different address spaces.
 char* cmp(_AS1 char *x,  _AS2 char *y) {
-  return x < y ? x : y; // expected-error{{conditional operator with the second and third operands of type  ('__attribute__((address_space(1))) char *' and '__attribute__((address_space(2))) char *') which are pointers to non-overlapping address spaces}}
+  return x < y ? x : y; // expected-error{{conditional operator with the second and third operands of type  ('_AS1 char *' and '_AS2 char *') which are pointers to non-overlapping address spaces}}
 }
Index: test/Sema/address_space_print_macro.c
===================================================================
--- /dev/null
+++ test/Sema/address_space_print_macro.c
@@ -0,0 +1,25 @@
+// RUN: %clang_cc1 %s -fsyntax-only -verify
+
+#define _AS1 __attribute__((address_space(1)))
+#define _AS2 __attribute__((address_space(2)))
+
+char *cmp(_AS1 char *x, _AS2 char *y) {
+  return x < y ? x : y; // expected-error{{conditional operator with the second and third operands of type  ('_AS1 char *' and '_AS2 char *') which are pointers to non-overlapping address spaces}}
+}
+
+__attribute__((address_space(1))) char test_array[10];
+void test3(void) {
+  extern void test3_helper(char *p); // expected-note{{passing argument to parameter 'p' here}}
+  test3_helper(test_array);          // expected-error{{passing '__attribute__((address_space(1))) char *' to parameter of type 'char *' changes address space of pointer}}
+}
+
+char _AS2 *test4_array;
+void test4(void) {
+  extern void test3_helper(char *p); // expected-note{{passing argument to parameter 'p' here}}
+  test3_helper(test4_array);         // expected-error{{passing '_AS2 char *' to parameter of type 'char *' changes address space of pointer}}
+}
+
+void func() {
+  char _AS1 *x;
+  char *y = x; // expected-error{{initializing 'char *' with an expression of type '_AS1 char *' changes address space of pointer}}
+}
Index: lib/Sema/SemaType.cpp
===================================================================
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -243,9 +243,10 @@
     /// Get an attributed type for the given attribute, and remember the Attr
     /// object so that we can attach it to the AttributedTypeLoc.
     QualType getAttributedType(Attr *A, QualType ModifiedType,
-                               QualType EquivType) {
-      QualType T =
-          sema.Context.getAttributedType(A->getKind(), ModifiedType, EquivType);
+                               QualType EquivType,
+                               IdentifierInfo *AddressSpaceMacroII = nullptr) {
+      QualType T = sema.Context.getAttributedType(
+          A->getKind(), ModifiedType, EquivType, AddressSpaceMacroII);
       AttrsForTypes.push_back({cast<AttributedType>(T.getTypePtr()), A});
       AttrsForTypesSorted = false;
       return T;
@@ -5813,11 +5814,17 @@
     QualType T = S.BuildAddressSpaceAttr(Type, ASArgExpr, Attr.getLoc());
 
     if (!T.isNull()) {
+      IdentifierInfo *AddressSpaceMacroII = nullptr;
+      auto FoundMacro = S.PP.AddressSpaceMacros.find(
+          S.SourceMgr.getSpellingLoc(Attr.getLoc()));
+      if (FoundMacro != S.PP.AddressSpaceMacros.end())
+        AddressSpaceMacroII = FoundMacro->second;
+
       ASTContext &Ctx = S.Context;
       auto *ASAttr = ::new (Ctx) AddressSpaceAttr(
           Attr.getRange(), Ctx, Attr.getAttributeSpellingListIndex(),
           static_cast<unsigned>(T.getQualifiers().getAddressSpace()));
-      Type = State.getAttributedType(ASAttr, T, T);
+      Type = State.getAttributedType(ASAttr, T, T, AddressSpaceMacroII);
     } else {
       Attr.setInvalid();
     }
Index: lib/Lex/PPDirectives.cpp
===================================================================
--- lib/Lex/PPDirectives.cpp
+++ lib/Lex/PPDirectives.cpp
@@ -2577,6 +2577,22 @@
   MI->setDefinitionEndLoc(LastTok.getLocation());
   return MI;
 }
+
+static bool DefinesAddressSpace(const Preprocessor &PP, const ArrayRef<Token> &Tokens,
+                                SourceLocation &Loc) {
+  if (Tokens.empty()) return false;
+
+  unsigned size = Tokens.size();
+  for (unsigned i = 0; i < size; ++i) {
+    const Token &Tok = Tokens[i];
+    if (Tok.is(tok::identifier) && Tok.getIdentifierInfo()->getName() == "address_space") {
+      Loc = Tok.getLocation();
+      return true;
+    }
+  }
+  return false;
+}
+
 /// HandleDefineDirective - Implements \#define.  This consumes the entire macro
 /// line then lets the caller lex the next real token.
 void Preprocessor::HandleDefineDirective(
@@ -2692,6 +2708,12 @@
     WarnUnusedMacroLocs.insert(MI->getDefinitionLoc());
   }
 
+  // If the macro is an attribute that contains address_space(), save this for
+  // diagnosing later.
+  SourceLocation AttrLoc;
+  if (DefinesAddressSpace(*this, MD->getInfo()->tokens(), AttrLoc))
+    AddressSpaceMacros[AttrLoc] = MacroNameTok.getIdentifierInfo();
+
   // If the callbacks want to know, tell them about the macro definition.
   if (Callbacks)
     Callbacks->MacroDefined(MacroNameTok, MD);
Index: lib/AST/TypePrinter.cpp
===================================================================
--- lib/AST/TypePrinter.cpp
+++ lib/AST/TypePrinter.cpp
@@ -1364,7 +1364,17 @@
   if (T->getAttrKind() == attr::ObjCKindOf)
     OS << "__kindof ";
 
-  printBefore(T->getModifiedType(), OS);
+  if (T->getAttrKind() == attr::AddressSpace && T->hasAddressSpaceMacroII()) {
+    OS << T->getAddressSpaceMacroII()->getName() << " ";
+
+    // Remove the underlying address space so it won't be printed.
+    SplitQualType SplitTy = T->getModifiedType().split();
+    Qualifiers Quals = SplitTy.Quals;
+    Quals.removeAddressSpace();
+    printBefore(SplitTy.Ty, Quals, OS);
+  } else {
+    printBefore(T->getModifiedType(), OS);
+  }
 
   if (T->isMSTypeSpec()) {
     switch (T->getAttrKind()) {
Index: lib/AST/ASTContext.cpp
===================================================================
--- lib/AST/ASTContext.cpp
+++ lib/AST/ASTContext.cpp
@@ -3878,17 +3878,19 @@
 
 QualType ASTContext::getAttributedType(attr::Kind attrKind,
                                        QualType modifiedType,
-                                       QualType equivalentType) {
+                                       QualType equivalentType,
+                                       IdentifierInfo *AddressSpaceMacroII) {
   llvm::FoldingSetNodeID id;
-  AttributedType::Profile(id, attrKind, modifiedType, equivalentType);
+  AttributedType::Profile(id, attrKind, modifiedType, equivalentType,
+                          AddressSpaceMacroII);
 
   void *insertPos = nullptr;
   AttributedType *type = AttributedTypes.FindNodeOrInsertPos(id, insertPos);
   if (type) return QualType(type, 0);
 
   QualType canon = getCanonicalType(equivalentType);
-  type = new (*this, TypeAlignment)
-           AttributedType(canon, attrKind, modifiedType, equivalentType);
+  type = new (*this, TypeAlignment) AttributedType(
+      canon, attrKind, modifiedType, equivalentType, AddressSpaceMacroII);
 
   Types.push_back(type);
   AttributedTypes.InsertNode(type, insertPos);
Index: include/clang/Lex/Preprocessor.h
===================================================================
--- include/clang/Lex/Preprocessor.h
+++ include/clang/Lex/Preprocessor.h
@@ -341,6 +341,8 @@
           ElseLoc(ElseLoc) {}
   };
 
+  std::map<SourceLocation, IdentifierInfo *> AddressSpaceMacros;
+
 private:
   friend class ASTReader;
   friend class MacroArgs;
Index: include/clang/AST/Type.h
===================================================================
--- include/clang/AST/Type.h
+++ include/clang/AST/Type.h
@@ -4315,13 +4315,21 @@
   QualType ModifiedType;
   QualType EquivalentType;
 
+  // If the attribute this type holds is address_space and this attribute was
+  // declared as part of a macro, we store the macro identifier information.
+  // This will be used for printing the macro name instead of
+  // `__attribute__((address_space(n)))` in diagnostics relating to differing
+  // address_spaces between types.
+  IdentifierInfo *AddressSpaceMacroII;
+
   AttributedType(QualType canon, attr::Kind attrKind, QualType modified,
-                 QualType equivalent)
+                 QualType equivalent, IdentifierInfo *addrSpaceMacroII)
       : Type(Attributed, canon, equivalent->isDependentType(),
              equivalent->isInstantiationDependentType(),
              equivalent->isVariablyModifiedType(),
              equivalent->containsUnexpandedParameterPack()),
-        ModifiedType(modified), EquivalentType(equivalent) {
+        ModifiedType(modified), EquivalentType(equivalent),
+        AddressSpaceMacroII(addrSpaceMacroII) {
     AttributedTypeBits.AttrKind = attrKind;
   }
 
@@ -4332,6 +4340,8 @@
 
   QualType getModifiedType() const { return ModifiedType; }
   QualType getEquivalentType() const { return EquivalentType; }
+  IdentifierInfo *getAddressSpaceMacroII() const { return AddressSpaceMacroII; }
+  bool hasAddressSpaceMacroII() const { return AddressSpaceMacroII != nullptr; }
 
   bool isSugared() const { return true; }
   QualType desugar() const { return getEquivalentType(); }
@@ -4387,14 +4397,17 @@
   static Optional<NullabilityKind> stripOuterNullability(QualType &T);
 
   void Profile(llvm::FoldingSetNodeID &ID) {
-    Profile(ID, getAttrKind(), ModifiedType, EquivalentType);
+    Profile(ID, getAttrKind(), ModifiedType, EquivalentType,
+            AddressSpaceMacroII);
   }
 
   static void Profile(llvm::FoldingSetNodeID &ID, Kind attrKind,
-                      QualType modified, QualType equivalent) {
+                      QualType modified, QualType equivalent,
+                      IdentifierInfo *addrSpaceMacroII) {
     ID.AddInteger(attrKind);
     ID.AddPointer(modified.getAsOpaquePtr());
     ID.AddPointer(equivalent.getAsOpaquePtr());
+    ID.AddPointer(addrSpaceMacroII);
   }
 
   static bool classof(const Type *T) {
Index: include/clang/AST/ASTContext.h
===================================================================
--- include/clang/AST/ASTContext.h
+++ include/clang/AST/ASTContext.h
@@ -1423,9 +1423,9 @@
 
   QualType getInjectedClassNameType(CXXRecordDecl *Decl, QualType TST) const;
 
-  QualType getAttributedType(attr::Kind attrKind,
-                             QualType modifiedType,
-                             QualType equivalentType);
+  QualType getAttributedType(attr::Kind attrKind, QualType modifiedType,
+                             QualType equivalentType,
+                             IdentifierInfo *AddressSpaceMacroII = nullptr);
 
   QualType getSubstTemplateTypeParmType(const TemplateTypeParmType *Replaced,
                                         QualType Replacement) const;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to