================
Comment at: lib/AST/Expr.cpp:501
@@ -500,3 +500,3 @@
        Decl = Pattern;
      const FunctionType *AFT = Decl->getType()->getAs<FunctionType>();
      const FunctionProtoType *FT = nullptr;
----------------
The best fix (for the immediate issue) would probably be to use the declared type rather 
than the actual type: use `Decl->getTypeSourceInfo()->getType()` rather than 
`Decl->getType()` here.

As Reid points out, we may want to just stash the string into the 
`PredefinedExpr` object when we create it, rather than recomputing it each time.
Richard, I'll rework the patch and store string in PredefinedExpr.
Comment at: test/CodeGenCXX/predefined-expr-cxx14.cpp:3-4
@@ +2,4 @@
+
+// CHECK-DAG: private unnamed_addr constant [15 x i8] c"externFunction\00"
+// CHECK-DAG: private unnamed_addr constant [26 x i8] c"auto 
NS::externFunction()\00"
+// CHECK-DAG: private unnamed_addr constant [49 x i8] c"auto 
functionTemplateExplicitSpecialization(int)\00"
----------------
This test does not seem quite right:

   1) Testing all the different kinds of function declarations here is gaining 
you zero test coverage. The thing that's relevant here is how we handle the 
return type, and that does not change between these cases.

   2) You're missing test coverage for the cases you fixed: `decltype(auto)` as 
a return type, and types that contain `auto` (but are not *exactly* `auto`).

http://reviews.llvm.org/D5365
The test is too big, but it has test cases for , I'll reduce it.

Best regards,
Alexey Bataev
=============
Software Engineer
Intel Compiler Team

16.09.2014 22:48, Richard Smith пишет:
================
Comment at: lib/AST/Expr.cpp:501
@@ -500,3 +500,3 @@
        Decl = Pattern;
      const FunctionType *AFT = Decl->getType()->getAs<FunctionType>();
      const FunctionProtoType *FT = nullptr;
----------------
The best fix (for the immediate issue) would probably be to use the declared type rather 
than the actual type: use `Decl->getTypeSourceInfo()->getType()` rather than 
`Decl->getType()` here.

As Reid points out, we may want to just stash the string into the 
`PredefinedExpr` object when we create it, rather than recomputing it each time.

================
Comment at: lib/AST/Expr.cpp:606
@@ -604,3 +605,3 @@
           cast<CXXMethodDecl>(FD)->getParent()->isLambda()) ||
          (FT && FT->getReturnType()->getAs<AutoType>()))
        Proto = "auto " + Proto;
----------------
Remove the existing (broken) handling for this case.

================
Comment at: test/CodeGenCXX/predefined-expr-cxx14.cpp:3-4
@@ +2,4 @@
+
+// CHECK-DAG: private unnamed_addr constant [15 x i8] c"externFunction\00"
+// CHECK-DAG: private unnamed_addr constant [26 x i8] c"auto 
NS::externFunction()\00"
+// CHECK-DAG: private unnamed_addr constant [49 x i8] c"auto 
functionTemplateExplicitSpecialization(int)\00"
----------------
This test does not seem quite right:

   1) Testing all the different kinds of function declarations here is gaining 
you zero test coverage. The thing that's relevant here is how we handle the 
return type, and that does not change between these cases.

   2) You're missing test coverage for the cases you fixed: `decltype(auto)` as 
a return type, and types that contain `auto` (but are not *exactly* `auto`).

http://reviews.llvm.org/D5365




_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to