erichkeane accepted this revision.
erichkeane added a comment.

In D80836#2064182 <https://reviews.llvm.org/D80836#2064182>, @dblaikie wrote:

> (sorry @erichkeane didn't mean to usurp your feedback by approving before it 
> was answered - @aaron.ballman do treat my approval as contingent on that 
> feedback being addressed as you/both see fit)


No problem :)  The "KnownToGCC" issue is likely a separate issue that should be 
fixed in a separate patch.

The source-of-the-not-supported-by-C info is simply "if you got this elsewhere, 
please document the source for future reference", which can be handled without 
me double checking.  If its self experimentation, that makes me sad, but I 
don't think there is further action that could be taken about that one.



================
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);
----------------
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.


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