Re: [PATCH] D11403: [Modules] Add Darwin-specific compatibility module map parsing hacks
benlangmuir updated this revision to Diff 31354.
benlangmuir added a comment.
Sorry for the slow response, I somehow missed your original comment.
Add a more cohesive comment. I don't think we can usefully specify the SDK
range it affects until a fixed SDK is available (currently it's every SDK that
has module maps at all). I'd love to update the comment then. Ideally at that
point we could also check the SDK version before applying the hack, but I'm not
sure it's worth reading the SDK version plist file from disk just for that. We
already read this file when creating the module cache hash, but I'd like to
remove that at some point.
Repository:
rL LLVM
http://reviews.llvm.org/D11403
Files:
include/clang/Basic/Module.h
lib/Basic/Module.cpp
lib/Lex/ModuleMap.cpp
test/Modules/Inputs/System/usr/include/assert.h
test/Modules/Inputs/System/usr/include/module.map
test/Modules/Inputs/System/usr/include/tcl-private/header.h
test/Modules/darwin_specific_modulemap_hacks.m
Index: test/Modules/darwin_specific_modulemap_hacks.m
===
--- /dev/null
+++ test/Modules/darwin_specific_modulemap_hacks.m
@@ -0,0 +1,22 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t -fimplicit-module-maps -isystem %S/Inputs/System/usr/include -triple x86_64-apple-darwin10 %s -verify -fsyntax-only
+// expected-no-diagnostics
+
+@import Darwin.C.excluded; // no error, header is implicitly 'textual'
+@import Tcl.Private; // no error, header is implicitly 'textual'
+@import IOKit.avc; // no error, cplusplus requirement removed
+
+#if defined(DARWIN_C_EXCLUDED)
+#error assert.h should be textual
+#elif defined(TCL_PRIVATE)
+#error tcl-private/header.h should be textual
+#endif
+
+#import
+#import
+
+#if !defined(DARWIN_C_EXCLUDED)
+#error assert.h missing
+#elif !defined(TCL_PRIVATE)
+#error tcl-private/header.h missing
+#endif
Index: test/Modules/Inputs/System/usr/include/tcl-private/header.h
===
--- /dev/null
+++ test/Modules/Inputs/System/usr/include/tcl-private/header.h
@@ -0,0 +1,2 @@
+// tcl-private/header.h
+#define TCL_PRIVATE 1
Index: test/Modules/Inputs/System/usr/include/module.map
===
--- test/Modules/Inputs/System/usr/include/module.map
+++ test/Modules/Inputs/System/usr/include/module.map
@@ -30,3 +30,25 @@
header "uses_other_constants.h"
export *
}
+
+module Darwin {
+ module C {
+module excluded {
+ requires excluded
+ header "assert.h"
+}
+ }
+}
+
+module Tcl {
+ module Private {
+requires excluded
+umbrella ""
+ }
+}
+
+module IOKit {
+ module avc {
+requires cplusplus
+ }
+}
Index: test/Modules/Inputs/System/usr/include/assert.h
===
--- /dev/null
+++ test/Modules/Inputs/System/usr/include/assert.h
@@ -0,0 +1,2 @@
+// assert.h
+#define DARWIN_C_EXCLUDED 1
Index: lib/Lex/ModuleMap.cpp
===
--- lib/Lex/ModuleMap.cpp
+++ lib/Lex/ModuleMap.cpp
@@ -1570,6 +1570,41 @@
: File->getDir(), ExternLoc);
}
+/// Whether to add the requirement \p Feature to the module \p M.
+///
+/// This preserves backwards compatibility for two hacks in the Darwin system
+/// module map files:
+///
+/// 1. The use of 'requires excluded' to make headers non-modular, which
+///should really be mapped to 'textual' now that we have this feature. We
+///drop the 'excluded' requirement, and set \p IsRequiresExcludedHack to
+///true. Later, this bit will be used to map all the headers inside this
+///module to 'textual'.
+///
+///This affects Darwin.C.excluded (for assert.h) and Tcl.Private.
+///
+/// 2. Removes a bogus cplusplus requirement from IOKit.avc. This requirement
+///was never correct and causes issues now that we check it, so drop it.
+static bool shouldAddRequirement(Module *M, StringRef Feature,
+ bool &IsRequiresExcludedHack) {
+ if (Feature == "excluded" || Feature == "cplusplus") {
+std::string FullName = M->getFullModuleName();
+if (FullName == "Darwin.C.excluded" || FullName == "Tcl.Private") {
+ // We will mark the module contents non-modular. See doc comment for
+ // Module::UsesRequiresExcludedHack.
+ IsRequiresExcludedHack = true;
+ return false;
+} else if (FullName == "IOKit.avc") {
+ // This module was mistakenly marked 'requires cplusplus' in older Darwin
+ // SDK versions. As a backwards compatibility hack, don't add the
+ // requirement.
+ return false;
+}
+ }
+
+ return true;
+}
+
/// \brief Parse a requires declaration.
///
/// requires-declaration:
@@ -1605,9 +1640,16 @@
std::string Feature = Tok.getString();
consumeToken();
-// Add this feature.
-Acti
Re: [PATCH] D11403: [Modules] Add Darwin-specific compatibility module map parsing hacks
benlangmuir added inline comments.
Comment at: include/clang/Basic/Module.h:203-211
@@ -202,1 +202,11 @@
+ /// \brief Whether this module uses the 'requires excluded' hack to mark its
+ /// contents as 'textual'.
+ ///
+ /// On older Darwin SDK versions, 'requires excluded' is used to mark the
+ /// contents of the Darwin.C.excluded (assert.h) and Tcl.Private modules as
+ /// non-modular headers. For backwards compatibility, we continue to support
+ /// this idiom for just these modules, and map the headers to 'textual' to
+ /// match the original intent.
+ unsigned UsesRequiresExcludedHack : 1;
+
rsmith wrote:
> Do we really need to write this into the `Module` object? Could we instead
> just track this while we're parsing the module map?
Will do.
Comment at: lib/Lex/ModuleMap.cpp:1590-1603
@@ +1589,16 @@
+ bool &IsRequiresExcludedHack) {
+ if (Feature == "excluded" || Feature == "cplusplus") {
+std::string FullName = M->getFullModuleName();
+if (FullName == "Darwin.C.excluded" || FullName == "Tcl.Private") {
+ // We will mark the module contents non-modular. See doc comment for
+ // Module::UsesRequiresExcludedHack.
+ IsRequiresExcludedHack = true;
+ return false;
+} else if (FullName == "IOKit.avc") {
+ // This module was mistakenly marked 'requires cplusplus' in older Darwin
+ // SDK versions. As a backwards compatibility hack, don't add the
+ // requirement.
+ return false;
+}
+ }
+
rsmith wrote:
> Please handle the `excluded` and `cplusplus` cases separately, to keep this
> special case as narrow as possible.
Will do. Not sure why I ever combined them like that...
Comment at: lib/Lex/ModuleMap.cpp:1591
@@ +1590,3 @@
+ if (Feature == "excluded" || Feature == "cplusplus") {
+std::string FullName = M->getFullModuleName();
+if (FullName == "Darwin.C.excluded" || FullName == "Tcl.Private") {
rsmith wrote:
> Seems like overkill to compute the full module name for every module that
> `requires cplusplus`. Instead, how about walking the module path a component
> at a time and checking you get the expected sequence of components?
Makes sense, will do.
Comment at: lib/Lex/ModuleMap.cpp:1891-1908
@@ +1890,20 @@
+
+ if (ActiveModule->UsesRequiresExcludedHack) {
+// Mark this header 'textual' (see doc comment for
+// Module::UsesRequiresExcludedHack). Although iterating over the directory
+// is relatively expensive, in practice this only applies to the uncommonly
+// used Tcl module on Darwin platforms.
+std::error_code EC;
+for (llvm::sys::fs::recursive_directory_iterator I(Dir->getName(), EC), E;
+ I != E && !EC; I.increment(EC)) {
+ if (const FileEntry *FE = SourceMgr.getFileManager().getFile(I->path()))
{
+// FIXME: Taking the name from the FileEntry is unstable and can give
+// different results depending on how we've previously named that file
+// in this build.
+Module::Header Header = {FE->getName(), FE};
+Map.addHeader(ActiveModule, Header, ModuleMap::TextualHeader);
+ }
+}
+return;
+ }
+
rsmith wrote:
> Is there a better way of handling this? If the parent directory isn't itself
> an umbrella directory of some module, maybe you could just ignore the
> umbrella directory declaration for this module entirely?
This only affects Tcl.Private, and Tcl has an umbrella header so I don't think
that will work. The only other way I can think of making this work is to have
a notion of a *directory* that is exempt from its containing umbrella, but I'm
not sure that's a generally good feature and it seems like a lot more work.
Let me know if you have any suggestions though.
Repository:
rL LLVM
http://reviews.llvm.org/D11403
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D11403: [Modules] Add Darwin-specific compatibility module map parsing hacks
benlangmuir added inline comments.
Comment at: lib/Lex/ModuleMap.cpp:1891-1908
@@ +1890,20 @@
+
+ if (ActiveModule->UsesRequiresExcludedHack) {
+// Mark this header 'textual' (see doc comment for
+// Module::UsesRequiresExcludedHack). Although iterating over the directory
+// is relatively expensive, in practice this only applies to the uncommonly
+// used Tcl module on Darwin platforms.
+std::error_code EC;
+for (llvm::sys::fs::recursive_directory_iterator I(Dir->getName(), EC), E;
+ I != E && !EC; I.increment(EC)) {
+ if (const FileEntry *FE = SourceMgr.getFileManager().getFile(I->path()))
{
+// FIXME: Taking the name from the FileEntry is unstable and can give
+// different results depending on how we've previously named that file
+// in this build.
+Module::Header Header = {FE->getName(), FE};
+Map.addHeader(ActiveModule, Header, ModuleMap::TextualHeader);
+ }
+}
+return;
+ }
+
rsmith wrote:
> benlangmuir wrote:
> > rsmith wrote:
> > > Is there a better way of handling this? If the parent directory isn't
> > > itself an umbrella directory of some module, maybe you could just ignore
> > > the umbrella directory declaration for this module entirely?
> > This only affects Tcl.Private, and Tcl has an umbrella header so I don't
> > think that will work. The only other way I can think of making this work
> > is to have a notion of a *directory* that is exempt from its containing
> > umbrella, but I'm not sure that's a generally good feature and it seems
> > like a lot more work. Let me know if you have any suggestions though.
> Ugh, OK. In that case:
>
> * maybe take the file name from `I->path()` rather than `FE->getName()` (we
> want to imagine the files were named within the umbrella directory rather
> than some other way)
> * sort the paths before you add them so that the serialized pcm doesn't
> depend on file system traversal order
>
> Also, you'll be paying this file system traversal cost whenever the relevant
> module map is parsed, not only when the Tcl module is used -- and if it's the
> /usr/include module map, you'll walk this umbrella directory on every build.
> Just wanted to confirm you're OK with that.
> * maybe take the file name from I->path() rather than FE->getName()
> * sort the paths
Ah, good catches!
> Also, you'll be paying this file system traversal cost whenever the relevant
> module map is parsed,
Tcl is a framework so not affected by looking at usr/include. But your comment
made me double check where else we might parse this. I think the answer is:
1. When Tcl is named in an import or header include. This seems fine.
2. When we call collectAllModules()...
I'm not happy about (2) since it can happen during code completion of imports.
I suppose we're already iterating directories and parsing a tonne of other
module map files here. I'll benchmark this and see how awful it is...
Repository:
rL LLVM
http://reviews.llvm.org/D11403
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r333202 - [bash-completion] Fix tab separation on macOS
Author: benlangmuir
Date: Thu May 24 09:25:40 2018
New Revision: 333202
URL: http://llvm.org/viewvc/llvm-project?rev=333202&view=rev
Log:
[bash-completion] Fix tab separation on macOS
We have a regex that needs to match a tab character in the command
output, but on macOS sed doesn't support '\t', causing it to split on
the 't' character instead. Fix by having bash expand the \t first.
Modified:
cfe/trunk/utils/bash-autocomplete.sh
Modified: cfe/trunk/utils/bash-autocomplete.sh
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/utils/bash-autocomplete.sh?rev=333202&r1=333201&r2=333202&view=diff
==
--- cfe/trunk/utils/bash-autocomplete.sh (original)
+++ cfe/trunk/utils/bash-autocomplete.sh Thu May 24 09:25:40 2018
@@ -38,7 +38,8 @@ _clang()
# expand ~ to $HOME
eval local path=${COMP_WORDS[0]}
- flags=$( "$path" --autocomplete="$arg" 2>/dev/null | sed -e 's/\t.*//' )
+ # Use $'\t' so that bash expands the \t for older versions of sed.
+ flags=$( "$path" --autocomplete="$arg" 2>/dev/null | sed -e $'s/\t.*//' )
# If clang is old that it does not support --autocomplete,
# fall back to the filename completion.
if [[ "$?" != 0 ]]; then
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r313011 - [X86] Lower _mm[256|512]_[mask[z]]_avg_epu[8|16] intrinsics to native llvm IR
Sorry, I have no particular insight. My commit was reapplied verbatim shortly after that and the tests passed, so I’m not sure if there’s any relation. > On Sep 13, 2017, at 4:16 AM, Tsafrir, Yael wrote: > > Hello Galina, > > I tried to re-create the issue but with no success. I ran the test on > versions before and after my commit, using your builder’s cmake properties, > and the test passed each time. > There doesn’t seem to be a connection between my commit’s changes and the > failed test. > > We checked the test’s history and the message in > https://reviews.llvm.org/rL259715 <https://reviews.llvm.org/rL259715> > suggests that the last time this test was modified, there were buildbots > failures – could those failures be related or similar to the failures we are > seeing now? > > I’ve CC’d Ben Langmuir and Quentin Colombet as they dealt with rL259715. > > Is there anyone who could help or provide more details about the test failure? > > Thanks, > Yael > <> > From: Galina Kistanova [mailto:gkistan...@gmail.com > <mailto:gkistan...@gmail.com>] > Sent: Tuesday, September 12, 2017 21:16 > To: Tsafrir, Yael mailto:yael.tsaf...@intel.com>> > Cc: cfe-commits <mailto:cfe-commits@lists.llvm.org>> > Subject: Re: r313011 - [X86] Lower _mm[256|512]_[mask[z]]_avg_epu[8|16] > intrinsics to native llvm IR > > Hello Yael, > > It looks like this commit broke one of our builders: > http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/builds/17121 > > <http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/builds/17121> > > . . . > Failing Tests (1): > Clang :: Modules/builtins.m > > Please have a look? > > Thanks > > Galina > > > On Tue, Sep 12, 2017 at 12:46 AM, Yael Tsafrir via cfe-commits > mailto:cfe-commits@lists.llvm.org>> wrote: > Author: ytsafrir > Date: Tue Sep 12 00:46:32 2017 > New Revision: 313011 > > URL: http://llvm.org/viewvc/llvm-project?rev=313011&view=rev > <http://llvm.org/viewvc/llvm-project?rev=313011&view=rev> > Log: > [X86] Lower _mm[256|512]_[mask[z]]_avg_epu[8|16] intrinsics to native llvm IR > > Differential Revision: https://reviews.llvm.org/D37562 > <https://reviews.llvm.org/D37562> > > Modified: > cfe/trunk/include/clang/Basic/BuiltinsX86.def > cfe/trunk/lib/Headers/avx2intrin.h > cfe/trunk/lib/Headers/avx512bwintrin.h > cfe/trunk/lib/Headers/emmintrin.h > cfe/trunk/test/CodeGen/avx2-builtins.c > cfe/trunk/test/CodeGen/avx512bw-builtins.c > cfe/trunk/test/CodeGen/avx512vlbw-builtins.c > cfe/trunk/test/CodeGen/builtins-x86.c > cfe/trunk/test/CodeGen/sse2-builtins.c > > Modified: cfe/trunk/include/clang/Basic/BuiltinsX86.def > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/BuiltinsX86.def?rev=313011&r1=313010&r2=313011&view=diff > > <http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/BuiltinsX86.def?rev=313011&r1=313010&r2=313011&view=diff> > == > --- cfe/trunk/include/clang/Basic/BuiltinsX86.def (original) > +++ cfe/trunk/include/clang/Basic/BuiltinsX86.def Tue Sep 12 00:46:32 2017 > @@ -266,8 +266,6 @@ TARGET_BUILTIN(__builtin_ia32_paddusw128 > TARGET_BUILTIN(__builtin_ia32_psubusb128, "V16cV16cV16c", "", "sse2") > TARGET_BUILTIN(__builtin_ia32_psubusw128, "V8sV8sV8s", "", "sse2") > TARGET_BUILTIN(__builtin_ia32_pmulhw128, "V8sV8sV8s", "", "sse2") > -TARGET_BUILTIN(__builtin_ia32_pavgb128, "V16cV16cV16c", "", "sse2") > -TARGET_BUILTIN(__builtin_ia32_pavgw128, "V8sV8sV8s", "", "sse2") > TARGET_BUILTIN(__builtin_ia32_pmaxub128, "V16cV16cV16c", "", "sse2") > TARGET_BUILTIN(__builtin_ia32_pmaxsw128, "V8sV8sV8s", "", "sse2") > TARGET_BUILTIN(__builtin_ia32_pminub128, "V16cV16cV16c", "", "sse2") > @@ -522,8 +520,6 @@ TARGET_BUILTIN(__builtin_ia32_paddusw256 > TARGET_BUILTIN(__builtin_ia32_psubusb256, "V32cV32cV32c", "", "avx2") > TARGET_BUILTIN(__builtin_ia32_psubusw256, "V16sV16sV16s", "", "avx2") > TARGET_BUILTIN(__builtin_ia32_palignr256, "V32cV32cV32cIi", "", "avx2") > -TARGET_BUILTIN(__builtin_ia32_pavgb256, "V32cV32cV32c", "", "avx2") > -TARGET_BUILTIN(__builtin_ia32_pavgw256, "V16sV16sV16
[clang] f80a0ea - [clang][deps] Split translation units into individual -cc1 or other commands
Author: Ben Langmuir
Date: 2022-08-30T15:23:19-07:00
New Revision: f80a0ea760728e70f70debf744277bc3aa59bc17
URL:
https://github.com/llvm/llvm-project/commit/f80a0ea760728e70f70debf744277bc3aa59bc17
DIFF:
https://github.com/llvm/llvm-project/commit/f80a0ea760728e70f70debf744277bc3aa59bc17.diff
LOG: [clang][deps] Split translation units into individual -cc1 or other
commands
Instead of trying to "fix" the original driver invocation by appending
arguments to it, split it into multiple commands, and for each -cc1
command use a CompilerInvocation to give precise control over the
invocation.
This change should make it easier to (in the future) canonicalize the
command-line (e.g. to improve hits in something like ccache), apply
optimizations, or start supporting multi-arch builds, which would
require different modules for each arch.
In the long run it may make sense to treat the TU commands as a
dependency graph, each with their own dependencies on modules or earlier
TU commands, but for now they are simply a list that is executed in
order, and the dependencies are simply duplicated. Since we currently
only support single-arch builds, there is no parallelism available in
the execution.
Differential Revision: https://reviews.llvm.org/D132405
Added:
clang/test/ClangScanDeps/deprecated-driver-api.c
clang/test/ClangScanDeps/multiple-commands.c
Modified:
clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
clang/test/ClangScanDeps/diagnostics.c
clang/test/ClangScanDeps/header-search-pruning-transitive.c
clang/test/ClangScanDeps/modules-context-hash-ignore-macros.c
clang/test/ClangScanDeps/modules-context-hash-outputs.c
clang/test/ClangScanDeps/modules-context-hash-warnings.c
clang/test/ClangScanDeps/modules-context-hash.c
clang/test/ClangScanDeps/modules-dep-args.c
clang/test/ClangScanDeps/modules-fmodule-name-no-module-built.m
clang/test/ClangScanDeps/modules-full.cpp
clang/test/ClangScanDeps/modules-implicit-dot-private.m
clang/test/ClangScanDeps/modules-incomplete-umbrella.c
clang/test/ClangScanDeps/modules-inferred.m
clang/test/ClangScanDeps/modules-no-undeclared-includes.c
clang/test/ClangScanDeps/modules-pch-common-submodule.c
clang/test/ClangScanDeps/modules-pch-common-via-submodule.c
clang/test/ClangScanDeps/modules-pch.c
clang/test/ClangScanDeps/removed-args.c
clang/tools/clang-scan-deps/ClangScanDeps.cpp
clang/utils/module-deps-to-rsp.py
Removed:
diff --git
a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
index cc3f330828a39..c0d273297f18f 100644
--- a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
+++ b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
@@ -49,8 +49,16 @@ struct FullDependencies {
/// determined that the
diff erences are benign for this compilation.
std::vector ClangModuleDeps;
- /// The command line of the TU (excluding the compiler executable).
- std::vector CommandLine;
+ /// The sequence of commands required to build the translation unit. Commands
+ /// should be executed in order.
+ ///
+ /// FIXME: If we add support for multi-arch builds in clang-scan-deps, we
+ /// should make the dependencies between commands explicit to enable parallel
+ /// builds of each architecture.
+ std::vector Commands;
+
+ /// Deprecated driver command-line. This will be removed in a future version.
+ std::vector DriverCommandLine;
};
struct FullDependenciesResult {
@@ -99,6 +107,12 @@ class DependencyScanningTool {
LookupModuleOutputCallback LookupModuleOutput,
llvm::Optional ModuleName = None);
+ llvm::Expected
getFullDependenciesLegacyDriverCommand(
+ const std::vector &CommandLine, StringRef CWD,
+ const llvm::StringSet<> &AlreadySeen,
+ LookupModuleOutputCallback LookupModuleOutput,
+ llvm::Optional ModuleName = None);
+
private:
DependencyScanningWorker Worker;
};
@@ -111,6 +125,10 @@ class FullDependencyConsumer : public DependencyConsumer {
: AlreadySeen(AlreadySeen), LookupModuleOutput(LookupModuleOutput),
EagerLoadModules(EagerLoadModules) {}
+ void handleBuildCommand(Command Cmd) override {
+Commands.push_back(std::move(Cmd));
+ }
+
void handleDependencyOutputOpts(const DependencyOutputOptions &) override {}
void handleFileDepende
[clang] 1877d76 - Revert "[clang][deps] Split translation units into individual -cc1 or other commands"
Author: Ben Langmuir
Date: 2022-08-30T15:50:09-07:00
New Revision: 1877d76aa011e6e481630523be5ed2d86d2b10f0
URL:
https://github.com/llvm/llvm-project/commit/1877d76aa011e6e481630523be5ed2d86d2b10f0
DIFF:
https://github.com/llvm/llvm-project/commit/1877d76aa011e6e481630523be5ed2d86d2b10f0.diff
LOG: Revert "[clang][deps] Split translation units into individual -cc1 or
other commands"
Failing on some bots, reverting until I can fix it.
This reverts commit f80a0ea760728e70f70debf744277bc3aa59bc17.
Added:
Modified:
clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
clang/test/ClangScanDeps/diagnostics.c
clang/test/ClangScanDeps/header-search-pruning-transitive.c
clang/test/ClangScanDeps/modules-context-hash-ignore-macros.c
clang/test/ClangScanDeps/modules-context-hash-outputs.c
clang/test/ClangScanDeps/modules-context-hash-warnings.c
clang/test/ClangScanDeps/modules-context-hash.c
clang/test/ClangScanDeps/modules-dep-args.c
clang/test/ClangScanDeps/modules-fmodule-name-no-module-built.m
clang/test/ClangScanDeps/modules-full.cpp
clang/test/ClangScanDeps/modules-implicit-dot-private.m
clang/test/ClangScanDeps/modules-incomplete-umbrella.c
clang/test/ClangScanDeps/modules-inferred.m
clang/test/ClangScanDeps/modules-no-undeclared-includes.c
clang/test/ClangScanDeps/modules-pch-common-submodule.c
clang/test/ClangScanDeps/modules-pch-common-via-submodule.c
clang/test/ClangScanDeps/modules-pch.c
clang/test/ClangScanDeps/removed-args.c
clang/tools/clang-scan-deps/ClangScanDeps.cpp
clang/utils/module-deps-to-rsp.py
Removed:
clang/test/ClangScanDeps/deprecated-driver-api.c
clang/test/ClangScanDeps/multiple-commands.c
diff --git
a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
index c0d273297f18f..cc3f330828a39 100644
--- a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
+++ b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
@@ -49,16 +49,8 @@ struct FullDependencies {
/// determined that the
diff erences are benign for this compilation.
std::vector ClangModuleDeps;
- /// The sequence of commands required to build the translation unit. Commands
- /// should be executed in order.
- ///
- /// FIXME: If we add support for multi-arch builds in clang-scan-deps, we
- /// should make the dependencies between commands explicit to enable parallel
- /// builds of each architecture.
- std::vector Commands;
-
- /// Deprecated driver command-line. This will be removed in a future version.
- std::vector DriverCommandLine;
+ /// The command line of the TU (excluding the compiler executable).
+ std::vector CommandLine;
};
struct FullDependenciesResult {
@@ -107,12 +99,6 @@ class DependencyScanningTool {
LookupModuleOutputCallback LookupModuleOutput,
llvm::Optional ModuleName = None);
- llvm::Expected
getFullDependenciesLegacyDriverCommand(
- const std::vector &CommandLine, StringRef CWD,
- const llvm::StringSet<> &AlreadySeen,
- LookupModuleOutputCallback LookupModuleOutput,
- llvm::Optional ModuleName = None);
-
private:
DependencyScanningWorker Worker;
};
@@ -125,10 +111,6 @@ class FullDependencyConsumer : public DependencyConsumer {
: AlreadySeen(AlreadySeen), LookupModuleOutput(LookupModuleOutput),
EagerLoadModules(EagerLoadModules) {}
- void handleBuildCommand(Command Cmd) override {
-Commands.push_back(std::move(Cmd));
- }
-
void handleDependencyOutputOpts(const DependencyOutputOptions &) override {}
void handleFileDependency(StringRef File) override {
@@ -152,17 +134,14 @@ class FullDependencyConsumer : public DependencyConsumer {
return LookupModuleOutput(ID, Kind);
}
- FullDependenciesResult getFullDependenciesLegacyDriverCommand(
+ FullDependenciesResult getFullDependencies(
const std::vector &OriginalCommandLine) const;
- FullDependenciesResult takeFullDependencies();
-
private:
std::vector Dependencies;
std::vector PrebuiltModuleDeps;
llvm::MapVector>
ClangModuleDeps;
- std::vector Commands;
std::string ContextHash;
std::vector OutputPaths;
const llvm::StringSet<> &AlreadySeen;
diff --git
a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
b/clang/include/clang/Tooling/Depend
[clang] 83902c4 - Reapply "[clang][deps] Split translation units into individual -cc1 or other commands"
Author: Ben Langmuir
Date: 2022-08-31T09:45:11-07:00
New Revision: 83902c403611af3a52453867cb8848fb3fd6a39c
URL:
https://github.com/llvm/llvm-project/commit/83902c403611af3a52453867cb8848fb3fd6a39c
DIFF:
https://github.com/llvm/llvm-project/commit/83902c403611af3a52453867cb8848fb3fd6a39c.diff
LOG: Reapply "[clang][deps] Split translation units into individual -cc1 or
other commands"
Attempt to fix the test failures observed in CI:
* Add Option dependency, which caused BUILD_SHARED_LIBS builds to fail
* Adapt tests that accidentally depended on the host platform: platforms
that don't use an integrated assembler (e.g. AIX) get a different set
of commands from the driver. Most dependency scanner tests can use
-fsyntax-only or -E instead of -c to avoid this, and in the rare case
we want to check -c specifically, set an explicit target so the
behaviour is independent of the host.
Original commit message follows.
---
Instead of trying to "fix" the original driver invocation by appending
arguments to it, split it into multiple commands, and for each -cc1
command use a CompilerInvocation to give precise control over the
invocation.
This change should make it easier to (in the future) canonicalize the
command-line (e.g. to improve hits in something like ccache), apply
optimizations, or start supporting multi-arch builds, which would
require different modules for each arch.
In the long run it may make sense to treat the TU commands as a
dependency graph, each with their own dependencies on modules or earlier
TU commands, but for now they are simply a list that is executed in
order, and the dependencies are simply duplicated. Since we currently
only support single-arch builds, there is no parallelism available in
the execution.
Differential Revision: https://reviews.llvm.org/D132405
Added:
clang/test/ClangScanDeps/deprecated-driver-api.c
clang/test/ClangScanDeps/multiple-commands.c
Modified:
clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
clang/lib/Tooling/DependencyScanning/CMakeLists.txt
clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
clang/test/ClangScanDeps/diagnostics.c
clang/test/ClangScanDeps/header-search-pruning-transitive.c
clang/test/ClangScanDeps/modules-context-hash-ignore-macros.c
clang/test/ClangScanDeps/modules-context-hash-outputs.c
clang/test/ClangScanDeps/modules-context-hash-warnings.c
clang/test/ClangScanDeps/modules-context-hash.c
clang/test/ClangScanDeps/modules-dep-args.c
clang/test/ClangScanDeps/modules-fmodule-name-no-module-built.m
clang/test/ClangScanDeps/modules-full.cpp
clang/test/ClangScanDeps/modules-implicit-dot-private.m
clang/test/ClangScanDeps/modules-incomplete-umbrella.c
clang/test/ClangScanDeps/modules-inferred.m
clang/test/ClangScanDeps/modules-no-undeclared-includes.c
clang/test/ClangScanDeps/modules-pch-common-submodule.c
clang/test/ClangScanDeps/modules-pch-common-via-submodule.c
clang/test/ClangScanDeps/modules-pch.c
clang/test/ClangScanDeps/removed-args.c
clang/tools/clang-scan-deps/ClangScanDeps.cpp
clang/utils/module-deps-to-rsp.py
Removed:
diff --git
a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
index cc3f330828a39..c0d273297f18f 100644
--- a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
+++ b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
@@ -49,8 +49,16 @@ struct FullDependencies {
/// determined that the
diff erences are benign for this compilation.
std::vector ClangModuleDeps;
- /// The command line of the TU (excluding the compiler executable).
- std::vector CommandLine;
+ /// The sequence of commands required to build the translation unit. Commands
+ /// should be executed in order.
+ ///
+ /// FIXME: If we add support for multi-arch builds in clang-scan-deps, we
+ /// should make the dependencies between commands explicit to enable parallel
+ /// builds of each architecture.
+ std::vector Commands;
+
+ /// Deprecated driver command-line. This will be removed in a future version.
+ std::vector DriverCommandLine;
};
struct FullDependenciesResult {
@@ -99,6 +107,12 @@ class DependencyScanningTool {
LookupModuleOutputCallback LookupModuleOutput,
llvm::Optional ModuleName = None);
+ llvm::Expected
getFullDependenciesLegacyDriverCommand(
+ const std::vector &CommandLine, Str
[clang] d96f526 - [clang][test] Disallow using the default module cache path in lit tests
Author: Ben Langmuir
Date: 2022-09-12T09:54:56-07:00
New Revision: d96f526196ac4cebfdd318473816f6d4b9d76707
URL:
https://github.com/llvm/llvm-project/commit/d96f526196ac4cebfdd318473816f6d4b9d76707
DIFF:
https://github.com/llvm/llvm-project/commit/d96f526196ac4cebfdd318473816f6d4b9d76707.diff
LOG: [clang][test] Disallow using the default module cache path in lit tests
Make the default module cache path invalid when running lit tests so
that tests are forced to provide a cache path. This avoids accidentally
escaping to the system default location, and would have caught the
failure recently found in ClangScanDeps/multiple-commands.c.
Differential Revision: https://reviews.llvm.org/D133622
Added:
Modified:
clang/test/Driver/modules-cache-path.m
clang/test/Modules/driver.c
llvm/utils/lit/lit/llvm/config.py
Removed:
diff --git a/clang/test/Driver/modules-cache-path.m
b/clang/test/Driver/modules-cache-path.m
index 1da27d2143631..f700a9738742f 100644
--- a/clang/test/Driver/modules-cache-path.m
+++ b/clang/test/Driver/modules-cache-path.m
@@ -1,4 +1,4 @@
-// RUN: %clang -fmodules -### %s 2>&1 | FileCheck %s
-check-prefix=CHECK-DEFAULT
+// RUN: env -u CLANG_MODULE_CACHE_PATH %clang -fmodules -### %s 2>&1 |
FileCheck %s -check-prefix=CHECK-DEFAULT
// CHECK-DEFAULT: -fmodules-cache-path={{.*}}clang{{[/\\]+}}ModuleCache
// RUN: env CLANG_MODULE_CACHE_PATH=/dev/null \
diff --git a/clang/test/Modules/driver.c b/clang/test/Modules/driver.c
index 34fc163a5ccd4..8ffa23ba4e71c 100644
--- a/clang/test/Modules/driver.c
+++ b/clang/test/Modules/driver.c
@@ -1,4 +1,4 @@
-// RUN: %clang -fmodules -fimplicit-module-maps %s -### 2>&1 | FileCheck
-check-prefix CHECK-NO_MODULE_CACHE %s
+// RUN: env -u CLANG_MODULE_CACHE_PATH %clang -fmodules -fimplicit-module-maps
%s -### 2>&1 | FileCheck -check-prefix CHECK-NO_MODULE_CACHE %s
// RUN: %clang -fmodules -fimplicit-module-maps -fmodules-cache-path=blarg %s
-### 2>&1 | FileCheck -check-prefix CHECK-WITH_MODULE_CACHE %s
// CHECK-NO_MODULE_CACHE: {{clang.*"-fmodules-cache-path=.*ModuleCache"}}
diff --git a/llvm/utils/lit/lit/llvm/config.py
b/llvm/utils/lit/lit/llvm/config.py
index 591b9938f211f..6149f4db7e0ca 100644
--- a/llvm/utils/lit/lit/llvm/config.py
+++ b/llvm/utils/lit/lit/llvm/config.py
@@ -495,6 +495,10 @@ def use_clang(self, additional_tool_dirs=[],
additional_flags=[],
self.clear_environment(possibly_dangerous_env_vars)
+# Make the default module cache path invalid so that tests are forced
to
+# provide a cache path if they use implicit modules.
+self.with_environment('CLANG_MODULE_CACHE_PATH', '/dev/null')
+
# Tweak the PATH to include the tools dir and the scripts dir.
# Put Clang first to avoid LLVM from overriding out-of-tree clang
# builds.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 4a72459 - Revert "[clang][test] Disallow using the default module cache path in lit tests"
Author: Ben Langmuir
Date: 2022-09-12T13:10:22-07:00
New Revision: 4a72459ed639e3eb7188565c758181c43d3192aa
URL:
https://github.com/llvm/llvm-project/commit/4a72459ed639e3eb7188565c758181c43d3192aa
DIFF:
https://github.com/llvm/llvm-project/commit/4a72459ed639e3eb7188565c758181c43d3192aa.diff
LOG: Revert "[clang][test] Disallow using the default module cache path in lit
tests"
This reverts commit d96f526196ac4cebfdd318473816f6d4b9d76707.
Some systems do not support `env -u`.
Added:
Modified:
clang/test/Driver/modules-cache-path.m
clang/test/Modules/driver.c
llvm/utils/lit/lit/llvm/config.py
Removed:
diff --git a/clang/test/Driver/modules-cache-path.m
b/clang/test/Driver/modules-cache-path.m
index f700a9738742f..1da27d2143631 100644
--- a/clang/test/Driver/modules-cache-path.m
+++ b/clang/test/Driver/modules-cache-path.m
@@ -1,4 +1,4 @@
-// RUN: env -u CLANG_MODULE_CACHE_PATH %clang -fmodules -### %s 2>&1 |
FileCheck %s -check-prefix=CHECK-DEFAULT
+// RUN: %clang -fmodules -### %s 2>&1 | FileCheck %s
-check-prefix=CHECK-DEFAULT
// CHECK-DEFAULT: -fmodules-cache-path={{.*}}clang{{[/\\]+}}ModuleCache
// RUN: env CLANG_MODULE_CACHE_PATH=/dev/null \
diff --git a/clang/test/Modules/driver.c b/clang/test/Modules/driver.c
index 8ffa23ba4e71c..34fc163a5ccd4 100644
--- a/clang/test/Modules/driver.c
+++ b/clang/test/Modules/driver.c
@@ -1,4 +1,4 @@
-// RUN: env -u CLANG_MODULE_CACHE_PATH %clang -fmodules -fimplicit-module-maps
%s -### 2>&1 | FileCheck -check-prefix CHECK-NO_MODULE_CACHE %s
+// RUN: %clang -fmodules -fimplicit-module-maps %s -### 2>&1 | FileCheck
-check-prefix CHECK-NO_MODULE_CACHE %s
// RUN: %clang -fmodules -fimplicit-module-maps -fmodules-cache-path=blarg %s
-### 2>&1 | FileCheck -check-prefix CHECK-WITH_MODULE_CACHE %s
// CHECK-NO_MODULE_CACHE: {{clang.*"-fmodules-cache-path=.*ModuleCache"}}
diff --git a/llvm/utils/lit/lit/llvm/config.py
b/llvm/utils/lit/lit/llvm/config.py
index 6149f4db7e0ca..591b9938f211f 100644
--- a/llvm/utils/lit/lit/llvm/config.py
+++ b/llvm/utils/lit/lit/llvm/config.py
@@ -495,10 +495,6 @@ def use_clang(self, additional_tool_dirs=[],
additional_flags=[],
self.clear_environment(possibly_dangerous_env_vars)
-# Make the default module cache path invalid so that tests are forced
to
-# provide a cache path if they use implicit modules.
-self.with_environment('CLANG_MODULE_CACHE_PATH', '/dev/null')
-
# Tweak the PATH to include the tools dir and the scripts dir.
# Put Clang first to avoid LLVM from overriding out-of-tree clang
# builds.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] Avoid modules diagnostics for `__has_include()` (PR #71450)
https://github.com/benlangmuir approved this pull request. https://github.com/llvm/llvm-project/pull/71450 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [clang][DepScan] Make OptimizeArgs a bit mask enum and enable by default (PR #71588)
https://github.com/benlangmuir approved this pull request. 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] Revert "Reland [clang] Canonicalize system headers in dependency file when -canonical-prefixes" (PR #71697)
https://github.com/benlangmuir approved this pull request. Thanks! https://github.com/llvm/llvm-project/pull/71697 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][deps] Skip writing `DIAG_PRAGMA_MAPPINGS` record (PR #70874)
https://github.com/benlangmuir approved this pull request. https://github.com/llvm/llvm-project/pull/70874 ___ 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)
https://github.com/benlangmuir approved this pull request. 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
[llvm] [clang] [clang] NFC: Deprecate `FileEntry::getName()` (PR #68157)
@@ -157,6 +157,25 @@ #define LLVM_DEPRECATED(MSG, FIX) [[deprecated(MSG)]] #endif +// clang-format off +#if defined(__clang__) || defined(__GNUC__) +#define LLVM_IGNORE_DEPRECATIONS_OF_DECLARATIONS_BEGIN \ benlangmuir wrote: I'm not sure how often the general form of this is useful - I don't actually see many places in the code that disable diagnostics for both MSVC and clang/gcc. So I would stick with the version that is specific to deprecations. Maybe you should more closely match the name of `_LIBCPP_SUPPRESS_DEPRECATED_PUSH` -- e.g. `LLVM_SUPPRESS_DEPRECATED_PUSH` and `..._POP`. https://github.com/llvm/llvm-project/pull/68157 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [clang] NFC: Deprecate `FileEntry::getName()` (PR #68157)
https://github.com/benlangmuir approved this pull request. https://github.com/llvm/llvm-project/pull/68157 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] NFCI: Make `ModuleFile::File` non-optional (PR #74892)
https://github.com/benlangmuir edited https://github.com/llvm/llvm-project/pull/74892 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] NFCI: Make `ModuleFile::File` non-optional (PR #74892)
@@ -441,22 +434,19 @@ void
ModuleManager::visit(llvm::function_ref Visitor,
bool ModuleManager::lookupModuleFile(StringRef FileName, off_t ExpectedSize,
time_t ExpectedModTime,
OptionalFileEntryRef &File) {
- File = std::nullopt;
- if (FileName == "-")
+ if (FileName == "-") {
+File = expectedToOptional(FileMgr.getSTDIN());
return false;
+ }
// Open the file immediately to ensure there is no race between stat'ing and
// opening the file.
- OptionalFileEntryRef FileOrErr =
- expectedToOptional(FileMgr.getFileRef(FileName, /*OpenFile=*/true,
-/*CacheFailure=*/false));
- if (!FileOrErr)
-return false;
-
- File = *FileOrErr;
+ File = expectedToOptional(FileMgr.getFileRef(FileName, /*OpenFile=*/true,
benlangmuir wrote:
Yeah, I would recommend that.
https://github.com/llvm/llvm-project/pull/74892
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] NFCI: Make `ModuleFile::File` non-optional (PR #74892)
https://github.com/benlangmuir approved this pull request. https://github.com/llvm/llvm-project/pull/74892 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang] [clang] NFC: Remove `OptionalDirectoryEntryRefDegradesToDirectoryEntryPtr` (PR #74900)
https://github.com/benlangmuir approved this pull request. Nice simplification https://github.com/llvm/llvm-project/pull/74900 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang] [clang] NFC: Remove `OptionalFileEntryRefDegradesToFileEntryPtr` (PR #74899)
https://github.com/benlangmuir approved this pull request. https://github.com/llvm/llvm-project/pull/74899 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang] NFC: Remove `{File, Directory}Entry::getName()` (PR #74910)
https://github.com/benlangmuir approved this pull request. https://github.com/llvm/llvm-project/pull/74910 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-scan-deps] [P1689] Keep consistent behavior for make dependencies with clang (PR #69551)
@@ -666,13 +666,19 @@ static StringRef
makeAbsoluteAndPreferred(CompilerInstance &CI, StringRef Path,
}
void ModuleDepCollector::addFileDep(StringRef Path) {
- llvm::SmallString<256> Storage;
- Path = makeAbsoluteAndPreferred(ScanInstance, Path, Storage);
+ // Within P1689 format, we don't want all the paths to be absolute path
+ // since it may violate the tranditional make style dependencies info.
+ if (!IsStdModuleP1689Format) {
+llvm::SmallString<256> Storage;
+Path = makeAbsoluteAndPreferred(ScanInstance, Path, Storage);
+ }
FileDeps.push_back(std::string(Path));
benlangmuir wrote:
This is a use-after-free of `Storage` since `Path` will point to that buffer if
the string is modified. Same for the other case below.
https://github.com/llvm/llvm-project/pull/69551
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-scan-deps] [P1689] Keep consistent behavior for make dependencies with clang (PR #69551)
https://github.com/benlangmuir approved this pull request. https://github.com/llvm/llvm-project/pull/69551 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][deps] Skip slow `UNHASHED_CONTROL_BLOCK` records (PR #69975)
https://github.com/benlangmuir approved this pull request. https://github.com/llvm/llvm-project/pull/69975 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] Don't prevent translation of FW_Private includes when explicitly building FW (PR #70714)
https://github.com/benlangmuir approved this pull request. https://github.com/llvm/llvm-project/pull/70714 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Fix sorting header paths (PR #73323)
benlangmuir wrote:
You could try using `clang -cc1 -E -x c-module-map` which calls
`Module::print`. To trigger this code path you can follow the pattern in
`darwin_specific_modulemap_hacks.m`, ie create a module like
```
$ touch Tcl/x1.h
$ touch Tcl/x2.h
$ cat Tcl/module.modulemap
module Tcl {
module Private {
requires excluded
umbrella ""
}
}
$ clang -cc1 -E -x c-module-map Tcl/module.modulemap -fmodule-name=Tcl
# 1 "Tcl/module.modulemap"
module Tcl {
module Private {
textual header "" { size 0 mtime 1701196669 }
textual header "" { size 0 mtime 1701196741 }
textual header "" { size 75 mtime 1701196655 }
}
}
```
However I think the printing would need to be fixed to print the right header
name instead of "" first.
https://github.com/llvm/llvm-project/pull/73323
___
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)
https://github.com/benlangmuir edited 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)
@@ -4997,6 +4997,19 @@ ASTReader::ASTReadResult
ASTReader::readUnhashedControlBlockImpl(
F->SearchPathUsage[I] = true;
break;
}
+case VFS_USAGE: {
+ if (!F)
+break;
+ unsigned Count = Record[0];
+ const char *Byte = Blob.data();
+ F->VFSUsage = llvm::BitVector(Count, false);
+ for (unsigned I = 0; I < Count; ++Byte)
+for (unsigned Bit = 0; Bit < 8 && I < Count; ++Bit, ++I)
+ if (*Byte & (1 << Bit))
+F->VFSUsage[I] = true;
benlangmuir wrote:
Probably worth hoisting this read into its own function since we seem to be
accumulating bit vectors.
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)
https://github.com/benlangmuir commented: 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. 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)
@@ -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 =
benlangmuir wrote:
Is this callback type used?
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)
https://github.com/benlangmuir approved this pull request. 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] eab2a06 - Revert "Reland "[X86] Support `_Float16` on SSE2 and up""
Author: Ben Langmuir
Date: 2022-06-28T10:59:03-07:00
New Revision: eab2a06f0fde51eee6b1526bd301d60c4cc9b050
URL:
https://github.com/llvm/llvm-project/commit/eab2a06f0fde51eee6b1526bd301d60c4cc9b050
DIFF:
https://github.com/llvm/llvm-project/commit/eab2a06f0fde51eee6b1526bd301d60c4cc9b050.diff
LOG: Revert "Reland "[X86] Support `_Float16` on SSE2 and up""
Broke compiler-rt on Darwin:
https://green.lab.llvm.org/green/job/clang-stage1-RA/29920/
This reverts commit 527ef8ca981e88a35758c0e4143be6853ea26dfc.
Added:
clang/test/CodeGen/X86/avx512fp16-complex.c
Modified:
clang/docs/LanguageExtensions.rst
clang/docs/ReleaseNotes.rst
clang/lib/Basic/Targets/X86.cpp
clang/test/Sema/Float16.c
clang/test/Sema/conversion-target-dep.c
clang/test/SemaCXX/Float16.cpp
compiler-rt/test/builtins/CMakeLists.txt
Removed:
clang/test/CodeGen/X86/Float16-arithmetic.c
clang/test/CodeGen/X86/Float16-complex.c
diff --git a/clang/docs/LanguageExtensions.rst
b/clang/docs/LanguageExtensions.rst
index 1bac2aee84bd9..af697fafd8c41 100644
--- a/clang/docs/LanguageExtensions.rst
+++ b/clang/docs/LanguageExtensions.rst
@@ -743,13 +743,7 @@ targets pending ABI standardization:
* 64-bit ARM (AArch64)
* AMDGPU
* SPIR
-* X86 (see below)
-
-On X86 targets, ``_Float16`` is supported as long as SSE2 is available, which
-includes all 64-bit and all recent 32-bit processors. When the target supports
-AVX512-FP16, ``_Float16`` arithmetic is performed using that native support.
-Otherwise, ``_Float16`` arithmetic is performed by promoting to ``float``,
-performing the operation, and then truncating to ``_Float16``.
+* X86 (Only available under feature AVX512-FP16)
``_Float16`` will be supported on more targets as they define ABIs for it.
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 733f10a7e4d1a..3e29987ad2631 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -514,9 +514,6 @@ X86 Support in Clang
- Support ``-mharden-sls=[none|all|return|indirect-jmp]`` for straight-line
speculation hardening.
-- Support for the ``_Float16`` type has been added for all targets with SSE2.
- When AVX512-FP16 is not available, arithmetic on ``_Float16`` is emulated
- using ``float``.
DWARF Support in Clang
--
diff --git a/clang/lib/Basic/Targets/X86.cpp b/clang/lib/Basic/Targets/X86.cpp
index 0b3d87837ef6a..b83b3517ddf90 100644
--- a/clang/lib/Basic/Targets/X86.cpp
+++ b/clang/lib/Basic/Targets/X86.cpp
@@ -239,6 +239,7 @@ bool
X86TargetInfo::handleTargetFeatures(std::vector &Features,
HasAVX512ER = true;
} else if (Feature == "+avx512fp16") {
HasAVX512FP16 = true;
+ HasFloat16 = true;
} else if (Feature == "+avx512pf") {
HasAVX512PF = true;
} else if (Feature == "+avx512dq") {
@@ -354,9 +355,6 @@ bool
X86TargetInfo::handleTargetFeatures(std::vector &Features,
.Default(NoSSE);
SSELevel = std::max(SSELevel, Level);
-// Turn on _float16 for x86 (feature sse2)
-HasFloat16 = SSELevel >= SSE2;
-
MMX3DNowEnum ThreeDNowLevel = llvm::StringSwitch(Feature)
.Case("+3dnowa", AMD3DNowAthlon)
.Case("+3dnow", AMD3DNow)
diff --git a/clang/test/CodeGen/X86/Float16-arithmetic.c
b/clang/test/CodeGen/X86/Float16-arithmetic.c
deleted file mode 100644
index 726da22a22b08..0
--- a/clang/test/CodeGen/X86/Float16-arithmetic.c
+++ /dev/null
@@ -1,29 +0,0 @@
-// RUN: %clang_cc1 -triple x86_64-unknown-unknown \
-// RUN: -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK
-
-// CHECK-NOT: fpext
-// CHECK-NOT: fptrunc
-
-_Float16 add1(_Float16 a, _Float16 b) {
- return a + b;
-}
-
-_Float16 add2(_Float16 a, _Float16 b, _Float16 c) {
- return a + b + c;
-}
-
-_Float16 div(_Float16 a, _Float16 b) {
- return a / b;
-}
-
-_Float16 mul(_Float16 a, _Float16 b) {
- return a * b;
-}
-
-_Float16 add_and_mul1(_Float16 a, _Float16 b, _Float16 c, _Float16 d) {
- return a * b + c * d;
-}
-
-_Float16 add_and_mul2(_Float16 a, _Float16 b, _Float16 c, _Float16 d) {
- return (a - 6 * b) + c;
-}
diff --git a/clang/test/CodeGen/X86/Float16-complex.c
b/clang/test/CodeGen/X86/avx512fp16-complex.c
similarity index 96%
rename from clang/test/CodeGen/X86/Float16-complex.c
rename to clang/test/CodeGen/X86/avx512fp16-complex.c
index ebb290c976e7d..8a6b50eb0056b 100644
--- a/clang/test/CodeGen/X86/Float16-complex.c
+++ b/clang/test/CodeGen/X86/avx512fp16-complex.c
@@ -1,5 +1,4 @@
// RUN: %clang_cc1 %s -O0 -emit-llvm -triple x86_64-unknown-unknown
-target-feature +avx512fp16 -o - | FileCheck %s --check-prefix=X86
-// RUN: %clang_cc1 %s -O0 -emit-llvm -triple x86_64-unknown-unknown -o - |
FileCheck %s --check-pref
[clang] 67a84ec - [clang] Cleanup ASTContext before output files in crash recovery for modules
Author: Ben Langmuir
Date: 2022-07-07T10:23:57-07:00
New Revision: 67a84ec8105e590159b6303a1f0e3cb77c02b5fe
URL:
https://github.com/llvm/llvm-project/commit/67a84ec8105e590159b6303a1f0e3cb77c02b5fe
DIFF:
https://github.com/llvm/llvm-project/commit/67a84ec8105e590159b6303a1f0e3cb77c02b5fe.diff
LOG: [clang] Cleanup ASTContext before output files in crash recovery for
modules
When we recover from a crash in a module compilation thread, we need to
ensure any output streams owned by the ASTConsumer (e.g. in
RawPCHContainerGenerator) are deleted before we call clearOutputFiles().
This has the same theoretical issues with proxy streams that Duncan
discusses in the commit 2d133867833fe8eb. In practice, this was observed
as a use-after-free crash on a downstream branch that uses such a proxy
stream in this code path. Add an assertion so it won't regress.
Differential Revision: https://reviews.llvm.org/D129220
rdar://96525032
Added:
Modified:
clang/lib/Frontend/CompilerInstance.cpp
clang/lib/Frontend/FrontendAction.cpp
Removed:
diff --git a/clang/lib/Frontend/CompilerInstance.cpp
b/clang/lib/Frontend/CompilerInstance.cpp
index b982ca72c78ce..ba006b5d49f87 100644
--- a/clang/lib/Frontend/CompilerInstance.cpp
+++ b/clang/lib/Frontend/CompilerInstance.cpp
@@ -757,6 +757,8 @@ void CompilerInstance::createSema(TranslationUnitKind
TUKind,
// Output Files
void CompilerInstance::clearOutputFiles(bool EraseFiles) {
+ // The ASTConsumer can own streams that write to the output files.
+ assert(!hasASTConsumer() && "ASTConsumer should be reset");
// Ignore errors that occur when trying to discard the temp file.
for (OutputFile &OF : OutputFiles) {
if (EraseFiles) {
@@ -1235,8 +1237,7 @@ compileModuleImpl(CompilerInstance &ImportingInstance,
SourceLocation ImportLoc,
// Execute the action to actually build the module in-place. Use a separate
// thread so that we get a stack large enough.
- llvm::CrashRecoveryContext CRC;
- CRC.RunSafelyOnThread(
+ bool Crashed = !llvm::CrashRecoveryContext().RunSafelyOnThread(
[&]() {
GenerateModuleFromModuleMapAction Action;
Instance.ExecuteAction(Action);
@@ -1249,9 +1250,15 @@ compileModuleImpl(CompilerInstance &ImportingInstance,
SourceLocation ImportLoc,
diag::remark_module_build_done)
<< ModuleName;
- // Delete any remaining temporary files related to Instance, in case the
- // module generation thread crashed.
- Instance.clearOutputFiles(/*EraseFiles=*/true);
+ if (Crashed) {
+// Clear the ASTConsumer if it hasn't been already, in case it owns streams
+// that must be closed before clearing output files.
+Instance.setSema(nullptr);
+Instance.setASTConsumer(nullptr);
+
+// Delete any remaining temporary files related to Instance.
+Instance.clearOutputFiles(/*EraseFiles=*/true);
+ }
// If \p AllowPCMWithCompilerErrors is set return 'success' even if errors
// occurred.
diff --git a/clang/lib/Frontend/FrontendAction.cpp
b/clang/lib/Frontend/FrontendAction.cpp
index 81915e6330b03..ed3e314cc73bb 100644
--- a/clang/lib/Frontend/FrontendAction.cpp
+++ b/clang/lib/Frontend/FrontendAction.cpp
@@ -581,6 +581,7 @@ bool FrontendAction::BeginSourceFile(CompilerInstance &CI,
auto FailureCleanup = llvm::make_scope_exit([&]() {
if (HasBegunSourceFile)
CI.getDiagnosticClient().EndSourceFile();
+CI.setASTConsumer(nullptr);
CI.clearOutputFiles(/*EraseFiles=*/true);
CI.getLangOpts().setCompilingModule(LangOptions::CMK_None);
setCurrentInput(FrontendInputFile());
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 0287170 - [clang][deps] Include canonical invocation in ContextHash
Author: Ben Langmuir
Date: 2022-07-28T12:24:06-07:00
New Revision: 02871701400253a49de502a5fef770f92772f6bc
URL:
https://github.com/llvm/llvm-project/commit/02871701400253a49de502a5fef770f92772f6bc
DIFF:
https://github.com/llvm/llvm-project/commit/02871701400253a49de502a5fef770f92772f6bc.diff
LOG: [clang][deps] Include canonical invocation in ContextHash
The "strict context hash" is insufficient to identify module
dependencies during scanning, leading to different module build commands
being produced for a single module, and non-deterministically choosing
between them. This commit switches to hashing the canonicalized
`CompilerInvocation` of the module. By hashing the invocation we are
converting these from correctness issues to performance issues, and we
can then incrementally improve our ability to canonicalize
command-lines.
This change can cause a regression in the number of modules needed. Of
the 4 projects I tested, 3 had no regression, but 1, which was
clang+llvm itself, had a 66% regression in number of modules (4%
regression in total invocations). This is almost entirely due to
differences between -W options across targets. Of this, 25% of the
additional modules are system modules, which we could avoid if we
canonicalized -W options when -Wsystem-headers is not present --
unfortunately this is non-trivial due to some warnings being enabled in
system headers by default. The rest of the additional modules are mostly
real differences in potential warnings, reflecting incorrect behaviour
in the current scanner.
There were also a couple of differences due to `-DFOO`
`-fmodule-ignore-macro=FOO`, which I fixed here.
Since the output paths for the module depend on its context hash, we
hash the invocation before filling in outputs, and rely on the build
system to always return the same output paths for a given module.
Note: since the scanner itself uses an implicit modules build, there can
still be non-determinism, but it will now present as different
module+hashes rather than different command-lines for the same
module+hash.
Differential Revision: https://reviews.llvm.org/D129884
Added:
clang/test/ClangScanDeps/modules-context-hash-ignore-macros.c
clang/test/ClangScanDeps/modules-context-hash-module-map-path.c
clang/test/ClangScanDeps/modules-context-hash-outputs.c
clang/test/ClangScanDeps/modules-context-hash-warnings.c
Modified:
clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
Removed:
diff --git
a/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
b/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
index 05c9f56b4cf63..377dcd5a68fec 100644
--- a/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
+++ b/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
@@ -46,9 +46,11 @@ struct ModuleID {
/// or a header-name for C++20 header units.
std::string ModuleName;
- /// The context hash of a module represents the set of compiler options that
- /// may make one version of a module incompatible with another. This includes
- /// things like language mode, predefined macros, header search paths, etc...
+ /// The context hash of a module represents the compiler options that affect
+ /// the resulting command-line invocation.
+ ///
+ /// Modules with the same name and ContextHash but
diff erent invocations could
+ /// cause non-deterministic build results.
///
/// Modules with the same name but a
diff erent \c ContextHash should be
/// treated as separate modules for the purpose of a build.
diff --git a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
index 725bb2c318ac2..8d58a53a5971c 100644
--- a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
+++ b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
@@ -12,6 +12,7 @@
#include "clang/Frontend/CompilerInstance.h"
#include "clang/Lex/Preprocessor.h"
#include "clang/Tooling/DependencyScanning/DependencyScanningWorker.h"
+#include "llvm/Support/BLAKE3.h"
#include "llvm/Support/StringSaver.h"
using namespace clang;
@@ -77,6 +78,19 @@ CompilerInvocation
ModuleDepCollector::makeInvocationForModuleBuildWithoutPaths(
CI.getHeaderSearchOpts().ModuleCachePruneInterval = 7 * 24 * 60 * 60;
CI.getHeaderSearchOpts().ModuleCachePruneAfter = 31 * 24 * 60 * 60;
+ // Remove any macro definitions that are explicitly ignored.
+ if (!CI.getHeaderSearchOpts().ModulesIgnoreMacros.empty()) {
+llvm::erase_if(
+CI.getPreprocessorOpts().Macros,
+[&CI](const std::pair &Def) {
+ StringRef MacroDef = Def.first;
+ return CI.getHeaderSearchOpts().ModulesIgnoreMacros.conta
[clang] b4c6dc2 - [clang] Update code that assumes FileEntry::getName is absolute NFC
Author: Ben Langmuir
Date: 2022-08-01T14:48:37-07:00
New Revision: b4c6dc2e66370d94ff52075c2710a674d8e1e0f8
URL:
https://github.com/llvm/llvm-project/commit/b4c6dc2e66370d94ff52075c2710a674d8e1e0f8
DIFF:
https://github.com/llvm/llvm-project/commit/b4c6dc2e66370d94ff52075c2710a674d8e1e0f8.diff
LOG: [clang] Update code that assumes FileEntry::getName is absolute NFC
It's an accident that we started return asbolute paths from
FileEntry::getName for all relative paths. Prepare for getName to get
(closer to) return the requested path. Note: conceptually it might make
sense for the dependency scanner to allow relative paths and have the
DependencyConsumer decide if it wants to make them absolute, but we
currently document that it's absolute and I didn't want to change
behaviour here.
Differential Revision: https://reviews.llvm.org/D130934
Added:
Modified:
clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
Removed:
diff --git
a/clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
b/clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
index cb5caf1ca92a1..9757d99cd789b 100644
---
a/clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
+++
b/clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
@@ -156,11 +156,15 @@ groupReplacements(const TUReplacements &TUs, const
TUDiagnostics &TUDs,
const tooling::TranslationUnitDiagnostics *SourceTU,
const llvm::Optional BuildDir) {
// Use the file manager to deduplicate paths. FileEntries are
-// automatically canonicalized.
-auto PrevWorkingDir = SM.getFileManager().getFileSystemOpts().WorkingDir;
+// automatically canonicalized. Since relative paths can come from
diff erent
+// build directories, make them absolute immediately.
+SmallString<128> Path = R.getFilePath();
if (BuildDir)
- SM.getFileManager().getFileSystemOpts().WorkingDir =
std::move(*BuildDir);
-if (auto Entry = SM.getFileManager().getFile(R.getFilePath())) {
+ llvm::sys::fs::make_absolute(*BuildDir, Path);
+else
+ SM.getFileManager().makeAbsolutePath(Path);
+
+if (auto Entry = SM.getFileManager().getFile(Path)) {
if (SourceTU) {
auto &Replaces = DiagReplacements[*Entry];
auto It = Replaces.find(R);
@@ -171,11 +175,10 @@ groupReplacements(const TUReplacements &TUs, const
TUDiagnostics &TUDs,
return;
}
GroupedReplacements[*Entry].push_back(R);
-} else if (Warned.insert(R.getFilePath()).second) {
+} else if (Warned.insert(Path).second) {
errs() << "Described file '" << R.getFilePath()
<< "' doesn't exist. Ignoring...\n";
}
-SM.getFileManager().getFileSystemOpts().WorkingDir = PrevWorkingDir;
};
for (const auto &TU : TUs)
diff --git
a/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
b/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
index 377dcd5a68fec..f4f13a34b1746 100644
--- a/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
+++ b/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
@@ -233,6 +233,11 @@ class ModuleDepCollector final : public
DependencyCollector {
/// Checks whether the module is known as being prebuilt.
bool isPrebuiltModule(const Module *M);
+ /// Adds \p Path to \c FileDeps, making it absolute if necessary.
+ void addFileDep(StringRef Path);
+ /// Adds \p Path to \c MD.FileDeps, making it absolute if necessary.
+ void addFileDep(ModuleDeps &MD, StringRef Path);
+
/// Constructs a CompilerInvocation that can be used to build the given
/// module, excluding paths to discovered modular dependencies that are yet
to
/// be built.
diff --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
index 474808d888ec2..c84919138612a 100644
--- a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
+++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
@@ -28,8 +28,9 @@ namespace {
class DependencyConsumerForwarder : public DependencyFileGenerator {
public:
DependencyConsumerForwarder(std::unique_ptr Opts,
- DependencyConsumer &C)
- : DependencyFileGenerator(*Opts), Opts(std::move(Opts)), C(C) {}
+ StringRef WorkingDirectory, DependencyConsumer
&C)
+ : Depende
[clang] 98cf745 - [clang] Only modify FileEntryRef names that are externally remapped
Author: Ben Langmuir
Date: 2022-08-01T15:45:51-07:00
New Revision: 98cf745a032ea0d29fbddaa204760d4e823c237f
URL:
https://github.com/llvm/llvm-project/commit/98cf745a032ea0d29fbddaa204760d4e823c237f
DIFF:
https://github.com/llvm/llvm-project/commit/98cf745a032ea0d29fbddaa204760d4e823c237f.diff
LOG: [clang] Only modify FileEntryRef names that are externally remapped
As progress towards having FileEntryRef contain the requested name of
the file, this commit narrows the "remap" hack to only apply to paths
that were remapped to an external contents path by a VFS. That was
always the original intent of this code, and the fact it was making
relative paths absolute was an unintended side effect.
Differential Revision: https://reviews.llvm.org/D130935
Added:
Modified:
clang/lib/Basic/FileManager.cpp
clang/unittests/Basic/FileManagerTest.cpp
Removed:
diff --git a/clang/lib/Basic/FileManager.cpp b/clang/lib/Basic/FileManager.cpp
index b66780a1d1d1d..451c7c3b8a0dc 100644
--- a/clang/lib/Basic/FileManager.cpp
+++ b/clang/lib/Basic/FileManager.cpp
@@ -274,8 +274,8 @@ FileManager::getFileRef(StringRef Filename, bool openFile,
bool CacheFailure) {
if (!UFE)
UFE = new (FilesAlloc.Allocate()) FileEntry();
- if (Status.getName() == Filename) {
-// The name matches. Set the FileEntry.
+ if (!Status.ExposesExternalVFSPath || Status.getName() == Filename) {
+// Use the requested name. Set the FileEntry.
NamedFileEnt->second = FileEntryRef::MapValue(*UFE, DirInfo);
} else {
// Name mismatch. We need a redirect. First grab the actual entry we want
@@ -292,19 +292,7 @@ FileManager::getFileRef(StringRef Filename, bool openFile,
bool CacheFailure) {
// filesystems behave and confuses parts of clang expect to see the
// name-as-accessed on the \a FileEntryRef.
//
-// Further, it isn't *just* external names, but will also give back
absolute
-// paths when a relative path was requested - the check is comparing the
-// name from the status, which is passed an absolute path resolved from the
-// current working directory. `clang-apply-replacements` appears to depend
-// on this behaviour, though it's adjusting the working directory, which is
-// definitely not supported. Once that's fixed this hack should be able to
-// be narrowed to only when there's an externally mapped name given back.
-//
// A potential plan to remove this is as follows -
-// - Add API to determine if the name has been rewritten by the VFS.
-// - Fix `clang-apply-replacements` to pass down the absolute path rather
-// than changing the CWD. Narrow this hack down to just externally
-// mapped paths.
// - Expose the requested filename. One possibility would be to allow
// redirection-FileEntryRefs to be returned, rather than returning
// the pointed-at-FileEntryRef, and customizing `getName()` to look
diff --git a/clang/unittests/Basic/FileManagerTest.cpp
b/clang/unittests/Basic/FileManagerTest.cpp
index 535d4d9e09c67..6e60bba9b2a99 100644
--- a/clang/unittests/Basic/FileManagerTest.cpp
+++ b/clang/unittests/Basic/FileManagerTest.cpp
@@ -44,16 +44,16 @@ class FakeStatCache : public FileSystemStatCache {
}
}
-if (!StatPath)
- StatPath = Path;
-
auto fileType = IsFile ?
llvm::sys::fs::file_type::regular_file :
llvm::sys::fs::file_type::directory_file;
-llvm::vfs::Status Status(StatPath, llvm::sys::fs::UniqueID(1, INode),
+llvm::vfs::Status Status(StatPath ? StatPath : Path,
+ llvm::sys::fs::UniqueID(1, INode),
/*MTime*/{}, /*User*/0, /*Group*/0,
/*Size*/0, fileType,
llvm::sys::fs::perms::all_all);
+if (StatPath)
+ Status.ExposesExternalVFSPath = true;
StatCalls[Path] = Status;
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] 54110b8 - Fix use-after-free in clang-apply-replacements
Author: Ben Langmuir
Date: 2022-08-02T13:34:20-07:00
New Revision: 54110b8aa01073c428c636951511c2dc710c4a32
URL:
https://github.com/llvm/llvm-project/commit/54110b8aa01073c428c636951511c2dc710c4a32
DIFF:
https://github.com/llvm/llvm-project/commit/54110b8aa01073c428c636951511c2dc710c4a32.diff
LOG: Fix use-after-free in clang-apply-replacements
Accidentally introduced a dangling StringRef in b4c6dc2e6637.
Differential Revision: https://reviews.llvm.org/D131017
Added:
Modified:
clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
Removed:
diff --git
a/clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
b/clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
index 9757d99cd789b..b9e83aca0ab96 100644
---
a/clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
+++
b/clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
@@ -24,6 +24,7 @@
#include "clang/Tooling/ReplacementsYaml.h"
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/Optional.h"
+#include "llvm/ADT/StringSet.h"
#include "llvm/Support/FileSystem.h"
#include "llvm/Support/MemoryBuffer.h"
#include "llvm/Support/Path.h"
@@ -140,7 +141,7 @@ std::error_code collectReplacementsFromDirectory(
static llvm::DenseMap>
groupReplacements(const TUReplacements &TUs, const TUDiagnostics &TUDs,
const clang::SourceManager &SM) {
- std::set Warned;
+ llvm::StringSet<> Warned;
llvm::DenseMap>
GroupedReplacements;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 6a79e2f - [clang] Add FileEntryRef::getNameAsRequested()
Author: Ben Langmuir
Date: 2022-08-03T09:41:08-07:00
New Revision: 6a79e2ff1989b48f4a8ebf3ac51092eb8ad29e37
URL:
https://github.com/llvm/llvm-project/commit/6a79e2ff1989b48f4a8ebf3ac51092eb8ad29e37
DIFF:
https://github.com/llvm/llvm-project/commit/6a79e2ff1989b48f4a8ebf3ac51092eb8ad29e37.diff
LOG: [clang] Add FileEntryRef::getNameAsRequested()
As progress towards having FileManager::getFileRef() return the path
as-requested by default, return a FileEntryRef that can use
getNameAsRequested() to retrieve this path, with the ultimate goal that
this should be the behaviour of getName() and clients should explicitly
request the "external" name if they need to (see comment in
FileManager::getFileRef). For now, getName() continues to return the
external path by looking through the redirects.
For now, the new function is only used in unit tests.
Differential Revision: https://reviews.llvm.org/D131004
Added:
Modified:
clang/include/clang/Basic/FileEntry.h
clang/lib/Basic/FileManager.cpp
clang/unittests/Basic/FileEntryTest.cpp
clang/unittests/Basic/FileManagerTest.cpp
Removed:
diff --git a/clang/include/clang/Basic/FileEntry.h
b/clang/include/clang/Basic/FileEntry.h
index 8676604b48364..3ca07cb422b64 100644
--- a/clang/include/clang/Basic/FileEntry.h
+++ b/clang/include/clang/Basic/FileEntry.h
@@ -59,11 +59,21 @@ class FileEntry;
/// accessed by the FileManager's client.
class FileEntryRef {
public:
- StringRef getName() const { return ME->first(); }
+ /// The name of this FileEntry. If a VFS uses 'use-external-name', this is
+ /// the redirected name. See getRequestedName().
+ StringRef getName() const { return getBaseMapEntry().first(); }
+
+ /// The name of this FileEntry, as originally requested without applying any
+ /// remappings for VFS 'use-external-name'.
+ ///
+ /// FIXME: this should be the semantics of getName(). See comment in
+ /// FileManager::getFileRef().
+ StringRef getNameAsRequested() const { return ME->first(); }
+
const FileEntry &getFileEntry() const {
-return *ME->second->V.get();
+return *getBaseMapEntry().second->V.get();
}
- DirectoryEntryRef getDir() const { return *ME->second->Dir; }
+ DirectoryEntryRef getDir() const { return *getBaseMapEntry().second->Dir; }
inline off_t getSize() const;
inline unsigned getUID() const;
@@ -150,13 +160,20 @@ class FileEntryRef {
explicit FileEntryRef(const MapEntry &ME) : ME(&ME) {
assert(ME.second && "Expected payload");
assert(ME.second->V && "Expected non-null");
-assert(ME.second->V.is() && "Expected FileEntry");
}
/// Expose the underlying MapEntry to simplify packing in a PointerIntPair or
/// PointerUnion and allow construction in Optional.
const clang::FileEntryRef::MapEntry &getMapEntry() const { return *ME; }
+ /// Retrieve the base MapEntry after redirects.
+ const MapEntry &getBaseMapEntry() const {
+const MapEntry *ME = this->ME;
+while (const void *Next = ME->second->V.dyn_cast())
+ ME = static_cast(Next);
+return *ME;
+ }
+
private:
friend class FileMgr::MapEntryOptionalStorage;
struct optional_none_tag {};
diff --git a/clang/lib/Basic/FileManager.cpp b/clang/lib/Basic/FileManager.cpp
index 451c7c3b8a0dc..cb719762ec7c8 100644
--- a/clang/lib/Basic/FileManager.cpp
+++ b/clang/lib/Basic/FileManager.cpp
@@ -293,12 +293,8 @@ FileManager::getFileRef(StringRef Filename, bool openFile,
bool CacheFailure) {
// name-as-accessed on the \a FileEntryRef.
//
// A potential plan to remove this is as follows -
-// - Expose the requested filename. One possibility would be to allow
-// redirection-FileEntryRefs to be returned, rather than returning
-// the pointed-at-FileEntryRef, and customizing `getName()` to look
-// through the indirection.
// - Update callers such as `HeaderSearch::findUsableModuleForHeader()`
-// to explicitly use the requested filename rather than just using
+// to explicitly use the `getNameAsRequested()` rather than just using
// `getName()`.
// - Add a `FileManager::getExternalPath` API for explicitly getting the
// remapped external filename when there is one available. Adopt it in
@@ -329,9 +325,6 @@ FileManager::getFileRef(StringRef Filename, bool openFile,
bool CacheFailure) {
// Cache the redirection in the previously-inserted entry, still available
// in the tentative return value.
NamedFileEnt->second = FileEntryRef::MapValue(Redirection);
-
-// Fix the tentative return value.
-NamedFileEnt = &Redirection;
}
FileEntryRef ReturnedRef(*NamedFileEnt);
diff --git a/clang/unittests/Basic/FileEntryTest.cpp
b/clang/unittests/Ba
[clang] d038bb1 - [clang] Fix redirection behaviour for cached FileEntryRef
Author: Ben Langmuir
Date: 2022-08-05T12:23:38-07:00
New Revision: d038bb196c51dcf80cbe771f4229b4e227c6c5b6
URL:
https://github.com/llvm/llvm-project/commit/d038bb196c51dcf80cbe771f4229b4e227c6c5b6
DIFF:
https://github.com/llvm/llvm-project/commit/d038bb196c51dcf80cbe771f4229b4e227c6c5b6.diff
LOG: [clang] Fix redirection behaviour for cached FileEntryRef
In 6a79e2ff1989b we changed Filemanager::getEntryRef() to return the
redirecting FileEntryRef instead of looking through the redirection.
This commit fixes the case when looking up a cached file path to also
return the redirecting FileEntryRef. This mainly affects the behaviour
of calling getNameAsRequested() on the resulting entry ref.
Differential Revision: https://reviews.llvm.org/D131273
Added:
Modified:
clang/lib/Basic/FileManager.cpp
clang/unittests/Basic/FileManagerTest.cpp
Removed:
diff --git a/clang/lib/Basic/FileManager.cpp b/clang/lib/Basic/FileManager.cpp
index cb719762ec7c8..4ef25358ec82f 100644
--- a/clang/lib/Basic/FileManager.cpp
+++ b/clang/lib/Basic/FileManager.cpp
@@ -212,13 +212,7 @@ FileManager::getFileRef(StringRef Filename, bool openFile,
bool CacheFailure) {
if (!SeenFileInsertResult.first->second)
return llvm::errorCodeToError(
SeenFileInsertResult.first->second.getError());
-// Construct and return and FileEntryRef, unless it's a redirect to another
-// filename.
-FileEntryRef::MapValue Value = *SeenFileInsertResult.first->second;
-if (LLVM_LIKELY(Value.V.is()))
- return FileEntryRef(*SeenFileInsertResult.first);
-return FileEntryRef(*reinterpret_cast(
-Value.V.get()));
+return FileEntryRef(*SeenFileInsertResult.first);
}
// We've not seen this before. Fill it in.
diff --git a/clang/unittests/Basic/FileManagerTest.cpp
b/clang/unittests/Basic/FileManagerTest.cpp
index 419c497e95d68..6fe4a3d65d172 100644
--- a/clang/unittests/Basic/FileManagerTest.cpp
+++ b/clang/unittests/Basic/FileManagerTest.cpp
@@ -340,6 +340,7 @@ TEST_F(FileManagerTest, getFileRefEquality) {
auto F1Again = manager.getFileRef("dir/f1.cpp");
auto F1Also = manager.getFileRef("dir/f1-also.cpp");
auto F1Redirect = manager.getFileRef("dir/f1-redirect.cpp");
+ auto F1RedirectAgain = manager.getFileRef("dir/f1-redirect.cpp");
auto F2 = manager.getFileRef("dir/f2.cpp");
// Check Expected for error.
@@ -347,6 +348,7 @@ TEST_F(FileManagerTest, getFileRefEquality) {
ASSERT_FALSE(!F1Also);
ASSERT_FALSE(!F1Again);
ASSERT_FALSE(!F1Redirect);
+ ASSERT_FALSE(!F1RedirectAgain);
ASSERT_FALSE(!F2);
// Check names.
@@ -354,6 +356,7 @@ TEST_F(FileManagerTest, getFileRefEquality) {
EXPECT_EQ("dir/f1.cpp", F1Again->getName());
EXPECT_EQ("dir/f1-also.cpp", F1Also->getName());
EXPECT_EQ("dir/f1.cpp", F1Redirect->getName());
+ EXPECT_EQ("dir/f1.cpp", F1RedirectAgain->getName());
EXPECT_EQ("dir/f2.cpp", F2->getName());
EXPECT_EQ("dir/f1.cpp", F1->getNameAsRequested());
@@ -363,6 +366,7 @@ TEST_F(FileManagerTest, getFileRefEquality) {
EXPECT_EQ(&F1->getFileEntry(), *F1);
EXPECT_EQ(*F1, &F1->getFileEntry());
EXPECT_EQ(&F1->getFileEntry(), &F1Redirect->getFileEntry());
+ EXPECT_EQ(&F1->getFileEntry(), &F1RedirectAgain->getFileEntry());
EXPECT_NE(&F2->getFileEntry(), *F1);
EXPECT_NE(*F1, &F2->getFileEntry());
@@ -371,6 +375,7 @@ TEST_F(FileManagerTest, getFileRefEquality) {
EXPECT_EQ(*F1, *F1Again);
EXPECT_EQ(*F1, *F1Redirect);
EXPECT_EQ(*F1Also, *F1Redirect);
+ EXPECT_EQ(*F1, *F1RedirectAgain);
EXPECT_NE(*F2, *F1);
EXPECT_NE(*F2, *F1Also);
EXPECT_NE(*F2, *F1Again);
@@ -381,6 +386,7 @@ TEST_F(FileManagerTest, getFileRefEquality) {
EXPECT_FALSE(F1->isSameRef(*F1Redirect));
EXPECT_FALSE(F1->isSameRef(*F1Also));
EXPECT_FALSE(F1->isSameRef(*F2));
+ EXPECT_TRUE(F1Redirect->isSameRef(*F1RedirectAgain));
}
// getFile() Should return the same entry as getVirtualFile if the file
actually
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] fb89cc0 - [clang][modules] Don't depend on sharing FileManager during module build
Author: Ben Langmuir
Date: 2022-08-05T12:24:40-07:00
New Revision: fb89cc0ddbd9a588ee15148451e7d0bcbc1ef411
URL:
https://github.com/llvm/llvm-project/commit/fb89cc0ddbd9a588ee15148451e7d0bcbc1ef411
DIFF:
https://github.com/llvm/llvm-project/commit/fb89cc0ddbd9a588ee15148451e7d0bcbc1ef411.diff
LOG: [clang][modules] Don't depend on sharing FileManager during module build
Sharing the FileManager between the importer and the module build should
only be an optimization. Add a cc1 option -fno-modules-share-filemanager
to allow us to test this. Fix the path to modulemap files, which
previously depended on the shared FileManager when using path mapped to
an external file in a VFS.
Differential Revision: https://reviews.llvm.org/D131076
Added:
clang/test/Modules/submodule-in-private-mmap-vfs.m
Modified:
clang/include/clang/Driver/Options.td
clang/include/clang/Frontend/FrontendOptions.h
clang/lib/Frontend/CompilerInstance.cpp
clang/test/ClangScanDeps/modulemap-via-vfs.m
clang/test/VFS/module-import.m
Removed:
diff --git a/clang/include/clang/Driver/Options.td
b/clang/include/clang/Driver/Options.td
index 83dbcfd55d962..eaf8406d306f3 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -6032,6 +6032,9 @@ def fallow_pch_with_
diff erent_modules_cache_path :
Flag<["-"], "fallow-pch-with-
diff erent-modules-cache-path">,
HelpText<"Accept a PCH file that was created with a
diff erent modules cache path">,
MarshallingInfoFlag>;
+def fno_modules_share_filemanager : Flag<["-"],
"fno-modules-share-filemanager">,
+ HelpText<"Disable sharing the FileManager when building a module
implicitly">,
+ MarshallingInfoNegativeFlag>;
def dump_deserialized_pch_decls : Flag<["-"], "dump-deserialized-decls">,
HelpText<"Dump declarations that are deserialized from PCH, for testing">,
MarshallingInfoFlag>;
diff --git a/clang/include/clang/Frontend/FrontendOptions.h
b/clang/include/clang/Frontend/FrontendOptions.h
index 0c9c2489e4486..56e704d8a52f4 100644
--- a/clang/include/clang/Frontend/FrontendOptions.h
+++ b/clang/include/clang/Frontend/FrontendOptions.h
@@ -356,6 +356,9 @@ class FrontendOptions {
/// Output (and read) PCM files regardless of compiler errors.
unsigned AllowPCMWithCompilerErrors : 1;
+ /// Whether to share the FileManager when building modules.
+ unsigned ModulesShareFileManager : 1;
+
CodeCompleteOptions CodeCompleteOpts;
/// Specifies the output format of the AST.
@@ -523,7 +526,7 @@ class FrontendOptions {
BuildingImplicitModuleUsesLock(true), ModulesEmbedAllFiles(false),
IncludeTimestamps(true), UseTemporary(true),
OutputPathIndependentPCM(false), AllowPCMWithCompilerErrors(false),
-TimeTraceGranularity(500) {}
+ModulesShareFileManager(true), TimeTraceGranularity(500) {}
/// getInputKindForExtension - Return the appropriate input kind for a file
/// extension. For example, "c" would return Language::C.
diff --git a/clang/lib/Frontend/CompilerInstance.cpp
b/clang/lib/Frontend/CompilerInstance.cpp
index 2cd7efd862eca..51c7bc237a502 100644
--- a/clang/lib/Frontend/CompilerInstance.cpp
+++ b/clang/lib/Frontend/CompilerInstance.cpp
@@ -1213,11 +1213,16 @@ compileModuleImpl(CompilerInstance &ImportingInstance,
SourceLocation ImportLoc,
ImportingInstance.getDiagnosticClient()),
/*ShouldOwnClient=*/true);
- // Note that this module is part of the module build stack, so that we
- // can detect cycles in the module graph.
- Instance.setFileManager(&ImportingInstance.getFileManager());
+ if (FrontendOpts.ModulesShareFileManager) {
+Instance.setFileManager(&ImportingInstance.getFileManager());
+ } else {
+Instance.createFileManager(&ImportingInstance.getVirtualFileSystem());
+ }
Instance.createSourceManager(Instance.getFileManager());
SourceManager &SourceMgr = Instance.getSourceManager();
+
+ // Note that this module is part of the module build stack, so that we
+ // can detect cycles in the module graph.
SourceMgr.setModuleBuildStack(
ImportingInstance.getSourceManager().getModuleBuildStack());
SourceMgr.pushModuleBuildStack(ModuleName,
@@ -1303,12 +1308,16 @@ static bool compileModule(CompilerInstance
&ImportingInstance,
ModuleMapFile, ImportingInstance.getFileManager()))
ModuleMapFile = PublicMMFile;
+// FIXME: Update header search to keep FileEntryRef rather than rely on
+// getLastRef().
+StringRef ModuleMapFilePath =
+ModuleMapFile->getLastRef().getNameAsRequested();
+
// Use the module map where this module resides.
[clang] 33af4b2 - [clang][deps] Stop sharing FileManager across module builds in scanner
Author: Ben Langmuir
Date: 2022-08-08T12:13:54-07:00
New Revision: 33af4b22f8ea5dc8340002833802be95ae9f83a1
URL:
https://github.com/llvm/llvm-project/commit/33af4b22f8ea5dc8340002833802be95ae9f83a1
DIFF:
https://github.com/llvm/llvm-project/commit/33af4b22f8ea5dc8340002833802be95ae9f83a1.diff
LOG: [clang][deps] Stop sharing FileManager across module builds in scanner
Sharing the FileManager across implicit module builds currently leaks
paths looked up in an importer into the built module itself. This can
cause non-deterministic results across scans. It is especially bad for
modules since the path can be saved into the pcm file itself, leading to
stateful behaviour if the cache is shared.
This should not impact the number of real filesystem accesses in the
scanner, since it is already caching in the
DependencyScanningWorkerFilesystem.
Note: this change does not affect whether or not the FileManager is
shared across TUs in the scanner, which is a separate issue.
Differential Revision: https://reviews.llvm.org/D131412
Added:
clang/test/ClangScanDeps/modules-file-path-isolation.c
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 c84919138612a..e9aa78c5eb71d 100644
--- a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
+++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
@@ -170,6 +170,7 @@ class DependencyScanningAction : public tooling::ToolAction
{
ScanInstance.getFrontendOpts().GenerateGlobalModuleIndex = false;
ScanInstance.getFrontendOpts().UseGlobalModuleIndex = false;
+ScanInstance.getFrontendOpts().ModulesShareFileManager = false;
FileMgr->getFileSystemOpts().WorkingDir = std::string(WorkingDirectory);
ScanInstance.setFileManager(FileMgr);
diff --git a/clang/test/ClangScanDeps/modules-file-path-isolation.c
b/clang/test/ClangScanDeps/modules-file-path-isolation.c
new file mode 100644
index 0..e57157f2adbb3
--- /dev/null
+++ b/clang/test/ClangScanDeps/modules-file-path-isolation.c
@@ -0,0 +1,44 @@
+// Ensure that the spelling of a path seen outside a module (e.g. header via
+// symlink) does not leak into the compilation of that module unnecessarily.
+// Note: the spelling of the modulemap path still depends on the includer,
since
+// that is the only source of information about it.
+
+// REQUIRES: shell
+
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: sed -e "s|DIR|%/t|g" %t/cdb.json.in > %t/cdb.json
+// RUN: ln -s A.h %t/Z.h
+
+// RUN: clang-scan-deps -compilation-database %t/cdb.json -j 1 -format
experimental-full \
+// RUN: -mode preprocess-dependency-directives -generate-modules-path-args >
%t/output
+// RUN: FileCheck %s < %t/output
+
+// CHECK: "modules": [
+// CHECK-NEXT: {
+// CHECK: "file-deps": [
+// CHECK-NEXT: "{{.*}}A.h",
+// CHECK-NEXT: "{{.*}}module.modulemap"
+// CHECK-NEXT: ],
+// CHECK-NEXT: "name": "A"
+// CHECK-NEXT: }
+
+//--- cdb.json.in
+[{
+ "directory": "DIR",
+ "command": "clang -fsyntax-only DIR/tu.c -fmodules
-fmodules-cache-path=DIR/module-cache -fimplicit-modules
-fimplicit-module-maps",
+ "file": "DIR/tu.c"
+}]
+
+//--- module.modulemap
+module A { header "A.h" }
+module B { header "B.h" }
+module C { header "C.h" }
+
+//--- A.h
+
+//--- B.h
+#include "Z.h"
+
+//--- tu.c
+#include "B.h"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 6626f6f - [clang][deps] Override dependency and serialized diag files for modules
Author: Ben Langmuir
Date: 2022-07-12T08:19:52-07:00
New Revision: 6626f6fec3d37b78b628b858bdadbbb8301e1a2f
URL:
https://github.com/llvm/llvm-project/commit/6626f6fec3d37b78b628b858bdadbbb8301e1a2f
DIFF:
https://github.com/llvm/llvm-project/commit/6626f6fec3d37b78b628b858bdadbbb8301e1a2f.diff
LOG: [clang][deps] Override dependency and serialized diag files for modules
When building modules, override secondary outputs (dependency file,
dependency targets, serialized diagnostic file) in addition to the pcm
file path. This avoids inheriting per-TU command-line options that
cause non-determinism in the results (non-deterministic command-line for
the module build, non-determinism in which TU's .diag and .d files will
contain the module outputs). In clang-scan-deps we infer whether to
generate dependency or serialized diagnostic files based on an original
command-line. In a real build system this should be modeled explicitly.
Differential Revision: https://reviews.llvm.org/D129389
Added:
clang/test/ClangScanDeps/generate-modules-path-args.c
Modified:
clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
clang/test/ClangScanDeps/Inputs/removed-args/cdb.json.template
clang/test/ClangScanDeps/removed-args.c
clang/tools/clang-scan-deps/ClangScanDeps.cpp
Removed:
clang/test/ClangScanDeps/Inputs/preserved-args/cdb.json.template
clang/test/ClangScanDeps/Inputs/preserved-args/mod.h
clang/test/ClangScanDeps/Inputs/preserved-args/module.modulemap
clang/test/ClangScanDeps/Inputs/preserved-args/tu.c
clang/test/ClangScanDeps/preserved-args.c
diff --git
a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
index 3bb44e44187ba..a85d333ba6b18 100644
--- a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
+++ b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
@@ -47,12 +47,12 @@ struct FullDependencies {
/// Get the full command line.
///
- /// \param LookupPCMPath This function is called to fill in "-fmodule-file="
- /// arguments and the "-o" argument. It needs to return
- /// a path for where the PCM for the given module is to
- /// be located.
- std::vector
- getCommandLine(std::function LookupPCMPath) const;
+ /// \param LookupModuleOutput This function is called to fill in
+ /// "-fmodule-file=", "-o" and other output
+ /// arguments for dependencies.
+ std::vector getCommandLine(
+ llvm::function_ref
+ LookupOutput) const;
/// Get the full command line, excluding -fmodule-file=" arguments.
std::vector getCommandLineWithoutModulePaths() const;
diff --git
a/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
b/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
index e0a4d6a554eb2..cb68df8314da1 100644
--- a/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
+++ b/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
@@ -65,6 +65,19 @@ struct ModuleIDHasher {
}
};
+/// An output from a module compilation, such as the path of the module file.
+enum class ModuleOutputKind {
+ /// The module file (.pcm). Required.
+ ModuleFile,
+ /// The path of the dependency file (.d), if any.
+ DependencyFile,
+ /// The null-separated list of names to use as the targets in the dependency
+ /// file, if any.
+ DependencyTargets,
+ /// The path of the serialized diagnostic file (.dia), if any.
+ DiagnosticSerializationFile,
+};
+
struct ModuleDeps {
/// The identifier of the module.
ModuleID ID;
@@ -104,17 +117,25 @@ struct ModuleDeps {
// the primary TU.
bool ImportedByMainFile = false;
+ /// Whether the TU had a dependency file. The path in \c BuildInvocation is
+ /// cleared to avoid leaking the specific path from the TU into the module.
+ bool HadDependencyFile = false;
+
+ /// Whether the TU had serialized diagnostics. The path in \c BuildInvocation
+ /// is cleared to avoid leaking the specific path from the TU into the
module.
+ bool HadSerializedDiagnostics = false;
+
/// Compiler invocation that can be used to build this module (without
paths).
CompilerInvocation BuildInvocation;
/// Gets the canonical command line suitable for passing to clang.
///
- /// \param LookupPCMPath This function is called to fill in "-fmodule-file="
- /// arguments and the "-o" argument. It needs to ret
[clang] 3ce78cb - [clang][deps] Fix handling of -MT in module command-line
Author: Ben Langmuir
Date: 2022-07-13T13:36:15-07:00
New Revision: 3ce78cbd2392d7c98f97d1d424cd7ff582fc28f8
URL:
https://github.com/llvm/llvm-project/commit/3ce78cbd2392d7c98f97d1d424cd7ff582fc28f8
DIFF:
https://github.com/llvm/llvm-project/commit/3ce78cbd2392d7c98f97d1d424cd7ff582fc28f8.diff
LOG: [clang][deps] Fix handling of -MT in module command-line
Follow-up to 6626f6fec3d3, this fixes the handling of -MT
* If no targets are provided, we need to invent one since cc1 expects
the driver to have handled it. The default is to use -o, quoting as
necessary for a make target.
* Fix the splitting for empty string, which was incorrectly treated as
{""} instead of {}.
* Add a way to test this behaviour in clang-scan-deps.
Differential Revision: https://reviews.llvm.org/D129607
Added:
clang/include/clang/Basic/MakeSupport.h
clang/lib/Basic/MakeSupport.cpp
Modified:
clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
clang/lib/Basic/CMakeLists.txt
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
clang/test/ClangScanDeps/generate-modules-path-args.c
clang/tools/clang-scan-deps/ClangScanDeps.cpp
Removed:
diff --git a/clang/include/clang/Basic/MakeSupport.h
b/clang/include/clang/Basic/MakeSupport.h
new file mode 100644
index 0..c663014ba7bcf
--- /dev/null
+++ b/clang/include/clang/Basic/MakeSupport.h
@@ -0,0 +1,23 @@
+//===- MakeSupport.h - Make Utilities ---*- C++
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_BASIC_MAKESUPPORT_H
+#define LLVM_CLANG_BASIC_MAKESUPPORT_H
+
+#include "clang/Basic/LLVM.h"
+#include "llvm/ADT/StringRef.h"
+
+namespace clang {
+
+/// Quote target names for inclusion in GNU Make dependency files.
+/// Only the characters '$', '#', ' ', '\t' are quoted.
+void quoteMakeTarget(StringRef Target, SmallVectorImpl &Res);
+
+} // namespace clang
+
+#endif // LLVM_CLANG_BASIC_MAKESUPPORT_H
diff --git
a/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
b/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
index cb68df8314da1..05c9f56b4cf63 100644
--- a/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
+++ b/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
@@ -72,7 +72,7 @@ enum class ModuleOutputKind {
/// The path of the dependency file (.d), if any.
DependencyFile,
/// The null-separated list of names to use as the targets in the dependency
- /// file, if any.
+ /// file, if any. Defaults to the value of \c ModuleFile, as in the driver.
DependencyTargets,
/// The path of the serialized diagnostic file (.dia), if any.
DiagnosticSerializationFile,
diff --git a/clang/lib/Basic/CMakeLists.txt b/clang/lib/Basic/CMakeLists.txt
index c815b571bc9c0..89dd33260b29c 100644
--- a/clang/lib/Basic/CMakeLists.txt
+++ b/clang/lib/Basic/CMakeLists.txt
@@ -54,6 +54,7 @@ add_clang_library(clangBasic
IdentifierTable.cpp
LangOptions.cpp
LangStandards.cpp
+ MakeSupport.cpp
Module.cpp
ObjCRuntime.cpp
OpenCLOptions.cpp
diff --git a/clang/lib/Basic/MakeSupport.cpp b/clang/lib/Basic/MakeSupport.cpp
new file mode 100644
index 0..37838f7bbc7bc
--- /dev/null
+++ b/clang/lib/Basic/MakeSupport.cpp
@@ -0,0 +1,35 @@
+//===-- MakeSuport.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/Basic/MakeSupport.h"
+
+void clang::quoteMakeTarget(StringRef Target, SmallVectorImpl &Res) {
+ for (unsigned i = 0, e = Target.size(); i != e; ++i) {
+switch (Target[i]) {
+case ' ':
+case '\t':
+ // Escape the preceding backslashes
+ for (int j = i - 1; j >= 0 && Target[j] == '\\'; --j)
+Res.push_back('\\');
+
+ // Escape the space/tab
+ Res.push_back('\\');
+ break;
+case '$':
+ Res.push_back('$');
+ break;
+case '#':
+ Res.push_back('\\');
+ break;
+default:
+ break;
+}
+
+Res.push_back(Target[i]);
+ }
+}
\ No newline at end of file
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp
b/clang/lib/Driver/To
[clang] [clang][modules] giving the __stddef_ headers their own modules can cause redeclaration errors with -fbuiltin-headers-in-system-modules (PR #84127)
@@ -301,10 +301,9 @@ bool Module::directlyUses(const Module *Requested) {
if (Requested->isSubModuleOf(Use))
return true;
- // Anyone is allowed to use our builtin stdarg.h and stddef.h and their
- // accompanying modules.
- if (Requested->getTopLevelModuleName() == "_Builtin_stdarg" ||
- Requested->getTopLevelModuleName() == "_Builtin_stddef")
+ // Anyone is allowed to use our builtin stddef.h and its accompanying
modules.
+ if (Requested->fullModuleNameIs({"_Builtin_stddef", "max_align_t"}) ||
+ Requested->fullModuleNameIs({"_Builtin_stddef_wint_t"}))
benlangmuir wrote:
Why was stdarg removed?
https://github.com/llvm/llvm-project/pull/84127
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] giving the __stddef_ headers their own modules can cause redeclaration errors with -fbuiltin-headers-in-system-modules (PR #84127)
benlangmuir wrote: >> I'm not excited by the complexity we are moving toward with the builtin >> headers. But I don't have any alternatives. > When -fbuiltin-headers-in-system-modules goes away, things become simpler > than they were since modules were introduced. Even for the case with `-fbuiltin-headers-in-system-modules` things aren't any more complex than they were before https://reviews.llvm.org/D159064 except for the fundamental complexity of having two modes to build in, and hopefully -fbuiltin-headers-in-system-modules goes away. https://github.com/llvm/llvm-project/pull/84127 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] giving the __stddef_ headers their own modules can cause redeclaration errors with -fbuiltin-headers-in-system-modules (PR #84127)
@@ -301,10 +301,9 @@ bool Module::directlyUses(const Module *Requested) {
if (Requested->isSubModuleOf(Use))
return true;
- // Anyone is allowed to use our builtin stdarg.h and stddef.h and their
- // accompanying modules.
- if (Requested->getTopLevelModuleName() == "_Builtin_stdarg" ||
- Requested->getTopLevelModuleName() == "_Builtin_stddef")
+ // Anyone is allowed to use our builtin stddef.h and its accompanying
modules.
+ if (Requested->fullModuleNameIs({"_Builtin_stddef", "max_align_t"}) ||
+ Requested->fullModuleNameIs({"_Builtin_stddef_wint_t"}))
benlangmuir wrote:
Are we not considering the include of `stdarg.h` a use of `_Builtin_stdarg`
here if it doesn't use the modular headers from `_Builtin_stdarg`?
https://github.com/llvm/llvm-project/pull/84127
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] giving the __stddef_ headers their own modules can cause redeclaration errors with -fbuiltin-headers-in-system-modules (PR #84127)
@@ -301,10 +301,9 @@ bool Module::directlyUses(const Module *Requested) {
if (Requested->isSubModuleOf(Use))
return true;
- // Anyone is allowed to use our builtin stdarg.h and stddef.h and their
- // accompanying modules.
- if (Requested->getTopLevelModuleName() == "_Builtin_stdarg" ||
- Requested->getTopLevelModuleName() == "_Builtin_stddef")
+ // Anyone is allowed to use our builtin stddef.h and its accompanying
modules.
+ if (Requested->fullModuleNameIs({"_Builtin_stddef", "max_align_t"}) ||
+ Requested->fullModuleNameIs({"_Builtin_stddef_wint_t"}))
benlangmuir wrote:
Ah, got it. Thanks for explaining.
https://github.com/llvm/llvm-project/pull/84127
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] giving the __stddef_ headers their own modules can cause redeclaration errors with -fbuiltin-headers-in-system-modules (PR #84127)
https://github.com/benlangmuir approved this pull request. https://github.com/llvm/llvm-project/pull/84127 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][deps] Lazy dependency directives (PR #86347)
https://github.com/benlangmuir approved this pull request. https://github.com/llvm/llvm-project/pull/86347 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] Avoid calling expensive `SourceManager::translateFile()` (PR #86216)
@@ -1337,9 +1337,24 @@ static bool compileModule(CompilerInstance
&ImportingInstance,
// Get or create the module map that we'll use to build this module.
ModuleMap &ModMap
= ImportingInstance.getPreprocessor().getHeaderSearchInfo().getModuleMap();
+ SourceManager &SourceMgr = ImportingInstance.getSourceManager();
bool Result;
- if (OptionalFileEntryRef ModuleMapFile =
- ModMap.getContainingModuleMapFile(Module)) {
+ if (FileID ModuleMapFID = ModMap.getContainingModuleMapFileID(Module);
+ ModuleMapFID.isValid()) {
+// We want to use the top-level module map. If we don't, the compiling
benlangmuir wrote:
What is the connection between this change and the rest of the commit?
https://github.com/llvm/llvm-project/pull/86216
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] Avoid calling expensive `SourceManager::translateFile()` (PR #86216)
@@ -71,6 +71,7 @@
// CHECK-NEXT: "context-hash": "{{.*}}",
// CHECK-NEXT: "file-deps": [
// CHECK-NEXT: "[[PREFIX]]/first/module.modulemap",
+// CHECK-NEXT: "[[PREFIX]]/second/module.modulemap",
benlangmuir wrote:
Why did this change?
https://github.com/llvm/llvm-project/pull/86216
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] Avoid calling expensive `SourceManager::translateFile()` (PR #86216)
https://github.com/benlangmuir approved this pull request. Thanks for explaining; LGTM https://github.com/llvm/llvm-project/pull/86216 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Move state out of `PreprocessorOptions` (1/n) (PR #86358)
@@ -34,12 +34,17 @@ class InitOnlyAction : public FrontendAction {
/// Preprocessor-based frontend action that also loads PCH files.
class ReadPCHAndPreprocessAction : public FrontendAction {
+ llvm::function_ref OnCI;
benlangmuir wrote:
What does `OnCI` mean? Is there a clearer name we could choose and/or could we
document the role of this callback?
Also, how do we know function_ref is safe here?
https://github.com/llvm/llvm-project/pull/86358
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Move state out of `PreprocessorOptions` (1/n) (PR #86358)
@@ -736,6 +736,19 @@ class Preprocessor {
State ConditionalStackState = Off;
} PreambleConditionalStack;
+ /// Function for getting the dependency preprocessor directives of a file.
+ ///
+ /// These are directives derived from a special form of lexing where the
+ /// source input is scanned for the preprocessor directives that might have
an
+ /// effect on the dependencies for a compilation unit.
+ ///
+ /// Enables a client to cache the directives for a file and provide them
+ /// across multiple compiler invocations.
+ /// FIXME: Allow returning an error.
+ using DependencyDirectivesFn = std::functionhttps://github.com/llvm/llvm-project/pull/86358
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Move state out of `PreprocessorOptions` (1/n) (PR #86358)
@@ -34,12 +34,17 @@ class InitOnlyAction : public FrontendAction {
/// Preprocessor-based frontend action that also loads PCH files.
class ReadPCHAndPreprocessAction : public FrontendAction {
+ llvm::function_ref OnCI;
benlangmuir wrote:
My concern is that nothing prevents using this frontend action from some other
code where it wouldn't be safe. Maybe we can make it a unique_function?
https://github.com/llvm/llvm-project/pull/86358
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Move state out of `PreprocessorOptions` (1/n) (PR #86358)
https://github.com/benlangmuir approved this pull request. https://github.com/llvm/llvm-project/pull/86358 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Move state out of `PreprocessorOptions` (2/n) (PR #87099)
https://github.com/benlangmuir approved this pull request. https://github.com/llvm/llvm-project/pull/87099 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang][lex] Always pass suggested module to `InclusionDirective()` callback (PR #81061)
https://github.com/benlangmuir edited https://github.com/llvm/llvm-project/pull/81061 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang][lex] Always pass suggested module to `InclusionDirective()` callback (PR #81061)
@@ -2265,7 +2265,6 @@ Preprocessor::ImportAction
Preprocessor::HandleHeaderIncludeOrImport(
UsableHeaderUnit = true;
else if (!IsImportDecl) {
// This is a Header Unit that we do not include-translate
- SuggestedModule = ModuleMap::KnownHeader();
SM = nullptr;
benlangmuir wrote:
Maybe we should rename `SM` to reflect the difference in behaviour?
https://github.com/llvm/llvm-project/pull/81061
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang][lex] Always pass suggested module to `InclusionDirective()` callback (PR #81061)
https://github.com/benlangmuir commented: It would be good to update your commit message to talk about the benefits of this change: the callback now provides more information about the included header and models the associated module accurately regardless of whether it is translated to an import. https://github.com/llvm/llvm-project/pull/81061 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang][lex] Always pass suggested module to `InclusionDirective()` callback (PR #81061)
@@ -127,8 +127,12 @@ class PPCallbacks {
/// \param RelativePath The path relative to SearchPath, at which the include
/// file was found. This is equal to FileName except for framework includes.
///
- /// \param Imported The module, whenever an inclusion directive was
- /// automatically turned into a module import or null otherwise.
+ /// \param SuggestedModule The module, whenever an inclusion directive was
+ /// considered to be automatically turned into a module import, or null
benlangmuir wrote:
I think this comment should be updated since it's no longer always a "module
import".
https://github.com/llvm/llvm-project/pull/81061
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang][lex] Always pass suggested module to `InclusionDirective()` callback (PR #81061)
@@ -127,8 +127,12 @@ class PPCallbacks {
/// \param RelativePath The path relative to SearchPath, at which the include
/// file was found. This is equal to FileName except for framework includes.
///
- /// \param Imported The module, whenever an inclusion directive was
- /// automatically turned into a module import or null otherwise.
+ /// \param SuggestedModule The module, whenever an inclusion directive was
+ /// considered to be automatically turned into a module import, or null
benlangmuir wrote:
"considered to be" is ambiguous:
https://github.com/llvm/llvm-project/pull/81061
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang][lex] Always pass suggested module to `InclusionDirective()` callback (PR #81061)
https://github.com/benlangmuir deleted https://github.com/llvm/llvm-project/pull/81061 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang][lex] Always pass suggested module to `InclusionDirective()` callback (PR #81061)
@@ -127,8 +127,12 @@ class PPCallbacks {
/// \param RelativePath The path relative to SearchPath, at which the include
/// file was found. This is equal to FileName except for framework includes.
///
- /// \param Imported The module, whenever an inclusion directive was
- /// automatically turned into a module import or null otherwise.
+ /// \param SuggestedModule The module, whenever an inclusion directive was
+ /// considered to be automatically turned into a module import, or null
benlangmuir wrote:
"was considered to be" most commonly it means you are describing the result of
a consideration -- "Clang is considered to be a good compiler" -- rather than
simply saying that something was considered but without reflecting the final
judgment. It's ambiguous here.
Suggestion: "The module suggested for this header, if any. See \p
ModuleImported for whether this include was translated into a module imported"
https://github.com/llvm/llvm-project/pull/81061
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang][lex] Always pass suggested module to `InclusionDirective()` callback (PR #81061)
https://github.com/benlangmuir approved this pull request. https://github.com/llvm/llvm-project/pull/81061 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][deps] Fix dependency scanning with -working-directory (PR #84525)
https://github.com/benlangmuir created
https://github.com/llvm/llvm-project/pull/84525
Stop overriding -working-directory to CWD during argument parsing, which should
no longer necessary after we set the VFS working directory, and set FSOpts
correctly after parsing arguments so that working-directory behaves correctly.
>From e214b51d85fd203a6ec8be3d5aef6b3aa3d8d741 Mon Sep 17 00:00:00 2001
From: Ben Langmuir
Date: Fri, 8 Mar 2024 09:53:42 -0800
Subject: [PATCH] [clang][deps] Fix dependency scanning with -working-directory
Stop overriding -working-directory to CWD during argument parsing, which
should no longer necessary after we set the VFS working directory, and
set FSOpts correctly after parsing arguments so that working-directory
behaves correctly.
---
.../DependencyScanningWorker.cpp | 8 +++--
.../ClangScanDeps/working-directory-option.c | 30 +++
2 files changed, 35 insertions(+), 3 deletions(-)
create mode 100644 clang/test/ClangScanDeps/working-directory-option.c
diff --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
index 2b882f8a5e0793..f7c302854a2479 100644
--- a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
+++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
@@ -342,6 +342,9 @@ class DependencyScanningAction : public tooling::ToolAction
{
ScanInstance.getHeaderSearchOpts().ModulesIncludeVFSUsage =
any(OptimizeArgs & ScanningOptimizations::VFS);
+// FileMgr was created before option parsing; set its FileSystemOptions
now.
+FileMgr->getFileSystemOpts() = ScanInstance.getFileSystemOpts();
+
ScanInstance.setFileManager(FileMgr);
// Support for virtual file system overlays.
FileMgr->setVirtualFileSystem(createVFSFromCompilerInvocation(
@@ -624,9 +627,8 @@ bool DependencyScanningWorker::computeDependencies(
ModifiedCommandLine ? *ModifiedCommandLine : CommandLine;
auto &FinalFS = ModifiedFS ? ModifiedFS : BaseFS;
- FileSystemOptions FSOpts;
- FSOpts.WorkingDir = WorkingDirectory.str();
- auto FileMgr = llvm::makeIntrusiveRefCnt(FSOpts, FinalFS);
+ auto FileMgr =
+ llvm::makeIntrusiveRefCnt(FileSystemOptions{}, FinalFS);
std::vector FinalCCommandLine(FinalCommandLine.size(),
nullptr);
llvm::transform(FinalCommandLine, FinalCCommandLine.begin(),
diff --git a/clang/test/ClangScanDeps/working-directory-option.c
b/clang/test/ClangScanDeps/working-directory-option.c
new file mode 100644
index 00..d57497d405d30f
--- /dev/null
+++ b/clang/test/ClangScanDeps/working-directory-option.c
@@ -0,0 +1,30 @@
+// Test that -working-directory works even when it differs from the working
+// directory of the filesystem.
+
+// RUN: rm -rf %t
+// RUN: mkdir -p %t/other
+// RUN: split-file %s %t
+// RUN: sed -e "s|DIR|%/t|g" %t/cdb.json.template > %t/cdb.json
+
+// RUN: clang-scan-deps -compilation-database %t/cdb.json -format
experimental-full \
+// RUN: > %t/deps.json
+
+// RUN: cat %t/deps.json | sed 's:\?:/:g' | FileCheck %s -DPREFIX=%/t
+
+// CHECK: "file-deps": [
+// CHECK-NEXT: "[[PREFIX]]/cwd/t.c"
+// CHECK-NEXT: "[[PREFIX]]/cwd/relative/h1.h"
+// CHECK-NEXT: ]
+// CHECK-NEXT: "input-file": "[[PREFIX]]/cwd/t.c"
+
+//--- cdb.json.template
+[{
+ "directory": "DIR/other",
+ "command": "clang -c t.c -I relative -working-directory DIR/cwd",
+ "file": "DIR/cwd/t.c"
+}]
+
+//--- cwd/relative/h1.h
+
+//--- cwd/t.c
+#include "h1.h"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][deps] Fix dependency scanning with -working-directory (PR #84525)
benlangmuir wrote: > I can see a situation where we ask FileManager about the same relative path > before and after setting the parsed FileSystemOptions. The second call would > blindly return the cached result, effectively ignoring -working-directory for > that file. The driver calls `VFS->setCurrentWorkingDirectory` using the value of `-working-directory`. So in general relative paths will be resolved consistently with `-working-directory` if they're seen before configuring the `FileManager`. Now, a fair question is why are we changing both the VFS working directory *and* making paths absolute in the FileManager for `-working-directory`, which seems redundant, but I'm not looking to change that behaviour -- this PR should make scanning behave more consistently with compilation. To be clear this situation with mismatched relative paths would **currently** be broken, because the way we were setting `FSOpts.WorkingDir` was not using the value of `-working-directory` before this change. https://github.com/llvm/llvm-project/pull/84525 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][deps] Fix dependency scanning with -working-directory (PR #84525)
benlangmuir wrote: I don't think there's a difference we can test for here -- the VFS WD shouldn't be modified after the driver sets it and before the FM is used here, so it would be identical to `-working-directory` during the critical window. But I agree recreating the FileManager is probably a better solution since it defines away this problem. I'll look at doing it that way. https://github.com/llvm/llvm-project/pull/84525 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][deps] Fix dependency scanning with -working-directory (PR #84525)
https://github.com/benlangmuir updated
https://github.com/llvm/llvm-project/pull/84525
>From 24e2454b90c2aaabb999e84240d5b2263ff01719 Mon Sep 17 00:00:00 2001
From: Ben Langmuir
Date: Fri, 8 Mar 2024 09:53:42 -0800
Subject: [PATCH] [clang][deps] Fix dependency scanning with -working-directory
Stop overriding -working-directory to CWD during argument parsing, which
should no longer necessary after we set the VFS working directory, and
create a new FileManager after parsing arguments so that
working-directory behaves correctly.
---
.../DependencyScanningWorker.cpp | 14 -
.../ClangScanDeps/working-directory-option.c | 30 +++
2 files changed, 37 insertions(+), 7 deletions(-)
create mode 100644 clang/test/ClangScanDeps/working-directory-option.c
diff --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
index 2b882f8a5e0793..76f3d950a13b81 100644
--- a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
+++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
@@ -296,7 +296,7 @@ class DependencyScanningAction : public tooling::ToolAction
{
DisableFree(DisableFree), ModuleName(ModuleName) {}
bool runInvocation(std::shared_ptr Invocation,
- FileManager *FileMgr,
+ FileManager *DriverFileMgr,
std::shared_ptr PCHContainerOps,
DiagnosticConsumer *DiagConsumer) override {
// Make a deep copy of the original Clang invocation.
@@ -342,12 +342,13 @@ class DependencyScanningAction : public
tooling::ToolAction {
ScanInstance.getHeaderSearchOpts().ModulesIncludeVFSUsage =
any(OptimizeArgs & ScanningOptimizations::VFS);
-ScanInstance.setFileManager(FileMgr);
// Support for virtual file system overlays.
-FileMgr->setVirtualFileSystem(createVFSFromCompilerInvocation(
+auto FS = createVFSFromCompilerInvocation(
ScanInstance.getInvocation(), ScanInstance.getDiagnostics(),
-FileMgr->getVirtualFileSystemPtr()));
+DriverFileMgr->getVirtualFileSystemPtr());
+// Create a new FileManager to match the invocation's FileSystemOptions.
+auto *FileMgr = ScanInstance.createFileManager(FS);
ScanInstance.createSourceManager(*FileMgr);
// Store the list of prebuilt module files into header search options. This
@@ -624,9 +625,8 @@ bool DependencyScanningWorker::computeDependencies(
ModifiedCommandLine ? *ModifiedCommandLine : CommandLine;
auto &FinalFS = ModifiedFS ? ModifiedFS : BaseFS;
- FileSystemOptions FSOpts;
- FSOpts.WorkingDir = WorkingDirectory.str();
- auto FileMgr = llvm::makeIntrusiveRefCnt(FSOpts, FinalFS);
+ auto FileMgr =
+ llvm::makeIntrusiveRefCnt(FileSystemOptions{}, FinalFS);
std::vector FinalCCommandLine(FinalCommandLine.size(),
nullptr);
llvm::transform(FinalCommandLine, FinalCCommandLine.begin(),
diff --git a/clang/test/ClangScanDeps/working-directory-option.c
b/clang/test/ClangScanDeps/working-directory-option.c
new file mode 100644
index 00..d57497d405d30f
--- /dev/null
+++ b/clang/test/ClangScanDeps/working-directory-option.c
@@ -0,0 +1,30 @@
+// Test that -working-directory works even when it differs from the working
+// directory of the filesystem.
+
+// RUN: rm -rf %t
+// RUN: mkdir -p %t/other
+// RUN: split-file %s %t
+// RUN: sed -e "s|DIR|%/t|g" %t/cdb.json.template > %t/cdb.json
+
+// RUN: clang-scan-deps -compilation-database %t/cdb.json -format
experimental-full \
+// RUN: > %t/deps.json
+
+// RUN: cat %t/deps.json | sed 's:\?:/:g' | FileCheck %s -DPREFIX=%/t
+
+// CHECK: "file-deps": [
+// CHECK-NEXT: "[[PREFIX]]/cwd/t.c"
+// CHECK-NEXT: "[[PREFIX]]/cwd/relative/h1.h"
+// CHECK-NEXT: ]
+// CHECK-NEXT: "input-file": "[[PREFIX]]/cwd/t.c"
+
+//--- cdb.json.template
+[{
+ "directory": "DIR/other",
+ "command": "clang -c t.c -I relative -working-directory DIR/cwd",
+ "file": "DIR/cwd/t.c"
+}]
+
+//--- cwd/relative/h1.h
+
+//--- cwd/t.c
+#include "h1.h"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][deps] Fix dependency scanning with -working-directory (PR #84525)
https://github.com/benlangmuir closed https://github.com/llvm/llvm-project/pull/84525 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [NFC] [C++20] [Modules] [P1689] [Scanner] Don't use thread pool in P1689 per file mode (PR #84285)
https://github.com/benlangmuir approved this pull request. https://github.com/llvm/llvm-project/pull/84285 ___ 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)
@@ -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.
benlangmuir wrote:
I'm concerned about removing `IngoreWarnings = true`. There are lots of
default warnings we may not want. Also this would allow `#pragma clang
diagnostic` to **promote** arbitrary diagnostics to errors.
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)
@@ -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.
benlangmuir wrote:
> The scanner never sees #pragma clang diagnostic, so there's no issue with
> code that uses that to turn warnings on.
Ah sorry, I forgot we skipped over most pragmas.
> 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.
This goal makes some sense to me, but I'm not sure that using the default
warnings are a good idea. The default warnings can just as easily cause us to
emit a driver warning that the user was explicitly trying to hide.
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-scan-deps] [P1689] Keep consistent behavior for make dependencies with clang (PR #69551)
https://github.com/benlangmuir commented: Is the issue with MDC's FileDeps that we are calling `makeAbsoluteAndPreferred` on the paths? Maybe we could instead move that call into `FullDependencyConsumer`. Or are there other issues? The fact we need to add additional `MDC.IsStdModuleP1689Format` checks in this PR to prevent the normal consumer callbacks is not great. This makes MDC aware of implementation details of the specific output format. https://github.com/llvm/llvm-project/pull/69551 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][deps] Skip slow `UNHASHED_CONTROL_BLOCK` records (PR #69975)
@@ -2944,6 +2944,10 @@ def fno_modules_validate_textual_header_includes : MarshallingInfoNegativeFlag>, HelpText<"Do not enforce -fmodules-decluse and private header restrictions for textual headers. " "This flag will be removed in a future Clang release.">; +defm modules_skip_diagnostic_options : BoolFOption<"modules-skip-diagnostic-options", benlangmuir wrote: Should we do the same for HSOpts here so we can add a test for it? https://github.com/llvm/llvm-project/pull/69975 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][deps] Skip slow `UNHASHED_CONTROL_BLOCK` records (PR #69975)
@@ -1212,9 +1212,12 @@ void ASTWriter::writeUnhashedControlBlock(Preprocessor
&PP,
Record.clear();
}
+ const auto &HSOpts = PP.getHeaderSearchInfo().getHeaderSearchOpts();
+
// Diagnostic options.
const auto &Diags = Context.getDiagnostics();
const DiagnosticOptions &DiagOpts = Diags.getDiagnosticOptions();
+ if (!HSOpts.ModulesSkipDiagnosticOptions) {
benlangmuir wrote:
Now that we're doing this upstream we should re-indent this code.
https://github.com/llvm/llvm-project/pull/69975
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][deps] Skip slow `UNHASHED_CONTROL_BLOCK` records (PR #69975)
@@ -2944,6 +2944,10 @@ def fno_modules_validate_textual_header_includes : MarshallingInfoNegativeFlag>, HelpText<"Do not enforce -fmodules-decluse and private header restrictions for textual headers. " "This flag will be removed in a future Clang release.">; +defm modules_skip_diagnostic_options : BoolFOption<"modules-skip-diagnostic-options", benlangmuir wrote: Seems worth having a test, and a (frontend) option seems like the easiest way to do that. https://github.com/llvm/llvm-project/pull/69975 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][deps] Fix `__has_include` behavior with umbrella headers (PR #70144)
https://github.com/benlangmuir commented: Can you clarify how this bug occurs in terms of what information about the header is stored that causes us to get the wrong module? It seems bad that passing `nullptr` to `LookupFile` could cause incorrect behaviour and makes me wonder if we need to always trigger module lookup or if there is another way to fix this that would make it safe to not do module lookup here. https://github.com/llvm/llvm-project/pull/70144 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] use relative paths for builtin headers during module compilation (PR #68023)
benlangmuir wrote: Thanks for your patience, I should have looked at this sooner. The change to our downstream test looks expected; the `` buffer changed ```diff -#import "/^tc/lib/clang/18/include/stdbool.h" +#import "stdbool.h" ``` But the actual resolution of which header is used is identical. I'm happy with this change; @jansvoboda11 were you looking at anything else or is this okay for @rmaz to land? https://github.com/llvm/llvm-project/pull/68023 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][deps] Fix `__has_include` behavior with umbrella headers (PR #70144)
https://github.com/benlangmuir approved this pull request. > Yeah. I did try to fix up all calls to LookupFile to perform module map > lookup, but a bunch of tests started failing (mostly standard C++ modules > tests IIRC), so there's probably more nuance required there. Okay, I do think this is worth fixing long term, but I don't want to block on it. Your change LGTM in the meantime. https://github.com/llvm/llvm-project/pull/70144 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][deps] Cache `VFS::getRealPath()` (PR #68645)
@@ -230,6 +251,26 @@ class DependencyScanningFilesystemLocalCache {
assert(InsertedEntry == &Entry && "entry already present");
return *InsertedEntry;
}
+
+ /// Returns real path associated with the filename or nullptr if none is
+ /// found.
+ const CachedRealPath *findRealPathByFilename(StringRef Filename) const {
+assert(llvm::sys::path::is_absolute_gnu(Filename));
+auto It = RealPathCache.find(Filename);
+return It == RealPathCache.end() ? nullptr : It->getValue();
+ }
+
+ /// Associates the given real path with the filename and returns the given
+ /// entry pointer (for convenience).
+ const CachedRealPath &
+ insertRealPathForFilename(StringRef Filename,
+const CachedRealPath &RealPath) {
benlangmuir wrote:
Since you are storing a pointer to this not the value I would suggest this take
a pointer parameter. Otherwise it's easy to assume it's being copied and would
be safe to pass a stack variable.
https://github.com/llvm/llvm-project/pull/68645
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][deps] Cache `VFS::getRealPath()` (PR #68645)
@@ -168,6 +170,12 @@ class DependencyScanningFilesystemSharedCache {
/// The backing storage for cached contents.
llvm::SpecificBumpPtrAllocator ContentsStorage;
+/// Map from filenames to cached real paths.
+llvm::StringMap RealPathsByFilename;
benlangmuir wrote:
I would lean towards combining them unless there's a drawback I'm not seeing -
the keys are fairly large here so it's nice to share storage for that. I would
maybe feel differently for a DenseMap of small keys :shrug:
https://github.com/llvm/llvm-project/pull/68645
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-scan-deps] [P1689] Keep consistent behavior for make dependencies with clang (PR #69551)
benlangmuir wrote: Thanks for the ping, I had missed your question > How do you think about the idea to add a flag to the MDC about whether or not > calling makeAbsoluteAndPreferred? SGTM; this seems like a good compromise since we can't easily extract this into the consumer. https://github.com/llvm/llvm-project/pull/69551 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r311053 - [index] Add indexing for unresolved-using declarations
Author: benlangmuir
Date: Wed Aug 16 16:12:21 2017
New Revision: 311053
URL: http://llvm.org/viewvc/llvm-project?rev=311053&view=rev
Log:
[index] Add indexing for unresolved-using declarations
In dependent contexts we end up referencing these, so make sure they
have USRs, and have their declarations indexed. For the most part they
behave like typedefs, but we also need to worry about having multiple
using declarations with the same "name".
rdar://problem/33883650
Modified:
cfe/trunk/include/clang/Index/IndexSymbol.h
cfe/trunk/lib/Index/IndexDecl.cpp
cfe/trunk/lib/Index/IndexSymbol.cpp
cfe/trunk/lib/Index/USRGeneration.cpp
cfe/trunk/test/Index/Core/index-dependent-source.cpp
cfe/trunk/test/Index/index-templates.cpp
cfe/trunk/tools/libclang/CXIndexDataConsumer.cpp
Modified: cfe/trunk/include/clang/Index/IndexSymbol.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Index/IndexSymbol.h?rev=311053&r1=311052&r2=311053&view=diff
==
--- cfe/trunk/include/clang/Index/IndexSymbol.h (original)
+++ cfe/trunk/include/clang/Index/IndexSymbol.h Wed Aug 16 16:12:21 2017
@@ -53,6 +53,7 @@ enum class SymbolKind : uint8_t {
ConversionFunction,
Parameter,
+ Using,
};
enum class SymbolLanguage {
@@ -69,6 +70,8 @@ enum class SymbolSubKind {
CXXMoveConstructor,
AccessorGetter,
AccessorSetter,
+ UsingTypename,
+ UsingValue,
};
/// Set of properties that provide additional info about a symbol.
Modified: cfe/trunk/lib/Index/IndexDecl.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Index/IndexDecl.cpp?rev=311053&r1=311052&r2=311053&view=diff
==
--- cfe/trunk/lib/Index/IndexDecl.cpp (original)
+++ cfe/trunk/lib/Index/IndexDecl.cpp Wed Aug 16 16:12:21 2017
@@ -611,6 +611,24 @@ public:
SymbolRoleSet());
}
+ bool VisitUnresolvedUsingValueDecl(const UnresolvedUsingValueDecl *D) {
+TRY_DECL(D, IndexCtx.handleDecl(D));
+const DeclContext *DC = D->getDeclContext()->getRedeclContext();
+const NamedDecl *Parent = dyn_cast(DC);
+IndexCtx.indexNestedNameSpecifierLoc(D->getQualifierLoc(), Parent,
+ D->getLexicalDeclContext());
+return true;
+ }
+
+ bool VisitUnresolvedUsingTypenameDecl(const UnresolvedUsingTypenameDecl *D) {
+TRY_DECL(D, IndexCtx.handleDecl(D));
+const DeclContext *DC = D->getDeclContext()->getRedeclContext();
+const NamedDecl *Parent = dyn_cast(DC);
+IndexCtx.indexNestedNameSpecifierLoc(D->getQualifierLoc(), Parent,
+ D->getLexicalDeclContext());
+return true;
+ }
+
bool VisitClassTemplateSpecializationDecl(const
ClassTemplateSpecializationDecl *D)
{
// FIXME: Notify subsequent callbacks if info comes from implicit
Modified: cfe/trunk/lib/Index/IndexSymbol.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Index/IndexSymbol.cpp?rev=311053&r1=311052&r2=311053&view=diff
==
--- cfe/trunk/lib/Index/IndexSymbol.cpp (original)
+++ cfe/trunk/lib/Index/IndexSymbol.cpp Wed Aug 16 16:12:21 2017
@@ -300,6 +300,18 @@ SymbolInfo index::getSymbolInfo(const De
Info.Kind = SymbolKind::TypeAlias;
Info.Lang = SymbolLanguage::CXX;
break;
+case Decl::UnresolvedUsingTypename:
+ Info.Kind = SymbolKind::Using;
+ Info.SubKind = SymbolSubKind::UsingTypename;
+ Info.Lang = SymbolLanguage::CXX;
+ Info.Properties |= (unsigned)SymbolProperty::Generic;
+ break;
+case Decl::UnresolvedUsingValue:
+ Info.Kind = SymbolKind::Using;
+ Info.SubKind = SymbolSubKind::UsingValue;
+ Info.Lang = SymbolLanguage::CXX;
+ Info.Properties |= (unsigned)SymbolProperty::Generic;
+ break;
case Decl::Binding:
Info.Kind = SymbolKind::Variable;
Info.Lang = SymbolLanguage::CXX;
@@ -448,6 +460,7 @@ StringRef index::getSymbolKindString(Sym
case SymbolKind::Destructor: return "destructor";
case SymbolKind::ConversionFunction: return "coversion-func";
case SymbolKind::Parameter: return "param";
+ case SymbolKind::Using: return "using";
}
llvm_unreachable("invalid symbol kind");
}
@@ -459,6 +472,8 @@ StringRef index::getSymbolSubKindString(
case SymbolSubKind::CXXMoveConstructor: return "cxx-move-ctor";
case SymbolSubKind::AccessorGetter: return "acc-get";
case SymbolSubKind::AccessorSetter: return "acc-set";
+ case SymbolSubKind::UsingTypename: return "using-typename";
+ case SymbolSubKind::UsingValue: return "using-value";
}
llvm_unreachable("invalid symbol subkind");
}
Modified: cfe/trunk/lib/Index/USRGeneration.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Index/USRGeneration.cpp?rev=311053&r1=31105
[clang] 773ad55 - [index] Fix performance regression with indexing macros
Author: Ben Langmuir
Date: 2021-06-16T10:16:26-07:00
New Revision: 773ad55a393f368cc92b1611c52e493ed45a353f
URL:
https://github.com/llvm/llvm-project/commit/773ad55a393f368cc92b1611c52e493ed45a353f
DIFF:
https://github.com/llvm/llvm-project/commit/773ad55a393f368cc92b1611c52e493ed45a353f.diff
LOG: [index] Fix performance regression with indexing macros
When using FileIndexRecord with macros, symbol references can be seen
out of source order, which was causing a regression to insert the
symbols into a vector. Instead, we now lazily sort the vector. The
impact is small on most code, but in very large files with many macro
references (M) near the beginning of the file followed by many decl
references (D) it was O(M*D). A particularly bad protobuf-generated
header was observed with a 100% regression in practice.
rdar://78628133
Added:
Modified:
clang/lib/Index/FileIndexRecord.cpp
clang/lib/Index/FileIndexRecord.h
Removed:
diff --git a/clang/lib/Index/FileIndexRecord.cpp
b/clang/lib/Index/FileIndexRecord.cpp
index 950a99a2304b..d392a2bedeba 100644
--- a/clang/lib/Index/FileIndexRecord.cpp
+++ b/clang/lib/Index/FileIndexRecord.cpp
@@ -17,23 +17,16 @@
using namespace clang;
using namespace clang::index;
-static void addOccurrence(std::vector &Decls,
- DeclOccurrence Info) {
- auto IsNextOccurence = [&]() -> bool {
-if (Decls.empty())
- return true;
-auto &Last = Decls.back();
-return Last.Offset < Info.Offset;
- };
-
- if (IsNextOccurence()) {
-Decls.push_back(std::move(Info));
-return;
+ArrayRef
+FileIndexRecord::getDeclOccurrencesSortedByOffset() const {
+ if (!IsSorted) {
+llvm::stable_sort(Decls,
+ [](const DeclOccurrence &A, const DeclOccurrence &B) {
+return A.Offset < B.Offset;
+ });
+IsSorted = true;
}
-
- // We keep Decls in order as we need to access them in this order in all
cases.
- auto It = llvm::upper_bound(Decls, Info);
- Decls.insert(It, std::move(Info));
+ return Decls;
}
void FileIndexRecord::addDeclOccurence(SymbolRoleSet Roles, unsigned Offset,
@@ -41,13 +34,15 @@ void FileIndexRecord::addDeclOccurence(SymbolRoleSet Roles,
unsigned Offset,
ArrayRef Relations) {
assert(D->isCanonicalDecl() &&
"Occurrences should be associated with their canonical decl");
- addOccurrence(Decls, DeclOccurrence(Roles, Offset, D, Relations));
+ IsSorted = false;
+ Decls.emplace_back(Roles, Offset, D, Relations);
}
void FileIndexRecord::addMacroOccurence(SymbolRoleSet Roles, unsigned Offset,
const IdentifierInfo *Name,
const MacroInfo *MI) {
- addOccurrence(Decls, DeclOccurrence(Roles, Offset, Name, MI));
+ IsSorted = false;
+ Decls.emplace_back(Roles, Offset, Name, MI);
}
void FileIndexRecord::removeHeaderGuardMacros() {
diff --git a/clang/lib/Index/FileIndexRecord.h
b/clang/lib/Index/FileIndexRecord.h
index 037f39f10aef..621d5b78977d 100644
--- a/clang/lib/Index/FileIndexRecord.h
+++ b/clang/lib/Index/FileIndexRecord.h
@@ -27,14 +27,13 @@ class FileIndexRecord {
private:
FileID FID;
bool IsSystem;
- std::vector Decls;
+ mutable bool IsSorted = false;
+ mutable std::vector Decls;
public:
FileIndexRecord(FileID FID, bool IsSystem) : FID(FID), IsSystem(IsSystem) {}
- ArrayRef getDeclOccurrencesSortedByOffset() const {
-return Decls;
- }
+ ArrayRef getDeclOccurrencesSortedByOffset() const;
FileID getFileID() const { return FID; }
bool isSystem() const { return IsSystem; }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 211f5af - [clang] Move getenv call for SOURCE_DATE_EPOCH out of frontend NFC
Author: Ben Langmuir
Date: 2022-10-26T12:42:56-07:00
New Revision: 211f5af38af1810925343bb71d90ca8485e66300
URL:
https://github.com/llvm/llvm-project/commit/211f5af38af1810925343bb71d90ca8485e66300
DIFF:
https://github.com/llvm/llvm-project/commit/211f5af38af1810925343bb71d90ca8485e66300.diff
LOG: [clang] Move getenv call for SOURCE_DATE_EPOCH out of frontend NFC
Move the check for SOURCE_DATE_EPOCH to the driver and use a cc1 option
to pass it to the frontend. This avoids hidden state in the cc1
invocation and makes this env variable behave more like other env
variables that clang handles in the driver.
Differential Revision: https://reviews.llvm.org/D136717
Added:
clang/test/Driver/SOURCE_DATE_EPOCH.c
Modified:
clang/include/clang/Driver/Options.td
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/test/Preprocessor/SOURCE_DATE_EPOCH.c
Removed:
diff --git a/clang/include/clang/Driver/Options.td
b/clang/include/clang/Driver/Options.td
index 0f5b6d83330e1..61cb418e862cf 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -6303,6 +6303,9 @@ def setup_static_analyzer : Flag<["-"],
"setup-static-analyzer">,
def disable_pragma_debug_crash : Flag<["-"], "disable-pragma-debug-crash">,
HelpText<"Disable any #pragma clang __debug that can lead to crashing
behavior. This is meant for testing.">,
MarshallingInfoFlag>;
+def source_date_epoch : Separate<["-"], "source-date-epoch">,
+ MetaVarName<"">,
+ HelpText<"Time to be used in __DATE__, __TIME__, and __TIMESTAMP__ macros">;
} // let Flags = [CC1Option, NoDriverOption]
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp
b/clang/lib/Driver/ToolChains/Clang.cpp
index d1e5ad393b638..1ebb37eac25ea 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -1462,6 +1462,11 @@ void Clang::AddPreprocessingOptions(Compilation &C,
const JobAction &JA,
Args.AddLastArg(CmdArgs, options::OPT_ffile_reproducible,
options::OPT_fno_file_reproducible);
+
+ if (const char *Epoch = std::getenv("SOURCE_DATE_EPOCH")) {
+CmdArgs.push_back("-source-date-epoch");
+CmdArgs.push_back(Args.MakeArgString(Epoch));
+ }
}
// FIXME: Move to target hook.
diff --git a/clang/lib/Frontend/CompilerInvocation.cpp
b/clang/lib/Frontend/CompilerInvocation.cpp
index bcd5359a5f3b2..9cd6d86b40163 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -4227,6 +4227,9 @@ static void GeneratePreprocessorArgs(PreprocessorOptions
&Opts,
for (const auto &RF : Opts.RemappedFiles)
GenerateArg(Args, OPT_remap_file, RF.first + ";" + RF.second, SA);
+ if (Opts.SourceDateEpoch)
+GenerateArg(Args, OPT_source_date_epoch, Twine(*Opts.SourceDateEpoch), SA);
+
// Don't handle LexEditorPlaceholders. It is implied by the action that is
// generated elsewhere.
}
@@ -4309,14 +4312,15 @@ static bool ParsePreprocessorArgs(PreprocessorOptions
&Opts, ArgList &Args,
Opts.addRemappedFile(Split.first, Split.second);
}
- if (const char *Epoch = std::getenv("SOURCE_DATE_EPOCH")) {
+ if (const Arg *A = Args.getLastArg(OPT_source_date_epoch)) {
+StringRef Epoch = A->getValue();
// SOURCE_DATE_EPOCH, if specified, must be a non-negative decimal integer.
// On time64 systems, pick 253402300799 (the UNIX timestamp of
// -12-31T23:59:59Z) as the upper bound.
const uint64_t MaxTimestamp =
std::min(std::numeric_limits::max(), 253402300799);
uint64_t V;
-if (StringRef(Epoch).getAsInteger(10, V) || V > MaxTimestamp) {
+if (Epoch.getAsInteger(10, V) || V > MaxTimestamp) {
Diags.Report(diag::err_fe_invalid_source_date_epoch)
<< Epoch << MaxTimestamp;
} else {
diff --git a/clang/test/Driver/SOURCE_DATE_EPOCH.c
b/clang/test/Driver/SOURCE_DATE_EPOCH.c
new file mode 100644
index 0..69c0e1e7c3b33
--- /dev/null
+++ b/clang/test/Driver/SOURCE_DATE_EPOCH.c
@@ -0,0 +1,5 @@
+// RUN: %clang -E %s -### 2>&1 | FileCheck %s -check-prefix=NO_EPOCH
+// NO_EPOCH-NOT: "-source-date-epoch"
+
+// RUN: env SOURCE_DATE_EPOCH=123 %clang -E %s -### 2>&1 | FileCheck %s
+// CHECK: "-source-date-epoch" "123"
diff --git a/clang/test/Preprocessor/SOURCE_DATE_EPOCH.c
b/clang/test/Preprocessor/SOURCE_DATE_EPOCH.c
index 71de3c7586a07..30aec44a535d7 100644
--- a/clang/test/Preprocessor/SOURCE_DATE_EPOCH.c
+++ b/clang/test/Preprocessor/SOURCE_DATE_EPOCH.c
@@ -1,10 +1,10 @@
-// RUN: env SOURCE_DATE_EPOCH=0 %clang_cc1 -E %s | FileCheck %s
--check-prefi
[clang] e1f9983 - Move getenv for AS_SECURE_LOG_FILE to clang
Author: Ben Langmuir
Date: 2022-10-28T16:08:04-07:00
New Revision: e1f998302276cf227de6c6029ea25b2dbb84f3d8
URL:
https://github.com/llvm/llvm-project/commit/e1f998302276cf227de6c6029ea25b2dbb84f3d8
DIFF:
https://github.com/llvm/llvm-project/commit/e1f998302276cf227de6c6029ea25b2dbb84f3d8.diff
LOG: Move getenv for AS_SECURE_LOG_FILE to clang
Avoid calling getenv in the MC layer and let the clang driver do it so
that it is reflected in the command-line as an -mllvm option.
rdar://101558354
Differential Revision: https://reviews.llvm.org/D136888
Added:
clang/test/CodeGen/as-secure-log-file.c
clang/test/Driver/AS_SECURE_LOG_FILE.s
clang/test/Misc/cc1as-as-secure-log-file.s
Modified:
clang/include/clang/Basic/CodeGenOptions.h
clang/include/clang/Driver/Options.td
clang/lib/CodeGen/BackendUtil.cpp
clang/lib/Driver/ToolChains/Clang.cpp
clang/tools/driver/cc1as_main.cpp
llvm/include/llvm/MC/MCContext.h
llvm/include/llvm/MC/MCTargetOptions.h
llvm/include/llvm/MC/MCTargetOptionsCommandFlags.h
llvm/lib/MC/MCContext.cpp
llvm/lib/MC/MCParser/DarwinAsmParser.cpp
llvm/lib/MC/MCTargetOptionsCommandFlags.cpp
llvm/test/MC/AsmParser/secure_log_unique.s
Removed:
diff --git a/clang/include/clang/Basic/CodeGenOptions.h
b/clang/include/clang/Basic/CodeGenOptions.h
index f0fc630c46a6e..13794035c9076 100644
--- a/clang/include/clang/Basic/CodeGenOptions.h
+++ b/clang/include/clang/Basic/CodeGenOptions.h
@@ -432,6 +432,9 @@ class CodeGenOptions : public CodeGenOptionsBase {
/// values in order to be included in misexpect diagnostics.
Optional DiagnosticsMisExpectTolerance = 0;
+ /// The name of a file to use with \c .secure_log_unique directives.
+ std::string AsSecureLogFile;
+
public:
// Define accessors/mutators for code generation options of enumeration type.
#define CODEGENOPT(Name, Bits, Default)
diff --git a/clang/include/clang/Driver/Options.td
b/clang/include/clang/Driver/Options.td
index 048f28295b39b..6d2c999873733 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -5349,6 +5349,9 @@ def fno_use_ctor_homing: Flag<["-"],
"fno-use-ctor-homing">,
HelpText<"Don't use constructor homing for debug info">;
def fuse_ctor_homing: Flag<["-"], "fuse-ctor-homing">,
HelpText<"Use constructor homing if we are using limited debug info
already">;
+def as_secure_log_file : Separate<["-"], "as-secure-log-file">,
+ HelpText<"Emit .secure_log_unique directives to this filename.">,
+ MarshallingInfoString>;
} // let Flags = [CC1Option, CC1AsOption, NoDriverOption]
diff --git a/clang/lib/CodeGen/BackendUtil.cpp
b/clang/lib/CodeGen/BackendUtil.cpp
index 874fed795da9f..f5c125da10da6 100644
--- a/clang/lib/CodeGen/BackendUtil.cpp
+++ b/clang/lib/CodeGen/BackendUtil.cpp
@@ -497,6 +497,7 @@ static bool initTargetOptions(DiagnosticsEngine &Diags,
Entry.IgnoreSysRoot ? Entry.Path : HSOpts.Sysroot + Entry.Path);
Options.MCOptions.Argv0 = CodeGenOpts.Argv0;
Options.MCOptions.CommandLineArgs = CodeGenOpts.CommandLineArgs;
+ Options.MCOptions.AsSecureLogFile = CodeGenOpts.AsSecureLogFile;
Options.MisExpect = CodeGenOpts.MisExpect;
return true;
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp
b/clang/lib/Driver/ToolChains/Clang.cpp
index 6aa377c0a4309..0786298ba918d 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -2737,6 +2737,11 @@ static void
CollectArgsForIntegratedAssembler(Compilation &C,
if (C.getDriver().embedBitcodeEnabled() ||
C.getDriver().embedBitcodeMarkerOnly())
Args.AddLastArg(CmdArgs, options::OPT_fembed_bitcode_EQ);
+
+ if (const char *AsSecureLogFile = getenv("AS_SECURE_LOG_FILE")) {
+CmdArgs.push_back("-as-secure-log-file");
+CmdArgs.push_back(Args.MakeArgString(AsSecureLogFile));
+ }
}
static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,
diff --git a/clang/test/CodeGen/as-secure-log-file.c
b/clang/test/CodeGen/as-secure-log-file.c
new file mode 100644
index 0..7475a39037330
--- /dev/null
+++ b/clang/test/CodeGen/as-secure-log-file.c
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -triple x86_64-apple-darwin -emit-obj %s -o %t.o
-as-secure-log-file %t.log
+// RUN: FileCheck %s -input-file %t.log
+// CHECK: "foobar"
+
+void test(void) {
+ __asm__(".secure_log_unique \"foobar\"");
+}
diff --git a/clang/test/Driver/AS_SECURE_LOG_FILE.s
b/clang/test/Driver/AS_SECURE_LOG_FILE.s
new file mode 100644
index 0..43d0841de4dff
--- /dev/null
+++ b/clang/test/Driver/AS_SECURE_LOG_FILE.s
@@ -0,0 +1,3 @@
+// RUN: env AS_SECURE_LOG_FILE=log_file %clang -
[clang] fd35e15 - [clang][test] Require x86 target in a couple new tests
Author: Ben Langmuir Date: 2022-10-28T17:00:18-07:00 New Revision: fd35e1506011fca925661312315b14f169a2e82a URL: https://github.com/llvm/llvm-project/commit/fd35e1506011fca925661312315b14f169a2e82a DIFF: https://github.com/llvm/llvm-project/commit/fd35e1506011fca925661312315b14f169a2e82a.diff LOG: [clang][test] Require x86 target in a couple new tests Attempt to fix test failures on bots that don't configure x86 target. Added: Modified: clang/test/CodeGen/as-secure-log-file.c clang/test/Misc/cc1as-as-secure-log-file.s Removed: diff --git a/clang/test/CodeGen/as-secure-log-file.c b/clang/test/CodeGen/as-secure-log-file.c index 7475a39037330..1b753407d4cd3 100644 --- a/clang/test/CodeGen/as-secure-log-file.c +++ b/clang/test/CodeGen/as-secure-log-file.c @@ -1,3 +1,5 @@ +// REQUIRES: x86-registered-target + // RUN: %clang_cc1 -triple x86_64-apple-darwin -emit-obj %s -o %t.o -as-secure-log-file %t.log // RUN: FileCheck %s -input-file %t.log // CHECK: "foobar" diff --git a/clang/test/Misc/cc1as-as-secure-log-file.s b/clang/test/Misc/cc1as-as-secure-log-file.s index c1ce6b58b422b..fb82f80fac02e 100644 --- a/clang/test/Misc/cc1as-as-secure-log-file.s +++ b/clang/test/Misc/cc1as-as-secure-log-file.s @@ -1,3 +1,5 @@ +// REQUIRES: x86-registered-target + // RUN: %clang -cc1as -triple x86_64-apple-darwin %s -o %t.o -as-secure-log-file %t.log // RUN: FileCheck %s -input-file %t.log // CHECK: "foobar" ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 5482432 - [clang][deps] Compute command-lines for dependencies immediately
Author: Ben Langmuir
Date: 2022-08-16T14:25:27-07:00
New Revision: 5482432bf6cc7e334894734ebbdac0a97ee98b19
URL:
https://github.com/llvm/llvm-project/commit/5482432bf6cc7e334894734ebbdac0a97ee98b19
DIFF:
https://github.com/llvm/llvm-project/commit/5482432bf6cc7e334894734ebbdac0a97ee98b19.diff
LOG: [clang][deps] Compute command-lines for dependencies immediately
Instead of delaying the generation of command-lines to after all
dependencies are reported, compute them immediately. This is partly in
preparation for splitting the TU driver command into its constituent cc1
and other jobs, but it also just simplifies working with the compiler
invocation for modules if they are not "without paths".
Also change the computation of the default output path in
clang-scan-deps to scrape the implicit module cache from the
command-line rather than get it from the dependency, since that is now
unavailable at the time we make the callback.
Differential Revision: https://reviews.llvm.org/D131934
Added:
Modified:
clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
clang/tools/clang-scan-deps/ClangScanDeps.cpp
Removed:
diff --git
a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
index aabc1e6b16678..9b4257dd70316 100644
--- a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
+++ b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
@@ -23,6 +23,10 @@ namespace clang {
namespace tooling {
namespace dependencies {
+/// A callback to lookup module outputs for "-fmodule-file=", "-o" etc.
+using LookupModuleOutputCallback =
+llvm::function_ref;
+
/// The full dependencies and module graph for a specific input.
struct FullDependencies {
/// The identifier of the C++20 module this translation unit exports.
@@ -45,17 +49,8 @@ struct FullDependencies {
/// determined that the
diff erences are benign for this compilation.
std::vector ClangModuleDeps;
- /// The original command line of the TU (excluding the compiler executable).
- std::vector OriginalCommandLine;
-
- /// Get the full command line.
- ///
- /// \param LookupModuleOutput This function is called to fill in
- /// "-fmodule-file=", "-o" and other output
- /// arguments for dependencies.
- std::vector getCommandLine(
- llvm::function_ref
- LookupModuleOutput) const;
+ /// The command line of the TU (excluding the compiler executable).
+ std::vector CommandLine;
};
struct FullDependenciesResult {
@@ -92,12 +87,16 @@ class DependencyScanningTool {
///function for a single \c DependencyScanningTool in a
///single build. Use a
diff erent one for
diff erent tools,
///and clear it between builds.
+ /// \param LookupModuleOutput This function is called to fill in
+ /// "-fmodule-file=", "-o" and other output
+ /// arguments for dependencies.
///
/// \returns a \c StringError with the diagnostic output if clang errors
/// occurred, \c FullDependencies otherwise.
llvm::Expected
getFullDependencies(const std::vector &CommandLine,
StringRef CWD, const llvm::StringSet<> &AlreadySeen,
+ LookupModuleOutputCallback LookupModuleOutput,
llvm::Optional ModuleName = None);
private:
@@ -106,8 +105,9 @@ class DependencyScanningTool {
class FullDependencyConsumer : public DependencyConsumer {
public:
- FullDependencyConsumer(const llvm::StringSet<> &AlreadySeen)
- : AlreadySeen(AlreadySeen) {}
+ FullDependencyConsumer(const llvm::StringSet<> &AlreadySeen,
+ LookupModuleOutputCallback LookupModuleOutput)
+ : AlreadySeen(AlreadySeen), LookupModuleOutput(LookupModuleOutput) {}
void handleDependencyOutputOpts(const DependencyOutputOptions &) override {}
@@ -127,6 +127,11 @@ class FullDependencyConsumer : public DependencyConsumer {
ContextHash = std::move(Hash);
}
+ std::string lookupModuleOutput(const ModuleID &ID,
+ ModuleOutputKind Kind) override {
+return LookupModuleOutput(ID, Kind);
+ }
+
FullDependenciesResult getFullDependencies(
const std::vector &OriginalCommandLine) const;
@@ -138,6 +143,7 @@ class FullDependencyCon
[clang] 3708a14 - [clang] Pull some utility functions into CompilerInvocation NFC
Author: Ben Langmuir
Date: 2022-08-23T08:18:14-07:00
New Revision: 3708a148421fd0449081b9a91fba28f51f1dfb12
URL:
https://github.com/llvm/llvm-project/commit/3708a148421fd0449081b9a91fba28f51f1dfb12
DIFF:
https://github.com/llvm/llvm-project/commit/3708a148421fd0449081b9a91fba28f51f1dfb12.diff
LOG: [clang] Pull some utility functions into CompilerInvocation NFC
Move copying compiler arguments to a vector and modifying
common module-related options into CompilerInvocation in preparation for
using some of them in more places and to avoid duplicating this code
accidentally in the future.
Differential Revision: https://reviews.llvm.org/D132419
Added:
Modified:
clang/include/clang/Frontend/CompilerInvocation.h
clang/lib/Frontend/CompilerInstance.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
Removed:
diff --git a/clang/include/clang/Frontend/CompilerInvocation.h
b/clang/include/clang/Frontend/CompilerInvocation.h
index 0753a6632f81..9cf28c52f4d9 100644
--- a/clang/include/clang/Frontend/CompilerInvocation.h
+++ b/clang/include/clang/Frontend/CompilerInvocation.h
@@ -224,7 +224,7 @@ class CompilerInvocation : public CompilerInvocationRefBase,
std::string getModuleHash() const;
using StringAllocator = llvm::function_ref;
- /// Generate a cc1-compatible command line arguments from this instance.
+ /// Generate cc1-compatible command line arguments from this instance.
///
/// \param [out] Args - The generated arguments. Note that the caller is
/// responsible for inserting the path to the clang executable and "-cc1" if
@@ -235,6 +235,20 @@ class CompilerInvocation : public
CompilerInvocationRefBase,
void generateCC1CommandLine(llvm::SmallVectorImpl &Args,
StringAllocator SA) const;
+ /// Generate cc1-compatible command line arguments from this instance,
+ /// wrapping the result as a std::vector.
+ ///
+ /// This is a (less-efficient) wrapper over generateCC1CommandLine().
+ std::vector getCC1CommandLine() const;
+
+ /// Reset all of the options that are not considered when building a
+ /// module.
+ void resetNonModularOptions();
+
+ /// Disable implicit modules and canonicalize options that are only used by
+ /// implicit modules.
+ void clearImplicitModuleBuildOptions();
+
private:
static bool CreateFromArgsImpl(CompilerInvocation &Res,
ArrayRef CommandLineArgs,
diff --git a/clang/lib/Frontend/CompilerInstance.cpp
b/clang/lib/Frontend/CompilerInstance.cpp
index e3b54478997a..03b5d048c4b8 100644
--- a/clang/lib/Frontend/CompilerInstance.cpp
+++ b/clang/lib/Frontend/CompilerInstance.cpp
@@ -1150,8 +1150,7 @@ compileModuleImpl(CompilerInstance &ImportingInstance,
SourceLocation ImportLoc,
// For any options that aren't intended to affect how a module is built,
// reset them to their default values.
- Invocation->getLangOpts()->resetNonModularOptions();
- PPOpts.resetNonModularOptions();
+ Invocation->resetNonModularOptions();
// Remove any macro definitions that are explicitly ignored by the module.
// They aren't supposed to affect how the module is built anyway.
diff --git a/clang/lib/Frontend/CompilerInvocation.cpp
b/clang/lib/Frontend/CompilerInvocation.cpp
index 9731d4006d1e..7e6d0cd7f504 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -4672,6 +4672,37 @@ void CompilerInvocation::generateCC1CommandLine(
GenerateDependencyOutputArgs(DependencyOutputOpts, Args, SA);
}
+std::vector CompilerInvocation::getCC1CommandLine() const {
+ // Set up string allocator.
+ llvm::BumpPtrAllocator Alloc;
+ llvm::StringSaver Strings(Alloc);
+ auto SA = [&Strings](const Twine &Arg) { return Strings.save(Arg).data(); };
+
+ // Synthesize full command line from the CompilerInvocation, including
"-cc1".
+ SmallVector Args{"-cc1"};
+ generateCC1CommandLine(Args, SA);
+
+ // Convert arguments to the return type.
+ return std::vector{Args.begin(), Args.end()};
+}
+
+void CompilerInvocation::resetNonModularOptions() {
+ getLangOpts()->resetNonModularOptions();
+ getPreprocessorOpts().resetNonModularOptions();
+}
+
+void CompilerInvocation::clearImplicitModuleBuildOptions() {
+ getLangOpts()->ImplicitModules = false;
+ getHeaderSearchOpts().ImplicitModuleMaps = false;
+ getHeaderSearchOpts().ModuleCachePath.clear();
+ getHeaderSearchOpts().ModulesValidateOncePerBuildSession = false;
+ getHeaderSearchOpts().BuildSessionTimestamp = 0;
+ // The specific values we canonicalize to for pruning don't affect behaviour,
+ /// so use the default values so they may be dropped from the command-line.
+ getHeaderSearchOpts().ModuleCachePruneInterval = 7 * 24 * 60 * 60;
+ getHeader
[clang] bdc20d6 - [clang][tooling] Allow -cc1 arguments in ToolInvocation
Author: Ben Langmuir
Date: 2022-08-24T19:51:12-07:00
New Revision: bdc20d61b8e4a62ea00e1eb05ba87eefaf820434
URL:
https://github.com/llvm/llvm-project/commit/bdc20d61b8e4a62ea00e1eb05ba87eefaf820434
DIFF:
https://github.com/llvm/llvm-project/commit/bdc20d61b8e4a62ea00e1eb05ba87eefaf820434.diff
LOG: [clang][tooling] Allow -cc1 arguments in ToolInvocation
ToolInvocation is useful even if you already have a -cc1 invocation,
since it provides a standard way to setup diagnostics, parse arguments,
and handoff to a ToolAction. So teach it to support -cc1 commands by
skipping the driver bits.
Differential Revision: https://reviews.llvm.org/D132615
Added:
Modified:
clang/include/clang/Tooling/Tooling.h
clang/lib/Tooling/Tooling.cpp
clang/unittests/Tooling/ToolingTest.cpp
Removed:
diff --git a/clang/include/clang/Tooling/Tooling.h
b/clang/include/clang/Tooling/Tooling.h
index b36f58ad3046..52a81cd9e778 100644
--- a/clang/include/clang/Tooling/Tooling.h
+++ b/clang/include/clang/Tooling/Tooling.h
@@ -508,7 +508,7 @@ void
addTargetAndModeForProgramName(std::vector &CommandLine,
/// Creates a \c CompilerInvocation.
CompilerInvocation *newInvocation(DiagnosticsEngine *Diagnostics,
- const llvm::opt::ArgStringList &CC1Args,
+ ArrayRef CC1Args,
const char *const BinaryName);
} // namespace tooling
diff --git a/clang/lib/Tooling/Tooling.cpp b/clang/lib/Tooling/Tooling.cpp
index 6314615f83c8..65d850842432 100644
--- a/clang/lib/Tooling/Tooling.cpp
+++ b/clang/lib/Tooling/Tooling.cpp
@@ -161,7 +161,7 @@ getCC1Arguments(DiagnosticsEngine *Diagnostics,
/// Returns a clang build invocation initialized from the CC1 flags.
CompilerInvocation *newInvocation(DiagnosticsEngine *Diagnostics,
- const llvm::opt::ArgStringList &CC1Args,
+ ArrayRef CC1Args,
const char *const BinaryName) {
assert(!CC1Args.empty() && "Must at least contain the program name!");
CompilerInvocation *Invocation = new CompilerInvocation;
@@ -339,7 +339,7 @@ ToolInvocation::~ToolInvocation() {
}
bool ToolInvocation::run() {
- std::vector Argv;
+ llvm::opt::ArgStringList Argv;
for (const std::string &Str : CommandLine)
Argv.push_back(Str.c_str());
const char *const BinaryName = Argv[0];
@@ -362,6 +362,17 @@ bool ToolInvocation::run() {
SourceManager SrcMgr(*Diagnostics, *Files);
Diagnostics->setSourceManager(&SrcMgr);
+ // We already have a cc1, just create an invocation.
+ if (CommandLine.size() >= 2 && CommandLine[1] == "-cc1") {
+ArrayRef CC1Args = makeArrayRef(Argv).drop_front();
+std::unique_ptr Invocation(
+newInvocation(&*Diagnostics, CC1Args, BinaryName));
+if (Diagnostics->hasErrorOccurred())
+ return false;
+return Action->runInvocation(std::move(Invocation), Files,
+ std::move(PCHContainerOps), DiagConsumer);
+ }
+
const std::unique_ptr Driver(
newDriver(&*Diagnostics, BinaryName, &Files->getVirtualFileSystem()));
// The "input file not found" diagnostics from the driver are useful.
diff --git a/clang/unittests/Tooling/ToolingTest.cpp
b/clang/unittests/Tooling/ToolingTest.cpp
index aca343a963da..a476a9e3b510 100644
--- a/clang/unittests/Tooling/ToolingTest.cpp
+++ b/clang/unittests/Tooling/ToolingTest.cpp
@@ -326,6 +326,46 @@ TEST(ToolInvocation, DiagConsumerExpectingSourceManager) {
EXPECT_TRUE(Consumer.SawSourceManager);
}
+TEST(ToolInvocation, CC1Args) {
+ llvm::IntrusiveRefCntPtr OverlayFileSystem(
+ new llvm::vfs::OverlayFileSystem(llvm::vfs::getRealFileSystem()));
+ llvm::IntrusiveRefCntPtr InMemoryFileSystem(
+ new llvm::vfs::InMemoryFileSystem);
+ OverlayFileSystem->pushOverlay(InMemoryFileSystem);
+ llvm::IntrusiveRefCntPtr Files(
+ new FileManager(FileSystemOptions(), OverlayFileSystem));
+ std::vector Args;
+ Args.push_back("tool-executable");
+ Args.push_back("-cc1");
+ Args.push_back("-fsyntax-only");
+ Args.push_back("test.cpp");
+ clang::tooling::ToolInvocation Invocation(
+ Args, std::make_unique(), Files.get());
+ InMemoryFileSystem->addFile(
+ "test.cpp", 0, llvm::MemoryBuffer::getMemBuffer("void foo(void);\n"));
+ EXPECT_TRUE(Invocation.run());
+}
+
+TEST(ToolInvocation, CC1ArgsInvalid) {
+ llvm::IntrusiveRefCntPtr OverlayFileSystem(
+ new llvm::vfs::OverlayFileSystem(llvm::vfs::getRealFileSystem()));
+ llvm::IntrusiveRefCntPtr InMemoryFileSystem(
+ new llvm::vfs::InMemoryFileSystem);
+ OverlayFileSystem->pushOverlay(InMemoryFileSystem);
+ llvm::IntrusiveRefCntPtr Files(
+
[clang] e8febb2 - [clang][deps] Remove CompilerInvocation from ModuleDeps
Author: Ben Langmuir
Date: 2022-08-24T19:51:12-07:00
New Revision: e8febb23a07bde8c02aee5545a0206e6f3851237
URL:
https://github.com/llvm/llvm-project/commit/e8febb23a07bde8c02aee5545a0206e6f3851237
DIFF:
https://github.com/llvm/llvm-project/commit/e8febb23a07bde8c02aee5545a0206e6f3851237.diff
LOG: [clang][deps] Remove CompilerInvocation from ModuleDeps
The invocation is only ever used to serialize cc1 arguments from, so
instead serialize the arguments inside the dep scanner to simplify the
interface.
Differential Revision: https://reviews.llvm.org/D132616
Added:
Modified:
clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
clang/tools/clang-scan-deps/ClangScanDeps.cpp
Removed:
diff --git
a/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
b/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
index a82355e60ee8..37eb4ef215b5 100644
--- a/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
+++ b/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
@@ -119,11 +119,9 @@ struct ModuleDeps {
// the primary TU.
bool ImportedByMainFile = false;
- /// Compiler invocation that can be used to build this module (without
paths).
- CompilerInvocation BuildInvocation;
-
- /// Gets the canonical command line suitable for passing to clang.
- std::vector getCanonicalCommandLine() const;
+ /// Compiler invocation that can be used to build this module. Does not
+ /// include argv[0].
+ std::vector BuildArguments;
};
class ModuleDepCollector;
@@ -238,7 +236,7 @@ class ModuleDepCollector final : public DependencyCollector
{
llvm::function_ref Optimize) const;
/// Add paths that require looking up outputs to the given dependencies.
- void addOutputPaths(ModuleDeps &Deps);
+ void addOutputPaths(CompilerInvocation &CI, ModuleDeps &Deps);
};
} // end namespace dependencies
diff --git a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
index ad2d9939896e..fe0de00f6244 100644
--- a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
+++ b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
@@ -52,9 +52,8 @@ static std::vector splitString(std::string S,
char Separator) {
return Result;
}
-void ModuleDepCollector::addOutputPaths(ModuleDeps &Deps) {
- CompilerInvocation &CI = Deps.BuildInvocation;
-
+void ModuleDepCollector::addOutputPaths(CompilerInvocation &CI,
+ModuleDeps &Deps) {
// These are technically *inputs* to the compilation, but we populate them
// here in order to make \c getModuleContextHash() independent of
// \c lookupModuleOutput().
@@ -170,11 +169,8 @@
ModuleDepCollector::makeInvocationForModuleBuildWithoutOutputs(
return CI;
}
-std::vector ModuleDeps::getCanonicalCommandLine() const {
- return BuildInvocation.getCC1CommandLine();
-}
-
static std::string getModuleContextHash(const ModuleDeps &MD,
+const CompilerInvocation &CI,
bool EagerLoadModules) {
llvm::HashBuilder,
llvm::support::endianness::native>
@@ -188,7 +184,7 @@ static std::string getModuleContextHash(const ModuleDeps
&MD,
// Hash the BuildInvocation without any input files.
SmallVector DummyArgs;
- MD.BuildInvocation.generateCC1CommandLine(DummyArgs, [&](const Twine &Arg) {
+ CI.generateCC1CommandLine(DummyArgs, [&](const Twine &Arg) {
Scratch.clear();
StringRef Str = Arg.toStringRef(Scratch);
HashBuilder.add(Str);
@@ -397,7 +393,7 @@ ModuleID ModuleDepCollectorPP::handleTopLevelModule(const
Module *M) {
llvm::DenseSet ProcessedModules;
addAllAffectingModules(M, MD, ProcessedModules);
- MD.BuildInvocation = MDC.makeInvocationForModuleBuildWithoutOutputs(
+ CompilerInvocation CI = MDC.makeInvocationForModuleBuildWithoutOutputs(
MD, [&](CompilerInvocation &BuildInvocation) {
if (MDC.OptimizeArgs)
optimizeHeaderSearchOpts(BuildInvocation.getHeaderSearchOpts(),
@@ -405,9 +401,12 @@ ModuleID ModuleDepCollectorPP::handleTopLevelModule(const
Module *M) {
});
// Compute the context hash from the inputs. Requires dependencies.
- MD.ID.ContextHash = getModuleContextHash(MD, MDC.EagerLoadModules);
+ MD.ID.ContextHash = getModuleContextHash(MD, CI, MDC.EagerLoadModules);
// Finish the compiler invocation. Requires dependencies and the context
hash.
- MDC.addOutputPaths(MD);
+ MDC.addOutputPaths(CI, MD);
+
+ MD.BuildArguments = CI.getCC1CommandLine();
+
return MD.ID;
}
diff --git a/clang/tools/clang-scan-deps/ClangScanDeps.cpp
b/clang/tools/clang-scan-d
[clang] c0a5512 - [clang][deps] Minor ModuleDepCollector refactorings NFC
Author: Ben Langmuir
Date: 2022-08-25T06:51:06-07:00
New Revision: c0a55121618b2f3f5db613cfb0d104a9ae2b700e
URL:
https://github.com/llvm/llvm-project/commit/c0a55121618b2f3f5db613cfb0d104a9ae2b700e
DIFF:
https://github.com/llvm/llvm-project/commit/c0a55121618b2f3f5db613cfb0d104a9ae2b700e.diff
LOG: [clang][deps] Minor ModuleDepCollector refactorings NFC
* Factor module map and module file path functions out
* Use a secondary mapping to lookup module deps by ID instead of the
preprocessor module map.
* Sink DirectPrebuiltModularDeps into MDC.
Differential Revision: https://reviews.llvm.org/D132617
Added:
Modified:
clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
Removed:
diff --git
a/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
b/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
index 37eb4ef215b51..c0b7b2b57b583 100644
--- a/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
+++ b/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
@@ -61,12 +61,6 @@ struct ModuleID {
}
};
-struct ModuleIDHasher {
- std::size_t operator()(const ModuleID &MID) const {
-return llvm::hash_combine(MID.ModuleName, MID.ContextHash);
- }
-};
-
/// An output from a module compilation, such as the path of the module file.
enum class ModuleOutputKind {
/// The module file (.pcm). Required.
@@ -153,8 +147,6 @@ class ModuleDepCollectorPP final : public PPCallbacks {
ModuleDepCollector &MDC;
/// Working set of direct modular dependencies.
llvm::SetVector DirectModularDeps;
- /// Working set of direct modular dependencies that have already been built.
- llvm::SetVector DirectPrebuiltModularDeps;
void handleImport(const Module *Imported);
@@ -211,6 +203,11 @@ class ModuleDepCollector final : public
DependencyCollector {
std::vector FileDeps;
/// Direct and transitive modular dependencies of the main source file.
llvm::MapVector> ModularDeps;
+ /// Secondary mapping for \c ModularDeps allowing lookup by ModuleID without
+ /// a preprocessor. Storage owned by \c ModularDeps.
+ llvm::DenseMap ModuleDepsByID;
+ /// Direct modular dependencies that have already been built.
+ llvm::MapVector DirectPrebuiltModularDeps;
/// Options that control the dependency output generation.
std::unique_ptr Opts;
/// The original Clang invocation passed to dependency scanner.
@@ -235,12 +232,39 @@ class ModuleDepCollector final : public
DependencyCollector {
const ModuleDeps &Deps,
llvm::function_ref Optimize) const;
+ /// Add module map files to the invocation, if needed.
+ void addModuleMapFiles(CompilerInvocation &CI,
+ ArrayRef ClangModuleDeps) const;
+ /// Add module files (pcm) to the invocation, if needed.
+ void addModuleFiles(CompilerInvocation &CI,
+ ArrayRef ClangModuleDeps) const;
+
/// Add paths that require looking up outputs to the given dependencies.
void addOutputPaths(CompilerInvocation &CI, ModuleDeps &Deps);
+
+ /// Compute the context hash for \p Deps, and create the mapping
+ /// \c ModuleDepsByID[Deps.ID] = &Deps.
+ void associateWithContextHash(const CompilerInvocation &CI, ModuleDeps
&Deps);
};
} // end namespace dependencies
} // end namespace tooling
} // end namespace clang
+namespace llvm {
+template <> struct DenseMapInfo {
+ using ModuleID = clang::tooling::dependencies::ModuleID;
+ static inline ModuleID getEmptyKey() { return ModuleID{"", ""}; }
+ static inline ModuleID getTombstoneKey() {
+return ModuleID{"~", "~"}; // ~ is not a valid module name or context hash
+ }
+ static unsigned getHashValue(const ModuleID &ID) {
+return hash_combine(ID.ModuleName, ID.ContextHash);
+ }
+ static bool isEqual(const ModuleID &LHS, const ModuleID &RHS) {
+return LHS == RHS;
+ }
+};
+} // namespace llvm
+
#endif // LLVM_CLANG_TOOLING_DEPENDENCYSCANNING_MODULEDEPCOLLECTOR_H
diff --git a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
index fe0de00f6244f..977ad23866ffc 100644
--- a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
+++ b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
@@ -57,15 +57,7 @@ void ModuleDepCollector::addOutputPaths(CompilerInvocation
&CI,
// These are technically *inputs* to the compilation, but we populate them
// here in order to make \c getModuleContextHash() independent of
// \c lookupModuleOutput().
- for (ModuleID MID : Deps.ClangModuleDeps) {
-auto PCMPath =
-Consumer.lookupModuleOutput(MID, ModuleOutputKind::ModuleFile);
-if (EagerLoadMod
[clang] 5ea78c4 - [clang] Update ModuleMap::getModuleMapFile* to use FileEntryRef
Author: Ben Langmuir
Date: 2022-10-05T13:12:43-07:00
New Revision: 5ea78c4113f8d2c8be24152f2dd0cadaea352c9d
URL:
https://github.com/llvm/llvm-project/commit/5ea78c4113f8d2c8be24152f2dd0cadaea352c9d
DIFF:
https://github.com/llvm/llvm-project/commit/5ea78c4113f8d2c8be24152f2dd0cadaea352c9d.diff
LOG: [clang] Update ModuleMap::getModuleMapFile* to use FileEntryRef
Update SourceManager::ContentCache::OrigEntry to keep the original
FileEntryRef, and use that to enable ModuleMap::getModuleMapFile* to
return the original FileEntryRef. This change should be NFC for
most users of SourceManager::ContentCache, but it could affect behaviour
for users of getNameAsRequested such as in compileModuleImpl. I have not
found a way to detect that difference without additional functional
changes, other than incidental cases like changes from / to \ on
Windows so there is no new test.
Differential Revision: https://reviews.llvm.org/D135220
Added:
Modified:
clang/include/clang/Basic/FileEntry.h
clang/include/clang/Basic/SourceManager.h
clang/include/clang/Lex/ModuleMap.h
clang/lib/Basic/SourceManager.cpp
clang/lib/Frontend/CompilerInstance.cpp
clang/lib/Lex/HeaderSearch.cpp
clang/lib/Lex/ModuleMap.cpp
clang/lib/Serialization/ASTReader.cpp
clang/lib/Serialization/ASTWriter.cpp
clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
clang/test/Modules/malformed.cpp
clang/tools/libclang/CIndexInclusionStack.cpp
Removed:
diff --git a/clang/include/clang/Basic/FileEntry.h
b/clang/include/clang/Basic/FileEntry.h
index d9b92a73a1d82..4f2d6436b333b 100644
--- a/clang/include/clang/Basic/FileEntry.h
+++ b/clang/include/clang/Basic/FileEntry.h
@@ -341,6 +341,23 @@ static_assert(
OptionalFileEntryRefDegradesToFileEntryPtr>::value,
"OptionalFileEntryRefDegradesToFileEntryPtr should be trivially copyable");
+inline bool operator==(const FileEntry *LHS,
+ const Optional &RHS) {
+ return LHS == (RHS ? &RHS->getFileEntry() : nullptr);
+}
+inline bool operator==(const Optional &LHS,
+ const FileEntry *RHS) {
+ return (LHS ? &LHS->getFileEntry() : nullptr) == RHS;
+}
+inline bool operator!=(const FileEntry *LHS,
+ const Optional &RHS) {
+ return !(LHS == RHS);
+}
+inline bool operator!=(const Optional &LHS,
+ const FileEntry *RHS) {
+ return !(LHS == RHS);
+}
+
/// Cached information about one file (either on disk
/// or in the virtual file system).
///
diff --git a/clang/include/clang/Basic/SourceManager.h
b/clang/include/clang/Basic/SourceManager.h
index 8a737d6b4b398..1e0743ecd8b6c 100644
--- a/clang/include/clang/Basic/SourceManager.h
+++ b/clang/include/clang/Basic/SourceManager.h
@@ -139,8 +139,9 @@ class alignas(8) ContentCache {
/// It is possible for this to be NULL if the ContentCache encapsulates
/// an imaginary text buffer.
///
- /// FIXME: Turn this into a FileEntryRef and remove Filename.
- const FileEntry *OrigEntry;
+ /// FIXME: Make non-optional using a virtual file as needed, remove \c
+ /// Filename and use \c OrigEntry.getNameAsRequested() instead.
+ OptionalFileEntryRefDegradesToFileEntryPtr OrigEntry;
/// References the file which the contents were actually loaded from.
///
@@ -177,9 +178,13 @@ class alignas(8) ContentCache {
mutable unsigned IsBufferInvalid : 1;
- ContentCache(const FileEntry *Ent = nullptr) : ContentCache(Ent, Ent) {}
+ ContentCache()
+ : OrigEntry(None), ContentsEntry(nullptr), BufferOverridden(false),
+IsFileVolatile(false), IsTransient(false), IsBufferInvalid(false) {}
+
+ ContentCache(FileEntryRef Ent) : ContentCache(Ent, Ent) {}
- ContentCache(const FileEntry *Ent, const FileEntry *contentEnt)
+ ContentCache(FileEntryRef Ent, const FileEntry *contentEnt)
: OrigEntry(Ent), ContentsEntry(contentEnt), BufferOverridden(false),
IsFileVolatile(false), IsTransient(false), IsBufferInvalid(false) {}
@@ -660,7 +665,7 @@ class SourceManager : public RefCountedBase {
struct OverriddenFilesInfoTy {
/// Files that have been overridden with the contents from another
/// file.
-llvm::DenseMap OverriddenFiles;
+llvm::DenseMap OverriddenFiles;
/// Files that were overridden with a memory buffer.
llvm::DenseSet OverriddenFilesWithBuffer;
@@ -978,8 +983,7 @@ class SourceManager : public RefCountedBase {
///
/// \param NewFile the file whose contents will be used as the
/// data instead of the contents of the given source file.
- void overrideFileContents(const FileEntry *SourceFile,
-const FileEntry *NewFile);
+ void overrideFileContents(const FileEntry *SourceFile, FileEntryRef NewFile);
/// Returns true if the file contents have been overridden.
bool is
[clang] 074fcec - [clang][deps] Canonicalize module map path
Author: Ben Langmuir
Date: 2022-10-05T15:42:38-07:00
New Revision: 074fcec1eabfc992c46c95df215b1caf5cf58970
URL:
https://github.com/llvm/llvm-project/commit/074fcec1eabfc992c46c95df215b1caf5cf58970
DIFF:
https://github.com/llvm/llvm-project/commit/074fcec1eabfc992c46c95df215b1caf5cf58970.diff
LOG: [clang][deps] Canonicalize module map path
When dep-scanning, canonicalize the module map path as much as we can.
This avoids unnecessarily needing to build multiple versions of a module
due to symlinks or case-insensitive file paths.
Despite the name `tryGetRealPathName`, the previous implementation did
not actually return the realpath most of the time, and indeed it would
be incorrect to do so since the realpath could be outside the module
directory, which would have broken finding headers relative to the
module.
Instead, use a canonicalization that is specific to the needs of
modulemap files (canonicalize the directory separately from the
filename).
Differential Revision: https://reviews.llvm.org/D134923
Added:
clang/test/ClangScanDeps/modules-symlink-dir.c
Modified:
clang/include/clang/Lex/ModuleMap.h
clang/lib/Lex/HeaderSearch.cpp
clang/lib/Lex/ModuleMap.cpp
clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
Removed:
diff --git a/clang/include/clang/Lex/ModuleMap.h
b/clang/include/clang/Lex/ModuleMap.h
index 1084b49b3534a..10c5dfc25d9ce 100644
--- a/clang/include/clang/Lex/ModuleMap.h
+++ b/clang/include/clang/Lex/ModuleMap.h
@@ -622,6 +622,15 @@ class ModuleMap {
void setInferredModuleAllowedBy(Module *M, const FileEntry *ModMap);
+ /// Canonicalize \p Path in a manner suitable for a module map file. In
+ /// particular, this canonicalizes the parent directory separately from the
+ /// filename so that it does not affect header resolution relative to the
+ /// modulemap.
+ ///
+ /// \returns an error code if any filesystem operations failed. In this case
+ /// \p Path is not modified.
+ std::error_code canonicalizeModuleMapPath(SmallVectorImpl &Path);
+
/// Get any module map files other than getModuleMapFileForUniquing(M)
/// that define submodules of a top-level module \p M. This is cheaper than
/// getting the module map file for each submodule individually, since the
diff --git a/clang/lib/Lex/HeaderSearch.cpp b/clang/lib/Lex/HeaderSearch.cpp
index 99596b1378ba1..73f9678834969 100644
--- a/clang/lib/Lex/HeaderSearch.cpp
+++ b/clang/lib/Lex/HeaderSearch.cpp
@@ -255,18 +255,11 @@ std::string
HeaderSearch::getCachedModuleFileNameImpl(StringRef ModuleName,
//
// To avoid false-negatives, we form as canonical a path as we can, and map
// to lower-case in case we're on a case-insensitive file system.
-std::string Parent =
-std::string(llvm::sys::path::parent_path(ModuleMapPath));
-if (Parent.empty())
- Parent = ".";
-auto Dir = FileMgr.getDirectory(Parent);
-if (!Dir)
+SmallString<128> CanonicalPath(ModuleMapPath);
+if (getModuleMap().canonicalizeModuleMapPath(CanonicalPath))
return {};
-auto DirName = FileMgr.getCanonicalName(*Dir);
-auto FileName = llvm::sys::path::filename(ModuleMapPath);
-llvm::hash_code Hash =
- llvm::hash_combine(DirName.lower(), FileName.lower());
+llvm::hash_code Hash = llvm::hash_combine(CanonicalPath.str().lower());
SmallString<128> HashStr;
llvm::APInt(64, size_t(Hash)).toStringUnsigned(HashStr, /*Radix*/36);
diff --git a/clang/lib/Lex/ModuleMap.cpp b/clang/lib/Lex/ModuleMap.cpp
index dbb81dc0d1478..cbd3303f36636 100644
--- a/clang/lib/Lex/ModuleMap.cpp
+++ b/clang/lib/Lex/ModuleMap.cpp
@@ -1303,6 +1303,42 @@ void ModuleMap::setInferredModuleAllowedBy(Module *M,
const FileEntry *ModMap) {
InferredModuleAllowedBy[M] = ModMap;
}
+std::error_code
+ModuleMap::canonicalizeModuleMapPath(SmallVectorImpl &Path) {
+ StringRef Dir = llvm::sys::path::parent_path({Path.data(), Path.size()});
+
+ // Do not canonicalize within the framework; the module map parser expects
+ // Modules/ not Versions/A/Modules.
+ if (llvm::sys::path::filename(Dir) == "Modules") {
+StringRef Parent = llvm::sys::path::parent_path(Dir);
+if (Parent.endswith(".framework"))
+ Dir = Parent;
+ }
+
+ FileManager &FM = SourceMgr.getFileManager();
+ auto DirEntry = FM.getDirectory(Dir.empty() ? "." : Dir);
+ if (!DirEntry)
+return DirEntry.getError();
+
+ // Canonicalize the directory.
+ StringRef CanonicalDir = FM.getCanonicalName(*DirEntry);
+ if (CanonicalDir != Dir) {
+bool Done = llvm::sys::path::replace_path_prefix(Path, Dir, CanonicalDir);
+(void)Done;
+assert(Done && "Path should always start with Dir");
+ }
+
+ // In theory, the filename component should also be canonicalized if it
+ // on a case-insensitive filesystem. Howeve
[clang] 8d9a3a6 - [clang][test] Make headers unique to avoid linking issues
Author: Ben Langmuir Date: 2022-10-06T10:09:22-07:00 New Revision: 8d9a3a6b9bd10f294d0e5adb0972a1ed8845e4aa URL: https://github.com/llvm/llvm-project/commit/8d9a3a6b9bd10f294d0e5adb0972a1ed8845e4aa DIFF: https://github.com/llvm/llvm-project/commit/8d9a3a6b9bd10f294d0e5adb0972a1ed8845e4aa.diff LOG: [clang][test] Make headers unique to avoid linking issues Make the empty headers used by cl-pch-showincludes.cpp unique so that filesystems that link these files together by contents will not see different behaviour in this test, which is not testing linked files specifically. This was uncovered by 5ea78c4113f8 which made us stop mutating the name of the presumed loc for the file in ContentCache, but that just surfaced an underlying issue that the filename of multiple includes of linked files are not separately tracked. Differential Revision: https://reviews.llvm.org/D135373 Added: Modified: clang/test/Driver/Inputs/header0.h clang/test/Driver/Inputs/header1.h clang/test/Driver/Inputs/header2.h clang/test/Driver/Inputs/header3.h clang/test/Driver/Inputs/header4.h Removed: diff --git a/clang/test/Driver/Inputs/header0.h b/clang/test/Driver/Inputs/header0.h index e69de29bb2d1d..8de9e9058b96d 100644 --- a/clang/test/Driver/Inputs/header0.h +++ b/clang/test/Driver/Inputs/header0.h @@ -0,0 +1 @@ +// header0.h diff --git a/clang/test/Driver/Inputs/header1.h b/clang/test/Driver/Inputs/header1.h index e69de29bb2d1d..586f24a26b2e3 100644 --- a/clang/test/Driver/Inputs/header1.h +++ b/clang/test/Driver/Inputs/header1.h @@ -0,0 +1 @@ +// header1.h diff --git a/clang/test/Driver/Inputs/header2.h b/clang/test/Driver/Inputs/header2.h index 243468d879c7d..e1c05acfa651d 100644 --- a/clang/test/Driver/Inputs/header2.h +++ b/clang/test/Driver/Inputs/header2.h @@ -1 +1,2 @@ +// header2.h #include "header1.h" diff --git a/clang/test/Driver/Inputs/header3.h b/clang/test/Driver/Inputs/header3.h index e69de29bb2d1d..c71a93945aa00 100644 --- a/clang/test/Driver/Inputs/header3.h +++ b/clang/test/Driver/Inputs/header3.h @@ -0,0 +1 @@ +// header3.h diff --git a/clang/test/Driver/Inputs/header4.h b/clang/test/Driver/Inputs/header4.h index e69de29bb2d1d..236f5267cfa88 100644 --- a/clang/test/Driver/Inputs/header4.h +++ b/clang/test/Driver/Inputs/header4.h @@ -0,0 +1 @@ +// header4.h ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 05ec16d - [clang][deps] Avoid leaking modulemap paths across unrelated imports
Author: Ben Langmuir
Date: 2022-11-15T13:59:26-08:00
New Revision: 05ec16d90deb747414c8534f98617d25d90bb714
URL:
https://github.com/llvm/llvm-project/commit/05ec16d90deb747414c8534f98617d25d90bb714
DIFF:
https://github.com/llvm/llvm-project/commit/05ec16d90deb747414c8534f98617d25d90bb714.diff
LOG: [clang][deps] Avoid leaking modulemap paths across unrelated imports
Use a FileEntryRef when retrieving modulemap paths in the scanner so
that we use a path compatible with the original module import, rather
than a FileEntry which can allow unrelated modules to leak paths into
how we build a module due to FileManager mutating the path.
Note: the current change prevents an "unrelated" path, but does not
change how VFS mapped paths are handled (which would be calling
getNameAsRequested) nor canonicalize the path.
Differential Revision: https://reviews.llvm.org/D137989
Added:
clang/test/ClangScanDeps/modules-symlink-dir-from-module.c
Modified:
clang/include/clang/Serialization/ASTReader.h
clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
clang/lib/Frontend/FrontendAction.cpp
clang/lib/Serialization/ASTReader.cpp
clang/lib/Serialization/ASTWriter.cpp
clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
Removed:
diff --git a/clang/include/clang/Serialization/ASTReader.h
b/clang/include/clang/Serialization/ASTReader.h
index da8b9a9207a03..23c67fc8828d3 100644
--- a/clang/include/clang/Serialization/ASTReader.h
+++ b/clang/include/clang/Serialization/ASTReader.h
@@ -2338,8 +2338,7 @@ class ASTReader
/// Visit all the top-level module maps loaded when building the given module
/// file.
void visitTopLevelModuleMaps(serialization::ModuleFile &MF,
- llvm::function_ref<
- void(const FileEntry *)> Visitor);
+ llvm::function_ref Visitor);
bool isProcessingUpdateRecords() { return ProcessingUpdateRecords; }
};
diff --git
a/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
b/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
index 5062c4cc5e265..1cc6208a4c36a 100644
--- a/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
+++ b/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
@@ -237,7 +237,7 @@ class ModuleDepCollector final : public DependencyCollector
{
llvm::function_ref Optimize) const;
/// Collect module map files for given modules.
- llvm::StringSet<>
+ llvm::DenseSet
collectModuleMapFiles(ArrayRef ClangModuleDeps) const;
/// Add module map files to the invocation, if needed.
diff --git a/clang/lib/Frontend/FrontendAction.cpp
b/clang/lib/Frontend/FrontendAction.cpp
index 45f04eac818da..76ef0cb3b4051 100644
--- a/clang/lib/Frontend/FrontendAction.cpp
+++ b/clang/lib/Frontend/FrontendAction.cpp
@@ -638,11 +638,10 @@ bool FrontendAction::BeginSourceFile(CompilerInstance &CI,
if (&MF != &PrimaryModule)
CI.getFrontendOpts().ModuleFiles.push_back(MF.FileName);
- ASTReader->visitTopLevelModuleMaps(
- PrimaryModule, [&](const FileEntry *FE) {
-CI.getFrontendOpts().ModuleMapFiles.push_back(
-std::string(FE->getName()));
- });
+ ASTReader->visitTopLevelModuleMaps(PrimaryModule, [&](FileEntryRef FE) {
+CI.getFrontendOpts().ModuleMapFiles.push_back(
+std::string(FE.getName()));
+ });
}
// Set up the input file for replay purposes.
diff --git a/clang/lib/Serialization/ASTReader.cpp
b/clang/lib/Serialization/ASTReader.cpp
index 05e6c6ea7952b..cf3716201dd80 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -9164,14 +9164,14 @@ void
ASTReader::visitInputFiles(serialization::ModuleFile &MF,
void ASTReader::visitTopLevelModuleMaps(
serialization::ModuleFile &MF,
-llvm::function_ref Visitor) {
+llvm::function_ref Visitor) {
unsigned NumInputs = MF.InputFilesLoaded.size();
for (unsigned I = 0; I < NumInputs; ++I) {
InputFileInfo IFI = readInputFileInfo(MF, I + 1);
if (IFI.TopLevelModuleMap)
// FIXME: This unnecessarily re-reads the InputFileInfo.
if (auto FE = getInputFile(MF, I + 1).getFile())
-Visitor(FE);
+Visitor(*FE);
}
}
diff --git a/clang/lib/Serialization/ASTWriter.cpp
b/clang/lib/Serialization/ASTWriter.cpp
index 1dcb5426b314d..cdd65a4461d71 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -1486,12 +1486,14 @@ namespace {
/// An input file.
struct InputFileEntry {
- const FileEntry *File;
+ FileEntryRef File;
bool IsSystemFile;
bool IsTransient;
bool BufferOverridden;
bool IsTopLevelModuleMap;
uint32_t
[clang] dcfb250 - [clang][deps] Remove checks that were just for exhaustiveness
Author: Ben Langmuir
Date: 2022-11-15T14:22:23-08:00
New Revision: dcfb25078c2629450b393e696b9760721a9f
URL:
https://github.com/llvm/llvm-project/commit/dcfb25078c2629450b393e696b9760721a9f
DIFF:
https://github.com/llvm/llvm-project/commit/dcfb25078c2629450b393e696b9760721a9f.diff
LOG: [clang][deps] Remove checks that were just for exhaustiveness
Instead of checking all the paths, just ensure the one we care about is
correct. On a particular platform one of the paths seems to have been
more canonical than we were expecting, which is fine.
Added:
Modified:
clang/test/ClangScanDeps/modules-symlink-dir-from-module.c
Removed:
diff --git a/clang/test/ClangScanDeps/modules-symlink-dir-from-module.c
b/clang/test/ClangScanDeps/modules-symlink-dir-from-module.c
index 5dde7708c2538..d1f7479adc6f9 100644
--- a/clang/test/ClangScanDeps/modules-symlink-dir-from-module.c
+++ b/clang/test/ClangScanDeps/modules-symlink-dir-from-module.c
@@ -19,38 +19,11 @@
// CHECK: "modules": [
// CHECK: {
// CHECK: "command-line": [
+// CHECK-NOT: ]
// CHECK: "-fmodule-map-file=[[PREFIX]]/include/module/module.modulemap"
// CHECK: ]
// CHECK: "name": "Foo"
// CHECK: }
-// CHECK: {
-// CHECK: "command-line": [
-// FIXME: canonicalize this path
-// CHECK:
"-fmodule-map-file=[[PREFIX]]/include/symlink-to-module/module.modulemap"
-// CHECK: ]
-// CHECK: "name": "Foo_Private"
-// CHECK: }
-// CHECK: {
-// CHECK: "command-line": [
-// CHECK: "[[PREFIX]]/include/module/module.modulemap"
-// CHECK: ]
-// CHECK: "name": "Mod"
-// CHECK: }
-// CHECK: {
-// CHECK: "command-line": [
-// FIXME: canonicalize this path
-// CHECK:
"-fmodule-map-file=[[PREFIX]]/include/symlink-to-module/module.modulemap"
-// CHECK: ]
-// CHECK: "name": "Other"
-// CHECK: }
-// CHECK: {
-// CHECK: "command-line": [
-// FIXME: canonicalize this path
-// CHECK:
"-fmodule-map-file=[[PREFIX]]/include/symlink-to-module/module.modulemap"
-// CHECK: ]
-// CHECK: "name": "Test"
-// CHECK: }
-// CHECK: ]
//--- cdb.json.in
[{
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
