martong accepted this revision.
martong added a comment.
This revision is now accepted and ready to land.
Hi Vince, this looks good to me!
On the other hand I was pondering on @balazske's comment, this one:
> Or use the new flag added to every item in DefaultTestValuesForRunOptions,
> specially if there is relevant difference in the AST for the options in
> DefaultTestValuesForRunOptions in the code of this test (probably not).
So, the below test instantiation checks only with the "-ffixed-point" option
passed to the frontend.
INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportFixedPointExpr,
::testing::Values(ArgVector{"-ffixed-point"}), );
However, almost all other tests are exercised with a bunch of different options
that are listed in `DefaultTestValuesForRunOptions`. We have this mechanism,
because previously we had a lot of failures from windows build bots, where the
base AST can be different. Of course it is not super relevant to a literal, but
with structs and templates it is.
So, I think it would be really useful in the future, if we could extend
`DefaultTestValuesForRunOptions` with an additional option. And it is okay to
do that in a follow up patch (maybe I'll do it myself, just let me know if you
don't have time for that). Here is what I've been thinking:
--- a/clang/unittests/AST/ASTImporterFixtures.h
+++ b/clang/unittests/AST/ASTImporterFixtures.h
@@ -66,10 +66,13 @@ protected:
}
};
-const auto DefaultTestValuesForRunOptions = ::testing::Values(
+const auto DefaultTestArrayForRunOptions = std::array<ArgVector, 4>{
ArgVector(), ArgVector{"-fdelayed-template-parsing"},
ArgVector{"-fms-compatibility"},
- ArgVector{"-fdelayed-template-parsing", "-fms-compatibility"});
+ ArgVector{"-fdelayed-template-parsing", "-fms-compatibility"}};
+
+const auto DefaultTestValuesForRunOptions =
+ ::testing::ValuesIn(DefaultTestArrayForRunOptions);
--- a/clang/unittests/AST/ASTImporterTest.cpp
+++ b/clang/unittests/AST/ASTImporterTest.cpp
@@ -5922,6 +5922,28 @@ TEST_P(ASTImporterOptionSpecificTestBase,
ImportExprOfAlignmentAttr) {
EXPECT_TRUE(ToA);
}
+template <typename T>
+auto ExtendWithOptions(const T& Values, const ArgVector& Args) {
+ auto Copy = Values;
+ for (ArgVector& ArgV : Copy) {
+ for (const std::string& Arg : Args) {
+ ArgV.push_back(Arg);
+ }
+ }
+ return ::testing::ValuesIn(Copy);
+}
+
+INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportFixedPointExpr,
+ ExtendWithOptions(DefaultTestArrayForRunOptions,
+ ArgVector{"-ffixed-point"}), );
This would give us the following test instances:
ParameterizedTests/ImportFixedPointExpr.ImportFixedPointerLiteralExpr/0,
where GetParam() = { "-ffixed-point" }
ParameterizedTests/ImportFixedPointExpr.ImportFixedPointerLiteralExpr/1,
where GetParam() = { "-fdelayed-template-parsing", "-ffixed-point" }
ParameterizedTests/ImportFixedPointExpr.ImportFixedPointerLiteralExpr/2,
where GetParam() = { "-fms-compatibility", "-ffixed-point" }
ParameterizedTests/ImportFixedPointExpr.ImportFixedPointerLiteralExpr/3,
where GetParam() = { "-fdelayed-template-parsing", "-fms-compatibility",
"-ffixed-point" }
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D77721/new/
https://reviews.llvm.org/D77721
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
