mizvekov added inline comments.
================
Comment at: clang/lib/AST/ASTContext.cpp:5446
// Build a new, canonical decltype(expr) type.
Canon = new (*this, TypeAlignment) DependentDecltypeType(*this, e);
DependentDecltypeTypes.InsertNode(Canon, InsertPos);
----------------
rsmith wrote:
> If we don't track the extra parens here too, we can end up giving the same
> canonical type to dependent `decltype` types with and without implicit
> parens, even though they can instantiate to different types. (But I think the
> simplest way to handle this would be to include the parens in the expression;
> see other comment.)
Good catch, but yeah I'll see about the other solution.
================
Comment at: clang/lib/Sema/SemaExprCXX.cpp:8643
QualType MatchedType =
- BuildDecltypeType(E, E->getBeginLoc()).getCanonicalType();
+ BuildDecltypeType(E, E->getBeginLoc(), true, true).getCanonicalType();
llvm::SmallVector<TemplateArgument, 1> Args;
----------------
rsmith wrote:
> Instead of adding complexity to the type system to deal with this special
> case, can you directly create a `ParenExpr` here?
Hmm I thought about that. Since I am a bit unfamiliar with the code there, I
was not sure it was going to end up being more or less complicated than the
proposed solution. But I'll give it a shot if that looks simpler to you.
================
Comment at:
clang/test/CXX/expr/expr.prim/expr.prim.req/compound-requirement.cpp:97
template<typename T>
-concept Large = sizeof(T) >= 4; // expected-note{{because 'sizeof(short) >= 4'
(2 >= 4) evaluated to false}}
+concept LargeRef = sizeof(typename reference<T>::type) >= 4;
+// expected-note@-1{{because 'sizeof(typename reference<short &>::type) >= 4'
(2 >= 4) evaluated to false}}
----------------
Quuxplusone wrote:
> I think this is too large of a change. How about just keeping the old test
> code but changing it to
> ```
> template<typename T>
> concept Large = sizeof(std::decay_t<T>) >= 4; // expected-note{{because...
> etc}}
> ```
Yeah I thought about that, but on the other hand changing the test to make it a
bit more strict did not look like a problem to me.
Hmm, we cover this case in the test below anyway, might as well reduce the
amount of changes I guess.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D98160/new/
https://reviews.llvm.org/D98160
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
