Re: r356598 - Recommit "Support attribute used in member funcs of class templates"

2019-03-26 Thread Michael Spencer via cfe-commits
On Wed, Mar 20, 2019 at 12:21 PM Rafael Auler via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: rafauler
> Date: Wed Mar 20 12:22:24 2019
> New Revision: 356598
>
> URL: http://llvm.org/viewvc/llvm-project?rev=356598&view=rev
> Log:
> Recommit "Support attribute used in member funcs of class templates"
>
> This diff previously exposed a bug in LLVM's IRLinker, breaking
> buildbots that tried to self-host LLVM with monolithic LTO.
> The bug is now in LLVM by D59552
>
> Original commit message:
> As PR17480 describes, clang does not support the used attribute
> for member functions of class templates. This means that if the member
> function is not used, its definition is never instantiated. This patch
> changes clang to emit the definition if it has the used attribute.
>
> Test Plan: Added a testcase
>
> Reviewed By: aaron.ballman
>
> Differential Revision: https://reviews.llvm.org/D56928
>
> Added:
>
> cfe/trunk/test/CodeGenCXX/attr-used-member-function-implicit-instantiation.cpp
> Modified:
> cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
>
> Modified: cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp?rev=356598&r1=356597&r2=356598&view=diff
>
> ==
> --- cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp Wed Mar 20 12:22:24
> 2019
> @@ -2232,6 +2232,20 @@ TemplateDeclInstantiator::VisitCXXMethod
>  Owner->addDecl(Method);
>}
>
> +  // PR17480: Honor the used attribute to instantiate member function
> +  // definitions
> +  if (Method->hasAttr()) {
> +if (const auto *A = dyn_cast(Owner)) {
> +  SourceLocation Loc;
> +  if (const MemberSpecializationInfo *MSInfo =
> +  A->getMemberSpecializationInfo())
> +Loc = MSInfo->getPointOfInstantiation();
> +  else if (const auto *Spec =
> dyn_cast(A))
> +Loc = Spec->getPointOfInstantiation();
> +  SemaRef.MarkFunctionReferenced(Loc, Method);
> +}
> +  }
> +
>return Method;
>  }
>
>
> Added:
> cfe/trunk/test/CodeGenCXX/attr-used-member-function-implicit-instantiation.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/attr-used-member-function-implicit-instantiation.cpp?rev=356598&view=auto
>
> ==
> ---
> cfe/trunk/test/CodeGenCXX/attr-used-member-function-implicit-instantiation.cpp
> (added)
> +++
> cfe/trunk/test/CodeGenCXX/attr-used-member-function-implicit-instantiation.cpp
> Wed Mar 20 12:22:24 2019
> @@ -0,0 +1,19 @@
> +// RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm -o - %s |
> FileCheck %s
> +
> +// Check that PR17480 is fixed: __attribute__((used)) ignored in templated
> +// classes
> +namespace InstantiateUsedMemberDefinition {
> +template 
> +struct S {
> +  int __attribute__((used)) f() {
> +return 0;
> +  }
> +};
> +
> +void test() {
> +  // Check that InstantiateUsedMemberDefinition::S::f() is defined
> +  // as a result of the S class template implicit instantiation
> +  // CHECK: define linkonce_odr i32
> @_ZN31InstantiateUsedMemberDefinition1SIiE1fEv
> +  S inst;
> +}
> +} // namespace InstantiateUsedMemberDefinition
>
>
I believe this commit broke our (Apple) internal LTO bots.  I'm working on
getting more information and narrowing it down.  The symptom is:


Undefined symbols for architecture x86_64:
  "llvm::cfg::Update::dump() const", referenced from:
  DominatorTreeBatchUpdates_LegalizeDomUpdates_Test::TestBody() in
DominatorTreeBatchUpdatesTest.cpp.o
  DominatorTreeBatchUpdates_LegalizePostDomUpdates_Test::TestBody()
in DominatorTreeBatchUpdatesTest.cpp.o


Has anyone else seen a similar issue with this commit?

- Michael Spencer
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: r356598 - Recommit "Support attribute used in member funcs of class templates"

2019-03-28 Thread Michael Spencer via cfe-commits
On Tue, Mar 26, 2019 at 7:40 PM Rafael Auler  wrote:

> I’m familiar with this issue as I had to fix a memory bug in LLVM IRLinker
> that was exposed by this commit. That’s why I initially reverted it.
> However, after fixing it, I was able to do the full clang LTO self-hosting
> with lld on Linux. Is there any way I can repro this issue? It’s probably a
> bad interaction of attribute used and an optimization. See
> https://reviews.llvm.org/D59552
>

Was this with full LTO or thin LTO?  I'm still working on getting a
reproducer.  This may be related to this issue:
https://bugs.llvm.org/show_bug.cgi?id=41236

- Michael Spencer


>
>
>
>
> *From: *Michael Spencer 
> *Date: *Tuesday, March 26, 2019 at 6:59 PM
> *To: *Rafael Auler 
> *Cc: *"cfe-commits@lists.llvm.org" 
> *Subject: *Re: r356598 - Recommit "Support attribute used in member funcs
> of class templates"
>
>
>
> On Wed, Mar 20, 2019 at 12:21 PM Rafael Auler via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
> Author: rafauler
> Date: Wed Mar 20 12:22:24 2019
> New Revision: 356598
>
> URL: http://llvm.org/viewvc/llvm-project?rev=356598&view=rev
> 
> Log:
> Recommit "Support attribute used in member funcs of class templates"
>
> This diff previously exposed a bug in LLVM's IRLinker, breaking
> buildbots that tried to self-host LLVM with monolithic LTO.
> The bug is now in LLVM by D59552
>
> Original commit message:
> As PR17480 describes, clang does not support the used attribute
> for member functions of class templates. This means that if the member
> function is not used, its definition is never instantiated. This patch
> changes clang to emit the definition if it has the used attribute.
>
> Test Plan: Added a testcase
>
> Reviewed By: aaron.ballman
>
> Differential Revision: https://reviews.llvm.org/D56928
> 
>
> Added:
>
> cfe/trunk/test/CodeGenCXX/attr-used-member-function-implicit-instantiation.cpp
> Modified:
> cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
>
> Modified: cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp?rev=356598&r1=356597&r2=356598&view=diff
> 
>
> ==
> --- cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp Wed Mar 20 12:22:24
> 2019
> @@ -2232,6 +2232,20 @@ TemplateDeclInstantiator::VisitCXXMethod
>  Owner->addDecl(Method);
>}
>
> +  // PR17480: Honor the used attribute to instantiate member function
> +  // definitions
> +  if (Method->hasAttr()) {
> +if (const auto *A = dyn_cast(Owner)) {
> +  SourceLocation Loc;
> +  if (const MemberSpecializationInfo *MSInfo =
> +  A->getMemberSpecializationInfo())
> +Loc = MSInfo->getPointOfInstantiation();
> +  else if (const auto *Spec =
> dyn_cast(A))
> +Loc = Spec->getPointOfInstantiation();
> +  SemaRef.MarkFunctionReferenced(Loc, Method);
> +}
> +  }
> +
>return Method;
>  }
>
>
> Added:
> cfe/trunk/test/CodeGenCXX/attr-used-member-function-implicit-instantiation.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/attr-used-member-function-implicit-instantiation.cpp?rev=356598&view=auto
> 
>
> ==
> ---
> cfe/trunk/test/CodeGenCXX/attr-used-member-function-implicit-instantiation.cpp
> (added)
> +++
> cfe/trunk/test/CodeGenCXX/attr-used-member-function-implicit-instantiation.cpp
> Wed Mar 20 12:22:24 2019
> @@ -0,0 +1,19 @@
> +// RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm -o - %s |
> FileCheck %s

Re: r359367 - Reinstate r359059, reverted in r359361, with a fix to properly prevent

2019-09-09 Thread Michael Spencer via cfe-commits
On Fri, Apr 26, 2019 at 7:56 PM Richard Smith via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: rsmith
> Date: Fri Apr 26 19:58:17 2019
> New Revision: 359367
>
> URL: http://llvm.org/viewvc/llvm-project?rev=359367&view=rev
> Log:
> Reinstate r359059, reverted in r359361, with a fix to properly prevent
> us emitting the operand of __builtin_constant_p if it has side-effects.
>
> Original commit message:
>
> Fix interactions between __builtin_constant_p and constexpr to match
> current trunk GCC.
>
> GCC permits information from outside the operand of
> __builtin_constant_p (but in the same constant evaluation context) to be
> used within that operand; clang now does so too. A few other minor
> deviations from GCC's behavior showed up in my testing and are also
> fixed (matching GCC):
>   * Clang now supports nullptr_t as the argument type for
> __builtin_constant_p
> * Clang now returns true from __builtin_constant_p if called with a
> null pointer
> * Clang now returns true from __builtin_constant_p if called with an
> integer cast to pointer type
>
> Added:
> cfe/trunk/test/SemaCXX/builtin-constant-p.cpp
> Modified:
> cfe/trunk/lib/AST/ExprConstant.cpp
> cfe/trunk/lib/CodeGen/CGBuiltin.cpp
> cfe/trunk/lib/Sema/SemaChecking.cpp
> cfe/trunk/test/CodeGen/builtin-constant-p.c
> cfe/trunk/test/SemaCXX/enable_if.cpp
>
> Modified: cfe/trunk/lib/AST/ExprConstant.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=359367&r1=359366&r2=359367&view=diff
>
> ==
> --- cfe/trunk/lib/AST/ExprConstant.cpp (original)
> +++ cfe/trunk/lib/AST/ExprConstant.cpp Fri Apr 26 19:58:17 2019
> @@ -7801,19 +7801,33 @@ EvaluateBuiltinClassifyType(const CallEx
>  }
>
>  /// EvaluateBuiltinConstantPForLValue - Determine the result of
> -/// __builtin_constant_p when applied to the given lvalue.
> +/// __builtin_constant_p when applied to the given pointer.
>  ///
> -/// An lvalue is only "constant" if it is a pointer or reference to the
> first
> -/// character of a string literal.
> -template
> -static bool EvaluateBuiltinConstantPForLValue(const LValue &LV) {
> -  const Expr *E = LV.getLValueBase().template dyn_cast();
> -  return E && isa(E) && LV.getLValueOffset().isZero();
> +/// A pointer is only "constant" if it is null (or a pointer cast to
> integer)
> +/// or it points to the first character of a string literal.
> +static bool EvaluateBuiltinConstantPForLValue(const APValue &LV) {
> +  APValue::LValueBase Base = LV.getLValueBase();
> +  if (Base.isNull()) {
> +// A null base is acceptable.
> +return true;
> +  } else if (const Expr *E = Base.dyn_cast()) {
> +if (!isa(E))
> +  return false;
> +return LV.getLValueOffset().isZero();
> +  } else {
> +// Any other base is not constant enough for GCC.
> +return false;
> +  }
>  }
>
>  /// EvaluateBuiltinConstantP - Evaluate __builtin_constant_p as similarly
> to
>  /// GCC as we can manage.
> -static bool EvaluateBuiltinConstantP(ASTContext &Ctx, const Expr *Arg) {
> +static bool EvaluateBuiltinConstantP(EvalInfo &Info, const Expr *Arg) {
> +  // Constant-folding is always enabled for the operand of
> __builtin_constant_p
> +  // (even when the enclosing evaluation context otherwise requires a
> strict
> +  // language-specific constant expression).
> +  FoldConstant Fold(Info, true);
> +
>QualType ArgType = Arg->getType();
>
>// __builtin_constant_p always has one operand. The rules which gcc
> follows
> @@ -7821,34 +7835,30 @@ static bool EvaluateBuiltinConstantP(AST
>//
>//  - If the operand is of integral, floating, complex or enumeration
> type,
>//and can be folded to a known value of that type, it returns 1.
> -  //  - If the operand and can be folded to a pointer to the first
> character
> -  //of a string literal (or such a pointer cast to an integral type),
> it
> -  //returns 1.
> +  //  - If the operand can be folded to a pointer to the first character
> +  //of a string literal (or such a pointer cast to an integral type)
> +  //or to a null pointer or an integer cast to a pointer, it returns
> 1.
>//
>// Otherwise, it returns 0.
>//
>// FIXME: GCC also intends to return 1 for literals of aggregate types,
> but
> -  // its support for this does not currently work.
> -  if (ArgType->isIntegralOrEnumerationType()) {
> -Expr::EvalResult Result;
> -if (!Arg->EvaluateAsRValue(Result, Ctx) || Result.HasSideEffects)
> +  // its support for this did not work prior to GCC 9 and is not yet well
> +  // understood.
> +  if (ArgType->isIntegralOrEnumerationType() || ArgType->isFloatingType()
> ||
> +  ArgType->isAnyComplexType() || ArgType->isPointerType() ||
> +  ArgType->isNullPtrType()) {
> +APValue V;
> +if (!::EvaluateAsRValue(Info, Arg, V)) {
> +  Fold.keepDiagnostics();
>return false;
> + 

Re: [PATCH] D68528: [Implicit Modules] Add -cc1 option -fmodules-strict-hash which includes search paths and diagnostics.

2019-10-19 Thread Michael Spencer via cfe-commits
On Fri, Oct 18, 2019 at 7:38 PM Nico Weber via Phabricator <
revi...@reviews.llvm.org> wrote:

> thakis added a comment.
>
> Looks like this fails on win: http://45.33.8.238/win/841/step_6.txt
>
> Ptal!
>
> Maybe just cat'ing all files instead of echoing the first and piping into
> cat works?
>
>
Reverted in r375338. I'll fix it locally and recommit.

- Michael Spencer
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] 9ab6d82 - [clang-scan-deps] Add basic support for modules.

2019-10-24 Thread Michael Spencer via cfe-commits

Author: Michael Spencer
Date: 2019-10-24T16:19:11-07:00
New Revision: 9ab6d8236b176bf9dd43741f4d874a8afebed99c

URL: 
https://github.com/llvm/llvm-project/commit/9ab6d8236b176bf9dd43741f4d874a8afebed99c
DIFF: 
https://github.com/llvm/llvm-project/commit/9ab6d8236b176bf9dd43741f4d874a8afebed99c.diff

LOG: [clang-scan-deps] Add basic support for modules.

This fixes two issues that prevent simple uses of modules from working.

* We would previously minimize _every_ file opened by clang, even module maps
  and module pcm files. Now we only minimize files with known extensions. It
  would be better if we knew which files clang intended to open as a source
  file, but this works for now.

* We previously cached every lookup, even failed lookups. This is a problem
  because clang stats the module cache directory before building a module and
  creating that directory. If we cache that failure then the subsequent pcm
  load doesn't see the module cache and fails.

Overall this still leaves us building minmized modules on disk during scanning.
This will need to be improved eventually for performance, but this is correct,
and works for now.

Differential Revision: https://reviews.llvm.org/D68835

Added: 
clang/test/ClangScanDeps/Inputs/module.modulemap
clang/test/ClangScanDeps/Inputs/modules_cdb.json
clang/test/ClangScanDeps/modules.cpp

Modified: 
clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp

Removed: 




diff  --git 
a/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp 
b/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
index 7436c7256327..b4d5a29ca695 100644
--- a/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
+++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
@@ -122,6 +122,32 @@ DependencyScanningFilesystemSharedCache::get(StringRef 
Key) {
   return It.first->getValue();
 }
 
+/// Whitelist file extensions that should be minimized, treating no extension 
as
+/// a source file that should be minimized.
+///
+/// This is kinda hacky, it would be better if we knew what kind of file Clang
+/// was expecting instead.
+static bool shouldMinimize(StringRef Filename) {
+  StringRef Ext = llvm::sys::path::extension(Filename);
+  if (Ext.empty())
+return true; // C++ standard library
+  return llvm::StringSwitch(Ext)
+.CasesLower(".c", ".cc", ".cpp", ".c++", ".cxx", true)
+.CasesLower(".h", ".hh", ".hpp", ".h++", ".hxx", true)
+.CasesLower(".m", ".mm", true)
+.CasesLower(".i", ".ii", ".mi", ".mmi", true)
+.CasesLower(".def", ".inc", true)
+.Default(false);
+}
+
+
+static bool shouldCacheStatFailures(StringRef Filename) {
+  StringRef Ext = llvm::sys::path::extension(Filename);
+  if (Ext.empty())
+return false; // This may be the module cache directory.
+  return shouldMinimize(Filename); // Only cache stat failures on source files.
+}
+
 llvm::ErrorOr
 DependencyScanningWorkerFilesystem::getOrCreateFileSystemEntry(
 const StringRef Filename) {
@@ -132,7 +158,8 @@ 
DependencyScanningWorkerFilesystem::getOrCreateFileSystemEntry(
   // FIXME: Handle PCM/PCH files.
   // FIXME: Handle module map files.
 
-  bool KeepOriginalSource = IgnoredFiles.count(Filename);
+  bool KeepOriginalSource = IgnoredFiles.count(Filename) ||
+!shouldMinimize(Filename);
   DependencyScanningFilesystemSharedCache::SharedFileSystemEntry
   &SharedCacheEntry = SharedCache.get(Filename);
   const CachedFileSystemEntry *Result;
@@ -143,9 +170,16 @@ 
DependencyScanningWorkerFilesystem::getOrCreateFileSystemEntry(
 if (!CacheEntry.isValid()) {
   llvm::vfs::FileSystem &FS = getUnderlyingFS();
   auto MaybeStatus = FS.status(Filename);
-  if (!MaybeStatus)
-CacheEntry = CachedFileSystemEntry(MaybeStatus.getError());
-  else if (MaybeStatus->isDirectory())
+  if (!MaybeStatus) {
+if (!shouldCacheStatFailures(Filename))
+  // HACK: We need to always restat non source files if the stat fails.
+  //   This is because Clang first looks up the module cache and module
+  //   files before building them, and then looks for them again. If we
+  //   cache the stat failure, it won't see them the second time.
+  return MaybeStatus.getError();
+else
+  CacheEntry = CachedFileSystemEntry(MaybeStatus.getError());
+  } else if (MaybeStatus->isDirectory())
 CacheEntry = CachedFileSystemEntry::createDirectoryEntry(
 std::move(*MaybeStatus));
   else

diff  --git a/clang/test/ClangScanDeps/Inputs/module.modulemap 
b/clang/test/ClangScanDeps/Inputs/module.modulemap
new file mode 100644
index ..13171f29d69a
--- /dev/null
+++ b/clang/test/ClangScanDeps/Inputs/module.modulemap
@@ -0,0 +1,7 @@
+module header1 {
+  header "header.h"
+}
+
+module header2 {
+hea

[clang] 7af309a - [clang][DependencyScanning] clang-format.

2019-10-24 Thread Michael Spencer via cfe-commits

Author: Michael Spencer
Date: 2019-10-24T18:28:01-07:00
New Revision: 7af309a4ed1a105315a39af3c77da3a8912b8685

URL: 
https://github.com/llvm/llvm-project/commit/7af309a4ed1a105315a39af3c77da3a8912b8685
DIFF: 
https://github.com/llvm/llvm-project/commit/7af309a4ed1a105315a39af3c77da3a8912b8685.diff

LOG: [clang][DependencyScanning] clang-format.

Added: 


Modified: 
clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp

Removed: 




diff  --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp 
b/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
index 82b3ae806c65..5a6e2118a6ee 100644
--- a/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
+++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
@@ -16,8 +16,9 @@ namespace dependencies{
 DependencyScanningTool::DependencyScanningTool(DependencyScanningService 
&Service,
 const tooling::CompilationDatabase &Compilations) : Worker(Service), 
Compilations(Compilations) {}
 
-llvm::Expected DependencyScanningTool::getDependencyFile(const 
std::string &Input,
-  StringRef CWD) {
+llvm::Expected
+DependencyScanningTool::getDependencyFile(const std::string &Input,
+  StringRef CWD) {
   /// Prints out all of the gathered dependencies into a string.
   class DependencyPrinterConsumer : public DependencyConsumer {
   public:



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] 8da2056 - [clang][DependencyScanning] 80-col.

2019-10-25 Thread Michael Spencer via cfe-commits

Author: Michael Spencer
Date: 2019-10-25T15:43:57-07:00
New Revision: 8da20560ab0da11c47d4718712c9c455e71c2b51

URL: 
https://github.com/llvm/llvm-project/commit/8da20560ab0da11c47d4718712c9c455e71c2b51
DIFF: 
https://github.com/llvm/llvm-project/commit/8da20560ab0da11c47d4718712c9c455e71c2b51.diff

LOG: [clang][DependencyScanning] 80-col.

Added: 


Modified: 
clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp

Removed: 




diff  --git 
a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h 
b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
index 0c9efccb1d8b..c950cbe167cd 100644
--- a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
+++ b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
@@ -1,4 +1,4 @@
-//===- DependencyScanningTool.h - clang-scan-deps service ===//
+//===- DependencyScanningTool.h - clang-scan-deps service 
-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
@@ -26,7 +26,9 @@ class DependencyScanningTool {
   ///
   /// \param Compilations The reference to the compilation database that's
   /// used by the clang tool.
-  DependencyScanningTool(DependencyScanningService &Service, const 
clang::tooling::CompilationDatabase &Compilations);
+  DependencyScanningTool(
+  DependencyScanningService &Service,
+  const clang::tooling::CompilationDatabase &Compilations);
 
   /// Print out the dependency information into a string using the dependency
   /// file format that is specified in the options (-MD is the default) and
@@ -34,7 +36,8 @@ class DependencyScanningTool {
   ///
   /// \returns A \c StringError with the diagnostic output if clang errors
   /// occurred, dependency file contents otherwise.
-  llvm::Expected getDependencyFile(const std::string &Input, 
StringRef CWD);
+  llvm::Expected getDependencyFile(const std::string &Input,
+StringRef CWD);
 
 private:
   DependencyScanningWorker Worker;

diff  --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp 
b/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
index 5a6e2118a6ee..d2af1a9d110c 100644
--- a/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
+++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
@@ -1,4 +1,4 @@
-//===- DependencyScanningTool.cpp - clang-scan-deps service ===//
+//===- DependencyScanningTool.cpp - clang-scan-deps service 
---===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
@@ -13,8 +13,10 @@ namespace clang{
 namespace tooling{
 namespace dependencies{
 
-DependencyScanningTool::DependencyScanningTool(DependencyScanningService 
&Service,
-const tooling::CompilationDatabase &Compilations) : Worker(Service), 
Compilations(Compilations) {}
+DependencyScanningTool::DependencyScanningTool(
+DependencyScanningService &Service,
+const tooling::CompilationDatabase &Compilations)
+: Worker(Service), Compilations(Compilations) {}
 
 llvm::Expected
 DependencyScanningTool::getDependencyFile(const std::string &Input,



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] dddec1f - [clang][clang-scan-deps] Add -fcxx-modules to test for Darwin.

2019-10-28 Thread Michael Spencer via cfe-commits

Author: Michael Spencer
Date: 2019-10-28T17:37:25-07:00
New Revision: dddec1f1840b8a019c8c89dd1e7961cf39d845d3

URL: 
https://github.com/llvm/llvm-project/commit/dddec1f1840b8a019c8c89dd1e7961cf39d845d3
DIFF: 
https://github.com/llvm/llvm-project/commit/dddec1f1840b8a019c8c89dd1e7961cf39d845d3.diff

LOG: [clang][clang-scan-deps] Add -fcxx-modules to test for Darwin.

Added: 


Modified: 
clang/test/ClangScanDeps/Inputs/modules_cdb.json

Removed: 




diff  --git a/clang/test/ClangScanDeps/Inputs/modules_cdb.json 
b/clang/test/ClangScanDeps/Inputs/modules_cdb.json
index fec2df70d325..da5f9bc6a522 100644
--- a/clang/test/ClangScanDeps/Inputs/modules_cdb.json
+++ b/clang/test/ClangScanDeps/Inputs/modules_cdb.json
@@ -1,12 +1,12 @@
 [
 {
   "directory": "DIR",
-  "command": "clang -E -fsyntax-only DIR/modules_cdb_input2.cpp -IInputs -D 
INCLUDE_HEADER2 -MD -MF DIR/modules_cdb2.d -fmodules 
-fmodules-cache-path=DIR/module-cache -fimplicit-modules 
-fimplicit-module-maps",
+  "command": "clang -E -fsyntax-only DIR/modules_cdb_input2.cpp -IInputs -D 
INCLUDE_HEADER2 -MD -MF DIR/modules_cdb2.d -fmodules -fcxx-modules 
-fmodules-cache-path=DIR/module-cache -fimplicit-modules 
-fimplicit-module-maps",
   "file": "DIR/modules_cdb_input2.cpp"
 },
 {
   "directory": "DIR",
-  "command": "clang -E DIR/modules_cdb_input.cpp -IInputs -fmodules 
-fmodules-cache-path=DIR/module-cache -fimplicit-modules 
-fimplicit-module-maps",
+  "command": "clang -E DIR/modules_cdb_input.cpp -IInputs -fmodules 
-fcxx-modules -fmodules-cache-path=DIR/module-cache -fimplicit-modules 
-fimplicit-module-maps",
   "file": "DIR/modules_cdb_input.cpp"
 }
 ]



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] 7fcc1bb - [clangd] Fix the build with clang <3.9.

2020-07-17 Thread Michael Spencer via cfe-commits

Author: Michael Spencer
Date: 2020-07-17T16:12:31-06:00
New Revision: 7fcc1bb4b654461c3109b01e1fe7eae191a86f7f

URL: 
https://github.com/llvm/llvm-project/commit/7fcc1bb4b654461c3109b01e1fe7eae191a86f7f
DIFF: 
https://github.com/llvm/llvm-project/commit/7fcc1bb4b654461c3109b01e1fe7eae191a86f7f.diff

LOG: [clangd] Fix the build with clang <3.9.

In clang <3.9 the `unique_ptr` constructor that is supposed to allow
for Derived to Base conversion does not work. Remove this if we drop
support for such configurations.

This is the same fix as in fda901a987ddd, and it updates the comments
to better reflect the actual issue. The same thing reproduces with
libc++ with older clangs.

Added: 


Modified: 
clang-tools-extra/clangd/ConfigProvider.cpp
llvm/utils/TableGen/OptParserEmitter.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/ConfigProvider.cpp 
b/clang-tools-extra/clangd/ConfigProvider.cpp
index eec1ae992194..a56cdd755322 100644
--- a/clang-tools-extra/clangd/ConfigProvider.cpp
+++ b/clang-tools-extra/clangd/ConfigProvider.cpp
@@ -209,7 +209,11 @@ Provider::combine(std::vector Providers) 
{
   };
   auto Result = std::make_unique();
   Result->Providers = std::move(Providers);
-  return Result;
+  // FIXME: This is a workaround for a bug in older versions of clang (< 3.9)
+  //   The constructor that is supposed to allow for Derived to Base
+  //   conversion does not work. Remove this if we drop support for such
+  //   configurations.
+  return std::unique_ptr(Result.release());
 }
 
 Config Provider::getConfig(const Params &P, DiagnosticCallback DC) const {

diff  --git a/llvm/utils/TableGen/OptParserEmitter.cpp 
b/llvm/utils/TableGen/OptParserEmitter.cpp
index 34699b55e274..6e49e248e4b8 100644
--- a/llvm/utils/TableGen/OptParserEmitter.cpp
+++ b/llvm/utils/TableGen/OptParserEmitter.cpp
@@ -110,10 +110,10 @@ class MarshallingFlagInfo final : public 
MarshallingKindInfo {
   static std::unique_ptr create(const Record &R) {
 std::unique_ptr Ret(new MarshallingFlagInfo(R));
 Ret->IsPositive = R.getValueAsBit("IsPositive");
-// FIXME: This is a workaround for a bug in older versions of libstdc++ 
when
-//   compiled with Clang. The constructor that is supposed to allow for
-//   Derived to Base conversion does not work. Remove this if we drop
-//   support for such configurations.
+// FIXME: This is a workaround for a bug in older versions of clang (< 3.9)
+//   The constructor that is supposed to allow for Derived to Base
+//   conversion does not work. Remove this if we drop support for such
+//   configurations.
 return std::unique_ptr(Ret.release());
   }
 
@@ -208,10 +208,10 @@ struct SimpleEnumValueTable {
  "values");
 }
 
-// FIXME: This is a workaround for a bug in older versions of libstdc++ 
when
-//   compiled with Clang. The constructor that is supposed to allow for
-//   Derived to Base conversion does not work. Remove this if we drop
-//   support for such configurations.
+// FIXME: This is a workaround for a bug in older versions of clang (< 3.9)
+//   The constructor that is supposed to allow for Derived to Base
+//   conversion does not work. Remove this if we drop support for such
+//   configurations.
 return std::unique_ptr(Ret.release());
   }
 



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] 92e8af0 - [Clang] Expose RequiresNullTerminator in FileManager.

2020-04-15 Thread Michael Spencer via cfe-commits

Author: Michael Spencer
Date: 2020-04-15T14:17:51-07:00
New Revision: 92e8af0ecbe7eb36bc03a211afa9151c81b7b531

URL: 
https://github.com/llvm/llvm-project/commit/92e8af0ecbe7eb36bc03a211afa9151c81b7b531
DIFF: 
https://github.com/llvm/llvm-project/commit/92e8af0ecbe7eb36bc03a211afa9151c81b7b531.diff

LOG: [Clang] Expose RequiresNullTerminator in FileManager.

This is needed to fix the reason
0a2be46cfdb698fe (Modules: Invalidate out-of-date PCMs as they're
discovered) and 5b44a4b07fc1d ([modules] Do not cache invalid state for
modules that we attempted to load.) were reverted.

These patches changed Clang to use `isVolatile` when loading modules.
This had the side effect of not using mmap when loading modules, and
thus greatly increased memory usage.

The reason it wasn't using mmap is because `MemoryBuffer` plays some
games with file size when you request null termination, and it has to
disable these when `isVolatile` is set as the size may change by the
time it's mmapped. Clang by default passes
`RequiresNullTerminator = true`, and `shouldUseMmap` ignored if
`RequiresNullTerminator` was even requested.

This patch adds `RequiresNullTerminator` to the `FileManager` interface
so Clang can use it when loading modules, and changes `shouldUseMmap` to
only take volatility into account if `RequiresNullTerminator` is true.
This is fine as both `mmap` and a `read` loop are vulnerable to
modifying the file while reading, but are immune to the rename Clang
does when replacing a module file.

Differential Revision: https://reviews.llvm.org/D2

Added: 


Modified: 
clang/include/clang/Basic/FileManager.h
clang/lib/Basic/FileManager.cpp
llvm/lib/Support/MemoryBuffer.cpp
llvm/unittests/Support/MemoryBufferTest.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/FileManager.h 
b/clang/include/clang/Basic/FileManager.h
index fed43786d410..b481f5846936 100644
--- a/clang/include/clang/Basic/FileManager.h
+++ b/clang/include/clang/Basic/FileManager.h
@@ -378,15 +378,19 @@ class FileManager : public RefCountedBase {
   /// Open the specified file as a MemoryBuffer, returning a new
   /// MemoryBuffer if successful, otherwise returning null.
   llvm::ErrorOr>
-  getBufferForFile(const FileEntry *Entry, bool isVolatile = false);
+  getBufferForFile(const FileEntry *Entry, bool isVolatile = false,
+   bool RequiresNullTerminator = true);
   llvm::ErrorOr>
-  getBufferForFile(StringRef Filename, bool isVolatile = false) {
-return getBufferForFileImpl(Filename, /*FileSize=*/-1, isVolatile);
+  getBufferForFile(StringRef Filename, bool isVolatile = false,
+   bool RequiresNullTerminator = true) {
+return getBufferForFileImpl(Filename, /*FileSize=*/-1, isVolatile,
+RequiresNullTerminator);
   }
 
 private:
   llvm::ErrorOr>
-  getBufferForFileImpl(StringRef Filename, int64_t FileSize, bool isVolatile);
+  getBufferForFileImpl(StringRef Filename, int64_t FileSize, bool isVolatile,
+   bool RequiresNullTerminator);
 
 public:
   /// Get the 'stat' information for the given \p Path.

diff  --git a/clang/lib/Basic/FileManager.cpp b/clang/lib/Basic/FileManager.cpp
index ac8af8fcaf4a..e92e9d5911c0 100644
--- a/clang/lib/Basic/FileManager.cpp
+++ b/clang/lib/Basic/FileManager.cpp
@@ -458,7 +458,8 @@ void FileManager::fillRealPathName(FileEntry *UFE, 
llvm::StringRef FileName) {
 }
 
 llvm::ErrorOr>
-FileManager::getBufferForFile(const FileEntry *Entry, bool isVolatile) {
+FileManager::getBufferForFile(const FileEntry *Entry, bool isVolatile,
+  bool RequiresNullTerminator) {
   uint64_t FileSize = Entry->getSize();
   // If there's a high enough chance that the file have changed since we
   // got its size, force a stat before opening it.
@@ -468,28 +469,29 @@ FileManager::getBufferForFile(const FileEntry *Entry, 
bool isVolatile) {
   StringRef Filename = Entry->getName();
   // If the file is already open, use the open file descriptor.
   if (Entry->File) {
-auto Result =
-Entry->File->getBuffer(Filename, FileSize,
-   /*RequiresNullTerminator=*/true, isVolatile);
+auto Result = Entry->File->getBuffer(Filename, FileSize,
+ RequiresNullTerminator, isVolatile);
 Entry->closeFile();
 return Result;
   }
 
   // Otherwise, open the file.
-  return getBufferForFileImpl(Filename, FileSize, isVolatile);
+  return getBufferForFileImpl(Filename, FileSize, isVolatile,
+  RequiresNullTerminator);
 }
 
 llvm::ErrorOr>
 FileManager::getBufferForFileImpl(StringRef Filename, int64_t FileSize,
-  bool isVolatile) {
+  bool isVolatile,
+  bool RequiresNullTerminator) {
   if (FileSystemOpts.WorkingDir.empty())

[clang] 1727c6a - [clang] Use IsVolatile=true and RequiresNullTerminator=false for PCMs

2020-06-10 Thread Michael Spencer via cfe-commits

Author: Michael Spencer
Date: 2020-06-10T14:37:30-06:00
New Revision: 1727c6aab34012f0cefc8a3f29ede5f1f718c832

URL: 
https://github.com/llvm/llvm-project/commit/1727c6aab34012f0cefc8a3f29ede5f1f718c832
DIFF: 
https://github.com/llvm/llvm-project/commit/1727c6aab34012f0cefc8a3f29ede5f1f718c832.diff

LOG: [clang] Use IsVolatile=true and RequiresNullTerminator=false for PCMs

This change got missed while upstreaming
https://reviews.llvm.org/D2. This is the part of that change that
actually passes the correct arguments when opening a PCM.

The test didn't catch this because it starts at the
`MemoryBuffer::getOpenFile` level. It's not really possible to test
`ModuleManager::addModule` itself to verify how the file was opened.

Added: 


Modified: 
clang/lib/Serialization/ModuleManager.cpp

Removed: 




diff  --git a/clang/lib/Serialization/ModuleManager.cpp 
b/clang/lib/Serialization/ModuleManager.cpp
index 2656220f306d..a42ed2f3c179 100644
--- a/clang/lib/Serialization/ModuleManager.cpp
+++ b/clang/lib/Serialization/ModuleManager.cpp
@@ -185,7 +185,14 @@ ModuleManager::addModule(StringRef FileName, ModuleKind 
Type,
   Buf = llvm::MemoryBuffer::getSTDIN();
 } else {
   // Get a buffer of the file and close the file descriptor when done.
-  Buf = FileMgr.getBufferForFile(NewModule->File, /*isVolatile=*/false);
+  // The file is volatile because in a parallel build we expect multiple
+  // compiler processes to use the same module file rebuilding it if 
needed.
+  //
+  // RequiresNullTerminator is false because module files don't need it, 
and
+  // this allows the file to still be mmapped.
+  Buf = FileMgr.getBufferForFile(NewModule->File,
+ /*IsVolatile=*/true,
+ /*RequiresNullTerminator=*/false);
 }
 
 if (!Buf) {



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] 64788d7 - [clang] Include missing LangOpts in `getModuleHash`.

2020-07-07 Thread Michael Spencer via cfe-commits

Author: Michael Spencer
Date: 2020-07-07T17:13:23-06:00
New Revision: 64788d7d5377345af5e3080d26cb6a76c324ab5b

URL: 
https://github.com/llvm/llvm-project/commit/64788d7d5377345af5e3080d26cb6a76c324ab5b
DIFF: 
https://github.com/llvm/llvm-project/commit/64788d7d5377345af5e3080d26cb6a76c324ab5b.diff

LOG: [clang] Include missing LangOpts in `getModuleHash`.

`ObjCRuntime` and `CommentOpts.BlockCommandNames` are checked by
`ASTReader::checkLanguageOptions`, but are not part of the module
context hash. This can lead to errors when using implicit modules if
different TUs have different values for these options when using the
same module cache.

This was not hit very often due to the rare usage of
`-fblock-command-names=` and that `ObjCRuntime` is by default set by
the target triple, which is part of the existing context hash.

Added: 


Modified: 
clang/include/clang/Basic/ObjCRuntime.h
clang/lib/Frontend/CompilerInvocation.cpp
clang/test/Modules/context-hash.c
llvm/include/llvm/Support/VersionTuple.h

Removed: 




diff  --git a/clang/include/clang/Basic/ObjCRuntime.h 
b/clang/include/clang/Basic/ObjCRuntime.h
index 1c4a69269dee..26403bfa98c9 100644
--- a/clang/include/clang/Basic/ObjCRuntime.h
+++ b/clang/include/clang/Basic/ObjCRuntime.h
@@ -476,6 +476,10 @@ class ObjCRuntime {
   friend bool operator!=(const ObjCRuntime &left, const ObjCRuntime &right) {
 return !(left == right);
   }
+
+  friend llvm::hash_code hash_value(const ObjCRuntime &OCR) {
+return llvm::hash_combine(OCR.getKind(), OCR.getVersion());
+  }
 };
 
 raw_ostream &operator<<(raw_ostream &out, const ObjCRuntime &value);

diff  --git a/clang/lib/Frontend/CompilerInvocation.cpp 
b/clang/lib/Frontend/CompilerInvocation.cpp
index f58854cd9e08..6f6af917e3a3 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -3852,6 +3852,10 @@ std::string CompilerInvocation::getModuleHash() const {
   for (StringRef Feature : LangOpts->ModuleFeatures)
 code = hash_combine(code, Feature);
 
+  code = hash_combine(code, LangOpts->ObjCRuntime);
+  const auto &BCN = LangOpts->CommentOpts.BlockCommandNames;
+  code = hash_combine(code, hash_combine_range(BCN.begin(), BCN.end()));
+
   // Extend the signature with the target options.
   code = hash_combine(code, TargetOpts->Triple, TargetOpts->CPU,
   TargetOpts->ABI);

diff  --git a/clang/test/Modules/context-hash.c 
b/clang/test/Modules/context-hash.c
index 33dfb2f15a2c..8bb7422f6a54 100644
--- a/clang/test/Modules/context-hash.c
+++ b/clang/test/Modules/context-hash.c
@@ -1,3 +1,6 @@
+// This test verifies that only strict hashing includes search paths and
+// diagnostics in the module context hash.
+
 // RUN: rm -rf %t
 // RUN: %clang_cc1 -fsyntax-only -internal-isystem \
 // RUN:   %S/Inputs/System/usr/include -fmodules -fimplicit-module-maps \
@@ -20,8 +23,25 @@
 // RUN: echo %t > %t.path
 // RUN: cat %t.path %t1 %t2 %t3 %t4 | FileCheck %s
 
-// This test verifies that only strict hashing includes search paths and
-// diagnostics in the module context hash.
+// This tests things verified by ASTReader::checkLanguageOptions that are not
+// part of LangOpts.def.
+
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fsyntax-only -internal-isystem \
+// RUN:   %S/Inputs/System/usr/include -fmodules -fimplicit-module-maps \
+// RUN:   -fmodules-cache-path=%t -x objective-c %s -Rmodule-build 2> %t1
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fsyntax-only -internal-isystem \
+// RUN:   %S/Inputs/System/usr/include -fmodules -fimplicit-module-maps \
+// RUN:   -fobjc-runtime=macosx-1.0.0.0 \
+// RUN:   -fmodules-cache-path=%t -x objective-c %s -Rmodule-build 2> %t2
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fsyntax-only -internal-isystem \
+// RUN:   %S/Inputs/System/usr/include -fmodules -fimplicit-module-maps \
+// RUN:   -fcomment-block-commands=lp,bj \
+// RUN:   -fmodules-cache-path=%t -x objective-c %s -Rmodule-build 2> %t3
+// RUN: echo %t > %t.path
+// RUN: cat %t.path %t1 %t2 %t3 | FileCheck --check-prefix=LANGOPTS %s
 
 #include 
 
@@ -32,3 +52,10 @@
 // CHECK: cstd-[[AST_HASH]].pcm'
 // CHECK-NOT: building module 'cstd' as '{{.*[/\\]}}[[CONTEXT_HASH]]{{[/\\]}}
 // CHECK: cstd-[[AST_HASH]].pcm'
+
+// LANGOPTS: [[PREFIX:(.*[/\\])+[a-zA-Z0-9.-]+]]
+// LANGOPTS: building module 'cstd' as 
'[[PREFIX]]{{[/\\]}}[[CONTEXT_HASH:[A-Z0-9]+]]{{[/\\]}}cstd-[[AST_HASH:[A-Z0-9]+]].pcm'
+// LANGOPTS-NOT: building module 'cstd' as 
'{{.*[/\\]}}[[CONTEXT_HASH]]{{[/\\]}}
+// LANGOPTS: cstd-[[AST_HASH]].pcm'
+// LANGOPTS-NOT: building module 'cstd' as 
'{{.*[/\\]}}[[CONTEXT_HASH]]{{[/\\]}}
+// LANGOPTS: cstd-[[AST_HASH]].pcm'

diff  --git a/llvm/include/llvm/Support/VersionTuple.h 
b/llvm/include/llvm/Support/VersionTuple.h
index ad89e40f0f14..6f3711f06f1a 100644
--- a/llvm/include/llvm/Support/VersionTuple.h
+++ b/llvm/include/llvm/Support/VersionTuple.h
@

[llvm] [clang] [clang][DepScan] Make OptimizeArgs a bit mask enum and enable by default (PR #71588)

2023-11-07 Thread Michael Spencer via cfe-commits

https://github.com/Bigcheese created 
https://github.com/llvm/llvm-project/pull/71588

Make it easier to control which optimizations are enabled by making
OptimizeArgs a bit masked enum. There's currently only one such
optimization, but more will be added in followup commits.

>From 7d8281c6a4880006b590fd68b0c348dd0baef4eb Mon Sep 17 00:00:00 2001
From: Michael Spencer 
Date: Tue, 7 Nov 2023 10:00:59 -1000
Subject: [PATCH 1/2] [clang][DepScan] Make OptimizeArgs a bit mask enum.

Make it easier to control which optimizations are enabled by making
OptimizeArgs a bit masked enum. There's currently only one such
optimization, but more will be added in followup commits.
---
 .../DependencyScanningService.h   | 22 ++
 .../DependencyScanningWorker.h|  2 +-
 .../DependencyScanning/ModuleDepCollector.h   |  8 ---
 .../DependencyScanningService.cpp |  4 ++--
 .../DependencyScanningWorker.cpp  |  9 
 .../DependencyScanning/ModuleDepCollector.cpp |  5 ++--
 .../header-search-pruning-transitive.c|  4 ++--
 .../ClangScanDeps/header-search-pruning.cpp   |  6 ++---
 .../modules-symlink-dir-from-module.c |  2 +-
 .../test/ClangScanDeps/modules-symlink-dir.c  |  2 +-
 clang/tools/clang-scan-deps/ClangScanDeps.cpp | 23 +--
 clang/tools/clang-scan-deps/Opts.td   |  2 +-
 llvm/include/llvm/ADT/BitmaskEnum.h   |  8 ++-
 13 files changed, 69 insertions(+), 28 deletions(-)

diff --git 
a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h 
b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h
index 109cf049a65231e..60daeec6fe2a678 100644
--- a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h
+++ b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h
@@ -10,6 +10,7 @@
 #define LLVM_CLANG_TOOLING_DEPENDENCYSCANNING_DEPENDENCYSCANNINGSERVICE_H
 
 #include "clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h"
+#include "llvm/ADT/BitmaskEnum.h"
 
 namespace clang {
 namespace tooling {
@@ -44,19 +45,30 @@ enum class ScanningOutputFormat {
   P1689,
 };
 
+enum class ScanningOptimizations {
+  None = 0,
+
+  /// Remove unused header search paths including header maps.
+  HeaderSearch = 1,
+
+  LLVM_MARK_AS_BITMASK_ENUM(HeaderSearch),
+  All = HeaderSearch
+};
+
 /// The dependency scanning service contains shared configuration and state 
that
 /// is used by the individual dependency scanning workers.
 class DependencyScanningService {
 public:
-  DependencyScanningService(ScanningMode Mode, ScanningOutputFormat Format,
-bool OptimizeArgs = false,
-bool EagerLoadModules = false);
+  DependencyScanningService(
+  ScanningMode Mode, ScanningOutputFormat Format,
+  ScanningOptimizations OptimizeArgs = ScanningOptimizations::None,
+  bool EagerLoadModules = false);
 
   ScanningMode getMode() const { return Mode; }
 
   ScanningOutputFormat getFormat() const { return Format; }
 
-  bool canOptimizeArgs() const { return OptimizeArgs; }
+  ScanningOptimizations getOptimizeArgs() const { return OptimizeArgs; }
 
   bool shouldEagerLoadModules() const { return EagerLoadModules; }
 
@@ -68,7 +80,7 @@ class DependencyScanningService {
   const ScanningMode Mode;
   const ScanningOutputFormat Format;
   /// Whether to optimize the modules' command-line arguments.
-  const bool OptimizeArgs;
+  const ScanningOptimizations OptimizeArgs;
   /// Whether to set up command-lines to load PCM files eagerly.
   const bool EagerLoadModules;
   /// The global file system cache.
diff --git 
a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h 
b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
index 778d579bfb50154..0f607862194b316 100644
--- a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
+++ b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
@@ -116,7 +116,7 @@ class DependencyScanningWorker {
   llvm::IntrusiveRefCntPtr DepFS;
   ScanningOutputFormat Format;
   /// Whether to optimize the modules' command-line arguments.
-  bool OptimizeArgs;
+  ScanningOptimizations OptimizeArgs;
   /// Whether to set up command-lines to load PCM files eagerly.
   bool EagerLoadModules;
 };
diff --git 
a/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h 
b/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
index f5dbb26452da4e6..051363b075de997 100644
--- a/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
+++ b/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
@@ -16,6 +16,7 @@
 #include "clang/Lex/HeaderSearch.h"
 #include "clang/Lex/PPCallbacks.h"
 #include "clang/Serialization/ASTReader.h"
+#include "clang/Tooling/DependencyScanning/DependencyScanningService.h"
 #include "llvm/ADT/DenseMap.h"
 #in

[clang] [llvm] [clang][DepScan] Make OptimizeArgs a bit mask enum and enable by default (PR #71588)

2023-11-07 Thread Michael Spencer via cfe-commits

https://github.com/Bigcheese closed 
https://github.com/llvm/llvm-project/pull/71588
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][DependencyScanner] Remove all warning flags when suppressing warnings (PR #71612)

2023-11-07 Thread Michael Spencer via cfe-commits

https://github.com/Bigcheese created 
https://github.com/llvm/llvm-project/pull/71612

Since system modules don't emit most warnings, remove the warning flags to 
increase module reuse.

>From 6c8c4cde5698a1693ad64a14a156030f249032a6 Mon Sep 17 00:00:00 2001
From: Michael Spencer 
Date: Tue, 7 Nov 2023 14:02:38 -1000
Subject: [PATCH] [clang][DependencyScanner] Remove all warning flags when
 suppressing warnings

Since system modules don't emit most warnings, remove the warning
flags to increase module reuse.
---
 .../DependencyScanningService.h   |  7 +-
 .../DependencyScanning/ModuleDepCollector.cpp | 26 ++
 .../ClangScanDeps/optimize-system-warnings.m  | 84 +++
 clang/tools/clang-scan-deps/ClangScanDeps.cpp |  1 +
 4 files changed, 116 insertions(+), 2 deletions(-)
 create mode 100644 clang/test/ClangScanDeps/optimize-system-warnings.m

diff --git 
a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h 
b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h
index a4a03b88b205175..dcdf1c171f6d731 100644
--- a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h
+++ b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h
@@ -51,8 +51,11 @@ enum class ScanningOptimizations {
   /// Remove unused header search paths including header maps.
   HeaderSearch = 1,
 
-  LLVM_MARK_AS_BITMASK_ENUM(HeaderSearch),
-  All = HeaderSearch,
+  /// Remove warnings from system modules.
+  SystemWarnings = 2,
+
+  LLVM_MARK_AS_BITMASK_ENUM(SystemWarnings),
+  All = HeaderSearch | SystemWarnings,
   Default = All
 };
 
diff --git a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp 
b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
index 5d95bb305bc38d8..9099c18391e4d29 100644
--- a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
+++ b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
@@ -52,6 +52,28 @@ static void optimizeHeaderSearchOpts(HeaderSearchOptions 
&Opts,
 Opts.UserEntries.push_back(Entries[Idx]);
 }
 
+static void optimizeDiagnosticOpts(DiagnosticOptions &Opts,
+   bool IsSystemModule) {
+  // If this is not a system module or -Wsystem-headers was passed, don't
+  // optimize.
+  if (!IsSystemModule)
+return;
+  bool Wsystem_headers = false;
+  for (StringRef Opt : Opts.Warnings) {
+bool isPositive = !Opt.consume_front("no-");
+if (Opt == "system-headers")
+  Wsystem_headers = isPositive;
+  }
+  if (Wsystem_headers)
+return;
+
+  // Remove all warning flags. System modules suppress most, but not all,
+  // warnings.
+  Opts.Warnings.clear();
+  Opts.UndefPrefixes.clear();
+  Opts.Remarks.clear();
+}
+
 static std::vector splitString(std::string S, char Separator) {
   SmallVector Segments;
   StringRef(S).split(Segments, Separator, /*MaxSplit=*/-1, 
/*KeepEmpty=*/false);
@@ -532,6 +554,10 @@ ModuleDepCollectorPP::handleTopLevelModule(const Module 
*M) {
 if (any(MDC.OptimizeArgs & ScanningOptimizations::HeaderSearch))
   
optimizeHeaderSearchOpts(BuildInvocation.getMutHeaderSearchOpts(),
*MDC.ScanInstance.getASTReader(), *MF);
+if (any(MDC.OptimizeArgs & ScanningOptimizations::SystemWarnings))
+  optimizeDiagnosticOpts(
+  BuildInvocation.getMutDiagnosticOpts(),
+  BuildInvocation.getFrontendOpts().IsSystemModule);
   });
 
   MDC.associateWithContextHash(CI, MD);
diff --git a/clang/test/ClangScanDeps/optimize-system-warnings.m 
b/clang/test/ClangScanDeps/optimize-system-warnings.m
new file mode 100644
index 000..d61724c6f18fff8
--- /dev/null
+++ b/clang/test/ClangScanDeps/optimize-system-warnings.m
@@ -0,0 +1,84 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: sed -e "s|DIR|%/t|g" %t/build/compile-commands.json.in > 
%t/build/compile-commands.json
+// RUN: clang-scan-deps -compilation-database %t/build/compile-commands.json \
+// RUN:   -j 1 -format experimental-full -optimize-args=system-warnings > 
%t/deps.db
+// RUN: cat %t/deps.db | sed 's:\?:/:g' | FileCheck %s -DPREFIX=%/t
+
+// CHECK:  {
+// CHECK-NEXT:   "modules": [
+// CHECK-NEXT: {
+// CHECK-NEXT:   "clang-module-deps": [],
+// CHECK-NEXT:   "clang-modulemap-file":
+// CHECK-NEXT:   "command-line": [
+// CHECK-NOT:  "-W
+// CHECK:],
+// CHECK-NEXT:   "context-hash": "{{.*}}",
+// CHECK-NEXT:   "file-deps": [
+// CHECK:],
+// CHECK-NEXT:   "name": "A"
+// CHECK-NEXT: },
+// CHECK-NEXT: {
+// CHECK-NEXT:   "clang-module-deps": [],
+// CHECK-NEXT:   "clang-modulemap-file":
+// CHECK-NEXT:   "command-line": [
+// CHECK-NOT:  "-W
+// CHECK:],
+// CHECK-NEXT:   "context-hash": "{{.*}}",
+// CHECK-NEXT:   "file-deps": [
+// CHECK:],
+// CHECK-NEXT:   "name": "B"
+// C

[clang] [clang][DependencyScanner] Remove all warning flags when suppressing warnings (PR #71612)

2023-11-08 Thread Michael Spencer via cfe-commits


@@ -52,6 +52,28 @@ static void optimizeHeaderSearchOpts(HeaderSearchOptions 
&Opts,
 Opts.UserEntries.push_back(Entries[Idx]);
 }
 
+static void optimizeDiagnosticOpts(DiagnosticOptions &Opts,
+   bool IsSystemModule) {
+  // If this is not a system module or -Wsystem-headers was passed, don't
+  // optimize.
+  if (!IsSystemModule)
+return;
+  bool Wsystem_headers = false;
+  for (StringRef Opt : Opts.Warnings) {
+bool isPositive = !Opt.consume_front("no-");
+if (Opt == "system-headers")
+  Wsystem_headers = isPositive;
+  }
+  if (Wsystem_headers)
+return;

Bigcheese wrote:

I don't really see a good way to reuse that, it's doing a bunch of other work 
on the `DiagnosticsEngine`, and the loop itself wants to process `no-` once and 
then check a bunch of different warning flags.

https://github.com/llvm/llvm-project/pull/71612
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][DependencyScanner] Remove all warning flags when suppressing warnings (PR #71612)

2023-11-08 Thread Michael Spencer via cfe-commits


@@ -0,0 +1,84 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: sed -e "s|DIR|%/t|g" %t/build/compile-commands.json.in > 
%t/build/compile-commands.json
+// RUN: clang-scan-deps -compilation-database %t/build/compile-commands.json \
+// RUN:   -j 1 -format experimental-full -optimize-args=system-warnings > 
%t/deps.db
+// RUN: cat %t/deps.db | sed 's:\?:/:g' | FileCheck %s -DPREFIX=%/t
+
+// CHECK:  {
+// CHECK-NEXT:   "modules": [
+// CHECK-NEXT: {
+// CHECK-NEXT:   "clang-module-deps": [],
+// CHECK-NEXT:   "clang-modulemap-file":
+// CHECK-NEXT:   "command-line": [
+// CHECK-NOT:  "-W
+// CHECK:],
+// CHECK-NEXT:   "context-hash": "{{.*}}",
+// CHECK-NEXT:   "file-deps": [
+// CHECK:],
+// CHECK-NEXT:   "name": "A"
+// CHECK-NEXT: },
+// CHECK-NEXT: {
+// CHECK-NEXT:   "clang-module-deps": [],
+// CHECK-NEXT:   "clang-modulemap-file":
+// CHECK-NEXT:   "command-line": [
+// CHECK-NOT:  "-W
+// CHECK:],
+// CHECK-NEXT:   "context-hash": "{{.*}}",
+// CHECK-NEXT:   "file-deps": [
+// CHECK:],
+// CHECK-NEXT:   "name": "B"
+// CHECK-NEXT: }
+// CHECK-NEXT:   ],
+// CHECK-NEXT:   "translation-units": [
+// CHECK:]
+// CHECK:  }
+
+//--- build/compile-commands.json.in
+
+[
+{
+  "directory": "DIR",
+  "command": "clang -c DIR/A.m -isystem modules/A -I modules/B -fmodules 
-fmodules-cache-path=DIR/module-cache -fimplicit-module-maps",

Bigcheese wrote:

I believe this was checking that context hash's matched, but we changed how 
those work so they no longer should. Module A can be removed.

https://github.com/llvm/llvm-project/pull/71612
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][DependencyScanner] Remove all warning flags when suppressing warnings (PR #71612)

2023-11-08 Thread Michael Spencer via cfe-commits


@@ -0,0 +1,84 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: sed -e "s|DIR|%/t|g" %t/build/compile-commands.json.in > 
%t/build/compile-commands.json
+// RUN: clang-scan-deps -compilation-database %t/build/compile-commands.json \
+// RUN:   -j 1 -format experimental-full -optimize-args=system-warnings > 
%t/deps.db
+// RUN: cat %t/deps.db | sed 's:\?:/:g' | FileCheck %s -DPREFIX=%/t
+
+// CHECK:  {
+// CHECK-NEXT:   "modules": [
+// CHECK-NEXT: {
+// CHECK-NEXT:   "clang-module-deps": [],
+// CHECK-NEXT:   "clang-modulemap-file":
+// CHECK-NEXT:   "command-line": [
+// CHECK-NOT:  "-W
+// CHECK:],
+// CHECK-NEXT:   "context-hash": "{{.*}}",
+// CHECK-NEXT:   "file-deps": [
+// CHECK:],
+// CHECK-NEXT:   "name": "A"
+// CHECK-NEXT: },
+// CHECK-NEXT: {
+// CHECK-NEXT:   "clang-module-deps": [],
+// CHECK-NEXT:   "clang-modulemap-file":
+// CHECK-NEXT:   "command-line": [
+// CHECK-NOT:  "-W
+// CHECK:],
+// CHECK-NEXT:   "context-hash": "{{.*}}",
+// CHECK-NEXT:   "file-deps": [
+// CHECK:],
+// CHECK-NEXT:   "name": "B"
+// CHECK-NEXT: }
+// CHECK-NEXT:   ],
+// CHECK-NEXT:   "translation-units": [
+// CHECK:]
+// CHECK:  }
+
+//--- build/compile-commands.json.in
+
+[
+{
+  "directory": "DIR",
+  "command": "clang -c DIR/A.m -isystem modules/A -I modules/B -fmodules 
-fmodules-cache-path=DIR/module-cache -fimplicit-module-maps",

Bigcheese wrote:

Although I think I also misunderstood your question. Scanning both `A.m` and 
`B.m` verifies that the warning gets dropped and the hashes match due to there 
only being one module variant.

https://github.com/llvm/llvm-project/pull/71612
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][DependencyScanner] Remove all warning flags when suppressing warnings (PR #71612)

2023-11-08 Thread Michael Spencer via cfe-commits


@@ -0,0 +1,84 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: sed -e "s|DIR|%/t|g" %t/build/compile-commands.json.in > 
%t/build/compile-commands.json
+// RUN: clang-scan-deps -compilation-database %t/build/compile-commands.json \
+// RUN:   -j 1 -format experimental-full -optimize-args=system-warnings > 
%t/deps.db
+// RUN: cat %t/deps.db | sed 's:\?:/:g' | FileCheck %s -DPREFIX=%/t
+
+// CHECK:  {
+// CHECK-NEXT:   "modules": [
+// CHECK-NEXT: {
+// CHECK-NEXT:   "clang-module-deps": [],
+// CHECK-NEXT:   "clang-modulemap-file":
+// CHECK-NEXT:   "command-line": [
+// CHECK-NOT:  "-W
+// CHECK:],
+// CHECK-NEXT:   "context-hash": "{{.*}}",
+// CHECK-NEXT:   "file-deps": [
+// CHECK:],
+// CHECK-NEXT:   "name": "A"
+// CHECK-NEXT: },
+// CHECK-NEXT: {
+// CHECK-NEXT:   "clang-module-deps": [],
+// CHECK-NEXT:   "clang-modulemap-file":
+// CHECK-NEXT:   "command-line": [
+// CHECK-NOT:  "-W
+// CHECK:],
+// CHECK-NEXT:   "context-hash": "{{.*}}",
+// CHECK-NEXT:   "file-deps": [
+// CHECK:],
+// CHECK-NEXT:   "name": "B"
+// CHECK-NEXT: }
+// CHECK-NEXT:   ],
+// CHECK-NEXT:   "translation-units": [
+// CHECK:]
+// CHECK:  }
+
+//--- build/compile-commands.json.in
+
+[
+{
+  "directory": "DIR",
+  "command": "clang -c DIR/A.m -isystem modules/A -I modules/B -fmodules 
-fmodules-cache-path=DIR/module-cache -fimplicit-module-maps",

Bigcheese wrote:

Ah, I missed the `-isystem`. It's verifying that both `-isystem` and `[system]` 
work.

https://github.com/llvm/llvm-project/pull/71612
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][DependencyScanner] Remove all warning flags when suppressing warnings (PR #71612)

2023-11-08 Thread Michael Spencer via cfe-commits

https://github.com/Bigcheese edited 
https://github.com/llvm/llvm-project/pull/71612
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][DependencyScanner] Remove all warning flags when suppressing warnings (PR #71612)

2023-11-08 Thread Michael Spencer via cfe-commits

https://github.com/Bigcheese updated 
https://github.com/llvm/llvm-project/pull/71612

>From 6c8c4cde5698a1693ad64a14a156030f249032a6 Mon Sep 17 00:00:00 2001
From: Michael Spencer 
Date: Tue, 7 Nov 2023 14:02:38 -1000
Subject: [PATCH 1/2] [clang][DependencyScanner] Remove all warning flags when
 suppressing warnings

Since system modules don't emit most warnings, remove the warning
flags to increase module reuse.
---
 .../DependencyScanningService.h   |  7 +-
 .../DependencyScanning/ModuleDepCollector.cpp | 26 ++
 .../ClangScanDeps/optimize-system-warnings.m  | 84 +++
 clang/tools/clang-scan-deps/ClangScanDeps.cpp |  1 +
 4 files changed, 116 insertions(+), 2 deletions(-)
 create mode 100644 clang/test/ClangScanDeps/optimize-system-warnings.m

diff --git 
a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h 
b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h
index a4a03b88b205175..dcdf1c171f6d731 100644
--- a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h
+++ b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h
@@ -51,8 +51,11 @@ enum class ScanningOptimizations {
   /// Remove unused header search paths including header maps.
   HeaderSearch = 1,
 
-  LLVM_MARK_AS_BITMASK_ENUM(HeaderSearch),
-  All = HeaderSearch,
+  /// Remove warnings from system modules.
+  SystemWarnings = 2,
+
+  LLVM_MARK_AS_BITMASK_ENUM(SystemWarnings),
+  All = HeaderSearch | SystemWarnings,
   Default = All
 };
 
diff --git a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp 
b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
index 5d95bb305bc38d8..9099c18391e4d29 100644
--- a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
+++ b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
@@ -52,6 +52,28 @@ static void optimizeHeaderSearchOpts(HeaderSearchOptions 
&Opts,
 Opts.UserEntries.push_back(Entries[Idx]);
 }
 
+static void optimizeDiagnosticOpts(DiagnosticOptions &Opts,
+   bool IsSystemModule) {
+  // If this is not a system module or -Wsystem-headers was passed, don't
+  // optimize.
+  if (!IsSystemModule)
+return;
+  bool Wsystem_headers = false;
+  for (StringRef Opt : Opts.Warnings) {
+bool isPositive = !Opt.consume_front("no-");
+if (Opt == "system-headers")
+  Wsystem_headers = isPositive;
+  }
+  if (Wsystem_headers)
+return;
+
+  // Remove all warning flags. System modules suppress most, but not all,
+  // warnings.
+  Opts.Warnings.clear();
+  Opts.UndefPrefixes.clear();
+  Opts.Remarks.clear();
+}
+
 static std::vector splitString(std::string S, char Separator) {
   SmallVector Segments;
   StringRef(S).split(Segments, Separator, /*MaxSplit=*/-1, 
/*KeepEmpty=*/false);
@@ -532,6 +554,10 @@ ModuleDepCollectorPP::handleTopLevelModule(const Module 
*M) {
 if (any(MDC.OptimizeArgs & ScanningOptimizations::HeaderSearch))
   
optimizeHeaderSearchOpts(BuildInvocation.getMutHeaderSearchOpts(),
*MDC.ScanInstance.getASTReader(), *MF);
+if (any(MDC.OptimizeArgs & ScanningOptimizations::SystemWarnings))
+  optimizeDiagnosticOpts(
+  BuildInvocation.getMutDiagnosticOpts(),
+  BuildInvocation.getFrontendOpts().IsSystemModule);
   });
 
   MDC.associateWithContextHash(CI, MD);
diff --git a/clang/test/ClangScanDeps/optimize-system-warnings.m 
b/clang/test/ClangScanDeps/optimize-system-warnings.m
new file mode 100644
index 000..d61724c6f18fff8
--- /dev/null
+++ b/clang/test/ClangScanDeps/optimize-system-warnings.m
@@ -0,0 +1,84 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: sed -e "s|DIR|%/t|g" %t/build/compile-commands.json.in > 
%t/build/compile-commands.json
+// RUN: clang-scan-deps -compilation-database %t/build/compile-commands.json \
+// RUN:   -j 1 -format experimental-full -optimize-args=system-warnings > 
%t/deps.db
+// RUN: cat %t/deps.db | sed 's:\?:/:g' | FileCheck %s -DPREFIX=%/t
+
+// CHECK:  {
+// CHECK-NEXT:   "modules": [
+// CHECK-NEXT: {
+// CHECK-NEXT:   "clang-module-deps": [],
+// CHECK-NEXT:   "clang-modulemap-file":
+// CHECK-NEXT:   "command-line": [
+// CHECK-NOT:  "-W
+// CHECK:],
+// CHECK-NEXT:   "context-hash": "{{.*}}",
+// CHECK-NEXT:   "file-deps": [
+// CHECK:],
+// CHECK-NEXT:   "name": "A"
+// CHECK-NEXT: },
+// CHECK-NEXT: {
+// CHECK-NEXT:   "clang-module-deps": [],
+// CHECK-NEXT:   "clang-modulemap-file":
+// CHECK-NEXT:   "command-line": [
+// CHECK-NOT:  "-W
+// CHECK:],
+// CHECK-NEXT:   "context-hash": "{{.*}}",
+// CHECK-NEXT:   "file-deps": [
+// CHECK:],
+// CHECK-NEXT:   "name": "B"
+// CHECK-NEXT: }
+// CHECK-NEXT:   ],
+// CHECK-NEXT:   "translation-units": [
+// CHECK:   

[clang] [clang][DependencyScanner] Remove all warning flags when suppressing warnings (PR #71612)

2023-11-08 Thread Michael Spencer via cfe-commits

Bigcheese wrote:

Added test comments and added a test for `-Wsystem-headers` being present.

https://github.com/llvm/llvm-project/pull/71612
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][DependencyScanner] Remove all warning flags when suppressing warnings (PR #71612)

2023-11-13 Thread Michael Spencer via cfe-commits

https://github.com/Bigcheese closed 
https://github.com/llvm/llvm-project/pull/71612
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [C++20] [Modules] Warn if we found #include in module purview (PR #69555)

2023-11-02 Thread Michael Spencer via cfe-commits

Bigcheese wrote:

This is fine, it's definitely wrong to include framework headers like this too.

https://github.com/llvm/llvm-project/pull/69555
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][modules] Strip LLVM options (PR #75405)

2023-12-13 Thread Michael Spencer via cfe-commits

https://github.com/Bigcheese approved this pull request.


https://github.com/llvm/llvm-project/pull/75405
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang-tools-extra] [clang][modules] Deprecate module.map in favor of module.modulemap (PR #75142)

2023-12-14 Thread Michael Spencer via cfe-commits

https://github.com/Bigcheese closed 
https://github.com/llvm/llvm-project/pull/75142
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][DependencyScanner] Include the working directory in the context hash (PR #73719)

2023-11-28 Thread Michael Spencer via cfe-commits

https://github.com/Bigcheese created 
https://github.com/llvm/llvm-project/pull/73719

The working directory is included in the PCM, but is not currently part of the 
context hash. This causes problems because different builds of a PCM with 
exactly the same command line can end up with different binary content for a 
PCM. If a build system tracks tasks by both working directory and command line, 
it may build a given PCM multiple times, causing a "module file out of date" 
error when loading the PCM due to different sizes.

>From 4122b4a5d394bc7b6923e33b1e7a10606b52c83c Mon Sep 17 00:00:00 2001
From: Michael Spencer 
Date: Mon, 20 Mar 2023 18:48:10 -0700
Subject: [PATCH] [clang][DependencyScanner] Include the working directory in
 the context hash

The working directory is included in the PCM, but is not currently
part of the context hash. This causes problems because different
builds of a PCM with exactly the same command line can end up with
different binary content for a PCM. If a build system tracks tasks by
both working directory and command line, it may build a given PCM
multiple times, causing a "module file out of date" error when loading
the PCM due to different sizes.
---
 .../DependencyScanning/ModuleDepCollector.cpp |  13 ++-
 clang/test/ClangScanDeps/working-dir.m| 107 ++
 2 files changed, 118 insertions(+), 2 deletions(-)
 create mode 100644 clang/test/ClangScanDeps/working-dir.m

diff --git a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp 
b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
index 9099c18391e4d29..3684ac46fbd4717 100644
--- a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
+++ b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
@@ -316,7 +316,8 @@ void 
ModuleDepCollector::applyDiscoveredDependencies(CompilerInvocation &CI) {
 
 static std::string getModuleContextHash(const ModuleDeps &MD,
 const CowCompilerInvocation &CI,
-bool EagerLoadModules) {
+bool EagerLoadModules,
+llvm::vfs::FileSystem &VFS) {
   llvm::HashBuilder, llvm::endianness::native>
   HashBuilder;
   SmallString<32> Scratch;
@@ -325,6 +326,13 @@ static std::string getModuleContextHash(const ModuleDeps 
&MD,
   // will be readable.
   HashBuilder.add(getClangFullRepositoryVersion());
   HashBuilder.add(serialization::VERSION_MAJOR, serialization::VERSION_MINOR);
+  if (CI.getFileSystemOpts().WorkingDir.empty()) {
+llvm::ErrorOr CWD = VFS.getCurrentWorkingDirectory();
+if (CWD)
+  HashBuilder.add(*CWD);
+  }
+  // If any of the above options are set, then there must have been a command
+  // line argument to set them, which will already be hashed.
 
   // Hash the BuildInvocation without any input files.
   SmallString<0> ArgVec;
@@ -356,7 +364,8 @@ static std::string getModuleContextHash(const ModuleDeps 
&MD,
 
 void ModuleDepCollector::associateWithContextHash(
 const CowCompilerInvocation &CI, ModuleDeps &Deps) {
-  Deps.ID.ContextHash = getModuleContextHash(Deps, CI, EagerLoadModules);
+  Deps.ID.ContextHash = getModuleContextHash(
+  Deps, CI, EagerLoadModules, ScanInstance.getVirtualFileSystem());
   bool Inserted = ModuleDepsByID.insert({Deps.ID, &Deps}).second;
   (void)Inserted;
   assert(Inserted && "duplicate module mapping");
diff --git a/clang/test/ClangScanDeps/working-dir.m 
b/clang/test/ClangScanDeps/working-dir.m
new file mode 100644
index 000..a43df2351f67f88
--- /dev/null
+++ b/clang/test/ClangScanDeps/working-dir.m
@@ -0,0 +1,107 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: sed -e "s|DIR|%/t|g" %t/build/compile-commands.json.in > 
%t/build/compile-commands.json
+// RUN: clang-scan-deps -compilation-database %t/build/compile-commands.json \
+// RUN:   -j 1 -format experimental-full --optimize-args=all > %t/deps.db
+// RUN: cat %t/deps.db | sed 's:\?:/:g' | FileCheck %s -DPREFIX=%/t
+
+// Check that there are two separate modules hashes. One for each working dir.
+
+// CHECK:  {
+// CHECK-NEXT:   "modules": [
+// CHECK-NEXT: {
+// CHECK-NEXT:   "clang-module-deps":
+// CHECK-NEXT:   "clang-modulemap-file":
+// CHECK-NEXT:   "command-line": [
+// CHECK:],
+// CHECK-NEXT:   "context-hash": "{{.*}}",
+// CHECK-NEXT:   "file-deps": [
+// CHECK:],
+// CHECK-NEXT:   "name": "A"
+// CHECK-NEXT: },
+// CHECK-NEXT: {
+// CHECK-NEXT:   "clang-module-deps":
+// CHECK-NEXT:   "clang-modulemap-file":
+// CHECK-NEXT:   "command-line": [
+// CHECK:],
+// CHECK-NEXT:   "context-hash": "{{.*}}",
+// CHECK-NEXT:   "file-deps": [
+// CHECK:],
+// CHECK-NEXT:   "name": "A"
+// CHECK-NEXT: },
+// CHECK-NEXT: {
+// CHECK-NEXT:   "clang-module-deps":
+// CHECK:],
+// CHECK-NEXT:   "clang-modulemap-f

[clang] [llvm] [clang][DependencyScanner] Remove unused -ivfsoverlay files (PR #73734)

2023-11-28 Thread Michael Spencer via cfe-commits

https://github.com/Bigcheese created 
https://github.com/llvm/llvm-project/pull/73734

`-ivfsoverlay` files are unused when building most modules. Enable
removing them by,
* adding a way to visit the filesystem tree with extensible RTTI to
  access each `RedirectingFileSystem`.
* Adding tracking to `RedirectingFileSystem` to record when it
  actually redirects a file access.
* Storing this information in each PCM.

Usage tracking is disabled during implicit modulemap search as this
ends up touching a lot of files that aren't actually used. The used
files are later touched by other parts of Clang so relevant VFS
overlays get marked as used.

>From 25f3b9202654aa007b5a7fd2b82a93ef8521003c Mon Sep 17 00:00:00 2001
From: Michael Spencer 
Date: Fri, 24 Feb 2023 17:18:51 -0800
Subject: [PATCH 1/2] Remove unused -ivfsoverlay files

`-ivfsoverlay` files are unused when building most modules. Enable
removing them by,
* adding a way to visit the filesystem tree with extensible RTTI to
  access each `RedirectingFileSystem`.
* Adding tracking to `RedirectingFileSystem` to record when it
  actually redirects a file access.
* Storing this information in each PCM.

Usage tracking is disabled during implicit modulemap search as this
ends up touching a lot of files that aren't actually used. The used
files are later touched by other parts of Clang so relevant VFS
overlays get marked as used.
---
 clang/include/clang/Basic/FileManager.h   |   4 +
 clang/include/clang/Lex/HeaderSearch.h|   6 +
 .../include/clang/Serialization/ASTBitCodes.h |   3 +
 .../include/clang/Serialization/ModuleFile.h  |   3 +
 .../DependencyScanningFilesystem.h|   6 +-
 .../DependencyScanningService.h   |   5 +-
 clang/lib/Basic/FileManager.cpp   |   7 +
 clang/lib/Lex/HeaderSearch.cpp|  20 ++
 clang/lib/Serialization/ASTReader.cpp |  15 +-
 clang/lib/Serialization/ASTWriter.cpp |  24 ++-
 .../DependencyScanningFilesystem.cpp  |   6 +-
 .../DependencyScanning/ModuleDepCollector.cpp |  65 +--
 clang/test/ClangScanDeps/optimize-vfs.m   | 181 ++
 clang/tools/clang-scan-deps/ClangScanDeps.cpp |   1 +
 llvm/include/llvm/Support/VirtualFileSystem.h |  57 +-
 llvm/lib/Support/VirtualFileSystem.cpp|  26 +++
 .../Support/VirtualFileSystemTest.cpp |  84 
 17 files changed, 474 insertions(+), 39 deletions(-)
 create mode 100644 clang/test/ClangScanDeps/optimize-vfs.m

diff --git a/clang/include/clang/Basic/FileManager.h 
b/clang/include/clang/Basic/FileManager.h
index 56cb093dd8c376f..997c17a0ffcfcce 100644
--- a/clang/include/clang/Basic/FileManager.h
+++ b/clang/include/clang/Basic/FileManager.h
@@ -248,6 +248,10 @@ class FileManager : public RefCountedBase {
 return FS;
   }
 
+  /// Enable or disable tracking of VFS usage. Used to not track full header
+  /// search and implicit modulemap lookup.
+  void trackVFSUsage(bool Active);
+
   void setVirtualFileSystem(IntrusiveRefCntPtr FS) {
 this->FS = std::move(FS);
   }
diff --git a/clang/include/clang/Lex/HeaderSearch.h 
b/clang/include/clang/Lex/HeaderSearch.h
index a100598c80155fa..7c21796b0460238 100644
--- a/clang/include/clang/Lex/HeaderSearch.h
+++ b/clang/include/clang/Lex/HeaderSearch.h
@@ -579,6 +579,12 @@ class HeaderSearch {
   /// Note: implicit module maps don't contribute to entry usage.
   std::vector computeUserEntryUsage() const;
 
+  /// Determine which HeaderSearchOptions::VFSOverlayFiles have been
+  /// successfully used so far and mark their index with 'true' in the 
resulting
+  /// bit vector.
+  /// Note: implicit module maps don't contribute to entry usage.
+  std::vector computeVFSUsage() const;
+
   /// This method returns a HeaderMap for the specified
   /// FileEntry, uniquing them through the 'HeaderMaps' datastructure.
   const HeaderMap *CreateHeaderMap(FileEntryRef FE);
diff --git a/clang/include/clang/Serialization/ASTBitCodes.h 
b/clang/include/clang/Serialization/ASTBitCodes.h
index fdd64f2abbe9375..f4abfe6f560664f 100644
--- a/clang/include/clang/Serialization/ASTBitCodes.h
+++ b/clang/include/clang/Serialization/ASTBitCodes.h
@@ -405,6 +405,9 @@ enum UnhashedControlBlockRecordTypes {
 
   /// Record code for the indices of used header search entries.
   HEADER_SEARCH_ENTRY_USAGE,
+
+  /// Record code for the indices of used VFSs.
+  VFS_USAGE,
 };
 
 /// Record code for extension blocks.
diff --git a/clang/include/clang/Serialization/ModuleFile.h 
b/clang/include/clang/Serialization/ModuleFile.h
index 48be8676cc26a4c..a2d49507a579427 100644
--- a/clang/include/clang/Serialization/ModuleFile.h
+++ b/clang/include/clang/Serialization/ModuleFile.h
@@ -189,6 +189,9 @@ class ModuleFile {
   /// The bit vector denoting usage of each header search entry (true = used).
   llvm::BitVector SearchPathUsage;
 
+  /// The bit vector denoting usage of each VFS entry (true = used).
+  llvm::BitVector VFSUsage;
+
   /// Whether this modul

[llvm] [clang] [clang][DependencyScanner] Remove unused -ivfsoverlay files (PR #73734)

2023-11-29 Thread Michael Spencer via cfe-commits

Bigcheese wrote:

> It's odd to me that tracking is enabled by default. I would have expected 
> tracking be off by default and enabled explicitly for scanning. Similarly, in 
> the modulemap case it could save-and-restore rather than enable the tracking 
> if it was previously off.

It has to be on by default because of PCH. You never know if a PCM will be used 
for scanning.

https://github.com/llvm/llvm-project/pull/73734
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[llvm] [clang] [clang][DependencyScanner] Remove unused -ivfsoverlay files (PR #73734)

2023-11-29 Thread Michael Spencer via cfe-commits


@@ -498,11 +518,18 @@ class NamedNodeOrError {
 } // namespace detail
 
 /// An in-memory file system.
-class InMemoryFileSystem : public FileSystem {
+class InMemoryFileSystem : public RTTIExtends {
   std::unique_ptr Root;
   std::string WorkingDirectory;
   bool UseNormalizedPaths = true;
 
+public:
+  static const char ID;
+  using GetFileContentsCallback =

Bigcheese wrote:

Ah, looks like it snuck in while rebasing.

https://github.com/llvm/llvm-project/pull/73734
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[llvm] [clang] [clang][DependencyScanner] Remove unused -ivfsoverlay files (PR #73734)

2023-11-29 Thread Michael Spencer via cfe-commits


@@ -142,6 +142,21 @@ std::vector HeaderSearch::computeUserEntryUsage() 
const {
   return UserEntryUsage;
 }
 
+std::vector HeaderSearch::computeVFSUsage() const {
+  std::vector VFSUsage;
+  llvm::vfs::FileSystem &RootFS = FileMgr.getVirtualFileSystem();
+  // TODO: This only works if the `RedirectingFileSystem`s were all created by

Bigcheese wrote:

We could set a bit, and then just skip over VFSs without the bit. That would 
work even if people manually pass in a VFS with an extra 
`RedirectingFileSystem`, and we have a check later that would fire if 
`createVFSFromOverlayFiles ` wasn't used at all.

https://github.com/llvm/llvm-project/pull/73734
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][DependencyScanner] Include the working directory in the context hash (PR #73719)

2023-11-30 Thread Michael Spencer via cfe-commits

https://github.com/Bigcheese closed 
https://github.com/llvm/llvm-project/pull/73719
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [llvm] [clang][MBD] set up module build daemon infrastructure (PR #67562)

2024-01-04 Thread Michael Spencer via cfe-commits


@@ -0,0 +1,60 @@
+//=== SocketMsgSupport.cpp 
===//
+//
+// 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 "llvm/ADT/StringRef.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Support/raw_socket_stream.h"
+
+#include 
+#include 
+
+using namespace llvm;
+
+namespace clang::tooling::cc1modbuildd {
+
+Expected>
+connectToSocket(StringRef SocketPath) {
+
+  Expected> MaybeClient =
+  raw_socket_stream::createConnectedUnix(SocketPath);
+  if (!MaybeClient)
+return std::move(MaybeClient.takeError());
+
+  return std::move(*MaybeClient);
+}
+
+Expected readBufferFromSocket(raw_socket_stream &Socket) {
+
+  constexpr const unsigned short MAX_BUFFER = 4096;
+  char Buffer[MAX_BUFFER];
+  std::string ReturnBuffer;
+
+  ssize_t n = 0;
+  while ((n = Socket.read(Buffer, MAX_BUFFER)) > 0) {
+ReturnBuffer.append(Buffer, n);
+// Read until \n... encountered which is the last line of a YAML document
+if (ReturnBuffer.find("\n...") != std::string::npos)

Bigcheese wrote:

The `\n...` could be anywhere in the buffer if multiple messages are sent.

https://github.com/llvm/llvm-project/pull/67562
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Driver] Ignore non-clang pch files when -include a.h probes a.h.gch (PR #69204)

2024-01-10 Thread Michael Spencer via cfe-commits

Bigcheese wrote:

This incorrectly handles `-gmodules` which produces an object file that 
contains a clang PCH. Detecting that is a bit harder, is it fine if it just 
accepts all object files as returned by `llvm::identify_magic`?

https://github.com/llvm/llvm-project/pull/69204
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [llvm] [clang][Driver] Don't ignore -gmodules .gch files (PR #77711)

2024-01-10 Thread Michael Spencer via cfe-commits

https://github.com/Bigcheese created 
https://github.com/llvm/llvm-project/pull/77711

A previous commit (82f75ed) made clang ignore .gch files that were not Clang 
AST files. This broke `-gmodules`, which embeds the Clang AST into an object 
file containing debug info.

This changes the probing to detect any file format recognized by 
`llvm::identify_magic()` as potentially containing a Clang AST.

Previous PR: https://github.com/llvm/llvm-project/pull/69204

>From e2cd3ee8741eabc1f251f64d389ed5dcd07810d5 Mon Sep 17 00:00:00 2001
From: Michael Spencer 
Date: Wed, 10 Jan 2024 16:58:59 -0800
Subject: [PATCH] [clang][Driver] Don't ignore -gmodules .gch files

A previous commit (82f75ed) made clang ignore .gch files that were not
Clang AST files. This broke `-gmodules`, which embeds the Clang AST
into an object file containing debug info.

This changes the probing to detect any file format recognized by
`llvm::identify_magic()` as potentially containing a Clang AST.
---
 clang/lib/Driver/ToolChains/Clang.cpp  | 12 
 clang/test/PCH/gch-probe.c |  4 
 llvm/include/llvm/BinaryFormat/Magic.h |  1 +
 llvm/lib/BinaryFormat/Magic.cpp|  2 ++
 llvm/lib/Object/Binary.cpp |  1 +
 llvm/lib/Object/ObjectFile.cpp |  1 +
 6 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index 2d8ef841d4f6be..36f1545e285e14 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -43,6 +43,7 @@
 #include "clang/Driver/XRayArgs.h"
 #include "llvm/ADT/SmallSet.h"
 #include "llvm/ADT/StringExtras.h"
+#include "llvm/BinaryFormat/Magic.h"
 #include "llvm/Config/llvm-config.h"
 #include "llvm/Option/ArgList.h"
 #include "llvm/Support/CodeGen.h"
@@ -948,10 +949,13 @@ static void handleAMDGPUCodeObjectVersionOptions(const 
Driver &D,
   }
 }
 
-static bool hasClangPchSignature(const Driver &D, StringRef Path) {
+static bool maybeHasClangPchSignature(const Driver &D, StringRef Path) {
   if (llvm::ErrorOr> MemBuf =
   D.getVFS().getBufferForFile(Path))
-return (*MemBuf)->getBuffer().starts_with("CPCH");
+// Return true for both raw Clang AST files and object files which may
+// contain a __clangast section.
+return llvm::identify_magic((*MemBuf)->getBuffer()) !=
+   llvm::file_magic::unknown;
   return false;
 }
 
@@ -964,14 +968,14 @@ static bool gchProbe(const Driver &D, StringRef Path) {
 std::error_code EC;
 for (llvm::vfs::directory_iterator DI = D.getVFS().dir_begin(Path, EC), DE;
  !EC && DI != DE; DI = DI.increment(EC)) {
-  if (hasClangPchSignature(D, DI->path()))
+  if (maybeHasClangPchSignature(D, DI->path()))
 return true;
 }
 D.Diag(diag::warn_drv_pch_ignoring_gch_dir) << Path;
 return false;
   }
 
-  if (hasClangPchSignature(D, Path))
+  if (maybeHasClangPchSignature(D, Path))
 return true;
   D.Diag(diag::warn_drv_pch_ignoring_gch_file) << Path;
   return false;
diff --git a/clang/test/PCH/gch-probe.c b/clang/test/PCH/gch-probe.c
index 8b1e1fab5ad97b..0905c9baebdae0 100644
--- a/clang/test/PCH/gch-probe.c
+++ b/clang/test/PCH/gch-probe.c
@@ -2,6 +2,10 @@
 // RUN: %clang -x c-header -c %s -o %t.h.gch
 // RUN: %clang -fsyntax-only -include %t.h %s
 
+// -gmodules embeds the Clang AST file in an object file.
+// RUN: %clang -x c-header -c %s -gmodules -o %t.h.gch
+// RUN: %clang -fsyntax-only -include %t.h %s
+
 // gch probing should ignore files which are not clang pch files.
 // RUN: %clang -fsyntax-only -include %S/Inputs/gch-probe.h %s 2>&1 | 
FileCheck %s
 // CHECK: warning: precompiled header '{{.*}}gch-probe.h.gch' was ignored 
because it is not a clang PCH file
diff --git a/llvm/include/llvm/BinaryFormat/Magic.h 
b/llvm/include/llvm/BinaryFormat/Magic.h
index c635a269576587..6978c066bda468 100644
--- a/llvm/include/llvm/BinaryFormat/Magic.h
+++ b/llvm/include/llvm/BinaryFormat/Magic.h
@@ -21,6 +21,7 @@ struct file_magic {
   enum Impl {
 unknown = 0,   ///< Unrecognized file
 bitcode,   ///< Bitcode file
+clang_ast, ///< Clang PCH or PCM
 archive,   ///< ar style archive file
 elf,   ///< ELF Unknown type
 elf_relocatable,   ///< ELF Relocatable object file
diff --git a/llvm/lib/BinaryFormat/Magic.cpp b/llvm/lib/BinaryFormat/Magic.cpp
index 45a0b7e11452b4..bd378337ed3338 100644
--- a/llvm/lib/BinaryFormat/Magic.cpp
+++ b/llvm/lib/BinaryFormat/Magic.cpp
@@ -98,6 +98,8 @@ file_magic llvm::identify_magic(StringRef Magic) {
   case 'C':
 if (startswith(Magic, "CCOB"))
   return file_magic::offload_bundle_compressed;
+if (startswith(Magic, "CPCH"))
+  return file_magic::clang_ast;
 break;
   case '!':
 if (startswith(Magic, "!\n") || startswith(Magic, "!\n"))
diff --git a/llvm/lib/Object/Binary.cpp b/llvm/lib/Object/Binary.cpp
index 0b9d95485287dc..2dfae8ab5d3c64 100644
--- a/llvm/

[clang] [llvm] [clang][Driver] Don't ignore -gmodules .gch files (PR #77711)

2024-01-11 Thread Michael Spencer via cfe-commits

Bigcheese wrote:

> What does the code that reads these files look like, could we leverage that 
> somehow?

You can call `clang::ObjectFilePCHContainerReader::ExtractPCH()` and then check 
the magic. This lives in the CodeGen library which I don't think the driver 
currently (or should) links against, but this is the best way to know if 
something is valid.
 
> An alternative would be turn the logic around, and only ignore GCC PCH files 
> (I believe they all start with the file magic `gpch`). However I do think 
> that the current approach of "whitelisting" the kind of file we're looking 
> for is better.

I would be fine with this approach, but agree that it's best if we can be more 
selective. My concern is that 
`clang::ObjectFilePCHContainerReader::ExtractPCH()` and this detection may get 
out of sync, as support for `-gmodules` is automatic anytime someone adds a new 
object format.

https://github.com/llvm/llvm-project/pull/77711
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Headers] Don't declare unreachable() from stddef.h in C++ (PR #86748)

2024-03-26 Thread Michael Spencer via cfe-commits

Bigcheese wrote:

I'm checking with the C and C++ Compatibility study group (SG22) for what's 
expected here.

https://github.com/llvm/llvm-project/pull/86748
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang] Diagnose config_macros before building modules (PR #83641)

2024-03-05 Thread Michael Spencer via cfe-commits

https://github.com/Bigcheese closed 
https://github.com/llvm/llvm-project/pull/83641
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang-tools-extra] [llvm] [llvm][Support] Add and use errnoAsErrorCode (PR #84423)

2024-03-07 Thread Michael Spencer via cfe-commits

https://github.com/Bigcheese created 
https://github.com/llvm/llvm-project/pull/84423

LLVM is inconsistent about how it converts `errno` to `std::error_code`. This 
can cause problems because values outside of `std::errc` compare differently if 
one is system and one is generic on POSIX systems.

This is even more of a problem on Windows where use of the system category is 
just wrong, as that is for Windows errors, which have a completely different 
mapping than POSIX/generic errors. This patch fixes one instance of this 
mistake in `JSONTransport.cpp`.

This patch adds `errnoAsErrorCode()` which makes it so people do not need to 
think about this issue in the future. It also cleans up a lot of usage of errno 
in LLVM and Clang.

>From 858effb8f8225e9ca7c367037046f07b576a3348 Mon Sep 17 00:00:00 2001
From: Michael Spencer 
Date: Thu, 7 Mar 2024 17:36:33 -0800
Subject: [PATCH] [llvm][Support] Add and use errnoAsErrorCode

LLVM is inconsistent about how it converts `errno` to
`std::error_code`. This can cause problems because values outside of
`std::errc` compare differently if one is system and one is generic on
POSIX systems.

This is even more of a problem on Windows where use of the system
category is just wrong, as that is for Windows errors, which have a
completely different mapping than POSIX/generic errors. This patch
fixes one instance of this mistake in `JSONTransport.cpp`.

This patch adds `errnoAsErrorCode()` which makes it so people do not
need to think about this issue in the future. It also cleans up a lot
of usage of errno in LLVM and Clang.
---
 clang-tools-extra/clangd/JSONTransport.cpp|  3 +-
 .../linux/DirectoryWatcher-linux.cpp  |  9 +--
 llvm/include/llvm/Support/Error.h | 14 
 llvm/lib/ExecutionEngine/Orc/MemoryMapper.cpp |  9 +--
 .../ExecutorSharedMemoryMapperService.cpp | 11 ++-
 llvm/lib/Object/ArchiveWriter.cpp |  2 +-
 llvm/lib/Support/AutoConvert.cpp  |  8 +--
 llvm/lib/Support/LockFileManager.cpp  |  2 +-
 llvm/lib/Support/Path.cpp | 19 ++
 llvm/lib/Support/RandomNumberGenerator.cpp|  7 +-
 llvm/lib/Support/Unix/Memory.inc  | 10 +--
 llvm/lib/Support/Unix/Path.inc| 67 +--
 llvm/lib/Support/Unix/Process.inc | 12 ++--
 llvm/lib/Support/Windows/Process.inc  |  2 +-
 llvm/lib/Support/Windows/Program.inc  |  4 +-
 llvm/lib/Support/raw_ostream.cpp  |  6 +-
 llvm/lib/Support/raw_socket_stream.cpp|  2 +-
 17 files changed, 94 insertions(+), 93 deletions(-)

diff --git a/clang-tools-extra/clangd/JSONTransport.cpp 
b/clang-tools-extra/clangd/JSONTransport.cpp
index 346c7dfb66a1db..3c0e198433f360 100644
--- a/clang-tools-extra/clangd/JSONTransport.cpp
+++ b/clang-tools-extra/clangd/JSONTransport.cpp
@@ -107,8 +107,7 @@ class JSONTransport : public Transport {
 return error(std::make_error_code(std::errc::operation_canceled),
  "Got signal, shutting down");
   if (ferror(In))
-return llvm::errorCodeToError(
-std::error_code(errno, std::system_category()));
+return llvm::errorCodeToError(llvm::errnoAsErrorCode());
   if (readRawMessage(JSON)) {
 ThreadCrashReporter ScopedReporter([&JSON]() {
   auto &OS = llvm::errs();
diff --git a/clang/lib/DirectoryWatcher/linux/DirectoryWatcher-linux.cpp 
b/clang/lib/DirectoryWatcher/linux/DirectoryWatcher-linux.cpp
index beca9586988b52..2ffbc1a226958a 100644
--- a/clang/lib/DirectoryWatcher/linux/DirectoryWatcher-linux.cpp
+++ b/clang/lib/DirectoryWatcher/linux/DirectoryWatcher-linux.cpp
@@ -333,8 +333,7 @@ llvm::Expected> 
clang::DirectoryWatcher::creat
   const int InotifyFD = inotify_init1(IN_CLOEXEC);
   if (InotifyFD == -1)
 return llvm::make_error(
-std::string("inotify_init1() error: ") + strerror(errno),
-llvm::inconvertibleErrorCode());
+llvm::errnoAsErrorCode(), std::string(": inotify_init1()"));
 
   const int InotifyWD = inotify_add_watch(
   InotifyFD, Path.str().c_str(),
@@ -346,15 +345,13 @@ llvm::Expected> 
clang::DirectoryWatcher::creat
   );
   if (InotifyWD == -1)
 return llvm::make_error(
-std::string("inotify_add_watch() error: ") + strerror(errno),
-llvm::inconvertibleErrorCode());
+llvm::errnoAsErrorCode(), std::string(": inotify_add_watch()"));
 
   auto InotifyPollingStopper = SemaphorePipe::create();
 
   if (!InotifyPollingStopper)
 return llvm::make_error(
-std::string("SemaphorePipe::create() error: ") + strerror(errno),
-llvm::inconvertibleErrorCode());
+llvm::errnoAsErrorCode(), std::string(": SemaphorePipe::create()"));
 
   return std::make_unique(
   Path, Receiver, WaitForInitialSync, InotifyFD, InotifyWD,
diff --git a/llvm/include/llvm/Support/Error.h 
b/llvm/include/llvm/Support/Error.h
index bb4f38f7ec355e..894b6484336aef 100644
--- a/llvm/include/llvm/Support

[clang] [clang-tools-extra] [llvm] [llvm][Support] Add and use errnoAsErrorCode (PR #84423)

2024-03-08 Thread Michael Spencer via cfe-commits

Bigcheese wrote:

The other cases of `std::system_category` were in `LLVM_ON_UNIX` (or similar) 
blocks that would only be used on systems where it's mostly fine to use either 
one, as most of the time you'll get an error that's in `std::errc`, and then 
there's no difference (or they just are never compared in general). The initial 
desire to do this came from spending 30m looking into which one to use on UNIX 
systems in general and wanting to avoid that in the future. The 
`JSONTransport.cpp` case was just more indication to me that the existing way 
was error prone.

In auditing all uses of `errno` I did find a few other places where the code 
isn't quite wrong, but it's not really using `llvm::Error` correctly. There's 
quite a few places where people use `llvm::inconvertibleErrorCode()` where they 
really shouldn't be. For example `llvm-exegesis` has a bunch of places where 
they call `strerror(errno)` to construct an `llvm::Error` that implicitly uses 
`llvm::inconvertibleErrorCode()` as the `std::error_code` value. Our existing 
documentation here is pretty nice for `Error` and `Expected` 
(https://llvm.org/docs/ProgrammersManual.html#error-handling), but it would be 
nice to better cover how `std::error_code` is supposed to be propagated, as it 
currently kind of implies they are going away, but they are always needed for 
OS errors. I'll try to get to this when I can find time.

As for test coverage, some of these have existing error tests, but for a lot of 
the rest it's either incredibly difficult or just not possible (without using 
`LD_PRELOAD` or something similar) to test. Given that, it's good to make 
handling them as easy to get correct as practicable.



https://github.com/llvm/llvm-project/pull/84423
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang-tools-extra] [llvm] [llvm][Support] Add and use errnoAsErrorCode (PR #84423)

2024-03-08 Thread Michael Spencer via cfe-commits

https://github.com/Bigcheese closed 
https://github.com/llvm/llvm-project/pull/84423
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] reland: [clang][ScanDeps] Canonicalize -D and -U flags (PR #82568)

2024-02-23 Thread Michael Spencer via cfe-commits

https://github.com/Bigcheese updated 
https://github.com/llvm/llvm-project/pull/82568

>From eb622c20b8d84afabdbb82543c1f9e4889639735 Mon Sep 17 00:00:00 2001
From: Michael Spencer 
Date: Fri, 16 Feb 2024 22:05:25 -0800
Subject: [PATCH] [clang][ScanDeps] Canonicalize -D and -U flags

Canonicalize `-D` and `-U` flags by sorting them and only keeping the
last instance of a given name.

This optimization will only fire if all `-D` and `-U` flags start with
a simple identifier that we can guarantee a simple analysis of can
determine if two flags refer to the same identifier or not. See the
comment on `getSimpleMacroName()` for details of what the issues are.
---
 .../DependencyScanningService.h   |  5 +-
 .../DependencyScanningWorker.cpp  | 74 
 .../optimize-canonicalize-macros.m| 87 +++
 clang/tools/clang-scan-deps/ClangScanDeps.cpp |  1 +
 4 files changed, 166 insertions(+), 1 deletion(-)
 create mode 100644 clang/test/ClangScanDeps/optimize-canonicalize-macros.m

diff --git 
a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h 
b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h
index 4f9867262a275c..557f0e547ab4a8 100644
--- a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h
+++ b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h
@@ -60,7 +60,10 @@ enum class ScanningOptimizations {
   /// Remove unused -ivfsoverlay arguments.
   VFS = 4,
 
-  DSS_LAST_BITMASK_ENUM(VFS),
+  /// Canonicalize -D and -U options.
+  Macros = 8,
+
+  DSS_LAST_BITMASK_ENUM(Macros),
   Default = All
 };
 
diff --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp 
b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
index 3cf3ad8a4e4907..7477b930188b4f 100644
--- a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
+++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
@@ -179,6 +179,78 @@ static void sanitizeDiagOpts(DiagnosticOptions &DiagOpts) {
   DiagOpts.IgnoreWarnings = true;
 }
 
+// Clang implements -D and -U by splatting text into a predefines buffer. This
+// allows constructs such as `-DFඞ=3 "-D F\u{0D9E} 4 3 2”` to be accepted and
+// define the same macro, or adding C++ style comments before the macro name.
+//
+// This function checks that the first non-space characters in the macro
+// obviously form an identifier that can be uniqued on without lexing. Failing
+// to do this could lead to changing the final definition of a macro.
+//
+// We could set up a preprocessor and actually lex the name, but that's very
+// heavyweight for a situation that will almost never happen in practice.
+static std::optional getSimpleMacroName(StringRef Macro) {
+  StringRef Name = Macro.split("=").first.ltrim(" \t");
+  std::size_t I = 0;
+
+  auto FinishName = [&]() -> std::optional {
+StringRef SimpleName = Name.slice(0, I);
+if (SimpleName.empty())
+  return std::nullopt;
+return SimpleName;
+  };
+
+  for (; I != Name.size(); ++I) {
+switch (Name[I]) {
+case '(': // Start of macro parameter list
+case ' ': // End of macro name
+case '\t':
+  return FinishName();
+case '_':
+  continue;
+default:
+  if (llvm::isAlnum(Name[I]))
+continue;
+  return std::nullopt;
+}
+  }
+  return FinishName();
+}
+
+static void canonicalizeDefines(PreprocessorOptions &PPOpts) {
+  using MacroOpt = std::pair;
+  std::vector SimpleNames;
+  SimpleNames.reserve(PPOpts.Macros.size());
+  std::size_t Index = 0;
+  for (const auto &M : PPOpts.Macros) {
+auto SName = getSimpleMacroName(M.first);
+// Skip optimizing if we can't guarantee we can preserve relative order.
+if (!SName)
+  return;
+SimpleNames.emplace_back(*SName, Index);
+++Index;
+  }
+
+  llvm::stable_sort(SimpleNames, [](const MacroOpt &A, const MacroOpt &B) {
+return A.first < B.first;
+  });
+  // Keep the last instance of each macro name by going in reverse
+  auto NewEnd = std::unique(
+  SimpleNames.rbegin(), SimpleNames.rend(),
+  [](const MacroOpt &A, const MacroOpt &B) { return A.first == B.first; });
+  SimpleNames.erase(SimpleNames.begin(), NewEnd.base());
+
+  // Apply permutation.
+  decltype(PPOpts.Macros) NewMacros;
+  NewMacros.reserve(SimpleNames.size());
+  for (std::size_t I = 0, E = SimpleNames.size(); I != E; ++I) {
+std::size_t OriginalIndex = SimpleNames[I].second;
+// We still emit undefines here as they may be undefining a predefined 
macro
+NewMacros.push_back(std::move(PPOpts.Macros[OriginalIndex]));
+  }
+  std::swap(PPOpts.Macros, NewMacros);
+}
+
 /// A clang tool that runs the preprocessor in a mode that's optimized for
 /// dependency scanning for the given compiler invocation.
 class DependencyScanningAction : public tooling::ToolAction {
@@ -203,6 +275,8 @@ class DependencyScanningAction : public tooling::ToolAc

[clang] reland: [clang][ScanDeps] Canonicalize -D and -U flags (PR #82568)

2024-02-23 Thread Michael Spencer via cfe-commits

https://github.com/Bigcheese updated 
https://github.com/llvm/llvm-project/pull/82568

>From a690c96562dea29a760390644d78a01a263993ca Mon Sep 17 00:00:00 2001
From: Michael Spencer 
Date: Fri, 16 Feb 2024 22:05:25 -0800
Subject: [PATCH] [clang][ScanDeps] Canonicalize -D and -U flags

Canonicalize `-D` and `-U` flags by sorting them and only keeping the
last instance of a given name.

This optimization will only fire if all `-D` and `-U` flags start with
a simple identifier that we can guarantee a simple analysis of can
determine if two flags refer to the same identifier or not. See the
comment on `getSimpleMacroName()` for details of what the issues are.
---
 .../DependencyScanningService.h   |  5 +-
 .../DependencyScanningWorker.cpp  | 74 
 .../optimize-canonicalize-macros.m| 87 +++
 clang/tools/clang-scan-deps/ClangScanDeps.cpp |  1 +
 4 files changed, 166 insertions(+), 1 deletion(-)
 create mode 100644 clang/test/ClangScanDeps/optimize-canonicalize-macros.m

diff --git 
a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h 
b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h
index 4f9867262a275c..557f0e547ab4a8 100644
--- a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h
+++ b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h
@@ -60,7 +60,10 @@ enum class ScanningOptimizations {
   /// Remove unused -ivfsoverlay arguments.
   VFS = 4,
 
-  DSS_LAST_BITMASK_ENUM(VFS),
+  /// Canonicalize -D and -U options.
+  Macros = 8,
+
+  DSS_LAST_BITMASK_ENUM(Macros),
   Default = All
 };
 
diff --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp 
b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
index 3cf3ad8a4e4907..7477b930188b4f 100644
--- a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
+++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
@@ -179,6 +179,78 @@ static void sanitizeDiagOpts(DiagnosticOptions &DiagOpts) {
   DiagOpts.IgnoreWarnings = true;
 }
 
+// Clang implements -D and -U by splatting text into a predefines buffer. This
+// allows constructs such as `-DFඞ=3 "-D F\u{0D9E} 4 3 2”` to be accepted and
+// define the same macro, or adding C++ style comments before the macro name.
+//
+// This function checks that the first non-space characters in the macro
+// obviously form an identifier that can be uniqued on without lexing. Failing
+// to do this could lead to changing the final definition of a macro.
+//
+// We could set up a preprocessor and actually lex the name, but that's very
+// heavyweight for a situation that will almost never happen in practice.
+static std::optional getSimpleMacroName(StringRef Macro) {
+  StringRef Name = Macro.split("=").first.ltrim(" \t");
+  std::size_t I = 0;
+
+  auto FinishName = [&]() -> std::optional {
+StringRef SimpleName = Name.slice(0, I);
+if (SimpleName.empty())
+  return std::nullopt;
+return SimpleName;
+  };
+
+  for (; I != Name.size(); ++I) {
+switch (Name[I]) {
+case '(': // Start of macro parameter list
+case ' ': // End of macro name
+case '\t':
+  return FinishName();
+case '_':
+  continue;
+default:
+  if (llvm::isAlnum(Name[I]))
+continue;
+  return std::nullopt;
+}
+  }
+  return FinishName();
+}
+
+static void canonicalizeDefines(PreprocessorOptions &PPOpts) {
+  using MacroOpt = std::pair;
+  std::vector SimpleNames;
+  SimpleNames.reserve(PPOpts.Macros.size());
+  std::size_t Index = 0;
+  for (const auto &M : PPOpts.Macros) {
+auto SName = getSimpleMacroName(M.first);
+// Skip optimizing if we can't guarantee we can preserve relative order.
+if (!SName)
+  return;
+SimpleNames.emplace_back(*SName, Index);
+++Index;
+  }
+
+  llvm::stable_sort(SimpleNames, [](const MacroOpt &A, const MacroOpt &B) {
+return A.first < B.first;
+  });
+  // Keep the last instance of each macro name by going in reverse
+  auto NewEnd = std::unique(
+  SimpleNames.rbegin(), SimpleNames.rend(),
+  [](const MacroOpt &A, const MacroOpt &B) { return A.first == B.first; });
+  SimpleNames.erase(SimpleNames.begin(), NewEnd.base());
+
+  // Apply permutation.
+  decltype(PPOpts.Macros) NewMacros;
+  NewMacros.reserve(SimpleNames.size());
+  for (std::size_t I = 0, E = SimpleNames.size(); I != E; ++I) {
+std::size_t OriginalIndex = SimpleNames[I].second;
+// We still emit undefines here as they may be undefining a predefined 
macro
+NewMacros.push_back(std::move(PPOpts.Macros[OriginalIndex]));
+  }
+  std::swap(PPOpts.Macros, NewMacros);
+}
+
 /// A clang tool that runs the preprocessor in a mode that's optimized for
 /// dependency scanning for the given compiler invocation.
 class DependencyScanningAction : public tooling::ToolAction {
@@ -203,6 +275,8 @@ class DependencyScanningAction : public tooling::ToolAc

[clang] [llvm] [clang][ScanDeps] Allow PCHs to have different VFS overlays (PR #82294)

2024-02-23 Thread Michael Spencer via cfe-commits

https://github.com/Bigcheese updated 
https://github.com/llvm/llvm-project/pull/82294

>From 45852f569575d0735c686376ad30753fe791db26 Mon Sep 17 00:00:00 2001
From: Michael Spencer 
Date: Thu, 15 Feb 2024 16:44:45 -0800
Subject: [PATCH] [clang][ScanDeps] Allow PCHs to have different VFS overlays

It turns out it's not that uncommon for real code to pass a different
set of VFSs while building a PCH than while using the PCH. This can
cause problems as seen in `test/ClangScanDeps/optimize-vfs-pch.m`. If
you scan `compile-commands-tu-no-vfs-error.json` without -Werror and
run the resulting commands, Clang will emit a fatal error while trying
to emit a note saying that it can't find a remapped header.

This also adds textual tracking of VFSs for prebuilt modules that are
part of an included PCH, as the same issue can occur in a module we
are building if we drop VFSs. This has to be textual because we have
no guarantee the PCH had the same list of VFSs as the current TU.
---
 .../Basic/DiagnosticSerializationKinds.td |   4 +-
 .../DependencyScanning/ModuleDepCollector.h   |   5 +
 .../DependencyScanningWorker.cpp  |  58 +++--
 .../DependencyScanning/ModuleDepCollector.cpp |  34 --
 clang/test/ClangScanDeps/optimize-vfs-pch.m   | 114 --
 llvm/include/llvm/ADT/StringSet.h |   4 +
 6 files changed, 190 insertions(+), 29 deletions(-)

diff --git a/clang/include/clang/Basic/DiagnosticSerializationKinds.td 
b/clang/include/clang/Basic/DiagnosticSerializationKinds.td
index 4c4659ed517e0a..eb27de5921d6a1 100644
--- a/clang/include/clang/Basic/DiagnosticSerializationKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSerializationKinds.td
@@ -44,7 +44,9 @@ def err_pch_diagopt_mismatch : Error<"%0 is currently 
enabled, but was not in "
   "the PCH file">;
 def err_pch_modulecache_mismatch : Error<"PCH was compiled with module cache "
   "path '%0', but the path is currently '%1'">;
-def err_pch_vfsoverlay_mismatch : Error<"PCH was compiled with different VFS 
overlay files than are currently in use">;
+def warn_pch_vfsoverlay_mismatch : Warning<
+  "PCH was compiled with different VFS overlay files than are currently in 
use">,
+  InGroup>;
 def note_pch_vfsoverlay_files : Note<"%select{PCH|current translation unit}0 
has the following VFS overlays:\n%1">;
 def note_pch_vfsoverlay_empty : Note<"%select{PCH|current translation unit}0 
has no VFS overlays">;
 
diff --git 
a/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h 
b/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
index 13ad2530864927..081899cc2c8503 100644
--- a/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
+++ b/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
@@ -149,6 +149,8 @@ struct ModuleDeps {
   BuildInfo;
 };
 
+using PrebuiltModuleVFSMapT = llvm::StringMap>;
+
 class ModuleDepCollector;
 
 /// Callback that records textual includes and direct modular includes/imports
@@ -214,6 +216,7 @@ class ModuleDepCollector final : public DependencyCollector 
{
  CompilerInstance &ScanInstance, DependencyConsumer &C,
  DependencyActionController &Controller,
  CompilerInvocation OriginalCI,
+ PrebuiltModuleVFSMapT PrebuiltModuleVFSMap,
  ScanningOptimizations OptimizeArgs, bool EagerLoadModules,
  bool IsStdModuleP1689Format);
 
@@ -233,6 +236,8 @@ class ModuleDepCollector final : public DependencyCollector 
{
   DependencyConsumer &Consumer;
   /// Callbacks for computing dependency information.
   DependencyActionController &Controller;
+  /// Mapping from prebuilt AST files to their sorted list of VFS overlay 
files.
+  PrebuiltModuleVFSMapT PrebuiltModuleVFSMap;
   /// Path to the main source file.
   std::string MainFile;
   /// Hash identifying the compilation conditions of the current TU.
diff --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp 
b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
index 3cf3ad8a4e4907..b252463a08b8d7 100644
--- a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
+++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
@@ -24,6 +24,7 @@
 #include "clang/Tooling/DependencyScanning/DependencyScanningService.h"
 #include "clang/Tooling/DependencyScanning/ModuleDepCollector.h"
 #include "clang/Tooling/Tooling.h"
+#include "llvm/ADT/ScopeExit.h"
 #include "llvm/Support/Allocator.h"
 #include "llvm/Support/Error.h"
 #include "llvm/TargetParser/Host.h"
@@ -67,7 +68,7 @@ static bool checkHeaderSearchPaths(const HeaderSearchOptions 
&HSOpts,
   if (LangOpts.Modules) {
 if (HSOpts.VFSOverlayFiles != ExistingHSOpts.VFSOverlayFiles) {
   if (Diags) {
-Diags->Report(diag::err_pch_vfsoverlay_mismatch);
+Diags->Report(diag::warn_pch_vfsoverlay_mismatch);
 auto VFSNote = [&](int Type, ArrayRef 

[clang] reland: [clang][ScanDeps] Canonicalize -D and -U flags (PR #82568)

2024-02-23 Thread Michael Spencer via cfe-commits

Bigcheese wrote:

CI failure was a preexisting trailing whitespace issue.

https://github.com/llvm/llvm-project/pull/82568
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] reland: [clang][ScanDeps] Canonicalize -D and -U flags (PR #82568)

2024-02-23 Thread Michael Spencer via cfe-commits

https://github.com/Bigcheese closed 
https://github.com/llvm/llvm-project/pull/82568
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [llvm] [clang][ScanDeps] Allow PCHs to have different VFS overlays (PR #82294)

2024-02-23 Thread Michael Spencer via cfe-commits

Bigcheese wrote:

CI failure is a preexisting Flang test failure and a preexisting trailing 
whitespace issue.

https://github.com/llvm/llvm-project/pull/82294
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [llvm] [clang][ScanDeps] Allow PCHs to have different VFS overlays (PR #82294)

2024-02-23 Thread Michael Spencer via cfe-commits

https://github.com/Bigcheese closed 
https://github.com/llvm/llvm-project/pull/82294
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [llvm] [clang][ScanDeps] Allow PCHs to have different VFS overlays (PR #82294)

2024-02-28 Thread Michael Spencer via cfe-commits


@@ -175,8 +192,19 @@ static void sanitizeDiagOpts(DiagnosticOptions &DiagOpts) {
   DiagOpts.ShowCarets = false;
   // Don't write out diagnostic file.
   DiagOpts.DiagnosticSerializationFile.clear();
-  // Don't emit warnings as errors (and all other warnings too).
-  DiagOpts.IgnoreWarnings = true;
+  // Don't emit warnings except for scanning specific warnings.
+  // TODO: It would be useful to add a more principled way to ignore all
+  //   warnings that come from source code. The issue is that we need to
+  //   ignore warnings that could be surpressed by
+  //   `#pragma clang diagnostic`, while still allowing some scanning
+  //   warnings for things we're not ready to turn into errors yet.

Bigcheese wrote:

The scanner never sees `#pragma clang diagnostic`, so there's no issue with 
code that uses that to turn warnings on. The original issue was with warnings 
getting turned off via `#pragma clang diagnostic`, but the new code removes all 
warnings and `Werror`s, so you're just left with default warnings.

The goal here was to keep driver warnings (which are lost otherwise) and allow 
us to have dedicated scanner warnings. I do think we want more control over 
this, possibly add a scanner bit to diagnostics so we can be explicit about 
which warnings we expect from the scanner, but I think this change is fine for 
now.

https://github.com/llvm/llvm-project/pull/82294
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang] Diagnose config_macros before building modules (PR #83641)

2024-03-01 Thread Michael Spencer via cfe-commits

https://github.com/Bigcheese created 
https://github.com/llvm/llvm-project/pull/83641

Before this patch, if a module fails to build because of a missing 
config_macro, the user will never see the config macro warning. This patch 
diagnoses this before building, and each subsequent time a module is imported.

rdar://123921931

>From 4edc58151460ae21baa312a91aa5a7955857c8a5 Mon Sep 17 00:00:00 2001
From: Michael Spencer 
Date: Fri, 1 Mar 2024 17:18:20 -0800
Subject: [PATCH] [clang] Diagnose config_macros before building modules

Before this patch, if a module fails to build because of a missing
config_macro, the user will never see the config macro warning. This
patch diagnoses this before building, and each subsequent time a
module is imported.

rdar://123921931
---
 clang/lib/Frontend/CompilerInstance.cpp| 28 ++
 clang/test/Modules/Inputs/config.h |  7 
 clang/test/Modules/Inputs/module.modulemap |  5 ---
 clang/test/Modules/config_macros.m | 45 --
 4 files changed, 62 insertions(+), 23 deletions(-)
 delete mode 100644 clang/test/Modules/Inputs/config.h

diff --git a/clang/lib/Frontend/CompilerInstance.cpp 
b/clang/lib/Frontend/CompilerInstance.cpp
index 4443073775..14fd140f03cc36 100644
--- a/clang/lib/Frontend/CompilerInstance.cpp
+++ b/clang/lib/Frontend/CompilerInstance.cpp
@@ -1591,6 +1591,14 @@ static void checkConfigMacro(Preprocessor &PP, StringRef 
ConfigMacro,
   }
 }
 
+static void checkConfigMacros(Preprocessor &PP, Module *M,
+  SourceLocation ImportLoc) {
+  clang::Module *TopModule = M->getTopLevelModule();
+  for (unsigned I = 0, N = TopModule->ConfigMacros.size(); I != N; ++I) {
+checkConfigMacro(PP, TopModule->ConfigMacros[I], M, ImportLoc);
+  }
+}
+
 /// Write a new timestamp file with the given path.
 static void writeTimestampFile(StringRef TimestampFile) {
   std::error_code EC;
@@ -1829,6 +1837,12 @@ ModuleLoadResult 
CompilerInstance::findOrCompileModuleAndReadAST(
   Module *M =
   HS.lookupModule(ModuleName, ImportLoc, true, !IsInclusionDirective);
 
+  // Check for any configuration macros that have changed. This is done
+  // immediately before potentially building a module in case this module
+  // depends on having one of its configuration macros defined to successfully
+  // build. If this is not done the user will never see the warning.
+  checkConfigMacros(getPreprocessor(), M, ImportLoc);
+
   // Select the source and filename for loading the named module.
   std::string ModuleFilename;
   ModuleSource Source =
@@ -2006,6 +2020,11 @@ CompilerInstance::loadModule(SourceLocation ImportLoc,
   if (auto MaybeModule = MM.getCachedModuleLoad(*Path[0].first)) {
 // Use the cached result, which may be nullptr.
 Module = *MaybeModule;
+// Config macros are already checked before building a module, but they 
need
+// to be checked at each import location in case any of the config macros
+// have a new value at the current `ImportLoc`.
+if (Module)
+  checkConfigMacros(getPreprocessor(), Module, ImportLoc);
   } else if (ModuleName == getLangOpts().CurrentModule) {
 // This is the module we're building.
 Module = PP->getHeaderSearchInfo().lookupModule(
@@ -2146,18 +2165,11 @@ CompilerInstance::loadModule(SourceLocation ImportLoc,
 TheASTReader->makeModuleVisible(Module, Visibility, ImportLoc);
   }
 
-  // Check for any configuration macros that have changed.
-  clang::Module *TopModule = Module->getTopLevelModule();
-  for (unsigned I = 0, N = TopModule->ConfigMacros.size(); I != N; ++I) {
-checkConfigMacro(getPreprocessor(), TopModule->ConfigMacros[I],
- Module, ImportLoc);
-  }
-
   // Resolve any remaining module using export_as for this one.
   getPreprocessor()
   .getHeaderSearchInfo()
   .getModuleMap()
-  .resolveLinkAsDependencies(TopModule);
+  .resolveLinkAsDependencies(Module->getTopLevelModule());
 
   LastModuleImportLoc = ImportLoc;
   LastModuleImportResult = ModuleLoadResult(Module);
diff --git a/clang/test/Modules/Inputs/config.h 
b/clang/test/Modules/Inputs/config.h
deleted file mode 100644
index 4c124b0bf82b7c..00
--- a/clang/test/Modules/Inputs/config.h
+++ /dev/null
@@ -1,7 +0,0 @@
-#ifdef WANT_FOO
-int* foo(void);
-#endif
-
-#ifdef WANT_BAR
-char *bar(void);
-#endif
diff --git a/clang/test/Modules/Inputs/module.modulemap 
b/clang/test/Modules/Inputs/module.modulemap
index e7cb4b27bc08b9..47f6c5c1010d7d 100644
--- a/clang/test/Modules/Inputs/module.modulemap
+++ b/clang/test/Modules/Inputs/module.modulemap
@@ -260,11 +260,6 @@ module cxx_decls_merged {
   header "cxx-decls-merged.h"
 }
 
-module config {
-  header "config.h"
-  config_macros [exhaustive] WANT_FOO, WANT_BAR
-}
-
 module diag_flags {
   header "diag_flags.h"
 }
diff --git a/clang/test/Modules/config_macros.m 
b/clang/test/Modules/config_macros.m
index 15e2c16606ba29..65dd2a89a05c32 100644
--- a/clang/test/Mod

[clang] [clang] Diagnose config_macros before building modules (PR #83641)

2024-03-01 Thread Michael Spencer via cfe-commits

https://github.com/Bigcheese updated 
https://github.com/llvm/llvm-project/pull/83641

>From c2445d426e374592522ac392254c9909ab52fc40 Mon Sep 17 00:00:00 2001
From: Michael Spencer 
Date: Fri, 1 Mar 2024 17:18:20 -0800
Subject: [PATCH] [clang] Diagnose config_macros before building modules

Before this patch, if a module fails to build because of a missing
config_macro, the user will never see the config macro warning. This
patch diagnoses this before building, and each subsequent time a
module is imported.

rdar://123921931
---
 clang/lib/Frontend/CompilerInstance.cpp| 29 ++
 clang/test/Modules/Inputs/config.h |  7 
 clang/test/Modules/Inputs/module.modulemap |  5 ---
 clang/test/Modules/config_macros.m | 45 --
 4 files changed, 63 insertions(+), 23 deletions(-)
 delete mode 100644 clang/test/Modules/Inputs/config.h

diff --git a/clang/lib/Frontend/CompilerInstance.cpp 
b/clang/lib/Frontend/CompilerInstance.cpp
index 4443073775..378f940d8da2cf 100644
--- a/clang/lib/Frontend/CompilerInstance.cpp
+++ b/clang/lib/Frontend/CompilerInstance.cpp
@@ -1591,6 +1591,14 @@ static void checkConfigMacro(Preprocessor &PP, StringRef 
ConfigMacro,
   }
 }
 
+static void checkConfigMacros(Preprocessor &PP, Module *M,
+  SourceLocation ImportLoc) {
+  clang::Module *TopModule = M->getTopLevelModule();
+  for (unsigned I = 0, N = TopModule->ConfigMacros.size(); I != N; ++I) {
+checkConfigMacro(PP, TopModule->ConfigMacros[I], M, ImportLoc);
+  }
+}
+
 /// Write a new timestamp file with the given path.
 static void writeTimestampFile(StringRef TimestampFile) {
   std::error_code EC;
@@ -1829,6 +1837,13 @@ ModuleLoadResult 
CompilerInstance::findOrCompileModuleAndReadAST(
   Module *M =
   HS.lookupModule(ModuleName, ImportLoc, true, !IsInclusionDirective);
 
+  // Check for any configuration macros that have changed. This is done
+  // immediately before potentially building a module in case this module
+  // depends on having one of its configuration macros defined to successfully
+  // build. If this is not done the user will never see the warning.
+  if (M)
+checkConfigMacros(getPreprocessor(), M, ImportLoc);
+
   // Select the source and filename for loading the named module.
   std::string ModuleFilename;
   ModuleSource Source =
@@ -2006,6 +2021,11 @@ CompilerInstance::loadModule(SourceLocation ImportLoc,
   if (auto MaybeModule = MM.getCachedModuleLoad(*Path[0].first)) {
 // Use the cached result, which may be nullptr.
 Module = *MaybeModule;
+// Config macros are already checked before building a module, but they 
need
+// to be checked at each import location in case any of the config macros
+// have a new value at the current `ImportLoc`.
+if (Module)
+  checkConfigMacros(getPreprocessor(), Module, ImportLoc);
   } else if (ModuleName == getLangOpts().CurrentModule) {
 // This is the module we're building.
 Module = PP->getHeaderSearchInfo().lookupModule(
@@ -2146,18 +2166,11 @@ CompilerInstance::loadModule(SourceLocation ImportLoc,
 TheASTReader->makeModuleVisible(Module, Visibility, ImportLoc);
   }
 
-  // Check for any configuration macros that have changed.
-  clang::Module *TopModule = Module->getTopLevelModule();
-  for (unsigned I = 0, N = TopModule->ConfigMacros.size(); I != N; ++I) {
-checkConfigMacro(getPreprocessor(), TopModule->ConfigMacros[I],
- Module, ImportLoc);
-  }
-
   // Resolve any remaining module using export_as for this one.
   getPreprocessor()
   .getHeaderSearchInfo()
   .getModuleMap()
-  .resolveLinkAsDependencies(TopModule);
+  .resolveLinkAsDependencies(Module->getTopLevelModule());
 
   LastModuleImportLoc = ImportLoc;
   LastModuleImportResult = ModuleLoadResult(Module);
diff --git a/clang/test/Modules/Inputs/config.h 
b/clang/test/Modules/Inputs/config.h
deleted file mode 100644
index 4c124b0bf82b7c..00
--- a/clang/test/Modules/Inputs/config.h
+++ /dev/null
@@ -1,7 +0,0 @@
-#ifdef WANT_FOO
-int* foo(void);
-#endif
-
-#ifdef WANT_BAR
-char *bar(void);
-#endif
diff --git a/clang/test/Modules/Inputs/module.modulemap 
b/clang/test/Modules/Inputs/module.modulemap
index e7cb4b27bc08b9..47f6c5c1010d7d 100644
--- a/clang/test/Modules/Inputs/module.modulemap
+++ b/clang/test/Modules/Inputs/module.modulemap
@@ -260,11 +260,6 @@ module cxx_decls_merged {
   header "cxx-decls-merged.h"
 }
 
-module config {
-  header "config.h"
-  config_macros [exhaustive] WANT_FOO, WANT_BAR
-}
-
 module diag_flags {
   header "diag_flags.h"
 }
diff --git a/clang/test/Modules/config_macros.m 
b/clang/test/Modules/config_macros.m
index 15e2c16606ba29..65dd2a89a05c32 100644
--- a/clang/test/Modules/config_macros.m
+++ b/clang/test/Modules/config_macros.m
@@ -1,3 +1,39 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: %clang_cc1 -std=c99 -fmodules -fimplicit-module-maps -x objective-c 
-fmodules-cache-path=%t -DWA

[clang] [clang] Diagnose config_macros before building modules (PR #83641)

2024-03-04 Thread Michael Spencer via cfe-commits


@@ -2006,6 +2021,11 @@ CompilerInstance::loadModule(SourceLocation ImportLoc,
   if (auto MaybeModule = MM.getCachedModuleLoad(*Path[0].first)) {
 // Use the cached result, which may be nullptr.
 Module = *MaybeModule;
+// Config macros are already checked before building a module, but they 
need
+// to be checked at each import location in case any of the config macros
+// have a new value at the current `ImportLoc`.
+if (Module)
+  checkConfigMacros(getPreprocessor(), Module, ImportLoc);
   } else if (ModuleName == getLangOpts().CurrentModule) {
 // This is the module we're building.
 Module = PP->getHeaderSearchInfo().lookupModule(

Bigcheese wrote:

Hmm, I assumed it wasn't needed because this is just `-fmodule-name=` which 
causes textual inclusion, but it looks like the original code would warn in 
that case, even though they actually would apply if it was the first inclusion. 
I'll check and see if it matters.

https://github.com/llvm/llvm-project/pull/83641
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang] Diagnose config_macros before building modules (PR #83641)

2024-03-04 Thread Michael Spencer via cfe-commits


@@ -22,7 +58,10 @@
 #define WANT_BAR 1 // expected-note{{macro was defined here}}
 @import config; // expected-warning{{definition of configuration macro 
'WANT_BAR' has no effect on the import of 'config'; pass '-DWANT_BAR=...' on 
the command line to configure the module}}
 
-// RUN: rm -rf %t
-// RUN: %clang_cc1 -std=c99 -fmodules -fimplicit-module-maps -x objective-c 
-fmodules-cache-path=%t -DWANT_FOO=1 -emit-module -fmodule-name=config 
%S/Inputs/module.modulemap
-// RUN: %clang_cc1 -std=c99 -fmodules -fimplicit-module-maps 
-fmodules-cache-path=%t -I %S/Inputs -DWANT_FOO=1 %s -verify
+//--- config_error.m

Bigcheese wrote:

I can just add comments explaining what it's testing above the run line.

https://github.com/llvm/llvm-project/pull/83641
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang] Diagnose config_macros before building modules (PR #83641)

2024-03-04 Thread Michael Spencer via cfe-commits


@@ -22,7 +58,10 @@
 #define WANT_BAR 1 // expected-note{{macro was defined here}}
 @import config; // expected-warning{{definition of configuration macro 
'WANT_BAR' has no effect on the import of 'config'; pass '-DWANT_BAR=...' on 
the command line to configure the module}}
 
-// RUN: rm -rf %t
-// RUN: %clang_cc1 -std=c99 -fmodules -fimplicit-module-maps -x objective-c 
-fmodules-cache-path=%t -DWANT_FOO=1 -emit-module -fmodule-name=config 
%S/Inputs/module.modulemap
-// RUN: %clang_cc1 -std=c99 -fmodules -fimplicit-module-maps 
-fmodules-cache-path=%t -I %S/Inputs -DWANT_FOO=1 %s -verify
+//--- config_error.m
 
+#ifdef TEST_ERROR

Bigcheese wrote:

Not needed, that was left over from when I tried to use a single file for all 
the cases.

https://github.com/llvm/llvm-project/pull/83641
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang] Diagnose config_macros before building modules (PR #83641)

2024-03-04 Thread Michael Spencer via cfe-commits


@@ -22,7 +58,10 @@
 #define WANT_BAR 1 // expected-note{{macro was defined here}}
 @import config; // expected-warning{{definition of configuration macro 
'WANT_BAR' has no effect on the import of 'config'; pass '-DWANT_BAR=...' on 
the command line to configure the module}}
 
-// RUN: rm -rf %t
-// RUN: %clang_cc1 -std=c99 -fmodules -fimplicit-module-maps -x objective-c 
-fmodules-cache-path=%t -DWANT_FOO=1 -emit-module -fmodule-name=config 
%S/Inputs/module.modulemap
-// RUN: %clang_cc1 -std=c99 -fmodules -fimplicit-module-maps 
-fmodules-cache-path=%t -I %S/Inputs -DWANT_FOO=1 %s -verify
+//--- config_error.m
 
+#ifdef TEST_ERROR
+#define SOME_VALUE 5 // expected-note{{macro was defined here}}
+@import config_error; // expected-error{{could not build module}} \
+  // expected-warning{{definition of configuration macro 
'SOME_VALUE' has no effect on the import of 'config_error';}}

Bigcheese wrote:

There were already other tests that check the exact wording, intent here was 
just to make it easier to reword if ever needed, as this specific test doesn't 
care about the exact wording.

https://github.com/llvm/llvm-project/pull/83641
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang] Diagnose config_macros before building modules (PR #83641)

2024-03-04 Thread Michael Spencer via cfe-commits


@@ -2006,6 +2021,11 @@ CompilerInstance::loadModule(SourceLocation ImportLoc,
   if (auto MaybeModule = MM.getCachedModuleLoad(*Path[0].first)) {
 // Use the cached result, which may be nullptr.
 Module = *MaybeModule;
+// Config macros are already checked before building a module, but they 
need
+// to be checked at each import location in case any of the config macros
+// have a new value at the current `ImportLoc`.
+if (Module)
+  checkConfigMacros(getPreprocessor(), Module, ImportLoc);
   } else if (ModuleName == getLangOpts().CurrentModule) {
 // This is the module we're building.
 Module = PP->getHeaderSearchInfo().lookupModule(

Bigcheese wrote:

Yeah, given how `Preprocessor::HandleHeaderIncludeOrImport` works, you will 
never hit this case.

https://github.com/llvm/llvm-project/pull/83641
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang] Diagnose config_macros before building modules (PR #83641)

2024-03-04 Thread Michael Spencer via cfe-commits

https://github.com/Bigcheese updated 
https://github.com/llvm/llvm-project/pull/83641

>From e8993b51f0dcdecd2fcb72f91d7e4631e95c2c09 Mon Sep 17 00:00:00 2001
From: Michael Spencer 
Date: Fri, 1 Mar 2024 17:18:20 -0800
Subject: [PATCH] [clang] Diagnose config_macros before building modules

Before this patch, if a module fails to build because of a missing
config_macro, the user will never see the config macro warning. This
patch diagnoses this before building, and each subsequent time a
module is imported.

rdar://123921931
---
 clang/lib/Frontend/CompilerInstance.cpp| 35 ++
 clang/test/Modules/Inputs/config.h |  7 ---
 clang/test/Modules/Inputs/module.modulemap |  5 --
 clang/test/Modules/config_macros.m | 54 --
 4 files changed, 78 insertions(+), 23 deletions(-)
 delete mode 100644 clang/test/Modules/Inputs/config.h

diff --git a/clang/lib/Frontend/CompilerInstance.cpp 
b/clang/lib/Frontend/CompilerInstance.cpp
index 4443073775..ec4e68209d657d 100644
--- a/clang/lib/Frontend/CompilerInstance.cpp
+++ b/clang/lib/Frontend/CompilerInstance.cpp
@@ -1591,6 +1591,14 @@ static void checkConfigMacro(Preprocessor &PP, StringRef 
ConfigMacro,
   }
 }
 
+static void checkConfigMacros(Preprocessor &PP, Module *M,
+  SourceLocation ImportLoc) {
+  clang::Module *TopModule = M->getTopLevelModule();
+  for (const StringRef ConMacro : TopModule->ConfigMacros) {
+checkConfigMacro(PP, ConMacro, M, ImportLoc);
+  }
+}
+
 /// Write a new timestamp file with the given path.
 static void writeTimestampFile(StringRef TimestampFile) {
   std::error_code EC;
@@ -1829,6 +1837,13 @@ ModuleLoadResult 
CompilerInstance::findOrCompileModuleAndReadAST(
   Module *M =
   HS.lookupModule(ModuleName, ImportLoc, true, !IsInclusionDirective);
 
+  // Check for any configuration macros that have changed. This is done
+  // immediately before potentially building a module in case this module
+  // depends on having one of its configuration macros defined to successfully
+  // build. If this is not done the user will never see the warning.
+  if (M)
+checkConfigMacros(getPreprocessor(), M, ImportLoc);
+
   // Select the source and filename for loading the named module.
   std::string ModuleFilename;
   ModuleSource Source =
@@ -2006,12 +2021,23 @@ CompilerInstance::loadModule(SourceLocation ImportLoc,
   if (auto MaybeModule = MM.getCachedModuleLoad(*Path[0].first)) {
 // Use the cached result, which may be nullptr.
 Module = *MaybeModule;
+// Config macros are already checked before building a module, but they 
need
+// to be checked at each import location in case any of the config macros
+// have a new value at the current `ImportLoc`.
+if (Module)
+  checkConfigMacros(getPreprocessor(), Module, ImportLoc);
   } else if (ModuleName == getLangOpts().CurrentModule) {
 // This is the module we're building.
 Module = PP->getHeaderSearchInfo().lookupModule(
 ModuleName, ImportLoc, /*AllowSearch*/ true,
 /*AllowExtraModuleMapSearch*/ !IsInclusionDirective);
 
+// Config macros do not need to be checked here for two reasons.
+// * This will always be textual inclusion, and thus the config macros
+//   actually do impact the content of the header.
+// * `Preprocessor::HandleHeaderIncludeOrImport` will never call this
+//   function as the `#include` or `#import` is textual.
+
 MM.cacheModuleLoad(*Path[0].first, Module);
   } else {
 ModuleLoadResult Result = findOrCompileModuleAndReadAST(
@@ -2146,18 +2172,11 @@ CompilerInstance::loadModule(SourceLocation ImportLoc,
 TheASTReader->makeModuleVisible(Module, Visibility, ImportLoc);
   }
 
-  // Check for any configuration macros that have changed.
-  clang::Module *TopModule = Module->getTopLevelModule();
-  for (unsigned I = 0, N = TopModule->ConfigMacros.size(); I != N; ++I) {
-checkConfigMacro(getPreprocessor(), TopModule->ConfigMacros[I],
- Module, ImportLoc);
-  }
-
   // Resolve any remaining module using export_as for this one.
   getPreprocessor()
   .getHeaderSearchInfo()
   .getModuleMap()
-  .resolveLinkAsDependencies(TopModule);
+  .resolveLinkAsDependencies(Module->getTopLevelModule());
 
   LastModuleImportLoc = ImportLoc;
   LastModuleImportResult = ModuleLoadResult(Module);
diff --git a/clang/test/Modules/Inputs/config.h 
b/clang/test/Modules/Inputs/config.h
deleted file mode 100644
index 4c124b0bf82b7c..00
--- a/clang/test/Modules/Inputs/config.h
+++ /dev/null
@@ -1,7 +0,0 @@
-#ifdef WANT_FOO
-int* foo(void);
-#endif
-
-#ifdef WANT_BAR
-char *bar(void);
-#endif
diff --git a/clang/test/Modules/Inputs/module.modulemap 
b/clang/test/Modules/Inputs/module.modulemap
index e7cb4b27bc08b9..47f6c5c1010d7d 100644
--- a/clang/test/Modules/Inputs/module.modulemap
+++ b/clang/test/Modules/Inputs/module.modulemap
@@ -260,11 +260,6 @@ module cxx_decls_merged {
   hea

[clang] [llvm] [clang][DependencyScanner] Remove unused -ivfsoverlay files (PR #73734)

2024-01-30 Thread Michael Spencer via cfe-commits

https://github.com/Bigcheese updated 
https://github.com/llvm/llvm-project/pull/73734

>From 42666e6c0b46a83f0b4fbc7d7945825c56e6ac5a Mon Sep 17 00:00:00 2001
From: Michael Spencer 
Date: Fri, 24 Feb 2023 17:18:51 -0800
Subject: [PATCH] [clang][DepScan] Remove unused -ivfsoverlay files

`-ivfsoverlay` files are unused when building most modules. Enable
removing them by,
* adding a way to visit the filesystem tree with extensible RTTI to
  access each `RedirectingFileSystem`.
* Adding tracking to `RedirectingFileSystem` to record when it
  actually redirects a file access.
* Storing this information in each PCM.

Usage tracking is only enabled when iterating over the source manager
and affecting modulemaps. Here each path is stated to cause an access.
During scanning these stats all hit the cache.
---
 .../Basic/DiagnosticSerializationKinds.td |   3 +
 clang/include/clang/Basic/FileManager.h   |   4 +
 clang/include/clang/Lex/HeaderSearch.h|   7 +
 clang/include/clang/Lex/HeaderSearchOptions.h |   6 +-
 .../include/clang/Serialization/ASTBitCodes.h |   3 +
 clang/include/clang/Serialization/ASTReader.h |  16 +-
 clang/include/clang/Serialization/ASTWriter.h |   4 +-
 .../include/clang/Serialization/ModuleFile.h  |   3 +
 .../DependencyScanningFilesystem.h|   6 +-
 .../DependencyScanningService.h   |  11 +-
 clang/lib/Basic/FileManager.cpp   |   7 +
 clang/lib/Frontend/CompilerInvocation.cpp |   1 +
 clang/lib/Lex/HeaderSearch.cpp|  22 ++
 clang/lib/Serialization/ASTReader.cpp |  36 ++--
 clang/lib/Serialization/ASTWriter.cpp |  52 -
 .../DependencyScanningFilesystem.cpp  |   6 +-
 .../DependencyScanningWorker.cpp  |  86 ++--
 .../DependencyScanning/ModuleDepCollector.cpp |  74 +--
 .../ClangScanDeps/optimize-vfs-edgecases.m|  84 
 clang/test/ClangScanDeps/optimize-vfs-leak.m  | 105 ++
 clang/test/ClangScanDeps/optimize-vfs-pch.m   | 129 
 clang/test/ClangScanDeps/optimize-vfs.m   | 193 ++
 clang/tools/clang-scan-deps/ClangScanDeps.cpp |   1 +
 llvm/include/llvm/Support/VirtualFileSystem.h |  55 -
 llvm/lib/Support/VirtualFileSystem.cpp|  26 +++
 .../Support/VirtualFileSystemTest.cpp |  86 
 26 files changed, 947 insertions(+), 79 deletions(-)
 create mode 100644 clang/test/ClangScanDeps/optimize-vfs-edgecases.m
 create mode 100644 clang/test/ClangScanDeps/optimize-vfs-leak.m
 create mode 100644 clang/test/ClangScanDeps/optimize-vfs-pch.m
 create mode 100644 clang/test/ClangScanDeps/optimize-vfs.m

diff --git a/clang/include/clang/Basic/DiagnosticSerializationKinds.td 
b/clang/include/clang/Basic/DiagnosticSerializationKinds.td
index 11c706ebf84b5..4c4659ed517e0 100644
--- a/clang/include/clang/Basic/DiagnosticSerializationKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSerializationKinds.td
@@ -44,6 +44,9 @@ def err_pch_diagopt_mismatch : Error<"%0 is currently 
enabled, but was not in "
   "the PCH file">;
 def err_pch_modulecache_mismatch : Error<"PCH was compiled with module cache "
   "path '%0', but the path is currently '%1'">;
+def err_pch_vfsoverlay_mismatch : Error<"PCH was compiled with different VFS 
overlay files than are currently in use">;
+def note_pch_vfsoverlay_files : Note<"%select{PCH|current translation unit}0 
has the following VFS overlays:\n%1">;
+def note_pch_vfsoverlay_empty : Note<"%select{PCH|current translation unit}0 
has no VFS overlays">;
 
 def err_pch_version_too_old : Error<
 "PCH file uses an older PCH format that is no longer supported">;
diff --git a/clang/include/clang/Basic/FileManager.h 
b/clang/include/clang/Basic/FileManager.h
index 56cb093dd8c37..997c17a0ffcfc 100644
--- a/clang/include/clang/Basic/FileManager.h
+++ b/clang/include/clang/Basic/FileManager.h
@@ -248,6 +248,10 @@ class FileManager : public RefCountedBase {
 return FS;
   }
 
+  /// Enable or disable tracking of VFS usage. Used to not track full header
+  /// search and implicit modulemap lookup.
+  void trackVFSUsage(bool Active);
+
   void setVirtualFileSystem(IntrusiveRefCntPtr FS) {
 this->FS = std::move(FS);
   }
diff --git a/clang/include/clang/Lex/HeaderSearch.h 
b/clang/include/clang/Lex/HeaderSearch.h
index a2c33842924b1..705dcfa8aacc3 100644
--- a/clang/include/clang/Lex/HeaderSearch.h
+++ b/clang/include/clang/Lex/HeaderSearch.h
@@ -576,6 +576,13 @@ class HeaderSearch {
   /// Note: implicit module maps don't contribute to entry usage.
   std::vector computeUserEntryUsage() const;
 
+  /// Collect which HeaderSearchOptions::VFSOverlayFiles have been meaningfully
+  /// used so far and mark their index with 'true' in the resulting bit vector.
+  ///
+  /// Note: this ignores VFSs that redirect non-affecting files such as unused
+  /// modulemaps.
+  std::vector collectVFSUsageAndClear() const;
+
   /// This method returns a HeaderMap for the specified
   /// FileEntry, uniquing t

[clang] [llvm] [clang][DependencyScanner] Remove unused -ivfsoverlay files (PR #73734)

2024-01-30 Thread Michael Spencer via cfe-commits

https://github.com/Bigcheese closed 
https://github.com/llvm/llvm-project/pull/73734
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] b21a2f9 - [clang][scan-deps] Stop scanning if any scanning setup emits an error.

2024-01-30 Thread Michael Spencer via cfe-commits

Author: Michael Spencer
Date: 2024-01-30T17:03:13-08:00
New Revision: b21a2f9365b6c5fd464a97be5dfe7085742870ef

URL: 
https://github.com/llvm/llvm-project/commit/b21a2f9365b6c5fd464a97be5dfe7085742870ef
DIFF: 
https://github.com/llvm/llvm-project/commit/b21a2f9365b6c5fd464a97be5dfe7085742870ef.diff

LOG: [clang][scan-deps] Stop scanning if any scanning setup emits an error.

Without this scanning will continue and later hit an assert that the
number of `RedirectingFileSystem`s matches the number of -ivfsoverlay
arguments.

Added: 
clang/test/ClangScanDeps/missing-vfs.m

Modified: 
clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp

Removed: 




diff  --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp 
b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
index 390cbe5aa65e1..3cf3ad8a4e490 100644
--- a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
+++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
@@ -322,6 +322,9 @@ class DependencyScanningAction : public tooling::ToolAction 
{
 else
   Action = std::make_unique();
 
+if (ScanInstance.getDiagnostics().hasErrorOccurred())
+  return false;
+
 const bool Result = ScanInstance.ExecuteAction(*Action);
 
 if (Result)

diff  --git a/clang/test/ClangScanDeps/missing-vfs.m 
b/clang/test/ClangScanDeps/missing-vfs.m
new file mode 100644
index 0..e825b00526728
--- /dev/null
+++ b/clang/test/ClangScanDeps/missing-vfs.m
@@ -0,0 +1,18 @@
+// Check that a missing VFS errors before trying to scan anything.
+
+// RUN: rm -rf %t && split-file %s %t
+// RUN: sed -e "s|DIR|%/t|g" %t/build/cdb.json.in > %t/build/cdb.json
+// RUN: not clang-scan-deps -compilation-database %t/build/cdb.json \
+// RUN:   -format experimental-full 2>&1 | FileCheck %s
+
+// CHECK: virtual filesystem overlay file
+// CHECK: not found
+
+//--- build/cdb.json.in
+[{
+  "directory": "DIR",
+  "command": "clang -c DIR/tu.m -ivfsoverlay DIR/vfs.yaml",
+  "file": "DIR/tu.m"
+}]
+
+//--- tu.m



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][DependencyScanner] Remove unused -fmodule-map-file arguments (PR #80090)

2024-01-30 Thread Michael Spencer via cfe-commits

https://github.com/Bigcheese created 
https://github.com/llvm/llvm-project/pull/80090

Since we already add a `-fmodule-map-file=` argument for every used modulemap, 
we can remove all `ModuleMapFiles` entries before adding them.

This reduces the number of module variants when `-fmodule-map-file=` appears on 
the original command line.

>From 73c023e0f1b8d81ee25c75c19bfd0322675fcf44 Mon Sep 17 00:00:00 2001
From: Michael Spencer 
Date: Tue, 30 Jan 2024 17:42:48 -0800
Subject: [PATCH] [clang][DependencyScanner] Remove unused -fmodule-map-file
 arguments

Since we already add a `-fmodule-map-file=` argument for every used
modulemap, we can remove all `ModuleMapFiles` entries before adding
them.

This reduces the number of module variants when `-fmodule-map-file=`
appears on the original command line.
---
 .../DependencyScanning/ModuleDepCollector.cpp |  4 ++
 .../test/ClangScanDeps/optimize-fmodulemap.m  | 66 +++
 2 files changed, 70 insertions(+)
 create mode 100644 clang/test/ClangScanDeps/optimize-fmodulemap.m

diff --git a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp 
b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
index b807dc8432185..995d8b2899c8d 100644
--- a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
+++ b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
@@ -211,6 +211,10 @@ 
ModuleDepCollector::getInvocationAdjustedForModuleBuildWithoutOutputs(
   ScanInstance.getFileManager().getFile(Deps.ClangModuleMapFile);
   assert(CurrentModuleMapEntry && "module map file entry not found");
 
+  // Remove directly passed modulemap files. They will get added back if they
+  // were actually used.
+  CI.getMutFrontendOpts().ModuleMapFiles.clear();
+
   auto DepModuleMapFiles = collectModuleMapFiles(Deps.ClangModuleDeps);
   for (StringRef ModuleMapFile : Deps.ModuleMapFileDeps) {
 // TODO: Track these as `FileEntryRef` to simplify the equality check 
below.
diff --git a/clang/test/ClangScanDeps/optimize-fmodulemap.m 
b/clang/test/ClangScanDeps/optimize-fmodulemap.m
new file mode 100644
index 0..5e9affb30b9c1
--- /dev/null
+++ b/clang/test/ClangScanDeps/optimize-fmodulemap.m
@@ -0,0 +1,66 @@
+// Check that unused directly passed -fmodule-map-file options get dropped.
+
+// RUN: rm -rf %t && split-file %s %t
+// RUN: sed -e "s|DIR|%/t|g" %t/build/cdb.json.in > %t/build/cdb.json
+// RUN: clang-scan-deps -compilation-database %t/build/cdb.json \
+// RUN:   -format experimental-full > %t/deps.json
+// RUN: cat %t/deps.json | sed 's:\?:/:g' | FileCheck %s -DPREFIX=%/t
+
+// CHECK:  {
+// CHECK-NEXT:   "modules": [
+// CHECK-NEXT: {
+// CHECK-NEXT:   "clang-module-deps": [
+// CHECK-NEXT: {
+// CHECK-NEXT:   "context-hash": "{{.*}}",
+// CHECK-NEXT:   "module-name": "B"
+// CHECK-NEXT: }
+// CHECK-NEXT:   ],
+// CHECK-NEXT:   "clang-modulemap-file": 
"[[PREFIX]]/modules/A/module.modulemap",
+// CHECK-NEXT:   "command-line": [
+// CHECK-NOT:  
"-fmodule-map-file=[[PREFIX]]/modules/A/module.modulemap"
+// CHECK:  
"-fmodule-map-file=[[PREFIX]]/modules/B/module.modulemap"
+// CHECK-NOT:  
"-fmodule-map-file=[[PREFIX]]/modules/A/module.modulemap"
+// CHECK:],
+// CHECK-NEXT:   "context-hash": "{{.*}}",
+// CHECK-NEXT:   "file-deps": [
+// CHECK:],
+// CHECK-NEXT:   "name": "A"
+// CHECK-NEXT: },
+// CHECK-NEXT: {
+// CHECK-NEXT:   "clang-module-deps": [],
+// CHECK-NEXT:   "clang-modulemap-file": 
"[[PREFIX]]/modules/B/module.modulemap",
+// CHECK-NEXT:   "command-line": [
+// CHECK-NOT:  "-fmodule-map-file=
+// CHECK:],
+// CHECK-NEXT:   "context-hash": "{{.*}}",
+// CHECK-NEXT:   "file-deps": [
+// CHECK:],
+// CHECK-NEXT:   "name": "B"
+// CHECK-NEXT: }
+// CHECK-NEXT:   ],
+// CHECK-NEXT:   "translation-units": [
+// CHECK:]
+// CHECK:  }
+
+//--- build/cdb.json.in
+[{
+  "directory": "DIR",
+  "command": "clang -c DIR/tu.m -I DIR/modules/B 
-fmodule-map-file=DIR/modules/A/module.modulemap -fmodules 
-fmodules-cache-path=DIR/cache -fimplicit-module-maps",
+  "file": "DIR/tu.m"
+}]
+
+//--- build/vfs.yaml.in
+
+//--- tu.m
+@import A;
+
+//--- modules/A/module.modulemap
+module A { header "A.h" }
+
+//--- modules/A/A.h
+#include 
+
+//--- modules/B/module.modulemap
+module B { header "B.h" }
+
+//--- modules/B/B.h

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][DependencyScanner] Remove unused -fmodule-map-file arguments (PR #80090)

2024-01-31 Thread Michael Spencer via cfe-commits

https://github.com/Bigcheese closed 
https://github.com/llvm/llvm-project/pull/80090
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [llvm] [clang][DependencyScanner] Remove unused -ivfsoverlay files (PR #73734)

2024-01-25 Thread Michael Spencer via cfe-commits

https://github.com/Bigcheese updated 
https://github.com/llvm/llvm-project/pull/73734

>From 3c7f36b087e09e1b7ab3231e8267bcdd8400fac4 Mon Sep 17 00:00:00 2001
From: Michael Spencer 
Date: Fri, 24 Feb 2023 17:18:51 -0800
Subject: [PATCH] [clang][DepScan] Remove unused -ivfsoverlay files

`-ivfsoverlay` files are unused when building most modules. Enable
removing them by,
* adding a way to visit the filesystem tree with extensible RTTI to
  access each `RedirectingFileSystem`.
* Adding tracking to `RedirectingFileSystem` to record when it
  actually redirects a file access.
* Storing this information in each PCM.

Usage tracking is only enabled when iterating over the source manager
and affecting modulemaps. Here each path is stated to cause an access.
During scanning these stats all hit the cache.
---
 .../Basic/DiagnosticSerializationKinds.td |   3 +
 clang/include/clang/Basic/FileManager.h   |   4 +
 clang/include/clang/Lex/HeaderSearch.h|   6 +
 .../include/clang/Serialization/ASTBitCodes.h |   3 +
 clang/include/clang/Serialization/ASTReader.h |   6 +-
 .../include/clang/Serialization/ModuleFile.h  |   3 +
 .../DependencyScanningFilesystem.h|   6 +-
 .../DependencyScanningService.h   |  11 +-
 clang/lib/Basic/FileManager.cpp   |   7 +
 clang/lib/Frontend/CompilerInvocation.cpp |   1 +
 clang/lib/Lex/HeaderSearch.cpp|  16 ++
 clang/lib/Serialization/ASTReader.cpp |  41 ++--
 clang/lib/Serialization/ASTWriter.cpp |  35 +++-
 .../DependencyScanningFilesystem.cpp  |   6 +-
 .../DependencyScanningWorker.cpp  |  74 +--
 .../DependencyScanning/ModuleDepCollector.cpp |  74 +--
 .../ClangScanDeps/optimize-vfs-edgecases.m|  84 
 clang/test/ClangScanDeps/optimize-vfs-leak.m  | 105 ++
 clang/test/ClangScanDeps/optimize-vfs-pch.m   | 129 
 clang/test/ClangScanDeps/optimize-vfs.m   | 193 ++
 clang/tools/clang-scan-deps/ClangScanDeps.cpp |   1 +
 llvm/include/llvm/Support/VirtualFileSystem.h |  55 -
 llvm/lib/Support/VirtualFileSystem.cpp|  26 +++
 .../Support/VirtualFileSystemTest.cpp |  84 
 24 files changed, 906 insertions(+), 67 deletions(-)
 create mode 100644 clang/test/ClangScanDeps/optimize-vfs-edgecases.m
 create mode 100644 clang/test/ClangScanDeps/optimize-vfs-leak.m
 create mode 100644 clang/test/ClangScanDeps/optimize-vfs-pch.m
 create mode 100644 clang/test/ClangScanDeps/optimize-vfs.m

diff --git a/clang/include/clang/Basic/DiagnosticSerializationKinds.td 
b/clang/include/clang/Basic/DiagnosticSerializationKinds.td
index 11c706ebf84b542..4c4659ed517e0a0 100644
--- a/clang/include/clang/Basic/DiagnosticSerializationKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSerializationKinds.td
@@ -44,6 +44,9 @@ def err_pch_diagopt_mismatch : Error<"%0 is currently 
enabled, but was not in "
   "the PCH file">;
 def err_pch_modulecache_mismatch : Error<"PCH was compiled with module cache "
   "path '%0', but the path is currently '%1'">;
+def err_pch_vfsoverlay_mismatch : Error<"PCH was compiled with different VFS 
overlay files than are currently in use">;
+def note_pch_vfsoverlay_files : Note<"%select{PCH|current translation unit}0 
has the following VFS overlays:\n%1">;
+def note_pch_vfsoverlay_empty : Note<"%select{PCH|current translation unit}0 
has no VFS overlays">;
 
 def err_pch_version_too_old : Error<
 "PCH file uses an older PCH format that is no longer supported">;
diff --git a/clang/include/clang/Basic/FileManager.h 
b/clang/include/clang/Basic/FileManager.h
index 56cb093dd8c376f..997c17a0ffcfcce 100644
--- a/clang/include/clang/Basic/FileManager.h
+++ b/clang/include/clang/Basic/FileManager.h
@@ -248,6 +248,10 @@ class FileManager : public RefCountedBase {
 return FS;
   }
 
+  /// Enable or disable tracking of VFS usage. Used to not track full header
+  /// search and implicit modulemap lookup.
+  void trackVFSUsage(bool Active);
+
   void setVirtualFileSystem(IntrusiveRefCntPtr FS) {
 this->FS = std::move(FS);
   }
diff --git a/clang/include/clang/Lex/HeaderSearch.h 
b/clang/include/clang/Lex/HeaderSearch.h
index a2c33842924b101..49e1a4a124b55df 100644
--- a/clang/include/clang/Lex/HeaderSearch.h
+++ b/clang/include/clang/Lex/HeaderSearch.h
@@ -576,6 +576,12 @@ class HeaderSearch {
   /// Note: implicit module maps don't contribute to entry usage.
   std::vector computeUserEntryUsage() const;
 
+  /// Determine which HeaderSearchOptions::VFSOverlayFiles have been
+  /// successfully used so far and mark their index with 'true' in the 
resulting
+  /// bit vector.
+  /// Note: implicit module maps don't contribute to entry usage.
+  std::vector computeVFSUsageAndClear() const;
+
   /// This method returns a HeaderMap for the specified
   /// FileEntry, uniquing them through the 'HeaderMaps' datastructure.
   const HeaderMap *CreateHeaderMap(FileEntryRef FE);
diff --git a/clang/include/clang/

[llvm] [clang] [clang][DependencyScanner] Remove unused -ivfsoverlay files (PR #73734)

2024-01-25 Thread Michael Spencer via cfe-commits

Bigcheese wrote:

I've updated the patch to use the alternative implementation Jan suggested.

https://github.com/llvm/llvm-project/pull/73734
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [llvm] [clang][DependencyScanner] Remove unused -ivfsoverlay files (PR #73734)

2024-01-25 Thread Michael Spencer via cfe-commits

https://github.com/Bigcheese updated 
https://github.com/llvm/llvm-project/pull/73734

>From 0559ec44d2d3c39292bae6d6431dde9626ac755e Mon Sep 17 00:00:00 2001
From: Michael Spencer 
Date: Fri, 24 Feb 2023 17:18:51 -0800
Subject: [PATCH] [clang][DepScan] Remove unused -ivfsoverlay files

`-ivfsoverlay` files are unused when building most modules. Enable
removing them by,
* adding a way to visit the filesystem tree with extensible RTTI to
  access each `RedirectingFileSystem`.
* Adding tracking to `RedirectingFileSystem` to record when it
  actually redirects a file access.
* Storing this information in each PCM.

Usage tracking is only enabled when iterating over the source manager
and affecting modulemaps. Here each path is stated to cause an access.
During scanning these stats all hit the cache.
---
 .../Basic/DiagnosticSerializationKinds.td |   3 +
 clang/include/clang/Basic/FileManager.h   |   4 +
 clang/include/clang/Lex/HeaderSearch.h|   6 +
 clang/include/clang/Lex/HeaderSearchOptions.h |   6 +-
 .../include/clang/Serialization/ASTBitCodes.h |   3 +
 clang/include/clang/Serialization/ASTReader.h |  16 +-
 .../include/clang/Serialization/ModuleFile.h  |   3 +
 .../DependencyScanningFilesystem.h|   6 +-
 .../DependencyScanningService.h   |  11 +-
 clang/lib/Basic/FileManager.cpp   |   7 +
 clang/lib/Frontend/CompilerInvocation.cpp |   1 +
 clang/lib/Lex/HeaderSearch.cpp|  16 ++
 clang/lib/Serialization/ASTReader.cpp |  41 ++--
 clang/lib/Serialization/ASTWriter.cpp |  41 +++-
 .../DependencyScanningFilesystem.cpp  |   6 +-
 .../DependencyScanningWorker.cpp  |  86 ++--
 .../DependencyScanning/ModuleDepCollector.cpp |  74 +--
 .../ClangScanDeps/optimize-vfs-edgecases.m|  84 
 clang/test/ClangScanDeps/optimize-vfs-leak.m  | 105 ++
 clang/test/ClangScanDeps/optimize-vfs-pch.m   | 129 
 clang/test/ClangScanDeps/optimize-vfs.m   | 193 ++
 clang/tools/clang-scan-deps/ClangScanDeps.cpp |   1 +
 llvm/include/llvm/Support/VirtualFileSystem.h |  55 -
 llvm/lib/Support/VirtualFileSystem.cpp|  26 +++
 .../Support/VirtualFileSystemTest.cpp |  86 
 25 files changed, 932 insertions(+), 77 deletions(-)
 create mode 100644 clang/test/ClangScanDeps/optimize-vfs-edgecases.m
 create mode 100644 clang/test/ClangScanDeps/optimize-vfs-leak.m
 create mode 100644 clang/test/ClangScanDeps/optimize-vfs-pch.m
 create mode 100644 clang/test/ClangScanDeps/optimize-vfs.m

diff --git a/clang/include/clang/Basic/DiagnosticSerializationKinds.td 
b/clang/include/clang/Basic/DiagnosticSerializationKinds.td
index 11c706ebf84b542..4c4659ed517e0a0 100644
--- a/clang/include/clang/Basic/DiagnosticSerializationKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSerializationKinds.td
@@ -44,6 +44,9 @@ def err_pch_diagopt_mismatch : Error<"%0 is currently 
enabled, but was not in "
   "the PCH file">;
 def err_pch_modulecache_mismatch : Error<"PCH was compiled with module cache "
   "path '%0', but the path is currently '%1'">;
+def err_pch_vfsoverlay_mismatch : Error<"PCH was compiled with different VFS 
overlay files than are currently in use">;
+def note_pch_vfsoverlay_files : Note<"%select{PCH|current translation unit}0 
has the following VFS overlays:\n%1">;
+def note_pch_vfsoverlay_empty : Note<"%select{PCH|current translation unit}0 
has no VFS overlays">;
 
 def err_pch_version_too_old : Error<
 "PCH file uses an older PCH format that is no longer supported">;
diff --git a/clang/include/clang/Basic/FileManager.h 
b/clang/include/clang/Basic/FileManager.h
index 56cb093dd8c376f..997c17a0ffcfcce 100644
--- a/clang/include/clang/Basic/FileManager.h
+++ b/clang/include/clang/Basic/FileManager.h
@@ -248,6 +248,10 @@ class FileManager : public RefCountedBase {
 return FS;
   }
 
+  /// Enable or disable tracking of VFS usage. Used to not track full header
+  /// search and implicit modulemap lookup.
+  void trackVFSUsage(bool Active);
+
   void setVirtualFileSystem(IntrusiveRefCntPtr FS) {
 this->FS = std::move(FS);
   }
diff --git a/clang/include/clang/Lex/HeaderSearch.h 
b/clang/include/clang/Lex/HeaderSearch.h
index a2c33842924b101..49e1a4a124b55df 100644
--- a/clang/include/clang/Lex/HeaderSearch.h
+++ b/clang/include/clang/Lex/HeaderSearch.h
@@ -576,6 +576,12 @@ class HeaderSearch {
   /// Note: implicit module maps don't contribute to entry usage.
   std::vector computeUserEntryUsage() const;
 
+  /// Determine which HeaderSearchOptions::VFSOverlayFiles have been
+  /// successfully used so far and mark their index with 'true' in the 
resulting
+  /// bit vector.
+  /// Note: implicit module maps don't contribute to entry usage.
+  std::vector computeVFSUsageAndClear() const;
+
   /// This method returns a HeaderMap for the specified
   /// FileEntry, uniquing them through the 'HeaderMaps' datastructure.
   const HeaderMap *CreateHead

[llvm] [clang] [clang][DependencyScanner] Remove unused -ivfsoverlay files (PR #73734)

2024-01-29 Thread Michael Spencer via cfe-commits

https://github.com/Bigcheese updated 
https://github.com/llvm/llvm-project/pull/73734

>From 0559ec44d2d3c39292bae6d6431dde9626ac755e Mon Sep 17 00:00:00 2001
From: Michael Spencer 
Date: Fri, 24 Feb 2023 17:18:51 -0800
Subject: [PATCH 1/2] [clang][DepScan] Remove unused -ivfsoverlay files

`-ivfsoverlay` files are unused when building most modules. Enable
removing them by,
* adding a way to visit the filesystem tree with extensible RTTI to
  access each `RedirectingFileSystem`.
* Adding tracking to `RedirectingFileSystem` to record when it
  actually redirects a file access.
* Storing this information in each PCM.

Usage tracking is only enabled when iterating over the source manager
and affecting modulemaps. Here each path is stated to cause an access.
During scanning these stats all hit the cache.
---
 .../Basic/DiagnosticSerializationKinds.td |   3 +
 clang/include/clang/Basic/FileManager.h   |   4 +
 clang/include/clang/Lex/HeaderSearch.h|   6 +
 clang/include/clang/Lex/HeaderSearchOptions.h |   6 +-
 .../include/clang/Serialization/ASTBitCodes.h |   3 +
 clang/include/clang/Serialization/ASTReader.h |  16 +-
 .../include/clang/Serialization/ModuleFile.h  |   3 +
 .../DependencyScanningFilesystem.h|   6 +-
 .../DependencyScanningService.h   |  11 +-
 clang/lib/Basic/FileManager.cpp   |   7 +
 clang/lib/Frontend/CompilerInvocation.cpp |   1 +
 clang/lib/Lex/HeaderSearch.cpp|  16 ++
 clang/lib/Serialization/ASTReader.cpp |  41 ++--
 clang/lib/Serialization/ASTWriter.cpp |  41 +++-
 .../DependencyScanningFilesystem.cpp  |   6 +-
 .../DependencyScanningWorker.cpp  |  86 ++--
 .../DependencyScanning/ModuleDepCollector.cpp |  74 +--
 .../ClangScanDeps/optimize-vfs-edgecases.m|  84 
 clang/test/ClangScanDeps/optimize-vfs-leak.m  | 105 ++
 clang/test/ClangScanDeps/optimize-vfs-pch.m   | 129 
 clang/test/ClangScanDeps/optimize-vfs.m   | 193 ++
 clang/tools/clang-scan-deps/ClangScanDeps.cpp |   1 +
 llvm/include/llvm/Support/VirtualFileSystem.h |  55 -
 llvm/lib/Support/VirtualFileSystem.cpp|  26 +++
 .../Support/VirtualFileSystemTest.cpp |  86 
 25 files changed, 932 insertions(+), 77 deletions(-)
 create mode 100644 clang/test/ClangScanDeps/optimize-vfs-edgecases.m
 create mode 100644 clang/test/ClangScanDeps/optimize-vfs-leak.m
 create mode 100644 clang/test/ClangScanDeps/optimize-vfs-pch.m
 create mode 100644 clang/test/ClangScanDeps/optimize-vfs.m

diff --git a/clang/include/clang/Basic/DiagnosticSerializationKinds.td 
b/clang/include/clang/Basic/DiagnosticSerializationKinds.td
index 11c706ebf84b54..4c4659ed517e0a 100644
--- a/clang/include/clang/Basic/DiagnosticSerializationKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSerializationKinds.td
@@ -44,6 +44,9 @@ def err_pch_diagopt_mismatch : Error<"%0 is currently 
enabled, but was not in "
   "the PCH file">;
 def err_pch_modulecache_mismatch : Error<"PCH was compiled with module cache "
   "path '%0', but the path is currently '%1'">;
+def err_pch_vfsoverlay_mismatch : Error<"PCH was compiled with different VFS 
overlay files than are currently in use">;
+def note_pch_vfsoverlay_files : Note<"%select{PCH|current translation unit}0 
has the following VFS overlays:\n%1">;
+def note_pch_vfsoverlay_empty : Note<"%select{PCH|current translation unit}0 
has no VFS overlays">;
 
 def err_pch_version_too_old : Error<
 "PCH file uses an older PCH format that is no longer supported">;
diff --git a/clang/include/clang/Basic/FileManager.h 
b/clang/include/clang/Basic/FileManager.h
index 56cb093dd8c376..997c17a0ffcfcc 100644
--- a/clang/include/clang/Basic/FileManager.h
+++ b/clang/include/clang/Basic/FileManager.h
@@ -248,6 +248,10 @@ class FileManager : public RefCountedBase {
 return FS;
   }
 
+  /// Enable or disable tracking of VFS usage. Used to not track full header
+  /// search and implicit modulemap lookup.
+  void trackVFSUsage(bool Active);
+
   void setVirtualFileSystem(IntrusiveRefCntPtr FS) {
 this->FS = std::move(FS);
   }
diff --git a/clang/include/clang/Lex/HeaderSearch.h 
b/clang/include/clang/Lex/HeaderSearch.h
index a2c33842924b10..49e1a4a124b55d 100644
--- a/clang/include/clang/Lex/HeaderSearch.h
+++ b/clang/include/clang/Lex/HeaderSearch.h
@@ -576,6 +576,12 @@ class HeaderSearch {
   /// Note: implicit module maps don't contribute to entry usage.
   std::vector computeUserEntryUsage() const;
 
+  /// Determine which HeaderSearchOptions::VFSOverlayFiles have been
+  /// successfully used so far and mark their index with 'true' in the 
resulting
+  /// bit vector.
+  /// Note: implicit module maps don't contribute to entry usage.
+  std::vector computeVFSUsageAndClear() const;
+
   /// This method returns a HeaderMap for the specified
   /// FileEntry, uniquing them through the 'HeaderMaps' datastructure.
   const HeaderMap *CreateHeader

[llvm] [clang] [clang][Driver] Don't ignore -gmodules .gch files (PR #77711)

2024-01-11 Thread Michael Spencer via cfe-commits

https://github.com/Bigcheese updated 
https://github.com/llvm/llvm-project/pull/77711

>From ef781002ef63817afa4df4834742ec3c2f22 Mon Sep 17 00:00:00 2001
From: Michael Spencer 
Date: Wed, 10 Jan 2024 16:58:59 -0800
Subject: [PATCH] [clang][Driver] Don't ignore -gmodules .gch files

A previous commit (82f75ed) made clang ignore .gch files that were not
Clang AST files. This broke `-gmodules`, which embeds the Clang AST
into an object file containing debug info.

This changes the probing to detect any valid object file as a
potential Clang PCH.
---
 clang/lib/Driver/ToolChains/Clang.cpp  | 26 +++---
 clang/test/PCH/gch-probe.c |  4 
 llvm/include/llvm/BinaryFormat/Magic.h |  1 +
 llvm/lib/BinaryFormat/Magic.cpp|  2 ++
 llvm/lib/Object/Binary.cpp |  1 +
 llvm/lib/Object/ObjectFile.cpp |  1 +
 6 files changed, 28 insertions(+), 7 deletions(-)

diff --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index 1ee7ae602f3ce5..526c1789651011 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -43,7 +43,9 @@
 #include "clang/Driver/XRayArgs.h"
 #include "llvm/ADT/SmallSet.h"
 #include "llvm/ADT/StringExtras.h"
+#include "llvm/BinaryFormat/Magic.h"
 #include "llvm/Config/llvm-config.h"
+#include "llvm/Object/ObjectFile.h"
 #include "llvm/Option/ArgList.h"
 #include "llvm/Support/CodeGen.h"
 #include "llvm/Support/Compiler.h"
@@ -948,11 +950,21 @@ static void handleAMDGPUCodeObjectVersionOptions(const 
Driver &D,
   }
 }
 
-static bool hasClangPchSignature(const Driver &D, StringRef Path) {
-  if (llvm::ErrorOr> MemBuf =
-  D.getVFS().getBufferForFile(Path))
-return (*MemBuf)->getBuffer().starts_with("CPCH");
-  return false;
+static bool maybeHasClangPchSignature(const Driver &D, StringRef Path) {
+  llvm::ErrorOr> MemBuf =
+  D.getVFS().getBufferForFile(Path);
+  if (!MemBuf)
+return false;
+  llvm::file_magic Magic = llvm::identify_magic((*MemBuf)->getBuffer());
+  if (Magic == llvm::file_magic::unknown)
+return false;
+  // Return true for both raw Clang AST files and object files which may
+  // contain a __clangast section.
+  if (Magic == llvm::file_magic::clang_ast)
+return true;
+  Expected> Obj =
+  llvm::object::ObjectFile::createObjectFile(**MemBuf, Magic);
+  return !Obj.takeError();
 }
 
 static bool gchProbe(const Driver &D, StringRef Path) {
@@ -964,14 +976,14 @@ static bool gchProbe(const Driver &D, StringRef Path) {
 std::error_code EC;
 for (llvm::vfs::directory_iterator DI = D.getVFS().dir_begin(Path, EC), DE;
  !EC && DI != DE; DI = DI.increment(EC)) {
-  if (hasClangPchSignature(D, DI->path()))
+  if (maybeHasClangPchSignature(D, DI->path()))
 return true;
 }
 D.Diag(diag::warn_drv_pch_ignoring_gch_dir) << Path;
 return false;
   }
 
-  if (hasClangPchSignature(D, Path))
+  if (maybeHasClangPchSignature(D, Path))
 return true;
   D.Diag(diag::warn_drv_pch_ignoring_gch_file) << Path;
   return false;
diff --git a/clang/test/PCH/gch-probe.c b/clang/test/PCH/gch-probe.c
index 8b1e1fab5ad97b..0905c9baebdae0 100644
--- a/clang/test/PCH/gch-probe.c
+++ b/clang/test/PCH/gch-probe.c
@@ -2,6 +2,10 @@
 // RUN: %clang -x c-header -c %s -o %t.h.gch
 // RUN: %clang -fsyntax-only -include %t.h %s
 
+// -gmodules embeds the Clang AST file in an object file.
+// RUN: %clang -x c-header -c %s -gmodules -o %t.h.gch
+// RUN: %clang -fsyntax-only -include %t.h %s
+
 // gch probing should ignore files which are not clang pch files.
 // RUN: %clang -fsyntax-only -include %S/Inputs/gch-probe.h %s 2>&1 | 
FileCheck %s
 // CHECK: warning: precompiled header '{{.*}}gch-probe.h.gch' was ignored 
because it is not a clang PCH file
diff --git a/llvm/include/llvm/BinaryFormat/Magic.h 
b/llvm/include/llvm/BinaryFormat/Magic.h
index c635a269576587..6978c066bda468 100644
--- a/llvm/include/llvm/BinaryFormat/Magic.h
+++ b/llvm/include/llvm/BinaryFormat/Magic.h
@@ -21,6 +21,7 @@ struct file_magic {
   enum Impl {
 unknown = 0,   ///< Unrecognized file
 bitcode,   ///< Bitcode file
+clang_ast, ///< Clang PCH or PCM
 archive,   ///< ar style archive file
 elf,   ///< ELF Unknown type
 elf_relocatable,   ///< ELF Relocatable object file
diff --git a/llvm/lib/BinaryFormat/Magic.cpp b/llvm/lib/BinaryFormat/Magic.cpp
index 45a0b7e11452b4..bd378337ed3338 100644
--- a/llvm/lib/BinaryFormat/Magic.cpp
+++ b/llvm/lib/BinaryFormat/Magic.cpp
@@ -98,6 +98,8 @@ file_magic llvm::identify_magic(StringRef Magic) {
   case 'C':
 if (startswith(Magic, "CCOB"))
   return file_magic::offload_bundle_compressed;
+if (startswith(Magic, "CPCH"))
+  return file_magic::clang_ast;
 break;
   case '!':
 if (startswith(Magic, "!\n") || startswith(Magic, "!\n"))
diff --git a/llvm/lib/Object/Binary.cpp b/llvm/lib/Object/Binary.cpp
index 0b9d95485287dc

[llvm] [clang] [clang][Driver] Don't ignore -gmodules .gch files (PR #77711)

2024-01-11 Thread Michael Spencer via cfe-commits

Bigcheese wrote:

I updated the patch to try to actually open the object file.

https://github.com/llvm/llvm-project/pull/77711
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[llvm] [clang] [clang][Driver] Don't ignore -gmodules .gch files (PR #77711)

2024-01-15 Thread Michael Spencer via cfe-commits

https://github.com/Bigcheese updated 
https://github.com/llvm/llvm-project/pull/77711

>From d3e928fc5b725cb3e484a8cfd50fa23c26f7eb22 Mon Sep 17 00:00:00 2001
From: Michael Spencer 
Date: Wed, 10 Jan 2024 16:58:59 -0800
Subject: [PATCH] [clang][Driver] Don't ignore -gmodules .gch files

A previous commit (82f75ed) made clang ignore .gch files that were not
Clang AST files. This broke `-gmodules`, which embeds the Clang AST
into an object file containing debug info.

This changes the probing to detect any valid object file as a
potential Clang PCH.
---
 clang/lib/Driver/ToolChains/Clang.cpp  | 26 +++---
 clang/test/PCH/gch-probe.c |  4 
 llvm/include/llvm/BinaryFormat/Magic.h |  1 +
 llvm/lib/BinaryFormat/Magic.cpp|  2 ++
 llvm/lib/Object/Binary.cpp |  1 +
 llvm/lib/Object/ObjectFile.cpp |  1 +
 6 files changed, 28 insertions(+), 7 deletions(-)

diff --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index 9edae3fec91a87..997ec2d491d02c 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -43,7 +43,9 @@
 #include "clang/Driver/XRayArgs.h"
 #include "llvm/ADT/SmallSet.h"
 #include "llvm/ADT/StringExtras.h"
+#include "llvm/BinaryFormat/Magic.h"
 #include "llvm/Config/llvm-config.h"
+#include "llvm/Object/ObjectFile.h"
 #include "llvm/Option/ArgList.h"
 #include "llvm/Support/CodeGen.h"
 #include "llvm/Support/Compiler.h"
@@ -948,11 +950,21 @@ static void handleAMDGPUCodeObjectVersionOptions(const 
Driver &D,
   }
 }
 
-static bool hasClangPchSignature(const Driver &D, StringRef Path) {
-  if (llvm::ErrorOr> MemBuf =
-  D.getVFS().getBufferForFile(Path))
-return (*MemBuf)->getBuffer().starts_with("CPCH");
-  return false;
+static bool maybeHasClangPchSignature(const Driver &D, StringRef Path) {
+  llvm::ErrorOr> MemBuf =
+  D.getVFS().getBufferForFile(Path);
+  if (!MemBuf)
+return false;
+  llvm::file_magic Magic = llvm::identify_magic((*MemBuf)->getBuffer());
+  if (Magic == llvm::file_magic::unknown)
+return false;
+  // Return true for both raw Clang AST files and object files which may
+  // contain a __clangast section.
+  if (Magic == llvm::file_magic::clang_ast)
+return true;
+  Expected> Obj =
+  llvm::object::ObjectFile::createObjectFile(**MemBuf, Magic);
+  return !Obj.takeError();
 }
 
 static bool gchProbe(const Driver &D, StringRef Path) {
@@ -964,14 +976,14 @@ static bool gchProbe(const Driver &D, StringRef Path) {
 std::error_code EC;
 for (llvm::vfs::directory_iterator DI = D.getVFS().dir_begin(Path, EC), DE;
  !EC && DI != DE; DI = DI.increment(EC)) {
-  if (hasClangPchSignature(D, DI->path()))
+  if (maybeHasClangPchSignature(D, DI->path()))
 return true;
 }
 D.Diag(diag::warn_drv_pch_ignoring_gch_dir) << Path;
 return false;
   }
 
-  if (hasClangPchSignature(D, Path))
+  if (maybeHasClangPchSignature(D, Path))
 return true;
   D.Diag(diag::warn_drv_pch_ignoring_gch_file) << Path;
   return false;
diff --git a/clang/test/PCH/gch-probe.c b/clang/test/PCH/gch-probe.c
index 8b1e1fab5ad97b..0905c9baebdae0 100644
--- a/clang/test/PCH/gch-probe.c
+++ b/clang/test/PCH/gch-probe.c
@@ -2,6 +2,10 @@
 // RUN: %clang -x c-header -c %s -o %t.h.gch
 // RUN: %clang -fsyntax-only -include %t.h %s
 
+// -gmodules embeds the Clang AST file in an object file.
+// RUN: %clang -x c-header -c %s -gmodules -o %t.h.gch
+// RUN: %clang -fsyntax-only -include %t.h %s
+
 // gch probing should ignore files which are not clang pch files.
 // RUN: %clang -fsyntax-only -include %S/Inputs/gch-probe.h %s 2>&1 | 
FileCheck %s
 // CHECK: warning: precompiled header '{{.*}}gch-probe.h.gch' was ignored 
because it is not a clang PCH file
diff --git a/llvm/include/llvm/BinaryFormat/Magic.h 
b/llvm/include/llvm/BinaryFormat/Magic.h
index c635a269576587..6978c066bda468 100644
--- a/llvm/include/llvm/BinaryFormat/Magic.h
+++ b/llvm/include/llvm/BinaryFormat/Magic.h
@@ -21,6 +21,7 @@ struct file_magic {
   enum Impl {
 unknown = 0,   ///< Unrecognized file
 bitcode,   ///< Bitcode file
+clang_ast, ///< Clang PCH or PCM
 archive,   ///< ar style archive file
 elf,   ///< ELF Unknown type
 elf_relocatable,   ///< ELF Relocatable object file
diff --git a/llvm/lib/BinaryFormat/Magic.cpp b/llvm/lib/BinaryFormat/Magic.cpp
index 45a0b7e11452b4..bd378337ed3338 100644
--- a/llvm/lib/BinaryFormat/Magic.cpp
+++ b/llvm/lib/BinaryFormat/Magic.cpp
@@ -98,6 +98,8 @@ file_magic llvm::identify_magic(StringRef Magic) {
   case 'C':
 if (startswith(Magic, "CCOB"))
   return file_magic::offload_bundle_compressed;
+if (startswith(Magic, "CPCH"))
+  return file_magic::clang_ast;
 break;
   case '!':
 if (startswith(Magic, "!\n") || startswith(Magic, "!\n"))
diff --git a/llvm/lib/Object/Binary.cpp b/llvm/lib/Object/Binary.cpp
index 0b9d95485287dc

[llvm] [clang] [clang][Driver] Don't ignore -gmodules .gch files (PR #77711)

2024-01-16 Thread Michael Spencer via cfe-commits

https://github.com/Bigcheese closed 
https://github.com/llvm/llvm-project/pull/77711
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [llvm] [clang][Driver] Don't ignore -gmodules .gch files (PR #77711)

2024-01-17 Thread Michael Spencer via cfe-commits

Bigcheese wrote:

Yep, I'll take a look.

https://github.com/llvm/llvm-project/pull/77711
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [llvm] [clang][Driver] Don't ignore -gmodules .gch files (PR #77711)

2024-01-17 Thread Michael Spencer via cfe-commits

Bigcheese wrote:

Ah, looks like all the other gmodules tests have:
```
// Unsupported on AIX because we don't support the requisite "__clangast"
// section in XCOFF yet.
// UNSUPPORTED: target={{.*}}-aix{{.*}}
```

I'll just add that.

https://github.com/llvm/llvm-project/pull/77711
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang] Disable gch-probe.c on AIX as `-gmodules` is not supported there yet. (PR #78513)

2024-01-17 Thread Michael Spencer via cfe-commits

https://github.com/Bigcheese created 
https://github.com/llvm/llvm-project/pull/78513

Followup fix for https://github.com/llvm/llvm-project/pull/77711

>From a12b3ab8663c591f66848a7e46620a79583f5045 Mon Sep 17 00:00:00 2001
From: Michael Spencer 
Date: Wed, 17 Jan 2024 14:27:03 -0800
Subject: [PATCH] [clang] Disable gch-probe.c on AIX as `-gmodules` is not
 supported there yet.

Followup fix for https://github.com/llvm/llvm-project/pull/77711
---
 clang/test/PCH/gch-probe.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/clang/test/PCH/gch-probe.c b/clang/test/PCH/gch-probe.c
index 0905c9baebdae04..370abd26d251749 100644
--- a/clang/test/PCH/gch-probe.c
+++ b/clang/test/PCH/gch-probe.c
@@ -1,3 +1,7 @@
+// Unsupported on AIX because we don't support the requisite "__clangast"
+// section in XCOFF yet.
+// UNSUPPORTED: target={{.*}}-aix{{.*}}
+
 // For GCC compatibility, clang should probe also with the .gch extension.
 // RUN: %clang -x c-header -c %s -o %t.h.gch
 // RUN: %clang -fsyntax-only -include %t.h %s

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang] Disable gch-probe.c on AIX as `-gmodules` is not supported there yet. (PR #78513)

2024-01-17 Thread Michael Spencer via cfe-commits

https://github.com/Bigcheese closed 
https://github.com/llvm/llvm-project/pull/78513
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][modules] Fix CodeGen options that can affect the AST. (PR #78816)

2024-01-19 Thread Michael Spencer via cfe-commits

https://github.com/Bigcheese approved this pull request.

LGTM, although are we sure none of the other options should be affecting? I 
just did a quick search and it seems like this is it.

I don't think it matters here, but the actual option that controls the macro is 
on `LangOptions` and where it's set has a "FIXME: Eliminate this dependency" 
where it directly parses the command line. As long as we're not changing the 
value the `LangOptions` version should always match.

https://github.com/llvm/llvm-project/pull/78816
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Modules] textual headers in submodules never resolve their `use`s (PR #69651)

2023-10-19 Thread Michael Spencer via cfe-commits

https://github.com/Bigcheese approved this pull request.

LGTM. The rest of clang reasons about this at the top level module, so so 
should this.

https://github.com/llvm/llvm-project/pull/69651
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][MBD] set up module build daemon infrastructure (PR #67562)

2023-10-25 Thread Michael Spencer via cfe-commits

Bigcheese wrote:

> > Implementing a timeout as part of this patch as a safety measure seems 
> > worth while too but I'm not sure I understand your final solution. You all 
> > spawned a clang job that had to communicate with the daemon to make sure 
> > one did not exist?
> 
> It is something like this: 
> https://github.com/apple/llvm-project/blob/next/clang/test/CAS/fdepscan-daemon.c
> 
> The "daemon" binary can be launched as a normal process and it takes `--` 
> option, which has the arguments of the clang process it needs to run that 
> talk to itself. The lifetime of the "daemon" is now bounded to that normal 
> process and will not be left behind after testing is done.

I agree with doing this for all the non daemon launch tests. For those you can 
just run clang in a way that lit doesn't care if it fails, kill the daemon, 
then check the output of clang.

It doesn't really need to do a fully clean shutdown as no other process should 
be connecting, it can just call `shutdownDaemon`.

https://github.com/llvm/llvm-project/pull/67562
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][MBD] set up module build daemon infrastructure (PR #67562)

2023-10-25 Thread Michael Spencer via cfe-commits

Bigcheese wrote:

> I had a couple of questions. I guess my main concern here is that the TODO 
> for portability might turn out to be quite heavy lifting. Perhaps we should 
> be looking to abstract the socket-like interface at a lower level.

We have someone working on a simple socket library for LLVM support that can 
handle the portability issues, plan is to land that first.

https://github.com/llvm/llvm-project/pull/67562
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][MBD] set up module build daemon infrastructure (PR #67562)

2023-10-25 Thread Michael Spencer via cfe-commits


@@ -15,21 +12,12 @@ int main() {return 0;}
 // RUN: %clang -fmodule-build-daemon=mbd-handshake -Rmodule-build-daemon 
%t/main.c &> %t/output-existing
 // RUN: cat %t/output-existing | FileCheck %s --check-prefix=CHECK-EXIST
 
-// CHECK: remark: Trying to spawn module build daemon [-Rmodule-build-daemon]
+// COM: Check that clang invocation can spawn and handshake with module build 
daemon

Bigcheese wrote:

No need for `COM:`.

https://github.com/llvm/llvm-project/pull/67562
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] 22b65a3 - [NFC][Clang][ASTTests] Use ASSERT instead of EXPECT for nullptr checks

2023-01-04 Thread Michael Spencer via cfe-commits

Author: Michael Spencer
Date: 2023-01-04T17:33:12-08:00
New Revision: 22b65a329ac2621279ae211dd6a868198e68d149

URL: 
https://github.com/llvm/llvm-project/commit/22b65a329ac2621279ae211dd6a868198e68d149
DIFF: 
https://github.com/llvm/llvm-project/commit/22b65a329ac2621279ae211dd6a868198e68d149.diff

LOG: [NFC][Clang][ASTTests] Use ASSERT instead of EXPECT for nullptr checks

This avoids basically guaranteed crashes when the check fails.

Added: 


Modified: 
clang/unittests/AST/DeclTest.cpp

Removed: 




diff  --git a/clang/unittests/AST/DeclTest.cpp 
b/clang/unittests/AST/DeclTest.cpp
index 6d525dc8ac3fe..940ff17f8c80f 100644
--- a/clang/unittests/AST/DeclTest.cpp
+++ b/clang/unittests/AST/DeclTest.cpp
@@ -404,7 +404,7 @@ TEST(Decl, ImplicitlyDeclaredAllocationFunctionsInModules) {
  hasParameter(0, hasType(isUnsignedInteger(
 .bind("operator new"),
 Ctx));
-  EXPECT_TRUE(SizedOperatorNew->getOwningModule());
+  ASSERT_TRUE(SizedOperatorNew->getOwningModule());
   EXPECT_TRUE(SizedOperatorNew->getOwningModule()->isGlobalModule());
 
   // void* operator new(std::size_t, std::align_val_t);
@@ -416,7 +416,7 @@ TEST(Decl, ImplicitlyDeclaredAllocationFunctionsInModules) {
 hasParameter(1, 
hasType(enumDecl(hasName("std::align_val_t")
 .bind("operator new"),
 Ctx));
-  EXPECT_TRUE(SizedAlignedOperatorNew->getOwningModule());
+  ASSERT_TRUE(SizedAlignedOperatorNew->getOwningModule());
   EXPECT_TRUE(SizedAlignedOperatorNew->getOwningModule()->isGlobalModule());
 
   // void* operator new[](std::size_t);
@@ -426,7 +426,7 @@ TEST(Decl, ImplicitlyDeclaredAllocationFunctionsInModules) {
  hasParameter(0, hasType(isUnsignedInteger(
 .bind("operator new[]"),
 Ctx));
-  EXPECT_TRUE(SizedArrayOperatorNew->getOwningModule());
+  ASSERT_TRUE(SizedArrayOperatorNew->getOwningModule());
   EXPECT_TRUE(SizedArrayOperatorNew->getOwningModule()->isGlobalModule());
 
   // void* operator new[](std::size_t, std::align_val_t);
@@ -438,7 +438,7 @@ TEST(Decl, ImplicitlyDeclaredAllocationFunctionsInModules) {
 hasParameter(1, 
hasType(enumDecl(hasName("std::align_val_t")
 .bind("operator new[]"),
 Ctx));
-  EXPECT_TRUE(SizedAlignedArrayOperatorNew->getOwningModule());
+  ASSERT_TRUE(SizedAlignedArrayOperatorNew->getOwningModule());
   EXPECT_TRUE(
   SizedAlignedArrayOperatorNew->getOwningModule()->isGlobalModule());
 
@@ -450,7 +450,7 @@ TEST(Decl, ImplicitlyDeclaredAllocationFunctionsInModules) {
 hasParameter(0, hasType(pointerType(pointee(voidType())
 .bind("operator delete"),
 Ctx));
-  EXPECT_TRUE(Delete->getOwningModule());
+  ASSERT_TRUE(Delete->getOwningModule());
   EXPECT_TRUE(Delete->getOwningModule()->isGlobalModule());
 
   // void operator delete(void*, std::align_val_t) noexcept;
@@ -462,7 +462,7 @@ TEST(Decl, ImplicitlyDeclaredAllocationFunctionsInModules) {
 hasParameter(1, 
hasType(enumDecl(hasName("std::align_val_t")
 .bind("operator delete"),
 Ctx));
-  EXPECT_TRUE(AlignedDelete->getOwningModule());
+  ASSERT_TRUE(AlignedDelete->getOwningModule());
   EXPECT_TRUE(AlignedDelete->getOwningModule()->isGlobalModule());
 
   // Sized deallocation is not enabled by default. So we skip it here.
@@ -475,7 +475,7 @@ TEST(Decl, ImplicitlyDeclaredAllocationFunctionsInModules) {
 hasParameter(0, hasType(pointerType(pointee(voidType())
 .bind("operator delete[]"),
 Ctx));
-  EXPECT_TRUE(ArrayDelete->getOwningModule());
+  ASSERT_TRUE(ArrayDelete->getOwningModule());
   EXPECT_TRUE(ArrayDelete->getOwningModule()->isGlobalModule());
 
   // void operator delete[](void*, std::align_val_t) noexcept;
@@ -487,6 +487,6 @@ TEST(Decl, ImplicitlyDeclaredAllocationFunctionsInModules) {
 hasParameter(1, 
hasType(enumDecl(hasName("std::align_val_t")
 .bind("operator delete[]"),
 Ctx));
-  EXPECT_TRUE(AlignedArrayDelete->getOwningModule());
+  ASSERT_TRUE(AlignedArrayDelete->getOwningModule());
   EXPECT_TRUE(AlignedArrayDelete->getOwningModule()->isGlobalModule());
 }



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] ffa2a64 - [Clang][DependencyScanner] Remove secondary actions from -cc1

2023-02-01 Thread Michael Spencer via cfe-commits

Author: Michael Spencer
Date: 2023-02-01T15:17:32-08:00
New Revision: ffa2a647c6a9dd2fab13a5e1bd4832b5b200d908

URL: 
https://github.com/llvm/llvm-project/commit/ffa2a647c6a9dd2fab13a5e1bd4832b5b200d908
DIFF: 
https://github.com/llvm/llvm-project/commit/ffa2a647c6a9dd2fab13a5e1bd4832b5b200d908.diff

LOG: [Clang][DependencyScanner] Remove secondary actions from -cc1

The -arcmt-action= and -objcmt-migrate* actions were being passed to
module builds. This caused these builds to fail, as they are secondary
actions that suppress emitting modules.

Differential Revision: https://reviews.llvm.org/D143040

Added: 
clang/test/ClangScanDeps/strip-input-args.m

Modified: 
clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp

Removed: 




diff  --git a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp 
b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
index eeeb5c265a7a..1130c2bc21de 100644
--- a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
+++ b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
@@ -111,6 +111,9 @@ 
ModuleDepCollector::makeInvocationForModuleBuildWithoutOutputs(
   CI.getDependencyOutputOpts().Targets.clear();
 
   CI.getFrontendOpts().ProgramAction = frontend::GenerateModule;
+  CI.getFrontendOpts().ARCMTAction = FrontendOptions::ARCMT_None;
+  CI.getFrontendOpts().ObjCMTAction = FrontendOptions::ObjCMT_None;
+  CI.getFrontendOpts().MTMigrateDir.clear();
   CI.getLangOpts()->ModuleName = Deps.ID.ModuleName;
   CI.getFrontendOpts().IsSystemModule = Deps.IsSystem;
 

diff  --git a/clang/test/ClangScanDeps/strip-input-args.m 
b/clang/test/ClangScanDeps/strip-input-args.m
new file mode 100644
index ..e1954ecc2891
--- /dev/null
+++ b/clang/test/ClangScanDeps/strip-input-args.m
@@ -0,0 +1,52 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: sed -e "s|DIR|%/t|g" %t/cdb1.json.template > %t/cdb1.json
+
+// RUN: clang-scan-deps -compilation-database %t/cdb1.json -format 
experimental-full -mode preprocess-dependency-directives > %t/result1.txt
+
+// RUN: FileCheck %s -input-file %t/result1.txt
+
+// Verify that secondary actions get stripped, and that there's a single 
version
+// of module A.
+
+// CHECK:"modules": [
+// CHECK-NEXT: {
+// CHECK:"command-line": [
+// CHECK-NOT:  "-arcmt-action=check"
+// CHECK-NOT:  "-objcmt-migrate-literals"
+// CHECK-NOT:  "-mt-migrate-directory"
+// CHECK:]
+// CHECK:"name": "A"
+// CHECK:  }
+// CHECK-NOT:"name": "A"
+// CHECK:"translation-units"
+
+//--- cdb1.json.template
+[
+  {
+"directory": "DIR",
+"command": "clang -Imodules/A -fmodules 
-fmodules-cache-path=DIR/module-cache -fimplicit-modules -fimplicit-module-maps 
-ccc-arcmt-check -fsyntax-only DIR/t1.m",
+"file": "DIR/t1.m"
+  },
+  {
+"directory": "DIR",
+"command": "clang -Imodules/A -fmodules 
-fmodules-cache-path=DIR/module-cache -fimplicit-modules -fimplicit-module-maps 
-ccc-objcmt-migrate bob -fsyntax-only DIR/t2.m",
+"file": "DIR/t2.m"
+  }
+]
+
+//--- modules/A/module.modulemap
+
+module A {
+  umbrella header "A.h"
+}
+
+//--- modules/A/A.h
+
+typedef int A_t;
+
+//--- t1.m
+@import A;
+
+//--- t2.m
+@import A;



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] 1694175 - [Clang][Modules] Merge availability attributes on imported decls

2022-06-15 Thread Michael Spencer via cfe-commits

Author: Michael Spencer
Date: 2022-06-15T17:46:16-07:00
New Revision: 169417531578501e955f12c79ecb8ff05333ae15

URL: 
https://github.com/llvm/llvm-project/commit/169417531578501e955f12c79ecb8ff05333ae15
DIFF: 
https://github.com/llvm/llvm-project/commit/169417531578501e955f12c79ecb8ff05333ae15.diff

LOG: [Clang][Modules] Merge availability attributes on imported decls

Currently we do not in general merge attributes when importing decls from 
modules. This patch handles availability, but long term we need to properly 
handle all attributes.

I tried to use Sema::mergeDeclAttributes, but it caused test crashes as I don't 
think it expects to be called in this context. We really shouldn't have 
duplicate code for merging attributes long term, but for now this fixes 
availability. There's already a TODO for this in the declaration of 
ASTDeclReader::mergeInheritableAttributes.

Differential Revision: https://reviews.llvm.org/D127182

rdar://85820301

Added: 
clang/test/Modules/decl-attr-merge.mm

Modified: 
clang/lib/Serialization/ASTReaderDecl.cpp

Removed: 




diff  --git a/clang/lib/Serialization/ASTReaderDecl.cpp 
b/clang/lib/Serialization/ASTReaderDecl.cpp
index 86a9c7733069e..ae298557ee49c 100644
--- a/clang/lib/Serialization/ASTReaderDecl.cpp
+++ b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -3296,6 +3296,13 @@ void ASTDeclReader::mergeInheritableAttributes(ASTReader 
&Reader, Decl *D,
 NewAttr->setInherited(true);
 D->addAttr(NewAttr);
   }
+
+  const auto *AA = Previous->getAttr();
+  if (AA && !D->hasAttr()) {
+NewAttr = AA->clone(Context);
+NewAttr->setInherited(true);
+D->addAttr(NewAttr);
+  }
 }
 
 template

diff  --git a/clang/test/Modules/decl-attr-merge.mm 
b/clang/test/Modules/decl-attr-merge.mm
new file mode 100644
index 0..57f3f93df480e
--- /dev/null
+++ b/clang/test/Modules/decl-attr-merge.mm
@@ -0,0 +1,41 @@
+// RUN: rm -rf %t.dir
+// RUN: split-file %s %t.dir
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps \
+// RUN:   -fmodules-cache-path=%t.dir/cache -triple x86_64-apple-macosx10.11.0 
\
+// RUN:   -I%t.dir/headers %t.dir/main.m -emit-llvm -o %t.dir/main.ll
+// RUN: cat %t.dir/main.ll | FileCheck %s
+
+//--- headers/a.h
+
+__attribute__((availability(macos,introduced=10.16)))
+@interface INIntent
+- (instancetype)self;
+@end
+
+//--- headers/b.h
+
+@class INIntent;
+
+//--- headers/module.modulemap
+
+module A {
+  header "a.h"
+}
+
+module B {
+  header "b.h"
+}
+
+//--- main.m
+
+#import 
+#import  // NOTE: Non attributed decl imported after one with attrs.
+
+void F(id);
+
+int main() {
+  if (@available(macOS 11.0, *))
+F([INIntent self]);
+}
+
+// CHECK: @"OBJC_CLASS_$_INIntent" = extern_weak



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [clang] f06abbb - LLVM Driver Multicall tool

2022-06-06 Thread Michael Spencer via cfe-commits
I believe this broke the shared library build. CMake complains with a cycle:

CMake Error: The inter-target dependency graph contains the following
strongly connected component (cycle):


I'm verifying that this occurs with a clean upstream build.

- Michael Spencer


On Sun, Jun 5, 2022 at 9:28 PM Alex Brachet via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

>
> Author: Chris Bieneman
> Date: 2022-06-06T04:27:32Z
> New Revision: f06abbb393800b0d466c88e283c06f75561c432c
>
> URL:
> https://github.com/llvm/llvm-project/commit/f06abbb393800b0d466c88e283c06f75561c432c
> DIFF:
> https://github.com/llvm/llvm-project/commit/f06abbb393800b0d466c88e283c06f75561c432c.diff
>
> LOG: LLVM Driver Multicall tool
>
> This patch adds an llvm-driver multicall tool that can combine multiple
> LLVM-based tools. The build infrastructure is enabled for a tool by
> adding the GENERATE_DRIVER option to the add_llvm_executable CMake
> call, and changing the tool's main function to a canonicalized
> tool_name_main format (i.e. llvm_ar_main, clang_main, etc...).
>
> As currently implemented llvm-driver contains dsymutil, llvm-ar,
> llvm-cxxfilt, llvm-objcopy, and clang (if clang is included in the
> build).
>
> llvm-driver can be enabled from builds by setting
> LLVM_TOOL_LLVM_DRIVER_BUILD=On.
>
> There are several limitations in the current implementation, which can
> be addressed in subsequent patches:
>
> (1) the multicall binary cannot currently properly handle
> multi-dispatch tools. This means symlinking llvm-ranlib to llvm-driver
> will not properly result in llvm-ar's main being called.
> (2) the multicall binary cannot be comprised of tools containing
> conflicting cl::opt options as the global cl::opt option list cannot
> contain duplicates.
>
> These limitations can be addressed in subsequent patches.
>
> Differential revision: https://reviews.llvm.org/D109977
>
> Added:
> llvm/cmake/driver-template.cpp.in
> llvm/test/tools/llvm-driver/help-passthrough.test
> llvm/test/tools/llvm-driver/help.test
> llvm/test/tools/llvm-driver/symlink-call.test
> llvm/tools/llvm-driver/CMakeLists.txt
> llvm/tools/llvm-driver/llvm-driver.cpp
>
> Modified:
> clang/cmake/modules/AddClang.cmake
> clang/tools/driver/CMakeLists.txt
> clang/tools/driver/driver.cpp
> llvm/CMakeLists.txt
> llvm/cmake/modules/AddLLVM.cmake
> llvm/lib/Support/Path.cpp
> llvm/lib/Support/Unix/Path.inc
> llvm/lib/Support/Windows/Path.inc
> llvm/test/CMakeLists.txt
> llvm/test/lit.cfg.py
> llvm/test/lit.site.cfg.py.in
> llvm/tools/CMakeLists.txt
> llvm/tools/dsymutil/CMakeLists.txt
> llvm/tools/dsymutil/dsymutil.cpp
> llvm/tools/llvm-ar/CMakeLists.txt
> llvm/tools/llvm-ar/llvm-ar.cpp
> llvm/tools/llvm-cxxfilt/CMakeLists.txt
> llvm/tools/llvm-cxxfilt/llvm-cxxfilt.cpp
> llvm/tools/llvm-objcopy/CMakeLists.txt
> llvm/tools/llvm-objcopy/llvm-objcopy.cpp
> utils/bazel/llvm-project-overlay/clang/BUILD.bazel
> utils/bazel/llvm-project-overlay/llvm/BUILD.bazel
>
> Removed:
>
>
>
>
> 
> diff  --git a/clang/cmake/modules/AddClang.cmake
> b/clang/cmake/modules/AddClang.cmake
> index 9bbbfc032b7df..299f8ce6e2fb4 100644
> --- a/clang/cmake/modules/AddClang.cmake
> +++ b/clang/cmake/modules/AddClang.cmake
> @@ -184,5 +184,8 @@ function(clang_target_link_libraries target type)
>else()
>  target_link_libraries(${target} ${type} ${ARGN})
>endif()
> +  if (TARGET obj.${target})
> +target_link_libraries(obj.${target} ${ARGN})
> +  endif()
>
>  endfunction()
>
> diff  --git a/clang/tools/driver/CMakeLists.txt
> b/clang/tools/driver/CMakeLists.txt
> index 6b3e159d1b648..d05b71db13f21 100644
> --- a/clang/tools/driver/CMakeLists.txt
> +++ b/clang/tools/driver/CMakeLists.txt
> @@ -31,6 +31,7 @@ add_clang_tool(clang
>DEPENDS
>intrinsics_gen
>${support_plugins}
> +  GENERATE_DRIVER
>)
>
>  clang_target_link_libraries(clang
>
> diff  --git a/clang/tools/driver/driver.cpp b/clang/tools/driver/driver.cpp
> index d361457f8cecd..fa1f09b44f4da 100644
> --- a/clang/tools/driver/driver.cpp
> +++ b/clang/tools/driver/driver.cpp
> @@ -327,7 +327,7 @@ static int ExecuteCC1Tool(SmallVectorImpl *> &ArgV) {
>return 1;
>  }
>
> -int main(int Argc, const char **Argv) {
> +int clang_main(int Argc, char **Argv) {
>noteBottomOfStack();
>llvm::InitLLVM X(Argc, Argv);
>llvm::setBugReportMsg("PLEASE submit a bug report to " BUG_REPORT_URL
>
> diff  --git a/llvm/CMakeLists.txt b/llvm/CMakeLists.txt
> index 1effbde06b80e..fab16edd7d532 100644
> --- a/llvm/CMakeLists.txt
> +++ b/llvm/CMakeLists.txt
> @@ -269,6 +269,8 @@ include(VersionFromVCS)
>  option(LLVM_APPEND_VC_REV
>"Embed the version control system revision in LLVM" ON)
>
> +option(LLVM_TOOL_LLVM_DRIVER_BUILD "Enables building the llvm multicall
> tool" OFF)
> +
>  set(PACKAGE_NAME LLVM)
>  set(PACKAGE_STRING "${PACKA

Re: [clang] f06abbb - LLVM Driver Multicall tool

2022-06-06 Thread Michael Spencer via cfe-commits
Hmm, I'm not able to reproduce this with upstream. Sorry for the noise.

- Michael Spencer


On Mon, Jun 6, 2022 at 2:44 PM Michael Spencer 
wrote:

> I believe this broke the shared library build. CMake complains with a
> cycle:
>
> CMake Error: The inter-target dependency graph contains the following 
> strongly connected component (cycle):
>
>
> I'm verifying that this occurs with a clean upstream build.
>
> - Michael Spencer
>
>
> On Sun, Jun 5, 2022 at 9:28 PM Alex Brachet via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>>
>> Author: Chris Bieneman
>> Date: 2022-06-06T04:27:32Z
>> New Revision: f06abbb393800b0d466c88e283c06f75561c432c
>>
>> URL:
>> https://github.com/llvm/llvm-project/commit/f06abbb393800b0d466c88e283c06f75561c432c
>> DIFF:
>> https://github.com/llvm/llvm-project/commit/f06abbb393800b0d466c88e283c06f75561c432c.diff
>>
>> LOG: LLVM Driver Multicall tool
>>
>> This patch adds an llvm-driver multicall tool that can combine multiple
>> LLVM-based tools. The build infrastructure is enabled for a tool by
>> adding the GENERATE_DRIVER option to the add_llvm_executable CMake
>> call, and changing the tool's main function to a canonicalized
>> tool_name_main format (i.e. llvm_ar_main, clang_main, etc...).
>>
>> As currently implemented llvm-driver contains dsymutil, llvm-ar,
>> llvm-cxxfilt, llvm-objcopy, and clang (if clang is included in the
>> build).
>>
>> llvm-driver can be enabled from builds by setting
>> LLVM_TOOL_LLVM_DRIVER_BUILD=On.
>>
>> There are several limitations in the current implementation, which can
>> be addressed in subsequent patches:
>>
>> (1) the multicall binary cannot currently properly handle
>> multi-dispatch tools. This means symlinking llvm-ranlib to llvm-driver
>> will not properly result in llvm-ar's main being called.
>> (2) the multicall binary cannot be comprised of tools containing
>> conflicting cl::opt options as the global cl::opt option list cannot
>> contain duplicates.
>>
>> These limitations can be addressed in subsequent patches.
>>
>> Differential revision: https://reviews.llvm.org/D109977
>>
>> Added:
>> llvm/cmake/driver-template.cpp.in
>> llvm/test/tools/llvm-driver/help-passthrough.test
>> llvm/test/tools/llvm-driver/help.test
>> llvm/test/tools/llvm-driver/symlink-call.test
>> llvm/tools/llvm-driver/CMakeLists.txt
>> llvm/tools/llvm-driver/llvm-driver.cpp
>>
>> Modified:
>> clang/cmake/modules/AddClang.cmake
>> clang/tools/driver/CMakeLists.txt
>> clang/tools/driver/driver.cpp
>> llvm/CMakeLists.txt
>> llvm/cmake/modules/AddLLVM.cmake
>> llvm/lib/Support/Path.cpp
>> llvm/lib/Support/Unix/Path.inc
>> llvm/lib/Support/Windows/Path.inc
>> llvm/test/CMakeLists.txt
>> llvm/test/lit.cfg.py
>> llvm/test/lit.site.cfg.py.in
>> llvm/tools/CMakeLists.txt
>> llvm/tools/dsymutil/CMakeLists.txt
>> llvm/tools/dsymutil/dsymutil.cpp
>> llvm/tools/llvm-ar/CMakeLists.txt
>> llvm/tools/llvm-ar/llvm-ar.cpp
>> llvm/tools/llvm-cxxfilt/CMakeLists.txt
>> llvm/tools/llvm-cxxfilt/llvm-cxxfilt.cpp
>> llvm/tools/llvm-objcopy/CMakeLists.txt
>> llvm/tools/llvm-objcopy/llvm-objcopy.cpp
>> utils/bazel/llvm-project-overlay/clang/BUILD.bazel
>> utils/bazel/llvm-project-overlay/llvm/BUILD.bazel
>>
>> Removed:
>>
>>
>>
>>
>> 
>> diff  --git a/clang/cmake/modules/AddClang.cmake
>> b/clang/cmake/modules/AddClang.cmake
>> index 9bbbfc032b7df..299f8ce6e2fb4 100644
>> --- a/clang/cmake/modules/AddClang.cmake
>> +++ b/clang/cmake/modules/AddClang.cmake
>> @@ -184,5 +184,8 @@ function(clang_target_link_libraries target type)
>>else()
>>  target_link_libraries(${target} ${type} ${ARGN})
>>endif()
>> +  if (TARGET obj.${target})
>> +target_link_libraries(obj.${target} ${ARGN})
>> +  endif()
>>
>>  endfunction()
>>
>> diff  --git a/clang/tools/driver/CMakeLists.txt
>> b/clang/tools/driver/CMakeLists.txt
>> index 6b3e159d1b648..d05b71db13f21 100644
>> --- a/clang/tools/driver/CMakeLists.txt
>> +++ b/clang/tools/driver/CMakeLists.txt
>> @@ -31,6 +31,7 @@ add_clang_tool(clang
>>DEPENDS
>>intrinsics_gen
>>${support_plugins}
>> +  GENERATE_DRIVER
>>)
>>
>>  clang_target_link_libraries(clang
>>
>> diff  --git a/clang/tools/driver/driver.cpp
>> b/clang/tools/driver/driver.cpp
>> index d361457f8cecd..fa1f09b44f4da 100644
>> --- a/clang/tools/driver/driver.cpp
>> +++ b/clang/tools/driver/driver.cpp
>> @@ -327,7 +327,7 @@ static int ExecuteCC1Tool(SmallVectorImpl> *> &ArgV) {
>>return 1;
>>  }
>>
>> -int main(int Argc, const char **Argv) {
>> +int clang_main(int Argc, char **Argv) {
>>noteBottomOfStack();
>>llvm::InitLLVM X(Argc, Argv);
>>llvm::setBugReportMsg("PLEASE submit a bug report to " BUG_REPORT_URL
>>
>> diff  --git a/llvm/CMakeLists.txt b/llvm/CMakeLists.txt
>> index 1effbde06b80e..fab16edd7d532 100644
>> --- a/llvm/CMakeLists.txt
>> +++ b/llvm/

[clang] [clang][MBD] set up module build daemon infrastructure (PR #67562)

2023-09-27 Thread Michael Spencer via cfe-commits

https://github.com/Bigcheese edited 
https://github.com/llvm/llvm-project/pull/67562
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][MBD] set up module build daemon infrastructure (PR #67562)

2023-09-27 Thread Michael Spencer via cfe-commits


@@ -0,0 +1,44 @@
+//===- Protocol.h 
-===//

Bigcheese wrote:

Client.h

https://github.com/llvm/llvm-project/pull/67562
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][MBD] set up module build daemon infrastructure (PR #67562)

2023-09-27 Thread Michael Spencer via cfe-commits


@@ -2836,6 +2836,18 @@ defm declspec : BoolOption<"f", "declspec",
   NegFlag,
   BothFlags<[], [ClangOption, CC1Option],
   " __declspec as a keyword">>, Group;
+
+def fmodule_build_daemon : Flag<["-"], "fmodule-build-daemon">, Group,
+  Flags<[NoXarchOption]>,
+  Visibility<[ClangOption, CC1Option]>,
+  HelpText<"Enables module build daemon functionality">,
+  MarshallingInfoFlag>;
+def fmodule_build_daemon_EQ : Joined<["-"], "fmodule-build-daemon=">, 
Group,
+  Flags<[NoXarchOption]>,
+  Visibility<[ClangOption, CC1Option]>,
+  HelpText<"Enables module build daemon functionality and defines location of 
output files">,

Bigcheese wrote:

I think this is less "output files" and more just the lock and log files.

https://github.com/llvm/llvm-project/pull/67562
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][MBD] set up module build daemon infrastructure (PR #67562)

2023-09-27 Thread Michael Spencer via cfe-commits


@@ -0,0 +1,18 @@
+// Check that clang invocation can spawn and handshake with module build daemon
+
+// REQUIRES: !system-windows
+
+//  RUN: if pgrep -f "cc1modbuildd mbd-handshake"; then pkill -f "cc1modbuildd 
mbd-handshake"; fi
+//  RUN: rm -rf mbd-handshake %t

Bigcheese wrote:

`mbd-handshake` should be `%t/mbd-handshake` in these tests.

https://github.com/llvm/llvm-project/pull/67562
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][MBD] set up module build daemon infrastructure (PR #67562)

2023-09-27 Thread Michael Spencer via cfe-commits


@@ -0,0 +1,167 @@
+//===- Client.cpp 
-===//
+//
+// 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 "clang/Tooling/ModuleBuildDaemon/Client.h"
+#include "clang/Basic/Version.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Tooling/ModuleBuildDaemon/SocketMsgSupport.h"
+#include "clang/Tooling/ModuleBuildDaemon/SocketSupport.h"
+#include "clang/Tooling/ModuleBuildDaemon/Utils.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/ScopeExit.h"
+#include "llvm/ADT/SmallString.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/BLAKE3.h"
+
+// TODO: Make portable
+#if LLVM_ON_UNIX
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+using namespace clang;
+using namespace llvm;
+using namespace cc1modbuildd;
+
+std::string cc1modbuildd::getBasePath() {
+  llvm::BLAKE3 Hash;
+  Hash.update(getClangFullVersion());
+  auto HashResult = Hash.final();
+  uint64_t HashValue =
+  llvm::support::endian::read(
+  HashResult.data());
+  std::string Key = toString(llvm::APInt(64, HashValue), 36, /*Signed*/ false);
+
+  // set paths
+  SmallString<128> BasePath;
+  llvm::sys::path::system_temp_directory(/*erasedOnReboot*/ true, BasePath);
+  llvm::sys::path::append(BasePath, "clang-" + Key);
+  return BasePath.c_str();
+}
+
+llvm::Error cc1modbuildd::attemptHandshake(int SocketFD) {
+
+  HandshakeMsg Request{ActionType::HANDSHAKE, StatusType::REQUEST};
+  std::string Buffer = getBufferFromSocketMsg(Request);
+
+  if (llvm::Error Err = writeToSocket(Buffer, SocketFD))
+return std::move(Err);
+
+  Expected MaybeServerResponse =
+  readSocketMsgFromSocket(SocketFD);
+  if (!MaybeServerResponse)
+return std::move(MaybeServerResponse.takeError());
+
+  HandshakeMsg ServerResponse = std::move(*MaybeServerResponse);
+
+  assert(ServerResponse.MsgAction == ActionType::HANDSHAKE &&
+ "At this point response ActionType should only ever be HANDSHAKE");
+
+  if (ServerResponse.MsgStatus == StatusType::SUCCESS)
+return llvm::Error::success();
+
+  return llvm::make_error(
+  "Received failed handshake response from module build daemon",
+  inconvertibleErrorCode());
+}
+
+llvm::Error cc1modbuildd::spawnModuleBuildDaemon(StringRef BasePath,
+ const char *Argv0) {
+  std::string BasePathStr = BasePath.str();
+  const char *Args[] = {Argv0, "-cc1modbuildd", BasePathStr.c_str(), nullptr};
+  pid_t pid;
+  int EC = posix_spawn(&pid, Args[0],
+   /*file_actions*/ nullptr,
+   /*spawnattr*/ nullptr, const_cast(Args),
+   /*envp*/ nullptr);
+  if (EC)
+return createStringError(std::error_code(EC, std::generic_category()),
+ "failed to spawn module build daemon");
+
+  return llvm::Error::success();
+}
+
+Expected cc1modbuildd::getModuleBuildDaemon(const char *Argv0,
+ StringRef BasePath) {
+
+  SmallString<128> SocketPath = BasePath;
+  llvm::sys::path::append(SocketPath, SOCKET_FILE_NAME);
+
+  if (llvm::sys::fs::exists(SocketPath)) {
+Expected MaybeFD = connectToSocket(SocketPath);
+if (MaybeFD)
+  return std::move(*MaybeFD);
+consumeError(MaybeFD.takeError());
+  }
+
+  if (llvm::Error Err = cc1modbuildd::spawnModuleBuildDaemon(BasePath, Argv0))
+return std::move(Err);
+
+  const unsigned int MICROSEC_IN_SEC = 100;
+  constexpr unsigned int MAX_TIME = 30 * MICROSEC_IN_SEC;
+  const unsigned short INTERVAL = 100;
+
+  unsigned int CumulativeTime = 0;
+  unsigned int WaitTime = 0;
+
+  while (CumulativeTime <= MAX_TIME) {
+// Wait a bit then check to see if the module build daemon has initialized
+usleep(WaitTime);
+
+if (llvm::sys::fs::exists(SocketPath)) {
+  Expected MaybeFD = connectToSocket(SocketPath);
+  if (MaybeFD)
+return std::move(*MaybeFD);
+  consumeError(MaybeFD.takeError());
+}
+
+CumulativeTime += INTERVAL;

Bigcheese wrote:

I think we want an exponential backoff here rather than constant.

https://github.com/llvm/llvm-project/pull/67562
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][MBD] set up module build daemon infrastructure (PR #67562)

2023-09-27 Thread Michael Spencer via cfe-commits


@@ -0,0 +1,31 @@
+//===-- SocketSupport.h 
---===//
+//
+// 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_TOOLING_MODULEBUILDDAEMON_SOCKETSUPPORT_H
+#define LLVM_CLANG_TOOLING_MODULEBUILDDAEMON_SOCKETSUPPORT_H
+
+#include "clang/Frontend/CompilerInstance.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/YAMLParser.h"
+#include "llvm/Support/YAMLTraits.h"
+
+using namespace clang;
+using namespace llvm;
+
+namespace cc1modbuildd {
+
+Expected createSocket();
+Expected connectToSocket(StringRef SocketPath);
+Expected connectAndWriteToSocket(std::string Buffer, StringRef 
SocketPath);

Bigcheese wrote:

The write functions should take a `StringRef`.

https://github.com/llvm/llvm-project/pull/67562
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][MBD] set up module build daemon infrastructure (PR #67562)

2023-09-27 Thread Michael Spencer via cfe-commits

https://github.com/Bigcheese requested changes to this pull request.


https://github.com/llvm/llvm-project/pull/67562
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][MBD] set up module build daemon infrastructure (PR #67562)

2023-09-27 Thread Michael Spencer via cfe-commits


@@ -0,0 +1,128 @@
+//===- SocketSupport.cpp 
--===//
+//
+// 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 "clang/Tooling/ModuleBuildDaemon/SocketSupport.h"
+#include "clang/Basic/Version.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Tooling/ModuleBuildDaemon/Client.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/ScopeExit.h"
+#include "llvm/ADT/SmallString.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Config/llvm-config.h"
+#include "llvm/Support/BLAKE3.h"
+
+// TODO: Make portable
+#if LLVM_ON_UNIX
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+Expected cc1modbuildd::createSocket() {
+  int FD;
+  if ((FD = socket(AF_UNIX, SOCK_STREAM, 0)) == -1) {
+std::string Msg = "socket create error: " + std::string(strerror(errno));
+return createStringError(inconvertibleErrorCode(), Msg);

Bigcheese wrote:

You should be able to just form an `llvm::Error` from `errno` here.

https://github.com/llvm/llvm-project/pull/67562
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][MBD] set up module build daemon infrastructure (PR #67562)

2023-09-27 Thread Michael Spencer via cfe-commits


@@ -0,0 +1,128 @@
+//===- SocketSupport.cpp 
--===//
+//
+// 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 "clang/Tooling/ModuleBuildDaemon/SocketSupport.h"
+#include "clang/Basic/Version.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Tooling/ModuleBuildDaemon/Client.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/ScopeExit.h"
+#include "llvm/ADT/SmallString.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Config/llvm-config.h"
+#include "llvm/Support/BLAKE3.h"
+
+// TODO: Make portable
+#if LLVM_ON_UNIX
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+Expected cc1modbuildd::createSocket() {
+  int FD;
+  if ((FD = socket(AF_UNIX, SOCK_STREAM, 0)) == -1) {
+std::string Msg = "socket create error: " + std::string(strerror(errno));
+return createStringError(inconvertibleErrorCode(), Msg);
+  }
+  return FD;
+}
+
+Expected cc1modbuildd::connectToSocket(StringRef SocketPath) {
+
+  Expected MaybeFD = cc1modbuildd::createSocket();
+  if (!MaybeFD)
+return std::move(MaybeFD.takeError());
+
+  int FD = std::move(*MaybeFD);
+
+  struct sockaddr_un Addr;
+  memset(&Addr, 0, sizeof(Addr));
+  Addr.sun_family = AF_UNIX;
+  strncpy(Addr.sun_path, SocketPath.str().c_str(), sizeof(Addr.sun_path) - 1);
+
+  if (connect(FD, (struct sockaddr *)&Addr, sizeof(Addr)) == -1) {
+close(FD);
+std::string msg = "socket connect error: " + std::string(strerror(errno));
+return createStringError(inconvertibleErrorCode(), msg);
+  }
+  return FD;
+}
+
+Expected cc1modbuildd::connectAndWriteToSocket(std::string Buffer,
+StringRef SocketPath) {
+
+  Expected MaybeConnectedFD = connectToSocket(SocketPath);
+  if (!MaybeConnectedFD)
+return std::move(MaybeConnectedFD.takeError());
+
+  int ConnectedFD = std::move(*MaybeConnectedFD);
+  llvm::Error Err = writeToSocket(Buffer, ConnectedFD);
+  if (Err)
+return std::move(Err);
+
+  return ConnectedFD;
+}
+
+Expected cc1modbuildd::readFromSocket(int FD) {
+
+  const size_t BUFFER_SIZE = 4096;
+  std::vector Buffer(BUFFER_SIZE);
+  size_t TotalBytesRead = 0;
+
+  ssize_t n;
+  while ((n = read(FD, Buffer.data() + TotalBytesRead,
+   Buffer.size() - TotalBytesRead)) > 0) {
+
+TotalBytesRead += n;
+// Read until ...\n encountered (last line of YAML document)
+if (std::string(&Buffer[TotalBytesRead - 4], 4) == "...\n")
+  break;
+if (Buffer.size() - TotalBytesRead < BUFFER_SIZE)
+  Buffer.resize(Buffer.size() + BUFFER_SIZE);

Bigcheese wrote:

This ends up dropping any other messages if they get read at the same time, and 
may miss `...` in the middle of a buffer. You need to look for the first 
occurrence of `\n...` anywhere in the buffer, and safe the rest of it for the 
next read.

https://github.com/llvm/llvm-project/pull/67562
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][MBD] set up module build daemon infrastructure (PR #67562)

2023-09-27 Thread Michael Spencer via cfe-commits


@@ -0,0 +1,18 @@
+// Check that clang invocation can spawn and handshake with module build daemon

Bigcheese wrote:

This test should also check that clang can connect to an existing MBD without 
spawning a new one.

https://github.com/llvm/llvm-project/pull/67562
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][MBD] set up module build daemon infrastructure (PR #67562)

2023-09-27 Thread Michael Spencer via cfe-commits


@@ -0,0 +1,167 @@
+//===- Client.cpp 
-===//
+//
+// 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 "clang/Tooling/ModuleBuildDaemon/Client.h"
+#include "clang/Basic/Version.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Tooling/ModuleBuildDaemon/SocketMsgSupport.h"
+#include "clang/Tooling/ModuleBuildDaemon/SocketSupport.h"
+#include "clang/Tooling/ModuleBuildDaemon/Utils.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/ScopeExit.h"
+#include "llvm/ADT/SmallString.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/BLAKE3.h"
+
+// TODO: Make portable
+#if LLVM_ON_UNIX
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+using namespace clang;
+using namespace llvm;
+using namespace cc1modbuildd;
+
+std::string cc1modbuildd::getBasePath() {
+  llvm::BLAKE3 Hash;
+  Hash.update(getClangFullVersion());
+  auto HashResult = Hash.final();
+  uint64_t HashValue =
+  llvm::support::endian::read(
+  HashResult.data());
+  std::string Key = toString(llvm::APInt(64, HashValue), 36, /*Signed*/ false);
+
+  // set paths
+  SmallString<128> BasePath;
+  llvm::sys::path::system_temp_directory(/*erasedOnReboot*/ true, BasePath);
+  llvm::sys::path::append(BasePath, "clang-" + Key);
+  return BasePath.c_str();
+}
+
+llvm::Error cc1modbuildd::attemptHandshake(int SocketFD) {
+
+  HandshakeMsg Request{ActionType::HANDSHAKE, StatusType::REQUEST};
+  std::string Buffer = getBufferFromSocketMsg(Request);
+
+  if (llvm::Error Err = writeToSocket(Buffer, SocketFD))
+return std::move(Err);
+
+  Expected MaybeServerResponse =
+  readSocketMsgFromSocket(SocketFD);
+  if (!MaybeServerResponse)
+return std::move(MaybeServerResponse.takeError());
+
+  HandshakeMsg ServerResponse = std::move(*MaybeServerResponse);
+
+  assert(ServerResponse.MsgAction == ActionType::HANDSHAKE &&
+ "At this point response ActionType should only ever be HANDSHAKE");
+
+  if (ServerResponse.MsgStatus == StatusType::SUCCESS)
+return llvm::Error::success();
+
+  return llvm::make_error(
+  "Received failed handshake response from module build daemon",
+  inconvertibleErrorCode());
+}
+
+llvm::Error cc1modbuildd::spawnModuleBuildDaemon(StringRef BasePath,
+ const char *Argv0) {
+  std::string BasePathStr = BasePath.str();
+  const char *Args[] = {Argv0, "-cc1modbuildd", BasePathStr.c_str(), nullptr};
+  pid_t pid;
+  int EC = posix_spawn(&pid, Args[0],
+   /*file_actions*/ nullptr,
+   /*spawnattr*/ nullptr, const_cast(Args),
+   /*envp*/ nullptr);
+  if (EC)
+return createStringError(std::error_code(EC, std::generic_category()),
+ "failed to spawn module build daemon");
+
+  return llvm::Error::success();
+}
+
+Expected cc1modbuildd::getModuleBuildDaemon(const char *Argv0,
+ StringRef BasePath) {
+
+  SmallString<128> SocketPath = BasePath;
+  llvm::sys::path::append(SocketPath, SOCKET_FILE_NAME);
+
+  if (llvm::sys::fs::exists(SocketPath)) {
+Expected MaybeFD = connectToSocket(SocketPath);
+if (MaybeFD)
+  return std::move(*MaybeFD);
+consumeError(MaybeFD.takeError());
+  }
+
+  if (llvm::Error Err = cc1modbuildd::spawnModuleBuildDaemon(BasePath, Argv0))
+return std::move(Err);
+
+  const unsigned int MICROSEC_IN_SEC = 100;
+  constexpr unsigned int MAX_TIME = 30 * MICROSEC_IN_SEC;
+  const unsigned short INTERVAL = 100;
+
+  unsigned int CumulativeTime = 0;
+  unsigned int WaitTime = 0;
+
+  while (CumulativeTime <= MAX_TIME) {
+// Wait a bit then check to see if the module build daemon has initialized
+usleep(WaitTime);

Bigcheese wrote:

This ends up sleeping for 0 every time.

https://github.com/llvm/llvm-project/pull/67562
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][MBD] set up module build daemon infrastructure (PR #67562)

2023-09-27 Thread Michael Spencer via cfe-commits


@@ -0,0 +1,18 @@
+// Check that clang invocation can spawn and handshake with module build daemon
+
+// REQUIRES: !system-windows
+
+//  RUN: if pgrep -f "cc1modbuildd mbd-handshake"; then pkill -f "cc1modbuildd 
mbd-handshake"; fi
+//  RUN: rm -rf mbd-handshake %t
+//  RUN: split-file %s %t
+
+//--- main.c
+int main() {return 0;}
+
+// RUN: %clang -fmodule-build-daemon=mbd-handshake %t/main.c > output
+// RUN: cat output | FileCheck %s
+
+// CHECK: Completed successfull handshake with module build daemon
+
+// RUN: if pgrep -f "cc1modbuildd mbd-handshake"; then pkill -f "cc1modbuildd 
mbd-handshake"; fi
+// RUN: rm -rf mbd-handshake %t

Bigcheese wrote:

Don't delete the temp dir at the end.

https://github.com/llvm/llvm-project/pull/67562
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][MBD] set up module build daemon infrastructure (PR #67562)

2023-09-27 Thread Michael Spencer via cfe-commits


@@ -0,0 +1,267 @@
+//===--- cc1modbuildd_main.cpp - Clang CC1 Module Build Daemon 
===//
+//
+// 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 "clang/Tooling/ModuleBuildDaemon/SocketMsgSupport.h"
+#include "clang/Tooling/ModuleBuildDaemon/SocketSupport.h"
+#include "clang/Tooling/ModuleBuildDaemon/Utils.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/SmallString.h"
+#include "llvm/Support/Program.h"
+#include "llvm/Support/ThreadPool.h"
+#include "llvm/Support/Threading.h"
+#include "llvm/Support/YAMLParser.h"
+#include "llvm/Support/YAMLTraits.h"
+
+// TODO: Make portable
+#if LLVM_ON_UNIX
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+using namespace llvm;
+using namespace clang;
+using namespace cc1modbuildd;
+
+// Create unbuffered STDOUT stream so that any logging done by module build
+// daemon can be viewed without having to terminate the process
+static raw_fd_ostream &unbuff_outs() {
+  static raw_fd_ostream S(STDOUT_FILENO, false, true);
+  return S;
+}
+
+namespace {
+
+class ModuleBuildDaemonServer {
+public:
+  SmallString<128> BasePath;
+  SmallString<128> SocketPath;
+  SmallString<128> PidPath;
+
+  ModuleBuildDaemonServer(SmallString<128> Path, ArrayRef Argv)
+  : BasePath(Path), SocketPath(Path) {
+llvm::sys::path::append(SocketPath, SOCKET_FILE_NAME);
+  }
+
+  ~ModuleBuildDaemonServer() { shutdownDaemon(SIGTERM); }
+
+  int forkDaemon();
+  int launchDaemon();
+  int listenForClients();
+
+  static void handleClient(int Client);
+
+  void shutdownDaemon(int signal) {
+unlink(SocketPath.c_str());
+shutdown(ListenSocketFD, SHUT_RD);
+close(ListenSocketFD);
+exit(EXIT_SUCCESS);
+  }
+
+private:
+  // Initializes and returns DiagnosticsEngine
+  pid_t Pid = -1;
+  int ListenSocketFD = -1;
+};
+
+// Required to handle SIGTERM by calling Shutdown
+ModuleBuildDaemonServer *DaemonPtr = nullptr;
+void handleSignal(int Signal) {
+  if (DaemonPtr != nullptr) {
+DaemonPtr->shutdownDaemon(Signal);
+  }
+}
+} // namespace
+
+static bool verbose = false;
+static void verbose_print(const llvm::Twine &message) {
+  if (verbose) {
+unbuff_outs() << message << '\n';
+  }
+}
+
+// Forks and detaches process, creating module build daemon
+int ModuleBuildDaemonServer::forkDaemon() {
+
+  pid_t pid = fork();
+
+  if (pid < 0) {
+exit(EXIT_FAILURE);
+  }
+  if (pid > 0) {
+exit(EXIT_SUCCESS);
+  }
+
+  Pid = getpid();
+
+  close(STDIN_FILENO);
+  close(STDOUT_FILENO);
+  close(STDERR_FILENO);
+
+  SmallString<128> STDOUT = BasePath;
+  llvm::sys::path::append(STDOUT, STDOUT_FILE_NAME);
+  freopen(STDOUT.c_str(), "a", stdout);
+
+  SmallString<128> STDERR = BasePath;
+  llvm::sys::path::append(STDERR, STDERR_FILE_NAME);
+  freopen(STDERR.c_str(), "a", stderr);
+
+  if (signal(SIGTERM, handleSignal) == SIG_ERR) {
+errs() << "failed to handle SIGTERM" << '\n';
+exit(EXIT_FAILURE);
+  }
+  if (signal(SIGHUP, SIG_IGN) == SIG_ERR) {
+errs() << "failed to ignore SIGHUP" << '\n';
+exit(EXIT_FAILURE);
+  }
+  if (setsid() == -1) {
+errs() << "setsid failed" << '\n';
+exit(EXIT_FAILURE);
+  }
+
+  return EXIT_SUCCESS;
+}
+
+// Creates unix socket for IPC with module build daemon
+int ModuleBuildDaemonServer::launchDaemon() {
+
+  // new socket
+  if ((ListenSocketFD = socket(AF_UNIX, SOCK_STREAM, 0)) == -1) {
+std::perror("Socket create error: ");
+exit(EXIT_FAILURE);
+  }
+
+  struct sockaddr_un addr;
+  memset(&addr, 0, sizeof(struct sockaddr_un));
+  addr.sun_family = AF_UNIX;
+  strncpy(addr.sun_path, SocketPath.c_str(), sizeof(addr.sun_path) - 1);
+
+  // bind to local address
+  if (bind(ListenSocketFD, (struct sockaddr *)&addr, sizeof(addr)) == -1) {
+
+// If the socket address is already in use, exit because another module
+// build daemon has successfully launched. When translation units are
+// compiled in parallel, until the socket file is created, all clang
+// invocations will spawn a module build daemon.
+if (errno == EADDRINUSE) {
+  close(ListenSocketFD);
+  exit(EXIT_SUCCESS);
+}
+std::perror("Socket bind error: ");
+exit(EXIT_FAILURE);
+  }
+  verbose_print("mbd created and binded to socket address at: " + SocketPath);
+
+  // set socket to accept incoming connection request
+  unsigned MaxBacklog = llvm::hardware_concurrency().compute_thread_count();
+  if (listen(ListenSocketFD, MaxBacklog) == -1) {
+std::perror("Socket listen error: ");
+exit(EXIT_FAILURE);
+  }
+
+  return 0;
+}
+
+// Function submitted to thread pool with each client connection. Not
+// responsible for closing client c

[clang] [clang][MBD] set up module build daemon infrastructure (PR #67562)

2023-09-27 Thread Michael Spencer via cfe-commits


@@ -0,0 +1,267 @@
+//===--- cc1modbuildd_main.cpp - Clang CC1 Module Build Daemon 
===//
+//
+// 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 "clang/Tooling/ModuleBuildDaemon/SocketMsgSupport.h"
+#include "clang/Tooling/ModuleBuildDaemon/SocketSupport.h"
+#include "clang/Tooling/ModuleBuildDaemon/Utils.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/SmallString.h"
+#include "llvm/Support/Program.h"
+#include "llvm/Support/ThreadPool.h"
+#include "llvm/Support/Threading.h"
+#include "llvm/Support/YAMLParser.h"
+#include "llvm/Support/YAMLTraits.h"
+
+// TODO: Make portable
+#if LLVM_ON_UNIX
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+using namespace llvm;
+using namespace clang;
+using namespace cc1modbuildd;
+
+// Create unbuffered STDOUT stream so that any logging done by module build
+// daemon can be viewed without having to terminate the process
+static raw_fd_ostream &unbuff_outs() {
+  static raw_fd_ostream S(STDOUT_FILENO, false, true);
+  return S;
+}
+
+namespace {
+
+class ModuleBuildDaemonServer {
+public:
+  SmallString<128> BasePath;
+  SmallString<128> SocketPath;
+  SmallString<128> PidPath;
+
+  ModuleBuildDaemonServer(SmallString<128> Path, ArrayRef Argv)
+  : BasePath(Path), SocketPath(Path) {
+llvm::sys::path::append(SocketPath, SOCKET_FILE_NAME);
+  }
+
+  ~ModuleBuildDaemonServer() { shutdownDaemon(SIGTERM); }
+
+  int forkDaemon();
+  int launchDaemon();
+  int listenForClients();
+
+  static void handleClient(int Client);
+
+  void shutdownDaemon(int signal) {
+unlink(SocketPath.c_str());
+shutdown(ListenSocketFD, SHUT_RD);
+close(ListenSocketFD);
+exit(EXIT_SUCCESS);
+  }
+
+private:
+  // Initializes and returns DiagnosticsEngine
+  pid_t Pid = -1;
+  int ListenSocketFD = -1;
+};
+
+// Required to handle SIGTERM by calling Shutdown
+ModuleBuildDaemonServer *DaemonPtr = nullptr;
+void handleSignal(int Signal) {
+  if (DaemonPtr != nullptr) {
+DaemonPtr->shutdownDaemon(Signal);

Bigcheese wrote:

This may race with other threads, not sure.

https://github.com/llvm/llvm-project/pull/67562
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][MBD] set up module build daemon infrastructure (PR #67562)

2023-09-27 Thread Michael Spencer via cfe-commits


@@ -0,0 +1,267 @@
+//===--- cc1modbuildd_main.cpp - Clang CC1 Module Build Daemon 
===//
+//
+// 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 "clang/Tooling/ModuleBuildDaemon/SocketMsgSupport.h"
+#include "clang/Tooling/ModuleBuildDaemon/SocketSupport.h"
+#include "clang/Tooling/ModuleBuildDaemon/Utils.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/SmallString.h"
+#include "llvm/Support/Program.h"
+#include "llvm/Support/ThreadPool.h"
+#include "llvm/Support/Threading.h"
+#include "llvm/Support/YAMLParser.h"
+#include "llvm/Support/YAMLTraits.h"
+
+// TODO: Make portable
+#if LLVM_ON_UNIX
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+using namespace llvm;
+using namespace clang;
+using namespace cc1modbuildd;
+
+// Create unbuffered STDOUT stream so that any logging done by module build
+// daemon can be viewed without having to terminate the process
+static raw_fd_ostream &unbuff_outs() {
+  static raw_fd_ostream S(STDOUT_FILENO, false, true);
+  return S;
+}
+
+namespace {
+
+class ModuleBuildDaemonServer {
+public:
+  SmallString<128> BasePath;
+  SmallString<128> SocketPath;
+  SmallString<128> PidPath;
+
+  ModuleBuildDaemonServer(SmallString<128> Path, ArrayRef Argv)
+  : BasePath(Path), SocketPath(Path) {
+llvm::sys::path::append(SocketPath, SOCKET_FILE_NAME);
+  }
+
+  ~ModuleBuildDaemonServer() { shutdownDaemon(SIGTERM); }
+
+  int forkDaemon();
+  int launchDaemon();
+  int listenForClients();
+
+  static void handleClient(int Client);
+
+  void shutdownDaemon(int signal) {
+unlink(SocketPath.c_str());
+shutdown(ListenSocketFD, SHUT_RD);
+close(ListenSocketFD);
+exit(EXIT_SUCCESS);
+  }
+
+private:
+  // Initializes and returns DiagnosticsEngine
+  pid_t Pid = -1;
+  int ListenSocketFD = -1;
+};
+
+// Required to handle SIGTERM by calling Shutdown
+ModuleBuildDaemonServer *DaemonPtr = nullptr;
+void handleSignal(int Signal) {
+  if (DaemonPtr != nullptr) {
+DaemonPtr->shutdownDaemon(Signal);
+  }
+}
+} // namespace
+
+static bool verbose = false;
+static void verbose_print(const llvm::Twine &message) {
+  if (verbose) {
+unbuff_outs() << message << '\n';
+  }
+}
+
+// Forks and detaches process, creating module build daemon
+int ModuleBuildDaemonServer::forkDaemon() {
+
+  pid_t pid = fork();
+
+  if (pid < 0) {
+exit(EXIT_FAILURE);
+  }
+  if (pid > 0) {
+exit(EXIT_SUCCESS);
+  }
+
+  Pid = getpid();
+
+  close(STDIN_FILENO);
+  close(STDOUT_FILENO);
+  close(STDERR_FILENO);
+
+  SmallString<128> STDOUT = BasePath;
+  llvm::sys::path::append(STDOUT, STDOUT_FILE_NAME);
+  freopen(STDOUT.c_str(), "a", stdout);
+
+  SmallString<128> STDERR = BasePath;
+  llvm::sys::path::append(STDERR, STDERR_FILE_NAME);
+  freopen(STDERR.c_str(), "a", stderr);
+
+  if (signal(SIGTERM, handleSignal) == SIG_ERR) {
+errs() << "failed to handle SIGTERM" << '\n';
+exit(EXIT_FAILURE);
+  }
+  if (signal(SIGHUP, SIG_IGN) == SIG_ERR) {
+errs() << "failed to ignore SIGHUP" << '\n';
+exit(EXIT_FAILURE);
+  }
+  if (setsid() == -1) {
+errs() << "setsid failed" << '\n';
+exit(EXIT_FAILURE);
+  }
+
+  return EXIT_SUCCESS;
+}
+
+// Creates unix socket for IPC with module build daemon
+int ModuleBuildDaemonServer::launchDaemon() {
+
+  // new socket
+  if ((ListenSocketFD = socket(AF_UNIX, SOCK_STREAM, 0)) == -1) {
+std::perror("Socket create error: ");
+exit(EXIT_FAILURE);
+  }
+
+  struct sockaddr_un addr;
+  memset(&addr, 0, sizeof(struct sockaddr_un));
+  addr.sun_family = AF_UNIX;
+  strncpy(addr.sun_path, SocketPath.c_str(), sizeof(addr.sun_path) - 1);
+
+  // bind to local address
+  if (bind(ListenSocketFD, (struct sockaddr *)&addr, sizeof(addr)) == -1) {
+
+// If the socket address is already in use, exit because another module
+// build daemon has successfully launched. When translation units are
+// compiled in parallel, until the socket file is created, all clang
+// invocations will spawn a module build daemon.
+if (errno == EADDRINUSE) {
+  close(ListenSocketFD);
+  exit(EXIT_SUCCESS);
+}
+std::perror("Socket bind error: ");
+exit(EXIT_FAILURE);
+  }
+  verbose_print("mbd created and binded to socket address at: " + SocketPath);
+
+  // set socket to accept incoming connection request
+  unsigned MaxBacklog = llvm::hardware_concurrency().compute_thread_count();
+  if (listen(ListenSocketFD, MaxBacklog) == -1) {
+std::perror("Socket listen error: ");
+exit(EXIT_FAILURE);
+  }
+
+  return 0;
+}
+
+// Function submitted to thread pool with each client connection. Not
+// responsible for closing client c

[clang] [clang][MBD] set up module build daemon infrastructure (PR #67562)

2023-09-27 Thread Michael Spencer via cfe-commits


@@ -0,0 +1,267 @@
+//===--- cc1modbuildd_main.cpp - Clang CC1 Module Build Daemon 
===//
+//
+// 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 "clang/Tooling/ModuleBuildDaemon/SocketMsgSupport.h"
+#include "clang/Tooling/ModuleBuildDaemon/SocketSupport.h"
+#include "clang/Tooling/ModuleBuildDaemon/Utils.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/SmallString.h"
+#include "llvm/Support/Program.h"
+#include "llvm/Support/ThreadPool.h"
+#include "llvm/Support/Threading.h"
+#include "llvm/Support/YAMLParser.h"
+#include "llvm/Support/YAMLTraits.h"
+
+// TODO: Make portable
+#if LLVM_ON_UNIX
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+using namespace llvm;
+using namespace clang;
+using namespace cc1modbuildd;
+
+// Create unbuffered STDOUT stream so that any logging done by module build
+// daemon can be viewed without having to terminate the process
+static raw_fd_ostream &unbuff_outs() {
+  static raw_fd_ostream S(STDOUT_FILENO, false, true);
+  return S;
+}
+
+namespace {
+
+class ModuleBuildDaemonServer {
+public:
+  SmallString<128> BasePath;
+  SmallString<128> SocketPath;
+  SmallString<128> PidPath;
+
+  ModuleBuildDaemonServer(SmallString<128> Path, ArrayRef Argv)
+  : BasePath(Path), SocketPath(Path) {
+llvm::sys::path::append(SocketPath, SOCKET_FILE_NAME);
+  }
+
+  ~ModuleBuildDaemonServer() { shutdownDaemon(SIGTERM); }
+
+  int forkDaemon();
+  int launchDaemon();
+  int listenForClients();
+
+  static void handleClient(int Client);
+
+  void shutdownDaemon(int signal) {
+unlink(SocketPath.c_str());
+shutdown(ListenSocketFD, SHUT_RD);
+close(ListenSocketFD);
+exit(EXIT_SUCCESS);
+  }
+
+private:
+  // Initializes and returns DiagnosticsEngine
+  pid_t Pid = -1;
+  int ListenSocketFD = -1;
+};
+
+// Required to handle SIGTERM by calling Shutdown
+ModuleBuildDaemonServer *DaemonPtr = nullptr;
+void handleSignal(int Signal) {
+  if (DaemonPtr != nullptr) {
+DaemonPtr->shutdownDaemon(Signal);

Bigcheese wrote:

I did some more checking on this. `DaemonPtr` is fine as the signal handler is 
installed after this is set. As for `shutdownDaemon`, all the functions it 
currently calls are signal safe according to POSIX, but `ListenSocketFD` also 
needs to be atomic as it is written after the signal handler is installed.

Basically you need to treat the signal handler as running in another thread 
that starts and synchronizes with when the signal handler gets installed.

https://github.com/llvm/llvm-project/pull/67562
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


  1   2   3   4   >