It may be that we could even generalize the fix-it along with my set of patches. If something expects a string or identifier, we know about it in the tablegen. So we could probably automate fixits where the user has provided a string literal instead of an identifier, or vice versa. I'll keep that in the back of my mind as I move forward.
~Aaron On Mon, Sep 9, 2013 at 5:39 PM, Benjamin Kramer <[email protected]> wrote: > > On 09.09.2013, at 23:19, Jordan Rose <[email protected]> wrote: > >> Is it possible to get a fix-it here? Or is that best saved for after Aaron's >> refactoring? > > Adding a fixit would make some refactoring of the code necessary that > collides with Aaron's patches, so I'd rather postpone it until the patches > are committed. Can you file a bug so we don't forget about it? > > - Ben >> >> >> On Sep 9, 2013, at 8:08 , Benjamin Kramer <[email protected]> wrote: >> >>> Author: d0k >>> Date: Mon Sep 9 10:08:57 2013 >>> New Revision: 190312 >>> >>> URL: http://llvm.org/viewvc/llvm-project?rev=190312&view=rev >>> Log: >>> Sema: Don't crash on visibility attributes with an identifier argument. >>> >>> PR17105. >>> >>> Modified: >>> cfe/trunk/lib/Sema/SemaDeclAttr.cpp >>> cfe/trunk/test/Sema/attr-visibility.c >>> >>> Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp >>> URL: >>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=190312&r1=190311&r2=190312&view=diff >>> ============================================================================== >>> --- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original) >>> +++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Mon Sep 9 10:08:57 2013 >>> @@ -2438,9 +2438,10 @@ static void handleVisibilityAttr(Sema &S >>> if (!checkAttributeNumArgs(S, Attr, 1)) >>> return; >>> >>> - Expr *Arg = Attr.getArgAsExpr(0); >>> - Arg = Arg->IgnoreParenCasts(); >>> - StringLiteral *Str = dyn_cast<StringLiteral>(Arg); >>> + // Check that the argument is a string literal. >>> + StringLiteral *Str = 0; >>> + if (Attr.isArgExpr(0)) >>> + Str = >>> dyn_cast<StringLiteral>(Attr.getArgAsExpr(0)->IgnoreParenCasts()); >>> >>> if (!Str || !Str->isAscii()) { >>> S.Diag(Attr.getLoc(), diag::err_attribute_argument_type) >>> >>> Modified: cfe/trunk/test/Sema/attr-visibility.c >>> URL: >>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/attr-visibility.c?rev=190312&r1=190311&r2=190312&view=diff >>> ============================================================================== >>> --- cfe/trunk/test/Sema/attr-visibility.c (original) >>> +++ cfe/trunk/test/Sema/attr-visibility.c Mon Sep 9 10:08:57 2013 >>> @@ -24,3 +24,5 @@ extern int test7 __attribute__((visibili >>> typedef int __attribute__((visibility("default"))) bar; // expected-warning >>> {{'visibility' attribute ignored}} >>> >>> int x __attribute__((type_visibility("default"))); // expected-error >>> {{'type_visibility' attribute only applies to types and namespaces}} >>> + >>> +int PR17105 __attribute__((visibility(hidden))); // expected-error >>> {{'visibility' attribute requires a string}} >>> >>> >>> _______________________________________________ >>> cfe-commits mailing list >>> [email protected] >>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >> > > > _______________________________________________ > cfe-commits mailing list > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
