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

[njames93]

Summary

Flags variables and functions with external linkage that don't have a declaration in the corresponding header file.
Addresses Feature request: possible check for definition with external linkage without corresponding declaration in a header.

This is the output from running against clang/lib with CheckAnyHeader enabled

[njames93]

• Remove unnecessary duplicated code

[merge_guards_bot]

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
failed: libc++.std/language_support/cmp/cmp_strongord/strongord.pass.cpp
failed: libc++.std/language_support/cmp/cmp_weakeq/cmp.weakeq.pass.cpp
failed: libc++.std/language_support/cmp/cmp_weakord/weakord.pass.cpp

clang-tidy: fail. clang-tidy found 0 errors and 1 warnings. 0 of them are added as review comments below (why?).

clang-format: fail. Please format your changes with clang-format by running git-clang-format HEAD^ or applying this patch.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.

[merge_guards_bot]

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
failed: libc++.std/language_support/cmp/cmp_strongord/strongord.pass.cpp
failed: libc++.std/language_support/cmp/cmp_weakeq/cmp.weakeq.pass.cpp
failed: libc++.std/language_support/cmp/cmp_weakord/weakord.pass.cpp

clang-tidy: fail. clang-tidy found 0 errors and 1 warnings. 0 of them are added as review comments below (why?).

clang-format: fail. Please format your changes with clang-format by running git-clang-format HEAD^ or applying this patch.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.

[njames93]

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

[njames93]

• fix formatting

[merge_guards_bot]

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
failed: libc++.std/language_support/cmp/cmp_strongord/strongord.pass.cpp
failed: libc++.std/language_support/cmp/cmp_weakeq/cmp.weakeq.pass.cpp
failed: libc++.std/language_support/cmp/cmp_weakord/weakord.pass.cpp

clang-tidy: fail. clang-tidy found 0 errors and 1 warnings. 0 of them are added as review comments below (why?).

clang-format: pass.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.

[njames93]

• Ignores implicit template instantiations

[Eugene.Zelenko]

clang-tools-extra/clang-tidy/misc/MissingHeaderFileDeclarationCheck.cpp

15

Please also include StringRef.

clang-tools-extra/test/clang-tidy/checkers/Inputs/misc-missing-header-file-declaration/wrong_header.h

8

Please add newline.

clang-tools-extra/test/clang-tidy/checkers/misc-missing-header-file-declaration-any-header.cpp

80

Please add newline.

[merge_guards_bot]

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
failed: libc++.std/language_support/cmp/cmp_strongord/strongord.pass.cpp
failed: libc++.std/language_support/cmp/cmp_weakeq/cmp.weakeq.pass.cpp
failed: libc++.std/language_support/cmp/cmp_weakord/weakord.pass.cpp

clang-tidy: fail. clang-tidy found 0 errors and 1 warnings. 0 of them are added as review comments below (why?).

clang-format: pass.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.

[njames93]

• Address nits

[merge_guards_bot]

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
failed: libc++.std/language_support/cmp/cmp_strongord/strongord.pass.cpp
failed: libc++.std/language_support/cmp/cmp_weakeq/cmp.weakeq.pass.cpp
failed: libc++.std/language_support/cmp/cmp_weakord/weakord.pass.cpp

clang-tidy: fail. clang-tidy found 0 errors and 1 warnings. 0 of them are added as review comments below (why?).

clang-format: pass.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.

[njames93]

• Warn extern declarations in source files

[merge_guards_bot]

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
failed: libc++.std/language_support/cmp/cmp_strongord/strongord.pass.cpp
failed: libc++.std/language_support/cmp/cmp_weakeq/cmp.weakeq.pass.cpp
failed: libc++.std/language_support/cmp/cmp_weakord/weakord.pass.cpp

clang-tidy: fail. clang-tidy found 0 errors and 1 warnings. 0 of them are added as review comments below (why?).

clang-format: pass.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.

[grandinj]

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
https://cgit.freedesktop.org/libreoffice/core/tree/compilerplugins/clang/test/external.cxx

https://cgit.freedesktop.org/libreoffice/core/tree/compilerplugins/clang/externandnotdefined.cxx

https://cgit.freedesktop.org/libreoffice/core/tree/compilerplugins/clang/externvar.cxx
https://cgit.freedesktop.org/libreoffice/core/tree/compilerplugins/clang/test/externvar.hxx
https://cgit.freedesktop.org/libreoffice/core/tree/compilerplugins/clang/test/externvar.cxx

As you can see, we are a little less organised :-)

[gribozavr2]

Thank you for the contribution! I didn't review the code thoroughly yet, only the tests.

clang-tools-extra/clang-tidy/misc/MissingHeaderFileDeclarationCheck.cpp

30

I think we should consider any file other than the main file to be a header (anything brought in by #include). Projects have lots of different convention -- for example, LLVM uses a .inc extension for some generated files and x-macro files. The standard library does not have any extensions at all (and I'm sure some projects imitate that) etc.

76

This heuristic should be a lot more complex. In practice people have more complex naming conventions (for example, in Clang, Sema.h corresponds to many files named SemaSomethingSomething.cpp).

I think there is a high chance that checking only a header with a matching name will satisfy too few projects to be worth implementing.

On the other hand, if you could use C++ or Clang modules to narrow down the list of headers where the declaration should appear, that would be a much stronger signal.

clang-tools-extra/docs/clang-tidy/checks/misc-missing-header-file-declaration.rst

15

Unclear what happens when CheckAnyHeader=0.

clang-tools-extra/test/clang-tidy/checkers/Inputs/misc-missing-header-file-declaration/misc-missing-header-file-declaration.cpp.tmp.h

23

Need a test for a template defined in a header (shouldn't produce a warning).

Need a test for an inline function defined in a header (also no warning).

Need a test for a static function defined in a header (should produce no warning, but it is bad for other reasons, however I think they are out of scope for this checker).

Need a test for an anonymous namespace in a header (no warning).

clang-tools-extra/test/clang-tidy/checkers/misc-missing-header-file-declaration.cpp

2

Need lots of tests for classes, out-of-line member declarations, member templates, out-of-line member templates, class templates, variable templates etc.

81

Need a test for a template in an anonymous namespace.

[njames93]

I'll work up those other test cases

clang-tools-extra/clang-tidy/misc/MissingHeaderFileDeclarationCheck.cpp

30

I agree in part, but there is one case that I didn't want to happen and that is when clang-tidy is ran as a text editor plugin. Often it will blindly treat the current open file as the main file which would lead to a lot of false positives, Would a better metric being matching on file extensions not corresponding to source files (c, cc, cxx, cpp)?

76

That is the reason I added the CheckAnyHeader option. For small projects the matching name would be usually be enough, but for big complex projects there is no feasible check that would work. Falling back to making sure every external definition has a declaration in at least one header will always be a good warning

[gribozavr2]

clang-tools-extra/clang-tidy/misc/MissingHeaderFileDeclarationCheck.cpp

30

I agree in part, but there is one case that I didn't want to happen and that is when clang-tidy is ran as a text editor plugin.

Does that really happen? That sounds like broken editor integration to me, because this is not how C++ without modules works...

Would a better metric being matching on file extensions not corresponding to source files (c, cc, cxx, cpp)?

Yes, I think that would be better, if we have to sniff file extensions.

76

That's the thing -- due to the lack of consistency in projects and style guides for C++, I think most projects will have to turn on CheckAnyHeader. We get implementation complexity in ClangTidy, false positives in high-profile projects, force users to configure ClangTidy to work well for their projects (unless we set CheckAnyHeader=1 by default... which then nobody would flip back to zero), and little benefit for end users.

[njames93]

clang-tools-extra/clang-tidy/misc/MissingHeaderFileDeclarationCheck.cpp

30

Well it depends on the editor and language server, I know that if you have a valid compile commands clangd will look for the best source file to compile against. But I feel that there would be some basic editors which will blindly run clang-tidy against the open file. There is also the case of header only libraries that people want to get code analysis on, not sure if they maybe create a proxy file to include the header though.

76

Would you say the best way would be check if the source file name starts with the header file name by default. Or is that very specific to Clang?

/// <SomeHeaderImpl.cpp>
#include "SomeHeader.h"

This file would check for declarations in SomeHeader.h

Or maybe check if the header file name starts with the source file?

/// <SomeHeader.cpp>
#include "SomeHeaderImpl.h"

This file would check for declarations in SomeHeaderImpl.h.
Or even check both?

[gribozavr2]

clang-tools-extra/clang-tidy/misc/MissingHeaderFileDeclarationCheck.cpp

76

Would you say the best way would be check if the source file name starts with the header file name by default. Or is that very specific to Clang?

I don't think it is too specific, it should cover many projects I think.

Or maybe check if the header file name starts with the source file?

Seems like an unusual case to me, but I'm willing to be convinced otherwise.

[njames93]

• Tweaked default corresponding header settings

[njames93]

clang-tools-extra/clang-tidy/misc/MissingHeaderFileDeclarationCheck.cpp

76

I thought it was an unusual case and wasn't thinking it would be a good idea to add. I'll just set it up so that it will always look in header files whose names appear at the start of the main source file.

[merge_guards_bot]

Unit tests: pass. 62197 tests passed, 0 failed and 815 were skipped.

clang-tidy: fail. clang-tidy found 0 errors and 1 warnings. 0 of them are added as review comments below (why?).

clang-format: pass.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.

[alexfh]

How is this different from -Wmissing-prototypes?

[njames93]

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 prototype of a function before its defined. It also checks based on linkage of the function/variable

[Eugene.Zelenko]

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 prototype specifically in the header files. missing-prototypes just ensures there is a prototype of a function before its defined. It also checks based on linkage of the function/variable

I observed a lot of cases when people cheated GCC -Wmissing-prototypes by adding prototypes in source file instead of header.

[tonyelewis]

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? I'm inclined to say it should.

clang-tools-extra/clang-tidy/misc/MissingHeaderFileDeclarationCheck.cpp

76

FWIW, my instinct is that there are two separate questions:

1. What sort of files should have their external-linkage definitions checked?
2. What sort of included files should be part of the search for adequate declarations that match those definitions?

...and that my answers, in reverse order, are:

2. All included files should be included (ie a check for a given external-linkage definition should be appeased by _any_ matching declaration in _any_ included file)
3. Only main files. And (to prevent lots of false-positives when the tool is run over headers), only "c", "cc", "cxx", "cpp" files by default but with an option to check in _all_ main files.

I think this has the merits that it:

• allows users to put their valid declaration in files with any extension they like
• defaults to only firing when run against c/cc/cxx/cpp files but provides the option to fire when run against a file with _any_ extension

So I propose that there be one option and it be as follows:

.. option:: CheckExtDefnsInAllMainFiles

If set to 0 only check external-linkage definitions in main files with typical source-file extensions ("c", "cc", "cxx", "cpp").
If set to 1 check external linkage definitions in all main files.
Default value is 0.

clang-tools-extra/docs/clang-tidy/checks/misc-missing-header-file-declaration.rst

9

I think it would be worth an extra comment here to explain to users why this situation is fishy and what they should do about it. Eg along the lines of:

Giving a symbol external linkage indicates that you intend that symbol to be usable within other translation units, suggesting there should be a declaration of the symbol in a header. No such symbol was found in this translation unit.

If you want the symbol to be accessible from other translation units, add a declaration of it to a suitable header file included as part of this translation unit.

Otherwise, give the definition internal linkage, using static or an anonymous namespace.

If you think the symbol is already declared in a file that's included as part of this translation unit, check for any mismatches between the declaration and definition, including the namespace, spelling and any arguments.

[njames93]

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: 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? I'm inclined to say it should.

Support for checking types should be either opt-in(or opt-out) but never forced. I feel like it would generate too much noise.

clang-tools-extra/clang-tidy/misc/MissingHeaderFileDeclarationCheck.cpp

76

So current behaviour is kind of explained that you can either search in headers that are a starting substring of the source file or just search for all headers, I tried to explain it in the documentation. By default it only checks for definitions in files with the extension c, cc, cpp or cxx. I could add an option to check all definitions expanded in the main file, regardless of the extension.

clang-tools-extra/docs/clang-tidy/checks/misc-missing-header-file-declaration.rst

9

I may copy that exact documentation. The current documentation is definitely needing some improvement.

[tonyelewis]

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.

clang-tools-extra/clang-tidy/misc/MissingHeaderFileDeclarationCheck.cpp

76

Yep - I think I understand the current behaviour (but could well be wrong about that of course :) ).

But IIUC, I share @gribozavr2's concerns that the current behaviour's default of only accepting matching declarations in included files that match a specific pattern is going to trigger a lot of false-positives in a lot of projects. And I don't think this check should care about where the matching declaration is found, so long as there is such a declaration in a file included within the TU.

To my thinking, the implication of having a default policy that restricts the names of files in which we'll accept matching declarations is that we're policing the way that users name and organise their files (by spamming them with lots of false-positives if they don't do it in a way that satisfies our rule). I'd argue that file naming/organisation doesn't belong as part of this check (or even as part of any other clang-tidy check).

The other half of my previous comment was about a CheckExtDefnsInAllMainFiles option but I think that that's less important and that having a restriction to c, cc, cpp and cxx seems pretty reasonable to me—it could always be broadened in the future.

Overall, I think it's better to err on the side of not triggering the warning so as to avoid excessive false-positives; the check can always be made more sensitive in the future. I see the implications of that stance as being:

• default to only checking the definitions in files that look like source files (so I think restricting to c, cc, cpp and cxx is good)
• default to accepting matching declarations in all included files.

clang-tools-extra/docs/clang-tidy/checks/misc-missing-header-file-declaration.rst

9

Great.

[njames93]

clang-tools-extra/clang-tidy/misc/MissingHeaderFileDeclarationCheck.cpp

76

The reason for the unrelated header check is to prevent potential bugs. If a declaration is in a completely unrelated header that happens to get included, it more than likely could be a collision which could then break ODR rules during linking. Perhaps a better thing to do would be if the decl isn't in a "corresponding" header, emit a warning, but say there was a decl found in example.h

[tonyelewis]

clang-tools-extra/clang-tidy/misc/MissingHeaderFileDeclarationCheck.cpp

76

Thanks.

When you say "warning", do you mean something different from a check violation? Does that mean the check isn't fired but a warning message is printed?

Either way, my feeling is that it would be better to avoid diagnosing this situation (of a definition in a source-file matching a declaration in a source file with a filename stem that isn't a leading substring of the source file's name) at all.

One minor aspect: if this ODR-motivated diagnostic message is tied to the misc-missing-header-file-declaration check, that has the potential to be confusing to users. The existing check is about one specific issue ("you said you want X to be accessible in multiple TUs but you haven't provided a declaration for other TUs to share") which IMHO would be muddied by being conflated with a quite different issue ("the names of the files containing the definition and declaration of X are quite different, which we think smells a bit like an ODR violation").

Another issue: it feels like an inappropriate thing to be enforcing. C++ gives users complete freedom about the way they name their source-code files (indeed I don't think the standard even assumes source-code comes in files) and I don't think there's anything like universally agreed upon best-practice that deviating from my_stuff.cpp / my_stuff.hpp pairs is always bad. So it feels wrong to start making life difficult for users who deviate from that pattern for any reason.

And I think the rest of my concern relates to two empirical questions:

• what fraction of the all such instances in the world's source code are actually OK (not an ODR violation)?
• what fraction of projects contain such instances?

If I was convinced these numbers were both really low, I'd feel much more comfortable with clang-tidy firing warnings/check-violations. In that world, we could say: even though it's not wrong to deviate from matching my_stuff.cpp / my_stuff.hpp pairs, it's worth complete adherence to that pattern for the resultant ODR-violation-detection gains. (Side note: another empirical question is how many ODR-violations this check would detect that aren't already caught by other ODR-violation detecting tools, eg linkers' and ASan's)

But my instinct is that those fractions are both pretty high: that lots of projects involve some file-naming structures that would fall afoul of the rule and cause lots of false positives.

In other words, my instinct is that this pattern isn't very strong evidence of an ODR violation and that the line-noise/inconvenience cost would far outweigh the ODR-detecting benefit.

So I think that the check should always accept matching declarations in any header (or at least that that should be the default policy and that users explicitly opt in to the stricter policy).

Again: thanks for all the work on this check - it's really great and I really appreciate it.

[njames93]

• Relaxed corresponding header
• Added support for tag types

[tonyelewis]

Thanks very much.

[njames93]

I thought I'd bring this back up.

• Rebased and tweaked to use a few new library features in clang-tidy since when this was first proposed.
• It definitely has value but still maybe got a little bit before its ready to land.
• Right now it will flag template definitions in source files, Is this good or bad behaviour?
• It won't flag instantiations or explicit specializations, though there should be some better tests for that.
• An option has been added to specify ImplementationFileExtensions. The check will only run if the main file matches those extensions. This is useful for editor workflows(like clangd) which run clang-tidy over a header file.

Running it under my local clangd instance has already found quite a few true positives as I go, (Probably should push a patch to fix those.)

[njames93]

Fix failing test case.

[tonyelewis]

Thank you very much for coming back to this and for bringing the changes up to date. I still think this would be really valuable.

Right now it will flag template definitions in source files, Is this good or bad behaviour?

Do you mean that both of the following would be flagged?

// a.cpp
template <typename> void f() {}
template <typename> class C {};

I'm not sure how I feel about that. It feels to me like a less clear signal that the implementation file is mistakenly out of sync with the headers. Also, I think it's better to err on the side of being conservative about what we flag at first, knowing we can always consider flagging more later on. But I'm not well enough informed to have strong feelings.

[njames93]

Removed warnings on template definitions.
Added some basic fix-it support, need some lexer magic to create the fix-it for declarations declared with extern.

[njames93]

Added fixit to remove extern.

[njames93]

Ping?

[tonyelewis]

@njames93 Thank you very much for your continued input on this. Is your ping for an automated process or a human? If the latter, is it for me or for the reviewers?

[aaron.ballman]

clang-tools-extra/clang-tidy/misc/MissingHeaderFileDeclarationCheck.cpp

35

85

88

163

clang-tidy diagnostics do not start with a capital letter, so these string literals should all be lowercase.

191

Should this be using configurationDiag() instead of llvm::errs()?

233–235

These string literals should also be lowercased.

254

clang-tools-extra/test/clang-tidy/checkers/misc-missing-header-file-declaration.cpp

13

Warning on this construct worries me -- it's not uncommon to use extern declarations in some language modes. For instance, in C, you'll still find plenty of older code bases that use an extern declaration of a system function rather than #include'ing the standard library header. Also, folks will use extern definitions as a way to smuggle data between TUs without exposing a public interface (not always the best of practices, but it is an approach to hiding implementation details).

clang-tools-extra/clang-tidy/misc/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS support)
 
 add_clang_library(clangTidyMiscModule
   DefinitionsInHeadersCheck.cpp
   MiscTidyModule.cpp
   MisplacedConstCheck.cpp
+  MissingHeaderFileDeclarationCheck.cpp
   NewDeleteOverloadsCheck.cpp
   NonCopyableObjects.cpp
   NonPrivateMemberVariablesInClassesCheck.cpp
   RedundantExpressionCheck.cpp
   StaticAssertCheck.cpp
   ThrowByValueCatchByReferenceCheck.cpp
   UnconventionalAssignOperatorCheck.cpp
   UniqueptrResetReleaseCheck.cpp

clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp

 //===--- MiscTidyModule.cpp - clang-tidy ----------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "../ClangTidy.h"
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
 #include "DefinitionsInHeadersCheck.h"
 #include "MisplacedConstCheck.h"
+#include "MissingHeaderFileDeclarationCheck.h"
 #include "NewDeleteOverloadsCheck.h"
 #include "NonCopyableObjects.h"
 #include "NonPrivateMemberVariablesInClassesCheck.h"
 #include "RedundantExpressionCheck.h"
 #include "StaticAssertCheck.h"
 #include "ThrowByValueCatchByReferenceCheck.h"
 #include "UnconventionalAssignOperatorCheck.h"
 #include "UniqueptrResetReleaseCheck.h"
 #include "UnusedAliasDeclsCheck.h"
 #include "UnusedParametersCheck.h"
 #include "UnusedUsingDeclsCheck.h"
 
 namespace clang {
 namespace tidy {
 namespace misc {
 
 class MiscModule : public ClangTidyModule {
 public:
   void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
     CheckFactories.registerCheck<DefinitionsInHeadersCheck>(
         "misc-definitions-in-headers");
     CheckFactories.registerCheck<MisplacedConstCheck>("misc-misplaced-const");
+    CheckFactories.registerCheck<MissingHeaderFileDeclarationCheck>(
+        "misc-missing-header-file-declaration");
     CheckFactories.registerCheck<NewDeleteOverloadsCheck>(
         "misc-new-delete-overloads");
     CheckFactories.registerCheck<NonCopyableObjectsCheck>(
         "misc-non-copyable-objects");
     CheckFactories.registerCheck<NonPrivateMemberVariablesInClassesCheck>(
         "misc-non-private-member-variables-in-classes");
     CheckFactories.registerCheck<RedundantExpressionCheck>(
         "misc-redundant-expression");

clang-tools-extra/clang-tidy/misc/MissingHeaderFileDeclarationCheck.h

+//===--- MissingHeaderFileDeclarationCheck.h - clang-tidy -------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_MISSINGHEADERFILEDECLARATIONCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_MISSINGHEADERFILEDECLARATIONCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+/// Flags definitions with external linkage in source files that don't have a
+/// declaration in the corresponding header file.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/misc-missing-header-file-declaration.html
+class MissingHeaderFileDeclarationCheck : public ClangTidyCheck {
+public:
+  MissingHeaderFileDeclarationCheck(StringRef Name, ClangTidyContext *Context);
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+private:
+  const bool CheckAnyHeader;
+};
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_MISSINGHEADERFILEDECLARATIONCHECK_H

clang-tools-extra/clang-tidy/misc/MissingHeaderFileDeclarationCheck.cpp

+//===--- MissingHeaderFileDeclarationCheck.cpp - clang-tidy ---------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "MissingHeaderFileDeclarationCheck.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Basic/SourceLocation.h"
+#include "llvm/ADT/Optional.h"
+#include "llvm/ADT/StringSwitch.h"
+#include <type_traits>
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+struct DecomposedFileName {
+  StringRef NamePart;
+  StringRef Extension;
+
+  bool isHeader() const {
+    return llvm::StringSwitch<bool>(Extension)
+        .CasesLower("h", "hpp", "hxx", true)
+        .Default(false);
+  }
+};
+
+llvm::Optional<DecomposedFileName> decomposeFileName(StringRef FileName) {
+  // Undecided about using llvm::sys::path
+  StringRef::size_type Pos = FileName.find_last_of("\\/");
+  if (Pos != StringRef::npos) {
+    FileName = FileName.drop_front(Pos + 1);
+  }
+  Pos = FileName.find_last_of('.');
+  if (Pos == StringRef::npos)
+    return None;
+  return DecomposedFileName{FileName.take_front(Pos),
+                            FileName.drop_front(Pos + 1)};
+}
+
+template <typename Redecl, typename DiagEmitter>
+static void checkDefinition(const Redecl &Decl, const SourceManager &SM,
+                            bool AnyHeader, StringRef PrettTypeName,
+                            DiagEmitter Diags) {
+  static_assert(std::is_base_of<Redeclarable<Redecl>, Redecl>::value,
+                "Not a Redeclarable Decl");
+  static_assert(std::is_base_of<NamedDecl, Redecl>::value, "Not a Named Decl");
+  llvm::Optional<DecomposedFileName> File =
+      decomposeFileName(SM.getFilename(Decl.getBeginLoc()));
+  if (!File || File->isHeader())
+    // File being detected as a header would occur if clang-tidy was being
+    // invoked on a header file.
+    // If the file is a header and this gets tripped, probably not a good thing
+    // either, definitions in headers is a receipe for disaster, but that't not
+    // what this specific check is looking for.
+    return;
+  for (const auto *RD : Decl.redecls()) {
+    llvm::Optional<DecomposedFileName> ThisFile =
+        decomposeFileName(SM.getFilename(RD->getBeginLoc()));
+    if (!ThisFile || !ThisFile->isHeader())
+      continue;
+    if (AnyHeader || File->NamePart.equals_lower(ThisFile->NamePart))
+      return; // Found a good candidate for matching decl
+  }
+  Diags(Decl.getLocation(),
+        "%0 %1 is defined with external linkage but has no corresponding "
+        "declaration in %select{the corresponding|any}2 header file")
+      << PrettTypeName << cast<NamedDecl>(&Decl) << static_cast<int>(AnyHeader);
+}
+
+MissingHeaderFileDeclarationCheck::MissingHeaderFileDeclarationCheck(
+    StringRef Name, ClangTidyContext *Context)
+    : ClangTidyCheck(Name, Context),
+      CheckAnyHeader(Options.get("CheckAnyHeader", false)) {}
+
+void MissingHeaderFileDeclarationCheck::storeOptions(
+    ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "CheckAnyHeader", CheckAnyHeader);
+}
+
+void MissingHeaderFileDeclarationCheck::registerMatchers(MatchFinder *Finder) {
+  if (getLangOpts().ObjC)
+    return;
+  Finder->addMatcher(varDecl(hasExternalFormalLinkage(), isDefinition(),
+                             isExpansionInMainFile(), unless(isConstexpr()))
+                         .bind("VarDecl"),
+                     this);
+  Finder->addMatcher(
+      functionDecl(
+          hasExternalFormalLinkage(), isDefinition(), isExpansionInMainFile(),
+          unless(anyOf(isConstexpr(), isInline(), cxxMethodDecl(), isMain())))
+          .bind("FunctionDecl"),
+      this);
+}
+
+void MissingHeaderFileDeclarationCheck::check(
+    const MatchFinder::MatchResult &Result) {
+  auto DiagEmitter = [&](SourceLocation Loc, StringRef Msg) {
+    return diag(Loc, Msg);
+  };
+
+  if (const auto *VD = Result.Nodes.getNodeAs<VarDecl>("VarDecl")) {
+    checkDefinition(*VD, *Result.SourceManager, CheckAnyHeader, "Variable",
+                    DiagEmitter);
+  }
+  if (const auto *FD = Result.Nodes.getNodeAs<FunctionDecl>("FunctionDecl")) {
+    checkDefinition(*FD, *Result.SourceManager, CheckAnyHeader, "Function",
+                    DiagEmitter);
+  }
+}
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang

clang-tools-extra/docs/ReleaseNotes.rst

- New :doc:`bugprone-reserved-identifier
   Checks for usages of identifiers reserved for use by the implementation.
   
 - New :doc:`cert-oop57-cpp
   <clang-tidy/checks/cert-oop57-cpp>` check.
   
   Flags use of the `C` standard library functions ``memset``, ``memcpy`` and
   ``memcmp`` and similar derivatives on non-trivial types.
 
+- New :doc:`misc-missing-header-file-declaration
+  <clang-tidy/checks/misc-missing-header-file-declaration>` check.
+
+  Flags definitions with external linkage in source files that don't have a
+  declaration in the corresponding header file
+
 New aliases
 ^^^^^^^^^^^
 
 - New alias :doc:`cert-dcl37-c
   <clang-tidy/checks/cert-dcl37-c>` to
   :doc:`bugprone-reserved-identifier
   <clang-tidy/checks/bugprone-reserved-identifier>` was added.
 

clang-tools-extra/docs/clang-tidy/checks/list.rst

.. csv-table::
    `llvm-header-guard <llvm-header-guard.html>`_,
    `llvm-include-order <llvm-include-order.html>`_, "Yes"
    `llvm-namespace-comment <llvm-namespace-comment.html>`_,
    `llvm-prefer-isa-or-dyn-cast-in-conditionals <llvm-prefer-isa-or-dyn-cast-in-conditionals.html>`_, "Yes"
    `llvm-prefer-register-over-unsigned <llvm-prefer-register-over-unsigned.html>`_, "Yes"
    `llvm-twine-local <llvm-twine-local.html>`_, "Yes"
    `misc-definitions-in-headers <misc-definitions-in-headers.html>`_, "Yes"
    `misc-misplaced-const <misc-misplaced-const.html>`_,
+   `misc-missing-header-file-declaration <misc-missing-header-file-declaration.html>`_, "Yes"
    `misc-new-delete-overloads <misc-new-delete-overloads.html>`_,
    `misc-non-copyable-objects <misc-non-copyable-objects.html>`_,
    `misc-non-private-member-variables-in-classes <misc-non-private-member-variables-in-classes.html>`_,
    `misc-redundant-expression <misc-redundant-expression.html>`_, "Yes"
    `misc-static-assert <misc-static-assert.html>`_, "Yes"
    `misc-throw-by-value-catch-by-reference <misc-throw-by-value-catch-by-reference.html>`_,
    `misc-unconventional-assign-operator <misc-unconventional-assign-operator.html>`_,
    `misc-uniqueptr-reset-release <misc-uniqueptr-reset-release.html>`_, "Yes"

clang-tools-extra/docs/clang-tidy/checks/misc-missing-header-file-declaration.rst

+.. title:: clang-tidy - misc-missing-header-file-declaration
+
+misc-missing-header-file-declaration
+====================================
+
+Flags variable and function definitions with external linkage in source files 
+that don't have a declaration in the corresponding header file. 
+
+Options
+-------
+
+.. option:: CheckAnyHeader
+   
+   If set to `1` the check will look in any header file for a matching
+   declaration.

clang-tools-extra/test/clang-tidy/checkers/Inputs/misc-missing-header-file-declaration/misc-missing-header-file-declaration.cpp.tmp.h

+#ifndef MISC_MISSING_HEADER_FILE_DECLARATON_H
+#define MISC_MISSING_HEADER_FILE_DECLARATON_H
+
+extern bool DeclInHeader;
+extern bool DeclInBoth;
+
+extern void declInHeader();
+extern void declInBoth();
+
+struct Foo {
+  static int Bar;
+};
+
+namespace ns1 {
+extern bool NS;
+extern void nS();
+} // namespace ns1
+
+namespace ns2 {
+extern bool NS;
+extern void nS();
+} // namespace ns2
+
+#endif
 No newline at end of file

clang-tools-extra/test/clang-tidy/checkers/Inputs/misc-missing-header-file-declaration/wrong_header.h

+#ifndef WRONG_HEADER
+#define WRONG_HEADER
+
+extern bool DeclInWrongHeader;
+extern void declInWrongHeader();
+
+#endif
 No newline at end of file

clang-tools-extra/test/clang-tidy/checkers/misc-missing-header-file-declaration-any-header.cpp

+// RUN: %check_clang_tidy %s misc-missing-header-file-declaration %t -- \
+// RUN: -config='{CheckOptions: \
+// RUN: [{key: misc-missing-header-file-declaration.CheckAnyHeader, value: 1}]}' \
+// RUN: -- -I%S/Inputs/misc-missing-header-file-declaration
+
+// NOTE in this file everything is actually in the wrong header file, as the
+// corresponding header is called
+// 'misc-missing-header-file-declaration-any-header.cpp.tmp.h'
+
+#include "misc-missing-header-file-declaration.cpp.tmp.h"
+// Include file has to be named *.cpp.tmp.h to account for the script renaming
+// this file to *.cpp.tmp.cpp.
+// Outside this test environment it will work normally
+// <my_file.cpp> -> <my_file.h>
+#include "wrong_header.h"
+
+// These declarations should be ignored by the check as they are in the same
+// file.
+extern bool DeclInSource;
+extern void declInSource();
+
+// These declarations should be ignored by the check as they are in the same
+// file, however there is a corresponding decl in the header that will prevent
+// a failing check.
+extern bool DeclInBoth;
+extern void declInBoth();
+
+// No external linkage so no warning
+static bool StaticOK = false;
+constexpr bool ConstexprOK = false;
+inline void inlineOK() {}
+static void staticOK() {}
+constexpr bool constexprOK() { return true; }
+
+// External linkage but decl in header so no warning
+bool DeclInHeader = false;
+bool DeclInBoth = false;
+void declInHeader() {}
+void declInBoth() {}
+
+
+//Decls don't appear in corresponding header so issue a warning
+bool DeclInSource = false;
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: Variable 'DeclInSource' is defined with external linkage
+bool NoDecl = false;
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: Variable 'NoDecl' is defined with external linkage
+void declInSource() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: Function 'declInSource' is defined with external linkage
+void noDecl() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: Function 'noDecl' is defined with external linkage
+
+// Checking any header, this is declared so no warning
+bool DeclInWrongHeader = false;
+void declInWrongHeader(){}
+
+
+// Decls in an anonymous namespace don't have external linkage, so no warning
+// should be emitted
+namespace {
+bool AnonNS = false;
+void anonNS() {}
+} // namespace
+
+// Ensure in namespace definitions are correctly resolved
+namespace ns1 {
+bool NS = false;
+void nS() {}
+} // namespace ns1
+
+// Ensure out of namespace definitions are correctly resolved
+bool /*namespace*/ns2::NS = false;
+void /*namespace*/ns2::nS() {}
+
+// Static class members declared in the header shouldn't be warned on.
+int /*struct*/Foo::Bar = 0;
+
+// main is special, don't warn for it.
+int main() {
+}
 No newline at end of file

clang-tools-extra/test/clang-tidy/checkers/misc-missing-header-file-declaration.cpp

+// RUN: %check_clang_tidy %s misc-missing-header-file-declaration %t -- \
+// RUN: -- -I%S/Inputs/misc-missing-header-file-declaration
+
+#include "misc-missing-header-file-declaration.cpp.tmp.h"
+// Include file has to be named *.cpp.tmp.h to account for the script renaming
+// this file to *.cpp.tmp.cpp.
+// Outside this test environment it will work normally
+// <my_file.cpp> -> <my_file.h>
+#include "wrong_header.h"
+
+// These declarations should be ignored by the check as they are in the same
+// file.
+extern bool DeclInSource;
+extern void declInSource();
+
+// These declarations should be ignored by the check as they are in the same
+// file, however there is a corresponding decl in the header that will prevent
+// a failing check.
+extern bool DeclInBoth;
+extern void declInBoth();
+
+// No external linkage so no warning
+static bool StaticOK = false;
+constexpr bool ConstexprOK = false;
+inline void inlineOK() {}
+static void staticOK() {}
+constexpr bool constexprOK() { return true; }
+
+// External linkage but decl in header so no warning
+bool DeclInHeader = false;
+bool DeclInBoth = false;
+void declInHeader() {}
+void declInBoth() {}
+
+
+//Decls don't appear in corresponding header so issue a warning
+bool DeclInSource = false;
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: Variable 'DeclInSource' is defined with external linkage
+bool NoDecl = false;
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: Variable 'NoDecl' is defined with external linkage
+void declInSource() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: Function 'declInSource' is defined with external linkage
+void noDecl() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: Function 'noDecl' is defined with external linkage
+
+bool DeclInWrongHeader = false;
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: Variable 'DeclInWrongHeader' is defined with external linkage
+void declInWrongHeader(){}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: Function 'declInWrongHeader' is defined with external linkage
+
+
+// Decls in an anonymous namespace don't have external linkage, so no warning
+// should be emitted
+namespace {
+bool AnonNS = false;
+void anonNS() {}
+} // namespace
+
+// Ensure in namespace definitions are correctly resolved
+namespace ns1 {
+bool NS = false;
+void nS() {}
+} // namespace ns1
+
+// Ensure out of namespace definitions are correctly resolved
+bool /*namespace*/ns2::NS = false;
+void /*namespace*/ns2::nS() {}
+
+// Static class members declared in the header shouldn't be warned on.
+int /*struct*/Foo::Bar = 0;
+
+// main is special, don't warn for it.
+int main() {
+}
 No newline at end of file