[PATCH] D77721: [ASTImporter] Add support for importing fixed point literals

[Balázs Kéri via Phabricator via cfe-commits]

balazske added inline comments.


================
Comment at: clang/unittests/AST/ASTImporterTest.cpp:1000
+                       hasDescendant(fixedPointLiteral()))));
+}
+
----------------
See test `ImportFloatinglLiteralExpr` for a better implementation of this test. The new test could be included after `ImportFloatinglLiteralExpr`. But a new base class is needed like `struct ImportFixedPointExpr : ImportExpr {};`.


================
Comment at: clang/unittests/AST/ASTImporterTest.cpp:5954
 INSTANTIATE_TEST_CASE_P(ParameterizedTests, ASTImporterOptionSpecificTestBase,
-                        DefaultTestValuesForRunOptions, );
+                        ::testing::Values(ArgVector{"-ffixed-point"}), );
 
----------------
The default options should not be changed for every (...) test as this change does that. Something like this is better:
```
INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportFixedPointExpr,
                        ::testing::Values(ArgVector{"-ffixed-point"}), );
```
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).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77721