[PATCH] D105759: Implement P2361 Unevaluated string literals

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a subscriber: clang-vendors.
aaron.ballman added a comment.

In D105759#4457041 <https://reviews.llvm.org/D105759#4457041>, @cor3ntin wrote:

> In D105759#4456864 <https://reviews.llvm.org/D105759#4456864>, @aaron.ballman wrote:
>
>> I don't think it's correct to assume that all string arguments to attributes are unevaluated, but it is hard to tell where to draw the line sometimes. Backing up a step, as I understand P2361 <https://reviews.llvm.org/P2361>, an unevaluated string is one which is not converted into the execution character set (effectively). Is that correct? If so, then as an example, `[[clang::annotate()]]` should almost certainly be using an evaluated string because the argument is passed down to LLVM IR and is used in ways we cannot predict. What's more, an unevaluated string cannot have some kinds of escape characters (numeric and conditional escape sequences) and those are currently allowed by `clang::annotate` and could potentially be used by a backend plugin.
>>
>> I think other attributes may have similar issues. For example, the `alias` attribute is a bit of a question mark for me -- that takes a string literal representing an external identifier that is looked up. I'm not certain whether that should be in the execution character set or not, but we do support escape sequences for it: https://godbolt.org/z/v65Yd7a68
>>
>> I think we need to track evaluated vs not on the argument level so that the attributes in Attr.td can decide which form to use. I think we should default to "evaluated" for any attribute we're on the fence about because that's the current behavior they get today (so we should avoid regressions).
>
> I really don't think it makes sense to have both "unevaluated" and "evaluated" arguments.
> We chatted offline and we struggle to find places where escape sequences are used, or examples of attributes intended to be in the execution character set.

In general I agree, but the one scenario that I keep coming back to are attributes like `diagnose_if` where they take an expression we're evaluating at compile time (condition expression) and a string literal that's not evaluated (warning vs error, diagnostic message itself). But I think the "evaluating at compile time" is part of why I don't think we intend the attribute to be considering the execution character set.

> My suggestion would be to land the non-attributes changes now, and the attributes bits in early clang 18.

I think we're almost safe enough to make the attribute changes in Clang 17 so that no attribute uses an evaluated argument, but given that there's less than a month before we make the 17 branch, I think it's probably a good idea to make these changes after the branch point so folks have longer to react. Adding `clang-vendors` to the review for awareness of the potential for a breaking change.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105759/new/

https://reviews.llvm.org/D105759