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

Fix redecl chain of classes and class templates #489

Closed martong closed 6 years ago

martong commented 6 years ago

The crux of the issue was that lookup could not find previous decls of a friend class. The solution involves making the friend declarations visible in their decl context (i.e. adding them to the lookup table). This fix involves two other repairs: (1) We could not handle the addition of injected class types properly when a redecl chain was involved, now this is fixed. (2) DeclContext::removeDecl failed if the lookup table in Vector form did not contain the to be removed element. This caused troubles in ASTImporter::ImportDeclContext. This is also fixed.

Fixes #484

martong commented 6 years ago

Unfortunately dos not fix #487 . Perhaps because there are other cases (other than broken redecl chains of classes) when structural equivalency does not match and thus an empty DeclGroup is created.

martong commented 6 years ago

Update: Changed the style where you indicated, and added an assert, in a new commit so you can follow the diff.

balazske commented 6 years ago

The code looks OK. But in the tests it could be better to use descriptive names (not Xxx0 and Xxx1) and not re-use existing variables with different meaning. (If not corrected here, this can be problem in the opensource review. My test code was not accepted with such problems once.)

balazske commented 6 years ago

Another thing: We can check if there are more similar tests that are not in the ImportFriendClasses fixture and move these in. (Similar issues may exist with other tests.)

martong commented 6 years ago

Update: changed the PR to properly build up the redecl chain not just in case of Friends. Now this seems to fix #487 too. I am going to create additional tests as well.

martong commented 6 years ago

ctu-clang7: https://github.com/Ericsson/clang/commit/9d42ac3de0e1292ad018d12fd4ef57d803e42f25

martong commented 5 years ago

https://reviews.llvm.org/D53655