v.g.vassilev added a comment.
Thanks for working on this! I left some comments. I did not look very deep in
the patch but it seems quite consistent to the rest of the code in Sema.
Can you provide some performance numbers for this patch. I believe that the
less confident reviewers will be more comfortable letting that patch in. I
think this work is important and should be merged before GSoC ends. It will
allow clang to diagnose better errors in template instantiations even for cases
such as llvm.org/PR12853
================
Comment at: clang/lib/AST/ASTContext.cpp:12129
+ return cast_or_null<T>(
+ getCommonDecl(const_cast<Decl *>(cast_or_null<Decl>(X)),
+ const_cast<Decl *>(cast_or_null<Decl>(Y))));
----------------
Maybe `getCommonDecl` should take `const Decl*` instead of `const_cast`-ing
here.
================
Comment at: clang/lib/AST/ASTContext.cpp:12172
+ return TemplateArgument(
+ Ctx.getCommonSugaredType(X.getNullPtrType(), Y.getNullPtrType()),
true);
+ default:
----------------
================
Comment at: clang/lib/AST/ASTContext.cpp:12408
+ case Type::BlockPointer: {
+ const auto *PX = cast<BlockPointerType>(X), *PY =
cast<BlockPointerType>(Y);
+ return Ctx.getBlockPointerType(getCommonPointeeType(Ctx, PX, PY));
----------------
Defining decl groups does not seem common in the llvm/clang codebase. I don't
think we have a specific rule discouraging that. However, I think stepping
through with a debugger will be harder. Maybe make this and other occurrences
more consistent to the rest of the codebase?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D111283/new/
https://reviews.llvm.org/D111283
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
