erichkeane added inline comments.

================
Comment at: clang/utils/TableGen/ClangAttrEmitter.cpp:3318
+    for (const auto &Spelling : Attr->getValueAsListOfDefs("Spellings")) {
+      if (Spelling->getValueAsString("Variety") == Variety ||
+          Spelling->getValueAsString("Variety") == "Clang") {
----------------
arphaman wrote:
> erichkeane wrote:
> > Why is this =="Clang" specific?  Since you've added the Version to the 
> > spelling, I'd anticipate us to just be able to grab it for the current 
> > spelling.  I wouldn't want an individual spelling here to override it, 
> > particularly since with this change Clang could potentially override the 
> > standards version.
> I needed it since there's no specific "Clang" variety that's being called for 
> this function. Otherwise the "GNU" variety passed to the function doesn't 
> match "Clang" variety in the record. What's the best way to compute the 
> current spelling in this case?
Hmm... that is strange.  I would have expected this to be called for each of 
the individual spellings, it doesn't really make sense that it wouldn't pick it 
up from the base 'Spellings'.  I've unfortunately not debugged this code 
generation in a while.  BUT, we care about the 'version' on a per-spelling 
basis, so it would presumably need to 'pick' an actual spelling.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141324/new/

https://reviews.llvm.org/D141324

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to