aaron.ballman accepted this revision.
aaron.ballman added a comment.
LGTM, though I did have a nit about diagnostic wording that you can feel free
to take or leave at your discretion. Thank you for this work, this was a very
large patch with a whole lot of review comments, but I think what we ended up
with is really great!
================
Comment at: clang/include/clang/Sema/DeclSpec.h:424-431
+ static bool isTransformTypeTrait(TST T) {
+ constexpr TST Traits[] = {
+#define TRANSFORM_TYPE_TRAIT_DEF(_, Trait) TST_##Trait,
+#include "clang/AST/TransformTypeTraits.def"
+ };
+
+ return (T >= Traits[0] && T <= std::end(Traits)[-1]);
----------------
cjdb wrote:
> aaron.ballman wrote:
> > Errr, this isn't Python, so that `[-1]` terrifies me. It took me a good
> > solid ten minutes to convince myself this was actually valid. Any interest
> > in something along the lines of this form?
> I have many questions about the decisions I made in this snippet.
It was definitely a great way to wake up this morning. "Wait, what? WHAT?!?"
followed by a bunch of furious standards reading. :-D
================
Comment at: clang/lib/AST/TypePrinter.cpp:1158
raw_ostream &OS) {
IncludeStrongLifetimeRAII Strong(Policy);
}
----------------
cjdb wrote:
> aaron.ballman wrote:
> > cjdb wrote:
> > > rsmith wrote:
> > > > Remove this line too. No point building an RAII scope with nothing in
> > > > it.
> > > Can we get rid of this function entirely in that case?
> > I believe you can.
> It looks like other things also have this as just empty, so maybe it's best
> to keep it?
That's fine by me!
================
Comment at: clang/test/SemaCXX/type-traits.cpp:3505
+ { using ExpectedError = __make_unsigned(unsigned _BitInt(1)); }
+ // expected-error@*:*{{'make_unsigned' is only compatible with non-bool
integers and enum types, but was given 'unsigned _BitInt(1)'}}
+ { using ExpectedError = __make_unsigned(UnscopedBit); }
----------------
This diagnostic is a bit unfortunate because it was given a non-bool integer,
so I don't know that users will know how to fix the issue from the text. I
leave it to your discretion where to split that diagnostic up with a `%select`
so that `_BitInt` is special or not; I don't think users will hit this
diagnostic all that often, but I do have a little bit of a worry about template
metaprogramming accidentally getting into this state.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D116203/new/
https://reviews.llvm.org/D116203
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
