c-rhodes added a comment. @DavidTruby thanks for updating, just one last comment otherwise LGTM
================ Comment at: clang/lib/Sema/SemaChecking.cpp:13591-13595 + if (!Target->isVLSTBuiltinType() && !isa<VectorType>(OriginalTarget)) { + if (S.SourceMgr.isInSystemMacro(CC)) + return; + return DiagnoseImpCast(S, E, T, CC, diag::warn_impcast_vector_scalar); + } ---------------- can a test be added for this? ================ Comment at: clang/lib/Sema/SemaChecking.cpp:13582 + // Need the original target type for vector type checks + const Type *OriginalTarget = S.Context.getCanonicalType(T).getTypePtr(); + // Handle conversion from scalable to fixed when msve-vector-bits is ---------------- DavidTruby wrote: > c-rhodes wrote: > > this is the same as `Target` defined on line 13473 > The control flow in this function is a little odd, but it isn't necessarily > the same target; we could have entered one or more of the if statements above > that modify the target, and we need to inspect the original target here. > The control flow in this function is a little odd, but it isn't necessarily > the same target; we could have entered one or more of the if statements above > that modify the target, and we need to inspect the original target here. Ah ofc, I missed that, thanks for clarifying. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126380/new/ https://reviews.llvm.org/D126380 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits