aaron.ballman added a reviewer: rsmith.
aaron.ballman added a subscriber: jfb.
aaron.ballman added inline comments.
================
Comment at: clang/lib/Lex/LiteralSupport.cpp:95-96
+ case '?':
+ case 'n':
+ case 't':
+ return true;
----------------
cor3ntin wrote:
> aaron.ballman wrote:
> > Do you intend to miss a bunch of escapes like `\'` and `\r` (etc)?
> \' is there. I am less sure about '\r' and '\a'. for example. This is
> something I realized after writing P2361.
> what does '\a` in static assert mean? even '\r' is not so obvious
Looking at the list again, I think only `\a` is really of interest here. I know
some folks like @jfb have mentioned that `\a` could be used to generate an
alert sound on a terminal, which is a somewhat useful feature for a failed
static assertion if you squint at it hard enough.
But the rest of the missing ones do seem more questionable to support.
================
Comment at: clang/lib/Parse/ParseDecl.cpp:422-423
- ExprResult ArgExpr(
- Actions.CorrectDelayedTyposInExpr(ParseAssignmentExpression()));
+ ExprResult ArgExpr(Actions.CorrectDelayedTyposInExpr(
+ ParseAttributeArgAsUnevaluatedLiteralOrExpression(AttrKind)));
if (ArgExpr.isInvalid()) {
----------------
cor3ntin wrote:
> aaron.ballman wrote:
> > Hmmm, I'm not certain about these changes.
> >
> > For some attributes, the standard currently requires accepting any kind of
> > string literal (like `[[deprecated]]`
> > https://eel.is/c++draft/dcl.attr.deprecated#1). P2361 is proposing to
> > change that, but it's not yet accepted by WG21 (let alone WG14). So giving
> > errors in those cases is a bit of a hard sell -- I think warnings would be
> > a bit more reasonable.
> >
> > But for other attributes (like `annotate`), it's a bit less clear whether
> > we should *prevent* literal prefixes because the attribute can be used to
> > have runtime impacts (for example, I can imagine someone using `annotate`
> > to emit the string literal bytes into the resulting binary). In some cases,
> > I think it's very reasonable (e.g., `diagnose_if` should behave the same as
> > `deprecated` and `nodiscard` because those are purely about generating
> > diagnostics at compile time).
> >
> > I kind of wonder whether we're going to want to tablegenerate whether the
> > argument needs to be parsed as unevaluated or not on an
> > attribute-by-attribute basis.
> Yep, I would not expect this to get merge before P2361 but I think the
> implementation experience is useful and raised a bunch of good questions.
> I don't think it ever makes sense to have `L` outside of literals - but
> people *might* do it currently, in which case there is a concern about
> whether it breaks code (I have found no evidence of that though).
>
> If we wanted to inject these strings in the binary - in some form, then we
> might have to transcode them at that point.
> I don't think the user would know if the string would be injected as wide or
> narrow (or something else) by the compiler.
>
> `L` is really is want to convert that string _at that point_. in an
> attribute, strings might have multiple usages so it's better to delay any
> transcoding.
> Does that make sense?
>
> But I agree that a survey of what each attribute expects is in order.
>
>
>
> Yep, I would not expect this to get merge before P2361 but I think the
> implementation experience is useful and raised a bunch of good questions.
Absolutely agreed, this is worthwhile effort!
> If we wanted to inject these strings in the binary - in some form, then we
> might have to transcode them at that point.
> I don't think the user would know if the string would be injected as wide or
> narrow (or something else) by the compiler.
My intuition is that a user who writes `L"foo"` will expect a wide `"foo"` to
appear in the binary in the cases where the string ends up making it that far.
> L is really is want to convert that string _at that point_. in an attribute,
> strings might have multiple usages so it's better to delay any transcoding.
> Does that make sense?
Not yet, but I might get there eventually. :-D My concern is that vendor
attributes can basically do anything, so there's no reason to assume that any
given string literal usage should or should not transcode. I think we have to
decide on a case by case basis by letting the attributes specify what they
intend in their argument lists. However, my intuition is that *most* attributes
will expect unevaluated string literals because the string argument doesn't get
passed to LLVM.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D105759/new/
https://reviews.llvm.org/D105759
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
