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