tahonermann added a comment.
> I am bit afraid about release builds:
>
> if (!Initializer)
>
> return;
>
> Is this semantically incorrect?
I think so. And now I think I see a path where the proposed `assert` is
incorrect as well. The `else` branches at lines 5919, 5921, and 5923 appear to
handle cases where `Initializer` may be null.
clang/lib/Sema/SemaInit.cpp:
5824 // Handle default initialization.
5825 if (Kind.getKind() == InitializationKind::IK_Default) {
5826 TryDefaultInitialization(S, Entity, Kind, *this);
5827 return;
5828 }
<Proposed assertion>
....
5835 if (const ArrayType *DestAT = Context.getAsArrayType(DestType)) {
5836 if (Initializer && isa<VariableArrayType>(DestAT)) {
....
5912 else if (S.getLangOpts().CPlusPlus &&
5913 Entity.getKind() == InitializedEntity::EK_Member &&
5914 Initializer && isa<InitListExpr>(Initializer)) {
....
5918 } else if (DestAT->getElementType()->isCharType())
5919 SetFailed(FK_ArrayNeedsInitListOrStringLiteral);
5920 else if (IsWideCharCompatible(DestAT->getElementType(), Context))
5921 SetFailed(FK_ArrayNeedsInitListOrWideStringLiteral);
5922 else
5923 SetFailed(FK_ArrayNeedsInitList);
5924
5925 return;
5926 }
Perhaps the` assert` should be added after line 5926 above. I would be
concerned about adding a return statement conditional on `Initializer` being
null there though. If the intention is that an initializer must be present
after that point, an early return could result in a miscompile; I'd rather have
a crash.
I think the checks for `Initializer` being non-null following the addition of
an `assert` should be removed.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D139148/new/
https://reviews.llvm.org/D139148
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
