hubert.reinterpretcast added a comment.
I think we're good after some last updates.
================
Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:340-342
+ // Ignore all attributes except ConstInit when IgnoreAttrs is true.
+ bool isEmittedWithConstantInitializer(const VarDecl *VD,
+ bool IgnoreAttrs = false) const {
----------------
See suggested change for renaming the parameter name from `IgnoreAttrs` to
`InspectInitForWeakDef`.
================
Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:349
// the variable is weak, such examination would not be correct.
- if (VD->isWeak() || VD->hasAttr<SelectAnyAttr>())
+ if (!IgnoreAttrs && (VD->isWeak() || VD->hasAttr<SelectAnyAttr>()))
return false;
----------------
================
Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:2957-2958
+ // also when the symbol is weak.
+ if (CGM.getTriple().isOSAIX() &&
+ isEmittedWithConstantInitializer(VD, true) &&
+ !VD->needsDestruction(getContext())) {
----------------
The block here is meant to be entered only if there is a definition of the
variable (even if weak) in the current translation unit. This is mostly
enforced with `isEmittedWithConstantInitializer`, but that will return true for
`constinit` even if only a declaration is available.
================
Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:2971
+ isEmittedWithConstantInitializer(VD, true) &&
+ !VD->needsDestruction(getContext())) {
+ // Emit a weak global function referring to the initialization function.
----------------
jamieschmeiser wrote:
> hubert.reinterpretcast wrote:
> > Unfortunately `needsDestruction` cannot be counted on at this time for some
> > incomplete types, see https://llvm.org/pr51079.
> I think it is okay to leave the code as-is as it will then be fixed when that
> problem is fixed.
Yes: I think (if the code works correctly), the `needsDestruction` query here
only becomes significant when the type is complete -- and, in that case, we
want it to work the way it currently does in relation to classes.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D104420/new/
https://reviews.llvm.org/D104420
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
