Respond to @rsmith's comments.
================
Comment at: lib/Sema/SemaChecking.cpp:1411-1415
@@ +1410,7 @@
+static Qualifiers getPointeeQualifiers(Sema& S, Expr *ArgPtr) {
+ ArgPtr = S.DefaultFunctionArrayLvalueConversion(ArgPtr).get();
+ QualType Ty = ArgPtr->getType();
+ const PointerType *PointerTy = Ty->getAs<PointerType>();
+ return PointerTy ? PointerTy->getPointeeType().getQualifiers()
+ : Qualifiers();
+}
----------------
rsmith wrote:
> It's wasteful to build a converted expression up here and throw it away. What
> you want seems to be something like:
>
> QualType T = ArgPtr->getType();
> auto *PtrType = T->getAs<PointerType>();
> return PtrType ? PtrType->getPointeeType().getQualifiers() :
> T->getQualifiers();
I'll drop the "S.DefaultFunctionArrayLValueConversion" call and use
T->getQualifiers() when the type is not a pointer.
================
Comment at: lib/Sema/SemaChecking.cpp:1570-1577
@@ -1550,3 +1569,10 @@
ValType = AtomTy->getAs<AtomicType>()->getValueType();
+ } else if (Form != Load && Op != AtomicExpr::AO__atomic_load) {
+ if (ValType.isConstQualified()) {
+ Diag(DRE->getLocStart(), diag::err_atomic_op_needs_non_const_pointer)
+ << ValType.getUnqualifiedType() << Ptr->getType()
+ << Ptr->getSourceRange();
+ return ExprError();
+ }
}
----------------
rsmith wrote:
> This part LGTM; feel free to factor out this chunk and its tests, and commit
> it separately.
I'll keep it in this patch. Hopefully I can get this in soon.
================
Comment at: lib/Sema/SemaChecking.cpp:1682-1683
@@ +1681,4 @@
+ // as their third argument.
+ if (Op == AtomicExpr::AO__atomic_exchange ||
+ Op == AtomicExpr::AO__atomic_compare_exchange) {
+ Qualifiers ThirdParamQuals = getPointeeQualifiers(*this,
TheCall->getArg(2));
----------------
rsmith wrote:
> Please don't do this. The first half of this function is carefully
> classifying the different forms of atomic builtin so that we don't need to
> reason about the behavior of individual builtins down here.
>
> It'd be better to add a `bool PassInputsByValue = IsC11 || IsN;` in place of
> the existing `ByValType`, and use either `ValType` or some pointer type built
> from it depending on the value of `PassInputsByValue`.
Ack. Although your formulation for `PassInputsByValue` is not quite correct.
I'll update it with something similar.
================
Comment at: lib/Sema/SemaChecking.cpp:1689
@@ +1688,3 @@
+ if (Op == AtomicExpr::AO__atomic_compare_exchange
+ && ThirdParamQuals.hasConst())
+ Ty.addConst();
----------------
rsmith wrote:
> `&&` should go at the end of the line, not the start.
ack.
================
Comment at: lib/Sema/SemaChecking.cpp:1692-1693
@@ +1691,4 @@
+ Ty = Context.getPointerType(Ty);
+ }
+ else
+ Ty = ByValType;
----------------
rsmith wrote:
> `}` and `else` should go on the same line.
ack.
http://reviews.llvm.org/D10407
EMAIL PREFERENCES
http://reviews.llvm.org/settings/panel/emailpreferences/
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
