arphaman added inline comments.

================
Comment at: clang/include/clang/Basic/Attr.td:962
+  let Spellings = [Clang<"external_source_symbol", /*allowInC=*/1,
+                   /*version=*/2>];
   let Args = [StringArgument<"language", 1>,
----------------
erichkeane wrote:
> For standards version numbers, we tend to set this to a 'date' more or less, 
> so something like `20230119`.  I wonder if there is value to making THAT how 
> we do this here too?
That's a good idea, I can update it to be a specific date.


================
Comment at: clang/utils/TableGen/ClangAttrEmitter.cpp:3318
+    for (const auto &Spelling : Attr->getValueAsListOfDefs("Spellings")) {
+      if (Spelling->getValueAsString("Variety") == Variety ||
+          Spelling->getValueAsString("Variety") == "Clang") {
----------------
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?


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