[PATCH] D73413: [clang-tidy] Add check to detect external definitions with no header declaration

2020-02-11 Thread Tony Lewis via Phabricator via cfe-commits
tonyelewis added a comment. Thanks very much. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73413/new/ https://reviews.llvm.org/D73413 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D73413: [clang-tidy] Add check to detect external definitions with no header declaration

2020-02-10 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 243545. njames93 added a comment. - Relaxed corresponding header - Added support for tag types Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73413/new/ https://reviews.llvm.org/D73413 Files:

[PATCH] D73413: [clang-tidy] Add check to detect external definitions with no header declaration

2020-02-03 Thread Tony Lewis via Phabricator via cfe-commits
tonyelewis added inline comments. Comment at: clang-tools-extra/clang-tidy/misc/MissingHeaderFileDeclarationCheck.cpp:75 + continue; +if (AnyHeader || File->NamePart.equals_lower(ThisFile->NamePart)) + return; // Found a good candidate for matching decl

[PATCH] D73413: [clang-tidy] Add check to detect external definitions with no header declaration

2020-02-01 Thread Nathan James via Phabricator via cfe-commits
njames93 marked an inline comment as done. njames93 added inline comments. Comment at: clang-tools-extra/clang-tidy/misc/MissingHeaderFileDeclarationCheck.cpp:75 + continue; +if (AnyHeader || File->NamePart.equals_lower(ThisFile->NamePart)) + return; // Found a

[PATCH] D73413: [clang-tidy] Add check to detect external definitions with no header declaration

2020-01-31 Thread Tony Lewis via Phabricator via cfe-commits
tonyelewis added a comment. > Support for checking types should be either opt-in(or opt-out) but never > forced. I feel like it would generate too much noise. Okey dokes. That option could always be added as another feature in the future. Thanks very much for all work on this.

[PATCH] D73413: [clang-tidy] Add check to detect external definitions with no header declaration

2020-01-31 Thread Nathan James via Phabricator via cfe-commits
njames93 marked 2 inline comments as done. njames93 added a comment. In D73413#1851432 , @tonyelewis wrote: > Thanks so much for all the effort on this. I think it's really wonderful. > > I've added a couple of comments elsewhere. > > My other query:

[PATCH] D73413: [clang-tidy] Add check to detect external definitions with no header declaration

2020-01-31 Thread Tony Lewis via Phabricator via cfe-commits
tonyelewis added a comment. Thanks so much for all the effort on this. I think it's really wonderful. I've added a couple of comments elsewhere. My other query: does/should this check cover types? Eg does/should it fire on a class definition in a .cpp file that isn't given internal-linkage?

[PATCH] D73413: [clang-tidy] Add check to detect external definitions with no header declaration

2020-01-27 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment. In D73413#1843368 , @njames93 wrote: > In D73413#1843103 , @alexfh wrote: > > > How is this different from `-Wmissing-prototypes`? > > > This checks variables too, and it looks for a

[PATCH] D73413: [clang-tidy] Add check to detect external definitions with no header declaration

2020-01-27 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. In D73413#1843103 , @alexfh wrote: > How is this different from `-Wmissing-prototypes`? This checks variables too, and it looks for a prototype specifically in the header files. missing-prototypes just ensures there is a

[PATCH] D73413: [clang-tidy] Add check to detect external definitions with no header declaration

2020-01-27 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added a comment. How is this different from `-Wmissing-prototypes`? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73413/new/ https://reviews.llvm.org/D73413 ___ cfe-commits mailing list

[PATCH] D73413: [clang-tidy] Add check to detect external definitions with no header declaration

2020-01-27 Thread pre-merge checks [bot] via Phabricator via cfe-commits
merge_guards_bot added a comment. {icon check-circle color=green} Unit tests: pass. 62197 tests passed, 0 failed and 815 were skipped. {icon times-circle color=red} clang-tidy: fail. clang-tidy found 0 errors and 1 warnings

[PATCH] D73413: [clang-tidy] Add check to detect external definitions with no header declaration

2020-01-27 Thread Nathan James via Phabricator via cfe-commits
njames93 added inline comments. Comment at: clang-tools-extra/clang-tidy/misc/MissingHeaderFileDeclarationCheck.cpp:75 + continue; +if (AnyHeader || File->NamePart.equals_lower(ThisFile->NamePart)) + return; // Found a good candidate for matching decl

[PATCH] D73413: [clang-tidy] Add check to detect external definitions with no header declaration

2020-01-27 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 240611. njames93 marked 6 inline comments as done. njames93 added a comment. - Tweaked default corresponding header settings Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73413/new/

[PATCH] D73413: [clang-tidy] Add check to detect external definitions with no header declaration

2020-01-27 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 added inline comments. Comment at: clang-tools-extra/clang-tidy/misc/MissingHeaderFileDeclarationCheck.cpp:75 + continue; +if (AnyHeader || File->NamePart.equals_lower(ThisFile->NamePart)) + return; // Found a good candidate for matching decl

[PATCH] D73413: [clang-tidy] Add check to detect external definitions with no header declaration

2020-01-27 Thread Nathan James via Phabricator via cfe-commits
njames93 marked 2 inline comments as done. njames93 added inline comments. Comment at: clang-tools-extra/clang-tidy/misc/MissingHeaderFileDeclarationCheck.cpp:29 + + bool isHeader() const { +return llvm::StringSwitch(Extension) gribozavr2 wrote: > njames93

[PATCH] D73413: [clang-tidy] Add check to detect external definitions with no header declaration

2020-01-27 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 added inline comments. Comment at: clang-tools-extra/clang-tidy/misc/MissingHeaderFileDeclarationCheck.cpp:29 + + bool isHeader() const { +return llvm::StringSwitch(Extension) njames93 wrote: > gribozavr2 wrote: > > I think we should consider

[PATCH] D73413: [clang-tidy] Add check to detect external definitions with no header declaration

2020-01-27 Thread Nathan James via Phabricator via cfe-commits
njames93 marked 2 inline comments as done. njames93 added a comment. I'll work up those other test cases Comment at: clang-tools-extra/clang-tidy/misc/MissingHeaderFileDeclarationCheck.cpp:29 + + bool isHeader() const { +return llvm::StringSwitch(Extension)

[PATCH] D73413: [clang-tidy] Add check to detect external definitions with no header declaration

2020-01-27 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 added a comment. Thank you for the contribution! I didn't review the code thoroughly yet, only the tests. Comment at: clang-tools-extra/clang-tidy/misc/MissingHeaderFileDeclarationCheck.cpp:29 + + bool isHeader() const { +return llvm::StringSwitch(Extension)

[PATCH] D73413: [clang-tidy] Add check to detect external definitions with no header declaration

2020-01-26 Thread Noel Grandin via Phabricator via cfe-commits
grandinj added a comment. You might want to look at the equivalent LibreOffice checkers, which we have been using for some time https://cgit.freedesktop.org/libreoffice/core/tree/compilerplugins/clang/external.cxx

[PATCH] D73413: [clang-tidy] Add check to detect external definitions with no header declaration

2020-01-25 Thread pre-merge checks [bot] via Phabricator via cfe-commits
merge_guards_bot added a comment. {icon times-circle color=red} Unit tests: fail. 62128 tests passed, 5 failed and 807 were skipped. failed: libc++.std/language_support/cmp/cmp_partialord/partialord.pass.cpp failed: libc++.std/language_support/cmp/cmp_strongeq/cmp.strongeq.pass.cpp

[PATCH] D73413: [clang-tidy] Add check to detect external definitions with no header declaration

2020-01-25 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 240387. njames93 added a comment. - Warn extern declarations in source files Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73413/new/ https://reviews.llvm.org/D73413 Files:

[PATCH] D73413: [clang-tidy] Add check to detect external definitions with no header declaration

2020-01-25 Thread pre-merge checks [bot] via Phabricator via cfe-commits
merge_guards_bot added a comment. {icon times-circle color=red} Unit tests: fail. 62128 tests passed, 5 failed and 807 were skipped. failed: libc++.std/language_support/cmp/cmp_partialord/partialord.pass.cpp failed: libc++.std/language_support/cmp/cmp_strongeq/cmp.strongeq.pass.cpp

[PATCH] D73413: [clang-tidy] Add check to detect external definitions with no header declaration

2020-01-25 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 240375. njames93 marked 3 inline comments as done. njames93 added a comment. - Address nits Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73413/new/ https://reviews.llvm.org/D73413 Files:

[PATCH] D73413: [clang-tidy] Add check to detect external definitions with no header declaration

2020-01-25 Thread pre-merge checks [bot] via Phabricator via cfe-commits
merge_guards_bot added a comment. {icon times-circle color=red} Unit tests: fail. 62128 tests passed, 5 failed and 807 were skipped. failed: libc++.std/language_support/cmp/cmp_partialord/partialord.pass.cpp failed: libc++.std/language_support/cmp/cmp_strongeq/cmp.strongeq.pass.cpp

[PATCH] D73413: [clang-tidy] Add check to detect external definitions with no header declaration

2020-01-25 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/clang-tidy/misc/MissingHeaderFileDeclarationCheck.cpp:14 +#include "llvm/ADT/Optional.h" +#include "llvm/ADT/StringSwitch.h" +#include Please also include StringRef. Comment

[PATCH] D73413: [clang-tidy] Add check to detect external definitions with no header declaration

2020-01-25 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 240373. njames93 added a comment. - Ignores implicit template instantiations Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73413/new/ https://reviews.llvm.org/D73413 Files:

[PATCH] D73413: [clang-tidy] Add check to detect external definitions with no header declaration

2020-01-25 Thread pre-merge checks [bot] via Phabricator via cfe-commits
merge_guards_bot added a comment. {icon times-circle color=red} Unit tests: fail. 62128 tests passed, 5 failed and 807 were skipped. failed: libc++.std/language_support/cmp/cmp_partialord/partialord.pass.cpp failed: libc++.std/language_support/cmp/cmp_strongeq/cmp.strongeq.pass.cpp

[PATCH] D73413: [clang-tidy] Add check to detect external definitions with no header declaration

2020-01-25 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 240372. njames93 added a comment. - fix formatting Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73413/new/ https://reviews.llvm.org/D73413 Files: clang-tools-extra/clang-tidy/misc/CMakeLists.txt

[PATCH] D73413: [clang-tidy] Add check to detect external definitions with no header declaration

2020-01-25 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. This is what gets flagged when ran on clang/lib with CheckAnyHeader enabled (with it disabled it just goes crazy as a lot of clang headers files seem to share a source file for implementation) clang/lib

[PATCH] D73413: [clang-tidy] Add check to detect external definitions with no header declaration

2020-01-25 Thread pre-merge checks [bot] via Phabricator via cfe-commits
merge_guards_bot added a comment. {icon times-circle color=red} Unit tests: fail. 62128 tests passed, 5 failed and 807 were skipped. failed: libc++.std/language_support/cmp/cmp_partialord/partialord.pass.cpp failed: libc++.std/language_support/cmp/cmp_strongeq/cmp.strongeq.pass.cpp

[PATCH] D73413: [clang-tidy] Add check to detect external definitions with no header declaration

2020-01-25 Thread pre-merge checks [bot] via Phabricator via cfe-commits
merge_guards_bot added a comment. {icon times-circle color=red} Unit tests: fail. 62128 tests passed, 5 failed and 807 were skipped. failed: libc++.std/language_support/cmp/cmp_partialord/partialord.pass.cpp failed: libc++.std/language_support/cmp/cmp_strongeq/cmp.strongeq.pass.cpp

[PATCH] D73413: [clang-tidy] Add check to detect external definitions with no header declaration

2020-01-25 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 240371. njames93 added a comment. - Remove unnecessary duplicated code Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73413/new/ https://reviews.llvm.org/D73413 Files:

[PATCH] D73413: [clang-tidy] Add check to detect external definitions with no header declaration

2020-01-25 Thread Nathan James via Phabricator via cfe-commits
njames93 created this revision. Herald added subscribers: cfe-commits, xazax.hun, mgorny. Herald added a project: clang. Flags variables and functions with external linkage that don't have a declaration in the corresponding header file. Repository: rG LLVM Github Monorepo