rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.
OK, a couple of trivial changes then I think this is fine to commit.
================
Comment at: include/clang/Basic/AttrDocs.td:351-355
@@ +350,7 @@
+
+* Only one use of ``pass_object_size`` is allowed per parameter.
+
+* It is an error to take the address of a function with ``pass_object_size`` on
+ any of its parameters. If you wish to do this, you can create an overload
+ without pass_object_size on any parameters.
+
----------------
Not done? =)
================
Comment at: lib/AST/ExprConstant.cpp:6507-6509
@@ -6506,5 +6544,1 @@
- // handle all cases where the expression has side-effects.
- // Likewise, if Type is 3, we must handle this because CodeGen cannot give
a
- // conservatively correct answer in that case.
- if (E->getArg(0)->HasSideEffects(Info.Ctx) || Type == 3)
return Success((Type & 2) ? 0 : -1, E);
----------------
I'd view the GCC documentation as more what you'd call "guidelines" than actual
rules. The key part here is that side-effects in the operand of
`__builtin_object_size` do not occur; I think the fact that GCC always returns
`-1` or `0` if there are side-effects is an implementation detail of GCC's
approach rather than a formal part of the design, and we should not be required
to treat `__builtin_object_size` as a constant if it's applied to an expression
with side-effects. If CodeGen can compute the object size, it should be
permitted to do so even if side-effects are present, so long as those
side-effects do not actually occur.
However, I think this change is doing enough already, so let's go ahead with
things as they are; we can consider changing this at a later date if we are so
inclined.
================
Comment at: lib/AST/MicrosoftMangle.cpp:1844-1857
@@ -1843,4 +1843,16 @@
// Happens for function pointer type arguments for example.
- for (const QualType &Arg : Proto->param_types())
- mangleArgumentType(Arg, Range);
+ for (unsigned I = 0, E = Proto->getNumParams(); I != E; ++I) {
+ mangleArgumentType(Proto->getParamType(I), Range);
+ // Mangle each pass_object_size parameter as if it's a paramater of enum
+ // type passed directly after the parameter with the pass_object_size
+ // attribute. The aforementioned enum's name is __pass_object_size, and
we
+ // pretend it resides in a top-level namespace called __ext.
+ //
+ // FIXME: Is there a defined extension notation for the MS ABI, or is it
+ // necessary to just cross our fingers and hope this type+namespace
+ // combination doesn't conflict with anything?
+ if (D)
+ if (auto *P = D->getParamDecl(I)->getAttr<PassObjectSizeAttr>())
+ Out << "W4__pass_object_size" << P->getType() << "@__ext@@";
+ }
// <builtin-type> ::= Z # ellipsis
----------------
majnemer wrote:
> This seems fine to me, I'd change the namespace to something like `__clang`
> though.
Yes, `__clang` would make more sense to me too.
http://reviews.llvm.org/D13263
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
