Please use GitHub pull requests for new patches. Avoid migrating existing patches. Phabricator shutdown timeline
Changeset View
Standalone View
lib/Sema/SemaExpr.cpp
- This file is larger than 256 KB, so syntax highlighting is disabled by default.
| Show First 20 Lines • Show All 5,993 Lines • ▼ Show 20 Lines | Sema::ActOnCastExpr(Scope *S, SourceLocation LParenLoc, | ||||
| if (getLangOpts().CPlusPlus && !castType->isVoidType() && | if (getLangOpts().CPlusPlus && !castType->isVoidType() && | ||||
| !getSourceManager().isInSystemMacro(LParenLoc)) | !getSourceManager().isInSystemMacro(LParenLoc)) | ||||
| Diag(LParenLoc, diag::warn_old_style_cast) << CastExpr->getSourceRange(); | Diag(LParenLoc, diag::warn_old_style_cast) << CastExpr->getSourceRange(); | ||||
| CheckTollFreeBridgeCast(castType, CastExpr); | CheckTollFreeBridgeCast(castType, CastExpr); | ||||
| CheckObjCBridgeRelatedCast(castType, CastExpr); | CheckObjCBridgeRelatedCast(castType, CastExpr); | ||||
| DiscardMisalignedMemberAddress(castType.getTypePtr(), CastExpr); | |||||
| return BuildCStyleCastExpr(LParenLoc, castTInfo, RParenLoc, CastExpr); | return BuildCStyleCastExpr(LParenLoc, castTInfo, RParenLoc, CastExpr); | ||||
| } | } | ||||
| ExprResult Sema::BuildVectorLiteral(SourceLocation LParenLoc, | ExprResult Sema::BuildVectorLiteral(SourceLocation LParenLoc, | ||||
| SourceLocation RParenLoc, Expr *E, | SourceLocation RParenLoc, Expr *E, | ||||
| TypeSourceInfo *TInfo) { | TypeSourceInfo *TInfo) { | ||||
| assert((isa<ParenListExpr>(E) || isa<ParenExpr>(E)) && | assert((isa<ParenListExpr>(E) || isa<ParenExpr>(E)) && | ||||
| "Expected paren or paren list expression"); | "Expected paren or paren list expression"); | ||||
| ▲ Show 20 Lines • Show All 4,498 Lines • ▼ Show 20 Lines | QualType Sema::CheckAddressOfOperand(ExprResult &OrigOp, SourceLocation OpLoc) { | ||||
| // OpenCL v2.0 s6.12.5 - The unary operators & cannot be used with a block. | // OpenCL v2.0 s6.12.5 - The unary operators & cannot be used with a block. | ||||
| if (getLangOpts().OpenCL && OrigOp.get()->getType()->isBlockPointerType()) { | if (getLangOpts().OpenCL && OrigOp.get()->getType()->isBlockPointerType()) { | ||||
| Diag(OpLoc, diag::err_typecheck_unary_expr) << OrigOp.get()->getType() | Diag(OpLoc, diag::err_typecheck_unary_expr) << OrigOp.get()->getType() | ||||
| << op->getSourceRange(); | << op->getSourceRange(); | ||||
| return QualType(); | return QualType(); | ||||
| } | } | ||||
| CheckAddressOfPackedMember(op); | |||||
aaron.ballman: Missing a full stop at the end of the sentence. | |||||
| return Context.getPointerType(op->getType()); | return Context.getPointerType(op->getType()); | ||||
Can use const auto * here instead of repeating the type name. aaron.ballman: Can use `const auto *` here instead of repeating the type name. | |||||
The * binds to the identifier instead of the type; you should run the patch through clang-format (formatting issues are also in the test files). aaron.ballman: The * binds to the identifier instead of the type; you should run the patch through clang… | |||||
| } | } | ||||
| static void RecordModifiableNonNullParam(Sema &S, const Expr *Exp) { | static void RecordModifiableNonNullParam(Sema &S, const Expr *Exp) { | ||||
| const DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(Exp); | const DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(Exp); | ||||
| if (!DRE) | if (!DRE) | ||||
| return; | return; | ||||
| const Decl *D = DRE->getDecl(); | const Decl *D = DRE->getDecl(); | ||||
| if (!D) | if (!D) | ||||
Not Done ReplyInline ActionsI wonder if we should diagnose only when the resulting pointer is actually misaligned. For instance, the following code should be just fine: struct __attribute__((packed)) S {
int i;
};
void f(S s) {
int *ip = &s.i; // Why should this warn?
}Have you run this patch over any large code bases to see how high the false-positive rate is? How do you envision users silencing this diagnostic if they have a false-positive? Something like &(s.x) (since I you're not ignoring paren imp casts)? aaron.ballman: I wonder if we should diagnose only when the resulting pointer is actually misaligned. For… | |||||
Not Done ReplyInline ActionsI think it should warn because in this case &s == &s.i and the alignment of the whole struct is 1 (due to the packed attribute) so s.i is also aligned to 1 which would not be the suitable alignment for an int. I'm currently building Firefox, I'll add a comment once it ends. Regarding silencing the diagnostic: -Wno-address-of-packed-member should do but I understand it might be too coarse in some cases. rogfer01: I think it should warn because in this case &s == &s.i and the alignment of the whole struct is… | |||||
Ah, I forgot that the attribute also affected the alignment of the struct itself. Amending my false-positive idea, the following should be fine, shouldn't it?: struct __attribute__((packed)) S {
char c;
int i;
char c2;
};
void f(S s) {
char *cp = &s.c;
char *cp2 = &s.c2;
}I think perhaps we should not diagnose if the member's type also has a one-byte alignment (or, for instance, uses alignas(1) to achieve that). What do you think? aaron.ballman: Ah, I forgot that the attribute also affected the alignment of the struct itself. Amending my… | |||||
Yes. I like the idea since the packed attribute has no effect on already 1-aligned data (like char). But note that C++11 does not allow reducing the alignment through alignas. rogfer01: Yes. I like the idea since the packed attribute has no effect on already 1-aligned data (like… | |||||
Not Done ReplyInline ActionsHmm, okay, I was not remembering properly; I thought one of alignas, __declspec(align), or __attribute__((aligned)) allowed for decreasing the alignment, but documentation implies otherwise. aaron.ballman: Hmm, okay, I was not remembering properly; I thought one of `alignas`, `__declspec(align)`, or… | |||||
| return; | return; | ||||
| const ParmVarDecl *Param = dyn_cast<ParmVarDecl>(D); | const ParmVarDecl *Param = dyn_cast<ParmVarDecl>(D); | ||||
Comments should be complete sentences including capitalization and punctuation, so perhaps instead: // Packed does not have any effect. aaron.ballman: Comments should be complete sentences including capitalization and punctuation, so perhaps… | |||||
| if (!Param) | if (!Param) | ||||
| return; | return; | ||||
| if (const FunctionDecl* FD = dyn_cast<FunctionDecl>(Param->getDeclContext())) | if (const FunctionDecl* FD = dyn_cast<FunctionDecl>(Param->getDeclContext())) | ||||
| if (!FD->hasAttr<NonNullAttr>() && !Param->hasAttr<NonNullAttr>()) | if (!FD->hasAttr<NonNullAttr>() && !Param->hasAttr<NonNullAttr>()) | ||||
| return; | return; | ||||
| if (FunctionScopeInfo *FD = S.getCurFunction()) | if (FunctionScopeInfo *FD = S.getCurFunction()) | ||||
| if (!FD->ModifiedNonNullParams.count(Param)) | if (!FD->ModifiedNonNullParams.count(Param)) | ||||
| FD->ModifiedNonNullParams.insert(Param); | FD->ModifiedNonNullParams.insert(Param); | ||||
| } | } | ||||
I do not think this diagnostic should have a FixIt hint. This isn't actually *fixing* the problem, it is simply a way to silence the diagnostic while retaining the same behavior. aaron.ballman: I do not think this diagnostic should have a FixIt hint. This isn't actually *fixing* the… | |||||
Not Done ReplyInline ActionsOK. I will remove it. rogfer01: OK. I will remove it. | |||||
| /// CheckIndirectionOperand - Type check unary indirection (prefix '*'). | /// CheckIndirectionOperand - Type check unary indirection (prefix '*'). | ||||
| static QualType CheckIndirectionOperand(Sema &S, Expr *Op, ExprValueKind &VK, | static QualType CheckIndirectionOperand(Sema &S, Expr *Op, ExprValueKind &VK, | ||||
| SourceLocation OpLoc) { | SourceLocation OpLoc) { | ||||
| if (Op->isTypeDependent()) | if (Op->isTypeDependent()) | ||||
| return S.Context.DependentTy; | return S.Context.DependentTy; | ||||
| ExprResult ConvResult = S.UsualUnaryConversions(Op); | ExprResult ConvResult = S.UsualUnaryConversions(Op); | ||||
| ▲ Show 20 Lines • Show All 4,481 Lines • Show Last 20 Lines | |||||
Missing a full stop at the end of the sentence.