bolshakov-a added a comment. Btw, formatting of unrelated lines has leaked into `TemplateBase.h`. Sorry.
================ Comment at: clang/lib/AST/MicrosoftMangle.cpp:1624-1626 + return const_cast<ValueDecl *>( + V.getLValueBase().dyn_cast<const ValueDecl *>()); +} ---------------- aaron.ballman wrote: > Does this work or is the `const_cast` actually required? No, it doesn't compile, likewise the standard C++ `dynamic_cast` cannot remove constness. ================ Comment at: clang/lib/AST/TemplateBase.cpp:408-409 case Integral: - getAsIntegral().Profile(ID); getIntegralType().Profile(ID); + getAsIntegral().Profile(ID); + break; ---------------- aaron.ballman wrote: > Why did the order of these calls change? I don't know, it is from 9e08e51a20d0d2. I've tried to invert the order along with the order for `StructuralValue`, and all tests have been passed. ================ Comment at: clang/lib/Sema/SemaOverload.cpp:5983-5985 + Expr *E = Result.get(); + if (!isa<ConstantExpr>(E)) + E = ConstantExpr::Create(S.Context, Result.get(), Value); ---------------- aaron.ballman wrote: > I thought we could run into situations where we had a `ConstantExpr` but it > did not yet have its result stored in it. Should we assert that isn't the > case here? If I understand correctly, the sole place where `ConstantExpr` is constructed which may occur here is `BuildExpressionFromNonTypeTemplateArgumentValue` function, and a value is set into it there. Should I add the assertion into code? ================ Comment at: clang/test/SemaTemplate/temp_arg_nontype_cxx20.cpp:56 +using CF = ComplexFloat<1.0f + 3.0fi>; +using CF = ComplexFloat<3.0fi + 1.0f>; ---------------- aaron.ballman wrote: > bolshakov-a wrote: > > shafik wrote: > > > Can we add an enum example e.g.: > > > > > > ``` > > > enum E{ E1, E2}; > > > template <E> struct SE {}; > > > using IPE = SE<E1>; > > > using IPE = SE<E2>; > > > > > > ``` > > What for? Enumerators as non-type template arguments are allowed since > > C++98, AFAIK. And this test is about changes in C++20. > Sometimes we're lacking coverage for existing features, so when updating code > in the area, we'll sometimes ask for extra coverage just to be sure we're not > regressing something we think might not have a lot of existing test coverage. `temp_arg_nontype.cpp` test already has some `enum` cases. If a case with type alias should be added, it shoud be added there, not in the `temp_arg_nontype_cxx20.cpp`, I think. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140996/new/ https://reviews.llvm.org/D140996 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits