[PATCH] Refactor attribute arguments
aaron at aaronballman.com
Mon Jul 22 14:37:51 PDT 2013
My testing showed it to be the case -- when using the index returned
from checkFunctionOrMethodArgumentIndex, it was always off by one or
two depending on context. Specifically:
const char* g2(const char*) __attribute__((format_arg(2)));
checkFunctionOrMethodIndex would return index 0 because it cares about
the const char * -- adding 1 to it when passing in the argument to
addAttr (to make it one based instead of zero based) was not
sufficient; I would get crashes in other parts of the system for that
test case. Having it pass in two is consistent with the previous
On Mon, Jul 22, 2013 at 5:30 PM, Eli Friedman <eli.friedman at gmail.com> wrote:
> On Mon, Jul 22, 2013 at 11:13 AM, Aaron Ballman <aaron at aaronballman.com> wrote:
>> And then I promptly attached the wrong patch. Sorry. Correct patch
>> attached here.
>> On Mon, Jul 22, 2013 at 2:09 PM, Aaron Ballman <aaron at aaronballman.com> wrote:
>>> There was already a helper method named
>>> checkFunctionOrMethodArgumentIndex whcih would perform semantic
>>> checking for attributes. This patch refactors alloc_size, nonnull,
>>> format_args and the ownership attributes to use the helper method
>>> instead of the copy and pasted code.
>>> No functional changes intended, but there are some new test cases to
>>> test coverage.
> @@ -3082,8 +2964,14 @@
> + // We cannot use the ArgIdx returned from checkFunctionOrMethodArgumentIndex
> + // because that has corrected for the implicit this parameter, and is zero-
> + // based. The attribute expects what the user wrote explicitly.
> + llvm::APSInt Val;
> + IdxExpr->EvaluateAsInt(Val, S.Context);
> D->addAttr(::new (S.Context)
> - FormatArgAttr(Attr.getRange(), S.Context, Idx.getZExtValue(),
> + FormatArgAttr(Attr.getRange(), S.Context, Val.getZExtValue(),
> Are you sure? We have tests in test/SemaCXX/attr-format.cpp which
> seem to say this is wrong.
More information about the cfe-commits