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

Reply via email to