[clang-tools-extra] 4190017 - [clangd] Add tests covering existing header-guard behavior. NFC
Sam McCall via cfe-commits
cfe-commits at lists.llvm.org
Tue Jul 20 05:12:36 PDT 2021
Author: Sam McCall
Date: 2021-07-20T14:12:23+02:00
New Revision: 419001724542d4b0d3423a71f0d42857af6a6227
URL: https://github.com/llvm/llvm-project/commit/419001724542d4b0d3423a71f0d42857af6a6227
DIFF: https://github.com/llvm/llvm-project/commit/419001724542d4b0d3423a71f0d42857af6a6227.diff
LOG: [clangd] Add tests covering existing header-guard behavior. NFC
A few different mechanisms here that will need some work to untangle:
- self-include in a preamble being an error even if the file is ifdef-guarded
- the is-include-guarded flag not being propagated from preamble to main ast
- preambles containing the first half on an include guard discard that info
For now just record current behavior.
Relevant to:
- https://github.com/clangd/clangd/issues/811
- https://github.com/clangd/clangd/issues/377
- https://github.com/clangd/clangd/issues/262
Differential Revision: https://reviews.llvm.org/D106201
Added:
Modified:
clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp b/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
index 5435648cd9be1..1c960c8944cbf 100644
--- a/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
+++ b/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
@@ -46,6 +46,7 @@ namespace {
using ::testing::AllOf;
using ::testing::ElementsAre;
using ::testing::ElementsAreArray;
+using ::testing::IsEmpty;
MATCHER_P(DeclNamed, Name, "") {
if (NamedDecl *ND = dyn_cast<NamedDecl>(arg))
@@ -633,6 +634,260 @@ TEST(ParsedASTTest, PatchesDeletedIncludes) {
testPath("foo.cpp"))));
}
+// Returns Code guarded by #ifndef guards
+std::string guard(llvm::StringRef Code) {
+ static int GuardID = 0;
+ std::string GuardName = ("GUARD_" + llvm::Twine(++GuardID)).str();
+ return llvm::formatv("#ifndef {0}\n#define {0}\n{1}\n#endif\n", GuardName,
+ Code);
+}
+
+std::string once(llvm::StringRef Code) {
+ return llvm::formatv("#pragma once\n{0}\n", Code);
+}
+
+bool mainIsGuarded(const ParsedAST &AST) {
+ const auto &SM = AST.getSourceManager();
+ const FileEntry *MainFE = SM.getFileEntryForID(SM.getMainFileID());
+ return AST.getPreprocessor()
+ .getHeaderSearchInfo()
+ .isFileMultipleIncludeGuarded(MainFE);
+}
+
+MATCHER_P(Diag, Desc, "") {
+ return llvm::StringRef(arg.Message).contains(Desc);
+}
+
+// Check our understanding of whether the main file is header guarded or not.
+TEST(ParsedASTTest, HeaderGuards) {
+ TestTU TU;
+ TU.ImplicitHeaderGuard = false;
+
+ TU.Code = ";";
+ EXPECT_FALSE(mainIsGuarded(TU.build()));
+
+ TU.Code = guard(";");
+ EXPECT_FALSE(mainIsGuarded(TU.build())); // FIXME: true
+
+ TU.Code = once(";");
+ EXPECT_FALSE(mainIsGuarded(TU.build())); // FIXME: true
+
+ TU.Code = R"cpp(
+ ;
+ #pragma once
+ )cpp";
+ EXPECT_FALSE(mainIsGuarded(TU.build())); // FIXME: true
+
+ TU.Code = R"cpp(
+ ;
+ #ifndef GUARD
+ #define GUARD
+ ;
+ #endif
+ )cpp";
+ EXPECT_FALSE(mainIsGuarded(TU.build()));
+}
+
+// Check our handling of files that include themselves.
+// Ideally we allow this if the file has header guards.
+//
+// Note: the semicolons (empty statements) are significant!
+// - they force the preamble to end and the body to begin. Directives can have
+//
diff erent effects in the preamble vs main file (which we try to hide).
+// - if the preamble would otherwise cover the whole file, a trailing semicolon
+// forces their sizes to be
diff erent. This is significant because the file
+// size is part of the lookup key for HeaderFileInfo, and we don't want to
+// rely on the preamble's HFI being looked up when parsing the main file.
+TEST(ParsedASTTest, HeaderGuardsSelfInclude) {
+ TestTU TU;
+ TU.ImplicitHeaderGuard = false;
+ TU.Filename = "self.h";
+
+ TU.Code = R"cpp(
+ #include "self.h" // error-ok
+ ;
+ )cpp";
+ auto AST = TU.build();
+ EXPECT_THAT(*AST.getDiagnostics(),
+ ElementsAre(Diag("recursively when building a preamble")));
+ EXPECT_FALSE(mainIsGuarded(AST));
+
+ TU.Code = R"cpp(
+ ;
+ #include "self.h" // error-ok
+ )cpp";
+ AST = TU.build();
+ EXPECT_THAT(*AST.getDiagnostics(), ElementsAre(Diag("nested too deeply")));
+ EXPECT_FALSE(mainIsGuarded(AST));
+
+ TU.Code = R"cpp(
+ #pragma once
+ #include "self.h"
+ ;
+ )cpp";
+ AST = TU.build();
+ EXPECT_THAT(*AST.getDiagnostics(), IsEmpty());
+ EXPECT_FALSE(mainIsGuarded(AST)); // FIXME: true
+
+ TU.Code = R"cpp(
+ #pragma once
+ ;
+ #include "self.h"
+ )cpp";
+ AST = TU.build();
+ EXPECT_THAT(*AST.getDiagnostics(), IsEmpty());
+ EXPECT_TRUE(mainIsGuarded(AST));
+
+ TU.Code = R"cpp(
+ ;
+ #pragma once
+ #include "self.h"
+ )cpp";
+ AST = TU.build();
+ EXPECT_THAT(*AST.getDiagnostics(), IsEmpty());
+ EXPECT_TRUE(mainIsGuarded(AST));
+
+ TU.Code = R"cpp(
+ #ifndef GUARD
+ #define GUARD
+ #include "self.h" // error-ok: FIXME, this would be nice to support
+ #endif
+ ;
+ )cpp";
+ AST = TU.build();
+ EXPECT_THAT(*AST.getDiagnostics(),
+ ElementsAre(Diag("recursively when building a preamble")));
+ EXPECT_FALSE(mainIsGuarded(AST)); // FIXME: true
+
+ TU.Code = R"cpp(
+ #ifndef GUARD
+ #define GUARD
+ ;
+ #include "self.h"
+ #endif
+ )cpp";
+ AST = TU.build();
+ EXPECT_THAT(*AST.getDiagnostics(), IsEmpty());
+ EXPECT_FALSE(mainIsGuarded(AST)); // FIXME: true
+
+ // Guarded too late...
+ TU.Code = R"cpp(
+ #include "self.h" // error-ok
+ #ifndef GUARD
+ #define GUARD
+ ;
+ #endif
+ )cpp";
+ AST = TU.build();
+ EXPECT_THAT(*AST.getDiagnostics(),
+ ElementsAre(Diag("recursively when building a preamble")));
+ EXPECT_FALSE(mainIsGuarded(AST));
+
+ TU.Code = R"cpp(
+ #include "self.h" // error-ok
+ ;
+ #ifndef GUARD
+ #define GUARD
+ #endif
+ )cpp";
+ AST = TU.build();
+ EXPECT_THAT(*AST.getDiagnostics(),
+ ElementsAre(Diag("recursively when building a preamble")));
+ EXPECT_FALSE(mainIsGuarded(AST));
+
+ TU.Code = R"cpp(
+ ;
+ #ifndef GUARD
+ #define GUARD
+ #include "self.h"
+ #endif
+ )cpp";
+ AST = TU.build();
+ EXPECT_THAT(*AST.getDiagnostics(), IsEmpty());
+ EXPECT_FALSE(mainIsGuarded(AST));
+
+ TU.Code = R"cpp(
+ #include "self.h" // error-ok
+ #pragma once
+ ;
+ )cpp";
+ AST = TU.build();
+ EXPECT_THAT(*AST.getDiagnostics(),
+ ElementsAre(Diag("recursively when building a preamble")));
+ EXPECT_FALSE(mainIsGuarded(AST)); // FIXME: true
+
+ TU.Code = R"cpp(
+ #include "self.h" // error-ok
+ ;
+ #pragma once
+ )cpp";
+ AST = TU.build();
+ EXPECT_THAT(*AST.getDiagnostics(),
+ ElementsAre(Diag("recursively when building a preamble")));
+ EXPECT_TRUE(mainIsGuarded(AST));
+}
+
+// Tests how we handle common idioms for splitting a header-only library
+// into interface and implementation files (e.g. *.h vs *.inl).
+// These files mutually include each other, and need careful handling of include
+// guards (which interact with preambles).
+TEST(ParsedASTTest, HeaderGuardsImplIface) {
+ std::string Interface = R"cpp(
+ // error-ok: we assert on diagnostics explicitly
+ template <class T> struct Traits {
+ unsigned size();
+ };
+ #include "impl.h"
+ )cpp";
+ std::string Implementation = R"cpp(
+ // error-ok: we assert on diagnostics explicitly
+ #include "iface.h"
+ template <class T> unsigned Traits<T>::size() {
+ return sizeof(T);
+ }
+ )cpp";
+
+ TestTU TU;
+ TU.ImplicitHeaderGuard = false; // We're testing include guard handling!
+ TU.ExtraArgs.push_back("-xc++-header");
+
+ // Editing the interface file, which is include guarded (easy case).
+ // We mostly get this right via PP if we don't recognize the include guard.
+ TU.Filename = "iface.h";
+ TU.Code = guard(Interface);
+ TU.AdditionalFiles = {{"impl.h", Implementation}};
+ auto AST = TU.build();
+ EXPECT_THAT(*AST.getDiagnostics(), IsEmpty());
+ EXPECT_FALSE(mainIsGuarded(AST)); // FIXME: true
+ // Slightly harder: the `#pragma once` is part of the preamble, and we
+ // need to transfer it to the main file's HeaderFileInfo.
+ TU.Code = once(Interface);
+ AST = TU.build();
+ // FIXME: empty
+ EXPECT_THAT(*AST.getDiagnostics(),
+ ElementsAre(Diag("in included file: redefinition of 'Traits'")));
+ EXPECT_TRUE(mainIsGuarded(AST));
+
+ // Editing the implementation file, which is not include guarded.
+ TU.Filename = "impl.h";
+ TU.Code = Implementation;
+ TU.AdditionalFiles = {{"iface.h", guard(Interface)}};
+ AST = TU.build();
+ // The diagnostic is unfortunate in this case, but correct per our model.
+ // Ultimately the include is skipped and the code is parsed correctly though.
+ EXPECT_THAT(*AST.getDiagnostics(),
+ ElementsAre(Diag("in included file: main file cannot be included "
+ "recursively when building a preamble")));
+ EXPECT_FALSE(mainIsGuarded(AST));
+ // Interface is pragma once guarded, same thing.
+ TU.AdditionalFiles = {{"iface.h", once(Interface)}};
+ AST = TU.build();
+ EXPECT_THAT(*AST.getDiagnostics(),
+ ElementsAre(Diag("in included file: main file cannot be included "
+ "recursively when building a preamble")));
+ EXPECT_FALSE(mainIsGuarded(AST));
+}
+
} // namespace
} // namespace clangd
} // namespace clang
More information about the cfe-commits
mailing list