[PATCH] D28166: Properly merge K&R functions that have attributes

Richard Smith via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Dec 29 13:17:28 PST 2016


rsmith added inline comments.


================
Comment at: lib/Sema/SemaDecl.cpp:7464-7470
+    const Type *NonAttributedFTy = R.getTypePtr();
+    while (const auto *AttrTy = NonAttributedFTy->getAs<AttributedType>()) {
+      NonAttributedFTy = AttrTy->getModifiedType().getTypePtr();
+    }
     bool HasPrototype =
       (D.isFunctionDeclarator() && D.getFunctionTypeInfo().hasPrototype) ||
+      (!isa<FunctionType>(NonAttributedFTy) && R->isFunctionProtoType());
----------------
Rather than hardcoding the forms of type sugar that can appear here, can we just use `R.getTypePtr()->getAs<FunctionType>()`? I expect we can also have `ParenType`s wrapping the `FunctionNoProtoType` (`int (f)();`).


================
Comment at: lib/Sema/SemaDecl.cpp:11958-11962
+        // The type location may be attributed; strip the attributes to get to
+        // the function type location.
+        while (auto ATL = TL.getAs<AttributedTypeLoc>()) {
+          TL = ATL.getModifiedLoc();
+        }
----------------
Again, I don't like having this knowledge about what kinds of type sugar can appear in a function declaration hardcoded here. Please put this somewhere more central.

A quick look finds that `FunctionDecl::getReturnTypeSourceRange()` gets this wrong in the opposite direction: it skips parens but not attributes. Maybe we should have a `TypeLoc::getAsAdjusted<T>` or similar, that walks over type sugar nodes that represent some kind of type adjustment from a type that was written as a T to another type that is still canonically a T (`ParenType`, `AttributedType`, `ElaboratedType`).


https://reviews.llvm.org/D28166





More information about the cfe-commits mailing list