r335084 - Append new attributes to the end of an AttributeList.

[Michael Kruse via cfe-commits]

2018-06-25 14:20 GMT-05:00 Richard Smith <richard at metafoo.co.uk>:
> (I'm not sure what the problem is, but as a data point, Sema::checkCall
> iterates over the FormatAttrs in order, so it's possible that changing the
> order may have triggered a new warning. That may be due to a pre-existing
> order-dependence bug, or it may be that we're losing an attribute here.)

In this case the following patch should fix this.


diff --git a/lib/Sema/SemaChecking.cpp b/lib/Sema/SemaChecking.cpp
index 12a599b4f1..aeb7a83fa6 100644
--- a/lib/Sema/SemaChecking.cpp
+++ b/lib/Sema/SemaChecking.cpp
@@ -3172,7 +3172,8 @@ void Sema::checkCall(NamedDecl *FDecl, const
FunctionProtoType *Proto,
   // Printf and scanf checking.
   llvm::SmallBitVector CheckedVarArgs;
   if (FDecl) {
-    for (const auto *I : FDecl->specific_attrs<FormatAttr>()) {
+    SmallVector<FormatAttr*,2>
FormatAttrs(FDecl->specific_attrs<FormatAttr>());
+    for (const auto *I : llvm::reverse(FormatAttrs)) {
       // Only create vector if there are format attributes.
       CheckedVarArgs.resize(Args.size());


 However, if I understand __attribute__((format(...))) correctly,
there should be just one FormatAttr.


>> > Please let me know your plan. (If I don't hear back in a day or so, I'll
>> > go
>> > ahead and revert for you as the safe default course of action.)
>>
>> On a weekend?
>
>
> Yes; our policy is generally to revert to green if a patch causes
> regressions that aren't going to be fixed in a few hours. This is generally
> good for the committer, because it lets you figure out what's wrong and deal
> with it on your own schedule rather than being pressured to fix it urgently
> because you're blocking the work of others.

Should I revert the commit then?

Michael