aaron.ballman added inline comments.
================ Comment at: include/clang/Basic/DiagnosticSemaKinds.td:7922-7925 +def err_type_tag_out_of_range : Error< + "type tag index %0 is greater than the number of arguments specified">; +def err_arg_tag_out_of_range : Error< + "argument tag index %0 is greater than the number of arguments specified">; ---------------- These should be combined into a single diagnostic: `%select{type|argument}0 tag index %1 is greater than the number of arguments specified` ================ Comment at: include/clang/Sema/Sema.h:10458 void CheckArgumentWithTypeTag(const ArgumentWithTypeTagAttr *Attr, - const Expr * const *ExprArgs); + const ArrayRef<const Expr*> ExprArgs, + SourceLocation CallSiteLoc); ---------------- `const Expr *` (space before the `*`). Same below in the definition. ================ Comment at: lib/Sema/SemaChecking.cpp:12345 const Expr *TypeTagExpr = ExprArgs[Attr->getTypeTagIdx()]; + bool FoundWrongKind; ---------------- Spurious newline? ================ Comment at: lib/Sema/SemaChecking.cpp:12366 const Expr *ArgumentExpr = ExprArgs[Attr->getArgumentIdx()]; + if (IsPointerAttr) { ---------------- Spurious newline? ================ Comment at: test/Sema/error-type-safety.cpp:3-4 + +static const int test_void + __attribute__((type_tag_for_datatype(test, void))) = 0; + ---------------- Is this declaration necessary? ================ Comment at: test/Sema/error-type-safety.cpp:16 + test_invalid_arg_index(0); // expected-error {{argument tag index 99 is greater than the number of arguments specified}} +} ---------------- Can you also add a test for a boundary case so we can be sure there are no off-by-one errors introduced later? https://reviews.llvm.org/D40574 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits