echristo added a comment. Hi Peng,
Looks interesting, but I think the language support needs some more comments about what's expected and in addition through everything else please? I've added some inline comments with places I think could use it for sure. -eric ================ Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:2389 } - if (!hasRequiredFeatures(ReqFeatures, CGM, FD, MissingFeature)) + if (!llvm::all_of(ReqFeatures, [&](StringRef Feature) { + if (!CallerFeatureMap.lookup(Feature)) { ---------------- Not sure why the change here. It'd be good to be able to reuse the same code here. What's up? ================ Comment at: clang/lib/CodeGen/CodeGenFunction.h:4683 +class TargetFeatures { + struct FeatureListStatus { ---------------- Needs comments including an explanation of syntax. ================ Comment at: clang/unittests/CodeGen/CheckTargetFeaturesTest.cpp:6 + +TEST(CheckTargetFeaturesTest, checkBuiltinFeatures) { + auto doCheck = [](StringRef BuiltinFeatures, StringRef FuncFeatures) { ---------------- Comment the test with what you're testing. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89184/new/ https://reviews.llvm.org/D89184 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits