> ================ > 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 > > http://reviews.llvm.org/D5365 _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
