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

Reply via email to