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 cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits