[PATCH] Fix for bug 17427 - Assertion failed: "Computed __func__ length differs from type!"

[Richard Smith]

================
Comment at: include/clang/AST/Expr.h:1164
@@ -1163,3 +1163,1 @@
 
-/// PredefinedExpr - [C99 6.4.2.2] - A predefined identifier such as __func__.
-class PredefinedExpr : public Expr {
----------------
Please don't move this class to a different place in the file (and likewise in Expr.cpp); it creates history churn and makes the patch hard to review. Instead, you can move the `getStringLiteral` method definitions out of the class (either to after the definition of `StringLiteral` or into the .cpp file).

================
Comment at: include/clang/AST/Expr.h:1577
@@ +1576,3 @@
+public:
+  PredefinedExpr(SourceLocation L, QualType T, IdentType IT, StringLiteral *SL)
+    : Expr(PredefinedExprClass, T, VK_LValue, OK_Ordinary,
----------------
I don't think you need to pass in `T` here; it can be obtained from the `StringLiteral`.

================
Comment at: include/clang/AST/Expr.h:1598
@@ +1597,3 @@
+  StringLiteral *getStringLiteral() { return cast<StringLiteral>(StringLit); }
+  void setStringLiteral(StringLiteral *SL) { StringLit = SL; }
+
----------------
Generally, we prefer to make the `ASTReader` a friend of the class being deserialized rather than adding setters. Please do that here (you should also be able to remove the other setters in the process).

================
Comment at: lib/AST/ExprConstant.cpp:2654
@@ -2651,3 +2653,3 @@
       return extractSubobject(Info, Conv, LitObj, LVal.Designator, RVal);
-    } else if (isa<StringLiteral>(Base)) {
+    } else if (isa<StringLiteral>(Base) || isa<PredefinedExpr>(Base)) {
       // We represent a string literal array as an lvalue pointing at the
----------------
C++ guarantees that `__func__` has the same address for all evaluations within the same function. It looks like this patch will make them all be treated as distinct objects.

================
Comment at: lib/AST/StmtProfile.cpp:472
@@ -471,2 +471,3 @@
   ID.AddInteger(S->getIdentType());
+  VisitStringLiteral(S->getStringLiteral());
 }
----------------
This doesn't seem necessary.

================
Comment at: lib/CodeGen/CGExpr.cpp:2057
@@ -2134,1 +2056,3 @@
+         "No string literal is associated with PredefinedExpr");
+  return EmitStringLiteralLValue(E->getStringLiteral());
 }
----------------
I think we need to emit this with a specific mangled name in order to merge the __func__ objects across translation units if it's used in an inline function.

================
Comment at: lib/Sema/TreeTransform.h:6967
@@ -6966,3 +6966,3 @@
 TreeTransform<Derived>::TransformPredefinedExpr(PredefinedExpr *E) {
-  return E;
+  return getDerived().TransformPredefinedExpr(E);
 }
----------------
This is wrong: unless this is overridden by the derived class, it would just call itself. Instead you should add a RebuildPredefinedExpr function, call it from here, and make it call into Sema to rebuild the PredefinedExpr.

================
Comment at: test/CodeGenCXX/funcsig.cpp:11
@@ -10,3 +10,3 @@
 }
-// CHECK: private unnamed_addr constant [{{.*}} x i8] c"void __cdecl freeFunc(int *, char)\00"
+// CHECK: linkonce_odr unnamed_addr constant [{{.*}} x i8] c"void __cdecl freeFunc(int *, char)\00"
 
----------------
If this is supposed to be linkonce_odr, you should also check its mangled name.

http://reviews.llvm.org/D5365