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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
