kadircet created this revision.
kadircet added a reviewer: sammccall.
Herald added a subscriber: arphaman.
Herald added a project: All.
kadircet requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.
Since we redefine all macros in preamble-patch, and it's parsed after
consuming the preamble macros, we can get false missing-include diagnostics
while a fresh preamble is being rebuilt.
This patch makes sure preamble-patch is treated same as main file for
include-cleaner purposes.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D148143
Files:
clang-tools-extra/clangd/IncludeCleaner.cpp
clang-tools-extra/clangd/unittests/PreambleTests.cpp
Index: clang-tools-extra/clangd/unittests/PreambleTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/PreambleTests.cpp
+++ clang-tools-extra/clangd/unittests/PreambleTests.cpp
@@ -666,6 +666,7 @@
Config Cfg;
Cfg.Diagnostics.AllowStalePreamble = true;
Cfg.Diagnostics.UnusedIncludes = Config::IncludesPolicy::Strict;
+ Cfg.Diagnostics.MissingIncludes = Config::IncludesPolicy::Strict;
WithContextValue WithCfg(Config::Key, std::move(Cfg));
llvm::StringMap<std::string> AdditionalFiles;
@@ -699,6 +700,8 @@
{
Annotations Code("#define [[FOO]] 1\n");
// Check ranges for notes.
+ // This also makes sure we don't generate missing-include diagnostics
+ // because macros are redefined in preamble-patch.
Annotations NewCode(R"(#define BARXYZ 1
#define $foo1[[FOO]] 1
void foo();
Index: clang-tools-extra/clangd/IncludeCleaner.cpp
===================================================================
--- clang-tools-extra/clangd/IncludeCleaner.cpp
+++ clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -11,6 +11,7 @@
#include "Diagnostics.h"
#include "Headers.h"
#include "ParsedAST.h"
+#include "Preamble.h"
#include "Protocol.h"
#include "SourceCode.h"
#include "URI.h"
@@ -112,12 +113,12 @@
return false;
// Check if main file is the public interface for a private header. If so
we
// shouldn't diagnose it as unused.
- if(auto PHeader = PI->getPublic(*FE); !PHeader.empty()) {
+ if (auto PHeader = PI->getPublic(*FE); !PHeader.empty()) {
PHeader = PHeader.trim("<>\"");
// Since most private -> public mappings happen in a verbatim way, we
// check textually here. This might go wrong in presence of symlinks or
// header mappings. But that's not different than rest of the places.
- if(AST.tuPath().endswith(PHeader))
+ if (AST.tuPath().endswith(PHeader))
return false;
}
}
@@ -374,7 +375,8 @@
bool Satisfied = false;
for (const auto &H : Providers) {
if (H.kind() == include_cleaner::Header::Physical &&
- H.physical() == MainFile) {
+ (H.physical() == MainFile ||
+ H.physical()->getName().endswith(PreamblePatch::HeaderName))) {
Satisfied = true;
continue;
}
Index: clang-tools-extra/clangd/unittests/PreambleTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/PreambleTests.cpp
+++ clang-tools-extra/clangd/unittests/PreambleTests.cpp
@@ -666,6 +666,7 @@
Config Cfg;
Cfg.Diagnostics.AllowStalePreamble = true;
Cfg.Diagnostics.UnusedIncludes = Config::IncludesPolicy::Strict;
+ Cfg.Diagnostics.MissingIncludes = Config::IncludesPolicy::Strict;
WithContextValue WithCfg(Config::Key, std::move(Cfg));
llvm::StringMap<std::string> AdditionalFiles;
@@ -699,6 +700,8 @@
{
Annotations Code("#define [[FOO]] 1\n");
// Check ranges for notes.
+ // This also makes sure we don't generate missing-include diagnostics
+ // because macros are redefined in preamble-patch.
Annotations NewCode(R"(#define BARXYZ 1
#define $foo1[[FOO]] 1
void foo();
Index: clang-tools-extra/clangd/IncludeCleaner.cpp
===================================================================
--- clang-tools-extra/clangd/IncludeCleaner.cpp
+++ clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -11,6 +11,7 @@
#include "Diagnostics.h"
#include "Headers.h"
#include "ParsedAST.h"
+#include "Preamble.h"
#include "Protocol.h"
#include "SourceCode.h"
#include "URI.h"
@@ -112,12 +113,12 @@
return false;
// Check if main file is the public interface for a private header. If so we
// shouldn't diagnose it as unused.
- if(auto PHeader = PI->getPublic(*FE); !PHeader.empty()) {
+ if (auto PHeader = PI->getPublic(*FE); !PHeader.empty()) {
PHeader = PHeader.trim("<>\"");
// Since most private -> public mappings happen in a verbatim way, we
// check textually here. This might go wrong in presence of symlinks or
// header mappings. But that's not different than rest of the places.
- if(AST.tuPath().endswith(PHeader))
+ if (AST.tuPath().endswith(PHeader))
return false;
}
}
@@ -374,7 +375,8 @@
bool Satisfied = false;
for (const auto &H : Providers) {
if (H.kind() == include_cleaner::Header::Physical &&
- H.physical() == MainFile) {
+ (H.physical() == MainFile ||
+ H.physical()->getName().endswith(PreamblePatch::HeaderName))) {
Satisfied = true;
continue;
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
