Page MenuHomePhabricator

Correctly classify main file includes if there is a prefix added
ClosedPublic

Authored by jbangert on Oct 26 2016, 3:24 PM.

Details

Summary

Prevents misclassifying includes based on the command-line filename (e.g. if a project is in a subdirectory).

This is slightly more robust than the additional duplicate detection, however the current classification scheme is still kind of brittle for a lot of code.

Diff Detail

Repository
rL LLVM

Event Timeline

jbangert updated this revision to Diff 75958.Oct 26 2016, 3:24 PM
jbangert retitled this revision from to Don't include the same file twice in IncludeInserter, even if misclassified..
jbangert updated this object.
jbangert added reviewers: alexfh, hokein.
jbangert added a subscriber: Restricted Project.
alexfh edited edge metadata.Oct 27 2016, 7:40 AM

Julian, I would appreciate if you could produce a test case for this fix, since otherwise it would be hard to track, if this regresses.

jbangert updated this revision to Diff 83156.Jan 4 2017, 4:00 PM
jbangert edited edge metadata.

Instead modify IncludeInserter to classify correctly.

jbangert retitled this revision from Don't include the same file twice in IncludeInserter, even if misclassified. to Correctly classify main file includes if there is a prefix added.Jan 4 2017, 4:02 PM
jbangert updated this object.

I slightly changed the approach of this CL, and added a test. This now also works in clang_tidy is called from a different working directory.

alexfh accepted this revision.Jan 11 2017, 12:41 AM
alexfh edited edge metadata.

LG. Thank you for the fix!

This revision is now accepted and ready to land.Jan 11 2017, 12:41 AM

Tests don't pass with this patch applied:

$ ninja check-clang-tools                                                                              
[22/23] Running the Clang extra tools' regression tests
FAIL: Extra Tools Unit Tests :: clang-tidy/ClangTidyTests/IncludeInserterTest.DontInsertDuplicateIncludeEvenIfMiscategorized (427 of 489)
******************** TEST 'Extra Tools Unit Tests :: clang-tidy/ClangTidyTests/IncludeInserterTest.DontInsertDuplicateIncludeEvenIfMiscategorized' FAILED ********************
Note: Google Test filter = IncludeInserterTest.DontInsertDuplicateIncludeEvenIfMiscategorized
[==========] Running 1 test from 1 test case.
[----------] Global test environment set-up.
[----------] 1 test from IncludeInserterTest
[ RUN      ] IncludeInserterTest.DontInsertDuplicateIncludeEvenIfMiscategorized
1 warning and 1 error generated.
LLVM ERROR: 'a/header.h' file not found
foo, bar


********************
Testing Time: 5.32s
********************
Failing Tests (1):
    Extra Tools Unit Tests :: clang-tidy/ClangTidyTests/IncludeInserterTest.DontInsertDuplicateIncludeEvenIfMiscategorized

  Expected Passes    : 487
  Expected Failures  : 1
  Unexpected Failures: 1
FAILED: tools/clang/tools/extra/test/CMakeFiles/check-clang-tools 
cd /build/tools/clang/tools/extra/test && /usr/bin/python2.7 /src/utils/lit/lit.py -sv /build/tools/clang/tools/extra/test
ninja: build stopped: subcommand failed.
alexfh requested changes to this revision.Jan 11 2017, 3:57 AM
alexfh edited edge metadata.
This revision now requires changes to proceed.Jan 11 2017, 3:57 AM
jbangert updated this revision to Diff 84001.Jan 11 2017, 10:42 AM
jbangert edited edge metadata.
jbangert updated this object.
  • Add header to unit test fixture.
This revision was automatically updated to reflect the committed changes.

LG, committed as r291767.