[PATCH] [AST] Introduce a new DecayedType sugar node

Richard Smith richard at metafoo.co.uk
Thu Jun 20 15:30:50 PDT 2013



================
Comment at: include/clang/AST/ASTContext.h:895-897
@@ +894,5 @@
+
+  /// \brief Returns a DecayedType if T is an array or function.  Otherwise
+  /// returns T.  Call this instead of getDecayedType() if T's type is unknown.
+  QualType getMaybeDecayedType(QualType T) const;
+
----------------
Please rename to getAdjustedParameterType, and make the existing getAdjustedParameterType local to ASTContext.cpp. I don't think we want to expose a way to perform parameter type decay that doesn't build a DecayedType node.

================
Comment at: include/clang/AST/RecursiveASTVisitor.h:850-853
@@ -849,1 +849,6 @@
 
+DEF_TRAVERSE_TYPE(DecayedType, {
+    TRY_TO(TraverseType(T->getDecayedType()));
+    TRY_TO(TraverseType(T->getOriginalType()));
+  })
+
----------------
Seems a bit weird to me to visit both types here. I also worry that this will be exponential-time if we have deeply-nested functions-taking-functions-taking-functions. I suggest you follow our behavior for AttributedType here and visit the original type, not the desugared type.

================
Comment at: include/clang/AST/Type.h:1994
@@ -1993,1 +1993,3 @@
 
+/// DecayedType - Represents a pointer type decayed from an array or function
+/// type.
----------------
Please use \brief, not "DecayedType - ".

================
Comment at: lib/Sema/SemaExpr.cpp:4024
@@ -4023,1 +4023,3 @@
+  if (DecayedTypeLoc DTL = TL.getAs<DecayedTypeLoc>())
+    TL = DTL.getNextTypeLoc();
   if (ArrayTypeLoc ATL = TL.getAs<ArrayTypeLoc>())
----------------
Use DTL.getOriginalLoc() here, please.

================
Comment at: lib/Sema/SemaType.cpp:3576-3578
@@ -3579,2 +3575,5 @@
     }
+    void VisitDecayedTypeLoc(DecayedTypeLoc TL) {
+      llvm_unreachable("decayed type locs not expected here!");
+    }
 
----------------
How do we ever fill in a DecayedTypeLoc's inner type?

================
Comment at: lib/Serialization/ASTReader.cpp:4523-4526
@@ +4522,6 @@
+    QualType OriginalType = readType(*Loc.F, Record, Idx);
+    QualType DT = Context.getMaybeDecayedType(OriginalType);
+    if (!isa<DecayedType>(DT))
+      Error("Decayed type does not decay");
+    return DT;
+  }
----------------
This is just 'return Context.getDecayedType(OriginalType);', right?

================
Comment at: lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp:599-602
@@ -598,5 +598,6 @@
   if (FTP->getNumArgs() == 1) {
-    // Is the argument an 'unsigned short *'?
+    // Is the canonical argument type 'unsigned short *'?
     // (Actually any integer type is allowed.)
-    const PointerType *PT = dyn_cast<PointerType>(FTP->getArgType(0));
+    const PointerType *PT =
+        dyn_cast<PointerType>(FTP->getArgType(0).getCanonicalType());
     if (!PT)
----------------
This should be replaced with FTP->getArgType(0)->getAs<PointerType>().

================
Comment at: lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp:593-594
@@ -592,4 +592,4 @@
 
   const FunctionProtoType *FTP
     = dyn_cast<FunctionProtoType>(FD->getType().IgnoreParens());
   if (!FTP)
----------------
This should use getAs too.

================
Comment at: lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp:571
@@ -570,3 +570,3 @@
     // Verify that the arguments are pointers.
     const PointerType *PT = dyn_cast<PointerType>(FPT->getArgType(i));
     if (!PT)
----------------
And this.

================
Comment at: lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp:559
@@ -558,3 +558,3 @@
   const FunctionProtoType *FPT
     = dyn_cast<FunctionProtoType>(FD->getType().IgnoreParens());
   if (!FPT)
----------------
And this.

================
Comment at: lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp:394
@@ -393,3 +393,3 @@
   // Verify that the argument is Pointer Type.
   const PointerType *PT = dyn_cast<PointerType>(FPT->getArgType(0));
   if (!PT)
----------------
OK, someone needs to review this entire file for this bug.

================
Comment at: tools/libclang/CIndex.cpp:1550-1553
@@ +1549,6 @@
+bool CursorVisitor::VisitDecayedTypeLoc(DecayedTypeLoc TL) {
+  if (Visit(TL.getOriginalLoc()))
+    return true;
+
+  return false;
+}
----------------
'return Visit(TL.getOriginalLoc());' ?


http://llvm-reviews.chandlerc.com/D1014



More information about the cfe-commits mailing list