From e97766c007844498bd2ba908b785c9bb6ae12d3b Mon Sep 17 00:00:00 2001 From: Fabrice de Gans Date: Thu, 7 May 2026 14:59:22 +0200 Subject: [PATCH 1/5] Add argument to parse headers via a source's flags idt requires input to have an entry in the compilation database in order to be properly parsed. This is not always the case when parsing headers, depending on the project configuration. This change adds `--cdb-source=`. When set, idt looks up `` in the compilation database and adjusts the command line to parse the positional argument with the same flags as ``, assuming that the positional argument is a header. Fixits are gated behind the `is_in_main_tu()` helper when `--cdb-source` is set to prevent spilling of fixits outside of the positional input. The helper is a no-op otherwise, preserving existing behavior. --- Sources/idt/idt.cc | 111 ++++++++++++++++++++++++++- Tests/CDBSource.cc | 5 ++ Tests/CDBSourceMainTU.cc | 9 +++ Tests/include/CDBSource.h | 1 + Tests/include/CDBSourceMainTU.h | 2 + Tests/include/CDBSourceMainTUInner.h | 1 + 6 files changed, 127 insertions(+), 2 deletions(-) create mode 100644 Tests/CDBSource.cc create mode 100644 Tests/CDBSourceMainTU.cc create mode 100644 Tests/include/CDBSource.h create mode 100644 Tests/include/CDBSourceMainTU.h create mode 100644 Tests/include/CDBSourceMainTUInner.h diff --git a/Sources/idt/idt.cc b/Sources/idt/idt.cc index d383a91..2bac488 100644 --- a/Sources/idt/idt.cc +++ b/Sources/idt/idt.cc @@ -12,12 +12,17 @@ #include "llvm/ADT/SmallPtrSet.h" #include +#include +#include #include -#include +#include +#include #include +#include #include #include #include +#include #include namespace idt { @@ -63,6 +68,12 @@ ignored_symbols("ignore", llvm::cl::CommaSeparated, llvm::cl::cat(idt::category)); +llvm::cl::opt + cdb_source("cdb-source", + llvm::cl::desc("Path of a translation unit with an entry in the " + "compilation database."), + llvm::cl::value_desc("path"), llvm::cl::cat(idt::category)); + template bool contains(const std::set& set, const Key& key) { return set.find(key) != set.end(); @@ -271,6 +282,16 @@ class visitor : public clang::RecursiveASTVisitor { return false; } + // When `--cdb-source` is set, the positional argument is parsed as the + // translation unit's main file; restrict fixits to it so we don't + // accidentally rewrite decls in transitively-included headers. Outside + // of `--cdb-source` mode, this is a no-op. + template bool is_in_main_tu(const Decl_ *D) const { + if (cdb_source.empty()) + return true; + return source_manager_.isInMainFile(get_location(D)); + } + template inline bool is_in_system_header(const Decl_ *D) const { return source_manager_.isInSystemHeader(get_location(D)); @@ -347,6 +368,10 @@ class visitor : public clang::RecursiveASTVisitor { if (!is_in_header(FD)) return; + // Restrict to the main TU when `--cdb-source` is set. + if (!is_in_main_tu(FD)) + return; + // Ignore friend declarations. if (FD->getFriendObjectKind() != clang::Decl::FOK_None) return; @@ -431,6 +456,10 @@ class visitor : public clang::RecursiveASTVisitor { if (!is_in_header(VD)) return; + // Restrict to the main TU when `--cdb-source` is set. + if (!is_in_main_tu(VD)) + return; + // Skip local variables. We are only interested in static fields. if (VD->getParentFunctionOrMethod()) return; @@ -501,6 +530,10 @@ class visitor : public clang::RecursiveASTVisitor { if (is_in_system_header(RD)) return; + // Restrict to the main TU when `--cdb-source` is set. + if (!is_in_main_tu(RD)) + return; + // Skip exporting template classes. For fully-specialized template classes, // isTemplated() returns false so they will be annotated if needed. if (RD->isTemplated()) @@ -737,6 +770,74 @@ struct factory : clang::tooling::FrontendActionFactory { return std::make_unique(); } }; + +// CompilationDatabase wrapper that looks up flags under one path (the +// "donor") and reports them as belonging to a different path (the actual +// input). Used by `--cdb-source` for projects where headers do not appear in +// the compilation database. +class cdb_source_database : public clang::tooling::CompilationDatabase { +public: + cdb_source_database(const clang::tooling::CompilationDatabase &inner, + std::string donor) + : inner_(inner), donor_(std::move(donor)) {} + + std::vector + getCompileCommands(llvm::StringRef path) const override { + auto commands = inner_.getCompileCommands(donor_); + for (auto &command : commands) + rewrite(command, path); + return commands; + } + + // Stub these out for safety since we do not use them. + std::vector getAllFiles() const override { return {}; } + std::vector + getAllCompileCommands() const override { + return {}; + } + +private: + static void rewrite(clang::tooling::CompileCommand &command, + llvm::StringRef path) { + static constexpr std::array kSourceExts{ + ".cpp", ".cc", ".cxx", ".c++", ".C"}; + + // Replace the first source-file argument with `path`. If none is + // present, append `path` as the input. + bool swapped = false; + for (auto &arg : command.CommandLine) { + if (swapped) + break; + llvm::StringRef ref{arg}; + for (llvm::StringRef ext : kSourceExts) { + if (ref.ends_with(ext)) { + arg = path.str(); + swapped = true; + break; + } + } + } + if (!swapped) + command.CommandLine.push_back(path.str()); + + // Enforce `-x c++-header`. Replace any existing `-x ` value, or + // insert `-x c++-header` immediately after argv[0]. + auto x_it = + std::find(command.CommandLine.begin(), command.CommandLine.end(), "-x"); + if (x_it != command.CommandLine.end() && + std::next(x_it) != command.CommandLine.end()) { + *std::next(x_it) = "c++-header"; + } else if (!command.CommandLine.empty()) { + command.CommandLine.insert(std::next(command.CommandLine.begin()), + {"-x", "c++-header"}); + } + + command.Filename = path.str(); + } + + const clang::tooling::CompilationDatabase &inner_; + std::string donor_; +}; } int main(int argc, char *argv[]) { @@ -746,7 +847,13 @@ int main(int argc, char *argv[]) { CommonOptionsParser::create(argc, const_cast(argv), idt::category, llvm::cl::OneOrMore); if (options) { - ClangTool tool{options->getCompilations(), options->getSourcePathList()}; + std::unique_ptr wrapped; + CompilationDatabase *cdb = &options->getCompilations(); + if (!cdb_source.empty()) { + wrapped = std::make_unique(*cdb, cdb_source); + cdb = wrapped.get(); + } + ClangTool tool{*cdb, options->getSourcePathList()}; return tool.run(new idt::factory{}); } else { llvm::logAllUnhandledErrors(std::move(options.takeError()), llvm::errs()); diff --git a/Tests/CDBSource.cc b/Tests/CDBSource.cc new file mode 100644 index 0000000..72eef2c --- /dev/null +++ b/Tests/CDBSource.cc @@ -0,0 +1,5 @@ +// RUN: rm -rf %t && mkdir %t +// RUN: echo "[{\"directory\":\"%p\",\"command\":\"clang++ -c %s\",\"file\":\"%s\"}]" | sed -e 's/\\/\\\\/g' > %t/compile_commands.json +// RUN: %idt -p %t --cdb-source %s -export-macro IDT_TEST_ABI %S/include/CDBSource.h 2>&1 | %FileCheck %s + +// CHECK: CDBSource.h:1:1: remark: unexported public interface 'cdb_source_target' diff --git a/Tests/CDBSourceMainTU.cc b/Tests/CDBSourceMainTU.cc new file mode 100644 index 0000000..7c7a933 --- /dev/null +++ b/Tests/CDBSourceMainTU.cc @@ -0,0 +1,9 @@ +// RUN: rm -rf %t && mkdir %t +// RUN: echo "[{\"directory\":\"%p\",\"command\":\"clang++ -c %s\",\"file\":\"%s\"}]" | sed -e 's/\\/\\\\/g' > %t/compile_commands.json +// RUN: %idt -p %t --cdb-source %s -export-macro IDT_TEST_ABI %S/include/CDBSourceMainTU.h 2>&1 | %FileCheck %s + +// The outer header's decl is in the main TU and gets a fixit. +// CHECK: CDBSourceMainTU.h:2:1: remark: unexported public interface 'outer_decl' +// The inner header is transitively included but is *not* the main file, +// so its decl must be skipped. +// CHECK-NOT: CDBSourceMainTUInner.h diff --git a/Tests/include/CDBSource.h b/Tests/include/CDBSource.h new file mode 100644 index 0000000..9a50cfe --- /dev/null +++ b/Tests/include/CDBSource.h @@ -0,0 +1 @@ +int cdb_source_target(int); diff --git a/Tests/include/CDBSourceMainTU.h b/Tests/include/CDBSourceMainTU.h new file mode 100644 index 0000000..2308942 --- /dev/null +++ b/Tests/include/CDBSourceMainTU.h @@ -0,0 +1,2 @@ +#include "CDBSourceMainTUInner.h" +int outer_decl(int); diff --git a/Tests/include/CDBSourceMainTUInner.h b/Tests/include/CDBSourceMainTUInner.h new file mode 100644 index 0000000..69dd2e0 --- /dev/null +++ b/Tests/include/CDBSourceMainTUInner.h @@ -0,0 +1 @@ +int inner_decl(int); From 75094ec279709499ccd8e335b344999023dd4673 Mon Sep 17 00:00:00 2001 From: Fabrice de Gans Date: Mon, 18 May 2026 17:23:40 +0200 Subject: [PATCH 2/5] Address review comments --- Sources/idt/idt.cc | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/Sources/idt/idt.cc b/Sources/idt/idt.cc index 2bac488..93c4ba6 100644 --- a/Sources/idt/idt.cc +++ b/Sources/idt/idt.cc @@ -15,7 +15,6 @@ #include #include #include -#include #include #include #include @@ -800,7 +799,8 @@ class cdb_source_database : public clang::tooling::CompilationDatabase { static void rewrite(clang::tooling::CompileCommand &command, llvm::StringRef path) { static constexpr std::array kSourceExts{ - ".cpp", ".cc", ".cxx", ".c++", ".C"}; + ".cpp", ".cc", ".cxx", ".c++", ".C", + }; // Replace the first source-file argument with `path`. If none is // present, append `path` as the input. @@ -820,18 +820,6 @@ class cdb_source_database : public clang::tooling::CompilationDatabase { if (!swapped) command.CommandLine.push_back(path.str()); - // Enforce `-x c++-header`. Replace any existing `-x ` value, or - // insert `-x c++-header` immediately after argv[0]. - auto x_it = - std::find(command.CommandLine.begin(), command.CommandLine.end(), "-x"); - if (x_it != command.CommandLine.end() && - std::next(x_it) != command.CommandLine.end()) { - *std::next(x_it) = "c++-header"; - } else if (!command.CommandLine.empty()) { - command.CommandLine.insert(std::next(command.CommandLine.begin()), - {"-x", "c++-header"}); - } - command.Filename = path.str(); } From f064e922b53ec6194c35c1e73db535ed26265785 Mon Sep 17 00:00:00 2001 From: Fabrice de Gans Date: Thu, 21 May 2026 19:57:12 +0200 Subject: [PATCH 3/5] Rename the new parameter to `--main-file` --- Sources/idt/idt.cc | 33 ++++++++++--------- Tests/CDBSource.cc | 5 --- Tests/MainFile.cc | 5 +++ .../{CDBSourceMainTU.cc => MainFileMainTU.cc} | 6 ++-- Tests/include/CDBSource.h | 1 - Tests/include/CDBSourceMainTU.h | 2 -- Tests/include/MainFile.h | 1 + Tests/include/MainFileMainTU.h | 2 ++ ...rceMainTUInner.h => MainFileMainTUInner.h} | 0 9 files changed, 28 insertions(+), 27 deletions(-) delete mode 100644 Tests/CDBSource.cc create mode 100644 Tests/MainFile.cc rename Tests/{CDBSourceMainTU.cc => MainFileMainTU.cc} (58%) delete mode 100644 Tests/include/CDBSource.h delete mode 100644 Tests/include/CDBSourceMainTU.h create mode 100644 Tests/include/MainFile.h create mode 100644 Tests/include/MainFileMainTU.h rename Tests/include/{CDBSourceMainTUInner.h => MainFileMainTUInner.h} (100%) diff --git a/Sources/idt/idt.cc b/Sources/idt/idt.cc index 93c4ba6..f773d7b 100644 --- a/Sources/idt/idt.cc +++ b/Sources/idt/idt.cc @@ -68,10 +68,11 @@ ignored_symbols("ignore", llvm::cl::cat(idt::category)); llvm::cl::opt - cdb_source("cdb-source", - llvm::cl::desc("Path of a translation unit with an entry in the " - "compilation database."), - llvm::cl::value_desc("path"), llvm::cl::cat(idt::category)); + main_file("main-file", + llvm::cl::desc("Path of a translation unit with an entry in the " + "compilation database whose flags will be used to " + "parse the positional argument"), + llvm::cl::value_desc("path"), llvm::cl::cat(idt::category)); template bool contains(const std::set& set, const Key& key) { @@ -281,12 +282,12 @@ class visitor : public clang::RecursiveASTVisitor { return false; } - // When `--cdb-source` is set, the positional argument is parsed as the + // When `--main-file` is set, the positional argument is parsed as the // translation unit's main file; restrict fixits to it so we don't // accidentally rewrite decls in transitively-included headers. Outside - // of `--cdb-source` mode, this is a no-op. + // of `--main-file` mode, this is a no-op. template bool is_in_main_tu(const Decl_ *D) const { - if (cdb_source.empty()) + if (main_file.empty()) return true; return source_manager_.isInMainFile(get_location(D)); } @@ -367,7 +368,7 @@ class visitor : public clang::RecursiveASTVisitor { if (!is_in_header(FD)) return; - // Restrict to the main TU when `--cdb-source` is set. + // Restrict to the main TU when `--main-file` is set. if (!is_in_main_tu(FD)) return; @@ -455,7 +456,7 @@ class visitor : public clang::RecursiveASTVisitor { if (!is_in_header(VD)) return; - // Restrict to the main TU when `--cdb-source` is set. + // Restrict to the main TU when `--main-file` is set. if (!is_in_main_tu(VD)) return; @@ -529,7 +530,7 @@ class visitor : public clang::RecursiveASTVisitor { if (is_in_system_header(RD)) return; - // Restrict to the main TU when `--cdb-source` is set. + // Restrict to the main TU when `--main-file` is set. if (!is_in_main_tu(RD)) return; @@ -772,12 +773,12 @@ struct factory : clang::tooling::FrontendActionFactory { // CompilationDatabase wrapper that looks up flags under one path (the // "donor") and reports them as belonging to a different path (the actual -// input). Used by `--cdb-source` for projects where headers do not appear in +// input). Used by `--main-file` for projects where headers do not appear in // the compilation database. -class cdb_source_database : public clang::tooling::CompilationDatabase { +class main_file_database : public clang::tooling::CompilationDatabase { public: - cdb_source_database(const clang::tooling::CompilationDatabase &inner, - std::string donor) + main_file_database(const clang::tooling::CompilationDatabase &inner, + std::string donor) : inner_(inner), donor_(std::move(donor)) {} std::vector @@ -837,8 +838,8 @@ int main(int argc, char *argv[]) { if (options) { std::unique_ptr wrapped; CompilationDatabase *cdb = &options->getCompilations(); - if (!cdb_source.empty()) { - wrapped = std::make_unique(*cdb, cdb_source); + if (!main_file.empty()) { + wrapped = std::make_unique(*cdb, main_file); cdb = wrapped.get(); } ClangTool tool{*cdb, options->getSourcePathList()}; diff --git a/Tests/CDBSource.cc b/Tests/CDBSource.cc deleted file mode 100644 index 72eef2c..0000000 --- a/Tests/CDBSource.cc +++ /dev/null @@ -1,5 +0,0 @@ -// RUN: rm -rf %t && mkdir %t -// RUN: echo "[{\"directory\":\"%p\",\"command\":\"clang++ -c %s\",\"file\":\"%s\"}]" | sed -e 's/\\/\\\\/g' > %t/compile_commands.json -// RUN: %idt -p %t --cdb-source %s -export-macro IDT_TEST_ABI %S/include/CDBSource.h 2>&1 | %FileCheck %s - -// CHECK: CDBSource.h:1:1: remark: unexported public interface 'cdb_source_target' diff --git a/Tests/MainFile.cc b/Tests/MainFile.cc new file mode 100644 index 0000000..0c42c40 --- /dev/null +++ b/Tests/MainFile.cc @@ -0,0 +1,5 @@ +// RUN: rm -rf %t && mkdir %t +// RUN: echo "[{\"directory\":\"%p\",\"command\":\"clang++ -c %s\",\"file\":\"%s\"}]" | sed -e 's/\\/\\\\/g' > %t/compile_commands.json +// RUN: %idt -p %t --main-file %s -export-macro IDT_TEST_ABI %S/include/MainFile.h 2>&1 | %FileCheck %s + +// CHECK: MainFile.h:1:1: remark: unexported public interface 'main_file_target' diff --git a/Tests/CDBSourceMainTU.cc b/Tests/MainFileMainTU.cc similarity index 58% rename from Tests/CDBSourceMainTU.cc rename to Tests/MainFileMainTU.cc index 7c7a933..54935cc 100644 --- a/Tests/CDBSourceMainTU.cc +++ b/Tests/MainFileMainTU.cc @@ -1,9 +1,9 @@ // RUN: rm -rf %t && mkdir %t // RUN: echo "[{\"directory\":\"%p\",\"command\":\"clang++ -c %s\",\"file\":\"%s\"}]" | sed -e 's/\\/\\\\/g' > %t/compile_commands.json -// RUN: %idt -p %t --cdb-source %s -export-macro IDT_TEST_ABI %S/include/CDBSourceMainTU.h 2>&1 | %FileCheck %s +// RUN: %idt -p %t --main-file %s -export-macro IDT_TEST_ABI %S/include/MainFileMainTU.h 2>&1 | %FileCheck %s // The outer header's decl is in the main TU and gets a fixit. -// CHECK: CDBSourceMainTU.h:2:1: remark: unexported public interface 'outer_decl' +// CHECK: MainFileMainTU.h:2:1: remark: unexported public interface 'outer_decl' // The inner header is transitively included but is *not* the main file, // so its decl must be skipped. -// CHECK-NOT: CDBSourceMainTUInner.h +// CHECK-NOT: MainFileMainTUInner.h diff --git a/Tests/include/CDBSource.h b/Tests/include/CDBSource.h deleted file mode 100644 index 9a50cfe..0000000 --- a/Tests/include/CDBSource.h +++ /dev/null @@ -1 +0,0 @@ -int cdb_source_target(int); diff --git a/Tests/include/CDBSourceMainTU.h b/Tests/include/CDBSourceMainTU.h deleted file mode 100644 index 2308942..0000000 --- a/Tests/include/CDBSourceMainTU.h +++ /dev/null @@ -1,2 +0,0 @@ -#include "CDBSourceMainTUInner.h" -int outer_decl(int); diff --git a/Tests/include/MainFile.h b/Tests/include/MainFile.h new file mode 100644 index 0000000..7d358e9 --- /dev/null +++ b/Tests/include/MainFile.h @@ -0,0 +1 @@ +int main_file_target(int); diff --git a/Tests/include/MainFileMainTU.h b/Tests/include/MainFileMainTU.h new file mode 100644 index 0000000..f7bc8db --- /dev/null +++ b/Tests/include/MainFileMainTU.h @@ -0,0 +1,2 @@ +#include "MainFileMainTUInner.h" +int outer_decl(int); diff --git a/Tests/include/CDBSourceMainTUInner.h b/Tests/include/MainFileMainTUInner.h similarity index 100% rename from Tests/include/CDBSourceMainTUInner.h rename to Tests/include/MainFileMainTUInner.h From 79af34c788d6b341f2ee9fe93de483419c82273f Mon Sep 17 00:00:00 2001 From: Saleem Abdulrasool Date: Thu, 21 May 2026 16:28:08 -0700 Subject: [PATCH 4/5] Apply suggestions from code review Co-authored-by: Saleem Abdulrasool --- Sources/idt/idt.cc | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/Sources/idt/idt.cc b/Sources/idt/idt.cc index f773d7b..7a997b4 100644 --- a/Sources/idt/idt.cc +++ b/Sources/idt/idt.cc @@ -68,11 +68,11 @@ ignored_symbols("ignore", llvm::cl::cat(idt::category)); llvm::cl::opt - main_file("main-file", - llvm::cl::desc("Path of a translation unit with an entry in the " - "compilation database whose flags will be used to " - "parse the positional argument"), - llvm::cl::value_desc("path"), llvm::cl::cat(idt::category)); +main_file("main-file", + llvm::cl::desc("Path of a translation unit with an entry in the " + "compilation database whose flags will be used to " + "parse the positional argument"), + llvm::cl::value_desc("path"), llvm::cl::cat(idt::category)); template bool contains(const std::set& set, const Key& key) { @@ -791,6 +791,7 @@ class main_file_database : public clang::tooling::CompilationDatabase { // Stub these out for safety since we do not use them. std::vector getAllFiles() const override { return {}; } + std::vector getAllCompileCommands() const override { return {}; From 1ad6f40e2fd969cd21ff1855f4384d64d274c34a Mon Sep 17 00:00:00 2001 From: Fabrice de Gans Date: Fri, 22 May 2026 17:05:39 +0200 Subject: [PATCH 5/5] Rewrite loop --- Sources/idt/idt.cc | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/Sources/idt/idt.cc b/Sources/idt/idt.cc index 7a997b4..b68e0fe 100644 --- a/Sources/idt/idt.cc +++ b/Sources/idt/idt.cc @@ -79,6 +79,11 @@ bool contains(const std::set& set, const Key& key) { return set.find(key) != set.end(); } +template +bool contains(const std::array& arr, const T& value) { + return std::find(arr.begin(), arr.end(), value) != arr.end(); +} + const std::set &get_ignored_symbols() { static auto kIgnoredFunctions = [&]() -> std::set { return { ignored_symbols.begin(), ignored_symbols.end() }; @@ -808,15 +813,10 @@ class main_file_database : public clang::tooling::CompilationDatabase { // present, append `path` as the input. bool swapped = false; for (auto &arg : command.CommandLine) { - if (swapped) + if (contains(kSourceExts, llvm::sys::path::extension(llvm::StringRef(arg)))) { + arg = path.str(); + swapped = true; break; - llvm::StringRef ref{arg}; - for (llvm::StringRef ext : kSourceExts) { - if (ref.ends_with(ext)) { - arg = path.str(); - swapped = true; - break; - } } } if (!swapped)