aaron.ballman added inline comments.

================
Comment at: clang/include/clang/Basic/Attr.td:1570
+def PassDynamicObjectSize : InheritableParamAttr {
+  let Spellings = [Clang<"pass_dynamic_object_size">];
+  let Args = [IntArgument<"Type">];
----------------
Why use a separate attribute as opposed to a separate spelling and an accessor 
on `PassObjectSizeAttr` to ask whether it's dynamic or not? The two attributes 
seem to have identical semantics aside from which builtin is called, so I think 
it makes sense to use the same semantic attribute type.


================
Comment at: clang/lib/Sema/SemaDeclAttr.cpp:1108
+static void handlePassObjectSizeAttr(Sema &S, Decl *D, const ParsedAttr &AL,
+                                     bool IsDynamic) {
+  assert(isa<ParmVarDecl>(D) && "Should be checked already!");
----------------
Please do not add a parameter here -- we keep all of the `handleFooAttr()` 
function signatures the same because some day we'd like to refactor the switch 
to be more tablegenerated as well.


================
Comment at: clang/lib/Sema/SemaDeclAttr.cpp:1140
 
-  D->addAttr(::new (S.Context) PassObjectSizeAttr(
-      AL.getRange(), S.Context, (int)Type, 
AL.getAttributeSpellingListIndex()));
+  if (IsDynamic) {
+    D->addAttr(::new (S.Context) PassDynamicObjectSizeAttr(
----------------
Elide braces here and below.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58757/new/

https://reviews.llvm.org/D58757



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to