aaron.ballman added a comment.
Overall, I think this LGTM, but I did find a few nits. Can you fix the
clang-format issues? Also, I'd like to see some C++ test coverage that shows
how this works on template (partial) specializations, lambdas (with GNU-style
syntax), and overloaded functions. If we deviate from the GCC behavior in any
cases, it'd be great to capture it in comments (unless you think the deviation
is intentional, at which point we should document it in AttrDocs.td, or you
think the deviation needs to be fixed before we land it).
================
Comment at: clang/include/clang/Basic/Attr.td:2706
+ featuresStrs_begin(), featuresStrs_begin() + Index,
+ [ FeatureStr ](StringRef S) { return S == FeatureStr; });
+ }
----------------
================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:11289
+ "or a string literal containing a comma-separated list of versions">,
+ InGroup<DiagGroup<"target-clones-mixed-specifiers">>;
+def warn_target_clone_duplicate_options
----------------
Should this ad hoc group be listed within the `FunctionMultiVersioning` group?
================
Comment at: clang/lib/Sema/SemaDeclAttr.cpp:1968-1972
+ if (checkAttrMutualExclusion<TargetAttr>(S, D, AL) ||
+ checkAttrMutualExclusion<TargetClonesAttr>(S, D, AL) ||
+ checkAttrMutualExclusion<CPUDispatchAttr>(S, D, AL) ||
+ checkAttrMutualExclusion<CPUSpecificAttr>(S, D, AL))
+ return;
----------------
This should be handled in Attr.td via a `def : MutualExclusions<[....]>;` if
possible.
================
Comment at: clang/lib/Sema/SemaDeclAttr.cpp:3274
static void handleTargetAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
+ if (checkAttrMutualExclusion<TargetAttr>(S, D, AL) ||
+ checkAttrMutualExclusion<TargetClonesAttr>(S, D, AL) ||
----------------
Same here.
================
Comment at: clang/lib/Sema/SemaDeclAttr.cpp:3342
+static void handleTargetClonesAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
+ if (checkAttrMutualExclusion<TargetAttr>(S, D, AL) ||
+ checkAttrMutualExclusion<TargetClonesAttr>(S, D, AL) ||
----------------
And here
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D51650/new/
https://reviews.llvm.org/D51650
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
