search

Ericsson / clang

Cross Translation Unit analysis capability for Clang Static Analyzer. (Fork of official clang at http://llvm.org/git/clang)
http://clang.llvm.org/
Other
15 stars 10 forks source link

ASTImporterTest ImportRecordDeclInFuncParams related fixes #372

Closed balazske closed 6 years ago

balazske commented 6 years ago

Added a new lit test to test that this is not supported. The warning message could not be printed out in ASTImporterTest, this is corrected. The ImportRecordDeclInFuncParams test still fails but does not crash.

balazske commented 6 years ago

fixes #371 (maybe remove the unit test?)

Xazax-hun commented 6 years ago

Oh, I see now, the test still fails. Why is there the failure? Since we have a test to demonstrate we do not crash I am ok with removing the disabled test.

martong commented 6 years ago

Instead of deletion I'd rather change the unittest to reflect what really happens, i.e. a nullptr is returned when the function is imported, right?

balazske commented 6 years ago

This means a new 'testNotImport' function is needed?

martong commented 6 years ago

@balazske I think you could just use the ASTImportTestBase with getTuDecl and then Import.

balazske commented 6 years ago

Is it bad to have unit test and lit test for the same problem? (But these test not exactly the same thing.)

martong commented 6 years ago

@balazske No, that is absolutely not bad at all. Lit tests are rather integration tests, i.e. they are in a higher level in the test pyramid. Lit tests checks the output of the whole program, while in the unit test we test very specific parts only.

balazske commented 6 years ago

updated code

balazske commented 6 years ago

updated code again beginSourceFile is called only once

balazske commented 6 years ago

Phabricator review of a part of the changes: https://reviews.llvm.org/D47445

balazske commented 6 years ago

Phabricator review back-ported in pull request: https://github.com/Ericsson/clang/pull/411