[clang] [clang-tools-extra] [llvm] [clang-format] Add support for BasedOnStyle referencing an arbitrary file (PR #107312)

2024-09-04 Thread Ryan Saunders via cfe-commits 

https://github.com/jediry created 
https://github.com/llvm/llvm-project/pull/107312

At present, the ```BasedOnStyle``` directive can reference a predefined style 
name, or ```InheritFromParent```, instructing clang-format to search upward in 
the directory hierarchy for a .clang-format file. This works fine when 
variations in codebase formatting align with the directory hierarchy, but 
becomes problematic when it is desired to share formatting across portions of a 
repository with no common parent directory. For example, consider the case of a 
large, multi-team repository containing many projects as sub-directories of the 
repository root, where each of the various engineering teams owns multiple of 
these projects, and wants a common coding style across all of its owned 
projects.

In this PR, I'm extending ```BasedOnStyle``` to allow referencing an arbitrary 
file. While this could be an absolute path, that's not typically useful across 
machines. In typical usage, this will be a relative path (e.g., 
```BasedOnStyle: format/team1.clang-format```).  For resolving relative paths, 
I've mimicked the "include path" model of C/C++ (and, in fact, I'm able to 
leverage ```SourceMgr```'s "include paths" mechanism to implement this). The 
list of style search paths is specified on the command-line via one or more 
```--style-search-path ``` parameters.

The interesting stuff is in Format.cpp...most of the rest of this change is 
plumbing to get the StyleSearchPaths from the command-line to the call to 
```getStyle()```.

Unfinished aspects of this change:
* No unit tests. I am happy to write some, but I'm unsure how to do this in a 
harmonious way, seeing as my change inherently relies on external files on the 
filesystem. Most of the unit tests I've seen in Clang rely on in-code strings.
* ```--style-search-path .``` does not seem to work for specifying the current 
directory as a search path. Evidently the ```SourceMgr``` include paths 
mechanism does not handle this natively. Can someone point me to the proper 
"Clang" way to resolve paths like . or .. into paths that ```SourceMgr``` can 
handle?

>From 163657095741b2292540fce7ff424cb4600391da Mon Sep 17 00:00:00 2001
From: Ryan Saunders 
Date: Fri, 2 Aug 2024 15:08:24 -0700
Subject: [PATCH] Add support for BasedOnStyle referencing an arbitrary file,
 locatable relative to directories specified via --style-search-path

Improve command-line help

Clean up formatting

Fix unit tests

Fix other clang tools that leverage format::getStyle()

Fix up clangd
---
 .../tool/ClangApplyReplacementsMain.cpp   |  19 
 .../ChangeNamespace.cpp   |   7 +-
 .../clang-change-namespace/ChangeNamespace.h  |   6 +-
 .../tool/ClangChangeNamespace.cpp |  18 ++-
 .../tool/ClangIncludeFixer.cpp|  22 +++-
 clang-tools-extra/clang-move/Move.cpp |   1 +
 clang-tools-extra/clang-move/Move.h   |   3 +
 .../clang-move/tool/ClangMove.cpp |  22 +++-
 clang-tools-extra/clang-tidy/ClangTidy.cpp|   3 +-
 .../clang-tidy/ClangTidyOptions.h |   4 +
 .../clang-tidy/misc/IncludeCleanerCheck.cpp   |   5 +-
 .../clang-tidy/misc/IncludeCleanerCheck.h |   1 +
 .../clang-tidy/tool/ClangTidyMain.cpp |  15 +++
 clang-tools-extra/clangd/ClangdLSPServer.cpp  |   2 +-
 clang-tools-extra/clangd/ClangdServer.cpp |  13 ++-
 clang-tools-extra/clangd/ClangdServer.h   |  11 +-
 clang-tools-extra/clangd/Config.h |   4 +
 clang-tools-extra/clangd/IncludeCleaner.cpp   |   1 +
 clang-tools-extra/clangd/SourceCode.cpp   |   3 +
 clang-tools-extra/clangd/tool/Check.cpp   |   2 +-
 clang-tools-extra/clangd/tool/ClangdMain.cpp  |  23 
 .../clangd/unittests/ClangdTests.cpp  |   2 +-
 .../include-cleaner/tool/IncludeCleaner.cpp   |  15 ++-
 .../ChangeNamespaceTests.cpp  |   2 +-
 .../unittests/clang-move/ClangMoveTests.cpp   |   4 +-
 clang/include/clang/Format/Format.h   |  29 -
 clang/include/clang/Tooling/Refactoring.h |   4 +-
 clang/lib/Format/Format.cpp   |  51 +++--
 clang/lib/Tooling/Refactoring.cpp |   4 +-
 clang/tools/clang-format/ClangFormat.cpp  |  21 +++-
 clang/unittests/Format/ConfigParseTest.cpp| 106 ++
 clang/unittests/Format/FormatTestObjC.cpp |  56 -
 .../unittests/Format/FormatTestRawStrings.cpp |   2 +-
 .../ObjCPropertyAttributeOrderFixerTest.cpp   |   6 +-
 clang/unittests/Format/QualifierFixerTest.cpp |   6 +-
 clang/unittests/Rename/ClangRenameTest.h  |   2 +-
 clang/unittests/Tooling/RefactoringTest.cpp   |   3 +-
 llvm/include/llvm/Support/YAMLTraits.h|   6 +
 llvm/lib/Support/YAMLTraits.cpp   |  12 ++
 39 files changed, 391 insertions(+), 125 deletions(-)

diff --git 
a/clang-tools-extra/clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp
 
b/clang-tools-extra/clang-apply-replacements/tool/ClangApplyReplacementsMa

[clang] [clang-tools-extra] [llvm] [clang-format] Add support for BasedOnStyle referencing an arbitrary file (PR #107312)

2024-09-06 Thread Ryan Saunders via cfe-commits 


@@ -5274,9 +5274,17 @@ struct FormatStyle {
 
   friend std::error_code
   parseConfiguration(llvm::MemoryBufferRef Config, FormatStyle *Style,
+ const std::vector &StyleSearchPaths,

jediry wrote:

Yes, I can certainly do that if that is preferred. And, if I'm to do this as 
multiple PRs as several have requested, then a default param (or an overload) 
is needed. FWIW, though, I did this intentionally, to flush out all callers of 
lib/format: once this feature is in and used by actual users, this needs to be 
pushed through all the tools that leverage lib/format, or else those tools will 
choke on peoples' valid .clang-format files.

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

[clang] [clang-tools-extra] [llvm] [clang-format] Add support for BasedOnStyle referencing an arbitrary file (PR #107312)

2024-09-06 Thread Ryan Saunders via cfe-commits 


@@ -39,6 +39,10 @@ IO::IO(void *Context) : Ctxt(Context) {}
 
 IO::~IO() = default;
 
+SourceMgr& IO::getSourceMgr() {

jediry wrote:

Sure, I can break this up.

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

[clang] [clang-tools-extra] [llvm] [clang-format] Add support for BasedOnStyle referencing an arbitrary file (PR #107312)

2024-09-06 Thread Ryan Saunders via cfe-commits 

jediry wrote:

> I am not sure if it's best to push a new command line flag to all other tools 
> that use clang-format as a library.
> 
> Have you considered any other alternatives that can self-contain this in 
> clang-format? e.g can we just treat paths as relative to current 
> `.clang-format` file ?

@kadircet @mydeveloperday @HazardyKnusperkeks I would love to make this more 
self-contained, as this certainly does hit a lot of layers of code. I 
specifically selected this approach because of its similarity to the familiar 
```-I``` mechanism for specifying include paths for C/C++. Two other ideas I 
did consider are:
1. Allow the use of environment variables in the path (e.g., ```BasedOnStyle: 
$(FORMATDIR)\team1.clang-format```). Not sure if this is better or worse than 
the current approach, though it's certainly a smaller code change. (That said, 
I find that most command-line tools that find things via environment variable 
also support command-line overrides for this, since it's sometimes cumbersome 
to have to set the environment variable in order to use the tool.)
2. As you say, do this relative to the current .clang-format file. This is also 
more self-contained, though it has the downside of being easily broken by code 
moves (e.g., if I "git mv" a directory to a different path, expressions like 
```BasedOnStyle: ../../../format/team1.clang-format``` become invalid).

I'm happy to go a different way with this if that is preferred.

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

[clang] [clang-tools-extra] [llvm] [clang-format] Add support for BasedOnStyle referencing an arbitrary file (PR #107312)

2024-09-08 Thread Ryan Saunders via cfe-commits 

jediry wrote:

@owenca I've opened an issue 
[here](https://github.com/llvm/llvm-project/issues/107808)

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

[clang] Support BasedOnStyle referencing an arbitrary file (PR #110634)

2024-10-08 Thread Ryan Saunders via cfe-commits 

https://github.com/jediry updated 
https://github.com/llvm/llvm-project/pull/110634

>From 82160d1e6de3f197c847bf8ed21ea1fc314b3cf4 Mon Sep 17 00:00:00 2001
From: Ryan Saunders 
Date: Tue, 1 Oct 2024 00:03:50 -0700
Subject: [PATCH 1/2] Support BasedOnStyle: file:blah.clang-format

---
 clang/include/clang/Format/Format.h |  22 ++-
 clang/lib/Format/Format.cpp | 207 +---
 2 files changed, 174 insertions(+), 55 deletions(-)

diff --git a/clang/include/clang/Format/Format.h 
b/clang/include/clang/Format/Format.h
index d8b62c7652a0f6..1b833256500431 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -53,10 +53,24 @@ std::error_code make_error_code(ParseError e);
 /// The ``FormatStyle`` is used to configure the formatting to follow
 /// specific guidelines.
 struct FormatStyle {
-  // If the BasedOn: was InheritParentConfig and this style needs the file from
-  // the parent directories. It is not part of the actual style for formatting.
-  // Thus the // instead of ///.
-  bool InheritsParentConfig;
+  // If the BasedOnStyle: was InheritParentConfig, this is the string
+  // "", indicating to search upwards until a _clang-format or
+  // .clang-format file is found.
+  //
+  // Else, if the BasedOnStyle: was an explicit "file:" reference, this is
+  // that reference, verbatim, including the "file:" prefix. The string
+  // after "file:" may start with $(CLANG_FORMAT_DIR), in which case the value
+  // of the CLANG_FORMAT_DIR environment variable (which must be defined) is
+  // substituted; otherwise, the string after "file:" is interpreted as a
+  // path relative to the current config file. (Absolute paths are not
+  // permitted, for security reasons.)
+  //
+  // Else (i.e., if the BasedOnStyle is omitted or a predefined style), this is
+  // the empty string.
+  //
+  // This field is not part of the actual style for formatting, thus the //
+  // instead of ///.
+  std::string InheritsConfig;
 
   /// The extra indent or outdent of access modifiers, e.g. ``public:``.
   /// \version 3.3
diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index d2463b892fbb96..bfca01d1a67230 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -1543,7 +1543,7 @@ FormatStyle getLLVMStyle(FormatStyle::LanguageKind 
Language) {
   LLVMStyle.IndentRequiresClause = true;
   LLVMStyle.IndentWidth = 2;
   LLVMStyle.IndentWrappedFunctionNames = false;
-  LLVMStyle.InheritsParentConfig = false;
+  LLVMStyle.InheritsConfig.clear();
   LLVMStyle.InsertBraces = false;
   LLVMStyle.InsertNewlineAtEOF = false;
   LLVMStyle.InsertTrailingCommas = FormatStyle::TCS_None;
@@ -1992,7 +1992,9 @@ bool getPredefinedStyle(StringRef Name, 
FormatStyle::LanguageKind Language,
   else if (Name.equals_insensitive("none"))
 *Style = getNoStyle();
   else if (Name.equals_insensitive("inheritparentconfig"))
-Style->InheritsParentConfig = true;
+Style->InheritsConfig = "";
+  else if (Name.starts_with_insensitive("file:"))
+Style->InheritsConfig = Name;
   else
 return false;
 
@@ -4004,6 +4006,45 @@ loadAndParseConfigFile(StringRef ConfigFile, 
llvm::vfs::FileSystem *FS,
   return Text;
 }
 
+// Resolves an explicit file: reference in a BasedOnStyle directive to a
+// canonical, absolute path, substituting the value of the CLANG_FORMAT_DIR
+// environment variable if the path starts with $(CLANG_FORMAT_DIR), or 
treating
+// BasedOnFile as relative to the current ConfigFile otherwise.
+static Expected>
+resolveExplicitParentConfigFile(StringRef ConfigFile, StringRef BasedOnFile,
+llvm::vfs::FileSystem *FS) {
+  constexpr const char *EnvVar = "CLANG_FORMAT_DIR";
+  constexpr const char *EnvVarExpansion = "$(CLANG_FORMAT_DIR)";
+  if (BasedOnFile.starts_with(EnvVarExpansion)) {
+const char *ClangFormatDir = getenv(EnvVar);
+if (ClangFormatDir == nullptr) {
+  return make_string_error(ConfigFile + ": 'BasedOnStyle: " + BasedOnFile +
+   "' uses " + EnvVarExpansion + ", but the " +
+   EnvVar + " environment variable is not 
defined");
+} else {
+  auto Status = FS->status(ClangFormatDir);
+  if (!Status ||
+  Status->getType() != llvm::sys::fs::file_type::directory_file) {
+return make_string_error(
+StringRef("Environment variable ") + EnvVar + " = " +
+ClangFormatDir + "does not refer to a valid, readable directory");
+  }
+
+  SmallString<128> FileName{ClangFormatDir};
+  llvm::sys::path::append(FileName,
+  BasedOnFile.substr(strlen(EnvVarExpansion)));
+  ;
+  llvm::sys::path::remove_dots(FileName, true /*remove_dot_dot*/);
+  return FileName;
+}
+  } else {
+SmallString<128> FileName = llvm::sys::path::parent_path(ConfigFile);
+llvm::sys::path::append(FileName, BasedOnFile);
+llvm::sys::path::re

[clang] Support BasedOnStyle referencing an arbitrary file (PR #110634)

2024-10-08 Thread Ryan Saunders via cfe-commits 

https://github.com/jediry updated 
https://github.com/llvm/llvm-project/pull/110634

>From 82160d1e6de3f197c847bf8ed21ea1fc314b3cf4 Mon Sep 17 00:00:00 2001
From: Ryan Saunders 
Date: Tue, 1 Oct 2024 00:03:50 -0700
Subject: [PATCH 1/2] Support BasedOnStyle: file:blah.clang-format

---
 clang/include/clang/Format/Format.h |  22 ++-
 clang/lib/Format/Format.cpp | 207 +---
 2 files changed, 174 insertions(+), 55 deletions(-)

diff --git a/clang/include/clang/Format/Format.h 
b/clang/include/clang/Format/Format.h
index d8b62c7652a0f6..1b833256500431 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -53,10 +53,24 @@ std::error_code make_error_code(ParseError e);
 /// The ``FormatStyle`` is used to configure the formatting to follow
 /// specific guidelines.
 struct FormatStyle {
-  // If the BasedOn: was InheritParentConfig and this style needs the file from
-  // the parent directories. It is not part of the actual style for formatting.
-  // Thus the // instead of ///.
-  bool InheritsParentConfig;
+  // If the BasedOnStyle: was InheritParentConfig, this is the string
+  // "", indicating to search upwards until a _clang-format or
+  // .clang-format file is found.
+  //
+  // Else, if the BasedOnStyle: was an explicit "file:" reference, this is
+  // that reference, verbatim, including the "file:" prefix. The string
+  // after "file:" may start with $(CLANG_FORMAT_DIR), in which case the value
+  // of the CLANG_FORMAT_DIR environment variable (which must be defined) is
+  // substituted; otherwise, the string after "file:" is interpreted as a
+  // path relative to the current config file. (Absolute paths are not
+  // permitted, for security reasons.)
+  //
+  // Else (i.e., if the BasedOnStyle is omitted or a predefined style), this is
+  // the empty string.
+  //
+  // This field is not part of the actual style for formatting, thus the //
+  // instead of ///.
+  std::string InheritsConfig;
 
   /// The extra indent or outdent of access modifiers, e.g. ``public:``.
   /// \version 3.3
diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index d2463b892fbb96..bfca01d1a67230 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -1543,7 +1543,7 @@ FormatStyle getLLVMStyle(FormatStyle::LanguageKind 
Language) {
   LLVMStyle.IndentRequiresClause = true;
   LLVMStyle.IndentWidth = 2;
   LLVMStyle.IndentWrappedFunctionNames = false;
-  LLVMStyle.InheritsParentConfig = false;
+  LLVMStyle.InheritsConfig.clear();
   LLVMStyle.InsertBraces = false;
   LLVMStyle.InsertNewlineAtEOF = false;
   LLVMStyle.InsertTrailingCommas = FormatStyle::TCS_None;
@@ -1992,7 +1992,9 @@ bool getPredefinedStyle(StringRef Name, 
FormatStyle::LanguageKind Language,
   else if (Name.equals_insensitive("none"))
 *Style = getNoStyle();
   else if (Name.equals_insensitive("inheritparentconfig"))
-Style->InheritsParentConfig = true;
+Style->InheritsConfig = "";
+  else if (Name.starts_with_insensitive("file:"))
+Style->InheritsConfig = Name;
   else
 return false;
 
@@ -4004,6 +4006,45 @@ loadAndParseConfigFile(StringRef ConfigFile, 
llvm::vfs::FileSystem *FS,
   return Text;
 }
 
+// Resolves an explicit file: reference in a BasedOnStyle directive to a
+// canonical, absolute path, substituting the value of the CLANG_FORMAT_DIR
+// environment variable if the path starts with $(CLANG_FORMAT_DIR), or 
treating
+// BasedOnFile as relative to the current ConfigFile otherwise.
+static Expected>
+resolveExplicitParentConfigFile(StringRef ConfigFile, StringRef BasedOnFile,
+llvm::vfs::FileSystem *FS) {
+  constexpr const char *EnvVar = "CLANG_FORMAT_DIR";
+  constexpr const char *EnvVarExpansion = "$(CLANG_FORMAT_DIR)";
+  if (BasedOnFile.starts_with(EnvVarExpansion)) {
+const char *ClangFormatDir = getenv(EnvVar);
+if (ClangFormatDir == nullptr) {
+  return make_string_error(ConfigFile + ": 'BasedOnStyle: " + BasedOnFile +
+   "' uses " + EnvVarExpansion + ", but the " +
+   EnvVar + " environment variable is not 
defined");
+} else {
+  auto Status = FS->status(ClangFormatDir);
+  if (!Status ||
+  Status->getType() != llvm::sys::fs::file_type::directory_file) {
+return make_string_error(
+StringRef("Environment variable ") + EnvVar + " = " +
+ClangFormatDir + "does not refer to a valid, readable directory");
+  }
+
+  SmallString<128> FileName{ClangFormatDir};
+  llvm::sys::path::append(FileName,
+  BasedOnFile.substr(strlen(EnvVarExpansion)));
+  ;
+  llvm::sys::path::remove_dots(FileName, true /*remove_dot_dot*/);
+  return FileName;
+}
+  } else {
+SmallString<128> FileName = llvm::sys::path::parent_path(ConfigFile);
+llvm::sys::path::append(FileName, BasedOnFile);
+llvm::sys::path::re

[clang] Support BasedOnStyle referencing an arbitrary file (PR #110634)

2024-10-08 Thread Ryan Saunders via cfe-commits 

jediry wrote:

> Can this have some unit test?

Done. Added docs as well.

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

[clang] Support BasedOnStyle referencing an arbitrary file (PR #110634)

2024-10-09 Thread Ryan Saunders via cfe-commits 

https://github.com/jediry updated 
https://github.com/llvm/llvm-project/pull/110634

>From 82160d1e6de3f197c847bf8ed21ea1fc314b3cf4 Mon Sep 17 00:00:00 2001
From: Ryan Saunders 
Date: Tue, 1 Oct 2024 00:03:50 -0700
Subject: [PATCH 1/3] Support BasedOnStyle: file:blah.clang-format

---
 clang/include/clang/Format/Format.h |  22 ++-
 clang/lib/Format/Format.cpp | 207 +---
 2 files changed, 174 insertions(+), 55 deletions(-)

diff --git a/clang/include/clang/Format/Format.h 
b/clang/include/clang/Format/Format.h
index d8b62c7652a0f6..1b833256500431 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -53,10 +53,24 @@ std::error_code make_error_code(ParseError e);
 /// The ``FormatStyle`` is used to configure the formatting to follow
 /// specific guidelines.
 struct FormatStyle {
-  // If the BasedOn: was InheritParentConfig and this style needs the file from
-  // the parent directories. It is not part of the actual style for formatting.
-  // Thus the // instead of ///.
-  bool InheritsParentConfig;
+  // If the BasedOnStyle: was InheritParentConfig, this is the string
+  // "", indicating to search upwards until a _clang-format or
+  // .clang-format file is found.
+  //
+  // Else, if the BasedOnStyle: was an explicit "file:" reference, this is
+  // that reference, verbatim, including the "file:" prefix. The string
+  // after "file:" may start with $(CLANG_FORMAT_DIR), in which case the value
+  // of the CLANG_FORMAT_DIR environment variable (which must be defined) is
+  // substituted; otherwise, the string after "file:" is interpreted as a
+  // path relative to the current config file. (Absolute paths are not
+  // permitted, for security reasons.)
+  //
+  // Else (i.e., if the BasedOnStyle is omitted or a predefined style), this is
+  // the empty string.
+  //
+  // This field is not part of the actual style for formatting, thus the //
+  // instead of ///.
+  std::string InheritsConfig;
 
   /// The extra indent or outdent of access modifiers, e.g. ``public:``.
   /// \version 3.3
diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index d2463b892fbb96..bfca01d1a67230 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -1543,7 +1543,7 @@ FormatStyle getLLVMStyle(FormatStyle::LanguageKind 
Language) {
   LLVMStyle.IndentRequiresClause = true;
   LLVMStyle.IndentWidth = 2;
   LLVMStyle.IndentWrappedFunctionNames = false;
-  LLVMStyle.InheritsParentConfig = false;
+  LLVMStyle.InheritsConfig.clear();
   LLVMStyle.InsertBraces = false;
   LLVMStyle.InsertNewlineAtEOF = false;
   LLVMStyle.InsertTrailingCommas = FormatStyle::TCS_None;
@@ -1992,7 +1992,9 @@ bool getPredefinedStyle(StringRef Name, 
FormatStyle::LanguageKind Language,
   else if (Name.equals_insensitive("none"))
 *Style = getNoStyle();
   else if (Name.equals_insensitive("inheritparentconfig"))
-Style->InheritsParentConfig = true;
+Style->InheritsConfig = "";
+  else if (Name.starts_with_insensitive("file:"))
+Style->InheritsConfig = Name;
   else
 return false;
 
@@ -4004,6 +4006,45 @@ loadAndParseConfigFile(StringRef ConfigFile, 
llvm::vfs::FileSystem *FS,
   return Text;
 }
 
+// Resolves an explicit file: reference in a BasedOnStyle directive to a
+// canonical, absolute path, substituting the value of the CLANG_FORMAT_DIR
+// environment variable if the path starts with $(CLANG_FORMAT_DIR), or 
treating
+// BasedOnFile as relative to the current ConfigFile otherwise.
+static Expected>
+resolveExplicitParentConfigFile(StringRef ConfigFile, StringRef BasedOnFile,
+llvm::vfs::FileSystem *FS) {
+  constexpr const char *EnvVar = "CLANG_FORMAT_DIR";
+  constexpr const char *EnvVarExpansion = "$(CLANG_FORMAT_DIR)";
+  if (BasedOnFile.starts_with(EnvVarExpansion)) {
+const char *ClangFormatDir = getenv(EnvVar);
+if (ClangFormatDir == nullptr) {
+  return make_string_error(ConfigFile + ": 'BasedOnStyle: " + BasedOnFile +
+   "' uses " + EnvVarExpansion + ", but the " +
+   EnvVar + " environment variable is not 
defined");
+} else {
+  auto Status = FS->status(ClangFormatDir);
+  if (!Status ||
+  Status->getType() != llvm::sys::fs::file_type::directory_file) {
+return make_string_error(
+StringRef("Environment variable ") + EnvVar + " = " +
+ClangFormatDir + "does not refer to a valid, readable directory");
+  }
+
+  SmallString<128> FileName{ClangFormatDir};
+  llvm::sys::path::append(FileName,
+  BasedOnFile.substr(strlen(EnvVarExpansion)));
+  ;
+  llvm::sys::path::remove_dots(FileName, true /*remove_dot_dot*/);
+  return FileName;
+}
+  } else {
+SmallString<128> FileName = llvm::sys::path::parent_path(ConfigFile);
+llvm::sys::path::append(FileName, BasedOnFile);
+llvm::sys::path::re

[clang] Support BasedOnStyle referencing an arbitrary file (PR #110634)

2024-10-21 Thread Ryan Saunders via cfe-commits 

jediry wrote:

Ping @mydeveloperday @owenca 

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

[clang] Support BasedOnStyle referencing an arbitrary file (PR #110634)

2024-10-15 Thread Ryan Saunders via cfe-commits 

jediry wrote:

Ping

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

[clang] Support BasedOnStyle referencing an arbitrary file (PR #110634)

2024-10-15 Thread Ryan Saunders via cfe-commits 

https://github.com/jediry updated 
https://github.com/llvm/llvm-project/pull/110634

>From 82160d1e6de3f197c847bf8ed21ea1fc314b3cf4 Mon Sep 17 00:00:00 2001
From: Ryan Saunders 
Date: Tue, 1 Oct 2024 00:03:50 -0700
Subject: [PATCH 1/3] Support BasedOnStyle: file:blah.clang-format

---
 clang/include/clang/Format/Format.h |  22 ++-
 clang/lib/Format/Format.cpp | 207 +---
 2 files changed, 174 insertions(+), 55 deletions(-)

diff --git a/clang/include/clang/Format/Format.h 
b/clang/include/clang/Format/Format.h
index d8b62c7652a0f6..1b833256500431 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -53,10 +53,24 @@ std::error_code make_error_code(ParseError e);
 /// The ``FormatStyle`` is used to configure the formatting to follow
 /// specific guidelines.
 struct FormatStyle {
-  // If the BasedOn: was InheritParentConfig and this style needs the file from
-  // the parent directories. It is not part of the actual style for formatting.
-  // Thus the // instead of ///.
-  bool InheritsParentConfig;
+  // If the BasedOnStyle: was InheritParentConfig, this is the string
+  // "", indicating to search upwards until a _clang-format or
+  // .clang-format file is found.
+  //
+  // Else, if the BasedOnStyle: was an explicit "file:" reference, this is
+  // that reference, verbatim, including the "file:" prefix. The string
+  // after "file:" may start with $(CLANG_FORMAT_DIR), in which case the value
+  // of the CLANG_FORMAT_DIR environment variable (which must be defined) is
+  // substituted; otherwise, the string after "file:" is interpreted as a
+  // path relative to the current config file. (Absolute paths are not
+  // permitted, for security reasons.)
+  //
+  // Else (i.e., if the BasedOnStyle is omitted or a predefined style), this is
+  // the empty string.
+  //
+  // This field is not part of the actual style for formatting, thus the //
+  // instead of ///.
+  std::string InheritsConfig;
 
   /// The extra indent or outdent of access modifiers, e.g. ``public:``.
   /// \version 3.3
diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index d2463b892fbb96..bfca01d1a67230 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -1543,7 +1543,7 @@ FormatStyle getLLVMStyle(FormatStyle::LanguageKind 
Language) {
   LLVMStyle.IndentRequiresClause = true;
   LLVMStyle.IndentWidth = 2;
   LLVMStyle.IndentWrappedFunctionNames = false;
-  LLVMStyle.InheritsParentConfig = false;
+  LLVMStyle.InheritsConfig.clear();
   LLVMStyle.InsertBraces = false;
   LLVMStyle.InsertNewlineAtEOF = false;
   LLVMStyle.InsertTrailingCommas = FormatStyle::TCS_None;
@@ -1992,7 +1992,9 @@ bool getPredefinedStyle(StringRef Name, 
FormatStyle::LanguageKind Language,
   else if (Name.equals_insensitive("none"))
 *Style = getNoStyle();
   else if (Name.equals_insensitive("inheritparentconfig"))
-Style->InheritsParentConfig = true;
+Style->InheritsConfig = "";
+  else if (Name.starts_with_insensitive("file:"))
+Style->InheritsConfig = Name;
   else
 return false;
 
@@ -4004,6 +4006,45 @@ loadAndParseConfigFile(StringRef ConfigFile, 
llvm::vfs::FileSystem *FS,
   return Text;
 }
 
+// Resolves an explicit file: reference in a BasedOnStyle directive to a
+// canonical, absolute path, substituting the value of the CLANG_FORMAT_DIR
+// environment variable if the path starts with $(CLANG_FORMAT_DIR), or 
treating
+// BasedOnFile as relative to the current ConfigFile otherwise.
+static Expected>
+resolveExplicitParentConfigFile(StringRef ConfigFile, StringRef BasedOnFile,
+llvm::vfs::FileSystem *FS) {
+  constexpr const char *EnvVar = "CLANG_FORMAT_DIR";
+  constexpr const char *EnvVarExpansion = "$(CLANG_FORMAT_DIR)";
+  if (BasedOnFile.starts_with(EnvVarExpansion)) {
+const char *ClangFormatDir = getenv(EnvVar);
+if (ClangFormatDir == nullptr) {
+  return make_string_error(ConfigFile + ": 'BasedOnStyle: " + BasedOnFile +
+   "' uses " + EnvVarExpansion + ", but the " +
+   EnvVar + " environment variable is not 
defined");
+} else {
+  auto Status = FS->status(ClangFormatDir);
+  if (!Status ||
+  Status->getType() != llvm::sys::fs::file_type::directory_file) {
+return make_string_error(
+StringRef("Environment variable ") + EnvVar + " = " +
+ClangFormatDir + "does not refer to a valid, readable directory");
+  }
+
+  SmallString<128> FileName{ClangFormatDir};
+  llvm::sys::path::append(FileName,
+  BasedOnFile.substr(strlen(EnvVarExpansion)));
+  ;
+  llvm::sys::path::remove_dots(FileName, true /*remove_dot_dot*/);
+  return FileName;
+}
+  } else {
+SmallString<128> FileName = llvm::sys::path::parent_path(ConfigFile);
+llvm::sys::path::append(FileName, BasedOnFile);
+llvm::sys::path::re