[PATCH] D52047: [clangd] Add a "benchmark" for tracking memory
kbobyrev added inline comments. Comment at: clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp:69 +// This is not a *real* benchmark: it shows size of built MemIndex (in bytes). +// Same for the next "benchmark". ioeric wrote: > The hack might not be obvious for other people who run these benchmarks. Is > it possible to print some extra message along with the result to explain the > hack? Yes, that makes sense. I might try to use [[ https://github.com/google/benchmark#user-defined-counters | User-Defined Counters ]] for that. Comment at: clang-tools-extra/clangd/index/dex/Dex.cpp:239 for (const auto &P : InvertedIndex) -Bytes += P.second.bytes(); +Bytes += P.first.Data.size() + P.second.bytes() * sizeof(DocID); return Bytes + BackingDataSize; ioeric wrote: > Why do we need `P.second.bytes() * sizeof(DocID)`? Isn't `P.second.bytes()` > already the memory size of the posting list? Yes, the patch is out of sync. Will fix! https://reviews.llvm.org/D52047 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51926: [scan-build-py] Prevent crashes of CTU analysis by suppressing warnings
lorincbalog added a comment. Yes, `-w` suppresses all warnings (without raising an error) regardless of the options' sequence, even if `-Wall -Werror` are present,. Repository: rC Clang https://reviews.llvm.org/D51926 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52034: [Clang] Add options -Xclang -coverage-filter and -Xclang -coverage-exclude to filter the files to instrument with gcov
marco-c added inline comments. Comment at: include/clang/Driver/CC1Options.td:236 +def coverage_exclude_EQ : Joined<["-"], "coverage-exclude=">, + Alias; def coverage_exit_block_before_body : Flag<["-"], "coverage-exit-block-before-body">, calixte wrote: > vsk wrote: > > Have you checked whether gcc supports similar options? If so, it would be > > great if we could match their name & behavior. > The only one I found -finstrument-functions-exclude-file-list > (https://gcc.gnu.org/onlinedocs/gcc/Instrumentation-Options.html). > But no regex and no way to include one file only. > I took the names from gcovr: > https://manpages.debian.org/jessie/gcovr/gcovr.1.en.html We could file a bug in GCC's Bugzilla and agree with them about the options. Repository: rC Clang https://reviews.llvm.org/D52034 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52047: [clangd] Add a "benchmark" for tracking memory
kbobyrev updated this revision to Diff 166240.
kbobyrev marked 4 inline comments as done.
https://reviews.llvm.org/D52047
Files:
clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
clang-tools-extra/clangd/index/SymbolYAML.cpp
clang-tools-extra/clangd/index/dex/Dex.cpp
Index: clang-tools-extra/clangd/index/dex/Dex.cpp
===
--- clang-tools-extra/clangd/index/dex/Dex.cpp
+++ clang-tools-extra/clangd/index/dex/Dex.cpp
@@ -124,9 +124,6 @@
for (const auto &TokenToPostingList : TempInvertedIndex)
InvertedIndex.insert({TokenToPostingList.first,
PostingList(move(TokenToPostingList.second))});
-
- vlog("Built Dex with estimated memory usage {0} bytes.",
- estimateMemoryUsage());
}
/// Constructs iterators over tokens extracted from the query and exhausts it
@@ -239,7 +236,7 @@
Bytes += LookupTable.getMemorySize();
Bytes += InvertedIndex.getMemorySize();
for (const auto &P : InvertedIndex)
-Bytes += P.second.bytes();
+Bytes += P.first.Data.size() + P.second.bytes();
return Bytes + BackingDataSize;
}
Index: clang-tools-extra/clangd/index/SymbolYAML.cpp
===
--- clang-tools-extra/clangd/index/SymbolYAML.cpp
+++ clang-tools-extra/clangd/index/SymbolYAML.cpp
@@ -9,6 +9,7 @@
#include "SymbolYAML.h"
#include "Index.h"
+#include "Logger.h"
#include "Serialization.h"
#include "Trace.h"
#include "dex/Dex.h"
@@ -228,8 +229,12 @@
if (!Slab)
return nullptr;
trace::Span Tracer("BuildIndex");
- return UseDex ? dex::Dex::build(std::move(*Slab), URISchemes)
-: MemIndex::build(std::move(*Slab), RefSlab());
+ auto Index = UseDex ? dex::Dex::build(std::move(*Slab), URISchemes)
+ : MemIndex::build(std::move(*Slab), RefSlab());
+ vlog("Loaded {0} from {1} with estimated memory usage {2}",
+ UseDex ? "Dex" : "MemIndex", SymbolFilename,
+ Index->estimateMemoryUsage());
+ return Index;
}
} // namespace clangd
Index: clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
===
--- clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
+++ clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
@@ -66,23 +66,46 @@
return Requests;
}
+// This is not a *real* benchmark: it shows size of built MemIndex (in bytes).
+// Same for the next "benchmark".
+// FIXME(kbobyrev): Should this be separated into the BackingMemorySize
+// (underlying SymbolSlab size) and Symbol Index (MemIndex/Dex) overhead?
+static void MemSize(benchmark::State &State) {
+ const auto Mem = buildMem();
+ for (auto _ : State)
+// Divide size of Mem by 1000 so that it will be correctly displayed in the
+// benchmark report (possible options for time units are ms, ns and us).
+State.SetIterationTime(/*double Seconds=*/Mem->estimateMemoryUsage() /
+ 1000);
+ State.counters["Memory Usage (bytes)"] = Mem->estimateMemoryUsage();
+}
+BENCHMARK(MemSize)->UseManualTime()->Unit(benchmark::kMillisecond);
+
+static void DexSize(benchmark::State &State) {
+ const auto Dex = buildDex();
+ for (auto _ : State)
+State.SetIterationTime(Dex->estimateMemoryUsage() / 1000);
+ State.counters["Memory Usage (bytes)"] = Dex->estimateMemoryUsage();
+}
+BENCHMARK(DexSize)->UseManualTime()->Unit(benchmark::kMillisecond);
+
static void MemQueries(benchmark::State &State) {
const auto Mem = buildMem();
const auto Requests = extractQueriesFromLogs();
for (auto _ : State)
for (const auto &Request : Requests)
Mem->fuzzyFind(Request, [](const Symbol &S) {});
}
-BENCHMARK(MemQueries);
+BENCHMARK(MemQueries)->Unit(benchmark::kMillisecond);
static void DexQueries(benchmark::State &State) {
const auto Dex = buildDex();
const auto Requests = extractQueriesFromLogs();
for (auto _ : State)
for (const auto &Request : Requests)
Dex->fuzzyFind(Request, [](const Symbol &S) {});
}
-BENCHMARK(DexQueries);
+BENCHMARK(DexQueries)->Unit(benchmark::kMillisecond);
} // namespace
} // namespace clangd
@@ -108,4 +131,5 @@
argc -= 2;
::benchmark::Initialize(&argc, argv);
::benchmark::RunSpecifiedBenchmarks();
+ return 0;
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52290: [driver][mips] Adjust target triple accordingly to provided ABI name
atanasyan created this revision.
atanasyan added reviewers: rsmith, rnk, zturner.
Herald added subscribers: jrtc27, arichardson, sdardis.
All MIPS target architectures can be divided into four categories: 32-bit
little-endian, 32-bit big-endian, 64-bit little-endian, and 64-bit big-endian.
If, for example, default target triple used to build compiler is a
little-endian one, but user wants to generate big-endian output it's enough to
provide just `-EB` option to implicitly convert default target triple to
big-endian variant. But it's impossible to implicitly convert 32-bit target
triple to 64-bit variant and vice versa. In other words, if user has Clang
built with "mips-linux-gnu" default target triple and wants to generate 64-bit
output, he or she has to explicitly provide 64-bit target triple
"mips64-linux-gnu".
clang -target mips64-linux-gnu -c test.c
While gcc in the same case allows to specify just a correct ABI name.
gcc -mabi=64 -c test.c
The patch solves this problem. If there is no explicit `-target` option, target
triple is adjusted accordingly provided ABI name.
For testing this change we need to build Clang with mips default target triple.
To catch this case (on MIPS buildbot) and ran a corresponding test case I have
to add new "lit" feature `mips-default-target`.
Repository:
rC Clang
https://reviews.llvm.org/D52290
Files:
lib/Driver/Driver.cpp
test/Driver/mips-target-abi.c
test/lit.cfg.py
Index: test/lit.cfg.py
===
--- test/lit.cfg.py
+++ test/lit.cfg.py
@@ -136,6 +136,10 @@
if os.path.exists('/dev/fd/0') and sys.platform not in ['cygwin']:
config.available_features.add('dev-fd-fs')
+# Test that default target triple is mips*
+if re.match(r'^mips*', config.target_triple):
+config.available_features.add('mips-default-target')
+
# Not set on native MS environment.
if not re.match(r'.*-(windows-msvc)$', config.target_triple):
config.available_features.add('non-ms-sdk')
Index: test/Driver/mips-target-abi.c
===
--- /dev/null
+++ test/Driver/mips-target-abi.c
@@ -0,0 +1,24 @@
+// Check default target triple adjusting by ABI option.
+//
+// REQUIRES: mips-default-target
+//
+// RUN: %clang -mabi=32 -### %s 2>&1 | FileCheck -check-prefix=O32 %s
+// O32: "-triple" "mips{{[^"]*}}"
+// O32: "-target-cpu" "mips32r2"
+// O32: "-target-abi" "o32"
+// O32: ld{{.*}}"
+// O32: "-m" "elf32btsmip"
+
+// RUN: %clang -mabi=n32 -### %s 2>&1 | FileCheck -check-prefix=N32 %s
+// N32: "-triple" "mips64{{[^"]*}}"
+// N32: "-target-cpu" "mips64r2"
+// N32: "-target-abi" "n32"
+// N32: ld{{.*}}"
+// N32: "-m" "elf32btsmipn32"
+
+// RUN: %clang -mabi=64 -### %s 2>&1 | FileCheck -check-prefix=N64 %s
+// N64: "-triple" "mips64{{[^"]*}}"
+// N64: "-target-cpu" "mips64r2"
+// N64: "-target-abi" "n64"
+// N64: ld{{.*}}"
+// N64: "-m" "elf64btsmip"
Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -481,6 +481,17 @@
Target.setVendorName("intel");
}
+ // If target is MIPS and there is no explicit `-target` option,
+ // adjust the target triple accordingly to provided ABI name.
+ if (Target.isMIPS() && !Args.getLastArg(options::OPT_target)) {
+if (Arg *A = Args.getLastArg(options::OPT_mabi_EQ))
+ Target = llvm::StringSwitch(A->getValue())
+ .Case("32", Target.get32BitArchVariant())
+ .Case("n32", Target.get64BitArchVariant())
+ .Case("64", Target.get64BitArchVariant())
+ .Default(Target);
+ }
+
return Target;
}
Index: test/lit.cfg.py
===
--- test/lit.cfg.py
+++ test/lit.cfg.py
@@ -136,6 +136,10 @@
if os.path.exists('/dev/fd/0') and sys.platform not in ['cygwin']:
config.available_features.add('dev-fd-fs')
+# Test that default target triple is mips*
+if re.match(r'^mips*', config.target_triple):
+config.available_features.add('mips-default-target')
+
# Not set on native MS environment.
if not re.match(r'.*-(windows-msvc)$', config.target_triple):
config.available_features.add('non-ms-sdk')
Index: test/Driver/mips-target-abi.c
===
--- /dev/null
+++ test/Driver/mips-target-abi.c
@@ -0,0 +1,24 @@
+// Check default target triple adjusting by ABI option.
+//
+// REQUIRES: mips-default-target
+//
+// RUN: %clang -mabi=32 -### %s 2>&1 | FileCheck -check-prefix=O32 %s
+// O32: "-triple" "mips{{[^"]*}}"
+// O32: "-target-cpu" "mips32r2"
+// O32: "-target-abi" "o32"
+// O32: ld{{.*}}"
+// O32: "-m" "elf32btsmip"
+
+// RUN: %clang -mabi=n32 -### %s 2>&1 | FileCheck -check-prefix=N32 %s
+// N32: "-triple" "mips64{{[^"]*}}"
+// N32: "-target-cpu" "mips64r2"
+// N32: "-target-abi" "n32"
+// N32: ld{{.*}}"
+// N32: "-m" "e
[PATCH] D52233: [dexp] Dump JSON representations of fuzzy find requests
kbobyrev updated this revision to Diff 166241.
kbobyrev marked an inline comment as done.
kbobyrev retitled this revision from "[dexp] Allow users to dump JSON
representations of fuzzy find requests" to "[dexp] Dump JSON representations of
fuzzy find requests".
kbobyrev edited the summary of this revision.
kbobyrev added a comment.
I thought that we might not want to produce too much information, but I don't
have anything against printing the JSON representation of every user request.
https://reviews.llvm.org/D52233
Files:
clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp
Index: clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp
===
--- clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp
+++ clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp
@@ -123,6 +123,7 @@
StringRef(this->Scopes).split(Scopes, ',');
Request.Scopes = {Scopes.begin(), Scopes.end()};
}
+llvm::outs() << llvm::formatv("Request:\n{0}\n\n", toJSON(Request));
// FIXME(kbobyrev): Print symbol final scores to see the distribution.
static const auto OutputFormat = "{0,-4} | {1,-40} | {2,-25}\n";
llvm::outs() << llvm::formatv(OutputFormat, "Rank", "Symbol ID",
Index: clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp
===
--- clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp
+++ clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp
@@ -123,6 +123,7 @@
StringRef(this->Scopes).split(Scopes, ',');
Request.Scopes = {Scopes.begin(), Scopes.end()};
}
+llvm::outs() << llvm::formatv("Request:\n{0}\n\n", toJSON(Request));
// FIXME(kbobyrev): Print symbol final scores to see the distribution.
static const auto OutputFormat = "{0,-4} | {1,-40} | {2,-25}\n";
llvm::outs() << llvm::formatv(OutputFormat, "Rank", "Symbol ID",
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r342636 - FileCheckify test/Driver/Xarch.c
Author: hans Date: Thu Sep 20 02:29:35 2018 New Revision: 342636 URL: http://llvm.org/viewvc/llvm-project?rev=342636&view=rev Log: FileCheckify test/Driver/Xarch.c Modified: cfe/trunk/test/Driver/Xarch.c Modified: cfe/trunk/test/Driver/Xarch.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/Xarch.c?rev=342636&r1=342635&r2=342636&view=diff == --- cfe/trunk/test/Driver/Xarch.c (original) +++ cfe/trunk/test/Driver/Xarch.c Thu Sep 20 02:29:35 2018 @@ -1,9 +1,12 @@ -// RUN: %clang -target i386-apple-darwin9 -m32 -Xarch_i386 -O2 %s -S -### 2> %t.log -// RUN: grep ' "-O2" ' %t.log | count 1 -// RUN: %clang -target i386-apple-darwin9 -m64 -Xarch_i386 -O2 %s -S -### 2> %t.log -// RUN: not grep ' "-O2" ' %t.log -// RUN: grep "argument unused during compilation: '-Xarch_i386 -O2'" %t.log -// RUN: not %clang -target i386-apple-darwin9 -m32 -Xarch_i386 -o -Xarch_i386 -S %s -S -Xarch_i386 -o 2> %t.log -// RUN: grep "error: invalid Xarch argument: '-Xarch_i386 -o'" %t.log | count 2 -// RUN: grep "error: invalid Xarch argument: '-Xarch_i386 -S'" %t.log +// RUN: %clang -target i386-apple-darwin9 -m32 -Xarch_i386 -O2 %s -S -### 2>&1 | FileCheck -check-prefix=O2ONCE %s +// O2ONCE: "-O2" +// O2ONCE-NOT: "-O2" +// RUN: %clang -target i386-apple-darwin9 -m64 -Xarch_i386 -O2 %s -S -### 2>&1 | FileCheck -check-prefix=O2NONE %s +// O2NONE-NOT: "-O2" +// O2NONE: argument unused during compilation: '-Xarch_i386 -O2' + +// RUN: not %clang -target i386-apple-darwin9 -m32 -Xarch_i386 -o -Xarch_i386 -S %s -S -Xarch_i386 -o 2>&1 | FileCheck -check-prefix=INVALID %s +// INVALID: error: invalid Xarch argument: '-Xarch_i386 -o' +// INVALID: error: invalid Xarch argument: '-Xarch_i386 -S' +// INVALID: error: invalid Xarch argument: '-Xarch_i386 -o' ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52292: [Sema][OpenCL] Improve diagnostics for not viable overloadable function candidates
sidorovd created this revision.
sidorovd added reviewers: Anastasia, yaxunl.
Herald added a subscriber: cfe-commits.
Allowed extension name (that ought to be disabled) printing in the note message.
This diagnostic was proposed here: https://reviews.llvm.org/D51341
Repository:
rC Clang
https://reviews.llvm.org/D52292
Files:
include/clang/Basic/DiagnosticSemaKinds.td
include/clang/Sema/Sema.h
lib/Sema/Sema.cpp
lib/Sema/SemaOverload.cpp
test/SemaOpenCL/extension-begin.cl
Index: test/SemaOpenCL/extension-begin.cl
===
--- test/SemaOpenCL/extension-begin.cl
+++ test/SemaOpenCL/extension-begin.cl
@@ -48,7 +48,7 @@
PointerOfA test_A_pointer; // expected-error {{use of type 'PointerOfA' (aka
'const struct A *') requires my_ext extension to be enabled}}
f(); // expected-error {{use of declaration 'f' requires my_ext extension to
be enabled}}
g(0); // expected-error {{no matching function for call to 'g'}}
-// expected-note@-26 {{candidate disabled due to OpenCL extension}}
+// expected-note@-26 {{candidate unavailable as it requires OpenCL
extension 'my_ext' to be disabled}}
// expected-note@-22 {{candidate function not viable: requires 0
arguments, but 1 was provided}}
}
Index: lib/Sema/SemaOverload.cpp
===
--- lib/Sema/SemaOverload.cpp
+++ lib/Sema/SemaOverload.cpp
@@ -10242,7 +10242,8 @@
FunctionDecl *Callee = Cand->Function;
S.Diag(Callee->getLocation(),
- diag::note_ovl_candidate_disabled_by_extension);
+ diag::note_ovl_candidate_disabled_by_extension)
+<< S.getOpenCLExtensionsFromDeclExtMap(Callee);
}
/// Generates a 'note' diagnostic for an overload candidate. We've
Index: lib/Sema/Sema.cpp
===
--- lib/Sema/Sema.cpp
+++ lib/Sema/Sema.cpp
@@ -1853,6 +1853,27 @@
setOpenCLExtensionForDecl(D, CurrOpenCLExtension);
}
+std::string Sema::getOpenCLExtensionsFromDeclExtMap(FunctionDecl *FD) {
+ if (!OpenCLDeclExtMap.empty())
+return getOpenCLExtensionsFromExtMap(FD, OpenCLDeclExtMap);
+
+ return "";
+}
+
+template
+std::string Sema::getOpenCLExtensionsFromExtMap(T *FDT, MapT &Map) {
+ std::string ExtensionNames = "";
+ auto Loc = Map.find(FDT);
+
+ for (auto const& I : Loc->second) {
+ExtensionNames += I;
+ExtensionNames += " ";
+ }
+ ExtensionNames.pop_back();
+
+ return ExtensionNames;
+}
+
bool Sema::isOpenCLDisabledDecl(Decl *FD) {
auto Loc = OpenCLDeclExtMap.find(FD);
if (Loc == OpenCLDeclExtMap.end())
Index: include/clang/Sema/Sema.h
===
--- include/clang/Sema/Sema.h
+++ include/clang/Sema/Sema.h
@@ -8567,6 +8567,16 @@
llvm::StringRef getCurrentOpenCLExtension() const {
return CurrOpenCLExtension;
}
+
+ /// Check if a function declaration \p FD associates with any
+ /// extensions present in OpenCLDeclExtMap and if so return the
+ /// extension(s) name(s).
+ std::string getOpenCLExtensionsFromDeclExtMap(FunctionDecl *FD);
+
+ /// Find and extension in an extension map and return its name
+ template
+ std::string getOpenCLExtensionsFromExtMap(T* FT, MapT &Map);
+
void setCurrentOpenCLExtension(llvm::StringRef Ext) {
CurrOpenCLExtension = Ext;
}
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -3652,7 +3652,7 @@
def note_ovl_candidate_disabled_by_function_cond_attr : Note<
"candidate disabled: %0">;
def note_ovl_candidate_disabled_by_extension : Note<
-"candidate disabled due to OpenCL extension">;
+"candidate unavailable as it requires OpenCL extension '%0' to be
disabled">;
def err_addrof_function_disabled_by_enable_if_attr : Error<
"cannot take address of function %0 because it has one or more "
"non-tautological enable_if conditions">;
Index: test/SemaOpenCL/extension-begin.cl
===
--- test/SemaOpenCL/extension-begin.cl
+++ test/SemaOpenCL/extension-begin.cl
@@ -48,7 +48,7 @@
PointerOfA test_A_pointer; // expected-error {{use of type 'PointerOfA' (aka 'const struct A *') requires my_ext extension to be enabled}}
f(); // expected-error {{use of declaration 'f' requires my_ext extension to be enabled}}
g(0); // expected-error {{no matching function for call to 'g'}}
-// expected-note@-26 {{candidate disabled due to OpenCL extension}}
+// expected-note@-26 {{candidate unavailable as it requires OpenCL extension 'my_ext' to be disabled}}
// expected-note@-22 {{candidate function not viable: requires 0 arguments, but 1 was provided}}
}
Index: lib/Sema/SemaOverload.cpp
=
[PATCH] D51432: [AArch64] Unwinding support for return address signing
olista01 accepted this revision. olista01 added a comment. This revision is now accepted and ready to land. LGTM, thanks! https://reviews.llvm.org/D51432 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52266: [clang-cl] Provide separate flags for all the /O variants
hans added a comment. Sorry, I didn't realize we both set off to do this in parallel. I've incorporated your changes into my patch. Comment at: test/Driver/Xarch.c:5 +// RUN: not grep ' "-O3" ' %t.log +// RUN: grep "argument unused during compilation: '-Xarch_i386 -O3'" %t.log // RUN: not %clang -target i386-apple-darwin9 -m32 -Xarch_i386 -o -Xarch_i386 -S %s -S -Xarch_i386 -o 2> %t.log compnerd wrote: > I know that this isn’t your doing, but while you’re here, would you mind > replacing the temp files with pipes and grep with FileCheck? r342636 https://reviews.llvm.org/D52266 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52266: [clang-cl] Provide separate flags for all the /O variants
hans updated this revision to Diff 166252.
hans added a comment.
Uploading new diff.
https://reviews.llvm.org/D52266
Files:
include/clang/Driver/CLCompatOptions.td
lib/Driver/ToolChains/MSVC.cpp
test/Driver/Xarch.c
Index: test/Driver/Xarch.c
===
--- test/Driver/Xarch.c
+++ test/Driver/Xarch.c
@@ -1,10 +1,10 @@
-// RUN: %clang -target i386-apple-darwin9 -m32 -Xarch_i386 -O2 %s -S -### 2>&1 | FileCheck -check-prefix=O2ONCE %s
-// O2ONCE: "-O2"
-// O2ONCE-NOT: "-O2"
+// RUN: %clang -target i386-apple-darwin9 -m32 -Xarch_i386 -O3 %s -S -### 2>&1 | FileCheck -check-prefix=O3ONCE %s
+// O3ONCE: "-O3"
+// O3ONCE-NOT: "-O3"
-// RUN: %clang -target i386-apple-darwin9 -m64 -Xarch_i386 -O2 %s -S -### 2>&1 | FileCheck -check-prefix=O2NONE %s
-// O2NONE-NOT: "-O2"
-// O2NONE: argument unused during compilation: '-Xarch_i386 -O2'
+// RUN: %clang -target i386-apple-darwin9 -m64 -Xarch_i386 -O3 %s -S -### 2>&1 | FileCheck -check-prefix=O3NONE %s
+// O3NONE-NOT: "-O3"
+// O3NONE: argument unused during compilation: '-Xarch_i386 -O3'
// RUN: not %clang -target i386-apple-darwin9 -m32 -Xarch_i386 -o -Xarch_i386 -S %s -S -Xarch_i386 -o 2>&1 | FileCheck -check-prefix=INVALID %s
// INVALID: error: invalid Xarch argument: '-Xarch_i386 -o'
Index: lib/Driver/ToolChains/MSVC.cpp
===
--- lib/Driver/ToolChains/MSVC.cpp
+++ lib/Driver/ToolChains/MSVC.cpp
@@ -1378,6 +1378,7 @@
}
break;
case 'g':
+ A->claim();
break;
case 'i':
if (I + 1 != E && OptStr[I + 1] == '-') {
Index: include/clang/Driver/CLCompatOptions.td
===
--- include/clang/Driver/CLCompatOptions.td
+++ include/clang/Driver/CLCompatOptions.td
@@ -85,16 +85,17 @@
HelpText<"Assume thread-local variables are defined in the executable">;
def _SLASH_GR : CLFlag<"GR">, HelpText<"Enable emission of RTTI data">;
def _SLASH_GR_ : CLFlag<"GR-">, HelpText<"Disable emission of RTTI data">;
+def _SLASH_GF : CLIgnoredFlag<"GF">, HelpText<"Enable string pooling (default)">;
def _SLASH_GF_ : CLFlag<"GF-">, HelpText<"Disable string pooling">,
Alias;
-def _SLASH_GS : CLFlag<"GS">, HelpText<"Enable buffer security check">;
+def _SLASH_GS : CLFlag<"GS">, HelpText<"Enable buffer security check (default)">;
def _SLASH_GS_ : CLFlag<"GS-">, HelpText<"Disable buffer security check">;
-def _SLASH_Gs : CLJoined<"Gs">, HelpText<"Set stack probe size">,
- Alias;
+def : CLFlag<"Gs">, HelpText<"Same as /Gs0">, Alias, AliasArgs<["0"]>;
+def _SLASH_Gs : CLJoined<"Gs">, HelpText<"Set stack probe size">, Alias;
def _SLASH_Gy : CLFlag<"Gy">, HelpText<"Put each function in its own section">,
Alias;
def _SLASH_Gy_ : CLFlag<"Gy-">,
- HelpText<"Don't put each function in its own section">,
+ HelpText<"Don't put each function in its own section (default)">,
Alias;
def _SLASH_Gw : CLFlag<"Gw">, HelpText<"Put each data item in its own section">,
Alias;
@@ -109,18 +110,26 @@
Alias;
def _SLASH_J : CLFlag<"J">, HelpText<"Make char type unsigned">,
Alias;
-def _SLASH_O0 : CLFlag<"O0">, Alias;
-// /Oy- is handled by the /O option because /Oy- only has an effect on 32-bit.
-def _SLASH_O : CLJoined<"O">, HelpText<"Optimization level">;
-def _SLASH_Od : CLFlag<"Od">, HelpText<"Disable optimization">, Alias;
-def _SLASH_Oi : CLFlag<"Oi">, HelpText<"Enable use of builtin functions">,
- Alias;
-def _SLASH_Oi_ : CLFlag<"Oi-">, HelpText<"Disable use of builtin functions">,
- Alias;
-def _SLASH_Os : CLFlag<"Os">, HelpText<"Optimize for size">, Alias,
- AliasArgs<["s"]>;
-def _SLASH_Ot : CLFlag<"Ot">, HelpText<"Optimize for speed">, Alias,
- AliasArgs<["2"]>;
+
+// The _SLASH_O option handles all the /O flags, but we also provide separate aliased options to provide separate help messages.
+// FIXME: Not sure why we have -O0 here; MSVC doesn't support that.
+def _SLASH_O : CLJoined<"O">, HelpText<"Set several /O flags at once; e.g. '/O2y-' is the same as '/O2 /y-'">, MetaVarName<"">;
+def : CLFlag<"O0">, Alias, HelpText<"Disable optimization">;
+def : CLFlag<"O1">, Alias<_SLASH_O>, AliasArgs<["1"]>, HelpText<"Optimize for small size (equivalent to /Og /Os /Oy /Ob2 /Gs /GF /Gy)">;
+def : CLFlag<"O2">, Alias<_SLASH_O>, AliasArgs<["2"]>, HelpText<"Optimize for maximum speed (equivalent to /Og /Oi /Ot /Oy /Ob2 /Gs /GF /Gy)">;
+def : CLFlag<"Ob0">, Alias<_SLASH_O>, AliasArgs<["b0"]>, HelpText<"Disable function inlining">;
+def : CLFlag<"Ob1">, Alias<_SLASH_O>, AliasArgs<["b1"]>, HelpText<"Only inline functions which are (explicitly or implicitly) marked inline">;
+def : CLFlag<"Ob2">, Alias<_SLASH_O>, AliasArgs<["b2"]>, HelpText<"Inline functions as deemed beneficial by the compiler">;
+def : CLFlag<"Od">, Alias, HelpText<"Disable optimization">;
+def : CLFlag<"Og">, Alias<_SLASH_O>, AliasArgs<["g"]>, HelpText<"No effect">;
+def : CLFlag<"Oi">, Alias<_SLA
r342638 - [OpenCL] Diagnose redundant address space conversion
Author: svenvh
Date: Thu Sep 20 03:07:27 2018
New Revision: 342638
URL: http://llvm.org/viewvc/llvm-project?rev=342638&view=rev
Log:
[OpenCL] Diagnose redundant address space conversion
Add a warning if a parameter with a named address space is passed
to a to_addr builtin.
For example:
int i;
to_private(&i); // generate warning as conversion from private to private is
redundant.
Patch by Alistair Davies.
Differential Revision: https://reviews.llvm.org/D51411
Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/Sema/SemaChecking.cpp
cfe/trunk/test/SemaOpenCL/to_addr_builtin.cl
Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=342638&r1=342637&r2=342638&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Thu Sep 20 03:07:27
2018
@@ -8613,6 +8613,10 @@ def err_opencl_variadic_function : Error
"invalid prototype, variadic arguments are not allowed in OpenCL">;
def err_opencl_requires_extension : Error<
"use of %select{type|declaration}0 %1 requires %2 extension to be enabled">;
+def warn_opencl_generic_address_space_arg : Warning<
+ "passing non-generic address space pointer to %0"
+ " may cause dynamic conversion affecting performance">,
+ InGroup, DefaultIgnore;
// OpenCL v2.0 s6.13.6 -- Builtin Pipe Functions
def err_opencl_builtin_pipe_first_arg : Error<
Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=342638&r1=342637&r2=342638&view=diff
==
--- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
+++ cfe/trunk/lib/Sema/SemaChecking.cpp Thu Sep 20 03:07:27 2018
@@ -849,6 +849,13 @@ static bool SemaOpenCLBuiltinToAddr(Sema
return true;
}
+ if (RT->getPointeeType().getAddressSpace() != LangAS::opencl_generic) {
+S.Diag(Call->getArg(0)->getBeginLoc(),
+ diag::warn_opencl_generic_address_space_arg)
+<< Call->getDirectCallee()->getNameInfo().getAsString()
+<< Call->getArg(0)->getSourceRange();
+ }
+
RT = RT->getPointeeType();
auto Qual = RT.getQualifiers();
switch (BuiltinID) {
Modified: cfe/trunk/test/SemaOpenCL/to_addr_builtin.cl
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaOpenCL/to_addr_builtin.cl?rev=342638&r1=342637&r2=342638&view=diff
==
--- cfe/trunk/test/SemaOpenCL/to_addr_builtin.cl (original)
+++ cfe/trunk/test/SemaOpenCL/to_addr_builtin.cl Thu Sep 20 03:07:27 2018
@@ -1,5 +1,5 @@
// RUN: %clang_cc1 -verify -fsyntax-only %s
-// RUN: %clang_cc1 -verify -fsyntax-only -cl-std=CL2.0 %s
+// RUN: %clang_cc1 -Wconversion -verify -fsyntax-only -cl-std=CL2.0 %s
void test(void) {
global int *glob;
@@ -43,6 +43,7 @@ void test(void) {
// expected-warning@-2{{incompatible integer to pointer conversion assigning
to '__local int *' from 'int'}}
#else
// expected-error@-4{{assigning '__global int *' to '__local int *' changes
address space of pointer}}
+ // expected-warning@-5{{passing non-generic address space pointer to
to_global may cause dynamic conversion affecting performance}}
#endif
global char *glob_c = to_global(loc);
@@ -50,6 +51,7 @@ void test(void) {
// expected-warning@-2{{incompatible integer to pointer conversion
initializing '__global char *' with an expression of type 'int'}}
#else
// expected-warning@-4{{incompatible pointer types initializing '__global
char *' with an expression of type '__global int *'}}
+ // expected-warning@-5{{passing non-generic address space pointer to
to_global may cause dynamic conversion affecting performance}}
#endif
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51411: [OpenCL] Improve diagnostic of argument in address space conversion builtins
This revision was automatically updated to reflect the committed changes.
Closed by commit rL342638: [OpenCL] Diagnose redundant address space conversion
(authored by svenvh, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D51411?vs=164835&id=166253#toc
Repository:
rL LLVM
https://reviews.llvm.org/D51411
Files:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/Sema/SemaChecking.cpp
cfe/trunk/test/SemaOpenCL/to_addr_builtin.cl
Index: cfe/trunk/lib/Sema/SemaChecking.cpp
===
--- cfe/trunk/lib/Sema/SemaChecking.cpp
+++ cfe/trunk/lib/Sema/SemaChecking.cpp
@@ -849,6 +849,13 @@
return true;
}
+ if (RT->getPointeeType().getAddressSpace() != LangAS::opencl_generic) {
+S.Diag(Call->getArg(0)->getBeginLoc(),
+ diag::warn_opencl_generic_address_space_arg)
+<< Call->getDirectCallee()->getNameInfo().getAsString()
+<< Call->getArg(0)->getSourceRange();
+ }
+
RT = RT->getPointeeType();
auto Qual = RT.getQualifiers();
switch (BuiltinID) {
Index: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
===
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
@@ -8613,6 +8613,10 @@
"invalid prototype, variadic arguments are not allowed in OpenCL">;
def err_opencl_requires_extension : Error<
"use of %select{type|declaration}0 %1 requires %2 extension to be enabled">;
+def warn_opencl_generic_address_space_arg : Warning<
+ "passing non-generic address space pointer to %0"
+ " may cause dynamic conversion affecting performance">,
+ InGroup, DefaultIgnore;
// OpenCL v2.0 s6.13.6 -- Builtin Pipe Functions
def err_opencl_builtin_pipe_first_arg : Error<
Index: cfe/trunk/test/SemaOpenCL/to_addr_builtin.cl
===
--- cfe/trunk/test/SemaOpenCL/to_addr_builtin.cl
+++ cfe/trunk/test/SemaOpenCL/to_addr_builtin.cl
@@ -1,5 +1,5 @@
// RUN: %clang_cc1 -verify -fsyntax-only %s
-// RUN: %clang_cc1 -verify -fsyntax-only -cl-std=CL2.0 %s
+// RUN: %clang_cc1 -Wconversion -verify -fsyntax-only -cl-std=CL2.0 %s
void test(void) {
global int *glob;
@@ -43,13 +43,15 @@
// expected-warning@-2{{incompatible integer to pointer conversion assigning
to '__local int *' from 'int'}}
#else
// expected-error@-4{{assigning '__global int *' to '__local int *' changes
address space of pointer}}
+ // expected-warning@-5{{passing non-generic address space pointer to
to_global may cause dynamic conversion affecting performance}}
#endif
global char *glob_c = to_global(loc);
#if __OPENCL_C_VERSION__ < CL_VERSION_2_0
// expected-warning@-2{{incompatible integer to pointer conversion
initializing '__global char *' with an expression of type 'int'}}
#else
// expected-warning@-4{{incompatible pointer types initializing '__global
char *' with an expression of type '__global int *'}}
+ // expected-warning@-5{{passing non-generic address space pointer to
to_global may cause dynamic conversion affecting performance}}
#endif
}
Index: cfe/trunk/lib/Sema/SemaChecking.cpp
===
--- cfe/trunk/lib/Sema/SemaChecking.cpp
+++ cfe/trunk/lib/Sema/SemaChecking.cpp
@@ -849,6 +849,13 @@
return true;
}
+ if (RT->getPointeeType().getAddressSpace() != LangAS::opencl_generic) {
+S.Diag(Call->getArg(0)->getBeginLoc(),
+ diag::warn_opencl_generic_address_space_arg)
+<< Call->getDirectCallee()->getNameInfo().getAsString()
+<< Call->getArg(0)->getSourceRange();
+ }
+
RT = RT->getPointeeType();
auto Qual = RT.getQualifiers();
switch (BuiltinID) {
Index: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
===
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
@@ -8613,6 +8613,10 @@
"invalid prototype, variadic arguments are not allowed in OpenCL">;
def err_opencl_requires_extension : Error<
"use of %select{type|declaration}0 %1 requires %2 extension to be enabled">;
+def warn_opencl_generic_address_space_arg : Warning<
+ "passing non-generic address space pointer to %0"
+ " may cause dynamic conversion affecting performance">,
+ InGroup, DefaultIgnore;
// OpenCL v2.0 s6.13.6 -- Builtin Pipe Functions
def err_opencl_builtin_pipe_first_arg : Error<
Index: cfe/trunk/test/SemaOpenCL/to_addr_builtin.cl
===
--- cfe/trunk/test/SemaOpenCL/to_addr_builtin.cl
+++ cfe/trunk/test/SemaOpenCL/to_addr_builtin.cl
@@ -1,5 +1,5 @@
// RUN: %clang_cc1 -verify -fsyntax-only %s
-// RUN: %clang_cc1 -verify -fsyntax-only -cl-std=CL2.0 %s
+// RUN: %clang_cc1 -Wconversion -
[PATCH] D52296: [Clang] - Add -gsingle-file-split-dwarf option.
grimar created this revision.
grimar added reviewers: dblaikie, echristo, probinson, compnerd.
Herald added subscribers: JDevlieghere, aprantl.
The DWARF5 specification says(Appendix F.1):
"The sections that do not require relocation, however, **can be written to the
relocatable object (.o) file but ignored by the
linker** or they can be written to a separate DWARF object (.dwo) file that
need not be accessed by the linker."
The first part describes a single file split DWARF feature and there is no way
to trigger this behavior atm I think.
Fortunately, no many changes are required to keep *.dwo sections in a .o, the
patch does that.
Not sure who is an appropriate reviewer for that.
https://reviews.llvm.org/D52296
Files:
include/clang/Driver/CC1Options.td
include/clang/Driver/Options.td
include/clang/Frontend/CodeGenOptions.h
lib/CodeGen/BackendUtil.cpp
lib/Driver/ToolChains/Clang.cpp
lib/Driver/ToolChains/CommonArgs.cpp
lib/Driver/ToolChains/CommonArgs.h
lib/Driver/ToolChains/Gnu.cpp
lib/Driver/ToolChains/MinGW.cpp
lib/Frontend/CompilerInvocation.cpp
test/CodeGen/split-debug-single-file.c
test/Driver/split-debug.c
test/Driver/split-debug.s
Index: test/Driver/split-debug.s
===
--- test/Driver/split-debug.s
+++ test/Driver/split-debug.s
@@ -5,6 +5,9 @@
//
// CHECK-ACTIONS: "-split-dwarf-file" "split-debug.dwo"
+// Check we do not pass any -split-dwarf commands to `as` if -gsingle-file-split-dwarf is specified.
+// RUN: %clang -target x86_64-unknown-linux-gnu -gsplit-dwarf -gsingle-file-split-dwarf -c -### %s 2> %t
+// RUN: FileCheck -check-prefix=CHECK-NO-ACTIONS < %t %s
// RUN: %clang -target x86_64-macosx -gsplit-dwarf -c -### %s 2> %t
// RUN: FileCheck -check-prefix=CHECK-NO-ACTIONS < %t %s
Index: test/Driver/split-debug.c
===
--- test/Driver/split-debug.c
+++ test/Driver/split-debug.c
@@ -5,6 +5,17 @@
//
// CHECK-ACTIONS: "-split-dwarf-file" "split-debug.dwo"
+// RUN: %clang -target x86_64-unknown-linux-gnu -gsplit-dwarf -gsingle-file-split-dwarf -c -### %s 2> %t
+// RUN: FileCheck -check-prefix=CHECK-ACTIONS-SINGLE-SPLIT < %t %s
+//
+// CHECK-ACTIONS-SINGLE-SPLIT: "-single-file-split-dwarf"
+// CHECK-ACTIONS-SINGLE-SPLIT: "-split-dwarf-file" "split-debug.o"
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -gsplit-dwarf -gsingle-file-split-dwarf -c -### -o %tfoo.o %s 2> %t
+// RUN: FileCheck -check-prefix=CHECK-SINGLE-SPLIT-FILENAME < %t %s
+//
+// CHECK-SINGLE-SPLIT-FILENAME: "-split-dwarf-file" "{{.*}}foo.o"
+
// RUN: %clang -target x86_64-macosx -gsplit-dwarf -c -### %s 2> %t
// RUN: FileCheck -check-prefix=CHECK-NO-ACTIONS < %t %s
Index: test/CodeGen/split-debug-single-file.c
===
--- test/CodeGen/split-debug-single-file.c
+++ test/CodeGen/split-debug-single-file.c
@@ -0,0 +1,16 @@
+// REQUIRES: x86-registered-target
+
+// Testing to ensure -single-file-split-dwarf allows to place .dwo sections into regular output object.
+// RUN: %clang_cc1 -debug-info-kind=limited -triple x86_64-unknown-linux \
+// RUN: -enable-split-dwarf -split-dwarf-file %t.o -emit-obj -single-file-split-dwarf -o %t.o %s
+// RUN: llvm-objdump -section-headers %t.o | FileCheck --check-prefix=SINGLE-FILE-SPLIT %s
+// SINGLE-FILE-SPLIT: .dwo
+
+// Testing to ensure that the dwo name gets output into the compile unit.
+// RUN: %clang_cc1 -debug-info-kind=limited -split-dwarf-file foo.o -single-file-split-dwarf \
+// RUN: -S -emit-llvm -o - %s | FileCheck %s --check-prefix=DWONAME
+// DWONAME: !DICompileUnit({{.*}}, splitDebugFilename: "foo.o"
+
+int main (void) {
+ return 0;
+}
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -597,6 +597,8 @@
Opts.EnableSplitDwarf = Args.hasArg(OPT_enable_split_dwarf);
Opts.SplitDwarfFile = Args.getLastArgValue(OPT_split_dwarf_file);
Opts.SplitDwarfInlining = !Args.hasArg(OPT_fno_split_dwarf_inlining);
+ Opts.SingleFileSplitDwarf = Args.hasArg(OPT_single_file_split_dwarf);
+
Opts.DebugTypeExtRefs = Args.hasArg(OPT_dwarf_ext_refs);
Opts.DebugExplicitImport = Args.hasArg(OPT_dwarf_explicit_import);
Opts.DebugFwdTemplateParams = Args.hasArg(OPT_debug_forward_template_params);
Index: lib/Driver/ToolChains/MinGW.cpp
===
--- lib/Driver/ToolChains/MinGW.cpp
+++ lib/Driver/ToolChains/MinGW.cpp
@@ -52,7 +52,7 @@
if (Args.hasArg(options::OPT_gsplit_dwarf))
SplitDebugInfo(getToolChain(), C, *this, JA, Args, Output,
- SplitDebugName(Args, Inputs[0]));
+ SplitDebugName(Args, Inputs[0], Output));
}
void tools::MinGW::Linker::AddLibGCC(const ArgList &Args,
Index: lib/Driver/ToolChains/
[PATCH] D51464: clang: fix MIPS/N32 triple and paths
atanasyan requested changes to this revision.
atanasyan added a comment.
This revision now requires changes to proceed.
This patch fails the following test cases:
- tools/clang/test/CodeGen/target-data.c
- tools/clang/test/Driver/mips-cs.cpp
Comment at: lib/Basic/Targets/Mips.h:75
+ : "n64";
+setABI(getTriple().isMIPS32() ? "o32" : Mips64Abi);
Let's write all cases in a uniform manner:
```
if (Triple.isMIPS32())
setABI("o32");
else if (Triple.getEnvironment() == llvm::Triple::GNUABIN32)
setABI("n32");
else
setABI("n64");
```
Comment at: lib/Driver/ToolChains/Arch/Mips.cpp:109
+ if (ABIName.empty() && (Triple.getEnvironment() == llvm::Triple::GNUABIN32))
+ABIName = "n32";
What about the following combination of a command line arguments?
-target mips-mti-linux-gnuabin32 -mips64
In that case, ABIName is empty, Triple.getVendor() returns MipsTechnologies,
CPUName is "mips64". So ABIName becomes "n64". And this new `if` statement
doesn't work.
Comment at: lib/Driver/ToolChains/Gnu.cpp:2426
if (getTriple().getEnvironment() == llvm::Triple::GNUABI64 ||
-getTriple().isAndroid() ||
-getTriple().isOSFreeBSD() ||
+getTriple().getEnvironment() == llvm::Triple::GNUABIN32 ||
+getTriple().isAndroid() || getTriple().isOSFreeBSD() ||
Before this change we enable integrated assembler for mips64/mips64el
architectures only when we are sure that target ABI is N64. The problem is that
there are some bugs which do not allow the integrated assembler generates
correct N32 code in all cases. After this change we enable integrated assembler
for N32 ABI. I do not think it's a good idea now.
If we can pass command line arguments to this routine, it probably would be
possible to detect N32 ABI by checking both GNUABIN32 environment and
`-mabi=n32` option. And disable integrated assembler for MIPS targets in that
case only. But anyway this change is for another patch.
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] D51484: [OpenCL] Add support of cl_intel_device_side_avc_motion_estimation extension
Anastasia added inline comments. Comment at: include/clang/Basic/OpenCLExtensionTypes.def:27 + +INTEL_SGAVC_TYPE(mce_payload_t, McePayload) +INTEL_SGAVC_TYPE(ime_payload_t, ImePayload) AlexeySotkin wrote: > Anastasia wrote: > > AlexeySachkov wrote: > > > Anastasia wrote: > > > > From the specification of this extension I can't quite see if these > > > > types have to be in Clang instead of the header. Can you please > > > > elaborate on any example where it wouldn't be possible for this type to > > > > be declared in the header using the technique explained in: > > > > https://clang.llvm.org/docs/UsersManual.html#opencl-extensions > > > We cannot define these types in header because their layout is not > > > defined in specification, i.e. all of these types are opaque > > This is not the reason to add functionality to Clang. You can easily sort > > such things with target specific headers or even general headers (see > > `ndrange_t` for example). Spec doesn't have to describe everything. The > > question is whether there is something about those types that can't be > > handled using standard include mechanisms. Usually it's prohibited > > behaviors that can't be represented with such mechanisms. Like if some > > operations have to be disallowed or allowed (since in OpenCL C you can't > > define user defined operators) with the types. > > > > I think the trend is to avoid adding complexity into Clang, unless there is > > no other way to implement some feature correctly. > Major part of these types must support initialization only by zero. > intel_sub_group_avc_mce_payload_t and intel_sub_group_avc_mce_result_t must > support initialization only via special builtins defined in the spec. > Corresponding errors must be reported. I think we can't implement this > behavior using standard include mechanism, can we? > > Possible value of the additional complexity, except builtin declaration, is > that the patch is quite generic. So next time anyone wants to implement an > extension with a type restrictions which can't be handled with the include > mechanism, all that they needs to do is to modify this single file. > Major part of these types must support initialization only by zero. > intel_sub_group_avc_mce_payload_t and intel_sub_group_avc_mce_result_t must > support initialization only via special builtins defined in the spec. > Corresponding errors must be reported. I think we can't implement this > behavior using standard include mechanism, can we? Are these restrictions not mentioned in the specification document then? Or is it not the final version yet (not sure since it says First Draft). Do you plan to add the diagnostics for the restrictions afterwards? It doesn't have to be in the same patch, but just checking because if not I don't think it would make sense to go this route. > Possible value of the additional complexity, except builtin declaration, is > that the patch is quite generic. So next time anyone wants to implement an > extension with a type restrictions which can't be handled with the include > mechanism, all that they needs to do is to modify this single file. > It seems reasonable to implement this extra mechanism, provided that there are more of similar use cases. Btw, considering that there are some modifications to core language spec restriction sections in this document, does this extension invalidate or change any core language rules that might affect parsing/diagnostics? Repository: rC Clang https://reviews.llvm.org/D51484 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52292: [Sema][OpenCL] Improve diagnostics for not viable overloadable function candidates
Anastasia added inline comments.
Comment at: include/clang/Sema/Sema.h:8576
+
+ /// Find and extension in an extension map and return its name
+ template
and extension -> an extension ?
Comment at: lib/Sema/Sema.cpp:1856
+std::string Sema::getOpenCLExtensionsFromDeclExtMap(FunctionDecl *FD) {
+ if (!OpenCLDeclExtMap.empty())
Is this function to be used for both `OpenCLDeclExtMap` and `OpenCLTypeExtMap`?
If yes, may be we could give it more generic name like
'getOpenCLExtensionsFromExtMap'...
Repository:
rC Clang
https://reviews.llvm.org/D52292
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51402: [OpenCL] Adding cl_intel_planar_yuv extension
Anastasia added inline comments. Comment at: lib/Headers/opencl-c.h:26 +#if __OPENCL_C_VERSION__ >= CL_VERSION_1_2 +#ifndef cl_intel_planar_yuv +#define cl_intel_planar_yuv Anastasia wrote: > @yaxunl, do you think we need to add some kind of architecture guard for such > things? Like it should only be added if the architecture supports the > extension? But I guess `-cl-ext=+cl_intel_planar_yuv` trick might not work > here because it's not a Clang known extension? > > So may be the right solution here is to introduce a target specific header? > For now it can be explicitly included but we could think of a target hook to > preload a target specific header... @sidorovd, I am wondering if we could instead extend '-cl-ext=+cl_intel_planar_yuv' to work with non-builtin extensions? Would that be an acceptable solution for vendor extensions to be added to the common header? https://reviews.llvm.org/D51402 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52268: [AST] Squeeze some bits in LinkageComputer
riccibruno updated this revision to Diff 166264.
riccibruno marked 3 inline comments as done.
riccibruno added a comment.
Removed the superfluous static_assert.
Repository:
rC Clang
https://reviews.llvm.org/D52268
Files:
lib/AST/Linkage.h
Index: lib/AST/Linkage.h
===
--- lib/AST/Linkage.h
+++ lib/AST/Linkage.h
@@ -20,6 +20,7 @@
#include "clang/AST/Type.h"
#include "llvm/ADT/DenseMap.h"
#include "llvm/ADT/Optional.h"
+#include "llvm/ADT/PointerIntPair.h"
namespace clang {
/// Kinds of LV computation. The linkage side of the computation is
@@ -36,6 +37,8 @@
/// in computing linkage.
unsigned IgnoreAllVisibility : 1;
+ enum { NumLVComputationKindBits = 3 };
+
explicit LVComputationKind(NamedDecl::ExplicitVisibilityKind EK)
: ExplicitKind(EK), IgnoreExplicitVisibility(false),
IgnoreAllVisibility(false) {}
@@ -78,12 +81,14 @@
// using C = Foo;
// using D = Foo;
//
- // The unsigned represents an LVComputationKind.
- using QueryType = std::pair;
+ // The integer represents an LVComputationKind.
+ using QueryType =
+ llvm::PointerIntPair;
llvm::SmallDenseMap CachedLinkageInfo;
static QueryType makeCacheKey(const NamedDecl *ND, LVComputationKind Kind) {
-return std::make_pair(ND, Kind.toBits());
+return QueryType(ND, Kind.toBits());
}
llvm::Optional lookup(const NamedDecl *ND,
Index: lib/AST/Linkage.h
===
--- lib/AST/Linkage.h
+++ lib/AST/Linkage.h
@@ -20,6 +20,7 @@
#include "clang/AST/Type.h"
#include "llvm/ADT/DenseMap.h"
#include "llvm/ADT/Optional.h"
+#include "llvm/ADT/PointerIntPair.h"
namespace clang {
/// Kinds of LV computation. The linkage side of the computation is
@@ -36,6 +37,8 @@
/// in computing linkage.
unsigned IgnoreAllVisibility : 1;
+ enum { NumLVComputationKindBits = 3 };
+
explicit LVComputationKind(NamedDecl::ExplicitVisibilityKind EK)
: ExplicitKind(EK), IgnoreExplicitVisibility(false),
IgnoreAllVisibility(false) {}
@@ -78,12 +81,14 @@
// using C = Foo;
// using D = Foo;
//
- // The unsigned represents an LVComputationKind.
- using QueryType = std::pair;
+ // The integer represents an LVComputationKind.
+ using QueryType =
+ llvm::PointerIntPair;
llvm::SmallDenseMap CachedLinkageInfo;
static QueryType makeCacheKey(const NamedDecl *ND, LVComputationKind Kind) {
-return std::make_pair(ND, Kind.toBits());
+return QueryType(ND, Kind.toBits());
}
llvm::Optional lookup(const NamedDecl *ND,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52280: Don't trim non-alphanumeric characters in 'file not found' errors for include directives.
christylee accepted this revision. christylee added a comment. This revision is now accepted and ready to land. Thanks for the fix! Repository: rC Clang https://reviews.llvm.org/D52280 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52047: [clangd] Add building benchmark and memory consumption tracking
kbobyrev updated this revision to Diff 166266.
kbobyrev retitled this revision from "[clangd] Add a "benchmark" for tracking
memory" to "[clangd] Add building benchmark and memory consumption tracking".
kbobyrev edited the summary of this revision.
kbobyrev added a comment.
Add symbol index building benchmarks, split `loadIndex()` into
`symbolsFromFile()` and actual index build.
https://reviews.llvm.org/D52047
Files:
clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
clang-tools-extra/clangd/index/SymbolYAML.cpp
clang-tools-extra/clangd/index/SymbolYAML.h
clang-tools-extra/clangd/index/dex/Dex.cpp
Index: clang-tools-extra/clangd/index/dex/Dex.cpp
===
--- clang-tools-extra/clangd/index/dex/Dex.cpp
+++ clang-tools-extra/clangd/index/dex/Dex.cpp
@@ -124,9 +124,6 @@
for (const auto &TokenToPostingList : TempInvertedIndex)
InvertedIndex.insert({TokenToPostingList.first,
PostingList(move(TokenToPostingList.second))});
-
- vlog("Built Dex with estimated memory usage {0} bytes.",
- estimateMemoryUsage());
}
/// Constructs iterators over tokens extracted from the query and exhausts it
@@ -239,7 +236,7 @@
Bytes += LookupTable.getMemorySize();
Bytes += InvertedIndex.getMemorySize();
for (const auto &P : InvertedIndex)
-Bytes += P.second.bytes();
+Bytes += P.first.Data.size() + P.second.bytes();
return Bytes + BackingDataSize;
}
Index: clang-tools-extra/clangd/index/SymbolYAML.h
===
--- clang-tools-extra/clangd/index/SymbolYAML.h
+++ clang-tools-extra/clangd/index/SymbolYAML.h
@@ -29,6 +29,9 @@
// Read symbols from a YAML-format string.
SymbolSlab symbolsFromYAML(llvm::StringRef YAMLContent);
+// Read symbols from YAML or RIFF file.
+llvm::Optional symbolsFromFile(llvm::StringRef SymbolFilename);
+
// Read one symbol from a YAML-stream.
// The returned symbol is backed by Input.
Symbol SymbolFromYAML(llvm::yaml::Input &Input);
Index: clang-tools-extra/clangd/index/SymbolYAML.cpp
===
--- clang-tools-extra/clangd/index/SymbolYAML.cpp
+++ clang-tools-extra/clangd/index/SymbolYAML.cpp
@@ -9,6 +9,7 @@
#include "SymbolYAML.h"
#include "Index.h"
+#include "Logger.h"
#include "Serialization.h"
#include "Trace.h"
#include "dex/Dex.h"
@@ -182,6 +183,28 @@
return std::move(Syms).build();
}
+llvm::Optional symbolsFromFile(llvm::StringRef SymbolFilename) {
+ auto Buffer = llvm::MemoryBuffer::getFile(SymbolFilename);
+ if (!Buffer) {
+llvm::errs() << "Can't open " << SymbolFilename << "\n";
+return llvm::None;
+ }
+ StringRef Data = Buffer->get()->getBuffer();
+
+ llvm::Optional Slab;
+ if (Data.startswith("RIFF")) { // Magic for binary index file.
+trace::Span Tracer("ParseRIFF");
+if (auto RIFF = readIndexFile(Data))
+ Slab = std::move(RIFF->Symbols);
+else
+ llvm::errs() << "Bad RIFF: " << llvm::toString(RIFF.takeError()) << "\n";
+ } else {
+trace::Span Tracer("ParseYAML");
+Slab = symbolsFromYAML(Data);
+ }
+ return Slab;
+}
+
Symbol SymbolFromYAML(llvm::yaml::Input &Input) {
Symbol S;
Input >> S;
@@ -206,30 +229,16 @@
llvm::ArrayRef URISchemes,
bool UseDex) {
trace::Span OverallTracer("LoadIndex");
- auto Buffer = llvm::MemoryBuffer::getFile(SymbolFilename);
- if (!Buffer) {
-llvm::errs() << "Can't open " << SymbolFilename << "\n";
-return nullptr;
- }
- StringRef Data = Buffer->get()->getBuffer();
-
- llvm::Optional Slab;
- if (Data.startswith("RIFF")) { // Magic for binary index file.
-trace::Span Tracer("ParseRIFF");
-if (auto RIFF = readIndexFile(Data))
- Slab = std::move(RIFF->Symbols);
-else
- llvm::errs() << "Bad RIFF: " << llvm::toString(RIFF.takeError()) << "\n";
- } else {
-trace::Span Tracer("ParseYAML");
-Slab = symbolsFromYAML(Data);
- }
-
+ auto Slab = symbolsFromFile(SymbolFilename);
if (!Slab)
return nullptr;
trace::Span Tracer("BuildIndex");
- return UseDex ? dex::Dex::build(std::move(*Slab), URISchemes)
-: MemIndex::build(std::move(*Slab), RefSlab());
+ auto Index = UseDex ? dex::Dex::build(std::move(*Slab), URISchemes)
+ : MemIndex::build(std::move(*Slab), RefSlab());
+ vlog("Loaded {0} from {1} with estimated memory usage {2}",
+ UseDex ? "Dex" : "MemIndex", SymbolFilename,
+ Index->estimateMemoryUsage());
+ return Index;
}
} // namespace clangd
Index: clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
===
--- clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
+++ clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
@@ -33,6 +33,16 @@
return loadIndex(IndexFilename, {}, true);
}
+Symbol
[PATCH] D52292: [Sema][OpenCL] Improve diagnostics for not viable overloadable function candidates
sidorovd updated this revision to Diff 166270.
sidorovd marked an inline comment as done.
https://reviews.llvm.org/D52292
Files:
include/clang/Basic/DiagnosticSemaKinds.td
include/clang/Sema/Sema.h
lib/Sema/Sema.cpp
lib/Sema/SemaOverload.cpp
test/SemaOpenCL/extension-begin.cl
Index: test/SemaOpenCL/extension-begin.cl
===
--- test/SemaOpenCL/extension-begin.cl
+++ test/SemaOpenCL/extension-begin.cl
@@ -48,7 +48,7 @@
PointerOfA test_A_pointer; // expected-error {{use of type 'PointerOfA' (aka
'const struct A *') requires my_ext extension to be enabled}}
f(); // expected-error {{use of declaration 'f' requires my_ext extension to
be enabled}}
g(0); // expected-error {{no matching function for call to 'g'}}
-// expected-note@-26 {{candidate disabled due to OpenCL extension}}
+// expected-note@-26 {{candidate unavailable as it requires OpenCL
extension 'my_ext' to be disabled}}
// expected-note@-22 {{candidate function not viable: requires 0
arguments, but 1 was provided}}
}
Index: lib/Sema/SemaOverload.cpp
===
--- lib/Sema/SemaOverload.cpp
+++ lib/Sema/SemaOverload.cpp
@@ -10242,7 +10242,8 @@
FunctionDecl *Callee = Cand->Function;
S.Diag(Callee->getLocation(),
- diag::note_ovl_candidate_disabled_by_extension);
+ diag::note_ovl_candidate_disabled_by_extension)
+<< S.getOpenCLExtensionsFromDeclExtMap(Callee);
}
/// Generates a 'note' diagnostic for an overload candidate. We've
Index: lib/Sema/Sema.cpp
===
--- lib/Sema/Sema.cpp
+++ lib/Sema/Sema.cpp
@@ -1853,6 +1853,27 @@
setOpenCLExtensionForDecl(D, CurrOpenCLExtension);
}
+std::string Sema::getOpenCLExtensionsFromDeclExtMap(FunctionDecl *FD) {
+ if (!OpenCLDeclExtMap.empty())
+return getOpenCLExtensionsFromExtMap(FD, OpenCLDeclExtMap);
+
+ return "";
+}
+
+template
+std::string Sema::getOpenCLExtensionsFromExtMap(T *FDT, MapT &Map) {
+ std::string ExtensionNames = "";
+ auto Loc = Map.find(FDT);
+
+ for (auto const& I : Loc->second) {
+ExtensionNames += I;
+ExtensionNames += " ";
+ }
+ ExtensionNames.pop_back();
+
+ return ExtensionNames;
+}
+
bool Sema::isOpenCLDisabledDecl(Decl *FD) {
auto Loc = OpenCLDeclExtMap.find(FD);
if (Loc == OpenCLDeclExtMap.end())
Index: include/clang/Sema/Sema.h
===
--- include/clang/Sema/Sema.h
+++ include/clang/Sema/Sema.h
@@ -8567,6 +8567,16 @@
llvm::StringRef getCurrentOpenCLExtension() const {
return CurrOpenCLExtension;
}
+
+ /// Check if a function declaration \p FD associates with any
+ /// extensions present in OpenCLDeclExtMap and if so return the
+ /// extension(s) name(s).
+ std::string getOpenCLExtensionsFromDeclExtMap(FunctionDecl *FD);
+
+ /// Find an extension in appropriate extension map and return its name
+ template
+ std::string getOpenCLExtensionsFromExtMap(T* FT, MapT &Map);
+
void setCurrentOpenCLExtension(llvm::StringRef Ext) {
CurrOpenCLExtension = Ext;
}
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -3652,7 +3652,7 @@
def note_ovl_candidate_disabled_by_function_cond_attr : Note<
"candidate disabled: %0">;
def note_ovl_candidate_disabled_by_extension : Note<
-"candidate disabled due to OpenCL extension">;
+"candidate unavailable as it requires OpenCL extension '%0' to be
disabled">;
def err_addrof_function_disabled_by_enable_if_attr : Error<
"cannot take address of function %0 because it has one or more "
"non-tautological enable_if conditions">;
Index: test/SemaOpenCL/extension-begin.cl
===
--- test/SemaOpenCL/extension-begin.cl
+++ test/SemaOpenCL/extension-begin.cl
@@ -48,7 +48,7 @@
PointerOfA test_A_pointer; // expected-error {{use of type 'PointerOfA' (aka 'const struct A *') requires my_ext extension to be enabled}}
f(); // expected-error {{use of declaration 'f' requires my_ext extension to be enabled}}
g(0); // expected-error {{no matching function for call to 'g'}}
-// expected-note@-26 {{candidate disabled due to OpenCL extension}}
+// expected-note@-26 {{candidate unavailable as it requires OpenCL extension 'my_ext' to be disabled}}
// expected-note@-22 {{candidate function not viable: requires 0 arguments, but 1 was provided}}
}
Index: lib/Sema/SemaOverload.cpp
===
--- lib/Sema/SemaOverload.cpp
+++ lib/Sema/SemaOverload.cpp
@@ -10242,7 +10242,8 @@
FunctionDecl *Callee = Cand->Function;
S.Diag(Callee->getLocation(),
- diag::no
[PATCH] D52047: [clangd] Add building benchmark and memory consumption tracking
kbobyrev updated this revision to Diff 166269.
kbobyrev added a comment.
Add benchmark for `SymbolSlab` loading from file. This might be useful for
RIFF/YAML symbol loader optimizations.
https://reviews.llvm.org/D52047
Files:
clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
clang-tools-extra/clangd/index/SymbolYAML.cpp
clang-tools-extra/clangd/index/SymbolYAML.h
clang-tools-extra/clangd/index/dex/Dex.cpp
Index: clang-tools-extra/clangd/index/dex/Dex.cpp
===
--- clang-tools-extra/clangd/index/dex/Dex.cpp
+++ clang-tools-extra/clangd/index/dex/Dex.cpp
@@ -124,9 +124,6 @@
for (const auto &TokenToPostingList : TempInvertedIndex)
InvertedIndex.insert({TokenToPostingList.first,
PostingList(move(TokenToPostingList.second))});
-
- vlog("Built Dex with estimated memory usage {0} bytes.",
- estimateMemoryUsage());
}
/// Constructs iterators over tokens extracted from the query and exhausts it
@@ -239,7 +236,7 @@
Bytes += LookupTable.getMemorySize();
Bytes += InvertedIndex.getMemorySize();
for (const auto &P : InvertedIndex)
-Bytes += P.second.bytes();
+Bytes += P.first.Data.size() + P.second.bytes();
return Bytes + BackingDataSize;
}
Index: clang-tools-extra/clangd/index/SymbolYAML.h
===
--- clang-tools-extra/clangd/index/SymbolYAML.h
+++ clang-tools-extra/clangd/index/SymbolYAML.h
@@ -29,6 +29,9 @@
// Read symbols from a YAML-format string.
SymbolSlab symbolsFromYAML(llvm::StringRef YAMLContent);
+// Read symbols from YAML or RIFF file.
+llvm::Optional symbolsFromFile(llvm::StringRef SymbolFilename);
+
// Read one symbol from a YAML-stream.
// The returned symbol is backed by Input.
Symbol SymbolFromYAML(llvm::yaml::Input &Input);
Index: clang-tools-extra/clangd/index/SymbolYAML.cpp
===
--- clang-tools-extra/clangd/index/SymbolYAML.cpp
+++ clang-tools-extra/clangd/index/SymbolYAML.cpp
@@ -9,6 +9,7 @@
#include "SymbolYAML.h"
#include "Index.h"
+#include "Logger.h"
#include "Serialization.h"
#include "Trace.h"
#include "dex/Dex.h"
@@ -182,6 +183,28 @@
return std::move(Syms).build();
}
+llvm::Optional symbolsFromFile(llvm::StringRef SymbolFilename) {
+ auto Buffer = llvm::MemoryBuffer::getFile(SymbolFilename);
+ if (!Buffer) {
+llvm::errs() << "Can't open " << SymbolFilename << "\n";
+return llvm::None;
+ }
+ StringRef Data = Buffer->get()->getBuffer();
+
+ llvm::Optional Slab;
+ if (Data.startswith("RIFF")) { // Magic for binary index file.
+trace::Span Tracer("ParseRIFF");
+if (auto RIFF = readIndexFile(Data))
+ Slab = std::move(RIFF->Symbols);
+else
+ llvm::errs() << "Bad RIFF: " << llvm::toString(RIFF.takeError()) << "\n";
+ } else {
+trace::Span Tracer("ParseYAML");
+Slab = symbolsFromYAML(Data);
+ }
+ return Slab;
+}
+
Symbol SymbolFromYAML(llvm::yaml::Input &Input) {
Symbol S;
Input >> S;
@@ -206,30 +229,16 @@
llvm::ArrayRef URISchemes,
bool UseDex) {
trace::Span OverallTracer("LoadIndex");
- auto Buffer = llvm::MemoryBuffer::getFile(SymbolFilename);
- if (!Buffer) {
-llvm::errs() << "Can't open " << SymbolFilename << "\n";
-return nullptr;
- }
- StringRef Data = Buffer->get()->getBuffer();
-
- llvm::Optional Slab;
- if (Data.startswith("RIFF")) { // Magic for binary index file.
-trace::Span Tracer("ParseRIFF");
-if (auto RIFF = readIndexFile(Data))
- Slab = std::move(RIFF->Symbols);
-else
- llvm::errs() << "Bad RIFF: " << llvm::toString(RIFF.takeError()) << "\n";
- } else {
-trace::Span Tracer("ParseYAML");
-Slab = symbolsFromYAML(Data);
- }
-
+ auto Slab = symbolsFromFile(SymbolFilename);
if (!Slab)
return nullptr;
trace::Span Tracer("BuildIndex");
- return UseDex ? dex::Dex::build(std::move(*Slab), URISchemes)
-: MemIndex::build(std::move(*Slab), RefSlab());
+ auto Index = UseDex ? dex::Dex::build(std::move(*Slab), URISchemes)
+ : MemIndex::build(std::move(*Slab), RefSlab());
+ vlog("Loaded {0} from {1} with estimated memory usage {2}",
+ UseDex ? "Dex" : "MemIndex", SymbolFilename,
+ Index->estimateMemoryUsage());
+ return Index;
}
} // namespace clangd
Index: clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
===
--- clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
+++ clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
@@ -33,6 +33,16 @@
return loadIndex(IndexFilename, {}, true);
}
+SymbolSlab loadSlab() {
+ auto Slab = symbolsFromFile(IndexFilename);
+ if (!Slab) {
+llvm::errs() << "Error when loading SymbolSlab from file: " << IndexFilename
+ << '\n';
+e
[PATCH] D52292: [Sema][OpenCL] Improve diagnostics for not viable overloadable function candidates
sidorovd added inline comments.
Comment at: include/clang/Sema/Sema.h:8576
+
+ /// Find and extension in an extension map and return its name
+ template
Anastasia wrote:
> and extension -> an extension ?
Thanks!
Comment at: lib/Sema/Sema.cpp:1856
+std::string Sema::getOpenCLExtensionsFromDeclExtMap(FunctionDecl *FD) {
+ if (!OpenCLDeclExtMap.empty())
Anastasia wrote:
> Is this function to be used for both `OpenCLDeclExtMap` and
> `OpenCLTypeExtMap`? If yes, may be we could give it more generic name like
> 'getOpenCLExtensionsFromExtMap'...
No, this exact function is only for 'OpenCLDeclExtMap', for the type map one
should implement a new function 'getOpenCLExtensionsFromTypeExtMap'. Actually I
have done this for https://reviews.llvm.org/D51341 to make the patch more
generic, but since I'm not sure if it ever came to light I can add it here
unused.
https://reviews.llvm.org/D52292
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52047: [clangd] Add building benchmark and memory consumption tracking
kbobyrev updated this revision to Diff 166271.
kbobyrev added a comment.
Remove `BuildMem` benchmark, which collects data about `MemIndex` building time
(which is essentially nothing and therefore not really interesting).
https://reviews.llvm.org/D52047
Files:
clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
clang-tools-extra/clangd/index/SymbolYAML.cpp
clang-tools-extra/clangd/index/SymbolYAML.h
clang-tools-extra/clangd/index/dex/Dex.cpp
Index: clang-tools-extra/clangd/index/dex/Dex.cpp
===
--- clang-tools-extra/clangd/index/dex/Dex.cpp
+++ clang-tools-extra/clangd/index/dex/Dex.cpp
@@ -124,9 +124,6 @@
for (const auto &TokenToPostingList : TempInvertedIndex)
InvertedIndex.insert({TokenToPostingList.first,
PostingList(move(TokenToPostingList.second))});
-
- vlog("Built Dex with estimated memory usage {0} bytes.",
- estimateMemoryUsage());
}
/// Constructs iterators over tokens extracted from the query and exhausts it
@@ -239,7 +236,7 @@
Bytes += LookupTable.getMemorySize();
Bytes += InvertedIndex.getMemorySize();
for (const auto &P : InvertedIndex)
-Bytes += P.second.bytes();
+Bytes += P.first.Data.size() + P.second.bytes();
return Bytes + BackingDataSize;
}
Index: clang-tools-extra/clangd/index/SymbolYAML.h
===
--- clang-tools-extra/clangd/index/SymbolYAML.h
+++ clang-tools-extra/clangd/index/SymbolYAML.h
@@ -29,6 +29,9 @@
// Read symbols from a YAML-format string.
SymbolSlab symbolsFromYAML(llvm::StringRef YAMLContent);
+// Read symbols from YAML or RIFF file.
+llvm::Optional symbolsFromFile(llvm::StringRef SymbolFilename);
+
// Read one symbol from a YAML-stream.
// The returned symbol is backed by Input.
Symbol SymbolFromYAML(llvm::yaml::Input &Input);
Index: clang-tools-extra/clangd/index/SymbolYAML.cpp
===
--- clang-tools-extra/clangd/index/SymbolYAML.cpp
+++ clang-tools-extra/clangd/index/SymbolYAML.cpp
@@ -9,6 +9,7 @@
#include "SymbolYAML.h"
#include "Index.h"
+#include "Logger.h"
#include "Serialization.h"
#include "Trace.h"
#include "dex/Dex.h"
@@ -182,6 +183,28 @@
return std::move(Syms).build();
}
+llvm::Optional symbolsFromFile(llvm::StringRef SymbolFilename) {
+ auto Buffer = llvm::MemoryBuffer::getFile(SymbolFilename);
+ if (!Buffer) {
+llvm::errs() << "Can't open " << SymbolFilename << "\n";
+return llvm::None;
+ }
+ StringRef Data = Buffer->get()->getBuffer();
+
+ llvm::Optional Slab;
+ if (Data.startswith("RIFF")) { // Magic for binary index file.
+trace::Span Tracer("ParseRIFF");
+if (auto RIFF = readIndexFile(Data))
+ Slab = std::move(RIFF->Symbols);
+else
+ llvm::errs() << "Bad RIFF: " << llvm::toString(RIFF.takeError()) << "\n";
+ } else {
+trace::Span Tracer("ParseYAML");
+Slab = symbolsFromYAML(Data);
+ }
+ return Slab;
+}
+
Symbol SymbolFromYAML(llvm::yaml::Input &Input) {
Symbol S;
Input >> S;
@@ -206,30 +229,16 @@
llvm::ArrayRef URISchemes,
bool UseDex) {
trace::Span OverallTracer("LoadIndex");
- auto Buffer = llvm::MemoryBuffer::getFile(SymbolFilename);
- if (!Buffer) {
-llvm::errs() << "Can't open " << SymbolFilename << "\n";
-return nullptr;
- }
- StringRef Data = Buffer->get()->getBuffer();
-
- llvm::Optional Slab;
- if (Data.startswith("RIFF")) { // Magic for binary index file.
-trace::Span Tracer("ParseRIFF");
-if (auto RIFF = readIndexFile(Data))
- Slab = std::move(RIFF->Symbols);
-else
- llvm::errs() << "Bad RIFF: " << llvm::toString(RIFF.takeError()) << "\n";
- } else {
-trace::Span Tracer("ParseYAML");
-Slab = symbolsFromYAML(Data);
- }
-
+ auto Slab = symbolsFromFile(SymbolFilename);
if (!Slab)
return nullptr;
trace::Span Tracer("BuildIndex");
- return UseDex ? dex::Dex::build(std::move(*Slab), URISchemes)
-: MemIndex::build(std::move(*Slab), RefSlab());
+ auto Index = UseDex ? dex::Dex::build(std::move(*Slab), URISchemes)
+ : MemIndex::build(std::move(*Slab), RefSlab());
+ vlog("Loaded {0} from {1} with estimated memory usage {2}",
+ UseDex ? "Dex" : "MemIndex", SymbolFilename,
+ Index->estimateMemoryUsage());
+ return Index;
}
} // namespace clangd
Index: clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
===
--- clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
+++ clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
@@ -33,6 +33,16 @@
return loadIndex(IndexFilename, {}, true);
}
+SymbolSlab loadSlab() {
+ auto Slab = symbolsFromFile(IndexFilename);
+ if (!Slab) {
+llvm::errs() << "Error when loading SymbolSlab from file: " << IndexFilenam
[PATCH] D52281: [clang-tidy] Add modernize check to use std::invoke in generic code
pfultz2 added a comment. This needs a test when calling in a `constexpr` function. I believe `std::invoke` is not `constepxr`, so a function object call in a `constexpr` function should not suggest `std::invoke`. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52281 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52300: [clangd] Implement VByte PostingList compression
kbobyrev created this revision.
kbobyrev added reviewers: ioeric, sammccall, ilya-biryukov.
kbobyrev added a project: clang-tools-extra.
Herald added subscribers: kadircet, arphaman, jkorous, MaskRay, mgorny.
kbobyrev edited the summary of this revision.
This patch implements Variable-length Byte compression of `PostingList`s to
sacrifice some performance for lower memory consumption.
`PostingList` compression and decompression was extensively tested using fuzzer
for multiple hours and runnning significant number of realistic
`FuzzyFindRequests`. AddressSanitizer and UndefinedBehaviorSanitizer were used
to ensure the correct behaviour.
Performance evaluation was conducted with recent LLVM symbol index (292k
symbols) and the collection of user-recorded queries (5540 `FuzzyFindRequest`
JSON dumps):
| Metrics | Before | After | Change (%) |
| --- | -- | - | -- |
| Memory consumption (index only), MB | 65 | 52| -20% |
| Time to process queries, sec| 5.370 | 7.145 | +25% |
https://reviews.llvm.org/D52300
Files:
clang-tools-extra/clangd/CMakeLists.txt
clang-tools-extra/clangd/index/dex/Dex.cpp
clang-tools-extra/clangd/index/dex/PostingList.cpp
clang-tools-extra/clangd/index/dex/PostingList.h
clang-tools-extra/clangd/index/dex/fuzzer/CMakeLists.txt
clang-tools-extra/clangd/index/dex/fuzzer/VByteFuzzer.cpp
Index: clang-tools-extra/clangd/index/dex/fuzzer/VByteFuzzer.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/index/dex/fuzzer/VByteFuzzer.cpp
@@ -0,0 +1,64 @@
+//===-- VByteFuzzer.cpp - Fuzz VByte Posting List encoding ===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+///
+/// \file
+/// \brief This file implements a function that runs clangd on a single input.
+/// This function is then linked into the Fuzzer library.
+///
+//===--===//
+
+#include "../Iterator.h"
+#include "../PostingList.h"
+#include "llvm/Support/Compiler.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/raw_ostream.h"
+#include
+#include
+
+using DocID = clang::clangd::dex::DocID;
+
+/// Transform raw byte sequence into list of DocIDs.
+std::vector generateDocuments(uint8_t *Data, size_t Size) {
+ std::vector Result;
+ DocID ID = 0;
+ for (size_t I = 0; I < Size; ++I) {
+size_t Offset = I % 4;
+if (Offset == 0 && I != 0) {
+ ID = 0;
+ Result.push_back(ID);
+}
+ID |= (Data[I] << Offset);
+ }
+ if (Size > 4 && Size % 4 != 0)
+Result.push_back(ID);
+ return Result;
+}
+
+/// This fuzzer checks that compressed PostingList contains can be successfully
+/// decoded into the original sequence.
+extern "C" int LLVMFuzzerTestOneInput(uint8_t *Data, size_t Size) {
+ if (Size == 0)
+return 0;
+ const auto OriginalDocuments = generateDocuments(Data, Size);
+ if (OriginalDocuments.empty())
+return 0;
+ // Ensure that given sequence of DocIDs is sorted.
+ for (size_t I = 1; I < OriginalDocuments.size(); ++I)
+if (OriginalDocuments[I] <= OriginalDocuments[I - 1])
+ return 0;
+ const clang::clangd::dex::PostingList List(OriginalDocuments);
+ const auto DecodedDocuments = clang::clangd::dex::consume(*List.iterator());
+ // Compare decoded sequence against the original PostingList contents.
+ if (DecodedDocuments.size() != OriginalDocuments.size())
+LLVM_BUILTIN_TRAP;
+ for (size_t I = 0; I < DecodedDocuments.size(); ++I)
+if (DecodedDocuments[I].first != OriginalDocuments[I])
+ LLVM_BUILTIN_TRAP;
+ return 0;
+}
Index: clang-tools-extra/clangd/index/dex/fuzzer/CMakeLists.txt
===
--- /dev/null
+++ clang-tools-extra/clangd/index/dex/fuzzer/CMakeLists.txt
@@ -0,0 +1,19 @@
+include_directories(${CMAKE_CURRENT_SOURCE_DIR}/..)
+
+set(LLVM_LINK_COMPONENTS Support)
+
+if(LLVM_USE_SANITIZE_COVERAGE)
+ set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=fuzzer")
+endif()
+
+add_clang_executable(clangd-vbyte-fuzzer
+ EXCLUDE_FROM_ALL
+ VByteFuzzer.cpp
+ )
+
+target_link_libraries(clangd-vbyte-fuzzer
+ PRIVATE
+ clangBasic
+ clangDaemon
+ ${LLVM_LIB_FUZZING_ENGINE}
+ )
Index: clang-tools-extra/clangd/index/dex/PostingList.h
===
--- clang-tools-extra/clangd/index/dex/PostingList.h
+++ clang-tools-extra/clangd/index/dex/PostingList.h
@@ -6,13 +6,19 @@
// License. See LICENSE.TXT for details.
//
//===--===//
-//
-// This defines posting list interface: a storage for identifiers of
[PATCH] D52300: [clangd] Implement VByte PostingList compression
kbobyrev updated this revision to Diff 166278.
kbobyrev added a comment.
- Update unit tests with iterator tree string representation to comply with the
new format
- Don't mark constructor `explicit` (previously it only had one parameter)
- Fix `Limits` explanation comment (`ID > Limits[I]` -> `ID >= Limits[I]`)
https://reviews.llvm.org/D52300
Files:
clang-tools-extra/clangd/CMakeLists.txt
clang-tools-extra/clangd/index/dex/Dex.cpp
clang-tools-extra/clangd/index/dex/PostingList.cpp
clang-tools-extra/clangd/index/dex/PostingList.h
clang-tools-extra/clangd/index/dex/fuzzer/CMakeLists.txt
clang-tools-extra/clangd/index/dex/fuzzer/VByteFuzzer.cpp
clang-tools-extra/unittests/clangd/DexTests.cpp
Index: clang-tools-extra/unittests/clangd/DexTests.cpp
===
--- clang-tools-extra/unittests/clangd/DexTests.cpp
+++ clang-tools-extra/unittests/clangd/DexTests.cpp
@@ -262,13 +262,14 @@
const PostingList L4({0, 1, 5});
const PostingList L5({});
- EXPECT_EQ(llvm::to_string(*(L0.iterator())), "[4]");
+ EXPECT_EQ(llvm::to_string(*(L0.iterator())), "[4 ...]");
auto Nested =
createAnd(createAnd(L1.iterator(), L2.iterator()),
createOr(L3.iterator(), L4.iterator(), L5.iterator()));
- EXPECT_EQ(llvm::to_string(*Nested), "(& (| [5] [1] [END]) (& [1] [1]))");
+ EXPECT_EQ(llvm::to_string(*Nested),
+"(& (| [... 5] [... 1 ...] [END]) (& [1 ...] [1 ...]))");
}
TEST(DexIterators, Limit) {
Index: clang-tools-extra/clangd/index/dex/fuzzer/VByteFuzzer.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/index/dex/fuzzer/VByteFuzzer.cpp
@@ -0,0 +1,64 @@
+//===-- VByteFuzzer.cpp - Fuzz VByte Posting List encoding ===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+///
+/// \file
+/// \brief This file implements a function that runs clangd on a single input.
+/// This function is then linked into the Fuzzer library.
+///
+//===--===//
+
+#include "../Iterator.h"
+#include "../PostingList.h"
+#include "llvm/Support/Compiler.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/raw_ostream.h"
+#include
+#include
+
+using DocID = clang::clangd::dex::DocID;
+
+/// Transform raw byte sequence into list of DocIDs.
+std::vector generateDocuments(uint8_t *Data, size_t Size) {
+ std::vector Result;
+ DocID ID = 0;
+ for (size_t I = 0; I < Size; ++I) {
+size_t Offset = I % 4;
+if (Offset == 0 && I != 0) {
+ ID = 0;
+ Result.push_back(ID);
+}
+ID |= (Data[I] << Offset);
+ }
+ if (Size > 4 && Size % 4 != 0)
+Result.push_back(ID);
+ return Result;
+}
+
+/// This fuzzer checks that compressed PostingList contains can be successfully
+/// decoded into the original sequence.
+extern "C" int LLVMFuzzerTestOneInput(uint8_t *Data, size_t Size) {
+ if (Size == 0)
+return 0;
+ const auto OriginalDocuments = generateDocuments(Data, Size);
+ if (OriginalDocuments.empty())
+return 0;
+ // Ensure that given sequence of DocIDs is sorted.
+ for (size_t I = 1; I < OriginalDocuments.size(); ++I)
+if (OriginalDocuments[I] <= OriginalDocuments[I - 1])
+ return 0;
+ const clang::clangd::dex::PostingList List(OriginalDocuments);
+ const auto DecodedDocuments = clang::clangd::dex::consume(*List.iterator());
+ // Compare decoded sequence against the original PostingList contents.
+ if (DecodedDocuments.size() != OriginalDocuments.size())
+LLVM_BUILTIN_TRAP;
+ for (size_t I = 0; I < DecodedDocuments.size(); ++I)
+if (DecodedDocuments[I].first != OriginalDocuments[I])
+ LLVM_BUILTIN_TRAP;
+ return 0;
+}
Index: clang-tools-extra/clangd/index/dex/fuzzer/CMakeLists.txt
===
--- /dev/null
+++ clang-tools-extra/clangd/index/dex/fuzzer/CMakeLists.txt
@@ -0,0 +1,19 @@
+include_directories(${CMAKE_CURRENT_SOURCE_DIR}/..)
+
+set(LLVM_LINK_COMPONENTS Support)
+
+if(LLVM_USE_SANITIZE_COVERAGE)
+ set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=fuzzer")
+endif()
+
+add_clang_executable(clangd-vbyte-fuzzer
+ EXCLUDE_FROM_ALL
+ VByteFuzzer.cpp
+ )
+
+target_link_libraries(clangd-vbyte-fuzzer
+ PRIVATE
+ clangBasic
+ clangDaemon
+ ${LLVM_LIB_FUZZING_ENGINE}
+ )
Index: clang-tools-extra/clangd/index/dex/PostingList.h
===
--- clang-tools-extra/clangd/index/dex/PostingList.h
+++ clang-tools-extra/clangd/index/dex/PostingList.h
@@ -6,13 +6,19 @@
// License. See LICENSE.TXT for details.
//
//===--===//
-
[PATCH] D51211: [Sema] Emit -Wformat properly for bitfield promotions.
ebevhan added a comment. Another ping. Anyone up for reviewing this patch? Repository: rC Clang https://reviews.llvm.org/D51211 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52301: [clang] Set TypeSourceInfo for vardecl's in addition to type when we can deduce.
kadircet created this revision.
kadircet added a reviewer: ilya-biryukov.
Herald added a subscriber: cfe-commits.
Repository:
rC Clang
https://reviews.llvm.org/D52301
Files:
include/clang/AST/PrettyPrinter.h
include/clang/Basic/DiagnosticSemaKinds.td
include/clang/Sema/Sema.h
lib/AST/TypePrinter.cpp
lib/Frontend/ASTConsumers.cpp
lib/Sema/SemaChecking.cpp
lib/Sema/SemaDecl.cpp
lib/Sema/SemaExprCXX.cpp
lib/Sema/SemaLambda.cpp
lib/Sema/SemaStmt.cpp
lib/Sema/SemaTemplate.cpp
lib/Sema/SemaTemplateDeduction.cpp
lib/Tooling/Refactoring/Rename/USRLocFinder.cpp
test/Analysis/scopes-cfg-output.cpp
test/Driver/Xarch.c
test/SemaOpenCL/to_addr_builtin.cl
Index: test/SemaOpenCL/to_addr_builtin.cl
===
--- test/SemaOpenCL/to_addr_builtin.cl
+++ test/SemaOpenCL/to_addr_builtin.cl
@@ -1,5 +1,5 @@
// RUN: %clang_cc1 -verify -fsyntax-only %s
-// RUN: %clang_cc1 -Wconversion -verify -fsyntax-only -cl-std=CL2.0 %s
+// RUN: %clang_cc1 -verify -fsyntax-only -cl-std=CL2.0 %s
void test(void) {
global int *glob;
@@ -43,15 +43,13 @@
// expected-warning@-2{{incompatible integer to pointer conversion assigning to '__local int *' from 'int'}}
#else
// expected-error@-4{{assigning '__global int *' to '__local int *' changes address space of pointer}}
- // expected-warning@-5{{passing non-generic address space pointer to to_global may cause dynamic conversion affecting performance}}
#endif
global char *glob_c = to_global(loc);
#if __OPENCL_C_VERSION__ < CL_VERSION_2_0
// expected-warning@-2{{incompatible integer to pointer conversion initializing '__global char *' with an expression of type 'int'}}
#else
// expected-warning@-4{{incompatible pointer types initializing '__global char *' with an expression of type '__global int *'}}
- // expected-warning@-5{{passing non-generic address space pointer to to_global may cause dynamic conversion affecting performance}}
#endif
}
Index: test/Driver/Xarch.c
===
--- test/Driver/Xarch.c
+++ test/Driver/Xarch.c
@@ -1,12 +1,9 @@
-// RUN: %clang -target i386-apple-darwin9 -m32 -Xarch_i386 -O2 %s -S -### 2>&1 | FileCheck -check-prefix=O2ONCE %s
-// O2ONCE: "-O2"
-// O2ONCE-NOT: "-O2"
+// RUN: %clang -target i386-apple-darwin9 -m32 -Xarch_i386 -O2 %s -S -### 2> %t.log
+// RUN: grep ' "-O2" ' %t.log | count 1
+// RUN: %clang -target i386-apple-darwin9 -m64 -Xarch_i386 -O2 %s -S -### 2> %t.log
+// RUN: not grep ' "-O2" ' %t.log
+// RUN: grep "argument unused during compilation: '-Xarch_i386 -O2'" %t.log
+// RUN: not %clang -target i386-apple-darwin9 -m32 -Xarch_i386 -o -Xarch_i386 -S %s -S -Xarch_i386 -o 2> %t.log
+// RUN: grep "error: invalid Xarch argument: '-Xarch_i386 -o'" %t.log | count 2
+// RUN: grep "error: invalid Xarch argument: '-Xarch_i386 -S'" %t.log
-// RUN: %clang -target i386-apple-darwin9 -m64 -Xarch_i386 -O2 %s -S -### 2>&1 | FileCheck -check-prefix=O2NONE %s
-// O2NONE-NOT: "-O2"
-// O2NONE: argument unused during compilation: '-Xarch_i386 -O2'
-
-// RUN: not %clang -target i386-apple-darwin9 -m32 -Xarch_i386 -o -Xarch_i386 -S %s -S -Xarch_i386 -o 2>&1 | FileCheck -check-prefix=INVALID %s
-// INVALID: error: invalid Xarch argument: '-Xarch_i386 -o'
-// INVALID: error: invalid Xarch argument: '-Xarch_i386 -S'
-// INVALID: error: invalid Xarch argument: '-Xarch_i386 -o'
Index: test/Analysis/scopes-cfg-output.cpp
===
--- test/Analysis/scopes-cfg-output.cpp
+++ test/Analysis/scopes-cfg-output.cpp
@@ -820,7 +820,7 @@
// CHECK-NEXT: 3: __end1
// CHECK-NEXT: 4: [B2.3] (ImplicitCastExpr, LValueToRValue, class A *)
// CHECK-NEXT: 5: [B2.2] != [B2.4]
-// CHECK-NEXT: T: for (auto &i : [B5.4]) {
+// CHECK-NEXT: T: for (A &i : [B5.4]) {
// CHECK: [B4.11];
// CHECK-NEXT:}
// CHECK-NEXT: Preds (2): B3 B5
@@ -835,7 +835,7 @@
// CHECK-NEXT: 2: __begin1
// CHECK-NEXT: 3: [B4.2] (ImplicitCastExpr, LValueToRValue, class A *)
// CHECK-NEXT: 4: *[B4.3]
-// CHECK-NEXT: 5: auto &i = *__begin1;
+// CHECK-NEXT: 5: A &i = *__begin1;
// CHECK-NEXT: 6: operator=
// CHECK-NEXT: 7: [B4.6] (ImplicitCastExpr, FunctionToPointerDecay, class A &(*)(const class A &)
// CHECK-NEXT: 8: i
@@ -850,16 +850,16 @@
// CHECK-NEXT: 2: (CXXConstructExpr, [B5.3], class A [10])
// CHECK-NEXT: 3: A a[10];
// CHECK-NEXT: 4: a
-// CHECK-NEXT: 5: auto &&__range1 = a;
+// CHECK-NEXT: 5: A (&__range1)[10] = a;
// CHECK-NEXT: 6: CFGScopeBegin(__end1)
// CHECK-NEXT: 7: __range1
// CHECK-NEXT: 8: [B5.7] (ImplicitCastExpr, ArrayToPointerDecay, class A *)
// CHECK-NEXT: 9: 10
// CHECK-NEXT: 10: [B5.8] + [B5.9]
-// CHECK-NEXT: 11: auto __end1 = __range1 + 10
+// CHECK-NEXT: 11: A *__end1 = __range1 + 10
// CHECK-NEXT: 12: __range1
// CHECK-NEXT: 13: [B5.12] (ImplicitCastExpr, ArrayToPointerDecay, class A *)
-
[PATCH] D52047: [clangd] Add building benchmark and memory consumption tracking
ilya-biryukov added a comment.
Also not sure about the trick:
- Would be surprising to see the "ms" instead of "mbytes"
- Time tends to vary between runs, so using Google Benchmark's capabilities to
run multiple times and report aggregated times seems useful. Not so much for
the memory usage: it's deterministic and running multiple times is just a waste
of time.
I wonder if a separate (and trivial) C++ binary that would load the index and
report the index size is any worse than the benchmark hack? What are the pros
of the latter?
Comment at: clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp:81
+// Same for the next "benchmark".
+// FIXME(kbobyrev): Should this be separated into the BackingMemorySize
+// (underlying SymbolSlab size) and Symbol Index (MemIndex/Dex) overhead?
Given the trick we use for display, how are we going to show **two** memory
uses?
Comment at: clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp:154
::benchmark::RunSpecifiedBenchmarks();
+ return 0;
}
Since `return 0` is implied for main in C++, there's no need to add one.
May add clarity though, so feel free to keep it!
Comment at: clang-tools-extra/clangd/index/SymbolYAML.cpp:189
+ if (!Buffer) {
+llvm::errs() << "Can't open " << SymbolFilename << "\n";
+return llvm::None;
Return `llvm::Expected` instead of `Optional` and create errors with the
specified text instead.
This is a slightly better option for the library code (callers can report
errors in various ways, e.g. one can imagine reporting it in the LSP instead of
stderr)
Comment at: clang-tools-extra/clangd/index/SymbolYAML.cpp:200
+else
+ llvm::errs() << "Bad RIFF: " << llvm::toString(RIFF.takeError()) << "\n";
+ } else {
Same here: just propagate the error to the caller.
Comment at: clang-tools-extra/clangd/index/dex/Dex.cpp:239
for (const auto &P : InvertedIndex)
-Bytes += P.second.bytes();
+Bytes += P.first.Data.size() + P.second.bytes();
return Bytes + BackingDataSize;
Just to clarify: `P.first.Data.size()` is the size of the arena for all the
symbols?
https://reviews.llvm.org/D52047
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52261: [Sema] Generate completion fix-its for C code as well
ilya-biryukov added a comment. In https://reviews.llvm.org/D52261#1240143, @yvvan wrote: > I tried that first but did not I find a way just to copy an expression (we > basically need the same expr for such case). Do you know how to properly > generate a copy of expression or some other way to get the same expression? It seems `Sema::ActOnStartCXXMemberReference` only changes expression when overloading for C++'s `operator ->` is required, otherwise it keeps the same expression. Since C does not have that, we can just leave the expression as is. So setting `CorrectedLHS = LHS` for C should do the trick (no need to copy the expression IIUC, it's fine to use the same pointer for both `CorrectedLHS` and `LHS`). https://reviews.llvm.org/D52261 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r342648 - [OPENMP] Add support for mapping memory pointed by member pointer.
Author: abataev
Date: Thu Sep 20 06:54:02 2018
New Revision: 342648
URL: http://llvm.org/viewvc/llvm-project?rev=342648&view=rev
Log:
[OPENMP] Add support for mapping memory pointed by member pointer.
Added support for map(s, s.ptr[0:1]) kind of mapping.
Modified:
cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
cfe/trunk/lib/Sema/SemaOpenMP.cpp
cfe/trunk/test/OpenMP/target_map_codegen.cpp
cfe/trunk/test/OpenMP/target_map_messages.cpp
Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp?rev=342648&r1=342647&r2=342648&view=diff
==
--- cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp Thu Sep 20 06:54:02 2018
@@ -6752,7 +6752,9 @@ private:
MapBaseValuesArrayTy &BasePointers, MapValuesArrayTy &Pointers,
MapValuesArrayTy &Sizes, MapFlagsArrayTy &Types,
StructRangeInfoTy &PartialStruct, bool IsFirstComponentList,
- bool IsImplicit) const {
+ bool IsImplicit,
+ ArrayRef
+ OverlappedElements = llvm::None) const {
// The following summarizes what has to be generated for each map and the
// types below. The generated information is expressed in this order:
// base pointer, section pointer, size, flags
@@ -7023,7 +7025,6 @@ private:
Address LB =
CGF.EmitOMPSharedLValue(I->getAssociatedExpression()).getAddress();
-llvm::Value *Size = getExprTypeSize(I->getAssociatedExpression());
// If this component is a pointer inside the base struct then we don't
// need to create any entry for it - it will be combined with the
object
@@ -7032,6 +7033,70 @@ private:
IsPointer && EncounteredME &&
(dyn_cast(I->getAssociatedExpression()) ==
EncounteredME);
+if (!OverlappedElements.empty()) {
+ // Handle base element with the info for overlapped elements.
+ assert(!PartialStruct.Base.isValid() && "The base element is set.");
+ assert(Next == CE &&
+ "Expected last element for the overlapped elements.");
+ assert(!IsPointer &&
+ "Unexpected base element with the pointer type.");
+ // Mark the whole struct as the struct that requires allocation on
the
+ // device.
+ PartialStruct.LowestElem = {0, LB};
+ CharUnits TypeSize = CGF.getContext().getTypeSizeInChars(
+ I->getAssociatedExpression()->getType());
+ Address HB = CGF.Builder.CreateConstGEP(
+ CGF.Builder.CreatePointerBitCastOrAddrSpaceCast(LB,
+ CGF.VoidPtrTy),
+ TypeSize.getQuantity() - 1, CharUnits::One());
+ PartialStruct.HighestElem = {
+ std::numeric_limits::max(),
+ HB};
+ PartialStruct.Base = BP;
+ // Emit data for non-overlapped data.
+ OpenMPOffloadMappingFlags Flags =
+ OMP_MAP_MEMBER_OF |
+ getMapTypeBits(MapType, MapTypeModifier, IsImplicit,
+ /*AddPtrFlag=*/false,
+ /*AddIsTargetParamFlag=*/false);
+ LB = BP;
+ llvm::Value *Size = nullptr;
+ // Do bitcopy of all non-overlapped structure elements.
+ for (OMPClauseMappableExprCommon::MappableExprComponentListRef
+ Component : OverlappedElements) {
+Address ComponentLB = Address::invalid();
+for (const OMPClauseMappableExprCommon::MappableComponent &MC :
+ Component) {
+ if (MC.getAssociatedDeclaration()) {
+ComponentLB =
+CGF.EmitOMPSharedLValue(MC.getAssociatedExpression())
+.getAddress();
+Size = CGF.Builder.CreatePtrDiff(
+CGF.EmitCastToVoidPtr(ComponentLB.getPointer()),
+CGF.EmitCastToVoidPtr(LB.getPointer()));
+break;
+ }
+}
+BasePointers.push_back(BP.getPointer());
+Pointers.push_back(LB.getPointer());
+Sizes.push_back(Size);
+Types.push_back(Flags);
+LB = CGF.Builder.CreateConstGEP(ComponentLB, 1,
+CGF.getPointerSize());
+ }
+ BasePointers.push_back(BP.getPointer());
+ Pointers.push_back(LB.getPointer());
+ Size = CGF.Builder.CreatePtrDiff(
+ CGF.EmitCastToVoidPtr(
+ CGF.Builder.CreateConstGEP(HB, 1, CharUnits::One())
+ .getPointer()),
+ CGF.EmitCastToVoidPtr(LB.getPointer()));
+ Sizes.push_back(Size);
+ Types.push_back(Flags);
+ break;
+}
+llvm::Value *Size = getExprTypeSize(I->getAssoc
[PATCH] D52219: [analyzer] (1/n) Support pointee mutation analysis in ExprMutationAnalyzer.
JonasToth added inline comments.
Comment at: unittests/Analysis/ExprMutationAnalyzerTest.cpp:67
+if (const auto *DRE = dyn_cast(E)) {
+ if (DRE->getNameInfo().getAsString()[0] == 'p')
+Finder = PointeeMutationFinder;
shuaiwang wrote:
> JonasToth wrote:
> > That doesn't look like a super idea. It is super hidden that variable 'p*'
> > will be analyzed for pointee mutation and others aren't. Especially in 12
> > months and when someone else wants to change something, this will be the
> > source of headaches.
> >
> > Don't you think there could be a better way of doing this switch?
> Yeah... agree :)
> But how about let's keep it this way just for now, and I'll pause the work on
> supporting pointee analysis and fix it properly but in a separate diff
> following this one?
>
> I've been thinking about this and also did some prototyping, and here are my
> thoughts:
> In order to properly fix this we need to change the interface of
> `ExprMutationAnalyzer` to not simply return a `Stmt` but instead return a
> `MutationTrace` with additional metadata. This wasn't needed before because
> we can reconstruct a mutation chain by continuously calling `findMutation`,
> and that works well for cases like "int x; int& r0 = x; int& r1 = r0; r1 =
> 123;". But now that pointers come into play, and we need to handle cases like
> "int *p; int *p2 = p; int& r = *p2; r = 123;", and in order to reconstruct
> the mutation chain, we need to do `findPointeeMutation(p)` ->
> `findPointeeMutation(p2)` -> `findMutation(r)`, notice that we need to switch
> from calling `findPointeeMutation` to calling `findMutation`. However we
> don't know where the switch should happen simply based on what's returned
> from `findPointeeMutation`, we need additional metadata for that.
>
> That interface change will unavoidably affect how memoization is done so we
> need to change memoization as well.
>
> Now since we need to change both the interface and memoization anyway, we
> might as well design them properly so that multiple-layers of pointers can be
> supported nicely. I think we can end up with something like this:
> ```
> class ExprMutationAnalyzer {
> public:
> struct MutationTrace {
> const Stmt *Value;
> const MutationTrace *Next;
> // Other metadata
> };
>
> // Finds whether any mutative operations are performed on the given
> expression after dereferencing `DerefLayers` times.
> const MutationTrace *findMutation(const Expr *, int DerefLayers = 0);
> const MutationTrace *findMutation(const Decl *, int DerefLayers = 0);
> };
> ```
> This involves quite some refactoring, and doing this refactoring before this
> diff and later rebasing seems quite some trouble that I'd like to avoid.
>
> Let me know what do you think :)
Is the second part of your answer part to adressing this testing issue?
Given the whole Analyzer is WIP it is ok to postpone the testing change to
later for me. But I feel that this is a quality issue that more experience
clang develops should comment on because it still lands in trunk. Are you aware
of other patches then clang-tidy that rely on EMA?
Regarding you second part (its related to multi-layer pointer mutation?!):
Having a dependency-graph that follows the general data flow _with_ mutation
annotations would be nice to have. There is probably even something in clang
(e.g. `CFG` is probably similar in idea, just with all execution paths), but I
don't know enough to properly judge here.
In my opinion it is ok, to not do the refactoring now and just support one
layer of pointer indirection. Especially in C++ there is usually no need for
multi-layer pointers but more a relict from C-style.
C on the other hand relies on that.
Designing EMA new for the more generalized functionality (if necessary) would
be of course great, but again: Better analyzer ppl should be involved then me,
even though I would still be very interested to help :)
Comment at: unittests/Analysis/ExprMutationAnalyzerTest.cpp:156
EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("x.mf()"));
+
+ AST = tooling::buildASTFromCode(
JonasToth wrote:
> I feel that there a multiple tests missing:
>
> - multiple levels of pointers `int ***`, `int * const *`
> - pointers to references `int &*`
> - references to pointers `int *&`
> - ensure that having a const pointer does no influence the pointee analysis
> `int * const p = &i; *p = 42;`
> - a class with `operator*` + `operator->` const/non-const and the analysis
> for pointers to that class
> - pointer returned from a function
> - non-const reference returned
> ```
> int& foo(int *p) {
> return *p;
> }
> ```
>
for the multi-level pointer mutation: it would be enough to test, that the
second layer is analyzed properly, and that the `int * >const< *` would be
detected.
Repository:
rC Clang
https://reviews.llvm.org/D52219
__
r342614 - [PowerPC] [Clang] Add vector int128 pack/unpack builtins
Author: qshanz
Date: Wed Sep 19 22:04:57 2018
New Revision: 342614
URL: http://llvm.org/viewvc/llvm-project?rev=342614&view=rev
Log:
[PowerPC] [Clang] Add vector int128 pack/unpack builtins
unsigned long long builtin_unpack_vector_int128 (vector int128_t, int);
vector int128_t builtin_pack_vector_int128 (unsigned long long, unsigned long
long);
Builtins should behave the same way as in GCC.
Patch By: wuzish (Zixuan Wu)
Differential Revision: https://reviews.llvm.org/D52074
Modified:
cfe/trunk/include/clang/Basic/BuiltinsPPC.def
cfe/trunk/lib/CodeGen/CGBuiltin.cpp
cfe/trunk/lib/Sema/SemaChecking.cpp
cfe/trunk/test/CodeGen/builtins-ppc-error.c
cfe/trunk/test/CodeGen/builtins-ppc-p7-disabled.c
cfe/trunk/test/CodeGen/builtins-ppc-vsx.c
Modified: cfe/trunk/include/clang/Basic/BuiltinsPPC.def
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/BuiltinsPPC.def?rev=342614&r1=342613&r2=342614&view=diff
==
--- cfe/trunk/include/clang/Basic/BuiltinsPPC.def (original)
+++ cfe/trunk/include/clang/Basic/BuiltinsPPC.def Wed Sep 19 22:04:57 2018
@@ -470,6 +470,10 @@ BUILTIN(__builtin_divde, "SLLiSLLiSLLi",
BUILTIN(__builtin_divdeu, "ULLiULLiULLi", "")
BUILTIN(__builtin_bpermd, "SLLiSLLiSLLi", "")
+// Vector int128 (un)pack
+BUILTIN(__builtin_unpack_vector_int128, "ULLiV1LLLii", "")
+BUILTIN(__builtin_pack_vector_int128, "V1LLLiULLiULLi", "")
+
// FIXME: Obviously incomplete.
#undef BUILTIN
Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBuiltin.cpp?rev=342614&r1=342613&r2=342614&view=diff
==
--- cfe/trunk/lib/CodeGen/CGBuiltin.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp Wed Sep 19 22:04:57 2018
@@ -11233,6 +11233,28 @@ Value *CodeGenFunction::EmitPPCBuiltinEx
auto RetTy = ConvertType(BIRetType);
return Builder.CreateBitCast(ShuffleCall, RetTy);
}
+
+ case PPC::BI__builtin_pack_vector_int128: {
+bool isLittleEndian = getTarget().isLittleEndian();
+Value *UndefValue =
+llvm::UndefValue::get(llvm::VectorType::get(Ops[0]->getType(), 2));
+Value *Res = Builder.CreateInsertElement(
+UndefValue, Ops[0], (uint64_t)(isLittleEndian ? 1 : 0));
+Res = Builder.CreateInsertElement(Res, Ops[1],
+ (uint64_t)(isLittleEndian ? 0 : 1));
+return Builder.CreateBitCast(Res, ConvertType(E->getType()));
+ }
+
+ case PPC::BI__builtin_unpack_vector_int128: {
+ConstantInt *Index = cast(Ops[1]);
+Value *Unpacked = Builder.CreateBitCast(
+Ops[0], llvm::VectorType::get(ConvertType(E->getType()), 2));
+
+if (getTarget().isLittleEndian())
+ Index = ConstantInt::get(Index->getType(), 1 - Index->getZExtValue());
+
+return Builder.CreateExtractElement(Unpacked, Index);
+ }
}
}
Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=342614&r1=342613&r2=342614&view=diff
==
--- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
+++ cfe/trunk/lib/Sema/SemaChecking.cpp Wed Sep 19 22:04:57 2018
@@ -2970,6 +2970,13 @@ bool Sema::CheckPPCBuiltinFunctionCall(u
return Diag(TheCall->getBeginLoc(), diag::err_ppc_builtin_only_on_pwr7)
<< TheCall->getSourceRange();
+ auto SemaVSXCheck = [&](CallExpr *TheCall) -> bool {
+if (!Context.getTargetInfo().hasFeature("vsx"))
+ return Diag(TheCall->getBeginLoc(), diag::err_ppc_builtin_only_on_pwr7)
+ << TheCall->getSourceRange();
+return false;
+ };
+
switch (BuiltinID) {
default: return false;
case PPC::BI__builtin_altivec_crypto_vshasigmaw:
@@ -2988,6 +2995,11 @@ bool Sema::CheckPPCBuiltinFunctionCall(u
case PPC::BI__builtin_vsx_xxpermdi:
case PPC::BI__builtin_vsx_xxsldwi:
return SemaBuiltinVSX(TheCall);
+ case PPC::BI__builtin_unpack_vector_int128:
+return SemaVSXCheck(TheCall) ||
+ SemaBuiltinConstantArgRange(TheCall, 1, 0, 1);
+ case PPC::BI__builtin_pack_vector_int128:
+return SemaVSXCheck(TheCall);
}
return SemaBuiltinConstantArgRange(TheCall, i, l, u);
}
Modified: cfe/trunk/test/CodeGen/builtins-ppc-error.c
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/builtins-ppc-error.c?rev=342614&r1=342613&r2=342614&view=diff
==
--- cfe/trunk/test/CodeGen/builtins-ppc-error.c (original)
+++ cfe/trunk/test/CodeGen/builtins-ppc-error.c Wed Sep 19 22:04:57 2018
@@ -14,6 +14,7 @@ extern vector signed int vsi;
extern vector signed int vui;
extern vector float vf;
extern vector unsigned char vuc;
+extern vector signed __int128 vsllli;
void testInsertWord(void) {
int index = 5;
@@ -67,3 +68,8 @@ void
[PATCH] D52308: [Sema][CodeCompletion] Fix return type of C++ destructors in code completion
jkorous created this revision.
jkorous added reviewers: arphaman, vsapsai, sammccall, ilya-biryukov.
jkorous added a project: clang.
Herald added subscribers: cfe-commits, dexonsmith, eraman.
Destructors don't have return type "void", they don't have any return type at
all.
Repository:
rC Clang
https://reviews.llvm.org/D52308
Files:
Index/code-completion.cpp
Index/complete-access-checks.cpp
Index/complete-arrow-dot.cpp
Index/complete-cxx-inline-methods.cpp
Index/complete-qualified.cpp
Index/complete-with-annotations.cpp
Sema/SemaCodeComplete.cpp
Index: Index/complete-with-annotations.cpp
===
--- Index/complete-with-annotations.cpp
+++ Index/complete-with-annotations.cpp
@@ -19,5 +19,5 @@
// CHECK: FieldDecl:{ResultType int}{TypedText member2} (35) ("another annotation", "some annotation")
// CHECK: CXXMethod:{ResultType X &}{TypedText operator=}{LeftParen (}{Placeholder const X &}{RightParen )} (79)
// CHECK: ClassDecl:{TypedText X}{Text ::} (75)
-// CHECK: CXXDestructor:{ResultType void}{TypedText ~X}{LeftParen (}{RightParen )} (79)
+// CHECK: CXXDestructor:{TypedText ~X}{LeftParen (}{RightParen )} (79)
Index: Index/complete-qualified.cpp
===
--- Index/complete-qualified.cpp
+++ Index/complete-qualified.cpp
@@ -17,4 +17,4 @@
// CHECK-CC1: FieldDecl:{ResultType C}{TypedText c} (35)
// CHECK-CC1: ClassDecl:{TypedText Foo} (35)
// CHECK-CC1: CXXMethod:{ResultType Foo &}{TypedText operator=}{LeftParen (}{Placeholder const Foo &}{RightParen )}
-// CHECK-CC1: CXXDestructor:{ResultType void}{TypedText ~Foo}{LeftParen (}{RightParen )} (80)
+// CHECK-CC1: CXXDestructor:{TypedText ~Foo}{LeftParen (}{RightParen )} (80)
Index: Index/complete-cxx-inline-methods.cpp
===
--- Index/complete-cxx-inline-methods.cpp
+++ Index/complete-cxx-inline-methods.cpp
@@ -29,7 +29,7 @@
// CHECK-NEXT: StructDecl:{TypedText Vec}{Text ::} (75)
// CHECK-NEXT: FieldDecl:{ResultType int}{TypedText x} (35)
// CHECK-NEXT: FieldDecl:{ResultType int}{TypedText y} (35)
-// CHECK-NEXT: CXXDestructor:{ResultType void}{TypedText ~Vec}{LeftParen (}{RightParen )} (79)
+// CHECK-NEXT: CXXDestructor:{TypedText ~Vec}{LeftParen (}{RightParen )} (79)
// CHECK-NEXT: Completion contexts:
// CHECK-NEXT: Dot member access
// CHECK-NEXT: Container Kind: StructDecl
Index: Index/complete-arrow-dot.cpp
===
--- Index/complete-arrow-dot.cpp
+++ Index/complete-arrow-dot.cpp
@@ -22,33 +22,33 @@
// CHECK-NOT: CXXMethod:{ResultType X &}{TypedText operator=}{LeftParen (}{Placeholder const X &}{RightParen )} (79) (requires fix-it:{{.*}}
// CHECK-NOT: CXXMethod:{ResultType X &}{TypedText operator=}{LeftParen (}{Placeholder X &&}{RightParen )} (79) (requires fix-it:{{.*}}
// CHECK-NOT: StructDecl:{TypedText X}{Text ::} (75) (requires fix-it:{{.*}}
-// CHECK-NOT: CXXDestructor:{ResultType void}{TypedText ~X}{LeftParen (}{RightParen )} (79) (requires fix-it:{{.*}}
+// CHECK-NOT: CXXDestructor:{TypedText ~X}{LeftParen (}{RightParen )} (79) (requires fix-it:{{.*}}
// CHECK: Completion contexts:
// CHECK-NEXT: Dot member access
// CHECK-WITH-CORRECTION: CXXMethod:{ResultType void}{TypedText doSomething}{LeftParen (}{RightParen )} (34) (requires fix-it:{{.*}}
// CHECK-WITH-CORRECTION-NEXT: FieldDecl:{ResultType int}{TypedText field} (35) (requires fix-it:{{.*}}
// CHECK-WITH-CORRECTION-NEXT: CXXMethod:{ResultType X &}{TypedText operator=}{LeftParen (}{Placeholder const X &}{RightParen )} (79) (requires fix-it:{{.*}}
// CHECK-WITH-CORRECTION-NEXT: CXXMethod:{ResultType X &}{TypedText operator=}{LeftParen (}{Placeholder X &&}{RightParen )} (79) (requires fix-it:{{.*}}
// CHECK-WITH-CORRECTION-NEXT: StructDecl:{TypedText X}{Text ::} (75) (requires fix-it:{{.*}}
-// CHECK-WITH-CORRECTION-NEXT: CXXDestructor:{ResultType void}{TypedText ~X}{LeftParen (}{RightParen )} (79) (requires fix-it:{{.*}}
+// CHECK-WITH-CORRECTION-NEXT: CXXDestructor:{TypedText ~X}{LeftParen (}{RightParen )} (79) (requires fix-it:{{.*}}
// CHECK-WITH-CORRECTION-NEXT: Completion contexts:
// CHECK-WITH-CORRECTION-NEXT: Dot member access
// CHECK-ARROW-TO-DOT-NOT: CXXMethod:{ResultType void}{TypedText doSomething}{LeftParen (}{RightParen )} (34) (requires fix-it:{{.*}}
// CHECK-ARROW-TO-DOT-NOT: FieldDecl:{ResultType int}{TypedText field} (35) (requires fix-it:{{.*}}
// CHECK-ARROW-TO-DOT-NOT: CXXMethod:{ResultType X &}{TypedText operator=}{LeftParen (}{Placeholder const X &}{RightParen )} (79) (requires fix-it:{{.*}}
// CHECK-ARROW-TO-DOT-NOT: CXXMethod:{ResultType X &}{TypedText operator=}{LeftParen (}{Placeholder X &&}{RightParen )} (79) (requires fix-it:{{.*}}
// CHECK-ARROW-TO-DOT-NOT: StructDecl:{TypedText X}{Text ::} (75) (requires fix-it:{{.*}}
-// CHECK-ARROW-TO-DOT-NOT: CXXDestructor:{ResultType
[PATCH] D52266: [clang-cl] Provide separate flags for all the /O variants
thakis added a comment. In https://reviews.llvm.org/D52266#1240304, @hans wrote: > Sorry, I didn't realize we both set off to do this in parallel. I've > incorporated your changes into my patch. No worries, I didn't do anything I wouldn't have done for reviewing this :-) Thoughts on "As far as I can tell we do not make /https://reviews.llvm.org/owners/package/1/ and /https://reviews.llvm.org/owners/package/2/ imply /Gs -- we keep it at the default value of 4096 (?) Fixing this probably means increasing chrome's size (?)."? Comment at: include/clang/Driver/CLCompatOptions.td:94 +def : CLFlag<"Gs">, HelpText<"Same as /Gs0">, Alias, AliasArgs<["0"]>; +def _SLASH_Gs : CLJoined<"Gs">, HelpText<"Set stack probe size">, Alias; def _SLASH_Gy : CLFlag<"Gy">, HelpText<"Put each function in its own section">, Want to add "(default 4096)" here? Comment at: include/clang/Driver/CLCompatOptions.td:115 +// The _SLASH_O option handles all the /O flags, but we also provide separate aliased options to provide separate help messages. +// FIXME: Not sure why we have -O0 here; MSVC doesn't support that. +def _SLASH_O : CLJoined<"O">, HelpText<"Set several /O flags at once; e.g. '/O2y-' is the same as '/O2 /y-'">, MetaVarName<"">; Move FIXME down one so it's next to the O0 flag? Comment at: include/clang/Driver/CLCompatOptions.td:118 +def : CLFlag<"O0">, Alias, HelpText<"Disable optimization">; +def : CLFlag<"O1">, Alias<_SLASH_O>, AliasArgs<["1"]>, HelpText<"Optimize for small size (equivalent to /Og /Os /Oy /Ob2 /Gs /GF /Gy)">; +def : CLFlag<"O2">, Alias<_SLASH_O>, AliasArgs<["2"]>, HelpText<"Optimize for maximum speed (equivalent to /Og /Oi /Ot /Oy /Ob2 /Gs /GF /Gy)">; Nit: Just "optimize for size" / "optimize for speed" is shorter Comment at: include/clang/Driver/CLCompatOptions.td:123 +def : CLFlag<"Ob2">, Alias<_SLASH_O>, AliasArgs<["b2"]>, HelpText<"Inline functions as deemed beneficial by the compiler">; +def : CLFlag<"Od">, Alias, HelpText<"Disable optimization">; +def : CLFlag<"Og">, Alias<_SLASH_O>, AliasArgs<["g"]>, HelpText<"No effect">; Why not alias this to _SLASH_O, d like the rest? Comment at: include/clang/Driver/CLCompatOptions.td:128 +def : CLFlag<"Os">, Alias<_SLASH_O>, AliasArgs<["s"]>, HelpText<"Optimize for small size over speed">; +def : CLFlag<"Ot">, Alias<_SLASH_O>, AliasArgs<["t"]>, HelpText<"Optimize for speed over small size">; +def : CLFlag<"Ox">, Alias<_SLASH_O>, AliasArgs<["x"]>, HelpText<"Deprecated; use /O2 instead (equivalent to /Og /Oi /Ot /Oy /Ob2)">; nit: I liked the old help text for the previous 4 more, it was more concise Comment at: include/clang/Driver/CLCompatOptions.td:129 +def : CLFlag<"Ot">, Alias<_SLASH_O>, AliasArgs<["t"]>, HelpText<"Optimize for speed over small size">; +def : CLFlag<"Ox">, Alias<_SLASH_O>, AliasArgs<["x"]>, HelpText<"Deprecated; use /O2 instead (equivalent to /Og /Oi /Ot /Oy /Ob2)">; +def : CLFlag<"Oy">, Alias<_SLASH_O>, AliasArgs<["y"]>, HelpText<"Enable frame pointer omission (x86 only)">; nit: With this punctuation it looks ambiguous to me if the parenthetical refers to /O2 or /Ox. https://reviews.llvm.org/D52266 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52308: [Sema][CodeCompletion] Fix return type of C++ destructors in code completion
sammccall added a comment. When you're *calling* a destructor, I believe the expression does have type void. Are we sure this is incorrect? Calling a destructor is really unusual though. IIRC we decided to just not show them in clangd in member context (maybe this is broken or was never implemented, though). Repository: rC Clang https://reviews.llvm.org/D52308 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52280: Don't trim non-alphanumeric characters in 'file not found' errors for include directives.
aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM! Repository: rC Clang https://reviews.llvm.org/D52280 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52308: [Sema][CodeCompletion] Fix return type of C++ destructors in code completion
jkorous added a comment. You might be right - I am assuming here that we want consistent behaviour between constructors and destructors. IIUC ctors are currently skipped in code completions (in most cases) but they also don't have any type associated while result of their call definitely has some type. Currently we are showing destructors in code completion in clangd (with detail "void") - that's actually how I noticed this. I would assume that canonical type in function/method code completion is "it's type" not "type of result of it's call". WDYT? Repository: rC Clang https://reviews.llvm.org/D52308 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51949: [WIP][clang-tidy] initial ideas to isolate variable declarations
kbobyrev added a comment.
Sorry, I didn't get time to review the patch properly, these are few stylistic
comments. Hopefully, I'll be able to give more feedback when I get more time.
Comment at: clang-tidy/readability/IsolateDeclCheck.cpp:21
+
+#define PRINT_DEBUG 1
+
Do you plan to submit the patch with debugging messages or are you just using
these for better local debugging experience?
If you plan to upload the patch with the messages, please use `LLVM_DEBUG` (see
`readability/IdentifierNamingCheck.cpp` for reference) and `llvm::dbgs()` ([[
https://llvm.org/docs/CodingStandards.html#include-iostream-is-forbidden |
`` traits shouldn't be used ]] and should be replaced with their LLVM
counterparts: `llvm::outs()`, `llvm::errs()`, etc). Also, I believe the
messages should be more informative if you think debug info is really important
here.
Comment at: clang-tidy/readability/IsolateDeclCheck.cpp:63
+
+if (!CurrentToken.hasValue())
+ return SourceLocation();
nit: `if (!CurrentToken)`
Comment at: clang-tidy/readability/IsolateDeclCheck.cpp:323
+
+static std::string &TrimRight(std::string &str) {
+ auto it1 = std::find_if(str.rbegin(), str.rend(), [](char ch) {
`llvm::StringRef::rtrim()`?
Also, naming `str` should be at least `Str`, `it1` -> `It`, `ch` -> `Ch` (or
better `Symbol`) ...
Comment at: clang-tidy/readability/IsolateDeclCheck.cpp:331
+
+static std::string Concat(const std::vector &Decls,
+ StringRef Indent) {
Use `llvm::join` with `";\n" + Indent` as the `Separator`?
Comment at: clang-tidy/readability/IsolateDeclCheck.cpp:333
+ StringRef Indent) {
+ std::string R;
+ for (const StringRef &D : Decls)
`Range`?
Comment at: clang-tidy/readability/IsolateDeclCheck.cpp:343
+ auto Diag =
+ diag(WholeDecl->getBeginLoc(), "this statement declares %0 variables")
+ << static_cast(
How about `multiple declarations within a single statement hurts readability`?
Comment at: clang-tidy/readability/IsolateDeclCheck.cpp:51
+
+return TypeAndName + " = " + Initializer + ";";
+ }
JonasToth wrote:
> kbobyrev wrote:
> > JonasToth wrote:
> > > kbobyrev wrote:
> > > > This seems to replace `int x = 5, y = 42;` with `int x = 5;int y = 42`.
> > > > I don't think that it becomes cleaner (in fact, without spaces in
> > > > between it looks cryptic). Consider formatting it or simply applying
> > > > newlines (if there were no newlines inbetween before).
> > > I do not plan to do a lot of formatting here (maybe space or newline),
> > > because that clang-format area.
> > While Clang-Tidy can apply Clang-Format on top of the Fix-Its, it will
> > still look weird in the Fix-Its previews. While supporting proper
> > formatting, in general, might be hard, it totally makes sense to do some
> > basic formatting so that editor integration warnings would look better, for
> > example.
> The current version adds a new line for each decl and keeps the indendation
> (as the other check does).
>
> Because it does the slicing on commas the manual/custom formatting of the
> original code will stay. That might result in weird looking output for exotic
> variable declarations. I would like to ignore these cases, what do you think
> @kbobyrev ?
Yes, sure, it's hard (and probably impossible) to support the generic case.
This approach sounds good to me, thanks!
Comment at: clang-tidy/readability/IsolateDeclCheck.h:1
+//===--- IsolateDeclCheck.h - clang-tidy-*- C++
-*-===//
+//
JonasToth wrote:
> kbobyrev wrote:
> > nit: space between clang-tidy (also, in .cpp file)
> That comes from the template `add_new_check.py`, do you want me to fix it
> there?
Ah, okay, I was thinking whether it was a template issue (since I thought it's
unlikely that one would edit the header template). Yes, it looks like a typo in
the `clang-tidy` check adding tool implementation. It seems to be fixed in
rL342601.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D51949
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51809: [CUDA][HIP] Fix assertion in LookupSpecialMember
yaxunl added a comment. ping https://reviews.llvm.org/D51809 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52308: [Sema][CodeCompletion] Fix return type of C++ destructors in code completion
jkorous added a comment.
Sorry my bad. You are right, we aren't showing destructors in clangd for normal
classes. The case where I noticed is kind of a corner case with template class.
{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
---
{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///main.cpp","languageId":"cpp","version":1,"text":"template struct foo {}; template<> struct foo {}; foo::~"}}}
---
{"jsonrpc":"2.0","id":1,"method":"textDocument/completion","params":{"textDocument":{"uri":"test:///main.cpp"},"position":{"line":0,"character":76}}}
{
"detail": "void",
"filterText": "~foo",
"insertText": "~foo",
"insertTextFormat": 1,
"kind": 2,
"label": " ~foo()",
"sortText": "3f2c~foo",
"textEdit": {
"newText": "~foo",
"range": {
"end": {
"character": 76,
"line": 0
},
"start": {
"character": 76,
"line": 0
}
}
}
},
Repository:
rC Clang
https://reviews.llvm.org/D52308
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52311: [clangd] Add support for hierarchical documentSymbol
ilya-biryukov created this revision.
ilya-biryukov added reviewers: ioeric, sammccall, simark.
Herald added subscribers: kadircet, arphaman, jkorous, MaskRay.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52311
Files:
clangd/ClangdLSPServer.cpp
clangd/ClangdServer.cpp
clangd/ClangdServer.h
clangd/FindSymbols.cpp
clangd/FindSymbols.h
clangd/Protocol.cpp
clangd/Protocol.h
clangd/clients/clangd-vscode/package.json
unittests/clangd/SyncAPI.cpp
unittests/clangd/SyncAPI.h
Index: unittests/clangd/SyncAPI.h
===
--- unittests/clangd/SyncAPI.h
+++ unittests/clangd/SyncAPI.h
@@ -47,7 +47,7 @@
llvm::Expected>
runWorkspaceSymbols(ClangdServer &Server, StringRef Query, int Limit);
-llvm::Expected>
+llvm::Expected>
runDocumentSymbols(ClangdServer &Server, PathRef File);
} // namespace clangd
Index: unittests/clangd/SyncAPI.cpp
===
--- unittests/clangd/SyncAPI.cpp
+++ unittests/clangd/SyncAPI.cpp
@@ -118,7 +118,7 @@
return std::move(*Result);
}
-llvm::Expected>
+llvm::Expected>
runDocumentSymbols(ClangdServer &Server, PathRef File) {
llvm::Optional>> Result;
Server.documentSymbols(File, capture(Result));
Index: clangd/clients/clangd-vscode/package.json
===
--- clangd/clients/clangd-vscode/package.json
+++ clangd/clients/clangd-vscode/package.json
@@ -6,7 +6,7 @@
"publisher": "llvm-vs-code-extensions",
"homepage": "https://clang.llvm.org/extra/clangd.html";,
"engines": {
-"vscode": "^1.18.0"
+"vscode": "^1.27.0"
},
"categories": [
"Programming Languages",
@@ -32,8 +32,8 @@
"test": "node ./node_modules/vscode/bin/test"
},
"dependencies": {
-"vscode-languageclient": "^4.0.0",
-"vscode-languageserver": "^4.0.0"
+"vscode-languageclient": "^5.1.0",
+"vscode-languageserver": "^5.1.0"
},
"devDependencies": {
"typescript": "^2.0.3",
Index: clangd/Protocol.h
===
--- clangd/Protocol.h
+++ clangd/Protocol.h
@@ -40,6 +40,7 @@
InvalidRequest = -32600,
MethodNotFound = -32601,
InvalidParams = -32602,
+
InternalError = -32603,
ServerNotInitialized = -32002,
@@ -623,6 +624,38 @@
llvm::json::Value toJSON(const Command &C);
+/// Represents programming constructs like variables, classes, interfaces etc.
+/// that appear in a document. Document symbols can be hierarchical and they
+/// have two ranges: one that encloses its definition and one that points to its
+/// most interesting range, e.g. the range of an identifier.
+struct DocumentSymbol {
+ /// The name of this symbol.
+ std::string name;
+
+ /// More detail for this symbol, e.g the signature of a function.
+ std::string detail;
+
+ /// The kind of this symbol.
+ SymbolKind kind;
+
+ /// Indicates if this symbol is deprecated.
+ bool deprecated;
+
+ /// The range enclosing this symbol not including leading/trailing whitespace
+ /// but everything else like comments. This information is typically used to
+ /// determine if the clients cursor is inside the symbol to reveal in the
+ /// symbol in the UI.
+ Range range;
+
+ /// The range that should be selected and revealed when this symbol is being
+ /// picked, e.g the name of a function. Must be contained by the `range`.
+ Range selectionRange;
+
+ /// Children of this symbol, e.g. properties of a class.
+ std::vector children;
+};
+llvm::json::Value toJSON(const DocumentSymbol &S);
+
/// Represents information about programming constructs like variables, classes,
/// interfaces etc.
struct SymbolInformation {
Index: clangd/Protocol.cpp
===
--- clangd/Protocol.cpp
+++ clangd/Protocol.cpp
@@ -18,6 +18,7 @@
#include "llvm/ADT/SmallString.h"
#include "llvm/Support/Format.h"
#include "llvm/Support/FormatVariadic.h"
+#include "llvm/Support/JSON.h"
#include "llvm/Support/Path.h"
#include "llvm/Support/raw_ostream.h"
@@ -448,6 +449,16 @@
return std::move(Cmd);
}
+llvm::json::Value toJSON(const DocumentSymbol &S) {
+ return json::Object{{"name", S.name},
+ {"detail", S.detail},
+ {"kind", static_cast(S.kind)},
+ {"deprecated", S.deprecated},
+ {"children", json::Array{S.children}},
+ {"range", S.range},
+ {"selectionRange", S.selectionRange}};
+}
+
json::Value toJSON(const WorkspaceEdit &WE) {
if (!WE.changes)
return json::Object{};
Index: clangd/FindSymbols.h
===
--- clangd/FindSymbols.h
+++ clangd/FindSymbols.h
@@ -36,8 +36,7 @@
/// Retrieves the symbols contained in the "main file" section of a
[PATCH] D52308: [Sema][CodeCompletion] Fix return type of C++ destructors in code completion
sammccall added a comment.
In https://reviews.llvm.org/D52308#1240642, @jkorous wrote:
> You might be right - I am assuming here that we want consistent behaviour
> between constructors and destructors.
>
> IIUC ctors are currently skipped in code completions (in most cases) but they
> also don't have any type associated while result of their call definitely has
> some type.
Tricky.
**MyClass()** has a type, and should probably have return type MyClass, though
it's pretty clear from the name (one could make the same argument about
destructors).
Note that the "return type matches" ranking heuristics will trigger on these
types so it's not just presentational, I think we should be including the types
for constructors.
But class Derived : MyClass { Derived : **MyClass()** {} }; doesn't have a type
(it's not an expression).
And MyClass::**MyClass()** has a type, but it's probably more likely that the
user is going for a plain **MyClass::MyClass** (e.g. in a class-scoped using
decl) which doesn't have a useful type.
> Currently we are showing destructors in code completion in clangd (with
> detail "void") - that's actually how I noticed this.
>
> I would assume that canonical type in function/method code completion is
> "it's type" not "type of result of it's call". WDYT?
I don't think so, for better or worse it's result of it's call. Or rather: type
of the expression that we guess the user's going to form, or that we suggest.
Again, this seems to give the best code completion presentation, and also best
ranking results (when the expected type of the context is known)
Repository:
rC Clang
https://reviews.llvm.org/D52308
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52311: [clangd] Add support for hierarchical documentSymbol
ilya-biryukov planned changes to this revision. ilya-biryukov added a comment. Posted to make sure it's visible that I've started doing this. Still need to update the tests and check for the capability from the client (and fallback to SymbolInformation if client does not support the new implementation). Nevertheless, it works and looks really good in VSCode. Will update the thread when the code is ready for review. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52311 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52308: [Sema][CodeCompletion] Fix return type of C++ destructors in code completion
sammccall added a comment.
In https://reviews.llvm.org/D52308#1240680, @jkorous wrote:
> Sorry my bad. You are right, we aren't showing destructors in clangd for
> normal classes. The case where I noticed is kind of a corner case with
> template class.
>
>
> {"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
> ---
>
> {"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///main.cpp","languageId":"cpp","version":1,"text":"template T> struct foo {}; template<> struct foo {}; foo::~"}}}
> ---
>
> {"jsonrpc":"2.0","id":1,"method":"textDocument/completion","params":{"textDocument":{"uri":"test:///main.cpp"},"position":{"line":0,"character":76}}}
>
>
>
>
> {
> "detail": "void",
> "filterText": "~foo",
> "insertText": "~foo",
> "insertTextFormat": 1,
> "kind": 2,
> "label": " ~foo()",
> "sortText": "3f2c~foo",
> "textEdit": {
> "newText": "~foo",
> "range": {
> "end": {
> "character": 76,
> "line": 0
> },
> "start": {
> "character": 76,
> "line": 0
> }
> }
> }
> },
>
Indeed, this looks like a bug to me.
Repository:
rC Clang
https://reviews.llvm.org/D52308
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52219: [analyzer] (1/n) Support pointee mutation analysis in ExprMutationAnalyzer.
shuaiwang added inline comments.
Comment at: unittests/Analysis/ExprMutationAnalyzerTest.cpp:67
+if (const auto *DRE = dyn_cast(E)) {
+ if (DRE->getNameInfo().getAsString()[0] == 'p')
+Finder = PointeeMutationFinder;
JonasToth wrote:
> shuaiwang wrote:
> > JonasToth wrote:
> > > That doesn't look like a super idea. It is super hidden that variable
> > > 'p*' will be analyzed for pointee mutation and others aren't. Especially
> > > in 12 months and when someone else wants to change something, this will
> > > be the source of headaches.
> > >
> > > Don't you think there could be a better way of doing this switch?
> > Yeah... agree :)
> > But how about let's keep it this way just for now, and I'll pause the work
> > on supporting pointee analysis and fix it properly but in a separate diff
> > following this one?
> >
> > I've been thinking about this and also did some prototyping, and here are
> > my thoughts:
> > In order to properly fix this we need to change the interface of
> > `ExprMutationAnalyzer` to not simply return a `Stmt` but instead return a
> > `MutationTrace` with additional metadata. This wasn't needed before because
> > we can reconstruct a mutation chain by continuously calling `findMutation`,
> > and that works well for cases like "int x; int& r0 = x; int& r1 = r0; r1 =
> > 123;". But now that pointers come into play, and we need to handle cases
> > like "int *p; int *p2 = p; int& r = *p2; r = 123;", and in order to
> > reconstruct the mutation chain, we need to do `findPointeeMutation(p)` ->
> > `findPointeeMutation(p2)` -> `findMutation(r)`, notice that we need to
> > switch from calling `findPointeeMutation` to calling `findMutation`.
> > However we don't know where the switch should happen simply based on what's
> > returned from `findPointeeMutation`, we need additional metadata for that.
> >
> > That interface change will unavoidably affect how memoization is done so we
> > need to change memoization as well.
> >
> > Now since we need to change both the interface and memoization anyway, we
> > might as well design them properly so that multiple-layers of pointers can
> > be supported nicely. I think we can end up with something like this:
> > ```
> > class ExprMutationAnalyzer {
> > public:
> > struct MutationTrace {
> > const Stmt *Value;
> > const MutationTrace *Next;
> > // Other metadata
> > };
> >
> > // Finds whether any mutative operations are performed on the given
> > expression after dereferencing `DerefLayers` times.
> > const MutationTrace *findMutation(const Expr *, int DerefLayers = 0);
> > const MutationTrace *findMutation(const Decl *, int DerefLayers = 0);
> > };
> > ```
> > This involves quite some refactoring, and doing this refactoring before
> > this diff and later rebasing seems quite some trouble that I'd like to
> > avoid.
> >
> > Let me know what do you think :)
> Is the second part of your answer part to adressing this testing issue?
> Given the whole Analyzer is WIP it is ok to postpone the testing change to
> later for me. But I feel that this is a quality issue that more experience
> clang develops should comment on because it still lands in trunk. Are you
> aware of other patches then clang-tidy that rely on EMA?
>
> Regarding you second part (its related to multi-layer pointer mutation?!):
> Having a dependency-graph that follows the general data flow _with_ mutation
> annotations would be nice to have. There is probably even something in clang
> (e.g. `CFG` is probably similar in idea, just with all execution paths), but
> I don't know enough to properly judge here.
>
> In my opinion it is ok, to not do the refactoring now and just support one
> layer of pointer indirection. Especially in C++ there is usually no need for
> multi-layer pointers but more a relict from C-style.
> C on the other hand relies on that.
> Designing EMA new for the more generalized functionality (if necessary) would
> be of course great, but again: Better analyzer ppl should be involved then
> me, even though I would still be very interested to help :)
The second part is more of trying to explain why I'd prefer to keep the test
this way just for this diff.
Basically we need a refactoring that makes EMA returns MutationTrace in order
to fix this test util here.
But thinking more about this, maybe we don't need that (for now), this util is
to help restore a mutation chain, and since we don't support a chain of
mutation for pointers yet we can have a simpler version of `pointeeMutatedBy`
that doesn't support chaining.
I mentioned multi-layer pointer mutation because _if_ we were to do a
refactoring, we'd better just do it right and take possible features needed in
the future into account. But the refactoring itself is motivated by the need of
fixing this testing util (and making the return value from `findMutation`
generally more useful.)
BTW by `MutationTr
[PATCH] D52219: [analyzer] (1/n) Support pointee mutation analysis in ExprMutationAnalyzer.
JonasToth added inline comments.
Comment at: unittests/Analysis/ExprMutationAnalyzerTest.cpp:67
+if (const auto *DRE = dyn_cast(E)) {
+ if (DRE->getNameInfo().getAsString()[0] == 'p')
+Finder = PointeeMutationFinder;
shuaiwang wrote:
> JonasToth wrote:
> > shuaiwang wrote:
> > > JonasToth wrote:
> > > > That doesn't look like a super idea. It is super hidden that variable
> > > > 'p*' will be analyzed for pointee mutation and others aren't.
> > > > Especially in 12 months and when someone else wants to change
> > > > something, this will be the source of headaches.
> > > >
> > > > Don't you think there could be a better way of doing this switch?
> > > Yeah... agree :)
> > > But how about let's keep it this way just for now, and I'll pause the
> > > work on supporting pointee analysis and fix it properly but in a separate
> > > diff following this one?
> > >
> > > I've been thinking about this and also did some prototyping, and here are
> > > my thoughts:
> > > In order to properly fix this we need to change the interface of
> > > `ExprMutationAnalyzer` to not simply return a `Stmt` but instead return a
> > > `MutationTrace` with additional metadata. This wasn't needed before
> > > because we can reconstruct a mutation chain by continuously calling
> > > `findMutation`, and that works well for cases like "int x; int& r0 = x;
> > > int& r1 = r0; r1 = 123;". But now that pointers come into play, and we
> > > need to handle cases like "int *p; int *p2 = p; int& r = *p2; r = 123;",
> > > and in order to reconstruct the mutation chain, we need to do
> > > `findPointeeMutation(p)` -> `findPointeeMutation(p2)` ->
> > > `findMutation(r)`, notice that we need to switch from calling
> > > `findPointeeMutation` to calling `findMutation`. However we don't know
> > > where the switch should happen simply based on what's returned from
> > > `findPointeeMutation`, we need additional metadata for that.
> > >
> > > That interface change will unavoidably affect how memoization is done so
> > > we need to change memoization as well.
> > >
> > > Now since we need to change both the interface and memoization anyway, we
> > > might as well design them properly so that multiple-layers of pointers
> > > can be supported nicely. I think we can end up with something like this:
> > > ```
> > > class ExprMutationAnalyzer {
> > > public:
> > > struct MutationTrace {
> > > const Stmt *Value;
> > > const MutationTrace *Next;
> > > // Other metadata
> > > };
> > >
> > > // Finds whether any mutative operations are performed on the given
> > > expression after dereferencing `DerefLayers` times.
> > > const MutationTrace *findMutation(const Expr *, int DerefLayers = 0);
> > > const MutationTrace *findMutation(const Decl *, int DerefLayers = 0);
> > > };
> > > ```
> > > This involves quite some refactoring, and doing this refactoring before
> > > this diff and later rebasing seems quite some trouble that I'd like to
> > > avoid.
> > >
> > > Let me know what do you think :)
> > Is the second part of your answer part to adressing this testing issue?
> > Given the whole Analyzer is WIP it is ok to postpone the testing change to
> > later for me. But I feel that this is a quality issue that more experience
> > clang develops should comment on because it still lands in trunk. Are you
> > aware of other patches then clang-tidy that rely on EMA?
> >
> > Regarding you second part (its related to multi-layer pointer mutation?!):
> > Having a dependency-graph that follows the general data flow _with_
> > mutation annotations would be nice to have. There is probably even
> > something in clang (e.g. `CFG` is probably similar in idea, just with all
> > execution paths), but I don't know enough to properly judge here.
> >
> > In my opinion it is ok, to not do the refactoring now and just support one
> > layer of pointer indirection. Especially in C++ there is usually no need
> > for multi-layer pointers but more a relict from C-style.
> > C on the other hand relies on that.
> > Designing EMA new for the more generalized functionality (if necessary)
> > would be of course great, but again: Better analyzer ppl should be involved
> > then me, even though I would still be very interested to help :)
> The second part is more of trying to explain why I'd prefer to keep the test
> this way just for this diff.
> Basically we need a refactoring that makes EMA returns MutationTrace in order
> to fix this test util here.
>
> But thinking more about this, maybe we don't need that (for now), this util
> is to help restore a mutation chain, and since we don't support a chain of
> mutation for pointers yet we can have a simpler version of `pointeeMutatedBy`
> that doesn't support chaining.
>
> I mentioned multi-layer pointer mutation because _if_ we were to do a
> refactoring, we'd better just do it right and take possible features needed
> in t
[PATCH] D52300: [clangd] Implement VByte PostingList compression
sammccall added a comment.
Very nice!
I think the data structures can be slightly tighter, and some of the
implementation could be easier to follow. But this seems like a nice win.
Right-sizing the vectors seems like an important optimization.
Comment at: clang-tools-extra/clangd/index/dex/PostingList.cpp:29
+ DecompressedChunk = ChunkIndex->decompress();
+ InnerIndex = begin(DecompressedChunk);
+}
nit: we generally use members (DecompressedChunk.begin()) unless actually
dealing with arrays or templates, since lookup rules are simpler
Comment at: clang-tools-extra/clangd/index/dex/PostingList.cpp:39
"Posting List iterator can't advance() at the end.");
-++Index;
+if (++InnerIndex != end(DecompressedChunk))
+ return; // Return if current chunk is not exhausted.
nit: I think this might be clearer with the special/unlikely cases (hit end)
inside the if:
```
if (++InnerIndex == DecompressedChunks.begin()) { // end of chunk
if (++ChunkIndex == Chunks.end()) // end of iterator
return;
DecompressedChunk = ChunkIndex->decompress();
InnerIndex = DecompressedChunk.begin();
}
```
also I think the indirection via `reachedEnd()` mostly serves to confuse here,
as the other lines deal with the data structures directly. It's not clear
(without reading the implementation) what the behavior is when class invariants
are violated.
Comment at: clang-tools-extra/clangd/index/dex/PostingList.cpp:58
+// does.
+if ((ChunkIndex != end(Chunks) - 1) && ((ChunkIndex + 1)->Head <= ID)) {
+ // Find the next chunk with Head >= ID.
this again puts the "normal case" (need to choose a chunk) inside the if(),
instead of the exceptional case.
In order to write this more naturally, I think pulling out a private helper
`advanceToChunk(DocID)` might be best here, you can early return from there.
Comment at: clang-tools-extra/clangd/index/dex/PostingList.cpp:61
+ ChunkIndex = std::lower_bound(
+ ChunkIndex, end(Chunks), ID,
+ [](const Chunk &C, const DocID ID) { return C.Head < ID; });
ChunkIndex + 1? You've already eliminated the current chunk.
Comment at: clang-tools-extra/clangd/index/dex/PostingList.cpp:62
+ ChunkIndex, end(Chunks), ID,
+ [](const Chunk &C, const DocID ID) { return C.Head < ID; });
+ if (reachedEnd())
This seems unneccesarily two-step (found the chunk... or it could be the first
element of the next).
Understandably, because std::*_bound has such a silly API.
You want to find the *last* chunk such that Head <= ID.
So find the first one with Head > ID, and subtract one.
std::lower_bound returns the first element for which its predicate is false.
Therefore:
```
ChunkIndex = std::lower_bound(ChunkIndex, Chunks.end(), ID,
[](const Chunk &C, const DocID D) { return C.Head <= ID; }) - 1;
```
Comment at: clang-tools-extra/clangd/index/dex/PostingList.cpp:63
+ [](const Chunk &C, const DocID ID) { return C.Head < ID; });
+ if (reachedEnd())
+return;
(again I'd avoid reachedEnd() here as you haven't reestablished invariants, so
it's easier to just deal with the data structures)
Comment at: clang-tools-extra/clangd/index/dex/PostingList.cpp:76
+// Return if the position was found in current chunk.
+if (InnerIndex != std::end(DecompressedChunk))
+ return;
(this can become an assert)
Comment at: clang-tools-extra/clangd/index/dex/PostingList.cpp:115
+ // Iterator over chunks.
+ std::vector::const_iterator ChunkIndex;
+ std::vector DecompressedChunk;
nit: please don't call these indexes if they're actually iterators:
CurrentChunk seems fine
Comment at: clang-tools-extra/clangd/index/dex/PostingList.cpp:116
+ std::vector::const_iterator ChunkIndex;
+ std::vector DecompressedChunk;
+ // Iterator over DecompressedChunk.
(again, SmallVector)
Comment at: clang-tools-extra/clangd/index/dex/PostingList.cpp:121
+/// Single-byte masks used for VByte compression bit manipulations.
+constexpr uint8_t SevenBytesMask = 0x7f; // 0b0111
move to the function where they're used
Comment at: clang-tools-extra/clangd/index/dex/PostingList.cpp:127
+/// Fills chunk with the maximum number of bits available.
+Chunk createChunk(DocID Head, std::queue &Payload,
+ size_t DocumentsCount, size_t MeaningfulBytes) {
What's the purpose of this? Why can't the caller just construct the Chunk
themselves - what does the std::queue buy us?
Comment at: clang-tools-extra/clangd/index/dex/PostingList.cpp:140
+
+/// Byte offsets of Payload contents within DocID.
+const size_t
[PATCH] D52313: [clang-doc] Avoid parsing undefined base classes
juliehockett created this revision.
juliehockett added reviewers: jakehehrlich, leonardchan, lebedev.ri.
juliehockett added a project: clang-tools-extra.
Don't try to parse base classes for declarations that are not definitions
(segfaults, as there is no DefinitionData to access).
https://reviews.llvm.org/D52313
Files:
clang-tools-extra/clang-doc/Serialize.cpp
Index: clang-tools-extra/clang-doc/Serialize.cpp
===
--- clang-tools-extra/clang-doc/Serialize.cpp
+++ clang-tools-extra/clang-doc/Serialize.cpp
@@ -244,6 +244,9 @@
}
static void parseBases(RecordInfo &I, const CXXRecordDecl *D) {
+ // Don't parse bases if this isn't a definition.
+ if (!D->isThisDeclarationADefinition())
+return;
for (const CXXBaseSpecifier &B : D->bases()) {
if (B.isVirtual())
continue;
Index: clang-tools-extra/clang-doc/Serialize.cpp
===
--- clang-tools-extra/clang-doc/Serialize.cpp
+++ clang-tools-extra/clang-doc/Serialize.cpp
@@ -244,6 +244,9 @@
}
static void parseBases(RecordInfo &I, const CXXRecordDecl *D) {
+ // Don't parse bases if this isn't a definition.
+ if (!D->isThisDeclarationADefinition())
+return;
for (const CXXBaseSpecifier &B : D->bases()) {
if (B.isVirtual())
continue;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52311: [clangd] Add support for hierarchical documentSymbol
simark added a comment. Ohh awesome, I didn't know the LSP supported that. I'll try it it Theia when I have time. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52311 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52315: [clang-tidy] Fix for performance-unnecessary-value-param, performance-unnecessary-copy-initialization and performance-for-range-copy
baloghadamsoftware created this revision.
baloghadamsoftware added reviewers: alexfh, aaron.ballman, flx.
The three checks mentioned in the Title are two noisy if the code uses
intrusive smart (reference counting) pointers. LLVM/Clang is such a code, it
has lots of such types, e.g. StringRef, SymbolRef or ProgramStateRef, all of
them based llvm::IntrusiveRefCntPtr. Every time such a type is passed as
parameter, used for initialization or in a range-based for loop a false
positive warning is issued together with an (unnecessary) fix.
This patch changes the term "expensive to copy" to also regard the size of the
type so it disables warnings and fixes if the type is not greater than a
pointer. This removes false positives for intrusive smart pointers. False
positives for non-intrusive smart pointers are not addressed in this patch.
https://reviews.llvm.org/D52315
Files:
clang-tidy/utils/TypeTraits.cpp
docs/clang-tidy/checks/performance-for-range-copy.rst
docs/clang-tidy/checks/performance-unnecessary-copy-initialization.rst
docs/clang-tidy/checks/performance-unnecessary-value-param.rst
test/clang-tidy/Inputs/performance-unnecessary-value-param/header-fixed.h
test/clang-tidy/Inputs/performance-unnecessary-value-param/header.h
test/clang-tidy/performance-for-range-copy-warn-on-all-auto-copies.cpp
test/clang-tidy/performance-for-range-copy.cpp
test/clang-tidy/performance-unnecessary-copy-initialization.cpp
test/clang-tidy/performance-unnecessary-value-param-delayed.cpp
test/clang-tidy/performance-unnecessary-value-param.cpp
Index: test/clang-tidy/performance-unnecessary-value-param.cpp
===
--- test/clang-tidy/performance-unnecessary-value-param.cpp
+++ test/clang-tidy/performance-unnecessary-value-param.cpp
@@ -8,6 +8,9 @@
}
void nonConstMethod();
virtual ~ExpensiveToCopyType();
+private:
+ // Greater than a pointer
+ long Data1, Data2;
};
void mutate(ExpensiveToCopyType &);
@@ -27,6 +30,9 @@
iterator end();
const_iterator begin() const;
const_iterator end() const;
+private:
+ // Greater than a pointer
+ long Data1, Data2;
};
// This class simulates std::pair<>. It is trivially copy constructible
@@ -37,13 +43,19 @@
SomewhatTrivial(const SomewhatTrivial&) = default;
~SomewhatTrivial() = default;
SomewhatTrivial& operator=(const SomewhatTrivial&);
+private:
+ // Greater than a pointer
+ long Data1, Data2;
};
struct MoveOnlyType {
MoveOnlyType(const MoveOnlyType &) = delete;
MoveOnlyType(MoveOnlyType &&) = default;
~MoveOnlyType();
void constMethod() const;
+private:
+ // Greater than a pointer
+ long Data1, Data2;
};
struct ExpensiveMovableType {
@@ -53,6 +65,33 @@
ExpensiveMovableType &operator=(const ExpensiveMovableType &) = default;
ExpensiveMovableType &operator=(ExpensiveMovableType &&);
~ExpensiveMovableType();
+private:
+ // Greater than a pointer
+ long Data1, Data2;
+};
+
+// Intrusive smart pointers are usually passed by value
+template struct IntrusiveSmartPointerType {
+ IntrusiveSmartPointerType(const T* data);
+ IntrusiveSmartPointerType(const IntrusiveSmartPointerType& rhs);
+ const IntrusiveSmartPointerType& operator=(const T* data);
+ const IntrusiveSmartPointerType&
+ operator=(const IntrusiveSmartPointerType& data);
+
+ operator T*();
+private:
+ T *Data;
+};
+
+// Wrapper for builtin types used by intrusive smart pointers
+template
+class RefCntType {
+ unsigned count;
+ T value;
+public:
+ RefCntType(T val) : value(val) {}
+ operator T() { return value; }
+ friend class IntrusiveSmartPoitnerType;
};
void positiveExpensiveConstValue(const ExpensiveToCopyType Obj);
@@ -381,3 +420,10 @@
// CHECK-FIXES: void templateFunction(ExpensiveToCopyType E) {
E.constReference();
}
+
+// Do not warn for intrusive smart pointers which are usually passed by value
+void negativeIntrusiveSmartPointer(const IntrusiveSmartPointerType> Obj);
+
+void negativeIntrusiveSmartPointer(const IntrusiveSmartPointerType> Obj) {
+}
+
Index: test/clang-tidy/performance-unnecessary-value-param-delayed.cpp
===
--- test/clang-tidy/performance-unnecessary-value-param-delayed.cpp
+++ test/clang-tidy/performance-unnecessary-value-param-delayed.cpp
@@ -6,6 +6,9 @@
}
void nonConstMethod();
virtual ~ExpensiveToCopyType();
+private:
+ // Greater than a pointer
+ long Data1, Data2;
};
void mutate(ExpensiveToCopyType &);
@@ -21,6 +24,9 @@
SomewhatTrivial(const SomewhatTrivial&) = default;
~SomewhatTrivial() = default;
SomewhatTrivial& operator=(const SomewhatTrivial&);
+private:
+ // Greater than a pointer
+ long Data1, Data2;
};
void positiveExpensiveConstValue(const ExpensiveToCopyType Obj);
Index: test/clang-tidy/performance-unnecessary-copy-initialization.cpp
===
--- test/clang-tidy/p
r342666 - [OPENMP] Fix spelling of getLoopCounter (NFC)
Author: mikerice
Date: Thu Sep 20 10:19:41 2018
New Revision: 342666
URL: http://llvm.org/viewvc/llvm-project?rev=342666&view=rev
Log:
[OPENMP] Fix spelling of getLoopCounter (NFC)
Modified:
cfe/trunk/include/clang/AST/OpenMPClause.h
cfe/trunk/lib/AST/OpenMPClause.cpp
cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp
cfe/trunk/lib/Serialization/ASTWriter.cpp
Modified: cfe/trunk/include/clang/AST/OpenMPClause.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/OpenMPClause.h?rev=342666&r1=342665&r2=342666&view=diff
==
--- cfe/trunk/include/clang/AST/OpenMPClause.h (original)
+++ cfe/trunk/include/clang/AST/OpenMPClause.h Thu Sep 20 10:19:41 2018
@@ -990,8 +990,8 @@ public:
/// Set loop counter for the specified loop.
void setLoopCounter(unsigned NumLoop, Expr *Counter);
/// Get loops counter for the specified loop.
- Expr *getLoopCunter(unsigned NumLoop);
- const Expr *getLoopCunter(unsigned NumLoop) const;
+ Expr *getLoopCounter(unsigned NumLoop);
+ const Expr *getLoopCounter(unsigned NumLoop) const;
child_range children() { return child_range(&NumForLoops, &NumForLoops + 1);
}
Modified: cfe/trunk/lib/AST/OpenMPClause.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/OpenMPClause.cpp?rev=342666&r1=342665&r2=342666&view=diff
==
--- cfe/trunk/lib/AST/OpenMPClause.cpp (original)
+++ cfe/trunk/lib/AST/OpenMPClause.cpp Thu Sep 20 10:19:41 2018
@@ -222,12 +222,12 @@ void OMPOrderedClause::setLoopCounter(un
getTrailingObjects()[NumberOfLoops + NumLoop] = Counter;
}
-Expr *OMPOrderedClause::getLoopCunter(unsigned NumLoop) {
+Expr *OMPOrderedClause::getLoopCounter(unsigned NumLoop) {
assert(NumLoop < NumberOfLoops && "out of loops number.");
return getTrailingObjects()[NumberOfLoops + NumLoop];
}
-const Expr *OMPOrderedClause::getLoopCunter(unsigned NumLoop) const {
+const Expr *OMPOrderedClause::getLoopCounter(unsigned NumLoop) const {
assert(NumLoop < NumberOfLoops && "out of loops number.");
return getTrailingObjects()[NumberOfLoops + NumLoop];
}
Modified: cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp?rev=342666&r1=342665&r2=342666&view=diff
==
--- cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp Thu Sep 20 10:19:41 2018
@@ -1516,7 +1516,7 @@ void CodeGenFunction::EmitOMPPrivateLoop
for (unsigned I = S.getCollapsedNumber(),
E = C->getLoopNumIterations().size();
I < E; ++I) {
- const auto *DRE = cast(C->getLoopCunter(I));
+ const auto *DRE = cast(C->getLoopCounter(I));
const auto *VD = cast(DRE->getDecl());
// Override only those variables that are really emitted already.
if (LocalDeclMap.count(VD)) {
@@ -4966,7 +4966,7 @@ void CodeGenFunction::EmitSimpleOMPExecu
E = C->getLoopNumIterations().size();
I < E; ++I) {
if (const auto *VD = dyn_cast(
-cast(C->getLoopCunter(I))->getDecl())) {
+cast(C->getLoopCounter(I))->getDecl())) {
// Emit only those that were not explicitly referenced in
clauses.
if (!CGF.LocalDeclMap.count(VD))
CGF.EmitVarDecl(*VD);
Modified: cfe/trunk/lib/Serialization/ASTWriter.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTWriter.cpp?rev=342666&r1=342665&r2=342666&view=diff
==
--- cfe/trunk/lib/Serialization/ASTWriter.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTWriter.cpp Thu Sep 20 10:19:41 2018
@@ -6555,7 +6555,7 @@ void OMPClauseWriter::VisitOMPOrderedCla
for (Expr *NumIter : C->getLoopNumIterations())
Record.AddStmt(NumIter);
for (unsigned I = 0, E = C->getLoopNumIterations().size(); I getLoopCunter(I));
+Record.AddStmt(C->getLoopCounter(I));
Record.AddSourceLocation(C->getLParenLoc());
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52315: [clang-tidy] Fix for performance-unnecessary-value-param, performance-unnecessary-copy-initialization and performance-for-range-copy
JonasToth added inline comments. Comment at: clang-tidy/utils/TypeTraits.cpp:47 + + // Do not consider "expensive to copy" types not greater than a pointer + if (Context.getTypeSize(Type) <= Context.getTypeSize(Context.VoidPtrTy)) Please make that comment a sentence, and maybe don't use `not` twice. What do you think about making this parameter configurable and/or set it in relation to the size of a cache-line. This is of course target dependent, but given the CPU effects more accurate. https://reviews.llvm.org/D52315 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51926: [scan-build-py] Prevent crashes of CTU analysis by suppressing warnings
george.karpenkov accepted this revision. george.karpenkov added a comment. This revision is now accepted and ready to land. If that helps you, then sure. I'm not sure I understand why having warnings causes the collection process to fail, but I guess ultimately it's not important. Repository: rC Clang https://reviews.llvm.org/D51926 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52280: Don't trim non-alphanumeric characters in 'file not found' errors for include directives.
This revision was automatically updated to reflect the committed changes.
Closed by commit rC342667: r342177 introduced a hint in cases where an
#included file is not found. It… (authored by echristo, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D52280?vs=166184&id=166327#toc
Repository:
rC Clang
https://reviews.llvm.org/D52280
Files:
lib/Lex/PPDirectives.cpp
Index: lib/Lex/PPDirectives.cpp
===
--- lib/Lex/PPDirectives.cpp
+++ lib/Lex/PPDirectives.cpp
@@ -1887,8 +1887,8 @@
// Check for likely typos due to leading or trailing non-isAlphanumeric
// characters
+ StringRef OriginalFilename = Filename;
if (!File) {
-StringRef OriginalFilename = Filename;
while (!isAlphanumeric(Filename.front())) {
Filename = Filename.drop_front();
}
@@ -1915,7 +1915,7 @@
// If the file is still not found, just go with the vanilla diagnostic
if (!File)
-Diag(FilenameTok, diag::err_pp_file_not_found) << Filename
+Diag(FilenameTok, diag::err_pp_file_not_found) << OriginalFilename
<< FilenameRange;
}
}
Index: lib/Lex/PPDirectives.cpp
===
--- lib/Lex/PPDirectives.cpp
+++ lib/Lex/PPDirectives.cpp
@@ -1887,8 +1887,8 @@
// Check for likely typos due to leading or trailing non-isAlphanumeric
// characters
+ StringRef OriginalFilename = Filename;
if (!File) {
-StringRef OriginalFilename = Filename;
while (!isAlphanumeric(Filename.front())) {
Filename = Filename.drop_front();
}
@@ -1915,7 +1915,7 @@
// If the file is still not found, just go with the vanilla diagnostic
if (!File)
-Diag(FilenameTok, diag::err_pp_file_not_found) << Filename
+Diag(FilenameTok, diag::err_pp_file_not_found) << OriginalFilename
<< FilenameRange;
}
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52280: Don't trim non-alphanumeric characters in 'file not found' errors for include directives.
This revision was automatically updated to reflect the committed changes.
Closed by commit rL342667: r342177 introduced a hint in cases where an
#included file is not found. It… (authored by echristo, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D52280?vs=166184&id=166326#toc
Repository:
rL LLVM
https://reviews.llvm.org/D52280
Files:
cfe/trunk/lib/Lex/PPDirectives.cpp
Index: cfe/trunk/lib/Lex/PPDirectives.cpp
===
--- cfe/trunk/lib/Lex/PPDirectives.cpp
+++ cfe/trunk/lib/Lex/PPDirectives.cpp
@@ -1887,8 +1887,8 @@
// Check for likely typos due to leading or trailing non-isAlphanumeric
// characters
+ StringRef OriginalFilename = Filename;
if (!File) {
-StringRef OriginalFilename = Filename;
while (!isAlphanumeric(Filename.front())) {
Filename = Filename.drop_front();
}
@@ -1915,7 +1915,7 @@
// If the file is still not found, just go with the vanilla diagnostic
if (!File)
-Diag(FilenameTok, diag::err_pp_file_not_found) << Filename
+Diag(FilenameTok, diag::err_pp_file_not_found) << OriginalFilename
<< FilenameRange;
}
}
Index: cfe/trunk/lib/Lex/PPDirectives.cpp
===
--- cfe/trunk/lib/Lex/PPDirectives.cpp
+++ cfe/trunk/lib/Lex/PPDirectives.cpp
@@ -1887,8 +1887,8 @@
// Check for likely typos due to leading or trailing non-isAlphanumeric
// characters
+ StringRef OriginalFilename = Filename;
if (!File) {
-StringRef OriginalFilename = Filename;
while (!isAlphanumeric(Filename.front())) {
Filename = Filename.drop_front();
}
@@ -1915,7 +1915,7 @@
// If the file is still not found, just go with the vanilla diagnostic
if (!File)
-Diag(FilenameTok, diag::err_pp_file_not_found) << Filename
+Diag(FilenameTok, diag::err_pp_file_not_found) << OriginalFilename
<< FilenameRange;
}
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r342667 - r342177 introduced a hint in cases where an #included file is not found. It tries to find a suggestion by removing leading or trailing non-alphanumeric characters and checking if a matching
Author: echristo
Date: Thu Sep 20 10:21:56 2018
New Revision: 342667
URL: http://llvm.org/viewvc/llvm-project?rev=342667&view=rev
Log:
r342177 introduced a hint in cases where an #included file is not found. It
tries to find a suggestion by removing leading or trailing non-alphanumeric
characters and checking if a matching file exists, then it reports an error
like:
include-likely-typo.c:3:10: error: '' file not found,
did you mean 'empty_file_to_include.h'?
^~~
"empty_file_to_include.h"
1 error generated.
However, if a hint is not found, the error message will show only the trimmed
name we use to look for a hint, so:
will result in:
include-leading-nonalpha-no-suggest.c:3:10: fatal error:
'non_existing_file_to_include.h' file not found
^~~~
1 error generated.
where the name reported after "fatal error:" doesn't match what the user wrote.
Patch by Jorge Gorbe!
Differential Revision: https://reviews.llvm.org/D52280
This change reports the original file name instead of the trimmed one when a
suggestion is not found.
Modified:
cfe/trunk/lib/Lex/PPDirectives.cpp
Modified: cfe/trunk/lib/Lex/PPDirectives.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/PPDirectives.cpp?rev=342667&r1=342666&r2=342667&view=diff
==
--- cfe/trunk/lib/Lex/PPDirectives.cpp (original)
+++ cfe/trunk/lib/Lex/PPDirectives.cpp Thu Sep 20 10:21:56 2018
@@ -1887,8 +1887,8 @@ void Preprocessor::HandleIncludeDirectiv
// Check for likely typos due to leading or trailing non-isAlphanumeric
// characters
+ StringRef OriginalFilename = Filename;
if (!File) {
-StringRef OriginalFilename = Filename;
while (!isAlphanumeric(Filename.front())) {
Filename = Filename.drop_front();
}
@@ -1915,7 +1915,7 @@ void Preprocessor::HandleIncludeDirectiv
// If the file is still not found, just go with the vanilla diagnostic
if (!File)
-Diag(FilenameTok, diag::err_pp_file_not_found) << Filename
+Diag(FilenameTok, diag::err_pp_file_not_found) << OriginalFilename
<< FilenameRange;
}
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r342668 - Add testcases for r342667.
Author: echristo
Date: Thu Sep 20 10:22:43 2018
New Revision: 342668
URL: http://llvm.org/viewvc/llvm-project?rev=342668&view=rev
Log:
Add testcases for r342667.
Added:
cfe/trunk/test/Preprocessor/include-leading-nonalpha-no-suggest.c
cfe/trunk/test/Preprocessor/include-leading-nonalpha-suggest.c
Added: cfe/trunk/test/Preprocessor/include-leading-nonalpha-no-suggest.c
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Preprocessor/include-leading-nonalpha-no-suggest.c?rev=342668&view=auto
==
--- cfe/trunk/test/Preprocessor/include-leading-nonalpha-no-suggest.c (added)
+++ cfe/trunk/test/Preprocessor/include-leading-nonalpha-no-suggest.c Thu Sep
20 10:22:43 2018
@@ -0,0 +1,3 @@
+// RUN: %clang_cc1 %s -verify
+
+#include "/non_existing_file_to_include.h" // expected-error
{{'/non_existing_file_to_include.h' file not found}}
Added: cfe/trunk/test/Preprocessor/include-leading-nonalpha-suggest.c
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Preprocessor/include-leading-nonalpha-suggest.c?rev=342668&view=auto
==
--- cfe/trunk/test/Preprocessor/include-leading-nonalpha-suggest.c (added)
+++ cfe/trunk/test/Preprocessor/include-leading-nonalpha-suggest.c Thu Sep 20
10:22:43 2018
@@ -0,0 +1,3 @@
+// RUN: %clang_cc1 %s -verify
+
+#include "/empty_file_to_include.h" // expected-error
{{'/empty_file_to_include.h' file not found, did you mean
'empty_file_to_include.h'?}}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52315: [clang-tidy] Fix for performance-unnecessary-value-param, performance-unnecessary-copy-initialization and performance-for-range-copy
lebedev.ri added a comment. This looks questionable to me. I don't disagree with the reasons stated about llvm types. But is that *always* true? I would personally be very surprized, and consider this a false-positive. This should at least be optional. Not sure about the default, but setting the `StrictMode` should certainly disable this relaxation. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52315 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r229575 - clang-cl: Disable frame pointer elimination at -O0
It looks like we don't do anything special if you run clang-cl -O0 or /O0, but it's not an error. I don't have my computer and can't run a test, but from the outside, it looks like clang-cl -O0 does generate unoptimized code without warning about an unrecognized flag, but it doesn't disable FP elimination. It's also a GCC style flag, and usually if a core option is accepted, it has the same behavior in both driver modes. I'd be fine removing this extra handling so long as the driver explicitly tells the user that /O0 isn't recognized. While we're at it, maybe we should warn about other unrecognized /O flags. On Wed, Sep 19, 2018 at 6:10 PM Nico Weber wrote: > The generic O flag handling doesn't support 0 either. Would you be ok with > removing this? > > Does /Od do what you want? > > On Wed, Sep 19, 2018 at 4:52 PM Reid Kleckner > wrote: > >> I was probably using it myself, and was surprised that /O0 and -O0 had >> different behavior, because -O0 will hit the special O0 flag that disables >> FPO, but /O0 will hit the generic 'O' flag handling. >> >> On Wed, Sep 19, 2018 at 7:10 AM Nico Weber wrote: >> >>> Do you remember why you landed this? cl.exe doesn't support /O0; why >>> does clang-cl? >>> >>> On Tue, Feb 17, 2015 at 5:53 PM Reid Kleckner wrote: >>> Author: rnk Date: Tue Feb 17 16:40:42 2015 New Revision: 229575 URL: http://llvm.org/viewvc/llvm-project?rev=229575&view=rev Log: clang-cl: Disable frame pointer elimination at -O0 This wasn't kicking in because the _SLASH_O flag didn't match our check for OPT_O0. Add an alias that does to keep the logic simple. Modified: cfe/trunk/include/clang/Driver/CLCompatOptions.td Modified: cfe/trunk/include/clang/Driver/CLCompatOptions.td URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/CLCompatOptions.td?rev=229575&r1=229574&r2=229575&view=diff == --- cfe/trunk/include/clang/Driver/CLCompatOptions.td (original) +++ cfe/trunk/include/clang/Driver/CLCompatOptions.td Tue Feb 17 16:40:42 2015 @@ -80,6 +80,7 @@ def _SLASH_I : CLJoinedOrSeparate<"I">, Alias; def _SLASH_J : CLFlag<"J">, HelpText<"Make char type unsigned">, Alias; +def _SLASH_O0 : CLFlag<"O0">, Alias; def _SLASH_O : CLJoined<"O">, HelpText<"Optimization level">, MetaVarName<"">, Alias; def _SLASH_Ob0 : CLFlag<"Ob0">, HelpText<"Disable inlining">, ___ cfe-commits mailing list cfe-comm...@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >>> ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46140: [coroutines] std::task type (WIP)
lewissbaker added inline comments. Comment at: test/std/experimental/task/sync_wait.hpp:36-37 + __isSet_ = true; +} +__cv_.notify_all(); + } The call to `notify_all()` needs to be inside the lock. Otherwise, it's possible the waiting thread may see the write to `__isSet_` inside `wait()` below and return, destroying the `condition_variable` before `__cv_.notify_all()` call completes. https://reviews.llvm.org/D46140 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52267: [AST] Various optimizations + refactoring in DeclarationName(Table)
riccibruno updated this revision to Diff 166324.
riccibruno marked 9 inline comments as done.
riccibruno added a comment.
Address rjmccall comments:
1. Renamed `CXXSpecialName` to `CXXSpecialNameExtra`
2. Introduced a constant `IdentifierInfoAlignment` for
`alignas(IdentifierInfoAlignment)`
3. Updated some comments
4. Cleaned up the `static_assert` which checked the consistency of the
numerical values of the enumerators.
Regarding using a `PointerIntPair` it would work if we pass a custom
`PointerLikeTypeTraits` to state that, yes, our `void *` are aligned to 8 bytes
instead of 4. However I am not sure this is a good idea since the
`PointerIntPair`
do not really simplify the code. What we really want here is a pointer union
but we
also need precise control over the low bits.
Repository:
rC Clang
https://reviews.llvm.org/D52267
Files:
include/clang/AST/DeclarationName.h
include/clang/Basic/IdentifierTable.h
lib/AST/DeclarationName.cpp
lib/Basic/IdentifierTable.cpp
lib/Sema/IdentifierResolver.cpp
lib/Serialization/ASTReader.cpp
lib/Serialization/ASTWriter.cpp
Index: lib/Serialization/ASTWriter.cpp
===
--- lib/Serialization/ASTWriter.cpp
+++ lib/Serialization/ASTWriter.cpp
@@ -3574,7 +3574,7 @@
II->isPoisoned() ||
(IsModule ? II->hasRevertedBuiltin() : II->getObjCOrBuiltinID()) ||
II->hasRevertedTokenIDToIdentifier() ||
-(NeedDecls && II->getFETokenInfo()))
+(NeedDecls && II->getFETokenInfo()))
return true;
return false;
Index: lib/Serialization/ASTReader.cpp
===
--- lib/Serialization/ASTReader.cpp
+++ lib/Serialization/ASTReader.cpp
@@ -908,7 +908,7 @@
(IsModule ? II.hasRevertedBuiltin() : II.getObjCOrBuiltinID()) ||
II.hasRevertedTokenIDToIdentifier() ||
(!(IsModule && Reader.getPreprocessor().getLangOpts().CPlusPlus) &&
- II.getFETokenInfo());
+ II.getFETokenInfo());
}
static bool readBit(unsigned &Bits) {
Index: lib/Sema/IdentifierResolver.cpp
===
--- lib/Sema/IdentifierResolver.cpp
+++ lib/Sema/IdentifierResolver.cpp
@@ -147,7 +147,7 @@
if (IdentifierInfo *II = Name.getAsIdentifierInfo())
updatingIdentifier(*II);
- void *Ptr = Name.getFETokenInfo();
+ void *Ptr = Name.getFETokenInfo();
if (!Ptr) {
Name.setFETokenInfo(D);
@@ -172,7 +172,7 @@
if (IdentifierInfo *II = Name.getAsIdentifierInfo())
updatingIdentifier(*II);
- void *Ptr = Name.getFETokenInfo();
+ void *Ptr = Name.getFETokenInfo();
if (!Ptr) {
AddDecl(D);
@@ -213,7 +213,7 @@
if (IdentifierInfo *II = Name.getAsIdentifierInfo())
updatingIdentifier(*II);
- void *Ptr = Name.getFETokenInfo();
+ void *Ptr = Name.getFETokenInfo();
assert(Ptr && "Didn't find this decl on its identifier's chain!");
@@ -232,7 +232,7 @@
if (IdentifierInfo *II = Name.getAsIdentifierInfo())
readingIdentifier(*II);
- void *Ptr = Name.getFETokenInfo();
+ void *Ptr = Name.getFETokenInfo();
if (!Ptr) return end();
if (isDeclPtr(Ptr))
@@ -304,7 +304,7 @@
if (IdentifierInfo *II = Name.getAsIdentifierInfo())
readingIdentifier(*II);
- void *Ptr = Name.getFETokenInfo();
+ void *Ptr = Name.getFETokenInfo();
if (!Ptr) {
Name.setFETokenInfo(D);
@@ -397,7 +397,7 @@
/// It creates a new IdDeclInfo if one was not created before for this id.
IdentifierResolver::IdDeclInfo &
IdentifierResolver::IdDeclInfoMap::operator[](DeclarationName Name) {
- void *Ptr = Name.getFETokenInfo();
+ void *Ptr = Name.getFETokenInfo();
if (Ptr) return *toIdDeclInfo(Ptr);
@@ -415,7 +415,7 @@
void IdentifierResolver::iterator::incrementSlowCase() {
NamedDecl *D = **this;
- void *InfoPtr = D->getDeclName().getFETokenInfo();
+ void *InfoPtr = D->getDeclName().getFETokenInfo();
assert(!isDeclPtr(InfoPtr) && "Decl with wrong id ?");
IdDeclInfo *Info = toIdDeclInfo(InfoPtr);
Index: lib/Basic/IdentifierTable.cpp
===
--- lib/Basic/IdentifierTable.cpp
+++ lib/Basic/IdentifierTable.cpp
@@ -382,50 +382,49 @@
namespace clang {
-/// MultiKeywordSelector - One of these variable length records is kept for each
+/// One of these variable length records is kept for each
/// selector containing more than one keyword. We use a folding set
/// to unique aggregate names (keyword selectors in ObjC parlance). Access to
/// this class is provided strictly through Selector.
-class MultiKeywordSelector
- : public DeclarationNameExtra, public llvm::FoldingSetNode {
- MultiKeywordSelector(unsigned nKeys) {
-ExtraKindOrNumArgs = NUM_EXTRA_KINDS + nKeys;
- }
+class alignas(IdentifierInfoAlignment) MultiKeywordSelector
+: public detail::DeclarationNameExtra,
+ public llvm::FoldingSetNode {
+ MultiKeywor
[PATCH] D52267: [AST] Various optimizations + refactoring in DeclarationName(Table)
riccibruno added a comment.
Addressed some inline comments.
Comment at: include/clang/AST/DeclarationName.h:46
-/// DeclarationName - The name of a declaration. In the common case,
-/// this just stores an IdentifierInfo pointer to a normal
-/// name. However, it also provides encodings for Objective-C
-/// selectors (optimizing zero- and one-argument selectors, which make
-/// up 78% percent of all selectors in Cocoa.h) and special C++ names
-/// for constructors, destructors, and conversion functions.
+namespace detail {
+
rjmccall wrote:
> Should `DeclarationNameExtra` be moved here? I'm not sure why it's in
> `IdentifierTable.h` in the first place, and your refactor seems to make that
> even more pointless.
`DeclarationNameExtra` is unfortunately needed by `MultiKeywordSelector`
in `IdentifierInfo.cpp`.
Comment at: include/clang/AST/DeclarationName.h:164
+CXXUsingDirective = 10,
+ObjCMultiArgSelector = 11
+ };
rjmccall wrote:
> Is the criterion for inclusion in the first seven really just frequency of
> use, or is it a combination of that and relative benefit?
>
> The only one I would really quibble about is that multi-argument selectors
> are probably more common and important to Objective-C code than conversion
> operators are to C++. But it's quite possible that the relative benefit is
> much higher for C++, since selectors only appear on specific kinds of
> declarations that know they're declared with selectors — relatively little
> code actually needs to be polymorphic about them — and since they have to be
> defined out-of-line.
I did not do an in-depth analysis regarding the selection of the first seven
inline kinds.
My thought process was that C++ constructors and operator names should
definitely be
inline. C++ destructors seemed much more frequent than c++ literal operators and
deduction guides. This leaves one slot available and since C++ conversion
operators
share the same class `CXXSpecialName` including it as an inline kind simplifies
the code.
Also a practical point is that I don't really have a representative sample of
ObjC code
to benchmark this. For C++ I am using all of Boost which I hope is
representative
enough. If you deem it necessary I will try to do some benchmarks with
`ObjCMultiArgSelector` stored inline.
Repository:
rC Clang
https://reviews.llvm.org/D52267
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52315: [clang-tidy] Fix for performance-unnecessary-value-param, performance-unnecessary-copy-initialization and performance-for-range-copy
flx added inline comments. Comment at: clang-tidy/utils/TypeTraits.cpp:49 + if (Context.getTypeSize(Type) <= Context.getTypeSize(Context.VoidPtrTy)) +return false; + This early return now ignores the fact that the type has non-trivial copy constructors, virtual destructors etc which makes the type expensive to copy. Could we achieve the same desired outcome by adding a blacklist that allows users exclude types they deem to be not expensive? This way they would get a warning the first time they run the check and then could disable such types. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52315 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52266: [clang-cl] Provide separate flags for all the /O variants
thakis added inline comments. Comment at: include/clang/Driver/CLCompatOptions.td:121 +def : CLFlag<"Ob0">, Alias<_SLASH_O>, AliasArgs<["b0"]>, HelpText<"Disable function inlining">; +def : CLFlag<"Ob1">, Alias<_SLASH_O>, AliasArgs<["b1"]>, HelpText<"Only inline functions which are (explicitly or implicitly) marked inline">; +def : CLFlag<"Ob2">, Alias<_SLASH_O>, AliasArgs<["b2"]>, HelpText<"Inline functions as deemed beneficial by the compiler">; Also, can we try to keep these at 80 columns? The other flags in this file put HelpText before Alias and AliasArg (see my diff), but your order is fine. The 80 columns help trying to keep the help text concise, so it encourages better UI in this case :-) https://reviews.llvm.org/D52266 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r342672 - [Sema] Retain __restrict qualifiers when substituting a reference type.
Author: epilk
Date: Thu Sep 20 11:12:24 2018
New Revision: 342672
URL: http://llvm.org/viewvc/llvm-project?rev=342672&view=rev
Log:
[Sema] Retain __restrict qualifiers when substituting a reference type.
Fixes rdar://43760099
Differential revision: https://reviews.llvm.org/D52271
Added:
cfe/trunk/test/SemaCXX/subst-restrict.cpp
Modified:
cfe/trunk/lib/Sema/TreeTransform.h
Modified: cfe/trunk/lib/Sema/TreeTransform.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/TreeTransform.h?rev=342672&r1=342671&r2=342672&view=diff
==
--- cfe/trunk/lib/Sema/TreeTransform.h (original)
+++ cfe/trunk/lib/Sema/TreeTransform.h Thu Sep 20 11:12:24 2018
@@ -4252,14 +4252,20 @@ QualType TreeTransform::Rebuild
// C++ [dcl.fct]p7:
// [When] adding cv-qualifications on top of the function type [...] the
// cv-qualifiers are ignored.
+ if (T->isFunctionType())
+return T;
+
// C++ [dcl.ref]p1:
// when the cv-qualifiers are introduced through the use of a typedef-name
// or decltype-specifier [...] the cv-qualifiers are ignored.
// Note that [dcl.ref]p1 lists all cases in which cv-qualifiers can be
// applied to a reference type.
- // FIXME: This removes all qualifiers, not just cv-qualifiers!
- if (T->isFunctionType() || T->isReferenceType())
-return T;
+ if (T->isReferenceType()) {
+// The only qualifier that applies to a reference type is restrict.
+if (!Quals.hasRestrict())
+ return T;
+Quals = Qualifiers::fromCVRMask(Qualifiers::Restrict);
+ }
// Suppress Objective-C lifetime qualifiers if they don't make sense for the
// resulting type.
Added: cfe/trunk/test/SemaCXX/subst-restrict.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/subst-restrict.cpp?rev=342672&view=auto
==
--- cfe/trunk/test/SemaCXX/subst-restrict.cpp (added)
+++ cfe/trunk/test/SemaCXX/subst-restrict.cpp Thu Sep 20 11:12:24 2018
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -std=c++17 -verify %s
+
+// expected-no-diagnostics
+
+template struct add_restrict {
+ typedef T __restrict type;
+};
+
+template struct is_same {
+ static constexpr bool value = false;
+};
+
+template struct is_same {
+ static constexpr bool value = true;
+};
+
+static_assert(is_same::type>::value, "");
+static_assert(is_same::type>::value, "");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52271: [Sema] Ensure that we retain __restrict qualifiers when substituting a reference type.
This revision was automatically updated to reflect the committed changes.
Closed by commit rC342672: [Sema] Retain __restrict qualifiers when
substituting a reference type. (authored by epilk, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D52271?vs=166224&id=166334#toc
Repository:
rC Clang
https://reviews.llvm.org/D52271
Files:
lib/Sema/TreeTransform.h
test/SemaCXX/subst-restrict.cpp
Index: test/SemaCXX/subst-restrict.cpp
===
--- test/SemaCXX/subst-restrict.cpp
+++ test/SemaCXX/subst-restrict.cpp
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -std=c++17 -verify %s
+
+// expected-no-diagnostics
+
+template struct add_restrict {
+ typedef T __restrict type;
+};
+
+template struct is_same {
+ static constexpr bool value = false;
+};
+
+template struct is_same {
+ static constexpr bool value = true;
+};
+
+static_assert(is_same::type>::value, "");
+static_assert(is_same::type>::value, "");
Index: lib/Sema/TreeTransform.h
===
--- lib/Sema/TreeTransform.h
+++ lib/Sema/TreeTransform.h
@@ -4252,14 +4252,20 @@
// C++ [dcl.fct]p7:
// [When] adding cv-qualifications on top of the function type [...] the
// cv-qualifiers are ignored.
+ if (T->isFunctionType())
+return T;
+
// C++ [dcl.ref]p1:
// when the cv-qualifiers are introduced through the use of a typedef-name
// or decltype-specifier [...] the cv-qualifiers are ignored.
// Note that [dcl.ref]p1 lists all cases in which cv-qualifiers can be
// applied to a reference type.
- // FIXME: This removes all qualifiers, not just cv-qualifiers!
- if (T->isFunctionType() || T->isReferenceType())
-return T;
+ if (T->isReferenceType()) {
+// The only qualifier that applies to a reference type is restrict.
+if (!Quals.hasRestrict())
+ return T;
+Quals = Qualifiers::fromCVRMask(Qualifiers::Restrict);
+ }
// Suppress Objective-C lifetime qualifiers if they don't make sense for the
// resulting type.
Index: test/SemaCXX/subst-restrict.cpp
===
--- test/SemaCXX/subst-restrict.cpp
+++ test/SemaCXX/subst-restrict.cpp
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -std=c++17 -verify %s
+
+// expected-no-diagnostics
+
+template struct add_restrict {
+ typedef T __restrict type;
+};
+
+template struct is_same {
+ static constexpr bool value = false;
+};
+
+template struct is_same {
+ static constexpr bool value = true;
+};
+
+static_assert(is_same::type>::value, "");
+static_assert(is_same::type>::value, "");
Index: lib/Sema/TreeTransform.h
===
--- lib/Sema/TreeTransform.h
+++ lib/Sema/TreeTransform.h
@@ -4252,14 +4252,20 @@
// C++ [dcl.fct]p7:
// [When] adding cv-qualifications on top of the function type [...] the
// cv-qualifiers are ignored.
+ if (T->isFunctionType())
+return T;
+
// C++ [dcl.ref]p1:
// when the cv-qualifiers are introduced through the use of a typedef-name
// or decltype-specifier [...] the cv-qualifiers are ignored.
// Note that [dcl.ref]p1 lists all cases in which cv-qualifiers can be
// applied to a reference type.
- // FIXME: This removes all qualifiers, not just cv-qualifiers!
- if (T->isFunctionType() || T->isReferenceType())
-return T;
+ if (T->isReferenceType()) {
+// The only qualifier that applies to a reference type is restrict.
+if (!Quals.hasRestrict())
+ return T;
+Quals = Qualifiers::fromCVRMask(Qualifiers::Restrict);
+ }
// Suppress Objective-C lifetime qualifiers if they don't make sense for the
// resulting type.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52296: [Clang] - Add -gsingle-file-split-dwarf option.
dblaikie added a comment. Do you/what's your particular use case for this scenario? I guess this looks a bit like Apple's format (where debug info stays in the object file and isn't linked into the final binary), but don't expect they'd be moving to this any time soon. https://reviews.llvm.org/D52296 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52253: Fix an assert in the implementation of -Wquoted-include-in-framework-header
bruno accepted this revision. bruno added a comment. This revision is now accepted and ready to land. LGTM with one more small change. Comment at: clang/test/Modules/double-quotes.m:35 +// rdar://43692300 +#import "NotAFramework/Headers/Headers/Thing1.h" Please write a meaningful message here instead, the radar number in the commit message should be enough to track this down later if needed. https://reviews.llvm.org/D52253 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52267: [AST] Various optimizations + refactoring in DeclarationName(Table)
rjmccall accepted this revision.
rjmccall added inline comments.
This revision is now accepted and ready to land.
Comment at: include/clang/AST/DeclarationName.h:46
-/// DeclarationName - The name of a declaration. In the common case,
-/// this just stores an IdentifierInfo pointer to a normal
-/// name. However, it also provides encodings for Objective-C
-/// selectors (optimizing zero- and one-argument selectors, which make
-/// up 78% percent of all selectors in Cocoa.h) and special C++ names
-/// for constructors, destructors, and conversion functions.
+namespace detail {
+
riccibruno wrote:
> rjmccall wrote:
> > Should `DeclarationNameExtra` be moved here? I'm not sure why it's in
> > `IdentifierTable.h` in the first place, and your refactor seems to make
> > that even more pointless.
> `DeclarationNameExtra` is unfortunately needed by `MultiKeywordSelector`
> in `IdentifierInfo.cpp`.
Oh, and one is in Basic instead of AST.
I'm not sure there's really a good reason for Selector to exist at the Basic
level instead of AST; it's probably more an artifact of old divisions than
something that's really useful. But changing that would probably be painful.
Comment at: include/clang/AST/DeclarationName.h:164
+CXXUsingDirective = 10,
+ObjCMultiArgSelector = 11
+ };
riccibruno wrote:
> rjmccall wrote:
> > Is the criterion for inclusion in the first seven really just frequency of
> > use, or is it a combination of that and relative benefit?
> >
> > The only one I would really quibble about is that multi-argument selectors
> > are probably more common and important to Objective-C code than conversion
> > operators are to C++. But it's quite possible that the relative benefit is
> > much higher for C++, since selectors only appear on specific kinds of
> > declarations that know they're declared with selectors — relatively little
> > code actually needs to be polymorphic about them — and since they have to
> > be defined out-of-line.
> I did not do an in-depth analysis regarding the selection of the first seven
> inline kinds.
> My thought process was that C++ constructors and operator names should
> definitely be
> inline. C++ destructors seemed much more frequent than c++ literal operators
> and
> deduction guides. This leaves one slot available and since C++ conversion
> operators
> share the same class `CXXSpecialName` including it as an inline kind
> simplifies the code.
>
> Also a practical point is that I don't really have a representative sample of
> ObjC code
> to benchmark this. For C++ I am using all of Boost which I hope is
> representative
> enough. If you deem it necessary I will try to do some benchmarks with
> `ObjCMultiArgSelector` stored inline.
I think I can accept this as-is without extra investigation.
Comment at: include/clang/AST/DeclarationName.h:220
+ detail::DeclarationNameExtra::ObjCMultiArgSelector
};
Thanks, this looks great.
Repository:
rC Clang
https://reviews.llvm.org/D52267
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50539: [VFS] Add property 'fallthrough' that controls fallback to real file system.
vsapsai added a comment. Ping. https://reviews.llvm.org/D50539 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r342679 - Fix an assert in -Wquoted-include-in-framework-header
Author: epilk
Date: Thu Sep 20 12:00:03 2018
New Revision: 342679
URL: http://llvm.org/viewvc/llvm-project?rev=342679&view=rev
Log:
Fix an assert in -Wquoted-include-in-framework-header
Fixes rdar://43692300
Differential revision: https://reviews.llvm.org/D52253
Added:
cfe/trunk/test/Modules/Inputs/double-quotes/NotAFramework/
cfe/trunk/test/Modules/Inputs/double-quotes/NotAFramework/Headers/
cfe/trunk/test/Modules/Inputs/double-quotes/NotAFramework/Headers/Headers/
cfe/trunk/test/Modules/Inputs/double-quotes/NotAFramework/Headers/Headers/Thing1.h
cfe/trunk/test/Modules/Inputs/double-quotes/NotAFramework/Headers/Headers/Thing2.h
Modified:
cfe/trunk/lib/Lex/HeaderSearch.cpp
cfe/trunk/test/Modules/double-quotes.m
Modified: cfe/trunk/lib/Lex/HeaderSearch.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/HeaderSearch.cpp?rev=342679&r1=342678&r2=342679&view=diff
==
--- cfe/trunk/lib/Lex/HeaderSearch.cpp (original)
+++ cfe/trunk/lib/Lex/HeaderSearch.cpp Thu Sep 20 12:00:03 2018
@@ -648,7 +648,7 @@ static bool isFrameworkStylePath(StringR
++I;
}
- return FoundComp >= 2;
+ return !FrameworkName.empty() && FoundComp >= 2;
}
static void
Added:
cfe/trunk/test/Modules/Inputs/double-quotes/NotAFramework/Headers/Headers/Thing1.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/double-quotes/NotAFramework/Headers/Headers/Thing1.h?rev=342679&view=auto
==
---
cfe/trunk/test/Modules/Inputs/double-quotes/NotAFramework/Headers/Headers/Thing1.h
(added)
+++
cfe/trunk/test/Modules/Inputs/double-quotes/NotAFramework/Headers/Headers/Thing1.h
Thu Sep 20 12:00:03 2018
@@ -0,0 +1 @@
+#include "Thing2.h"
Added:
cfe/trunk/test/Modules/Inputs/double-quotes/NotAFramework/Headers/Headers/Thing2.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/double-quotes/NotAFramework/Headers/Headers/Thing2.h?rev=342679&view=auto
==
---
cfe/trunk/test/Modules/Inputs/double-quotes/NotAFramework/Headers/Headers/Thing2.h
(added)
+++
cfe/trunk/test/Modules/Inputs/double-quotes/NotAFramework/Headers/Headers/Thing2.h
Thu Sep 20 12:00:03 2018
@@ -0,0 +1 @@
+// Empty file!
Modified: cfe/trunk/test/Modules/double-quotes.m
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/double-quotes.m?rev=342679&r1=342678&r2=342679&view=diff
==
--- cfe/trunk/test/Modules/double-quotes.m (original)
+++ cfe/trunk/test/Modules/double-quotes.m Thu Sep 20 12:00:03 2018
@@ -32,6 +32,9 @@
#import "A.h"
#import
+// Make sure we correctly handle paths that resemble frameworks, but aren't.
+#import "NotAFramework/Headers/Headers/Thing1.h"
+
int bar() { return foo(); }
//
expected-warning@Inputs/double-quotes/A.framework/Headers/A.h:1{{double-quoted
include "A0.h" in framework header, expected angle-bracketed instead}}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52253: Fix an assert in the implementation of -Wquoted-include-in-framework-header
This revision was automatically updated to reflect the committed changes.
Closed by commit rL342679: Fix an assert in
-Wquoted-include-in-framework-header (authored by epilk, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D52253?vs=166070&id=166344#toc
Repository:
rL LLVM
https://reviews.llvm.org/D52253
Files:
cfe/trunk/lib/Lex/HeaderSearch.cpp
cfe/trunk/test/Modules/Inputs/double-quotes/NotAFramework/Headers/Headers/Thing1.h
cfe/trunk/test/Modules/Inputs/double-quotes/NotAFramework/Headers/Headers/Thing2.h
cfe/trunk/test/Modules/double-quotes.m
Index: cfe/trunk/lib/Lex/HeaderSearch.cpp
===
--- cfe/trunk/lib/Lex/HeaderSearch.cpp
+++ cfe/trunk/lib/Lex/HeaderSearch.cpp
@@ -648,7 +648,7 @@
++I;
}
- return FoundComp >= 2;
+ return !FrameworkName.empty() && FoundComp >= 2;
}
static void
Index: cfe/trunk/test/Modules/double-quotes.m
===
--- cfe/trunk/test/Modules/double-quotes.m
+++ cfe/trunk/test/Modules/double-quotes.m
@@ -32,6 +32,9 @@
#import "A.h"
#import
+// Make sure we correctly handle paths that resemble frameworks, but aren't.
+#import "NotAFramework/Headers/Headers/Thing1.h"
+
int bar() { return foo(); }
//
expected-warning@Inputs/double-quotes/A.framework/Headers/A.h:1{{double-quoted
include "A0.h" in framework header, expected angle-bracketed instead}}
Index:
cfe/trunk/test/Modules/Inputs/double-quotes/NotAFramework/Headers/Headers/Thing2.h
===
---
cfe/trunk/test/Modules/Inputs/double-quotes/NotAFramework/Headers/Headers/Thing2.h
+++
cfe/trunk/test/Modules/Inputs/double-quotes/NotAFramework/Headers/Headers/Thing2.h
@@ -0,0 +1 @@
+// Empty file!
Index:
cfe/trunk/test/Modules/Inputs/double-quotes/NotAFramework/Headers/Headers/Thing1.h
===
---
cfe/trunk/test/Modules/Inputs/double-quotes/NotAFramework/Headers/Headers/Thing1.h
+++
cfe/trunk/test/Modules/Inputs/double-quotes/NotAFramework/Headers/Headers/Thing1.h
@@ -0,0 +1 @@
+#include "Thing2.h"
Index: cfe/trunk/lib/Lex/HeaderSearch.cpp
===
--- cfe/trunk/lib/Lex/HeaderSearch.cpp
+++ cfe/trunk/lib/Lex/HeaderSearch.cpp
@@ -648,7 +648,7 @@
++I;
}
- return FoundComp >= 2;
+ return !FrameworkName.empty() && FoundComp >= 2;
}
static void
Index: cfe/trunk/test/Modules/double-quotes.m
===
--- cfe/trunk/test/Modules/double-quotes.m
+++ cfe/trunk/test/Modules/double-quotes.m
@@ -32,6 +32,9 @@
#import "A.h"
#import
+// Make sure we correctly handle paths that resemble frameworks, but aren't.
+#import "NotAFramework/Headers/Headers/Thing1.h"
+
int bar() { return foo(); }
// expected-warning@Inputs/double-quotes/A.framework/Headers/A.h:1{{double-quoted include "A0.h" in framework header, expected angle-bracketed instead}}
Index: cfe/trunk/test/Modules/Inputs/double-quotes/NotAFramework/Headers/Headers/Thing2.h
===
--- cfe/trunk/test/Modules/Inputs/double-quotes/NotAFramework/Headers/Headers/Thing2.h
+++ cfe/trunk/test/Modules/Inputs/double-quotes/NotAFramework/Headers/Headers/Thing2.h
@@ -0,0 +1 @@
+// Empty file!
Index: cfe/trunk/test/Modules/Inputs/double-quotes/NotAFramework/Headers/Headers/Thing1.h
===
--- cfe/trunk/test/Modules/Inputs/double-quotes/NotAFramework/Headers/Headers/Thing1.h
+++ cfe/trunk/test/Modules/Inputs/double-quotes/NotAFramework/Headers/Headers/Thing1.h
@@ -0,0 +1 @@
+#include "Thing2.h"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52313: [clang-doc] Avoid parsing undefined base classes
lebedev.ri accepted this revision. lebedev.ri added a comment. This revision is now accepted and ready to land. If it **no longer** crashes, i guess you can test for that? Other than that, SG. https://reviews.llvm.org/D52313 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52320: AMDGPU: add __builtin_amdgcn_update_dpp
yaxunl created this revision.
yaxunl added reviewers: kzhuravl, b-sumner, arsenm.
Herald added subscribers: t-tye, tpr, dstuttard, nhaehnle, wdng, jvesely.
https://reviews.llvm.org/D52320
Files:
include/clang/Basic/BuiltinsAMDGPU.def
lib/CodeGen/CGBuiltin.cpp
test/CodeGenOpenCL/builtins-amdgcn-vi.cl
test/SemaOpenCL/builtins-amdgcn-error.cl
Index: test/SemaOpenCL/builtins-amdgcn-error.cl
===
--- test/SemaOpenCL/builtins-amdgcn-error.cl
+++ test/SemaOpenCL/builtins-amdgcn-error.cl
@@ -102,6 +102,15 @@
*out = __builtin_amdgcn_mov_dpp(a, 0, 0, 0, e); // expected-error {{argument
to '__builtin_amdgcn_mov_dpp' must be a constant integer}}
}
+void test_update_dpp2(global int* out, int a, int b, int c, int d, int e, bool
f)
+{
+ *out = __builtin_amdgcn_update_dpp(a, b, 0, 0, 0, false);
+ *out = __builtin_amdgcn_update_dpp(a, 0, c, 0, 0, false); // expected-error
{{argument to '__builtin_amdgcn_update_dpp' must be a constant integer}}
+ *out = __builtin_amdgcn_update_dpp(a, 0, 0, d, 0, false); // expected-error
{{argument to '__builtin_amdgcn_update_dpp' must be a constant integer}}
+ *out = __builtin_amdgcn_update_dpp(a, 0, 0, 0, e, false); // expected-error
{{argument to '__builtin_amdgcn_update_dpp' must be a constant integer}}
+ *out = __builtin_amdgcn_update_dpp(a, 0, 0, 0, 0, f); // expected-error
{{argument to '__builtin_amdgcn_update_dpp' must be a constant integer}}
+}
+
void test_ds_faddf(local float *out, float src, int a) {
*out = __builtin_amdgcn_ds_faddf(out, src, a, 0, false); // expected-error
{{argument to '__builtin_amdgcn_ds_faddf' must be a constant integer}}
*out = __builtin_amdgcn_ds_faddf(out, src, 0, a, false); // expected-error
{{argument to '__builtin_amdgcn_ds_faddf' must be a constant integer}}
Index: test/CodeGenOpenCL/builtins-amdgcn-vi.cl
===
--- test/CodeGenOpenCL/builtins-amdgcn-vi.cl
+++ test/CodeGenOpenCL/builtins-amdgcn-vi.cl
@@ -96,6 +96,13 @@
*out = __builtin_amdgcn_mov_dpp(src, 0, 0, 0, false);
}
+// CHECK-LABEL: @test_update_dpp
+// CHECK: call i32 @llvm.amdgcn.update.dpp.i32(i32 %arg1, i32 %arg2, i32 0,
i32 0, i32 0, i1 false)
+void test_update_dpp(global int* out, int arg1, int arg2)
+{
+ *out = __builtin_amdgcn_update_dpp(arg1, arg2, 0, 0, 0, false);
+}
+
// CHECK-LABEL: @test_ds_fadd
// CHECK: call float @llvm.amdgcn.ds.fadd(float addrspace(3)* %out, float
%src, i32 0, i32 0, i1 false)
void test_ds_faddf(local float *out, float src) {
Index: lib/CodeGen/CGBuiltin.cpp
===
--- lib/CodeGen/CGBuiltin.cpp
+++ lib/CodeGen/CGBuiltin.cpp
@@ -11310,6 +11310,14 @@
Args[0]->getType());
return Builder.CreateCall(F, Args);
}
+ case AMDGPU::BI__builtin_amdgcn_update_dpp: {
+llvm::SmallVector Args;
+for (unsigned I = 0; I != 6; ++I)
+ Args.push_back(EmitScalarExpr(E->getArg(I)));
+Value *F =
+CGM.getIntrinsic(Intrinsic::amdgcn_update_dpp, Args[0]->getType());
+return Builder.CreateCall(F, Args);
+ }
case AMDGPU::BI__builtin_amdgcn_div_fixup:
case AMDGPU::BI__builtin_amdgcn_div_fixupf:
case AMDGPU::BI__builtin_amdgcn_div_fixuph:
Index: include/clang/Basic/BuiltinsAMDGPU.def
===
--- include/clang/Basic/BuiltinsAMDGPU.def
+++ include/clang/Basic/BuiltinsAMDGPU.def
@@ -122,6 +122,7 @@
TARGET_BUILTIN(__builtin_amdgcn_classh, "bhi", "nc", "16-bit-insts")
TARGET_BUILTIN(__builtin_amdgcn_s_memrealtime, "LUi", "n", "s-memrealtime")
TARGET_BUILTIN(__builtin_amdgcn_mov_dpp, "iiIiIiIiIb", "nc", "dpp")
+TARGET_BUILTIN(__builtin_amdgcn_update_dpp, "iiiIiIiIiIb", "nc", "dpp")
TARGET_BUILTIN(__builtin_amdgcn_s_dcache_wb, "v", "n", "vi-insts")
//===--===//
Index: test/SemaOpenCL/builtins-amdgcn-error.cl
===
--- test/SemaOpenCL/builtins-amdgcn-error.cl
+++ test/SemaOpenCL/builtins-amdgcn-error.cl
@@ -102,6 +102,15 @@
*out = __builtin_amdgcn_mov_dpp(a, 0, 0, 0, e); // expected-error {{argument to '__builtin_amdgcn_mov_dpp' must be a constant integer}}
}
+void test_update_dpp2(global int* out, int a, int b, int c, int d, int e, bool f)
+{
+ *out = __builtin_amdgcn_update_dpp(a, b, 0, 0, 0, false);
+ *out = __builtin_amdgcn_update_dpp(a, 0, c, 0, 0, false); // expected-error {{argument to '__builtin_amdgcn_update_dpp' must be a constant integer}}
+ *out = __builtin_amdgcn_update_dpp(a, 0, 0, d, 0, false); // expected-error {{argument to '__builtin_amdgcn_update_dpp' must be a constant integer}}
+ *out = __builtin_amdgcn_update_dpp(a, 0, 0, 0, e, false); // expected-error {{argument to '__builtin_amdgcn_update_dpp' must be a constant integer}}
+ *out = __builtin_amdgcn_updat
[PATCH] D52248: [SEMA] ignore duplicate declaration specifiers from typeof exprs
nickdesaulniers updated this revision to Diff 166352.
nickdesaulniers added a comment.
- git-clang-format HEAD~
Repository:
rC Clang
https://reviews.llvm.org/D52248
Files:
lib/Sema/SemaType.cpp
test/Sema/gnu89.c
Index: test/Sema/gnu89.c
===
--- test/Sema/gnu89.c
+++ test/Sema/gnu89.c
@@ -1,5 +1,23 @@
-// RUN: %clang_cc1 %s -std=gnu89 -pedantic -fsyntax-only -verify
+// RUN: %clang_cc1 %s -std=gnu89 -pedantic -fsyntax-only 2>&1 | FileCheck
-check-prefix=CHECK-PEDANTIC %s
+// RUN: %clang_cc1 %s -std=gnu89 -fsyntax-only 2>&1 | FileCheck %s
int f(int restrict);
-void main() {} // expected-warning {{return type of 'main' is not 'int'}}
expected-note {{change return type to 'int'}}
+void main() {}
+// CHECK: return type of 'main' is not 'int'
+// CHECK: change return type to 'int'
+
+// Do not warn about duplicate const declaration specifier as the result of
+// typeof in gnu89.
+const int c_i;
+const typeof(c_i) c_i3;
+// CHECK-PEDANTIC: warning: extension used
+// CHECK-PEDANTIC: warning: duplicate 'const' declaration specifier
+// CHECK-otherwise-NOT: warning: duplicate 'const' declaration specifier
+
+const const int x;
+// CHECK: warning: duplicate 'const' declaration specifier
+
+typedef const int t;
+const t x2;
+// CHECK: warning: duplicate 'const' declaration specifier
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -1679,8 +1679,13 @@
// C90 6.5.3 constraints: "The same type qualifier shall not appear more
// than once in the same specifier-list or qualifier-list, either directly
// or via one or more typedefs."
-if (!S.getLangOpts().C99 && !S.getLangOpts().CPlusPlus
-&& TypeQuals & Result.getCVRQualifiers()) {
+//
+// Not checked for gnu89 if the TST is from a typeof expression and
+// -pedantic was not set.
+if (!S.getLangOpts().C99 && !S.getLangOpts().CPlusPlus &&
+TypeQuals & Result.getCVRQualifiers() &&
+!(S.getLangOpts().GNUMode && !S.Diags.getDiagnosticOptions().Pedantic
&&
+ DS.getTypeSpecType() == DeclSpec::TST_typeofExpr)) {
if (TypeQuals & DeclSpec::TQ_const && Result.isConstQualified()) {
S.Diag(DS.getConstSpecLoc(), diag::ext_duplicate_declspec)
<< "const";
Index: test/Sema/gnu89.c
===
--- test/Sema/gnu89.c
+++ test/Sema/gnu89.c
@@ -1,5 +1,23 @@
-// RUN: %clang_cc1 %s -std=gnu89 -pedantic -fsyntax-only -verify
+// RUN: %clang_cc1 %s -std=gnu89 -pedantic -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-PEDANTIC %s
+// RUN: %clang_cc1 %s -std=gnu89 -fsyntax-only 2>&1 | FileCheck %s
int f(int restrict);
-void main() {} // expected-warning {{return type of 'main' is not 'int'}} expected-note {{change return type to 'int'}}
+void main() {}
+// CHECK: return type of 'main' is not 'int'
+// CHECK: change return type to 'int'
+
+// Do not warn about duplicate const declaration specifier as the result of
+// typeof in gnu89.
+const int c_i;
+const typeof(c_i) c_i3;
+// CHECK-PEDANTIC: warning: extension used
+// CHECK-PEDANTIC: warning: duplicate 'const' declaration specifier
+// CHECK-otherwise-NOT: warning: duplicate 'const' declaration specifier
+
+const const int x;
+// CHECK: warning: duplicate 'const' declaration specifier
+
+typedef const int t;
+const t x2;
+// CHECK: warning: duplicate 'const' declaration specifier
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -1679,8 +1679,13 @@
// C90 6.5.3 constraints: "The same type qualifier shall not appear more
// than once in the same specifier-list or qualifier-list, either directly
// or via one or more typedefs."
-if (!S.getLangOpts().C99 && !S.getLangOpts().CPlusPlus
-&& TypeQuals & Result.getCVRQualifiers()) {
+//
+// Not checked for gnu89 if the TST is from a typeof expression and
+// -pedantic was not set.
+if (!S.getLangOpts().C99 && !S.getLangOpts().CPlusPlus &&
+TypeQuals & Result.getCVRQualifiers() &&
+!(S.getLangOpts().GNUMode && !S.Diags.getDiagnosticOptions().Pedantic &&
+ DS.getTypeSpecType() == DeclSpec::TST_typeofExpr)) {
if (TypeQuals & DeclSpec::TQ_const && Result.isConstQualified()) {
S.Diag(DS.getConstSpecLoc(), diag::ext_duplicate_declspec)
<< "const";
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52248: [SEMA] ignore duplicate declaration specifiers from typeof exprs
manojgupta added a comment. lgtm. But someone more familiar with these code paths should approve. Repository: rC Clang https://reviews.llvm.org/D52248 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52248: [SEMA] ignore duplicate declaration specifiers from typeof exprs
lebedev.ri added inline comments. Comment at: test/Sema/gnu89.c:1-2 -// RUN: %clang_cc1 %s -std=gnu89 -pedantic -fsyntax-only -verify +// RUN: %clang_cc1 %s -std=gnu89 -pedantic -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-PEDANTIC %s +// RUN: %clang_cc1 %s -std=gnu89 -fsyntax-only 2>&1 | FileCheck %s This ideally needs positive tests. E.g.: * `-std=c89` * `-std=c89 -pedantic` * `-std=gnu99` * `-std=gnu99 -pedantic` * `-std=c99` * `-std=c99 -pedantic` Comment at: test/Sema/gnu89.c:16 +// CHECK-PEDANTIC: warning: duplicate 'const' declaration specifier +// CHECK-otherwise-NOT: warning: duplicate 'const' declaration specifier + There is no `otherwise` check prefix in the run lines. Repository: rC Clang https://reviews.llvm.org/D52248 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52193: RFC: [clang] Multithreaded compilation support
aganea marked an inline comment as done.
aganea added a comment.
It seems Reid's change has done good: `cmake` is not as significant as before
in the build process. See below the improved timings:
The tests consist in a full cleanup (delete build folder), cmake to regenerate,
then a full rebuild of LLVM + Clang + LLD (at r342552), Release target,
optimized tablegen.
VS2017 15.8.3, Ninja 1.8.2, CMake 3.12.2
For the `clang-cl` tests, I'm not using any official LLVM release, only the
binaries I built myself. `lld-link` is used in that case.
//(built with MSVC)// means the LLVM toolchain used to perfom this test was
compiled in a previous run with MSVC cl 15.8.3
//(built with Clang)// means the LLVM toolchain used to perform this test was
compiled in a previous run with Clang at r342552
I took the best figures from several runs (ran in a random order).
---
**Config 1 :** Intel Xeon Haswell 6 cores / 12 HW threads, 3.5 GHz, 15M cache,
128 GB RAM, SSD 550 MB/s
__MSBuild :__
| MSVC cl /MP | (50min 26sec) | 2 parallel msbuild
|
| MSVC cl /MP | (40min 23sec) | 8 parallel msbuild
|
| MSVC cl /MP | (40min 5sec) | 16 parallel msbuild
|
| clang-cl //(built with MSVC)// | (43min 36sec) | 16 parallel msbuild
|
| clang-cl //(built with Clang//) | (43min 42sec) | 16 parallel msbuild
|
| clang-cl **/MP** //(built with MSVC)// | not tested|
|
| clang-cl **/MP** //(built with Clang)// | (36min 13sec) | 8 parallel msbuild
|
| clang-cl **/MP** //(built with Clang)// | (34min 57sec) | 16 parallel msbuild
|
|
__Ninja:__
| MSVC cl | (33min 29sec) |
| clang-cl //(built with MSVC)// | (30min 2sec) |
| clang-cl //(built with Clang)// | (28min 29sec) |
|
-
**Config 2 :** Intel Xeon Skylake 18 cores / 36 HW threads, x2 (Dual CPU), 72
HW threads total, 2.3 GHz, 24.75M cache, 128 GB RAM, NVMe 4.6 GB/s
__MSBuild :__
| MSVC cl /MP | (10min 3sec) | 32 parallel msbuild
|
| clang-cl //(built with MSVC)// | (24min 15sec) | 32 parallel msbuild
|
| clang-cl //(built with Clang)// | (21min 21sec) | 32 parallel msbuild
|
| clang-cl **/MP** //(built with MSVC)// | (7min 52sec) | 32 parallel msbuild
|
| clang-cl **/MP** //(built with Clang)// | (7min 30sec) | 32 parallel msbuild
|
|
__Ninja:__
| MSVC cl | (7min 25sec) |
| clang-cl //(built with MSVC)// | (8min 23sec) |
| clang-cl //(built with Clang)// | (8min) |
|
Comment at: llvm/trunk/lib/Support/Windows/Program.inc:424
-ProcessInfo sys::Wait(const ProcessInfo &PI, unsigned SecondsToWait,
- bool WaitUntilChildTerminates, std::string *ErrMsg) {
- assert(PI.Pid && "invalid pid to wait on, process not started?");
- assert((PI.Process && PI.Process != INVALID_HANDLE_VALUE) &&
- "invalid process handle to wait on, process not started?");
+bool sys::WaitMany(MutableArrayRef PIArray, bool WaitOnAll,
+ unsigned SecondsToWait, bool WaitUntilProcessTerminates) {
rnk wrote:
> I guess no Posix implementation? It's kind of hard to know if we made the
> right abstractions without doing it for Windows and *nix.
Yes, I currenly don't have an Unix box at hand. But I will implement it shortly
as it needs to be atomically commited with the Windows part.
Repository:
rC Clang
https://reviews.llvm.org/D52193
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52248: [SEMA] ignore duplicate declaration specifiers from typeof exprs
nickdesaulniers updated this revision to Diff 166355.
nickdesaulniers added a comment.
- move test to new file, use check-prefix for both cases
Repository:
rC Clang
https://reviews.llvm.org/D52248
Files:
lib/Sema/SemaType.cpp
test/Sema/gnu89-const.c
Index: test/Sema/gnu89-const.c
===
--- /dev/null
+++ test/Sema/gnu89-const.c
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 %s -std=gnu89 -pedantic -fsyntax-only 2>&1 | FileCheck
-check-prefix=CHECK-PEDANTIC %s
+// RUN: %clang_cc1 %s -std=gnu89 -fsyntax-only 2>&1 | FileCheck
-check-prefix=CHECK-ABSTRUSE %s
+
+// Do not warn about duplicate const declaration specifier as the result of
+// typeof in gnu89.
+const int c_i;
+const typeof(c_i) c_i2;
+// CHECK-PEDANTIC: warning: extension used
+// CHECK-PEDANTIC: warning: duplicate 'const' declaration specifier
+// CHECK-ABSTRUSE-NOT: warning: duplicate 'const' declaration specifier
+
+const const int c_i3;
+// CHECK: warning: duplicate 'const' declaration specifier
+
+typedef const int t;
+const t c_i4;
+// CHECK: warning: duplicate 'const' declaration specifier
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -1679,8 +1679,13 @@
// C90 6.5.3 constraints: "The same type qualifier shall not appear more
// than once in the same specifier-list or qualifier-list, either directly
// or via one or more typedefs."
-if (!S.getLangOpts().C99 && !S.getLangOpts().CPlusPlus
-&& TypeQuals & Result.getCVRQualifiers()) {
+//
+// Not checked for gnu89 if the TST is from a typeof expression and
+// -pedantic was not set.
+if (!S.getLangOpts().C99 && !S.getLangOpts().CPlusPlus &&
+TypeQuals & Result.getCVRQualifiers() &&
+!(S.getLangOpts().GNUMode && !S.Diags.getDiagnosticOptions().Pedantic
&&
+ DS.getTypeSpecType() == DeclSpec::TST_typeofExpr)) {
if (TypeQuals & DeclSpec::TQ_const && Result.isConstQualified()) {
S.Diag(DS.getConstSpecLoc(), diag::ext_duplicate_declspec)
<< "const";
Index: test/Sema/gnu89-const.c
===
--- /dev/null
+++ test/Sema/gnu89-const.c
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 %s -std=gnu89 -pedantic -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-PEDANTIC %s
+// RUN: %clang_cc1 %s -std=gnu89 -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-ABSTRUSE %s
+
+// Do not warn about duplicate const declaration specifier as the result of
+// typeof in gnu89.
+const int c_i;
+const typeof(c_i) c_i2;
+// CHECK-PEDANTIC: warning: extension used
+// CHECK-PEDANTIC: warning: duplicate 'const' declaration specifier
+// CHECK-ABSTRUSE-NOT: warning: duplicate 'const' declaration specifier
+
+const const int c_i3;
+// CHECK: warning: duplicate 'const' declaration specifier
+
+typedef const int t;
+const t c_i4;
+// CHECK: warning: duplicate 'const' declaration specifier
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -1679,8 +1679,13 @@
// C90 6.5.3 constraints: "The same type qualifier shall not appear more
// than once in the same specifier-list or qualifier-list, either directly
// or via one or more typedefs."
-if (!S.getLangOpts().C99 && !S.getLangOpts().CPlusPlus
-&& TypeQuals & Result.getCVRQualifiers()) {
+//
+// Not checked for gnu89 if the TST is from a typeof expression and
+// -pedantic was not set.
+if (!S.getLangOpts().C99 && !S.getLangOpts().CPlusPlus &&
+TypeQuals & Result.getCVRQualifiers() &&
+!(S.getLangOpts().GNUMode && !S.Diags.getDiagnosticOptions().Pedantic &&
+ DS.getTypeSpecType() == DeclSpec::TST_typeofExpr)) {
if (TypeQuals & DeclSpec::TQ_const && Result.isConstQualified()) {
S.Diag(DS.getConstSpecLoc(), diag::ext_duplicate_declspec)
<< "const";
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52268: [AST] Squeeze some bits in LinkageComputer
rjmccall added a comment. `LinkageComputer` isn't actually persisted anywhere, right? And there's maybe one computer active at once? So this compression is theoretically saving one pointer of stack space but forcing a bunch of bit-manipulation every time these fields are accessed. Repository: rC Clang https://reviews.llvm.org/D52268 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50539: [VFS] Add property 'fallthrough' that controls fallback to real file system.
bruno added a comment.
Hi Volodymyr,
Thanks for working on this, really nice work with the tests! Comments below.
> - No support for 'fallthrough' in crash reproducer.
That'd be nice to have at some point to make sure we never escape into the real
fs.
> - Current way of working with modules in VFS "root" is clunky and error-prone.
Why?
> Also I don't like that VFSFromYamlDirIterImpl is similar to overlay iterator
> but doesn't share code. At the moment I think it's not worth it to rework
> those abstractions to make more code reusable. If you've noticed problems
> with those iterators before, maybe it makes sense to try to find a better
> approach.
Maybe add FIXMEs for it?
Comment at: clang/lib/Basic/VirtualFileSystem.cpp:934
RedirectingFileSystem &FS;
RedirectingDirectoryEntry::iterator Current, End;
+ bool IsExternalFSCurrent;
Can you add comments to these new members? Specifically, what's the purpose of
`IsExternalFSCurrent` and how it connects to `ExternalFS`
Comment at: clang/lib/Basic/VirtualFileSystem.cpp:940
- std::error_code incrementImpl();
+ std::error_code incrementExternal();
+ std::error_code incrementContent(bool IsFirstTime);
Same for these methods
Comment at: clang/lib/Basic/VirtualFileSystem.cpp:1738
+ErrorOr RedirectingFileSystem::status(const Twine &Path,
+ bool ShouldCheckExternalFS) {
ErrorOr Result = lookupPath(Path);
Is passing `ShouldCheckExternalFS ` really necessary? Why checking the status
of the member isn't enough?
Comment at: clang/lib/Basic/VirtualFileSystem.cpp:2041
+ IsExternalFSCurrent(false), ExternalFS(ExternalFS) {
+ EC = incrementImpl(true);
}
`incrementImpl(true/*IsFirstTime*/)`
Comment at: clang/lib/Basic/VirtualFileSystem.cpp:2045
std::error_code VFSFromYamlDirIterImpl::increment() {
- assert(Current != End && "cannot iterate past end");
- ++Current;
- return incrementImpl();
+ return incrementImpl(false);
+}
`incrementImpl(false/*IsFirstTime*/)`
https://reviews.llvm.org/D50539
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52248: [SEMA] ignore duplicate declaration specifiers from typeof exprs
nickdesaulniers updated this revision to Diff 166357.
nickdesaulniers added a comment.
- add line numbers to match specific warning lines
Repository:
rC Clang
https://reviews.llvm.org/D52248
Files:
lib/Sema/SemaType.cpp
test/Sema/gnu89-const.c
Index: test/Sema/gnu89-const.c
===
--- /dev/null
+++ test/Sema/gnu89-const.c
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 %s -std=gnu89 -pedantic -fsyntax-only 2>&1 | FileCheck
-check-prefix=CHECK-PEDANTIC %s
+// RUN: %clang_cc1 %s -std=gnu89 -fsyntax-only 2>&1 | FileCheck
-check-prefix=CHECK-ABSTRUSE %s
+
+// Do not warn about duplicate const declaration specifier as the result of
+// typeof in gnu89.
+const int c_i;
+const typeof(c_i) c_i2;
+// CHECK-PEDANTIC: 7:7: warning: extension used
+// CHECK-PEDANTIC: 7:1: warning: duplicate 'const' declaration specifier
+// CHECK-ABSTRUSE-NOT: 7:1: warning: duplicate 'const' declaration specifier
+
+const const int c_i3;
+// CHECK: 12:7: warning: duplicate 'const' declaration specifier
+
+typedef const int t;
+const t c_i4;
+// CHECK: 16:1: warning: duplicate 'const' declaration specifier
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -1679,8 +1679,13 @@
// C90 6.5.3 constraints: "The same type qualifier shall not appear more
// than once in the same specifier-list or qualifier-list, either directly
// or via one or more typedefs."
-if (!S.getLangOpts().C99 && !S.getLangOpts().CPlusPlus
-&& TypeQuals & Result.getCVRQualifiers()) {
+//
+// Not checked for gnu89 if the TST is from a typeof expression and
+// -pedantic was not set.
+if (!S.getLangOpts().C99 && !S.getLangOpts().CPlusPlus &&
+TypeQuals & Result.getCVRQualifiers() &&
+!(S.getLangOpts().GNUMode && !S.Diags.getDiagnosticOptions().Pedantic
&&
+ DS.getTypeSpecType() == DeclSpec::TST_typeofExpr)) {
if (TypeQuals & DeclSpec::TQ_const && Result.isConstQualified()) {
S.Diag(DS.getConstSpecLoc(), diag::ext_duplicate_declspec)
<< "const";
Index: test/Sema/gnu89-const.c
===
--- /dev/null
+++ test/Sema/gnu89-const.c
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 %s -std=gnu89 -pedantic -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-PEDANTIC %s
+// RUN: %clang_cc1 %s -std=gnu89 -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-ABSTRUSE %s
+
+// Do not warn about duplicate const declaration specifier as the result of
+// typeof in gnu89.
+const int c_i;
+const typeof(c_i) c_i2;
+// CHECK-PEDANTIC: 7:7: warning: extension used
+// CHECK-PEDANTIC: 7:1: warning: duplicate 'const' declaration specifier
+// CHECK-ABSTRUSE-NOT: 7:1: warning: duplicate 'const' declaration specifier
+
+const const int c_i3;
+// CHECK: 12:7: warning: duplicate 'const' declaration specifier
+
+typedef const int t;
+const t c_i4;
+// CHECK: 16:1: warning: duplicate 'const' declaration specifier
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -1679,8 +1679,13 @@
// C90 6.5.3 constraints: "The same type qualifier shall not appear more
// than once in the same specifier-list or qualifier-list, either directly
// or via one or more typedefs."
-if (!S.getLangOpts().C99 && !S.getLangOpts().CPlusPlus
-&& TypeQuals & Result.getCVRQualifiers()) {
+//
+// Not checked for gnu89 if the TST is from a typeof expression and
+// -pedantic was not set.
+if (!S.getLangOpts().C99 && !S.getLangOpts().CPlusPlus &&
+TypeQuals & Result.getCVRQualifiers() &&
+!(S.getLangOpts().GNUMode && !S.Diags.getDiagnosticOptions().Pedantic &&
+ DS.getTypeSpecType() == DeclSpec::TST_typeofExpr)) {
if (TypeQuals & DeclSpec::TQ_const && Result.isConstQualified()) {
S.Diag(DS.getConstSpecLoc(), diag::ext_duplicate_declspec)
<< "const";
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52193: RFC: [clang] Multithreaded compilation support
zturner added a comment. The process stuff looks cool and useful independently of `/MP`. Would it be possible to break that into a separate patch, and add a unit test for the behavior of the `WaitAll` flag? Repository: rC Clang https://reviews.llvm.org/D52193 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50214: Add inherited attributes before parsed attributes.
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Repository: rC Clang https://reviews.llvm.org/D50214 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52248: [SEMA] ignore duplicate declaration specifiers from typeof exprs
srhines added inline comments.
Comment at: lib/Sema/SemaType.cpp:1682
// or via one or more typedefs."
-if (!S.getLangOpts().C99 && !S.getLangOpts().CPlusPlus
-&& TypeQuals & Result.getCVRQualifiers()) {
This is broken for C11 and C17 (even before you touch anything). As we just
talked about, let's have a helper function to detect the oldest version (and
maybe even that should get promoted as a LANGOPT).
Repository:
rC Clang
https://reviews.llvm.org/D52248
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51866: [analyzer][UninitializedObjectChecker][WIP] New flag to ignore guarded uninitialized fields
NoQ added a comment. You can retrieve any node from the `ASTMatcher` by `.bind()`ing sub-matchers to string keys and later retrieving them from the `MatchResult` dictionary by those keys. It's like a regex capturing groups (and, well, `ASTMatchers` also support backreferences to such groups, i.e. `equalsBoundNode()`). Repository: rC Clang https://reviews.llvm.org/D51866 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52136: [clang-tidy] Add modernize-concat-nested-namespaces check
aaron.ballman added inline comments.
Comment at: clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp:52
+const NamespaceContextVec &Namespaces) {
+ std::ostringstream Result;
+ bool First = true;
Can this be rewritten with `llvm::for_each()` and a `Twine` so that we don't
have to use `ostringstream` (which is a big hammer for this).
Comment at: clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp:73
+ diag(Namespaces.front()->getBeginLoc(),
+ "Nested namespaces can be concatenated", DiagnosticIDs::Warning)
+ << FixItHint::CreateReplacement(FrontReplacement,
Diagnostics should not start with a capital letter.
Comment at: clang-tidy/modernize/ConcatNestedNamespacesCheck.h:29
+private:
+ using NamespaceContextVec = llvm::SmallVector;
+
Why 6?
Comment at: docs/clang-tidy/checks/list.rst:13
abseil-str-cat-append
+ abseil-string-find-startswith
android-cloexec-accept
This is an unrelated change -- feel free to make a separate commit that fixes
this (no review needed).
https://reviews.llvm.org/D52136
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52193: RFC: [clang] Multithreaded compilation support
aganea added a comment. In https://reviews.llvm.org/D52193#1241056, @zturner wrote: > The process stuff looks cool and useful independently of `/MP`. Would it be > possible to break that into a separate patch, and add a unit test for the > behavior of the `WaitAll` flag? Of course, will do. Repository: rC Clang https://reviews.llvm.org/D52193 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52323: Add necessary support for storing code-model to module IR.
cmtice created this revision.
cmtice added reviewers: tejohnson, pcc.
Herald added subscribers: cfe-commits, dexonsmith, mehdi_amini.
Currently the code-model does not get saved in the module IR, so if a code
model is specified when compiling with LTO, it gets lost and is not propagated
properly to LTO. This patch does what is necessary in the front end to pass the
code-model to the module, so that the back end can store it in the Module (see
https://reviews.llvm.org/D52322 for the back-end patch).
Fixes PR33306.
Repository:
rC Clang
https://reviews.llvm.org/D52323
Files:
lib/CodeGen/CodeGenModule.cpp
lib/CodeGen/CodeGenModule.h
test/CodeGen/codemodels.c
Index: test/CodeGen/codemodels.c
===
--- test/CodeGen/codemodels.c
+++ test/CodeGen/codemodels.c
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -emit-llvm %s -o - | FileCheck %s
-check-prefix=CHECK-NOMODEL
+// RUN: %clang_cc1 -emit-llvm -mcode-model small %s -o - | FileCheck %s
-check-prefix=CHECK-SMALL
+// RUN: %clang_cc1 -emit-llvm -mcode-model kernel %s -o - | FileCheck %s
-check-prefix=CHECK-KERNEL
+// RUN: %clang_cc1 -emit-llvm -mcode-model medium %s -o - | FileCheck %s
-check-prefix=CHECK-MEDIUM
+// RUN: %clang_cc1 -emit-llvm -mcode-model large %s -o - | FileCheck %s
-check-prefix=CHECK-LARGE
+
+// CHECK-SMALL: !llvm.module.flags = !{{{.*}}}
+// CHECK-SMALL: !{{[0-9]+}} = !{i32 1, !"Code Model", i32 1}
+// CHECK-KERNEL: !llvm.module.flags = !{{{.*}}}
+// CHECK-KERNEL: !{{[0-9]+}} = !{i32 1, !"Code Model", i32 2}
+// CHECK-MEDIUM: !llvm.module.flags = !{{{.*}}}
+// CHECK-MEDIUM: !{{[0-9]+}} = !{i32 1, !"Code Model", i32 3}
+// CHECK-LARGE: !llvm.module.flags = !{{{.*}}}
+// CHECK-LARGE: !{{[0-9]+}} = !{i32 1, !"Code Model", i32 4}
+// CHECK-NOMODEL-NOT: Code Model
Index: lib/CodeGen/CodeGenModule.h
===
--- lib/CodeGen/CodeGenModule.h
+++ lib/CodeGen/CodeGenModule.h
@@ -35,6 +35,7 @@
#include "llvm/ADT/StringMap.h"
#include "llvm/IR/Module.h"
#include "llvm/IR/ValueHandle.h"
+#include "llvm/Support/CodeGen.h"
#include "llvm/Transforms/Utils/SanitizerStats.h"
namespace llvm {
Index: lib/CodeGen/CodeGenModule.cpp
===
--- lib/CodeGen/CodeGenModule.cpp
+++ lib/CodeGen/CodeGenModule.cpp
@@ -44,6 +44,7 @@
#include "clang/CodeGen/ConstantInitBuilder.h"
#include "clang/Frontend/CodeGenOptions.h"
#include "clang/Sema/SemaDiagnostic.h"
+#include "llvm/ADT/StringSwitch.h"
#include "llvm/ADT/Triple.h"
#include "llvm/Analysis/TargetLibraryInfo.h"
#include "llvm/IR/CallSite.h"
@@ -556,6 +557,21 @@
getModule().setPIELevel(static_cast(PLevel));
}
+ if (getCodeGenOpts().CodeModel.size() > 0) {
+unsigned CM = llvm::StringSwitch(getCodeGenOpts().CodeModel)
+ .Case("tiny", llvm::CodeModel::Tiny)
+ .Case("small", llvm::CodeModel::Small)
+ .Case("kernel", llvm::CodeModel::Kernel)
+ .Case("medium", llvm::CodeModel::Medium)
+ .Case("large", llvm::CodeModel::Large)
+ .Case("default", ~1u)
+ .Default(~0u);
+if ((CM != ~0u) && (CM != ~1u)) {
+ llvm::CodeModel::Model codeModel =
static_cast(CM);
+ getModule().setCodeModel(codeModel);
+}
+ }
+
if (CodeGenOpts.NoPLT)
getModule().setRtLibUseGOT();
Index: test/CodeGen/codemodels.c
===
--- test/CodeGen/codemodels.c
+++ test/CodeGen/codemodels.c
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -emit-llvm %s -o - | FileCheck %s -check-prefix=CHECK-NOMODEL
+// RUN: %clang_cc1 -emit-llvm -mcode-model small %s -o - | FileCheck %s -check-prefix=CHECK-SMALL
+// RUN: %clang_cc1 -emit-llvm -mcode-model kernel %s -o - | FileCheck %s -check-prefix=CHECK-KERNEL
+// RUN: %clang_cc1 -emit-llvm -mcode-model medium %s -o - | FileCheck %s -check-prefix=CHECK-MEDIUM
+// RUN: %clang_cc1 -emit-llvm -mcode-model large %s -o - | FileCheck %s -check-prefix=CHECK-LARGE
+
+// CHECK-SMALL: !llvm.module.flags = !{{{.*}}}
+// CHECK-SMALL: !{{[0-9]+}} = !{i32 1, !"Code Model", i32 1}
+// CHECK-KERNEL: !llvm.module.flags = !{{{.*}}}
+// CHECK-KERNEL: !{{[0-9]+}} = !{i32 1, !"Code Model", i32 2}
+// CHECK-MEDIUM: !llvm.module.flags = !{{{.*}}}
+// CHECK-MEDIUM: !{{[0-9]+}} = !{i32 1, !"Code Model", i32 3}
+// CHECK-LARGE: !llvm.module.flags = !{{{.*}}}
+// CHECK-LARGE: !{{[0-9]+}} = !{i32 1, !"Code Model", i32 4}
+// CHECK-NOMODEL-NOT: Code Model
Index: lib/CodeGen/CodeGenModule.h
===
--- lib/CodeGen/CodeGenModule.h
+++ lib/CodeGen/CodeGenModule.h
@@ -35,6 +35,7 @@
#include "llvm/ADT/StringMap.h"
#include "llvm/IR/Module.h"
#include "llvm/IR/ValueHandle.h"
+#include "llvm/Support/CodeGen.h"
#include "llvm/Transforms/Utils/SanitizerStats.h"
namespace llvm {
Index
RE: r342668 - Add testcases for r342667.
Hi Eric,
The test that you added in this commit is failing on the PS4 Windows bot. Can
you please take a look?
http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast/builds/20052
FAIL: Clang :: Preprocessor/include-leading-nonalpha-suggest.c (10765 of 43992)
TEST 'Clang ::
Preprocessor/include-leading-nonalpha-suggest.c' FAILED
Script:
--
: 'RUN: at line 1';
c:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.obj\bin\clang.EXE
-cc1 -internal-isystem
c:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.obj\lib\clang\8.0.0\include
-nostdsysteminc
C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\tools\clang\test\Preprocessor\include-leading-nonalpha-suggest.c
-verify
--
Exit Code: 1
Command Output (stdout):
--
$ ":" "RUN: at line 1"
$
"c:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.obj\bin\clang.EXE"
"-cc1" "-internal-isystem"
"c:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.obj\lib\clang\8.0.0\include"
"-nostdsysteminc"
"C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\tools\clang\test\Preprocessor\include-leading-nonalpha-suggest.c"
"-verify"
# command stderr:
error: 'error' diagnostics expected but not seen:
File
C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\tools\clang\test\Preprocessor\include-leading-nonalpha-suggest.c
Line 3: '/empty_file_to_include.h' file not found, did you mean
'empty_file_to_include.h'?
1 error generated.
error: command failed with exit status: 1
Douglas Yung
> -Original Message-
> From: cfe-commits [mailto:cfe-commits-boun...@lists.llvm.org] On Behalf
> Of Eric Christopher via cfe-commits
> Sent: Thursday, September 20, 2018 10:23
> To: cfe-commits@lists.llvm.org
> Subject: r342668 - Add testcases for r342667.
>
> Author: echristo
> Date: Thu Sep 20 10:22:43 2018
> New Revision: 342668
>
> URL: http://llvm.org/viewvc/llvm-project?rev=342668&view=rev
> Log:
> Add testcases for r342667.
>
> Added:
> cfe/trunk/test/Preprocessor/include-leading-nonalpha-no-suggest.c
> cfe/trunk/test/Preprocessor/include-leading-nonalpha-suggest.c
>
> Added: cfe/trunk/test/Preprocessor/include-leading-nonalpha-no-
> suggest.c
> URL: http://llvm.org/viewvc/llvm-
> project/cfe/trunk/test/Preprocessor/include-leading-nonalpha-no-
> suggest.c?rev=342668&view=auto
> ===
> ===
> --- cfe/trunk/test/Preprocessor/include-leading-nonalpha-no-suggest.c
> (added)
> +++ cfe/trunk/test/Preprocessor/include-leading-nonalpha-no-suggest.c
> Thu Sep 20 10:22:43 2018
> @@ -0,0 +1,3 @@
> +// RUN: %clang_cc1 %s -verify
> +
> +#include "/non_existing_file_to_include.h" // expected-error
> {{'/non_existing_file_to_include.h' file not found}}
>
> Added: cfe/trunk/test/Preprocessor/include-leading-nonalpha-suggest.c
> URL: http://llvm.org/viewvc/llvm-
> project/cfe/trunk/test/Preprocessor/include-leading-nonalpha-
> suggest.c?rev=342668&view=auto
> ===
> ===
> --- cfe/trunk/test/Preprocessor/include-leading-nonalpha-suggest.c
> (added)
> +++ cfe/trunk/test/Preprocessor/include-leading-nonalpha-suggest.c Thu
> Sep 20 10:22:43 2018
> @@ -0,0 +1,3 @@
> +// RUN: %clang_cc1 %s -verify
> +
> +#include "/empty_file_to_include.h" // expected-error
> {{'/empty_file_to_include.h' file not found, did you mean
> 'empty_file_to_include.h'?}}
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r342668 - Add testcases for r342667.
Adding Jorge...
On Thu, Sep 20, 2018 at 2:36 PM wrote:
> Hi Eric,
>
> The test that you added in this commit is failing on the PS4 Windows bot.
> Can you please take a look?
>
>
> http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast/builds/20052
>
> FAIL: Clang :: Preprocessor/include-leading-nonalpha-suggest.c (10765 of
> 43992)
> TEST 'Clang ::
> Preprocessor/include-leading-nonalpha-suggest.c' FAILED
> Script:
> --
> : 'RUN: at line 1';
>
> c:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.obj\bin\clang.EXE
> -cc1 -internal-isystem
> c:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.obj\lib\clang\8.0.0\include
> -nostdsysteminc
> C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\tools\clang\test\Preprocessor\include-leading-nonalpha-suggest.c
> -verify
> --
> Exit Code: 1
>
> Command Output (stdout):
> --
> $ ":" "RUN: at line 1"
> $
> "c:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.obj\bin\clang.EXE"
> "-cc1" "-internal-isystem"
> "c:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.obj\lib\clang\8.0.0\include"
> "-nostdsysteminc"
> "C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\tools\clang\test\Preprocessor\include-leading-nonalpha-suggest.c"
> "-verify"
> # command stderr:
> error: 'error' diagnostics expected but not seen:
>
> File
> C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\tools\clang\test\Preprocessor\include-leading-nonalpha-suggest.c
> Line 3: '/empty_file_to_include.h' file not found, did you mean
> 'empty_file_to_include.h'?
>
> 1 error generated.
>
>
> error: command failed with exit status: 1
>
>
Oof. Thanks. If I don't have something in 10 minutes I'll just revert.
Thanks!
-eric
> Douglas Yung
>
> > -Original Message-
> > From: cfe-commits [mailto:cfe-commits-boun...@lists.llvm.org] On Behalf
> > Of Eric Christopher via cfe-commits
> > Sent: Thursday, September 20, 2018 10:23
> > To: cfe-commits@lists.llvm.org
> > Subject: r342668 - Add testcases for r342667.
> >
> > Author: echristo
> > Date: Thu Sep 20 10:22:43 2018
> > New Revision: 342668
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=342668&view=rev
> > Log:
> > Add testcases for r342667.
> >
> > Added:
> > cfe/trunk/test/Preprocessor/include-leading-nonalpha-no-suggest.c
> > cfe/trunk/test/Preprocessor/include-leading-nonalpha-suggest.c
> >
> > Added: cfe/trunk/test/Preprocessor/include-leading-nonalpha-no-
> > suggest.c
> > URL: http://llvm.org/viewvc/llvm-
> > project/cfe/trunk/test/Preprocessor/include-leading-nonalpha-no-
> > suggest.c?rev=342668&view=auto
> > ===
> > ===
> > --- cfe/trunk/test/Preprocessor/include-leading-nonalpha-no-suggest.c
> > (added)
> > +++ cfe/trunk/test/Preprocessor/include-leading-nonalpha-no-suggest.c
> > Thu Sep 20 10:22:43 2018
> > @@ -0,0 +1,3 @@
> > +// RUN: %clang_cc1 %s -verify
> > +
> > +#include "/non_existing_file_to_include.h" // expected-error
> > {{'/non_existing_file_to_include.h' file not found}}
> >
> > Added: cfe/trunk/test/Preprocessor/include-leading-nonalpha-suggest.c
> > URL: http://llvm.org/viewvc/llvm-
> > project/cfe/trunk/test/Preprocessor/include-leading-nonalpha-
> > suggest.c?rev=342668&view=auto
> > ===
> > ===
> > --- cfe/trunk/test/Preprocessor/include-leading-nonalpha-suggest.c
> > (added)
> > +++ cfe/trunk/test/Preprocessor/include-leading-nonalpha-suggest.c Thu
> > Sep 20 10:22:43 2018
> > @@ -0,0 +1,3 @@
> > +// RUN: %clang_cc1 %s -verify
> > +
> > +#include "/empty_file_to_include.h" // expected-error
> > {{'/empty_file_to_include.h' file not found, did you mean
> > 'empty_file_to_include.h'?}}
> >
> >
> > ___
> > cfe-commits mailing list
> > cfe-commits@lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r342668 - Add testcases for r342667.
FWIW we're trying to reproduce here real fast and then will revert or fix
real fast.
Thanks!
-eric
On Thu, Sep 20, 2018 at 2:46 PM Eric Christopher wrote:
> Adding Jorge...
>
> On Thu, Sep 20, 2018 at 2:36 PM wrote:
>
>> Hi Eric,
>>
>> The test that you added in this commit is failing on the PS4 Windows bot.
>> Can you please take a look?
>>
>>
>> http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast/builds/20052
>>
>> FAIL: Clang :: Preprocessor/include-leading-nonalpha-suggest.c (10765 of
>> 43992)
>> TEST 'Clang ::
>> Preprocessor/include-leading-nonalpha-suggest.c' FAILED
>> Script:
>> --
>> : 'RUN: at line 1';
>>
>> c:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.obj\bin\clang.EXE
>> -cc1 -internal-isystem
>> c:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.obj\lib\clang\8.0.0\include
>> -nostdsysteminc
>> C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\tools\clang\test\Preprocessor\include-leading-nonalpha-suggest.c
>> -verify
>> --
>> Exit Code: 1
>>
>> Command Output (stdout):
>> --
>> $ ":" "RUN: at line 1"
>> $
>> "c:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.obj\bin\clang.EXE"
>> "-cc1" "-internal-isystem"
>> "c:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.obj\lib\clang\8.0.0\include"
>> "-nostdsysteminc"
>> "C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\tools\clang\test\Preprocessor\include-leading-nonalpha-suggest.c"
>> "-verify"
>> # command stderr:
>> error: 'error' diagnostics expected but not seen:
>>
>> File
>> C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\tools\clang\test\Preprocessor\include-leading-nonalpha-suggest.c
>> Line 3: '/empty_file_to_include.h' file not found, did you mean
>> 'empty_file_to_include.h'?
>>
>> 1 error generated.
>>
>>
>> error: command failed with exit status: 1
>>
>>
> Oof. Thanks. If I don't have something in 10 minutes I'll just revert.
>
> Thanks!
>
> -eric
>
>
>
>> Douglas Yung
>>
>> > -Original Message-
>> > From: cfe-commits [mailto:cfe-commits-boun...@lists.llvm.org] On Behalf
>> > Of Eric Christopher via cfe-commits
>> > Sent: Thursday, September 20, 2018 10:23
>> > To: cfe-commits@lists.llvm.org
>> > Subject: r342668 - Add testcases for r342667.
>> >
>> > Author: echristo
>> > Date: Thu Sep 20 10:22:43 2018
>> > New Revision: 342668
>> >
>> > URL: http://llvm.org/viewvc/llvm-project?rev=342668&view=rev
>> > Log:
>> > Add testcases for r342667.
>> >
>> > Added:
>> > cfe/trunk/test/Preprocessor/include-leading-nonalpha-no-suggest.c
>> > cfe/trunk/test/Preprocessor/include-leading-nonalpha-suggest.c
>> >
>> > Added: cfe/trunk/test/Preprocessor/include-leading-nonalpha-no-
>> > suggest.c
>> > URL: http://llvm.org/viewvc/llvm-
>> > project/cfe/trunk/test/Preprocessor/include-leading-nonalpha-no-
>> > suggest.c?rev=342668&view=auto
>> > ===
>> > ===
>> > --- cfe/trunk/test/Preprocessor/include-leading-nonalpha-no-suggest.c
>> > (added)
>> > +++ cfe/trunk/test/Preprocessor/include-leading-nonalpha-no-suggest.c
>> > Thu Sep 20 10:22:43 2018
>> > @@ -0,0 +1,3 @@
>> > +// RUN: %clang_cc1 %s -verify
>> > +
>> > +#include "/non_existing_file_to_include.h" // expected-error
>> > {{'/non_existing_file_to_include.h' file not found}}
>> >
>> > Added: cfe/trunk/test/Preprocessor/include-leading-nonalpha-suggest.c
>> > URL: http://llvm.org/viewvc/llvm-
>> > project/cfe/trunk/test/Preprocessor/include-leading-nonalpha-
>> > suggest.c?rev=342668&view=auto
>> > ===
>> > ===
>> > --- cfe/trunk/test/Preprocessor/include-leading-nonalpha-suggest.c
>> > (added)
>> > +++ cfe/trunk/test/Preprocessor/include-leading-nonalpha-suggest.c Thu
>> > Sep 20 10:22:43 2018
>> > @@ -0,0 +1,3 @@
>> > +// RUN: %clang_cc1 %s -verify
>> > +
>> > +#include "/empty_file_to_include.h" // expected-error
>> > {{'/empty_file_to_include.h' file not found, did you mean
>> > 'empty_file_to_include.h'?}}
>> >
>> >
>> > ___
>> > cfe-commits mailing list
>> > cfe-commits@lists.llvm.org
>> > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52248: [SEMA] ignore duplicate declaration specifiers from typeof exprs
nickdesaulniers added inline comments. Comment at: test/Sema/gnu89.c:1-2 -// RUN: %clang_cc1 %s -std=gnu89 -pedantic -fsyntax-only -verify +// RUN: %clang_cc1 %s -std=gnu89 -pedantic -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-PEDANTIC %s +// RUN: %clang_cc1 %s -std=gnu89 -fsyntax-only 2>&1 | FileCheck %s lebedev.ri wrote: > This ideally needs positive tests. E.g.: > * `-std=c89` > * `-std=c89 -pedantic` > * `-std=gnu99` > * `-std=gnu99 -pedantic` > * `-std=c99` > * `-std=c99 -pedantic` > Since `typeof` is a gnu extension, its use constitutes an error for all non gnu C standards, so it's moot to check for duplicate const specifiers from typeof exprs. Since we're trying to match GCC's behavior here, GCC does not warn for `-std=gnu99` or `-std=gnu99 -pedantic` so I will add those test cases. Repository: rC Clang https://reviews.llvm.org/D52248 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52248: [SEMA] ignore duplicate declaration specifiers from typeof exprs
nickdesaulniers added inline comments. Comment at: test/Sema/gnu89.c:1-2 -// RUN: %clang_cc1 %s -std=gnu89 -pedantic -fsyntax-only -verify +// RUN: %clang_cc1 %s -std=gnu89 -pedantic -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-PEDANTIC %s +// RUN: %clang_cc1 %s -std=gnu89 -fsyntax-only 2>&1 | FileCheck %s nickdesaulniers wrote: > lebedev.ri wrote: > > This ideally needs positive tests. E.g.: > > * `-std=c89` > > * `-std=c89 -pedantic` > > * `-std=gnu99` > > * `-std=gnu99 -pedantic` > > * `-std=c99` > > * `-std=c99 -pedantic` > > > Since `typeof` is a gnu extension, its use constitutes an error for all non > gnu C standards, so it's moot to check for duplicate const specifiers from > typeof exprs. > > Since we're trying to match GCC's behavior here, GCC does not warn for > `-std=gnu99` or `-std=gnu99 -pedantic` so I will add those test cases. https://godbolt.org/z/3trZdl Repository: rC Clang https://reviews.llvm.org/D52248 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r342668 - Add testcases for r342667.
Zach and I were able to find the cause.
Clang on Windows manages to find "file.h" when you #include "/file.h" and
that makes the expected diagnostic not appear. MSVC inteprets an #include
with a leading slash as an absolute path so I think we have accidentally
hit a different bug in Clang :)
One option to fix the test would be replacing the slash with another random
non-alphanumeric character that can't be interpreted as a directory
separator, but at that point I think we can just delete the failing test
and rely on the existing include-likely-typo.c that tests with both leading
and trailing non-alphanumeric characters.
The other test in r342668 works because it includes a file that doesn't
exist even if you interpret the path as relative so it should be OK to keep
while the bug is found.
I'll go find a bug about the behavior on windows. Thanks!
Jorge
On Thu, Sep 20, 2018 at 2:51 PM Eric Christopher wrote:
> FWIW we're trying to reproduce here real fast and then will revert or fix
> real fast.
>
> Thanks!
>
> -eric
>
> On Thu, Sep 20, 2018 at 2:46 PM Eric Christopher
> wrote:
>
>> Adding Jorge...
>>
>> On Thu, Sep 20, 2018 at 2:36 PM wrote:
>>
>>> Hi Eric,
>>>
>>> The test that you added in this commit is failing on the PS4 Windows
>>> bot. Can you please take a look?
>>>
>>>
>>> http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast/builds/20052
>>>
>>> FAIL: Clang :: Preprocessor/include-leading-nonalpha-suggest.c (10765 of
>>> 43992)
>>> TEST 'Clang ::
>>> Preprocessor/include-leading-nonalpha-suggest.c' FAILED
>>> Script:
>>> --
>>> : 'RUN: at line 1';
>>>
>>> c:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.obj\bin\clang.EXE
>>> -cc1 -internal-isystem
>>> c:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.obj\lib\clang\8.0.0\include
>>> -nostdsysteminc
>>> C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\tools\clang\test\Preprocessor\include-leading-nonalpha-suggest.c
>>> -verify
>>> --
>>> Exit Code: 1
>>>
>>> Command Output (stdout):
>>> --
>>> $ ":" "RUN: at line 1"
>>> $
>>> "c:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.obj\bin\clang.EXE"
>>> "-cc1" "-internal-isystem"
>>> "c:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.obj\lib\clang\8.0.0\include"
>>> "-nostdsysteminc"
>>> "C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\tools\clang\test\Preprocessor\include-leading-nonalpha-suggest.c"
>>> "-verify"
>>> # command stderr:
>>> error: 'error' diagnostics expected but not seen:
>>>
>>> File
>>> C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\tools\clang\test\Preprocessor\include-leading-nonalpha-suggest.c
>>> Line 3: '/empty_file_to_include.h' file not found, did you mean
>>> 'empty_file_to_include.h'?
>>>
>>> 1 error generated.
>>>
>>>
>>> error: command failed with exit status: 1
>>>
>>>
>> Oof. Thanks. If I don't have something in 10 minutes I'll just revert.
>>
>> Thanks!
>>
>> -eric
>>
>>
>>
>>> Douglas Yung
>>>
>>> > -Original Message-
>>> > From: cfe-commits [mailto:cfe-commits-boun...@lists.llvm.org] On
>>> Behalf
>>> > Of Eric Christopher via cfe-commits
>>> > Sent: Thursday, September 20, 2018 10:23
>>> > To: cfe-commits@lists.llvm.org
>>> > Subject: r342668 - Add testcases for r342667.
>>> >
>>> > Author: echristo
>>> > Date: Thu Sep 20 10:22:43 2018
>>> > New Revision: 342668
>>> >
>>> > URL: http://llvm.org/viewvc/llvm-project?rev=342668&view=rev
>>> > Log:
>>> > Add testcases for r342667.
>>> >
>>> > Added:
>>> > cfe/trunk/test/Preprocessor/include-leading-nonalpha-no-suggest.c
>>> > cfe/trunk/test/Preprocessor/include-leading-nonalpha-suggest.c
>>> >
>>> > Added: cfe/trunk/test/Preprocessor/include-leading-nonalpha-no-
>>> > suggest.c
>>> > URL: http://llvm.org/viewvc/llvm-
>>> > project/cfe/trunk/test/Preprocessor/include-leading-nonalpha-no-
>>> > suggest.c?rev=342668&view=auto
>>> > ===
>>> > ===
>>> > --- cfe/trunk/test/Preprocessor/include-leading-nonalpha-no-suggest.c
>>> > (added)
>>> > +++ cfe/trunk/test/Preprocessor/include-leading-nonalpha-no-suggest.c
>>> > Thu Sep 20 10:22:43 2018
>>> > @@ -0,0 +1,3 @@
>>> > +// RUN: %clang_cc1 %s -verify
>>> > +
>>> > +#include "/non_existing_file_to_include.h" // expected-error
>>> > {{'/non_existing_file_to_include.h' file not found}}
>>> >
>>> > Added: cfe/trunk/test/Preprocessor/include-leading-nonalpha-suggest.c
>>> > URL: http://llvm.org/viewvc/llvm-
>>> > project/cfe/trunk/test/Preprocessor/include-leading-nonalpha-
>>> > suggest.c?rev=342668&view=auto
>>> > ===
>>> > ===
>>> > --- cfe/trunk/test/Preprocessor/include-leading-nonalpha-suggest.c
>>> > (added)
>>> > +++ cfe/trunk/test/Preprocessor/include-leading-
r342693 - Remove failing test.
Author: zturner
Date: Thu Sep 20 15:32:51 2018
New Revision: 342693
URL: http://llvm.org/viewvc/llvm-project?rev=342693&view=rev
Log:
Remove failing test.
Removing on behalf of Jorge Moya. This test is broken on
Windows due to it actually being able to resolve the path. There
is an actual Windows-specific bug somewhere, but we already have
sufficient test coverage of this with a different test, so removing
this was the approach suggested by Jorge.
Removed:
cfe/trunk/test/Preprocessor/include-leading-nonalpha-suggest.c
Removed: cfe/trunk/test/Preprocessor/include-leading-nonalpha-suggest.c
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Preprocessor/include-leading-nonalpha-suggest.c?rev=342692&view=auto
==
--- cfe/trunk/test/Preprocessor/include-leading-nonalpha-suggest.c (original)
+++ cfe/trunk/test/Preprocessor/include-leading-nonalpha-suggest.c (removed)
@@ -1,3 +0,0 @@
-// RUN: %clang_cc1 %s -verify
-
-#include "/empty_file_to_include.h" // expected-error
{{'/empty_file_to_include.h' file not found, did you mean
'empty_file_to_include.h'?}}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r342668 - Add testcases for r342667.
Test removed in r342693.
On Thu, Sep 20, 2018 at 3:30 PM Jorge Gorbe Moya wrote:
> Zach and I were able to find the cause.
>
> Clang on Windows manages to find "file.h" when you #include "/file.h" and
> that makes the expected diagnostic not appear. MSVC inteprets an #include
> with a leading slash as an absolute path so I think we have accidentally
> hit a different bug in Clang :)
>
> One option to fix the test would be replacing the slash with another
> random non-alphanumeric character that can't be interpreted as a directory
> separator, but at that point I think we can just delete the failing test
> and rely on the existing include-likely-typo.c that tests with both leading
> and trailing non-alphanumeric characters.
>
> The other test in r342668 works because it includes a file that doesn't
> exist even if you interpret the path as relative so it should be OK to keep
> while the bug is found.
>
> I'll go find a bug about the behavior on windows. Thanks!
>
> Jorge
>
> On Thu, Sep 20, 2018 at 2:51 PM Eric Christopher
> wrote:
>
>> FWIW we're trying to reproduce here real fast and then will revert or fix
>> real fast.
>>
>> Thanks!
>>
>> -eric
>>
>> On Thu, Sep 20, 2018 at 2:46 PM Eric Christopher
>> wrote:
>>
>>> Adding Jorge...
>>>
>>> On Thu, Sep 20, 2018 at 2:36 PM wrote:
>>>
Hi Eric,
The test that you added in this commit is failing on the PS4 Windows
bot. Can you please take a look?
http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast/builds/20052
FAIL: Clang :: Preprocessor/include-leading-nonalpha-suggest.c (10765
of 43992)
TEST 'Clang ::
Preprocessor/include-leading-nonalpha-suggest.c' FAILED
Script:
--
: 'RUN: at line 1';
c:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.obj\bin\clang.EXE
-cc1 -internal-isystem
c:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.obj\lib\clang\8.0.0\include
-nostdsysteminc
C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\tools\clang\test\Preprocessor\include-leading-nonalpha-suggest.c
-verify
--
Exit Code: 1
Command Output (stdout):
--
$ ":" "RUN: at line 1"
$
"c:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.obj\bin\clang.EXE"
"-cc1" "-internal-isystem"
"c:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.obj\lib\clang\8.0.0\include"
"-nostdsysteminc"
"C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\tools\clang\test\Preprocessor\include-leading-nonalpha-suggest.c"
"-verify"
# command stderr:
error: 'error' diagnostics expected but not seen:
File
C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\tools\clang\test\Preprocessor\include-leading-nonalpha-suggest.c
Line 3: '/empty_file_to_include.h' file not found, did you mean
'empty_file_to_include.h'?
1 error generated.
error: command failed with exit status: 1
>>> Oof. Thanks. If I don't have something in 10 minutes I'll just revert.
>>>
>>> Thanks!
>>>
>>> -eric
>>>
>>>
>>>
Douglas Yung
> -Original Message-
> From: cfe-commits [mailto:cfe-commits-boun...@lists.llvm.org] On
Behalf
> Of Eric Christopher via cfe-commits
> Sent: Thursday, September 20, 2018 10:23
> To: cfe-commits@lists.llvm.org
> Subject: r342668 - Add testcases for r342667.
>
> Author: echristo
> Date: Thu Sep 20 10:22:43 2018
> New Revision: 342668
>
> URL: http://llvm.org/viewvc/llvm-project?rev=342668&view=rev
> Log:
> Add testcases for r342667.
>
> Added:
> cfe/trunk/test/Preprocessor/include-leading-nonalpha-no-suggest.c
> cfe/trunk/test/Preprocessor/include-leading-nonalpha-suggest.c
>
> Added: cfe/trunk/test/Preprocessor/include-leading-nonalpha-no-
> suggest.c
> URL: http://llvm.org/viewvc/llvm-
> project/cfe/trunk/test/Preprocessor/include-leading-nonalpha-no-
> suggest.c?rev=342668&view=auto
>
===
> ===
> --- cfe/trunk/test/Preprocessor/include-leading-nonalpha-no-suggest.c
> (added)
> +++ cfe/trunk/test/Preprocessor/include-leading-nonalpha-no-suggest.c
> Thu Sep 20 10:22:43 2018
> @@ -0,0 +1,3 @@
> +// RUN: %clang_cc1 %s -verify
> +
> +#include "/non_existing_file_to_include.h" // expected-error
> {{'/non_existing_file_to_include.h' file not found}}
>
> Added: cfe/trunk/test/Preprocessor/include-leading-nonalpha-suggest.c
> URL: http://llvm.org/viewvc/llvm-
> project/cfe/trunk/test/Preprocessor/include-leading-nonalpha-
> suggest.c?rev=
