[PATCH] D51575: [clang-tidy] Implement a clang-tidy check to verify Google Objective-C function naming conventions
stephanemoore updated this revision to Diff 163638.
stephanemoore added a comment.
Minor fixes:
- Fixed header guard.
- Removed unnecessary imports in header.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D51575
Files:
clang-tidy/google/CMakeLists.txt
clang-tidy/google/FunctionNamingCheck.cpp
clang-tidy/google/FunctionNamingCheck.h
clang-tidy/google/GoogleTidyModule.cpp
docs/ReleaseNotes.rst
docs/clang-tidy/checks/google-objc-function-naming.rst
docs/clang-tidy/checks/list.rst
test/clang-tidy/google-objc-function-naming.m
unittests/clang-tidy/GoogleModuleTest.cpp
Index: unittests/clang-tidy/GoogleModuleTest.cpp
===
--- unittests/clang-tidy/GoogleModuleTest.cpp
+++ unittests/clang-tidy/GoogleModuleTest.cpp
@@ -1,6 +1,7 @@
#include "ClangTidyTest.h"
#include "google/ExplicitConstructorCheck.h"
#include "google/GlobalNamesInHeadersCheck.h"
+#include "google/FunctionNamingCheck.h"
#include "gtest/gtest.h"
using namespace clang::tidy::google;
@@ -105,6 +106,28 @@
EXPECT_FALSE(runCheckOnCode("namespace {}", "foo.h"));
}
+TEST(ObjCFunctionNaming, AllowedStaticFunctionName) {
+ std::vector Errors;
+ runCheckOnCode(
+ "static void FooBar(void) {}",
+ ,
+ "input.m");
+ EXPECT_EQ(0ul, Errors.size());
+}
+
+TEST(ObjCFunctionNaming, LowerCamelCaseStaticFunctionName) {
+ std::vector Errors;
+ runCheckOnCode(
+ "static void fooBar(void) {}\n",
+ ,
+ "input.m");
+ EXPECT_EQ(1ul, Errors.size());
+ EXPECT_EQ(
+ "function name 'fooBar' not using function naming conventions described by "
+ "Google Objective-C style guide",
+ Errors[0].Message.Message);
+}
+
} // namespace test
} // namespace tidy
} // namespace clang
Index: test/clang-tidy/google-objc-function-naming.m
===
--- /dev/null
+++ test/clang-tidy/google-objc-function-naming.m
@@ -0,0 +1,43 @@
+// RUN: %check_clang_tidy %s google-objc-function-naming %t
+
+typedef _Bool bool;
+
+static bool ispositive(int a) { return a > 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function name 'ispositive' not using function naming conventions described by Google Objective-C style guide
+
+static bool is_positive(int a) { return a > 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function name 'is_positive' not using function naming conventions described by Google Objective-C style guide
+
+static bool isPositive(int a) { return a > 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function name 'isPositive' not using function naming conventions described by Google Objective-C style guide
+
+static bool Is_Positive(int a) { return a > 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function name 'Is_Positive' not using function naming conventions described by Google Objective-C style guide
+
+static bool IsPositive(int a) { return a > 0; }
+
+static const char *md5(const char *str) { return 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: function name 'md5' not using function naming conventions described by Google Objective-C style guide
+
+static const char *MD5(const char *str) { return 0; }
+
+static const char *URL(void) { return "https://clang.llvm.org/;; }
+
+static const char *DEFURL(void) { return "https://clang.llvm.org/;; }
+
+static const char *DEFFooURL(void) { return "https://clang.llvm.org/;; }
+
+static const char *StringFromNSString(id str) { return ""; }
+
+bool ispalindrome(const char *str);
+
+void ABLog_String(const char *str) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function name 'ABLog_String' not using function naming conventions described by Google Objective-C style guide
+
+void ABLogString(const char *str) {}
+
+const char *ABURL(void) { return "https://clang.llvm.org/;; }
+
+const char *ABFooURL(void) { return "https://clang.llvm.org/;; }
+
+int main(int argc, const char **argv) { return 0; }
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -118,6 +118,7 @@
google-explicit-constructor
google-global-names-in-headers
google-objc-avoid-throwing-exception
+ google-objc-function-naming
google-objc-global-variable-declaration
google-readability-braces-around-statements (redirects to readability-braces-around-statements)
google-readability-casting
Index: docs/clang-tidy/checks/google-objc-function-naming.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/google-objc-function-naming.rst
@@ -0,0 +1,29 @@
+.. title:: clang-tidy - google-objc-function-naming
+
+google-objc-function-naming
+===
+
+Finds function definitions in Objective-C files that do not follow the pattern
+described in the Google Objective-C Style Guide.
+
+The corresponding style guide rule can be
[PATCH] D51575: [clang-tidy] Implement a clang-tidy check to verify Google Objective-C function naming conventions
stephanemoore created this revision.
Herald added subscribers: cfe-commits, xazax.hun, mgorny.
§1 Description
This check finds function names in function definitions in Objective-C files
that do not follow the naming pattern described in the Google Objective-C Style
Guide. Function names should be in UpperCamelCase and functions that are not of
static storage class should have an appropriate prefix as described in the
Google Objective-C Style Guide. The function `main` is a notable exception.
Example conforming function definitions:
static bool IsPositive(int i) { return i > 0; }
static bool ABIsPositive(int i) { return i > 0; }
bool ABIsNegative(int i) { return i < 0; }
§2 Test Notes
- Verified clang-tidy tests pass successfully.
- Used check_clang_tidy.py to verify expected output of processing
google-objc-function-naming.m
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D51575
Files:
clang-tidy/google/CMakeLists.txt
clang-tidy/google/FunctionNamingCheck.cpp
clang-tidy/google/FunctionNamingCheck.h
clang-tidy/google/GoogleTidyModule.cpp
docs/ReleaseNotes.rst
docs/clang-tidy/checks/google-objc-function-naming.rst
docs/clang-tidy/checks/list.rst
test/clang-tidy/google-objc-function-naming.m
unittests/clang-tidy/GoogleModuleTest.cpp
Index: unittests/clang-tidy/GoogleModuleTest.cpp
===
--- unittests/clang-tidy/GoogleModuleTest.cpp
+++ unittests/clang-tidy/GoogleModuleTest.cpp
@@ -1,6 +1,7 @@
#include "ClangTidyTest.h"
#include "google/ExplicitConstructorCheck.h"
#include "google/GlobalNamesInHeadersCheck.h"
+#include "google/FunctionNamingCheck.h"
#include "gtest/gtest.h"
using namespace clang::tidy::google;
@@ -105,6 +106,28 @@
EXPECT_FALSE(runCheckOnCode("namespace {}", "foo.h"));
}
+TEST(ObjCFunctionNaming, AllowedStaticFunctionName) {
+ std::vector Errors;
+ runCheckOnCode(
+ "static void FooBar(void) {}",
+ ,
+ "input.m");
+ EXPECT_EQ(0ul, Errors.size());
+}
+
+TEST(ObjCFunctionNaming, LowerCamelCaseStaticFunctionName) {
+ std::vector Errors;
+ runCheckOnCode(
+ "static void fooBar(void) {}\n",
+ ,
+ "input.m");
+ EXPECT_EQ(1ul, Errors.size());
+ EXPECT_EQ(
+ "function name 'fooBar' not using function naming conventions described by "
+ "Google Objective-C style guide",
+ Errors[0].Message.Message);
+}
+
} // namespace test
} // namespace tidy
} // namespace clang
Index: test/clang-tidy/google-objc-function-naming.m
===
--- /dev/null
+++ test/clang-tidy/google-objc-function-naming.m
@@ -0,0 +1,43 @@
+// RUN: %check_clang_tidy %s google-objc-function-naming %t
+
+typedef _Bool bool;
+
+static bool ispositive(int a) { return a > 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function name 'ispositive' not using function naming conventions described by Google Objective-C style guide
+
+static bool is_positive(int a) { return a > 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function name 'is_positive' not using function naming conventions described by Google Objective-C style guide
+
+static bool isPositive(int a) { return a > 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function name 'isPositive' not using function naming conventions described by Google Objective-C style guide
+
+static bool Is_Positive(int a) { return a > 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function name 'Is_Positive' not using function naming conventions described by Google Objective-C style guide
+
+static bool IsPositive(int a) { return a > 0; }
+
+static const char *md5(const char *str) { return 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: function name 'md5' not using function naming conventions described by Google Objective-C style guide
+
+static const char *MD5(const char *str) { return 0; }
+
+static const char *URL(void) { return "https://clang.llvm.org/;; }
+
+static const char *DEFURL(void) { return "https://clang.llvm.org/;; }
+
+static const char *DEFFooURL(void) { return "https://clang.llvm.org/;; }
+
+static const char *StringFromNSString(id str) { return ""; }
+
+bool ispalindrome(const char *str);
+
+void ABLog_String(const char *str) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function name 'ABLog_String' not using function naming conventions described by Google Objective-C style guide
+
+void ABLogString(const char *str) {}
+
+const char *ABURL(void) { return "https://clang.llvm.org/;; }
+
+const char *ABFooURL(void) { return "https://clang.llvm.org/;; }
+
+int main(int argc, const char **argv) { return 0; }
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -118,6 +118,7 @@
google-explicit-constructor
google-global-names-in-headers
google-objc-avoid-throwing-exception
+
[PATCH] D51573: [Driver] Search LibraryPaths when handling -print-file-name
mcgrathr accepted this revision.
mcgrathr added a comment.
This revision is now accepted and ready to land.
The log message could give concrete examples.
Comment at: clang/lib/Driver/Driver.cpp:4169
std::string Driver::GetFilePath(StringRef Name, const ToolChain ) const {
- // Respect a limited subset of the '-Bprefix' functionality in GCC by
- // attempting to use this prefix when looking for file paths.
- for (const std::string : PrefixDirs) {
-if (Dir.empty())
- continue;
-SmallString<128> P(Dir[0] == '=' ? SysRoot + Dir.substr(1) : Dir);
-llvm::sys::path::append(P, Name);
-if (llvm::sys::fs::exists(Twine(P)))
- return P.str();
- }
+ auto FindPath = [&](const llvm::SmallVectorImpl )
+ -> llvm::Optional {
A one-line comment before the lambda would help: `// Check for Name in Path.`
(rename the param to be clearer)
Comment at: clang/lib/Driver/Driver.cpp:4173
+// attempting to use this prefix when looking for file paths.
+for (const std::string : P) {
+ if (Dir.empty())
I'd always use `const auto&` in a range-for like this, but I guess this was
just moved around and maybe LLVM style frowns on auto?
Comment at: clang/lib/Driver/Driver.cpp:4179
+ if (llvm::sys::fs::exists(Twine(P)))
+return std::string(P.str());
+}
Can this be `return {P.str()};`?
Comment at: clang/lib/Driver/Driver.cpp:4184
+
+ if (Optional D = FindPath(PrefixDirs))
+return *D;
I'd always use auto in these situations (here and below).
Comment at: clang/lib/Driver/Driver.cpp:4187
SmallString<128> R(ResourceDir);
llvm::sys::path::append(R, Name);
You could fold these cases into the lambda too by having it take a bool
UseSysRoot and passing `{ResourceDir}, false`.
Then the main body is a uniform and concise list of paths to check.
Repository:
rC Clang
https://reviews.llvm.org/D51573
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51573: [Driver] Search LibraryPaths when handling -print-file-name
phosek updated this revision to Diff 163633.
Repository:
rC Clang
https://reviews.llvm.org/D51573
Files:
clang/lib/Driver/Driver.cpp
clang/test/Driver/linux-per-target-runtime-dir.c
Index: clang/test/Driver/linux-per-target-runtime-dir.c
===
--- clang/test/Driver/linux-per-target-runtime-dir.c
+++ clang/test/Driver/linux-per-target-runtime-dir.c
@@ -19,3 +19,9 @@
// RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
// RUN: | FileCheck --check-prefix=CHECK-CLANGRT-X8664 %s
// CHECK-CLANGRT-X8664:
x86_64-linux-gnu{{/|\\}}lib{{/|\\}}libclang_rt.builtins.a
+
+// RUN: %clang -rtlib=compiler-rt -print-file-name=libclang_rt.builtins.a 2>&1
\
+// RUN: --target=x86_64-linux-gnu \
+// RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
+// RUN: | FileCheck --check-prefix=CHECK-FILE-NAME-X8664 %s
+// CHECK-FILE-NAME-X8664:
x86_64-linux-gnu{{/|\\}}lib{{/|\\}}libclang_rt.builtins.a
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -4166,16 +4166,23 @@
}
std::string Driver::GetFilePath(StringRef Name, const ToolChain ) const {
- // Respect a limited subset of the '-Bprefix' functionality in GCC by
- // attempting to use this prefix when looking for file paths.
- for (const std::string : PrefixDirs) {
-if (Dir.empty())
- continue;
-SmallString<128> P(Dir[0] == '=' ? SysRoot + Dir.substr(1) : Dir);
-llvm::sys::path::append(P, Name);
-if (llvm::sys::fs::exists(Twine(P)))
- return P.str();
- }
+ auto FindPath = [&](const llvm::SmallVectorImpl )
+ -> llvm::Optional {
+// Respect a limited subset of the '-Bprefix' functionality in GCC by
+// attempting to use this prefix when looking for file paths.
+for (const std::string : P) {
+ if (Dir.empty())
+continue;
+ SmallString<128> P(Dir[0] == '=' ? SysRoot + Dir.substr(1) : Dir);
+ llvm::sys::path::append(P, Name);
+ if (llvm::sys::fs::exists(Twine(P)))
+return std::string(P.str());
+}
+return None;
+ };
+
+ if (Optional D = FindPath(PrefixDirs))
+return *D;
SmallString<128> R(ResourceDir);
llvm::sys::path::append(R, Name);
@@ -4187,14 +4194,11 @@
if (llvm::sys::fs::exists(Twine(P)))
return P.str();
- for (const std::string : TC.getFilePaths()) {
-if (Dir.empty())
- continue;
-SmallString<128> P(Dir[0] == '=' ? SysRoot + Dir.substr(1) : Dir);
-llvm::sys::path::append(P, Name);
-if (llvm::sys::fs::exists(Twine(P)))
- return P.str();
- }
+ if (Optional D = FindPath(TC.getLibraryPaths()))
+return *D;
+
+ if (Optional D = FindPath(TC.getFilePaths()))
+return *D;
return Name;
}
Index: clang/test/Driver/linux-per-target-runtime-dir.c
===
--- clang/test/Driver/linux-per-target-runtime-dir.c
+++ clang/test/Driver/linux-per-target-runtime-dir.c
@@ -19,3 +19,9 @@
// RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
// RUN: | FileCheck --check-prefix=CHECK-CLANGRT-X8664 %s
// CHECK-CLANGRT-X8664: x86_64-linux-gnu{{/|\\}}lib{{/|\\}}libclang_rt.builtins.a
+
+// RUN: %clang -rtlib=compiler-rt -print-file-name=libclang_rt.builtins.a 2>&1 \
+// RUN: --target=x86_64-linux-gnu \
+// RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
+// RUN: | FileCheck --check-prefix=CHECK-FILE-NAME-X8664 %s
+// CHECK-FILE-NAME-X8664: x86_64-linux-gnu{{/|\\}}lib{{/|\\}}libclang_rt.builtins.a
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -4166,16 +4166,23 @@
}
std::string Driver::GetFilePath(StringRef Name, const ToolChain ) const {
- // Respect a limited subset of the '-Bprefix' functionality in GCC by
- // attempting to use this prefix when looking for file paths.
- for (const std::string : PrefixDirs) {
-if (Dir.empty())
- continue;
-SmallString<128> P(Dir[0] == '=' ? SysRoot + Dir.substr(1) : Dir);
-llvm::sys::path::append(P, Name);
-if (llvm::sys::fs::exists(Twine(P)))
- return P.str();
- }
+ auto FindPath = [&](const llvm::SmallVectorImpl )
+ -> llvm::Optional {
+// Respect a limited subset of the '-Bprefix' functionality in GCC by
+// attempting to use this prefix when looking for file paths.
+for (const std::string : P) {
+ if (Dir.empty())
+continue;
+ SmallString<128> P(Dir[0] == '=' ? SysRoot + Dir.substr(1) : Dir);
+ llvm::sys::path::append(P, Name);
+ if (llvm::sys::fs::exists(Twine(P)))
+return std::string(P.str());
+}
+return None;
+ };
+
+ if (Optional D = FindPath(PrefixDirs))
+return *D;
SmallString<128> R(ResourceDir);
[PATCH] D51573: [Driver] Search LibraryPaths when handling -print-file-name
phosek updated this revision to Diff 163630.
Repository:
rC Clang
https://reviews.llvm.org/D51573
Files:
clang/lib/Driver/Driver.cpp
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -4166,16 +4166,23 @@
}
std::string Driver::GetFilePath(StringRef Name, const ToolChain ) const {
- // Respect a limited subset of the '-Bprefix' functionality in GCC by
- // attempting to use this prefix when looking for file paths.
- for (const std::string : PrefixDirs) {
-if (Dir.empty())
- continue;
-SmallString<128> P(Dir[0] == '=' ? SysRoot + Dir.substr(1) : Dir);
-llvm::sys::path::append(P, Name);
-if (llvm::sys::fs::exists(Twine(P)))
- return P.str();
- }
+ auto FindPath = [&](const llvm::SmallVectorImpl )
+ -> llvm::Optional {
+// Respect a limited subset of the '-Bprefix' functionality in GCC by
+// attempting to use this prefix when looking for file paths.
+for (const std::string : P) {
+ if (Dir.empty())
+continue;
+ SmallString<128> P(Dir[0] == '=' ? SysRoot + Dir.substr(1) : Dir);
+ llvm::sys::path::append(P, Name);
+ if (llvm::sys::fs::exists(Twine(P)))
+return std::string(P.str());
+}
+return None;
+ };
+
+ if (Optional D = FindPath(PrefixDirs))
+return *D;
SmallString<128> R(ResourceDir);
llvm::sys::path::append(R, Name);
@@ -4187,14 +4194,11 @@
if (llvm::sys::fs::exists(Twine(P)))
return P.str();
- for (const std::string : TC.getFilePaths()) {
-if (Dir.empty())
- continue;
-SmallString<128> P(Dir[0] == '=' ? SysRoot + Dir.substr(1) : Dir);
-llvm::sys::path::append(P, Name);
-if (llvm::sys::fs::exists(Twine(P)))
- return P.str();
- }
+ if (Optional D = FindPath(TC.getLibraryPaths()))
+return *D;
+
+ if (Optional D = FindPath(TC.getFilePaths()))
+return *D;
return Name;
}
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -4166,16 +4166,23 @@
}
std::string Driver::GetFilePath(StringRef Name, const ToolChain ) const {
- // Respect a limited subset of the '-Bprefix' functionality in GCC by
- // attempting to use this prefix when looking for file paths.
- for (const std::string : PrefixDirs) {
-if (Dir.empty())
- continue;
-SmallString<128> P(Dir[0] == '=' ? SysRoot + Dir.substr(1) : Dir);
-llvm::sys::path::append(P, Name);
-if (llvm::sys::fs::exists(Twine(P)))
- return P.str();
- }
+ auto FindPath = [&](const llvm::SmallVectorImpl )
+ -> llvm::Optional {
+// Respect a limited subset of the '-Bprefix' functionality in GCC by
+// attempting to use this prefix when looking for file paths.
+for (const std::string : P) {
+ if (Dir.empty())
+continue;
+ SmallString<128> P(Dir[0] == '=' ? SysRoot + Dir.substr(1) : Dir);
+ llvm::sys::path::append(P, Name);
+ if (llvm::sys::fs::exists(Twine(P)))
+return std::string(P.str());
+}
+return None;
+ };
+
+ if (Optional D = FindPath(PrefixDirs))
+return *D;
SmallString<128> R(ResourceDir);
llvm::sys::path::append(R, Name);
@@ -4187,14 +4194,11 @@
if (llvm::sys::fs::exists(Twine(P)))
return P.str();
- for (const std::string : TC.getFilePaths()) {
-if (Dir.empty())
- continue;
-SmallString<128> P(Dir[0] == '=' ? SysRoot + Dir.substr(1) : Dir);
-llvm::sys::path::append(P, Name);
-if (llvm::sys::fs::exists(Twine(P)))
- return P.str();
- }
+ if (Optional D = FindPath(TC.getLibraryPaths()))
+return *D;
+
+ if (Optional D = FindPath(TC.getFilePaths()))
+return *D;
return Name;
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51573: [Driver] Search LibraryPaths when handling -print-file-name
phosek created this revision.
phosek added a reviewer: beanz.
Herald added a subscriber: cfe-commits.
This is necessary to handle the multiarch runtime directories.
Repository:
rC Clang
https://reviews.llvm.org/D51573
Files:
clang/lib/Driver/Driver.cpp
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -4166,16 +4166,23 @@
}
std::string Driver::GetFilePath(StringRef Name, const ToolChain ) const {
+ auto FindPath = [&](const llvm::SmallVectorImpl )
+ -> llvm::Optional {
+for (const std::string : P) {
+ if (Dir.empty())
+continue;
+ SmallString<128> P(Dir[0] == '=' ? SysRoot + Dir.substr(1) : Dir);
+ llvm::sys::path::append(P, Name);
+ if (llvm::sys::fs::exists(Twine(P)))
+return std::string(P.str());
+}
+return None;
+ };
+
// Respect a limited subset of the '-Bprefix' functionality in GCC by
// attempting to use this prefix when looking for file paths.
- for (const std::string : PrefixDirs) {
-if (Dir.empty())
- continue;
-SmallString<128> P(Dir[0] == '=' ? SysRoot + Dir.substr(1) : Dir);
-llvm::sys::path::append(P, Name);
-if (llvm::sys::fs::exists(Twine(P)))
- return P.str();
- }
+ if (Optional D = FindPath(PrefixDirs))
+return *D;
SmallString<128> R(ResourceDir);
llvm::sys::path::append(R, Name);
@@ -4187,14 +4194,11 @@
if (llvm::sys::fs::exists(Twine(P)))
return P.str();
- for (const std::string : TC.getFilePaths()) {
-if (Dir.empty())
- continue;
-SmallString<128> P(Dir[0] == '=' ? SysRoot + Dir.substr(1) : Dir);
-llvm::sys::path::append(P, Name);
-if (llvm::sys::fs::exists(Twine(P)))
- return P.str();
- }
+ if (Optional D = FindPath(TC.getLibraryPaths()))
+return *D;
+
+ if (Optional D = FindPath(TC.getFilePaths()))
+return *D;
return Name;
}
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -4166,16 +4166,23 @@
}
std::string Driver::GetFilePath(StringRef Name, const ToolChain ) const {
+ auto FindPath = [&](const llvm::SmallVectorImpl )
+ -> llvm::Optional {
+for (const std::string : P) {
+ if (Dir.empty())
+continue;
+ SmallString<128> P(Dir[0] == '=' ? SysRoot + Dir.substr(1) : Dir);
+ llvm::sys::path::append(P, Name);
+ if (llvm::sys::fs::exists(Twine(P)))
+return std::string(P.str());
+}
+return None;
+ };
+
// Respect a limited subset of the '-Bprefix' functionality in GCC by
// attempting to use this prefix when looking for file paths.
- for (const std::string : PrefixDirs) {
-if (Dir.empty())
- continue;
-SmallString<128> P(Dir[0] == '=' ? SysRoot + Dir.substr(1) : Dir);
-llvm::sys::path::append(P, Name);
-if (llvm::sys::fs::exists(Twine(P)))
- return P.str();
- }
+ if (Optional D = FindPath(PrefixDirs))
+return *D;
SmallString<128> R(ResourceDir);
llvm::sys::path::append(R, Name);
@@ -4187,14 +4194,11 @@
if (llvm::sys::fs::exists(Twine(P)))
return P.str();
- for (const std::string : TC.getFilePaths()) {
-if (Dir.empty())
- continue;
-SmallString<128> P(Dir[0] == '=' ? SysRoot + Dir.substr(1) : Dir);
-llvm::sys::path::append(P, Name);
-if (llvm::sys::fs::exists(Twine(P)))
- return P.str();
- }
+ if (Optional D = FindPath(TC.getLibraryPaths()))
+return *D;
+
+ if (Optional D = FindPath(TC.getFilePaths()))
+return *D;
return Name;
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44609: [Clang-Format] New option BeforeLambdaBody to manage lambda line break inside function parameter call (in Allman style)
Wawha marked 2 inline comments as done.
Wawha added inline comments.
Comment at: lib/Format/ContinuationIndenter.cpp:1307
+ (Style.BraceWrapping.BeforeLambdaBody && Current.Next != nullptr &&
+Current.Next->is(TT_LambdaLSquare)));
State.Stack.back().IsInsideObjCArrayLiteral =
klimek wrote:
> klimek wrote:
> > I think I misunderstood some of this the first time around (and thanks for
> > bearing with me :) - iiuc we want to break for Allman even when there's
> > only one nested block. I think I'll need to take another look when I'm back
> > from vacation, unless Daniel or somebody else finds time before then (will
> > be back in 1.5 weeks)
> So, HasMultipleNestedBlocks should only be true if there are multiple nested
> blocks.
> What tests break if you remove this change to HasMultipleNestedBlocks?
All cases with only one lambda in parameter are break. The Lambda body with be
put on the same line as the function and aligned with [] instead of putting the
body [] on another line with just one simple indentation.
So if restore the line to :
```
State.Stack.back().HasMultipleNestedBlocks = Current.BlockParameterCount > 1;
```
Here some cases.
FormatTest.cpp, ligne 11412:
```
// With my modification
FunctionWithOneParam(
[]()
{
// A cool function...
return 43;
});
// Without my modification
FunctionWithOneParam([]()
{
// A cool function...
return 43;
});
```
The "{" block of the lambda will be aligned on the "[" depending of the
function name length.
You have the same case with multiple lambdas (FormatTest.cpp, ligne 11433):
```
// With my modification
TwoNestedLambdas(
[]()
{
return Call(
[]()
{
return 17;
});
});
// Without my modification
TwoNestedLambdas([]()
{
return Call([]()
{
return 17;
});
});
```
Repository:
rC Clang
https://reviews.llvm.org/D44609
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50850: clang: Add triples support for MIPS r6
wzssyqa updated this revision to Diff 163624. https://reviews.llvm.org/D50850 Files: lib/Driver/ToolChains/Arch/Mips.cpp lib/Driver/ToolChains/Gnu.cpp lib/Driver/ToolChains/Linux.cpp test/CodeGen/atomics-inlining.c test/CodeGen/mips-zero-sized-struct.c test/CodeGen/target-data.c test/CodeGen/xray-attributes-supported.cpp test/Driver/clang-translation.c Index: test/Driver/clang-translation.c === --- test/Driver/clang-translation.c +++ test/Driver/clang-translation.c @@ -291,13 +291,27 @@ // MIPS: "-target-cpu" "mips32r2" // MIPS: "-mfloat-abi" "hard" +// RUN: %clang -target mipsisa32r6-linux-gnu -### -S %s 2>&1 | \ +// RUN: FileCheck -check-prefix=MIPSR6 %s +// MIPSR6: clang +// MIPSR6: "-cc1" +// MIPSR6: "-target-cpu" "mips32r6" +// MIPSR6: "-mfloat-abi" "hard" + // RUN: %clang -target mipsel-linux-gnu -### -S %s 2>&1 | \ // RUN: FileCheck -check-prefix=MIPSEL %s // MIPSEL: clang // MIPSEL: "-cc1" // MIPSEL: "-target-cpu" "mips32r2" // MIPSEL: "-mfloat-abi" "hard" +// RUN: %clang -target mipsisa32r6el-linux-gnu -### -S %s 2>&1 | \ +// RUN: FileCheck -check-prefix=MIPSR6EL %s +// MIPSR6EL: clang +// MIPSR6EL: "-cc1" +// MIPSR6EL: "-target-cpu" "mips32r6" +// MIPSR6EL: "-mfloat-abi" "hard" + // RUN: %clang -target mipsel-linux-android -### -S %s 2>&1 | \ // RUN: FileCheck -check-prefix=MIPSEL-ANDROID %s // MIPSEL-ANDROID: clang @@ -323,45 +337,91 @@ // MIPS64: "-target-cpu" "mips64r2" // MIPS64: "-mfloat-abi" "hard" +// RUN: %clang -target mipsisa64r6-linux-gnu -### -S %s 2>&1 | \ +// RUN: FileCheck -check-prefix=MIPS64R6 %s +// MIPS64R6: clang +// MIPS64R6: "-cc1" +// MIPS64R6: "-target-cpu" "mips64r6" +// MIPS64R6: "-mfloat-abi" "hard" + // RUN: %clang -target mips64el-linux-gnu -### -S %s 2>&1 | \ // RUN: FileCheck -check-prefix=MIPS64EL %s // MIPS64EL: clang // MIPS64EL: "-cc1" // MIPS64EL: "-target-cpu" "mips64r2" // MIPS64EL: "-mfloat-abi" "hard" +// RUN: %clang -target mipsisa64r6el-linux-gnu -### -S %s 2>&1 | \ +// RUN: FileCheck -check-prefix=MIPS64R6EL %s +// MIPS64R6EL: clang +// MIPS64R6EL: "-cc1" +// MIPS64R6EL: "-target-cpu" "mips64r6" +// MIPS64R6EL: "-mfloat-abi" "hard" + // RUN: %clang -target mips64-linux-gnuabi64 -### -S %s 2>&1 | \ // RUN: FileCheck -check-prefix=MIPS64-GNUABI64 %s // MIPS64-GNUABI64: clang // MIPS64-GNUABI64: "-cc1" // MIPS64-GNUABI64: "-target-cpu" "mips64r2" // MIPS64-GNUABI64: "-target-abi" "n64" // MIPS64-GNUABI64: "-mfloat-abi" "hard" +// RUN: %clang -target mipsisa64r6-linux-gnuabi64 -### -S %s 2>&1 | \ +// RUN: FileCheck -check-prefix=MIPS64R6-GNUABI64 %s +// MIPS64R6-GNUABI64: clang +// MIPS64R6-GNUABI64: "-cc1" +// MIPS64R6-GNUABI64: "-target-cpu" "mips64r6" +// MIPS64R6-GNUABI64: "-target-abi" "n64" +// MIPS64R6-GNUABI64: "-mfloat-abi" "hard" + // RUN: %clang -target mips64el-linux-gnuabi64 -### -S %s 2>&1 | \ // RUN: FileCheck -check-prefix=MIPS64EL-GNUABI64 %s // MIPS64EL-GNUABI64: clang // MIPS64EL-GNUABI64: "-cc1" // MIPS64EL-GNUABI64: "-target-cpu" "mips64r2" // MIPS64EL-GNUABI64: "-target-abi" "n64" // MIPS64EL-GNUABI64: "-mfloat-abi" "hard" +// RUN: %clang -target mipsisa64r6el-linux-gnuabi64 -### -S %s 2>&1 | \ +// RUN: FileCheck -check-prefix=MIPS64R6EL-GNUABI64 %s +// MIPS64R6EL-GNUABI64: clang +// MIPS64R6EL-GNUABI64: "-cc1" +// MIPS64R6EL-GNUABI64: "-target-cpu" "mips64r6" +// MIPS64R6EL-GNUABI64: "-target-abi" "n64" +// MIPS64R6EL-GNUABI64: "-mfloat-abi" "hard" + // RUN: %clang -target mips64-linux-gnuabin32 -### -S %s 2>&1 | \ // RUN: FileCheck -check-prefix=MIPSN32 %s // MIPSN32: clang // MIPSN32: "-cc1" // MIPSN32: "-target-cpu" "mips64r2" // MIPSN32: "-target-abi" "n32" // MIPSN32: "-mfloat-abi" "hard" +// RUN: %clang -target mipsisa64r6-linux-gnuabin32 -### -S %s 2>&1 | \ +// RUN: FileCheck -check-prefix=MIPSN32R6 %s +// MIPSN32R6: clang +// MIPSN32R6: "-cc1" +// MIPSN32R6: "-target-cpu" "mips64r6" +// MIPSN32R6: "-target-abi" "n32" +// MIPSN32R6: "-mfloat-abi" "hard" + // RUN: %clang -target mips64el-linux-gnuabin32 -### -S %s 2>&1 | \ // RUN: FileCheck -check-prefix=MIPSN32EL %s // MIPSN32EL: clang // MIPSN32EL: "-cc1" // MIPSN32EL: "-target-cpu" "mips64r2" // MIPSN32EL: "-target-abi" "n32" // MIPSN32EL: "-mfloat-abi" "hard" +// RUN: %clang -target mipsisa64r6el-linux-gnuabin32 -### -S %s 2>&1 | \ +// RUN: FileCheck -check-prefix=MIPSN32R6EL %s +// MIPSN32R6EL: clang +// MIPSN32R6EL: "-cc1" +// MIPSN32R6EL: "-target-cpu" "mips64r6" +// MIPSN32R6EL: "-target-abi" "n32" +// MIPSN32R6EL: "-mfloat-abi" "hard" + // RUN: %clang -target mips64el-linux-android -### -S %s 2>&1 | \ // RUN: FileCheck -check-prefix=MIPS64EL-ANDROID %s // MIPS64EL-ANDROID: clang Index: test/CodeGen/xray-attributes-supported.cpp === --- test/CodeGen/xray-attributes-supported.cpp +++ test/CodeGen/xray-attributes-supported.cpp @@ -1,13 +1,21 @@ // RUN: %clang_cc1
[PATCH] D51464: clang: fix MIPS/N32 triple and paths
wzssyqa updated this revision to Diff 163622. wzssyqa added a comment. Remove unused MipsCpu. Repository: rC Clang https://reviews.llvm.org/D51464 Files: lib/Basic/Targets/Mips.h lib/Driver/ToolChains/Arch/Mips.cpp lib/Driver/ToolChains/Gnu.cpp lib/Driver/ToolChains/Linux.cpp test/CodeGen/atomics-inlining.c test/CodeGen/mips-zero-sized-struct.c test/CodeGen/target-data.c test/CodeGen/xray-attributes-supported.cpp test/Driver/clang-translation.c Index: test/Driver/clang-translation.c === --- test/Driver/clang-translation.c +++ test/Driver/clang-translation.c @@ -330,6 +330,38 @@ // MIPS64EL: "-target-cpu" "mips64r2" // MIPS64EL: "-mfloat-abi" "hard" +// RUN: %clang -target mips64-linux-gnuabi64 -### -S %s 2>&1 | \ +// RUN: FileCheck -check-prefix=MIPS64-GNUABI64 %s +// MIPS64-GNUABI64: clang +// MIPS64-GNUABI64: "-cc1" +// MIPS64-GNUABI64: "-target-cpu" "mips64r2" +// MIPS64-GNUABI64: "-target-abi" "n64" +// MIPS64-GNUABI64: "-mfloat-abi" "hard" + +// RUN: %clang -target mips64el-linux-gnuabi64 -### -S %s 2>&1 | \ +// RUN: FileCheck -check-prefix=MIPS64EL-GNUABI64 %s +// MIPS64EL-GNUABI64: clang +// MIPS64EL-GNUABI64: "-cc1" +// MIPS64EL-GNUABI64: "-target-cpu" "mips64r2" +// MIPS64EL-GNUABI64: "-target-abi" "n64" +// MIPS64EL-GNUABI64: "-mfloat-abi" "hard" + +// RUN: %clang -target mips64-linux-gnuabin32 -### -S %s 2>&1 | \ +// RUN: FileCheck -check-prefix=MIPSN32 %s +// MIPSN32: clang +// MIPSN32: "-cc1" +// MIPSN32: "-target-cpu" "mips64r2" +// MIPSN32: "-target-abi" "n32" +// MIPSN32: "-mfloat-abi" "hard" + +// RUN: %clang -target mips64el-linux-gnuabin32 -### -S %s 2>&1 | \ +// RUN: FileCheck -check-prefix=MIPSN32EL %s +// MIPSN32EL: clang +// MIPSN32EL: "-cc1" +// MIPSN32EL: "-target-cpu" "mips64r2" +// MIPSN32EL: "-target-abi" "n32" +// MIPSN32EL: "-mfloat-abi" "hard" + // RUN: %clang -target mips64el-linux-android -### -S %s 2>&1 | \ // RUN: FileCheck -check-prefix=MIPS64EL-ANDROID %s // MIPS64EL-ANDROID: clang Index: test/CodeGen/xray-attributes-supported.cpp === --- test/CodeGen/xray-attributes-supported.cpp +++ test/CodeGen/xray-attributes-supported.cpp @@ -4,6 +4,10 @@ // RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - -triple mipsel-unknown-linux-gnu | FileCheck %s // RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - -triple mips64-unknown-linux-gnu | FileCheck %s // RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - -triple mips64el-unknown-linux-gnu | FileCheck %s +// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - -triple mips64-unknown-linux-gnuabi64 | FileCheck %s +// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - -triple mips64el-unknown-linux-gnuabi64 | FileCheck %s +// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - -triple mips64-unknown-linux-gnuabin32 | FileCheck %s +// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - -triple mips64el-unknown-linux-gnuabin32 | FileCheck %s // RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - -triple powerpc64le-unknown-linux-gnu | FileCheck %s // Make sure that the LLVM attribute for XRay-annotated functions do show up. Index: test/CodeGen/target-data.c === --- test/CodeGen/target-data.c +++ test/CodeGen/target-data.c @@ -42,18 +42,34 @@ // RUN: FileCheck %s -check-prefix=MIPS-64EL // MIPS-64EL: target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-n32:64-S128" +// RUN: %clang_cc1 -triple mips64el-linux-gnuabi64 -o - -emit-llvm %s | \ +// RUN: FileCheck %s -check-prefix=MIPS-64EL +// MIPS-64EL: target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-n32:64-S128" + // RUN: %clang_cc1 -triple mips64el-linux-gnu -o - -emit-llvm -target-abi n32 \ // RUN: %s | FileCheck %s -check-prefix=MIPS-64EL-N32 // MIPS-64EL-N32: target datalayout = "e-m:e-p:32:32-i8:8:32-i16:16:32-i64:64-n32:64-S128" +// RUN: %clang_cc1 -triple mips64el-linux-gnuabin32 -o - -emit-llvm \ +// RUN: %s | FileCheck %s -check-prefix=MIPS-64EL-N32 +// MIPS-64EL-N32: target datalayout = "e-m:e-p:32:32-i8:8:32-i16:16:32-i64:64-n32:64-S128" + // RUN: %clang_cc1 -triple mips64-linux-gnu -o - -emit-llvm %s | \ // RUN: FileCheck %s -check-prefix=MIPS-64EB // MIPS-64EB: target datalayout = "E-m:e-i8:8:32-i16:16:32-i64:64-n32:64-S128" +// RUN: %clang_cc1 -triple mips64-linux-gnuabi64 -o - -emit-llvm %s | \ +// RUN: FileCheck %s -check-prefix=MIPS-64EB +// MIPS-64EB: target datalayout = "E-m:e-i8:8:32-i16:16:32-i64:64-n32:64-S128" + // RUN: %clang_cc1 -triple mips64-linux-gnu -o - -emit-llvm %s -target-abi n32 \ // RUN: | FileCheck %s -check-prefix=MIPS-64EB-N32 // MIPS-64EB-N32: target datalayout = "E-m:e-p:32:32-i8:8:32-i16:16:32-i64:64-n32:64-S128" +// RUN: %clang_cc1 -triple
[PATCH] D51568: [modules] Add `-fdisable-module-directory` flag for relocatable modules
andrewjcg updated this revision to Diff 163621.
andrewjcg added a comment.
fix umbrella writing
Repository:
rC Clang
https://reviews.llvm.org/D51568
Files:
include/clang/Driver/CC1Options.td
include/clang/Lex/HeaderSearchOptions.h
lib/Frontend/CompilerInvocation.cpp
lib/Serialization/ASTWriter.cpp
test/Modules/relocatable-modules.modulemap
Index: test/Modules/relocatable-modules.modulemap
===
--- /dev/null
+++ test/Modules/relocatable-modules.modulemap
@@ -0,0 +1,29 @@
+// Build two otherwise identical modules in two different directories and
+// verify that using `-fdisable-module-directory` makes them identical.
+//
+// RUN: rm -rf %t
+//
+// RUN: mkdir -p %t/p1
+// RUN: cd %t/p1
+// RUN: mkdir -p main other
+// RUN: grep "" %s > main/a.modulemap
+// RUN: grep "" %s > main/a.h
+// RUN: grep "" %s > other/b.h
+// RUN: %clang_cc1 -x c++ -fmodules -emit-module -fdisable-module-directory \
+// RUN: -fmodule-name="a" -Imain -I. -o - main/a.modulemap > a.pcm
+//
+// RUN: mkdir -p %t/p2
+// RUN: cd %t/p2
+// RUN: mkdir -p main other
+// RUN: grep "" %s > main/a.modulemap
+// RUN: grep "" %s > main/a.h
+// RUN: grep "" %s > other/b.h
+// RUN: %clang_cc1 -x c++ -fmodules -emit-module -fdisable-module-directory \
+// RUN: -fmodule-name="a" -Imain -I. -o - main/a.modulemap > a.pcm
+//
+// RUN: diff %t/p1/a.pcm %t/p2/a.pcm
+
+module "a" {//
+} //
+
+#include "b.h" //
Index: lib/Serialization/ASTWriter.cpp
===
--- lib/Serialization/ASTWriter.cpp
+++ lib/Serialization/ASTWriter.cpp
@@ -1339,9 +1339,14 @@
///
/// \return \c true if the path was changed.
static bool cleanPathForOutput(FileManager ,
- SmallVectorImpl ) {
- bool Changed = FileMgr.makeAbsolutePath(Path);
- return Changed | llvm::sys::path::remove_dots(Path);
+ SmallVectorImpl ,
+ bool MakeAbsolute = true) {
+ bool Changed = false;
+ if (MakeAbsolute) {
+Changed |= FileMgr.makeAbsolutePath(Path);
+ }
+ Changed |= llvm::sys::path::remove_dots(Path);
+ return Changed;
}
/// Adjusts the given filename to only write out the portion of the
@@ -1503,7 +1508,11 @@
Stream.EmitRecordWithBlob(AbbrevCode, Record, WritingModule->Name);
}
- if (WritingModule && WritingModule->Directory) {
+ if (WritingModule &&
+ WritingModule->Directory &&
+ !PP.getHeaderSearchInfo()
+ .getHeaderSearchOpts()
+ .DisableModuleDirectory) {
SmallString<128> BaseDir(WritingModule->Directory->getName());
cleanPathForOutput(Context.getSourceManager().getFileManager(), BaseDir);
@@ -2952,12 +2961,26 @@
// Emit the umbrella header, if there is one.
if (auto UmbrellaHeader = Mod->getUmbrellaHeader()) {
RecordData::value_type Record[] = {SUBMODULE_UMBRELLA_HEADER};
- Stream.EmitRecordWithBlob(UmbrellaAbbrev, Record,
-UmbrellaHeader.NameAsWritten);
+ SmallString<128> Buffer;
+ if (PP->getHeaderSearchInfo()
+ .getHeaderSearchOpts()
+ .DisableModuleDirectory) {
+llvm::sys::path::append(Buffer, Mod->Directory->getName());
+ }
+ llvm::sys::path::append(Buffer, UmbrellaHeader.NameAsWritten);
+ llvm::sys::path::remove_dots(Buffer);
+ Stream.EmitRecordWithBlob(UmbrellaAbbrev, Record, Buffer);
} else if (auto UmbrellaDir = Mod->getUmbrellaDir()) {
RecordData::value_type Record[] = {SUBMODULE_UMBRELLA_DIR};
- Stream.EmitRecordWithBlob(UmbrellaDirAbbrev, Record,
-UmbrellaDir.NameAsWritten);
+ SmallString<128> Buffer;
+ if (PP->getHeaderSearchInfo()
+ .getHeaderSearchOpts()
+ .DisableModuleDirectory) {
+llvm::sys::path::append(Buffer, Mod->Directory->getName());
+ }
+ llvm::sys::path::append(Buffer, UmbrellaDir.NameAsWritten);
+ llvm::sys::path::remove_dots(Buffer);
+ Stream.EmitRecordWithBlob(UmbrellaDirAbbrev, Record, Buffer);
}
// Emit the headers.
@@ -4515,7 +4538,11 @@
assert(Context && "should have context when outputting path");
bool Changed =
- cleanPathForOutput(Context->getSourceManager().getFileManager(), Path);
+ cleanPathForOutput(Context->getSourceManager().getFileManager(),
+ Path,
+ !PP->getHeaderSearchInfo()
+ .getHeaderSearchOpts()
+ .DisableModuleDirectory);
// Remove a prefix to make the path relative, if relevant.
const char *PathBegin = Path.data();
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -1746,6 +1746,7 @@
[PATCH] D51568: [modules] Add `-fdisable-module-directory` flag for relocatable modules
andrewjcg added a comment. I'm not sure this is the best approach, but I wasn't sure of a better one (to support module files w/o absolute paths). Another approach I tried, was relativizing the other input files (from outside the module directory) using chains of `../` (e.g. `../../../../../other/modules/module.modulemap`), but this causes issues when symlinks appear in the module directory path. Another potential option could be to add a bit to the serialized module format for each input, to allow some inputs to mark themselves as relative to the CWD, rather than the module directory. Repository: rC Clang https://reviews.llvm.org/D51568 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51568: [modules] Add `-fdisable-module-directory` flag for relocatable modules
andrewjcg created this revision.
Herald added a subscriber: cfe-commits.
Currently, modules built using module map files embed the path to the directory
that houses their inputs (e.g. headers and module map file) into the output
module file. This path is embedded as an absolute path and the various input
files are either relativized to this path (if they are underneath it) or also
embedded as an absolute path.
This adds a flag which disables use of this absolute module directory path and
instead writes out paths into the module unmodified (e.g. if they were specified
on the command line as relative paths to the CWD, then they remain relative to
the CWD). This allows building modules without any absolute paths, allowing them
to be built and shared between different working directories.
Repository:
rC Clang
https://reviews.llvm.org/D51568
Files:
include/clang/Driver/CC1Options.td
include/clang/Lex/HeaderSearchOptions.h
lib/Frontend/CompilerInvocation.cpp
lib/Serialization/ASTWriter.cpp
test/Modules/relocatable-modules.modulemap
Index: test/Modules/relocatable-modules.modulemap
===
--- /dev/null
+++ test/Modules/relocatable-modules.modulemap
@@ -0,0 +1,29 @@
+// Build two otherwise identical modules in two different directories and
+// verify that using `-fdisable-module-directory` makes them identical.
+//
+// RUN: rm -rf %t
+//
+// RUN: mkdir -p %t/p1
+// RUN: cd %t/p1
+// RUN: mkdir -p main other
+// RUN: grep "" %s > main/a.modulemap
+// RUN: grep "" %s > main/a.h
+// RUN: grep "" %s > other/b.h
+// RUN: %clang_cc1 -x c++ -fmodules -emit-module -fdisable-module-directory \
+// RUN: -fmodule-name="a" -Imain -I. -o - main/a.modulemap > a.pcm
+//
+// RUN: mkdir -p %t/p2
+// RUN: cd %t/p2
+// RUN: mkdir -p main other
+// RUN: grep "" %s > main/a.modulemap
+// RUN: grep "" %s > main/a.h
+// RUN: grep "" %s > other/b.h
+// RUN: %clang_cc1 -x c++ -fmodules -emit-module -fdisable-module-directory \
+// RUN: -fmodule-name="a" -Imain -I. -o - main/a.modulemap > a.pcm
+//
+// RUN: diff %t/p1/a.pcm %t/p2/a.pcm
+
+module "a" {//
+} //
+
+#include "b.h" //
Index: lib/Serialization/ASTWriter.cpp
===
--- lib/Serialization/ASTWriter.cpp
+++ lib/Serialization/ASTWriter.cpp
@@ -1339,9 +1339,14 @@
///
/// \return \c true if the path was changed.
static bool cleanPathForOutput(FileManager ,
- SmallVectorImpl ) {
- bool Changed = FileMgr.makeAbsolutePath(Path);
- return Changed | llvm::sys::path::remove_dots(Path);
+ SmallVectorImpl ,
+ bool MakeAbsolute = true) {
+ bool Changed = false;
+ if (MakeAbsolute) {
+Changed |= FileMgr.makeAbsolutePath(Path);
+ }
+ Changed |= llvm::sys::path::remove_dots(Path);
+ return Changed;
}
/// Adjusts the given filename to only write out the portion of the
@@ -1503,7 +1508,11 @@
Stream.EmitRecordWithBlob(AbbrevCode, Record, WritingModule->Name);
}
- if (WritingModule && WritingModule->Directory) {
+ if (WritingModule &&
+ WritingModule->Directory &&
+ !PP.getHeaderSearchInfo()
+ .getHeaderSearchOpts()
+ .DisableModuleDirectory) {
SmallString<128> BaseDir(WritingModule->Directory->getName());
cleanPathForOutput(Context.getSourceManager().getFileManager(), BaseDir);
@@ -2952,12 +2961,20 @@
// Emit the umbrella header, if there is one.
if (auto UmbrellaHeader = Mod->getUmbrellaHeader()) {
RecordData::value_type Record[] = {SUBMODULE_UMBRELLA_HEADER};
- Stream.EmitRecordWithBlob(UmbrellaAbbrev, Record,
-UmbrellaHeader.NameAsWritten);
+ SmallString<128> Buffer;
+ llvm::sys::path::append(Buffer,
+ Mod->Directory->getName(),
+ UmbrellaHeader.NameAsWritten);
+ llvm::sys::path::remove_dots(Buffer);
+ Stream.EmitRecordWithBlob(UmbrellaAbbrev, Record, Buffer);
} else if (auto UmbrellaDir = Mod->getUmbrellaDir()) {
RecordData::value_type Record[] = {SUBMODULE_UMBRELLA_DIR};
- Stream.EmitRecordWithBlob(UmbrellaDirAbbrev, Record,
-UmbrellaDir.NameAsWritten);
+ SmallString<128> Buffer;
+ llvm::sys::path::append(Buffer,
+ Mod->Directory->getName(),
+ UmbrellaDir.NameAsWritten);
+ llvm::sys::path::remove_dots(Buffer);
+ Stream.EmitRecordWithBlob(UmbrellaDirAbbrev, Record, Buffer);
}
// Emit the headers.
@@ -4515,7 +4532,11 @@
assert(Context && "should have context when outputting path");
bool Changed =
- cleanPathForOutput(Context->getSourceManager().getFileManager(), Path);
+ cleanPathForOutput(Context->getSourceManager().getFileManager(),
+
[clang-tools-extra] r341273 - [clangd] Fix many typos. NFC
Author: maskray
Date: Sat Sep 1 00:47:03 2018
New Revision: 341273
URL: http://llvm.org/viewvc/llvm-project?rev=341273=rev
Log:
[clangd] Fix many typos. NFC
Modified:
clang-tools-extra/trunk/clangd/Logger.h
clang-tools-extra/trunk/clangd/index/FileIndex.h
clang-tools-extra/trunk/clangd/index/Index.cpp
clang-tools-extra/trunk/clangd/index/Index.h
clang-tools-extra/trunk/clangd/index/MemIndex.cpp
clang-tools-extra/trunk/clangd/index/Merge.cpp
clang-tools-extra/trunk/clangd/index/Merge.h
clang-tools-extra/trunk/clangd/index/SymbolCollector.h
clang-tools-extra/trunk/clangd/index/dex/Trigram.h
Modified: clang-tools-extra/trunk/clangd/Logger.h
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Logger.h?rev=341273=341272=341273=diff
==
--- clang-tools-extra/trunk/clangd/Logger.h (original)
+++ clang-tools-extra/trunk/clangd/Logger.h Sat Sep 1 00:47:03 2018
@@ -56,7 +56,7 @@ void log(Logger::Level L, const char *Fm
template void elog(const char *Fmt, Ts &&... Vals) {
detail::log(Logger::Error, Fmt, std::forward(Vals)...);
}
-// log() is used for information important to understanding a clangd session.
+// log() is used for information important to understand a clangd session.
// e.g. the names of LSP messages sent are logged at this level.
// This level could be enabled in production builds to allow later inspection.
template void log(const char *Fmt, Ts &&... Vals) {
Modified: clang-tools-extra/trunk/clangd/index/FileIndex.h
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/FileIndex.h?rev=341273=341272=341273=diff
==
--- clang-tools-extra/trunk/clangd/index/FileIndex.h (original)
+++ clang-tools-extra/trunk/clangd/index/FileIndex.h Sat Sep 1 00:47:03 2018
@@ -36,7 +36,7 @@ namespace clangd {
/// the snapshot, either this class or the symbol index.
///
/// The snapshot semantics keeps critical sections minimal since we only need
-/// locking when we swap or obtain refereces to snapshots.
+/// locking when we swap or obtain references to snapshots.
class FileSymbols {
public:
/// \brief Updates all symbols and occurrences in a file.
@@ -60,7 +60,7 @@ private:
llvm::StringMap> FileToOccurrenceSlabs;
};
-/// \brief This manages symbls from files and an in-memory index on all
symbols.
+/// \brief This manages symbols from files and an in-memory index on all
symbols.
class FileIndex : public SymbolIndex {
public:
/// If URISchemes is empty, the default schemes in SymbolCollector will be
Modified: clang-tools-extra/trunk/clangd/index/Index.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Index.cpp?rev=341273=341272=341273=diff
==
--- clang-tools-extra/trunk/clangd/index/Index.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/Index.cpp Sat Sep 1 00:47:03 2018
@@ -152,7 +152,7 @@ void SymbolOccurrenceSlab::insert(const
}
void SymbolOccurrenceSlab::freeze() {
- // Deduplicate symbol occurrenes.
+ // Deduplicate symbol occurrences.
for (auto : Occurrences) {
auto = IDAndOccurrence.getSecond();
std::sort(Occurrence.begin(), Occurrence.end());
Modified: clang-tools-extra/trunk/clangd/index/Index.h
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Index.h?rev=341273=341272=341273=diff
==
--- clang-tools-extra/trunk/clangd/index/Index.h (original)
+++ clang-tools-extra/trunk/clangd/index/Index.h Sat Sep 1 00:47:03 2018
@@ -456,7 +456,7 @@ public:
/// CrossReference finds all symbol occurrences (e.g. references,
/// declarations, definitions) and applies \p Callback on each result.
///
- /// Resutls are returned in arbitrary order.
+ /// Results are returned in arbitrary order.
///
/// The returned result must be deep-copied if it's used outside Callback.
virtual void findOccurrences(
Modified: clang-tools-extra/trunk/clangd/index/MemIndex.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/MemIndex.cpp?rev=341273=341272=341273=diff
==
--- clang-tools-extra/trunk/clangd/index/MemIndex.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/MemIndex.cpp Sat Sep 1 00:47:03 2018
@@ -44,7 +44,7 @@ void MemIndex::build(std::shared_ptr Lock(Mutex);
Index = std::move(TempIndex);
-Symbols = std::move(Syms); // Relase old symbols.
+Symbols = std::move(Syms); // Release old symbols.
Occurrences = std::move(AllOccurrences);
}
Modified: clang-tools-extra/trunk/clangd/index/Merge.cpp
URL:
[PATCH] D50850: clang: Add triples support for MIPS r6
atanasyan added a comment. Could you please include more context to patches sent for review? https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface Comment at: lib/Driver/ToolChains/Linux.cpp:46 TargetTriple.getEnvironment(); + llvm::Triple::SubArchType TargetSubArch = TargetTriple.getSubArch(); bool IsAndroid = TargetTriple.isAndroid(); Maybe use `bool IsR6` to make the following expressions shorter? Comment at: lib/Driver/ToolChains/Linux.cpp:111 +(TargetSubArch == llvm::Triple::MipsSubArch_r6) ? "mipsisa32" + : "mips"; +if (D.getVFS().exists(SysRoot + "/lib/" + MipsCpu + "-linux-gnu")) clang-format these lines Repository: rC Clang https://reviews.llvm.org/D50850 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51464: clang: fix MIPS/N32 triple and paths
atanasyan added a comment. Could you please include more context to patches sent for review? https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface Comment at: lib/Driver/ToolChains/Arch/Mips.cpp:109 + if (ABIName.empty() && (Triple.getEnvironment() == llvm::Triple::GNUABIN32)) +ABIName = "n32"; If target triple is mips-mti-linux-gnuabin32 the code above set (incorrectly) the `ABIName` to `n64` and this statement will be `false`. Comment at: lib/Driver/ToolChains/Linux.cpp:47 bool IsAndroid = TargetTriple.isAndroid(); + std::string MipsCpu = "", Mips64Abi = "gnuabi64"; + if (TargetEnvironment == llvm::Triple::GNUABIN32) - Do you need `MipsCpu` variable? - Is it possible to use any lightweight type like `StringRef` for the `Mips64Abi`? Comment at: lib/Driver/ToolChains/Linux.cpp:118 case llvm::Triple::mips64: if (D.getVFS().exists(SysRoot + "/lib/mips64-linux-gnu")) return "mips64-linux-gnu"; If a user has two toolchains installed into "/lib/mips64-linux-gnu" and "/lib/mips64-linux-gnuabin32", this code always selects mips64-linux-gnu even if N32 ABI is requested. Is it intended? Repository: rC Clang https://reviews.llvm.org/D51464 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51564: Distinguish `__block` variables that are captured by escaping blocks from those that aren't
rjmccall added inline comments.
Comment at: include/clang/AST/Decl.h:977
+
+unsigned EscapingByref : 1;
};
This doesn't actually seem to be initialized anywhere.
Comment at: include/clang/AST/Decl.h:1422
+
+ bool isNonEscapingByref() const;
+
Documentation comments? I'm not opposed to using `Byref` as a shorthand here,
but please remember that a lot of Clang maintainers aren't familiar with the
blocks extension, and those that are aren't necessarily familiar with the
non-escaping optimization.
Comment at: include/clang/AST/Decl.h:3888
+bool isNonEscapingByref() const {
+ return VariableAndFlags.getPointer()->isNonEscapingByref();
+}
Please call `getVariable()` here for clarity.
Comment at: include/clang/Sema/ScopeInfo.h:438
+EscapingBlocks.erase(BD);
+ }
+
Please describe the expected operation in comments here: that blocks are added
by default and then removed when the expression is provably used in a
non-escaping way.
Comment at: lib/CodeGen/CGBlocks.cpp:497
+ return VD->isNonEscapingByref() ?
+ CGF.getContext().getLValueReferenceType(VD->getType()) :
VD->getType();
}
Do you need to worry about the variable already having a reference type?
Comment at: lib/CodeGen/CGBlocks.cpp:1310
- if (capture.fieldType()->isReferenceType())
+ if (capture.fieldType()->isReferenceType() || variable->isNonEscapingByref())
addr = EmitLoadOfReference(MakeAddrLValue(addr, capture.fieldType()));
Isn't the field type already a reference type in this case?
You should leave a comment that you're relying on that, of course.
Comment at: lib/Sema/Sema.cpp:1410
+VD->setEscapingByref();
+}
+
It's okay to walk over an unordered set here because you're just changing
memory. However, you don't actually need it to be a set; you can just remember
all the blocks in the function and then ignore them if they're marked as
non-escaping.
Comment at: lib/Sema/Sema.cpp:1412
+
+ for (VarDecl *VD : FSI.ByrefBlockVars)
+// __block variables might require us to capture a copy-initializer.
In contrast, it's not okay to walk over an unordered set here because the loop
body can emit diagnostics and/or instantiate code.
Fortunately, you don't actually need `ByrefBlockVars` to be a set at all; you
can just use an `llvm::TinyPtrVector'.
Comment at: lib/Sema/Sema.cpp:1414
+// __block variables might require us to capture a copy-initializer.
+if (VD->isEscapingByref()) {
+ QualType T = VD->getType();
Please use an early `continue` so you can de-indent the rest of the loop body.
Comment at: lib/Sema/Sema.cpp:1420
+ // constructing this copy.
+ if (T->isStructureOrClassType()) {
+EnterExpressionEvaluationContext scope(
Should the contents of this block be in a helper function?
Comment at: lib/Sema/Sema.cpp:1451
+ // Call this function before popping the current function scope.
+ markEscapingByrefs(*FunctionScopes.back(), *this);
+
Why before?
Repository:
rC Clang
https://reviews.llvm.org/D51564
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51084: Implement -Watomic-implicit-seq-cst
rjmccall added inline comments. Comment at: lib/Sema/SemaChecking.cpp:4924 +<< Callee->getSourceRange(); + } + rjmccall wrote: > Why is the diagnostic at the end location? And why are you separately > checking whether it's ignored at the begin location? I guess all the other places in this function are diagnosing at the end location, so we should probably just be consistent. Sorry for the run-around. Comment at: lib/Sema/SemaChecking.cpp:10407 + if (E->getLHS()->getType()->isAtomicType()) +S.Diag(E->getLHS()->getBeginLoc(), diag::warn_atomic_implicit_seq_cst); + Why are the operator-driven diagnostics here all pointing at the LHS instead of at the operator? Comment at: lib/Sema/SemaChecking.cpp:10671 + if (Source->isAtomicType()) +S.Diag(E->getBeginLoc(), diag::warn_atomic_implicit_seq_cst); + When pointing at an arbitrary expression, it's generally consider best to point at its caret location — the result of `getLoc()` — and then highlight its source range. Comment at: lib/Sema/SemaChecking.cpp:10974 + if (E->IgnoreParenImpCasts()->getType()->isAtomicType()) +return; CheckImplicitConversion(S, E->IgnoreParenImpCasts(), S.Context.BoolTy, CC); Can you explain this one? Repository: rC Clang https://reviews.llvm.org/D51084 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
