This revision was automatically updated to reflect the committed changes.
Closed by commit rC335464: [ASTImporter] Add ms compatibility to tests which
use the TestBase (authored by martong, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D47367?vs=152674=152675#toc
martong updated this revision to Diff 152674.
martong added a comment.
- Update commit comment and fix broken format in a comment.
Repository:
rC Clang
https://reviews.llvm.org/D47367
Files:
unittests/AST/ASTImporterTest.cpp
unittests/AST/Language.cpp
unittests/AST/Language.h
Index:
a_sidorin accepted this revision.
a_sidorin added a comment.
This revision is now accepted and ready to land.
LGTM, thank you!
Could you describe the refactoring in the commit message?
Comment at: unittests/AST/ASTImporterTest.cpp:212
+ /// The verification is done by
martong requested review of this revision.
martong added a comment.
Ping.
Repository:
rC Clang
https://reviews.llvm.org/D47367
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
balazske accepted this revision.
balazske added a comment.
This revision is now accepted and ready to land.
Found no big problems. But not all extra options are applicable to all
languages (template related things to C) so there may be redundant tests.
Repository:
rC Clang
martong updated this revision to Diff 149096.
martong added a comment.
- Remove unused RunOptions typedef and isCXX function
Repository:
rC Clang
https://reviews.llvm.org/D47367
Files:
unittests/AST/ASTImporterTest.cpp
unittests/AST/Language.cpp
unittests/AST/Language.h
Index:
martong updated this revision to Diff 149095.
martong added a comment.
- Remove unused function
Repository:
rC Clang
https://reviews.llvm.org/D47367
Files:
unittests/AST/ASTImporterTest.cpp
unittests/AST/Language.cpp
unittests/AST/Language.h
Index: unittests/AST/Language.h
martong added a reviewer: balazske.
martong added a comment.
Balazs, could you please review this patch as well? (This code is not in our
fork yet.)
Repository:
rC Clang
https://reviews.llvm.org/D47367
___
cfe-commits mailing list
martong updated this revision to Diff 149094.
martong added a comment.
- Forgot to instantiate some of the parameterized tests
Repository:
rC Clang
https://reviews.llvm.org/D47367
Files:
unittests/AST/ASTImporterTest.cpp
Index: unittests/AST/ASTImporterTest.cpp
martong updated this revision to Diff 149093.
martong added a comment.
- Moved the family of `testImport` functions under a test fixture class, so we
can use parameterized test.
- Refactored `testImport` and `testImportSequence`, because for loops over the
different compiler options is no
a.sidorin added a comment.
I meant that we can use this approach for testImport() too.
Repository:
rC Clang
https://reviews.llvm.org/D47367
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
a.sidorin added a comment.
> I think, it is overkill to test all possible combinations of the options, we
> should come up with something better here.
I agree with that. I think we need to test just import pairs
{/*From*/no_option, /*To*/no_option}, {option_1, option1}, {option_2,
option_2},
martong added a comment.
This patch does not target `testImport` because I am not sure how to handle the
options there:
auto RunOptsFrom = getRunOptionsForLanguage(FromLang);
auto RunOptsTo = getRunOptionsForLanguage(ToLang);
for (const auto : RunOptsFrom)
for (const auto :
martong created this revision.
martong added reviewers: a.sidorin, r.stahl, xazax.hun.
Herald added subscribers: cfe-commits, dkrupp, rnkovacs.
In order to avoid build failures on MS, we use -fms-compatibility too in the
tests which use the TestBase.
Repository:
rC Clang
14 matches
Mail list logo
