Fznamznon added inline comments.
================
Comment at: clang/lib/Sema/SemaCXXScopeSpec.cpp:134
"specifier in SFINAE context?");
- if (!hasReachableDefinition(PartialSpec))
+ if (PartialSpec->hasDefinition() &&
+ !hasReachableDefinition(PartialSpec))
----------------
Fznamznon wrote:
> shafik wrote:
> > So I see in another location we are basically checking `!isBeginDefined()`
> > and in another place `hasCompleteDefinition()`. It is not clear to me if
> > these are all basically checking the same thing or if we should figure out
> > what is the right thing to check and do that everywhere.
> I suppose you meant `isBeingDefined()`. So, IMO `isBeingDefined()` is not
> exactly the same as having a definition, this is set to `true` when
> `TagDecl::startDefinition` I suppose this should be done when parser meets a
> definition so at this point it can't be or have a complete definition. But at
> the places where `!isBeingDefined()` is checked prior
> `hasReachableDefinition` I saw a pointer representing a definition checked
> for not being null. Interestingly, `hasReachableDefinition()` early exits if
> `isBeingDefined()` returns true, so probably this check additional doesn't
> make sense at all.
> Maybe we should just move both checks on having a definition and not being in
> the process of definition under `hasReachableDefinition` after all. That will
> require changing unrelated places, so I would prefer doing this separately in
> any case.
>
> I can't grep `hasCompleteDefinition()` either, so I suppose you meant
> `isCompleteDefinition()`, I think this is basically the same thing as having
> definition and additionally the declaration through which the method called
> is checked that IT IS the definition. I'm not sure we have to require it here.
>
@shafik , are you satisfied with the explanation?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D148330/new/
https://reviews.llvm.org/D148330
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits