dblaikie added inline comments.
================
Comment at: clang/lib/Sema/SemaExprMember.cpp:519-521
+ if (const ElaboratedType *ET = dyn_cast<const ElaboratedType>(BaseType)) {
+ if (const TemplateSpecializationType *TST =
+ dyn_cast<TemplateSpecializationType>(ET->getNamedType())) {
----------------
I think you can skip the `const` in the first `dyn_cast` (think it works
implicitly?) & could use `const auto *` on the LHS of both these `if`s, since
the RHS makes it clear what the type is
================
Comment at: clang/lib/Sema/SemaExprMember.cpp:523-524
+ auto *TD = TST->getTemplateName().getAsTemplateDecl();
+ assert(isa<ClassTemplateDecl>(TD) &&
+ "template decl in member access is not ClassTemplateDecl");
+ for (FieldDecl *Field :
----------------
No need for the assert if you're immediately going to `cast` anyway, it'll
assert. Though perhaps the custom assert here gives you a chance to make it
more explicit that this is intentional.
================
Comment at: clang/lib/Sema/SemaExprMember.cpp:525-527
+ for (FieldDecl *Field :
+ cast<ClassTemplateDecl>(TD)->getTemplatedDecl()->fields()) {
+ if (Field->getDeclName() == NameInfo.getName()) {
----------------
This could be `llvm::find_if`, maybe? Not sure if it'd be tidier, but maybe
more legible
================
Comment at: clang/lib/Sema/SemaExprMember.cpp:1023-1025
+ if (MemberNameStr.find('@') != std::string::npos) {
+ return ExprEmpty();
+ }
----------------
Usually don't put `{}` on single-line scopes in the LLVM codebase.
================
Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:3292
+ std::string NewFieldName =
+ PackedField->getName().str() + "@" + std::to_string(Arg);
+ PackedField->setDeclName(&Context.Idents.get(NewFieldName));
----------------
cjdb wrote:
> Does LLVM have some form of `absl::StrCat` that we can use instead of
> `operator+`?
there's at least `llvm::Twine` which can avoid manifesting the intermediate
strings
================
Comment at: clang/lib/Sema/TreeTransform.h:4171
+ if (CXXDependentScopeMemberExpr *MemberExpr =
+ dyn_cast<CXXDependentScopeMemberExpr>(Pattern)) {
----------------
Might be worth pulling out this new code as a separate function - the
`continue` at the end is many lines away from the start or end of the loop,
making control flow a bit hard to follow (probably the existing code could do
with some of this massaging even before/regardless of the new code you're
adding here)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D158006/new/
https://reviews.llvm.org/D158006
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
