[PATCH] D43547: [NameMangling] Make ASTContext owning the ManglingContext during entire compilation
pacxx created this revision.
pacxx added reviewers: rnk, rsmith.
Herald added a subscriber: cfe-commits.
The ASTContext is only used to create a Mangling Context and forwards ownership
to everyone who requests a ManglingContext.
The problem fixed by this commit is located in the handling of the
__FUNCDNAME__ generation. Every time a __FUNCDNAME__ expression is handled a
new ManglingContext is created. The MC and its cached manglings are thrown away
every time the __FUNCDNAME__ was handled which results in wrong numbering of
lambda expressions in name manglings (every lambda gets id 0) what again leads
to inconsistencies between __FUNCDNAME__ and the name manglings in the
generated code.
Repository:
rC Clang
https://reviews.llvm.org/D43547
Files:
include/clang/AST/ASTContext.h
lib/AST/ASTContext.cpp
lib/Index/CodegenNameGenerator.cpp
Index: lib/Index/CodegenNameGenerator.cpp
===
--- lib/Index/CodegenNameGenerator.cpp
+++ lib/Index/CodegenNameGenerator.cpp
@@ -26,11 +26,11 @@
using namespace clang::index;
struct CodegenNameGenerator::Implementation {
- std::unique_ptr MC;
+ MangleContext& MC;
llvm::DataLayout DL;
Implementation(ASTContext &Ctx)
-: MC(Ctx.createMangleContext()),
+: MC(Ctx.getMangleContext()),
DL(Ctx.getTargetInfo().getDataLayout()) {}
bool writeName(const Decl *D, raw_ostream &OS) {
@@ -106,7 +106,6 @@
const NamedDecl *ND = cast(D);
ASTContext &Ctx = ND->getASTContext();
-std::unique_ptr M(Ctx.createMangleContext());
std::vector Manglings;
Index: lib/AST/ASTContext.cpp
===
--- lib/AST/ASTContext.cpp
+++ lib/AST/ASTContext.cpp
@@ -9570,6 +9570,12 @@
llvm_unreachable("Unsupported ABI");
}
+MangleContext &ASTContext::getMangleContext() {
+ if (!MContext)
+MContext.reset(createMangleContext());
+ return *MContext.get();
+}
+
CXXABI::~CXXABI() = default;
size_t ASTContext::getSideTableAllocatedMemory() const {
Index: include/clang/AST/ASTContext.h
===
--- include/clang/AST/ASTContext.h
+++ include/clang/AST/ASTContext.h
@@ -2158,6 +2158,7 @@
VTableContextBase *getVTableContext();
MangleContext *createMangleContext();
+ MangleContext &getMangleContext();
void DeepCollectObjCIvars(const ObjCInterfaceDecl *OI, bool leafClass,
SmallVectorImpl &Ivars) const;
@@ -2828,6 +2829,7 @@
std::unique_ptr OtherParents;
std::unique_ptr VTContext;
+ std::unique_ptr MContext;
void ReleaseDeclContextMaps();
void ReleaseParentMapEntries();
Index: lib/Index/CodegenNameGenerator.cpp
===
--- lib/Index/CodegenNameGenerator.cpp
+++ lib/Index/CodegenNameGenerator.cpp
@@ -26,11 +26,11 @@
using namespace clang::index;
struct CodegenNameGenerator::Implementation {
- std::unique_ptr MC;
+ MangleContext& MC;
llvm::DataLayout DL;
Implementation(ASTContext &Ctx)
-: MC(Ctx.createMangleContext()),
+: MC(Ctx.getMangleContext()),
DL(Ctx.getTargetInfo().getDataLayout()) {}
bool writeName(const Decl *D, raw_ostream &OS) {
@@ -106,7 +106,6 @@
const NamedDecl *ND = cast(D);
ASTContext &Ctx = ND->getASTContext();
-std::unique_ptr M(Ctx.createMangleContext());
std::vector Manglings;
Index: lib/AST/ASTContext.cpp
===
--- lib/AST/ASTContext.cpp
+++ lib/AST/ASTContext.cpp
@@ -9570,6 +9570,12 @@
llvm_unreachable("Unsupported ABI");
}
+MangleContext &ASTContext::getMangleContext() {
+ if (!MContext)
+MContext.reset(createMangleContext());
+ return *MContext.get();
+}
+
CXXABI::~CXXABI() = default;
size_t ASTContext::getSideTableAllocatedMemory() const {
Index: include/clang/AST/ASTContext.h
===
--- include/clang/AST/ASTContext.h
+++ include/clang/AST/ASTContext.h
@@ -2158,6 +2158,7 @@
VTableContextBase *getVTableContext();
MangleContext *createMangleContext();
+ MangleContext &getMangleContext();
void DeepCollectObjCIvars(const ObjCInterfaceDecl *OI, bool leafClass,
SmallVectorImpl &Ivars) const;
@@ -2828,6 +2829,7 @@
std::unique_ptr OtherParents;
std::unique_ptr VTContext;
+ std::unique_ptr MContext;
void ReleaseDeclContextMaps();
void ReleaseParentMapEntries();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43497: [analyzer] Introduce correct lifetime extension behavior in simple cases.
a.sidorin accepted this revision.
a.sidorin added a comment.
Thank you! Just some of nits inline.
Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:394
+
+bool ExprEngine::areTemporaryMaterializationsClear(
+ProgramStateRef State, const LocationContext *FromLC,
```
for (const auto &I : State->get()) {
auto *LCtx = I.first.second;
if (LCtx == FromLC || (LCtx->isParentOf(From) && (!To ||
To->isParentOf(LCtx)))
return false;
}
return true;
```
is a bit shorter but less evident so I won't insist.
Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:396
+ProgramStateRef State, const LocationContext *FromLC,
+const LocationContext *ToLC) {
+ const LocationContext *LC = FromLC;
EndLC? (similar to iterators)
Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:400
+assert(LC && "ToLC must be a parent of FromLC!");
+for (auto I : State->get())
+ if (I.first.second == LC)
const auto &?
Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:503
+Key.first->printPretty(Out, nullptr, PP);
+if (Value)
+ Out << " : " << Value;
As I see from line 350, `Value` is always non-null. And there is same check on
line 659. Am I missing something?
https://reviews.llvm.org/D43497
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42545: [Sema] Classify conversions from enum to float as narrowing
This revision was automatically updated to reflect the committed changes.
Closed by commit rC325668: [Sema] Classify conversions from enum to float as
narrowing (authored by miyuki, committed by ).
Repository:
rC Clang
https://reviews.llvm.org/D42545
Files:
lib/Sema/SemaOverload.cpp
test/CXX/dcl.decl/dcl.init/dcl.init.list/p7-0x.cpp
Index: lib/Sema/SemaOverload.cpp
===
--- lib/Sema/SemaOverload.cpp
+++ lib/Sema/SemaOverload.cpp
@@ -327,7 +327,8 @@
FloatingIntegralConversion:
if (FromType->isRealFloatingType() && ToType->isIntegralType(Ctx)) {
return NK_Type_Narrowing;
-} else if (FromType->isIntegralType(Ctx) && ToType->isRealFloatingType()) {
+} else if (FromType->isIntegralOrUnscopedEnumerationType() &&
+ ToType->isRealFloatingType()) {
llvm::APSInt IntConstantValue;
const Expr *Initializer = IgnoreNarrowingConversion(Converted);
assert(Initializer && "Unknown conversion expression");
Index: test/CXX/dcl.decl/dcl.init/dcl.init.list/p7-0x.cpp
===
--- test/CXX/dcl.decl/dcl.init/dcl.init.list/p7-0x.cpp
+++ test/CXX/dcl.decl/dcl.init/dcl.init.list/p7-0x.cpp
@@ -24,6 +24,10 @@
{ 2, f(2), f(2.0) }; // OK: the double-to-int conversion is not at the
top level
}
+enum UnscopedEnum {
+ EnumVal = 300
+};
+
// Test each rule individually.
template
@@ -115,15 +119,21 @@
void int_to_float() {
// Not a constant expression.
char c = 1;
+ UnscopedEnum e = EnumVal;
// Variables. Yes, even though all char's will fit into any floating type.
Agg f1 = {c}; // expected-error {{ cannot be narrowed }}
expected-note {{silence}}
Agg f2 = {c}; // expected-error {{ cannot be narrowed }}
expected-note {{silence}}
Agg f3 = {c}; // expected-error {{ cannot be narrowed }}
expected-note {{silence}}
+ Agg f4 = {e}; // expected-error {{ cannot be narrowed }}
expected-note {{silence}}
+ Agg f5 = {e}; // expected-error {{ cannot be narrowed }}
expected-note {{silence}}
+ Agg f6 = {e}; // expected-error {{ cannot be narrowed }}
expected-note {{silence}}
+
// Constants.
- Agg f4 = {12345678}; // OK (exactly fits in a float)
- Agg f5 = {123456789}; // expected-error {{ cannot be narrowed }}
expected-note {{silence}}
+ Agg f7 = {12345678}; // OK (exactly fits in a float)
+ Agg f8 = {EnumVal}; // OK
+ Agg f9 = {123456789}; // expected-error {{ cannot be narrowed }}
expected-note {{silence}}
Agg ce1 = { Convert(123456789) }; // expected-error {{constant
expression evaluates to 123456789 which cannot be narrowed to type 'float'}}
expected-note {{silence}}
Agg ce2 = { ConvertVar() }; // expected-error
{{non-constant-expression cannot be narrowed from type 'long long' to
'double'}} expected-note {{silence}}
Index: lib/Sema/SemaOverload.cpp
===
--- lib/Sema/SemaOverload.cpp
+++ lib/Sema/SemaOverload.cpp
@@ -327,7 +327,8 @@
FloatingIntegralConversion:
if (FromType->isRealFloatingType() && ToType->isIntegralType(Ctx)) {
return NK_Type_Narrowing;
-} else if (FromType->isIntegralType(Ctx) && ToType->isRealFloatingType()) {
+} else if (FromType->isIntegralOrUnscopedEnumerationType() &&
+ ToType->isRealFloatingType()) {
llvm::APSInt IntConstantValue;
const Expr *Initializer = IgnoreNarrowingConversion(Converted);
assert(Initializer && "Unknown conversion expression");
Index: test/CXX/dcl.decl/dcl.init/dcl.init.list/p7-0x.cpp
===
--- test/CXX/dcl.decl/dcl.init/dcl.init.list/p7-0x.cpp
+++ test/CXX/dcl.decl/dcl.init/dcl.init.list/p7-0x.cpp
@@ -24,6 +24,10 @@
{ 2, f(2), f(2.0) }; // OK: the double-to-int conversion is not at the top level
}
+enum UnscopedEnum {
+ EnumVal = 300
+};
+
// Test each rule individually.
template
@@ -115,15 +119,21 @@
void int_to_float() {
// Not a constant expression.
char c = 1;
+ UnscopedEnum e = EnumVal;
// Variables. Yes, even though all char's will fit into any floating type.
Agg f1 = {c}; // expected-error {{ cannot be narrowed }} expected-note {{silence}}
Agg f2 = {c}; // expected-error {{ cannot be narrowed }} expected-note {{silence}}
Agg f3 = {c}; // expected-error {{ cannot be narrowed }} expected-note {{silence}}
+ Agg f4 = {e}; // expected-error {{ cannot be narrowed }} expected-note {{silence}}
+ Agg f5 = {e}; // expected-error {{ cannot be narrowed }} expected-note {{silence}}
+ Agg f6 = {e}; // expected-error {{ cannot be narrowed }} expected-note {{silence}}
+
// Constants.
- Agg f4 = {12345678}; // OK (exactly fits in a float)
- Agg f5 = {123456789}; // expected-error {{ cannot be narrowed }} expected-note {{silence}}
+ Agg f7 = {12345678}; // OK (exactly fits in a float)
+ Agg f8 = {EnumVal}; // OK
[PATCH] D43518: [clangd] Allow embedders some control over when diagnostics are generated.
ilya-biryukov added inline comments. Comment at: clangd/ClangdServer.h:282 scheduleReparseAndDiags(PathRef File, VersionedDraft Contents, + WantDiagnostics, Tagged> TaggedFS); Maybe add a parameter name here? It's mostly a personal preference, I tend to copy-paste parameter lists between declaration/definition site if I change them. Missing parameter names totally break this workflow for me :-) Comment at: clangd/TUScheduler.cpp:298 + while (shouldSkipHeadLocked()) +Requests.pop_front(); + assert(!Requests.empty() && "skipped the whole queue"); Instead of skipping requests here we could try removing them from back of the queue in `startTask` (or rewriting the last request instead of adding a new one). It feels the code could be simpler, as we will only ever have to remove a single request from the queue. And it could also keep the queue smaller in case of many subsequent `Auto` requests. WDYT? Comment at: clangd/TUScheduler.cpp:339 +// Used unless followed by an update that generates diagnostics. +for (; Next != Requests.end(); ++Next) + if (Next->UpdateType == WantDiagnostics::Yes || Maybe skip updates directly in this function and make it return void? Calling a function in a loop that loops through elements itself is a little confusing. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D43518 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43550: [clangd] Not collect include headers for dynamic index for now.
ioeric created this revision.
ioeric added reviewers: ilya-biryukov, sammccall, hokein.
Herald added subscribers: cfe-commits, jkorous-apple, klimek.
The new behaviors introduced by this patch:
o When include collection is enabled, we always set IncludeHeader field in
Symbol
even if it's the same as FileURI in decl.
o Disable include collection in FileIndex which is currently only used to build
dynamic index. We should revisit when we actually want to use FileIndex to
global
index.
o Code-completion only uses IncludeHeader to insert headers but not FileURI in
CanonicalDeclaration. This ensures that inserted headers are always
canonicalized.
Note that include insertion can still be triggered for symbols that are already
included if they are merged from dynamic index and static index, but we would
only use includes that are already canonicalized (e.g. from static index).
Reason for change:
Collecting header includes in dynamic index enables inserting includes for
headers
that are not indexed but opened in the editor. Comparing to inserting includes
for
symbols in global/static index, this is nice-to-have but would probably require
non-trivial amount of work to get right. For example:
o Currently it's not easy to fully support CanonicalIncludes in dynamic index,
given the way
we run dynamic index.
o It's also harder to reason about the correctness of include canonicalization
for dynamic index
(i.e. symbols in the current file/TU) than static index where symbols are
collected
offline and sanity check is possible before shipping to production.
o We have less control/flexibility over symbol info in the dynamic index
(e.g. URIs, path normalization), which could be used to help make decision when
inserting includes.
As header collection (especially canonicalization) is relatively new, and
enabling
it for dynamic index would immediately affect current users with only dynamic
index support, I propose we disable it for dynamic index for now to avoid
compromising other hot features like code completion and only support it for
static index where include insertion would likely to bring more value.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D43550
Files:
clangd/CodeComplete.cpp
clangd/index/FileIndex.cpp
clangd/index/Index.h
clangd/index/SymbolCollector.cpp
unittests/clangd/FileIndexTests.cpp
unittests/clangd/SymbolCollectorTests.cpp
Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -585,7 +585,7 @@
runSymbolCollector("class Foo {};", /*Main=*/"");
EXPECT_THAT(Symbols,
UnorderedElementsAre(AllOf(QName("Foo"), DeclURI(TestHeaderURI),
- IncludeHeader("";
+ IncludeHeader(TestHeaderURI;
}
#ifndef LLVM_ON_WIN32
Index: unittests/clangd/FileIndexTests.cpp
===
--- unittests/clangd/FileIndexTests.cpp
+++ unittests/clangd/FileIndexTests.cpp
@@ -181,19 +181,19 @@
EXPECT_THAT(match(M, Req), UnorderedElementsAre("X"));
}
-#ifndef LLVM_ON_WIN32
-TEST(FileIndexTest, CanonicalizeSystemHeader) {
+TEST(FileIndexTest, NoIncludeCollected) {
FileIndex M;
- std::string File = testPath("bits/basic_string");
- M.update(File, build(File, "class string {};").getPointer());
+ M.update("f", build("f", "class string {};").getPointer());
FuzzyFindRequest Req;
Req.Query = "";
+ bool SeenSymbol = false;
M.fuzzyFind(Req, [&](const Symbol &Sym) {
-EXPECT_EQ(Sym.Detail->IncludeHeader, "");
+EXPECT_TRUE(Sym.Detail->IncludeHeader.empty());
+SeenSymbol = true;
});
+ EXPECT_TRUE(SeenSymbol);
}
-#endif
} // namespace
} // namespace clangd
Index: clangd/index/SymbolCollector.cpp
===
--- clangd/index/SymbolCollector.cpp
+++ clangd/index/SymbolCollector.cpp
@@ -150,8 +150,9 @@
}
}
-/// Gets a canonical include ( or "header") for header of \p Loc.
-/// Returns None if the header has no canonical include.
+/// Gets a canonical include (URI of the header or or "header") for
+/// header of \p Loc.
+/// Returns None if fails to get include header for \p Loc.
/// FIXME: we should handle .inc files whose symbols are expected be exported by
/// their containing headers.
llvm::Optional
@@ -167,10 +168,11 @@
? Mapped.str()
: ("\"" + Mapped + "\"").str();
}
- // If the header path is the same as the file path of the declaration, we skip
- // storing the #include path; users can use the URI in declaration location to
- // calculate the #include path.
- return llvm::None;
+
+ auto U = toURI(SM, SM.getFilename(Loc), Opts);
+ if (!U)
+return llvm::None;
+ return std::move(*U);
}
// Return the symbol location of the given decl
[PATCH] D42366: [CodeGen] Fix generation of TBAA tags for may-alias accesses
kosarev added a comment. I think zero would serve better as the unknown-size value. People who are not aware of TBAA internals would guess that since zero-sized accesses make no sense, they are likely to have some special meaning. Similarly, for code that is supposed to process the size fields of access descriptors zero would be an obvious "illegal size value". In contrast, UINT64_MAX is just a very large number that doesn't hint anything on its special purpose. Either way, we should reflect the convention in the documentation, https://reviews.llvm.org/D40975. Repository: rC Clang https://reviews.llvm.org/D42366 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D39571: [clangd] DidChangeConfiguration Notification
ilya-biryukov added inline comments.
Comment at: unittests/clangd/ClangdTests.cpp:83
+// least one error.
+class MultipleErrorCHeckingDiagConsumer : public DiagnosticsConsumer {
+public:
NIT: misspelling: ErrorCHecking instead of ErrorChecking
Comment at: unittests/clangd/ClangdTests.cpp:94
+
+ bool contains(PathRef P) {
+std::lock_guard Lock(Mutex);
This function should be `const`.
(Would require making `Mutex` mutable, but that's fine)
Comment at: unittests/clangd/ClangdTests.cpp:99
+
+ bool lastHadError(PathRef P) {
+std::lock_guard Lock(Mutex);
This function should be const and assert that P is in the map.
Comment at: unittests/clangd/ClangdTests.cpp:111
+ std::mutex Mutex;
+ std::map LastDiagsHadError;
+};
Maybe replace `std::map` to `llvm::StringMap`?
Comment at: unittests/clangd/ClangdTests.cpp:492
+
+ EXPECT_TRUE(DiagConsumer.contains(FooCpp));
+ EXPECT_TRUE(DiagConsumer.contains(BarCpp));
Maybe expose a copy of the map from DiagConsumer and check for all files in a
single line?
```
class MultipleErrorCHeckingDiagConsumer {
/// Exposes all files consumed by onDiagnosticsReady in an unspecified order.
/// For each file, a bool value indicates whether the last diagnostics
contained an error.
std::vector> filesWithDiags() const { /* ... */ }
};
///
EXPECT_THAT(DiagConsumer.filesWithDiags(), UnorderedElementsAre(Pair(FooCpp,
false), Pair(BarCpp, true), Pair(BazCpp, false));
```
It would make the test more concise.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D39571
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r325655 - [X86] Disable CLWB in Cannon Lake
Merged to 6.0 in r325672. On Wed, Feb 21, 2018 at 1:16 AM, Craig Topper via cfe-commits wrote: > Author: ctopper > Date: Tue Feb 20 16:16:50 2018 > New Revision: 325655 > > URL: http://llvm.org/viewvc/llvm-project?rev=325655&view=rev > Log: > [X86] Disable CLWB in Cannon Lake > > Cannon Lake does not support CLWB, therefore it > does not include all features listed under SKX. > > Patch by Gabor Buella > > Differential Revision: https://reviews.llvm.org/D43459 > > Modified: > cfe/trunk/lib/Basic/Targets/X86.cpp > cfe/trunk/test/Preprocessor/predefined-arch-macros.c > > Modified: cfe/trunk/lib/Basic/Targets/X86.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets/X86.cpp?rev=325655&r1=325654&r2=325655&view=diff > == > --- cfe/trunk/lib/Basic/Targets/X86.cpp (original) > +++ cfe/trunk/lib/Basic/Targets/X86.cpp Tue Feb 20 16:16:50 2018 > @@ -175,7 +175,8 @@ bool X86TargetInfo::initFeatureMap( > setFeatureEnabledImpl(Features, "avx512bw", true); > setFeatureEnabledImpl(Features, "avx512vl", true); > setFeatureEnabledImpl(Features, "pku", true); > -setFeatureEnabledImpl(Features, "clwb", true); > +if (Kind != CK_Cannonlake) // CNL inherits all SKX features, except CLWB > + setFeatureEnabledImpl(Features, "clwb", true); > LLVM_FALLTHROUGH; >case CK_SkylakeClient: > setFeatureEnabledImpl(Features, "xsavec", true); > > Modified: cfe/trunk/test/Preprocessor/predefined-arch-macros.c > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Preprocessor/predefined-arch-macros.c?rev=325655&r1=325654&r2=325655&view=diff > == > --- cfe/trunk/test/Preprocessor/predefined-arch-macros.c (original) > +++ cfe/trunk/test/Preprocessor/predefined-arch-macros.c Tue Feb 20 16:16:50 > 2018 > @@ -974,7 +974,7 @@ > // CHECK_CNL_M32: #define __BMI2__ 1 > // CHECK_CNL_M32: #define __BMI__ 1 > // CHECK_CNL_M32: #define __CLFLUSHOPT__ 1 > -// CHECK_CNL_M32: #define __CLWB__ 1 > +// CHECK_CNL_M32-NOT: #define __CLWB__ 1 > // CHECK_CNL_M32: #define __F16C__ 1 > // CHECK_CNL_M32: #define __FMA__ 1 > // CHECK_CNL_M32: #define __LZCNT__ 1 > @@ -1022,7 +1022,7 @@ > // CHECK_CNL_M64: #define __BMI2__ 1 > // CHECK_CNL_M64: #define __BMI__ 1 > // CHECK_CNL_M64: #define __CLFLUSHOPT__ 1 > -// CHECK_CNL_M64: #define __CLWB__ 1 > +// CHECK_CNL_M64-NOT: #define __CLWB__ 1 > // CHECK_CNL_M64: #define __F16C__ 1 > // CHECK_CNL_M64: #define __FMA__ 1 > // CHECK_CNL_M64: #define __LZCNT__ 1 > > > ___ > 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] D43550: [clangd] Not collect include headers for dynamic index for now.
ilya-biryukov added a comment.
Is there a way to still enable include insertion but in a restricted manner,
e.g. only for the current project?
File of canonical declaration is usually a pretty good guess and it would be
nice to have it. Maybe we could allowing to disable the include insertion via a
flag or config parameter rather than disabling it for dynamic index entirely.
It bugs me that internal STL headers are added all the time, but it also lets
me test the feature without building the static index. I find the second more
valuable, given that we're still actively working on it.
Comment at: clangd/index/FileIndex.cpp:18
-const CanonicalIncludes *canonicalIncludesForSystemHeaders() {
- static const auto *Includes = [] {
Should we also remove the mutex from `CanonicalIncludes` now?
Comment at: clangd/index/SymbolCollector.cpp:172
+
+ auto U = toURI(SM, SM.getFilename(Loc), Opts);
+ if (!U)
Maybe simplify to `return toURI(...)`?
Or is there a reason why it doesn't work?
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D43550
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43550: [clangd] Not collect include headers for dynamic index for now.
ioeric added a comment. In https://reviews.llvm.org/D43550#1014319, @ilya-biryukov wrote: > Is there a way to still enable include insertion but in a restricted manner, > e.g. only for the current project? I'm not sure if this is currently supported, as we don't have a good definition of a "project" yet. Could you provide a use case? > File of canonical declaration is usually a pretty good guess and it would be > nice to have it. Maybe we could allowing to disable the include insertion via > a flag or config parameter rather than disabling it for dynamic index > entirely. Yes, it is a nice-to-have, when it works :) Without proper canonicalization, this could easily go wrong (e.g. we don't want to insert gtest/gmock internal headers guarded by IWYU pragma). Do you think it's beneficial to collect headers in dynamic index even when it doesn't work correctly? Again, I'm not arguing that collecting includes in dynamic index is not worth doing; I just think it's a rabbit hole that we don't need to chase down at this point :) > It bugs me that internal STL headers are added all the time, but it also lets > me test the feature without building the static index. I find the second more > valuable, given that we're still actively working on it. I think we can test the feature in a safer approach (e.g. build a small static index offline) while not bugging normal users with the wrong includes, which IMO would hurt code completion experience very badly... In general, I think user experience is much more valuable than us being able to test features. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D43550 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43533: [CFG] [analyzer] NFC: Refactor ConstructionContext into a finite set of cases.
a.sidorin accepted this revision. a.sidorin added inline comments. Comment at: include/clang/Analysis/ConstructionContext.h:119 + static const ConstructionContext * + finalize(BumpVectorContext &C, const ConstructionContextLayer *TopLayer); + Maybe just `build()`? For me, `finalize()` strongly associates with Java's broken clean-up mechanism. https://reviews.llvm.org/D43533 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42645: New simple Checker for mmap calls
a.sidorin accepted this revision. a.sidorin added a comment. Looks good to me as an 'alpha' checker. Thank you David! I'd prefer this patch to be accepted by somebody else as well before committing it. https://reviews.llvm.org/D42645 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40731: Integrate CHash into CLang
stettberger updated this revision to Diff 135227.
stettberger added a comment.
Rebased to HEAD, Run (external) clang-hash testsuite and ASTTest
Repository:
rC Clang
https://reviews.llvm.org/D40731
Files:
include/clang/AST/AttrDataCollectors.td
include/clang/AST/CHashVisitor.h
include/clang/AST/CMakeLists.txt
include/clang/AST/DataCollection.h
include/clang/AST/DeclDataCollectors.td
include/clang/AST/StmtDataCollectors.td
include/clang/AST/TypeDataCollectors.td
unittests/AST/CHashTest.cpp
unittests/AST/CMakeLists.txt
Index: unittests/AST/CMakeLists.txt
===
--- unittests/AST/CMakeLists.txt
+++ unittests/AST/CMakeLists.txt
@@ -17,6 +17,7 @@
NamedDeclPrinterTest.cpp
SourceLocationTest.cpp
StmtPrinterTest.cpp
+ CHashTest.cpp
)
target_link_libraries(ASTTests
Index: unittests/AST/CHashTest.cpp
===
--- /dev/null
+++ unittests/AST/CHashTest.cpp
@@ -0,0 +1,91 @@
+//===- unittests/AST/DataCollectionTest.cpp ---===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+// This file contains tests for the DataCollection module.
+//
+// They work by hashing the collected data of two nodes and asserting that the
+// hash values are equal iff the nodes are considered equal.
+//
+//===--===//
+
+#include "clang/AST/CHashVisitor.h"
+#include "clang/Tooling/Tooling.h"
+#include "gtest/gtest.h"
+#include
+
+using namespace clang;
+using namespace tooling;
+
+
+class CHashConsumer : public ASTConsumer {
+CompilerInstance &CI;
+llvm::MD5::MD5Result *ASTHash;
+
+public:
+
+CHashConsumer(CompilerInstance &CI, llvm::MD5::MD5Result *ASTHash)
+: CI(CI), ASTHash(ASTHash){}
+
+virtual void HandleTranslationUnit(clang::ASTContext &Context) override {
+TranslationUnitDecl *TU = Context.getTranslationUnitDecl();
+
+// Traversing the translation unit decl via a RecursiveASTVisitor
+// will visit all nodes in the AST.
+CHashVisitor<> Visitor(Context);
+Visitor.TraverseDecl(TU);
+// Copy Away the resulting hash
+*ASTHash = *Visitor.getHash(TU);
+
+}
+
+~CHashConsumer() override {}
+};
+
+struct CHashAction : public ASTFrontendAction {
+llvm::MD5::MD5Result *Hash;
+
+CHashAction(llvm::MD5::MD5Result *Hash) : Hash(Hash) {}
+
+std::unique_ptr CreateASTConsumer(CompilerInstance &CI,
+ StringRef) override {
+return std::unique_ptr(new CHashConsumer(CI, Hash));
+}
+};
+
+static testing::AssertionResult
+isASTHashEqual(StringRef Code1, StringRef Code2) {
+llvm::MD5::MD5Result Hash1, Hash2;
+if (!runToolOnCode(new CHashAction(&Hash1), Code1)) {
+return testing::AssertionFailure()
+<< "Parsing error in (A)\"" << Code1.str() << "\"";
+}
+if (!runToolOnCode(new CHashAction(&Hash2), Code2)) {
+return testing::AssertionFailure()
+<< "Parsing error in (B) \"" << Code2.str() << "\"";
+}
+return testing::AssertionResult(Hash1 == Hash2);
+}
+
+TEST(CHashVisitor, TestRecordTypes) {
+ASSERT_TRUE(isASTHashEqual( // Unused struct
+ "struct foobar { int a0; char a1; unsigned long a2; };",
+ "struct foobar { int a0; char a1;};"
+ ));
+
+}
+
+TEST(CHashVisitor, TestSourceStructure) {
+ASSERT_FALSE(isASTHashEqual(
+ "void foo() { int c; if (0) { c = 1; } }",
+ "void foo() { int c; if (0) { } c = 1; }"));
+
+ASSERT_FALSE(isASTHashEqual(
+ "void f1() {} void f2() { }",
+ "void f1() {} void f2() { f1(); }"));
+}
Index: include/clang/AST/TypeDataCollectors.td
===
--- include/clang/AST/TypeDataCollectors.td
+++ include/clang/AST/TypeDataCollectors.td
@@ -0,0 +1,78 @@
+//--- Types ---//
+
+class Type {
+ code Code = [{
+ addData(llvm::hash_value(S->getTypeClass()));
+ }];
+}
+
+class BuiltinType {
+ code Code = [{
+ addData(S->getKind());
+ }];
+}
+
+class ArrayType {
+ code Code = [{
+ addData(S->getSizeModifier());
+ addData(S->getIndexTypeCVRQualifiers());
+ }];
+}
+
+class ConstantArrayType {
+ code Code = [{
+ addData(S->getSize().getZExtValue());
+ }];
+}
+
+class VectorType {
+ code Code = [{
+ addData(S->getNumElements());
+ addData(S->getVectorKind());
+ }];
+}
+
+class FunctionType {
+ code Code = [{
+ addData(S->getRegParmType
[PATCH] D43550: [clangd] Not collect include headers for dynamic index for now.
ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. As discussed offline, properly supporting IWYU pragma in the dynamic index seems like too much design work for now and it's not gonna get fixed soon. So opting for disabling the feature seems like the right approach. Therefore, LGTM. (There are a few nits in inline comments from my previous reply that you might want to look at before landing this) Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D43550 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D39571: [clangd] DidChangeConfiguration Notification
simark marked 5 inline comments as done.
simark added inline comments.
Comment at: unittests/clangd/ClangdTests.cpp:492
+
+ EXPECT_TRUE(DiagConsumer.contains(FooCpp));
+ EXPECT_TRUE(DiagConsumer.contains(BarCpp));
ilya-biryukov wrote:
> Maybe expose a copy of the map from DiagConsumer and check for all files in a
> single line?
>
> ```
> class MultipleErrorCHeckingDiagConsumer {
>/// Exposes all files consumed by onDiagnosticsReady in an unspecified
> order.
>/// For each file, a bool value indicates whether the last diagnostics
> contained an error.
>std::vector> filesWithDiags() const { /* ... */ }
> };
>
> ///
> EXPECT_THAT(DiagConsumer.filesWithDiags(), UnorderedElementsAre(Pair(FooCpp,
> false), Pair(BarCpp, true), Pair(BazCpp, false));
> ```
>
> It would make the test more concise.
Nice :)
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D39571
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D39571: [clangd] DidChangeConfiguration Notification
simark marked an inline comment as done.
simark added inline comments.
Comment at: unittests/clangd/ClangdTests.cpp:492
+
+ EXPECT_TRUE(DiagConsumer.contains(FooCpp));
+ EXPECT_TRUE(DiagConsumer.contains(BarCpp));
simark wrote:
> ilya-biryukov wrote:
> > Maybe expose a copy of the map from DiagConsumer and check for all files in
> > a single line?
> >
> > ```
> > class MultipleErrorCHeckingDiagConsumer {
> >/// Exposes all files consumed by onDiagnosticsReady in an unspecified
> > order.
> >/// For each file, a bool value indicates whether the last diagnostics
> > contained an error.
> >std::vector> filesWithDiags() const { /* ... */ }
> > };
> >
> > ///
> > EXPECT_THAT(DiagConsumer.filesWithDiags(),
> > UnorderedElementsAre(Pair(FooCpp, false), Pair(BarCpp, true), Pair(BazCpp,
> > false));
> > ```
> >
> > It would make the test more concise.
> Nice :)
It's also better because we don't have to explicitly check that Baz is not
there. So if some other file sneaks in the result for some reason and
shouldn't be there, the test will now fail, where it wouldn't have previously.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D39571
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D39571: [clangd] DidChangeConfiguration Notification
simark updated this revision to Diff 135229.
simark added a comment.
New version
Address comments in https://reviews.llvm.org/D39571#1014237
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D39571
Files:
clangd/ClangdLSPServer.cpp
clangd/ClangdLSPServer.h
clangd/ClangdServer.cpp
clangd/ClangdServer.h
clangd/DraftStore.cpp
clangd/DraftStore.h
clangd/GlobalCompilationDatabase.cpp
clangd/GlobalCompilationDatabase.h
clangd/Protocol.cpp
clangd/Protocol.h
clangd/ProtocolHandlers.cpp
clangd/ProtocolHandlers.h
unittests/clangd/ClangdTests.cpp
Index: unittests/clangd/ClangdTests.cpp
===
--- unittests/clangd/ClangdTests.cpp
+++ unittests/clangd/ClangdTests.cpp
@@ -7,6 +7,7 @@
//
//===--===//
+#include "Annotations.h"
#include "ClangdLSPServer.h"
#include "ClangdServer.h"
#include "Matchers.h"
@@ -77,6 +78,43 @@
VFSTag LastVFSTag = VFSTag();
};
+/// For each file, record whether the last published diagnostics contained at
+/// least one error.
+class MultipleErrorCheckingDiagConsumer : public DiagnosticsConsumer {
+public:
+ void
+ onDiagnosticsReady(PathRef File,
+ Tagged> Diagnostics) override {
+bool HadError = diagsContainErrors(Diagnostics.Value);
+
+std::lock_guard Lock(Mutex);
+LastDiagsHadError[File] = HadError;
+ }
+
+ /// Exposes all files consumed by onDiagnosticsReady in an unspecified order.
+ /// For each file, a bool value indicates whether the last diagnostics
+ /// contained an error.
+ std::vector> filesWithDiags() const {
+std::vector> Result;
+std::lock_guard Lock(Mutex);
+
+for (const auto &it : LastDiagsHadError) {
+ Result.emplace_back(it.first(), it.second);
+}
+
+return Result;
+ }
+
+ void clear() {
+std::lock_guard Lock(Mutex);
+LastDiagsHadError.clear();
+ }
+
+private:
+ mutable std::mutex Mutex;
+ llvm::StringMap LastDiagsHadError;
+};
+
/// Replaces all patterns of the form 0x123abc with spaces
std::string replacePtrsInDump(std::string const &Dump) {
llvm::Regex RE("0x[0-9a-fA-F]+");
@@ -413,6 +451,72 @@
EXPECT_FALSE(DiagConsumer.hadErrorInLastDiags());
}
+// Test ClangdServer.reparseOpenedFiles.
+TEST_F(ClangdVFSTest, ReparseOpenedFiles) {
+ Annotations FooSource(R"cpp(
+#ifdef MACRO
+$one[[static void bob() {}]]
+#else
+$two[[static void bob() {}]]
+#endif
+
+int main () { bo^b (); return 0; }
+)cpp");
+
+ Annotations BarSource(R"cpp(
+#ifdef MACRO
+this is an error
+#endif
+)cpp");
+
+ Annotations BazSource(R"cpp(
+int hello;
+)cpp");
+
+ MockFSProvider FS;
+ MockCompilationDatabase CDB;
+ MultipleErrorCheckingDiagConsumer DiagConsumer;
+ ClangdServer Server(CDB, DiagConsumer, FS,
+ /*AsyncThreadsCount=*/0,
+ /*StorePreamblesInMemory=*/true);
+
+ auto FooCpp = testPath("foo.cpp");
+ auto BarCpp = testPath("bar.cpp");
+ auto BazCpp = testPath("baz.cpp");
+
+ FS.Files[FooCpp] = "";
+ FS.Files[BarCpp] = "";
+ FS.Files[BazCpp] = "";
+
+ CDB.ExtraClangFlags = {"-DMACRO=1"};
+ Server.addDocument(FooCpp, FooSource.code());
+ Server.addDocument(BarCpp, BarSource.code());
+ Server.addDocument(BazCpp, BazSource.code());
+
+ EXPECT_THAT(DiagConsumer.filesWithDiags(),
+ UnorderedElementsAre(Pair(FooCpp, false), Pair(BarCpp, true),
+ Pair(BazCpp, false)));
+
+ auto Locations = runFindDefinitions(Server, FooCpp, FooSource.point());
+ EXPECT_TRUE(bool(Locations));
+ EXPECT_THAT(Locations->Value, ElementsAre(Location{URIForFile{FooCpp},
+ FooSource.range("one")}));
+
+ // Undefine MACRO, close baz.cpp.
+ CDB.ExtraClangFlags.clear();
+ DiagConsumer.clear();
+ Server.removeDocument(BazCpp);
+ Server.reparseOpenedFiles();
+
+ EXPECT_THAT(DiagConsumer.filesWithDiags(),
+ UnorderedElementsAre(Pair(FooCpp, false), Pair(BarCpp, false)));
+
+ Locations = runFindDefinitions(Server, FooCpp, FooSource.point());
+ EXPECT_TRUE(bool(Locations));
+ EXPECT_THAT(Locations->Value, ElementsAre(Location{URIForFile{FooCpp},
+ FooSource.range("two")}));
+}
+
TEST_F(ClangdVFSTest, MemoryUsage) {
MockFSProvider FS;
ErrorCheckingDiagConsumer DiagConsumer;
Index: clangd/ProtocolHandlers.h
===
--- clangd/ProtocolHandlers.h
+++ clangd/ProtocolHandlers.h
@@ -52,6 +52,7 @@
virtual void onRename(RenameParams &Parames) = 0;
virtual void onDocumentHighlight(TextDocumentPositionParams &Params) = 0;
virtual void onHover(TextDocumentPositionParams &Params) = 0;
+ virtual void onChangeConfiguration(DidChangeConfigurationParams &Params) = 0;
};
void registerCallbackHandlers(JSONRPCDispatcher &Dispatcher, JSONOutput &Out,
Index: clangd/ProtocolHandl
[PATCH] D43567: [ASTMatchers] isTemplateInstantiation: also match explicit instantiation declaration.
ioeric created this revision.
ioeric added a reviewer: bkramer.
Herald added subscribers: cfe-commits, klimek.
Example:
template class X {}; class A {};
// Explicit instantiation declaration.
extern template class X;
Repository:
rC Clang
https://reviews.llvm.org/D43567
Files:
include/clang/ASTMatchers/ASTMatchers.h
unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
Index: unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -1623,6 +1623,14 @@
"template class X;",
cxxRecordDecl(isTemplateInstantiation(), hasDescendant(
fieldDecl(hasType(recordDecl(hasName("A";
+
+ // Make sure that we match the instantiation instead of the template
+ // definition by checking whether the member function is present.
+ EXPECT_TRUE(
+ matches("template class X { void f() { T t; } };"
+ "extern template class X;",
+ cxxRecordDecl(isTemplateInstantiation(),
+unless(hasDescendant(varDecl(hasName("t")));
}
TEST(IsTemplateInstantiation,
Index: include/clang/ASTMatchers/ASTMatchers.h
===
--- include/clang/ASTMatchers/ASTMatchers.h
+++ include/clang/ASTMatchers/ASTMatchers.h
@@ -4649,6 +4649,10 @@
/// \code
/// template class X {}; class A {}; template class X;
/// \endcode
+/// or
+/// \code
+/// template class X {}; class A {}; extern template class X;
+/// \endcode
/// cxxRecordDecl(hasName("::X"), isTemplateInstantiation())
/// matches the template instantiation of X.
///
@@ -4666,7 +4670,9 @@
CXXRecordDecl)) {
return (Node.getTemplateSpecializationKind() == TSK_ImplicitInstantiation ||
Node.getTemplateSpecializationKind() ==
- TSK_ExplicitInstantiationDefinition);
+ TSK_ExplicitInstantiationDefinition ||
+ Node.getTemplateSpecializationKind() ==
+ TSK_ExplicitInstantiationDeclaration);
}
/// \brief Matches declarations that are template instantiations or are inside
Index: unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -1623,6 +1623,14 @@
"template class X;",
cxxRecordDecl(isTemplateInstantiation(), hasDescendant(
fieldDecl(hasType(recordDecl(hasName("A";
+
+ // Make sure that we match the instantiation instead of the template
+ // definition by checking whether the member function is present.
+ EXPECT_TRUE(
+ matches("template class X { void f() { T t; } };"
+ "extern template class X;",
+ cxxRecordDecl(isTemplateInstantiation(),
+unless(hasDescendant(varDecl(hasName("t")));
}
TEST(IsTemplateInstantiation,
Index: include/clang/ASTMatchers/ASTMatchers.h
===
--- include/clang/ASTMatchers/ASTMatchers.h
+++ include/clang/ASTMatchers/ASTMatchers.h
@@ -4649,6 +4649,10 @@
/// \code
/// template class X {}; class A {}; template class X;
/// \endcode
+/// or
+/// \code
+/// template class X {}; class A {}; extern template class X;
+/// \endcode
/// cxxRecordDecl(hasName("::X"), isTemplateInstantiation())
/// matches the template instantiation of X.
///
@@ -4666,7 +4670,9 @@
CXXRecordDecl)) {
return (Node.getTemplateSpecializationKind() == TSK_ImplicitInstantiation ||
Node.getTemplateSpecializationKind() ==
- TSK_ExplicitInstantiationDefinition);
+ TSK_ExplicitInstantiationDefinition ||
+ Node.getTemplateSpecializationKind() ==
+ TSK_ExplicitInstantiationDeclaration);
}
/// \brief Matches declarations that are template instantiations or are inside
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41537: Optionally add code completion results for arrow instead of dot
yvvan added a comment. I've already added hints in this patch and it also do not add extra completions unless the flag IncludeCorrections is set. So this will not force editors to use corrections. Did you mean that you think it's good to have extra fixit hints even if this flag is not set? https://reviews.llvm.org/D41537 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43567: [ASTMatchers] isTemplateInstantiation: also match explicit instantiation declaration.
bkramer accepted this revision. bkramer added a comment. This revision is now accepted and ready to land. lg Repository: rC Clang https://reviews.llvm.org/D43567 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43392: [clang-tidy] Add Fuchsia checker for visibility attributes
aaron.ballman added inline comments.
Comment at: clang-tidy/fuchsia/AddVisibilityCheck.cpp:23-25
+ // if (const auto *D = Node.getDefinition()) {
+ // return D->getExplicitVisibility(NamedDecl::VisibilityForType) == V;
+ // }
Can remove these comments.
Comment at: clang-tidy/fuchsia/AddVisibilityCheck.cpp:52-53
+ Finder->addMatcher(
+ functionDecl(allOf(hasAnyName(SmallVector(Names.begin(),
+ Names.end())),
+ isDefinition(), unless(hasVisibilityAttr(V
Can you use `makeArrayRef()` rather than using a `SmallVector` that allocates?
Comment at: clang-tidy/fuchsia/AddVisibilityCheck.cpp:62
+ Result.Nodes.getNodeAs("no-visibility")) {
+ const Attr *A = D->getAttr();
+ if (A && !A->isInherited() && A->getLocation().isValid())
`const auto *`
Comment at: clang-tidy/fuchsia/AddVisibilityCheck.cpp:66
+"%0 visibility attribute not set for specified function")
+ << VisAttr << D
+ << FixItHint::CreateReplacement(A->getRange(),
Why are you passing `D` as well?
Comment at: clang-tidy/fuchsia/AddVisibilityCheck.cpp:72
+"no explicit visibility attribute set for specified function")
+ << VisAttr << D
+ << FixItHint::CreateInsertion(D->getLocStart(),
Why are you passing in both `VisAttr` and `D`?
Comment at: test/clang-tidy/fuchsia-add-visibility.cpp:25
+// CHECK-MESSAGES: [[@LINE-2]]:1: warning: default visibility attribute not
set for specified function
+// CHECK-FIXES: __attribute__((visibility("default")))
Should this test case diagnose, assuming `foo` is in the list of functions to
check?
```
__attribute__((visibility("default"))) void foo(int);
void foo(double); // Diagnose?
```
How about this case?
```
template
__attribute__((visibility("default"))) void foo(Ty);
template <>
void foo(int); // Diagnose?
```
https://reviews.llvm.org/D43392
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43538: [clang-tidy] Update run-clang-tidy.py with config arg
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. Aside from a nit with the help text, this LGTM. Comment at: clang-tidy/tool/run-clang-tidy.py:188-189 + ' value: y}]}"' + 'When the value is empty, clang-tidy will' + 'attempt to find a file named .clang-tidy for' + 'each source file in its parent directories.') Missing some trailing spaces at the end of the literals. https://reviews.llvm.org/D43538 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r325668 - [Sema] Classify conversions from enum to float as narrowing
Author: miyuki
Date: Wed Feb 21 02:08:18 2018
New Revision: 325668
URL: http://llvm.org/viewvc/llvm-project?rev=325668&view=rev
Log:
[Sema] Classify conversions from enum to float as narrowing
Summary:
According to [dcl.init.list]p7:
A narrowing conversion is an implicit conversion
- ...
- from an integer type or unscoped enumeration type to a
floating-point type, except where the source is a constant
expression and the actual value after conversion will fit into
the target type and will produce the original value when
converted back to the original type, or
- ...
Currently clang does not handle the 'unscoped enumeration' case. This
patch fixes the corresponding check.
Reviewers: faisalv, rsmith, rogfer01
Reviewed By: rogfer01
Subscribers: rogfer01, cfe-commits
Differential Revision: https://reviews.llvm.org/D42545
Modified:
cfe/trunk/lib/Sema/SemaOverload.cpp
cfe/trunk/test/CXX/dcl.decl/dcl.init/dcl.init.list/p7-0x.cpp
Modified: cfe/trunk/lib/Sema/SemaOverload.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOverload.cpp?rev=325668&r1=325667&r2=325668&view=diff
==
--- cfe/trunk/lib/Sema/SemaOverload.cpp (original)
+++ cfe/trunk/lib/Sema/SemaOverload.cpp Wed Feb 21 02:08:18 2018
@@ -327,7 +327,8 @@ StandardConversionSequence::getNarrowing
FloatingIntegralConversion:
if (FromType->isRealFloatingType() && ToType->isIntegralType(Ctx)) {
return NK_Type_Narrowing;
-} else if (FromType->isIntegralType(Ctx) && ToType->isRealFloatingType()) {
+} else if (FromType->isIntegralOrUnscopedEnumerationType() &&
+ ToType->isRealFloatingType()) {
llvm::APSInt IntConstantValue;
const Expr *Initializer = IgnoreNarrowingConversion(Converted);
assert(Initializer && "Unknown conversion expression");
Modified: cfe/trunk/test/CXX/dcl.decl/dcl.init/dcl.init.list/p7-0x.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/dcl.decl/dcl.init/dcl.init.list/p7-0x.cpp?rev=325668&r1=325667&r2=325668&view=diff
==
--- cfe/trunk/test/CXX/dcl.decl/dcl.init/dcl.init.list/p7-0x.cpp (original)
+++ cfe/trunk/test/CXX/dcl.decl/dcl.init/dcl.init.list/p7-0x.cpp Wed Feb 21
02:08:18 2018
@@ -24,6 +24,10 @@ void std_example() {
{ 2, f(2), f(2.0) }; // OK: the double-to-int conversion is not at the
top level
}
+enum UnscopedEnum {
+ EnumVal = 300
+};
+
// Test each rule individually.
template
@@ -115,15 +119,21 @@ void shrink_float() {
void int_to_float() {
// Not a constant expression.
char c = 1;
+ UnscopedEnum e = EnumVal;
// Variables. Yes, even though all char's will fit into any floating type.
Agg f1 = {c}; // expected-error {{ cannot be narrowed }}
expected-note {{silence}}
Agg f2 = {c}; // expected-error {{ cannot be narrowed }}
expected-note {{silence}}
Agg f3 = {c}; // expected-error {{ cannot be narrowed }}
expected-note {{silence}}
+ Agg f4 = {e}; // expected-error {{ cannot be narrowed }}
expected-note {{silence}}
+ Agg f5 = {e}; // expected-error {{ cannot be narrowed }}
expected-note {{silence}}
+ Agg f6 = {e}; // expected-error {{ cannot be narrowed }}
expected-note {{silence}}
+
// Constants.
- Agg f4 = {12345678}; // OK (exactly fits in a float)
- Agg f5 = {123456789}; // expected-error {{ cannot be narrowed }}
expected-note {{silence}}
+ Agg f7 = {12345678}; // OK (exactly fits in a float)
+ Agg f8 = {EnumVal}; // OK
+ Agg f9 = {123456789}; // expected-error {{ cannot be narrowed }}
expected-note {{silence}}
Agg ce1 = { Convert(123456789) }; // expected-error {{constant
expression evaluates to 123456789 which cannot be narrowed to type 'float'}}
expected-note {{silence}}
Agg ce2 = { ConvertVar() }; // expected-error
{{non-constant-expression cannot be narrowed from type 'long long' to
'double'}} expected-note {{silence}}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43569: [clangd] Correct setting ignoreWarnings in CodeCompletion.
hokein created this revision. hokein added a reviewer: ilya-biryukov. Herald added subscribers: ioeric, jkorous-apple, klimek. We should set the flag before creating ComplierInstance -- when CopmilerInstance gets initialized, it also initializes the DiagnosticsEngine using the DiagnosticOptions. This was hidden deeply -- as clang suppresses all diagnostics when we hit the code-completion (but internally it does do unnecessary analysis stuff). As a bonus point, this fix will optmize the completion speed -- clang won't do any analysis (e.g. -Wunreachable-code, -Wthread-safety-analysisi) at all internally. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D43569 Files: clangd/CodeComplete.cpp Index: clangd/CodeComplete.cpp === --- clangd/CodeComplete.cpp +++ clangd/CodeComplete.cpp @@ -702,11 +702,10 @@ Input.Preamble->CanReuse(*CI, ContentsBuffer.get(), Bounds, Input.VFS.get()); } + CI->getDiagnosticOpts().IgnoreWarnings = true; auto Clang = prepareCompilerInstance( std::move(CI), Input.Preamble, std::move(ContentsBuffer), std::move(Input.PCHs), std::move(Input.VFS), DummyDiagsConsumer); - auto &DiagOpts = Clang->getDiagnosticOpts(); - DiagOpts.IgnoreWarnings = true; // Disable typo correction in Sema. Clang->getLangOpts().SpellChecking = false; Index: clangd/CodeComplete.cpp === --- clangd/CodeComplete.cpp +++ clangd/CodeComplete.cpp @@ -702,11 +702,10 @@ Input.Preamble->CanReuse(*CI, ContentsBuffer.get(), Bounds, Input.VFS.get()); } + CI->getDiagnosticOpts().IgnoreWarnings = true; auto Clang = prepareCompilerInstance( std::move(CI), Input.Preamble, std::move(ContentsBuffer), std::move(Input.PCHs), std::move(Input.VFS), DummyDiagsConsumer); - auto &DiagOpts = Clang->getDiagnosticOpts(); - DiagOpts.IgnoreWarnings = true; // Disable typo correction in Sema. Clang->getLangOpts().SpellChecking = false; ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43570: [OpenCL] Add '-cl-uniform-work-group-size' compile option
krisb created this revision.
Herald added subscribers: cfe-commits, Anastasia, yaxunl.
krisb added reviewers: yaxunl, Anastasia.
OpenCL 2.0 specification defines '-cl-uniform-work-group-size' option,
which requires that the global work-size be a multiple of the work-group
size specified to clEnqueueNDRangeKernel and allows optimizations that
are made possible by this restriction.
The patch introduces the support of this option.
To keep information about whether an OpenCL kernel has uniform work
group size or not, clang generates 'uniform-work-group-size' function
attribute for every kernel:
- "uniform-work-group-size"="true" for OpenCL 1.2 and lower,
- "uniform-work-group-size"="true" for OpenCL 2.0 and higher if
'-cl-uniform-work-group-size' option was specified,
- "uniform-work-group-size"="false" for OpenCL 2.0 and higher if no
'-cl-uniform-work-group-size' options was specified.
If the function is not an OpenCL kernel, 'uniform-work-group-size'
attribute isn't generated.
Repository:
rC Clang
https://reviews.llvm.org/D43570
Files:
include/clang/Driver/Options.td
include/clang/Frontend/CodeGenOptions.def
lib/CodeGen/CGCall.cpp
lib/Driver/ToolChains/Clang.cpp
lib/Frontend/CompilerInvocation.cpp
test/CodeGenOpenCL/cl-uniform-wg-size.cl
test/CodeGenOpenCL/convergent.cl
test/Driver/opencl.cl
Index: test/Driver/opencl.cl
===
--- test/Driver/opencl.cl
+++ test/Driver/opencl.cl
@@ -13,6 +13,7 @@
// RUN: %clang -S -### -cl-no-signed-zeros %s 2>&1 | FileCheck --check-prefix=CHECK-NO-SIGNED-ZEROS %s
// RUN: %clang -S -### -cl-denorms-are-zero %s 2>&1 | FileCheck --check-prefix=CHECK-DENORMS-ARE-ZERO %s
// RUN: %clang -S -### -cl-fp32-correctly-rounded-divide-sqrt %s 2>&1 | FileCheck --check-prefix=CHECK-ROUND-DIV %s
+// RUN: %clang -S -### -cl-uniform-work-group-size %s 2>&1 | FileCheck --check-prefix=CHECK-UNIFORM-WG %s
// RUN: not %clang -cl-std=c99 -DOPENCL %s 2>&1 | FileCheck --check-prefix=CHECK-C99 %s
// RUN: not %clang -cl-std=invalid -DOPENCL %s 2>&1 | FileCheck --check-prefix=CHECK-INVALID %s
@@ -31,6 +32,7 @@
// CHECK-NO-SIGNED-ZEROS: "-cc1" {{.*}} "-cl-no-signed-zeros"
// CHECK-DENORMS-ARE-ZERO: "-cc1" {{.*}} "-cl-denorms-are-zero"
// CHECK-ROUND-DIV: "-cc1" {{.*}} "-cl-fp32-correctly-rounded-divide-sqrt"
+// CHECK-UNIFORM-WG: "-cc1" {{.*}} "-cl-uniform-work-group-size"
// CHECK-C99: error: invalid value 'c99' in '-cl-std=c99'
// CHECK-INVALID: error: invalid value 'invalid' in '-cl-std=invalid'
Index: test/CodeGenOpenCL/convergent.cl
===
--- test/CodeGenOpenCL/convergent.cl
+++ test/CodeGenOpenCL/convergent.cl
@@ -127,7 +127,7 @@
// CHECK: declare spir_func void @nodupfun(){{[^#]*}} #[[attr3:[0-9]+]]
// CHECK-LABEL: @assume_convergent_asm
-// CHECK: tail call void asm sideeffect "s_barrier", ""() #4
+// CHECK: tail call void asm sideeffect "s_barrier", ""() #5
kernel void assume_convergent_asm()
{
__asm__ volatile("s_barrier");
@@ -138,4 +138,5 @@
// CHECK: attributes #2 = { {{[^}]*}}convergent{{[^}]*}} }
// CHECK: attributes #3 = { {{[^}]*}}convergent noduplicate{{[^}]*}} }
// CHECK: attributes #4 = { {{[^}]*}}convergent{{[^}]*}} }
-// CHECK: attributes #5 = { {{[^}]*}}convergent noduplicate{{[^}]*}} }
+// CHECK: attributes #5 = { {{[^}]*}}convergent{{[^}]*}} }
+// CHECK: attributes #6 = { {{[^}]*}}convergent noduplicate{{[^}]*}} }
Index: test/CodeGenOpenCL/cl-uniform-wg-size.cl
===
--- /dev/null
+++ test/CodeGenOpenCL/cl-uniform-wg-size.cl
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -emit-llvm -O0 -cl-std=CL1.2 -o - %s 2>&1 | FileCheck %s -check-prefixes CHECK,CHECK-UNIFORM
+// RUN: %clang_cc1 -emit-llvm -O0 -cl-std=CL2.0 -o - %s 2>&1 | FileCheck %s -check-prefixes CHECK,CHECK-NONUNIFORM
+// RUN: %clang_cc1 -emit-llvm -O0 -cl-std=CL2.0 -cl-uniform-work-group-size -o - %s 2>&1 | FileCheck %s -check-prefixes CHECK,CHECK-UNIFORM
+
+kernel void ker() {};
+// CHECK: define{{.*}}@ker() #0
+
+void foo() {};
+// CHECK: define{{.*}}@foo() #1
+
+// CHECK-LABEL: attributes #0
+// CHECK-UNIFORM: "uniform-work-group-size"="true"
+// CHECK-NONUNIFORM: "uniform-work-group-size"="false"
+
+// CHECK-LABEL: attributes #1
+// CHECK-NOT: uniform-work-group-size
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -659,6 +659,8 @@
Opts.FlushDenorm = Args.hasArg(OPT_cl_denorms_are_zero);
Opts.CorrectlyRoundedDivSqrt =
Args.hasArg(OPT_cl_fp32_correctly_rounded_divide_sqrt);
+ Opts.UniformWGSize =
+ Args.hasArg(OPT_cl_uniform_work_group_size);
Opts.Reciprocals = Args.getAllArgValues(OPT_mrecip_EQ);
Opts.ReciprocalMath = Args.hasArg(OPT_freciprocal_math);
Opts.NoTrappingMath = Args.hasArg(OPT_fno_trapping_math);
Index: lib
r325678 - [ASTMatchers] isTemplateInstantiation: also match explicit instantiation declaration.
Author: ioeric
Date: Wed Feb 21 05:51:27 2018
New Revision: 325678
URL: http://llvm.org/viewvc/llvm-project?rev=325678&view=rev
Log:
[ASTMatchers] isTemplateInstantiation: also match explicit instantiation
declaration.
Summary:
Example:
template class X {}; class A {};
// Explicit instantiation declaration.
extern template class X;
Reviewers: bkramer
Subscribers: klimek, cfe-commits
Differential Revision: https://reviews.llvm.org/D43567
Modified:
cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h
cfe/trunk/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
Modified: cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h?rev=325678&r1=325677&r2=325678&view=diff
==
--- cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h (original)
+++ cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h Wed Feb 21 05:51:27 2018
@@ -4649,6 +4649,10 @@ AST_MATCHER_P(UsingShadowDecl, hasTarget
/// \code
/// template class X {}; class A {}; template class X;
/// \endcode
+/// or
+/// \code
+/// template class X {}; class A {}; extern template class X;
+/// \endcode
/// cxxRecordDecl(hasName("::X"), isTemplateInstantiation())
/// matches the template instantiation of X.
///
@@ -4666,7 +4670,9 @@ AST_POLYMORPHIC_MATCHER(isTemplateInstan
CXXRecordDecl)) {
return (Node.getTemplateSpecializationKind() == TSK_ImplicitInstantiation ||
Node.getTemplateSpecializationKind() ==
- TSK_ExplicitInstantiationDefinition);
+ TSK_ExplicitInstantiationDefinition ||
+ Node.getTemplateSpecializationKind() ==
+ TSK_ExplicitInstantiationDeclaration);
}
/// \brief Matches declarations that are template instantiations or are inside
Modified: cfe/trunk/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp?rev=325678&r1=325677&r2=325678&view=diff
==
--- cfe/trunk/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp (original)
+++ cfe/trunk/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp Wed Feb 21
05:51:27 2018
@@ -1623,6 +1623,14 @@ TEST(IsTemplateInstantiation, MatchesExp
"template class X;",
cxxRecordDecl(isTemplateInstantiation(), hasDescendant(
fieldDecl(hasType(recordDecl(hasName("A";
+
+ // Make sure that we match the instantiation instead of the template
+ // definition by checking whether the member function is present.
+ EXPECT_TRUE(
+ matches("template class X { void f() { T t; } };"
+ "extern template class X;",
+ cxxRecordDecl(isTemplateInstantiation(),
+unless(hasDescendant(varDecl(hasName("t")));
}
TEST(IsTemplateInstantiation,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r325678 - [ASTMatchers] isTemplateInstantiation: also match explicit instantiation declaration.
On Wed, Feb 21, 2018 at 8:51 AM, Eric Liu via cfe-commits
wrote:
> Author: ioeric
> Date: Wed Feb 21 05:51:27 2018
> New Revision: 325678
>
> URL: http://llvm.org/viewvc/llvm-project?rev=325678&view=rev
> Log:
> [ASTMatchers] isTemplateInstantiation: also match explicit instantiation
> declaration.
>
> Summary:
> Example:
> template class X {}; class A {};
> // Explicit instantiation declaration.
> extern template class X;
>
> Reviewers: bkramer
>
> Subscribers: klimek, cfe-commits
>
> Differential Revision: https://reviews.llvm.org/D43567
>
> Modified:
> cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h
> cfe/trunk/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
Please regenerate the AST matcher documentation as well.
~Aaron
>
> Modified: cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h?rev=325678&r1=325677&r2=325678&view=diff
> ==
> --- cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h (original)
> +++ cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h Wed Feb 21 05:51:27 2018
> @@ -4649,6 +4649,10 @@ AST_MATCHER_P(UsingShadowDecl, hasTarget
> /// \code
> /// template class X {}; class A {}; template class X;
> /// \endcode
> +/// or
> +/// \code
> +/// template class X {}; class A {}; extern template class
> X;
> +/// \endcode
> /// cxxRecordDecl(hasName("::X"), isTemplateInstantiation())
> /// matches the template instantiation of X.
> ///
> @@ -4666,7 +4670,9 @@ AST_POLYMORPHIC_MATCHER(isTemplateInstan
> CXXRecordDecl)) {
>return (Node.getTemplateSpecializationKind() == TSK_ImplicitInstantiation
> ||
>Node.getTemplateSpecializationKind() ==
> - TSK_ExplicitInstantiationDefinition);
> + TSK_ExplicitInstantiationDefinition ||
> + Node.getTemplateSpecializationKind() ==
> + TSK_ExplicitInstantiationDeclaration);
> }
>
> /// \brief Matches declarations that are template instantiations or are
> inside
>
> Modified: cfe/trunk/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp?rev=325678&r1=325677&r2=325678&view=diff
> ==
> --- cfe/trunk/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp (original)
> +++ cfe/trunk/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp Wed Feb 21
> 05:51:27 2018
> @@ -1623,6 +1623,14 @@ TEST(IsTemplateInstantiation, MatchesExp
>"template class X;",
> cxxRecordDecl(isTemplateInstantiation(), hasDescendant(
>fieldDecl(hasType(recordDecl(hasName("A";
> +
> + // Make sure that we match the instantiation instead of the template
> + // definition by checking whether the member function is present.
> + EXPECT_TRUE(
> + matches("template class X { void f() { T t; } };"
> + "extern template class X;",
> + cxxRecordDecl(isTemplateInstantiation(),
> +unless(hasDescendant(varDecl(hasName("t")));
> }
>
> TEST(IsTemplateInstantiation,
>
>
> ___
> 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] D43567: [ASTMatchers] isTemplateInstantiation: also match explicit instantiation declaration.
This revision was automatically updated to reflect the committed changes.
Closed by commit rL325678: [ASTMatchers] isTemplateInstantiation: also match
explicit instantiation… (authored by ioeric, committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
https://reviews.llvm.org/D43567
Files:
cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h
cfe/trunk/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
Index: cfe/trunk/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- cfe/trunk/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ cfe/trunk/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -1623,6 +1623,14 @@
"template class X;",
cxxRecordDecl(isTemplateInstantiation(), hasDescendant(
fieldDecl(hasType(recordDecl(hasName("A";
+
+ // Make sure that we match the instantiation instead of the template
+ // definition by checking whether the member function is present.
+ EXPECT_TRUE(
+ matches("template class X { void f() { T t; } };"
+ "extern template class X;",
+ cxxRecordDecl(isTemplateInstantiation(),
+unless(hasDescendant(varDecl(hasName("t")));
}
TEST(IsTemplateInstantiation,
Index: cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h
===
--- cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h
+++ cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h
@@ -4649,6 +4649,10 @@
/// \code
/// template class X {}; class A {}; template class X;
/// \endcode
+/// or
+/// \code
+/// template class X {}; class A {}; extern template class X;
+/// \endcode
/// cxxRecordDecl(hasName("::X"), isTemplateInstantiation())
/// matches the template instantiation of X.
///
@@ -4666,7 +4670,9 @@
CXXRecordDecl)) {
return (Node.getTemplateSpecializationKind() == TSK_ImplicitInstantiation ||
Node.getTemplateSpecializationKind() ==
- TSK_ExplicitInstantiationDefinition);
+ TSK_ExplicitInstantiationDefinition ||
+ Node.getTemplateSpecializationKind() ==
+ TSK_ExplicitInstantiationDeclaration);
}
/// \brief Matches declarations that are template instantiations or are inside
Index: cfe/trunk/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- cfe/trunk/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ cfe/trunk/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -1623,6 +1623,14 @@
"template class X;",
cxxRecordDecl(isTemplateInstantiation(), hasDescendant(
fieldDecl(hasType(recordDecl(hasName("A";
+
+ // Make sure that we match the instantiation instead of the template
+ // definition by checking whether the member function is present.
+ EXPECT_TRUE(
+ matches("template class X { void f() { T t; } };"
+ "extern template class X;",
+ cxxRecordDecl(isTemplateInstantiation(),
+unless(hasDescendant(varDecl(hasName("t")));
}
TEST(IsTemplateInstantiation,
Index: cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h
===
--- cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h
+++ cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h
@@ -4649,6 +4649,10 @@
/// \code
/// template class X {}; class A {}; template class X;
/// \endcode
+/// or
+/// \code
+/// template class X {}; class A {}; extern template class X;
+/// \endcode
/// cxxRecordDecl(hasName("::X"), isTemplateInstantiation())
/// matches the template instantiation of X.
///
@@ -4666,7 +4670,9 @@
CXXRecordDecl)) {
return (Node.getTemplateSpecializationKind() == TSK_ImplicitInstantiation ||
Node.getTemplateSpecializationKind() ==
- TSK_ExplicitInstantiationDefinition);
+ TSK_ExplicitInstantiationDefinition ||
+ Node.getTemplateSpecializationKind() ==
+ TSK_ExplicitInstantiationDeclaration);
}
/// \brief Matches declarations that are template instantiations or are inside
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r325678 - [ASTMatchers] isTemplateInstantiation: also match explicit instantiation declaration.
Oops, I only saw this after landing the patch. I'll send another one to
update the doc. Sorry about that!
On Wed, Feb 21, 2018 at 2:54 PM Aaron Ballman
wrote:
> On Wed, Feb 21, 2018 at 8:51 AM, Eric Liu via cfe-commits
> wrote:
> > Author: ioeric
> > Date: Wed Feb 21 05:51:27 2018
> > New Revision: 325678
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=325678&view=rev
> > Log:
> > [ASTMatchers] isTemplateInstantiation: also match explicit instantiation
> declaration.
> >
> > Summary:
> > Example:
> > template class X {}; class A {};
> > // Explicit instantiation declaration.
> > extern template class X;
> >
> > Reviewers: bkramer
> >
> > Subscribers: klimek, cfe-commits
> >
> > Differential Revision: https://reviews.llvm.org/D43567
> >
> > Modified:
> > cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h
> > cfe/trunk/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
>
> Please regenerate the AST matcher documentation as well.
>
> ~Aaron
>
> >
> > Modified: cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h
> > URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h?rev=325678&r1=325677&r2=325678&view=diff
> >
> ==
> > --- cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h (original)
> > +++ cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h Wed Feb 21
> 05:51:27 2018
> > @@ -4649,6 +4649,10 @@ AST_MATCHER_P(UsingShadowDecl, hasTarget
> > /// \code
> > /// template class X {}; class A {}; template class X;
> > /// \endcode
> > +/// or
> > +/// \code
> > +/// template class X {}; class A {}; extern template
> class X;
> > +/// \endcode
> > /// cxxRecordDecl(hasName("::X"), isTemplateInstantiation())
> > /// matches the template instantiation of X.
> > ///
> > @@ -4666,7 +4670,9 @@ AST_POLYMORPHIC_MATCHER(isTemplateInstan
> > CXXRecordDecl))
> {
> >return (Node.getTemplateSpecializationKind() ==
> TSK_ImplicitInstantiation ||
> >Node.getTemplateSpecializationKind() ==
> > - TSK_ExplicitInstantiationDefinition);
> > + TSK_ExplicitInstantiationDefinition ||
> > + Node.getTemplateSpecializationKind() ==
> > + TSK_ExplicitInstantiationDeclaration);
> > }
> >
> > /// \brief Matches declarations that are template instantiations or are
> inside
> >
> > Modified: cfe/trunk/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp?rev=325678&r1=325677&r2=325678&view=diff
> >
> ==
> > --- cfe/trunk/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
> (original)
> > +++ cfe/trunk/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp Wed Feb
> 21 05:51:27 2018
> > @@ -1623,6 +1623,14 @@ TEST(IsTemplateInstantiation, MatchesExp
> >"template class X;",
> > cxxRecordDecl(isTemplateInstantiation(), hasDescendant(
> >fieldDecl(hasType(recordDecl(hasName("A";
> > +
> > + // Make sure that we match the instantiation instead of the template
> > + // definition by checking whether the member function is present.
> > + EXPECT_TRUE(
> > + matches("template class X { void f() { T t; } };"
> > + "extern template class X;",
> > + cxxRecordDecl(isTemplateInstantiation(),
> > +
> unless(hasDescendant(varDecl(hasName("t")));
> > }
> >
> > TEST(IsTemplateInstantiation,
> >
> >
> > ___
> > 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: r325678 - [ASTMatchers] isTemplateInstantiation: also match explicit instantiation declaration.
On Wed, Feb 21, 2018 at 8:56 AM, Eric Liu wrote:
> Oops, I only saw this after landing the patch. I'll send another one to
> update the doc. Sorry about that!
No worries! I didn't see the patch until after the commit anyway.
Thanks for taking care of it!
~Aaron
>
> On Wed, Feb 21, 2018 at 2:54 PM Aaron Ballman
> wrote:
>>
>> On Wed, Feb 21, 2018 at 8:51 AM, Eric Liu via cfe-commits
>> wrote:
>> > Author: ioeric
>> > Date: Wed Feb 21 05:51:27 2018
>> > New Revision: 325678
>> >
>> > URL: http://llvm.org/viewvc/llvm-project?rev=325678&view=rev
>> > Log:
>> > [ASTMatchers] isTemplateInstantiation: also match explicit instantiation
>> > declaration.
>> >
>> > Summary:
>> > Example:
>> > template class X {}; class A {};
>> > // Explicit instantiation declaration.
>> > extern template class X;
>> >
>> > Reviewers: bkramer
>> >
>> > Subscribers: klimek, cfe-commits
>> >
>> > Differential Revision: https://reviews.llvm.org/D43567
>> >
>> > Modified:
>> > cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h
>> > cfe/trunk/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
>>
>> Please regenerate the AST matcher documentation as well.
>>
>> ~Aaron
>>
>> >
>> > Modified: cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h
>> > URL:
>> > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h?rev=325678&r1=325677&r2=325678&view=diff
>> >
>> > ==
>> > --- cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h (original)
>> > +++ cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h Wed Feb 21
>> > 05:51:27 2018
>> > @@ -4649,6 +4649,10 @@ AST_MATCHER_P(UsingShadowDecl, hasTarget
>> > /// \code
>> > /// template class X {}; class A {}; template class
>> > X;
>> > /// \endcode
>> > +/// or
>> > +/// \code
>> > +/// template class X {}; class A {}; extern template
>> > class X;
>> > +/// \endcode
>> > /// cxxRecordDecl(hasName("::X"), isTemplateInstantiation())
>> > /// matches the template instantiation of X.
>> > ///
>> > @@ -4666,7 +4670,9 @@ AST_POLYMORPHIC_MATCHER(isTemplateInstan
>> > CXXRecordDecl))
>> > {
>> >return (Node.getTemplateSpecializationKind() ==
>> > TSK_ImplicitInstantiation ||
>> >Node.getTemplateSpecializationKind() ==
>> > - TSK_ExplicitInstantiationDefinition);
>> > + TSK_ExplicitInstantiationDefinition ||
>> > + Node.getTemplateSpecializationKind() ==
>> > + TSK_ExplicitInstantiationDeclaration);
>> > }
>> >
>> > /// \brief Matches declarations that are template instantiations or are
>> > inside
>> >
>> > Modified: cfe/trunk/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
>> > URL:
>> > http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp?rev=325678&r1=325677&r2=325678&view=diff
>> >
>> > ==
>> > --- cfe/trunk/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
>> > (original)
>> > +++ cfe/trunk/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp Wed Feb
>> > 21 05:51:27 2018
>> > @@ -1623,6 +1623,14 @@ TEST(IsTemplateInstantiation, MatchesExp
>> >"template class X;",
>> > cxxRecordDecl(isTemplateInstantiation(), hasDescendant(
>> >fieldDecl(hasType(recordDecl(hasName("A";
>> > +
>> > + // Make sure that we match the instantiation instead of the template
>> > + // definition by checking whether the member function is present.
>> > + EXPECT_TRUE(
>> > + matches("template class X { void f() { T t; } };"
>> > + "extern template class X;",
>> > + cxxRecordDecl(isTemplateInstantiation(),
>> > +
>> > unless(hasDescendant(varDecl(hasName("t")));
>> > }
>> >
>> > TEST(IsTemplateInstantiation,
>> >
>> >
>> > ___
>> > 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] D43572: [Sema] Improve test coverage of narrowing conversion diagnostics
miyuki created this revision.
miyuki added reviewers: faisalv, rsmith.
This patch adds tests of narrowing conversion diagnostics for the
'unscoped enum -> integer' case.q
https://reviews.llvm.org/D43572
Files:
test/CXX/dcl.decl/dcl.init/dcl.init.list/p7-0x.cpp
Index: test/CXX/dcl.decl/dcl.init/dcl.init.list/p7-0x.cpp
===
--- test/CXX/dcl.decl/dcl.init/dcl.init.list/p7-0x.cpp
+++ test/CXX/dcl.decl/dcl.init/dcl.init.list/p7-0x.cpp
@@ -147,8 +147,10 @@
void shrink_int() {
// Not a constant expression.
short s = 1;
+ UnscopedEnum e = EnumVal;
unsigned short us = 1;
Agg c1 = {s}; // expected-error {{ cannot be narrowed }}
expected-note {{silence}}
+ Agg c2 = {e}; // expected-error {{ cannot be narrowed }}
expected-note {{silence}}
Agg s1 = {s}; // expected-error {{ cannot be narrowed }}
expected-note {{silence}}
Agg s2 = {us}; // expected-error {{ cannot be narrowed }}
expected-note {{silence}}
@@ -158,16 +160,19 @@
// long).
long l1 = 1;
Agg i1 = {l1}; // expected-error {{ cannot be narrowed }}
expected-note {{silence}}
+ Agg i2 = {e}; // OK
long long ll = 1;
Agg l2 = {ll}; // OK
// Constants.
- Agg c2 = {127}; // OK
- Agg c3 = {300}; // expected-error {{ cannot be narrowed }}
expected-note {{silence}} expected-warning {{changes value}}
-
- Agg i2 = {0x7FFFU}; // OK
- Agg i3 = {0x8000U}; // expected-error {{ cannot be narrowed }}
expected-note {{silence}}
- Agg i4 = {-0x8000L}; // expected-error {{ cannot be
narrowed }} expected-note {{silence}}
+ Agg c3 = {127}; // OK
+ Agg c4 = {300}; // expected-error {{ cannot be narrowed }}
expected-note {{silence}} expected-warning {{changes value}}
+ Agg c5 = {EnumVal}; // expected-error {{ cannot be narrowed }}
expected-note {{silence}} expected-warning {{changes value}}
+
+ Agg i3 = {0x7FFFU}; // OK
+ Agg i4 = {EnumVal}; // OK
+ Agg i5 = {0x8000U}; // expected-error {{ cannot be narrowed }}
expected-note {{silence}}
+ Agg i6 = {-0x8000L}; // expected-error {{ cannot be
narrowed }} expected-note {{silence}}
// Bool is also an integer type, but conversions to it are a different AST
// node.
Index: test/CXX/dcl.decl/dcl.init/dcl.init.list/p7-0x.cpp
===
--- test/CXX/dcl.decl/dcl.init/dcl.init.list/p7-0x.cpp
+++ test/CXX/dcl.decl/dcl.init/dcl.init.list/p7-0x.cpp
@@ -147,8 +147,10 @@
void shrink_int() {
// Not a constant expression.
short s = 1;
+ UnscopedEnum e = EnumVal;
unsigned short us = 1;
Agg c1 = {s}; // expected-error {{ cannot be narrowed }} expected-note {{silence}}
+ Agg c2 = {e}; // expected-error {{ cannot be narrowed }} expected-note {{silence}}
Agg s1 = {s}; // expected-error {{ cannot be narrowed }} expected-note {{silence}}
Agg s2 = {us}; // expected-error {{ cannot be narrowed }} expected-note {{silence}}
@@ -158,16 +160,19 @@
// long).
long l1 = 1;
Agg i1 = {l1}; // expected-error {{ cannot be narrowed }} expected-note {{silence}}
+ Agg i2 = {e}; // OK
long long ll = 1;
Agg l2 = {ll}; // OK
// Constants.
- Agg c2 = {127}; // OK
- Agg c3 = {300}; // expected-error {{ cannot be narrowed }} expected-note {{silence}} expected-warning {{changes value}}
-
- Agg i2 = {0x7FFFU}; // OK
- Agg i3 = {0x8000U}; // expected-error {{ cannot be narrowed }} expected-note {{silence}}
- Agg i4 = {-0x8000L}; // expected-error {{ cannot be narrowed }} expected-note {{silence}}
+ Agg c3 = {127}; // OK
+ Agg c4 = {300}; // expected-error {{ cannot be narrowed }} expected-note {{silence}} expected-warning {{changes value}}
+ Agg c5 = {EnumVal}; // expected-error {{ cannot be narrowed }} expected-note {{silence}} expected-warning {{changes value}}
+
+ Agg i3 = {0x7FFFU}; // OK
+ Agg i4 = {EnumVal}; // OK
+ Agg i5 = {0x8000U}; // expected-error {{ cannot be narrowed }} expected-note {{silence}}
+ Agg i6 = {-0x8000L}; // expected-error {{ cannot be narrowed }} expected-note {{silence}}
// Bool is also an integer type, but conversions to it are a different AST
// node.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42645: New simple Checker for mmap calls
emaste added inline comments. Comment at: include/clang/StaticAnalyzer/Checkers/Checkers.td:417-419 +// Operating systems specific PROT_READ/PROT_WRITE values is not implemented, +// thus ought to be overriden with the proper analyser-config variables +// remain in alpha until the state changes I'm a bit confused by this comment; this checker works as-is for most common operating system cases, correct? Comment at: lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp:64 + if (!BT) +BT.reset(new BugType(this, "W^X check fails, Write Exec prot flags set", "Security")); + `Write & Exec` (or `Write and Exec`) perhaps (assuming it doesn't become over-long then)? https://reviews.llvm.org/D42645 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43378: FreeBSD driver / Xray flags moving pthread to compile flags.
emaste accepted this revision. emaste added a comment. In https://reviews.llvm.org/D43378#1010574, @devnexen wrote: > As I see only x86_64 arch implements everything (e.g. custom event), making > things easier maybe. arm family might be enabled, power pc might need to > rewrite as x86_64 arch some linux-ism ... might be doable in a timely manner > I'd say (to take with a grain of salt though). Ok, thanks. I am most interested in adding at least 64- and 32-bit Arm initially. https://reviews.llvm.org/D43378 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43550: [clangd] Not collect include headers for dynamic index for now.
ioeric added inline comments.
Comment at: clangd/index/FileIndex.cpp:18
-const CanonicalIncludes *canonicalIncludesForSystemHeaders() {
- static const auto *Includes = [] {
ilya-biryukov wrote:
> Should we also remove the mutex from `CanonicalIncludes` now?
I am inclined to keeping it as it brings thread-safety to `CanonicalIncludes`
(which we wouldn't need to worry about in the future), and the current users
are not in performance critical code path anyway.
Comment at: clangd/index/SymbolCollector.cpp:172
+
+ auto U = toURI(SM, SM.getFilename(Loc), Opts);
+ if (!U)
ilya-biryukov wrote:
> Maybe simplify to `return toURI(...)`?
> Or is there a reason why it doesn't work?
No reason... #this is what happened when copy-paste code...
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D43550
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43550: [clangd] Not collect include headers for dynamic index for now.
ioeric updated this revision to Diff 135244.
ioeric marked 2 inline comments as done.
ioeric added a comment.
Properly use toURI.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D43550
Files:
clangd/CodeComplete.cpp
clangd/index/FileIndex.cpp
clangd/index/Index.h
clangd/index/SymbolCollector.cpp
unittests/clangd/FileIndexTests.cpp
unittests/clangd/SymbolCollectorTests.cpp
Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -585,7 +585,7 @@
runSymbolCollector("class Foo {};", /*Main=*/"");
EXPECT_THAT(Symbols,
UnorderedElementsAre(AllOf(QName("Foo"), DeclURI(TestHeaderURI),
- IncludeHeader("";
+ IncludeHeader(TestHeaderURI;
}
#ifndef LLVM_ON_WIN32
Index: unittests/clangd/FileIndexTests.cpp
===
--- unittests/clangd/FileIndexTests.cpp
+++ unittests/clangd/FileIndexTests.cpp
@@ -181,19 +181,19 @@
EXPECT_THAT(match(M, Req), UnorderedElementsAre("X"));
}
-#ifndef LLVM_ON_WIN32
-TEST(FileIndexTest, CanonicalizeSystemHeader) {
+TEST(FileIndexTest, NoIncludeCollected) {
FileIndex M;
- std::string File = testPath("bits/basic_string");
- M.update(File, build(File, "class string {};").getPointer());
+ M.update("f", build("f", "class string {};").getPointer());
FuzzyFindRequest Req;
Req.Query = "";
+ bool SeenSymbol = false;
M.fuzzyFind(Req, [&](const Symbol &Sym) {
-EXPECT_EQ(Sym.Detail->IncludeHeader, "");
+EXPECT_TRUE(Sym.Detail->IncludeHeader.empty());
+SeenSymbol = true;
});
+ EXPECT_TRUE(SeenSymbol);
}
-#endif
} // namespace
} // namespace clangd
Index: clangd/index/SymbolCollector.cpp
===
--- clangd/index/SymbolCollector.cpp
+++ clangd/index/SymbolCollector.cpp
@@ -150,8 +150,9 @@
}
}
-/// Gets a canonical include ( or "header") for header of \p Loc.
-/// Returns None if the header has no canonical include.
+/// Gets a canonical include (URI of the header or or "header") for
+/// header of \p Loc.
+/// Returns None if fails to get include header for \p Loc.
/// FIXME: we should handle .inc files whose symbols are expected be exported by
/// their containing headers.
llvm::Optional
@@ -167,10 +168,8 @@
? Mapped.str()
: ("\"" + Mapped + "\"").str();
}
- // If the header path is the same as the file path of the declaration, we skip
- // storing the #include path; users can use the URI in declaration location to
- // calculate the #include path.
- return llvm::None;
+
+ return toURI(SM, SM.getFilename(Loc), Opts);
}
// Return the symbol location of the given declaration `D`.
Index: clangd/index/Index.h
===
--- clangd/index/Index.h
+++ clangd/index/Index.h
@@ -162,8 +162,8 @@
/// directly. When this is a URI, the exact #include path needs to be
/// calculated according to the URI scheme.
///
-/// If empty, FileURI in CanonicalDeclaration should be used to calculate
-/// the #include path.
+/// This is a canonical include for the symbol and can be different from
+/// FileURI in the CanonicalDeclaration.
llvm::StringRef IncludeHeader;
};
Index: clangd/index/FileIndex.cpp
===
--- clangd/index/FileIndex.cpp
+++ clangd/index/FileIndex.cpp
@@ -15,29 +15,22 @@
namespace clangd {
namespace {
-const CanonicalIncludes *canonicalIncludesForSystemHeaders() {
- static const auto *Includes = [] {
-auto *I = new CanonicalIncludes();
-addSystemHeadersMapping(I);
-return I;
- }();
- return Includes;
-}
-
/// Retrieves namespace and class level symbols in \p Decls.
std::unique_ptr indexAST(ASTContext &Ctx,
std::shared_ptr PP,
llvm::ArrayRef Decls) {
SymbolCollector::Options CollectorOpts;
// Although we do not index symbols in main files (e.g. cpp file), information
// in main files like definition locations of class declarations will still be
// collected; thus, the index works for go-to-definition.
- // FIXME(ioeric): handle IWYU pragma for dynamic index. We might want to make
- // SymbolCollector always provides include canonicalization (e.g. IWYU, STL).
// FIXME(ioeric): get rid of `IndexMainFiles` as this is always set to false.
CollectorOpts.IndexMainFiles = false;
- CollectorOpts.CollectIncludePath = true;
- CollectorOpts.Includes = canonicalIncludesForSystemHeaders();
+ // FIXME(ioeric): we might also want to collect include headers. We would need
+ // to make sure all includes are canonicalized (with CanonicalIncl
r325682 - [ASTMatchers] Regenerate documentation after r325678
Author: ioeric Date: Wed Feb 21 06:22:42 2018 New Revision: 325682 URL: http://llvm.org/viewvc/llvm-project?rev=325682&view=rev Log: [ASTMatchers] Regenerate documentation after r325678 Modified: cfe/trunk/docs/LibASTMatchersReference.html Modified: cfe/trunk/docs/LibASTMatchersReference.html URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/LibASTMatchersReference.html?rev=325682&r1=325681&r2=325682&view=diff == --- cfe/trunk/docs/LibASTMatchersReference.html (original) +++ cfe/trunk/docs/LibASTMatchersReference.html Wed Feb 21 06:22:42 2018 @@ -2376,6 +2376,8 @@ Given templateclass X {}; class A {}; X x; or template class X {}; class A {}; template class X; +or + template class X {}; class A {}; extern template class X; cxxRecordDecl(hasName("::X"), isTemplateInstantiation()) matches the template instantiation of X. @@ -2913,6 +2915,8 @@ Given template class X {}; class A {}; X x; or template class X {}; class A {}; template class X; +or + template class X {}; class A {}; extern template class X; cxxRecordDecl(hasName("::X"), isTemplateInstantiation()) matches the template instantiation of X. @@ -3873,6 +3877,8 @@ Given template class X {}; class A {}; X x; or template class X {}; class A {}; template class X; +or + template class X {}; class A {}; extern template class X; cxxRecordDecl(hasName("::X"), isTemplateInstantiation()) matches the template instantiation of X. ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42645: New simple Checker for mmap calls
devnexen added inline comments. Comment at: include/clang/StaticAnalyzer/Checkers/Checkers.td:417-419 +// Operating systems specific PROT_READ/PROT_WRITE values is not implemented, +// thus ought to be overriden with the proper analyser-config variables +// remain in alpha until the state changes emaste wrote: > I'm a bit confused by this comment; this checker works as-is for most common > operating system cases, correct? Most of them yes, at least Muslc linux most of glibc I tested too. Not to mention *BSD ... But might be safer to put it as alpha for a start. Comment at: lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp:64 + if (!BT) +BT.reset(new BugType(this, "W^X check fails, Write Exec prot flags set", "Security")); + emaste wrote: > `Write & Exec` (or `Write and Exec`) perhaps (assuming it doesn't become > over-long then)? I kept short intentionally indeed we can always change but the user in order to use it needs to enable it willingly so I assumed the user might know enough about the topic in question. https://reviews.llvm.org/D42645 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D16403: Add scope information to CFG
szepet added a comment. In https://reviews.llvm.org/D16403#1011218, @NoQ wrote: > Yeah, i mean, like, if we change the scope markers to also appear even when > no variables are present in the scope, then it would be possible to replace > loop markers with some of the scope markers, right? OK, probably I am a bit too slow for this, but I dont get it. Yes, it would be possible to replace them IF these markers appears even in case of no variables. However, this patch is based on LocalScopes which practically means that VarDecls are needed. Aaand here we are, it would require a different approach to consistently mark things like LoopExit. Another thing that what should be the TriggerStmt? I guess the Stmt of the loop. So, LoopExit and ScopeExit would be the same but the underlying TriggerStmt would decide which marks a loop and which marks a variable? Then I can just rewrite LoopExit into ScopeEnd. Or if you would like to see that, a subclass of ScopeEnd. However, the main functionality should stay the same (I guess). So, could you help me out what are those things I do not see/understand? Repository: rL LLVM https://reviews.llvm.org/D16403 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42366: [CodeGen] Fix generation of TBAA tags for may-alias accesses
rjmccall added a comment. In https://reviews.llvm.org/D42366#1014157, @kosarev wrote: > I think zero would serve better as the unknown-size value. People who are not > aware of TBAA internals would guess that since zero-sized accesses make no > sense, they are likely to have some special meaning. Similarly, for code that > is supposed to process the size fields of access descriptors zero would be an > obvious "illegal size value". In contrast, UINT64_MAX is just a very large > number that doesn't hint anything on its special purpose. My thoughts exactly. John. Repository: rC Clang https://reviews.llvm.org/D42366 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42645: New simple Checker for mmap calls
emaste added inline comments. Comment at: include/clang/StaticAnalyzer/Checkers/Checkers.td:417-419 +// Operating systems specific PROT_READ/PROT_WRITE values is not implemented, +// thus ought to be overriden with the proper analyser-config variables +// remain in alpha until the state changes devnexen wrote: > emaste wrote: > > I'm a bit confused by this comment; this checker works as-is for most > > common operating system cases, correct? > Most of them yes, at least Muslc linux most of glibc I tested too. Not to > mention *BSD ... But might be safer to put it as alpha for a start. OK - to me it implies that the checker only works (anywhere) if the user provides the flag values. Maybe something like "the defaults are correct for several common operating systems, but may need to be overridden " Comment at: lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp:64 + if (!BT) +BT.reset(new BugType(this, "W^X check fails, Write Exec prot flags set", "Security")); + devnexen wrote: > emaste wrote: > > `Write & Exec` (or `Write and Exec`) perhaps (assuming it doesn't become > > over-long then)? > I kept short intentionally indeed we can always change but the user in order > to use it needs to enable it willingly so I assumed the user might know > enough about the topic in question. Understood. To me it just read as "Write Exec" as one entity. https://reviews.llvm.org/D42645 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42645: New simple Checker for mmap calls
devnexen added inline comments. Comment at: include/clang/StaticAnalyzer/Checkers/Checkers.td:417-419 +// Operating systems specific PROT_READ/PROT_WRITE values is not implemented, +// thus ought to be overriden with the proper analyser-config variables +// remain in alpha until the state changes emaste wrote: > devnexen wrote: > > emaste wrote: > > > I'm a bit confused by this comment; this checker works as-is for most > > > common operating system cases, correct? > > Most of them yes, at least Muslc linux most of glibc I tested too. Not to > > mention *BSD ... But might be safer to put it as alpha for a start. > OK - to me it implies that the checker only works (anywhere) if the user > provides the flag values. Maybe something like "the defaults are correct for > several common operating systems, but may need to be overridden " Fair point, I ll rephrase a bit. https://reviews.llvm.org/D42645 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42645: New simple Checker for mmap calls
devnexen updated this revision to Diff 135258.
devnexen added a comment.
Rephrasing Checkers.td comment
https://reviews.llvm.org/D42645
Files:
include/clang/StaticAnalyzer/Checkers/Checkers.td
lib/StaticAnalyzer/Checkers/CMakeLists.txt
lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
test/Analysis/mmap-writeexec.c
Index: test/Analysis/mmap-writeexec.c
===
--- test/Analysis/mmap-writeexec.c
+++ test/Analysis/mmap-writeexec.c
@@ -0,0 +1,36 @@
+// RUN: %clang_analyze_cc1 -triple i686-unknown-linux -analyzer-checker=alpha.security.MmapWriteExec -analyzer-config alpha.security.MmapWriteExec:MmapProtExec=1 -analyzer-config alpha.security.MmapWriteExec:MmapProtRead=4 -DUSE_ALTERNATIVE_PROT_EXEC_DEFINITION -verify %s
+// RUN: %clang_analyze_cc1 -triple x86_64-unknown-apple-darwin10 -analyzer-checker=alpha.security.MmapWriteExec -verify %s
+
+#define PROT_WRITE 0x02
+#ifndef USE_ALTERNATIVE_PROT_EXEC_DEFINITION
+#define PROT_EXEC 0x04
+#define PROT_READ 0x01
+#else
+#define PROT_EXEC 0x01
+#define PROT_READ 0x04
+#endif
+#define MAP_PRIVATE 0x0002
+#define MAP_ANON0x1000
+#define MAP_FIXED 0x0010
+#define NULL((void *)0)
+
+typedef __typeof(sizeof(int)) size_t;
+void *mmap(void *, size_t, int, int, int, long);
+
+void f1()
+{
+ void *a = mmap(NULL, 16, PROT_READ | PROT_EXEC, MAP_PRIVATE | MAP_ANON, -1, 0); // no-warning
+ void *b = mmap(a, 16, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_FIXED | MAP_ANON, -1, 0); // no-warning
+ void *c = mmap(NULL, 32, PROT_READ | PROT_WRITE | PROT_EXEC, MAP_PRIVATE | MAP_ANON, -1, 0); // expected-warning{{Both PROT_WRITE and PROT_EXEC flags are set. This can lead to exploitable memory regions, which could be overwritten with malicious code}}
+ (void)a;
+ (void)b;
+ (void)c;
+}
+
+void f2()
+{
+ void *(*callm)(void *, size_t, int, int, int, long);
+ callm = mmap;
+ int prot = PROT_WRITE | PROT_EXEC;
+ (void)callm(NULL, 1024, prot, MAP_PRIVATE | MAP_ANON, -1, 0); // expected-warning{{Both PROT_WRITE and PROT_EXEC flags are set. This can lead to exploitable memory regions, which could be overwritten with malicious code}}
+}
Index: lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
+++ lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
@@ -0,0 +1,87 @@
+// MmapWriteExecChecker.cpp - Check for the prot argument -===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+// This checker tests the 3rd argument of mmap's calls to check if
+// it is writable and executable in the same time. It's somehow
+// an optional checker since for example in JIT libraries it is pretty common.
+//
+//===--===//
+
+#include "ClangSACheckers.h"
+
+#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
+#include "clang/StaticAnalyzer/Core/Checker.h"
+#include "clang/StaticAnalyzer/Core/CheckerManager.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+
+using namespace clang;
+using namespace ento;
+using llvm::APSInt;
+
+namespace {
+class MmapWriteExecChecker : public Checker {
+ CallDescription MmapFn;
+ static int ProtWrite;
+ static int ProtExec;
+ static int ProtRead;
+ mutable std::unique_ptr BT;
+public:
+ MmapWriteExecChecker() : MmapFn("mmap", 6) {}
+ void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
+ int ProtExecOv;
+ int ProtReadOv;
+};
+}
+
+int MmapWriteExecChecker::ProtWrite = 0x02;
+int MmapWriteExecChecker::ProtExec = 0x04;
+int MmapWriteExecChecker::ProtRead = 0x01;
+
+void MmapWriteExecChecker::checkPreCall(const CallEvent &Call,
+ CheckerContext &C) const {
+ if (Call.isCalled(MmapFn)) {
+SVal ProtVal = Call.getArgSVal(2);
+Optional ProtLoc = ProtVal.getAs();
+int64_t Prot = ProtLoc->getValue().getSExtValue();
+if (ProtExecOv != ProtExec)
+ ProtExec = ProtExecOv;
+if (ProtReadOv != ProtRead)
+ ProtRead = ProtReadOv;
+
+// Wrong settings
+if (ProtRead == ProtExec)
+ return;
+
+if ((Prot & (ProtWrite | ProtExec)) == (ProtWrite | ProtExec)) {
+ if (!BT)
+BT.reset(new BugType(this, "W^X check fails, Write Exec prot flags set", "Security"));
+
+ ExplodedNode *N = C.generateNonFatalErrorNode();
+ if (!N)
+return;
+
+ auto Report = llvm::make_unique(
+ *BT, "Both PROT_WRITE and PROT_EXEC flags are set. This can "
+ "lead to exploitable memory regions, which could be overwritten "
+ "
r325691 - [clang-format] New API guessLanguage()
Author: benhamilton
Date: Wed Feb 21 07:54:31 2018
New Revision: 325691
URL: http://llvm.org/viewvc/llvm-project?rev=325691&view=rev
Log:
[clang-format] New API guessLanguage()
Summary:
For clients which don't have a filesystem, calling getStyle() doesn't
make much sense (there's no .clang-format files to search for).
In this diff, I hoist out the language-guessing logic from getStyle()
and move it into a new API guessLanguage().
I also added support for guessing the language of files which have no
extension (they could be C++ or ObjC).
Test Plan: New tests added. Ran tests with:
% make -j12 FormatTests && ./tools/clang/unittests/Format/FormatTests
Reviewers: jolesiak, krasimir
Reviewed By: jolesiak, krasimir
Subscribers: klimek, cfe-commits, sammccall
Differential Revision: https://reviews.llvm.org/D43522
Modified:
cfe/trunk/include/clang/Format/Format.h
cfe/trunk/lib/Format/Format.cpp
cfe/trunk/unittests/Format/FormatTest.cpp
Modified: cfe/trunk/include/clang/Format/Format.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Format/Format.h?rev=325691&r1=325690&r2=325691&view=diff
==
--- cfe/trunk/include/clang/Format/Format.h (original)
+++ cfe/trunk/include/clang/Format/Format.h Wed Feb 21 07:54:31 2018
@@ -1981,6 +1981,10 @@ llvm::Expected getStyle(Str
StringRef Code = "",
vfs::FileSystem *FS = nullptr);
+// \brief Guesses the language from the ``FileName`` and ``Code`` to be
formatted.
+// Defaults to FormatStyle::LK_Cpp.
+FormatStyle::LanguageKind guessLanguage(StringRef FileName, StringRef Code);
+
// \brief Returns a string representation of ``Language``.
inline StringRef getLanguageName(FormatStyle::LanguageKind Language) {
switch (Language) {
Modified: cfe/trunk/lib/Format/Format.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=325691&r1=325690&r2=325691&view=diff
==
--- cfe/trunk/lib/Format/Format.cpp (original)
+++ cfe/trunk/lib/Format/Format.cpp Wed Feb 21 07:54:31 2018
@@ -2294,6 +2294,25 @@ static FormatStyle::LanguageKind getLang
return FormatStyle::LK_Cpp;
}
+FormatStyle::LanguageKind guessLanguage(StringRef FileName, StringRef Code) {
+ FormatStyle::LanguageKind result = getLanguageByFileName(FileName);
+ if (result == FormatStyle::LK_Cpp) {
+auto extension = llvm::sys::path::extension(FileName);
+// If there's no file extension (or it's .h), we need to check the contents
+// of the code to see if it contains Objective-C.
+if (extension.empty() || extension == ".h") {
+ std::unique_ptr Env =
+ Environment::CreateVirtualEnvironment(Code, FileName, /*Ranges=*/{});
+ ObjCHeaderStyleGuesser Guesser(*Env, getLLVMStyle());
+ Guesser.process();
+ if (Guesser.isObjC()) {
+result = FormatStyle::LK_ObjC;
+ }
+}
+ }
+ return result;
+}
+
llvm::Expected getStyle(StringRef StyleName, StringRef FileName,
StringRef FallbackStyleName,
StringRef Code, vfs::FileSystem *FS) {
@@ -2301,17 +2320,7 @@ llvm::Expected getStyle(Str
FS = vfs::getRealFileSystem().get();
}
FormatStyle Style = getLLVMStyle();
- Style.Language = getLanguageByFileName(FileName);
-
- if (Style.Language == FormatStyle::LK_Cpp && FileName.endswith(".h")) {
-std::unique_ptr Env =
-Environment::CreateVirtualEnvironment(Code, FileName, /*Ranges=*/{});
-ObjCHeaderStyleGuesser Guesser(*Env, Style);
-Guesser.process();
-if (Guesser.isObjC()) {
- Style.Language = FormatStyle::LK_ObjC;
-}
- }
+ Style.Language = guessLanguage(FileName, Code);
FormatStyle FallbackStyle = getNoStyle();
if (!getPredefinedStyle(FallbackStyleName, Style.Language, &FallbackStyle))
Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=325691&r1=325690&r2=325691&view=diff
==
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Wed Feb 21 07:54:31 2018
@@ -11952,6 +11952,34 @@ TEST_F(FormatTest, StructuredBindings) {
verifyFormat("auto const &[ a, b ] = f();", Spaces);
}
+struct GuessLanguageTestCase {
+ const char *const FileName;
+ const char *const Code;
+ const FormatStyle::LanguageKind ExpectedResult;
+};
+
+class GuessLanguageTest
+: public FormatTest,
+ public ::testing::WithParamInterface {};
+
+TEST_P(GuessLanguageTest, FileAndCode) {
+ auto TestCase = GetParam();
+ EXPECT_EQ(TestCase.ExpectedResult,
+guessLanguage(TestCase.FileName, TestCase.Code));
+}
+
+static const GuessLanguageTestCase TestCases[] = {
+{"foo.
[PATCH] D43570: [OpenCL] Add '-cl-uniform-work-group-size' compile option
Anastasia accepted this revision. Anastasia added a comment. This revision is now accepted and ready to land. LGTM! Thanks! Repository: rC Clang https://reviews.llvm.org/D43570 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43522: [clang-format] New API guessLanguage()
This revision was automatically updated to reflect the committed changes.
Closed by commit rC325691: [clang-format] New API guessLanguage() (authored by
benhamilton, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D43522?vs=135121&id=135261#toc
Repository:
rC Clang
https://reviews.llvm.org/D43522
Files:
include/clang/Format/Format.h
lib/Format/Format.cpp
unittests/Format/FormatTest.cpp
Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -2294,24 +2294,33 @@
return FormatStyle::LK_Cpp;
}
+FormatStyle::LanguageKind guessLanguage(StringRef FileName, StringRef Code) {
+ FormatStyle::LanguageKind result = getLanguageByFileName(FileName);
+ if (result == FormatStyle::LK_Cpp) {
+auto extension = llvm::sys::path::extension(FileName);
+// If there's no file extension (or it's .h), we need to check the contents
+// of the code to see if it contains Objective-C.
+if (extension.empty() || extension == ".h") {
+ std::unique_ptr Env =
+ Environment::CreateVirtualEnvironment(Code, FileName, /*Ranges=*/{});
+ ObjCHeaderStyleGuesser Guesser(*Env, getLLVMStyle());
+ Guesser.process();
+ if (Guesser.isObjC()) {
+result = FormatStyle::LK_ObjC;
+ }
+}
+ }
+ return result;
+}
+
llvm::Expected getStyle(StringRef StyleName, StringRef FileName,
StringRef FallbackStyleName,
StringRef Code, vfs::FileSystem *FS) {
if (!FS) {
FS = vfs::getRealFileSystem().get();
}
FormatStyle Style = getLLVMStyle();
- Style.Language = getLanguageByFileName(FileName);
-
- if (Style.Language == FormatStyle::LK_Cpp && FileName.endswith(".h")) {
-std::unique_ptr Env =
-Environment::CreateVirtualEnvironment(Code, FileName, /*Ranges=*/{});
-ObjCHeaderStyleGuesser Guesser(*Env, Style);
-Guesser.process();
-if (Guesser.isObjC()) {
- Style.Language = FormatStyle::LK_ObjC;
-}
- }
+ Style.Language = guessLanguage(FileName, Code);
FormatStyle FallbackStyle = getNoStyle();
if (!getPredefinedStyle(FallbackStyleName, Style.Language, &FallbackStyle))
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -11952,6 +11952,34 @@
verifyFormat("auto const &[ a, b ] = f();", Spaces);
}
+struct GuessLanguageTestCase {
+ const char *const FileName;
+ const char *const Code;
+ const FormatStyle::LanguageKind ExpectedResult;
+};
+
+class GuessLanguageTest
+: public FormatTest,
+ public ::testing::WithParamInterface {};
+
+TEST_P(GuessLanguageTest, FileAndCode) {
+ auto TestCase = GetParam();
+ EXPECT_EQ(TestCase.ExpectedResult,
+guessLanguage(TestCase.FileName, TestCase.Code));
+}
+
+static const GuessLanguageTestCase TestCases[] = {
+{"foo.cc", "", FormatStyle::LK_Cpp},
+{"foo.m", "", FormatStyle::LK_ObjC},
+{"foo.mm", "", FormatStyle::LK_ObjC},
+{"foo.h", "", FormatStyle::LK_Cpp},
+{"foo.h", "@interface Foo\n@end\n", FormatStyle::LK_ObjC},
+{"foo", "", FormatStyle::LK_Cpp},
+{"foo", "@interface Foo\n@end\n", FormatStyle::LK_ObjC},
+};
+INSTANTIATE_TEST_CASE_P(ValidLanguages, GuessLanguageTest,
+::testing::ValuesIn(TestCases));
+
} // end namespace
} // end namespace format
} // end namespace clang
Index: include/clang/Format/Format.h
===
--- include/clang/Format/Format.h
+++ include/clang/Format/Format.h
@@ -1981,6 +1981,10 @@
StringRef Code = "",
vfs::FileSystem *FS = nullptr);
+// \brief Guesses the language from the ``FileName`` and ``Code`` to be formatted.
+// Defaults to FormatStyle::LK_Cpp.
+FormatStyle::LanguageKind guessLanguage(StringRef FileName, StringRef Code);
+
// \brief Returns a string representation of ``Language``.
inline StringRef getLanguageName(FormatStyle::LanguageKind Language) {
switch (Language) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43522: [clang-format] New API guessLanguage()
This revision was automatically updated to reflect the committed changes.
Closed by commit rL325691: [clang-format] New API guessLanguage() (authored by
benhamilton, committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
https://reviews.llvm.org/D43522
Files:
cfe/trunk/include/clang/Format/Format.h
cfe/trunk/lib/Format/Format.cpp
cfe/trunk/unittests/Format/FormatTest.cpp
Index: cfe/trunk/include/clang/Format/Format.h
===
--- cfe/trunk/include/clang/Format/Format.h
+++ cfe/trunk/include/clang/Format/Format.h
@@ -1981,6 +1981,10 @@
StringRef Code = "",
vfs::FileSystem *FS = nullptr);
+// \brief Guesses the language from the ``FileName`` and ``Code`` to be formatted.
+// Defaults to FormatStyle::LK_Cpp.
+FormatStyle::LanguageKind guessLanguage(StringRef FileName, StringRef Code);
+
// \brief Returns a string representation of ``Language``.
inline StringRef getLanguageName(FormatStyle::LanguageKind Language) {
switch (Language) {
Index: cfe/trunk/lib/Format/Format.cpp
===
--- cfe/trunk/lib/Format/Format.cpp
+++ cfe/trunk/lib/Format/Format.cpp
@@ -2294,24 +2294,33 @@
return FormatStyle::LK_Cpp;
}
+FormatStyle::LanguageKind guessLanguage(StringRef FileName, StringRef Code) {
+ FormatStyle::LanguageKind result = getLanguageByFileName(FileName);
+ if (result == FormatStyle::LK_Cpp) {
+auto extension = llvm::sys::path::extension(FileName);
+// If there's no file extension (or it's .h), we need to check the contents
+// of the code to see if it contains Objective-C.
+if (extension.empty() || extension == ".h") {
+ std::unique_ptr Env =
+ Environment::CreateVirtualEnvironment(Code, FileName, /*Ranges=*/{});
+ ObjCHeaderStyleGuesser Guesser(*Env, getLLVMStyle());
+ Guesser.process();
+ if (Guesser.isObjC()) {
+result = FormatStyle::LK_ObjC;
+ }
+}
+ }
+ return result;
+}
+
llvm::Expected getStyle(StringRef StyleName, StringRef FileName,
StringRef FallbackStyleName,
StringRef Code, vfs::FileSystem *FS) {
if (!FS) {
FS = vfs::getRealFileSystem().get();
}
FormatStyle Style = getLLVMStyle();
- Style.Language = getLanguageByFileName(FileName);
-
- if (Style.Language == FormatStyle::LK_Cpp && FileName.endswith(".h")) {
-std::unique_ptr Env =
-Environment::CreateVirtualEnvironment(Code, FileName, /*Ranges=*/{});
-ObjCHeaderStyleGuesser Guesser(*Env, Style);
-Guesser.process();
-if (Guesser.isObjC()) {
- Style.Language = FormatStyle::LK_ObjC;
-}
- }
+ Style.Language = guessLanguage(FileName, Code);
FormatStyle FallbackStyle = getNoStyle();
if (!getPredefinedStyle(FallbackStyleName, Style.Language, &FallbackStyle))
Index: cfe/trunk/unittests/Format/FormatTest.cpp
===
--- cfe/trunk/unittests/Format/FormatTest.cpp
+++ cfe/trunk/unittests/Format/FormatTest.cpp
@@ -11952,6 +11952,34 @@
verifyFormat("auto const &[ a, b ] = f();", Spaces);
}
+struct GuessLanguageTestCase {
+ const char *const FileName;
+ const char *const Code;
+ const FormatStyle::LanguageKind ExpectedResult;
+};
+
+class GuessLanguageTest
+: public FormatTest,
+ public ::testing::WithParamInterface {};
+
+TEST_P(GuessLanguageTest, FileAndCode) {
+ auto TestCase = GetParam();
+ EXPECT_EQ(TestCase.ExpectedResult,
+guessLanguage(TestCase.FileName, TestCase.Code));
+}
+
+static const GuessLanguageTestCase TestCases[] = {
+{"foo.cc", "", FormatStyle::LK_Cpp},
+{"foo.m", "", FormatStyle::LK_ObjC},
+{"foo.mm", "", FormatStyle::LK_ObjC},
+{"foo.h", "", FormatStyle::LK_Cpp},
+{"foo.h", "@interface Foo\n@end\n", FormatStyle::LK_ObjC},
+{"foo", "", FormatStyle::LK_Cpp},
+{"foo", "@interface Foo\n@end\n", FormatStyle::LK_ObjC},
+};
+INSTANTIATE_TEST_CASE_P(ValidLanguages, GuessLanguageTest,
+::testing::ValuesIn(TestCases));
+
} // end namespace
} // end namespace format
} // end namespace clang
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r325692 - Remove use of the 'gmlt' term from the -fsplit-dwarf-inlining flag description to make it more readily legible
Author: dblaikie Date: Wed Feb 21 08:00:50 2018 New Revision: 325692 URL: http://llvm.org/viewvc/llvm-project?rev=325692&view=rev Log: Remove use of the 'gmlt' term from the -fsplit-dwarf-inlining flag description to make it more readily legible Modified: cfe/trunk/include/clang/Driver/Options.td Modified: cfe/trunk/include/clang/Driver/Options.td URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=325692&r1=325691&r2=325692&view=diff == --- cfe/trunk/include/clang/Driver/Options.td (original) +++ cfe/trunk/include/clang/Driver/Options.td Wed Feb 21 08:00:50 2018 @@ -1631,7 +1631,7 @@ def fdebug_types_section: Flag <["-"], " def fno_debug_types_section: Flag<["-"], "fno-debug-types-section">, Group, Flags<[CC1Option]>; def fsplit_dwarf_inlining: Flag <["-"], "fsplit-dwarf-inlining">, Group, - Flags<[CC1Option]>, HelpText<"Provide gmlt-like debug info in the object/executable to facilitate online symbolication/stack traces in the absence of .dwo/.dwp files when using Split DWARF">; + Flags<[CC1Option]>, HelpText<"Provide minimal debug info in the object/executable to facilitate online symbolication/stack traces in the absence of .dwo/.dwp files when using Split DWARF">; def fno_split_dwarf_inlining: Flag<["-"], "fno-split-dwarf-inlining">, Group, Flags<[CC1Option]>; def fdebug_prefix_map_EQ ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34367: CodeGen: Fix address space of indirect function argument
rjmccall added inline comments.
Comment at: lib/CodeGen/CGCall.cpp:3505
+if (AS != LangAS::Default && AS != LangAS::opencl_private &&
+AS != CGM.getASTAllocaAddressSpace())
+ Flag = CallArg::ByValArgNeedsCopy;
This is an odd condition. What are you really trying to express here?
Something about the IR address space that the AST address space maps to?
The condition `AS != LangAS::Default` kills the optimization for all practical
purposes, by the way, so something does need to change here.
I will note that if CallArg could take an LValue, neither this nor the
alignment check would be required here; they'd just be done by the
call-emission code.
Comment at: lib/CodeGen/CGCall.cpp:3511
+ static_cast(CallArg::NonByValArgNeedsCopy) |
+ static_cast(Flag)));
} else {
Please just add an `operator|` for CallArg::Flag.
Comment at: lib/CodeGen/CGCall.h:216
struct CallArg {
+enum Flag {
+ Default = 0x0,
I thing `Flags` would be a better name.
https://reviews.llvm.org/D34367
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43579: [libcxx] Do not include the C math.h header before __config
miyuki created this revision. miyuki added a reviewer: mclow.lists. Herald added a reviewer: EricWF. Certain C libraries require configuration macros defined in __config to provide the correct functionality for libc++. This patch ensures that the C header math.h is always included after the __config header. It also adds a Windows-specific #if guard for the case when the C math.h file is included the second time, as suggested by Marshall in https://reviews.llvm.org/rL323490. Fixes PR36382. https://reviews.llvm.org/D43579 Files: include/math.h Index: include/math.h === --- include/math.h +++ include/math.h @@ -8,16 +8,6 @@ // //===--===// -// This include lives outside the header guard in order to support an MSVC -// extension which allows users to do: -// -// #define _USE_MATH_DEFINES -// #include -// -// and receive the definitions of mathematical constants, even if -// has previously been included. -#include_next - #ifndef _LIBCPP_MATH_H #define _LIBCPP_MATH_H @@ -308,6 +298,8 @@ #pragma GCC system_header #endif +#include_next + #ifdef __cplusplus // We support including .h headers inside 'extern "C"' contexts, so switch @@ -1494,4 +1486,18 @@ #endif // __cplusplus +#else // _LIBCPP_MATH_H + +// This include lives outside the header guard in order to support an MSVC +// extension which allows users to do: +// +// #define _USE_MATH_DEFINES +// #include +// +// and receive the definitions of mathematical constants, even if +// has previously been included. +#if defined(_LIBCPP_MSVCRT) && defined(_USE_MATH_DEFINES) +#include_next +#endif + #endif // _LIBCPP_MATH_H Index: include/math.h === --- include/math.h +++ include/math.h @@ -8,16 +8,6 @@ // //===--===// -// This include lives outside the header guard in order to support an MSVC -// extension which allows users to do: -// -// #define _USE_MATH_DEFINES -// #include -// -// and receive the definitions of mathematical constants, even if -// has previously been included. -#include_next - #ifndef _LIBCPP_MATH_H #define _LIBCPP_MATH_H @@ -308,6 +298,8 @@ #pragma GCC system_header #endif +#include_next + #ifdef __cplusplus // We support including .h headers inside 'extern "C"' contexts, so switch @@ -1494,4 +1486,18 @@ #endif // __cplusplus +#else // _LIBCPP_MATH_H + +// This include lives outside the header guard in order to support an MSVC +// extension which allows users to do: +// +// #define _USE_MATH_DEFINES +// #include +// +// and receive the definitions of mathematical constants, even if +// has previously been included. +#if defined(_LIBCPP_MSVCRT) && defined(_USE_MATH_DEFINES) +#include_next +#endif + #endif // _LIBCPP_MATH_H ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r325693 - [analyzer] Prevent AnalyzerStatsChecker from crash
Author: szepet
Date: Wed Feb 21 08:06:56 2018
New Revision: 325693
URL: http://llvm.org/viewvc/llvm-project?rev=325693&view=rev
Log:
[analyzer] Prevent AnalyzerStatsChecker from crash
The checker marks the locations where the analyzer creates sinks. However, it
can happen that the sink was created because of a loop which does not contain
condition statement, only breaks in the body. The exhausted block is the block
which should contain the condition but empty, in this case.
This change only emits this marking in order to avoid the undefined behavior.
Differential Revision: https://reviews.llvm.org/D42266
Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/AnalyzerStatsChecker.cpp
cfe/trunk/test/Analysis/analyzer-stats.c
Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/AnalyzerStatsChecker.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/AnalyzerStatsChecker.cpp?rev=325693&r1=325692&r2=325693&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/AnalyzerStatsChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/AnalyzerStatsChecker.cpp Wed Feb 21
08:06:56 2018
@@ -122,6 +122,8 @@ void AnalyzerStatsChecker::checkEndAnaly
E = CE.blocks_exhausted_end(); I != E; ++I) {
const BlockEdge &BE = I->first;
const CFGBlock *Exit = BE.getDst();
+if (Exit->empty())
+ continue;
const CFGElement &CE = Exit->front();
if (Optional CS = CE.getAs()) {
SmallString<128> bufI;
Modified: cfe/trunk/test/Analysis/analyzer-stats.c
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/analyzer-stats.c?rev=325693&r1=325692&r2=325693&view=diff
==
--- cfe/trunk/test/Analysis/analyzer-stats.c (original)
+++ cfe/trunk/test/Analysis/analyzer-stats.c Wed Feb 21 08:06:56 2018
@@ -1,4 +1,4 @@
-// RUN: %clang_analyze_cc1
-analyzer-checker=core,deadcode.DeadStores,debug.Stats -verify
-Wno-unreachable-code -analyzer-opt-analyze-nested-blocks %s
+// RUN: %clang_analyze_cc1
-analyzer-checker=core,deadcode.DeadStores,debug.Stats -verify
-Wno-unreachable-code -analyzer-opt-analyze-nested-blocks -analyzer-max-loop 4
%s
int foo();
@@ -12,3 +12,19 @@ int test() { // expected-warning-re{{tes
a /= 4;
return a;
}
+
+
+int sink() // expected-warning-re{{sink -> Total CFGBlocks: {{[0-9]+}} |
Unreachable CFGBlocks: 1 | Exhausted Block: yes | Empty WorkList: yes}}
+{
+ for (int i = 0; i < 10; ++i) // expected-warning {{(sink): The analyzer
generated a sink at this point}}
+++i;
+
+ return 0;
+}
+
+int emptyConditionLoop() // expected-warning-re{{emptyConditionLoop -> Total
CFGBlocks: {{[0-9]+}} | Unreachable CFGBlocks: 0 | Exhausted Block: yes | Empty
WorkList: yes}}
+{
+ int num = 1;
+ for (;;)
+num++;
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42266: [analyzer] Prevent AnalyzerStatsChecker from crash
This revision was automatically updated to reflect the committed changes.
Closed by commit rC325693: [analyzer] Prevent AnalyzerStatsChecker from crash
(authored by szepet, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D42266?vs=130501&id=135266#toc
Repository:
rC Clang
https://reviews.llvm.org/D42266
Files:
lib/StaticAnalyzer/Checkers/AnalyzerStatsChecker.cpp
test/Analysis/analyzer-stats.c
Index: test/Analysis/analyzer-stats.c
===
--- test/Analysis/analyzer-stats.c
+++ test/Analysis/analyzer-stats.c
@@ -1,4 +1,4 @@
-// RUN: %clang_analyze_cc1
-analyzer-checker=core,deadcode.DeadStores,debug.Stats -verify
-Wno-unreachable-code -analyzer-opt-analyze-nested-blocks %s
+// RUN: %clang_analyze_cc1
-analyzer-checker=core,deadcode.DeadStores,debug.Stats -verify
-Wno-unreachable-code -analyzer-opt-analyze-nested-blocks -analyzer-max-loop 4
%s
int foo();
@@ -12,3 +12,19 @@
a /= 4;
return a;
}
+
+
+int sink() // expected-warning-re{{sink -> Total CFGBlocks: {{[0-9]+}} |
Unreachable CFGBlocks: 1 | Exhausted Block: yes | Empty WorkList: yes}}
+{
+ for (int i = 0; i < 10; ++i) // expected-warning {{(sink): The analyzer
generated a sink at this point}}
+++i;
+
+ return 0;
+}
+
+int emptyConditionLoop() // expected-warning-re{{emptyConditionLoop -> Total
CFGBlocks: {{[0-9]+}} | Unreachable CFGBlocks: 0 | Exhausted Block: yes | Empty
WorkList: yes}}
+{
+ int num = 1;
+ for (;;)
+num++;
+}
Index: lib/StaticAnalyzer/Checkers/AnalyzerStatsChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/AnalyzerStatsChecker.cpp
+++ lib/StaticAnalyzer/Checkers/AnalyzerStatsChecker.cpp
@@ -122,6 +122,8 @@
E = CE.blocks_exhausted_end(); I != E; ++I) {
const BlockEdge &BE = I->first;
const CFGBlock *Exit = BE.getDst();
+if (Exit->empty())
+ continue;
const CFGElement &CE = Exit->front();
if (Optional CS = CE.getAs()) {
SmallString<128> bufI;
Index: test/Analysis/analyzer-stats.c
===
--- test/Analysis/analyzer-stats.c
+++ test/Analysis/analyzer-stats.c
@@ -1,4 +1,4 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,deadcode.DeadStores,debug.Stats -verify -Wno-unreachable-code -analyzer-opt-analyze-nested-blocks %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,deadcode.DeadStores,debug.Stats -verify -Wno-unreachable-code -analyzer-opt-analyze-nested-blocks -analyzer-max-loop 4 %s
int foo();
@@ -12,3 +12,19 @@
a /= 4;
return a;
}
+
+
+int sink() // expected-warning-re{{sink -> Total CFGBlocks: {{[0-9]+}} | Unreachable CFGBlocks: 1 | Exhausted Block: yes | Empty WorkList: yes}}
+{
+ for (int i = 0; i < 10; ++i) // expected-warning {{(sink): The analyzer generated a sink at this point}}
+++i;
+
+ return 0;
+}
+
+int emptyConditionLoop() // expected-warning-re{{emptyConditionLoop -> Total CFGBlocks: {{[0-9]+}} | Unreachable CFGBlocks: 0 | Exhausted Block: yes | Empty WorkList: yes}}
+{
+ int num = 1;
+ for (;;)
+num++;
+}
Index: lib/StaticAnalyzer/Checkers/AnalyzerStatsChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/AnalyzerStatsChecker.cpp
+++ lib/StaticAnalyzer/Checkers/AnalyzerStatsChecker.cpp
@@ -122,6 +122,8 @@
E = CE.blocks_exhausted_end(); I != E; ++I) {
const BlockEdge &BE = I->first;
const CFGBlock *Exit = BE.getDst();
+if (Exit->empty())
+ continue;
const CFGElement &CE = Exit->front();
if (Optional CS = CE.getAs()) {
SmallString<128> bufI;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r325694 - [clangd] Update canonical header mapping for STL
Author: ioeric
Date: Wed Feb 21 08:17:25 2018
New Revision: 325694
URL: http://llvm.org/viewvc/llvm-project?rev=325694&view=rev
Log:
[clangd] Update canonical header mapping for STL
Modified:
clang-tools-extra/trunk/clangd/index/CanonicalIncludes.cpp
Modified: clang-tools-extra/trunk/clangd/index/CanonicalIncludes.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/CanonicalIncludes.cpp?rev=325694&r1=325693&r2=325694&view=diff
==
--- clang-tools-extra/trunk/clangd/index/CanonicalIncludes.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/CanonicalIncludes.cpp Wed Feb 21
08:17:25 2018
@@ -266,6 +266,10 @@ void addSystemHeadersMapping(CanonicalIn
{"cwctype$", ""},
{"cxxabi.h$", ""},
{"debug/debug.h$", ""},
+ {"debug/map.h$", ""},
+ {"debug/multimap.h$", ""},
+ {"debug/multiset.h$", ""},
+ {"debug/set.h$", ""},
{"deque$", ""},
{"exception$", ""},
{"ext/alloc_traits.h$", ""},
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43581: [clang-tidy/google] Fix the Objective-C global variable declaration check 🔧
stephanemoore created this revision.
Herald added a subscriber: cfe-commits.
The current Objective-C global variable declaration check restricts naming that
is permitted by the Objective-C style guide.
The Objective-C style guide states the following:
"Global and file scope constants should have an appropriate prefix. [...]
Constants may use a lowercase k prefix when appropriate"
http://google.github.io/styleguide/objcguide#constants
This change fixes the check to allow two or more capital letters as an
appropriate prefix. This change intentionally avoids making a decision
regarding whether to flag constants that use a two letter prefix (two letter
prefixes are reserved by Apple¹ but many projects seem to violate this
guideline).
❧
(1)
"Two-letter prefixes like these are reserved by Apple for use in framework
classes."
https://developer.apple.com/library/content/documentation/Cocoa/Conceptual/ProgrammingWithObjectiveC/Conventions/Conventions.html
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D43581
Files:
clang-tidy/google/GlobalVariableDeclarationCheck.cpp
test/clang-tidy/google-objc-global-variable-declaration.m
Index: test/clang-tidy/google-objc-global-variable-declaration.m
===
--- test/clang-tidy/google-objc-global-variable-declaration.m
+++ test/clang-tidy/google-objc-global-variable-declaration.m
@@ -30,6 +30,7 @@
// CHECK-FIXES: static NSString* const k_Alpha = @"SecondNotAlpha";
static NSString* const kGood = @"hello";
+static NSString* const XYGood = @"hello";
static NSString* gMyIntGood = 0;
@implementation Foo
Index: clang-tidy/google/GlobalVariableDeclarationCheck.cpp
===
--- clang-tidy/google/GlobalVariableDeclarationCheck.cpp
+++ clang-tidy/google/GlobalVariableDeclarationCheck.cpp
@@ -72,7 +72,7 @@
this);
Finder->addMatcher(varDecl(hasGlobalStorage(), hasType(isConstQualified()),
unless(isLocalVariable()),
- unless(matchesName("::k[A-Z]")))
+ unless(matchesName("::(k[A-Z]|[A-Z]{2,})")))
.bind("global_const"),
this);
}
Index: test/clang-tidy/google-objc-global-variable-declaration.m
===
--- test/clang-tidy/google-objc-global-variable-declaration.m
+++ test/clang-tidy/google-objc-global-variable-declaration.m
@@ -30,6 +30,7 @@
// CHECK-FIXES: static NSString* const k_Alpha = @"SecondNotAlpha";
static NSString* const kGood = @"hello";
+static NSString* const XYGood = @"hello";
static NSString* gMyIntGood = 0;
@implementation Foo
Index: clang-tidy/google/GlobalVariableDeclarationCheck.cpp
===
--- clang-tidy/google/GlobalVariableDeclarationCheck.cpp
+++ clang-tidy/google/GlobalVariableDeclarationCheck.cpp
@@ -72,7 +72,7 @@
this);
Finder->addMatcher(varDecl(hasGlobalStorage(), hasType(isConstQualified()),
unless(isLocalVariable()),
- unless(matchesName("::k[A-Z]")))
+ unless(matchesName("::(k[A-Z]|[A-Z]{2,})")))
.bind("global_const"),
this);
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43579: [libcxx] Do not include the C math.h header before __config
mclow.lists accepted this revision. mclow.lists added a comment. This revision is now accepted and ready to land. This LGTM. https://reviews.llvm.org/D43579 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43581: [clang-tidy/google] Fix the Objective-C global variable declaration check 🔧
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.
LGTM with a few additional test cases.
It'd be nice if the style guide linked actually described what a "good" prefix
is rather than make the reader guess.
Comment at: test/clang-tidy/google-objc-global-variable-declaration.m:33
static NSString* const kGood = @"hello";
+static NSString* const XYGood = @"hello";
static NSString* gMyIntGood = 0;
Can you also add the code from the style guide as a test case?
```
extern NSString *const GTLServiceErrorDomain;
typedef NS_ENUM(NSInteger, GTLServiceError) {
GTLServiceErrorQueryResultMissing = -3000,
GTLServiceErrorWaitTimedOut = -3001,
};
```
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D43581
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41102: Setup clang-doc frontend framework
lebedev.ri added a comment.
Nice!
Comment at: clang-doc/BitcodeWriter.cpp:33
+ llvm::IndexedMap BlockIdNameMap;
+ BlockIdNameMap.resize(BI_LAST - BI_FIRST + 1);
+
So here's the thing.
We know how many enumerators we have (`BI_LAST - BI_FIRST + 1`).
And we know how many enumerators we will init (`inits.size()`).
Those numbers should match.
Comment at: clang-doc/BitcodeWriter.cpp:47
+ {BI_COMMENT_BLOCK_ID, "CommentBlock"}};
+ for (const auto &init : inits) {
+BlockIdNameMap[init.first] = init.second;
Can elide `{}`
Comment at: clang-doc/BitcodeWriter.cpp:50
+ }
+ return BlockIdNameMap;
+}();
So i would recommend:
```
static constexpr unsigned ExpectedSize = BI_LAST - BI_FIRST + 1;
BlockIdNameMap.resize(ExpectedSize);
...
static_assert(inits.size() == ExpectedSize, "unexpected count of initializers");
for (const auto &init : inits)
BlockIdNameMap[init.first] = init.second;
assert(BlockIdNameMap.size() == ExpectedSize);
```
Comment at: clang-doc/BitcodeWriter.cpp:56
+ llvm::IndexedMap RecordIdNameMap;
+ RecordIdNameMap.resize(RI_LAST - RI_FIRST + 1);
+
Same
So the differences between the two are:
* functors
* `*_LAST` and `*_FIRST` params
* the actual initializer-list
I guess it might be //eventually// refactored as new `llvm::IndexedMap` ctor.
Comment at: clang-doc/BitcodeWriter.cpp:120
+void ClangDocBinaryWriter::emitHeader(BitstreamWriter &Stream) {
+ // Emit the file header.
+ Stream.Emit((unsigned)'D', BitCodeConstants::SignatureBitSize);
I wonder if this would work?
```
for(char c : StringRef("DOCS"))
Stream.Emit((unsigned)c, BitCodeConstants::SignatureBitSize);
```
Comment at: clang-doc/BitcodeWriter.cpp:258
+ StreamSubBlock Block(Stream, BI_COMMENT_BLOCK_ID);
+ emitStringRecord(I->Text, COMMENT_TEXT, Stream);
+ emitStringRecord(I->Name, COMMENT_NAME, Stream);
Hmm, you could try something like
```
for(const auto& L : std::initializer_list>{{I->Text, COMMENT_TEXT}, {I->Name, COMMENT_NAME}, ...})
emitStringRecord(L.first, S.second, Stream);
```
Comment at: clang-doc/BitcodeWriter.cpp:286
+ emitBlockID(BI_COMMENT_BLOCK_ID, Stream);
+ emitRecordID(COMMENT_KIND, Stream);
+ emitRecordID(COMMENT_TEXT, Stream);
```
for(RecordId RID : {COMMENT_KIND, COMMENT_TEXT, COMMENT_NAME, COMMENT_DIRECTION,
COMMENT_PARAMNAME, COMMENT_CLOSENAME, COMMENT_SELFCLOSING,
COMMENT_EXPLICIT, COMMENT_ATTRKEY, COMMENT_ATTRVAL,
COMMENT_ARG,
COMMENT_POSITION})
emitRecordID(RID, Stream);
```
should work
Comment at: clang-doc/BitcodeWriter.cpp:298
+ emitRecordID(COMMENT_POSITION, Stream);
+ emitStringAbbrev(COMMENT_KIND, BI_COMMENT_BLOCK_ID, Stream);
+ emitStringAbbrev(COMMENT_TEXT, BI_COMMENT_BLOCK_ID, Stream);
```
for(RecordId RID : {COMMENT_KIND, COMMENT_TEXT, COMMENT_NAME, COMMENT_DIRECTION,
COMMENT_PARAMNAME, COMMENT_CLOSENAME})
emitStringAbbrev(RID, BI_COMMENT_BLOCK_ID, Stream);
```
Comment at: clang-doc/BitcodeWriter.cpp:306
+ emitIntAbbrev(COMMENT_EXPLICIT, BI_COMMENT_BLOCK_ID, Stream);
+ emitStringAbbrev(COMMENT_ATTRKEY, BI_COMMENT_BLOCK_ID, Stream);
+ emitStringAbbrev(COMMENT_ATTRVAL, BI_COMMENT_BLOCK_ID, Stream);
```
for(RecordId RID : {COMMENT_ATTRKEY, COMMENT_ATTRVAL, COMMENT_ARG,
COMMENT_POSITION})
emitStringAbbrev(RID, BI_COMMENT_BLOCK_ID, Stream);
```
and maybe in a few other places
Comment at: clang-doc/BitcodeWriter.cpp:407
+
+void ClangDocBinaryWriter::writeBitstream(const EnumInfo &I,
+ BitstreamWriter &Stream,
Hmm, common pattern again
```
void ClangDocBinaryWriter::writeBitstream(const &I,
BitstreamWriter &Stream,
bool writeBlockInfo) {
if (writeBlockInfo) emitBlockInfoBlock(Stream);
StreamSubBlock Block(Stream, BI__BLOCK_ID);
...
}
```
Could be solved if a mapping from `TYPENAME` -> `BI__BLOCK_ID`
can be added.
If LLVM would be using C++14, that'd be easy, but with C++11, it would require
whole new class (although with just a single static variable).
Comment at: clang-doc/BitcodeWriter.h:11
+// This file implements a writer for serializing the clang-doc internal
+// represeentation to LLVM bitcode. The writer takes in a stream and emits the
+// generated bitcode to that stream.
representation
Comment at: clang-doc/BitcodeWriter.h:32
+
+#define VERSION_NUMBER 0
+
`static const unsigned` please :)
I'm not sure where to best to keep it, in anon namespace, here, or in
`BitCodeConstants` stru
[PATCH] D34367: CodeGen: Fix address space of indirect function argument
yaxunl added inline comments. Comment at: lib/CodeGen/CGCall.cpp:3505 +if (AS != LangAS::Default && AS != LangAS::opencl_private && +AS != CGM.getASTAllocaAddressSpace()) + Flag = CallArg::ByValArgNeedsCopy; rjmccall wrote: > This is an odd condition. What are you really trying to express here? > Something about the IR address space that the AST address space maps to? > > The condition `AS != LangAS::Default` kills the optimization for all > practical purposes, by the way, so something does need to change here. > > I will note that if CallArg could take an LValue, neither this nor the > alignment check would be required here; they'd just be done by the > call-emission code. This checks if an indirect by val arg can omit the copying. If the arg is in alloca or default addr space, the backend is able to copy it, so no explicit copying is needed by clang. Otherwise we need clang to insert copying. I will try to replace Flag with LVal and see if it simplifies things. https://reviews.llvm.org/D34367 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43522: [clang-format] New API guessLanguage()
djasper added inline comments.
Comment at: cfe/trunk/lib/Format/Format.cpp:2298
+FormatStyle::LanguageKind guessLanguage(StringRef FileName, StringRef Code) {
+ FormatStyle::LanguageKind result = getLanguageByFileName(FileName);
+ if (result == FormatStyle::LK_Cpp) {
Here and in several other places: Variables should be named with upper camel
case
(https://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly).
Comment at: cfe/trunk/lib/Format/Format.cpp:2308
+ Guesser.process();
+ if (Guesser.isObjC()) {
+result = FormatStyle::LK_ObjC;
In LLVM, we generally don't add braces for single statement ifs.
Comment at: cfe/trunk/lib/Format/Format.cpp:2309
+ if (Guesser.isObjC()) {
+result = FormatStyle::LK_ObjC;
+ }
Why not just return here?
Comment at: cfe/trunk/unittests/Format/FormatTest.cpp:11955
+struct GuessLanguageTestCase {
+ const char *const FileName;
IMO, this is a bit over-engineered. IIUC, this only calls a single free
standing function with two different parameters and expects a certain result.
You don't need this struct, a test fixture or parameterized tests for that.
Just write:
TEST(GuessLanguageTest, FileAndCode) {
EXPECT_EQ(guessLanguage("foo.cc", ""), FormatStyle::LK_Cpp);
EXPECT_EQ(guessLanguage("foo.m", ""), FormatStyle::LK_ObjC);
...
}
Yes, you'd be duplicating the "EXPECT_EQ" and "guessLanguage", but I think
that's actually good here. It makes the tests intuitively readable.
Repository:
rL LLVM
https://reviews.llvm.org/D43522
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42217: Set Module Metadata "AvoidPLT" when -fno-plt is used.
tmsriram added a comment. Ping. This patch goes with https://reviews.llvm.org/D42216, Rafael can you approve this too if you think it is ok? Thanks. https://reviews.llvm.org/D42217 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43522: [clang-format] New API guessLanguage()
benhamilton added inline comments.
Comment at: cfe/trunk/lib/Format/Format.cpp:2298
+FormatStyle::LanguageKind guessLanguage(StringRef FileName, StringRef Code) {
+ FormatStyle::LanguageKind result = getLanguageByFileName(FileName);
+ if (result == FormatStyle::LK_Cpp) {
djasper wrote:
> Here and in several other places: Variables should be named with upper camel
> case
> (https://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly).
Thanks, will send a follow-up.
Comment at: cfe/trunk/lib/Format/Format.cpp:2308
+ Guesser.process();
+ if (Guesser.isObjC()) {
+result = FormatStyle::LK_ObjC;
djasper wrote:
> In LLVM, we generally don't add braces for single statement ifs.
Mmmm.. is this a hard requirement? I've personally been bitten so many times by
adding statements after missing braces, I'd rather add them unless they're
required to not be there by the style guide.
Comment at: cfe/trunk/lib/Format/Format.cpp:2309
+ if (Guesser.isObjC()) {
+result = FormatStyle::LK_ObjC;
+ }
djasper wrote:
> Why not just return here?
I don't like early returns in case an else sneaks in later:
https://llvm.org/docs/CodingStandards.html#don-t-use-else-after-a-return
Comment at: cfe/trunk/unittests/Format/FormatTest.cpp:11955
+struct GuessLanguageTestCase {
+ const char *const FileName;
djasper wrote:
> IMO, this is a bit over-engineered. IIUC, this only calls a single free
> standing function with two different parameters and expects a certain result.
> You don't need this struct, a test fixture or parameterized tests for that.
> Just write:
>
> TEST(GuessLanguageTest, FileAndCode) {
> EXPECT_EQ(guessLanguage("foo.cc", ""), FormatStyle::LK_Cpp);
> EXPECT_EQ(guessLanguage("foo.m", ""), FormatStyle::LK_ObjC);
> ...
> }
>
> Yes, you'd be duplicating the "EXPECT_EQ" and "guessLanguage", but I think
> that's actually good here. It makes the tests intuitively readable.
I hear you. I much prefer it when a single test tests a single issue, so
failures are isolated to the actual change:
https://elgaard.blog/2011/02/06/multiple-asserts-in-a-single-unit-test-method/
If this isn't a hard requirement, I'd like to keep this the way it is.
Repository:
rL LLVM
https://reviews.llvm.org/D43522
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43585: [libunwind] Permit additional compiler flags to be passed to tests.
bsdjhb created this revision. bsdjhb added a reviewer: sdardis. Herald added subscribers: christof, mgorny. This is done via a new LIBUNWIND_TEST_CFLAGS variable. Repository: rUNW libunwind https://reviews.llvm.org/D43585 Files: CMakeLists.txt test/lit.site.cfg.in Index: test/lit.site.cfg.in === --- test/lit.site.cfg.in +++ test/lit.site.cfg.in @@ -22,6 +22,7 @@ config.sysroot = "@LIBUNWIND_SYSROOT@" config.gcc_toolchain= "@LIBUNWIND_GCC_TOOLCHAIN@" config.cxx_ext_threads = "@LIBUNWIND_BUILD_EXTERNAL_THREAD_LIBRARY@" +config.test_cflags = "@LIBUNWIND_TEST_CFLAGS@" # Let the main config do the real work. lit_config.load_config(config, "@LIBUNWIND_SOURCE_DIR@/test/lit.cfg") Index: CMakeLists.txt === --- CMakeLists.txt +++ CMakeLists.txt @@ -139,6 +139,7 @@ set(LIBUNWIND_TARGET_TRIPLE "" CACHE STRING "Target triple for cross compiling.") set(LIBUNWIND_GCC_TOOLCHAIN "" CACHE PATH "GCC toolchain for cross compiling.") set(LIBUNWIND_SYSROOT "" CACHE PATH "Sysroot for cross compiling.") +set(LIBUNWIND_TEST_CFLAGS "" CACHE PATH "Additional compiler flags for test programs.") if (NOT LIBUNWIND_ENABLE_SHARED AND NOT LIBUNWIND_ENABLE_STATIC) message(FATAL_ERROR "libunwind must be built as either a shared or static library.") Index: test/lit.site.cfg.in === --- test/lit.site.cfg.in +++ test/lit.site.cfg.in @@ -22,6 +22,7 @@ config.sysroot = "@LIBUNWIND_SYSROOT@" config.gcc_toolchain= "@LIBUNWIND_GCC_TOOLCHAIN@" config.cxx_ext_threads = "@LIBUNWIND_BUILD_EXTERNAL_THREAD_LIBRARY@" +config.test_cflags = "@LIBUNWIND_TEST_CFLAGS@" # Let the main config do the real work. lit_config.load_config(config, "@LIBUNWIND_SOURCE_DIR@/test/lit.cfg") Index: CMakeLists.txt === --- CMakeLists.txt +++ CMakeLists.txt @@ -139,6 +139,7 @@ set(LIBUNWIND_TARGET_TRIPLE "" CACHE STRING "Target triple for cross compiling.") set(LIBUNWIND_GCC_TOOLCHAIN "" CACHE PATH "GCC toolchain for cross compiling.") set(LIBUNWIND_SYSROOT "" CACHE PATH "Sysroot for cross compiling.") +set(LIBUNWIND_TEST_CFLAGS "" CACHE PATH "Additional compiler flags for test programs.") if (NOT LIBUNWIND_ENABLE_SHARED AND NOT LIBUNWIND_ENABLE_STATIC) message(FATAL_ERROR "libunwind must be built as either a shared or static library.") ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43585: [libunwind] Permit additional compiler flags to be passed to tests.
bsdjhb updated this revision to Diff 135285. bsdjhb added a comment. - Unexpand tabs. Repository: rUNW libunwind https://reviews.llvm.org/D43585 Files: CMakeLists.txt test/lit.site.cfg.in Index: test/lit.site.cfg.in === --- test/lit.site.cfg.in +++ test/lit.site.cfg.in @@ -22,6 +22,7 @@ config.sysroot = "@LIBUNWIND_SYSROOT@" config.gcc_toolchain= "@LIBUNWIND_GCC_TOOLCHAIN@" config.cxx_ext_threads = "@LIBUNWIND_BUILD_EXTERNAL_THREAD_LIBRARY@" +config.test_cflags = "@LIBUNWIND_TEST_CFLAGS@" # Let the main config do the real work. lit_config.load_config(config, "@LIBUNWIND_SOURCE_DIR@/test/lit.cfg") Index: CMakeLists.txt === --- CMakeLists.txt +++ CMakeLists.txt @@ -139,6 +139,7 @@ set(LIBUNWIND_TARGET_TRIPLE "" CACHE STRING "Target triple for cross compiling.") set(LIBUNWIND_GCC_TOOLCHAIN "" CACHE PATH "GCC toolchain for cross compiling.") set(LIBUNWIND_SYSROOT "" CACHE PATH "Sysroot for cross compiling.") +set(LIBUNWIND_TEST_CFLAGS "" CACHE PATH "Additional compiler flags for test programs.") if (NOT LIBUNWIND_ENABLE_SHARED AND NOT LIBUNWIND_ENABLE_STATIC) message(FATAL_ERROR "libunwind must be built as either a shared or static library.") Index: test/lit.site.cfg.in === --- test/lit.site.cfg.in +++ test/lit.site.cfg.in @@ -22,6 +22,7 @@ config.sysroot = "@LIBUNWIND_SYSROOT@" config.gcc_toolchain= "@LIBUNWIND_GCC_TOOLCHAIN@" config.cxx_ext_threads = "@LIBUNWIND_BUILD_EXTERNAL_THREAD_LIBRARY@" +config.test_cflags = "@LIBUNWIND_TEST_CFLAGS@" # Let the main config do the real work. lit_config.load_config(config, "@LIBUNWIND_SOURCE_DIR@/test/lit.cfg") Index: CMakeLists.txt === --- CMakeLists.txt +++ CMakeLists.txt @@ -139,6 +139,7 @@ set(LIBUNWIND_TARGET_TRIPLE "" CACHE STRING "Target triple for cross compiling.") set(LIBUNWIND_GCC_TOOLCHAIN "" CACHE PATH "GCC toolchain for cross compiling.") set(LIBUNWIND_SYSROOT "" CACHE PATH "Sysroot for cross compiling.") +set(LIBUNWIND_TEST_CFLAGS "" CACHE PATH "Additional compiler flags for test programs.") if (NOT LIBUNWIND_ENABLE_SHARED AND NOT LIBUNWIND_ENABLE_STATIC) message(FATAL_ERROR "libunwind must be built as either a shared or static library.") ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43585: [libunwind] Permit additional compiler flags to be passed to tests.
bsdjhb added a comment. @sdardis requested this functionality in the review of https://reviews.llvm.org/D39074. Simon, can you confirm that this works for you in your testing? Repository: rUNW libunwind https://reviews.llvm.org/D43585 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43581: [clang-tidy/google] Fix the Objective-C global variable declaration check 🔧
stephanemoore updated this revision to Diff 135291.
stephanemoore added a comment.
Added excerpts from the Google Objective-C style guide section on constant
naming (http://google.github.io/styleguide/objcguide#constants) as additional
test cases.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D43581
Files:
clang-tidy/google/GlobalVariableDeclarationCheck.cpp
test/clang-tidy/google-objc-global-variable-declaration.m
Index: test/clang-tidy/google-objc-global-variable-declaration.m
===
--- test/clang-tidy/google-objc-global-variable-declaration.m
+++ test/clang-tidy/google-objc-global-variable-declaration.m
@@ -30,8 +30,16 @@
// CHECK-FIXES: static NSString* const k_Alpha = @"SecondNotAlpha";
static NSString* const kGood = @"hello";
+static NSString* const XYGood = @"hello";
static NSString* gMyIntGood = 0;
+extern NSString* const GTLServiceErrorDomain;
+
+typedef NS_ENUM(NSInteger, GTLServiceError) {
+ GTLServiceErrorQueryResultMissing = -3000,
+ GTLServiceErrorWaitTimedOut = -3001,
+};
+
@implementation Foo
- (void)f {
int x = 0;
Index: clang-tidy/google/GlobalVariableDeclarationCheck.cpp
===
--- clang-tidy/google/GlobalVariableDeclarationCheck.cpp
+++ clang-tidy/google/GlobalVariableDeclarationCheck.cpp
@@ -72,7 +72,7 @@
this);
Finder->addMatcher(varDecl(hasGlobalStorage(), hasType(isConstQualified()),
unless(isLocalVariable()),
- unless(matchesName("::k[A-Z]")))
+ unless(matchesName("::(k[A-Z]|[A-Z]{2,})")))
.bind("global_const"),
this);
}
Index: test/clang-tidy/google-objc-global-variable-declaration.m
===
--- test/clang-tidy/google-objc-global-variable-declaration.m
+++ test/clang-tidy/google-objc-global-variable-declaration.m
@@ -30,8 +30,16 @@
// CHECK-FIXES: static NSString* const k_Alpha = @"SecondNotAlpha";
static NSString* const kGood = @"hello";
+static NSString* const XYGood = @"hello";
static NSString* gMyIntGood = 0;
+extern NSString* const GTLServiceErrorDomain;
+
+typedef NS_ENUM(NSInteger, GTLServiceError) {
+ GTLServiceErrorQueryResultMissing = -3000,
+ GTLServiceErrorWaitTimedOut = -3001,
+};
+
@implementation Foo
- (void)f {
int x = 0;
Index: clang-tidy/google/GlobalVariableDeclarationCheck.cpp
===
--- clang-tidy/google/GlobalVariableDeclarationCheck.cpp
+++ clang-tidy/google/GlobalVariableDeclarationCheck.cpp
@@ -72,7 +72,7 @@
this);
Finder->addMatcher(varDecl(hasGlobalStorage(), hasType(isConstQualified()),
unless(isLocalVariable()),
- unless(matchesName("::k[A-Z]")))
+ unless(matchesName("::(k[A-Z]|[A-Z]{2,})")))
.bind("global_const"),
this);
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41102: Setup clang-doc frontend framework
juliehockett added inline comments.
Comment at: clang-doc/BitcodeWriter.cpp:407
+
+void ClangDocBinaryWriter::writeBitstream(const EnumInfo &I,
+ BitstreamWriter &Stream,
lebedev.ri wrote:
> Hmm, common pattern again
> ```
> void ClangDocBinaryWriter::writeBitstream(const &I,
> BitstreamWriter &Stream,
> bool writeBlockInfo) {
> if (writeBlockInfo) emitBlockInfoBlock(Stream);
> StreamSubBlock Block(Stream, BI__BLOCK_ID);
> ...
> }
> ```
> Could be solved if a mapping from `TYPENAME` -> `BI__BLOCK_ID`
> can be added.
> If LLVM would be using C++14, that'd be easy, but with C++11, it would
> require whole new class (although with just a single static variable).
Do you want me to try to write that class, or leave it as it is?
Comment at: clang-doc/Mapper.cpp:113
+ populateInfo(I, D, C);
+ I.Loc.emplace_back(Location{LineNumber, File});
+}
lebedev.ri wrote:
> ```
> I.Loc.emplace_back({LineNumber, File});
> ```
That...doesn't work? Throws a no-matching-function error (suggesting to try
emplacing a Location instead of a
https://reviews.llvm.org/D41102
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41102: Setup clang-doc frontend framework
lebedev.ri added inline comments.
Comment at: clang-doc/BitcodeWriter.cpp:407
+
+void ClangDocBinaryWriter::writeBitstream(const EnumInfo &I,
+ BitstreamWriter &Stream,
juliehockett wrote:
> lebedev.ri wrote:
> > Hmm, common pattern again
> > ```
> > void ClangDocBinaryWriter::writeBitstream(const &I,
> > BitstreamWriter &Stream,
> > bool writeBlockInfo) {
> > if (writeBlockInfo) emitBlockInfoBlock(Stream);
> > StreamSubBlock Block(Stream, BI__BLOCK_ID);
> > ...
> > }
> > ```
> > Could be solved if a mapping from `TYPENAME` ->
> > `BI__BLOCK_ID` can be added.
> > If LLVM would be using C++14, that'd be easy, but with C++11, it would
> > require whole new class (although with just a single static variable).
> Do you want me to try to write that class, or leave it as it is?
It would be something like: (total guesswork, literally just wrote it here,
like rest of the snippets)
```
template
struct MapFromTypeToEnumerator {
static const BlockId id;
};
template <>
struct MapFromTypeToEnumerator {
static const BlockId id = BI_NAMESPACE_BLOCK_ID;
};
void ClangDocBitcodeWriter::writeBitstream(const NamespaceInfo &I) {
EMITINFO(NAMESPACE)
}
...
template
void ClangDocBitcodeWriter::writeBitstream(const TypeInfo &I, bool
writeBlockInfo) {
if (writeBlockInfo) emitBlockInfoBlock();
StreamSubBlockGuard Block(Stream, MapFromTypeToEnumerator::id);
writeBitstream(I);
}
```
Uhm, now that i have wrote it, it does not look as ugly as i though it would
look...
So maybe try integrating that, i *think* it is a bit cleaner?
https://reviews.llvm.org/D41102
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43581: [clang-tidy/google] Fix the Objective-C global variable declaration check 🔧
stephanemoore marked an inline comment as done. stephanemoore added a comment. In https://reviews.llvm.org/D43581#1014664, @aaron.ballman wrote: > LGTM with a few additional test cases. > > It'd be nice if the style guide linked actually described what a "good" > prefix is rather than make the reader guess. I will work on updating the Google Objective-C style guide to provide more clarity regarding what constitutes an appropriate prefix. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D43581 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34367: CodeGen: Fix address space of indirect function argument
rjmccall added inline comments.
Comment at: lib/CodeGen/CGCall.cpp:3510
+ args.add(RValue::getAggregate(Dest.getAddress()), type, L);
}
return;
Please move all this conditionality to the call-emission code; just check
whether you have a load of an aggregate l-value and, if so, add the LValue as
an unloaded aggregate to the argument list.
Comment at: lib/CodeGen/CGCall.h:220
+CallArg(RValue rv, QualType ty, LValue _LV = LValue{})
+: RV(rv), Ty(ty), LV(_LV) {}
+bool hasLValue() const { return LV.getPointer(); }
It should be either an RValue or an LValue, not both. The client needs to do
different things in the different cases. But you should add a convenience
method for getting the argument as an RValue that either returns the stored
RValue or copies out of the stored LValue, and most of the cases can just use
that.
Just overload the constructor and CallArgList::add. I would call the latter
`addUncopiedAggregate`.
https://reviews.llvm.org/D34367
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43584: [test] Permit additional CFLAGS for tests to be set via config.test_cflags.
bsdjhb created this revision.
bsdjhb added a reviewer: sdardis.
Herald added subscribers: cfe-commits, christof.
Currently this is only planned to be used by libunwind's tests.
Repository:
rCXX libc++
https://reviews.llvm.org/D43584
Files:
utils/libcxx/test/config.py
Index: utils/libcxx/test/config.py
===
--- utils/libcxx/test/config.py
+++ utils/libcxx/test/config.py
@@ -583,6 +583,9 @@
# FIXME(EricWF): variant_size.pass.cpp requires a slightly larger
# template depth with older Clang versions.
self.cxx.addFlagIfSupported('-ftemplate-depth=270')
+cflags = self.get_lit_conf('test_cflags')
+if cflags:
+self.cxx.flags += [cflags]
def configure_compile_flags_header_includes(self):
support_path = os.path.join(self.libcxx_src_root, 'test', 'support')
Index: utils/libcxx/test/config.py
===
--- utils/libcxx/test/config.py
+++ utils/libcxx/test/config.py
@@ -583,6 +583,9 @@
# FIXME(EricWF): variant_size.pass.cpp requires a slightly larger
# template depth with older Clang versions.
self.cxx.addFlagIfSupported('-ftemplate-depth=270')
+cflags = self.get_lit_conf('test_cflags')
+if cflags:
+self.cxx.flags += [cflags]
def configure_compile_flags_header_includes(self):
support_path = os.path.join(self.libcxx_src_root, 'test', 'support')
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34367: CodeGen: Fix address space of indirect function argument
yaxunl updated this revision to Diff 135301.
yaxunl added a comment.
Replace Flag with LValue in CallArg.
https://reviews.llvm.org/D34367
Files:
lib/CodeGen/CGCall.cpp
lib/CodeGen/CGCall.h
lib/CodeGen/CGClass.cpp
lib/CodeGen/CGDecl.cpp
lib/CodeGen/CGObjCGNU.cpp
lib/CodeGen/ItaniumCXXABI.cpp
lib/CodeGen/MicrosoftCXXABI.cpp
test/CodeGenCXX/amdgcn-func-arg.cpp
test/CodeGenOpenCL/addr-space-struct-arg.cl
test/CodeGenOpenCL/byval.cl
Index: test/CodeGenOpenCL/byval.cl
===
--- test/CodeGenOpenCL/byval.cl
+++ test/CodeGenOpenCL/byval.cl
@@ -1,5 +1,4 @@
// RUN: %clang_cc1 -emit-llvm -o - -triple amdgcn %s | FileCheck %s
-// RUN: %clang_cc1 -emit-llvm -o - -triple amdgcn---opencl %s | FileCheck %s
struct A {
int x[100];
Index: test/CodeGenOpenCL/addr-space-struct-arg.cl
===
--- test/CodeGenOpenCL/addr-space-struct-arg.cl
+++ test/CodeGenOpenCL/addr-space-struct-arg.cl
@@ -1,5 +1,6 @@
// RUN: %clang_cc1 %s -emit-llvm -o - -O0 -finclude-default-header -ffake-address-space-map -triple i686-pc-darwin | FileCheck -enable-var-scope -check-prefixes=COM,X86 %s
-// RUN: %clang_cc1 %s -emit-llvm -o - -O0 -finclude-default-header -triple amdgcn-amdhsa-amd-amdgizcl | FileCheck -enable-var-scope -check-prefixes=COM,AMD %s
+// RUN: %clang_cc1 %s -emit-llvm -o - -O0 -finclude-default-header -triple amdgcn-amdhsa-amd | FileCheck -enable-var-scope -check-prefixes=COM,AMDGCN %s
+// RUN: %clang_cc1 %s -emit-llvm -o - -cl-std=CL2.0 -O0 -finclude-default-header -triple amdgcn-amdhsa-amd | FileCheck -enable-var-scope -check-prefixes=COM,AMDGCN,AMDGCN20 %s
typedef struct {
int cells[9];
@@ -35,9 +36,12 @@
int2 y[20];
};
+#if __OPENCL_C_VERSION__ >= 200
+struct LargeStructOneMember g_s;
+#endif
// X86-LABEL: define void @foo(%struct.Mat4X4* noalias sret %agg.result, %struct.Mat3X3* byval align 4 %in)
-// AMD-LABEL: define %struct.Mat4X4 @foo([9 x i32] %in.coerce)
+// AMDGCN-LABEL: define %struct.Mat4X4 @foo([9 x i32] %in.coerce)
Mat4X4 __attribute__((noinline)) foo(Mat3X3 in) {
Mat4X4 out;
return out;
@@ -49,15 +53,15 @@
// X86: call void @llvm.memcpy.p0i8.p1i8.i32(i8*
// X86: call void @llvm.memcpy.p1i8.p0i8.i32(i8 addrspace(1)*
-// AMD: load [9 x i32], [9 x i32] addrspace(1)*
-// AMD: call %struct.Mat4X4 @foo([9 x i32]
-// AMD: call void @llvm.memcpy.p1i8.p5i8.i64(i8 addrspace(1)*
+// AMDGCN: load [9 x i32], [9 x i32] addrspace(1)*
+// AMDGCN: call %struct.Mat4X4 @foo([9 x i32]
+// AMDGCN: call void @llvm.memcpy.p1i8.p5i8.i64(i8 addrspace(1)*
kernel void ker(global Mat3X3 *in, global Mat4X4 *out) {
out[0] = foo(in[1]);
}
// X86-LABEL: define void @foo_large(%struct.Mat64X64* noalias sret %agg.result, %struct.Mat32X32* byval align 4 %in)
-// AMD-LABEL: define void @foo_large(%struct.Mat64X64 addrspace(5)* noalias sret %agg.result, %struct.Mat32X32 addrspace(5)* byval align 4 %in)
+// AMDGCN-LABEL: define void @foo_large(%struct.Mat64X64 addrspace(5)* noalias sret %agg.result, %struct.Mat32X32 addrspace(5)* byval align 4 %in)
Mat64X64 __attribute__((noinline)) foo_large(Mat32X32 in) {
Mat64X64 out;
return out;
@@ -68,66 +72,97 @@
// the return value.
// X86: call void @llvm.memcpy.p0i8.p1i8.i32(i8*
// X86: call void @llvm.memcpy.p1i8.p0i8.i32(i8 addrspace(1)*
-// AMD: call void @llvm.memcpy.p5i8.p1i8.i64(i8 addrspace(5)*
-// AMD: call void @llvm.memcpy.p1i8.p5i8.i64(i8 addrspace(1)*
+// AMDGCN: call void @llvm.memcpy.p5i8.p1i8.i64(i8 addrspace(5)*
+// AMDGCN: call void @llvm.memcpy.p1i8.p5i8.i64(i8 addrspace(1)*
kernel void ker_large(global Mat32X32 *in, global Mat64X64 *out) {
out[0] = foo_large(in[1]);
}
-// AMD-LABEL: define void @FuncOneMember(<2 x i32> %u.coerce)
+// AMDGCN-LABEL: define void @FuncOneMember(<2 x i32> %u.coerce)
void FuncOneMember(struct StructOneMember u) {
u.x = (int2)(0, 0);
}
-// AMD-LABEL: define void @FuncOneLargeMember(%struct.LargeStructOneMember addrspace(5)* byval align 8 %u)
+// AMDGCN-LABEL: define void @FuncOneLargeMember(%struct.LargeStructOneMember addrspace(5)* byval align 8 %u)
+// AMDGCN-NOT: addrspacecast
+// AMDGCN: store <2 x i32> %{{.*}}, <2 x i32> addrspace(5)*
void FuncOneLargeMember(struct LargeStructOneMember u) {
u.x[0] = (int2)(0, 0);
}
-// AMD-LABEL: define amdgpu_kernel void @KernelOneMember
-// AMD-SAME: (<2 x i32> %[[u_coerce:.*]])
-// AMD: %[[u:.*]] = alloca %struct.StructOneMember, align 8, addrspace(5)
-// AMD: %[[coerce_dive:.*]] = getelementptr inbounds %struct.StructOneMember, %struct.StructOneMember addrspace(5)* %[[u]], i32 0, i32 0
-// AMD: store <2 x i32> %[[u_coerce]], <2 x i32> addrspace(5)* %[[coerce_dive]]
-// AMD: call void @FuncOneMember(<2 x i32>
+// AMDGCN20-LABEL: define void @test_indirect_arg_globl()
+// AMDGCN20: %[[byval_temp:.*]] = alloca %struct.LargeStructOneMember, align 8, addrspace(5)
+// AMDGCN20: %[[r0:.*]] = bitcast %struct.L
[PATCH] D41102: Setup clang-doc frontend framework
lebedev.ri added inline comments.
Comment at: clang-doc/Representation.h:79
+ std::string Filename;
+};
+
Hmm, have you tried adding a constructor here?
```
struct Location {
int LineNumber;
std::string Filename;
Location() = default;
Location(int LineNumber_, std::string Filename_) : LineNumber(LineNumber_),
Filename(std::move(Filename_)) {}
};
```
?
https://reviews.llvm.org/D41102
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43586: CodeGen: handle blocks correctly when inalloca'ed
compnerd updated this revision to Diff 135304.
compnerd added a comment.
Update comment
Repository:
rC Clang
https://reviews.llvm.org/D43586
Files:
lib/CodeGen/CGDecl.cpp
test/CodeGenCXX/block-inalloca.cpp
Index: test/CodeGenCXX/block-inalloca.cpp
===
--- /dev/null
+++ test/CodeGenCXX/block-inalloca.cpp
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -triple i686-unknown-windows-msvc -fblocks -emit-llvm -o -
%s | FileCheck %s
+
+struct S {
+ S(const struct S &) {}
+};
+
+void (^b)(S) = ^(S) {};
+
+// CHECK: [[DESCRIPTOR:%.*]] = getelementptr inbounds <{ i8*, %struct.S, [3 x
i8] }>, <{ i8*, %struct.S, [3 x i8] }>* %0, i32 0, i32 0
+// CHECK: load i8*, i8** [[DESCRIPTOR]]
+
Index: lib/CodeGen/CGDecl.cpp
===
--- lib/CodeGen/CGDecl.cpp
+++ lib/CodeGen/CGDecl.cpp
@@ -1848,9 +1848,12 @@
// Use better IR generation for certain implicit parameters.
if (auto IPD = dyn_cast(&D)) {
// The only implicit argument a block has is its literal.
-// We assume this is always passed directly.
+// This may be passed as an inalloca'ed value on Windows x86.
if (BlockInfo) {
- setBlockContextParameter(IPD, ArgNo, Arg.getDirectValue());
+ llvm::Value *V = Arg.isIndirect()
+ ? Builder.CreateLoad(Arg.getIndirectAddress())
+ : Arg.getDirectValue();
+ setBlockContextParameter(IPD, ArgNo, V);
return;
}
}
Index: test/CodeGenCXX/block-inalloca.cpp
===
--- /dev/null
+++ test/CodeGenCXX/block-inalloca.cpp
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -triple i686-unknown-windows-msvc -fblocks -emit-llvm -o - %s | FileCheck %s
+
+struct S {
+ S(const struct S &) {}
+};
+
+void (^b)(S) = ^(S) {};
+
+// CHECK: [[DESCRIPTOR:%.*]] = getelementptr inbounds <{ i8*, %struct.S, [3 x i8] }>, <{ i8*, %struct.S, [3 x i8] }>* %0, i32 0, i32 0
+// CHECK: load i8*, i8** [[DESCRIPTOR]]
+
Index: lib/CodeGen/CGDecl.cpp
===
--- lib/CodeGen/CGDecl.cpp
+++ lib/CodeGen/CGDecl.cpp
@@ -1848,9 +1848,12 @@
// Use better IR generation for certain implicit parameters.
if (auto IPD = dyn_cast(&D)) {
// The only implicit argument a block has is its literal.
-// We assume this is always passed directly.
+// This may be passed as an inalloca'ed value on Windows x86.
if (BlockInfo) {
- setBlockContextParameter(IPD, ArgNo, Arg.getDirectValue());
+ llvm::Value *V = Arg.isIndirect()
+ ? Builder.CreateLoad(Arg.getIndirectAddress())
+ : Arg.getDirectValue();
+ setBlockContextParameter(IPD, ArgNo, V);
return;
}
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r325716 - Replace incorrect usage of isInvalidDecl with intended setInvalidDecl
Author: erichkeane
Date: Wed Feb 21 12:29:05 2018
New Revision: 325716
URL: http://llvm.org/viewvc/llvm-project?rev=325716&view=rev
Log:
Replace incorrect usage of isInvalidDecl with intended setInvalidDecl
This typo would cause an attempt to multiversion 'main' to issue an
error, but not mark the function as invalid. This patch fixes it.
Modified:
cfe/trunk/lib/Sema/SemaDecl.cpp
Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=325716&r1=325715&r2=325716&view=diff
==
--- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDecl.cpp Wed Feb 21 12:29:05 2018
@@ -9324,7 +9324,7 @@ static bool CheckMultiVersionFunction(Se
if (NewFD->isMain()) {
if (NewTA && NewTA->isDefaultVersion()) {
S.Diag(NewFD->getLocation(), diag::err_multiversion_not_allowed_on_main);
- NewFD->isInvalidDecl();
+ NewFD->setInvalidDecl();
return true;
}
return false;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43423: [SimplifyCFG] Create flag to disable simplifyCFG.
efriedma added a comment. > FWIW, I'd very much prefer the details of the optimizer wouldn't be exposed > as frontend flags. Yes, definitely this. Frontend flags to control the optimizer should state an expected result, not just turn off some completely arbitrary set of transforms. Otherwise, we're likely to end up in a situation in the future where we add a new optimization, or split an existing pass, and it isn't clear which transforms the frontend flag should apply to. https://reviews.llvm.org/D43423 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43581: [clang-tidy/google] Fix the Objective-C global variable declaration check 🔧
aaron.ballman added a comment. In https://reviews.llvm.org/D43581#1014903, @stephanemoore wrote: > In https://reviews.llvm.org/D43581#1014664, @aaron.ballman wrote: > > > LGTM with a few additional test cases. > > > > It'd be nice if the style guide linked actually described what a "good" > > prefix is rather than make the reader guess. > > > I will work on updating the Google Objective-C style guide to provide more > clarity regarding what constitutes an appropriate prefix. Fantastic, thank you! This patch LGTM. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D43581 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41228: [ObjC] Enable __strong pointers in structs under ARC
ahatanak added inline comments. Comment at: lib/CodeGen/CGDecl.cpp:1423 + if (capturedByInit) +drillIntoBlockVariable(*this, lvalue, cast(D)); + rjmccall wrote: > The "delay" is that we generally handle `__block` captures within > initializers by delaying the drill-into until we're ready to actually store > into the variable. Not delaying has the advantage of allowing us to just > project out the stack storage instead of actually chasing the forwarding > pointer, but this isn't safe if the forwarding pointer might no longer point > to the stack storage, which is what happens when a block capturing a > `__block` variable is copied to the heap. > > Delaying the drill-into is easy when the stores can be completely separated > from the initializer, as is true for scalar and complex types. It's not easy > for aggregates because we need to emit the initializer in place, but it's > possible that a block capturing the `__block` variable will be copied during > the emission of that initializer (e.g. during a later element of a > braced-init-list or within a C++ constructor call), meaning that we might be > copying an object that's only partly initialized. This is particularly > dangerous if there are destructors involved. > > Probably the best solution is to forbid capturing `__block` variables of > aggregate type within their own initializer. It's possible that that might > break existing code that's not doing the specific dangerous things, though. > > The more complex solution is to make IRGen force the `__block` variable to > the heap before actually initializing it. (It can do this by directly > calling `_Block_object_assign`.) We would then initialize the heap copy > in-place, safely knowing that there is no possibility that it will move > again. Effectively, the stack copy of the variable would never exist as an > object; only its header would matter, and only enough to allow the runtime to > "copy" it to the heap. We'd have to hack the __block variable's copy helper > to not try to call the move-constructor, and we'd need to suppress the > cleanup that destroys the stack copy of the variable. We already push a > cleanup to release the `__block` variable when it goes out of scope, and that > would stay. Ah I see. I'll see if I can implement the first solution you suggested in a separate patch. If it turns out that doing so breaks too much code, we can consider implementing the second solution. Is this something users do commonly? Comment at: lib/CodeGen/CGExprAgg.cpp:255 + Ty.isDestructedType() == QualType::DK_nontrivial_c_struct) +CGF.pushDestroy(Ty.isDestructedType(), src.getAggregateAddress(), Ty); + rjmccall wrote: > Hrm. Okay, I guess. This is specific to ignored results because otherwise > we assume that the caller is going to manage the lifetime of the object? Yes, this is specific to ignored results. This is needed because otherwise function return values that are ignored won't get destructed by the caller. Comment at: lib/CodeGen/CGNonTrivialStruct.cpp:193 + +TrivialFieldIsVolatile |= FT.isVolatileQualified(); +if (Start == End) rjmccall wrote: > I feel like maybe volatile fields should be individually copied instead of > being aggregated into a multi-field memcpy. This is a more natural > interpretation of the C volatile rules than we currently do. In fact, > arguably we should really add a PrimitiveCopyKind enumerator for volatile > fields (that are otherwise trivially-copyable) and force all copies of > structs with volatile fields into this path precisely so that we can make a > point of copying the volatile fields this way. (Obviously that part is not > something that's your responsibility to do.) > > To get that right with bit-fields, you'll need to propagate the actual > FieldDecl down. On the plus side, that should let you use EmitLValueForField > to do the field projection in the common case. I added method visitVolatileTrivial that copies volatile fields individually. Please see test case test_copy_constructor_Bitfield1 in test/CodeGenObjC/strong-in-c-struct.m. https://reviews.llvm.org/D41228 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43586: CodeGen: handle blocks correctly when inalloca'ed
compnerd created this revision.
compnerd added a reviewer: rnk.
Herald added a subscriber: cfe-commits.
When using blocks with C++ on Windows x86, it is possible to have the
block literal be pushed into the inalloca'ed parameters. Teach IRGen to
handle the case properly by extracting the block literal from the
inalloca parameter. This fixes the use of blocks with C++ on Windows
x86.
Repository:
rC Clang
https://reviews.llvm.org/D43586
Files:
lib/CodeGen/CGDecl.cpp
test/CodeGenCXX/block-inalloca.cpp
Index: test/CodeGenCXX/block-inalloca.cpp
===
--- /dev/null
+++ test/CodeGenCXX/block-inalloca.cpp
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -triple i686-unknown-windows-msvc -fblocks -emit-llvm -o -
%s | FileCheck %s
+
+struct S {
+ S(const struct S &) {}
+};
+
+void (^b)(S) = ^(S) {};
+
+// CHECK: [[DESCRIPTOR:%.*]] = getelementptr inbounds <{ i8*, %struct.S, [3 x
i8] }>, <{ i8*, %struct.S, [3 x i8] }>* %0, i32 0, i32 0
+// CHECK: load i8*, i8** [[DESCRIPTOR]]
+
Index: lib/CodeGen/CGDecl.cpp
===
--- lib/CodeGen/CGDecl.cpp
+++ lib/CodeGen/CGDecl.cpp
@@ -1850,7 +1850,10 @@
// The only implicit argument a block has is its literal.
// We assume this is always passed directly.
if (BlockInfo) {
- setBlockContextParameter(IPD, ArgNo, Arg.getDirectValue());
+ llvm::Value *V = Arg.isIndirect()
+ ? Builder.CreateLoad(Arg.getIndirectAddress())
+ : Arg.getDirectValue();
+ setBlockContextParameter(IPD, ArgNo, V);
return;
}
}
Index: test/CodeGenCXX/block-inalloca.cpp
===
--- /dev/null
+++ test/CodeGenCXX/block-inalloca.cpp
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -triple i686-unknown-windows-msvc -fblocks -emit-llvm -o - %s | FileCheck %s
+
+struct S {
+ S(const struct S &) {}
+};
+
+void (^b)(S) = ^(S) {};
+
+// CHECK: [[DESCRIPTOR:%.*]] = getelementptr inbounds <{ i8*, %struct.S, [3 x i8] }>, <{ i8*, %struct.S, [3 x i8] }>* %0, i32 0, i32 0
+// CHECK: load i8*, i8** [[DESCRIPTOR]]
+
Index: lib/CodeGen/CGDecl.cpp
===
--- lib/CodeGen/CGDecl.cpp
+++ lib/CodeGen/CGDecl.cpp
@@ -1850,7 +1850,10 @@
// The only implicit argument a block has is its literal.
// We assume this is always passed directly.
if (BlockInfo) {
- setBlockContextParameter(IPD, ArgNo, Arg.getDirectValue());
+ llvm::Value *V = Arg.isIndirect()
+ ? Builder.CreateLoad(Arg.getIndirectAddress())
+ : Arg.getDirectValue();
+ setBlockContextParameter(IPD, ArgNo, V);
return;
}
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41102: Setup clang-doc frontend framework
juliehockett updated this revision to Diff 135305.
juliehockett marked 20 inline comments as done.
juliehockett added a comment.
Cleaning up bitcode writer and fixing pointers for CommentInfos
https://reviews.llvm.org/D41102
Files:
CMakeLists.txt
clang-doc/BitcodeWriter.cpp
clang-doc/BitcodeWriter.h
clang-doc/CMakeLists.txt
clang-doc/ClangDoc.h
clang-doc/Mapper.cpp
clang-doc/Mapper.h
clang-doc/Representation.h
clang-doc/tool/CMakeLists.txt
clang-doc/tool/ClangDocMain.cpp
docs/clang-doc.rst
test/CMakeLists.txt
test/clang-doc/mapper-class.cpp
test/clang-doc/mapper-enum.cpp
test/clang-doc/mapper-function.cpp
test/clang-doc/mapper-method.cpp
test/clang-doc/mapper-namespace.cpp
test/clang-doc/mapper-struct.cpp
test/clang-doc/mapper-union.cpp
Index: test/clang-doc/mapper-union.cpp
===
--- /dev/null
+++ test/clang-doc/mapper-union.cpp
@@ -0,0 +1,29 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: echo "" > %t/compile_flags.txt
+// RUN: cp "%s" "%t/test.cpp"
+// RUN: clang-doc --dump --omit-filenames -doxygen -p %t %t/test.cpp -output=%t/docs
+// RUN: llvm-bcanalyzer %t/docs/c:@u...@d.bc --dump | FileCheck %s
+
+union D { int X; int Y; };
+// CHECK:
+// CHECK:
+ // CHECK:
+// CHECK:
+// CHECK:
+ // CHECK: blob data = 'D'
+ // CHECK:
+ // CHECK:
+ // CHECK:
+// CHECK: blob data = 'int'
+// CHECK: blob data = 'D::X'
+// CHECK:
+ // CHECK:
+ // CHECK:
+// CHECK: blob data = 'int'
+// CHECK: blob data = 'D::Y'
+// CHECK:
+ // CHECK:
+// CHECK:
+
+
Index: test/clang-doc/mapper-struct.cpp
===
--- /dev/null
+++ test/clang-doc/mapper-struct.cpp
@@ -0,0 +1,23 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: echo "" > %t/compile_flags.txt
+// RUN: cp "%s" "%t/test.cpp"
+// RUN: clang-doc --dump --omit-filenames -doxygen -p %t %t/test.cpp -output=%t/docs
+// RUN: llvm-bcanalyzer %t/docs/c:@s...@c.bc --dump | FileCheck %s
+
+struct C { int i; };
+// CHECK:
+// CHECK:
+ // CHECK:
+// CHECK:
+// CHECK:
+ // CHECK: blob data = 'C'
+ // CHECK:
+ // CHECK:
+// CHECK: blob data = 'int'
+// CHECK: blob data = 'C::i'
+// CHECK:
+ // CHECK:
+// CHECK:
+
+
Index: test/clang-doc/mapper-namespace.cpp
===
--- /dev/null
+++ test/clang-doc/mapper-namespace.cpp
@@ -0,0 +1,17 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: echo "" > %t/compile_flags.txt
+// RUN: cp "%s" "%t/test.cpp"
+// RUN: clang-doc --dump --omit-filenames -doxygen -p %t %t/test.cpp -output=%t/docs
+// RUN: llvm-bcanalyzer %t/docs/c:@n...@a.bc --dump | FileCheck %s
+
+namespace A {}
+// CHECK:
+// CHECK:
+ // CHECK:
+// CHECK:
+// CHECK:
+ // CHECK: blob data = 'A'
+// CHECK:
+
+
Index: test/clang-doc/mapper-method.cpp
===
--- /dev/null
+++ test/clang-doc/mapper-method.cpp
@@ -0,0 +1,31 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: echo "" > %t/compile_flags.txt
+// RUN: cp "%s" "%t/test.cpp"
+// RUN: clang-doc --dump --omit-filenames -doxygen -p %t %t/test.cpp -output=%t/docs
+// RUN: llvm-bcanalyzer %t/docs/c:@S@G@F@Method#I#.bc --dump | FileCheck %s
+
+class G {
+public:
+ int Method(int param) { return param; }
+};
+// CHECK:
+// CHECK:
+ // CHECK:
+// CHECK:
+// CHECK:
+ // CHECK: blob data = 'Method'
+ // CHECK:
+ // CHECK: blob data = 'c:@S@G'
+ // CHECK:
+// CHECK: blob data = 'int'
+ // CHECK:
+ // CHECK:
+// CHECK: blob data = 'int'
+// CHECK: blob data = 'param'
+ // CHECK:
+// CHECK:
+
+
+
+
Index: test/clang-doc/mapper-function.cpp
===
--- /dev/null
+++ test/clang-doc/mapper-function.cpp
@@ -0,0 +1,24 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: echo "" > %t/compile_flags.txt
+// RUN: cp "%s" "%t/test.cpp"
+// RUN: clang-doc --dump --omit-filenames -doxygen -p %t %t/test.cpp -output=%t/docs
+// RUN: llvm-bcanalyzer %t/docs/c:@F@F#I#.bc --dump | FileCheck %s
+
+int F(int param) { return param; }
+// CHECK:
+// CHECK:
+ // CHECK:
+// CHECK:
+// CHECK:
+ // CHECK: blob data = 'F'
+ // CHECK:
+ // CHECK:
+// CHECK: blob data = 'int'
+ // CHECK:
+ // CHECK:
+// CHECK: blob data = 'int'
+// CHECK: blob data = 'param'
+ // CHECK:
+// CHECK:
+
Index: test/clang-doc/mapper-enum.cpp
===
--- /dev/null
+++ test/clang-doc/mapper-enum.cpp
@@ -0,0 +1,25 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: echo "" > %t/compile_flags.txt
+// RUN: cp "%s" "%t/test.cpp"
+// RUN: clang-doc --dump --omit-filenames -doxygen -p %t %t/test.cpp -output=%t/docs
+// RUN: llvm-bcanalyzer %t/docs/c:@e...@b.bc --dump | FileCheck %s
+
+enum B { X, Y };
+// CHECK:
+// CHECK:
+ // CHECK:
+// CHECK:
+// CHEC
[PATCH] D43423: [SimplifyCFG] Create flag to disable simplifyCFG.
morehouse added a comment. In https://reviews.llvm.org/D43423#1011170, @davide wrote: > Some high level comments: > > 1. This is something that GCC does relatively frequently (adding frontend > options to control optimization passes), but LLVM tends to not expose these > details. FWIW, I'd very much prefer the details of the optimizer wouldn't be > exposed as frontend flags. We might not need the flag if we decide to always set `no_simplify_cfg` during codegen with coverage instrumentation. But we're not sure if we want to do that yet. > 2. I really don't think metadata is the correct way of conveying this > information to the optimizer. Among all the other problems, metadata can be > stripped willy-nilly. Maybe a function attribute will work better? Should be simple enough to change this to a function attribute if you think that is more appropriate. Wasn't sure whether metadata or an attribute made more sense. > Some alternatives which I don't like, but I can't currently propose anything > better are: > > 1. Having a separate optimization pipeline that you use in these cases This seems like a lot more work. > 2. Having an instrumentation pass that annotates your CFG to prevent passes > from destroying coverage signal. That said, I'm not really familiar to > comment on whether this is feasible or not. This patch allows us to annotate our functions with `no_simplify_cfg` to do what you're suggesting. Writing another instrumentation pass seems like overkill. > Please note that completely disabling SimplifyCFG will result in non-trivial > performance hits (at least, in my experience), so you might want to evaluate > this carefully. > @chandlerc might have thoughts on how to address this. We're still evaluating this with mixed results. On one hand, disabling `simplifyCFG` makes most of our libFuzzer tests pass with `-O2`, and libFuzzer seems to perform at the same executions/sec rate. On the other hand, executions/sec doesn't necessarily translate to more coverage/sec or bugs found. Comment at: llvm/lib/Transforms/Utils/SimplifyCFG.cpp:109 static cl::opt MergeCondStores( "simplifycfg-merge-cond-stores", cl::Hidden, cl::init(true), vitalybuka wrote: > why metadata instead of cl:opt like these? We want to be able to avoid `simplifyCFG` when we pass `-fsanitize=fuzzer` to the Clang driver. AFAICT, the frontend does not explicitly invoke `simplifyCFG`, so it can't control the options here. https://reviews.llvm.org/D43423 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41228: [ObjC] Enable __strong pointers in structs under ARC
ahatanak updated this revision to Diff 135309.
ahatanak marked 14 inline comments as done.
ahatanak added a comment.
Address review comments.
https://reviews.llvm.org/D41228
Files:
docs/LanguageExtensions.rst
include/clang/AST/Decl.h
include/clang/AST/Type.h
include/clang/Basic/DiagnosticSemaKinds.td
lib/AST/ASTContext.cpp
lib/AST/Decl.cpp
lib/AST/Type.cpp
lib/CodeGen/CGBlocks.cpp
lib/CodeGen/CGCall.cpp
lib/CodeGen/CGDecl.cpp
lib/CodeGen/CGDeclCXX.cpp
lib/CodeGen/CGExprAgg.cpp
lib/CodeGen/CGNonTrivialStruct.cpp
lib/CodeGen/CMakeLists.txt
lib/CodeGen/CodeGenFunction.h
lib/Lex/PPMacroExpansion.cpp
lib/Sema/JumpDiagnostics.cpp
lib/Sema/SemaDecl.cpp
lib/Sema/SemaExpr.cpp
test/ARCMT/checking.m
test/CodeGenObjC/nontrivial-c-struct-exception.m
test/CodeGenObjC/nontrivial-c-struct-func-name-collision.m
test/CodeGenObjC/strong-in-c-struct.m
test/Lexer/has_feature_objc_arc.m
test/SemaObjC/arc-decls.m
test/SemaObjC/arc-system-header.m
test/SemaObjC/strong-in-c-struct.m
Index: test/SemaObjC/strong-in-c-struct.m
===
--- /dev/null
+++ test/SemaObjC/strong-in-c-struct.m
@@ -0,0 +1,56 @@
+// RUN: %clang_cc1 -triple arm64-apple-ios11 -fobjc-arc -fblocks -fobjc-runtime=ios-11.0 -fsyntax-only -verify %s
+
+typedef struct {
+ id a;
+} Strong;
+
+void callee_variadic(const char *, ...);
+
+void test_variadic(void) {
+ Strong t;
+ callee_variadic("s", t); // expected-error {{cannot pass non-trivial C object of type 'Strong' by value to variadic function}}
+}
+
+void test_jump0(int cond) {
+ switch (cond) {
+ case 0:
+;
+Strong x; // expected-note {{jump bypasses initialization of variable of non-trivial C struct type}}
+break;
+ case 1: // expected-error {{cannot jump from switch statement to this case label}}
+x.a = 0;
+break;
+ }
+}
+
+void test_jump1(void) {
+ static void *ips[] = { &&L0 };
+L0: // expected-note {{possible target of indirect goto}}
+ ;
+ Strong x; // expected-note {{jump exits scope of variable with non-trivial destructor}}
+ goto *ips; // expected-error {{cannot jump}}
+}
+
+typedef void (^BlockTy)(void);
+void func(BlockTy);
+void func2(Strong);
+
+void test_block_scope0(int cond) {
+ Strong x; // expected-note {{jump enters lifetime of block which captures a C struct that is non-trivial to destroy}}
+ switch (cond) {
+ case 0:
+func(^{ func2(x); });
+break;
+ default: // expected-error {{cannot jump from switch statement to this case label}}
+break;
+ }
+}
+
+void test_block_scope1(void) {
+ static void *ips[] = { &&L0 };
+L0: // expected-note {{possible target of indirect goto}}
+ ;
+ Strong x; // expected-note {{jump exits scope of variable with non-trivial destructor}} expected-note {{jump exits lifetime of block which captures a C struct that is non-trivial to destroy}}
+ func(^{ func2(x); });
+ goto *ips; // expected-error {{cannot jump}}
+}
Index: test/SemaObjC/arc-system-header.m
===
--- test/SemaObjC/arc-system-header.m
+++ test/SemaObjC/arc-system-header.m
@@ -23,8 +23,7 @@
}
void test5(struct Test5 *p) {
- p->field = 0; // expected-error {{'field' is unavailable in ARC}}
-// expected-note@arc-system-header.h:25 {{field has non-trivial ownership qualification}}
+ p->field = 0;
}
id test6() {
@@ -49,8 +48,7 @@
extern void doSomething(Test9 arg);
void test9() {
-Test9 foo2 = {0, 0}; // expected-error {{'field' is unavailable in ARC}}
- // expected-note@arc-system-header.h:56 {{field has non-trivial ownership qualification}}
+Test9 foo2 = {0, 0};
doSomething(foo2);
}
#endif
Index: test/SemaObjC/arc-decls.m
===
--- test/SemaObjC/arc-decls.m
+++ test/SemaObjC/arc-decls.m
@@ -3,7 +3,7 @@
// rdar://8843524
struct A {
-id x; // expected-error {{ARC forbids Objective-C objects in struct}}
+id x;
};
union u {
@@ -13,7 +13,7 @@
@interface I {
struct A a;
struct B {
-id y[10][20]; // expected-error {{ARC forbids Objective-C objects in struct}}
+id y[10][20];
id z;
} b;
@@ -23,7 +23,7 @@
// rdar://10260525
struct r10260525 {
- id (^block) (); // expected-error {{ARC forbids blocks in struct}}
+ id (^block) ();
};
struct S {
Index: test/Lexer/has_feature_objc_arc.m
===
--- test/Lexer/has_feature_objc_arc.m
+++ test/Lexer/has_feature_objc_arc.m
@@ -13,8 +13,16 @@
void no_objc_arc_weak_feature();
#endif
+#if __has_feature(objc_arc_fields)
+void has_objc_arc_fields();
+#else
+void no_objc_arc_fields();
+#endif
+
// CHECK-ARC: void has_objc_arc_feature();
// CHECK-ARC: void has_objc_arc_weak_feature();
+// CHECK-ARC: void has_objc_arc_fields();
// CHECK-ARCLITE: void has_objc_arc_feature();
// CHECK-ARCLITE: void no_o
[PATCH] D43586: CodeGen: handle blocks correctly when inalloca'ed
smeenai added a comment.
Patch is missing context.
Comment at: lib/CodeGen/CGDecl.cpp:1851
// The only implicit argument a block has is its literal.
// We assume this is always passed directly.
if (BlockInfo) {
This comment needs to be updated.
Repository:
rC Clang
https://reviews.llvm.org/D43586
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43497: [analyzer] Introduce correct lifetime extension behavior in simple cases.
NoQ updated this revision to Diff 135312.
NoQ marked 4 inline comments as done.
NoQ added a comment.
- Address comments.
- Add a FIXME test case that demonstrates that automatic destructors don't fire
after lifetime extension through a POD field, even though lifetime extension
itself seems to work correctly. I should probably also add a test case for the
situation where sub-object adjustments actually kick in (here they suddenly
don't), because even though they are correctly handled in
`createTemporaryRegionIfNeeded`, they aren't implemented in
`findConstructionContexts`.
https://reviews.llvm.org/D43497
Files:
include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
lib/StaticAnalyzer/Core/ExprEngine.cpp
lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
test/Analysis/lifetime-extension.cpp
test/Analysis/temporaries.cpp
Index: test/Analysis/temporaries.cpp
===
--- test/Analysis/temporaries.cpp
+++ test/Analysis/temporaries.cpp
@@ -895,6 +895,33 @@
}
} // namespace test_match_constructors_and_destructors
+namespace dont_forget_destructor_around_logical_op {
+int glob;
+
+class C {
+public:
+ ~C() { glob = 1; }
+};
+
+C get();
+
+bool is(C);
+
+
+void test(int coin) {
+ // Here temporaries are being cleaned up after && is evaluated. There are two
+ // temporaries: the return value of get() and the elidable copy constructor
+ // of that return value into is(). According to the CFG, we need to cleanup
+ // both of them depending on whether the temporary corresponding to the
+ // return value of get() was initialized. However, for now we don't track
+ // temporaries returned from functions, so we take the wrong branch.
+ coin && is(get()); // no-crash
+ // FIXME: We should have called the destructor, i.e. should be TRUE,
+ // at least when we inline temporary destructors.
+ clang_analyzer_eval(glob == 1); // expected-warning{{UNKNOWN}}
+}
+} // namespace dont_forget_destructor_around_logical_op
+
#if __cplusplus >= 201103L
namespace temporary_list_crash {
class C {
Index: test/Analysis/lifetime-extension.cpp
===
--- test/Analysis/lifetime-extension.cpp
+++ test/Analysis/lifetime-extension.cpp
@@ -1,4 +1,5 @@
-// RUN: %clang_analyze_cc1 -Wno-unused -std=c++11 -analyzer-checker=debug.ExprInspection -verify %s
+// RUN: %clang_analyze_cc1 -Wno-unused -std=c++11 -analyzer-checker=core,debug.ExprInspection -verify %s
+// RUN: %clang_analyze_cc1 -Wno-unused -std=c++11 -analyzer-checker=core,debug.ExprInspection -analyzer-config cfg-temporary-dtors=true -DTEMPORARIES -verify %s
void clang_analyzer_eval(bool);
@@ -38,9 +39,157 @@
const int &y = A().j[1]; // no-crash
const int &z = (A().j[1], A().j[0]); // no-crash
- // FIXME: All of these should be TRUE, but constructors aren't inlined.
- clang_analyzer_eval(x == 1); // expected-warning{{UNKNOWN}}
- clang_analyzer_eval(y == 3); // expected-warning{{UNKNOWN}}
- clang_analyzer_eval(z == 2); // expected-warning{{UNKNOWN}}
+ clang_analyzer_eval(x == 1);
+ clang_analyzer_eval(y == 3);
+ clang_analyzer_eval(z == 2);
+#ifdef TEMPORARIES
+ // expected-warning@-4{{TRUE}}
+ // expected-warning@-4{{TRUE}}
+ // expected-warning@-4{{TRUE}}
+#else
+ // expected-warning@-8{{UNKNOWN}}
+ // expected-warning@-8{{UNKNOWN}}
+ // expected-warning@-8{{UNKNOWN}}
+#endif
}
} // end namespace pr19539_crash_on_destroying_an_integer
+
+namespace maintain_original_object_address_on_lifetime_extension {
+class C {
+ C **after, **before;
+
+public:
+ bool x;
+
+ C(bool x, C **after, C **before) : x(x), after(after), before(before) {
+*before = this;
+ }
+
+ // Don't track copies in our tests.
+ C(const C &c) : x(c.x), after(nullptr), before(nullptr) {}
+
+ ~C() { if (after) *after = this; }
+
+ operator bool() const { return x; }
+};
+
+void f1() {
+ C *after, *before;
+ {
+const C &c = C(true, &after, &before);
+ }
+ clang_analyzer_eval(after == before);
+#ifdef TEMPORARIES
+ // expected-warning@-2{{TRUE}}
+#else
+ // expected-warning@-4{{UNKNOWN}}
+#endif
+}
+
+void f2() {
+ C *after, *before;
+ C c = C(1, &after, &before);
+ clang_analyzer_eval(after == before);
+#ifdef TEMPORARIES
+ // expected-warning@-2{{TRUE}}
+#else
+ // expected-warning@-4{{UNKNOWN}}
+#endif
+}
+
+void f3(bool coin) {
+ C *after, *before;
+ {
+const C &c = coin ? C(true, &after, &before) : C(false, &after, &before);
+ }
+ clang_analyzer_eval(after == before);
+#ifdef TEMPORARIES
+ // expected-warning@-2{{TRUE}}
+#else
+ // expected-warning@-4{{UNKNOWN}}
+#endif
+}
+
+void f4(bool coin) {
+ C *after, *before;
+ {
+// no-crash
+const C &c = C(coin, &after, &before) ?: C(false, &after, &before);
+ }
+ // FIXME: Add support for lifetime extension through binary conditional
+ // operator. Ideally also add support for the binary conditional operator in
+ // C++. Because for now it calls the construc
[PATCH] D43497: [analyzer] Introduce correct lifetime extension behavior in simple cases.
NoQ added inline comments.
Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:396
+ProgramStateRef State, const LocationContext *FromLC,
+const LocationContext *ToLC) {
+ const LocationContext *LC = FromLC;
a.sidorin wrote:
> EndLC? (similar to iterators)
Sounds neat, i'd make another quick patch on all things together.
Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:281
+
+if (!TR) {
+ StorageDuration SD = MT->getStorageDuration();
dcoughlin wrote:
> Would it be safe for the body of the `if (!TR)` to be the else branch of `if
> constCXXTempObjectionRegion *const *TRPtr = ...` rather then its own if
> statement?
Yep. Fxd.
Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:289
}
if (!TR)
TR = MRMgr.getCXXTempObjectRegion(Init, LC);
dcoughlin wrote:
> Would it be safe for `TR = MRMgr.getCXXTempObjectRegion(Init, LC);` to be the
> else branch of `if (const MaterializeTemporaryExpr *MT =
> dyn_cast(Result))` rather than its own `if`
> statement?
>
> Ideally the number paths through this function on which we call
> `MRMgr.getCXXTempObjectRegion()` would be small and very clear.
>
Nope, it also covers the case when we have an MTE but it's not static. I guess
it's still more clear to add it twice than to have it as a fallback for an
indefinite amount of cases.
Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:490
+static void printTemporaryMaterializatinosForContext(
+raw_ostream &Out, ProgramStateRef State, const char *NL, const char *Sep,
dcoughlin wrote:
> Nit: There is a typo in the name here ("Materializatinos"). I guess this is
> the sub-atomic particle created as a byproduct of materializing a temporary!
> We have a lot of materializatinos in Cupertino.
Fixed to prevent further lifetime extensino of typos through autocompletinos.
Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:503
+Key.first->printPretty(Out, nullptr, PP);
+if (Value)
+ Out << " : " << Value;
a.sidorin wrote:
> As I see from line 350, `Value` is always non-null. And there is same check
> on line 659. Am I missing something?
Yep, all of these are non-null by construction. This actually makes me wonder
if it was correct to remove `VisitCXXBindTemporaryExpr` from `ExprEngine` -
might have done it too early (this is what makes the similar if in
`printInitializedTemporariesForContext` redundant). The
`dont_forget_destructor_around_logical_op` test also suggests that - seems to
actually be a regression. I guess i'm going to revert the
`VisitCXXBindTemporaryExpr` change in a follow-up commit.
Also we should allow `ostream << MR` to accept null pointers. It's a global
operator overload anyway.
Comment at: test/Analysis/lifetime-extension.cpp:45
+ clang_analyzer_eval(z == 2);
+#ifdef TEMPORARIES
+ // expected-warning@-4{{TRUE}}
dcoughlin wrote:
> Yay!
These worked before (with old lifetime extension, since temporary constructors
were inlined), i only added the run-line to see that.
https://reviews.llvm.org/D43497
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43590: [clang-format] Fix regression when getStyle() called with empty filename
benhamilton created this revision.
benhamilton added reviewers: vsapsai, jolesiak, krasimir.
Herald added subscribers: cfe-commits, klimek.
https://reviews.llvm.org/D43522 caused an assertion failure when getStyle() was
called with
an empty filename:
https://reviews.llvm.org/P8065
This adds a test to reproduce the failure and fixes the issue by
ensuring we never pass an empty filename to
Environment::CreateVirtualEnvironment().
Test Plan: New test added. Ran test with:
% make -j12 FormatTests && ./tools/clang/unittests/Format/FormatTests
Before diff, test failed with P8065. Now, test passes.
Repository:
rC Clang
https://reviews.llvm.org/D43590
Files:
lib/Format/Format.cpp
unittests/Format/FormatTest.cpp
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -11723,6 +11723,12 @@
verifyFormat("__super::FooBar();");
}
+TEST(FormatStyle, GetStyleWithEmptyFileName) {
+ auto Style1 = getStyle("file", "", "Google");
+ ASSERT_TRUE((bool)Style1);
+ ASSERT_EQ(*Style1, getGoogleStyle());
+}
+
TEST(FormatStyle, GetStyleOfFile) {
vfs::InMemoryFileSystem FS;
// Test 1: format file in the same directory.
Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -2297,12 +2297,13 @@
FormatStyle::LanguageKind guessLanguage(StringRef FileName, StringRef Code) {
FormatStyle::LanguageKind result = getLanguageByFileName(FileName);
if (result == FormatStyle::LK_Cpp) {
-auto extension = llvm::sys::path::extension(FileName);
+auto Extension = llvm::sys::path::extension(FileName);
// If there's no file extension (or it's .h), we need to check the contents
// of the code to see if it contains Objective-C.
-if (extension.empty() || extension == ".h") {
+if (Extension.empty() || Extension == ".h") {
+ auto NonEmptyFileName = FileName.empty() ? "guess.h" : FileName;
std::unique_ptr Env =
- Environment::CreateVirtualEnvironment(Code, FileName, /*Ranges=*/{});
+ Environment::CreateVirtualEnvironment(Code, NonEmptyFileName,
/*Ranges=*/{});
ObjCHeaderStyleGuesser Guesser(*Env, getLLVMStyle());
Guesser.process();
if (Guesser.isObjC()) {
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -11723,6 +11723,12 @@
verifyFormat("__super::FooBar();");
}
+TEST(FormatStyle, GetStyleWithEmptyFileName) {
+ auto Style1 = getStyle("file", "", "Google");
+ ASSERT_TRUE((bool)Style1);
+ ASSERT_EQ(*Style1, getGoogleStyle());
+}
+
TEST(FormatStyle, GetStyleOfFile) {
vfs::InMemoryFileSystem FS;
// Test 1: format file in the same directory.
Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -2297,12 +2297,13 @@
FormatStyle::LanguageKind guessLanguage(StringRef FileName, StringRef Code) {
FormatStyle::LanguageKind result = getLanguageByFileName(FileName);
if (result == FormatStyle::LK_Cpp) {
-auto extension = llvm::sys::path::extension(FileName);
+auto Extension = llvm::sys::path::extension(FileName);
// If there's no file extension (or it's .h), we need to check the contents
// of the code to see if it contains Objective-C.
-if (extension.empty() || extension == ".h") {
+if (Extension.empty() || Extension == ".h") {
+ auto NonEmptyFileName = FileName.empty() ? "guess.h" : FileName;
std::unique_ptr Env =
- Environment::CreateVirtualEnvironment(Code, FileName, /*Ranges=*/{});
+ Environment::CreateVirtualEnvironment(Code, NonEmptyFileName, /*Ranges=*/{});
ObjCHeaderStyleGuesser Guesser(*Env, getLLVMStyle());
Guesser.process();
if (Guesser.isObjC()) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r325691 - [clang-format] New API guessLanguage()
Thanks for the report, and sorry for the breakage. Here's the fix: https://reviews.llvm.org/D43590 Ben On Wed, Feb 21, 2018 at 1:47 PM wrote: > Hi Ben, > > Your change is causing a test failure on one of the bots, can you take a > look? > > > http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/builds/25515 > > FAIL: Clang Tools :: clang-apply-replacements/format.cpp (12526 of 38114) > TEST 'Clang Tools :: > clang-apply-replacements/format.cpp' FAILED > Script: > -- > mkdir -p > /home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/tools/clang/tools/extra/test/clang-apply-replacements/Output/Inputs/format > grep -Ev "// *[A-Z-]+:" > /home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/tools/extra/test/clang-apply-replacements/Inputs/format/yes.cpp > > > /home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/tools/clang/tools/extra/test/clang-apply-replacements/Output/Inputs/format/yes.cpp > grep -Ev "// *[A-Z-]+:" > /home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/tools/extra/test/clang-apply-replacements/Inputs/format/no.cpp > > > /home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/tools/clang/tools/extra/test/clang-apply-replacements/Output/Inputs/format/no.cpp > sed > "s#\$(path)#/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/tools/clang/tools/extra/test/clang-apply-replacements/Output/Inputs/format#" > /home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/tools/extra/test/clang-apply-replacements/Inputs/format/yes.yaml > > > /home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/tools/clang/tools/extra/test/clang-apply-replacements/Output/Inputs/format/yes.yaml > sed > "s#\$(path)#/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/tools/clang/tools/extra/test/clang-apply-replacements/Output/Inputs/format#" > /home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/tools/extra/test/clang-apply-replacements/Inputs/format/no.yaml > > > /home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/tools/clang/tools/extra/test/clang-apply-replacements/Output/Inputs/format/no.yaml > clang-apply-replacements -format > /home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/tools/clang/tools/extra/test/clang-apply-replacements/Output/Inputs/format > FileCheck --strict-whitespace > -input-file=/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/tools/clang/tools/extra/test/clang-apply-replacements/Output/Inputs/format/yes.cpp > /home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/tools/extra/test/clang-apply-replacements/Inputs/format/yes.cpp > FileCheck --strict-whitespace > -input-file=/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/tools/clang/tools/extra/test/clang-apply-replacements/Output/Inputs/format/no.cpp > /home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/tools/extra/test/clang-apply-replacements/Inputs/format/no.cpp > -- > Exit Code: 134 > > Command Output (stderr): > -- > clang-apply-replacements: > /home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/lib/Basic/SourceManager.cpp:393: > const clang::SrcMgr::ContentCache > *clang::SourceManager::getOrCreateContentCache(const clang::FileEntry *, > bool): Assertion `FileEnt && "Didn't specify a file entry to use?"' failed. > /home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/tools/clang/tools/extra/test/clang-apply-replacements/Output/format.cpp.script: > line 8: 49680 Aborted (core dumped) > clang-apply-replacements -format > /home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/tools/clang/tools/extra/test/clang-apply-replacements/Output/Inputs/format > > -- > > > > Douglas Yung > > > > -Original Message- > > From: cfe-commits [mailto:cfe-commits-boun...@lists.llvm.org] On Behalf > Of Ben > > Hamilton via cfe-commits > > Sent: Wednesday, February 21, 2018 7:55 > > To: cfe-commits@lists.llvm.org > > Subject: r325691 - [clang-format] New API guessLanguage() > > > > Author: benhamilton > > Date: Wed Feb 21 07:54:31 2018 > > New Revision: 325691 > > > > URL: http://llvm.org/viewvc/llvm-project?rev=325691&view=rev > > Log: > > [clang-format] New API guessLanguage() > > > > Summary: > > For clients which don't have a filesystem, calling getStyle() doesn't > make > > much sense (there's no .clang-format files to search for). > > > > In this diff, I hoist out the l
RE: r325691 - [clang-format] New API guessLanguage()
Hi Ben, Your change is causing a test failure on one of the bots, can you take a look? http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/builds/25515 FAIL: Clang Tools :: clang-apply-replacements/format.cpp (12526 of 38114) TEST 'Clang Tools :: clang-apply-replacements/format.cpp' FAILED Script: -- mkdir -p /home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/tools/clang/tools/extra/test/clang-apply-replacements/Output/Inputs/format grep -Ev "// *[A-Z-]+:" /home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/tools/extra/test/clang-apply-replacements/Inputs/format/yes.cpp > /home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/tools/clang/tools/extra/test/clang-apply-replacements/Output/Inputs/format/yes.cpp grep -Ev "// *[A-Z-]+:" /home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/tools/extra/test/clang-apply-replacements/Inputs/format/no.cpp > /home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/tools/clang/tools/extra/test/clang-apply-replacements/Output/Inputs/format/no.cpp sed "s#\$(path)#/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/tools/clang/tools/extra/test/clang-apply-replacements/Output/Inputs/format#" /home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/tools/extra/test/clang-apply-replacements/Inputs/format/yes.yaml > /home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/tools/clang/tools/extra/test/clang-apply-replacements/Output/Inputs/format/yes.yaml sed "s#\$(path)#/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/tools/clang/tools/extra/test/clang-apply-replacements/Output/Inputs/format#" /home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/tools/extra/test/clang-apply-replacements/Inputs/format/no.yaml > /home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/tools/clang/tools/extra/test/clang-apply-replacements/Output/Inputs/format/no.yaml clang-apply-replacements -format /home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/tools/clang/tools/extra/test/clang-apply-replacements/Output/Inputs/format FileCheck --strict-whitespace -input-file=/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/tools/clang/tools/extra/test/clang-apply-replacements/Output/Inputs/format/yes.cpp /home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/tools/extra/test/clang-apply-replacements/Inputs/format/yes.cpp FileCheck --strict-whitespace -input-file=/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/tools/clang/tools/extra/test/clang-apply-replacements/Output/Inputs/format/no.cpp /home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/tools/extra/test/clang-apply-replacements/Inputs/format/no.cpp -- Exit Code: 134 Command Output (stderr): -- clang-apply-replacements: /home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/lib/Basic/SourceManager.cpp:393: const clang::SrcMgr::ContentCache *clang::SourceManager::getOrCreateContentCache(const clang::FileEntry *, bool): Assertion `FileEnt && "Didn't specify a file entry to use?"' failed. /home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/tools/clang/tools/extra/test/clang-apply-replacements/Output/format.cpp.script: line 8: 49680 Aborted (core dumped) clang-apply-replacements -format /home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/tools/clang/tools/extra/test/clang-apply-replacements/Output/Inputs/format -- Douglas Yung > -Original Message- > From: cfe-commits [mailto:cfe-commits-boun...@lists.llvm.org] On Behalf Of Ben > Hamilton via cfe-commits > Sent: Wednesday, February 21, 2018 7:55 > To: cfe-commits@lists.llvm.org > Subject: r325691 - [clang-format] New API guessLanguage() > > Author: benhamilton > Date: Wed Feb 21 07:54:31 2018 > New Revision: 325691 > > URL: http://llvm.org/viewvc/llvm-project?rev=325691&view=rev > Log: > [clang-format] New API guessLanguage() > > Summary: > For clients which don't have a filesystem, calling getStyle() doesn't make > much sense (there's no .clang-format files to search for). > > In this diff, I hoist out the language-guessing logic from getStyle() and move > it into a new API guessLanguage(). > > I also added support for guessing the language of files which have no > extension (they could be C++ or ObjC). > > Test Plan: New tests added. Ran tests with: > % make -j12 Format
[PATCH] D43570: [OpenCL] Add '-cl-uniform-work-group-size' compile option
yaxunl accepted this revision. yaxunl added a comment. LGTM. Thanks. Repository: rC Clang https://reviews.llvm.org/D43570 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43590: [clang-format] Fix regression when getStyle() called with empty filename
vsapsai accepted this revision. vsapsai added a comment. This revision is now accepted and ready to land. I'm not familiar with this code, so post-commit review by someone more knowledgeable might be a good idea. Code-wise it looks good. I feel uneasy about using fake file name, but it is exactly the case being tested, so that's not a problem. Repository: rC Clang https://reviews.llvm.org/D43590 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r325722 - [clang-format] Fix regression when getStyle() called with empty filename
Author: benhamilton
Date: Wed Feb 21 13:27:27 2018
New Revision: 325722
URL: http://llvm.org/viewvc/llvm-project?rev=325722&view=rev
Log:
[clang-format] Fix regression when getStyle() called with empty filename
Summary:
D43522 caused an assertion failure when getStyle() was called with
an empty filename:
P8065
This adds a test to reproduce the failure and fixes the issue by
ensuring we never pass an empty filename to
Environment::CreateVirtualEnvironment().
Test Plan: New test added. Ran test with:
% make -j12 FormatTests && ./tools/clang/unittests/Format/FormatTests
Before diff, test failed with P8065. Now, test passes.
Reviewers: vsapsai, jolesiak, krasimir
Reviewed By: vsapsai
Subscribers: klimek, cfe-commits
Differential Revision: https://reviews.llvm.org/D43590
Modified:
cfe/trunk/lib/Format/Format.cpp
cfe/trunk/unittests/Format/FormatTest.cpp
Modified: cfe/trunk/lib/Format/Format.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=325722&r1=325721&r2=325722&view=diff
==
--- cfe/trunk/lib/Format/Format.cpp (original)
+++ cfe/trunk/lib/Format/Format.cpp Wed Feb 21 13:27:27 2018
@@ -2297,12 +2297,13 @@ static FormatStyle::LanguageKind getLang
FormatStyle::LanguageKind guessLanguage(StringRef FileName, StringRef Code) {
FormatStyle::LanguageKind result = getLanguageByFileName(FileName);
if (result == FormatStyle::LK_Cpp) {
-auto extension = llvm::sys::path::extension(FileName);
+auto Extension = llvm::sys::path::extension(FileName);
// If there's no file extension (or it's .h), we need to check the contents
// of the code to see if it contains Objective-C.
-if (extension.empty() || extension == ".h") {
+if (Extension.empty() || Extension == ".h") {
+ auto NonEmptyFileName = FileName.empty() ? "guess.h" : FileName;
std::unique_ptr Env =
- Environment::CreateVirtualEnvironment(Code, FileName, /*Ranges=*/{});
+ Environment::CreateVirtualEnvironment(Code, NonEmptyFileName,
/*Ranges=*/{});
ObjCHeaderStyleGuesser Guesser(*Env, getLLVMStyle());
Guesser.process();
if (Guesser.isObjC()) {
Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=325722&r1=325721&r2=325722&view=diff
==
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Wed Feb 21 13:27:27 2018
@@ -11723,6 +11723,12 @@ TEST_F(FormatTest, NoSpaceAfterSuper) {
verifyFormat("__super::FooBar();");
}
+TEST(FormatStyle, GetStyleWithEmptyFileName) {
+ auto Style1 = getStyle("file", "", "Google");
+ ASSERT_TRUE((bool)Style1);
+ ASSERT_EQ(*Style1, getGoogleStyle());
+}
+
TEST(FormatStyle, GetStyleOfFile) {
vfs::InMemoryFileSystem FS;
// Test 1: format file in the same directory.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41228: [ObjC] Enable __strong pointers in structs under ARC
rjmccall added inline comments. Comment at: include/clang/AST/Type.h:1108 +PCK_ARCStrong, // objc strong pointer. +PCK_Struct // non-trivial C struct. + }; These should all be /// documentation comments, and they mostly shouldn't talk about fields since this is a query on QualType, not FieldDecl. I would suggest something like: for Trivial - The type does not fall into any of the following categories. Note that this case is zero-valued so that values of this enum can be used as a boolean condition for non-triviality. for VolatileTrivial - The type would be trivial except that it is volatile-qualified. Types that fall into one of the other non-trivial cases may additionally be volatile-qualified. for ARCStrong - The type is an Objective-C retainable pointer type that is qualified with the ARC __strong qualifier. for Struct - The type is a struct containing a field whose type is not PCK_Trivial. Note that a C++ struct type does not necessarily match this; C++ copying semantics are too complex to express here, in part because they depend on the exact constructor or assignment operator that is chosen by overload resolution to do the copy. Comment at: include/clang/AST/Type.h:1121 + /// after it is moved, as opposed to a truely destructive move in which the + /// source object is placed in an uninitialized state. + PrimitiveCopyKind isNonTrivialToPrimitiveDestructiveMove() const; "truly" Hmm. Now that I'm thinking more about it, I'm not sure there's any point in tracking non-triviality of a C++-style destructive move separately from the non-triviality of a copy. It's hard to imagine that there would ever be a non-C++ type that primitively has non-trivial copies but trivial C++-style moves or vice-versa. Type-based destructors imply that the type represents some kind of resource, and a C++-style move will always be non-trivial for resource types because ownership of the resource needs to be given up by the old location. Otherwise, a type might be non-trivial to copy but not destroy because there's something special about how it's stored (like volatility), but it's hard to imagine what could possibly cause it to be non-trivial to destroy but not copy. If we were tracking non-triviality of an *unsafe* destructive move, one that leaves the source in an uninitialized state, that's quite different. I think there are three reasonable options here: - Ignore the argument I just made about the types that we're *likely* to care about modeling and generalize your tracking to also distinguish construction from assignment. In such an environment, I think you can absolutely make an argument that it's still interesting to track C++-style moves separately from copies. - Drop the tracking of destructive moves completely. If you want to keep the method around, find, but it can just call `isNonTrivialToPrimitiveCopy()`. - Change the tracking of *destructive* moves to instead track *deinitializing* moves. The implementation would stop considering `__strong` types to be non-trivial to move. But as things stand today, I do not see any point in separately tracking triviality of C++-style destructive moves. Comment at: lib/AST/Type.cpp:2231 + + Qualifiers::ObjCLifetime Lifetime = getQualifiers().getObjCLifetime(); + if (Lifetime == Qualifiers::OCL_Strong) You can re-use this call to getQualifiers() in the check for `volatile` below. Comment at: lib/AST/Type.cpp:3945 +if (RT->getDecl()->isNonTrivialToPrimitiveDestroy()) + return DK_nontrivial_c_struct; + Please combine this check with the getAsCXXRecordDecl() check below. That is, (1) check whether it's a record type once, and if so, (2) it's a CXXRecordDecl, ask about its destructor, and (3) if it's not a CXXRecordDecl, ask whether it's non-trivial to primitive destroy. Comment at: lib/CodeGen/CGBlocks.cpp:1779 +break; } This entire switch can be within an `#ifndef NDEBUG` if you really feel it's important to assert. Comment at: lib/CodeGen/CGBlocks.cpp:1792 +return std::make_pair(BlockCaptureEntityKind::BlockObject, + getBlockFieldFlagsForObjCObjectPointer(CI, T)); I think you should leave the byref case of that function here, but you can make it local to this if-clause. Comment at: lib/CodeGen/CGDecl.cpp:1421 +destroyer = CodeGenFunction::destroyNonTrivialCStruct; +cleanupKind = getARCCleanupKind(); +break; rjmccall wrote: > rjmccall wrote: > > ahatanak wrote: > > > rjmccall wrote: > > > > This can only be getARCCleanupKind() if it's non-trivial to destroy > > > > solely because of __strong members. Since the setup here is > > > > significantly more general than ARC, I think you need to default to th
Re: r325691 - [clang-format] New API guessLanguage()
Fix landed in r325722. On Wed, Feb 21, 2018 at 1:53 PM Ben Hamilton wrote: > Thanks for the report, and sorry for the breakage. > > Here's the fix: https://reviews.llvm.org/D43590 > > Ben > > On Wed, Feb 21, 2018 at 1:47 PM wrote: > >> Hi Ben, >> >> Your change is causing a test failure on one of the bots, can you take a >> look? >> >> >> http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/builds/25515 >> >> FAIL: Clang Tools :: clang-apply-replacements/format.cpp (12526 of 38114) >> TEST 'Clang Tools :: >> clang-apply-replacements/format.cpp' FAILED >> Script: >> -- >> mkdir -p >> /home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/tools/clang/tools/extra/test/clang-apply-replacements/Output/Inputs/format >> grep -Ev "// *[A-Z-]+:" >> /home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/tools/extra/test/clang-apply-replacements/Inputs/format/yes.cpp >> > >> /home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/tools/clang/tools/extra/test/clang-apply-replacements/Output/Inputs/format/yes.cpp >> grep -Ev "// *[A-Z-]+:" >> /home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/tools/extra/test/clang-apply-replacements/Inputs/format/no.cpp >> > >> /home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/tools/clang/tools/extra/test/clang-apply-replacements/Output/Inputs/format/no.cpp >> sed >> "s#\$(path)#/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/tools/clang/tools/extra/test/clang-apply-replacements/Output/Inputs/format#" >> /home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/tools/extra/test/clang-apply-replacements/Inputs/format/yes.yaml >> > >> /home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/tools/clang/tools/extra/test/clang-apply-replacements/Output/Inputs/format/yes.yaml >> sed >> "s#\$(path)#/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/tools/clang/tools/extra/test/clang-apply-replacements/Output/Inputs/format#" >> /home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/tools/extra/test/clang-apply-replacements/Inputs/format/no.yaml >> > >> /home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/tools/clang/tools/extra/test/clang-apply-replacements/Output/Inputs/format/no.yaml >> clang-apply-replacements -format >> /home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/tools/clang/tools/extra/test/clang-apply-replacements/Output/Inputs/format >> FileCheck --strict-whitespace >> -input-file=/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/tools/clang/tools/extra/test/clang-apply-replacements/Output/Inputs/format/yes.cpp >> /home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/tools/extra/test/clang-apply-replacements/Inputs/format/yes.cpp >> FileCheck --strict-whitespace >> -input-file=/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/tools/clang/tools/extra/test/clang-apply-replacements/Output/Inputs/format/no.cpp >> /home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/tools/extra/test/clang-apply-replacements/Inputs/format/no.cpp >> -- >> Exit Code: 134 >> >> Command Output (stderr): >> -- >> clang-apply-replacements: >> /home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/lib/Basic/SourceManager.cpp:393: >> const clang::SrcMgr::ContentCache >> *clang::SourceManager::getOrCreateContentCache(const clang::FileEntry *, >> bool): Assertion `FileEnt && "Didn't specify a file entry to use?"' failed. >> /home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/tools/clang/tools/extra/test/clang-apply-replacements/Output/format.cpp.script: >> line 8: 49680 Aborted (core dumped) >> clang-apply-replacements -format >> /home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/tools/clang/tools/extra/test/clang-apply-replacements/Output/Inputs/format >> >> -- >> >> >> >> Douglas Yung >> >> >> > -Original Message- >> > From: cfe-commits [mailto:cfe-commits-boun...@lists.llvm.org] On >> Behalf Of Ben >> > Hamilton via cfe-commits >> > Sent: Wednesday, February 21, 2018 7:55 >> > To: cfe-commits@lists.llvm.org >> > Subject: r325691 - [clang-format] New API guessLanguage() >> > >> > Author: benhamilton >> > Date: Wed Feb 21 07:54:31 2018 >> > New Revision: 325691 >> > >> > URL: http://llvm.org/viewvc/llvm-project?rev=325691&view=rev >> > Log: >> > [clang-format] New API guessLanguage() >> > >> > Summary: >> > For clie
[PATCH] D43590: [clang-format] Fix regression when getStyle() called with empty filename
This revision was automatically updated to reflect the committed changes.
Closed by commit rC325722: [clang-format] Fix regression when getStyle() called
with empty filename (authored by benhamilton, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D43590?vs=135314&id=135321#toc
Repository:
rC Clang
https://reviews.llvm.org/D43590
Files:
lib/Format/Format.cpp
unittests/Format/FormatTest.cpp
Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -2297,12 +2297,13 @@
FormatStyle::LanguageKind guessLanguage(StringRef FileName, StringRef Code) {
FormatStyle::LanguageKind result = getLanguageByFileName(FileName);
if (result == FormatStyle::LK_Cpp) {
-auto extension = llvm::sys::path::extension(FileName);
+auto Extension = llvm::sys::path::extension(FileName);
// If there's no file extension (or it's .h), we need to check the contents
// of the code to see if it contains Objective-C.
-if (extension.empty() || extension == ".h") {
+if (Extension.empty() || Extension == ".h") {
+ auto NonEmptyFileName = FileName.empty() ? "guess.h" : FileName;
std::unique_ptr Env =
- Environment::CreateVirtualEnvironment(Code, FileName, /*Ranges=*/{});
+ Environment::CreateVirtualEnvironment(Code, NonEmptyFileName,
/*Ranges=*/{});
ObjCHeaderStyleGuesser Guesser(*Env, getLLVMStyle());
Guesser.process();
if (Guesser.isObjC()) {
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -11723,6 +11723,12 @@
verifyFormat("__super::FooBar();");
}
+TEST(FormatStyle, GetStyleWithEmptyFileName) {
+ auto Style1 = getStyle("file", "", "Google");
+ ASSERT_TRUE((bool)Style1);
+ ASSERT_EQ(*Style1, getGoogleStyle());
+}
+
TEST(FormatStyle, GetStyleOfFile) {
vfs::InMemoryFileSystem FS;
// Test 1: format file in the same directory.
Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -2297,12 +2297,13 @@
FormatStyle::LanguageKind guessLanguage(StringRef FileName, StringRef Code) {
FormatStyle::LanguageKind result = getLanguageByFileName(FileName);
if (result == FormatStyle::LK_Cpp) {
-auto extension = llvm::sys::path::extension(FileName);
+auto Extension = llvm::sys::path::extension(FileName);
// If there's no file extension (or it's .h), we need to check the contents
// of the code to see if it contains Objective-C.
-if (extension.empty() || extension == ".h") {
+if (Extension.empty() || Extension == ".h") {
+ auto NonEmptyFileName = FileName.empty() ? "guess.h" : FileName;
std::unique_ptr Env =
- Environment::CreateVirtualEnvironment(Code, FileName, /*Ranges=*/{});
+ Environment::CreateVirtualEnvironment(Code, NonEmptyFileName, /*Ranges=*/{});
ObjCHeaderStyleGuesser Guesser(*Env, getLLVMStyle());
Guesser.process();
if (Guesser.isObjC()) {
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -11723,6 +11723,12 @@
verifyFormat("__super::FooBar();");
}
+TEST(FormatStyle, GetStyleWithEmptyFileName) {
+ auto Style1 = getStyle("file", "", "Google");
+ ASSERT_TRUE((bool)Style1);
+ ASSERT_EQ(*Style1, getGoogleStyle());
+}
+
TEST(FormatStyle, GetStyleOfFile) {
vfs::InMemoryFileSystem FS;
// Test 1: format file in the same directory.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43586: CodeGen: handle blocks correctly when inalloca'ed
rjmccall accepted this revision. rjmccall added a comment. This revision is now accepted and ready to land. I hate `inalloca` so much. Okay. Repository: rC Clang https://reviews.llvm.org/D43586 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[libcxx] r325723 - libcxx: Unbreak external thread library configuration.
Author: pcc Date: Wed Feb 21 13:36:18 2018 New Revision: 325723 URL: http://llvm.org/viewvc/llvm-project?rev=325723&view=rev Log: libcxx: Unbreak external thread library configuration. Differential Revision: https://reviews.llvm.org/D42503 Modified: libcxx/trunk/include/__threading_support Modified: libcxx/trunk/include/__threading_support URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/__threading_support?rev=325723&r1=325722&r2=325723&view=diff == --- libcxx/trunk/include/__threading_support (original) +++ libcxx/trunk/include/__threading_support Wed Feb 21 13:36:18 2018 @@ -31,6 +31,14 @@ _LIBCPP_PUSH_MACROS #include <__undef_macros> +#if defined(_LIBCPP_HAS_THREAD_LIBRARY_EXTERNAL) || \ +defined(_LIBCPP_BUILDING_THREAD_LIBRARY_EXTERNAL) || \ +defined(_LIBCPP_HAS_THREAD_API_WIN32) +#define _LIBCPP_THREAD_ABI_VISIBILITY _LIBCPP_FUNC_VIS +#else +#define _LIBCPP_THREAD_ABI_VISIBILITY inline _LIBCPP_INLINE_VISIBILITY +#endif + #if defined(__FreeBSD__) && defined(__clang__) && __has_attribute(no_thread_safety_analysis) #define _LIBCPP_NO_THREAD_SAFETY_ANALYSIS __attribute__((no_thread_safety_analysis)) #else @@ -39,15 +47,7 @@ _LIBCPP_PUSH_MACROS _LIBCPP_BEGIN_NAMESPACE_STD -#if defined(_LIBCPP_HAS_THREAD_LIBRARY_EXTERNAL) || \ -defined(_LIBCPP_BUILDING_THREAD_LIBRARY_EXTERNAL) - -#define _LIBCPP_THREAD_ABI_VISIBILITY _LIBCPP_FUNC_VIS - -#elif defined(_LIBCPP_HAS_THREAD_API_PTHREAD) - -#define _LIBCPP_THREAD_ABI_VISIBILITY inline _LIBCPP_INLINE_VISIBILITY - +#if defined(_LIBCPP_HAS_THREAD_API_PTHREAD) // Mutex typedef pthread_mutex_t __libcpp_mutex_t; #define _LIBCPP_MUTEX_INITIALIZER PTHREAD_MUTEX_INITIALIZER @@ -75,9 +75,6 @@ typedef pthread_key_t __libcpp_tls_key; #define _LIBCPP_TLS_DESTRUCTOR_CC #else - -#define _LIBCPP_THREAD_ABI_VISIBILITY _LIBCPP_FUNC_VIS - // Mutex typedef void* __libcpp_mutex_t; #define _LIBCPP_MUTEX_INITIALIZER 0 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43184: [WIP] [ItaniunCXXABI] Add an option for loading the type info vtable with dllimport
mstorsjo added a comment. Ping @rnk Repository: rC Clang https://reviews.llvm.org/D43184 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42503: libcxx: Unbreak external thread library configuration.
This revision was automatically updated to reflect the committed changes. Closed by commit rCXX325723: libcxx: Unbreak external thread library configuration. (authored by pcc, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit: https://reviews.llvm.org/D42503?vs=131344&id=135324#toc Repository: rCXX libc++ https://reviews.llvm.org/D42503 Files: include/__threading_support Index: include/__threading_support === --- include/__threading_support +++ include/__threading_support @@ -31,23 +31,23 @@ _LIBCPP_PUSH_MACROS #include <__undef_macros> +#if defined(_LIBCPP_HAS_THREAD_LIBRARY_EXTERNAL) || \ +defined(_LIBCPP_BUILDING_THREAD_LIBRARY_EXTERNAL) || \ +defined(_LIBCPP_HAS_THREAD_API_WIN32) +#define _LIBCPP_THREAD_ABI_VISIBILITY _LIBCPP_FUNC_VIS +#else +#define _LIBCPP_THREAD_ABI_VISIBILITY inline _LIBCPP_INLINE_VISIBILITY +#endif + #if defined(__FreeBSD__) && defined(__clang__) && __has_attribute(no_thread_safety_analysis) #define _LIBCPP_NO_THREAD_SAFETY_ANALYSIS __attribute__((no_thread_safety_analysis)) #else #define _LIBCPP_NO_THREAD_SAFETY_ANALYSIS #endif _LIBCPP_BEGIN_NAMESPACE_STD -#if defined(_LIBCPP_HAS_THREAD_LIBRARY_EXTERNAL) || \ -defined(_LIBCPP_BUILDING_THREAD_LIBRARY_EXTERNAL) - -#define _LIBCPP_THREAD_ABI_VISIBILITY _LIBCPP_FUNC_VIS - -#elif defined(_LIBCPP_HAS_THREAD_API_PTHREAD) - -#define _LIBCPP_THREAD_ABI_VISIBILITY inline _LIBCPP_INLINE_VISIBILITY - +#if defined(_LIBCPP_HAS_THREAD_API_PTHREAD) // Mutex typedef pthread_mutex_t __libcpp_mutex_t; #define _LIBCPP_MUTEX_INITIALIZER PTHREAD_MUTEX_INITIALIZER @@ -75,9 +75,6 @@ #define _LIBCPP_TLS_DESTRUCTOR_CC #else - -#define _LIBCPP_THREAD_ABI_VISIBILITY _LIBCPP_FUNC_VIS - // Mutex typedef void* __libcpp_mutex_t; #define _LIBCPP_MUTEX_INITIALIZER 0 Index: include/__threading_support === --- include/__threading_support +++ include/__threading_support @@ -31,23 +31,23 @@ _LIBCPP_PUSH_MACROS #include <__undef_macros> +#if defined(_LIBCPP_HAS_THREAD_LIBRARY_EXTERNAL) || \ +defined(_LIBCPP_BUILDING_THREAD_LIBRARY_EXTERNAL) || \ +defined(_LIBCPP_HAS_THREAD_API_WIN32) +#define _LIBCPP_THREAD_ABI_VISIBILITY _LIBCPP_FUNC_VIS +#else +#define _LIBCPP_THREAD_ABI_VISIBILITY inline _LIBCPP_INLINE_VISIBILITY +#endif + #if defined(__FreeBSD__) && defined(__clang__) && __has_attribute(no_thread_safety_analysis) #define _LIBCPP_NO_THREAD_SAFETY_ANALYSIS __attribute__((no_thread_safety_analysis)) #else #define _LIBCPP_NO_THREAD_SAFETY_ANALYSIS #endif _LIBCPP_BEGIN_NAMESPACE_STD -#if defined(_LIBCPP_HAS_THREAD_LIBRARY_EXTERNAL) || \ -defined(_LIBCPP_BUILDING_THREAD_LIBRARY_EXTERNAL) - -#define _LIBCPP_THREAD_ABI_VISIBILITY _LIBCPP_FUNC_VIS - -#elif defined(_LIBCPP_HAS_THREAD_API_PTHREAD) - -#define _LIBCPP_THREAD_ABI_VISIBILITY inline _LIBCPP_INLINE_VISIBILITY - +#if defined(_LIBCPP_HAS_THREAD_API_PTHREAD) // Mutex typedef pthread_mutex_t __libcpp_mutex_t; #define _LIBCPP_MUTEX_INITIALIZER PTHREAD_MUTEX_INITIALIZER @@ -75,9 +75,6 @@ #define _LIBCPP_TLS_DESTRUCTOR_CC #else - -#define _LIBCPP_THREAD_ABI_VISIBILITY _LIBCPP_FUNC_VIS - // Mutex typedef void* __libcpp_mutex_t; #define _LIBCPP_MUTEX_INITIALIZER 0 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r325724 - CodeGen: handle blocks correctly when inalloca'ed
Author: compnerd
Date: Wed Feb 21 13:47:51 2018
New Revision: 325724
URL: http://llvm.org/viewvc/llvm-project?rev=325724&view=rev
Log:
CodeGen: handle blocks correctly when inalloca'ed
When using blocks with C++ on Windows x86, it is possible to have the
block literal be pushed into the inalloca'ed parameters. Teach IRGen to
handle the case properly by extracting the block literal from the
inalloca parameter. This fixes the use of blocks with C++ on Windows
x86.
Added:
cfe/trunk/test/CodeGenCXX/block-inalloca.cpp
Modified:
cfe/trunk/lib/CodeGen/CGDecl.cpp
Modified: cfe/trunk/lib/CodeGen/CGDecl.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDecl.cpp?rev=325724&r1=325723&r2=325724&view=diff
==
--- cfe/trunk/lib/CodeGen/CGDecl.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGDecl.cpp Wed Feb 21 13:47:51 2018
@@ -1848,9 +1848,12 @@ void CodeGenFunction::EmitParmDecl(const
// Use better IR generation for certain implicit parameters.
if (auto IPD = dyn_cast(&D)) {
// The only implicit argument a block has is its literal.
-// We assume this is always passed directly.
+// This may be passed as an inalloca'ed value on Windows x86.
if (BlockInfo) {
- setBlockContextParameter(IPD, ArgNo, Arg.getDirectValue());
+ llvm::Value *V = Arg.isIndirect()
+ ? Builder.CreateLoad(Arg.getIndirectAddress())
+ : Arg.getDirectValue();
+ setBlockContextParameter(IPD, ArgNo, V);
return;
}
}
Added: cfe/trunk/test/CodeGenCXX/block-inalloca.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/block-inalloca.cpp?rev=325724&view=auto
==
--- cfe/trunk/test/CodeGenCXX/block-inalloca.cpp (added)
+++ cfe/trunk/test/CodeGenCXX/block-inalloca.cpp Wed Feb 21 13:47:51 2018
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -triple i686-unknown-windows-msvc -fblocks -emit-llvm -o -
%s | FileCheck %s
+
+struct S {
+ S(const struct S &) {}
+};
+
+void (^b)(S) = ^(S) {};
+
+// CHECK: [[DESCRIPTOR:%.*]] = getelementptr inbounds <{ i8*, %struct.S, [3 x
i8] }>, <{ i8*, %struct.S, [3 x i8] }>* %0, i32 0, i32 0
+// CHECK: load i8*, i8** [[DESCRIPTOR]]
+
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43545: [Driver] Make -fno-common default for Fuchsia
jakehehrlich accepted this revision. jakehehrlich added a comment. This revision is now accepted and ready to land. LGTM Repository: rC Clang https://reviews.llvm.org/D43545 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43586: CodeGen: handle blocks correctly when inalloca'ed
compnerd closed this revision. compnerd added a comment. SVN r325724 Repository: rC Clang https://reviews.llvm.org/D43586 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41102: Setup clang-doc frontend framework
jakehehrlich added inline comments.
Comment at: clang-doc/BitcodeWriter.cpp:407
+
+void ClangDocBinaryWriter::writeBitstream(const EnumInfo &I,
+ BitstreamWriter &Stream,
lebedev.ri wrote:
> juliehockett wrote:
> > lebedev.ri wrote:
> > > Hmm, common pattern again
> > > ```
> > > void ClangDocBinaryWriter::writeBitstream(const &I,
> > > BitstreamWriter &Stream,
> > > bool writeBlockInfo) {
> > > if (writeBlockInfo) emitBlockInfoBlock(Stream);
> > > StreamSubBlock Block(Stream, BI__BLOCK_ID);
> > > ...
> > > }
> > > ```
> > > Could be solved if a mapping from `TYPENAME` ->
> > > `BI__BLOCK_ID` can be added.
> > > If LLVM would be using C++14, that'd be easy, but with C++11, it would
> > > require whole new class (although with just a single static variable).
> > Do you want me to try to write that class, or leave it as it is?
> It would be something like: (total guesswork, literally just wrote it here,
> like rest of the snippets)
> ```
> template
> struct MapFromTypeToEnumerator {
> static const BlockId id;
> };
>
> template <>
> struct MapFromTypeToEnumerator {
> static const BlockId id = BI_NAMESPACE_BLOCK_ID;
> };
> void ClangDocBitcodeWriter::writeBitstream(const NamespaceInfo &I) {
> EMITINFO(NAMESPACE)
> }
> ...
>
> template
> void ClangDocBitcodeWriter::writeBitstream(const TypeInfo &I, bool
> writeBlockInfo) {
> if (writeBlockInfo) emitBlockInfoBlock();
> StreamSubBlockGuard Block(Stream, MapFromTypeToEnumerator::id);
> writeBitstream(I);
> }
> ```
> Uhm, now that i have wrote it, it does not look as ugly as i though it would
> look...
> So maybe try integrating that, i *think* it is a bit cleaner?
If we know the set of types then it should just be a static member of every
*Info type. Then the mapping is just TYPENAME::id
https://reviews.llvm.org/D41102
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
