Re: r338602 - Revert r338455 "[constexpr] Support for constant evaluation of __builtin_memcpy and __builtin_memmove (in non-type-punning cases)."
Merged to 7.0 in r338674.
On Wed, Aug 1, 2018 at 7:51 PM, Hans Wennborg via cfe-commits
wrote:
> Author: hans
> Date: Wed Aug 1 10:51:23 2018
> New Revision: 338602
>
> URL: http://llvm.org/viewvc/llvm-project?rev=338602&view=rev
> Log:
> Revert r338455 "[constexpr] Support for constant evaluation of
> __builtin_memcpy and __builtin_memmove (in non-type-punning cases)."
>
> It caused asserts during Chromium builds, see reply on the cfe-commits thread.
>
>> This is intended to permit libc++ to make std::copy etc constexpr
>> without sacrificing the optimization that uses memcpy on
>> trivially-copyable types.
>>
>> __builtin_strcpy and __builtin_wcscpy are not handled by this change.
>> They'd be straightforward to add, but we haven't encountered a need for
>> them just yet.
>
> Modified:
> cfe/trunk/include/clang/Basic/Builtins.def
> cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td
> cfe/trunk/lib/AST/ExprConstant.cpp
> cfe/trunk/test/CodeGen/builtin-memfns.c
> cfe/trunk/test/SemaCXX/constexpr-string.cpp
>
> Modified: cfe/trunk/include/clang/Basic/Builtins.def
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Builtins.def?rev=338602&r1=338601&r2=338602&view=diff
> ==
> --- cfe/trunk/include/clang/Basic/Builtins.def (original)
> +++ cfe/trunk/include/clang/Basic/Builtins.def Wed Aug 1 10:51:23 2018
> @@ -471,8 +471,6 @@ BUILTIN(__builtin_wcslen, "zwC*", "nF")
> BUILTIN(__builtin_wcsncmp, "iwC*wC*z", "nF")
> BUILTIN(__builtin_wmemchr, "w*wC*wz", "nF")
> BUILTIN(__builtin_wmemcmp, "iwC*wC*z", "nF")
> -BUILTIN(__builtin_wmemcpy, "w*w*wC*z", "nF")
> -BUILTIN(__builtin_wmemmove, "w*w*wC*z", "nF")
> BUILTIN(__builtin_return_address, "v*IUi", "n")
> BUILTIN(__builtin_extract_return_addr, "v*v*", "n")
> BUILTIN(__builtin_frame_address, "v*IUi", "n")
> @@ -910,8 +908,6 @@ LIBBUILTIN(wcslen, "zwC*", "f", "wc
> LIBBUILTIN(wcsncmp, "iwC*wC*z", "f", "wchar.h", ALL_LANGUAGES)
> LIBBUILTIN(wmemchr, "w*wC*wz", "f", "wchar.h", ALL_LANGUAGES)
> LIBBUILTIN(wmemcmp, "iwC*wC*z", "f", "wchar.h", ALL_LANGUAGES)
> -LIBBUILTIN(wmemcpy, "w*w*wC*z", "f", "wchar.h", ALL_LANGUAGES)
> -LIBBUILTIN(wmemmove,"w*w*wC*z", "f", "wchar.h", ALL_LANGUAGES)
>
> // C99
> // In some systems setjmp is a macro that expands to _setjmp. We undefine
>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td?rev=338602&r1=338601&r2=338602&view=diff
> ==
> --- cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td Wed Aug 1 10:51:23
> 2018
> @@ -163,20 +163,6 @@ def note_constexpr_unsupported_unsized_a
> def note_constexpr_unsized_array_indexed : Note<
>"indexing of array without known bound is not allowed "
>"in a constant expression">;
> -def note_constexpr_memcpy_type_pun : Note<
> - "cannot constant evaluate '%select{memcpy|memmove}0' from object of "
> - "type %1 to object of type %2">;
> -def note_constexpr_memcpy_nontrivial : Note<
> - "cannot constant evaluate '%select{memcpy|memmove}0' between objects of "
> - "non-trivially-copyable type %1">;
> -def note_constexpr_memcpy_overlap : Note<
> - "'%select{memcpy|wmemcpy}0' between overlapping memory regions">;
> -def note_constexpr_memcpy_unsupported : Note<
> - "'%select{%select{memcpy|wmemcpy}1|%select{memmove|wmemmove}1}0' "
> - "not supported: %select{"
> - "size to copy (%4) is not a multiple of size of element type %3 (%5)|"
> - "source is not a contiguous array of at least %4 elements of type %3|"
> - "destination is not a contiguous array of at least %4 elements of type
> %3}2">;
>
> def warn_integer_constant_overflow : Warning<
>"overflow in expression; result is %0 with type %1">,
>
> Modified: cfe/trunk/lib/AST/ExprConstant.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=338602&r1=338601&r2=338602&view=diff
> ==
> --- cfe/trunk/lib/AST/ExprConstant.cpp (original)
> +++ cfe/trunk/lib/AST/ExprConstant.cpp Wed Aug 1 10:51:23 2018
> @@ -319,25 +319,6 @@ namespace {
>return false;
> }
>
> -/// Get the range of valid index adjustments in the form
> -/// {maximum value that can be subtracted from this pointer,
> -///maximum value that can be added to this pointer}
> -std::pair validIndexAdjustments() {
> - if (Invalid || isMostDerivedAnUnsizedArray())
> -return {0, 0};
> -
> - // [expr.add]p4: For the purposes of these operators, a pointer to a
> - // nonarray object behaves the same as a pointer to the first element
> of
> - // an array of length one with the type of the object as its element
> type.
>
[PATCH] D49922: [P0936R0] add [[clang::lifetimebound]] attribute
kimgr added a comment.
Potential typo in the tests
Comment at: test/SemaCXX/attr-lifetimebound.cpp:24
+
+ // Do not diagnose non-void return types; they can still be lifetime-bound.
+ long long ptrintcast(int ¶m [[clang::lifetimebound]]) {
Should this say 'non-ref' instead of 'non-void'?
Repository:
rC Clang
https://reviews.llvm.org/D49922
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50003: Sema: Fix explicit address space cast involving void pointers
rjmccall added inline comments. Comment at: lib/Sema/SemaCast.cpp:1050 + SrcType->getAs()->getPointeeType().getAddressSpace() != + DestType->getAs()->getPointeeType().getAddressSpace(); +} yaxunl wrote: > rjmccall wrote: > > I know the code was like this before, but please rewrite this to just use > > `getAs()` instead of doing the separate `isPointerType()` > > check. > IsAddressSpaceConversion is also called in line 2218 of the same file, where > SrcType or DestType may not be pointer type. `getAs` is a query; it returns null if the type isn't of the target type. https://reviews.llvm.org/D50003 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50119: Compiler support for P1144R0 "__is_trivially_relocatable(T)"
Rakete added inline comments.
Comment at: compiler-explorer-llvm-commit.sh:1
+# This is the commit of LLVM that we're currently based on.
+git reset --hard 1fa19f68007cd126a04448093c171f40e556087e
What's this file? A mistake?
Comment at: include/clang/AST/DeclCXX.h:471
+/// True when this class's bases and fields are all trivially relocatable,
+/// and the class itself has a defaulted move constructor and a defaulted
That comment misses a "or is a reference".
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:2942
+ "not %select{move-constructible|destructible}2"
+ >;
+
Quuxplusone wrote:
> > Might be useful to add a note explaining why the type isn't trivially
> > relocatable instead of the general "because it is not destructible".
>
> You mean, like, a series of notes pointing at "because its base class B is
> not destructible... because B's destructor is defined as deleted here"? I
> agree that might be helpful, but since it only happens when the programmer is
> messing around with the attribute anyway, I wouldn't want to do anything too
> innovative or LoC-consuming. I'd cut and paste ~10 lines of code from
> somewhere else that already does something like that if you point me at it,
> but otherwise I think it's not worth the number of extra codepaths that would
> need to be tested.
Fair enough.
Comment at: include/clang/Sema/Sema.h:4304
+ bool IsTriviallyRelocatableType(QualType QT) const;
+
Quuxplusone wrote:
> Rakete wrote:
> > Any reason why this is a free function? Should be a member function of
> > `QualType`.
> `class QualType` is defined in `AST/`, whereas all the C++-specific
> TriviallyRelocatable stuff is currently confined to `Sema/`. My impression
> was that I should try to preserve that separation, even if it meant being
> ugly right here. I agree that this is a stupid hack, and would love to get
> rid of it, but I think I need some guidance as to how much mixing of `AST`
> and `Sema` is permitted.
Nah, it's fine. There are lots of C++ specific things in AST/, because the AST
nodes represent C++-y stuff. Trivially copyable is also part of `QualType`,
even though it's C++ specific.
Comment at: lib/Sema/SemaDecl.cpp:15823
CXXRecord->setImplicitDestructorIsDeleted();
+CXXRecord->setIsNotNaturallyTriviallyRelocatable();
SetDeclDeleted(Dtor, CXXRecord->getLocation());
You don't need this. This is already handled by `CheckCompleteCXXClass`.
Comment at: lib/Sema/SemaDeclAttr.cpp:6481
break;
+ case ParsedAttr::AT_TriviallyRelocatable:
+handleTriviallyRelocatableAttr(S, D, AL);
Why is this attribute under "Microsoft Attributes"? ;)
Comment at: lib/Sema/SemaDeclCXX.cpp:6166
+if (SMOR.getKind() != SpecialMemberOverloadResult::Success ||
+!SMOR.getMethod()->isDefaulted()) {
+ Record->setIsNotNaturallyTriviallyRelocatable();
Extra braces.
Comment at: lib/Sema/SemaDeclCXX.cpp:6187
+Record->dropAttr();
+ } else if (Record->needsImplicitMoveConstructor() &&
+ Record->defaultedMoveConstructorIsDeleted()) {
This is dead code. `Record` never needs an implicit move constructor at this
point, because either 1) it never did or 2) it was defined above by
`LookupSpecialMember`.
Comment at: lib/Sema/SemaDeclCXX.cpp:12631
ClassDecl->setImplicitMoveConstructorIsDeleted();
+ClassDecl->setIsNotNaturallyTriviallyRelocatable();
SetDeclDeleted(MoveConstructor, ClassLoc);
Same, already handled by `CheckCompleteCXXClass`.
Comment at: lib/Sema/SemaDeclCXX.cpp:6157
+
+ if (getLangOpts().CPlusPlus11 &&
+ !Record->hasAttr() &&
Quuxplusone wrote:
> Rakete wrote:
> > This really just checks whether the type has defaulted copy constructor. If
> > there was a move constructor, it would have been handled above. If the
> > copy/move constructor is implicitly deleted, it would have been handled
> > also above. Please simplify this accordingly.
> Can you elaborate on this one? I assume you mean that some of lines 6157
> through 6171 are superfluous, but I can't figure out which ones or how to
> simplify it.
Actually, nvm. I was thinking of calling a few functions instead of call
`LookupSpecialMember`, but I forgot that those functions can only work if there
was previously a call to `LookupSpecialMember` :/.
Comment at: test/SemaCXX/trivially-relocatable.cpp:42
+struct A6;
+struct [[trivially_relocatable]] A6 {};
+// expected-error@-1{{type A6 declared 'trivially_relocatable' after its first
declaration}}
Quuxplusone wrote:
> Rakete wrote:
> > Why does this restriction exi
[PATCH] D50003: Sema: Fix explicit address space cast involving void pointers
yaxunl added inline comments. Comment at: lib/Sema/SemaCast.cpp:1050 + SrcType->getAs()->getPointeeType().getAddressSpace() != + DestType->getAs()->getPointeeType().getAddressSpace(); +} rjmccall wrote: > I know the code was like this before, but please rewrite this to just use > `getAs()` instead of doing the separate `isPointerType()` check. IsAddressSpaceConversion is also called in line 2218 of the same file, where SrcType or DestType may not be pointer type. https://reviews.llvm.org/D50003 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49403: More aggressively complete RecordTypes with Function Pointers
rjmccall added a comment. My point is that IR pointer type information can't be "incorrect", because there is no semantic meaning for pointer types in LLVM IR. That has never been a part of the semantics of LLVM IR. Even if it were possible to change that — and I don't think it is, not without major changes, although I can very much understand why someone looking at compiler output might think "but we're so close right now" — we're not going to make that effort just for the benefit of one specific analysis, especially given that the consensus position is still that we should go further in the opposite direction and make pointer types stop carrying this meaningless information. https://reviews.llvm.org/D49403 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50152: [CodeGen] Merge equivalent block copy/helper functions
rjmccall added inline comments.
Comment at: lib/CodeGen/CGBlocks.cpp:1638
+switch (E.Kind) {
+case BlockCaptureEntityKind::CXXRecord: {
+ Name += "c";
I forget whether this case has already filtered out trivial copy-constructors,
but if not, you should consider doing that here.
Comment at: lib/CodeGen/CGBlocks.cpp:1647
+unsigned Qs;
+cast(CE)->getConstructor()->isCopyConstructor(Qs);
+if (Qs & Qualifiers::Volatile)
This doesn't always have to be a copy constructor. I mean, maybe this
procedure works anyway, but the constructor used to copy an object isn't always
a copy constructor. Blame constructor templates.
Comment at: lib/CodeGen/CGBlocks.cpp:1651
+ }
+ std::string Str = CaptureTy->getAsCXXRecordDecl()->getName();
+ Name += llvm::to_string(Str.size()) + Str;
The name of a C++ class is not unique; you need to use the mangler. Ideally,
you would find a way to mangle all the C++ types in the context of the same
mangler instance so that substitutions could be reused across it.
You also need to check for a type with non-external linkage and make this
helper private if you find one. (You can still unique within the translation
unit, for what it's worth.)
Repository:
rC Clang
https://reviews.llvm.org/D50152
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49361: [analyzer] Detect pointers escaped after return statement execution in MallocChecker
rnkovacs updated this revision to Diff 158681.
rnkovacs marked an inline comment as done.
rnkovacs added a comment.
Add helper function to be used in both callbacks.
https://reviews.llvm.org/D49361
Files:
lib/StaticAnalyzer/Checkers/MallocChecker.cpp
test/Analysis/inner-pointer.cpp
test/Analysis/malloc-free-after-return.cpp
Index: test/Analysis/malloc-free-after-return.cpp
===
--- /dev/null
+++ test/Analysis/malloc-free-after-return.cpp
@@ -0,0 +1,21 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=cplusplus.NewDelete -verify %s
+
+#include "Inputs/system-header-simulator-cxx.h"
+
+struct S {
+ S() : Data(new int) {}
+ ~S() { delete Data; }
+ int *getData() { return Data; }
+
+private:
+ int *Data;
+};
+
+int *freeAfterReturnTemp() {
+ return S().getData(); // expected-warning {{Use of memory after it is freed}}
+}
+
+int *freeAfterReturnLocal() {
+ S X;
+ return X.getData();
+} // expected-warning {{Use of memory after it is freed}}
Index: test/Analysis/inner-pointer.cpp
===
--- test/Analysis/inner-pointer.cpp
+++ test/Analysis/inner-pointer.cpp
@@ -361,3 +361,24 @@
consume(c);// expected-warning {{Use of memory after it is freed}}
// expected-note@-1 {{Use of memory after it is freed}}
}
+
+struct S {
+ std::string to_string() { return s; }
+private:
+ std::string s;
+};
+
+const char *escape_via_return_temp() {
+ S x;
+ return x.to_string().c_str(); // expected-note {{Dangling inner pointer obtained here}}
+ // expected-note@-1 {{Inner pointer invalidated by call to destructor}}
+ // expected-warning@-2 {{Use of memory after it is freed}}
+ // expected-note@-3 {{Use of memory after it is freed}}
+}
+
+const char *escape_via_return_local() {
+ std::string s;
+ return s.c_str(); // expected-note {{Dangling inner pointer obtained here}}
+// expected-note@-1 {{Inner pointer invalidated by call to destructor}}
+} // expected-warning {{Use of memory after it is freed}}
+// expected-note@-1 {{Use of memory after it is freed}}
Index: lib/StaticAnalyzer/Checkers/MallocChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -161,6 +161,7 @@
check::PointerEscape,
check::ConstPointerEscape,
check::PreStmt,
+ check::EndFunction,
check::PreCall,
check::PostStmt,
check::PostStmt,
@@ -217,6 +218,7 @@
void checkPostStmt(const BlockExpr *BE, CheckerContext &C) const;
void checkDeadSymbols(SymbolReaper &SymReaper, CheckerContext &C) const;
void checkPreStmt(const ReturnStmt *S, CheckerContext &C) const;
+ void checkEndFunction(const ReturnStmt *S, CheckerContext &C) const;
ProgramStateRef evalAssume(ProgramStateRef state, SVal Cond,
bool Assumption) const;
void checkLocation(SVal l, bool isLoad, const Stmt *S,
@@ -353,7 +355,7 @@
static ProgramStateRef CallocMem(CheckerContext &C, const CallExpr *CE,
ProgramStateRef State);
- ///Check if the memory associated with this symbol was released.
+ /// Check if the memory associated with this symbol was released.
bool isReleased(SymbolRef Sym, CheckerContext &C) const;
bool checkUseAfterFree(SymbolRef Sym, CheckerContext &C, const Stmt *S) const;
@@ -377,13 +379,16 @@
ProgramStateRef State,
SymbolRef &EscapingSymbol) const;
- // Implementation of the checkPointerEscape callabcks.
+ // Implementation of the checkPointerEscape callbacks.
ProgramStateRef checkPointerEscapeAux(ProgramStateRef State,
const InvalidatedSymbols &Escaped,
const CallEvent *Call,
PointerEscapeKind Kind,
bool(*CheckRefState)(const RefState*)) const;
+ // Implementation of the checkPreStmt and checkEndFunction callbacks.
+ void checkEscapeOnReturn(const ReturnStmt *S, CheckerContext &C) const;
+
///@{
/// Tells if a given family/call/symbol is tracked by the current checker.
/// Sets CheckKind to the kind of the checker responsible for this
@@ -2451,7 +2456,24 @@
}
}
-void MallocChecker::checkPreStmt(const ReturnStmt *S, CheckerContext &C) const {
+void MallocChecker::checkPreStmt(const ReturnStmt *S,
+ CheckerContext &C) const {
+ checkEscapeOnReturn(S, C);
+}
+
+// In the CFG, automatic destructors come after the return statement.
+// This callback checks for returning memory
[PATCH] D49811: [analyzer] Obtain a ReturnStmt from a CFGAutomaticObjDtor
rnkovacs updated this revision to Diff 158680.
rnkovacs added a comment.
In https://reviews.llvm.org/D49811#1175726, @NoQ wrote:
> I guess you could write a test with `debug.AnalysisOrder` (by making its
> `checkEndFunction` callback (that you'll have to define) print different
> things depending on the return statement), not sure if it's worth it; you can
> also merge this commit with https://reviews.llvm.org/D49361 instead.
This is what I could come up with, what do you think?
https://reviews.llvm.org/D49811
Files:
lib/StaticAnalyzer/Checkers/AnalysisOrderChecker.cpp
lib/StaticAnalyzer/Core/CoreEngine.cpp
test/Analysis/end-function-return-stmt.cpp
Index: test/Analysis/end-function-return-stmt.cpp
===
--- /dev/null
+++ test/Analysis/end-function-return-stmt.cpp
@@ -0,0 +1,34 @@
+//RUN: %clang_analyze_cc1 -analyzer-checker=debug.AnalysisOrder -analyzer-config debug.AnalysisOrder:EndFunction=true %s 2>&1 | FileCheck %s
+
+// At the end of a function, we can only obtain a ReturnStmt if the last
+// CFGElement in the CFGBlock is either a CFGStmt or a CFGAutomaticObjDtor.
+
+void noReturnStmt() {}
+
+struct S {
+ S();
+ ~S();
+};
+
+int dtorAfterReturnStmt() {
+ S s;
+ return 0;
+}
+
+S endsWithReturnStmt() {
+ return S();
+}
+
+// endsWithReturnStmt()
+// CHECK: EndFunction
+// CHECK-NEXT: ReturnStmt: yes
+// CHECK-NEXT: CFGElement: CFGStmt
+
+// dtorAfterReturnStmt()
+// CHECK: EndFunction
+// CHECK-NEXT: ReturnStmt: yes
+// CHECK-NEXT: CFGElement: CFGAutomaticObjDtor
+
+// noReturnStmt()
+// CHECK: EndFunction
+// CHECK-NEXT: ReturnStmt: no
Index: lib/StaticAnalyzer/Core/CoreEngine.cpp
===
--- lib/StaticAnalyzer/Core/CoreEngine.cpp
+++ lib/StaticAnalyzer/Core/CoreEngine.cpp
@@ -223,8 +223,12 @@
// Get return statement..
const ReturnStmt *RS = nullptr;
if (!L.getSrc()->empty()) {
- if (Optional LastStmt = L.getSrc()->back().getAs()) {
+ CFGElement LastElement = L.getSrc()->back();
+ if (Optional LastStmt = LastElement.getAs()) {
RS = dyn_cast(LastStmt->getStmt());
+ } else if (Optional AutoDtor =
+ LastElement.getAs()) {
+RS = dyn_cast(AutoDtor->getTriggerStmt());
}
}
Index: lib/StaticAnalyzer/Checkers/AnalysisOrderChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/AnalysisOrderChecker.cpp
+++ lib/StaticAnalyzer/Checkers/AnalysisOrderChecker.cpp
@@ -16,6 +16,7 @@
#include "ClangSACheckers.h"
#include "clang/AST/ExprCXX.h"
+#include "clang/Analysis/CFGStmtMap.h"
#include "clang/StaticAnalyzer/Core/Checker.h"
#include "clang/StaticAnalyzer/Core/CheckerManager.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
@@ -37,6 +38,7 @@
check::PostStmt,
check::PreCall,
check::PostCall,
+ check::EndFunction,
check::NewAllocator,
check::Bind,
check::RegionChanges,
@@ -121,6 +123,23 @@
}
}
+ void checkEndFunction(const ReturnStmt *S, CheckerContext &C) const {
+if (isCallbackEnabled(C, "EndFunction")) {
+ llvm::errs() << "EndFunction\nReturnStmt: " << (S ? "yes" : "no") << "\n";
+ if (!S)
+return;
+
+ llvm::errs() << "CFGElement: ";
+ CFGStmtMap *Map = C.getCurrentAnalysisDeclContext()->getCFGStmtMap();
+ CFGElement LastElement = Map->getBlock(S)->back();
+
+ if (LastElement.getAs())
+llvm::errs() << "CFGStmt\n";
+ else if (LastElement.getAs())
+llvm::errs() << "CFGAutomaticObjDtor\n";
+}
+ }
+
void checkNewAllocator(const CXXNewExpr *CNE, SVal Target,
CheckerContext &C) const {
if (isCallbackEnabled(C, "NewAllocator"))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49715: [analyzer] CallEvent: Add partially working methods for obtaining the callee stack frame.
NoQ added inline comments.
Comment at:
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h:431
+return ExprEngine::getObjectUnderConstruction(
+getState(), {getOriginExpr(), Index},
getCalleeStackFrame()).hasValue();
+ }
Whoops, this is wrong: should say `getLocationContext()`.
Repository:
rL LLVM
https://reviews.llvm.org/D49715
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[libcxx] r338668 - Implement P1023: constexpr comparison operators for std::array
Author: marshall
Date: Wed Aug 1 19:11:06 2018
New Revision: 338668
URL: http://llvm.org/viewvc/llvm-project?rev=338668&view=rev
Log:
Implement P1023: constexpr comparison operators for std::array
Modified:
libcxx/trunk/include/array
libcxx/trunk/test/std/containers/sequences/array/compare.pass.cpp
libcxx/trunk/www/cxx2a_status.html
Modified: libcxx/trunk/include/array
URL:
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/array?rev=338668&r1=338667&r2=338668&view=diff
==
--- libcxx/trunk/include/array (original)
+++ libcxx/trunk/include/array Wed Aug 1 19:11:06 2018
@@ -367,7 +367,7 @@ array(_Tp, _Args...)
template
inline _LIBCPP_INLINE_VISIBILITY
-bool
+_LIBCPP_CONSTEXPR_AFTER_CXX17 bool
operator==(const array<_Tp, _Size>& __x, const array<_Tp, _Size>& __y)
{
return _VSTD::equal(__x.begin(), __x.end(), __y.begin());
@@ -375,7 +375,7 @@ operator==(const array<_Tp, _Size>& __x,
template
inline _LIBCPP_INLINE_VISIBILITY
-bool
+_LIBCPP_CONSTEXPR_AFTER_CXX17 bool
operator!=(const array<_Tp, _Size>& __x, const array<_Tp, _Size>& __y)
{
return !(__x == __y);
@@ -383,7 +383,7 @@ operator!=(const array<_Tp, _Size>& __x,
template
inline _LIBCPP_INLINE_VISIBILITY
-bool
+_LIBCPP_CONSTEXPR_AFTER_CXX17 bool
operator<(const array<_Tp, _Size>& __x, const array<_Tp, _Size>& __y)
{
return _VSTD::lexicographical_compare(__x.begin(), __x.end(),
@@ -392,7 +392,7 @@ operator<(const array<_Tp, _Size>& __x,
template
inline _LIBCPP_INLINE_VISIBILITY
-bool
+_LIBCPP_CONSTEXPR_AFTER_CXX17 bool
operator>(const array<_Tp, _Size>& __x, const array<_Tp, _Size>& __y)
{
return __y < __x;
@@ -400,7 +400,7 @@ operator>(const array<_Tp, _Size>& __x,
template
inline _LIBCPP_INLINE_VISIBILITY
-bool
+_LIBCPP_CONSTEXPR_AFTER_CXX17 bool
operator<=(const array<_Tp, _Size>& __x, const array<_Tp, _Size>& __y)
{
return !(__y < __x);
@@ -408,7 +408,7 @@ operator<=(const array<_Tp, _Size>& __x,
template
inline _LIBCPP_INLINE_VISIBILITY
-bool
+_LIBCPP_CONSTEXPR_AFTER_CXX17 bool
operator>=(const array<_Tp, _Size>& __x, const array<_Tp, _Size>& __y)
{
return !(__x < __y);
Modified: libcxx/trunk/test/std/containers/sequences/array/compare.pass.cpp
URL:
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/containers/sequences/array/compare.pass.cpp?rev=338668&r1=338667&r2=338668&view=diff
==
--- libcxx/trunk/test/std/containers/sequences/array/compare.pass.cpp (original)
+++ libcxx/trunk/test/std/containers/sequences/array/compare.pass.cpp Wed Aug
1 19:11:06 2018
@@ -9,6 +9,7 @@
//
+// These are all constexpr in C++20
// bool operator==(array const&, array const&);
// bool operator!=(array const&, array const&);
// bool operator<(array const&, array const&);
@@ -40,6 +41,41 @@ void test_compare(const Array& LHS, cons
assert((LHS >= RHS) == (LHSV >= RHSV));
}
+#if TEST_STD_VER > 17
+template
+constexpr bool constexpr_compare(const Array &lhs, const Array &rhs, bool
isEqual, bool isLess)
+{
+ if (isEqual)
+ {
+if (!(lhs == rhs)) return false;
+if ( (lhs != rhs)) return false;
+if ( (lhs < rhs)) return false;
+if (!(lhs <= rhs)) return false;
+if ( (lhs > rhs)) return false;
+if (!(lhs >= rhs)) return false;
+ }
+ else if (isLess)
+ {
+if ( (lhs == rhs)) return false;
+if (!(lhs != rhs)) return false;
+if (!(lhs < rhs)) return false;
+if (!(lhs <= rhs)) return false;
+if ( (lhs > rhs)) return false;
+if ( (lhs >= rhs)) return false;
+ }
+ else // greater
+ {
+if ( (lhs == rhs)) return false;
+if (!(lhs != rhs)) return false;
+if ( (lhs < rhs)) return false;
+if ( (lhs <= rhs)) return false;
+if (!(lhs > rhs)) return false;
+if (!(lhs >= rhs)) return false;
+ }
+ return true;
+}
+#endif
+
int main()
{
{
@@ -60,4 +96,14 @@ int main()
C c2 = {};
test_compare(c1, c2);
}
+
+#if TEST_STD_VER > 17
+ {
+ constexpr std::array a1 = {1, 2, 3};
+ constexpr std::array a2 = {2, 3, 4};
+ static_assert(constexpr_compare(a1, a1, true, false), "");
+ static_assert(constexpr_compare(a1, a2, false, true), "");
+ static_assert(constexpr_compare(a2, a1, false, false), "");
+ }
+#endif
}
Modified: libcxx/trunk/www/cxx2a_status.html
URL:
http://llvm.org/viewvc/llvm-project/libcxx/trunk/www/cxx2a_status.html?rev=338668&r1=338667&r2=338668&view=diff
==
--- libcxx/trunk/www/cxx2a_status.html (original)
+++ libcxx/trunk/www/cxx2a_status.html Wed Aug 1 19:11:06 2018
@@ -72,7 +72,7 @@
https://wg21.link/P0767R1";>P0767R1CWGDeprecate
PODAlbuquerqueComplete7.0
https://wg21.link/P0768R1";>P0768R1CWGLibrary
Support for the Spaceship (Comparison)
OperatorAlbuquerque
https://wg21.link/P0777R1";>P0777R1LWGTr
r338667 - [analyzer] Extend NoStoreFuncVisitor to follow fields.
Author: george.karpenkov
Date: Wed Aug 1 19:02:40 2018
New Revision: 338667
URL: http://llvm.org/viewvc/llvm-project?rev=338667&view=rev
Log:
[analyzer] Extend NoStoreFuncVisitor to follow fields.
rdar://39701823
Differential Revision: https://reviews.llvm.org/D49901
Modified:
cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
cfe/trunk/test/Analysis/diagnostics/no-store-func-path-notes.c
cfe/trunk/test/Analysis/diagnostics/no-store-func-path-notes.cpp
cfe/trunk/test/Analysis/diagnostics/no-store-func-path-notes.m
Modified: cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp?rev=338667&r1=338666&r2=338667&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp Wed Aug 1
19:02:40 2018
@@ -269,10 +269,14 @@ namespace {
/// pointer dereference outside.
class NoStoreFuncVisitor final : public BugReporterVisitor {
const SubRegion *RegionOfInterest;
+ MemRegionManager &MmrMgr;
const SourceManager &SM;
const PrintingPolicy &PP;
- static constexpr const char *DiagnosticsMsg =
- "Returning without writing to '";
+
+ /// Recursion limit for dereferencing fields when looking for the
+ /// region of interest.
+ /// The limit of two indicates that we will dereference fields only once.
+ static const unsigned DEREFERENCE_LIMIT = 2;
/// Frames writing into \c RegionOfInterest.
/// This visitor generates a note only if a function does not write into
@@ -285,15 +289,17 @@ class NoStoreFuncVisitor final : public
llvm::SmallPtrSet FramesModifyingRegion;
llvm::SmallPtrSet FramesModifyingCalculated;
+ using RegionVector = SmallVector;
public:
NoStoreFuncVisitor(const SubRegion *R)
- : RegionOfInterest(R),
-SM(R->getMemRegionManager()->getContext().getSourceManager()),
-PP(R->getMemRegionManager()->getContext().getPrintingPolicy()) {}
+ : RegionOfInterest(R), MmrMgr(*R->getMemRegionManager()),
+SM(MmrMgr.getContext().getSourceManager()),
+PP(MmrMgr.getContext().getPrintingPolicy()) {}
void Profile(llvm::FoldingSetNodeID &ID) const override {
static int Tag = 0;
ID.AddPointer(&Tag);
+ID.AddPointer(RegionOfInterest);
}
std::shared_ptr VisitNode(const ExplodedNode *N,
@@ -307,48 +313,69 @@ public:
auto CallExitLoc = N->getLocationAs();
// No diagnostic if region was modified inside the frame.
-if (!CallExitLoc)
+if (!CallExitLoc || isRegionOfInterestModifiedInFrame(N))
return nullptr;
CallEventRef<> Call =
BRC.getStateManager().getCallEventManager().getCaller(SCtx, State);
+if (SM.isInSystemHeader(Call->getDecl()->getSourceRange().getBegin()))
+ return nullptr;
+
// Region of interest corresponds to an IVar, exiting a method
// which could have written into that IVar, but did not.
-if (const auto *MC = dyn_cast(Call))
- if (const auto *IvarR = dyn_cast(RegionOfInterest))
-if (potentiallyWritesIntoIvar(Call->getRuntimeDefinition().getDecl(),
- IvarR->getDecl()) &&
-!isRegionOfInterestModifiedInFrame(N))
- return notModifiedMemberDiagnostics(
- Ctx, *CallExitLoc, Call, MC->getReceiverSVal().getAsRegion());
+if (const auto *MC = dyn_cast(Call)) {
+ if (const auto *IvarR = dyn_cast(RegionOfInterest)) {
+const MemRegion *SelfRegion = MC->getReceiverSVal().getAsRegion();
+if (RegionOfInterest->isSubRegionOf(SelfRegion) &&
+potentiallyWritesIntoIvar(Call->getRuntimeDefinition().getDecl(),
+ IvarR->getDecl()))
+ return notModifiedDiagnostics(Ctx, *CallExitLoc, Call, {},
SelfRegion,
+"self", /*FirstIsReferenceType=*/false,
+1);
+ }
+}
if (const auto *CCall = dyn_cast(Call)) {
const MemRegion *ThisR = CCall->getCXXThisVal().getAsRegion();
if (RegionOfInterest->isSubRegionOf(ThisR)
- && !CCall->getDecl()->isImplicit()
- && !isRegionOfInterestModifiedInFrame(N))
-return notModifiedMemberDiagnostics(Ctx, *CallExitLoc, Call, ThisR);
+ && !CCall->getDecl()->isImplicit())
+return notModifiedDiagnostics(Ctx, *CallExitLoc, Call, {}, ThisR,
+ "this",
+ /*FirstIsReferenceType=*/false, 1);
+
+ // Do not generate diagnostics for not modified parameters in
+ // constructors.
+ return nullptr;
}
ArrayRef parameters = getCallParameters(Call);
for (unsigned I = 0; I < Call->getNumArgs() && I < parameters.size(); ++I)
{
- const ParmVarDecl *PVD = parameters
[libcxx] r338666 - Implement P0887: The identity metafunction
Author: marshall
Date: Wed Aug 1 18:56:02 2018
New Revision: 338666
URL: http://llvm.org/viewvc/llvm-project?rev=338666&view=rev
Log:
Implement P0887: The identity metafunction
Added:
libcxx/trunk/test/std/utilities/meta/meta.trans/meta.trans.other/type_identity.pass.cpp
Modified:
libcxx/trunk/include/type_traits
Modified: libcxx/trunk/include/type_traits
URL:
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/type_traits?rev=338666&r1=338665&r2=338666&view=diff
==
--- libcxx/trunk/include/type_traits (original)
+++ libcxx/trunk/include/type_traits Wed Aug 1 18:56:02 2018
@@ -75,6 +75,10 @@ namespace std
template struct remove_pointer;
template struct add_pointer;
+template struct type_identity; // C++20
+template
+ using type_identity_t = typename type_identity::type; // C++20
+
// Integral properties:
template struct is_signed;
template struct is_unsigned;
@@ -1226,6 +1230,12 @@ template struct _LIBCPP_TEMP
template using add_pointer_t = typename add_pointer<_Tp>::type;
#endif
+// type_identity
+#if _LIBCPP_STD_VER > 17
+template struct type_identity { typedef _Tp type; };
+template using type_identity_t = typename type_identity<_Tp>::type;
+#endif
+
// is_signed
template ::value>
Added:
libcxx/trunk/test/std/utilities/meta/meta.trans/meta.trans.other/type_identity.pass.cpp
URL:
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/utilities/meta/meta.trans/meta.trans.other/type_identity.pass.cpp?rev=338666&view=auto
==
---
libcxx/trunk/test/std/utilities/meta/meta.trans/meta.trans.other/type_identity.pass.cpp
(added)
+++
libcxx/trunk/test/std/utilities/meta/meta.trans/meta.trans.other/type_identity.pass.cpp
Wed Aug 1 18:56:02 2018
@@ -0,0 +1,40 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+// UNSUPPORTED: c++98, c++03, c++11, c++14, c++17
+
+// type_traits
+
+// type_identity
+
+#include
+
+#include "test_macros.h"
+
+template
+void test_type_identity()
+{
+static_assert((std::is_same::type,
T>::value), "");
+static_assert((std::is_same< std::type_identity_t,
T>::value), "");
+}
+
+int main()
+{
+test_type_identity();
+test_type_identity();
+test_type_identity();
+test_type_identity();
+test_type_identity< int[3]>();
+test_type_identity();
+
+test_type_identity();
+test_type_identity();
+test_type_identity();
+test_type_identity();
+test_type_identity();
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r338664 - Pass triple to RUN line to fix failing bots.
Author: ahatanak Date: Wed Aug 1 18:52:17 2018 New Revision: 338664 URL: http://llvm.org/viewvc/llvm-project?rev=338664&view=rev Log: Pass triple to RUN line to fix failing bots. This is a follow-up to r338656. Modified: cfe/trunk/test/PCH/no-escaping-block-tail-calls.cpp Modified: cfe/trunk/test/PCH/no-escaping-block-tail-calls.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/PCH/no-escaping-block-tail-calls.cpp?rev=338664&r1=338663&r2=338664&view=diff == --- cfe/trunk/test/PCH/no-escaping-block-tail-calls.cpp (original) +++ cfe/trunk/test/PCH/no-escaping-block-tail-calls.cpp Wed Aug 1 18:52:17 2018 @@ -1,5 +1,5 @@ -// RUN: %clang_cc1 -x c++-header -emit-pch -O1 -fblocks -fno-escaping-block-tail-calls -o %t %S/no-escaping-block-tail-calls.h -// RUN: %clang_cc1 -include-pch %t -emit-llvm -O1 -fblocks -fno-escaping-block-tail-calls -o - %s | FileCheck %s +// RUN: %clang_cc1 -x c++-header -triple x86_64-apple-darwin11 -emit-pch -O1 -fblocks -fno-escaping-block-tail-calls -o %t %S/no-escaping-block-tail-calls.h +// RUN: %clang_cc1 -triple x86_64-apple-darwin11 -include-pch %t -emit-llvm -O1 -fblocks -fno-escaping-block-tail-calls -o - %s | FileCheck %s // Check that -fno-escaping-block-tail-calls doesn't disable tail-call // optimization if the block is non-escaping. ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49952: Check for NULL Destination-Type when creating ArrayConstant
bviyer updated this revision to Diff 158676.
bviyer added a comment.
Added FileCheck as requested by John McCall
Made comments start with "//" instead of /*
Removed expected-no-diagnostics from the test case (as requested by Erik)
Repository:
rC Clang
https://reviews.llvm.org/D49952
Files:
lib/CodeGen/CGExprConstant.cpp
test/CodeGenCXX/empty-struct-init-list.cpp
Index: test/CodeGenCXX/empty-struct-init-list.cpp
===
--- /dev/null
+++ test/CodeGenCXX/empty-struct-init-list.cpp
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -std=c++11 -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -std=c++14 -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -std=c++17 -emit-llvm -o - %s | FileCheck %s
+
+// CHECK: struct.a
+typedef struct { } a;
+typedef struct {
+ a b[];
+} c;
+
+// CHECK: struct.c
+c d{ };
Index: lib/CodeGen/CGExprConstant.cpp
===
--- lib/CodeGen/CGExprConstant.cpp
+++ lib/CodeGen/CGExprConstant.cpp
@@ -2119,6 +2119,16 @@
Elts.push_back(C);
}
+// This means that the array type is probably "IncompleteType" or some
+// type that is not ConstantArray.
+if (CAT == nullptr && CommonElementType == nullptr && !NumInitElts) {
+ const ArrayType *AT = CGM.getContext().getAsArrayType(DestType);
+ CommonElementType = CGM.getTypes().ConvertType(AT->getElementType());
+ llvm::ArrayType *AType = llvm::ArrayType::get(CommonElementType,
+NumElements);
+ return llvm::ConstantAggregateZero::get(AType);
+}
+
return EmitArrayConstant(CGM, CAT, CommonElementType, NumElements, Elts,
Filler);
}
Index: test/CodeGenCXX/empty-struct-init-list.cpp
===
--- /dev/null
+++ test/CodeGenCXX/empty-struct-init-list.cpp
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -std=c++11 -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -std=c++14 -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -std=c++17 -emit-llvm -o - %s | FileCheck %s
+
+// CHECK: struct.a
+typedef struct { } a;
+typedef struct {
+ a b[];
+} c;
+
+// CHECK: struct.c
+c d{ };
Index: lib/CodeGen/CGExprConstant.cpp
===
--- lib/CodeGen/CGExprConstant.cpp
+++ lib/CodeGen/CGExprConstant.cpp
@@ -2119,6 +2119,16 @@
Elts.push_back(C);
}
+// This means that the array type is probably "IncompleteType" or some
+// type that is not ConstantArray.
+if (CAT == nullptr && CommonElementType == nullptr && !NumInitElts) {
+ const ArrayType *AT = CGM.getContext().getAsArrayType(DestType);
+ CommonElementType = CGM.getTypes().ConvertType(AT->getElementType());
+ llvm::ArrayType *AType = llvm::ArrayType::get(CommonElementType,
+NumElements);
+ return llvm::ConstantAggregateZero::get(AType);
+}
+
return EmitArrayConstant(CGM, CAT, CommonElementType, NumElements, Elts,
Filler);
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50168: [Builtins] Implement __builtin_clrsb to be compatible with gcc
craig.topper created this revision.
craig.topper added reviewers: bkramer, efriedma, spatel.
Herald added a reviewer: javed.absar.
Herald added a subscriber: kristof.beyls.
gcc defines an intrinsic called __builtin_clrsb which counts the number of
extra sign bits on a number. This is equivalent to counting the number of
leading zeros on a positive number or the number of leading ones on a negative
number and subtracting one from the result. Since we can't count leading ones
we need to invert negative numbers to count zeros.
The emitted sequence contains a bit of trickery stolen from an LLVM AArch64
test arm64-clrsb.ll to prevent passing a value of 0 to ctlz. I used a icmp slt
and a select to conditionally negate, but InstCombine will turn that into an
ashr+xor. I can emit that directly if that's prefered. I know @spatel has been
trying to remove some of the bit tricks from InstCombine so I'm not sure if the
ashr+xor form will be canonical going forward.
This patch will cause the builtin to be expanded inline while gcc uses a call
to a function like __clrsbdi2 that is implemented in libgcc. But this is
similar to what we already do for popcnt. And I don't think compiler-rt
supports __clrsbdi2.
https://reviews.llvm.org/D50168
Files:
include/clang/Basic/Builtins.def
lib/CodeGen/CGBuiltin.cpp
Index: lib/CodeGen/CGBuiltin.cpp
===
--- lib/CodeGen/CGBuiltin.cpp
+++ lib/CodeGen/CGBuiltin.cpp
@@ -1537,6 +1537,34 @@
return RValue::get(ComplexVal.second);
}
+ case Builtin::BI__builtin_clrsb:
+ case Builtin::BI__builtin_clrsbl:
+ case Builtin::BI__builtin_clrsbll: {
+// clrsb(x) -> clz(x < 0 ? ~x : x) - 1 or
+// -> clz(((x < 0 ? ~x : x) << 1) | 1)
+Value *ArgValue = EmitScalarExpr(E->getArg(0));
+
+llvm::Type *ArgType = ArgValue->getType();
+Value *F = CGM.getIntrinsic(Intrinsic::ctlz, ArgType);
+
+llvm::Type *ResultType = ConvertType(E->getType());
+Value *Zero = llvm::Constant::getNullValue(ArgType);
+Value *IsNeg = Builder.CreateICmpSLT(ArgValue, Zero, "isneg");
+Value *Inverse = Builder.CreateNot(ArgValue, "not");
+Value *Tmp = Builder.CreateSelect(IsNeg, Inverse, ArgValue);
+// Now we need to calculate ctlz(Tmp)-1, but Tmp might be zero. We know
+// the sign bit is zero, so we can shift it out. Then put a 1 in the LSB.
+// This removes one leading zero like the subtract does, and replaces it
+// with a guaranteed one to prevent the value being 0.
+Value *One = llvm::ConstantInt::get(ArgType, 1);
+Tmp = Builder.CreateShl(Tmp, One);
+Tmp = Builder.CreateOr(Tmp, One);
+Value *Result = Builder.CreateCall(F, {Tmp, Builder.getTrue()});
+if (Result->getType() != ResultType)
+ Result = Builder.CreateIntCast(Result, ResultType, /*isSigned*/true,
+ "cast");
+return RValue::get(Result);
+ }
case Builtin::BI__builtin_ctzs:
case Builtin::BI__builtin_ctz:
case Builtin::BI__builtin_ctzl:
Index: include/clang/Basic/Builtins.def
===
--- include/clang/Basic/Builtins.def
+++ include/clang/Basic/Builtins.def
@@ -413,6 +413,9 @@
BUILTIN(__builtin_popcount , "iUi" , "nc")
BUILTIN(__builtin_popcountl , "iULi" , "nc")
BUILTIN(__builtin_popcountll, "iULLi", "nc")
+BUILTIN(__builtin_clrsb , "ii" , "nc")
+BUILTIN(__builtin_clrsbl , "iLi" , "nc")
+BUILTIN(__builtin_clrsbll, "iLLi", "nc")
// FIXME: These type signatures are not correct for targets with int != 32-bits
// or with ULL != 64-bits.
Index: lib/CodeGen/CGBuiltin.cpp
===
--- lib/CodeGen/CGBuiltin.cpp
+++ lib/CodeGen/CGBuiltin.cpp
@@ -1537,6 +1537,34 @@
return RValue::get(ComplexVal.second);
}
+ case Builtin::BI__builtin_clrsb:
+ case Builtin::BI__builtin_clrsbl:
+ case Builtin::BI__builtin_clrsbll: {
+// clrsb(x) -> clz(x < 0 ? ~x : x) - 1 or
+// -> clz(((x < 0 ? ~x : x) << 1) | 1)
+Value *ArgValue = EmitScalarExpr(E->getArg(0));
+
+llvm::Type *ArgType = ArgValue->getType();
+Value *F = CGM.getIntrinsic(Intrinsic::ctlz, ArgType);
+
+llvm::Type *ResultType = ConvertType(E->getType());
+Value *Zero = llvm::Constant::getNullValue(ArgType);
+Value *IsNeg = Builder.CreateICmpSLT(ArgValue, Zero, "isneg");
+Value *Inverse = Builder.CreateNot(ArgValue, "not");
+Value *Tmp = Builder.CreateSelect(IsNeg, Inverse, ArgValue);
+// Now we need to calculate ctlz(Tmp)-1, but Tmp might be zero. We know
+// the sign bit is zero, so we can shift it out. Then put a 1 in the LSB.
+// This removes one leading zero like the subtract does, and replaces it
+// with a guaranteed one to prevent the value being 0.
+Value *One = llvm::ConstantInt::get(ArgType, 1);
+Tmp = Builder.CreateShl(Tmp, One);
+Tmp = Builder.CreateOr(Tmp, One);
+Value *Res
[PATCH] D48100: Append new attributes to the end of an AttributeList.
hfinkel accepted this revision. hfinkel added a comment. Assuming that @aaron.ballman is still okay with this, I think that we should move forward. @erichkeane, I suspect that fixing the ordering problems, which were often arbitrary before and are still so, is a larger change that we'd want to split out regardless (not to mention the fact that we'd need to decide how to fix them). Erich, is that alright with you? Repository: rC Clang https://reviews.llvm.org/D48100 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50160: [c-index-test] Use correct executable path to discover resource directory.
akyrtzi accepted this revision. akyrtzi added a comment. This revision is now accepted and ready to land. LGTM! https://reviews.llvm.org/D50160 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48808: [CodeGen] Emit parallel_loop_access for each loop in the loop stack.
hfinkel accepted this revision. hfinkel added a comment. This revision is now accepted and ready to land. LGTM Repository: rC Clang https://reviews.llvm.org/D48808 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D32747: [Analyzer] Iterator Checker - Part 3: Invalidation check, first for (copy) assignments
NoQ added a comment. I suspect that with https://reviews.llvm.org/rC338135 your operator this-argument re-assignment mechanism is no longer necessary. Due to the combination of support for materializing temporaries that i added previously and the change in the AST that turns the operator this-argument into a simple temporary, i believe that the memory region should no longer change and you don't need to move your metadata. Could you check this? https://reviews.llvm.org/D32747 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49627: [CFG] [analyzer] Constructors of member CXXOperatorCallExpr's argument 0 are not argument constructors.
NoQ updated this revision to Diff 158668.
NoQ added a comment.
Ok, so with https://reviews.llvm.org/rC338135 operator this-argument
constructors do indeed work exactly like temporaries. This patch is not
necessary anymore. I'd still prefer to add tests and i've added a
path-sensitive test as well to demonstrate that the constructor works correctly.
https://reviews.llvm.org/D49627
Files:
include/clang/Analysis/ConstructionContext.h
test/Analysis/cfg-rich-constructors.cpp
test/Analysis/temporaries.cpp
Index: test/Analysis/temporaries.cpp
===
--- test/Analysis/temporaries.cpp
+++ test/Analysis/temporaries.cpp
@@ -986,3 +986,21 @@
*i = 99; // no-warning
}
} // namespace ctor_argument
+
+namespace operator_implicit_argument {
+struct S {
+ bool x;
+ S(bool x): x(x) {}
+ operator bool() const { return x; }
+};
+
+void foo() {
+ if (S(false)) {
+clang_analyzer_warnIfReached(); // no-warning
+ }
+ if (S(true)) {
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+ }
+}
+} // namespace operator_implicit_argument
+
Index: test/Analysis/cfg-rich-constructors.cpp
===
--- test/Analysis/cfg-rich-constructors.cpp
+++ test/Analysis/cfg-rich-constructors.cpp
@@ -963,3 +963,35 @@
C c = C();
}
} // namespace copy_elision_with_extra_arguments
+
+
+namespace operators {
+class C {
+public:
+ C(int);
+ C &operator+(C Other);
+};
+
+// FIXME: Find construction context for the this-argument of the operator.
+// CHECK: void testOperators()
+// CHECK:[B1]
+// CHECK-NEXT: 1: operator+
+// CHECK-NEXT: 2: [B1.1] (ImplicitCastExpr, FunctionToPointerDecay, class
operators::C &(*)(class o
+// CHECK-NEXT: 3: 1
+// CHECK-NEXT: 4: [B1.3] (CXXConstructExpr, [B1.6], class operators::C)
+// CHECK-NEXT: 5: operators::C([B1.4]) (CXXFunctionalCastExpr,
ConstructorConversion, class operato
+// CHECK-NEXT: 6: [B1.5]
+// CHECK-NEXT: 7: 2
+// CXX11-ELIDE-NEXT: 8: [B1.7] (CXXConstructExpr, [B1.10], [B1.11], class
operators::C)
+// CXX11-NOELIDE-NEXT: 8: [B1.7] (CXXConstructExpr, [B1.10], class
operators::C)
+// CXX11-NEXT: 9: operators::C([B1.8]) (CXXFunctionalCastExpr,
ConstructorConversion, class operato
+// CXX11-NEXT:10: [B1.9]
+// CXX11-NEXT:11: [B1.10] (CXXConstructExpr, [B1.12]+1, class operators::C)
+// CXX11-NEXT:12: [B1.6] + [B1.11] (OperatorCall)
+// CXX17-NEXT: 8: [B1.7] (CXXConstructExpr, [B1.10]+1, class operators::C)
+// CXX17-NEXT: 9: operators::C([B1.8]) (CXXFunctionalCastExpr,
ConstructorConversion, class operato
+// CXX17-NEXT:10: [B1.6] + [B1.9] (OperatorCall)
+void testOperators() {
+ C(1) + C(2);
+}
+} // namespace operators
Index: include/clang/Analysis/ConstructionContext.h
===
--- include/clang/Analysis/ConstructionContext.h
+++ include/clang/Analysis/ConstructionContext.h
@@ -623,9 +623,16 @@
};
class ArgumentConstructionContext : public ConstructionContext {
- const Expr *CE; // The call of which the context is an argument.
- unsigned Index; // Which argument we're constructing.
- const CXXBindTemporaryExpr *BTE; // Whether the object needs to be destroyed.
+ // The call of which the context is an argument.
+ const Expr *CE;
+
+ // Which argument we're constructing. Note that when numbering between
+ // arguments and parameters is inconsistent (eg., operator calls),
+ // this is the index of the argument, not of the parameter.
+ unsigned Index;
+
+ // Whether the object needs to be destroyed.
+ const CXXBindTemporaryExpr *BTE;
friend class ConstructionContext; // Allows to create<>() itself.
Index: test/Analysis/temporaries.cpp
===
--- test/Analysis/temporaries.cpp
+++ test/Analysis/temporaries.cpp
@@ -986,3 +986,21 @@
*i = 99; // no-warning
}
} // namespace ctor_argument
+
+namespace operator_implicit_argument {
+struct S {
+ bool x;
+ S(bool x): x(x) {}
+ operator bool() const { return x; }
+};
+
+void foo() {
+ if (S(false)) {
+clang_analyzer_warnIfReached(); // no-warning
+ }
+ if (S(true)) {
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+ }
+}
+} // namespace operator_implicit_argument
+
Index: test/Analysis/cfg-rich-constructors.cpp
===
--- test/Analysis/cfg-rich-constructors.cpp
+++ test/Analysis/cfg-rich-constructors.cpp
@@ -963,3 +963,35 @@
C c = C();
}
} // namespace copy_elision_with_extra_arguments
+
+
+namespace operators {
+class C {
+public:
+ C(int);
+ C &operator+(C Other);
+};
+
+// FIXME: Find construction context for the this-argument of the operator.
+// CHECK: void testOperators()
+// CHECK:[B1]
+// CHECK-NEXT: 1: operator+
+// CHECK-NEXT: 2: [B1.1] (ImplicitCastExpr, Fu
[PATCH] D49396: [WebAssembly] Support for atomic.wait / atomic.wake builtins
dschuff accepted this revision. dschuff added inline comments. This revision is now accepted and ready to land. Comment at: include/clang/Basic/BuiltinsWebAssembly.def:38 +// Atomic wait and wake. +BUILTIN(__builtin_wasm_atomic_wait_i32, "Uii*iLLi", "n") +BUILTIN(__builtin_wasm_atomic_wait_i64, "UiLLi*LLiLLi", "n") aheejin wrote: > dschuff wrote: > > So this means that the signature is basically `unsigned int > > __builtin_wasm_atomic_wait_i32(int *, int, long long)`? We should maybe > > make it `int __builtin_wasm_atomic_wait_i32(const unsigned char *, int, > > unsigned long long)`. Returning int so that you could define a C enum with > > the possible return values and compare without type coercion; unsigned char > > * so that it aliases with everything (i.e. a byte ptr), and unsigned long > > long since a negative relative timeout isn't meaningful(?). Not sure > > whether we should use int or unsigned int as the expected value, can't > > think of any particular reason right now to use one or the other. > > > > Likewise with the other signatures. > > Returning int so that you could define a C enum with the possible return > > values and compare without type coercion; > Done. > > > unsigned char * so that it aliases with everything (i.e. a byte ptr), > From this pointer a value will be loaded and compared with the expected > value, which is an int. Shouldn't this be an int pointer then? Not sure why > it should alias with a byte ptr. > > > and unsigned long long since a negative relative timeout isn't > > meaningful(?). > [[ > https://github.com/WebAssembly/threads/blob/master/proposals/threads/Overview.md#wait > | Timeouts can be negative ]], in which case it never expires. The wake > count of `atomics.wake` builtin can be negative too, in which case it waits > for all waiters. > > > Not sure whether we should use int or unsigned int as the expected value, > > can't think of any particular reason right now to use one or the other. > We didn't impose any restrictions other than it is an int in the spec, so I > think it should be a (signed) int? Oh you're right, I misread the spec. So int * and signed timeout is fine. For the expected value (and the pointer type) it could still be either signed or unsigned because at the wasm level there's no distinction. but signed int seems fine as it is. Repository: rC Clang https://reviews.llvm.org/D49396 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r338656 - Serialize DoesNotEscape.
Author: ahatanak
Date: Wed Aug 1 16:51:53 2018
New Revision: 338656
URL: http://llvm.org/viewvc/llvm-project?rev=338656&view=rev
Log:
Serialize DoesNotEscape.
I forgot to commit this in r326530.
Added:
cfe/trunk/test/PCH/no-escaping-block-tail-calls.cpp
cfe/trunk/test/PCH/no-escaping-block-tail-calls.h
Modified:
cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
cfe/trunk/lib/Serialization/ASTWriterDecl.cpp
Modified: cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReaderDecl.cpp?rev=338656&r1=338655&r2=338656&view=diff
==
--- cfe/trunk/lib/Serialization/ASTReaderDecl.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTReaderDecl.cpp Wed Aug 1 16:51:53 2018
@@ -1472,6 +1472,7 @@ void ASTDeclReader::VisitBlockDecl(Block
BD->setIsVariadic(Record.readInt());
BD->setBlockMissingReturnType(Record.readInt());
BD->setIsConversionFromLambda(Record.readInt());
+ BD->setDoesNotEscape(Record.readInt());
bool capturesCXXThis = Record.readInt();
unsigned numCaptures = Record.readInt();
Modified: cfe/trunk/lib/Serialization/ASTWriterDecl.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTWriterDecl.cpp?rev=338656&r1=338655&r2=338656&view=diff
==
--- cfe/trunk/lib/Serialization/ASTWriterDecl.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTWriterDecl.cpp Wed Aug 1 16:51:53 2018
@@ -1097,6 +1097,7 @@ void ASTDeclWriter::VisitBlockDecl(Block
Record.push_back(D->isVariadic());
Record.push_back(D->blockMissingReturnType());
Record.push_back(D->isConversionFromLambda());
+ Record.push_back(D->doesNotEscape());
Record.push_back(D->capturesCXXThis());
Record.push_back(D->getNumCaptures());
for (const auto &capture : D->captures()) {
Added: cfe/trunk/test/PCH/no-escaping-block-tail-calls.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/PCH/no-escaping-block-tail-calls.cpp?rev=338656&view=auto
==
--- cfe/trunk/test/PCH/no-escaping-block-tail-calls.cpp (added)
+++ cfe/trunk/test/PCH/no-escaping-block-tail-calls.cpp Wed Aug 1 16:51:53 2018
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -x c++-header -emit-pch -O1 -fblocks
-fno-escaping-block-tail-calls -o %t %S/no-escaping-block-tail-calls.h
+// RUN: %clang_cc1 -include-pch %t -emit-llvm -O1 -fblocks
-fno-escaping-block-tail-calls -o - %s | FileCheck %s
+
+// Check that -fno-escaping-block-tail-calls doesn't disable tail-call
+// optimization if the block is non-escaping.
+
+// CHECK-LABEL: define internal i32 @___ZN1S1mEv_block_invoke(
+// CHECK: %[[CALL:.*]] = tail call i32 @_ZN1S3fooER2S0(
+// CHECK-NEXT: ret i32 %[[CALL]]
+
+void test() {
+ S s;
+ s.m();
+}
Added: cfe/trunk/test/PCH/no-escaping-block-tail-calls.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/PCH/no-escaping-block-tail-calls.h?rev=338656&view=auto
==
--- cfe/trunk/test/PCH/no-escaping-block-tail-calls.h (added)
+++ cfe/trunk/test/PCH/no-escaping-block-tail-calls.h Wed Aug 1 16:51:53 2018
@@ -0,0 +1,16 @@
+typedef int (^BlockTy)(void);
+
+struct S0 {
+ int a;
+};
+
+struct S {
+ int i;
+ void func(BlockTy __attribute__((noescape)));
+ int foo(S0 &);
+
+ void m() {
+__block S0 x;
+func(^{ return foo(x); });
+ }
+};
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49403: More aggressively complete RecordTypes with Function Pointers
andrew.w.kaylor added a comment.
The LLVM linker also seems to have a bunch of problems with resolving pointer
types in structures. I'm looking into a couple of those now.
I am aware of the opaque pointer effort, though as you say it seems to be
stalled. I agree that there aren't a lot of things you can do based on pointer
type information, but there are some things that you might like to do which can
be proven to be unsafe based on pointer type information that might be
incorrectly flagged as unsafe if the pointer type information is incorrect. An
example would be the sorts of data layout optimizations described here:
https://llvm.org/devmtg/2014-10/Slides/Prashanth-DLO.pdf Note, in particular,
on slide 11 ("Legality") the reference to "cast to/from a given struct type". I
would imagine that includes pointer casts. It should be possible to do the same
sort of analysis with opaque pointers by following their uses very carefully,
and maybe not having these infernal pointer type casts all over the place would
make that less prone to false negatives. In the meantime, the pointer casts are
there and have to be dealt with.
Getting back to the current review, Erich explained to me that this patch
involves a certain amount of risk in that it changes the way clang processes
things, but it has the merit of getting the correct answer.
https://reviews.llvm.org/D49403
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50160: [c-index-test] Use correct executable path to discover resource directory.
vsapsai marked an inline comment as done. vsapsai added inline comments. Comment at: clang/tools/c-index-test/core_main.cpp:210 + void *P = (void*) (intptr_t) indextest_core_main; + std::string Executable = llvm::sys::fs::getMainExecutable(ProgName, P); SmallVector ArgsWithProgName; akyrtzi wrote: > Could you move this up to `indextest_core_main` and have > `printSourceSymbols()` accept the executable path directly ? > This would come in handy to avoid duplication if we later on add another > function that also needs the executable path. Done. Thanks for the suggestion, I think the code looks better now. https://reviews.llvm.org/D50160 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50160: [c-index-test] Use correct executable path to discover resource directory.
vsapsai updated this revision to Diff 158658.
vsapsai added a comment.
- Address review comment: move getMainExecutable to indextest_core_main.
https://reviews.llvm.org/D50160
Files:
clang/tools/c-index-test/core_main.cpp
Index: clang/tools/c-index-test/core_main.cpp
===
--- clang/tools/c-index-test/core_main.cpp
+++ clang/tools/c-index-test/core_main.cpp
@@ -20,6 +20,7 @@
#include "clang/Lex/Preprocessor.h"
#include "clang/Serialization/ASTReader.h"
#include "llvm/Support/CommandLine.h"
+#include "llvm/Support/FileSystem.h"
#include "llvm/Support/Signals.h"
#include "llvm/Support/raw_ostream.h"
#include "llvm/Support/PrettyStackTrace.h"
@@ -202,11 +203,11 @@
});
}
-static bool printSourceSymbols(ArrayRef Args,
- bool dumpModuleImports,
- bool indexLocals) {
+static bool printSourceSymbols(const char *Executable,
+ ArrayRef Args,
+ bool dumpModuleImports, bool indexLocals) {
SmallVector ArgsWithProgName;
- ArgsWithProgName.push_back("clang");
+ ArgsWithProgName.push_back(Executable);
ArgsWithProgName.append(Args.begin(), Args.end());
IntrusiveRefCntPtr
Diags(CompilerInstance::createDiagnostics(new DiagnosticOptions));
@@ -314,6 +315,8 @@
int indextest_core_main(int argc, const char **argv) {
sys::PrintStackTraceOnErrorSignal(argv[0]);
PrettyStackTraceProgram X(argc, argv);
+ void *MainAddr = (void*) (intptr_t) indextest_core_main;
+ std::string Executable = llvm::sys::fs::getMainExecutable(argv[0], MainAddr);
assert(argv[1] == StringRef("core"));
++argv;
@@ -343,7 +346,9 @@
errs() << "error: missing compiler args; pass '-- '\n";
return 1;
}
-return printSourceSymbols(CompArgs, options::DumpModuleImports,
options::IncludeLocals);
+return printSourceSymbols(Executable.c_str(), CompArgs,
+ options::DumpModuleImports,
+ options::IncludeLocals);
}
return 0;
Index: clang/tools/c-index-test/core_main.cpp
===
--- clang/tools/c-index-test/core_main.cpp
+++ clang/tools/c-index-test/core_main.cpp
@@ -20,6 +20,7 @@
#include "clang/Lex/Preprocessor.h"
#include "clang/Serialization/ASTReader.h"
#include "llvm/Support/CommandLine.h"
+#include "llvm/Support/FileSystem.h"
#include "llvm/Support/Signals.h"
#include "llvm/Support/raw_ostream.h"
#include "llvm/Support/PrettyStackTrace.h"
@@ -202,11 +203,11 @@
});
}
-static bool printSourceSymbols(ArrayRef Args,
- bool dumpModuleImports,
- bool indexLocals) {
+static bool printSourceSymbols(const char *Executable,
+ ArrayRef Args,
+ bool dumpModuleImports, bool indexLocals) {
SmallVector ArgsWithProgName;
- ArgsWithProgName.push_back("clang");
+ ArgsWithProgName.push_back(Executable);
ArgsWithProgName.append(Args.begin(), Args.end());
IntrusiveRefCntPtr
Diags(CompilerInstance::createDiagnostics(new DiagnosticOptions));
@@ -314,6 +315,8 @@
int indextest_core_main(int argc, const char **argv) {
sys::PrintStackTraceOnErrorSignal(argv[0]);
PrettyStackTraceProgram X(argc, argv);
+ void *MainAddr = (void*) (intptr_t) indextest_core_main;
+ std::string Executable = llvm::sys::fs::getMainExecutable(argv[0], MainAddr);
assert(argv[1] == StringRef("core"));
++argv;
@@ -343,7 +346,9 @@
errs() << "error: missing compiler args; pass '-- '\n";
return 1;
}
-return printSourceSymbols(CompArgs, options::DumpModuleImports, options::IncludeLocals);
+return printSourceSymbols(Executable.c_str(), CompArgs,
+ options::DumpModuleImports,
+ options::IncludeLocals);
}
return 0;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r338641 - [AST][4/4] Move the bit-fields from ObjCMethodDecl and ObjCContainerDecl into DeclContext
(And it was fixed in r338648)
On Wed, Aug 1, 2018 at 3:24 PM David Jones wrote:
> FYI, this breaks clang:
>
> .../llvm/tools/clang/lib/AST/DeclObjC.cpp:793:7: error: static_assert
> expression is not an integral constant expression
> static_cast(ObjCMethodDeclBits.ObjCMethodFamilyBitWidth) ==
> ^
> .../llvm/tools/clang/lib/AST/DeclObjC.cpp:793:29: note: implicit use of
> 'this' pointer is only allowed within the evaluation of a call to a
> 'constexpr' member function
> static_cast(ObjCMethodDeclBits.ObjCMethodFamilyBitWidth) ==
> ^
> 1 error generated.
>
>
> On Wed, Aug 1, 2018 at 2:31 PM Erich Keane via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> Author: erichkeane
>> Date: Wed Aug 1 14:31:08 2018
>> New Revision: 338641
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=338641&view=rev
>> Log:
>> [AST][4/4] Move the bit-fields from ObjCMethodDecl and ObjCContainerDecl
>> into DeclContext
>>
>> This patch follows https://reviews.llvm.org/D49729,
>> https://reviews.llvm.org/D49732 and
>> https://reviews.llvm.org/D49733.
>>
>> Move the bits from ObjCMethodDecl and ObjCContainerDecl
>> into DeclContext.
>>
>> Differential Revision: https://reviews.llvm.org/D49734
>>
>> Patch By: bricci
>>
>> Modified:
>> cfe/trunk/include/clang/AST/DeclObjC.h
>> cfe/trunk/lib/AST/DeclObjC.cpp
>> cfe/trunk/lib/Sema/SemaDeclObjC.cpp
>> cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
>> cfe/trunk/lib/Serialization/ASTWriterDecl.cpp
>>
>> Modified: cfe/trunk/include/clang/AST/DeclObjC.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclObjC.h?rev=338641&r1=338640&r2=338641&view=diff
>>
>> ==
>> --- cfe/trunk/include/clang/AST/DeclObjC.h (original)
>> +++ cfe/trunk/include/clang/AST/DeclObjC.h Wed Aug 1 14:31:08 2018
>> @@ -141,58 +141,10 @@ public:
>>enum ImplementationControl { None, Required, Optional };
>>
>> private:
>> - // The conventional meaning of this method; an ObjCMethodFamily.
>> - // This is not serialized; instead, it is computed on demand and
>> - // cached.
>> - mutable unsigned Family : ObjCMethodFamilyBitWidth;
>> -
>> - /// instance (true) or class (false) method.
>> - unsigned IsInstance : 1;
>> - unsigned IsVariadic : 1;
>> -
>> - /// True if this method is the getter or setter for an explicit
>> property.
>> - unsigned IsPropertyAccessor : 1;
>> -
>> - // Method has a definition.
>> - unsigned IsDefined : 1;
>> -
>> - /// Method redeclaration in the same interface.
>> - unsigned IsRedeclaration : 1;
>> -
>> - /// Is redeclared in the same interface.
>> - mutable unsigned HasRedeclaration : 1;
>> -
>> - // NOTE: VC++ treats enums as signed, avoid using
>> ImplementationControl enum
>> - /// \@required/\@optional
>> - unsigned DeclImplementation : 2;
>> -
>> - // NOTE: VC++ treats enums as signed, avoid using the
>> ObjCDeclQualifier enum
>> - /// in, inout, etc.
>> - unsigned objcDeclQualifier : 7;
>> -
>> - /// Indicates whether this method has a related result type.
>> - unsigned RelatedResultType : 1;
>> -
>> - /// Whether the locations of the selector identifiers are in a
>> - /// "standard" position, a enum SelectorLocationsKind.
>> - unsigned SelLocsKind : 2;
>> -
>> - /// Whether this method overrides any other in the class hierarchy.
>> - ///
>> - /// A method is said to override any method in the class's
>> - /// base classes, its protocols, or its categories' protocols, that has
>> - /// the same selector and is of the same kind (class or instance).
>> - /// A method in an implementation is not considered as overriding the
>> same
>> - /// method in the interface or its categories.
>> - unsigned IsOverriding : 1;
>> -
>> - /// Indicates if the method was a definition but its body was skipped.
>> - unsigned HasSkippedBody : 1;
>> -
>> - // Return type of this method.
>> + /// Return type of this method.
>>QualType MethodDeclType;
>>
>> - // Type source information for the return type.
>> + /// Type source information for the return type.
>>TypeSourceInfo *ReturnTInfo;
>>
>>/// Array of ParmVarDecls for the formal parameters of this method
>> @@ -203,7 +155,7 @@ private:
>>/// List of attributes for this method declaration.
>>SourceLocation DeclEndLoc; // the location of the ';' or '{'.
>>
>> - // The following are only used for method definitions, null otherwise.
>> + /// The following are only used for method definitions, null otherwise.
>>LazyDeclStmtPtr Body;
>>
>>/// SelfDecl - Decl for the implicit self parameter. This is lazily
>> @@ -220,21 +172,14 @@ private:
>> bool isVariadic = false, bool isPropertyAccessor =
>> false,
>> bool isImplicitlyDeclared = false, bool isDefined =
>> false,
>> ImplementationControl impC
[PATCH] D50160: [c-index-test] Use correct executable path to discover resource directory.
akyrtzi added inline comments. Comment at: clang/tools/c-index-test/core_main.cpp:210 + void *P = (void*) (intptr_t) indextest_core_main; + std::string Executable = llvm::sys::fs::getMainExecutable(ProgName, P); SmallVector ArgsWithProgName; Could you move this up to `indextest_core_main` and have `printSourceSymbols()` accept the executable path directly ? This would come in handy to avoid duplication if we later on add another function that also needs the executable path. https://reviews.llvm.org/D50160 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50163: [AST] Remove the static_assert check in ObjCMethodDecl::ObjCMethodDecl
This revision was not accepted when it landed; it landed in state "Needs
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rC338648: [AST] Remove the static_assert check in
ObjCMethodDecl::ObjCMethodDecl (authored by vlad.tsyrklevich, committed by ).
Repository:
rC Clang
https://reviews.llvm.org/D50163
Files:
lib/AST/DeclObjC.cpp
Index: lib/AST/DeclObjC.cpp
===
--- lib/AST/DeclObjC.cpp
+++ lib/AST/DeclObjC.cpp
@@ -787,13 +787,6 @@
: NamedDecl(ObjCMethod, contextDecl, beginLoc, SelInfo),
DeclContext(ObjCMethod), MethodDeclType(T), ReturnTInfo(ReturnTInfo),
DeclEndLoc(endLoc) {
- // See the comment in ObjCMethodFamilyBitfields about
- // ObjCMethodFamilyBitWidth for why we check this.
- static_assert(
- static_cast(ObjCMethodDeclBits.ObjCMethodFamilyBitWidth) ==
- static_cast(ObjCMethodFamilyBitWidth),
- "ObjCMethodDeclBitfields::ObjCMethodFamilyBitWidth and "
- "ObjCMethodFamilyBitWidth do not match!");
// Initialized the bits stored in DeclContext.
ObjCMethodDeclBits.Family =
Index: lib/AST/DeclObjC.cpp
===
--- lib/AST/DeclObjC.cpp
+++ lib/AST/DeclObjC.cpp
@@ -787,13 +787,6 @@
: NamedDecl(ObjCMethod, contextDecl, beginLoc, SelInfo),
DeclContext(ObjCMethod), MethodDeclType(T), ReturnTInfo(ReturnTInfo),
DeclEndLoc(endLoc) {
- // See the comment in ObjCMethodFamilyBitfields about
- // ObjCMethodFamilyBitWidth for why we check this.
- static_assert(
- static_cast(ObjCMethodDeclBits.ObjCMethodFamilyBitWidth) ==
- static_cast(ObjCMethodFamilyBitWidth),
- "ObjCMethodDeclBitfields::ObjCMethodFamilyBitWidth and "
- "ObjCMethodFamilyBitWidth do not match!");
// Initialized the bits stored in DeclContext.
ObjCMethodDeclBits.Family =
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r338648 - [AST] Remove the static_assert check in ObjCMethodDecl::ObjCMethodDecl
Author: vlad.tsyrklevich
Date: Wed Aug 1 15:41:03 2018
New Revision: 338648
URL: http://llvm.org/viewvc/llvm-project?rev=338648&view=rev
Log:
[AST] Remove the static_assert check in ObjCMethodDecl::ObjCMethodDecl
Summary:
This check was introduced by r338641
but this broke some builds. For now remove it.
Subscribers: cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D50163
Modified:
cfe/trunk/lib/AST/DeclObjC.cpp
Modified: cfe/trunk/lib/AST/DeclObjC.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclObjC.cpp?rev=338648&r1=338647&r2=338648&view=diff
==
--- cfe/trunk/lib/AST/DeclObjC.cpp (original)
+++ cfe/trunk/lib/AST/DeclObjC.cpp Wed Aug 1 15:41:03 2018
@@ -787,13 +787,6 @@ ObjCMethodDecl::ObjCMethodDecl(SourceLoc
: NamedDecl(ObjCMethod, contextDecl, beginLoc, SelInfo),
DeclContext(ObjCMethod), MethodDeclType(T), ReturnTInfo(ReturnTInfo),
DeclEndLoc(endLoc) {
- // See the comment in ObjCMethodFamilyBitfields about
- // ObjCMethodFamilyBitWidth for why we check this.
- static_assert(
- static_cast(ObjCMethodDeclBits.ObjCMethodFamilyBitWidth) ==
- static_cast(ObjCMethodFamilyBitWidth),
- "ObjCMethodDeclBitfields::ObjCMethodFamilyBitWidth and "
- "ObjCMethodFamilyBitWidth do not match!");
// Initialized the bits stored in DeclContext.
ObjCMethodDeclBits.Family =
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49148: [DEBUGINFO] Disable unsupported debug info options for NVPTX target.
tra added a comment. We normally do not need to deviate from the host options all that often. I would argue that keeping options identical is a reasonable default for most options. For some options the driver may be able to derive a sensible value based on the host options. E.g. some options can be ignored. Some can be downgraded. Some can be replaced with a target-specific equivalent. For others we must require the user to provide the value. So, at the very least we must be able to put all options into one of the categories. We also need to figure out what kind of syntax we'll use to specify target-specific options. We currently have a `-Xarch...` hack in some toolchains, but it's rather awkward to us in practice as it does not give you much control over where are those options placed on the cc1's command line, they are also rather verbose and usually do not support options with arguments. We could make -Xarch=XYZ a sticky option which would consider following options to apply only to particular arch with, possibly, few special arch names to specify `host`, `device`, `all` subcompilations. It's also possible that I'm reinventing the wheel here. Are there existing precedents for command-line options with this kind of multi-consumer functionality? Repository: rC Clang https://reviews.llvm.org/D49148 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50163: [AST] Remove the static_assert check in ObjCMethodDecl::ObjCMethodDecl
bricci created this revision.
bricci added a project: clang.
Herald added a subscriber: cfe-commits.
This check was introduced by r338641
but this broke some builds. For now remove it.
Repository:
rC Clang
https://reviews.llvm.org/D50163
Files:
lib/AST/DeclObjC.cpp
Index: lib/AST/DeclObjC.cpp
===
--- lib/AST/DeclObjC.cpp
+++ lib/AST/DeclObjC.cpp
@@ -787,13 +787,6 @@
: NamedDecl(ObjCMethod, contextDecl, beginLoc, SelInfo),
DeclContext(ObjCMethod), MethodDeclType(T), ReturnTInfo(ReturnTInfo),
DeclEndLoc(endLoc) {
- // See the comment in ObjCMethodFamilyBitfields about
- // ObjCMethodFamilyBitWidth for why we check this.
- static_assert(
- static_cast(ObjCMethodDeclBits.ObjCMethodFamilyBitWidth) ==
- static_cast(ObjCMethodFamilyBitWidth),
- "ObjCMethodDeclBitfields::ObjCMethodFamilyBitWidth and "
- "ObjCMethodFamilyBitWidth do not match!");
// Initialized the bits stored in DeclContext.
ObjCMethodDeclBits.Family =
Index: lib/AST/DeclObjC.cpp
===
--- lib/AST/DeclObjC.cpp
+++ lib/AST/DeclObjC.cpp
@@ -787,13 +787,6 @@
: NamedDecl(ObjCMethod, contextDecl, beginLoc, SelInfo),
DeclContext(ObjCMethod), MethodDeclType(T), ReturnTInfo(ReturnTInfo),
DeclEndLoc(endLoc) {
- // See the comment in ObjCMethodFamilyBitfields about
- // ObjCMethodFamilyBitWidth for why we check this.
- static_assert(
- static_cast(ObjCMethodDeclBits.ObjCMethodFamilyBitWidth) ==
- static_cast(ObjCMethodFamilyBitWidth),
- "ObjCMethodDeclBitfields::ObjCMethodFamilyBitWidth and "
- "ObjCMethodFamilyBitWidth do not match!");
// Initialized the bits stored in DeclContext.
ObjCMethodDeclBits.Family =
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r338641 - [AST][4/4] Move the bit-fields from ObjCMethodDecl and ObjCContainerDecl into DeclContext
FYI, this breaks clang:
.../llvm/tools/clang/lib/AST/DeclObjC.cpp:793:7: error: static_assert
expression is not an integral constant expression
static_cast(ObjCMethodDeclBits.ObjCMethodFamilyBitWidth) ==
^
.../llvm/tools/clang/lib/AST/DeclObjC.cpp:793:29: note: implicit use of
'this' pointer is only allowed within the evaluation of a call to a
'constexpr' member function
static_cast(ObjCMethodDeclBits.ObjCMethodFamilyBitWidth) ==
^
1 error generated.
On Wed, Aug 1, 2018 at 2:31 PM Erich Keane via cfe-commits <
cfe-commits@lists.llvm.org> wrote:
> Author: erichkeane
> Date: Wed Aug 1 14:31:08 2018
> New Revision: 338641
>
> URL: http://llvm.org/viewvc/llvm-project?rev=338641&view=rev
> Log:
> [AST][4/4] Move the bit-fields from ObjCMethodDecl and ObjCContainerDecl
> into DeclContext
>
> This patch follows https://reviews.llvm.org/D49729,
> https://reviews.llvm.org/D49732 and
> https://reviews.llvm.org/D49733.
>
> Move the bits from ObjCMethodDecl and ObjCContainerDecl
> into DeclContext.
>
> Differential Revision: https://reviews.llvm.org/D49734
>
> Patch By: bricci
>
> Modified:
> cfe/trunk/include/clang/AST/DeclObjC.h
> cfe/trunk/lib/AST/DeclObjC.cpp
> cfe/trunk/lib/Sema/SemaDeclObjC.cpp
> cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
> cfe/trunk/lib/Serialization/ASTWriterDecl.cpp
>
> Modified: cfe/trunk/include/clang/AST/DeclObjC.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclObjC.h?rev=338641&r1=338640&r2=338641&view=diff
>
> ==
> --- cfe/trunk/include/clang/AST/DeclObjC.h (original)
> +++ cfe/trunk/include/clang/AST/DeclObjC.h Wed Aug 1 14:31:08 2018
> @@ -141,58 +141,10 @@ public:
>enum ImplementationControl { None, Required, Optional };
>
> private:
> - // The conventional meaning of this method; an ObjCMethodFamily.
> - // This is not serialized; instead, it is computed on demand and
> - // cached.
> - mutable unsigned Family : ObjCMethodFamilyBitWidth;
> -
> - /// instance (true) or class (false) method.
> - unsigned IsInstance : 1;
> - unsigned IsVariadic : 1;
> -
> - /// True if this method is the getter or setter for an explicit
> property.
> - unsigned IsPropertyAccessor : 1;
> -
> - // Method has a definition.
> - unsigned IsDefined : 1;
> -
> - /// Method redeclaration in the same interface.
> - unsigned IsRedeclaration : 1;
> -
> - /// Is redeclared in the same interface.
> - mutable unsigned HasRedeclaration : 1;
> -
> - // NOTE: VC++ treats enums as signed, avoid using ImplementationControl
> enum
> - /// \@required/\@optional
> - unsigned DeclImplementation : 2;
> -
> - // NOTE: VC++ treats enums as signed, avoid using the ObjCDeclQualifier
> enum
> - /// in, inout, etc.
> - unsigned objcDeclQualifier : 7;
> -
> - /// Indicates whether this method has a related result type.
> - unsigned RelatedResultType : 1;
> -
> - /// Whether the locations of the selector identifiers are in a
> - /// "standard" position, a enum SelectorLocationsKind.
> - unsigned SelLocsKind : 2;
> -
> - /// Whether this method overrides any other in the class hierarchy.
> - ///
> - /// A method is said to override any method in the class's
> - /// base classes, its protocols, or its categories' protocols, that has
> - /// the same selector and is of the same kind (class or instance).
> - /// A method in an implementation is not considered as overriding the
> same
> - /// method in the interface or its categories.
> - unsigned IsOverriding : 1;
> -
> - /// Indicates if the method was a definition but its body was skipped.
> - unsigned HasSkippedBody : 1;
> -
> - // Return type of this method.
> + /// Return type of this method.
>QualType MethodDeclType;
>
> - // Type source information for the return type.
> + /// Type source information for the return type.
>TypeSourceInfo *ReturnTInfo;
>
>/// Array of ParmVarDecls for the formal parameters of this method
> @@ -203,7 +155,7 @@ private:
>/// List of attributes for this method declaration.
>SourceLocation DeclEndLoc; // the location of the ';' or '{'.
>
> - // The following are only used for method definitions, null otherwise.
> + /// The following are only used for method definitions, null otherwise.
>LazyDeclStmtPtr Body;
>
>/// SelfDecl - Decl for the implicit self parameter. This is lazily
> @@ -220,21 +172,14 @@ private:
> bool isVariadic = false, bool isPropertyAccessor = false,
> bool isImplicitlyDeclared = false, bool isDefined =
> false,
> ImplementationControl impControl = None,
> - bool HasRelatedResultType = false)
> - : NamedDecl(ObjCMethod, contextDecl, beginLoc, SelInfo),
> -DeclContext(ObjCMethod), Family(InvalidObjCMethodFamily),
> -IsInstance(is
r338643 - Fix -Wcovered-switch-default uncovered after r338630
Author: rnk
Date: Wed Aug 1 15:10:03 2018
New Revision: 338643
URL: http://llvm.org/viewvc/llvm-project?rev=338643&view=rev
Log:
Fix -Wcovered-switch-default uncovered after r338630
Modified:
cfe/trunk/lib/AST/DeclBase.cpp
Modified: cfe/trunk/lib/AST/DeclBase.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclBase.cpp?rev=338643&r1=338642&r2=338643&view=diff
==
--- cfe/trunk/lib/AST/DeclBase.cpp (original)
+++ cfe/trunk/lib/AST/DeclBase.cpp Wed Aug 1 15:10:03 2018
@@ -154,11 +154,11 @@ void Decl::setInvalidDecl(bool Invalid)
const char *DeclContext::getDeclKindName() const {
switch (getDeclKind()) {
- default: llvm_unreachable("Declaration context not in DeclNodes.inc!");
#define DECL(DERIVED, BASE) case Decl::DERIVED: return #DERIVED;
#define ABSTRACT_DECL(DECL)
#include "clang/AST/DeclNodes.inc"
}
+ llvm_unreachable("Declaration context not in DeclNodes.inc!");
}
bool Decl::StatisticsEnabled = false;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49952: Check for NULL Destination-Type when creating ArrayConstant
rjmccall added inline comments.
Comment at: test/CodeGenCXX/empty-struct-init-list.cpp:11
+} c;
+c d{ };
Please make this a `FileCheck` test that tests the actual compiler output.
https://reviews.llvm.org/D49952
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50003: Sema: Fix explicit address space cast involving void pointers
rjmccall accepted this revision. rjmccall added a comment. This revision is now accepted and ready to land. One minor request; feel free to just do that when you commit. Comment at: lib/Sema/SemaCast.cpp:1050 + SrcType->getAs()->getPointeeType().getAddressSpace() != + DestType->getAs()->getPointeeType().getAddressSpace(); +} I know the code was like this before, but please rewrite this to just use `getAs()` instead of doing the separate `isPointerType()` check. https://reviews.llvm.org/D50003 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50055: Update the coding standard about NFC changes and whitespace
chandlerc added inline comments. Comment at: docs/DeveloperPolicy.rst:395-408 +Commits with No Functional Change +- + +It may be permissible to commit changes without prior review when the changes +have no semantic impact on the code if the changes being made are obvious and +not invasive. For instance, removing trailing whitespace from a line, fixing a +line ending to be consistent with the rest of the file, fixing a typo, code hubert.reinterpretcast wrote: > hfinkel wrote: > > aaron.ballman wrote: > > > chandlerc wrote: > > > > I think this is a much broader statement than is warranted to address > > > > the specific problem. And I'm not completely comfortable with it. > > > > > > > > I don't think guidance around "no functional change" is the right way > > > > to give guidance about what is or isn't "obvious" and fine to commit > > > > with post-commit review personally. > > > > > > > > I'd really suggest just being direct about *formatting* and whitespace > > > > changes, and specifically suggest that they not be made unless the file > > > > or code region in question is an area that the author is planning > > > > subsequent changes to. > > > We talk about formatting and whitespace in the CodingStandards document, > > > but we talk about obviousness and post-commit review in DeveloperPolicy. > > > Where would you like these new words to live? To me, they're more about > > > the policy and less about the coding standard -- the coding standard says > > > what the code should look like and the policy says what you should and > > > should not do procedurally, but then I feel the need to tie it back to > > > "obviousness". How about this in the developer policy: > > > ``` > > > The Obviousness of Formatting Changes > > > - > > > > > > While formatting and whitespace changes may be "obvious", they can also > > > create > > > needless churn that causes difficulties for out-of-tree users carrying > > > local > > > patches. Do not commit formatting or whitespace changes unless they are > > > related > > > to a file or code region that you intend to make subsequent changes to. > > > The > > > formatting and whitespace changes should be highly localized, committed > > > before > > > you begin functionality-changing work on the code region, and the commit > > > message > > > should clearly state that the commit is not intended to change > > > functionality, > > > usually by stating it is :ref:`NFC `. > > > > > > > > > If you wish to make a change to formatting or whitespace that touches an > > > entire > > > library or code base, please seek pre-commit approval first. > > > ``` > > I agree with @chandlerc about the current proposed text, and I think that > > this is better. I wonder if we want to insist on separating the commits, of > > if, combined localized commits are okay? > > > It depends on how much noise there is when combining the commits; and when > evaluating for that, we have to remember that people use different diff tools. I like Hal's separation in the other comment. Here, I tihnk we can address all of this by making this more of a (strong) suggestion and not a hard rule. "Avoid committing formatting or whitespace only changes outside of code you plan to make subsequent changes to." or something similar. Then it also becomes natural to suggest: "Also, try to separate formatting or whitespace changes from functional changes, either by correcting the format first (ideally) or afterward." I think you can also shorten some of the discussion along these lines. https://reviews.llvm.org/D50055 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50160: [c-index-test] Use correct executable path to discover resource directory.
vsapsai created this revision.
vsapsai added reviewers: nathawes, akyrtzi, bob.wilson.
Herald added a subscriber: dexonsmith.
Driver builds resource directory path based on provided executable path.
Instead of string "clang" use actual executable path.
rdar://problem/42699514
https://reviews.llvm.org/D50160
Files:
clang/tools/c-index-test/core_main.cpp
Index: clang/tools/c-index-test/core_main.cpp
===
--- clang/tools/c-index-test/core_main.cpp
+++ clang/tools/c-index-test/core_main.cpp
@@ -20,6 +20,7 @@
#include "clang/Lex/Preprocessor.h"
#include "clang/Serialization/ASTReader.h"
#include "llvm/Support/CommandLine.h"
+#include "llvm/Support/FileSystem.h"
#include "llvm/Support/Signals.h"
#include "llvm/Support/raw_ostream.h"
#include "llvm/Support/PrettyStackTrace.h"
@@ -202,11 +203,13 @@
});
}
-static bool printSourceSymbols(ArrayRef Args,
- bool dumpModuleImports,
- bool indexLocals) {
+static bool printSourceSymbols(const char *ProgName,
+ ArrayRef Args,
+ bool dumpModuleImports, bool indexLocals) {
+ void *P = (void*) (intptr_t) indextest_core_main;
+ std::string Executable = llvm::sys::fs::getMainExecutable(ProgName, P);
SmallVector ArgsWithProgName;
- ArgsWithProgName.push_back("clang");
+ ArgsWithProgName.push_back(Executable.c_str());
ArgsWithProgName.append(Args.begin(), Args.end());
IntrusiveRefCntPtr
Diags(CompilerInstance::createDiagnostics(new DiagnosticOptions));
@@ -314,6 +317,7 @@
int indextest_core_main(int argc, const char **argv) {
sys::PrintStackTraceOnErrorSignal(argv[0]);
PrettyStackTraceProgram X(argc, argv);
+ const char *ProgName = argv[0];
assert(argv[1] == StringRef("core"));
++argv;
@@ -343,7 +347,8 @@
errs() << "error: missing compiler args; pass '-- '\n";
return 1;
}
-return printSourceSymbols(CompArgs, options::DumpModuleImports,
options::IncludeLocals);
+return printSourceSymbols(ProgName, CompArgs, options::DumpModuleImports,
+ options::IncludeLocals);
}
return 0;
Index: clang/tools/c-index-test/core_main.cpp
===
--- clang/tools/c-index-test/core_main.cpp
+++ clang/tools/c-index-test/core_main.cpp
@@ -20,6 +20,7 @@
#include "clang/Lex/Preprocessor.h"
#include "clang/Serialization/ASTReader.h"
#include "llvm/Support/CommandLine.h"
+#include "llvm/Support/FileSystem.h"
#include "llvm/Support/Signals.h"
#include "llvm/Support/raw_ostream.h"
#include "llvm/Support/PrettyStackTrace.h"
@@ -202,11 +203,13 @@
});
}
-static bool printSourceSymbols(ArrayRef Args,
- bool dumpModuleImports,
- bool indexLocals) {
+static bool printSourceSymbols(const char *ProgName,
+ ArrayRef Args,
+ bool dumpModuleImports, bool indexLocals) {
+ void *P = (void*) (intptr_t) indextest_core_main;
+ std::string Executable = llvm::sys::fs::getMainExecutable(ProgName, P);
SmallVector ArgsWithProgName;
- ArgsWithProgName.push_back("clang");
+ ArgsWithProgName.push_back(Executable.c_str());
ArgsWithProgName.append(Args.begin(), Args.end());
IntrusiveRefCntPtr
Diags(CompilerInstance::createDiagnostics(new DiagnosticOptions));
@@ -314,6 +317,7 @@
int indextest_core_main(int argc, const char **argv) {
sys::PrintStackTraceOnErrorSignal(argv[0]);
PrettyStackTraceProgram X(argc, argv);
+ const char *ProgName = argv[0];
assert(argv[1] == StringRef("core"));
++argv;
@@ -343,7 +347,8 @@
errs() << "error: missing compiler args; pass '-- '\n";
return 1;
}
-return printSourceSymbols(CompArgs, options::DumpModuleImports, options::IncludeLocals);
+return printSourceSymbols(ProgName, CompArgs, options::DumpModuleImports,
+ options::IncludeLocals);
}
return 0;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50144: Add Windows support for the GNUstep Objective-C ABI V2.
DHowett-MSFT accepted this revision.
DHowett-MSFT added a comment.
This revision is now accepted and ready to land.
This looks good to me, but I don't have a strong understanding of "outlining."
Comment at: lib/CodeGen/CGBlocks.cpp:1262
+ if (IsWindows) {
+auto *Init = llvm::Function::Create(llvm::FunctionType::get(CGM.VoidTy,
+ {}), llvm::GlobalValue::InternalLinkage, ".block_isa_init",
Is there value in emitting a list of blocks that need to be initialized, then
initializing them in one go in a COMDAT-foldable function?
Comment at: lib/CodeGen/CGObjCGNU.cpp:1522
+ GetClassVar(clsAlias.first) }, sectionName());
+// On ELF platforms, add a null value fore each special section so that we
+// can always guarantee that the _start and _stop symbols will exist and be
nit: `fore`
Comment at: lib/CodeGen/CGObjCRuntime.cpp:234
+if ((CPI =
dyn_cast_or_null(Handler.Block->getFirstNonPHI( {
+CGF.CurrentFuncletPad = CPI;
+CPI->setOperand(2, CGF.getExceptionSlot().getPointer());
this region may need reformatting
Comment at: lib/CodeGen/CGObjCRuntime.cpp:274
CGF.EmitBranchThroughCleanup(Cont);
- }
+CGF.CurrentFuncletPad = nullptr;
+ }
There's a scoped save/restore helper for this that'll offer better support for
nested funclets.
```
SaveAndRestore
RestoreCurrentFuncletPad(CGF.CurrentFuncletPad);
```
Repository:
rC Clang
https://reviews.llvm.org/D50144
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49148: [DEBUGINFO] Disable unsupported debug info options for NVPTX target.
echristo added a comment. In https://reviews.llvm.org/D49148#1184957, @tra wrote: > I wonder, what's the right thing to do to silence the warnings. For instance, > we compile everything with -Werror and the warnings result in build breaks. > > Easy way out is to pass `-Wno-unsupported-target-opt`. It works, but it does > not really solve anything. It also may mask potential other problems. > > Another alternative is to change clang driver and filter out unsupported > options so they are not passed to cc1. That will also work, but it looks > wrong, because now we have two patches that effectively cancel each other for > no observable benefit. > > Third option is to grow a better way to specify target-specific > sub-compilation options and then consider fancy debug flags to be > attributable to host compilation only. Anything beyond the "safe" subset, > would have to be specified explicitly. This also sounds awkward -- I don't > really want to replicate bunch of options times number of GPUs I'm compiling > for. That may be alleviated by providing more coarse way to group options. > E.g. we could say "these are the options for *all* non-host compilations, and > here are few specifically for sm_XY". I think @echristo and I had discussed > something like this long time ago. I think some amount of #3 is the most reasonable way forward here. Basically the compilation model of "pass the flags and hope they all work on all the targets that we're trying to compile for in this giant multiple target compile" seems to be a long tail of pain :) There are probably things we can do in the clang driver to make this a little easier, e.g. the "all non-host target options" flag you mentioned seems reasonable, but otherwise I think this is up to the user to split out flags that might not work. Thoughts? Repository: rC Clang https://reviews.llvm.org/D49148 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49148: [DEBUGINFO] Disable unsupported debug info options for NVPTX target.
tra added a comment. I wonder, what's the right thing to do to silence the warnings. For instance, we compile everything with -Werror and the warnings result in build breaks. Easy way out is to pass `-Wno-unsupported-target-opt`. It works, but it does not really solve anything. It also may mask potential other problems. Another alternative is to change clang driver and filter out unsupported options so they are not passed to cc1. That will also work, but it looks wrong, because now we have two patches that effectively cancel each other for no observable benefit. Third option is to grow a better way to specify target-specific sub-compilation options and then consider fancy debug flags to be attributable to host compilation only. Anything beyond the "safe" subset, would have to be specified explicitly. This also sounds awkward -- I don't really want to replicate bunch of options times number of GPUs I'm compiling for. That may be alleviated by providing more coarse way to group options. E.g. we could say "these are the options for *all* non-host compilations, and here are few specifically for sm_XY". I think @echristo and I had discussed something like this long time ago. Any other ideas? Repository: rC Clang https://reviews.llvm.org/D49148 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49734: [AST][4/4] Move the bit-fields from ObjCMethodDecl and ObjCContainerDecl into DeclContext
This revision was automatically updated to reflect the committed changes.
Closed by commit rL338641: [AST][4/4] Move the bit-fields from ObjCMethodDecl
and ObjCContainerDecl into… (authored by erichkeane, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D49734?vs=158499&id=158641#toc
Repository:
rL LLVM
https://reviews.llvm.org/D49734
Files:
cfe/trunk/include/clang/AST/DeclObjC.h
cfe/trunk/lib/AST/DeclObjC.cpp
cfe/trunk/lib/Sema/SemaDeclObjC.cpp
cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
cfe/trunk/lib/Serialization/ASTWriterDecl.cpp
Index: cfe/trunk/lib/AST/DeclObjC.cpp
===
--- cfe/trunk/lib/AST/DeclObjC.cpp
+++ cfe/trunk/lib/AST/DeclObjC.cpp
@@ -65,6 +65,13 @@
// ObjCInterfaceDecl
//===--===//
+ObjCContainerDecl::ObjCContainerDecl(Kind DK, DeclContext *DC,
+ IdentifierInfo *Id, SourceLocation nameLoc,
+ SourceLocation atStartLoc)
+: NamedDecl(DK, DC, nameLoc, Id), DeclContext(DK) {
+ setAtStartLoc(atStartLoc);
+}
+
void ObjCContainerDecl::anchor() {}
/// getIvarDecl - This method looks up an ivar in this ContextDecl.
@@ -769,6 +776,44 @@
// ObjCMethodDecl
//===--===//
+ObjCMethodDecl::ObjCMethodDecl(SourceLocation beginLoc, SourceLocation endLoc,
+ Selector SelInfo, QualType T,
+ TypeSourceInfo *ReturnTInfo,
+ DeclContext *contextDecl, bool isInstance,
+ bool isVariadic, bool isPropertyAccessor,
+ bool isImplicitlyDeclared, bool isDefined,
+ ImplementationControl impControl,
+ bool HasRelatedResultType)
+: NamedDecl(ObjCMethod, contextDecl, beginLoc, SelInfo),
+ DeclContext(ObjCMethod), MethodDeclType(T), ReturnTInfo(ReturnTInfo),
+ DeclEndLoc(endLoc) {
+ // See the comment in ObjCMethodFamilyBitfields about
+ // ObjCMethodFamilyBitWidth for why we check this.
+ static_assert(
+ static_cast(ObjCMethodDeclBits.ObjCMethodFamilyBitWidth) ==
+ static_cast(ObjCMethodFamilyBitWidth),
+ "ObjCMethodDeclBitfields::ObjCMethodFamilyBitWidth and "
+ "ObjCMethodFamilyBitWidth do not match!");
+
+ // Initialized the bits stored in DeclContext.
+ ObjCMethodDeclBits.Family =
+ static_cast(InvalidObjCMethodFamily);
+ setInstanceMethod(isInstance);
+ setVariadic(isVariadic);
+ setPropertyAccessor(isPropertyAccessor);
+ setDefined(isDefined);
+ setIsRedeclaration(false);
+ setHasRedeclaration(false);
+ setDeclImplementation(impControl);
+ setObjCDeclQualifier(OBJC_TQ_None);
+ setRelatedResultType(HasRelatedResultType);
+ setSelLocsKind(SelLoc_StandardNoSpace);
+ setOverriding(false);
+ setHasSkippedBody(false);
+
+ setImplicit(isImplicitlyDeclared);
+}
+
ObjCMethodDecl *ObjCMethodDecl::Create(
ASTContext &C, SourceLocation beginLoc, SourceLocation endLoc,
Selector SelInfo, QualType T, TypeSourceInfo *ReturnTInfo,
@@ -810,8 +855,8 @@
void ObjCMethodDecl::setAsRedeclaration(const ObjCMethodDecl *PrevMethod) {
assert(PrevMethod);
getASTContext().setObjCMethodRedeclaration(PrevMethod, this);
- IsRedeclaration = true;
- PrevMethod->HasRedeclaration = true;
+ setIsRedeclaration(true);
+ PrevMethod->setHasRedeclaration(true);
}
void ObjCMethodDecl::setParamsAndSelLocs(ASTContext &C,
@@ -846,9 +891,9 @@
if (isImplicit())
return setParamsAndSelLocs(C, Params, llvm::None);
- SelLocsKind = hasStandardSelectorLocs(getSelector(), SelLocs, Params,
-DeclEndLoc);
- if (SelLocsKind != SelLoc_NonStandard)
+ setSelLocsKind(hasStandardSelectorLocs(getSelector(), SelLocs, Params,
+DeclEndLoc));
+ if (getSelLocsKind() != SelLoc_NonStandard)
return setParamsAndSelLocs(C, Params, llvm::None);
setParamsAndSelLocs(C, Params, SelLocs);
@@ -860,7 +905,7 @@
ObjCMethodDecl *ObjCMethodDecl::getNextRedeclarationImpl() {
ASTContext &Ctx = getASTContext();
ObjCMethodDecl *Redecl = nullptr;
- if (HasRedeclaration)
+ if (hasRedeclaration())
Redecl = const_cast(Ctx.getObjCMethodRedeclaration(this));
if (Redecl)
return Redecl;
@@ -938,7 +983,7 @@
}
ObjCMethodFamily ObjCMethodDecl::getMethodFamily() const {
- auto family = static_cast(Family);
+ auto family = static_cast(ObjCMethodDeclBits.Family);
if (family != static_cast(InvalidObjCMethodFamily))
return family;
@@ -954,7 +999,7 @@
case ObjCMethodFamilyAttr::OMF_mutableCopy: family = OMF_mutableCopy; break;
case ObjCMethodFamilyAttr::OMF_new: family = OMF_new; break;
}
-Family = static_cast(family)
r338641 - [AST][4/4] Move the bit-fields from ObjCMethodDecl and ObjCContainerDecl into DeclContext
Author: erichkeane
Date: Wed Aug 1 14:31:08 2018
New Revision: 338641
URL: http://llvm.org/viewvc/llvm-project?rev=338641&view=rev
Log:
[AST][4/4] Move the bit-fields from ObjCMethodDecl and ObjCContainerDecl into
DeclContext
This patch follows https://reviews.llvm.org/D49729,
https://reviews.llvm.org/D49732 and
https://reviews.llvm.org/D49733.
Move the bits from ObjCMethodDecl and ObjCContainerDecl
into DeclContext.
Differential Revision: https://reviews.llvm.org/D49734
Patch By: bricci
Modified:
cfe/trunk/include/clang/AST/DeclObjC.h
cfe/trunk/lib/AST/DeclObjC.cpp
cfe/trunk/lib/Sema/SemaDeclObjC.cpp
cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
cfe/trunk/lib/Serialization/ASTWriterDecl.cpp
Modified: cfe/trunk/include/clang/AST/DeclObjC.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclObjC.h?rev=338641&r1=338640&r2=338641&view=diff
==
--- cfe/trunk/include/clang/AST/DeclObjC.h (original)
+++ cfe/trunk/include/clang/AST/DeclObjC.h Wed Aug 1 14:31:08 2018
@@ -141,58 +141,10 @@ public:
enum ImplementationControl { None, Required, Optional };
private:
- // The conventional meaning of this method; an ObjCMethodFamily.
- // This is not serialized; instead, it is computed on demand and
- // cached.
- mutable unsigned Family : ObjCMethodFamilyBitWidth;
-
- /// instance (true) or class (false) method.
- unsigned IsInstance : 1;
- unsigned IsVariadic : 1;
-
- /// True if this method is the getter or setter for an explicit property.
- unsigned IsPropertyAccessor : 1;
-
- // Method has a definition.
- unsigned IsDefined : 1;
-
- /// Method redeclaration in the same interface.
- unsigned IsRedeclaration : 1;
-
- /// Is redeclared in the same interface.
- mutable unsigned HasRedeclaration : 1;
-
- // NOTE: VC++ treats enums as signed, avoid using ImplementationControl enum
- /// \@required/\@optional
- unsigned DeclImplementation : 2;
-
- // NOTE: VC++ treats enums as signed, avoid using the ObjCDeclQualifier enum
- /// in, inout, etc.
- unsigned objcDeclQualifier : 7;
-
- /// Indicates whether this method has a related result type.
- unsigned RelatedResultType : 1;
-
- /// Whether the locations of the selector identifiers are in a
- /// "standard" position, a enum SelectorLocationsKind.
- unsigned SelLocsKind : 2;
-
- /// Whether this method overrides any other in the class hierarchy.
- ///
- /// A method is said to override any method in the class's
- /// base classes, its protocols, or its categories' protocols, that has
- /// the same selector and is of the same kind (class or instance).
- /// A method in an implementation is not considered as overriding the same
- /// method in the interface or its categories.
- unsigned IsOverriding : 1;
-
- /// Indicates if the method was a definition but its body was skipped.
- unsigned HasSkippedBody : 1;
-
- // Return type of this method.
+ /// Return type of this method.
QualType MethodDeclType;
- // Type source information for the return type.
+ /// Type source information for the return type.
TypeSourceInfo *ReturnTInfo;
/// Array of ParmVarDecls for the formal parameters of this method
@@ -203,7 +155,7 @@ private:
/// List of attributes for this method declaration.
SourceLocation DeclEndLoc; // the location of the ';' or '{'.
- // The following are only used for method definitions, null otherwise.
+ /// The following are only used for method definitions, null otherwise.
LazyDeclStmtPtr Body;
/// SelfDecl - Decl for the implicit self parameter. This is lazily
@@ -220,21 +172,14 @@ private:
bool isVariadic = false, bool isPropertyAccessor = false,
bool isImplicitlyDeclared = false, bool isDefined = false,
ImplementationControl impControl = None,
- bool HasRelatedResultType = false)
- : NamedDecl(ObjCMethod, contextDecl, beginLoc, SelInfo),
-DeclContext(ObjCMethod), Family(InvalidObjCMethodFamily),
-IsInstance(isInstance), IsVariadic(isVariadic),
-IsPropertyAccessor(isPropertyAccessor), IsDefined(isDefined),
-IsRedeclaration(0), HasRedeclaration(0),
DeclImplementation(impControl),
-objcDeclQualifier(OBJC_TQ_None),
-RelatedResultType(HasRelatedResultType),
-SelLocsKind(SelLoc_StandardNoSpace), IsOverriding(0),
HasSkippedBody(0),
-MethodDeclType(T), ReturnTInfo(ReturnTInfo), DeclEndLoc(endLoc) {
-setImplicit(isImplicitlyDeclared);
- }
+ bool HasRelatedResultType = false);
SelectorLocationsKind getSelLocsKind() const {
-return (SelectorLocationsKind)SelLocsKind;
+return static_cast(ObjCMethodDeclBits.SelLocsKind);
+ }
+
+ void setSelLocsKind(SelectorLocationsKind Kind) {
+ObjCMethodDeclBits.SelLocsKind = Kind;
}
bool hasStandardSelLocs() const {
@@ -244,10 +189,10 @@ private:
/// Get
[PATCH] D50055: Update the coding standard about NFC changes and whitespace
hubert.reinterpretcast added inline comments. Comment at: docs/DeveloperPolicy.rst:395-408 +Commits with No Functional Change +- + +It may be permissible to commit changes without prior review when the changes +have no semantic impact on the code if the changes being made are obvious and +not invasive. For instance, removing trailing whitespace from a line, fixing a +line ending to be consistent with the rest of the file, fixing a typo, code hfinkel wrote: > aaron.ballman wrote: > > chandlerc wrote: > > > I think this is a much broader statement than is warranted to address the > > > specific problem. And I'm not completely comfortable with it. > > > > > > I don't think guidance around "no functional change" is the right way to > > > give guidance about what is or isn't "obvious" and fine to commit with > > > post-commit review personally. > > > > > > I'd really suggest just being direct about *formatting* and whitespace > > > changes, and specifically suggest that they not be made unless the file > > > or code region in question is an area that the author is planning > > > subsequent changes to. > > We talk about formatting and whitespace in the CodingStandards document, > > but we talk about obviousness and post-commit review in DeveloperPolicy. > > Where would you like these new words to live? To me, they're more about the > > policy and less about the coding standard -- the coding standard says what > > the code should look like and the policy says what you should and should > > not do procedurally, but then I feel the need to tie it back to > > "obviousness". How about this in the developer policy: > > ``` > > The Obviousness of Formatting Changes > > - > > > > While formatting and whitespace changes may be "obvious", they can also > > create > > needless churn that causes difficulties for out-of-tree users carrying local > > patches. Do not commit formatting or whitespace changes unless they are > > related > > to a file or code region that you intend to make subsequent changes to. The > > formatting and whitespace changes should be highly localized, committed > > before > > you begin functionality-changing work on the code region, and the commit > > message > > should clearly state that the commit is not intended to change > > functionality, > > usually by stating it is :ref:`NFC `. > > > > > > If you wish to make a change to formatting or whitespace that touches an > > entire > > library or code base, please seek pre-commit approval first. > > ``` > I agree with @chandlerc about the current proposed text, and I think that > this is better. I wonder if we want to insist on separating the commits, of > if, combined localized commits are okay? > It depends on how much noise there is when combining the commits; and when evaluating for that, we have to remember that people use different diff tools. https://reviews.llvm.org/D50055 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50152: [CodeGen] Merge equivalent block copy/helper functions
ahatanak added inline comments.
Comment at: test/CodeGenCXX/block-byref-cxx-objc.cpp:36
+// CHECK: call void @_Block_object_dispose(
+
+int testB() {
ahatanak wrote:
> Should the second call to @_Block_object_dispose be an invoke if the
> destructor can throw? The FIXME in CodeGenFunction::BuildBlockRelease seems
> to imply that we should consider whether the destructor can throw.
I mean the first call, not the second call. If the call to A's destructor
throws when the first object is being destructed, the second object should be
destructed on the EH path.
Repository:
rC Clang
https://reviews.llvm.org/D50152
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50152: [CodeGen] Merge equivalent block copy/helper functions
ahatanak updated this revision to Diff 158640.
ahatanak marked an inline comment as done.
ahatanak added a comment.
Use llvm::sort.
Repository:
rC Clang
https://reviews.llvm.org/D50152
Files:
lib/CodeGen/CGBlocks.cpp
lib/CodeGen/CGNonTrivialStruct.cpp
lib/CodeGen/CodeGenFunction.cpp
lib/CodeGen/CodeGenFunction.h
test/CodeGen/blocks-1.c
test/CodeGen/blocks.c
test/CodeGen/sanitize-thread-no-checking-at-run-time.m
test/CodeGenCXX/block-byref-cxx-objc.cpp
test/CodeGenCXX/blocks.cpp
test/CodeGenCXX/cxx-block-objects.cpp
test/CodeGenObjC/arc-blocks.m
test/CodeGenObjC/debug-info-block-helper.m
test/CodeGenObjC/debug-info-blocks.m
test/CodeGenObjC/mrc-weak.m
test/CodeGenObjC/strong-in-c-struct.m
test/CodeGenObjCXX/arc-blocks.mm
test/CodeGenObjCXX/lambda-to-block.mm
test/CodeGenObjCXX/mrc-weak.mm
Index: test/CodeGenObjCXX/mrc-weak.mm
===
--- test/CodeGenObjCXX/mrc-weak.mm
+++ test/CodeGenObjCXX/mrc-weak.mm
@@ -119,10 +119,10 @@
// CHECK: call void @use_block
// CHECK: call void @objc_destroyWeak
-// CHECK-LABEL: define internal void @__copy_helper_block
+// CHECK-LABEL: define linkonce_odr hidden void @__copy_helper_block
// CHECK: @objc_copyWeak
-// CHECK-LABEL: define internal void @__destroy_helper_block
+// CHECK-LABEL: define linkonce_odr hidden void @__destroy_helper_block
// CHECK: @objc_destroyWeak
void test8(void) {
@@ -142,8 +142,8 @@
// CHECK: call void @objc_destroyWeak
// CHECK-LABEL: define void @_Z14test9_baselinev()
-// CHECK: define internal void @__copy_helper
-// CHECK: define internal void @__destroy_helper
+// CHECK: define linkonce_odr hidden void @__copy_helper
+// CHECK: define linkonce_odr hidden void @__destroy_helper
void test9_baseline(void) {
Foo *p = get_object();
use_block(^{ [p run]; });
Index: test/CodeGenObjCXX/lambda-to-block.mm
===
--- test/CodeGenObjCXX/lambda-to-block.mm
+++ test/CodeGenObjCXX/lambda-to-block.mm
@@ -12,7 +12,7 @@
void hasLambda(Copyable x) {
takesBlock([x] () { });
}
-// CHECK-LABEL: define internal void @__copy_helper_block_
+// CHECK-LABEL: define linkonce_odr hidden void @__copy_helper_block_
// CHECK: call void @"_ZZ9hasLambda8CopyableEN3$_0C1ERKS0_"
// CHECK-LABEL: define internal void @"_ZZ9hasLambda8CopyableEN3$_0C2ERKS0_"
// CHECK: call void @_ZN8CopyableC1ERKS_
Index: test/CodeGenObjCXX/arc-blocks.mm
===
--- test/CodeGenObjCXX/arc-blocks.mm
+++ test/CodeGenObjCXX/arc-blocks.mm
@@ -1,5 +1,6 @@
// RUN: %clang_cc1 -std=gnu++98 -triple x86_64-apple-darwin10 -emit-llvm -fobjc-runtime-has-weak -fblocks -fobjc-arc -fexceptions -fobjc-arc-exceptions -o - %s | FileCheck -check-prefix CHECK %s
// RUN: %clang_cc1 -std=gnu++98 -triple x86_64-apple-darwin10 -emit-llvm -fobjc-runtime-has-weak -fblocks -fobjc-arc -fexceptions -fobjc-arc-exceptions -O1 -o - %s | FileCheck -check-prefix CHECK-O1 %s
+// RUN: %clang_cc1 -std=gnu++98 -triple x86_64-apple-darwin10 -emit-llvm -fobjc-runtime-has-weak -fblocks -fobjc-arc -o - %s | FileCheck -check-prefix CHECK-NOEXCP %s
// CHECK: [[A:.*]] = type { i64, [10 x i8*] }
// CHECK: %[[STRUCT_TEST1_S0:.*]] = type { i32 }
@@ -55,10 +56,16 @@
// Check that copy/dispose helper functions are exception safe.
-// CHECK-LABEL: define internal void @__copy_helper_block_(
+// CHECK-LABEL: define linkonce_odr hidden void @__copy_helper_block_ea8_s_32_b8_40_w_48_c2S0_56_c2S0_60(
// CHECK: %[[BLOCK_SOURCE:.*]] = bitcast i8* %{{.*}} to <{ i8*, i32, i32, i8*, %[[STRUCT_BLOCK_DESCRIPTOR]]*, i8*, i8*, i8*, %[[STRUCT_TEST1_S0]], %[[STRUCT_TEST1_S0]] }>*
// CHECK: %[[BLOCK_DEST:.*]] = bitcast i8* %{{.*}} to <{ i8*, i32, i32, i8*, %[[STRUCT_BLOCK_DESCRIPTOR]]*, i8*, i8*, i8*, %[[STRUCT_TEST1_S0]], %[[STRUCT_TEST1_S0]] }>*
+// CHECK: %[[V9:.*]] = getelementptr inbounds <{ i8*, i32, i32, i8*, %[[STRUCT_BLOCK_DESCRIPTOR]]*, i8*, i8*, i8*, %[[STRUCT_TEST1_S0]], %[[STRUCT_TEST1_S0]] }>, <{ i8*, i32, i32, i8*, %[[STRUCT_BLOCK_DESCRIPTOR]]*, i8*, i8*, i8*, %[[STRUCT_TEST1_S0]], %[[STRUCT_TEST1_S0]] }>* %[[BLOCK_SOURCE]], i32 0, i32 5
+// CHECK: %[[V10:.*]] = getelementptr inbounds <{ i8*, i32, i32, i8*, %[[STRUCT_BLOCK_DESCRIPTOR]]*, i8*, i8*, i8*, %[[STRUCT_TEST1_S0]], %[[STRUCT_TEST1_S0]] }>, <{ i8*, i32, i32, i8*, %[[STRUCT_BLOCK_DESCRIPTOR]]*, i8*, i8*, i8*, %[[STRUCT_TEST1_S0]], %[[STRUCT_TEST1_S0]] }>* %[[BLOCK_DEST]], i32 0, i32 5
+// CHECK: %[[BLOCKCOPY_SRC2:.*]] = load i8*, i8** %[[V9]], align 8
+// CHECK: store i8* null, i8** %[[V10]], align 8
+// CHECK: call void @objc_storeStrong(i8** %[[V10]], i8* %[[BLOCKCOPY_SRC2]])
+
// CHECK: %[[V4:.*]] = getelementptr inbounds <{ i8*, i32, i32, i8*, %[[STRUCT_BLOCK_DESCRIPTOR]]*, i8*, i8*, i8*, %[[STRUCT_TEST1_S0]], %[[STRUCT_TEST1_S0]] }>, <{ i8*, i32, i32, i8*, %[[STRUCT_BLOCK
[PATCH] D48100: Append new attributes to the end of an AttributeList.
Meinersbur updated this revision to Diff 158639.
Meinersbur marked an inline comment as done.
Meinersbur added a comment.
- Remove TODOs about the attribute order
Repository:
rC Clang
https://reviews.llvm.org/D48100
Files:
include/clang/Sema/ParsedAttr.h
lib/AST/ItaniumMangle.cpp
lib/Parse/ParseDeclCXX.cpp
lib/Parse/ParseObjc.cpp
lib/Sema/SemaAttr.cpp
lib/Sema/SemaOverload.cpp
lib/Sema/SemaType.cpp
lib/Serialization/ASTReaderDecl.cpp
test/Index/annotate-comments-availability-attrs.cpp
test/Index/complete-with-annotations.cpp
test/Misc/ast-print-pragmas.cpp
test/PCH/pragma-loop.cpp
test/Parser/pragma-loop-safety.cpp
test/Parser/pragma-loop.cpp
test/Parser/pragma-unroll-and-jam.cpp
test/Parser/pragma-unroll.cpp
test/Sema/attr-availability-tvos.c
test/Sema/attr-availability.c
test/Sema/attr-coldhot.c
test/Sema/attr-disable-tail-calls.c
test/Sema/attr-long-call.c
test/Sema/attr-micromips.c
test/Sema/attr-notail.c
test/Sema/attr-ownership.c
test/Sema/attr-ownership.cpp
test/Sema/attr-print.c
test/Sema/attr-visibility.c
test/Sema/internal_linkage.c
test/Sema/mips-interrupt-attr.c
test/Sema/nullability.c
test/SemaCXX/attr-print.cpp
test/SemaCXX/ms-uuid.cpp
test/SemaObjC/nullability.m
test/SemaOpenCL/address-spaces.cl
test/SemaTemplate/attributes.cpp
Index: test/SemaTemplate/attributes.cpp
===
--- test/SemaTemplate/attributes.cpp
+++ test/SemaTemplate/attributes.cpp
@@ -55,11 +55,11 @@
}
// CHECK: FunctionTemplateDecl {{.*}} HasAnnotations
-// CHECK: AnnotateAttr {{.*}} "ANNOTATE_BAR"
// CHECK: AnnotateAttr {{.*}} "ANNOTATE_FOO"
+// CHECK: AnnotateAttr {{.*}} "ANNOTATE_BAR"
// CHECK: FunctionDecl {{.*}} HasAnnotations
// CHECK: TemplateArgument type 'int'
-// CHECK: AnnotateAttr {{.*}} "ANNOTATE_BAR"
// CHECK: AnnotateAttr {{.*}} "ANNOTATE_FOO"
+// CHECK: AnnotateAttr {{.*}} "ANNOTATE_BAR"
template [[clang::annotate("ANNOTATE_FOO"), clang::annotate("ANNOTATE_BAR")]] void HasAnnotations();
void UseAnnotations() { HasAnnotations(); }
Index: test/SemaOpenCL/address-spaces.cl
===
--- test/SemaOpenCL/address-spaces.cl
+++ test/SemaOpenCL/address-spaces.cl
@@ -61,8 +61,8 @@
void func_multiple_addr(void) {
typedef __private int private_int_t;
- __local __private int var1; // expected-error {{multiple address spaces specified for type}}
- __local __private int *var2; // expected-error {{multiple address spaces specified for type}}
+ __private __local int var1; // expected-error {{multiple address spaces specified for type}}
+ __private __local int *var2; // expected-error {{multiple address spaces specified for type}}
__local private_int_t var3; // expected-error {{multiple address spaces specified for type}}
__local private_int_t *var4; // expected-error {{multiple address spaces specified for type}}
__private private_int_t var5; // expected-warning {{multiple identical address spaces specified for type}}
Index: test/SemaObjC/nullability.m
===
--- test/SemaObjC/nullability.m
+++ test/SemaObjC/nullability.m
@@ -36,14 +36,14 @@
- (nonnull NSFoo **)invalidMethod1; // expected-error{{nullability keyword 'nonnull' cannot be applied to multi-level pointer type 'NSFoo **'}}
// expected-note@-1{{use nullability type specifier '_Nonnull' to affect the innermost pointer type of 'NSFoo **'}}
-- (nonnull NSFoo * _Nullable)conflictingMethod1; // expected-error{{nullability specifier '_Nullable' conflicts with existing specifier '_Nonnull'}}
-- (nonnull NSFoo * _Nonnull)redundantMethod1; // expected-warning{{duplicate nullability specifier '_Nonnull'}}
+- (nonnull NSFoo * _Nullable)conflictingMethod1; // expected-error{{nullability specifier 'nonnull' conflicts with existing specifier '_Nullable'}}
+- (nonnull NSFoo * _Nonnull)redundantMethod1; // expected-warning{{duplicate nullability specifier 'nonnull'}}
@property(nonnull,retain) NSFoo *property1;
@property(nullable,assign) NSFoo ** invalidProperty1; // expected-error{{nullability keyword 'nullable' cannot be applied to multi-level pointer type 'NSFoo **'}}
// expected-note@-1{{use nullability type specifier '_Nullable' to affect the innermost pointer type of 'NSFoo **'}}
-@property(null_unspecified,retain) NSFoo * _Nullable conflictingProperty1; // expected-error{{nullability specifier '_Nullable' conflicts with existing specifier '_Null_unspecified'}}
-@property(retain,nonnull) NSFoo * _Nonnull redundantProperty1; // expected-warning{{duplicate nullability specifier '_Nonnull'}}
+@property(null_unspecified,retain) NSFoo * _Nullable conflictingProperty1; // expected-error{{nullability specifier 'null_unspecified' conflicts with existing specifier '_Nullable'}}
+@property(retain,nonnull) NSFoo * _Nonnull redundantProperty1; // expected-warning{{duplicate n
[PATCH] D48100: Append new attributes to the end of an AttributeList.
Meinersbur marked an inline comment as done.
Meinersbur added inline comments.
Comment at: test/Sema/attr-ownership.c:22
void f15(int, int)
- __attribute__((ownership_returns(foo, 1))) // expected-note {{declared with
index 1 here}}
- __attribute__((ownership_returns(foo, 2))); // expected-error
{{'ownership_returns' attribute index does not match; here it is 2}}
+ __attribute__((ownership_returns(foo, 1))) // expected-error
{{'ownership_returns' attribute index does not match; here it is 1}}
+ __attribute__((ownership_returns(foo, 2))); // expected-note {{declared with
index 2 here}}
erichkeane wrote:
> This seems wrong to me, the 2nd one should be the error condition, right?
This is the result of how attributes are combined. There are the two
possibilities
```
void f15(int, int) __attribute__((ownership_returns(foo, 1)));
void f15(int, int) __attribute__((ownership_returns(foo, 2)));
```
and
```
void f15(int, int) __attribute__((ownership_returns(foo, 1)))
__attribute__((ownership_returns(foo, 2)));
```
The error diagnosis seem to have been written with the first case in mind and
emits in the order there. In the second case attributes merged in the other
order (which is the naively correct order, see
https://reviews.llvm.org/D48100#1142865), resulting diagnosis to be emitted in
the reverse-textual order. There is no consistency in which SourceLocation is
the first and which one is the second, and many diagnoses are already not
printed in textual order.
I cannot change this without massively reworking how attributes are processed
in clang.
Comment at: test/Sema/attr-print.c:25
// TODO: the Type Printer has no way to specify the order to print attributes
// in, and so it currently always prints them in reverse order. Fix this.
erichkeane wrote:
> This TODO doesn't apply, right? Or is at least wrong...
Correct, I missed this todo.
Comment at: test/Sema/attr-visibility.c:18
-void test6() __attribute__((visibility("hidden"), // expected-note {{previous
attribute is here}}
-visibility("default"))); // expected-error
{{visibility does not match previous declaration}}
+void test6() __attribute__((visibility("default"), // expected-error
{{visibility does not match previous declaration}}
+visibility("hidden"))); // expected-note
{{previous attribute is here}}
erichkeane wrote:
> This order issue is likely to appear a couple of times I suspect.
See my previous reply and https://reviews.llvm.org/D48100#1142865
Comment at: test/SemaOpenCL/address-spaces.cl:64
typedef __private int private_int_t;
- __local __private int var1; // expected-error {{multiple address spaces
specified for type}}
- __local __private int *var2; // expected-error {{multiple address spaces
specified for type}}
+ __private __local int var1; // expected-error {{multiple address spaces
specified for type}}
+ __private __local int *var2; // expected-error {{multiple address spaces
specified for type}}
erichkeane wrote:
> These changes have me concerned... The error message isn't specific, but we
> have to change the order anyway?
An additional error is emitted with the reverse order (`non-kernel function
variable cannot be declared in local address space`; a result of which
attribute 'wins' in error resolution which is again related to which attribute
is already in the AttributeList). I'd say it is the responsibility diagnosis
code to emit the same set of messages independent of attribute order which I do
not try to fix here.
Repository:
rC Clang
https://reviews.llvm.org/D48100
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r338489 - [AST] CastExpr: BasePathSize is not large enough.
r338640
On Wed, Aug 1, 2018 at 8:38 PM, Roman Lebedev wrote:
> On Wed, Aug 1, 2018 at 8:36 PM, Richard Smith wrote:
>> On Tue, 31 Jul 2018, 23:06 Roman Lebedev via cfe-commits,
>> wrote:
>>>
>>> Author: lebedevri
>>> Date: Tue Jul 31 23:06:16 2018
>>> New Revision: 338489
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=338489&view=rev
>>> Log:
>>> [AST] CastExpr: BasePathSize is not large enough.
>>>
>>> Summary:
>>> rC337815 / D49508 had to cannibalize one bit of
>>> `CastExprBitfields::BasePathSize` in order to squeeze `PartOfExplicitCast`
>>> boolean.
>>> That reduced the maximal value of `PartOfExplicitCast` from 9 bits (~512)
>>> down to 8 bits (~256).
>>> Apparently, that mattered. Too bad there weren't any tests.
>>> It caused [[ https://bugs.llvm.org/show_bug.cgi?id=38356 | PR38356 ]].
>>>
>>> So we need to increase `PartOfExplicitCast` back at least to 9 bits, or a
>>> bit more.
>>> For obvious reasons, we can't do that in `CastExprBitfields` - that would
>>> blow up the size of every `Expr`.
>>> So we need to either just add a variable into the `CastExpr` (as done
>>> here),
>>> or use `llvm::TrailingObjects`. The latter does not seem to be
>>> straight-forward.
>>> Perhaps, that needs to be done not for the `CastExpr` itself, but for all
>>> of it's `final` children.
>>>
>>> Reviewers: rjmccall, rsmith, erichkeane
>>>
>>> Reviewed By: rjmccall
>>>
>>> Subscribers: bricci, hans, cfe-commits, waddlesplash
>>>
>>> Differential Revision: https://reviews.llvm.org/D50050
>>>
>>> Added:
>>> cfe/trunk/test/CodeGenCXX/castexpr-basepathsize-threshold.cpp
>>> Modified:
>>> cfe/trunk/include/clang/AST/Expr.h
>>> cfe/trunk/include/clang/AST/ExprCXX.h
>>> cfe/trunk/include/clang/AST/ExprObjC.h
>>> cfe/trunk/include/clang/AST/Stmt.h
>>> cfe/trunk/lib/AST/Expr.cpp
>>> cfe/trunk/lib/AST/ExprCXX.cpp
>>>
>>> Modified: cfe/trunk/include/clang/AST/Expr.h
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Expr.h?rev=338489&r1=338488&r2=338489&view=diff
>>>
>>> ==
>>> --- cfe/trunk/include/clang/AST/Expr.h (original)
>>> +++ cfe/trunk/include/clang/AST/Expr.h Tue Jul 31 23:06:16 2018
>>> @@ -2787,20 +2787,26 @@ public:
>>> /// representation in the source code (ExplicitCastExpr's derived
>>> /// classes).
>>> class CastExpr : public Expr {
>>> +public:
>>> + using BasePathSizeTy = unsigned int;
>>> + static_assert(std::numeric_limits::max() >= 16384,
>>> +"[implimits] Direct and indirect base classes [16384].");
>>> +
>>> private:
>>>Stmt *Op;
>>>
>>>bool CastConsistency() const;
>>>
>>> + BasePathSizeTy *BasePathSize();
>>> +
>>>const CXXBaseSpecifier * const *path_buffer() const {
>>> return const_cast(this)->path_buffer();
>>>}
>>>CXXBaseSpecifier **path_buffer();
>>>
>>> - void setBasePathSize(unsigned basePathSize) {
>>> -CastExprBits.BasePathSize = basePathSize;
>>> -assert(CastExprBits.BasePathSize == basePathSize &&
>>> - "basePathSize doesn't fit in bits of
>>> CastExprBits.BasePathSize!");
>>> + void setBasePathSize(BasePathSizeTy basePathSize) {
>>> +assert(!path_empty() && basePathSize != 0);
>>> +*(BasePathSize()) = basePathSize;
>>>}
>>>
>>> protected:
>>> @@ -2823,7 +2829,9 @@ protected:
>>> Op(op) {
>>> CastExprBits.Kind = kind;
>>> CastExprBits.PartOfExplicitCast = false;
>>> -setBasePathSize(BasePathSize);
>>> +CastExprBits.BasePathIsEmpty = BasePathSize == 0;
>>> +if (!path_empty())
>>> + setBasePathSize(BasePathSize);
>>> assert(CastConsistency());
>>>}
>>>
>>> @@ -2831,7 +2839,9 @@ protected:
>>>CastExpr(StmtClass SC, EmptyShell Empty, unsigned BasePathSize)
>>> : Expr(SC, Empty) {
>>> CastExprBits.PartOfExplicitCast = false;
>>> -setBasePathSize(BasePathSize);
>>> +CastExprBits.BasePathIsEmpty = BasePathSize == 0;
>>> +if (!path_empty())
>>> + setBasePathSize(BasePathSize);
>>>}
>>>
>>> public:
>>> @@ -2859,8 +2869,12 @@ public:
>>>
>>>typedef CXXBaseSpecifier **path_iterator;
>>>typedef const CXXBaseSpecifier * const *path_const_iterator;
>>> - bool path_empty() const { return CastExprBits.BasePathSize == 0; }
>>> - unsigned path_size() const { return CastExprBits.BasePathSize; }
>>> + bool path_empty() const { return CastExprBits.BasePathIsEmpty; }
>>> + unsigned path_size() const {
>>> +if (path_empty())
>>> + return 0U;
>>> +return *(const_cast(this)->BasePathSize());
>>> + }
>>>path_iterator path_begin() { return path_buffer(); }
>>>path_iterator path_end() { return path_buffer() + path_size(); }
>>>path_const_iterator path_begin() const { return path_buffer(); }
>>> @@ -2908,7 +2922,12 @@ public:
>>> /// @endcode
>>> class ImplicitCastExpr final
>>> : public CastExpr,
>>> - private llvm::TrailingObjects
>>> {
>>> + private llvm::TrailingObje
r338640 - [NFC][CodeGenCXX] Use -emit-llvm-only instead of -emit-llvm and ignoring it.
Author: lebedevri Date: Wed Aug 1 14:20:58 2018 New Revision: 338640 URL: http://llvm.org/viewvc/llvm-project?rev=338640&view=rev Log: [NFC][CodeGenCXX] Use -emit-llvm-only instead of -emit-llvm and ignoring it. As pointed out by Richard Smith in post-review of r338489. Modified: cfe/trunk/test/CodeGenCXX/castexpr-basepathsize-threshold.cpp Modified: cfe/trunk/test/CodeGenCXX/castexpr-basepathsize-threshold.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/castexpr-basepathsize-threshold.cpp?rev=338640&r1=338639&r2=338640&view=diff == --- cfe/trunk/test/CodeGenCXX/castexpr-basepathsize-threshold.cpp (original) +++ cfe/trunk/test/CodeGenCXX/castexpr-basepathsize-threshold.cpp Wed Aug 1 14:20:58 2018 @@ -1,4 +1,4 @@ -// RUN: %clang_cc1 %s -emit-llvm -o - +// RUN: %clang_cc1 %s -emit-llvm-only -o - // https://bugs.llvm.org/show_bug.cgi?id=38356 // We only check that we do not crash. ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49733: [AST][3/4] Move the bit-fields from BlockDecl, LinkageSpecDecl and OMPDeclareReductionDecl into DeclContext
This revision was automatically updated to reflect the committed changes.
Closed by commit rC338639: [AST][3/4] Move the bit-fields from BlockDecl,
LinkageSpecDecl and… (authored by erichkeane, committed by ).
Repository:
rC Clang
https://reviews.llvm.org/D49733
Files:
include/clang/AST/Decl.h
include/clang/AST/DeclCXX.h
include/clang/AST/DeclOpenMP.h
lib/AST/Decl.cpp
lib/AST/DeclCXX.cpp
lib/AST/DeclOpenMP.cpp
Index: lib/AST/DeclCXX.cpp
===
--- lib/AST/DeclCXX.cpp
+++ lib/AST/DeclCXX.cpp
@@ -2466,6 +2466,15 @@
getConversionType()->isBlockPointerType();
}
+LinkageSpecDecl::LinkageSpecDecl(DeclContext *DC, SourceLocation ExternLoc,
+ SourceLocation LangLoc, LanguageIDs lang,
+ bool HasBraces)
+: Decl(LinkageSpec, DC, LangLoc), DeclContext(LinkageSpec),
+ ExternLoc(ExternLoc), RBraceLoc(SourceLocation()) {
+ setLanguage(lang);
+ LinkageSpecDeclBits.HasBraces = HasBraces;
+}
+
void LinkageSpecDecl::anchor() {}
LinkageSpecDecl *LinkageSpecDecl::Create(ASTContext &C,
Index: lib/AST/DeclOpenMP.cpp
===
--- lib/AST/DeclOpenMP.cpp
+++ lib/AST/DeclOpenMP.cpp
@@ -57,6 +57,14 @@
// OMPDeclareReductionDecl Implementation.
//===--===//
+OMPDeclareReductionDecl::OMPDeclareReductionDecl(
+Kind DK, DeclContext *DC, SourceLocation L, DeclarationName Name,
+QualType Ty, OMPDeclareReductionDecl *PrevDeclInScope)
+: ValueDecl(DK, DC, L, Name, Ty), DeclContext(DK), Combiner(nullptr),
+ PrevDeclInScope(PrevDeclInScope) {
+ setInitializer(nullptr, CallInit);
+}
+
void OMPDeclareReductionDecl::anchor() {}
OMPDeclareReductionDecl *OMPDeclareReductionDecl::Create(
Index: lib/AST/Decl.cpp
===
--- lib/AST/Decl.cpp
+++ lib/AST/Decl.cpp
@@ -4215,6 +4215,15 @@
// BlockDecl Implementation
//===--===//
+BlockDecl::BlockDecl(DeclContext *DC, SourceLocation CaretLoc)
+: Decl(Block, DC, CaretLoc), DeclContext(Block) {
+ setIsVariadic(false);
+ setCapturesCXXThis(false);
+ setBlockMissingReturnType(true);
+ setIsConversionFromLambda(false);
+ setDoesNotEscape(false);
+}
+
void BlockDecl::setParams(ArrayRef NewParamInfo) {
assert(!ParamInfo && "Already has param info!");
@@ -4228,7 +4237,7 @@
void BlockDecl::setCaptures(ASTContext &Context, ArrayRef Captures,
bool CapturesCXXThis) {
- this->CapturesCXXThis = CapturesCXXThis;
+ this->setCapturesCXXThis(CapturesCXXThis);
this->NumCaptures = Captures.size();
if (Captures.empty()) {
Index: include/clang/AST/DeclOpenMP.h
===
--- include/clang/AST/DeclOpenMP.h
+++ include/clang/AST/DeclOpenMP.h
@@ -100,6 +100,8 @@
///
/// Here 'omp_out += omp_in' is a combiner and 'omp_priv = 0' is an initializer.
class OMPDeclareReductionDecl final : public ValueDecl, public DeclContext {
+ // This class stores some data in DeclContext::OMPDeclareReductionDeclBits
+ // to save some space. Use the provided accessors to access it.
public:
enum InitKind {
CallInit, // Initialized by function call.
@@ -113,8 +115,6 @@
Expr *Combiner;
/// Initializer for declare reduction construct.
Expr *Initializer;
- /// Kind of initializer - function call or omp_priv initializtion.
- InitKind InitializerKind = CallInit;
/// Reference to the previous declare reduction construct in the same
/// scope with the same name. Required for proper templates instantiation if
@@ -125,10 +125,7 @@
OMPDeclareReductionDecl(Kind DK, DeclContext *DC, SourceLocation L,
DeclarationName Name, QualType Ty,
- OMPDeclareReductionDecl *PrevDeclInScope)
- : ValueDecl(DK, DC, L, Name, Ty), DeclContext(DK), Combiner(nullptr),
-Initializer(nullptr), InitializerKind(CallInit),
-PrevDeclInScope(PrevDeclInScope) {}
+ OMPDeclareReductionDecl *PrevDeclInScope);
void setPrevDeclInScope(OMPDeclareReductionDecl *Prev) {
PrevDeclInScope = Prev;
@@ -154,11 +151,13 @@
Expr *getInitializer() { return Initializer; }
const Expr *getInitializer() const { return Initializer; }
/// Get initializer kind.
- InitKind getInitializerKind() const { return InitializerKind; }
+ InitKind getInitializerKind() const {
+return static_cast(OMPDeclareReductionDeclBits.InitializerKind);
+ }
/// Set initializer expression for the declare reduction construct.
void setInitializer(Expr *E, InitKind IK) {
Initializer = E;
-InitializerKind = IK;
+OMPDeclareReductionDeclBits.InitializerKind = IK;
}
/// Get reference to previous declar
r338639 - [AST][3/4] Move the bit-fields from BlockDecl, LinkageSpecDecl and OMPDeclareReductionDecl into DeclContext
Author: erichkeane
Date: Wed Aug 1 14:16:54 2018
New Revision: 338639
URL: http://llvm.org/viewvc/llvm-project?rev=338639&view=rev
Log:
[AST][3/4] Move the bit-fields from BlockDecl, LinkageSpecDecl and
OMPDeclareReductionDecl into DeclContext
This patch follows https://reviews.llvm.org/D49729
and https://reviews.llvm.org/D49732, and is
followed by https://reviews.llvm.org/D49734.
Move the bits from BlockDecl, LinkageSpecDecl and
OMPDeclareReductionDecl into DeclContext.
Differential Revision: https://reviews.llvm.org/D49733
Patch By: bricci
Modified:
cfe/trunk/include/clang/AST/Decl.h
cfe/trunk/include/clang/AST/DeclCXX.h
cfe/trunk/include/clang/AST/DeclOpenMP.h
cfe/trunk/lib/AST/Decl.cpp
cfe/trunk/lib/AST/DeclCXX.cpp
cfe/trunk/lib/AST/DeclOpenMP.cpp
Modified: cfe/trunk/include/clang/AST/Decl.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Decl.h?rev=338639&r1=338638&r2=338639&view=diff
==
--- cfe/trunk/include/clang/AST/Decl.h (original)
+++ cfe/trunk/include/clang/AST/Decl.h Wed Aug 1 14:16:54 2018
@@ -3823,6 +3823,8 @@ public:
/// unnamed FunctionDecl. For example:
/// ^{ statement-body } or ^(int arg1, float arg2){ statement-body }
class BlockDecl : public Decl, public DeclContext {
+ // This class stores some data in DeclContext::BlockDeclBits
+ // to save some space. Use the provided accessors to access it.
public:
/// A class which contains all the information about a particular
/// captured value.
@@ -3863,16 +3865,6 @@ public:
};
private:
- // FIXME: This can be packed into the bitfields in Decl.
- bool IsVariadic : 1;
- bool CapturesCXXThis : 1;
- bool BlockMissingReturnType : 1;
- bool IsConversionFromLambda : 1;
-
- /// A bit that indicates this block is passed directly to a function as a
- /// non-escaping parameter.
- bool DoesNotEscape : 1;
-
/// A new[]'d array of pointers to ParmVarDecls for the formal
/// parameters of this function. This is null if a prototype or if there are
/// no formals.
@@ -3889,10 +3881,7 @@ private:
Decl *ManglingContextDecl = nullptr;
protected:
- BlockDecl(DeclContext *DC, SourceLocation CaretLoc)
- : Decl(Block, DC, CaretLoc), DeclContext(Block), IsVariadic(false),
-CapturesCXXThis(false), BlockMissingReturnType(true),
-IsConversionFromLambda(false), DoesNotEscape(false) {}
+ BlockDecl(DeclContext *DC, SourceLocation CaretLoc);
public:
static BlockDecl *Create(ASTContext &C, DeclContext *DC, SourceLocation L);
@@ -3900,8 +3889,8 @@ public:
SourceLocation getCaretLocation() const { return getLocation(); }
- bool isVariadic() const { return IsVariadic; }
- void setIsVariadic(bool value) { IsVariadic = value; }
+ bool isVariadic() const { return BlockDeclBits.IsVariadic; }
+ void setIsVariadic(bool value) { BlockDeclBits.IsVariadic = value; }
CompoundStmt *getCompoundBody() const { return (CompoundStmt*) Body; }
Stmt *getBody() const override { return (Stmt*) Body; }
@@ -3944,7 +3933,7 @@ public:
/// True if this block (or its nested blocks) captures
/// anything of local storage from its enclosing scopes.
- bool hasCaptures() const { return NumCaptures != 0 || CapturesCXXThis; }
+ bool hasCaptures() const { return NumCaptures || capturesCXXThis(); }
/// Returns the number of captured variables.
/// Does not include an entry for 'this'.
@@ -3957,15 +3946,27 @@ public:
capture_const_iterator capture_begin() const { return captures().begin(); }
capture_const_iterator capture_end() const { return captures().end(); }
- bool capturesCXXThis() const { return CapturesCXXThis; }
- bool blockMissingReturnType() const { return BlockMissingReturnType; }
- void setBlockMissingReturnType(bool val) { BlockMissingReturnType = val; }
+ bool capturesCXXThis() const { return BlockDeclBits.CapturesCXXThis; }
+ void setCapturesCXXThis(bool B = true) { BlockDeclBits.CapturesCXXThis = B; }
+
+ bool blockMissingReturnType() const {
+return BlockDeclBits.BlockMissingReturnType;
+ }
+
+ void setBlockMissingReturnType(bool val = true) {
+BlockDeclBits.BlockMissingReturnType = val;
+ }
+
+ bool isConversionFromLambda() const {
+return BlockDeclBits.IsConversionFromLambda;
+ }
- bool isConversionFromLambda() const { return IsConversionFromLambda; }
- void setIsConversionFromLambda(bool val) { IsConversionFromLambda = val; }
+ void setIsConversionFromLambda(bool val = true) {
+BlockDeclBits.IsConversionFromLambda = val;
+ }
- bool doesNotEscape() const { return DoesNotEscape; }
- void setDoesNotEscape() { DoesNotEscape = true; }
+ bool doesNotEscape() const { return BlockDeclBits.DoesNotEscape; }
+ void setDoesNotEscape(bool B = true) { BlockDeclBits.DoesNotEscape = B; }
bool capturesVariable(const VarDecl *var) const;
Modified: cfe/trunk/include/clang/AST/DeclCXX.h
URL:
http://llvm.org/viewvc/
[PATCH] D50055: Update the coding standard about NFC changes and whitespace
hfinkel added inline comments. Comment at: docs/CodingStandards.rst:512 +Do not commit changes that include trailing whitespace. Some common editors will +automatically remove trailing whitespace when saving a file which causes This statement is confusing (mostly because it has two reasonable interpretations and I think you actually mean both). We should say two separate things: 1. As a coding guideline, make sure that lines don't have trailing whitespace. 2. If such whitespace exists, don't remove it unless you're otherwise changing that line of code (and here we can caution people about their editors). Comment at: docs/DeveloperPolicy.rst:395-408 +Commits with No Functional Change +- + +It may be permissible to commit changes without prior review when the changes +have no semantic impact on the code if the changes being made are obvious and +not invasive. For instance, removing trailing whitespace from a line, fixing a +line ending to be consistent with the rest of the file, fixing a typo, code aaron.ballman wrote: > chandlerc wrote: > > I think this is a much broader statement than is warranted to address the > > specific problem. And I'm not completely comfortable with it. > > > > I don't think guidance around "no functional change" is the right way to > > give guidance about what is or isn't "obvious" and fine to commit with > > post-commit review personally. > > > > I'd really suggest just being direct about *formatting* and whitespace > > changes, and specifically suggest that they not be made unless the file or > > code region in question is an area that the author is planning subsequent > > changes to. > We talk about formatting and whitespace in the CodingStandards document, but > we talk about obviousness and post-commit review in DeveloperPolicy. Where > would you like these new words to live? To me, they're more about the policy > and less about the coding standard -- the coding standard says what the code > should look like and the policy says what you should and should not do > procedurally, but then I feel the need to tie it back to "obviousness". How > about this in the developer policy: > ``` > The Obviousness of Formatting Changes > - > > While formatting and whitespace changes may be "obvious", they can also create > needless churn that causes difficulties for out-of-tree users carrying local > patches. Do not commit formatting or whitespace changes unless they are > related > to a file or code region that you intend to make subsequent changes to. The > formatting and whitespace changes should be highly localized, committed before > you begin functionality-changing work on the code region, and the commit > message > should clearly state that the commit is not intended to change functionality, > usually by stating it is :ref:`NFC `. > > > If you wish to make a change to formatting or whitespace that touches an > entire > library or code base, please seek pre-commit approval first. > ``` I agree with @chandlerc about the current proposed text, and I think that this is better. I wonder if we want to insist on separating the commits, of if, combined localized commits are okay? https://reviews.llvm.org/D50055 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49732: [AST][2/4] Move the bit-fields from FunctionDecl and CXXConstructorDecl into DeclContext
This revision was automatically updated to reflect the committed changes.
Closed by commit rC338636: [AST][2/4] Move the bit-fields from FunctionDecl and
CXXConstructorDecl into… (authored by erichkeane, committed by ).
Repository:
rC Clang
https://reviews.llvm.org/D49732
Files:
include/clang/AST/Decl.h
include/clang/AST/DeclCXX.h
lib/AST/Decl.cpp
lib/AST/DeclCXX.cpp
lib/Serialization/ASTReaderDecl.cpp
lib/Serialization/ASTWriterDecl.cpp
Index: include/clang/AST/DeclCXX.h
===
--- include/clang/AST/DeclCXX.h
+++ include/clang/AST/DeclCXX.h
@@ -1999,7 +1999,8 @@
SC_None, false, false) {
if (EndLocation.isValid())
setRangeEnd(EndLocation);
-IsExplicitSpecified = IsExplicit;
+setExplicitSpecified(IsExplicit);
+setIsCopyDeductionCandidate(false);
}
public:
@@ -2015,21 +2016,20 @@
static CXXDeductionGuideDecl *CreateDeserialized(ASTContext &C, unsigned ID);
/// Whether this deduction guide is explicit.
- bool isExplicit() const { return IsExplicitSpecified; }
-
- /// Whether this deduction guide was declared with the 'explicit' specifier.
- bool isExplicitSpecified() const { return IsExplicitSpecified; }
+ bool isExplicit() const { return isExplicitSpecified(); }
/// Get the template for which this guide performs deduction.
TemplateDecl *getDeducedTemplate() const {
return getDeclName().getCXXDeductionGuideTemplate();
}
- void setIsCopyDeductionCandidate() {
-IsCopyDeductionCandidate = true;
+ void setIsCopyDeductionCandidate(bool isCDC = true) {
+FunctionDeclBits.IsCopyDeductionCandidate = isCDC;
}
- bool isCopyDeductionCandidate() const { return IsCopyDeductionCandidate; }
+ bool isCopyDeductionCandidate() const {
+return FunctionDeclBits.IsCopyDeductionCandidate;
+ }
// Implement isa/cast/dyncast/etc.
static bool classof(const Decl *D) { return classofKind(D->getKind()); }
@@ -2475,31 +2475,20 @@
class CXXConstructorDecl final
: public CXXMethodDecl,
private llvm::TrailingObjects {
+ // This class stores some data in DeclContext::CXXConstructorDeclBits
+ // to save some space. Use the provided accessors to access it.
+
/// \name Support for base and member initializers.
/// \{
/// The arguments used to initialize the base or member.
LazyCXXCtorInitializersPtr CtorInitializers;
- unsigned NumCtorInitializers : 31;
- /// \}
-
- /// Whether this constructor declaration is an implicitly-declared
- /// inheriting constructor.
- unsigned IsInheritingConstructor : 1;
CXXConstructorDecl(ASTContext &C, CXXRecordDecl *RD, SourceLocation StartLoc,
const DeclarationNameInfo &NameInfo,
QualType T, TypeSourceInfo *TInfo,
bool isExplicitSpecified, bool isInline,
bool isImplicitlyDeclared, bool isConstexpr,
- InheritedConstructor Inherited)
-: CXXMethodDecl(CXXConstructor, C, RD, StartLoc, NameInfo, T, TInfo,
-SC_None, isInline, isConstexpr, SourceLocation()),
- NumCtorInitializers(0), IsInheritingConstructor((bool)Inherited) {
-setImplicit(isImplicitlyDeclared);
-if (Inherited)
- *getTrailingObjects() = Inherited;
-IsExplicitSpecified = isExplicitSpecified;
- }
+ InheritedConstructor Inherited);
void anchor() override;
@@ -2542,12 +2531,12 @@
/// Retrieve an iterator past the last initializer.
init_iterator init_end() {
-return init_begin() + NumCtorInitializers;
+return init_begin() + getNumCtorInitializers();
}
/// Retrieve an iterator past the last initializer.
init_const_iterator init_end() const {
-return init_begin() + NumCtorInitializers;
+return init_begin() + getNumCtorInitializers();
}
using init_reverse_iterator = std::reverse_iterator;
@@ -2571,20 +2560,22 @@
/// Determine the number of arguments used to initialize the member
/// or base.
unsigned getNumCtorInitializers() const {
- return NumCtorInitializers;
+ return CXXConstructorDeclBits.NumCtorInitializers;
}
void setNumCtorInitializers(unsigned numCtorInitializers) {
-NumCtorInitializers = numCtorInitializers;
+CXXConstructorDeclBits.NumCtorInitializers = numCtorInitializers;
+// This assert added because NumCtorInitializers is stored
+// in CXXConstructorDeclBits as a bitfield and its width has
+// been shrunk from 32 bits to fit into CXXConstructorDeclBitfields.
+assert(CXXConstructorDeclBits.NumCtorInitializers ==
+ numCtorInitializers && "NumCtorInitializers overflow!");
}
void setCtorInitializers(CXXCtorInitializer **Initializers) {
CtorInitializers = Initializers;
}
- /// Whether this function is marked as explicit explicitly.
- bool isExplicitSpecified() const { return IsExplicitSpecified; }
-
/// Whether th
r338636 - [AST][2/4] Move the bit-fields from FunctionDecl and CXXConstructorDecl into DeclContext
Author: erichkeane
Date: Wed Aug 1 14:02:40 2018
New Revision: 338636
URL: http://llvm.org/viewvc/llvm-project?rev=338636&view=rev
Log:
[AST][2/4] Move the bit-fields from FunctionDecl and CXXConstructorDecl into
DeclContext
This patch follows https://reviews.llvm.org/D49729
and is followed by https://reviews.llvm.org/D49733
and https://reviews.llvm.org/D49734.
Move the bits from FunctionDecl and CXXConstructorDecl
into DeclContext.
Differential Revision: https://reviews.llvm.org/D49732
Patch By: bricci
Modified:
cfe/trunk/include/clang/AST/Decl.h
cfe/trunk/include/clang/AST/DeclCXX.h
cfe/trunk/lib/AST/Decl.cpp
cfe/trunk/lib/AST/DeclCXX.cpp
cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
cfe/trunk/lib/Serialization/ASTWriterDecl.cpp
Modified: cfe/trunk/include/clang/AST/Decl.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Decl.h?rev=338636&r1=338635&r2=338636&view=diff
==
--- cfe/trunk/include/clang/AST/Decl.h (original)
+++ cfe/trunk/include/clang/AST/Decl.h Wed Aug 1 14:02:40 2018
@@ -1711,8 +1711,11 @@ private:
/// contains all of the information known about the function. Other,
/// previous declarations of the function are available via the
/// getPreviousDecl() chain.
-class FunctionDecl : public DeclaratorDecl, public DeclContext,
+class FunctionDecl : public DeclaratorDecl,
+ public DeclContext,
public Redeclarable {
+ // This class stores some data in DeclContext::FunctionDeclBits
+ // to save some space. Use the provided accessors to access it.
public:
/// The kind of templated function a FunctionDecl can be.
enum TemplatedKind {
@@ -1731,64 +1734,6 @@ private:
LazyDeclStmtPtr Body;
- // FIXME: This can be packed into the bitfields in DeclContext.
- // NOTE: VC++ packs bitfields poorly if the types differ.
- unsigned SClass : 3;
- unsigned IsInline : 1;
- unsigned IsInlineSpecified : 1;
-
-protected:
- // This is shared by CXXConstructorDecl, CXXConversionDecl, and
- // CXXDeductionGuideDecl.
- unsigned IsExplicitSpecified : 1;
-
-private:
- unsigned IsVirtualAsWritten : 1;
- unsigned IsPure : 1;
- unsigned HasInheritedPrototype : 1;
- unsigned HasWrittenPrototype : 1;
- unsigned IsDeleted : 1;
- unsigned IsTrivial : 1; // sunk from CXXMethodDecl
-
- /// This flag indicates whether this function is trivial for the purpose of
- /// calls. This is meaningful only when this function is a copy/move
- /// constructor or a destructor.
- unsigned IsTrivialForCall : 1;
-
- unsigned IsDefaulted : 1; // sunk from CXXMethoDecl
- unsigned IsExplicitlyDefaulted : 1; //sunk from CXXMethodDecl
- unsigned HasImplicitReturnZero : 1;
- unsigned IsLateTemplateParsed : 1;
- unsigned IsConstexpr : 1;
- unsigned InstantiationIsPending : 1;
-
- /// Indicates if the function uses __try.
- unsigned UsesSEHTry : 1;
-
- /// Indicates if the function was a definition but its body was
- /// skipped.
- unsigned HasSkippedBody : 1;
-
- /// Indicates if the function declaration will have a body, once we're done
- /// parsing it.
- unsigned WillHaveBody : 1;
-
- /// Indicates that this function is a multiversioned function using attribute
- /// 'target'.
- unsigned IsMultiVersion : 1;
-
-protected:
- /// [C++17] Only used by CXXDeductionGuideDecl. Declared here to avoid
- /// increasing the size of CXXDeductionGuideDecl by the size of an unsigned
- /// int as opposed to adding a single bit to FunctionDecl.
- /// Indicates that the Deduction Guide is the implicitly generated 'copy
- /// deduction candidate' (is used during overload resolution).
- unsigned IsCopyDeductionCandidate : 1;
-
-private:
-
- /// Store the ODRHash after first calculation.
- unsigned HasODRHash : 1;
unsigned ODRHash;
/// End part of this FunctionDecl's source range.
@@ -1858,25 +1803,22 @@ private:
void setParams(ASTContext &C, ArrayRef NewParamInfo);
+ // This is unfortunately needed because ASTDeclWriter::VisitFunctionDecl
+ // need to access this bit but we want to avoid making ASTDeclWriter
+ // a friend of FunctionDeclBitfields just for this.
+ bool isDeletedBit() const { return FunctionDeclBits.IsDeleted; }
+
+ /// Whether an ODRHash has been stored.
+ bool hasODRHash() const { return FunctionDeclBits.HasODRHash; }
+
+ /// State that an ODRHash has been stored.
+ void setHasODRHash(bool B = true) { FunctionDeclBits.HasODRHash = B; }
+
protected:
FunctionDecl(Kind DK, ASTContext &C, DeclContext *DC, SourceLocation
StartLoc,
const DeclarationNameInfo &NameInfo, QualType T,
TypeSourceInfo *TInfo, StorageClass S, bool isInlineSpecified,
- bool isConstexprSpecified)
- : DeclaratorDecl(DK, DC, NameInfo.getLoc(), NameInfo.getName(), T, TInfo,
- StartLoc),
-DeclContext(DK), redeclarable_base(C), SClass(S),
-IsInl
[PATCH] D48903: [VirtualFileSystem] InMemoryFileSystem::status: Return a Status with the requested name
simark updated this revision to Diff 158625.
simark added a comment.
Add -Wno-nonportable-include-path to test/PCH/case-insensitive-include.c
Repository:
rC Clang
https://reviews.llvm.org/D48903
Files:
lib/Basic/FileManager.cpp
lib/Basic/VirtualFileSystem.cpp
test/PCH/case-insensitive-include.c
unittests/Basic/VirtualFileSystemTest.cpp
unittests/Driver/ToolChainTest.cpp
Index: unittests/Driver/ToolChainTest.cpp
===
--- unittests/Driver/ToolChainTest.cpp
+++ unittests/Driver/ToolChainTest.cpp
@@ -113,7 +113,7 @@
std::replace(S.begin(), S.end(), '\\', '/');
#endif
EXPECT_EQ("Found candidate GCC installation: "
-"/home/test/lib/gcc/arm-linux-gnueabi/4.6.1\n"
+"/home/test/bin/../lib/gcc/arm-linux-gnueabi/4.6.1\n"
"Selected GCC installation: "
"/home/test/bin/../lib/gcc/arm-linux-gnueabi/4.6.1\n"
"Candidate multilib: .;@m32\n"
Index: unittests/Basic/VirtualFileSystemTest.cpp
===
--- unittests/Basic/VirtualFileSystemTest.cpp
+++ unittests/Basic/VirtualFileSystemTest.cpp
@@ -13,6 +13,7 @@
#include "llvm/Support/Errc.h"
#include "llvm/Support/Host.h"
#include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/Path.h"
#include "llvm/Support/SourceMgr.h"
#include "gtest/gtest.h"
#include
@@ -151,6 +152,13 @@
addEntry(Path, S);
}
};
+
+/// Replace back-slashes by front-slashes.
+std::string getPosixPath(std::string S) {
+ SmallString<128> Result;
+ llvm::sys::path::native(S, Result, llvm::sys::path::Style::posix);
+ return Result.str();
+};
} // end anonymous namespace
TEST(VirtualFileSystemTest, StatusQueries) {
@@ -782,7 +790,9 @@
I = FS.dir_begin("/b", EC);
ASSERT_FALSE(EC);
- ASSERT_EQ("/b/c", I->getName());
+ // When on Windows, we end up with "/b\\c" as the name. Convert to Posix
+ // path for the sake of the comparison.
+ ASSERT_EQ("/b/c", getPosixPath(I->getName()));
I.increment(EC);
ASSERT_FALSE(EC);
ASSERT_EQ(vfs::directory_iterator(), I);
@@ -794,23 +804,19 @@
auto Stat = FS.status("/b/c");
ASSERT_FALSE(Stat.getError()) << Stat.getError() << "\n" << FS.toString();
- ASSERT_EQ("c", Stat->getName());
+ ASSERT_EQ("/b/c", Stat->getName());
ASSERT_EQ("/b", *FS.getCurrentWorkingDirectory());
Stat = FS.status("c");
ASSERT_FALSE(Stat.getError()) << Stat.getError() << "\n" << FS.toString();
- auto ReplaceBackslashes = [](std::string S) {
-std::replace(S.begin(), S.end(), '\\', '/');
-return S;
- };
NormalizedFS.setCurrentWorkingDirectory("/b/c");
NormalizedFS.setCurrentWorkingDirectory(".");
- ASSERT_EQ("/b/c", ReplaceBackslashes(
-NormalizedFS.getCurrentWorkingDirectory().get()));
+ ASSERT_EQ("/b/c",
+getPosixPath(NormalizedFS.getCurrentWorkingDirectory().get()));
NormalizedFS.setCurrentWorkingDirectory("..");
- ASSERT_EQ("/b", ReplaceBackslashes(
- NormalizedFS.getCurrentWorkingDirectory().get()));
+ ASSERT_EQ("/b",
+getPosixPath(NormalizedFS.getCurrentWorkingDirectory().get()));
}
#if !defined(_WIN32)
@@ -919,6 +925,39 @@
ASSERT_TRUE(Stat->isRegularFile());
}
+// Test that the name returned by status() is in the same form as the path that
+// was requested (to match the behavior of RealFileSystem).
+TEST_F(InMemoryFileSystemTest, StatusName) {
+ NormalizedFS.addFile("/a/b/c", 0, MemoryBuffer::getMemBuffer("abc"),
+ /*User=*/None,
+ /*Group=*/None, sys::fs::file_type::regular_file);
+ NormalizedFS.setCurrentWorkingDirectory("/a/b");
+
+ // Access using InMemoryFileSystem::status.
+ auto Stat = NormalizedFS.status("../b/c");
+ ASSERT_FALSE(Stat.getError()) << Stat.getError() << "\n"
+<< NormalizedFS.toString();
+ ASSERT_TRUE(Stat->isRegularFile());
+ ASSERT_EQ("../b/c", Stat->getName());
+
+ // Access using InMemoryFileAdaptor::status.
+ auto File = NormalizedFS.openFileForRead("../b/c");
+ ASSERT_FALSE(File.getError()) << File.getError() << "\n"
+<< NormalizedFS.toString();
+ Stat = (*File)->status();
+ ASSERT_FALSE(Stat.getError()) << Stat.getError() << "\n"
+<< NormalizedFS.toString();
+ ASSERT_TRUE(Stat->isRegularFile());
+ ASSERT_EQ("../b/c", Stat->getName());
+
+ // Access using a directory iterator.
+ std::error_code EC;
+ clang::vfs::directory_iterator It = NormalizedFS.dir_begin("../b", EC);
+ // When on Windows, we end up with "../b\\c" as the name. Convert to Posix
+ // path for the sake of the comparison.
+ ASSERT_EQ("../b/c", getPosixPath(It->getName()));
+}
+
// NOTE: in the tests below, we use '//root/' as our root directory, since it is
// a legal *absolute* path on Windows as well as *nix.
class VFSFromYAMLTest : public ::testing::Test {
Index: test/PCH/c
[PATCH] D49729: [AST][1/4] Move the bit-fields from TagDecl, EnumDecl and RecordDecl into DeclContext
This revision was automatically updated to reflect the committed changes.
Closed by commit rC338630: [AST][1/4] Move the bit-fields from TagDecl,
EnumDecl and RecordDecl into… (authored by erichkeane, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D49729?vs=158498&id=158624#toc
Repository:
rC Clang
https://reviews.llvm.org/D49729
Files:
include/clang/AST/Decl.h
include/clang/AST/DeclBase.h
lib/AST/Decl.cpp
lib/AST/DeclBase.cpp
lib/AST/DeclCXX.cpp
lib/AST/DeclTemplate.cpp
lib/Serialization/ASTReaderDecl.cpp
lib/Serialization/ASTWriter.cpp
lib/Serialization/ASTWriterDecl.cpp
Index: include/clang/AST/DeclBase.h
===
--- include/clang/AST/DeclBase.h
+++ include/clang/AST/DeclBase.h
@@ -1260,37 +1260,407 @@
/// BlockDecl
/// OMPDeclareReductionDecl
class DeclContext {
- /// DeclKind - This indicates which class this is.
- unsigned DeclKind : 8;
+ // We use uint64_t in the bit-fields below since some bit-fields
+ // cross the unsigned boundary and this breaks the packing.
- /// Whether this declaration context also has some external
- /// storage that contains additional declarations that are lexically
- /// part of this context.
- mutable bool ExternalLexicalStorage : 1;
-
- /// Whether this declaration context also has some external
- /// storage that contains additional declarations that are visible
- /// in this context.
- mutable bool ExternalVisibleStorage : 1;
+ /// Stores the bits used by DeclContext.
+ /// If modified NumDeclContextBit, the ctor of DeclContext and the accessor
+ /// methods in DeclContext should be updated appropriately.
+ class DeclContextBitfields {
+friend class DeclContext;
+/// DeclKind - This indicates which class this is.
+uint64_t DeclKind : 7;
+
+/// Whether this declaration context also has some external
+/// storage that contains additional declarations that are lexically
+/// part of this context.
+mutable uint64_t ExternalLexicalStorage : 1;
+
+/// Whether this declaration context also has some external
+/// storage that contains additional declarations that are visible
+/// in this context.
+mutable uint64_t ExternalVisibleStorage : 1;
+
+/// Whether this declaration context has had externally visible
+/// storage added since the last lookup. In this case, \c LookupPtr's
+/// invariant may not hold and needs to be fixed before we perform
+/// another lookup.
+mutable uint64_t NeedToReconcileExternalVisibleStorage : 1;
+
+/// If \c true, this context may have local lexical declarations
+/// that are missing from the lookup table.
+mutable uint64_t HasLazyLocalLexicalLookups : 1;
+
+/// If \c true, the external source may have lexical declarations
+/// that are missing from the lookup table.
+mutable uint64_t HasLazyExternalLexicalLookups : 1;
+
+/// If \c true, lookups should only return identifier from
+/// DeclContext scope (for example TranslationUnit). Used in
+/// LookupQualifiedName()
+mutable uint64_t UseQualifiedLookup : 1;
+ };
- /// Whether this declaration context has had external visible
- /// storage added since the last lookup. In this case, \c LookupPtr's
- /// invariant may not hold and needs to be fixed before we perform
- /// another lookup.
- mutable bool NeedToReconcileExternalVisibleStorage : 1;
+ /// Number of bits in DeclContextBitfields.
+ enum { NumDeclContextBits = 13 };
- /// If \c true, this context may have local lexical declarations
- /// that are missing from the lookup table.
- mutable bool HasLazyLocalLexicalLookups : 1;
+ /// Stores the bits used by TagDecl.
+ /// If modified NumTagDeclBits and the accessor
+ /// methods in TagDecl should be updated appropriately.
+ class TagDeclBitfields {
+friend class TagDecl;
+/// For the bits in DeclContextBitfields
+uint64_t : NumDeclContextBits;
+
+/// The TagKind enum.
+uint64_t TagDeclKind : 3;
+
+/// True if this is a definition ("struct foo {};"), false if it is a
+/// declaration ("struct foo;"). It is not considered a definition
+/// until the definition has been fully processed.
+uint64_t IsCompleteDefinition : 1;
+
+/// True if this is currently being defined.
+uint64_t IsBeingDefined : 1;
+
+/// True if this tag declaration is "embedded" (i.e., defined or declared
+/// for the very first time) in the syntax of a declarator.
+uint64_t IsEmbeddedInDeclarator : 1;
+
+/// True if this tag is free standing, e.g. "struct foo;".
+uint64_t IsFreeStanding : 1;
+
+/// Indicates whether it is possible for declarations of this kind
+/// to have an out-of-date definition.
+///
+/// This option is only enabled when modules are enabled.
+uint64_t MayHaveOutOfDateDef : 1;
+
+/// Has the full definition of this type been required by a use somewhere in
+/// the TU.
r338630 - [AST][1/4] Move the bit-fields from TagDecl, EnumDecl and RecordDecl into DeclContext
Author: erichkeane
Date: Wed Aug 1 13:48:16 2018
New Revision: 338630
URL: http://llvm.org/viewvc/llvm-project?rev=338630&view=rev
Log:
[AST][1/4] Move the bit-fields from TagDecl, EnumDecl and RecordDecl into
DeclContext
DeclContext has a little less than 8 bytes free due to the alignment
requirements on 64 bits archs. This set of patches moves the
bit-fields from classes deriving from DeclContext into DeclContext.
On 32 bits archs this increases the size of DeclContext by 4 bytes
but this is balanced by an equal or larger reduction in the size
of the classes deriving from it.
On 64 bits archs the size of DeclContext stays the same but
most of the classes deriving from it shrink by 8/16 bytes.
(-print-stats diff here https://reviews.llvm.org/D49728)
When doing an -fsyntax-only on all of Boost this result
in a 3.6% reduction in the size of all Decls and
a 1% reduction in the run time due to the lower cache
miss rate.
For now CXXRecordDecl is not touched but there is
an easy 6 (if I count correctly) bytes gain available there
by moving some bits from DefinitionData into the free
space of DeclContext. This will be the subject of another patch.
This patch sequence also enable the possibility of refactoring
FunctionDecl: To save space some bits from classes deriving from
FunctionDecl were moved to FunctionDecl. This resulted in a
lot of stuff in FunctionDecl which do not belong logically to it.
After this set of patches however it is just a simple matter of
adding a SomethingDeclBitfields in DeclContext and moving the
bits to it from FunctionDecl.
This first patch introduces the anonymous union in DeclContext
and all the *DeclBitfields classes holding the bit-fields, and moves
the bits from TagDecl, EnumDecl and RecordDecl into DeclContext.
This patch is followed by https://reviews.llvm.org/D49732,
https://reviews.llvm.org/D49733 and https://reviews.llvm.org/D49734.
Differential Revision: https://reviews.llvm.org/D49729
Patch By: bricci
Modified:
cfe/trunk/include/clang/AST/Decl.h
cfe/trunk/include/clang/AST/DeclBase.h
cfe/trunk/lib/AST/Decl.cpp
cfe/trunk/lib/AST/DeclBase.cpp
cfe/trunk/lib/AST/DeclCXX.cpp
cfe/trunk/lib/AST/DeclTemplate.cpp
cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
cfe/trunk/lib/Serialization/ASTWriter.cpp
cfe/trunk/lib/Serialization/ASTWriterDecl.cpp
Modified: cfe/trunk/include/clang/AST/Decl.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Decl.h?rev=338630&r1=338629&r2=338630&view=diff
==
--- cfe/trunk/include/clang/AST/Decl.h (original)
+++ cfe/trunk/include/clang/AST/Decl.h Wed Aug 1 13:48:16 2018
@@ -3014,64 +3014,16 @@ public:
};
/// Represents the declaration of a struct/union/class/enum.
-class TagDecl
- : public TypeDecl, public DeclContext, public Redeclarable {
+class TagDecl : public TypeDecl,
+public DeclContext,
+public Redeclarable {
+ // This class stores some data in DeclContext::TagDeclBits
+ // to save some space. Use the provided accessors to access it.
public:
// This is really ugly.
using TagKind = TagTypeKind;
private:
- // FIXME: This can be packed into the bitfields in Decl.
- /// The TagKind enum.
- unsigned TagDeclKind : 3;
-
- /// True if this is a definition ("struct foo {};"), false if it is a
- /// declaration ("struct foo;"). It is not considered a definition
- /// until the definition has been fully processed.
- unsigned IsCompleteDefinition : 1;
-
-protected:
- /// True if this is currently being defined.
- unsigned IsBeingDefined : 1;
-
-private:
- /// True if this tag declaration is "embedded" (i.e., defined or declared
- /// for the very first time) in the syntax of a declarator.
- unsigned IsEmbeddedInDeclarator : 1;
-
- /// True if this tag is free standing, e.g. "struct foo;".
- unsigned IsFreeStanding : 1;
-
-protected:
- // These are used by (and only defined for) EnumDecl.
- unsigned NumPositiveBits : 8;
- unsigned NumNegativeBits : 8;
-
- /// True if this tag declaration is a scoped enumeration. Only
- /// possible in C++11 mode.
- unsigned IsScoped : 1;
-
- /// If this tag declaration is a scoped enum,
- /// then this is true if the scoped enum was declared using the class
- /// tag, false if it was declared with the struct tag. No meaning is
- /// associated if this tag declaration is not a scoped enum.
- unsigned IsScopedUsingClassTag : 1;
-
- /// True if this is an enumeration with fixed underlying type. Only
- /// possible in C++11, Microsoft extensions, or Objective C mode.
- unsigned IsFixed : 1;
-
- /// Indicates whether it is possible for declarations of this kind
- /// to have an out-of-date definition.
- ///
- /// This option is only enabled when modules are enabled.
- unsigned MayHaveOutOfDateDef : 1;
-
- /// Has the full definition of this type been required by a use somewhere in
- /// the TU.
- unsig
[PATCH] D50055: Update the coding standard about NFC changes and whitespace
aaron.ballman added inline comments. Comment at: docs/DeveloperPolicy.rst:395-408 +Commits with No Functional Change +- + +It may be permissible to commit changes without prior review when the changes +have no semantic impact on the code if the changes being made are obvious and +not invasive. For instance, removing trailing whitespace from a line, fixing a +line ending to be consistent with the rest of the file, fixing a typo, code chandlerc wrote: > I think this is a much broader statement than is warranted to address the > specific problem. And I'm not completely comfortable with it. > > I don't think guidance around "no functional change" is the right way to give > guidance about what is or isn't "obvious" and fine to commit with post-commit > review personally. > > I'd really suggest just being direct about *formatting* and whitespace > changes, and specifically suggest that they not be made unless the file or > code region in question is an area that the author is planning subsequent > changes to. We talk about formatting and whitespace in the CodingStandards document, but we talk about obviousness and post-commit review in DeveloperPolicy. Where would you like these new words to live? To me, they're more about the policy and less about the coding standard -- the coding standard says what the code should look like and the policy says what you should and should not do procedurally, but then I feel the need to tie it back to "obviousness". How about this in the developer policy: ``` The Obviousness of Formatting Changes - While formatting and whitespace changes may be "obvious", they can also create needless churn that causes difficulties for out-of-tree users carrying local patches. Do not commit formatting or whitespace changes unless they are related to a file or code region that you intend to make subsequent changes to. The formatting and whitespace changes should be highly localized, committed before you begin functionality-changing work on the code region, and the commit message should clearly state that the commit is not intended to change functionality, usually by stating it is :ref:`NFC `. If you wish to make a change to formatting or whitespace that touches an entire library or code base, please seek pre-commit approval first. ``` https://reviews.llvm.org/D50055 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r338627 - [test] Fix %hmaptool path for standalone builds
Author: mgorny
Date: Wed Aug 1 13:38:22 2018
New Revision: 338627
URL: http://llvm.org/viewvc/llvm-project?rev=338627&view=rev
Log:
[test] Fix %hmaptool path for standalone builds
Fix %hmaptool path to refer to clang_tools_dir instead of
llvm_tools_dir, in order to fix standalone builds. The tool is built
as part of clang, so it won't be found in installed LLVM tools.
Differential Revision: https://reviews.llvm.org/D50156
Modified:
cfe/trunk/test/lit.cfg.py
Modified: cfe/trunk/test/lit.cfg.py
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/lit.cfg.py?rev=338627&r1=338626&r2=338627&view=diff
==
--- cfe/trunk/test/lit.cfg.py (original)
+++ cfe/trunk/test/lit.cfg.py Wed Aug 1 13:38:22 2018
@@ -71,7 +71,7 @@ llvm_config.add_tool_substitutions(tools
config.substitutions.append(
('%hmaptool', "'%s' %s" % (config.python_executable,
- os.path.join(config.llvm_tools_dir, 'hmaptool'
+ os.path.join(config.clang_tools_dir,
'hmaptool'
# Plugins (loadable modules)
# TODO: This should be supplied by Makefile or autoconf.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50156: [test] Fix %hmaptool path for standalone builds
This revision was automatically updated to reflect the committed changes.
Closed by commit rC338627: [test] Fix %hmaptool path for standalone builds
(authored by mgorny, committed by ).
Repository:
rC Clang
https://reviews.llvm.org/D50156
Files:
test/lit.cfg.py
Index: test/lit.cfg.py
===
--- test/lit.cfg.py
+++ test/lit.cfg.py
@@ -71,7 +71,7 @@
config.substitutions.append(
('%hmaptool', "'%s' %s" % (config.python_executable,
- os.path.join(config.llvm_tools_dir, 'hmaptool'
+ os.path.join(config.clang_tools_dir,
'hmaptool'
# Plugins (loadable modules)
# TODO: This should be supplied by Makefile or autoconf.
Index: test/lit.cfg.py
===
--- test/lit.cfg.py
+++ test/lit.cfg.py
@@ -71,7 +71,7 @@
config.substitutions.append(
('%hmaptool', "'%s' %s" % (config.python_executable,
- os.path.join(config.llvm_tools_dir, 'hmaptool'
+ os.path.join(config.clang_tools_dir, 'hmaptool'
# Plugins (loadable modules)
# TODO: This should be supplied by Makefile or autoconf.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50104: [OpenCL] Always emit alloca in entry block for enqueue_kernel builtin
scott.linder updated this revision to Diff 158618.
scott.linder added a comment.
Update test
https://reviews.llvm.org/D50104
Files:
lib/CodeGen/CGBuiltin.cpp
test/CodeGenOpenCL/cl20-device-side-enqueue.cl
test/CodeGenOpenCL/enqueue-kernel-non-entry-block.cl
Index: test/CodeGenOpenCL/enqueue-kernel-non-entry-block.cl
===
--- /dev/null
+++ test/CodeGenOpenCL/enqueue-kernel-non-entry-block.cl
@@ -0,0 +1,31 @@
+// RUN: %clang_cc1 -cl-std=CL2.0 -O0 -emit-llvm -o - -triple amdgcn < %s | FileCheck %s --check-prefixes=COMMON,AMDGPU
+// RUN: %clang_cc1 -cl-std=CL2.0 -O0 -emit-llvm -o - -triple "spir-unknown-unknown" < %s | FileCheck %s --check-prefixes=COMMON,SPIR32
+// RUN: %clang_cc1 -cl-std=CL2.0 -O0 -emit-llvm -o - -triple "spir64-unknown-unknown" < %s | FileCheck %s --check-prefixes=COMMON,SPIR64
+// RUN: %clang_cc1 -cl-std=CL2.0 -O0 -debug-info-kind=limited -emit-llvm -o - -triple amdgcn < %s | FileCheck %s --check-prefixes=CHECK-DEBUG
+
+// Check that the enqueue_kernel array temporary is in the entry block to avoid
+// a dynamic alloca
+
+typedef struct {int a;} ndrange_t;
+
+kernel void test(int i) {
+// COMMON-LABEL: define {{.*}} void @test
+// COMMON-LABEL: entry:
+// AMDGPU: %block_sizes = alloca [1 x i64]
+// SPIR32: %block_sizes = alloca [1 x i32]
+// SPIR64: %block_sizes = alloca [1 x i64]
+// COMMON-LABEL: if.then:
+// COMMON-NOT: alloca
+// CHECK-DEBUG: getelementptr {{.*}} %block_sizes, {{.*}} !dbg !34
+// COMMON-LABEL: if.end
+ queue_t default_queue;
+ unsigned flags = 0;
+ ndrange_t ndrange;
+ if (i)
+enqueue_kernel(default_queue, flags, ndrange, ^(local void *a) { }, 32);
+}
+
+// Check that the temporary is scoped to the `if`
+
+// CHECK-DEBUG: !32 = distinct !DILexicalBlock(scope: !7, file: !1, line: 24)
+// CHECK-DEBUG: !34 = !DILocation(line: 25, scope: !32)
Index: test/CodeGenOpenCL/cl20-device-side-enqueue.cl
===
--- test/CodeGenOpenCL/cl20-device-side-enqueue.cl
+++ test/CodeGenOpenCL/cl20-device-side-enqueue.cl
@@ -46,8 +46,24 @@
// COMMON: %event_wait_list2 = alloca [1 x %opencl.clk_event_t*]
clk_event_t event_wait_list2[] = {clk_event};
- // Emits block literal on stack and block kernel [[INVLK1]].
// COMMON: [[NDR:%[a-z0-9]+]] = alloca %struct.ndrange_t, align 4
+
+ // B32: %[[BLOCK_SIZES1:.*]] = alloca [1 x i32]
+ // B64: %[[BLOCK_SIZES1:.*]] = alloca [1 x i64]
+ // B32: %[[BLOCK_SIZES2:.*]] = alloca [1 x i32]
+ // B64: %[[BLOCK_SIZES2:.*]] = alloca [1 x i64]
+ // B32: %[[BLOCK_SIZES3:.*]] = alloca [1 x i32]
+ // B64: %[[BLOCK_SIZES3:.*]] = alloca [1 x i64]
+ // B32: %[[BLOCK_SIZES4:.*]] = alloca [1 x i32]
+ // B64: %[[BLOCK_SIZES4:.*]] = alloca [1 x i64]
+ // B32: %[[BLOCK_SIZES5:.*]] = alloca [1 x i32]
+ // B64: %[[BLOCK_SIZES5:.*]] = alloca [1 x i64]
+ // B32: %[[BLOCK_SIZES6:.*]] = alloca [3 x i32]
+ // B64: %[[BLOCK_SIZES6:.*]] = alloca [3 x i64]
+ // B32: %[[BLOCK_SIZES7:.*]] = alloca [1 x i32]
+ // B64: %[[BLOCK_SIZES7:.*]] = alloca [1 x i64]
+
+ // Emits block literal on stack and block kernel [[INVLK1]].
// COMMON: [[DEF_Q:%[0-9]+]] = load %opencl.queue_t{{.*}}*, %opencl.queue_t{{.*}}** %default_queue
// COMMON: [[FLAGS:%[0-9]+]] = load i32, i32* %flags
// B32: [[BL:%[0-9]+]] = bitcast <{ i32, i32, i32 addrspace(1)*, i32, i32 addrspace(1)* }>* %block to void ()*
@@ -73,48 +89,44 @@
// COMMON-SAME: (%opencl.queue_t{{.*}}* [[DEF_Q]], i32 [[FLAGS]], %struct.ndrange_t* {{.*}}, i32 2, %opencl.clk_event_t{{.*}}* addrspace(4)* [[WAIT_EVNT]], %opencl.clk_event_t{{.*}}* addrspace(4)* [[EVNT]],
// COMMON-SAME: i8 addrspace(4)* addrspacecast (i8* bitcast ({{.*}} [[INVLK2:[^ ]+_kernel]] to i8*) to i8 addrspace(4)*),
// COMMON-SAME: i8 addrspace(4)* [[BL_I8]])
-
enqueue_kernel(default_queue, flags, ndrange, 2, &event_wait_list, &clk_event,
^(void) {
a[i] = b[i];
});
// Emits global block literal [[BLG1]] and block kernel [[INVGK1]].
// COMMON: [[DEF_Q:%[0-9]+]] = load %opencl.queue_t{{.*}}*, %opencl.queue_t{{.*}}** %default_queue
// COMMON: [[FLAGS:%[0-9]+]] = load i32, i32* %flags
- // B32: %[[TMP:.*]] = alloca [1 x i32]
- // B32: %[[TMP1:.*]] = getelementptr [1 x i32], [1 x i32]* %[[TMP]], i32 0, i32 0
- // B32: store i32 256, i32* %[[TMP1]], align 4
- // B64: %[[TMP:.*]] = alloca [1 x i64]
- // B64: %[[TMP1:.*]] = getelementptr [1 x i64], [1 x i64]* %[[TMP]], i32 0, i32 0
- // B64: store i64 256, i64* %[[TMP1]], align 8
+ // B32: %[[TMP:.*]] = getelementptr [1 x i32], [1 x i32]* %[[BLOCK_SIZES1]], i32 0, i32 0
+ // B32: store i32 256, i32* %[[TMP]], align 4
+ // B64: %[[TMP:.*]] = getelementptr [1 x i64], [1 x i64]* %[[BLOCK_SIZES1]], i32 0, i32 0
+ // B64: store i64 256, i64* %[[TMP]], align 8
// COMMON-LABEL: call i32 @__enqueue_kernel_varargs(
// COMMON-SAME: %opencl.queue_t{{.*}}* [[DEF_Q]], i32 [[FLAGS]]
[PATCH] D50119: Compiler support for P1144R0 "__is_trivially_relocatable(T)"
Quuxplusone updated this revision to Diff 158615.
Quuxplusone added a comment.
clang-format.
Repository:
rC Clang
https://reviews.llvm.org/D50119
Files:
compiler-explorer-llvm-commit.sh
docs/LanguageExtensions.rst
include/clang/AST/DeclCXX.h
include/clang/Basic/Attr.td
include/clang/Basic/AttrDocs.td
include/clang/Basic/DiagnosticSemaKinds.td
include/clang/Basic/Features.def
include/clang/Basic/TokenKinds.def
include/clang/Basic/TypeTraits.h
include/clang/Sema/Sema.h
lib/AST/ASTImporter.cpp
lib/AST/DeclCXX.cpp
lib/Parse/ParseDeclCXX.cpp
lib/Sema/SemaDecl.cpp
lib/Sema/SemaDeclAttr.cpp
lib/Sema/SemaDeclCXX.cpp
lib/Sema/SemaExprCXX.cpp
lib/Serialization/ASTReaderDecl.cpp
lib/Serialization/ASTWriter.cpp
test/Lexer/has_extension_cxx.cpp
test/Misc/pragma-attribute-supported-attributes-list.test
test/SemaCXX/trivially-relocatable.cpp
Index: test/SemaCXX/trivially-relocatable.cpp
===
--- /dev/null
+++ test/SemaCXX/trivially-relocatable.cpp
@@ -0,0 +1,495 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++11
+// expected-diagnostics
+
+static_assert(__has_extension(trivially_relocatable), "");
+
+// It shall appear at most once in each attribute-list
+// and no attribute-argument-clause shall be present.
+
+struct [[trivially_relocatable, trivially_relocatable]] B1 {};
+// expected-error@-1{{attribute 'trivially_relocatable' cannot appear multiple times in an attribute specifier}}
+
+struct [[trivially_relocatable]] [[trivially_relocatable]] B2 {}; // should really be an error
+
+struct [[trivially_relocatable(42)]] B3 {};
+// expected-error@-1{{attribute 'trivially_relocatable' cannot have an argument list}}
+
+
+// The first declaration of a type shall specify the
+// trivially_relocatable attribute if any declaration of that
+// type specifies the trivially_relocatable attribute.
+
+struct [[trivially_relocatable]] A1 {}; // ok
+struct [[trivially_relocatable]] A1;
+
+struct [[trivially_relocatable]] A2; // ok
+struct [[trivially_relocatable]] A2 {};
+
+struct [[trivially_relocatable]] A3 {}; // ok
+struct A3;
+
+struct [[trivially_relocatable]] A4; // ok
+struct A4 {};
+
+struct A5 {};
+struct [[trivially_relocatable]] A5;
+// expected-error@-1{{type A5 declared 'trivially_relocatable' after its first declaration}}
+// expected-note@-3{{declaration missing 'trivially_relocatable' attribute is here}}
+// expected-warning@-3{{attribute declaration must precede definition}}
+// expected-note@-5{{previous definition is here}}
+
+struct A6;
+struct [[trivially_relocatable]] A6 {};
+// expected-error@-1{{type A6 declared 'trivially_relocatable' after its first declaration}}
+// expected-note@-3{{declaration missing 'trivially_relocatable' attribute is here}}
+
+
+// If a type T is declared with the trivially_relocatable attribute, and T is either
+// not move-constructible or not destructible, the program is ill-formed.
+
+struct NonDestructible {
+NonDestructible(const NonDestructible&) = default;
+NonDestructible(NonDestructible&&) = default;
+~NonDestructible() = delete;
+};
+struct NonCopyConstructible {
+NonCopyConstructible(const NonCopyConstructible&) = delete;
+};
+struct NonMoveConstructible {
+NonMoveConstructible(const NonMoveConstructible&) = default;
+NonMoveConstructible(NonMoveConstructible&&) = delete;
+};
+static_assert(!__is_trivially_relocatable(NonDestructible), "");
+static_assert(!__is_trivially_relocatable(NonCopyConstructible), "");
+static_assert(!__is_constructible(NonCopyConstructible, NonCopyConstructible&&), "");
+static_assert(!__is_trivially_relocatable(NonMoveConstructible), "");
+static_assert(!__is_constructible(NonMoveConstructible, NonMoveConstructible&&), "");
+
+struct [[trivially_relocatable]] D1 { ~D1() = delete; };
+// expected-error@-1{{cannot be applied to struct 'D1' because it is not destructible}}
+
+struct [[trivially_relocatable]] D2 : private NonDestructible { };
+// expected-error@-1{{cannot be applied to struct 'D2' because it is not destructible}}
+
+struct [[trivially_relocatable]] D3 { D3(const D3&) = delete; };
+// expected-error@-1{{cannot be applied to struct 'D3' because it is not move-constructible}}
+
+struct [[trivially_relocatable]] D4 { D4(const D4&) = default; D4(D4&&) = delete; };
+// expected-error@-1{{cannot be applied to struct 'D4' because it is not move-constructible}}
+
+struct [[trivially_relocatable]] D5 : private NonCopyConstructible { };
+// expected-error@-1{{cannot be applied to struct 'D5' because it is not move-constructible}}
+static_assert(!__is_constructible(D5, D5&&), "");
+
+struct [[trivially_relocatable]] D6 : private NonMoveConstructible { D6(D6&&) = default; };
+// expected-error@-1{{cannot be applied to struct 'D6' because it is not move-constructible}}
+
+template
+struct [[trivially_relocatable]] DT1 : private T { }; // ok
+
+struct D7 {
+DT1 m;
+};
+
+class [[trivially_
[PATCH] D50156: [test] Fix %hmaptool path for standalone builds
bkramer accepted this revision. bkramer added a comment. This revision is now accepted and ready to land. lg, thanks Repository: rC Clang https://reviews.llvm.org/D50156 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48100: Append new attributes to the end of an AttributeList.
erichkeane added inline comments.
Comment at: test/Sema/attr-ownership.c:22
void f15(int, int)
- __attribute__((ownership_returns(foo, 1))) // expected-note {{declared with
index 1 here}}
- __attribute__((ownership_returns(foo, 2))); // expected-error
{{'ownership_returns' attribute index does not match; here it is 2}}
+ __attribute__((ownership_returns(foo, 1))) // expected-error
{{'ownership_returns' attribute index does not match; here it is 1}}
+ __attribute__((ownership_returns(foo, 2))); // expected-note {{declared with
index 2 here}}
This seems wrong to me, the 2nd one should be the error condition, right?
Comment at: test/Sema/attr-print.c:25
// TODO: the Type Printer has no way to specify the order to print attributes
// in, and so it currently always prints them in reverse order. Fix this.
This TODO doesn't apply, right? Or is at least wrong...
Comment at: test/Sema/attr-visibility.c:18
-void test6() __attribute__((visibility("hidden"), // expected-note {{previous
attribute is here}}
-visibility("default"))); // expected-error
{{visibility does not match previous declaration}}
+void test6() __attribute__((visibility("default"), // expected-error
{{visibility does not match previous declaration}}
+visibility("hidden"))); // expected-note
{{previous attribute is here}}
This order issue is likely to appear a couple of times I suspect.
Comment at: test/SemaOpenCL/address-spaces.cl:64
typedef __private int private_int_t;
- __local __private int var1; // expected-error {{multiple address spaces
specified for type}}
- __local __private int *var2; // expected-error {{multiple address spaces
specified for type}}
+ __private __local int var1; // expected-error {{multiple address spaces
specified for type}}
+ __private __local int *var2; // expected-error {{multiple address spaces
specified for type}}
These changes have me concerned... The error message isn't specific, but we
have to change the order anyway?
Repository:
rC Clang
https://reviews.llvm.org/D48100
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50119: Compiler support for P1144R0 "__is_trivially_relocatable(T)"
Quuxplusone added inline comments.
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:2942
+ "not %select{move-constructible|destructible}2"
+ >;
+
> Might be useful to add a note explaining why the type isn't trivially
> relocatable instead of the general "because it is not destructible".
You mean, like, a series of notes pointing at "because its base class B is not
destructible... because B's destructor is defined as deleted here"? I agree
that might be helpful, but since it only happens when the programmer is messing
around with the attribute anyway, I wouldn't want to do anything too innovative
or LoC-consuming. I'd cut and paste ~10 lines of code from somewhere else that
already does something like that if you point me at it, but otherwise I think
it's not worth the number of extra codepaths that would need to be tested.
Comment at: include/clang/Sema/Sema.h:4304
+ bool IsTriviallyRelocatableType(QualType QT) const;
+
Rakete wrote:
> Any reason why this is a free function? Should be a member function of
> `QualType`.
`class QualType` is defined in `AST/`, whereas all the C++-specific
TriviallyRelocatable stuff is currently confined to `Sema/`. My impression was
that I should try to preserve that separation, even if it meant being ugly
right here. I agree that this is a stupid hack, and would love to get rid of
it, but I think I need some guidance as to how much mixing of `AST` and `Sema`
is permitted.
Comment at: lib/Sema/SemaDeclCXX.cpp:6066
+if (M->hasAttr() || Record->hasAttr()) {
+ // Consider removing this case to simplify the Standard wording.
+} else {
Rakete wrote:
> This should be a `// TODO: ...`. Is this comment really appropriate? The
> intended audience isn't compiler writers I think.
Similar to the `//puts` comments, this comment is appropriate for me but not
for Clang. :) Will replace with a comment containing the actual proposed
wording.
Comment at: lib/Sema/SemaDeclCXX.cpp:6157
+
+ if (getLangOpts().CPlusPlus11 &&
+ !Record->hasAttr() &&
Rakete wrote:
> This really just checks whether the type has defaulted copy constructor. If
> there was a move constructor, it would have been handled above. If the
> copy/move constructor is implicitly deleted, it would have been handled also
> above. Please simplify this accordingly.
Can you elaborate on this one? I assume you mean that some of lines 6157
through 6171 are superfluous, but I can't figure out which ones or how to
simplify it.
Comment at: lib/Sema/SemaDeclCXX.cpp:6158
+ if (getLangOpts().CPlusPlus11 &&
+ !Record->hasAttr() &&
+ Record->isTriviallyRelocatable()) {
Rakete wrote:
> Why do you need to check whether the attribute is present or not? This is
> supposed to be whether the type is naturally trivially relocatable, so the
> presence of the attribute is not important.
I just thought that checking the presence of the attribute would be cheaper
than doing these lookups, so I was trying to short-circuit it. However, you are
correct for two reasons:
- The check passes 99.99% of the time anyway, so it's *everyone* paying a small
cost just so that the 0.01% can be a bit faster. This is a bad tradeoff.
- Skipping the check when the attribute is present is actually incorrect, in
the case that the type is not relocatable and the attribute is going to be
dropped. (It was not dropped in this revision. It will be dropped in the next
revision.) In that case, even with the attribute, we still want to execute this
codepath.
Comment at: lib/Sema/SemaDeclCXX.cpp:6656
SetDeclDeleted(MD, MD->getLocation());
+ if (CSM == CXXMoveConstructor || CSM == CXXDestructor) {
+//puts("because 6646");
Rakete wrote:
> You don't actually need those three lines. This is already handled in
> `CheckCompletedCXXClass`.
Confirmed. Nice!
Comment at: test/SemaCXX/trivially-relocatable.cpp:42
+struct A6;
+struct [[trivially_relocatable]] A6 {};
+// expected-error@-1{{type A6 declared 'trivially_relocatable' after its first
declaration}}
Rakete wrote:
> Why does this restriction exist? None of the existing attributes have it and
> I don't see why it would make sense to disallow this.
`[[noreturn]]` has it, and I am pretty sure that `[[trivial_abi]]` *ought* to
have it, even though it currently doesn't. The intent is to make it harder for
people to create ODR violations by declaring a type, using it in some way, and
then saying "oh by the way this type was x all along."
```
struct S { S(S&&); ~S(); };
std::vector vec;
struct [[trivially_relocatable]] S; // ha ha, now you have to re-do all of
vector's codegen!
```
Does that make sense as a reason it would be (mildly) beneficial to have thi
[PATCH] D50119: Compiler support for P1144R0 "__is_trivially_relocatable(T)"
Quuxplusone updated this revision to Diff 158613.
Quuxplusone added a comment.
Address feedback from @Rakete. Fix wrong ordering of 'class|struct|...' in
the diagnostic. Add `union` test cases. Correctly drop the attribute whenever
it is diagnosed as inapplicable.
Repository:
rC Clang
https://reviews.llvm.org/D50119
Files:
compiler-explorer-llvm-commit.sh
docs/LanguageExtensions.rst
include/clang/AST/DeclCXX.h
include/clang/Basic/Attr.td
include/clang/Basic/AttrDocs.td
include/clang/Basic/DiagnosticSemaKinds.td
include/clang/Basic/Features.def
include/clang/Basic/TokenKinds.def
include/clang/Basic/TypeTraits.h
include/clang/Sema/Sema.h
lib/AST/ASTImporter.cpp
lib/AST/DeclCXX.cpp
lib/Parse/ParseDeclCXX.cpp
lib/Sema/SemaDecl.cpp
lib/Sema/SemaDeclAttr.cpp
lib/Sema/SemaDeclCXX.cpp
lib/Sema/SemaExprCXX.cpp
lib/Serialization/ASTReaderDecl.cpp
lib/Serialization/ASTWriter.cpp
test/Lexer/has_extension_cxx.cpp
test/Misc/pragma-attribute-supported-attributes-list.test
test/SemaCXX/trivially-relocatable.cpp
Index: test/SemaCXX/trivially-relocatable.cpp
===
--- /dev/null
+++ test/SemaCXX/trivially-relocatable.cpp
@@ -0,0 +1,495 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++11
+// expected-diagnostics
+
+static_assert(__has_extension(trivially_relocatable), "");
+
+// It shall appear at most once in each attribute-list
+// and no attribute-argument-clause shall be present.
+
+struct [[trivially_relocatable, trivially_relocatable]] B1 {};
+// expected-error@-1{{attribute 'trivially_relocatable' cannot appear multiple times in an attribute specifier}}
+
+struct [[trivially_relocatable]] [[trivially_relocatable]] B2 {}; // should really be an error
+
+struct [[trivially_relocatable(42)]] B3 {};
+// expected-error@-1{{attribute 'trivially_relocatable' cannot have an argument list}}
+
+
+// The first declaration of a type shall specify the
+// trivially_relocatable attribute if any declaration of that
+// type specifies the trivially_relocatable attribute.
+
+struct [[trivially_relocatable]] A1 {}; // ok
+struct [[trivially_relocatable]] A1;
+
+struct [[trivially_relocatable]] A2; // ok
+struct [[trivially_relocatable]] A2 {};
+
+struct [[trivially_relocatable]] A3 {}; // ok
+struct A3;
+
+struct [[trivially_relocatable]] A4; // ok
+struct A4 {};
+
+struct A5 {};
+struct [[trivially_relocatable]] A5;
+// expected-error@-1{{type A5 declared 'trivially_relocatable' after its first declaration}}
+// expected-note@-3{{declaration missing 'trivially_relocatable' attribute is here}}
+// expected-warning@-3{{attribute declaration must precede definition}}
+// expected-note@-5{{previous definition is here}}
+
+struct A6;
+struct [[trivially_relocatable]] A6 {};
+// expected-error@-1{{type A6 declared 'trivially_relocatable' after its first declaration}}
+// expected-note@-3{{declaration missing 'trivially_relocatable' attribute is here}}
+
+
+// If a type T is declared with the trivially_relocatable attribute, and T is either
+// not move-constructible or not destructible, the program is ill-formed.
+
+struct NonDestructible {
+NonDestructible(const NonDestructible&) = default;
+NonDestructible(NonDestructible&&) = default;
+~NonDestructible() = delete;
+};
+struct NonCopyConstructible {
+NonCopyConstructible(const NonCopyConstructible&) = delete;
+};
+struct NonMoveConstructible {
+NonMoveConstructible(const NonMoveConstructible&) = default;
+NonMoveConstructible(NonMoveConstructible&&) = delete;
+};
+static_assert(!__is_trivially_relocatable(NonDestructible), "");
+static_assert(!__is_trivially_relocatable(NonCopyConstructible), "");
+static_assert(!__is_constructible(NonCopyConstructible, NonCopyConstructible&&), "");
+static_assert(!__is_trivially_relocatable(NonMoveConstructible), "");
+static_assert(!__is_constructible(NonMoveConstructible, NonMoveConstructible&&), "");
+
+struct [[trivially_relocatable]] D1 { ~D1() = delete; };
+// expected-error@-1{{cannot be applied to struct 'D1' because it is not destructible}}
+
+struct [[trivially_relocatable]] D2 : private NonDestructible { };
+// expected-error@-1{{cannot be applied to struct 'D2' because it is not destructible}}
+
+struct [[trivially_relocatable]] D3 { D3(const D3&) = delete; };
+// expected-error@-1{{cannot be applied to struct 'D3' because it is not move-constructible}}
+
+struct [[trivially_relocatable]] D4 { D4(const D4&) = default; D4(D4&&) = delete; };
+// expected-error@-1{{cannot be applied to struct 'D4' because it is not move-constructible}}
+
+struct [[trivially_relocatable]] D5 : private NonCopyConstructible { };
+// expected-error@-1{{cannot be applied to struct 'D5' because it is not move-constructible}}
+static_assert(!__is_constructible(D5, D5&&), "");
+
+struct [[trivially_relocatable]] D6 : private NonMoveConstructible { D6(D6&&) = default; };
+// expected-error@-1{{cannot be applied to
[PATCH] D50156: [test] Fix %hmaptool path for standalone builds
mgorny created this revision.
mgorny added reviewers: bkramer, bruno.
Fix %hmaptool path to refer to clang_tools_dir instead of
llvm_tools_dir, in order to fix standalone builds. The tool is built
as part of clang, so it won't be found in installed LLVM tools.
Repository:
rC Clang
https://reviews.llvm.org/D50156
Files:
test/lit.cfg.py
Index: test/lit.cfg.py
===
--- test/lit.cfg.py
+++ test/lit.cfg.py
@@ -71,7 +71,7 @@
config.substitutions.append(
('%hmaptool', "'%s' %s" % (config.python_executable,
- os.path.join(config.llvm_tools_dir, 'hmaptool'
+ os.path.join(config.clang_tools_dir,
'hmaptool'
# Plugins (loadable modules)
# TODO: This should be supplied by Makefile or autoconf.
Index: test/lit.cfg.py
===
--- test/lit.cfg.py
+++ test/lit.cfg.py
@@ -71,7 +71,7 @@
config.substitutions.append(
('%hmaptool', "'%s' %s" % (config.python_executable,
- os.path.join(config.llvm_tools_dir, 'hmaptool'
+ os.path.join(config.clang_tools_dir, 'hmaptool'
# Plugins (loadable modules)
# TODO: This should be supplied by Makefile or autoconf.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48100: Append new attributes to the end of an AttributeList.
Meinersbur added a comment. I am unsure how to proceed. Commit since already accepted? Wait for reconfirmation? Open new differential? Repository: rC Clang https://reviews.llvm.org/D48100 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48100: Append new attributes to the end of an AttributeList.
Meinersbur reopened this revision. Meinersbur added a comment. This revision is now accepted and ready to land. Herald added a subscriber: jrtc27. Reopen after revert (and to be able to update the diff) Repository: rC Clang https://reviews.llvm.org/D48100 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48100: Append new attributes to the end of an AttributeList.
Meinersbur updated this revision to Diff 158610.
Meinersbur added a comment.
Rebase after de-listifying in r336945
Repository:
rC Clang
https://reviews.llvm.org/D48100
Files:
include/clang/Sema/ParsedAttr.h
lib/AST/ItaniumMangle.cpp
lib/Parse/ParseDeclCXX.cpp
lib/Parse/ParseObjc.cpp
lib/Sema/SemaAttr.cpp
lib/Sema/SemaOverload.cpp
lib/Sema/SemaType.cpp
lib/Serialization/ASTReaderDecl.cpp
test/Index/annotate-comments-availability-attrs.cpp
test/Index/complete-with-annotations.cpp
test/Misc/ast-print-pragmas.cpp
test/PCH/pragma-loop.cpp
test/Parser/pragma-loop-safety.cpp
test/Parser/pragma-loop.cpp
test/Parser/pragma-unroll-and-jam.cpp
test/Parser/pragma-unroll.cpp
test/Sema/attr-availability-tvos.c
test/Sema/attr-availability.c
test/Sema/attr-coldhot.c
test/Sema/attr-disable-tail-calls.c
test/Sema/attr-long-call.c
test/Sema/attr-micromips.c
test/Sema/attr-notail.c
test/Sema/attr-ownership.c
test/Sema/attr-ownership.cpp
test/Sema/attr-print.c
test/Sema/attr-visibility.c
test/Sema/internal_linkage.c
test/Sema/mips-interrupt-attr.c
test/Sema/nullability.c
test/SemaCXX/attr-print.cpp
test/SemaCXX/ms-uuid.cpp
test/SemaObjC/nullability.m
test/SemaOpenCL/address-spaces.cl
test/SemaTemplate/attributes.cpp
Index: test/SemaTemplate/attributes.cpp
===
--- test/SemaTemplate/attributes.cpp
+++ test/SemaTemplate/attributes.cpp
@@ -55,11 +55,11 @@
}
// CHECK: FunctionTemplateDecl {{.*}} HasAnnotations
-// CHECK: AnnotateAttr {{.*}} "ANNOTATE_BAR"
// CHECK: AnnotateAttr {{.*}} "ANNOTATE_FOO"
+// CHECK: AnnotateAttr {{.*}} "ANNOTATE_BAR"
// CHECK: FunctionDecl {{.*}} HasAnnotations
// CHECK: TemplateArgument type 'int'
-// CHECK: AnnotateAttr {{.*}} "ANNOTATE_BAR"
// CHECK: AnnotateAttr {{.*}} "ANNOTATE_FOO"
+// CHECK: AnnotateAttr {{.*}} "ANNOTATE_BAR"
template [[clang::annotate("ANNOTATE_FOO"), clang::annotate("ANNOTATE_BAR")]] void HasAnnotations();
void UseAnnotations() { HasAnnotations(); }
Index: test/SemaOpenCL/address-spaces.cl
===
--- test/SemaOpenCL/address-spaces.cl
+++ test/SemaOpenCL/address-spaces.cl
@@ -61,8 +61,8 @@
void func_multiple_addr(void) {
typedef __private int private_int_t;
- __local __private int var1; // expected-error {{multiple address spaces specified for type}}
- __local __private int *var2; // expected-error {{multiple address spaces specified for type}}
+ __private __local int var1; // expected-error {{multiple address spaces specified for type}}
+ __private __local int *var2; // expected-error {{multiple address spaces specified for type}}
__local private_int_t var3; // expected-error {{multiple address spaces specified for type}}
__local private_int_t *var4; // expected-error {{multiple address spaces specified for type}}
__private private_int_t var5; // expected-warning {{multiple identical address spaces specified for type}}
Index: test/SemaObjC/nullability.m
===
--- test/SemaObjC/nullability.m
+++ test/SemaObjC/nullability.m
@@ -36,14 +36,14 @@
- (nonnull NSFoo **)invalidMethod1; // expected-error{{nullability keyword 'nonnull' cannot be applied to multi-level pointer type 'NSFoo **'}}
// expected-note@-1{{use nullability type specifier '_Nonnull' to affect the innermost pointer type of 'NSFoo **'}}
-- (nonnull NSFoo * _Nullable)conflictingMethod1; // expected-error{{nullability specifier '_Nullable' conflicts with existing specifier '_Nonnull'}}
-- (nonnull NSFoo * _Nonnull)redundantMethod1; // expected-warning{{duplicate nullability specifier '_Nonnull'}}
+- (nonnull NSFoo * _Nullable)conflictingMethod1; // expected-error{{nullability specifier 'nonnull' conflicts with existing specifier '_Nullable'}}
+- (nonnull NSFoo * _Nonnull)redundantMethod1; // expected-warning{{duplicate nullability specifier 'nonnull'}}
@property(nonnull,retain) NSFoo *property1;
@property(nullable,assign) NSFoo ** invalidProperty1; // expected-error{{nullability keyword 'nullable' cannot be applied to multi-level pointer type 'NSFoo **'}}
// expected-note@-1{{use nullability type specifier '_Nullable' to affect the innermost pointer type of 'NSFoo **'}}
-@property(null_unspecified,retain) NSFoo * _Nullable conflictingProperty1; // expected-error{{nullability specifier '_Nullable' conflicts with existing specifier '_Null_unspecified'}}
-@property(retain,nonnull) NSFoo * _Nonnull redundantProperty1; // expected-warning{{duplicate nullability specifier '_Nonnull'}}
+@property(null_unspecified,retain) NSFoo * _Nullable conflictingProperty1; // expected-error{{nullability specifier 'null_unspecified' conflicts with existing specifier '_Nullable'}}
+@property(retain,nonnull) NSFoo * _Nonnull redundantProperty1; // expected-warning{{duplicate nullability specifier 'nonnull'}}
@property(nu
[PATCH] D49403: More aggressively complete RecordTypes with Function Pointers
rjmccall added a comment. LLVM is sometimes confused by bitcasts of function pointers, but that's just a weird exception to the basic rule that pointer element types aren't supposed to actually matter in LLVM IR. There's a (stalled, unfortunately) effort to remove pointer element types from IR entirely for exactly that reason. https://reviews.llvm.org/D49403 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [clang-tools-extra] r338518 - [clangd] Receive compilationDatabasePath in 'initialize' request
On 1 August 2018 at 11:21, Simon Marchi wrote: > On 2018-08-01 01:30 PM, Alex L wrote: > > Is there a particular reason why this commit didn't have a corresponding > test included? > > Cheers, > > Alex > > Back when we made the corresponding change in "onChangeConfiguration", > there was no > straightforward way to make a lit test for it: > > https://reviews.llvm.org/D39571?id=124024#inline-359345 > > I am not sure, but I think the issue was that we had to hard-code the > length of the > messages we sent. Since we had to use temp directory names, the length > was not > known in advance. I don't think we have that limitation anymore. > Yes, I believe this limitation doesn't exist anymore. > > We would need to create a temporary directory hierarchy, and then refer to > it in > the messages we send to switch between build configurations. Is it > something that > sounds possible with lit? Do you know about other tests doing something > similar? > I think it's be possible, albeit in a more complicated way than the regular Clangd test. We just need to generate a new test file that we can give to the Clangd from the included test file. I think this kind of solution should work (*) : First, you would create a temporary directory in lit: RUN: rm -rf %t.dir RUN: mkdir %t.dir RUN: ... populate the temporary directory ... Then, you would generate the input file (i.e. replace INPUT_DIR in the original test with the temporary directory): RUN: rm -rf %t.test RUN: sed -e "s:INPUT_DIR:%t.dir:g" %s > %t.test And then you can invoke the test: RUN: clang -lit-test < %t.test | FileCheck -strict-whitespace %s * The only problem might be JSON string encoding of the INPUT_DIR (i.e. you'll generate a test file that contains "C:\\temp\foo" on Windows, which would be invalid JSON string). You should be able to use 'sed' here as well to fix this up. Thanks, Alex > > Thanks, > > Simon > ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50101: [asan] Update a vector's storage annotation during destruction.
bobsayshilol added a comment. Thanks! I don't have write access to svn so I can't commit the patch myself, but assuming that I've read the docs correctly if anyone is able to take it I'd be grateful. Repository: rCXX libc++ https://reviews.llvm.org/D50101 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50154: [clangd] capitalize diagnostic messages if '-capitialize-diagnostic-text' is provided
arphaman created this revision.
arphaman added reviewers: ilya-biryukov, ioeric, hokein, jkorous.
Herald added subscribers: dexonsmith, MaskRay.
This patch adds support for diagnostic message capitalization to Clangd. This
is enabled when '-capitialize-diagnostic-text' is provided to Clangd.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D50154
Files:
clangd/ClangdLSPServer.cpp
clangd/ClangdLSPServer.h
clangd/ClangdUnit.cpp
clangd/Diagnostics.cpp
clangd/Diagnostics.h
clangd/tool/ClangdMain.cpp
test/clangd/capitalize-diagnostics.test
Index: test/clangd/capitalize-diagnostics.test
===
--- /dev/null
+++ test/clangd/capitalize-diagnostics.test
@@ -0,0 +1,42 @@
+# RUN: clangd -capitialize-diagnostic-text -lit-test < %s | FileCheck -strict-whitespace %s
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
+---
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///foo.c","languageId":"c","version":1,"text":"void foo() { }\nvoid foo() { }"}}}
+# CHECK: "method": "textDocument/publishDiagnostics",
+# CHECK-NEXT: "params": {
+# CHECK-NEXT:"diagnostics": [
+# CHECK-NEXT: {
+# CHECK-NEXT:"message": "Redefinition of 'foo'\n\nfoo.c:1:6: note: previous definition is here",
+# CHECK-NEXT:"range": {
+# CHECK-NEXT: "end": {
+# CHECK-NEXT:"character": 8,
+# CHECK-NEXT:"line": 1
+# CHECK-NEXT: },
+# CHECK-NEXT: "start": {
+# CHECK-NEXT:"character": 5,
+# CHECK-NEXT:"line": 1
+# CHECK-NEXT: }
+# CHECK-NEXT:},
+# CHECK-NEXT:"severity": 1
+# CHECK-NEXT: },
+# CHECK-NEXT: {
+# CHECK-NEXT:"message": "Previous definition is here\n\nfoo.c:2:6: error: redefinition of 'foo'",
+# CHECK-NEXT:"range": {
+# CHECK-NEXT: "end": {
+# CHECK-NEXT:"character": 8,
+# CHECK-NEXT:"line": 0
+# CHECK-NEXT: },
+# CHECK-NEXT: "start": {
+# CHECK-NEXT:"character": 5,
+# CHECK-NEXT:"line": 0
+# CHECK-NEXT: }
+# CHECK-NEXT:},
+# CHECK-NEXT:"severity": 3
+# CHECK-NEXT: }
+# CHECK-NEXT:],
+# CHECK-NEXT:"uri": "file://{{.*}}/foo.c"
+# CHECK-NEXT: }
+---
+{"jsonrpc":"2.0","id":5,"method":"shutdown"}
+---
+{"jsonrpc":"2.0","method":"exit"}
Index: clangd/tool/ClangdMain.cpp
===
--- clangd/tool/ClangdMain.cpp
+++ clangd/tool/ClangdMain.cpp
@@ -185,6 +185,12 @@
"'compile_commands.json' files")),
llvm::cl::init(FilesystemCompileArgs), llvm::cl::Hidden);
+static llvm::cl::opt CapitalizeDiagnosticText(
+"capitialize-diagnostic-text",
+llvm::cl::desc(
+"Capitalize the first character in the diagnostic's message"),
+llvm::cl::init(false));
+
int main(int argc, char *argv[]) {
llvm::sys::PrintStackTraceOnErrorSignal(argv[0]);
llvm::cl::SetVersionPrinter([](llvm::raw_ostream &OS) {
@@ -300,9 +306,12 @@
CCOpts.IncludeIndicator.NoInsert.clear();
}
+ clangd::DiagnosticOptions DiagOpts;
+ DiagOpts.CapitalizeDiagnosticText = CapitalizeDiagnosticText;
+
// Initialize and run ClangdLSPServer.
ClangdLSPServer LSPServer(
- Out, CCOpts, CompileCommandsDirPath,
+ Out, CCOpts, DiagOpts, CompileCommandsDirPath,
/*ShouldUseInMemoryCDB=*/CompileArgsFrom == LSPCompileArgs, Opts);
constexpr int NoShutdownRequestErrorCode = 1;
llvm::set_thread_name("clangd.main");
Index: clangd/Diagnostics.h
===
--- clangd/Diagnostics.h
+++ clangd/Diagnostics.h
@@ -23,6 +23,11 @@
namespace clang {
namespace clangd {
+struct DiagnosticOptions {
+ /// Capitalize the first character in the diagnostic's message.
+ bool CapitalizeDiagnosticText = false;
+};
+
/// Contains basic information about a diagnostic.
struct DiagBase {
std::string Message;
@@ -65,7 +70,7 @@
/// file do not have a corresponding LSP diagnostic, but can still be included
/// as part of their main diagnostic's message.
void toLSPDiags(
-const Diag &D,
+const Diag &D, const DiagnosticOptions &DiagOpts,
llvm::function_ref)> OutFn);
/// Convert from clang diagnostic level to LSP severity.
Index: clangd/Diagnostics.cpp
===
--- clangd/Diagnostics.cpp
+++ clangd/Diagnostics.cpp
@@ -145,6 +145,14 @@
OS << diagLeveltoString(D.Severity) << ": " << D.Message;
}
+/// Applies common text transformations to a diagnostic's string.
+std::string capitalizeIfNeeded(std::string Message,
+ const DiagnosticOptions &DiagOpts) {
+ if (DiagOpts.CapitalizeDiagnosticText && !Message.empty())
+Message[0] = llvm::toUpper(Mess
[PATCH] D48903: [VirtualFileSystem] InMemoryFileSystem::status: Return a Status with the requested name
eric_niebler added a comment. > It seems to me like the warning is valid, even though we use precompiled > headers. Agreed. Repository: rC Clang https://reviews.llvm.org/D48903 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50152: [CodeGen] Merge equivalent block copy/helper functions
mgrang added inline comments. Comment at: lib/CodeGen/CGBlocks.cpp:1593 + // Sort the captures by offset. + std::sort(ManagedCaptures.begin(), ManagedCaptures.end()); } Please use llvm::sort instead of std::sort. See https://llvm.org/docs/CodingStandards.html#beware-of-non-deterministic-sorting-order-of-equal-elements Repository: rC Clang https://reviews.llvm.org/D50152 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50152: [CodeGen] Merge equivalent block copy/helper functions
ahatanak added inline comments.
Comment at: test/CodeGenCXX/block-byref-cxx-objc.cpp:36
+// CHECK: call void @_Block_object_dispose(
+
+int testB() {
Should the second call to @_Block_object_dispose be an invoke if the destructor
can throw? The FIXME in CodeGenFunction::BuildBlockRelease seems to imply that
we should consider whether the destructor can throw.
Repository:
rC Clang
https://reviews.llvm.org/D50152
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50152: [CodeGen] Merge equivalent block copy/helper functions
ahatanak created this revision.
ahatanak added reviewers: rjmccall, arphaman.
Herald added subscribers: dexonsmith, mgrang.
Currently, clang generates different copy or dispose helper functions for each
block literal on the stack if the block has the possibility of being copied to
the heap. This patch makes changes to merge equivalent copy and dispose helper
functions and reduce code size.
To enable merging equivalent copy/dispose functions, the types and offsets of
the captured objects are encoded into the helper function name. This allows
IRGen to check whether an equivalent helper function has already been emitted
and reuse the function instead of generating a new helper function whenever a
block is defined. In addition, the helper functions are marked as
`linkonce_odr` to enable merging helper functions that have the same name
across translation units and marked as `unnamed_addr` to enable the linker's
deduplication pass to merge functions that have different names but the same
content.
rdar://problem/22950898
Repository:
rC Clang
https://reviews.llvm.org/D50152
Files:
lib/CodeGen/CGBlocks.cpp
lib/CodeGen/CGNonTrivialStruct.cpp
lib/CodeGen/CodeGenFunction.cpp
lib/CodeGen/CodeGenFunction.h
test/CodeGen/blocks-1.c
test/CodeGen/blocks.c
test/CodeGen/sanitize-thread-no-checking-at-run-time.m
test/CodeGenCXX/block-byref-cxx-objc.cpp
test/CodeGenCXX/blocks.cpp
test/CodeGenCXX/cxx-block-objects.cpp
test/CodeGenObjC/arc-blocks.m
test/CodeGenObjC/debug-info-block-helper.m
test/CodeGenObjC/debug-info-blocks.m
test/CodeGenObjC/mrc-weak.m
test/CodeGenObjC/strong-in-c-struct.m
test/CodeGenObjCXX/arc-blocks.mm
test/CodeGenObjCXX/lambda-to-block.mm
test/CodeGenObjCXX/mrc-weak.mm
Index: test/CodeGenObjCXX/mrc-weak.mm
===
--- test/CodeGenObjCXX/mrc-weak.mm
+++ test/CodeGenObjCXX/mrc-weak.mm
@@ -119,10 +119,10 @@
// CHECK: call void @use_block
// CHECK: call void @objc_destroyWeak
-// CHECK-LABEL: define internal void @__copy_helper_block
+// CHECK-LABEL: define linkonce_odr hidden void @__copy_helper_block
// CHECK: @objc_copyWeak
-// CHECK-LABEL: define internal void @__destroy_helper_block
+// CHECK-LABEL: define linkonce_odr hidden void @__destroy_helper_block
// CHECK: @objc_destroyWeak
void test8(void) {
@@ -142,8 +142,8 @@
// CHECK: call void @objc_destroyWeak
// CHECK-LABEL: define void @_Z14test9_baselinev()
-// CHECK: define internal void @__copy_helper
-// CHECK: define internal void @__destroy_helper
+// CHECK: define linkonce_odr hidden void @__copy_helper
+// CHECK: define linkonce_odr hidden void @__destroy_helper
void test9_baseline(void) {
Foo *p = get_object();
use_block(^{ [p run]; });
Index: test/CodeGenObjCXX/lambda-to-block.mm
===
--- test/CodeGenObjCXX/lambda-to-block.mm
+++ test/CodeGenObjCXX/lambda-to-block.mm
@@ -12,7 +12,7 @@
void hasLambda(Copyable x) {
takesBlock([x] () { });
}
-// CHECK-LABEL: define internal void @__copy_helper_block_
+// CHECK-LABEL: define linkonce_odr hidden void @__copy_helper_block_
// CHECK: call void @"_ZZ9hasLambda8CopyableEN3$_0C1ERKS0_"
// CHECK-LABEL: define internal void @"_ZZ9hasLambda8CopyableEN3$_0C2ERKS0_"
// CHECK: call void @_ZN8CopyableC1ERKS_
Index: test/CodeGenObjCXX/arc-blocks.mm
===
--- test/CodeGenObjCXX/arc-blocks.mm
+++ test/CodeGenObjCXX/arc-blocks.mm
@@ -1,5 +1,6 @@
// RUN: %clang_cc1 -std=gnu++98 -triple x86_64-apple-darwin10 -emit-llvm -fobjc-runtime-has-weak -fblocks -fobjc-arc -fexceptions -fobjc-arc-exceptions -o - %s | FileCheck -check-prefix CHECK %s
// RUN: %clang_cc1 -std=gnu++98 -triple x86_64-apple-darwin10 -emit-llvm -fobjc-runtime-has-weak -fblocks -fobjc-arc -fexceptions -fobjc-arc-exceptions -O1 -o - %s | FileCheck -check-prefix CHECK-O1 %s
+// RUN: %clang_cc1 -std=gnu++98 -triple x86_64-apple-darwin10 -emit-llvm -fobjc-runtime-has-weak -fblocks -fobjc-arc -o - %s | FileCheck -check-prefix CHECK-NOEXCP %s
// CHECK: [[A:.*]] = type { i64, [10 x i8*] }
// CHECK: %[[STRUCT_TEST1_S0:.*]] = type { i32 }
@@ -55,10 +56,16 @@
// Check that copy/dispose helper functions are exception safe.
-// CHECK-LABEL: define internal void @__copy_helper_block_(
+// CHECK-LABEL: define linkonce_odr hidden void @__copy_helper_block_ea8_s_32_b8_40_w_48_c2S0_56_c2S0_60(
// CHECK: %[[BLOCK_SOURCE:.*]] = bitcast i8* %{{.*}} to <{ i8*, i32, i32, i8*, %[[STRUCT_BLOCK_DESCRIPTOR]]*, i8*, i8*, i8*, %[[STRUCT_TEST1_S0]], %[[STRUCT_TEST1_S0]] }>*
// CHECK: %[[BLOCK_DEST:.*]] = bitcast i8* %{{.*}} to <{ i8*, i32, i32, i8*, %[[STRUCT_BLOCK_DESCRIPTOR]]*, i8*, i8*, i8*, %[[STRUCT_TEST1_S0]], %[[STRUCT_TEST1_S0]] }>*
+// CHECK: %[[V9:.*]] = getelementptr inbounds <{ i8*, i32, i32, i8*, %[[STRUCT_BLOCK_DESCRIPTOR]]*, i8*, i8*,
[PATCH] D49438: [analyzer][UninitializedObjectChecker] New flag to turn off dereferencing
george.karpenkov added a comment. @Szelethus Hi, do you plan to finish this patch soon-ish? I would like to evaluate it on a few codebases, but the pointer chasing is currently way too fragile / generates too many FPs. Repository: rC Clang https://reviews.llvm.org/D49438 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49403: More aggressively complete RecordTypes with Function Pointers
andrew.w.kaylor added a comment.
I've talked to Olga about this, and I think we have a way to handle the problem
she was working on without this change.
However, I think this change is worth considering anyway. The test case shows
an example where clang is clearly not producing the output it intends to
produce. In most cases that probably doesn't matter, and I can't come up with
any case where it will result in incorrect code generation. One place that I
think it has the potential to introduce trouble is with LTO.
Modifying the example from the test case slightly, suppose you have that code
broken up into two source files like this.
f1.c:
struct has_fp;
typedef void (*fp)(const struct has_fp *f);
struct has_fp {
fp i;
};
void func(const struct has_fp *f) {}
f2.c:
struct has_fp;
typedef void (*fp)(const struct has_fp *f);
struct has_fp {
fp i;
};
void func(const struct has_fp *f);
void func2(const struct has_fp *f, int n) {
if (n == 0)
func(f);
}
Now if I compile both of these files with "-c -flto -O2" and then use
"llvm-link -S -o - f1.o f2.o" I'll see the following:
%struct.has_fp = type { {}* }
%struct.has_fp.0 = type { void (%struct.has_fp.0*)* }
define void @func(%struct.has_fp* %f) {
entry:
ret void
}
define void @func2(%struct.has_fp.0* %f, i32 %i) {
entry:
%cmp = icmp eq i32 %i, 0
br i1 %cmp, label %if.end, label %if.then
if.then:
tail call void bitcast (void (%struct.has_fp*)* @func to void
(%struct.has_fp.0*)*)(%struct.has_fp.0* %f)
br label %if.end
if.end:
ret void
}
Granted, this will ultimately produce correct code, and I don't have an example
handy that shows how the extra type and the function bitcast might inhibit
optimizations, but I think we're at least a step closer to being able to
imagine it.
https://reviews.llvm.org/D49403
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D49580: [clang-format] Adding style option for absolute formatting
Thanks for the review. I’ll update when I’m back from vacation Arnaud > On Jul 30, 2018, at 6:09 AM, Jacek Olesiak via Phabricator > wrote: > > jolesiak added a comment. > > In https://reviews.llvm.org/D49580#1179924, @djasper wrote: > >> Ok, so IIUC, understanding that @end effective ends a section much like "}" >> would address the currently observed problems? > > > I think so. > > > https://reviews.llvm.org/D49580 > > > ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [clang-tools-extra] r338518 - [clangd] Receive compilationDatabasePath in 'initialize' request
On 2018-08-01 01:30 PM, Alex L wrote: > Is there a particular reason why this commit didn't have a corresponding test > included? > Cheers, > Alex Back when we made the corresponding change in "onChangeConfiguration", there was no straightforward way to make a lit test for it: https://reviews.llvm.org/D39571?id=124024#inline-359345 I am not sure, but I think the issue was that we had to hard-code the length of the messages we sent. Since we had to use temp directory names, the length was not known in advance. I don't think we have that limitation anymore. We would need to create a temporary directory hierarchy, and then refer to it in the messages we send to switch between build configurations. Is it something that sounds possible with lit? Do you know about other tests doing something similar? Thanks, Simon ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [clang-tools-extra] r338518 - [clangd] Receive compilationDatabasePath in 'initialize' request
On 2018-08-01 01:30 PM, Alex L wrote: > Is there a particular reason why this commit didn't have a corresponding test > included? > Cheers, > Alex Back when we made the corresponding change in "onChangeConfiguration", there was no straightforward way to make a lit test for it: https://reviews.llvm.org/D39571?id=124024#inline-359345 I am not sure, but I think the issue was that we had to hard-code the length of the messages we sent. Since we had to use temp directory names, the length was not known in advance. I don't think we have that limitation anymore. We would need to create a temporary directory hierarchy, and then refer to it in the messages we send to switch between build configurations. Is it something that sounds possible with lit? Do you know about other tests doing something similar? Thanks, Simon ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50147: clang-format: support external styles
acoomans added a comment. Doesn’t clang-format support project-based //.clang-format// or //_clang-format//? How do they play with this diff? Repository: rC Clang https://reviews.llvm.org/D50147 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50112: [Android] Increase default new alignment for Android
This revision was automatically updated to reflect the committed changes. Closed by commit rC338603: [Android] Increase default new alignment for Android (authored by pirama, committed by ). Changed prior to commit: https://reviews.llvm.org/D50112?vs=158402&id=158574#toc Repository: rC Clang https://reviews.llvm.org/D50112 Files: lib/Basic/TargetInfo.cpp test/Preprocessor/init.c Index: lib/Basic/TargetInfo.cpp === --- lib/Basic/TargetInfo.cpp +++ lib/Basic/TargetInfo.cpp @@ -63,8 +63,9 @@ MinGlobalAlign = 0; // From the glibc documentation, on GNU systems, malloc guarantees 16-byte // alignment on 64-bit systems and 8-byte alignment on 32-bit systems. See - // https://www.gnu.org/software/libc/manual/html_node/Malloc-Examples.html - if (T.isGNUEnvironment() || T.isWindowsMSVCEnvironment()) + // https://www.gnu.org/software/libc/manual/html_node/Malloc-Examples.html. + // This alignment guarantee also applies to Windows and Android. + if (T.isGNUEnvironment() || T.isWindowsMSVCEnvironment() || T.isAndroid()) NewAlign = Triple.isArch64Bit() ? 128 : Triple.isArch32Bit() ? 64 : 0; else NewAlign = 0; // Infer from basic type alignment. Index: test/Preprocessor/init.c === --- test/Preprocessor/init.c +++ test/Preprocessor/init.c @@ -9019,7 +9019,7 @@ // ANDROID:#define __ANDROID__ 1 // // RUN: %clang_cc1 -x c++ -triple i686-linux-android -E -dM < /dev/null | FileCheck -match-full-lines -check-prefix I386-ANDROID-CXX %s -// I386-ANDROID-CXX:#define __STDCPP_DEFAULT_NEW_ALIGNMENT__ 4U +// I386-ANDROID-CXX:#define __STDCPP_DEFAULT_NEW_ALIGNMENT__ 8U // // RUN: %clang_cc1 -x c++ -triple x86_64-linux-android -E -dM < /dev/null | FileCheck -match-full-lines -check-prefix X86_64-ANDROID-CXX %s // X86_64-ANDROID-CXX:#define __STDCPP_DEFAULT_NEW_ALIGNMENT__ 16UL Index: lib/Basic/TargetInfo.cpp === --- lib/Basic/TargetInfo.cpp +++ lib/Basic/TargetInfo.cpp @@ -63,8 +63,9 @@ MinGlobalAlign = 0; // From the glibc documentation, on GNU systems, malloc guarantees 16-byte // alignment on 64-bit systems and 8-byte alignment on 32-bit systems. See - // https://www.gnu.org/software/libc/manual/html_node/Malloc-Examples.html - if (T.isGNUEnvironment() || T.isWindowsMSVCEnvironment()) + // https://www.gnu.org/software/libc/manual/html_node/Malloc-Examples.html. + // This alignment guarantee also applies to Windows and Android. + if (T.isGNUEnvironment() || T.isWindowsMSVCEnvironment() || T.isAndroid()) NewAlign = Triple.isArch64Bit() ? 128 : Triple.isArch32Bit() ? 64 : 0; else NewAlign = 0; // Infer from basic type alignment. Index: test/Preprocessor/init.c === --- test/Preprocessor/init.c +++ test/Preprocessor/init.c @@ -9019,7 +9019,7 @@ // ANDROID:#define __ANDROID__ 1 // // RUN: %clang_cc1 -x c++ -triple i686-linux-android -E -dM < /dev/null | FileCheck -match-full-lines -check-prefix I386-ANDROID-CXX %s -// I386-ANDROID-CXX:#define __STDCPP_DEFAULT_NEW_ALIGNMENT__ 4U +// I386-ANDROID-CXX:#define __STDCPP_DEFAULT_NEW_ALIGNMENT__ 8U // // RUN: %clang_cc1 -x c++ -triple x86_64-linux-android -E -dM < /dev/null | FileCheck -match-full-lines -check-prefix X86_64-ANDROID-CXX %s // X86_64-ANDROID-CXX:#define __STDCPP_DEFAULT_NEW_ALIGNMENT__ 16UL ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r338603 - [Android] Increase default new alignment for Android
Author: pirama Date: Wed Aug 1 10:55:34 2018 New Revision: 338603 URL: http://llvm.org/viewvc/llvm-project?rev=338603&view=rev Log: [Android] Increase default new alignment for Android Summary: Android's memory allocators also guarantee 8-byte alignment for 32-bit architectures and 16-byte alignment for 64-bit. Reviewers: rsmith Subscribers: cfe-commits, srhines, enh Differential Revision: https://reviews.llvm.org/D50112 Modified: cfe/trunk/lib/Basic/TargetInfo.cpp cfe/trunk/test/Preprocessor/init.c Modified: cfe/trunk/lib/Basic/TargetInfo.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/TargetInfo.cpp?rev=338603&r1=338602&r2=338603&view=diff == --- cfe/trunk/lib/Basic/TargetInfo.cpp (original) +++ cfe/trunk/lib/Basic/TargetInfo.cpp Wed Aug 1 10:55:34 2018 @@ -63,8 +63,9 @@ TargetInfo::TargetInfo(const llvm::Tripl MinGlobalAlign = 0; // From the glibc documentation, on GNU systems, malloc guarantees 16-byte // alignment on 64-bit systems and 8-byte alignment on 32-bit systems. See - // https://www.gnu.org/software/libc/manual/html_node/Malloc-Examples.html - if (T.isGNUEnvironment() || T.isWindowsMSVCEnvironment()) + // https://www.gnu.org/software/libc/manual/html_node/Malloc-Examples.html. + // This alignment guarantee also applies to Windows and Android. + if (T.isGNUEnvironment() || T.isWindowsMSVCEnvironment() || T.isAndroid()) NewAlign = Triple.isArch64Bit() ? 128 : Triple.isArch32Bit() ? 64 : 0; else NewAlign = 0; // Infer from basic type alignment. Modified: cfe/trunk/test/Preprocessor/init.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Preprocessor/init.c?rev=338603&r1=338602&r2=338603&view=diff == --- cfe/trunk/test/Preprocessor/init.c (original) +++ cfe/trunk/test/Preprocessor/init.c Wed Aug 1 10:55:34 2018 @@ -9019,7 +9019,7 @@ // ANDROID:#define __ANDROID__ 1 // // RUN: %clang_cc1 -x c++ -triple i686-linux-android -E -dM < /dev/null | FileCheck -match-full-lines -check-prefix I386-ANDROID-CXX %s -// I386-ANDROID-CXX:#define __STDCPP_DEFAULT_NEW_ALIGNMENT__ 4U +// I386-ANDROID-CXX:#define __STDCPP_DEFAULT_NEW_ALIGNMENT__ 8U // // RUN: %clang_cc1 -x c++ -triple x86_64-linux-android -E -dM < /dev/null | FileCheck -match-full-lines -check-prefix X86_64-ANDROID-CXX %s // X86_64-ANDROID-CXX:#define __STDCPP_DEFAULT_NEW_ALIGNMENT__ 16UL ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48903: [VirtualFileSystem] InMemoryFileSystem::status: Return a Status with the requested name
simark added a subscriber: eric_niebler. simark added a comment. @eric_niebler, question for you: This patch causes clang to emit a `-Wnonportable-include-path` warning where it did not before. It affects the following test on Windows: https://github.com/llvm-mirror/clang/blob/master/test/PCH/case-insensitive-include.c The warning is currently not emitted for the **last** clang invocation (the one that includes PCH), because the real path value is not available, and therefore this condition is false: https://github.com/llvm-mirror/clang/blob/fe1098c84823b8eac46b0bfffc5f5788b6c26d1a/lib/Lex/PPDirectives.cpp#L2015 With this patch, the real path value is available, so we emit the warning. Is it on purpose that this warning is not emitted in this case? If not should I simply add `-Wno-nonportable-include-path` to the last clang invocation, as was done earlier with the first invocation? It seems to me like the warning is valid, even though we use precompiled headers. Repository: rC Clang https://reviews.llvm.org/D48903 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r338455 - [constexpr] Support for constant evaluation of __builtin_memcpy and
We hit this in Chromium too, see crbug.com/869824
I've reverted in r338602
On Wed, Aug 1, 2018 at 5:26 PM, Benjamin Kramer via cfe-commits
wrote:
> It's pretty easy to make this crash
>
> $ cat memcpy.c
> void foo() {
> int a[1], b;
> memcpy((char*)a, (const char*)&b, (unsigned long)4);
> }
>
> $ clang memcpy.c
> llvm/include/llvm/ADT/SmallVector.h:178: const_reference
> llvm::SmallVectorTemplateCommon void>::back() const [T = clang::APValue::LValue
> PathEntry]: Assertion `!empty()' failed.
>
> On Wed, Aug 1, 2018 at 1:35 AM Richard Smith via cfe-commits
> wrote:
>>
>> Author: rsmith
>> Date: Tue Jul 31 16:35:09 2018
>> New Revision: 338455
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=338455&view=rev
>> Log:
>> [constexpr] Support for constant evaluation of __builtin_memcpy and
>> __builtin_memmove (in non-type-punning cases).
>>
>> This is intended to permit libc++ to make std::copy etc constexpr
>> without sacrificing the optimization that uses memcpy on
>> trivially-copyable types.
>>
>> __builtin_strcpy and __builtin_wcscpy are not handled by this change.
>> They'd be straightforward to add, but we haven't encountered a need for
>> them just yet.
>>
>> Modified:
>> cfe/trunk/include/clang/Basic/Builtins.def
>> cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td
>> cfe/trunk/lib/AST/ExprConstant.cpp
>> cfe/trunk/test/CodeGen/builtin-memfns.c
>> cfe/trunk/test/SemaCXX/constexpr-string.cpp
>>
>> Modified: cfe/trunk/include/clang/Basic/Builtins.def
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Builtins.def?rev=338455&r1=338454&r2=338455&view=diff
>>
>> ==
>> --- cfe/trunk/include/clang/Basic/Builtins.def (original)
>> +++ cfe/trunk/include/clang/Basic/Builtins.def Tue Jul 31 16:35:09 2018
>> @@ -471,6 +471,8 @@ BUILTIN(__builtin_wcslen, "zwC*", "nF")
>> BUILTIN(__builtin_wcsncmp, "iwC*wC*z", "nF")
>> BUILTIN(__builtin_wmemchr, "w*wC*wz", "nF")
>> BUILTIN(__builtin_wmemcmp, "iwC*wC*z", "nF")
>> +BUILTIN(__builtin_wmemcpy, "w*w*wC*z", "nF")
>> +BUILTIN(__builtin_wmemmove, "w*w*wC*z", "nF")
>> BUILTIN(__builtin_return_address, "v*IUi", "n")
>> BUILTIN(__builtin_extract_return_addr, "v*v*", "n")
>> BUILTIN(__builtin_frame_address, "v*IUi", "n")
>> @@ -908,6 +910,8 @@ LIBBUILTIN(wcslen, "zwC*", "f", "wc
>> LIBBUILTIN(wcsncmp, "iwC*wC*z", "f", "wchar.h", ALL_LANGUAGES)
>> LIBBUILTIN(wmemchr, "w*wC*wz", "f", "wchar.h", ALL_LANGUAGES)
>> LIBBUILTIN(wmemcmp, "iwC*wC*z", "f", "wchar.h", ALL_LANGUAGES)
>> +LIBBUILTIN(wmemcpy, "w*w*wC*z", "f", "wchar.h", ALL_LANGUAGES)
>> +LIBBUILTIN(wmemmove,"w*w*wC*z", "f", "wchar.h", ALL_LANGUAGES)
>>
>> // C99
>> // In some systems setjmp is a macro that expands to _setjmp. We undefine
>>
>> Modified: cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td?rev=338455&r1=338454&r2=338455&view=diff
>>
>> ==
>> --- cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td (original)
>> +++ cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td Tue Jul 31
>> 16:35:09 2018
>> @@ -163,6 +163,20 @@ def note_constexpr_unsupported_unsized_a
>> def note_constexpr_unsized_array_indexed : Note<
>>"indexing of array without known bound is not allowed "
>>"in a constant expression">;
>> +def note_constexpr_memcpy_type_pun : Note<
>> + "cannot constant evaluate '%select{memcpy|memmove}0' from object of "
>> + "type %1 to object of type %2">;
>> +def note_constexpr_memcpy_nontrivial : Note<
>> + "cannot constant evaluate '%select{memcpy|memmove}0' between objects of
>> "
>> + "non-trivially-copyable type %1">;
>> +def note_constexpr_memcpy_overlap : Note<
>> + "'%select{memcpy|wmemcpy}0' between overlapping memory regions">;
>> +def note_constexpr_memcpy_unsupported : Note<
>> + "'%select{%select{memcpy|wmemcpy}1|%select{memmove|wmemmove}1}0' "
>> + "not supported: %select{"
>> + "size to copy (%4) is not a multiple of size of element type %3 (%5)|"
>> + "source is not a contiguous array of at least %4 elements of type %3|"
>> + "destination is not a contiguous array of at least %4 elements of type
>> %3}2">;
>>
>> def warn_integer_constant_overflow : Warning<
>>"overflow in expression; result is %0 with type %1">,
>>
>> Modified: cfe/trunk/lib/AST/ExprConstant.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=338455&r1=338454&r2=338455&view=diff
>>
>> ==
>> --- cfe/trunk/lib/AST/ExprConstant.cpp (original)
>> +++ cfe/trunk/lib/AST/ExprConstant.cpp Tue Jul 31 16:35:09 2018
>> @@ -319,6 +319,25 @@ namespace {
>>return false;
>> }
>>
>> +/// Get the range of valid index adjustments in the form
>> +/// {maximum value that ca
r338602 - Revert r338455 "[constexpr] Support for constant evaluation of __builtin_memcpy and __builtin_memmove (in non-type-punning cases)."
Author: hans
Date: Wed Aug 1 10:51:23 2018
New Revision: 338602
URL: http://llvm.org/viewvc/llvm-project?rev=338602&view=rev
Log:
Revert r338455 "[constexpr] Support for constant evaluation of __builtin_memcpy
and __builtin_memmove (in non-type-punning cases)."
It caused asserts during Chromium builds, see reply on the cfe-commits thread.
> This is intended to permit libc++ to make std::copy etc constexpr
> without sacrificing the optimization that uses memcpy on
> trivially-copyable types.
>
> __builtin_strcpy and __builtin_wcscpy are not handled by this change.
> They'd be straightforward to add, but we haven't encountered a need for
> them just yet.
Modified:
cfe/trunk/include/clang/Basic/Builtins.def
cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td
cfe/trunk/lib/AST/ExprConstant.cpp
cfe/trunk/test/CodeGen/builtin-memfns.c
cfe/trunk/test/SemaCXX/constexpr-string.cpp
Modified: cfe/trunk/include/clang/Basic/Builtins.def
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Builtins.def?rev=338602&r1=338601&r2=338602&view=diff
==
--- cfe/trunk/include/clang/Basic/Builtins.def (original)
+++ cfe/trunk/include/clang/Basic/Builtins.def Wed Aug 1 10:51:23 2018
@@ -471,8 +471,6 @@ BUILTIN(__builtin_wcslen, "zwC*", "nF")
BUILTIN(__builtin_wcsncmp, "iwC*wC*z", "nF")
BUILTIN(__builtin_wmemchr, "w*wC*wz", "nF")
BUILTIN(__builtin_wmemcmp, "iwC*wC*z", "nF")
-BUILTIN(__builtin_wmemcpy, "w*w*wC*z", "nF")
-BUILTIN(__builtin_wmemmove, "w*w*wC*z", "nF")
BUILTIN(__builtin_return_address, "v*IUi", "n")
BUILTIN(__builtin_extract_return_addr, "v*v*", "n")
BUILTIN(__builtin_frame_address, "v*IUi", "n")
@@ -910,8 +908,6 @@ LIBBUILTIN(wcslen, "zwC*", "f", "wc
LIBBUILTIN(wcsncmp, "iwC*wC*z", "f", "wchar.h", ALL_LANGUAGES)
LIBBUILTIN(wmemchr, "w*wC*wz", "f", "wchar.h", ALL_LANGUAGES)
LIBBUILTIN(wmemcmp, "iwC*wC*z", "f", "wchar.h", ALL_LANGUAGES)
-LIBBUILTIN(wmemcpy, "w*w*wC*z", "f", "wchar.h", ALL_LANGUAGES)
-LIBBUILTIN(wmemmove,"w*w*wC*z", "f", "wchar.h", ALL_LANGUAGES)
// C99
// In some systems setjmp is a macro that expands to _setjmp. We undefine
Modified: cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td?rev=338602&r1=338601&r2=338602&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td Wed Aug 1 10:51:23 2018
@@ -163,20 +163,6 @@ def note_constexpr_unsupported_unsized_a
def note_constexpr_unsized_array_indexed : Note<
"indexing of array without known bound is not allowed "
"in a constant expression">;
-def note_constexpr_memcpy_type_pun : Note<
- "cannot constant evaluate '%select{memcpy|memmove}0' from object of "
- "type %1 to object of type %2">;
-def note_constexpr_memcpy_nontrivial : Note<
- "cannot constant evaluate '%select{memcpy|memmove}0' between objects of "
- "non-trivially-copyable type %1">;
-def note_constexpr_memcpy_overlap : Note<
- "'%select{memcpy|wmemcpy}0' between overlapping memory regions">;
-def note_constexpr_memcpy_unsupported : Note<
- "'%select{%select{memcpy|wmemcpy}1|%select{memmove|wmemmove}1}0' "
- "not supported: %select{"
- "size to copy (%4) is not a multiple of size of element type %3 (%5)|"
- "source is not a contiguous array of at least %4 elements of type %3|"
- "destination is not a contiguous array of at least %4 elements of type
%3}2">;
def warn_integer_constant_overflow : Warning<
"overflow in expression; result is %0 with type %1">,
Modified: cfe/trunk/lib/AST/ExprConstant.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=338602&r1=338601&r2=338602&view=diff
==
--- cfe/trunk/lib/AST/ExprConstant.cpp (original)
+++ cfe/trunk/lib/AST/ExprConstant.cpp Wed Aug 1 10:51:23 2018
@@ -319,25 +319,6 @@ namespace {
return false;
}
-/// Get the range of valid index adjustments in the form
-/// {maximum value that can be subtracted from this pointer,
-///maximum value that can be added to this pointer}
-std::pair validIndexAdjustments() {
- if (Invalid || isMostDerivedAnUnsizedArray())
-return {0, 0};
-
- // [expr.add]p4: For the purposes of these operators, a pointer to a
- // nonarray object behaves the same as a pointer to the first element of
- // an array of length one with the type of the object as its element
type.
- bool IsArray = MostDerivedPathLength == Entries.size() &&
- MostDerivedIsArrayElement;
- uint64_t ArrayIndex =
- IsArray ? Entries.back().ArrayIndex : (uint64_t)IsOnePastTheEnd;
- uint64_t ArraySize =
- IsArray ? getMostDerivedArra
[PATCH] D49729: [AST][1/4] Move the bit-fields from TagDecl, EnumDecl and RecordDecl into DeclContext
erichkeane accepted this revision. erichkeane added a comment. LGTM! Repository: rC Clang https://reviews.llvm.org/D49729 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49729: [AST][1/4] Move the bit-fields from TagDecl, EnumDecl and RecordDecl into DeclContext
bricci added a comment. @erichkeane do you have any additional comments regarding this set of patches ? I retested them on top of trunk and did not find any problem. Repository: rC Clang https://reviews.llvm.org/D49729 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r338597 - [clangd] allow clients to control the compilation database by passing in
Author: arphaman
Date: Wed Aug 1 10:39:29 2018
New Revision: 338597
URL: http://llvm.org/viewvc/llvm-project?rev=338597&view=rev
Log:
[clangd] allow clients to control the compilation database by passing in
compilationDatabaseChanges in the 'workspace/didChangeConfiguration' request
This commit allows clangd to use an in-memory compilation database that's
controlled from the LSP client (-compile_args_from=lsp). It extends the
'workspace/didChangeConfiguration' request to allow the client to pass in a
compilation database subset that needs to be updated in the workspace.
Differential Revision: https://reviews.llvm.org/D49758
Added:
clang-tools-extra/trunk/test/clangd/did-change-configuration-params.test
Modified:
clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
clang-tools-extra/trunk/clangd/ClangdLSPServer.h
clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp
clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.h
clang-tools-extra/trunk/clangd/Protocol.cpp
clang-tools-extra/trunk/clangd/Protocol.h
clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp?rev=338597&r1=338596&r2=338597&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp Wed Aug 1 10:39:29 2018
@@ -135,11 +135,8 @@ void ClangdLSPServer::onExit(ExitParams
void ClangdLSPServer::onDocumentDidOpen(DidOpenTextDocumentParams &Params) {
PathRef File = Params.textDocument.uri.file();
- if (Params.metadata && !Params.metadata->extraFlags.empty()) {
-NonCachedCDB.setExtraFlagsForFile(File,
- std::move(Params.metadata->extraFlags));
-CDB.invalidate(File);
- }
+ if (Params.metadata && !Params.metadata->extraFlags.empty())
+CDB.setExtraFlagsForFile(File, std::move(Params.metadata->extraFlags));
std::string &Contents = Params.textDocument.text;
@@ -250,6 +247,7 @@ void ClangdLSPServer::onDocumentDidClose
PathRef File = Params.textDocument.uri.file();
DraftMgr.removeDraft(File);
Server.removeDocument(File);
+ CDB.invalidate(File);
}
void ClangdLSPServer::onDocumentOnTypeFormatting(
@@ -405,12 +403,29 @@ void ClangdLSPServer::applyConfiguration
const ClangdConfigurationParamsChange &Settings) {
// Compilation database change.
if (Settings.compilationDatabasePath.hasValue()) {
-NonCachedCDB.setCompileCommandsDir(
-Settings.compilationDatabasePath.getValue());
-CDB.clear();
+CDB.setCompileCommandsDir(Settings.compilationDatabasePath.getValue());
reparseOpenedFiles();
}
+
+ // Update to the compilation database.
+ if (Settings.compilationDatabaseChanges) {
+const auto &CompileCommandUpdates = *Settings.compilationDatabaseChanges;
+bool ShouldReparseOpenFiles = false;
+for (auto &Entry : CompileCommandUpdates) {
+ /// The opened files need to be reparsed only when some existing
+ /// entries are changed.
+ PathRef File = Entry.first;
+ if (!CDB.setCompilationCommandForFile(
+ File, tooling::CompileCommand(
+std::move(Entry.second.workingDirectory), File,
+std::move(Entry.second.compilationCommand),
+/*Output=*/"")))
+ShouldReparseOpenFiles = true;
+}
+if (ShouldReparseOpenFiles)
+ reparseOpenedFiles();
+ }
}
// FIXME: This function needs to be properly tested.
@@ -422,10 +437,13 @@ void ClangdLSPServer::onChangeConfigurat
ClangdLSPServer::ClangdLSPServer(JSONOutput &Out,
const clangd::CodeCompleteOptions &CCOpts,
llvm::Optional CompileCommandsDir,
+ bool ShouldUseInMemoryCDB,
const ClangdServer::Options &Opts)
-: Out(Out), NonCachedCDB(std::move(CompileCommandsDir)), CDB(NonCachedCDB),
+: Out(Out), CDB(ShouldUseInMemoryCDB ? CompilationDB::makeInMemory()
+ : CompilationDB::makeDirectoryBased(
+ std::move(CompileCommandsDir))),
CCOpts(CCOpts), SupportedSymbolKinds(defaultSymbolKinds()),
- Server(CDB, FSProvider, /*DiagConsumer=*/*this, Opts) {}
+ Server(CDB.getCDB(), FSProvider, /*DiagConsumer=*/*this, Opts) {}
bool ClangdLSPServer::run(std::FILE *In, JSONStreamStyle InputStyle) {
assert(!IsDone && "Run was called before");
@@ -504,3 +522,67 @@ void ClangdLSPServer::reparseOpenedFiles
Server.addDocument(FilePath, *DraftMgr.getDraft(FilePath),
WantDiagnostics::Auto);
}
+
+ClangdLSPServer::CompilationDB ClangdLSPServer::CompilationDB::makeInMemory() {
+ return CompilationDB(
[PATCH] D49758: [clangd] allow clients to pass in compilationDatabaseChanges in the 'workspace/didChangeConfiguration' request
This revision was automatically updated to reflect the committed changes.
Closed by commit rL338597: [clangd] allow clients to control the compilation
database by passing in (authored by arphaman, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D49758?vs=158376&id=158568#toc
Repository:
rL LLVM
https://reviews.llvm.org/D49758
Files:
clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
clang-tools-extra/trunk/clangd/ClangdLSPServer.h
clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp
clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.h
clang-tools-extra/trunk/clangd/Protocol.cpp
clang-tools-extra/trunk/clangd/Protocol.h
clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
clang-tools-extra/trunk/test/clangd/did-change-configuration-params.test
Index: clang-tools-extra/trunk/clangd/Protocol.h
===
--- clang-tools-extra/trunk/clangd/Protocol.h
+++ clang-tools-extra/trunk/clangd/Protocol.h
@@ -322,11 +322,25 @@
bool fromJSON(const llvm::json::Value &, ClientCapabilities &);
+/// Clangd extension that's used in the 'compilationDatabaseChanges' in
+/// workspace/didChangeConfiguration to record updates to the in-memory
+/// compilation database.
+struct ClangdCompileCommand {
+ std::string workingDirectory;
+ std::vector compilationCommand;
+};
+bool fromJSON(const llvm::json::Value &, ClangdCompileCommand &);
+
/// Clangd extension to set clangd-specific "initializationOptions" in the
/// "initialize" request and for the "workspace/didChangeConfiguration"
/// notification since the data received is described as 'any' type in LSP.
struct ClangdConfigurationParamsChange {
llvm::Optional compilationDatabasePath;
+
+ // The changes that happened to the compilation database.
+ // The key of the map is a file name.
+ llvm::Optional>
+ compilationDatabaseChanges;
};
bool fromJSON(const llvm::json::Value &, ClangdConfigurationParamsChange &);
Index: clang-tools-extra/trunk/clangd/ClangdLSPServer.h
===
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.h
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.h
@@ -35,7 +35,7 @@
/// for compile_commands.json in all parent directories of each file.
ClangdLSPServer(JSONOutput &Out, const clangd::CodeCompleteOptions &CCOpts,
llvm::Optional CompileCommandsDir,
- const ClangdServer::Options &Opts);
+ bool ShouldUseInMemoryCDB, const ClangdServer::Options &Opts);
/// Run LSP server loop, receiving input for it from \p In. \p In must be
/// opened in binary mode. Output will be written using Out variable passed to
@@ -100,10 +100,57 @@
/// Caches FixIts per file and diagnostics
llvm::StringMap FixItsMap;
+ /// Encapsulates the directory-based or the in-memory compilation database
+ /// that's used by the LSP server.
+ class CompilationDB {
+ public:
+static CompilationDB makeInMemory();
+static CompilationDB
+makeDirectoryBased(llvm::Optional CompileCommandsDir);
+
+void invalidate(PathRef File);
+
+/// Sets the compilation command for a particular file.
+/// Only valid for in-memory CDB, no-op and error log on DirectoryBasedCDB.
+///
+/// \returns True if the File had no compilation command before.
+bool
+setCompilationCommandForFile(PathRef File,
+ tooling::CompileCommand CompilationCommand);
+
+/// Adds extra compilation flags to the compilation command for a particular
+/// file. Only valid for directory-based CDB, no-op and error log on
+/// InMemoryCDB;
+void setExtraFlagsForFile(PathRef File,
+ std::vector ExtraFlags);
+
+/// Set the compile commands directory to \p P.
+/// Only valid for directory-based CDB, no-op and error log on InMemoryCDB;
+void setCompileCommandsDir(Path P);
+
+/// Returns a CDB that should be used to get compile commands for the
+/// current instance of ClangdLSPServer.
+GlobalCompilationDatabase &getCDB();
+
+ private:
+CompilationDB(std::unique_ptr CDB,
+ std::unique_ptr CachingCDB,
+ bool IsDirectoryBased)
+: CDB(std::move(CDB)), CachingCDB(std::move(CachingCDB)),
+ IsDirectoryBased(IsDirectoryBased) {}
+
+// if IsDirectoryBased is true, an instance of InMemoryCDB.
+// If IsDirectoryBased is false, an instance of DirectoryBasedCDB.
+// unique_ptr CDB;
+std::unique_ptr CDB;
+// Non-null only for directory-based CDB
+std::unique_ptr CachingCDB;
+bool IsDirectoryBased;
+ };
+
// Various ClangdServer parameters go here. It's important they're created
// before ClangdServer.
- DirectoryBasedGlobalCompilationDatabase NonCachedCDB;
- CachingCompilationDb CDB;
+ CompilationDB CDB;
RealFileSystemPro
Re: r338489 - [AST] CastExpr: BasePathSize is not large enough.
On Wed, Aug 1, 2018 at 8:36 PM, Richard Smith wrote:
> On Tue, 31 Jul 2018, 23:06 Roman Lebedev via cfe-commits,
> wrote:
>>
>> Author: lebedevri
>> Date: Tue Jul 31 23:06:16 2018
>> New Revision: 338489
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=338489&view=rev
>> Log:
>> [AST] CastExpr: BasePathSize is not large enough.
>>
>> Summary:
>> rC337815 / D49508 had to cannibalize one bit of
>> `CastExprBitfields::BasePathSize` in order to squeeze `PartOfExplicitCast`
>> boolean.
>> That reduced the maximal value of `PartOfExplicitCast` from 9 bits (~512)
>> down to 8 bits (~256).
>> Apparently, that mattered. Too bad there weren't any tests.
>> It caused [[ https://bugs.llvm.org/show_bug.cgi?id=38356 | PR38356 ]].
>>
>> So we need to increase `PartOfExplicitCast` back at least to 9 bits, or a
>> bit more.
>> For obvious reasons, we can't do that in `CastExprBitfields` - that would
>> blow up the size of every `Expr`.
>> So we need to either just add a variable into the `CastExpr` (as done
>> here),
>> or use `llvm::TrailingObjects`. The latter does not seem to be
>> straight-forward.
>> Perhaps, that needs to be done not for the `CastExpr` itself, but for all
>> of it's `final` children.
>>
>> Reviewers: rjmccall, rsmith, erichkeane
>>
>> Reviewed By: rjmccall
>>
>> Subscribers: bricci, hans, cfe-commits, waddlesplash
>>
>> Differential Revision: https://reviews.llvm.org/D50050
>>
>> Added:
>> cfe/trunk/test/CodeGenCXX/castexpr-basepathsize-threshold.cpp
>> Modified:
>> cfe/trunk/include/clang/AST/Expr.h
>> cfe/trunk/include/clang/AST/ExprCXX.h
>> cfe/trunk/include/clang/AST/ExprObjC.h
>> cfe/trunk/include/clang/AST/Stmt.h
>> cfe/trunk/lib/AST/Expr.cpp
>> cfe/trunk/lib/AST/ExprCXX.cpp
>>
>> Modified: cfe/trunk/include/clang/AST/Expr.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Expr.h?rev=338489&r1=338488&r2=338489&view=diff
>>
>> ==
>> --- cfe/trunk/include/clang/AST/Expr.h (original)
>> +++ cfe/trunk/include/clang/AST/Expr.h Tue Jul 31 23:06:16 2018
>> @@ -2787,20 +2787,26 @@ public:
>> /// representation in the source code (ExplicitCastExpr's derived
>> /// classes).
>> class CastExpr : public Expr {
>> +public:
>> + using BasePathSizeTy = unsigned int;
>> + static_assert(std::numeric_limits::max() >= 16384,
>> +"[implimits] Direct and indirect base classes [16384].");
>> +
>> private:
>>Stmt *Op;
>>
>>bool CastConsistency() const;
>>
>> + BasePathSizeTy *BasePathSize();
>> +
>>const CXXBaseSpecifier * const *path_buffer() const {
>> return const_cast(this)->path_buffer();
>>}
>>CXXBaseSpecifier **path_buffer();
>>
>> - void setBasePathSize(unsigned basePathSize) {
>> -CastExprBits.BasePathSize = basePathSize;
>> -assert(CastExprBits.BasePathSize == basePathSize &&
>> - "basePathSize doesn't fit in bits of
>> CastExprBits.BasePathSize!");
>> + void setBasePathSize(BasePathSizeTy basePathSize) {
>> +assert(!path_empty() && basePathSize != 0);
>> +*(BasePathSize()) = basePathSize;
>>}
>>
>> protected:
>> @@ -2823,7 +2829,9 @@ protected:
>> Op(op) {
>> CastExprBits.Kind = kind;
>> CastExprBits.PartOfExplicitCast = false;
>> -setBasePathSize(BasePathSize);
>> +CastExprBits.BasePathIsEmpty = BasePathSize == 0;
>> +if (!path_empty())
>> + setBasePathSize(BasePathSize);
>> assert(CastConsistency());
>>}
>>
>> @@ -2831,7 +2839,9 @@ protected:
>>CastExpr(StmtClass SC, EmptyShell Empty, unsigned BasePathSize)
>> : Expr(SC, Empty) {
>> CastExprBits.PartOfExplicitCast = false;
>> -setBasePathSize(BasePathSize);
>> +CastExprBits.BasePathIsEmpty = BasePathSize == 0;
>> +if (!path_empty())
>> + setBasePathSize(BasePathSize);
>>}
>>
>> public:
>> @@ -2859,8 +2869,12 @@ public:
>>
>>typedef CXXBaseSpecifier **path_iterator;
>>typedef const CXXBaseSpecifier * const *path_const_iterator;
>> - bool path_empty() const { return CastExprBits.BasePathSize == 0; }
>> - unsigned path_size() const { return CastExprBits.BasePathSize; }
>> + bool path_empty() const { return CastExprBits.BasePathIsEmpty; }
>> + unsigned path_size() const {
>> +if (path_empty())
>> + return 0U;
>> +return *(const_cast(this)->BasePathSize());
>> + }
>>path_iterator path_begin() { return path_buffer(); }
>>path_iterator path_end() { return path_buffer() + path_size(); }
>>path_const_iterator path_begin() const { return path_buffer(); }
>> @@ -2908,7 +2922,12 @@ public:
>> /// @endcode
>> class ImplicitCastExpr final
>> : public CastExpr,
>> - private llvm::TrailingObjects
>> {
>> + private llvm::TrailingObjects> CastExpr::BasePathSizeTy,
>> +CXXBaseSpecifier *> {
>> + size_t numTrailingObjects(OverloadToken)
>> const {
>> +return path_empty() ? 0 : 1;
>> + }
>
Re: r338489 - [AST] CastExpr: BasePathSize is not large enough.
On Tue, 31 Jul 2018, 23:06 Roman Lebedev via cfe-commits, <
cfe-commits@lists.llvm.org> wrote:
> Author: lebedevri
> Date: Tue Jul 31 23:06:16 2018
> New Revision: 338489
>
> URL: http://llvm.org/viewvc/llvm-project?rev=338489&view=rev
> Log:
> [AST] CastExpr: BasePathSize is not large enough.
>
> Summary:
> rC337815 / D49508 had to cannibalize one bit of
> `CastExprBitfields::BasePathSize` in order to squeeze `PartOfExplicitCast`
> boolean.
> That reduced the maximal value of `PartOfExplicitCast` from 9 bits (~512)
> down to 8 bits (~256).
> Apparently, that mattered. Too bad there weren't any tests.
> It caused [[ https://bugs.llvm.org/show_bug.cgi?id=38356 | PR38356 ]].
>
> So we need to increase `PartOfExplicitCast` back at least to 9 bits, or a
> bit more.
> For obvious reasons, we can't do that in `CastExprBitfields` - that would
> blow up the size of every `Expr`.
> So we need to either just add a variable into the `CastExpr` (as done
> here),
> or use `llvm::TrailingObjects`. The latter does not seem to be
> straight-forward.
> Perhaps, that needs to be done not for the `CastExpr` itself, but for all
> of it's `final` children.
>
> Reviewers: rjmccall, rsmith, erichkeane
>
> Reviewed By: rjmccall
>
> Subscribers: bricci, hans, cfe-commits, waddlesplash
>
> Differential Revision: https://reviews.llvm.org/D50050
>
> Added:
> cfe/trunk/test/CodeGenCXX/castexpr-basepathsize-threshold.cpp
> Modified:
> cfe/trunk/include/clang/AST/Expr.h
> cfe/trunk/include/clang/AST/ExprCXX.h
> cfe/trunk/include/clang/AST/ExprObjC.h
> cfe/trunk/include/clang/AST/Stmt.h
> cfe/trunk/lib/AST/Expr.cpp
> cfe/trunk/lib/AST/ExprCXX.cpp
>
> Modified: cfe/trunk/include/clang/AST/Expr.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Expr.h?rev=338489&r1=338488&r2=338489&view=diff
>
> ==
> --- cfe/trunk/include/clang/AST/Expr.h (original)
> +++ cfe/trunk/include/clang/AST/Expr.h Tue Jul 31 23:06:16 2018
> @@ -2787,20 +2787,26 @@ public:
> /// representation in the source code (ExplicitCastExpr's derived
> /// classes).
> class CastExpr : public Expr {
> +public:
> + using BasePathSizeTy = unsigned int;
> + static_assert(std::numeric_limits::max() >= 16384,
> +"[implimits] Direct and indirect base classes [16384].");
> +
> private:
>Stmt *Op;
>
>bool CastConsistency() const;
>
> + BasePathSizeTy *BasePathSize();
> +
>const CXXBaseSpecifier * const *path_buffer() const {
> return const_cast(this)->path_buffer();
>}
>CXXBaseSpecifier **path_buffer();
>
> - void setBasePathSize(unsigned basePathSize) {
> -CastExprBits.BasePathSize = basePathSize;
> -assert(CastExprBits.BasePathSize == basePathSize &&
> - "basePathSize doesn't fit in bits of
> CastExprBits.BasePathSize!");
> + void setBasePathSize(BasePathSizeTy basePathSize) {
> +assert(!path_empty() && basePathSize != 0);
> +*(BasePathSize()) = basePathSize;
>}
>
> protected:
> @@ -2823,7 +2829,9 @@ protected:
> Op(op) {
> CastExprBits.Kind = kind;
> CastExprBits.PartOfExplicitCast = false;
> -setBasePathSize(BasePathSize);
> +CastExprBits.BasePathIsEmpty = BasePathSize == 0;
> +if (!path_empty())
> + setBasePathSize(BasePathSize);
> assert(CastConsistency());
>}
>
> @@ -2831,7 +2839,9 @@ protected:
>CastExpr(StmtClass SC, EmptyShell Empty, unsigned BasePathSize)
> : Expr(SC, Empty) {
> CastExprBits.PartOfExplicitCast = false;
> -setBasePathSize(BasePathSize);
> +CastExprBits.BasePathIsEmpty = BasePathSize == 0;
> +if (!path_empty())
> + setBasePathSize(BasePathSize);
>}
>
> public:
> @@ -2859,8 +2869,12 @@ public:
>
>typedef CXXBaseSpecifier **path_iterator;
>typedef const CXXBaseSpecifier * const *path_const_iterator;
> - bool path_empty() const { return CastExprBits.BasePathSize == 0; }
> - unsigned path_size() const { return CastExprBits.BasePathSize; }
> + bool path_empty() const { return CastExprBits.BasePathIsEmpty; }
> + unsigned path_size() const {
> +if (path_empty())
> + return 0U;
> +return *(const_cast(this)->BasePathSize());
> + }
>path_iterator path_begin() { return path_buffer(); }
>path_iterator path_end() { return path_buffer() + path_size(); }
>path_const_iterator path_begin() const { return path_buffer(); }
> @@ -2908,7 +2922,12 @@ public:
> /// @endcode
> class ImplicitCastExpr final
> : public CastExpr,
> - private llvm::TrailingObjects
> {
> + private llvm::TrailingObjects CastExpr::BasePathSizeTy,
> +CXXBaseSpecifier *> {
> + size_t numTrailingObjects(OverloadToken)
> const {
> +return path_empty() ? 0 : 1;
> + }
> +
> private:
>ImplicitCastExpr(QualType ty, CastKind kind, Expr *op,
> unsigned BasePathLength, ExprValueKind VK)
> @@ -3013,7 +3032,8 @@ pu
Re: [clang-tools-extra] r338518 - [clangd] Receive compilationDatabasePath in 'initialize' request
Is there a particular reason why this commit didn't have a corresponding
test included?
Cheers,
Alex
On 1 August 2018 at 04:28, Simon Marchi via cfe-commits <
cfe-commits@lists.llvm.org> wrote:
> Author: simark
> Date: Wed Aug 1 04:28:49 2018
> New Revision: 338518
>
> URL: http://llvm.org/viewvc/llvm-project?rev=338518&view=rev
> Log:
> [clangd] Receive compilationDatabasePath in 'initialize' request
>
> Summary:
> That way, as soon as the "initialize" is received by the server, it can
> start
> parsing/indexing with a valid compilation database and not have to wait
> for a
> an initial 'didChangeConfiguration' that might or might not happen.
> Then, when the user changes configuration, a didChangeConfiguration can be
> sent.
>
> Signed-off-by: Marc-Andre Laperle
>
> Reviewers: malaperle
>
> Subscribers: ilya-biryukov, ioeric, MaskRay, jkorous, arphaman, cfe-commits
>
> Differential Revision: https://reviews.llvm.org/D49833
>
> Modified:
> clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
> clang-tools-extra/trunk/clangd/ClangdLSPServer.h
> clang-tools-extra/trunk/clangd/Protocol.cpp
> clang-tools-extra/trunk/clangd/Protocol.h
>
> Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/
> trunk/clangd/ClangdLSPServer.cpp?rev=338518&r1=338517&r2=338518&view=diff
>
> ==
> --- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp (original)
> +++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp Wed Aug 1
> 04:28:49 2018
> @@ -72,6 +72,9 @@ SymbolKindBitset defaultSymbolKinds() {
> } // namespace
>
> void ClangdLSPServer::onInitialize(InitializeParams &Params) {
> + if (Params.initializationOptions)
> +applyConfiguration(*Params.initializationOptions);
> +
>if (Params.rootUri && *Params.rootUri)
> Server.setRootPath(Params.rootUri->file());
>else if (Params.rootPath && !Params.rootPath->empty())
> @@ -398,11 +401,8 @@ void ClangdLSPServer::onHover(TextDocume
> });
> }
>
> -// FIXME: This function needs to be properly tested.
> -void ClangdLSPServer::onChangeConfiguration(
> -DidChangeConfigurationParams &Params) {
> - ClangdConfigurationParamsChange &Settings = Params.settings;
> -
> +void ClangdLSPServer::applyConfiguration(
> +const ClangdConfigurationParamsChange &Settings) {
>// Compilation database change.
>if (Settings.compilationDatabasePath.hasValue()) {
> NonCachedCDB.setCompileCommandsDir(
> @@ -413,6 +413,12 @@ void ClangdLSPServer::onChangeConfigurat
>}
> }
>
> +// FIXME: This function needs to be properly tested.
> +void ClangdLSPServer::onChangeConfiguration(
> +DidChangeConfigurationParams &Params) {
> + applyConfiguration(Params.settings);
> +}
> +
> ClangdLSPServer::ClangdLSPServer(JSONOutput &Out,
> const clangd::CodeCompleteOptions
> &CCOpts,
> llvm::Optional CompileCommandsDir,
>
> Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.h
> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/
> trunk/clangd/ClangdLSPServer.h?rev=338518&r1=338517&r2=338518&view=diff
>
> ==
> --- clang-tools-extra/trunk/clangd/ClangdLSPServer.h (original)
> +++ clang-tools-extra/trunk/clangd/ClangdLSPServer.h Wed Aug 1 04:28:49
> 2018
> @@ -82,6 +82,7 @@ private:
>/// may be very expensive. This method is normally called when the
>/// compilation database is changed.
>void reparseOpenedFiles();
> + void applyConfiguration(const ClangdConfigurationParamsChange
> &Settings);
>
>JSONOutput &Out;
>/// Used to indicate that the 'shutdown' request was received from the
>
> Modified: clang-tools-extra/trunk/clangd/Protocol.cpp
> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/
> trunk/clangd/Protocol.cpp?rev=338518&r1=338517&r2=338518&view=diff
>
> ==
> --- clang-tools-extra/trunk/clangd/Protocol.cpp (original)
> +++ clang-tools-extra/trunk/clangd/Protocol.cpp Wed Aug 1 04:28:49 2018
> @@ -263,7 +263,7 @@ bool fromJSON(const json::Value &Params,
>O.map("rootPath", R.rootPath);
>O.map("capabilities", R.capabilities);
>O.map("trace", R.trace);
> - // initializationOptions, capabilities unused
> + O.map("initializationOptions", R.initializationOptions);
>return true;
> }
>
>
> Modified: clang-tools-extra/trunk/clangd/Protocol.h
> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/
> trunk/clangd/Protocol.h?rev=338518&r1=338517&r2=338518&view=diff
>
> ==
> --- clang-tools-extra/trunk/clangd/Protocol.h (original)
> +++ clang-tools-extra/trunk/clangd/Protocol.h Wed Aug 1 04:28:49 2018
> @@ -322,6 +322,16 @@ struct ClientCapa
[PATCH] D49771: CodeGen: use non-zero memset when possible for automatic variables
joerg added a comment. There are two different considerations here: (1) Create less target code (2) Create less IR If this code can significantly reduce the amount of IR, it can be useful in general. That's why the existing memset logic is helpful. Repository: rL LLVM https://reviews.llvm.org/D49771 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50104: [OpenCL] Always emit alloca in entry block for enqueue_kernel builtin
svenvh added a comment. You'll probably also need to update `test/CodeGenOpenCL/cl20-device-side-enqueue.cl`; please verify with make/ninja `check-clang`. https://reviews.llvm.org/D50104 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50119: Compiler support for P1144R0 "__is_trivially_relocatable(T)"
Rakete added a comment.
- There's a bug in your implementation:
struct X {
X &operator=(X &&);
};
static_assert(__is_trivially_relocatable(X)); // oops, fires!
`X` has a move constructor and a destructor, so it is trivially relocatable.
- Please run your code through clang-format.
- Might be useful to add a note explaining why the type isn't trivially
relocatable isn't of the general "because it is not destructible".
Comment at: include/clang/Sema/Sema.h:4304
+ bool IsTriviallyRelocatableType(QualType QT) const;
+
Any reason why this is a free function? Should be a member function of
`QualType`.
Comment at: lib/AST/DeclCXX.cpp:283
+if (Base->isVirtual() || !BaseClassDecl->isTriviallyRelocatable()) {
+ //puts("because 283");
+ setIsNotNaturallyTriviallyRelocatable();
Lingering debug message? :) There are many of them.
For a single expression, drop the braces of the if statement.
Comment at: lib/Sema/SemaDeclCXX.cpp:6066
+if (M->hasAttr() || Record->hasAttr()) {
+ // Consider removing this case to simplify the Standard wording.
+} else {
This should be a `// TODO: ...`. Is this comment really appropriate? The
intended audience isn't compiler writers I think.
Comment at: lib/Sema/SemaDeclCXX.cpp:6157
+
+ if (getLangOpts().CPlusPlus11 &&
+ !Record->hasAttr() &&
This really just checks whether the type has defaulted copy constructor. If
there was a move constructor, it would have been handled above. If the
copy/move constructor is implicitly deleted, it would have been handled also
above. Please simplify this accordingly.
Comment at: lib/Sema/SemaDeclCXX.cpp:6158
+ if (getLangOpts().CPlusPlus11 &&
+ !Record->hasAttr() &&
+ Record->isTriviallyRelocatable()) {
Why do you need to check whether the attribute is present or not? This is
supposed to be whether the type is naturally trivially relocatable, so the
presence of the attribute is not important.
Comment at: lib/Sema/SemaDeclCXX.cpp:6656
SetDeclDeleted(MD, MD->getLocation());
+ if (CSM == CXXMoveConstructor || CSM == CXXDestructor) {
+//puts("because 6646");
You don't actually need those three lines. This is already handled in
`CheckCompletedCXXClass`.
Comment at: test/SemaCXX/trivially-relocatable.cpp:42
+struct A6;
+struct [[trivially_relocatable]] A6 {};
+// expected-error@-1{{type A6 declared 'trivially_relocatable' after its first
declaration}}
Why does this restriction exist? None of the existing attributes have it and I
don't see why it would make sense to disallow this.
Comment at: test/SemaCXX/trivially-relocatable.cpp:471
+struct Incomplete; // expected-note {{forward declaration of 'Incomplete'}}
+struct Regression1 {
+ Incomplete i; // expected-error {{field has incomplete type 'Incomplete'}}
This is the right place for regression tests. Add them to
test/Parser/cxx-class.cpp.
Repository:
rC Clang
https://reviews.llvm.org/D50119
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
