aaron.ballman added inline comments.
================ Comment at: clang/utils/TableGen/ClangAttrEmitter.cpp:87 Ret.emplace_back("CXX11", std::string(Name), "gnu", true); + if (Spelling->getValueAsBit("AllowInC")) + Ret.emplace_back("C2x", std::string(Name), "gnu", true); ---------------- erichkeane wrote: > rsmith wrote: > > erichkeane wrote: > > > I guess its a problem for all of these, but why is the last > > > 'true'/'false' (KnownToGCC) not checked from the Spelling? It has: let > > > KnownToGCC = 1;. > > > > > > I would presume that line either is meaningless, or should be used for > > > the true/false bits in here. > > The `KnownToGCC` flag affects whether we produce certain warnings, not > > which spellings we register. This does seem fishy: we have the same > > information represented and examined both by considering whether the > > attribute has a `GNU` vs `GCC` spelling and by considering whether the > > `KnownToGCC` flag is set. I imagine we could factor this better. (The two > > concerns are, I suppose, notionally orthogonal, but I can't imagine we > > would ever want them to differ.) > Well, both are used to set the value in "FlattenedSpelling". Here we use > true/false, but if you construct one via a record it uses the KnownToGCC > flag. It seems to me that these values should be initialized consistently. > > The "spelling" Variety themselves I can see being different, but for > consistency's sake, these trues in the emplace_back should set KnownToGCC > with the attribute. That, or we abandon the KnownToGCC 'Value' in the > spelling and just always infer it based on the variety. > > Like I said, its just weird that sometimes we set this via the tablegen file, > sometimes we do it automagically. Yeah, I agree that this is a bit weird. IIRC, this came from a time when GCC hadn't yet made all __attribute__ spellings available as [[]] spellings and so there was a worry we'd have some __attribute__ spellings that use GNU instead of GCC, despite being supported by GCC. I don't think that situation is plausible any longer. I think a better approach is to keep the known to GCC bit on the `ParsedAttr` object so we don't have to check attribute vendor name when issuing a diagnostic, but we should infer that bit from the spelling instead of setting it in the Attr.td file. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80836/new/ https://reviews.llvm.org/D80836 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits