aaron.ballman added a comment.
Thank you for the context and explanation! I don't feel like I have enough
familiarity with this machinery to be an effective reviewer for signing off on
anything. I've commented with some nits, but @rsmith should give the sign-off
on this one.
================
Comment at: include/clang/AST/PrettyPrinter.h:229
+
+ /// When true, prints deduced type for auto typed variables if their type has
+ /// been deduced. When false, always prints auto(or other variants e.g.
----------------
prints deduced -> prints the deduced
================
Comment at: include/clang/AST/PrettyPrinter.h:230
+ /// When true, prints deduced type for auto typed variables if their type has
+ /// been deduced. When false, always prints auto(or other variants e.g.
+ /// decltype(auto)) even if type was deduced.
----------------
Add a whitespace before the left paren.
================
Comment at: include/clang/Sema/Sema.h:7063
- QualType deduceVarTypeFromInitializer(VarDecl *VDecl, DeclarationName Name,
- QualType Type, TypeSourceInfo *TSI,
- SourceRange Range, bool DirectInit,
- Expr *Init);
+ std::pair<QualType, TypeSourceInfo *>
+ deduceVarTypeFromInitializer(VarDecl *VDecl, DeclarationName Name,
----------------
I'd appreciate some comments as to why this is returning a pair and under what
circumstances the type information will be different, because it's not likely
to be obvious to anyone reading this header.
================
Comment at: lib/Frontend/ASTConsumers.cpp:92
PrintingPolicy Policy(D->getASTContext().getLangOpts());
+ // Since ASTPrinter is used for pretty printing and auto is generally
+ // prettier than real type itself, we'll choose to print auto always.
----------------
Since -> Because
That said, I am not convinced I agree with this being the default. "Prettier"
is pretty subjective, and I'd rather our default be for clarity instead of
brevity, at least here.
================
Comment at: lib/Sema/SemaChecking.cpp:852
- if (RT->getPointeeType().getAddressSpace() != LangAS::opencl_generic) {
- S.Diag(Call->getArg(0)->getBeginLoc(),
----------------
kadircet wrote:
> aaron.ballman wrote:
> > I'm not certain I understand why this code has been removed?
> It shouldn't have been, tried rebasing but it didn't go away. I think it was
> deleted at some point by a different change and somehow ended up showing in
> here as well. (Tried to revert, got an error stating
> warn_opencl_generic_address_space_arg doesn't exist)
That's truly strange. I bring it up because these sort of changes make it
harder for reviewers to test changes locally by applying the patch themselves.
================
Comment at: lib/Sema/SemaDecl.cpp:10912
Expr *Init) {
- QualType DeducedType = deduceVarTypeFromInitializer(
+ auto DeducedTypeAndTSI = deduceVarTypeFromInitializer(
VDecl, VDecl->getDeclName(), VDecl->getType(),
VDecl->getTypeSourceInfo(),
----------------
Don't use `auto` as the type is not spelled out in the initialization.
================
Comment at: lib/Sema/SemaStmt.cpp:1888
if (First) {
- QualType FirstType;
+ TypeSourceInfo* FirstTypeTSI;
if (DeclStmt *DS = dyn_cast<DeclStmt>(First)) {
----------------
Formatting is off here (the `*` should bind to the right).
================
Comment at: lib/Sema/SemaStmt.cpp:1975
// AddInitializerToDecl, so we can produce a more suitable diagnostic.
- QualType InitType;
+ TypeSourceInfo* InitTypeTSI = nullptr;
if ((!isa<InitListExpr>(Init) && Init->getType()->isVoidType()) ||
----------------
Formatting.
================
Comment at: lib/Sema/SemaStmt.cpp:3442
// argument deduction.
- DeduceAutoResult DAR = DeduceAutoType(OrigResultType, RetExpr, Deduced);
+ TypeSourceInfo* DeducedTSI = nullptr;
+ DeduceAutoResult DAR = DeduceAutoType(OrigResultType, RetExpr, DeducedTSI);
----------------
Same -- you should probably run the patch through clang-format
(https://clang.llvm.org/docs/ClangFormat.html#script-for-patch-reformatting).
================
Comment at: lib/Sema/SemaTemplateDeduction.cpp:4464
.Apply(Type);
- assert(!FuncParam.isNull() &&
+ assert(!FuncParamTSI->getType().isNull() &&
"substituting template parameter for 'auto' failed");
----------------
Null check?
Repository:
rC Clang
https://reviews.llvm.org/D52301
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
