aaron.ballman added a subscriber: Endill.
aaron.ballman added inline comments.
================
Comment at: clang/lib/Sema/SemaTemplateDeduction.cpp:5022
+ SourceLocation Loc) {
+ assert(FD->isImmediateEscalating());
+
----------------
cor3ntin wrote:
> aaron.ballman wrote:
> > I think I'm a bit surprised to see this assert in a function named
> > `CheckIf` -- I would assume that we'd return false in this case?
> I think we could but checking whether a non-escalating function is immediate
> does not make sense to me
Okay, that seems reasonable enough.
================
Comment at: clang/lib/Serialization/ASTWriterStmt.cpp:629
+ !E->refersToEnclosingVariableOrCapture() && !E->isNonOdrUse() &&
+ !E->isImmediateEscalating()) {
AbbrevToUse = Writer.getDeclRefExprAbbrev();
----------------
cor3ntin wrote:
> aaron.ballman wrote:
> > Can you explain why this checks that the expression is not immediate
> > escalating? (What test case exercises this?)
> Not entirely. My understanding is that we can only use an abbreviation when
> it's just a reference to a Decl with no additional non-default property,
> otherwise it needs to be fully serialized (otherwise the "immediate
> escalating" bit would be lost and deserIAlizing incorrect
Ahhhh, okay, that makes sense to me. I was thrown off because it seemed like
this was related to ODR captures in lambdas somehow, but I see what you mean
now that I look at how other abbreviations are handled. Thanks!
================
Comment at: clang/test/SemaCXX/cxx2b-consteval-propagate.cpp:8
+
+namespace examples {
+
----------------
cor3ntin wrote:
> aaron.ballman wrote:
> > Fznamznon wrote:
> > > cor3ntin wrote:
> > > > Fznamznon wrote:
> > > > > These examples exactly match the ones provided by P2564R3, should
> > > > > they be in a separate test in `CXX` directory then?
> > > > I don't have a string preference, should we move the paper examples?
> > > > the whole file?
> > > I meant the paper examples. I don't have a strong preference too, so in
> > > case it doesn't matter where the test actually is, please ignore this
> > > comment.
> > Because it's voted in as a DR, we should have a test in
> > `clang/test/CXX/drs/` with the appropriate DR number markings (and then
> > regenerate the DR status page as well).
> I'm not sure core makes dr numbers for papers
Yeah, I think you're right... I can't find a DR number for this either.
@Endill -- something to be aware of for DR conformance testing, I guess.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D151094/new/
https://reviews.llvm.org/D151094
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
