aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM
================ Comment at: clang/test/SemaCXX/lambda-expressions.cpp:675-676 +StringLiteral(const char (&array)[N]) + __attribute__((enable_if(__builtin_strlen(array) == 2, + "invalid string literal"))); +}; ---------------- cor3ntin wrote: > aaron.ballman wrote: > > cor3ntin wrote: > > > aaron.ballman wrote: > > > > 1) This code already compiles today without issue > > > > (https://godbolt.org/z/q6hPxoWq9), so are you sure this is testing what > > > > needs to be tested? > > > > > > > > 2) What about for non-GNU-style attributes ([[]] attributes that > > > > appertain to the function type)? e.g., > > > > ``` > > > > template <int N> > > > > void foo(const char (&array)[N]) [[clang::annotate_type("test", > > > > array)]]; > > > > ``` > > > Yes, this is something that was broken by this patch. > > > enable_if did create odr uses (incorrectly). I can add a test for > > > annotate_type but I don't think it would run into the same issue at all. > > So I'm not certain what's being tested then -- how are you validating that > > `array` is no longer ODR used? > > > > In terms of `annotate_type`, that attribute accepts a string literal > > followed by a variable number of expression arguments, so I think it should > > have the same issues as a GNU-style attribute accepting an expression > > argument. I was mostly trying to wrap my head around whether `[[]]` and > > `__attribute__` are expected to have different behaviors when written in > > that position. > Context https://reviews.llvm.org/D124351#4101611 I talked to @cor3ntin off-list to get a better understanding of the test situation, and I realized the issue was that this code used to give a diagnostic that it shouldn't have, so this is demonstrating we accept the code rather than reject it. That makes a lot more sense now! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124351/new/ https://reviews.llvm.org/D124351 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
