r331020 - [CodeGen] Avoid destructing a callee-destructued struct type in a
Author: ahatanak
Date: Thu Apr 26 23:57:00 2018
New Revision: 331020
URL: http://llvm.org/viewvc/llvm-project?rev=331020&view=rev
Log:
[CodeGen] Avoid destructing a callee-destructued struct type in a
function if a function delegates to another function.
Fix a bug introduced in r328731, which caused a struct with ObjC __weak
fields that was passed to a function to be destructed twice, once in the
callee function and once in another function the callee function
delegates to. To prevent this, keep track of the callee-destructed
structs passed to a function and disable their cleanups at the point of
the call to the delegated function.
This reapplies r331016, which was reverted in r331019 because it caused
an assertion to fail in EmitDelegateCallArg on a windows bot. I made
changes to EmitDelegateCallArg so that it doesn't try to deactivate
cleanups for structs that have trivial destructors (cleanups for those
structs are never pushed to the cleanup stack in EmitParmDecl).
rdar://problem/39194693
Differential Revision: https://reviews.llvm.org/D45382
Added:
cfe/trunk/test/CodeGenCXX/lambda-to-function-pointer-conversion.cpp
Modified:
cfe/trunk/lib/CodeGen/CGCall.cpp
cfe/trunk/lib/CodeGen/CGCleanup.cpp
cfe/trunk/lib/CodeGen/CGDecl.cpp
cfe/trunk/lib/CodeGen/CodeGenFunction.h
cfe/trunk/test/CodeGenObjCXX/arc-forwarded-lambda-call.mm
cfe/trunk/test/CodeGenObjCXX/arc-special-member-functions.mm
cfe/trunk/test/CodeGenObjCXX/lambda-expressions.mm
Modified: cfe/trunk/lib/CodeGen/CGCall.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCall.cpp?rev=331020&r1=331019&r2=331020&view=diff
==
--- cfe/trunk/lib/CodeGen/CGCall.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGCall.cpp Thu Apr 26 23:57:00 2018
@@ -3063,6 +3063,18 @@ void CodeGenFunction::EmitDelegateCallAr
} else {
args.add(convertTempToRValue(local, type, loc), type);
}
+
+ // Deactivate the cleanup for the callee-destructed param that was pushed.
+ if (hasAggregateEvaluationKind(type) && !CurFuncIsThunk &&
+ getContext().isParamDestroyedInCallee(type) && type.isDestructedType()) {
+EHScopeStack::stable_iterator cleanup =
+CalleeDestructedParamCleanups.lookup(cast(param));
+assert(cleanup.isValid() &&
+ "cleanup for callee-destructed param not recorded");
+// This unreachable is a temporary marker which will be removed later.
+llvm::Instruction *isActive = Builder.CreateUnreachable();
+args.addArgCleanupDeactivation(cleanup, isActive);
+ }
}
static bool isProvablyNull(llvm::Value *addr) {
Modified: cfe/trunk/lib/CodeGen/CGCleanup.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCleanup.cpp?rev=331020&r1=331019&r2=331020&view=diff
==
--- cfe/trunk/lib/CodeGen/CGCleanup.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGCleanup.cpp Thu Apr 26 23:57:00 2018
@@ -1233,8 +1233,10 @@ void CodeGenFunction::DeactivateCleanupB
EHCleanupScope &Scope = cast(*EHStack.find(C));
assert(Scope.isActive() && "double deactivation");
- // If it's the top of the stack, just pop it.
- if (C == EHStack.stable_begin()) {
+ // If it's the top of the stack, just pop it, but do so only if it belongs
+ // to the current RunCleanupsScope.
+ if (C == EHStack.stable_begin() &&
+ CurrentCleanupScopeDepth.strictlyEncloses(C)) {
// If it's a normal cleanup, we need to pretend that the
// fallthrough is unreachable.
CGBuilderTy::InsertPoint SavedIP = Builder.saveAndClearIP();
Modified: cfe/trunk/lib/CodeGen/CGDecl.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDecl.cpp?rev=331020&r1=331019&r2=331020&view=diff
==
--- cfe/trunk/lib/CodeGen/CGDecl.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGDecl.cpp Thu Apr 26 23:57:00 2018
@@ -1962,6 +1962,8 @@ void CodeGenFunction::EmitParmDecl(const
DtorKind == QualType::DK_nontrivial_c_struct) &&
"unexpected destructor type");
pushDestroy(DtorKind, DeclPtr, Ty);
+CalleeDestructedParamCleanups[cast(&D)] =
+EHStack.stable_begin();
}
}
} else {
Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.h?rev=331020&r1=331019&r2=331020&view=diff
==
--- cfe/trunk/lib/CodeGen/CodeGenFunction.h (original)
+++ cfe/trunk/lib/CodeGen/CodeGenFunction.h Thu Apr 26 23:57:00 2018
@@ -587,7 +587,7 @@ public:
/// \brief Enters a new scope for capturing cleanups, all of which
/// will be executed once the scope is exited.
class RunCleanupsScope {
-EHScopeStack::stable_iterator CleanupStackDepth;
+EHScopeStack::stable_iterator CleanupSt
r331021 - Make MultiplexASTDeserializationListener part of the API [NFC]
Author: teemperor
Date: Fri Apr 27 00:05:40 2018
New Revision: 331021
URL: http://llvm.org/viewvc/llvm-project?rev=331021&view=rev
Log:
Make MultiplexASTDeserializationListener part of the API [NFC]
Summary:
This patch moves the MultiplexASTDeserializationListener declaration into a
public header.
We're currently using this multiplexer in the cling interpreter to attach
another
ASTDeserializationListener during the execution (so, after the
MultiplexConsumer is already
attached which prevents us from attaching more). So far we're doing this by
patching clang
and making this class public, but it makes things easier if we make this
instead just public in
upstream.
Reviewers: thakis, v.g.vassilev, rsmith, bruno
Reviewed By: bruno
Subscribers: llvm-commits, cfe-commits, v.g.vassilev
Differential Revision: https://reviews.llvm.org/D37475
Modified:
cfe/trunk/include/clang/Frontend/MultiplexConsumer.h
cfe/trunk/lib/Frontend/MultiplexConsumer.cpp
Modified: cfe/trunk/include/clang/Frontend/MultiplexConsumer.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/MultiplexConsumer.h?rev=331021&r1=331020&r2=331021&view=diff
==
--- cfe/trunk/include/clang/Frontend/MultiplexConsumer.h (original)
+++ cfe/trunk/include/clang/Frontend/MultiplexConsumer.h Fri Apr 27 00:05:40
2018
@@ -17,13 +17,34 @@
#include "clang/Basic/LLVM.h"
#include "clang/Sema/SemaConsumer.h"
+#include "clang/Serialization/ASTDeserializationListener.h"
#include
#include
namespace clang {
class MultiplexASTMutationListener;
-class MultiplexASTDeserializationListener;
+
+// This ASTDeserializationListener forwards its notifications to a set of
+// child listeners.
+class MultiplexASTDeserializationListener : public ASTDeserializationListener {
+public:
+ // Does NOT take ownership of the elements in L.
+ MultiplexASTDeserializationListener(
+ const std::vector &L);
+ void ReaderInitialized(ASTReader *Reader) override;
+ void IdentifierRead(serialization::IdentID ID, IdentifierInfo *II) override;
+ void MacroRead(serialization::MacroID ID, MacroInfo *MI) override;
+ void TypeRead(serialization::TypeIdx Idx, QualType T) override;
+ void DeclRead(serialization::DeclID ID, const Decl *D) override;
+ void SelectorRead(serialization::SelectorID iD, Selector Sel) override;
+ void MacroDefinitionRead(serialization::PreprocessedEntityID,
+ MacroDefinitionRecord *MD) override;
+ void ModuleRead(serialization::SubmoduleID ID, Module *Mod) override;
+
+private:
+ std::vector Listeners;
+};
// Has a list of ASTConsumers and calls each of them. Owns its children.
class MultiplexConsumer : public SemaConsumer {
Modified: cfe/trunk/lib/Frontend/MultiplexConsumer.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/MultiplexConsumer.cpp?rev=331021&r1=331020&r2=331021&view=diff
==
--- cfe/trunk/lib/Frontend/MultiplexConsumer.cpp (original)
+++ cfe/trunk/lib/Frontend/MultiplexConsumer.cpp Fri Apr 27 00:05:40 2018
@@ -16,35 +16,11 @@
#include "clang/Frontend/MultiplexConsumer.h"
#include "clang/AST/ASTMutationListener.h"
#include "clang/AST/DeclGroup.h"
-#include "clang/Serialization/ASTDeserializationListener.h"
using namespace clang;
namespace clang {
-// This ASTDeserializationListener forwards its notifications to a set of
-// child listeners.
-class MultiplexASTDeserializationListener
-: public ASTDeserializationListener {
-public:
- // Does NOT take ownership of the elements in L.
- MultiplexASTDeserializationListener(
- const std::vector& L);
- void ReaderInitialized(ASTReader *Reader) override;
- void IdentifierRead(serialization::IdentID ID,
- IdentifierInfo *II) override;
- void MacroRead(serialization::MacroID ID, MacroInfo *MI) override;
- void TypeRead(serialization::TypeIdx Idx, QualType T) override;
- void DeclRead(serialization::DeclID ID, const Decl *D) override;
- void SelectorRead(serialization::SelectorID iD, Selector Sel) override;
- void MacroDefinitionRead(serialization::PreprocessedEntityID,
- MacroDefinitionRecord *MD) override;
- void ModuleRead(serialization::SubmoduleID ID, Module *Mod) override;
-
-private:
- std::vector Listeners;
-};
-
MultiplexASTDeserializationListener::MultiplexASTDeserializationListener(
const std::vector& L)
: Listeners(L) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37475: Make MultiplexASTDeserializationListener part of the API [NFC]
This revision was automatically updated to reflect the committed changes.
Closed by commit rL331021: Make MultiplexASTDeserializationListener part of the
API [NFC] (authored by teemperor, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D37475?vs=113847&id=144287#toc
Repository:
rL LLVM
https://reviews.llvm.org/D37475
Files:
cfe/trunk/include/clang/Frontend/MultiplexConsumer.h
cfe/trunk/lib/Frontend/MultiplexConsumer.cpp
Index: cfe/trunk/include/clang/Frontend/MultiplexConsumer.h
===
--- cfe/trunk/include/clang/Frontend/MultiplexConsumer.h
+++ cfe/trunk/include/clang/Frontend/MultiplexConsumer.h
@@ -17,13 +17,34 @@
#include "clang/Basic/LLVM.h"
#include "clang/Sema/SemaConsumer.h"
+#include "clang/Serialization/ASTDeserializationListener.h"
#include
#include
namespace clang {
class MultiplexASTMutationListener;
-class MultiplexASTDeserializationListener;
+
+// This ASTDeserializationListener forwards its notifications to a set of
+// child listeners.
+class MultiplexASTDeserializationListener : public ASTDeserializationListener {
+public:
+ // Does NOT take ownership of the elements in L.
+ MultiplexASTDeserializationListener(
+ const std::vector &L);
+ void ReaderInitialized(ASTReader *Reader) override;
+ void IdentifierRead(serialization::IdentID ID, IdentifierInfo *II) override;
+ void MacroRead(serialization::MacroID ID, MacroInfo *MI) override;
+ void TypeRead(serialization::TypeIdx Idx, QualType T) override;
+ void DeclRead(serialization::DeclID ID, const Decl *D) override;
+ void SelectorRead(serialization::SelectorID iD, Selector Sel) override;
+ void MacroDefinitionRead(serialization::PreprocessedEntityID,
+ MacroDefinitionRecord *MD) override;
+ void ModuleRead(serialization::SubmoduleID ID, Module *Mod) override;
+
+private:
+ std::vector Listeners;
+};
// Has a list of ASTConsumers and calls each of them. Owns its children.
class MultiplexConsumer : public SemaConsumer {
Index: cfe/trunk/lib/Frontend/MultiplexConsumer.cpp
===
--- cfe/trunk/lib/Frontend/MultiplexConsumer.cpp
+++ cfe/trunk/lib/Frontend/MultiplexConsumer.cpp
@@ -16,35 +16,11 @@
#include "clang/Frontend/MultiplexConsumer.h"
#include "clang/AST/ASTMutationListener.h"
#include "clang/AST/DeclGroup.h"
-#include "clang/Serialization/ASTDeserializationListener.h"
using namespace clang;
namespace clang {
-// This ASTDeserializationListener forwards its notifications to a set of
-// child listeners.
-class MultiplexASTDeserializationListener
-: public ASTDeserializationListener {
-public:
- // Does NOT take ownership of the elements in L.
- MultiplexASTDeserializationListener(
- const std::vector& L);
- void ReaderInitialized(ASTReader *Reader) override;
- void IdentifierRead(serialization::IdentID ID,
- IdentifierInfo *II) override;
- void MacroRead(serialization::MacroID ID, MacroInfo *MI) override;
- void TypeRead(serialization::TypeIdx Idx, QualType T) override;
- void DeclRead(serialization::DeclID ID, const Decl *D) override;
- void SelectorRead(serialization::SelectorID iD, Selector Sel) override;
- void MacroDefinitionRead(serialization::PreprocessedEntityID,
- MacroDefinitionRecord *MD) override;
- void ModuleRead(serialization::SubmoduleID ID, Module *Mod) override;
-
-private:
- std::vector Listeners;
-};
-
MultiplexASTDeserializationListener::MultiplexASTDeserializationListener(
const std::vector& L)
: Listeners(L) {
Index: cfe/trunk/include/clang/Frontend/MultiplexConsumer.h
===
--- cfe/trunk/include/clang/Frontend/MultiplexConsumer.h
+++ cfe/trunk/include/clang/Frontend/MultiplexConsumer.h
@@ -17,13 +17,34 @@
#include "clang/Basic/LLVM.h"
#include "clang/Sema/SemaConsumer.h"
+#include "clang/Serialization/ASTDeserializationListener.h"
#include
#include
namespace clang {
class MultiplexASTMutationListener;
-class MultiplexASTDeserializationListener;
+
+// This ASTDeserializationListener forwards its notifications to a set of
+// child listeners.
+class MultiplexASTDeserializationListener : public ASTDeserializationListener {
+public:
+ // Does NOT take ownership of the elements in L.
+ MultiplexASTDeserializationListener(
+ const std::vector &L);
+ void ReaderInitialized(ASTReader *Reader) override;
+ void IdentifierRead(serialization::IdentID ID, IdentifierInfo *II) override;
+ void MacroRead(serialization::MacroID ID, MacroInfo *MI) override;
+ void TypeRead(serialization::TypeIdx Idx, QualType T) override;
+ void DeclRead(serialization::DeclID ID, const Decl *D) override;
+ void SelectorRead(serialization::SelectorID iD, Selector Sel) override;
+ void MacroDefinitionRead(serialization::PreprocessedEntityID,
+
[PATCH] D44934: [analyzer] Improve the modeling of `memset()`.
MTC added a comment. ping^2 Repository: rC Clang https://reviews.llvm.org/D44934 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46176: Pull helper class Environment from clangFormat into libToolingCore [NFC]
ioeric created this revision.
ioeric added a reviewer: djasper.
Herald added subscribers: cfe-commits, mgorny, klimek.
This can be used to create a virtual environment (incl. VFS, source manager) for
code snippets. The existing clang::format::Environment is implemented based
on the new clang::tooling::Environment with additional formatting specific
environment.
Repository:
rC Clang
https://reviews.llvm.org/D46176
Files:
include/clang/Tooling/Core/Environment.h
lib/Format/Format.cpp
lib/Format/SortJavaScriptImports.cpp
lib/Format/TokenAnalyzer.cpp
lib/Format/TokenAnalyzer.h
lib/Tooling/Core/CMakeLists.txt
lib/Tooling/Core/Environment.cpp
Index: lib/Tooling/Core/Environment.cpp
===
--- /dev/null
+++ lib/Tooling/Core/Environment.cpp
@@ -0,0 +1,47 @@
+//===--- Environment.cpp - Sourece tooling environment --*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "clang/Tooling/Core/Environment.h"
+
+namespace clang {
+namespace tooling {
+
+std::unique_ptr
+Environment::createVirtualEnvironment(StringRef Content, StringRef FileName) {
+ // This is referenced by `FileMgr` and will be released by `FileMgr` when it
+ // is deleted.
+ IntrusiveRefCntPtr InMemoryFileSystem(
+ new vfs::InMemoryFileSystem);
+ // This is passed to `SM` as reference, so the pointer has to be referenced
+ // in `Environment` so that `FileMgr` can out-live this function scope.
+ std::unique_ptr FileMgr(
+ new FileManager(FileSystemOptions(), InMemoryFileSystem));
+ // This is passed to `SM` as reference, so the pointer has to be referenced
+ // by `Environment` due to the same reason above.
+ std::unique_ptr Diagnostics(new DiagnosticsEngine(
+ IntrusiveRefCntPtr(new DiagnosticIDs),
+ new DiagnosticOptions));
+ // This will be stored as reference, so the pointer has to be stored in
+ // due to the same reason above.
+ std::unique_ptr VirtualSM(
+ new SourceManager(*Diagnostics, *FileMgr));
+ InMemoryFileSystem->addFile(
+ FileName, 0,
+ llvm::MemoryBuffer::getMemBuffer(Content, FileName,
+ /*RequiresNullTerminator=*/false));
+ FileID ID = VirtualSM->createFileID(FileMgr->getFile(FileName),
+ SourceLocation(), clang::SrcMgr::C_User);
+ assert(ID.isValid());
+
+ return llvm::make_unique(
+ ID, std::move(FileMgr), std::move(VirtualSM), std::move(Diagnostics));
+}
+
+} // namespace tooling
+} // namespace clang
Index: lib/Tooling/Core/CMakeLists.txt
===
--- lib/Tooling/Core/CMakeLists.txt
+++ lib/Tooling/Core/CMakeLists.txt
@@ -1,9 +1,10 @@
set(LLVM_LINK_COMPONENTS support)
add_clang_library(clangToolingCore
+ Diagnostic.cpp
+ Environment.cpp
Lookup.cpp
Replacement.cpp
- Diagnostic.cpp
LINK_LIBS
clangAST
Index: lib/Format/TokenAnalyzer.h
===
--- lib/Format/TokenAnalyzer.h
+++ lib/Format/TokenAnalyzer.h
@@ -28,6 +28,7 @@
#include "clang/Basic/FileManager.h"
#include "clang/Basic/SourceManager.h"
#include "clang/Format/Format.h"
+#include "clang/Tooling/Core/Environment.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/Support/Debug.h"
@@ -37,43 +38,26 @@
class Environment {
public:
Environment(SourceManager &SM, FileID ID, ArrayRef Ranges)
- : ID(ID), CharRanges(Ranges.begin(), Ranges.end()), SM(SM),
- FirstStartColumn(0),
- NextStartColumn(0),
- LastStartColumn(0) {}
-
- Environment(FileID ID, std::unique_ptr FileMgr,
- std::unique_ptr VirtualSM,
- std::unique_ptr Diagnostics,
- const std::vector &CharRanges,
- unsigned FirstStartColumn,
- unsigned NextStartColumn,
- unsigned LastStartColumn)
- : ID(ID), CharRanges(CharRanges.begin(), CharRanges.end()),
-SM(*VirtualSM),
-FirstStartColumn(FirstStartColumn),
-NextStartColumn(NextStartColumn),
-LastStartColumn(LastStartColumn),
-FileMgr(std::move(FileMgr)),
-VirtualSM(std::move(VirtualSM)), Diagnostics(std::move(Diagnostics)) {}
+ : ToolEnv(new tooling::Environment(SM, ID)),
+CharRanges(Ranges.begin(), Ranges.end()), FirstStartColumn(0),
+NextStartColumn(0), LastStartColumn(0) {}
// This sets up an virtual file system with file \p FileName containing the
// fragment \p Code. Assumes that \p Code starts at \p FirstStartColumn,
// that the next lines of \p Code should start at \p NextStartColumn, and
// that \p Code should end at \p LastStartColumn if it ends in newline.
// See also the documenta
[PATCH] D46159: [clang-tidy] Add a flag to enable alpha checkers
lebedev.ri added a comment. I just feel like pointing out that you can already do that: $ clang-tidy -checks=clang-analyzer-alpha* -p <> (be wary of `*` expanding) `clang-tidy --help` says: -checks= - <...> This option's value is appended to the value of the 'Checks' option in .clang-tidy file, if any. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D46159 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45931: [ASTMatchers] Don't garble the profiling output when multiple TU's are processed
lebedev.ri added a comment. Ping? Repository: rC Clang https://reviews.llvm.org/D45931 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46180: [clang-format] Refactor #include insertion/deletion functionality into a class.
ioeric created this revision.
ioeric added a reviewer: ilya-biryukov.
Herald added subscribers: cfe-commits, klimek.
The class will be moved into libToolingCore as followup.
The new behaviors in this patch:
- New #include is inserted in the right position in a #include block to
preserver sorted #includes. This is best effort - only works when the
block is already sorted.
- When inserting multiple #includes to the end of a file which doesn't
end with a "\n" character, a "\n" will be prepended to each #include.
This is a special and rare case that was previously handled. This is now
relaxed to avoid complexity as it's rare in practice.
Repository:
rC Clang
https://reviews.llvm.org/D46180
Files:
lib/Format/Format.cpp
unittests/Format/CleanupTest.cpp
Index: unittests/Format/CleanupTest.cpp
===
--- unittests/Format/CleanupTest.cpp
+++ unittests/Format/CleanupTest.cpp
@@ -471,19 +471,77 @@
EXPECT_EQ(Expected, apply(Code, Replaces));
}
+TEST_F(CleanUpReplacementsTest, InsertIntoBlockSorted) {
+ std::string Code = "#include \"x/fix.h\"\n"
+ "#include \"a.h\"\n"
+ "#include \"c.h\"\n"
+ "#include \n";
+ std::string Expected = "#include \"x/fix.h\"\n"
+ "#include \"a.h\"\n"
+ "#include \"b.h\"\n"
+ "#include \"c.h\"\n"
+ "#include \n";
+ tooling::Replacements Replaces =
+ toReplacements({createInsertion("#include \"b.h\"")});
+ EXPECT_EQ(Expected, apply(Code, Replaces));
+}
+
+TEST_F(CleanUpReplacementsTest, InsertIntoFirstBlockOfSameKind) {
+ std::string Code = "#include \"x/fix.h\"\n"
+ "#include \"c.h\"\n"
+ "#include \"e.h\"\n"
+ "#include \"f.h\"\n"
+ "#include \n"
+ "#include \n"
+ "#include \"m.h\"\n"
+ "#include \"n.h\"\n";
+ std::string Expected = "#include \"x/fix.h\"\n"
+ "#include \"c.h\"\n"
+ "#include \"d.h\"\n"
+ "#include \"e.h\"\n"
+ "#include \"f.h\"\n"
+ "#include \n"
+ "#include \n"
+ "#include \"m.h\"\n"
+ "#include \"n.h\"\n";
+ tooling::Replacements Replaces =
+ toReplacements({createInsertion("#include \"d.h\"")});
+ EXPECT_EQ(Expected, apply(Code, Replaces));
+}
+
+TEST_F(CleanUpReplacementsTest, InsertIntoSystemBlockSorted) {
+ std::string Code = "#include \"x/fix.h\"\n"
+ "#include \"a.h\"\n"
+ "#include \"c.h\"\n"
+ "#include \n"
+ "#include \n";
+ std::string Expected = "#include \"x/fix.h\"\n"
+ "#include \"a.h\"\n"
+ "#include \"c.h\"\n"
+ "#include \n"
+ "#include \n"
+ "#include \n";
+ tooling::Replacements Replaces =
+ toReplacements({createInsertion("#include ")});
+ EXPECT_EQ(Expected, apply(Code, Replaces));
+}
+
+
TEST_F(CleanUpReplacementsTest, InsertMultipleIncludesLLVMStyle) {
std::string Code = "#include \"x/fix.h\"\n"
"#include \"a.h\"\n"
"#include \"b.h\"\n"
+ "#include \"z.h\"\n"
"#include \"clang/Format/Format.h\"\n"
"#include \n";
std::string Expected = "#include \"x/fix.h\"\n"
"#include \"a.h\"\n"
"#include \"b.h\"\n"
"#include \"new/new.h\"\n"
+ "#include \"z.h\"\n"
"#include \"clang/Format/Format.h\"\n"
- "#include \n"
- "#include \n";
+ "#include \n"
+ "#include \n";
tooling::Replacements Replaces =
toReplacements({createInsertion("#include "),
createInsertion("#include \"new/new.h\"")});
@@ -517,12 +575,12 @@
"#include \"z/b.h\"\n";
std::string Expected = "#include \"x/fix.h\"\n"
"\n"
- "#include \n"
"#include \n"
+ "#include \n"
"\n"
+ "#include \"x/x.h\"\n"
"#include \"y/a.h\"\n"
- "#include \"z/b.h\"\n"
- "#include \"x/x.h\"\n";
+ "#include \"z/b.h\"\n";
tooling::Replacements Replaces =
toReplacements({createInsertion("#include "),
createInsertion("#include \"x/x.h\"")});
@@ -776,8 +834,10 @@
TEST_F(CleanUpR
[PATCH] D46180: [clang-format] Refactor #include insertion/deletion functionality into a class.
ioeric added a comment. There isn't actually as much code changed as it appears to be, but phabricator doens't understand the diff very well... `git diff` might be an easier way to review the patch. Repository: rC Clang https://reviews.llvm.org/D46180 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42966: Fix USR generation in the presence of #line directives or linemarkes
sammccall added a comment. Re: locations in parameter USRs: OK, so reading through the code, it looks like locations are included in USRs: - for macros (except from system headers) - for decls that aren't externally visible (static etc, function parameters, locals) - an objective-c class extension case I don't really understand After thinking about this a bit, and use cases like rename and find-declaration that could be USR based, I think including some location information is the right way to go, which I think is the current behavior. (I think for the patch itself we're waiting on details about the case that doesn't currently work?) Repository: rC Clang https://reviews.llvm.org/D42966 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44609: [Clang-Format] New option BreakBeforeLambdaBody to manage lambda line break inside function parameter call
Wawha added a comment. Hi klimek, many thank for your comments. I will made the modifications you propose and then update this patch. Repository: rC Clang https://reviews.llvm.org/D44609 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45815: [libclang] Add options to limit skipping of function bodies
nik updated this revision to Diff 144303.
nik added a comment.
Reduction to skip-in-preamble-only functionality.
Repository:
rC Clang
https://reviews.llvm.org/D45815
Files:
include/clang-c/Index.h
include/clang/Frontend/ASTUnit.h
lib/Frontend/ASTUnit.cpp
test/Parser/skip-function-bodies.h
test/Parser/skip-function-bodies.mm
tools/c-index-test/c-index-test.c
tools/libclang/CIndex.cpp
unittests/Frontend/PCHPreambleTest.cpp
Index: unittests/Frontend/PCHPreambleTest.cpp
===
--- unittests/Frontend/PCHPreambleTest.cpp
+++ unittests/Frontend/PCHPreambleTest.cpp
@@ -103,7 +103,9 @@
}
bool ReparseAST(const std::unique_ptr &AST) {
-bool reparseFailed = AST->Reparse(PCHContainerOpts, GetRemappedFiles(), VFS);
+bool reparseFailed =
+AST->Reparse(PCHContainerOpts, GetRemappedFiles(),
+ ASTUnit::SkipFunctionBodiesScope::None, VFS);
return !reparseFailed;
}
Index: tools/libclang/CIndex.cpp
===
--- tools/libclang/CIndex.cpp
+++ tools/libclang/CIndex.cpp
@@ -3346,6 +3346,19 @@
Options);
}
+static ASTUnit::SkipFunctionBodiesScope
+skipFunctionBodiesScopeFromParseOptions(unsigned options) {
+ ASTUnit::SkipFunctionBodiesScope SkipFunctionBodiesScp =
+ ASTUnit::SkipFunctionBodiesScope::None;
+ if (options & CXTranslationUnit_SkipFunctionBodies) {
+SkipFunctionBodiesScp =
+(options & CXTranslationUnit_LimitSkipFunctionBodiesToPreamble)
+? ASTUnit::SkipFunctionBodiesScope::Preamble
+: ASTUnit::SkipFunctionBodiesScope::MainFileAndPreamble;
+ }
+ return SkipFunctionBodiesScp;
+}
+
static CXErrorCode
clang_parseTranslationUnit_Impl(CXIndex CIdx, const char *source_filename,
const char *const *command_line_args,
@@ -3376,9 +3389,10 @@
= options & CXTranslationUnit_CacheCompletionResults;
bool IncludeBriefCommentsInCodeCompletion
= options & CXTranslationUnit_IncludeBriefCommentsInCodeCompletion;
- bool SkipFunctionBodies = options & CXTranslationUnit_SkipFunctionBodies;
bool SingleFileParse = options & CXTranslationUnit_SingleFileParse;
bool ForSerialization = options & CXTranslationUnit_ForSerialization;
+ ASTUnit::SkipFunctionBodiesScope SkipFunctionBodiesScp =
+ skipFunctionBodiesScopeFromParseOptions(options);
// Configure the diagnostics.
IntrusiveRefCntPtr
@@ -3467,7 +3481,7 @@
/*CaptureDiagnostics=*/true, *RemappedFiles.get(),
/*RemappedFilesKeepOriginalName=*/true, PrecompilePreambleAfterNParses,
TUKind, CacheCodeCompletionResults, IncludeBriefCommentsInCodeCompletion,
- /*AllowPCHWithCompilerErrors=*/true, SkipFunctionBodies, SingleFileParse,
+ /*AllowPCHWithCompilerErrors=*/true, SkipFunctionBodiesScp, SingleFileParse,
/*UserFilesAreVolatile=*/true, ForSerialization,
CXXIdx->getPCHContainerOperations()->getRawReader().getFormat(),
&ErrUnit));
@@ -4054,8 +4068,9 @@
RemappedFiles->push_back(std::make_pair(UF.Filename, MB.release()));
}
- if (!CXXUnit->Reparse(CXXIdx->getPCHContainerOperations(),
-*RemappedFiles.get()))
+ if (!CXXUnit->Reparse(
+ CXXIdx->getPCHContainerOperations(), *RemappedFiles.get(),
+ skipFunctionBodiesScopeFromParseOptions(TU->ParsingOptions)))
return CXError_Success;
if (isASTReadError(CXXUnit))
return CXError_ASTReadError;
Index: tools/c-index-test/c-index-test.c
===
--- tools/c-index-test/c-index-test.c
+++ tools/c-index-test/c-index-test.c
@@ -82,6 +82,8 @@
options |= CXTranslationUnit_CreatePreambleOnFirstParse;
if (getenv("CINDEXTEST_KEEP_GOING"))
options |= CXTranslationUnit_KeepGoing;
+ if (getenv("CINDEXTEST_LIMIT_SKIP_FUNCTION_BODIES_TO_PREAMBLE"))
+options |= CXTranslationUnit_LimitSkipFunctionBodiesToPreamble;
return options;
}
Index: test/Parser/skip-function-bodies.mm
===
--- test/Parser/skip-function-bodies.mm
+++ test/Parser/skip-function-bodies.mm
@@ -1,4 +1,4 @@
-// RUN: env CINDEXTEST_SKIP_FUNCTION_BODIES=1 c-index-test -test-load-source all %s | FileCheck %s
+#include "skip-function-bodies.h"
class A {
class B {};
@@ -27,6 +27,7 @@
class K {};
}
+// RUN: env CINDEXTEST_SKIP_FUNCTION_BODIES=1 c-index-test -test-load-source all %s | FileCheck %s
// CHECK: skip-function-bodies.mm:3:7: ClassDecl=A:3:7 (Definition) Extent=[3:1 - 14:2]
// CHECK: skip-function-bodies.mm:4:9: ClassDecl=B:4:9 (Definition) Extent=[4:3 - 4:13]
// CHECK: skip-function-bodies.mm:6:1: CXXAccessSpecifier=:6:1 (Definition) Extent=[6:1 - 6:8]
@@ -43,3 +44,13 @@
// CHECK-NOT: skip-function-bodies.mm:21:11: TypeRef=class A:3:7 Extent=[21:11 - 21:12]
// CHECK: skip-function-bodies.mm:
[PATCH] D45815: [libclang] Add options to limit skipping of function bodies
nik added a comment.
In https://reviews.llvm.org/D45815#1079327, @ilya-biryukov wrote:
> > OK, I've rechecked this change. I don't see any obvious mistake :)
>
> I think I got to the bottom of it. We didn't expect a big win, because we
> expect people to not put their non-template code into the header files. Which
> is probably true.
> The problem with our reasoning, is that this patch also skip bodies of
> **non-template functions inside template classes**, e.g.
>
> template
> struct vector {
> template
> void append(T* pos, It begin, It end) {
> / * This function won't be skipped, it is a template. */
> }
>
> void push_back(T const&) {
> /* However, this function will be skipped! It is not a template! */
> }
> };
>
>
> So it's not surprising that we get a big win. Template function are
> (probably) much more rare than non-template functions inside templates.
Oh, right. That makes sense. I didn't have a test for this case and thus didn't
notice.
> However, note that we will still miss a lot of diagnostics if we skip bodies
> of non-template functions inside templates.
> So I don't see much value in an option like this: it still compromises
> correctness of diagnostics even inside the main file, while still missing out
> some performance opportunities that come from skipping the template
> functions. And it makes the code more complex.
>
> We should either have an option that **does not** skip functions inside
> template classes or keep the current boolean value.
> We should definitely measure the performance impact of having such an option
> before adding more complexity to the code.
> WDYT?
>
> BTW, maybe split this change out into two patches: one that allows to skip
> function bodies only in the preamble, the other that turns SkipFunctionBodies
> boolean into a enum and changes parser, etc.
> The ASTUnit changes LG and we could probably LGTM them sooner than the
> parser changes.
Agree. I'll first reduce this change to the skip-in-preamble-only functionality
and then will check some numbers with the new case.
Repository:
rC Clang
https://reviews.llvm.org/D45815
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46049: [OpenCL] Add constant address space to __func__ in AST
Anastasia added inline comments. Comment at: lib/AST/Expr.cpp:870 + if (C.getLangOpts().OpenCL && Ty.getAddressSpace() == LangAS::Default) +Ty = C.getAddrSpaceQualType(Ty, LangAS::opencl_constant); + bader wrote: > As `Ty` is passed by value, shouldn't we accept only data located in constant > address space? Do you mean to assert? Currently it should be passed with `constant` AS but I thought the idea is to modify this function so we can accept `Default` AS too but then replace by `constant`. Comment at: lib/Sema/SemaExpr.cpp:3057 +ResTy = Context.getAddrSpaceQualType(ResTy, LangAS::opencl_constant); ResTy = Context.getConstantArrayType(ResTy, LengthI, ArrayType::Normal, /*IndexTypeQuals*/ 0); bader wrote: > String type can only be a constant arrays. > Can we set constant address space inside this getter for OpenCL language? > Or we might want constant array in other address spaces e.g. private? Yes, I think we need to be able to declare const arrays in different ASes. I.e. this is valid code in OpenCL: __private const int* a = ...; Quick check for `getConstantArrayType`, it seems to be used in many different place. So I don't think I can just modify it. https://reviews.llvm.org/D46049 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r331026 - [OpenCL] Add separate read_only and write_only pipe IR types
Author: svenvh
Date: Fri Apr 27 03:37:04 2018
New Revision: 331026
URL: http://llvm.org/viewvc/llvm-project?rev=331026&view=rev
Log:
[OpenCL] Add separate read_only and write_only pipe IR types
SPIR-V encodes the read_only and write_only access qualifiers of pipes,
so separate LLVM IR types are required to target SPIR-V. Other backends
may also find this useful.
These new types are `opencl.pipe_ro_t` and `opencl.pipe_wo_t`, which
replace `opencl.pipe_t`.
This replaces __get_pipe_num_packets(...) and __get_pipe_max_packets(...)
which took a read_only pipe with separate versions for read_only and
write_only pipes, namely:
* __get_pipe_num_packets_ro(...)
* __get_pipe_num_packets_wo(...)
* __get_pipe_max_packets_ro(...)
* __get_pipe_max_packets_wo(...)
These separate versions exist to avoid needing a bitcast to one of the
two qualified pipe types.
Patch by Stuart Brady.
Differential Revision: https://reviews.llvm.org/D46015
Modified:
cfe/trunk/lib/CodeGen/CGBuiltin.cpp
cfe/trunk/lib/CodeGen/CGOpenCLRuntime.cpp
cfe/trunk/lib/CodeGen/CGOpenCLRuntime.h
cfe/trunk/test/CodeGenOpenCL/opencl_types.cl
cfe/trunk/test/CodeGenOpenCL/pipe_builtin.cl
cfe/trunk/test/CodeGenOpenCL/pipe_types.cl
cfe/trunk/test/Index/pipe-size.cl
Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBuiltin.cpp?rev=331026&r1=331025&r2=331026&view=diff
==
--- cfe/trunk/lib/CodeGen/CGBuiltin.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp Fri Apr 27 03:37:04 2018
@@ -3051,11 +3051,14 @@ RValue CodeGenFunction::EmitBuiltinExpr(
// OpenCL v2.0 s6.13.16.4 Built-in pipe query functions
case Builtin::BIget_pipe_num_packets:
case Builtin::BIget_pipe_max_packets: {
-const char *Name;
+const char *BaseName;
+const PipeType *PipeTy = E->getArg(0)->getType()->getAs();
if (BuiltinID == Builtin::BIget_pipe_num_packets)
- Name = "__get_pipe_num_packets";
+ BaseName = "__get_pipe_num_packets";
else
- Name = "__get_pipe_max_packets";
+ BaseName = "__get_pipe_max_packets";
+auto Name = std::string(BaseName) +
+std::string(PipeTy->isReadOnly() ? "_ro" : "_wo");
// Building the generic function prototype.
Value *Arg0 = EmitScalarExpr(E->getArg(0));
Modified: cfe/trunk/lib/CodeGen/CGOpenCLRuntime.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenCLRuntime.cpp?rev=331026&r1=331025&r2=331026&view=diff
==
--- cfe/trunk/lib/CodeGen/CGOpenCLRuntime.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGOpenCLRuntime.cpp Fri Apr 27 03:37:04 2018
@@ -66,13 +66,19 @@ llvm::Type *CGOpenCLRuntime::convertOpen
}
llvm::Type *CGOpenCLRuntime::getPipeType(const PipeType *T) {
- if (!PipeTy){
-uint32_t PipeAddrSpc = CGM.getContext().getTargetAddressSpace(
-CGM.getContext().getOpenCLTypeAddrSpace(T));
-PipeTy = llvm::PointerType::get(llvm::StructType::create(
- CGM.getLLVMContext(), "opencl.pipe_t"), PipeAddrSpc);
- }
+ if (T->isReadOnly())
+return getPipeType(T, "opencl.pipe_ro_t", PipeROTy);
+ else
+return getPipeType(T, "opencl.pipe_wo_t", PipeWOTy);
+}
+llvm::Type *CGOpenCLRuntime::getPipeType(const PipeType *T, StringRef Name,
+ llvm::Type *&PipeTy) {
+ if (!PipeTy)
+PipeTy = llvm::PointerType::get(llvm::StructType::create(
+ CGM.getLLVMContext(), Name),
+ CGM.getContext().getTargetAddressSpace(
+ CGM.getContext().getOpenCLTypeAddrSpace(T)));
return PipeTy;
}
Modified: cfe/trunk/lib/CodeGen/CGOpenCLRuntime.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenCLRuntime.h?rev=331026&r1=331025&r2=331026&view=diff
==
--- cfe/trunk/lib/CodeGen/CGOpenCLRuntime.h (original)
+++ cfe/trunk/lib/CodeGen/CGOpenCLRuntime.h Fri Apr 27 03:37:04 2018
@@ -35,7 +35,8 @@ class CodeGenModule;
class CGOpenCLRuntime {
protected:
CodeGenModule &CGM;
- llvm::Type *PipeTy;
+ llvm::Type *PipeROTy;
+ llvm::Type *PipeWOTy;
llvm::PointerType *SamplerTy;
/// Structure for enqueued block information.
@@ -47,9 +48,12 @@ protected:
/// Maps block expression to block information.
llvm::DenseMap EnqueuedBlockMap;
+ virtual llvm::Type *getPipeType(const PipeType *T, StringRef Name,
+ llvm::Type *&PipeTy);
+
public:
- CGOpenCLRuntime(CodeGenModule &CGM) : CGM(CGM), PipeTy(nullptr),
-SamplerTy(nullptr) {}
+ CGOpenCLRuntime(CodeGenModule &CGM) : CGM(CGM),
+PipeROTy(nullptr), PipeWOTy(nullptr), SamplerTy(nullptr) {}
virtual ~CGOpenCLRuntime();
/// Emit the IR required for a work-group-local variable declaration, and add
Modified: cfe/trunk/test/CodeGenOpenCL/opencl_types.cl
URL:
http:/
[PATCH] D46015: [OpenCL] Add separate read_only and write_only pipe IR types
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 rL331026: [OpenCL] Add separate read_only and write_only pipe
IR types (authored by svenvh, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D46015?vs=143938&id=144307#toc
Repository:
rL LLVM
https://reviews.llvm.org/D46015
Files:
cfe/trunk/lib/CodeGen/CGBuiltin.cpp
cfe/trunk/lib/CodeGen/CGOpenCLRuntime.cpp
cfe/trunk/lib/CodeGen/CGOpenCLRuntime.h
cfe/trunk/test/CodeGenOpenCL/opencl_types.cl
cfe/trunk/test/CodeGenOpenCL/pipe_builtin.cl
cfe/trunk/test/CodeGenOpenCL/pipe_types.cl
cfe/trunk/test/Index/pipe-size.cl
Index: cfe/trunk/test/CodeGenOpenCL/pipe_types.cl
===
--- cfe/trunk/test/CodeGenOpenCL/pipe_types.cl
+++ cfe/trunk/test/CodeGenOpenCL/pipe_types.cl
@@ -1,34 +1,35 @@
// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -O0 -cl-std=CL2.0 -o - %s | FileCheck %s
-// CHECK: %opencl.pipe_t = type opaque
+// CHECK: %opencl.pipe_ro_t = type opaque
+// CHECK: %opencl.pipe_wo_t = type opaque
typedef unsigned char __attribute__((ext_vector_type(3))) uchar3;
typedef int __attribute__((ext_vector_type(4))) int4;
void test1(read_only pipe int p) {
-// CHECK: define void @test1(%opencl.pipe_t* %p)
+// CHECK: define void @test1(%opencl.pipe_ro_t* %p)
reserve_id_t rid;
// CHECK: %rid = alloca %opencl.reserve_id_t
}
void test2(write_only pipe float p) {
-// CHECK: define void @test2(%opencl.pipe_t* %p)
+// CHECK: define void @test2(%opencl.pipe_wo_t* %p)
}
void test3(read_only pipe const int p) {
-// CHECK: define void @test3(%opencl.pipe_t* %p)
+// CHECK: define void @test3(%opencl.pipe_ro_t* %p)
}
void test4(read_only pipe uchar3 p) {
-// CHECK: define void @test4(%opencl.pipe_t* %p)
+// CHECK: define void @test4(%opencl.pipe_ro_t* %p)
}
void test5(read_only pipe int4 p) {
-// CHECK: define void @test5(%opencl.pipe_t* %p)
+// CHECK: define void @test5(%opencl.pipe_ro_t* %p)
}
typedef read_only pipe int MyPipe;
kernel void test6(MyPipe p) {
-// CHECK: define spir_kernel void @test6(%opencl.pipe_t* %p)
+// CHECK: define spir_kernel void @test6(%opencl.pipe_ro_t* %p)
}
struct Person {
@@ -41,7 +42,7 @@
read_only pipe struct Person SPipe) {
// CHECK: define void @test_reserved_read_pipe
read_pipe (SPipe, SDst);
- // CHECK: call i32 @__read_pipe_2(%opencl.pipe_t* %{{.*}}, i8* %{{.*}}, i32 16, i32 8)
+ // CHECK: call i32 @__read_pipe_2(%opencl.pipe_ro_t* %{{.*}}, i8* %{{.*}}, i32 16, i32 8)
read_pipe (SPipe, SDst);
- // CHECK: call i32 @__read_pipe_2(%opencl.pipe_t* %{{.*}}, i8* %{{.*}}, i32 16, i32 8)
+ // CHECK: call i32 @__read_pipe_2(%opencl.pipe_ro_t* %{{.*}}, i8* %{{.*}}, i32 16, i32 8)
}
Index: cfe/trunk/test/CodeGenOpenCL/opencl_types.cl
===
--- cfe/trunk/test/CodeGenOpenCL/opencl_types.cl
+++ cfe/trunk/test/CodeGenOpenCL/opencl_types.cl
@@ -63,9 +63,13 @@
// CHECK-AMDGCN: call {{.*}}void @fnc4smp(%opencl.sampler_t addrspace(4)*
}
-kernel void foo_pipe(read_only pipe int p) {}
-// CHECK-SPIR: @foo_pipe(%opencl.pipe_t addrspace(1)* %p)
-// CHECK_AMDGCN: @foo_pipe(%opencl.pipe_t addrspace(1)* %p)
+kernel void foo_ro_pipe(read_only pipe int p) {}
+// CHECK-SPIR: @foo_ro_pipe(%opencl.pipe_ro_t addrspace(1)* %p)
+// CHECK_AMDGCN: @foo_ro_pipe(%opencl.pipe_ro_t addrspace(1)* %p)
+
+kernel void foo_wo_pipe(write_only pipe int p) {}
+// CHECK-SPIR: @foo_wo_pipe(%opencl.pipe_wo_t addrspace(1)* %p)
+// CHECK_AMDGCN: @foo_wo_pipe(%opencl.pipe_wo_t addrspace(1)* %p)
void __attribute__((overloadable)) bad1(image1d_t b, image2d_t c, image2d_t d) {}
// CHECK-SPIR-LABEL: @{{_Z4bad114ocl_image1d_ro14ocl_image2d_roS0_|"\\01\?bad1@@\$\$J0YAXPAUocl_image1d_ro@@PAUocl_image2d_ro@@1@Z"}}
Index: cfe/trunk/test/CodeGenOpenCL/pipe_builtin.cl
===
--- cfe/trunk/test/CodeGenOpenCL/pipe_builtin.cl
+++ cfe/trunk/test/CodeGenOpenCL/pipe_builtin.cl
@@ -1,79 +1,93 @@
// RUN: %clang_cc1 -emit-llvm -cl-ext=+cl_khr_subgroups -O0 -cl-std=CL2.0 -o - %s | FileCheck %s
-// CHECK: %opencl.pipe_t = type opaque
-// CHECK: %opencl.reserve_id_t = type opaque
+// CHECK-DAG: %opencl.pipe_ro_t = type opaque
+// CHECK-DAG: %opencl.pipe_wo_t = type opaque
+// CHECK-DAG: %opencl.reserve_id_t = type opaque
#pragma OPENCL EXTENSION cl_khr_subgroups : enable
void test1(read_only pipe int p, global int *ptr) {
- // CHECK: call i32 @__read_pipe_2(%opencl.pipe_t* %{{.*}}, i8* %{{.*}}, i32 4, i32 4)
+ // CHECK: call i32 @__read_pipe_2(%opencl.pipe_ro_t* %{{.*}}, i8* %{{.*}}, i32 4, i32 4)
read_pipe(p, ptr);
- // CHECK: call %opencl.reserve_id_t* @__reserve_read_pipe(%opencl.pipe_t* %{{.*}}, i32 {{.*}}, i32 4, i32 4)
+
[PATCH] D46180: [clang-format] Refactor #include insertion/deletion functionality into a class.
ilya-biryukov added a comment.
This change LG as an extraction of the helper functionality to be reused in
clang, clang-tidy, etc.
However, I feel there are potential improvements both to the underlying code
and the new APIs that we could make.
I left some comments, trying to focus on interface of the class and keeping the
changes that would be required to address them to be purely cosmetic. As
chatted offline, any non-trivial changes to the underlying implementation are
out of scope of this patch.
> preserver sorted #includes.
A typo in the change description:
s/preserver/preserve
> This is best effort - only works when the
> block is already sorted.
Could we describe behavior for the unsorted case in a sensible way? E.g. is it
added to the end of the include list, added after closely related headers,
added to a totally random place (but deterministic) place, etc?
It seems like an important case, and I believe we shouldn't completely ignore
it and describe what the "best effort" means in that case.
> When inserting multiple #includes to the end of a file which doesn't
> end with a "\n" character, a "\n" will be prepended to each #include.
> This is a special and rare case that was previously handled. This is now
> relaxed to avoid complexity as it's rare in practice.
I don't quite get it, could you please elaborate on what has changed here
exactly? Maybe give an example?
Comment at: lib/Format/Format.cpp:1691
// 0. Otherwise, returns the priority of the matching category or INT_MAX.
- int getIncludePriority(StringRef IncludeName, bool CheckMainHeader) {
+ int getIncludePriority(StringRef IncludeName, bool CheckMainHeader) const {
int Ret = INT_MAX;
I would personally keep the function non-const and not use mutable fields here.
Even though it's logically const, I would strive towards keeping the things
`const` only if there are actually immutable
One particular problem that could be avoided is accidentally calling the const
methods concurrently on different threads.
But up to you if you actually want to make this change.
Comment at: lib/Format/Format.cpp:1988
-bool isDeletedHeader(llvm::StringRef HeaderName,
- const std::set &HeadersToDelete) {
- return HeadersToDelete.count(HeaderName) ||
- HeadersToDelete.count(HeaderName.trim("\"<>"));
-}
+/// Generates replacements for inserting or deleting #include headers in a
file.
+class HeaderIncludes {
Maybe s/#include headers/#include directives/?
This is how they called in terminology of preprocessing.
Comment at: lib/Format/Format.cpp:1995
+ /// Inserts an #include directive of \p IncludeName into the code. If \p
+ /// IncludeName is quoted e.g. <...> or "...", it will be #included as is;
+ /// by default, it is quoted with "".
Maybe make an API more explicit about the style of includes that should be used?
Make it literally impossible to misuse the API:
```
enum class IncludeStyle { Quoted, Angle };
/// \p IncludeName is the include path to be inserted, \p Style specifies
whether the included path should use quotes or angle brackets.
Optional insert(StringRef IncludeName, IncludeStyle Style =
IncludeStyle::Quoted) const;
```
Comment at: lib/Format/Format.cpp:2013
+
+ /// Removes all existing #includes of \p Header. If \p Header is not quoted
+ /// (e.g. without <> or ""), #include of \p Header with both quotations will
Have you considered changing the API slightly to allow iterating over all
includes in the file and APIs to remove includes pointed by the iterators?
It would give more flexibility, allowing the clients to implement useful things
like:
- Remove all #includes that start with a prefix, e.g. `clang/Basic/`.
- Find and remove all duplicate #include directives.
- etc, etc.
The current implementation seems tied to a specific use-case that we have in
clang-format, i.e. "preprocess once, remove headers in batches".
It feels wasteful to pay overhead of `StringMap>` sometimes,
e.g. if the code wants to insert just a single header or do other things, like
removing duplicate headers.
I.e. I propose to extract the code that parses the source code and produces a
list of #include directives with their respective ranges and filenames into a
separate function/class.
This would. arguably, improve readability of the include insertion code.
That might not be a trivial change, though, since it requires rewriting some of
the code used here, while the current patch seems to focus on simply extracting
useful functionality from `clang-format` for reuse in clangd, clang-tidy, etc.
Let me know what you think.
Comment at: lib/Format/Format.cpp:2015
+ /// (e.g. without <> or ""), #include of \p Header with both quotations will
+ /// be removed.
+ tooling::Replacements remove(llvm::StringRef Header) const;
-
[PATCH] D45999: [clangd] Retrieve minimally formatted comment text in completion.
sammccall added a comment.
My main question/concern is: if these APIs extract/format based on
CodeCompletionString and friends, what should our plan be using this this in
AST-based features, such as hover?
Comment at: clangd/CodeComplete.cpp:259
+ CodeCompletionString *SemaCCS,
+ llvm::StringRef DocComment) const {
assert(bool(SemaResult) == bool(SemaCCS));
This should probably be SemaDocComment, as this code is all about merging; we
want to emphasize sources.
Comment at: clangd/CodeComplete.h:48
- /// Add brief comments to completion items, if available.
- /// FIXME(ibiryukov): it looks like turning this option on significantly
slows
- /// down completion, investigate if it can be made faster.
IIUC this fixme no longer applies, because the expensive part was the (eager?)
parsing and we no longer do that?
Comment at: clangd/CodeCompletionStrings.h:24
+/// Gets a raw documentation comment of \p Result.
+/// Returns empty string when no comment is available.
What does raw mean - range of the file? indentation stripped?
Comment at: clangd/CodeCompletionStrings.h:29
+
+/// Gets a raw documentation comment of the current active parameter
+/// of \p Result.
"active" is a bit confusing - at this level we just care which arg you're
interested in, right?
Comment at: clangd/CodeCompletionStrings.h:29
+
+/// Gets a raw documentation comment of the current active parameter
+/// of \p Result.
sammccall wrote:
> "active" is a bit confusing - at this level we just care which arg you're
> interested in, right?
Is this typically a substring of getDocComment for the function? If so, noting
that would make this clearer.
Comment at: clangd/CodeCompletionStrings.h:33
+std::string
+getDocComment(const ASTContext &Ctx,
+ const CodeCompleteConsumer::OverloadCandidate &Result,
getParameterDocComment?
Comment at: clangd/CodeCompletionStrings.h:49
+/// getDocComment functions declared above.
+std::string getDocumentation(const CodeCompletionString &CCS,
+ llvm::StringRef DocComment);
The name here is confusingly similar to getDocComment, and the comment doesn't
really explain how they're different.
Consider calling this formatDocumentation, with a comment like "Assembles
formatted documentation for a completion result. This includes documentation
comments and other relevant information like annotations."
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D45999
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46035: [clangd] Fix unicode handling, using UTF-16 where LSP requires it.
sammccall marked 3 inline comments as done.
sammccall added a comment.
Thanks!
Comment at: clangd/SourceCode.cpp:25
+// Returns true if CB returned true, false if we hit the end of string.
+template
+bool iterateCodepoints(StringRef U8, const Callback &CB) {
hokein wrote:
> Can we make it `static`?
>
> The callback type is function, the reason why using template here
> is mainly to save some keystroke?
Added static.
The difference between using a template vs `std::function` for a lambda is
compile-time vs run-time polymorphism: invoking std::function is a virtual call
and (AFAIK) compilers don't manage to inline it well.
With the template, we get one copy of the function for each callsite, with the
lambda inlined.
Not sure the performance is a big deal here, but this code is at least
plausibly hot I guess? And I think there's very little readability cost to
using the template in this case.
Comment at: clangd/SourceCode.cpp:72
+ size_t Count = 0;
+ iterateCodepoints(U8, [&](int U8Len, int U16Len) {
+Count += U16Len;
hokein wrote:
> Maybe add an `assume` to ensure `iterateCodepoints` always return false
> (reach the end of the U8)?
I'm not sure there's enough value to this one, it's clear from the local code
that this isn't possible, and it doesn't seem likely a bug would manifest this
way (abort early even though our function returns false, *and return true* from
iterateCodepoints).
The small cost is added noise - I think this needs a new variable, and assert,
and a suppression of "unused variable" warning.
Comment at: clangd/SourceCode.cpp:137
+P.character =
+utf16Len(Code.substr(LocInfo.second - ColumnInBytes, ColumnInBytes));
+ }
hokein wrote:
> nit: it took me a while to understand what the sub-expression
> `Code.substr(LocInfo.second - ColumnInBytes, ColumnInBytes)` does, maybe
> abstract it out with a descriptive name? Also it is not straight-forward to
> understand what `LocInfo.second` is without navigating to
> `getDecomposedSpellingLoc`.
Called this `LineSoFar` and decomposed `LocInfo` int named variables.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D46035
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46002: [clangd] Parse all comments in Sema and completion.
sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. LG (once the dependencies are done!) Comment at: clangd/ClangdUnit.cpp:362 CI->getFrontendOpts().DisableFree = false; +CI->getLangOpts()->CommentOpts.ParseAllComments = true; } ilya-biryukov wrote: > sammccall wrote: > > Any idea about whether this will affect performance significantly? > > Less for this patch, and more for whether this should be an option in the > > future that we might e.g. only do during indexing. > Hopefully, there should be no significant difference, but I'll do some > benchmarks with both real code and artificial comment-heave code to make sure > that's the case. > It would certainly eat a bit more memory, but it shouldn't be significant as > we only store an extra list of sourceranges for comments. > Even for this patch, maybe we would want to only enable it in the AST but not > in the preamble. I'll get some numbers and come back to this. > No need to block on the numbers from my point of view, though I would be interested. > maybe we would want to only enable it in the AST but not in the preamble You're talking about gathering the information, right? How would this work? Given that e.g. class members come from the preamble and we need the comments from there. Comment at: clangd/CodeComplete.cpp:707 CI->getFrontendOpts().DisableFree = false; + CI->getLangOpts()->CommentOpts.ParseAllComments = true; ilya-biryukov wrote: > sammccall wrote: > > Are we sure we want to do this in code complete? I would have thought the > > more natural approach would be to implement resolve in terms of lookup in > > the index, and only provide it there. > > The lack of good support for resolve in YCM LSP shouldn't be a problem as > > YCM doesn't actually use doc comments (I think?). > It seems to give a better user-experience: > - Some items come only from Sema, so getting documentation for local > variables and class members is currently only possible through Sema. > We could probably omit the docs for the local vars and put class members > into the index, though. > - If the comment for the completion is in the current file, we should > prefer the latest version, not one from the index that might be stale. > > However, it does mean sending docs along with the results, I don't think LSP > will allow us to properly handle the lifetime of docs from the completion > AST, so we won't be able to delay their result until resolve time. Yeah, Sema-only results. I wasn't thinking. We can send eager docs from sema results and still fill ind index docs at resolve time. This all sounds good. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D46002 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r331029 - [clangd] Fix unicode handling, using UTF-16 where LSP requires it.
Author: sammccall Date: Fri Apr 27 04:59:28 2018 New Revision: 331029 URL: http://llvm.org/viewvc/llvm-project?rev=331029&view=rev Log: [clangd] Fix unicode handling, using UTF-16 where LSP requires it. Summary: The Language Server Protocol unfortunately mandates that locations in files be represented by line/column pairs, where the "column" is actually an index into the UTF-16-encoded text of the line. (This is because VSCode is written in JavaScript, which is UTF-16-native). Internally clangd treats source files at UTF-8, the One True Encoding, and generally deals with byte offsets (though there are exceptions). Before this patch, conversions between offsets and LSP Position pretended that Position.character was UTF-8 bytes, which is only true for ASCII lines. Now we examine the text to convert correctly (but don't actually need to transcode it, due to some nice details of the encodings). The updated functions in SourceCode are the blessed way to interact with the Position.character field, and anything else is likely to be wrong. So I also updated the other accesses: - CodeComplete needs a "clang-style" line/column, with column in utf-8 bytes. This is now converted via Position -> offset -> clang line/column (a new function is added to SourceCode.h for the second conversion). - getBeginningOfIdentifier skipped backwards in UTF-16 space, which is will behave badly when it splits a surrogate pair. Skipping backwards in UTF-8 coordinates gives the lexer a fighting chance of getting this right. While here, I clarified(?) the logic comments, fixed a bug with identifiers containing digits, simplified the signature slightly and added a test. This seems likely to cause problems with editors that have the same bug, and treat the protocol as if columns are UTF-8 bytes. But we can find and fix those. Reviewers: hokein Subscribers: klimek, ilya-biryukov, ioeric, MaskRay, jkorous, cfe-commits Differential Revision: https://reviews.llvm.org/D46035 Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp clang-tools-extra/trunk/clangd/ClangdUnit.cpp clang-tools-extra/trunk/clangd/ClangdUnit.h clang-tools-extra/trunk/clangd/CodeComplete.cpp clang-tools-extra/trunk/clangd/Protocol.h clang-tools-extra/trunk/clangd/SourceCode.cpp clang-tools-extra/trunk/clangd/SourceCode.h clang-tools-extra/trunk/clangd/XRefs.cpp clang-tools-extra/trunk/test/clangd/rename.test clang-tools-extra/trunk/unittests/clangd/ClangdUnitTests.cpp clang-tools-extra/trunk/unittests/clangd/DraftStoreTests.cpp clang-tools-extra/trunk/unittests/clangd/SourceCodeTests.cpp Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=331029&r1=331028&r2=331029&view=diff == --- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original) +++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Fri Apr 27 04:59:28 2018 @@ -232,14 +232,8 @@ void ClangdServer::rename(PathRef File, RefactoringResultCollector ResultCollector; const SourceManager &SourceMgr = AST.getASTContext().getSourceManager(); -const FileEntry *FE = -SourceMgr.getFileEntryForID(SourceMgr.getMainFileID()); -if (!FE) - return CB(llvm::make_error( - "rename called for non-added document", - llvm::errc::invalid_argument)); SourceLocation SourceLocationBeg = -clangd::getBeginningOfIdentifier(AST, Pos, FE); +clangd::getBeginningOfIdentifier(AST, Pos, SourceMgr.getMainFileID()); tooling::RefactoringRuleContext Context( AST.getASTContext().getSourceManager()); Context.setASTContext(AST.getASTContext()); Modified: clang-tools-extra/trunk/clangd/ClangdUnit.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdUnit.cpp?rev=331029&r1=331028&r2=331029&view=diff == --- clang-tools-extra/trunk/clangd/ClangdUnit.cpp (original) +++ clang-tools-extra/trunk/clangd/ClangdUnit.cpp Fri Apr 27 04:59:28 2018 @@ -215,19 +215,6 @@ ParsedAST::Build(std::unique_ptrhttp://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdUnit.h?rev=331029&r1=331028&r2=331029&view=diff == --- clang-tools-extra/trunk/clangd/ClangdUnit.h (original) +++ clang-tools-extra/trunk/clangd/ClangdUnit.h Fri Apr 27 04:59:28 2018 @@ -173,8 +173,9 @@ private: }; /// Get the beginning SourceLocation at a specified \p Pos. +/// May be invalid if Pos is, or if there's no identifier. SourceLocation getBeginningOfIdentifier(ParsedAST &Unit, const Position &Pos, -const FileEntry *FE); +const FileID FID); /// For testing/debugging purposes. Note that th
[PATCH] D46035: [clangd] Fix unicode handling, using UTF-16 where LSP requires it.
This revision was automatically updated to reflect the committed changes.
sammccall marked 2 inline comments as done.
Closed by commit rL331029: [clangd] Fix unicode handling, using UTF-16 where
LSP requires it. (authored by sammccall, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D46035?vs=143838&id=144312#toc
Repository:
rL LLVM
https://reviews.llvm.org/D46035
Files:
clang-tools-extra/trunk/clangd/ClangdServer.cpp
clang-tools-extra/trunk/clangd/ClangdUnit.cpp
clang-tools-extra/trunk/clangd/ClangdUnit.h
clang-tools-extra/trunk/clangd/CodeComplete.cpp
clang-tools-extra/trunk/clangd/Protocol.h
clang-tools-extra/trunk/clangd/SourceCode.cpp
clang-tools-extra/trunk/clangd/SourceCode.h
clang-tools-extra/trunk/clangd/XRefs.cpp
clang-tools-extra/trunk/test/clangd/rename.test
clang-tools-extra/trunk/unittests/clangd/ClangdUnitTests.cpp
clang-tools-extra/trunk/unittests/clangd/DraftStoreTests.cpp
clang-tools-extra/trunk/unittests/clangd/SourceCodeTests.cpp
Index: clang-tools-extra/trunk/unittests/clangd/DraftStoreTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/DraftStoreTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/DraftStoreTests.cpp
@@ -241,7 +241,7 @@
EXPECT_TRUE(!Result);
EXPECT_EQ(llvm::toString(Result.takeError()),
-"Character value is out of range (100)");
+"UTF-16 offset 100 is invalid for line 0");
}
TEST(DraftStoreIncrementalUpdateTest, EndCharOutOfRange) {
@@ -262,7 +262,7 @@
EXPECT_TRUE(!Result);
EXPECT_EQ(llvm::toString(Result.takeError()),
-"Character value is out of range (100)");
+"UTF-16 offset 100 is invalid for line 0");
}
TEST(DraftStoreIncrementalUpdateTest, StartLineOutOfRange) {
@@ -338,7 +338,7 @@
EXPECT_TRUE(!Result);
EXPECT_EQ(llvm::toString(Result.takeError()),
-"Character value is out of range (100)");
+"UTF-16 offset 100 is invalid for line 0");
llvm::Optional Contents = DS.getDraft(File);
EXPECT_TRUE(Contents);
Index: clang-tools-extra/trunk/unittests/clangd/SourceCodeTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/SourceCodeTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/SourceCodeTests.cpp
@@ -24,9 +24,10 @@
return arg.line == Line && arg.character == Col;
}
+// The = → 🡆 below are ASCII (1 byte), BMP (3 bytes), and astral (4 bytes).
const char File[] = R"(0:0 = 0
-1:0 = 8
-2:0 = 16)";
+1:0 → 8
+2:0 🡆 18)";
/// A helper to make tests easier to read.
Position position(int line, int character) {
@@ -66,25 +67,31 @@
EXPECT_THAT_EXPECTED(positionToOffset(File, position(1, 3), false),
HasValue(11));
EXPECT_THAT_EXPECTED(positionToOffset(File, position(1, 6)),
- HasValue(14)); // last character
+ HasValue(16)); // last character
EXPECT_THAT_EXPECTED(positionToOffset(File, position(1, 7)),
- HasValue(15)); // the newline itself
+ HasValue(17)); // the newline itself
EXPECT_THAT_EXPECTED(positionToOffset(File, position(1, 8)),
- HasValue(15)); // out of range
+ HasValue(17)); // out of range
EXPECT_THAT_EXPECTED(positionToOffset(File, position(1, 8), false),
Failed()); // out of range
// last line
EXPECT_THAT_EXPECTED(positionToOffset(File, position(2, -1)),
Failed()); // out of range
EXPECT_THAT_EXPECTED(positionToOffset(File, position(2, 0)),
- HasValue(16)); // first character
+ HasValue(18)); // first character
EXPECT_THAT_EXPECTED(positionToOffset(File, position(2, 3)),
- HasValue(19)); // middle character
- EXPECT_THAT_EXPECTED(positionToOffset(File, position(2, 7)),
- HasValue(23)); // last character
+ HasValue(21)); // middle character
+ EXPECT_THAT_EXPECTED(positionToOffset(File, position(2, 5), false),
+ Failed()); // middle of surrogate pair
+ EXPECT_THAT_EXPECTED(positionToOffset(File, position(2, 5)),
+ HasValue(26)); // middle of surrogate pair
+ EXPECT_THAT_EXPECTED(positionToOffset(File, position(2, 6), false),
+ HasValue(26)); // end of surrogate pair
EXPECT_THAT_EXPECTED(positionToOffset(File, position(2, 8)),
- HasValue(24)); // EOF
- EXPECT_THAT_EXPECTED(positionToOffset(File, position(2, 9), false),
+ HasValue(28)); // last character
+ EXPECT_THAT_EXPECTED(positionToOffset(File, position(2, 9)),
+ HasValue(29)); // EOF
+ EXPECT_THAT_EXPECTED(positionToOffset(File, position(2, 10), false),
Failed()
[PATCH] D46183: [clangd] Incorporate #occurrences in scoring code complete results.
sammccall created this revision.
sammccall added a reviewer: ilya-biryukov.
Herald added subscribers: cfe-commits, jkorous, MaskRay, ioeric, klimek.
needs tests
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D46183
Files:
clangd/CodeComplete.cpp
clangd/index/Index.cpp
clangd/index/Index.h
clangd/index/MemIndex.cpp
Index: clangd/index/MemIndex.cpp
===
--- clangd/index/MemIndex.cpp
+++ clangd/index/MemIndex.cpp
@@ -47,7 +47,7 @@
continue;
if (auto Score = Filter.match(Sym->Name)) {
-Top.emplace(-*Score, Sym);
+Top.emplace(-*Score * quality(*Sym), Sym);
if (Top.size() > Req.MaxCandidateCount) {
More = true;
Top.pop();
Index: clangd/index/Index.h
===
--- clangd/index/Index.h
+++ clangd/index/Index.h
@@ -199,6 +199,12 @@
};
llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const Symbol &S);
+// Computes query-independent quality score for a Symbol.
+// This currently falls in the range [1, ln(#indexed documents)].
+// FIXME: this should probably be split into symbol -> signals
+//and signals -> score, so it can be reused for Sema completions.
+double quality(const Symbol &S);
+
// An immutable symbol container that stores a set of symbols.
// The container will maintain the lifetime of the symbols.
class SymbolSlab {
Index: clangd/index/Index.cpp
===
--- clangd/index/Index.cpp
+++ clangd/index/Index.cpp
@@ -48,6 +48,14 @@
return OS << S.Scope << S.Name;
}
+double quality(const Symbol &S) {
+ // This avoids a sharp gradient for tail symbols, and also neatly avoids the
+ // question of whether 0 references means a bad symbol or missing data.
+ if (S.References < 3)
+return 1;
+ return std::log(S.References);
+}
+
SymbolSlab::const_iterator SymbolSlab::find(const SymbolID &ID) const {
auto It = std::lower_bound(Symbols.begin(), Symbols.end(), ID,
[](const Symbol &S, const SymbolID &I) {
Index: clangd/CodeComplete.cpp
===
--- clangd/CodeComplete.cpp
+++ clangd/CodeComplete.cpp
@@ -229,24 +229,26 @@
// Computes the "symbol quality" score for this completion. Higher is better.
float score() const {
-// For now we just use the Sema priority, mapping it onto a 0-1 interval.
-if (!SemaResult) // FIXME(sammccall): better scoring for index results.
- return 0.3f; // fixed mediocre score for index-only results.
-
-// Priority 80 is a really bad score.
-float Score = 1 - std::min(80, SemaResult->Priority) / 80;
+float Score = 1;
+if (IndexResult)
+ Score *= quality(*IndexResult);
+if (SemaResult) {
+ // For now we just use the Sema priority, mapping it onto a 0-2 interval.
+ // Priority 80 is a really bad score.
+ Score *= 2 - std::min(80, SemaResult->Priority) / 40;
-switch (static_cast(SemaResult->Availability)) {
-case CXAvailability_Available:
- // No penalty.
- break;
-case CXAvailability_Deprecated:
- Score *= 0.1f;
- break;
-case CXAvailability_NotAccessible:
-case CXAvailability_NotAvailable:
- Score = 0;
- break;
+ switch (static_cast(SemaResult->Availability)) {
+ case CXAvailability_Available:
+// No penalty.
+break;
+ case CXAvailability_Deprecated:
+Score *= 0.1f;
+break;
+ case CXAvailability_NotAccessible:
+ case CXAvailability_NotAvailable:
+Score = 0;
+break;
+ }
}
return Score;
}
Index: clangd/index/MemIndex.cpp
===
--- clangd/index/MemIndex.cpp
+++ clangd/index/MemIndex.cpp
@@ -47,7 +47,7 @@
continue;
if (auto Score = Filter.match(Sym->Name)) {
-Top.emplace(-*Score, Sym);
+Top.emplace(-*Score * quality(*Sym), Sym);
if (Top.size() > Req.MaxCandidateCount) {
More = true;
Top.pop();
Index: clangd/index/Index.h
===
--- clangd/index/Index.h
+++ clangd/index/Index.h
@@ -199,6 +199,12 @@
};
llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const Symbol &S);
+// Computes query-independent quality score for a Symbol.
+// This currently falls in the range [1, ln(#indexed documents)].
+// FIXME: this should probably be split into symbol -> signals
+//and signals -> score, so it can be reused for Sema completions.
+double quality(const Symbol &S);
+
// An immutable symbol container that stores a set of symbols.
// The container will maintain the lifetime of the symbols.
class SymbolSlab {
Index: clangd/index/Index.cpp
===
--- clangd/i
[PATCH] D46180: [clang-format] Refactor #include insertion/deletion functionality into a class.
ioeric updated this revision to Diff 144319.
ioeric marked 2 inline comments as done.
ioeric added a comment.
Addressed review comments.
Repository:
rC Clang
https://reviews.llvm.org/D46180
Files:
lib/Format/Format.cpp
unittests/Format/CleanupTest.cpp
Index: unittests/Format/CleanupTest.cpp
===
--- unittests/Format/CleanupTest.cpp
+++ unittests/Format/CleanupTest.cpp
@@ -471,19 +471,77 @@
EXPECT_EQ(Expected, apply(Code, Replaces));
}
+TEST_F(CleanUpReplacementsTest, InsertIntoBlockSorted) {
+ std::string Code = "#include \"x/fix.h\"\n"
+ "#include \"a.h\"\n"
+ "#include \"c.h\"\n"
+ "#include \n";
+ std::string Expected = "#include \"x/fix.h\"\n"
+ "#include \"a.h\"\n"
+ "#include \"b.h\"\n"
+ "#include \"c.h\"\n"
+ "#include \n";
+ tooling::Replacements Replaces =
+ toReplacements({createInsertion("#include \"b.h\"")});
+ EXPECT_EQ(Expected, apply(Code, Replaces));
+}
+
+TEST_F(CleanUpReplacementsTest, InsertIntoFirstBlockOfSameKind) {
+ std::string Code = "#include \"x/fix.h\"\n"
+ "#include \"c.h\"\n"
+ "#include \"e.h\"\n"
+ "#include \"f.h\"\n"
+ "#include \n"
+ "#include \n"
+ "#include \"m.h\"\n"
+ "#include \"n.h\"\n";
+ std::string Expected = "#include \"x/fix.h\"\n"
+ "#include \"c.h\"\n"
+ "#include \"d.h\"\n"
+ "#include \"e.h\"\n"
+ "#include \"f.h\"\n"
+ "#include \n"
+ "#include \n"
+ "#include \"m.h\"\n"
+ "#include \"n.h\"\n";
+ tooling::Replacements Replaces =
+ toReplacements({createInsertion("#include \"d.h\"")});
+ EXPECT_EQ(Expected, apply(Code, Replaces));
+}
+
+TEST_F(CleanUpReplacementsTest, InsertIntoSystemBlockSorted) {
+ std::string Code = "#include \"x/fix.h\"\n"
+ "#include \"a.h\"\n"
+ "#include \"c.h\"\n"
+ "#include \n"
+ "#include \n";
+ std::string Expected = "#include \"x/fix.h\"\n"
+ "#include \"a.h\"\n"
+ "#include \"c.h\"\n"
+ "#include \n"
+ "#include \n"
+ "#include \n";
+ tooling::Replacements Replaces =
+ toReplacements({createInsertion("#include ")});
+ EXPECT_EQ(Expected, apply(Code, Replaces));
+}
+
+
TEST_F(CleanUpReplacementsTest, InsertMultipleIncludesLLVMStyle) {
std::string Code = "#include \"x/fix.h\"\n"
"#include \"a.h\"\n"
"#include \"b.h\"\n"
+ "#include \"z.h\"\n"
"#include \"clang/Format/Format.h\"\n"
"#include \n";
std::string Expected = "#include \"x/fix.h\"\n"
"#include \"a.h\"\n"
"#include \"b.h\"\n"
"#include \"new/new.h\"\n"
+ "#include \"z.h\"\n"
"#include \"clang/Format/Format.h\"\n"
- "#include \n"
- "#include \n";
+ "#include \n"
+ "#include \n";
tooling::Replacements Replaces =
toReplacements({createInsertion("#include "),
createInsertion("#include \"new/new.h\"")});
@@ -517,12 +575,12 @@
"#include \"z/b.h\"\n";
std::string Expected = "#include \"x/fix.h\"\n"
"\n"
- "#include \n"
"#include \n"
+ "#include \n"
"\n"
+ "#include \"x/x.h\"\n"
"#include \"y/a.h\"\n"
- "#include \"z/b.h\"\n"
- "#include \"x/x.h\"\n";
+ "#include \"z/b.h\"\n";
tooling::Replacements Replaces =
toReplacements({createInsertion("#include "),
createInsertion("#include \"x/x.h\"")});
@@ -776,8 +834,10 @@
TEST_F(CleanUpReplacementsTest, NoNewLineAtTheEndOfCodeMultipleInsertions) {
std::string Code = "#include ";
+ // FIXME: a better behavior is to only append on newline to Code, but this
+ // case should be rare in practice.
std::string Expected =
- "#include \n#include \n#include \n";
+ "#include \n#include \n\n#include \n";
tooling::Replacements Replaces =
toReplacements({createInsertion("#include "),
createInsertion("#include ")});
@@ -795,14 +855,11 @@
EXPECT_EQ(E
[PATCH] D46180: [clang-format] Refactor #include insertion/deletion functionality into a class.
ioeric marked 7 inline comments as done.
ioeric added a comment.
Thanks for reviewing this!
In https://reviews.llvm.org/D46180#1080822, @ilya-biryukov wrote:
> This change LG as an extraction of the helper functionality to be reused in
> clang, clang-tidy, etc.
> However, I feel there are potential improvements both to the underlying code
> and the new APIs that we could make.
>
> I left some comments, trying to focus on interface of the class and keeping
> the changes that would be required to address them to be purely cosmetic. As
> chatted offline, any non-trivial changes to the underlying implementation are
> out of scope of this patch.
>
> > preserver sorted #includes.
>
> A typo in the change description:
> s/preserver/preserve
>
> > This is best effort - only works when the
> > block is already sorted.
>
> Could we describe behavior for the unsorted case in a sensible way? E.g. is
> it added to the end of the include list, added after closely related headers,
> added to a totally random place (but deterministic) place, etc?
> It seems like an important case, and I believe we shouldn't completely
> ignore it and describe what the "best effort" means in that case.
Sure. I added more comments to the API.
>
>
>> When inserting multiple #includes to the end of a file which doesn't
>> end with a "\n" character, a "\n" will be prepended to each #include.
>> This is a special and rare case that was previously handled. This is now
>> relaxed to avoid complexity as it's rare in practice.
>
> I don't quite get it, could you please elaborate on what has changed here
> exactly? Maybe give an example?
This is really a corner cases that users might not need to know about. But an
example is:
Code: `"#include "` (without \n at the end). After inserting , `#include
\n#include \n` (this is good). However, if you insert another , the
code would become `"#include \n#include \n\n#include \n"` (note the
double newline!).
Comment at: lib/Format/Format.cpp:1691
// 0. Otherwise, returns the priority of the matching category or INT_MAX.
- int getIncludePriority(StringRef IncludeName, bool CheckMainHeader) {
+ int getIncludePriority(StringRef IncludeName, bool CheckMainHeader) const {
int Ret = INT_MAX;
ilya-biryukov wrote:
> I would personally keep the function non-const and not use mutable fields
> here.
> Even though it's logically const, I would strive towards keeping the things
> `const` only if there are actually immutable
> One particular problem that could be avoided is accidentally calling the
> const methods concurrently on different threads.
>
> But up to you if you actually want to make this change.
Making this non-const would prevent us from making `HeaderIncludes::insert`
const, which I do want to keep. Added a mutex to prevent race-condition...
Comment at: lib/Format/Format.cpp:1995
+ /// Inserts an #include directive of \p IncludeName into the code. If \p
+ /// IncludeName is quoted e.g. <...> or "...", it will be #included as is;
+ /// by default, it is quoted with "".
ilya-biryukov wrote:
> Maybe make an API more explicit about the style of includes that should be
> used?
> Make it literally impossible to misuse the API:
> ```
> enum class IncludeStyle { Quoted, Angle };
> /// \p IncludeName is the include path to be inserted, \p Style specifies
> whether the included path should use quotes or angle brackets.
> Optional insert(StringRef IncludeName, IncludeStyle Style =
> IncludeStyle::Quoted) const;
> ```
>
I think the risk is pretty low, so I'm not sure if it'd be worth spending
another variable. From experience, the current callers would likely need to
either carry another variable for each include or use the following pattern (as
the IncludeName is often already quoted):
```
... = insert(Include.trime("\"<>"), QuoteStyle);
```
Comment at: lib/Format/Format.cpp:2013
+
+ /// Removes all existing #includes of \p Header. If \p Header is not quoted
+ /// (e.g. without <> or ""), #include of \p Header with both quotations will
ilya-biryukov wrote:
> Have you considered changing the API slightly to allow iterating over all
> includes in the file and APIs to remove includes pointed by the iterators?
> It would give more flexibility, allowing the clients to implement useful
> things like:
> - Remove all #includes that start with a prefix, e.g. `clang/Basic/`.
> - Find and remove all duplicate #include directives.
> - etc, etc.
>
> The current implementation seems tied to a specific use-case that we have in
> clang-format, i.e. "preprocess once, remove headers in batches".
> It feels wasteful to pay overhead of `StringMap>` sometimes,
> e.g. if the code wants to insert just a single header or do other things,
> like removing duplicate headers.
>
> I.e. I propose to extract the code that parses the source code and produces a
> list of #
[PATCH] D46159: [clang-tidy] Add a flag to enable alpha checkers
lebedev.ri added a comment. Oh, hm. I just needed something to view the call graph. I have almost wrote a clang-tidy check using `analysis/CallGraph.h`, but then i noticed/remembered that clang static analyzer has that already. But `$ clang-tidy -checks=* -list-checks | grep -i analyzer | grep -i debug` does not list it. Similarly `$ clang-tidy -checks=* -list-checks | grep -i analyzer | grep -i alpha`, contrarily to my expectations also does not list alpha checks... So while this change is needed, I think this is the larger problem that should be resolved. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D46159 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46183: [clangd] Incorporate #occurrences in scoring code complete results.
ilya-biryukov added a comment. the patch LG and fixes the problem of llvm namespace not showing up in completions from the static index. Let's add some tests and ship it! Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D46183 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45814: Fix an assertion when -print-prog-name=
chrib updated this revision to Diff 144322.
chrib added a comment.
Move the non-null name check out of GetProgramPath and add a test case.
Assume that we trust the callers to check for non null ProgramName.
Repository:
rC Clang
https://reviews.llvm.org/D45814
Files:
lib/Driver/Driver.cpp
test/Driver/print-empty-prog-name.c
Index: test/Driver/print-empty-prog-name.c
===
--- /dev/null
+++ test/Driver/print-empty-prog-name.c
@@ -0,0 +1,5 @@
+// Test that -print-prog-name= correctly returns an empty string
+
+// RUN: %clang -print-prog-name= 2>&1 | FileCheck %s
+// CHECK-NOT:{{.+}}
+
Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -1589,7 +1589,13 @@
}
if (Arg *A = C.getArgs().getLastArg(options::OPT_print_prog_name_EQ)) {
-llvm::outs() << GetProgramPath(A->getValue(), TC) << "\n";
+StringRef ProgName = A->getValue();
+
+// Null program name cannot have a path.
+if (! ProgName.empty())
+ llvm::outs() << GetProgramPath(ProgName, TC);
+
+llvm::outs() << "\n";
return false;
}
@@ -4053,6 +4059,7 @@
}
std::string Driver::GetProgramPath(StringRef Name, const ToolChain &TC) const {
+
SmallVector TargetSpecificExecutables;
generatePrefixedToolNames(Name, TC, TargetSpecificExecutables);
Index: test/Driver/print-empty-prog-name.c
===
--- /dev/null
+++ test/Driver/print-empty-prog-name.c
@@ -0,0 +1,5 @@
+// Test that -print-prog-name= correctly returns an empty string
+
+// RUN: %clang -print-prog-name= 2>&1 | FileCheck %s
+// CHECK-NOT:{{.+}}
+
Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -1589,7 +1589,13 @@
}
if (Arg *A = C.getArgs().getLastArg(options::OPT_print_prog_name_EQ)) {
-llvm::outs() << GetProgramPath(A->getValue(), TC) << "\n";
+StringRef ProgName = A->getValue();
+
+// Null program name cannot have a path.
+if (! ProgName.empty())
+ llvm::outs() << GetProgramPath(ProgName, TC);
+
+llvm::outs() << "\n";
return false;
}
@@ -4053,6 +4059,7 @@
}
std::string Driver::GetProgramPath(StringRef Name, const ToolChain &TC) const {
+
SmallVector TargetSpecificExecutables;
generatePrefixedToolNames(Name, TC, TargetSpecificExecutables);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45814: Fix an assertion when -print-prog-name=
chrib added a comment. Hi Saleem, Thanks for your review. I have amended the patch to avoid checking the name on the common path. About your second comment, I'm not sure we have to fix -print-file-name= for an equivalent problem since in case of empty parameter, we do a concatenation with the driver's path, which seems to be fine. Best Regards Christian Repository: rC Clang https://reviews.llvm.org/D45814 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46159: [clang-tidy] Add a flag to enable alpha checkers
lebedev.ri added a comment. In https://reviews.llvm.org/D46159#1080939, @lebedev.ri wrote: > Oh, hm. > I just needed something to view the call graph. > I have almost wrote a clang-tidy check using `analysis/CallGraph.h`, but > then i noticed/remembered that clang static analyzer has that already. > But `$ clang-tidy -checks=* -list-checks | grep -i analyzer | grep -i debug` > does not list it. > Similarly `$ clang-tidy -checks=* -list-checks | grep -i analyzer | grep -i > alpha`, contrarily to my expectations also does not list alpha checks... > > So while this change is needed, > I think this is the larger problem that should be resolved. ... And actually looking a bit more at this, i notice // RUN: clang-tidy -checks=* -list-checks -enable-alpha-checks | grep 'clang-analyzer-alpha' So ok, i think this is good. (I'll add debug checks ontop of this differential) Do we want to be more specific and specify that we are talking about /analyzer/ alpha checks? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D46159 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46049: [OpenCL] Add constant address space to __func__ in AST
bader added inline comments. Comment at: lib/AST/Expr.cpp:870 + if (C.getLangOpts().OpenCL && Ty.getAddressSpace() == LangAS::Default) +Ty = C.getAddrSpaceQualType(Ty, LangAS::opencl_constant); + Anastasia wrote: > bader wrote: > > As `Ty` is passed by value, shouldn't we accept only data located in > > constant address space? > Do you mean to assert? Currently it should be passed with `constant` AS but I > thought the idea is to modify this function so we can accept `Default` AS too > but then replace by `constant`. > Yes, but according to your other comment this idea doesn't work. > I have added the address space to the creation of StringLiteral, but > unfortunately it doesn't seems like we can remove similar code in other > places because **QualType created for StringLiteral is also used elsewhere > and has to match (contain right address space).** I.e. here is it used > further down to create PredefinedExpr. https://reviews.llvm.org/D46049 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D32902: [Analyzer] Iterator Checker - Part 7: Support for push and pop operations
baloghadamsoftware updated this revision to Diff 144330.
baloghadamsoftware added a comment.
Herald added a reviewer: george.karpenkov.
Rebased to Part6. Also fixed a test failing now: it is not enough to mark the
symbolic expressions as live, but if it is a SymIntExpr (it can be nothing
else, except SymbolConjured), we also have to mark the left side of it (which
is always SymbolConjured) as live. Using scanReachableSymbols() here would be
too heavyweight.
https://reviews.llvm.org/D32902
Files:
lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
test/Analysis/Inputs/system-header-simulator-cxx.h
test/Analysis/diagnostics/explicit-suppression.cpp
test/Analysis/invalidated-iterator.cpp
test/Analysis/iterator-range.cpp
Index: test/Analysis/iterator-range.cpp
===
--- test/Analysis/iterator-range.cpp
+++ test/Analysis/iterator-range.cpp
@@ -115,8 +115,66 @@
}
}
+void good_push_back(std::list &L, int n) {
+ auto i0 = --L.cend();
+ L.push_back(n);
+ *++i0; // no-warning
+}
+
+void bad_push_back(std::list &L, int n) {
+ auto i0 = --L.cend();
+ L.push_back(n);
+ ++i0;
+ *++i0; // expected-warning{{Iterator accessed outside of its range}}
+}
+
+void good_pop_back(std::list &L, int n) {
+ auto i0 = --L.cend(); --i0;
+ L.pop_back();
+ *i0; // no-warning
+}
+
+void bad_pop_back(std::list &L, int n) {
+ auto i0 = --L.cend(); --i0;
+ L.pop_back();
+ *++i0; // expected-warning{{Iterator accessed outside of its range}}
+}
+
+void good_push_front(std::list &L, int n) {
+ auto i0 = L.cbegin();
+ L.push_front(n);
+ *--i0; // no-warning
+}
+
+void bad_push_front(std::list &L, int n) {
+ auto i0 = L.cbegin();
+ L.push_front(n);
+ --i0;
+ *--i0; // expected-warning{{Iterator accessed outside of its range}}
+}
+
+void good_pop_front(std::list &L, int n) {
+ auto i0 = ++L.cbegin();
+ L.pop_front();
+ *i0; // no-warning
+}
+
+void bad_pop_front(std::list &L, int n) {
+ auto i0 = ++L.cbegin();
+ L.pop_front();
+ *--i0; // expected-warning{{Iterator accessed outside of its range}}
+}
+
void bad_move(std::list &L1, std::list &L2) {
auto i0 = --L2.cend();
L1 = std::move(L2);
*++i0; // expected-warning{{Iterator accessed outside of its range}}
}
+
+void bad_move_push_back(std::list &L1, std::list &L2, int n) {
+ auto i0 = --L2.cend();
+ L2.push_back(n);
+ L1 = std::move(L2);
+ ++i0;
+ *++i0; // expected-warning{{Iterator accessed outside of its range}}
+}
Index: test/Analysis/invalidated-iterator.cpp
===
--- test/Analysis/invalidated-iterator.cpp
+++ test/Analysis/invalidated-iterator.cpp
@@ -30,3 +30,170 @@
FL1 = FL2;
*i0; // expected-warning{{Invalidated iterator accessed}}
}
+
+void good_push_back_list1(std::list &L, int n) {
+ auto i0 = L.cbegin(), i1 = L.cend();
+ L.push_back(n);
+ *i0; // no-warning
+ --i1; // no-warning
+}
+
+void good_push_back_vector1(std::vector &V, int n) {
+ auto i0 = V.cbegin(), i1 = V.cend();
+ V.push_back(n);
+ *i0; // no-warning
+}
+
+void bad_push_back_vector1(std::vector &V, int n) {
+ auto i0 = V.cbegin(), i1 = V.cend();
+ V.push_back(n);
+ --i1; // expected-warning{{Invalidated iterator accessed}}
+}
+
+void bad_push_back_deque1(std::deque &D, int n) {
+ auto i0 = D.cbegin(), i1 = D.cend();
+ D.push_back(n);
+ *i0; // expected-warning{{Invalidated iterator accessed}}
+ --i1; // expected-warning{{Invalidated iterator accessed}}
+}
+
+void good_emplace_back_list1(std::list &L, int n) {
+ auto i0 = L.cbegin(), i1 = L.cend();
+ L.emplace_back(n);
+ *i0; // no-warning
+ --i1; // no-warning
+}
+
+void good_emplace_back_vector1(std::vector &V, int n) {
+ auto i0 = V.cbegin(), i1 = V.cend();
+ V.emplace_back(n);
+ *i0; // no-warning
+}
+
+void bad_emplace_back_vector1(std::vector &V, int n) {
+ auto i0 = V.cbegin(), i1 = V.cend();
+ V.emplace_back(n);
+ --i1; // expected-warning{{Invalidated iterator accessed}}
+}
+
+void bad_emplace_back_deque1(std::deque &D, int n) {
+ auto i0 = D.cbegin(), i1 = D.cend();
+ D.emplace_back(n);
+ *i0; // expected-warning{{Invalidated iterator accessed}}
+ --i1; // expected-warning{{Invalidated iterator accessed}}
+}
+
+void good_pop_back_list1(std::list &L, int n) {
+ auto i0 = L.cbegin(), i1 = L.cend(), i2 = i1--;
+ L.pop_back();
+ *i0; // no-warning
+ *i2; // no-warning
+}
+
+void bad_pop_back_list1(std::list &L, int n) {
+ auto i0 = L.cbegin(), i1 = L.cend(), i2 = i1--;
+ L.pop_back();
+ *i1; // expected-warning{{Invalidated iterator accessed}}
+}
+
+void good_pop_back_vector1(std::vector &V, int n) {
+ auto i0 = V.cbegin(), i1 = V.cend(), i2 = i1--;
+ V.pop_back();
+ *i0; // no-warning
+}
+
+void bad_pop_back_vector1(std::vector &V, int n) {
+ auto i0 = V.cbegin(), i1 = V.cend(), i2 = i1--;
+ V.pop_back();
+ *i1; // expected-warning{{Invalidated iterator accessed}}
+ --i2; // expected-warning{{Invalidated iterator accessed}}
+}
+
+voi
[PATCH] D46187: [Analyzer] getRegisteredCheckers(): handle debug checkers too.
lebedev.ri created this revision.
lebedev.ri added reviewers: dcoughlin, alexfh, aaron.ballman, george.karpenkov,
NoQ.
Herald added subscribers: a.sidorin, szepet, xazax.hun.
I would like to be able to trigger these debug checkers from clang-tidy, as a
follow-up patch for https://reviews.llvm.org/D46159
Repository:
rC Clang
https://reviews.llvm.org/D46187
Files:
include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
unittests/StaticAnalyzer/AnalyzerOptionsTest.cpp
Index: unittests/StaticAnalyzer/AnalyzerOptionsTest.cpp
===
--- unittests/StaticAnalyzer/AnalyzerOptionsTest.cpp
+++ unittests/StaticAnalyzer/AnalyzerOptionsTest.cpp
@@ -21,12 +21,18 @@
auto IsAlphaChecker = [](StringRef CheckerName) {
return CheckerName.startswith("alpha");
};
- const auto &AllCheckers =
- AnalyzerOptions::getRegisteredCheckers(/*IncludeExperimental=*/true);
- EXPECT_FALSE(llvm::any_of(AllCheckers, IsDebugChecker));
+ const auto &AllCheckers = AnalyzerOptions::getRegisteredCheckers(
+ /*IncludeExperimental=*/true, /*IncludeDebug=*/true);
+ EXPECT_TRUE(llvm::any_of(AllCheckers, IsDebugChecker));
EXPECT_TRUE(llvm::any_of(AllCheckers, IsAlphaChecker));
- const auto &StableCheckers = AnalyzerOptions::getRegisteredCheckers();
+ const auto &AllNonDebugCheckers = AnalyzerOptions::getRegisteredCheckers(
+ /*IncludeExperimental=*/true, /*IncludeDebug=*/false);
+ EXPECT_FALSE(llvm::any_of(AllNonDebugCheckers, IsDebugChecker));
+ EXPECT_TRUE(llvm::any_of(AllNonDebugCheckers, IsAlphaChecker));
+
+ const auto &StableCheckers = AnalyzerOptions::getRegisteredCheckers(
+ /*IncludeExperimental=false,IncludeDebug=false*/);
EXPECT_FALSE(llvm::any_of(StableCheckers, IsDebugChecker));
EXPECT_FALSE(llvm::any_of(StableCheckers, IsAlphaChecker));
}
Index: lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
===
--- lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
+++ lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
@@ -31,7 +31,8 @@
using namespace llvm;
std::vector
-AnalyzerOptions::getRegisteredCheckers(bool IncludeExperimental /* = false */)
{
+AnalyzerOptions::getRegisteredCheckers(bool IncludeExperimental /* = false */,
+ bool IncludeDebug /* = false */) {
static const StringRef StaticAnalyzerChecks[] = {
#define GET_CHECKERS
#define CHECKER(FULLNAME, CLASS, DESCFILE, HELPTEXT, GROUPINDEX, HIDDEN)
\
@@ -42,9 +43,10 @@
};
std::vector Result;
for (StringRef CheckName : StaticAnalyzerChecks) {
-if (!CheckName.startswith("debug.") &&
-(IncludeExperimental || !CheckName.startswith("alpha.")))
- Result.push_back(CheckName);
+if ((CheckName.startswith("alpha.") && !IncludeExperimental) ||
+(CheckName.startswith("debug.") && !IncludeDebug))
+ continue;
+Result.push_back(CheckName);
}
return Result;
}
Index: include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
===
--- include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
+++ include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
@@ -126,7 +126,8 @@
using ConfigTable = llvm::StringMap;
static std::vector
- getRegisteredCheckers(bool IncludeExperimental = false);
+ getRegisteredCheckers(bool IncludeExperimental = false,
+bool IncludeDebug = false);
/// \brief Pair of checker name and enable/disable.
std::vector> CheckersControlList;
Index: unittests/StaticAnalyzer/AnalyzerOptionsTest.cpp
===
--- unittests/StaticAnalyzer/AnalyzerOptionsTest.cpp
+++ unittests/StaticAnalyzer/AnalyzerOptionsTest.cpp
@@ -21,12 +21,18 @@
auto IsAlphaChecker = [](StringRef CheckerName) {
return CheckerName.startswith("alpha");
};
- const auto &AllCheckers =
- AnalyzerOptions::getRegisteredCheckers(/*IncludeExperimental=*/true);
- EXPECT_FALSE(llvm::any_of(AllCheckers, IsDebugChecker));
+ const auto &AllCheckers = AnalyzerOptions::getRegisteredCheckers(
+ /*IncludeExperimental=*/true, /*IncludeDebug=*/true);
+ EXPECT_TRUE(llvm::any_of(AllCheckers, IsDebugChecker));
EXPECT_TRUE(llvm::any_of(AllCheckers, IsAlphaChecker));
- const auto &StableCheckers = AnalyzerOptions::getRegisteredCheckers();
+ const auto &AllNonDebugCheckers = AnalyzerOptions::getRegisteredCheckers(
+ /*IncludeExperimental=*/true, /*IncludeDebug=*/false);
+ EXPECT_FALSE(llvm::any_of(AllNonDebugCheckers, IsDebugChecker));
+ EXPECT_TRUE(llvm::any_of(AllNonDebugCheckers, IsAlphaChecker));
+
+ const auto &StableCheckers = AnalyzerOptions::getRegisteredCheckers(
+ /*IncludeExperimental=false,IncludeDebug=false*/);
EXPECT_FALSE(llvm::any_of(StableCheckers, IsDebugChecker));
EXPECT_FALSE(llvm::any_of(StableCheckers, IsAlphaChecker));
}
I
[PATCH] D46188: [clang-tidy] Add a flag to enable debug checkers
lebedev.ri created this revision.
lebedev.ri added reviewers: alexfh, aaron.ballman, hokein.
Herald added a subscriber: xazax.hun.
After https://reviews.llvm.org/D46159 was created by @pfultz2, and i initially
thought it was already possible, i had the need to view the call graph.
I have almost wrote the clang-tidy check for that, but then i remembered of
`clang-analyzer-debug.ViewCallGraph`.
Naturally, it did not work out-of-the-box, thus this patch.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D46188
Files:
clang-tidy/ClangTidy.cpp
clang-tidy/ClangTidyOptions.cpp
clang-tidy/ClangTidyOptions.h
clang-tidy/tool/ClangTidyMain.cpp
docs/clang-tidy/index.rst
test/clang-tidy/enable-debug-checks.cpp
Index: test/clang-tidy/enable-debug-checks.cpp
===
--- /dev/null
+++ test/clang-tidy/enable-debug-checks.cpp
@@ -0,0 +1,6 @@
+// Check if '-enable-debug-checks' is visible for users
+// RUN: clang-tidy -help | not grep 'enable-debug-checks'
+
+// Check if '-enable-debug-checks' enables debug checks.
+// RUN: clang-tidy -checks=* -list-checks | not grep 'clang-analyzer-debug'
+// RUN: clang-tidy -checks=* -list-checks -enable-debug-checks | grep 'clang-analyzer-debug'
Index: docs/clang-tidy/index.rst
===
--- docs/clang-tidy/index.rst
+++ docs/clang-tidy/index.rst
@@ -117,6 +117,10 @@
clang-analyzer- checks.
This option overrides the value read from a
.clang-tidy file.
+-enable-alpha-checks - This option is not visible in help.
+ It allows to enable clang analyzer Alpha checks.
+-enable-debug-checks - This option is not visible in help.
+ It allows to enable clang analyzer Debug checks.
-checks= -
Comma-separated list of globs with optional '-'
prefix. Globs are processed in order of
Index: clang-tidy/tool/ClangTidyMain.cpp
===
--- clang-tidy/tool/ClangTidyMain.cpp
+++ clang-tidy/tool/ClangTidyMain.cpp
@@ -198,6 +198,12 @@
static cl::opt EnableAlphaChecks("enable-alpha-checks", cl::init(false),
cl::Hidden, cl::cat(ClangTidyCategory));
+/// This option Enables debug checkers from the static analyzer.
+/// This option is set to false and not visible in help, because they are not
+/// for general usage.
+static cl::opt EnableDebugChecks("enable-debug-checks", cl::init(false),
+ cl::Hidden, cl::cat(ClangTidyCategory));
+
static cl::opt ExportFixes("export-fixes", cl::desc(R"(
YAML file to store suggested fixes in. The
stored fixes can be applied to the input source
@@ -308,6 +314,7 @@
DefaultOptions.SystemHeaders = SystemHeaders;
DefaultOptions.AnalyzeTemporaryDtors = AnalyzeTemporaryDtors;
DefaultOptions.EnableAlphaChecks = EnableAlphaChecks;
+ DefaultOptions.EnableDebugChecks = EnableDebugChecks;
DefaultOptions.FormatStyle = FormatStyle;
DefaultOptions.User = llvm::sys::Process::GetEnv("USER");
// USERNAME is used on Windows.
Index: clang-tidy/ClangTidyOptions.h
===
--- clang-tidy/ClangTidyOptions.h
+++ clang-tidy/ClangTidyOptions.h
@@ -80,6 +80,9 @@
/// \brief Turns on experimental alpha checkers from the static analyzer.
llvm::Optional EnableAlphaChecks;
+ /// \brief Turns on debug checkers from the static analyzer.
+ llvm::Optional EnableDebugChecks;
+
/// \brief Format code around applied fixes with clang-format using this
/// style.
///
Index: clang-tidy/ClangTidyOptions.cpp
===
--- clang-tidy/ClangTidyOptions.cpp
+++ clang-tidy/ClangTidyOptions.cpp
@@ -88,6 +88,7 @@
IO.mapOptional("HeaderFilterRegex", Options.HeaderFilterRegex);
IO.mapOptional("AnalyzeTemporaryDtors", Options.AnalyzeTemporaryDtors);
IO.mapOptional("EnableAlphaChecks", Options.EnableAlphaChecks);
+IO.mapOptional("EnableDebugChecks", Options.EnableDebugChecks);
IO.mapOptional("FormatStyle", Options.FormatStyle);
IO.mapOptional("User", Options.User);
IO.mapOptional("CheckOptions", NOpts->Options);
@@ -110,6 +111,7 @@
Options.SystemHeaders = false;
Options.AnalyzeTemporaryDtors = false;
Options.EnableAlphaChecks = false;
+ Options.EnableDebugChecks = false;
Options.FormatStyle = "none";
Options.User = llvm::None;
for (ClangTidyModuleRegistry::iterator I = ClangTidyModuleRegistry::begin(),
@@ -151,6 +153,7 @@
overrideValue(Result.SystemHeaders, Other.SystemHeaders);
overrideValue(Result.AnalyzeTemporaryDtors, Other.AnalyzeTemporaryDtors);
o
[PATCH] D44143: [clang-tidy] Create properly seeded random generator check
boga95 updated this revision to Diff 144336.
https://reviews.llvm.org/D44143
Files:
clang-tidy/cert/CERTTidyModule.cpp
clang-tidy/cert/CMakeLists.txt
clang-tidy/cert/ProperlySeededRandomGeneratorCheck.cpp
clang-tidy/cert/ProperlySeededRandomGeneratorCheck.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/cert-msc32-c.rst
docs/clang-tidy/checks/cert-msc51-cpp.rst
test/clang-tidy/cert-msc32-c.c
test/clang-tidy/cert-msc51-cpp.cpp
Index: test/clang-tidy/cert-msc51-cpp.cpp
===
--- /dev/null
+++ test/clang-tidy/cert-msc51-cpp.cpp
@@ -0,0 +1,201 @@
+// RUN: %check_clang_tidy %s cert-msc51-cpp %t -- -config="{CheckOptions: [{key: cert-msc51-cpp.DisallowedSeedTypes, value: 'some_type,time_t'}]}" -- -std=c++11
+
+namespace std {
+
+template
+struct linear_congruential_engine {
+ linear_congruential_engine(int _ = 0);
+ void seed(int _ = 0);
+};
+using default_random_engine = linear_congruential_engine;
+
+using size_t = int;
+template
+struct mersenne_twister_engine {
+ mersenne_twister_engine(int _ = 0);
+ void seed(int _ = 0);
+};
+using mt19937 = mersenne_twister_engine;
+
+template
+struct subtract_with_carry_engine {
+ subtract_with_carry_engine(int _ = 0);
+ void seed(int _ = 0);
+};
+using ranlux24_base = subtract_with_carry_engine;
+
+template
+struct discard_block_engine {
+ discard_block_engine();
+ discard_block_engine(int _);
+ void seed();
+ void seed(int _);
+};
+using ranlux24 = discard_block_engine;
+
+template
+struct independent_bits_engine {
+ independent_bits_engine();
+ independent_bits_engine(int _);
+ void seed();
+ void seed(int _);
+};
+using independent_bits = independent_bits_engine;
+
+template
+struct shuffle_order_engine {
+ shuffle_order_engine();
+ shuffle_order_engine(int _);
+ void seed();
+ void seed(int _);
+};
+using shuffle_order = shuffle_order_engine;
+
+struct random_device {
+ random_device();
+ int operator()();
+};
+} // namespace std
+
+using time_t = unsigned int;
+time_t time(time_t *t);
+
+void f() {
+ const int seed = 2;
+ time_t t;
+
+ // One instantiation for every engine
+ std::default_random_engine engine1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: random number generator seeded with a default argument will generate a predictable sequence of values [cert-msc51-cpp]
+ std::default_random_engine engine2(1);
+ // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: random number generator seeded with a constant value will generate a predictable sequence of values [cert-msc51-cpp]
+ std::default_random_engine engine3(seed);
+ // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: random number generator seeded with a constant value will generate a predictable sequence of values [cert-msc51-cpp]
+ std::default_random_engine engine4(time(&t));
+ // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: random number generator seeded with a disallowed source of seed value will generate a predictable sequence of values [cert-msc51-cpp]
+ engine1.seed();
+ // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: random number generator seeded with a default argument will generate a predictable sequence of values [cert-msc51-cpp]
+ engine1.seed(1);
+ // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: random number generator seeded with a constant value will generate a predictable sequence of values [cert-msc51-cpp]
+ engine1.seed(seed);
+ // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: random number generator seeded with a constant value will generate a predictable sequence of values [cert-msc51-cpp]
+ engine1.seed(time(&t));
+ // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: random number generator seeded with a disallowed source of seed value will generate a predictable sequence of values [cert-msc51-cpp]
+
+ std::mt19937 engine5;
+ // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: random number generator seeded with a default argument will generate a predictable sequence of values [cert-msc51-cpp]
+ std::mt19937 engine6(1);
+ // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: random number generator seeded with a constant value will generate a predictable sequence of values [cert-msc51-cpp]
+ std::mt19937 engine7(seed);
+ // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: random number generator seeded with a constant value will generate a predictable sequence of values [cert-msc51-cpp]
+ std::mt19937 engine8(time(&t));
+ // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: random number generator seeded with a disallowed source of seed value will generate a predictable sequence of values [cert-msc51-cpp]
+ engine5.seed();
+ // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: random number generator seeded with a default argument will generate a predictable sequence of values [cert-msc51-cpp]
+ engine5.seed(1);
+ // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: random number generator seeded with a constant value will generate a predictable sequence of values [cert-msc51-cpp]
+ engine5.seed(seed);
+ // CHECK-MESSAGES: :[
[PATCH] D46108: [ARM] Add __ARM_FEATURE_DOTPROD pre-defined macro
This revision was automatically updated to reflect the committed changes.
Closed by commit rC331038: [ARM] Add __ARM_FEATURE_DOTPROD pre-defined macro
(authored by olista01, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D46108?vs=144080&id=144339#toc
Repository:
rC Clang
https://reviews.llvm.org/D46108
Files:
lib/Basic/Targets/ARM.cpp
lib/Basic/Targets/ARM.h
test/Preprocessor/arm-target-features.c
Index: test/Preprocessor/arm-target-features.c
===
--- test/Preprocessor/arm-target-features.c
+++ test/Preprocessor/arm-target-features.c
@@ -6,6 +6,7 @@
// CHECK-V8A: #define __ARM_FEATURE_DIRECTED_ROUNDING 1
// CHECK-V8A: #define __ARM_FEATURE_NUMERIC_MAXMIN 1
// CHECK-V8A-NOT: #define __ARM_FP 0x
+// CHECK-V8A-NOT: #define __ARM_FEATURE_DOTPROD
// RUN: %clang -target armv8a-none-linux-gnueabi -x c -E -dM %s -o - |
FileCheck -match-full-lines --check-prefix=CHECK-V8A-ALLOW-FP-INSTR %s
// RUN: %clang -target armv8a-none-linux-gnueabihf -x c -E -dM %s -o - |
FileCheck -match-full-lines --check-prefix=CHECK-V8A-ALLOW-FP-INSTR %s
@@ -18,6 +19,7 @@
// CHECK-V8A-ALLOW-FP-INSTR: #define __ARM_FP 0xe
// CHECK-V8A-ALLOW-FP-INSTR: #define __ARM_FP16_ARGS 1
// CHECK-V8A-ALLOW-FP-INSTR: #define __ARM_FP16_FORMAT_IEEE 1
+// CHECK-V8A-ALLOW-FP-INSTR-V8A-NOT: #define __ARM_FEATURE_DOTPROD
// RUN: %clang -target arm-none-linux-gnueabi -march=armv8.2a+fp16 -x c -E -dM
%s -o - | FileCheck -match-full-lines
--check-prefix=CHECK-FULLFP16-VECTOR-SCALAR %s
// CHECK-FULLFP16-VECTOR-SCALAR: #define __ARM_FEATURE_FP16_SCALAR_ARITHMETIC 1
@@ -30,6 +32,9 @@
// CHECK-FULLFP16-SCALAR-NOT: #define __ARM_FEATURE_FP16_VECTOR_ARITHMETIC 1
// CHECK-FULLFP16-SCALAR: #define __ARM_FP 0xe
// CHECK-FULLFP16-SCALAR: #define __ARM_FP16_FORMAT_IEEE 1
+//
+// RUN: %clang -target arm-none-linux-gnueabi -march=armv8.2a+dotprod -x c -E
-dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-DOTPROD %s
+// CHECK-DOTPROD: #define __ARM_FEATURE_DOTPROD 1
// RUN: %clang -target armv8r-none-linux-gnu -x c -E -dM %s -o - | FileCheck
-match-full-lines --check-prefix=CHECK-V8R %s
// CHECK-V8R: #define __ARMEL__ 1
Index: lib/Basic/Targets/ARM.cpp
===
--- lib/Basic/Targets/ARM.cpp
+++ lib/Basic/Targets/ARM.cpp
@@ -390,6 +390,7 @@
Unaligned = 1;
SoftFloat = SoftFloatABI = false;
HWDiv = 0;
+ DotProd = 0;
// This does not diagnose illegal cases like having both
// "+vfpv2" and "+vfpv3" or having "+neon" and "+fp-only-sp".
@@ -432,6 +433,8 @@
HW_FP |= HW_FP_HP;
} else if (Feature == "+fullfp16") {
HasLegalHalfType = true;
+} else if (Feature == "+dotprod") {
+ DotProd = true;
}
}
HW_FP &= ~HW_FP_remove;
@@ -731,6 +734,9 @@
if (HasLegalHalfType)
Builder.defineMacro("__ARM_FEATURE_FP16_SCALAR_ARITHMETIC", "1");
+ // Armv8.2-A dot product intrinsics
+ if (DotProd)
+Builder.defineMacro("__ARM_FEATURE_DOTPROD", "1");
switch (ArchKind) {
default:
Index: lib/Basic/Targets/ARM.h
===
--- lib/Basic/Targets/ARM.h
+++ lib/Basic/Targets/ARM.h
@@ -69,6 +69,7 @@
unsigned Crypto : 1;
unsigned DSP : 1;
unsigned Unaligned : 1;
+ unsigned DotProd : 1;
enum {
LDREX_B = (1 << 0), /// byte (8-bit)
Index: test/Preprocessor/arm-target-features.c
===
--- test/Preprocessor/arm-target-features.c
+++ test/Preprocessor/arm-target-features.c
@@ -6,6 +6,7 @@
// CHECK-V8A: #define __ARM_FEATURE_DIRECTED_ROUNDING 1
// CHECK-V8A: #define __ARM_FEATURE_NUMERIC_MAXMIN 1
// CHECK-V8A-NOT: #define __ARM_FP 0x
+// CHECK-V8A-NOT: #define __ARM_FEATURE_DOTPROD
// RUN: %clang -target armv8a-none-linux-gnueabi -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-V8A-ALLOW-FP-INSTR %s
// RUN: %clang -target armv8a-none-linux-gnueabihf -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-V8A-ALLOW-FP-INSTR %s
@@ -18,6 +19,7 @@
// CHECK-V8A-ALLOW-FP-INSTR: #define __ARM_FP 0xe
// CHECK-V8A-ALLOW-FP-INSTR: #define __ARM_FP16_ARGS 1
// CHECK-V8A-ALLOW-FP-INSTR: #define __ARM_FP16_FORMAT_IEEE 1
+// CHECK-V8A-ALLOW-FP-INSTR-V8A-NOT: #define __ARM_FEATURE_DOTPROD
// RUN: %clang -target arm-none-linux-gnueabi -march=armv8.2a+fp16 -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-FULLFP16-VECTOR-SCALAR %s
// CHECK-FULLFP16-VECTOR-SCALAR: #define __ARM_FEATURE_FP16_SCALAR_ARITHMETIC 1
@@ -30,6 +32,9 @@
// CHECK-FULLFP16-SCALAR-NOT: #define __ARM_FEATURE_FP16_VECTOR_ARITHMETIC 1
// CHECK-FULLFP16-SCALAR: #define __ARM_FP 0xe
// CHECK-FULLFP16-SCALAR: #define __ARM_FP16_FORMAT_IEEE 1
+//
+// RUN: %clang -target arm-none-linux-gnueabi -march=armv8.2a+dotprod -x c -E -dM %s -o - | FileCheck -match-full
r331038 - [ARM] Add __ARM_FEATURE_DOTPROD pre-defined macro
Author: olista01
Date: Fri Apr 27 06:56:02 2018
New Revision: 331038
URL: http://llvm.org/viewvc/llvm-project?rev=331038&view=rev
Log:
[ARM] Add __ARM_FEATURE_DOTPROD pre-defined macro
This adds a pre-defined macro to test if the compiler has support for the
v8.2-A dot rpoduct intrinsics in AArch32 mode.
The AAcrh64 equivalent has already been added by rL330229.
The ACLE spec which describes this macro hasn't been published yet, but this is
based on the final internal draft, and GCC has already implemented this.
Differential revision: https://reviews.llvm.org/D46108
Modified:
cfe/trunk/lib/Basic/Targets/ARM.cpp
cfe/trunk/lib/Basic/Targets/ARM.h
cfe/trunk/test/Preprocessor/arm-target-features.c
Modified: cfe/trunk/lib/Basic/Targets/ARM.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets/ARM.cpp?rev=331038&r1=331037&r2=331038&view=diff
==
--- cfe/trunk/lib/Basic/Targets/ARM.cpp (original)
+++ cfe/trunk/lib/Basic/Targets/ARM.cpp Fri Apr 27 06:56:02 2018
@@ -390,6 +390,7 @@ bool ARMTargetInfo::handleTargetFeatures
Unaligned = 1;
SoftFloat = SoftFloatABI = false;
HWDiv = 0;
+ DotProd = 0;
// This does not diagnose illegal cases like having both
// "+vfpv2" and "+vfpv3" or having "+neon" and "+fp-only-sp".
@@ -432,6 +433,8 @@ bool ARMTargetInfo::handleTargetFeatures
HW_FP |= HW_FP_HP;
} else if (Feature == "+fullfp16") {
HasLegalHalfType = true;
+} else if (Feature == "+dotprod") {
+ DotProd = true;
}
}
HW_FP &= ~HW_FP_remove;
@@ -731,6 +734,9 @@ void ARMTargetInfo::getTargetDefines(con
if (HasLegalHalfType)
Builder.defineMacro("__ARM_FEATURE_FP16_SCALAR_ARITHMETIC", "1");
+ // Armv8.2-A dot product intrinsics
+ if (DotProd)
+Builder.defineMacro("__ARM_FEATURE_DOTPROD", "1");
switch (ArchKind) {
default:
Modified: cfe/trunk/lib/Basic/Targets/ARM.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets/ARM.h?rev=331038&r1=331037&r2=331038&view=diff
==
--- cfe/trunk/lib/Basic/Targets/ARM.h (original)
+++ cfe/trunk/lib/Basic/Targets/ARM.h Fri Apr 27 06:56:02 2018
@@ -69,6 +69,7 @@ class LLVM_LIBRARY_VISIBILITY ARMTargetI
unsigned Crypto : 1;
unsigned DSP : 1;
unsigned Unaligned : 1;
+ unsigned DotProd : 1;
enum {
LDREX_B = (1 << 0), /// byte (8-bit)
Modified: cfe/trunk/test/Preprocessor/arm-target-features.c
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Preprocessor/arm-target-features.c?rev=331038&r1=331037&r2=331038&view=diff
==
--- cfe/trunk/test/Preprocessor/arm-target-features.c (original)
+++ cfe/trunk/test/Preprocessor/arm-target-features.c Fri Apr 27 06:56:02 2018
@@ -6,6 +6,7 @@
// CHECK-V8A: #define __ARM_FEATURE_DIRECTED_ROUNDING 1
// CHECK-V8A: #define __ARM_FEATURE_NUMERIC_MAXMIN 1
// CHECK-V8A-NOT: #define __ARM_FP 0x
+// CHECK-V8A-NOT: #define __ARM_FEATURE_DOTPROD
// RUN: %clang -target armv8a-none-linux-gnueabi -x c -E -dM %s -o - |
FileCheck -match-full-lines --check-prefix=CHECK-V8A-ALLOW-FP-INSTR %s
// RUN: %clang -target armv8a-none-linux-gnueabihf -x c -E -dM %s -o - |
FileCheck -match-full-lines --check-prefix=CHECK-V8A-ALLOW-FP-INSTR %s
@@ -18,6 +19,7 @@
// CHECK-V8A-ALLOW-FP-INSTR: #define __ARM_FP 0xe
// CHECK-V8A-ALLOW-FP-INSTR: #define __ARM_FP16_ARGS 1
// CHECK-V8A-ALLOW-FP-INSTR: #define __ARM_FP16_FORMAT_IEEE 1
+// CHECK-V8A-ALLOW-FP-INSTR-V8A-NOT: #define __ARM_FEATURE_DOTPROD
// RUN: %clang -target arm-none-linux-gnueabi -march=armv8.2a+fp16 -x c -E -dM
%s -o - | FileCheck -match-full-lines
--check-prefix=CHECK-FULLFP16-VECTOR-SCALAR %s
// CHECK-FULLFP16-VECTOR-SCALAR: #define __ARM_FEATURE_FP16_SCALAR_ARITHMETIC 1
@@ -30,6 +32,9 @@
// CHECK-FULLFP16-SCALAR-NOT: #define __ARM_FEATURE_FP16_VECTOR_ARITHMETIC 1
// CHECK-FULLFP16-SCALAR: #define __ARM_FP 0xe
// CHECK-FULLFP16-SCALAR: #define __ARM_FP16_FORMAT_IEEE 1
+//
+// RUN: %clang -target arm-none-linux-gnueabi -march=armv8.2a+dotprod -x c -E
-dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-DOTPROD %s
+// CHECK-DOTPROD: #define __ARM_FEATURE_DOTPROD 1
// RUN: %clang -target armv8r-none-linux-gnu -x c -E -dM %s -o - | FileCheck
-match-full-lines --check-prefix=CHECK-V8R %s
// CHECK-V8R: #define __ARMEL__ 1
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46109: [ARM, AArch64] Add intrinsics for dot product instructions
olista01 marked an inline comment as done. olista01 added inline comments. Comment at: test/CodeGen/arm-neon-dot-product.c:1 +// RUN: %clang_cc1 -triple armv8-linux-gnueabihf -target-cpu cortex-a57 -target-feature +dotprod \ +// RUN: -disable-O0-optnone -emit-llvm -o - %s | opt -S -instcombine | FileCheck %s efriedma wrote: > flyingforyou wrote: > > efriedma wrote: > > > flyingforyou wrote: > > > > I think proper target is cortex-a55 or cortex-a75. > > > > Do we need check routines for wrong target-cpu? > > > This is working as intended, I think: target-feature overrides target-cpu. > > dotprod is ARMv8.2's addition feature not ARMv8. Cortex-a57 only supports > > ARMv8 which couldn't have dotprod feature. Am I missing something? > Oh, sorry, misunderstood the question; yes, this might not be the most clear > "CHECK" line. I've changed this to cortex-a75 for clarity. Repository: rL LLVM https://reviews.llvm.org/D46109 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r331039 - [ARM,AArch64] Add intrinsics for dot product instructions
Author: olista01
Date: Fri Apr 27 07:03:32 2018
New Revision: 331039
URL: http://llvm.org/viewvc/llvm-project?rev=331039&view=rev
Log:
[ARM,AArch64] Add intrinsics for dot product instructions
The ACLE spec which describes these intrinsics hasn't been published yet, but
this is based on the final draft which will be published soon, and these have
already been implemented by GCC.
Differential revision: https://reviews.llvm.org/D46109
Added:
cfe/trunk/test/CodeGen/aarch64-neon-dot-product.c
cfe/trunk/test/CodeGen/arm-neon-dot-product.c
Modified:
cfe/trunk/include/clang/Basic/arm_neon.td
cfe/trunk/include/clang/Basic/arm_neon_incl.td
cfe/trunk/lib/CodeGen/CGBuiltin.cpp
cfe/trunk/utils/TableGen/NeonEmitter.cpp
Modified: cfe/trunk/include/clang/Basic/arm_neon.td
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/arm_neon.td?rev=331039&r1=331038&r2=331039&view=diff
==
--- cfe/trunk/include/clang/Basic/arm_neon.td (original)
+++ cfe/trunk/include/clang/Basic/arm_neon.td Fri Apr 27 07:03:32 2018
@@ -199,6 +199,13 @@ def OP_SCALAR_HALF_SET_LNQ : Op<(bitcast
(bitcast "int16_t", $p0),
(bitcast "int16x8_t", $p1), $p2))>;
+def OP_DOT_LN
+: Op<(call "vdot", $p0, $p1,
+ (bitcast $p1, (splat(bitcast "uint32x2_t", $p2), $p3)))>;
+def OP_DOT_LNQ
+: Op<(call "vdot", $p0, $p1,
+ (bitcast $p1, (splat(bitcast "uint32x4_t", $p2), $p3)))>;
+
//===--===//
// Instructions
//===--===//
@@ -1579,3 +1586,13 @@ let ArchGuard = "defined(__ARM_FEATURE_F
def SCALAR_VDUP_LANEH : IInst<"vdup_lane", "sdi", "Sh">;
def SCALAR_VDUP_LANEQH : IInst<"vdup_laneq", "sji", "Sh">;
}
+
+// v8.2-A dot product instructions.
+let ArchGuard = "defined(__ARM_FEATURE_DOTPROD)" in {
+ def DOT : SInst<"vdot", "dd88", "iQiUiQUi">;
+ def DOT_LANE : SOpInst<"vdot_lane", "dd87i", "iUiQiQUi", OP_DOT_LN>;
+}
+let ArchGuard = "defined(__ARM_FEATURE_DOTPROD) && defined(__aarch64__)" in {
+ // Variants indexing into a 128-bit vector are A64 only.
+ def UDOT_LANEQ : SOpInst<"vdot_laneq", "dd89i", "iUiQiQUi", OP_DOT_LNQ>;
+}
Modified: cfe/trunk/include/clang/Basic/arm_neon_incl.td
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/arm_neon_incl.td?rev=331039&r1=331038&r2=331039&view=diff
==
--- cfe/trunk/include/clang/Basic/arm_neon_incl.td (original)
+++ cfe/trunk/include/clang/Basic/arm_neon_incl.td Fri Apr 27 07:03:32 2018
@@ -253,6 +253,9 @@ def OP_UNAVAILABLE : Operation {
// B,C,D: array of default elts, force 'Q' size modifier.
// p: pointer type
// c: const pointer type
+// 7: vector of 8-bit elements, ignore 'Q' size modifier
+// 8: vector of 8-bit elements, same width as default type
+// 9: vector of 8-bit elements, force 'Q' size modifier
// Every intrinsic subclasses Inst.
class Inst {
Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBuiltin.cpp?rev=331039&r1=331038&r2=331039&view=diff
==
--- cfe/trunk/lib/CodeGen/CGBuiltin.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp Fri Apr 27 07:03:32 2018
@@ -3867,6 +3867,8 @@ static const NeonIntrinsicInfo ARMSIMDIn
NEONMAP0(vcvtq_u16_v),
NEONMAP0(vcvtq_u32_v),
NEONMAP0(vcvtq_u64_v),
+ NEONMAP2(vdot_v, arm_neon_udot, arm_neon_sdot, 0),
+ NEONMAP2(vdotq_v, arm_neon_udot, arm_neon_sdot, 0),
NEONMAP0(vext_v),
NEONMAP0(vextq_v),
NEONMAP0(vfma_v),
@@ -4061,6 +4063,8 @@ static const NeonIntrinsicInfo AArch64SI
NEONMAP1(vcvtq_n_u32_v, aarch64_neon_vcvtfp2fxu, 0),
NEONMAP1(vcvtq_n_u64_v, aarch64_neon_vcvtfp2fxu, 0),
NEONMAP1(vcvtx_f32_v, aarch64_neon_fcvtxn, AddRetType | Add1ArgType),
+ NEONMAP2(vdot_v, aarch64_neon_udot, aarch64_neon_sdot, 0),
+ NEONMAP2(vdotq_v, aarch64_neon_udot, aarch64_neon_sdot, 0),
NEONMAP0(vext_v),
NEONMAP0(vextq_v),
NEONMAP0(vfma_v),
@@ -4974,6 +4978,14 @@ Value *CodeGenFunction::EmitCommonNeonBu
}
return SV;
}
+ case NEON::BI__builtin_neon_vdot_v:
+ case NEON::BI__builtin_neon_vdotq_v: {
+llvm::Type *InputTy =
+llvm::VectorType::get(Int8Ty, Ty->getPrimitiveSizeInBits() / 8);
+llvm::Type *Tys[2] = { Ty, InputTy };
+Int = Usgn ? LLVMIntrinsic : AltLLVMIntrinsic;
+return EmitNeonCall(CGM.getIntrinsic(Int, Tys), Ops, "vdot");
+ }
}
assert(Int && "Expected valid intrinsic number");
Added: cfe/trunk/test/CodeGen/aarch64-neon-dot-product.c
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/aarch64-neon-dot-product.c?rev=331039&view=auto
==
[PATCH] D46109: [ARM, AArch64] Add intrinsics for dot product instructions
This revision was automatically updated to reflect the committed changes.
Closed by commit rC331039: [ARM,AArch64] Add intrinsics for dot product
instructions (authored by olista01, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D46109?vs=144081&id=144341#toc
Repository:
rC Clang
https://reviews.llvm.org/D46109
Files:
include/clang/Basic/arm_neon.td
include/clang/Basic/arm_neon_incl.td
lib/CodeGen/CGBuiltin.cpp
test/CodeGen/aarch64-neon-dot-product.c
test/CodeGen/arm-neon-dot-product.c
utils/TableGen/NeonEmitter.cpp
Index: test/CodeGen/arm-neon-dot-product.c
===
--- test/CodeGen/arm-neon-dot-product.c
+++ test/CodeGen/arm-neon-dot-product.c
@@ -0,0 +1,76 @@
+// RUN: %clang_cc1 -triple armv8-linux-gnueabihf -target-cpu cortex-a75 -target-feature +dotprod \
+// RUN: -disable-O0-optnone -emit-llvm -o - %s | opt -S -instcombine | FileCheck %s
+
+// REQUIRES: arm-registered-target
+
+// Test ARM v8.2-A dot product intrinsics
+
+#include
+
+uint32x2_t test_vdot_u32(uint32x2_t a, uint8x8_t b, uint8x8_t c) {
+// CHECK-LABEL: define <2 x i32> @test_vdot_u32(<2 x i32> %a, <8 x i8> %b, <8 x i8> %c)
+// CHECK: [[RESULT:%.*]] = call <2 x i32> @llvm.arm.neon.udot.v2i32.v8i8(<2 x i32> %a, <8 x i8> %b, <8 x i8> %c)
+// CHECK: ret <2 x i32> [[RESULT]]
+ return vdot_u32(a, b, c);
+}
+
+uint32x4_t test_vdotq_u32(uint32x4_t a, uint8x16_t b, uint8x16_t c) {
+// CHECK-LABEL: define <4 x i32> @test_vdotq_u32(<4 x i32> %a, <16 x i8> %b, <16 x i8> %c)
+// CHECK: [[RESULT:%.*]] = call <4 x i32> @llvm.arm.neon.udot.v4i32.v16i8(<4 x i32> %a, <16 x i8> %b, <16 x i8> %c)
+// CHECK: ret <4 x i32> [[RESULT]]
+ return vdotq_u32(a, b, c);
+}
+
+int32x2_t test_vdot_s32(int32x2_t a, int8x8_t b, int8x8_t c) {
+// CHECK-LABEL: define <2 x i32> @test_vdot_s32(<2 x i32> %a, <8 x i8> %b, <8 x i8> %c)
+// CHECK: [[RESULT:%.*]] = call <2 x i32> @llvm.arm.neon.sdot.v2i32.v8i8(<2 x i32> %a, <8 x i8> %b, <8 x i8> %c)
+// CHECK: ret <2 x i32> [[RESULT]]
+ return vdot_s32(a, b, c);
+}
+
+int32x4_t test_vdotq_s32(int32x4_t a, int8x16_t b, int8x16_t c) {
+// CHECK-LABEL: define <4 x i32> @test_vdotq_s32(<4 x i32> %a, <16 x i8> %b, <16 x i8> %c)
+// CHECK: [[RESULT:%.*]] = call <4 x i32> @llvm.arm.neon.sdot.v4i32.v16i8(<4 x i32> %a, <16 x i8> %b, <16 x i8> %c)
+// CHECK: ret <4 x i32> [[RESULT]]
+ return vdotq_s32(a, b, c);
+}
+
+uint32x2_t test_vdot_lane_u32(uint32x2_t a, uint8x8_t b, uint8x8_t c) {
+// CHECK-LABEL: define <2 x i32> @test_vdot_lane_u32(<2 x i32> %a, <8 x i8> %b, <8 x i8> %c)
+// CHECK: [[CAST1:%.*]] = bitcast <8 x i8> %c to <2 x i32>
+// CHECK: [[SHUFFLE:%.*]] = shufflevector <2 x i32> [[CAST1]], <2 x i32> undef, <2 x i32>
+// CHECK: [[CAST2:%.*]] = bitcast <2 x i32> [[SHUFFLE]] to <8 x i8>
+// CHECK: [[RESULT:%.*]] = call <2 x i32> @llvm.arm.neon.udot.v2i32.v8i8(<2 x i32> %a, <8 x i8> %b, <8 x i8> [[CAST2]])
+// CHECK: ret <2 x i32> [[RESULT]]
+ return vdot_lane_u32(a, b, c, 1);
+}
+
+uint32x4_t test_vdotq_lane_u32(uint32x4_t a, uint8x16_t b, uint8x8_t c) {
+// CHECK-LABEL: define <4 x i32> @test_vdotq_lane_u32(<4 x i32> %a, <16 x i8> %b, <8 x i8> %c)
+// CHECK: [[CAST1:%.*]] = bitcast <8 x i8> %c to <2 x i32>
+// CHECK: [[SHUFFLE:%.*]] = shufflevector <2 x i32> [[CAST1]], <2 x i32> undef, <4 x i32>
+// CHECK: [[CAST2:%.*]] = bitcast <4 x i32> [[SHUFFLE]] to <16 x i8>
+// CHECK: [[RESULT:%.*]] = call <4 x i32> @llvm.arm.neon.udot.v4i32.v16i8(<4 x i32> %a, <16 x i8> %b, <16 x i8> [[CAST2]])
+// CHECK: ret <4 x i32> [[RESULT]]
+ return vdotq_lane_u32(a, b, c, 1);
+}
+
+int32x2_t test_vdot_lane_s32(int32x2_t a, int8x8_t b, int8x8_t c) {
+// CHECK-LABEL: define <2 x i32> @test_vdot_lane_s32(<2 x i32> %a, <8 x i8> %b, <8 x i8> %c)
+// CHECK: [[CAST1:%.*]] = bitcast <8 x i8> %c to <2 x i32>
+// CHECK: [[SHUFFLE:%.*]] = shufflevector <2 x i32> [[CAST1]], <2 x i32> undef, <2 x i32>
+// CHECK: [[CAST2:%.*]] = bitcast <2 x i32> [[SHUFFLE]] to <8 x i8>
+// CHECK: [[RESULT:%.*]] = call <2 x i32> @llvm.arm.neon.sdot.v2i32.v8i8(<2 x i32> %a, <8 x i8> %b, <8 x i8> [[CAST2]])
+// CHECK: ret <2 x i32> [[RESULT]]
+ return vdot_lane_s32(a, b, c, 1);
+}
+
+int32x4_t test_vdotq_lane_s32(int32x4_t a, int8x16_t b, int8x8_t c) {
+// CHECK-LABEL: define <4 x i32> @test_vdotq_lane_s32(<4 x i32> %a, <16 x i8> %b, <8 x i8> %c)
+// CHECK: [[CAST1:%.*]] = bitcast <8 x i8> %c to <2 x i32>
+// CHECK: [[SHUFFLE:%.*]] = shufflevector <2 x i32> [[CAST1]], <2 x i32> undef, <4 x i32>
+// CHECK: [[CAST2:%.*]] = bitcast <4 x i32> [[SHUFFLE]] to <16 x i8>
+// CHECK: [[RESULT:%.*]] = call <4 x i32> @llvm.arm.neon.sdot.v4i32.v16i8(<4 x i32> %a, <16 x i8> %b, <16 x i8> [[CAST2]])
+// CHECK: ret <4 x i32> [[RESULT]]
+ return vdotq_lane_s32(a, b, c, 1);
+}
Index: test/CodeGen/aarch64-neon-dot-product.c
===
--- test/CodeGen/aarch64-neon-dot-product.c
+++ test/CodeGen/aarch64-neon-dot-product.c
@@ -0,0 +
[PATCH] D45815: [libclang] Allow skipping function bodies in preamble only
nik added inline comments.
Comment at: lib/Parse/ParseCXXInlineMethods.cpp:104
- if (SkipFunctionBodies && (!FnD || Actions.canSkipFunctionBody(FnD)) &&
- trySkippingFunctionBody()) {
+ if (SkipFunctionBodies != SkipFunctionBodiesKind::None &&
+ (!FnD || Actions.canSkipFunctionBody(FnD)) && trySkippingFunctionBody())
{
ilya-biryukov wrote:
> Can't this be a template too? Do we need to check for it here?
Correct. Here is also the point were we would check for the parent class being
a template.
Repository:
rC Clang
https://reviews.llvm.org/D45815
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46049: [OpenCL] Add constant address space to __func__ in AST
Anastasia added inline comments. Comment at: lib/AST/Expr.cpp:870 + if (C.getLangOpts().OpenCL && Ty.getAddressSpace() == LangAS::Default) +Ty = C.getAddrSpaceQualType(Ty, LangAS::opencl_constant); + bader wrote: > Anastasia wrote: > > bader wrote: > > > As `Ty` is passed by value, shouldn't we accept only data located in > > > constant address space? > > Do you mean to assert? Currently it should be passed with `constant` AS but > > I thought the idea is to modify this function so we can accept `Default` AS > > too but then replace by `constant`. > > > Yes, but according to your other comment this idea doesn't work. > > > I have added the address space to the creation of StringLiteral, but > > unfortunately it doesn't seems like we can remove similar code in other > > places because **QualType created for StringLiteral is also used elsewhere > > and has to match (contain right address space).** I.e. here is it used > > further down to create PredefinedExpr. > > > > Indeed for the current cases it doesn't... there might be some uncaught ones though where it can be useful. But we can add it later as well. So I am thinking to undo this? https://reviews.llvm.org/D46049 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r331041 - [Driver, CodeGen] add options to enable/disable an FP cast optimization
Author: spatel
Date: Fri Apr 27 07:22:48 2018
New Revision: 331041
URL: http://llvm.org/viewvc/llvm-project?rev=331041&view=rev
Log:
[Driver, CodeGen] add options to enable/disable an FP cast optimization
As discussed in the post-commit thread for:
rL330437 (
http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20180423/545906.html )
We need a way to opt-out of a float-to-int-to-float cast optimization because
too much
existing code relies on the platform-specific undefined result of those casts
when the
float-to-int overflows.
The LLVM changes associated with adding this function attribute are here:
rL330947
rL330950
rL330951
Also as suggested, I changed the LLVM doc to mention the specific sanitizer
flag that
catches this problem:
rL330958
Differential Revision: https://reviews.llvm.org/D46135
Added:
cfe/trunk/test/CodeGen/no-junk-ftrunc.c
Modified:
cfe/trunk/docs/UsersManual.rst
cfe/trunk/include/clang/Driver/Options.td
cfe/trunk/include/clang/Frontend/CodeGenOptions.def
cfe/trunk/lib/CodeGen/CGCall.cpp
cfe/trunk/lib/Driver/ToolChains/Clang.cpp
cfe/trunk/lib/Frontend/CompilerInvocation.cpp
cfe/trunk/test/Driver/fast-math.c
Modified: cfe/trunk/docs/UsersManual.rst
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/UsersManual.rst?rev=331041&r1=331040&r2=331041&view=diff
==
--- cfe/trunk/docs/UsersManual.rst (original)
+++ cfe/trunk/docs/UsersManual.rst Fri Apr 27 07:22:48 2018
@@ -1255,6 +1255,16 @@ are listed below.
flushed-to-zero number is preserved in the sign of 0, denormals are
flushed to positive zero, respectively.
+.. option:: -f[no-]fp-cast-overflow-workaround
+
+ Enable a workaround for code that casts floating-point values to
+ integers and back to floating-point. If the floating-point value
+ is not representable in the intermediate integer type, the code is
+ incorrect according to the language standard. This flag will attempt
+ to generate code as if the result of an overflowing conversion matches
+ the overflowing behavior of a target's native float-to-int conversion
+ instructions.
+
.. option:: -fwhole-program-vtables
Enable whole-program vtable optimizations, such as single-implementation
Modified: cfe/trunk/include/clang/Driver/Options.td
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=331041&r1=331040&r2=331041&view=diff
==
--- cfe/trunk/include/clang/Driver/Options.td (original)
+++ cfe/trunk/include/clang/Driver/Options.td Fri Apr 27 07:22:48 2018
@@ -1029,6 +1029,11 @@ def ffp_contract : Joined<["-"], "ffp-co
Flags<[CC1Option]>, HelpText<"Form fused FP ops (e.g. FMAs): fast
(everywhere)"
" | on (according to FP_CONTRACT pragma, default) | off (never fuse)">,
Values<"fast,on,off">;
+def ffp_cast_overflow_workaround : Flag<["-"],
+ "ffp-cast-overflow-workaround">, Group, Flags<[CC1Option]>;
+def fno_fp_cast_overflow_workaround : Flag<["-"],
+ "fno-fp-cast-overflow-workaround">, Group, Flags<[CC1Option]>;
+
def ffor_scope : Flag<["-"], "ffor-scope">, Group;
def fno_for_scope : Flag<["-"], "fno-for-scope">, Group;
Modified: cfe/trunk/include/clang/Frontend/CodeGenOptions.def
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/CodeGenOptions.def?rev=331041&r1=331040&r2=331041&view=diff
==
--- cfe/trunk/include/clang/Frontend/CodeGenOptions.def (original)
+++ cfe/trunk/include/clang/Frontend/CodeGenOptions.def Fri Apr 27 07:22:48 2018
@@ -136,6 +136,12 @@ CODEGENOPT(NoTrappingMath, 1, 0) ///
CODEGENOPT(NoNaNsFPMath , 1, 0) ///< Assume FP arguments, results not NaN.
CODEGENOPT(FlushDenorm , 1, 0) ///< Allow FP denorm numbers to be
flushed to zero
CODEGENOPT(CorrectlyRoundedDivSqrt, 1, 0) ///<
-cl-fp32-correctly-rounded-divide-sqrt
+
+/// Disable a float-to-int-to-float cast optimization. This attempts to
generate
+/// code as if the result of an overflowing conversion matches the overflowing
+/// behavior of a target's native float-to-int conversion instructions.
+CODEGENOPT(FPCastOverflowWorkaround, 1, 0)
+
CODEGENOPT(UniformWGSize , 1, 0) ///< -cl-uniform-work-group-size
CODEGENOPT(NoZeroInitializedInBSS , 1, 0) ///< -fno-zero-initialized-in-bss.
/// \brief Method of Objective-C dispatch to use.
Modified: cfe/trunk/lib/CodeGen/CGCall.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCall.cpp?rev=331041&r1=331040&r2=331041&view=diff
==
--- cfe/trunk/lib/CodeGen/CGCall.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGCall.cpp Fri Apr 27 07:22:48 2018
@@ -1727,6 +1727,9 @@ void CodeGenModule::ConstructDefaultFnAt
FuncAttrs.addAttribute("no-trapping-math",
[PATCH] D46135: [Driver, CodeGen] add options to enable/disable an FP cast optimization
This revision was automatically updated to reflect the committed changes.
Closed by commit rC331041: [Driver, CodeGen] add options to enable/disable an
FP cast optimization (authored by spatel, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D46135?vs=144226&id=144342#toc
Repository:
rC Clang
https://reviews.llvm.org/D46135
Files:
docs/UsersManual.rst
include/clang/Driver/Options.td
include/clang/Frontend/CodeGenOptions.def
lib/CodeGen/CGCall.cpp
lib/Driver/ToolChains/Clang.cpp
lib/Frontend/CompilerInvocation.cpp
test/CodeGen/no-junk-ftrunc.c
test/Driver/fast-math.c
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -699,6 +699,8 @@
Opts.Reciprocals = Args.getAllArgValues(OPT_mrecip_EQ);
Opts.ReciprocalMath = Args.hasArg(OPT_freciprocal_math);
Opts.NoTrappingMath = Args.hasArg(OPT_fno_trapping_math);
+ Opts.FPCastOverflowWorkaround = Args.hasArg(OPT_ffp_cast_overflow_workaround);
+
Opts.NoZeroInitializedInBSS = Args.hasArg(OPT_mno_zero_initialized_in_bss);
Opts.NumRegisterParameters = getLastArgIntValue(Args, OPT_mregparm, 0, Diags);
Opts.NoExecStack = Args.hasArg(OPT_mno_exec_stack);
Index: lib/CodeGen/CGCall.cpp
===
--- lib/CodeGen/CGCall.cpp
+++ lib/CodeGen/CGCall.cpp
@@ -1727,6 +1727,9 @@
FuncAttrs.addAttribute("no-trapping-math",
llvm::toStringRef(CodeGenOpts.NoTrappingMath));
+if (CodeGenOpts.FPCastOverflowWorkaround)
+ FuncAttrs.addAttribute("fp-cast-overflow-workaround", "true");
+
// TODO: Are these all needed?
// unsafe/inf/nan/nsz are handled by instruction-level FastMathFlags.
FuncAttrs.addAttribute("no-infs-fp-math",
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -2241,6 +2241,11 @@
CmdArgs.push_back("-mfpmath");
CmdArgs.push_back(A->getValue());
}
+
+ // Disable a codegen optimization for floating-point casts.
+ if (Args.hasFlag(options::OPT_ffp_cast_overflow_workaround,
+ options::OPT_fno_fp_cast_overflow_workaround, false))
+CmdArgs.push_back("-ffp-cast-overflow-workaround");
}
static void RenderAnalyzerOptions(const ArgList &Args, ArgStringList &CmdArgs,
Index: docs/UsersManual.rst
===
--- docs/UsersManual.rst
+++ docs/UsersManual.rst
@@ -1255,6 +1255,16 @@
flushed-to-zero number is preserved in the sign of 0, denormals are
flushed to positive zero, respectively.
+.. option:: -f[no-]fp-cast-overflow-workaround
+
+ Enable a workaround for code that casts floating-point values to
+ integers and back to floating-point. If the floating-point value
+ is not representable in the intermediate integer type, the code is
+ incorrect according to the language standard. This flag will attempt
+ to generate code as if the result of an overflowing conversion matches
+ the overflowing behavior of a target's native float-to-int conversion
+ instructions.
+
.. option:: -fwhole-program-vtables
Enable whole-program vtable optimizations, such as single-implementation
Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -1029,6 +1029,11 @@
Flags<[CC1Option]>, HelpText<"Form fused FP ops (e.g. FMAs): fast (everywhere)"
" | on (according to FP_CONTRACT pragma, default) | off (never fuse)">, Values<"fast,on,off">;
+def ffp_cast_overflow_workaround : Flag<["-"],
+ "ffp-cast-overflow-workaround">, Group, Flags<[CC1Option]>;
+def fno_fp_cast_overflow_workaround : Flag<["-"],
+ "fno-fp-cast-overflow-workaround">, Group, Flags<[CC1Option]>;
+
def ffor_scope : Flag<["-"], "ffor-scope">, Group;
def fno_for_scope : Flag<["-"], "fno-for-scope">, Group;
Index: include/clang/Frontend/CodeGenOptions.def
===
--- include/clang/Frontend/CodeGenOptions.def
+++ include/clang/Frontend/CodeGenOptions.def
@@ -136,6 +136,12 @@
CODEGENOPT(NoNaNsFPMath , 1, 0) ///< Assume FP arguments, results not NaN.
CODEGENOPT(FlushDenorm , 1, 0) ///< Allow FP denorm numbers to be flushed to zero
CODEGENOPT(CorrectlyRoundedDivSqrt, 1, 0) ///< -cl-fp32-correctly-rounded-divide-sqrt
+
+/// Disable a float-to-int-to-float cast optimization. This attempts to generate
+/// code as if the result of an overflowing conversion matches the overflowing
+/// behavior of a target's native float-to-int conversion instructions.
+CODEGENOPT(FPCastOverflowWorkaround, 1, 0)
+
CODEGENOPT(UniformWGSize , 1, 0) ///< -cl-uniform-work-group-size
CODEGENOPT(N
[PATCH] D45815: [libclang] Allow skipping function bodies in preamble only
nik added a comment.
OK, to skip all function bodies in the preamble except for template functions
and functions within a template class, I've amended the previous diff with:
- a/lib/Parse/ParseCXXInlineMethods.cpp +++
b/lib/Parse/ParseCXXInlineMethods.cpp @@ -102,9 +102,14 @@ NamedDecl
*Parser::ParseCXXInlineMethodDef(AccessSpecifier AS, }
if (SkipFunctionBodies != SkipFunctionBodiesKind::None && +
TemplateInfo.Kind == ParsedTemplateInfo::NonTemplate && +
!isa(getCurrentClass().TagOrTemplate) && +
!isa(getCurrentClass().TagOrTemplate) && +
!isa( +
getCurrentClass().TagOrTemplate) && (!FnD || Actions.canSkipFunctionBody(FnD))
&& trySkippingFunctionBody()) { Actions.ActOnSkippedFunctionBody(FnD);
I'm not sure whether this covers all cases, but here are the timing in relation
to others:
Reparse for skipping all function bodies: 0.2s
Reparse for skipping all function bodies except template functions (previous
patch set, not handling function bodies within template classes): 0.27s
Reparse for skipping all function bodies with the diff above (fixing another
case + ignoring function bodies in templates): 0.44s
Reparse without skipping any function bodies in the preamble: 0.51s
As I said, I'm not sure whether this diff covered all cases, but it can get
only more worse than 0.44s :) That's enough for me to stop here.
Repository:
rC Clang
https://reviews.llvm.org/D45815
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45815: [libclang] Allow skipping function bodies in preamble only
nik added a comment.
Trying to format the diff in the previous comment:
--- a/lib/Parse/ParseCXXInlineMethods.cpp
+++ b/lib/Parse/ParseCXXInlineMethods.cpp
@@ -102,9 +102,14 @@ NamedDecl
*Parser::ParseCXXInlineMethodDef(AccessSpecifier AS,
}
if (SkipFunctionBodies != SkipFunctionBodiesKind::None &&
+ TemplateInfo.Kind == ParsedTemplateInfo::NonTemplate &&
+ !isa(getCurrentClass().TagOrTemplate) &&
+ !isa(getCurrentClass().TagOrTemplate)
&&
+ !isa(
+ getCurrentClass().TagOrTemplate) &&
(!FnD || Actions.canSkipFunctionBody(FnD)) &&
trySkippingFunctionBody()) {
Actions.ActOnSkippedFunctionBody(FnD);
Repository:
rC Clang
https://reviews.llvm.org/D45815
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46049: [OpenCL] Add constant address space to __func__ in AST
Anastasia added inline comments. Comment at: lib/AST/Expr.cpp:870 + if (C.getLangOpts().OpenCL && Ty.getAddressSpace() == LangAS::Default) +Ty = C.getAddrSpaceQualType(Ty, LangAS::opencl_constant); + Anastasia wrote: > bader wrote: > > Anastasia wrote: > > > bader wrote: > > > > As `Ty` is passed by value, shouldn't we accept only data located in > > > > constant address space? > > > Do you mean to assert? Currently it should be passed with `constant` AS > > > but I thought the idea is to modify this function so we can accept > > > `Default` AS too but then replace by `constant`. > > > > > Yes, but according to your other comment this idea doesn't work. > > > > > I have added the address space to the creation of StringLiteral, but > > > unfortunately it doesn't seems like we can remove similar code in other > > > places because **QualType created for StringLiteral is also used > > > elsewhere and has to match (contain right address space).** I.e. here is > > > it used further down to create PredefinedExpr. > > > > > > > > > Indeed for the current cases it doesn't... there might be some uncaught ones > though where it can be useful. But we can add it later as well. So I am > thinking to undo this? I quite liked the idea initially, but unfortunately it seems like it's not applicable to the current use cases. https://reviews.llvm.org/D46049 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46190: For an ODR declaration, set the 'Used' bit on its associated declarations.
CarlosAlbertoEnciso created this revision.
CarlosAlbertoEnciso added reviewers: rsmith, erichkeane, probinson, dblaikie.
CarlosAlbertoEnciso added a project: clang.
Herald added a subscriber: aprantl.
The following submission under review
https://reviews.llvm.org/D44826
introduced an option to warn on unused 'using declarations'.
But it can't detect cases like
namespace nsp {
void foo();
int var;
}using var
using foo; <-- warning
using var; <-- MISSING warning
void bar() {
using foo; <-- warning
using var; <-- warning
}
void bar() {
nsp::var = 1;
}
One of the reviewers (dblaikie) mentioned the possible cause for that
limitation.
You mention a missed case in the description - where the target of a using
decl is
used and that causes the unused-using not to warn about the using decl? That
seems like a big limitation. Does that mean the 'used' bit is being tracked
in the
wrong place, on the target of the using decl instead of on the using decl
itself?
When a declaration is found to be ODR, only its associated 'Used' bit is set.
This patch, traverse its associated declarations and sets the 'Used' bit. The
goal
is to create additional information to help in the reduction of the debug
information
size, which is affected by extra generation of 'non-used' declarations.
Note:
To solve the debug information issue (size), there are 3 pieces of
work:
- Set the 'Used' bit recursively. This patch.
- Review the - Wunused-usings and -Wunused-local-typedefs implementation to
take advantage of new 'Used' settings.
- Do not generate debug information for the unused using
Thanks for your view on this issue and on the general approach.
Repository:
rC Clang
https://reviews.llvm.org/D46190
Files:
include/clang/Sema/Sema.h
include/clang/Sema/SemaInternal.h
lib/AST/ASTDumper.cpp
lib/AST/DeclBase.cpp
lib/Sema/Sema.cpp
lib/Sema/SemaDeclCXX.cpp
lib/Sema/SemaExpr.cpp
lib/Sema/SemaLambda.cpp
lib/Sema/SemaOpenMP.cpp
lib/Sema/SemaStmt.cpp
lib/Sema/SemaStmtAsm.cpp
test/Frontend/float16.cpp
test/Misc/ast-dump-attr.cpp
test/Misc/ast-dump-color.cpp
test/PCH/cxx-templates.cpp
test/SemaCXX/used_class_struct_union.cpp
test/SemaCXX/used_enum.cpp
test/SemaCXX/used_goto.cpp
test/SemaCXX/used_pointer.cpp
test/SemaCXX/used_template.cpp
test/SemaCXX/used_typedef.cpp
test/SemaCXX/used_using.cpp
Index: test/SemaCXX/used_using.cpp
===
--- test/SemaCXX/used_using.cpp
+++ test/SemaCXX/used_using.cpp
@@ -0,0 +1,162 @@
+// RUN: %clang_cc1 -std=c++11 -ast-dump %s | FileCheck %s --strict-whitespace
+
+namespace A {
+ typedef char CHAR;
+}
+
+//CHECK: |-NamespaceDecl {{.*}} used A
+//CHECK-NEXT: | `-TypedefDecl {{.*}} used CHAR 'char'
+//CHECK-NEXT: | `-BuiltinType {{.*}}
+
+namespace B {
+ typedef char CHAR;
+}
+
+//CHECK: |-NamespaceDecl {{.*}} used B
+//CHECK-NEXT: | `-TypedefDecl {{.*}} used CHAR 'char'
+//CHECK-NEXT: | `-BuiltinType {{.*}}
+
+namespace C {
+ typedef char CHAR;
+ typedef int INTEGER;
+ typedef unsigned UNSIGNED;
+
+ void FA();
+ void FB();
+
+ int Var1;
+ int Var2;
+}
+
+//CHECK: |-NamespaceDecl {{.*}} used C
+//CHECK-NEXT: | |-TypedefDecl {{.*}} CHAR 'char'
+//CHECK-NEXT: | | `-BuiltinType {{.*}}
+//CHECK-NEXT: | |-TypedefDecl {{.*}} INTEGER 'int'
+//CHECK-NEXT: | | `-BuiltinType {{.*}}
+//CHECK-NEXT: | |-TypedefDecl {{.*}} UNSIGNED 'unsigned int'
+//CHECK-NEXT: | | `-BuiltinType {{.*}}
+//CHECK-NEXT: | |-FunctionDecl {{.*}} used FA 'void ()'
+//CHECK-NEXT: | |-FunctionDecl {{.*}} used FB 'void ()'
+//CHECK-NEXT: | |-VarDecl {{.*}} used Var1 'int'
+//CHECK-NEXT: | `-VarDecl {{.*}} used Var2 'int'
+
+using A::CHAR;
+using B::CHAR;
+
+//CHECK: |-UsingDecl {{.*}} A::CHAR
+//CHECK-NEXT: |-UsingShadowDecl {{.*}} 'CHAR'
+//CHECK-NEXT: | `-TypedefType {{.*}} 'A::CHAR' sugar
+//CHECK-NEXT: | |-Typedef {{.*}}
+//CHECK-NEXT: | `-BuiltinType {{.*}}
+//CHECK-NEXT: |-UsingDecl {{.*}} B::CHAR
+//CHECK-NEXT: |-UsingShadowDecl {{.*}} 'CHAR'
+//CHECK-NEXT: | `-TypedefType {{.*}} 'B::CHAR' sugar
+//CHECK-NEXT: | |-Typedef {{.*}} 'CHAR'
+//CHECK-NEXT: | `-BuiltinType {{.*}}
+
+void F1() {
+ using A::CHAR;
+ CHAR ca;
+ ca = '1';
+
+ using B::CHAR;
+ B::CHAR cb;
+ cb = '1';
+}
+
+//CHECK: |-FunctionDecl {{.*}} used F1 'void ()'
+//CHECK-NEXT: | `-CompoundStmt {{.*}}
+//CHECK-NEXT: | |-DeclStmt {{.*}}
+//CHECK-NEXT: | | `-UsingDecl {{.*}} used A::CHAR
+//CHECK-NEXT: | |-DeclStmt {{.*}}
+//CHECK-NEXT: | | `-VarDecl {{.*}} used ca 'A::CHAR':'char'
+//CHECK-NEXT: | |-BinaryOperator {{.*}}
+//CHECK-NEXT: | | |-DeclRefExpr {{.*}} 'ca' 'A::CHAR':'char'
+//CHECK-NEXT: | | `-CharacterLiteral {{.*}}
+//CHECK-NEXT: | |-DeclStmt {{.*}}
+//CHECK-NEXT: | | `-UsingDecl {{.*}} B::CHAR
+//CHECK-NEXT: | |-DeclStmt {{.*}}
+//CHECK-NEXT: | | `-VarDecl {{.*}} used cb 'B::CHAR':'char'
+//CHECK-NEXT: | `-B
[PATCH] D46190: For an ODR declaration, set the 'Used' bit on its associated declarations.
CarlosAlbertoEnciso added a comment. Note: The patch includes the entry points to set Objective-C and OpenMP declarations, but I do not know anything about Objective-C or OpenMP. The functions just return without updating any extra 'Used' bit. Any suggestion on how to deal with those functions, is very much appreciated. Thanks. Repository: rC Clang https://reviews.llvm.org/D46190 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44684: [mips] Improve handling of -fno-[pic/PIC] option
abeserminji added inline comments.
Comment at: include/clang/Basic/DiagnosticDriverKinds.td:340
+ "ignoring '%0' option as it cannot be used with "
+ "explicit use of -mabicalls and the N64 ABI">,
InGroup;
sdardis wrote:
> Use the %select{optionA|optionB|..|optionZ}$NUM operator here like:
>
> "cannot be used with the %select{explicit|implicit}1 usage of -mabicalls and
> the N64 ABI"
>
> Which eliminates the need for two separate warnings.
Thanks for the hint. I simplified it now.
https://reviews.llvm.org/D44684
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44684: [mips] Improve handling of -fno-[pic/PIC] option
abeserminji updated this revision to Diff 144344.
abeserminji marked 2 inline comments as done.
abeserminji added a comment.
Comments resolved.
https://reviews.llvm.org/D44684
Files:
include/clang/Basic/DiagnosticDriverKinds.td
lib/Driver/ToolChains/Arch/Mips.cpp
lib/Driver/ToolChains/CommonArgs.cpp
test/Driver/mips-abicalls-error.c
test/Driver/mips-abicalls-warning.c
test/Driver/mips-as.c
test/Driver/mips-features.c
Index: test/Driver/mips-features.c
===
--- test/Driver/mips-features.c
+++ test/Driver/mips-features.c
@@ -11,7 +11,7 @@
// CHECK-MNOABICALLS: "-target-feature" "+noabicalls"
//
// -mno-abicalls non-PIC N64
-// RUN: %clang -target mips64-linux-gnu -### -c -fno-PIC %s 2>&1 \
+// RUN: %clang -target mips64-linux-gnu -### -c -fno-PIC -mno-abicalls %s 2>&1 \
// RUN: | FileCheck --check-prefix=CHECK-MNOABICALLS-N64NPIC %s
// CHECK-MNOABICALLS-N64NPIC: "-target-feature" "+noabicalls"
//
@@ -86,13 +86,13 @@
// CHECK-MEMBEDDEDDATADEF-NOT: "-mllvm" "-membedded-data"
//
// MIPS64 + N64: -fno-pic -> -mno-abicalls -mgpopt
-// RUN: %clang -target mips64-mti-elf -mabi=64 -### -c %s -fno-pic 2>&1 \
+// RUN: %clang -target mips64-mti-elf -mabi=64 -### -c %s -fno-pic -mno-abicalls 2>&1 \
// RUN: | FileCheck --check-prefix=CHECK-N64-GPOPT %s
// CHECK-N64-GPOPT: "-target-feature" "+noabicalls"
// CHECK-N64-GPOPT: "-mllvm" "-mgpopt"
//
// MIPS64 + N64: -fno-pic -mno-gpopt
-// RUN: %clang -target mips64-mti-elf -mabi=64 -### -c %s -fno-pic -mno-gpopt 2>&1 \
+// RUN: %clang -target mips64-mti-elf -mabi=64 -### -c %s -fno-pic -mno-abicalls -mno-gpopt 2>&1 \
// RUN: | FileCheck --check-prefix=CHECK-N64-MNO-GPOPT %s
// CHECK-N64-MNO-GPOPT: "-target-feature" "+noabicalls"
// CHECK-N64-MNO-GPOPT-NOT: "-mllvm" "-mgpopt"
Index: test/Driver/mips-as.c
===
--- test/Driver/mips-as.c
+++ test/Driver/mips-as.c
@@ -21,7 +21,7 @@
// MIPS32R2-DEF-EL-AS: as{{(.exe)?}}" "-march" "mips32r2" "-mabi" "32" "-mno-shared" "-call_nonpic" "-EL"
//
// RUN: %clang -target mips64-linux-gnu -### \
-// RUN: -no-integrated-as -fno-pic -c %s 2>&1 \
+// RUN: -no-integrated-as -fno-pic -mno-abicalls -c %s 2>&1 \
// RUN: | FileCheck -check-prefix=MIPS64R2-EB-AS %s
// MIPS64R2-EB-AS: as{{(.exe)?}}" "-march" "mips64r2" "-mabi" "64" "-mno-shared" "-EB"
//
@@ -31,7 +31,7 @@
// MIPS64R2-EB-AS-PIC: as{{(.exe)?}}" "-march" "mips64r2" "-mabi" "64" "-EB" "-KPIC"
//
// RUN: %clang -target mips64el-linux-gnu -### \
-// RUN: -no-integrated-as -c -fno-pic %s 2>&1 \
+// RUN: -no-integrated-as -c -fno-pic -mno-abicalls %s 2>&1 \
// RUN: | FileCheck -check-prefix=MIPS64R2-DEF-EL-AS %s
// MIPS64R2-DEF-EL-AS: as{{(.exe)?}}" "-march" "mips64r2" "-mabi" "64" "-mno-shared" "-EL"
//
@@ -64,7 +64,7 @@
// MIPS64R2-EL-AS-PIC: as{{(.exe)?}}" "-march" "mips64r2" "-mabi" "64" "-EL" "-KPIC"
//
// RUN: %clang -target mips64el-linux-gnu -mabi=64 -### \
-// RUN: -no-integrated-as -c %s -fno-pic 2>&1 \
+// RUN: -no-integrated-as -c %s -fno-pic -mno-abicalls 2>&1 \
// RUN: | FileCheck -check-prefix=MIPS64R2-EL-AS %s
// MIPS64R2-EL-AS: as{{(.exe)?}}" "-march" "mips64r2" "-mabi" "64" "-mno-shared" "-EL"
//
@@ -84,7 +84,7 @@
// MIPS-OCTEON-PIC: as{{(.exe)?}}" "-march" "octeon" "-mabi" "64" "-EB" "-KPIC"
//
// RUN: %clang -target mips64-linux-gnu -march=octeon -### \
-// RUN: -no-integrated-as -c %s -fno-pic 2>&1 \
+// RUN: -no-integrated-as -c %s -fno-pic -mno-abicalls 2>&1 \
// RUN: | FileCheck -check-prefix=MIPS-OCTEON %s
// MIPS-OCTEON: as{{(.exe)?}}" "-march" "octeon" "-mabi" "64" "-mno-shared" "-EB"
//
@@ -144,7 +144,7 @@
// MIPS-ALIAS-64-PIC: as{{(.exe)?}}" "-march" "mips64" "-mabi" "64" "-EB" "-KPIC"
//
// RUN: %clang -target mips64-linux-gnu -mips64 -### \
-// RUN: -no-integrated-as -c -fno-pic %s 2>&1 \
+// RUN: -no-integrated-as -c -fno-pic -mno-abicalls %s 2>&1 \
// RUN: | FileCheck -check-prefix=MIPS-ALIAS-64 %s
// MIPS-ALIAS-64: as{{(.exe)?}}" "-march" "mips64" "-mabi" "64" "-mno-shared" "-EB"
//
@@ -159,7 +159,7 @@
// MIPS-ALIAS-64R3-PIC: as{{(.exe)?}}" "-march" "mips64r3" "-mabi" "64" "-EB" "-KPIC"
//
// RUN: %clang -target mips64-linux-gnu -mips64r3 -### \
-// RUN: -no-integrated-as -c %s -fno-pic 2>&1 \
+// RUN: -no-integrated-as -c %s -fno-pic -mno-abicalls 2>&1 \
// RUN: | FileCheck -check-prefix=MIPS-ALIAS-64R3 %s
// MIPS-ALIAS-64R3: as{{(.exe)?}}" "-march" "mips64r3" "-mabi" "64" "-mno-shared" "-EB"
//
@@ -169,7 +169,7 @@
// MIPS-ALIAS-64R5-PIC: as{{(.exe)?}}" "-march" "mips64r5" "-mabi" "64" "-EB" "-KPIC"
//
// RUN: %clang -target mips64-linux-gnu -mips64r5 -### \
-// RUN: -no-integrated-as -c %s -fno-pic 2>&1 \
+// RUN: -no-integrated-as -c %s -fno-pic -mno-abicalls 2>&1 \
// RUN: | FileCheck -check-prefix=MIPS-ALIAS-64R5 %s
// MIPS-ALIAS-64R5: as{{(.exe)?}}" "-march" "mips64r5" "-mabi" "64" "-mno-shared" "-EB"
//
[PATCH] D46159: [clang-tidy] Add a flag to enable alpha checkers
lebedev.ri accepted this revision. lebedev.ri added a comment. This revision is now accepted and ready to land. So i suspect the history here is: https://reviews.llvm.org/D26310 added a way to filter-out these clang analyzer checks, which removed these checks from being available in clang-tidy, but did not add an option (like in this differential) to get them back, so it was a clear regression. I'm going to accept this because of the reasoning in my previous comments here, but please wait for @alexfh or someone else to LGTM this too. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D46159 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46135: [Driver, CodeGen] add options to enable/disable an FP cast optimization
spatel added a comment. Should I add a bullet for this new flag/attribute to the clang release notes, the LLVM release, both? Or do we view this as temporary and not making it to the release? Repository: rC Clang https://reviews.llvm.org/D46135 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r331048 - [MC] Modify MCAsmStreamer to always build MCAssembler. NFCI.
Author: niravd
Date: Fri Apr 27 08:45:54 2018
New Revision: 331048
URL: http://llvm.org/viewvc/llvm-project?rev=331048&view=rev
Log:
[MC] Modify MCAsmStreamer to always build MCAssembler. NFCI.
Modified:
cfe/trunk/tools/driver/cc1as_main.cpp
Modified: cfe/trunk/tools/driver/cc1as_main.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/driver/cc1as_main.cpp?rev=331048&r1=331047&r2=331048&view=diff
==
--- cfe/trunk/tools/driver/cc1as_main.cpp (original)
+++ cfe/trunk/tools/driver/cc1as_main.cpp Fri Apr 27 08:45:54 2018
@@ -398,17 +398,19 @@ static bool ExecuteAssembler(AssemblerIn
if (Opts.OutputType == AssemblerInvocation::FT_Asm) {
MCInstPrinter *IP = TheTarget->createMCInstPrinter(
llvm::Triple(Opts.Triple), Opts.OutputAsmVariant, *MAI, *MCII, *MRI);
-MCCodeEmitter *CE = nullptr;
-MCAsmBackend *MAB = nullptr;
-if (Opts.ShowEncoding) {
- CE = TheTarget->createMCCodeEmitter(*MCII, *MRI, Ctx);
- MCTargetOptions Options;
- MAB = TheTarget->createMCAsmBackend(*STI, *MRI, Options);
-}
+
+std::unique_ptr CE;
+if (Opts.ShowEncoding)
+ CE.reset(TheTarget->createMCCodeEmitter(*MCII, *MRI, Ctx));
+MCTargetOptions MCOptions;
+std::unique_ptr MAB(
+TheTarget->createMCAsmBackend(*STI, *MRI, MCOptions));
+
auto FOut = llvm::make_unique(*Out);
Str.reset(TheTarget->createAsmStreamer(
Ctx, std::move(FOut), /*asmverbose*/ true,
-/*useDwarfDirectory*/ true, IP, CE, MAB, Opts.ShowInst));
+/*useDwarfDirectory*/ true, IP, std::move(CE), std::move(MAB),
+Opts.ShowInst));
} else if (Opts.OutputType == AssemblerInvocation::FT_Null) {
Str.reset(createNullStreamer(Ctx));
} else {
@@ -419,13 +421,16 @@ static bool ExecuteAssembler(AssemblerIn
Out = BOS.get();
}
-MCCodeEmitter *CE = TheTarget->createMCCodeEmitter(*MCII, *MRI, Ctx);
-MCTargetOptions Options;
-MCAsmBackend *MAB = TheTarget->createMCAsmBackend(*STI, *MRI, Options);
+std::unique_ptr CE(
+TheTarget->createMCCodeEmitter(*MCII, *MRI, Ctx));
+MCTargetOptions MCOptions;
+std::unique_ptr MAB(
+TheTarget->createMCAsmBackend(*STI, *MRI, MCOptions));
+
Triple T(Opts.Triple);
Str.reset(TheTarget->createMCObjectStreamer(
-T, Ctx, std::unique_ptr(MAB), *Out,
std::unique_ptr(CE), *STI,
-Opts.RelaxAll, Opts.IncrementalLinkerCompatible,
+T, Ctx, std::move(MAB), *Out, std::move(CE), *STI, Opts.RelaxAll,
+Opts.IncrementalLinkerCompatible,
/*DWARFMustBeAtTheEnd*/ true));
Str.get()->InitSections(Opts.NoExecStack);
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46135: [Driver, CodeGen] add options to enable/disable an FP cast optimization
lebedev.ri added a comment. In https://reviews.llvm.org/D46135#1081165, @spatel wrote: > Should I add a bullet for this new flag/attribute to the clang release notes, > the LLVM release, both? Or do we view this as temporary and not making it to > the release? (Not everyone is always using trunk snapshots) I'd guess it would be present for at least one release, so **I** would expect to see that in both the clang and llvm release notes. Repository: rC Clang https://reviews.llvm.org/D46135 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46143: [clang-format/ObjC] Use getIdentifierInfo() instead of tok::identifier
benhamilton updated this revision to Diff 144357.
benhamilton added a comment.
Add helper canBeObjCSelectorComponent() with comments.
Repository:
rC Clang
https://reviews.llvm.org/D46143
Files:
lib/Format/TokenAnnotator.cpp
unittests/Format/FormatTestObjC.cpp
Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -945,15 +945,26 @@
" }]) {\n}");
}
-TEST_F(FormatTestObjC, ObjCNew) {
+TEST_F(FormatTestObjC, ObjCCxxKeywords) {
verifyFormat("+ (instancetype)new {\n"
" return nil;\n"
"}\n");
verifyFormat("+ (instancetype)myNew {\n"
" return [self new];\n"
"}\n");
verifyFormat("SEL NewSelector(void) { return @selector(new); }\n");
verifyFormat("SEL MacroSelector(void) { return MACRO(new); }\n");
+ verifyFormat("+ (instancetype)delete {\n"
+ " return nil;\n"
+ "}\n");
+ verifyFormat("+ (instancetype)myDelete {\n"
+ " return [self delete];\n"
+ "}\n");
+ verifyFormat("SEL DeleteSelector(void) { return @selector(delete); }\n");
+ verifyFormat("SEL MacroSelector(void) { return MACRO(delete); }\n");
+ verifyFormat("MACRO(new:)\n");
+ verifyFormat("MACRO(delete:)\n");
+ verifyFormat("foo = @{MACRO(new:) : MACRO(delete:)}\n");
}
TEST_F(FormatTestObjC, ObjCLiterals) {
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -25,6 +25,21 @@
namespace {
+/// \brief Returns \c true if the token can be used as an identifier in
+/// an Objective-C \c @selector, \c false otherwise.
+///
+/// Because getFormattingLangOpts() always lexes source code as
+/// Objective-C++, C++ keywords like \c new and \c delete are
+/// lexed as tok::kw_*, not tok::identifier, even for Objective-C.
+///
+/// For Objective-C and Objective-C++, both identifiers and keywords
+/// are valid inside @selector(...) (or a macro which
+/// invokes @selector(...)). So, we allow treat any identifier or
+/// keyword as a potential Objective-C selector component.
+static bool canBeObjCSelectorComponent(const FormatToken &Tok) {
+ return Tok.Tok.getIdentifierInfo() != nullptr;
+}
+
/// \brief A parser that gathers additional information about tokens.
///
/// The \c TokenAnnotator tries to match parenthesis and square brakets and
@@ -703,9 +718,10 @@
Tok->Type = TT_CtorInitializerColon;
else
Tok->Type = TT_InheritanceColon;
- } else if (Tok->Previous->is(tok::identifier) && Tok->Next &&
+ } else if (canBeObjCSelectorComponent(*Tok->Previous) && Tok->Next &&
(Tok->Next->isOneOf(tok::r_paren, tok::comma) ||
- Tok->Next->startsSequence(tok::identifier, tok::colon))) {
+ (canBeObjCSelectorComponent(*Tok->Next) && Tok->Next->Next &&
+ Tok->Next->Next->is(tok::colon {
// This handles a special macro in ObjC code where selectors including
// the colon are passed as macro arguments.
Tok->Type = TT_ObjCMethodExpr;
@@ -1346,7 +1362,7 @@
TT_LeadingJavaAnnotation)) {
Current.Type = Current.Previous->Type;
}
-} else if (Current.isOneOf(tok::identifier, tok::kw_new) &&
+} else if (canBeObjCSelectorComponent(Current) &&
// FIXME(bug 36976): ObjC return types shouldn't use
TT_CastRParen.
Current.Previous && Current.Previous->is(TT_CastRParen) &&
Current.Previous->MatchingParen &&
@@ -2650,7 +2666,7 @@
if (Line.Type == LT_ObjCMethodDecl) {
if (Left.is(TT_ObjCMethodSpecifier))
return true;
-if (Left.is(tok::r_paren) && Right.isOneOf(tok::identifier, tok::kw_new))
+if (Left.is(tok::r_paren) && canBeObjCSelectorComponent(Right))
// Don't space between ')' and or ')' and 'new'. 'new' is not a
// keyword in Objective-C, and '+ (instancetype)new;' is a standard class
// method declaration.
@@ -3128,6 +3144,7 @@
for (unsigned i = 0, e = Tok->FakeLParens.size(); i != e; ++i)
llvm::errs() << Tok->FakeLParens[i] << "/";
llvm::errs() << " FakeRParens=" << Tok->FakeRParens;
+llvm::errs() << " II=" << Tok->Tok.getIdentifierInfo();
llvm::errs() << " Text='" << Tok->TokenText << "'\n";
if (!Tok->Next)
assert(Tok == Line.Last);
Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -945,15 +945,26 @@
" }]) {\n}");
}
-TEST_F(FormatTestObjC, ObjCNew) {
+TEST_F(FormatTestObjC, ObjCCxxKeywords) {
verifyFormat("+ (instancetype)new {\n"
" retu
[PATCH] D46049: [OpenCL] Add constant address space to __func__ in AST
yaxunl added a comment. Can you add a codegen test for 1.2, 2.0? Also, I am wondering whether this is valid for OpenCL C++. Do we need special handling for string literal? Comment at: lib/AST/Expr.cpp:870 + if (C.getLangOpts().OpenCL && Ty.getAddressSpace() == LangAS::Default) +Ty = C.getAddrSpaceQualType(Ty, LangAS::opencl_constant); + Anastasia wrote: > Anastasia wrote: > > bader wrote: > > > Anastasia wrote: > > > > bader wrote: > > > > > As `Ty` is passed by value, shouldn't we accept only data located in > > > > > constant address space? > > > > Do you mean to assert? Currently it should be passed with `constant` AS > > > > but I thought the idea is to modify this function so we can accept > > > > `Default` AS too but then replace by `constant`. > > > > > > > Yes, but according to your other comment this idea doesn't work. > > > > > > > I have added the address space to the creation of StringLiteral, but > > > > unfortunately it doesn't seems like we can remove similar code in other > > > > places because **QualType created for StringLiteral is also used > > > > elsewhere and has to match (contain right address space).** I.e. here > > > > is it used further down to create PredefinedExpr. > > > > > > > > > > > > > > Indeed for the current cases it doesn't... there might be some uncaught > > ones though where it can be useful. But we can add it later as well. So I > > am thinking to undo this? > I quite liked the idea initially, but unfortunately it seems like it's not > applicable to the current use cases. Can you extract this as ``` QualType ASTContext::getStringLiteralType(QualType); ``` then use it here and below? Comment at: test/SemaOpenCL/predefined-expr.cl:1 +// RUN: %clang_cc1 %s -verify + can you also check for cl 2.0? https://reviews.llvm.org/D46049 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45814: Fix an assertion when -print-prog-name=
compnerd accepted this revision. compnerd added a comment. This revision is now accepted and ready to land. Do you have commit access or do you need someone to commit this on your behalf? Repository: rC Clang https://reviews.llvm.org/D45814 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46115: [ASTImporter] properly import SrcLoc of Attr
a.sidorin added a comment. Hi all. I'm already here, invited by the Herald - just had no time to take a look (will change the script to add me as a reviewer). But thank you anyway! :) In the initial implementation (https://raw.githubusercontent.com/haoNoQ/clang/summary-ipa-draft/lib/AST/ASTImporter.cpp), we already had this feature, and it was even called similarly - `ImportAttributes()`. But the centralized import inside `Imported` (or even `Import()?), as it is done in this patch, looks to be a better option to me. I'm not sure that we have to "merge" attributes. Moreover, declarations with different attributes can be considered structurally different (possibly, in an another patch). But I can possibly be missing something so feel free to correct me. Repository: rC Clang https://reviews.llvm.org/D46115 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r331052 - [MC] Provide default value for IsResolved.
Author: niravd Date: Fri Apr 27 09:11:24 2018 New Revision: 331052 URL: http://llvm.org/viewvc/llvm-project?rev=331052&view=rev Log: [MC] Provide default value for IsResolved. Modified: cfe/trunk/tools/driver/cc1as_main.cpp Modified: cfe/trunk/tools/driver/cc1as_main.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/driver/cc1as_main.cpp?rev=331052&r1=331051&r2=331052&view=diff == --- cfe/trunk/tools/driver/cc1as_main.cpp (original) +++ cfe/trunk/tools/driver/cc1as_main.cpp Fri Apr 27 09:11:24 2018 @@ -435,6 +435,9 @@ static bool ExecuteAssembler(AssemblerIn Str.get()->InitSections(Opts.NoExecStack); } + // Use Assembler information for parsing. + Str->setUseAssemblerInfoForParsing(true); + bool Failed = false; std::unique_ptr Parser( ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r331053 - Test commit removing trailing whitespace
Author: stuart.brady Date: Fri Apr 27 09:11:56 2018 New Revision: 331053 URL: http://llvm.org/viewvc/llvm-project?rev=331053&view=rev Log: Test commit removing trailing whitespace Modified: cfe/trunk/lib/Sema/SemaDecl.cpp Modified: cfe/trunk/lib/Sema/SemaDecl.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=331053&r1=331052&r2=331053&view=diff == --- cfe/trunk/lib/Sema/SemaDecl.cpp (original) +++ cfe/trunk/lib/Sema/SemaDecl.cpp Fri Apr 27 09:11:56 2018 @@ -6979,7 +6979,7 @@ NamedDecl *Sema::getShadowedDeclaration( // Don't warn if typedef declaration is part of a class if (D->getDeclContext()->isRecord()) return nullptr; - + if (!shouldWarnIfShadowedDecl(Diags, R)) return nullptr; ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r331055 - [MC] Undo spurious commit added into r331052.
Author: niravd Date: Fri Apr 27 09:16:06 2018 New Revision: 331055 URL: http://llvm.org/viewvc/llvm-project?rev=331055&view=rev Log: [MC] Undo spurious commit added into r331052. Modified: cfe/trunk/tools/driver/cc1as_main.cpp Modified: cfe/trunk/tools/driver/cc1as_main.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/driver/cc1as_main.cpp?rev=331055&r1=331054&r2=331055&view=diff == --- cfe/trunk/tools/driver/cc1as_main.cpp (original) +++ cfe/trunk/tools/driver/cc1as_main.cpp Fri Apr 27 09:16:06 2018 @@ -435,9 +435,6 @@ static bool ExecuteAssembler(AssemblerIn Str.get()->InitSections(Opts.NoExecStack); } - // Use Assembler information for parsing. - Str->setUseAssemblerInfoForParsing(true); - bool Failed = false; std::unique_ptr Parser( ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r331056 - [docs] add -ffp-cast-overflow-workaround to the release notes
Author: spatel Date: Fri Apr 27 09:21:22 2018 New Revision: 331056 URL: http://llvm.org/viewvc/llvm-project?rev=331056&view=rev Log: [docs] add -ffp-cast-overflow-workaround to the release notes This option was added with: D46135 rL331041 ...copying the text from UsersManual.rst for more exposure. Modified: cfe/trunk/docs/ReleaseNotes.rst Modified: cfe/trunk/docs/ReleaseNotes.rst URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ReleaseNotes.rst?rev=331056&r1=331055&r2=331056&view=diff == --- cfe/trunk/docs/ReleaseNotes.rst (original) +++ cfe/trunk/docs/ReleaseNotes.rst Fri Apr 27 09:21:22 2018 @@ -83,6 +83,15 @@ Non-comprehensive list of changes in thi New Compiler Flags -- +- :option:`-ffp-cast-overflow-workaround` and + :option:`-fnofp-cast-overflow-workaround` + enable (disable) a workaround for code that casts floating-point values to + integers and back to floating-point. If the floating-point value is not + representable in the intermediate integer type, the code is incorrect + according to the language standard. This flag will attempt to generate code + as if the result of an overflowing conversion matches the overflowing behavior + of a target's native float-to-int conversion instructions. + - ... Deprecated Compiler Flags ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r331057 - [docs] more dashes
Author: spatel Date: Fri Apr 27 09:24:39 2018 New Revision: 331057 URL: http://llvm.org/viewvc/llvm-project?rev=331057&view=rev Log: [docs] more dashes Modified: cfe/trunk/docs/ReleaseNotes.rst Modified: cfe/trunk/docs/ReleaseNotes.rst URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ReleaseNotes.rst?rev=331057&r1=331056&r2=331057&view=diff == --- cfe/trunk/docs/ReleaseNotes.rst (original) +++ cfe/trunk/docs/ReleaseNotes.rst Fri Apr 27 09:24:39 2018 @@ -84,7 +84,7 @@ New Compiler Flags -- - :option:`-ffp-cast-overflow-workaround` and - :option:`-fnofp-cast-overflow-workaround` + :option:`-fno-fp-cast-overflow-workaround` enable (disable) a workaround for code that casts floating-point values to integers and back to floating-point. If the floating-point value is not representable in the intermediate integer type, the code is incorrect ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46135: [Driver, CodeGen] add options to enable/disable an FP cast optimization
spatel added a comment. In https://reviews.llvm.org/D46135#1081193, @lebedev.ri wrote: > (Not everyone is always using trunk snapshots) > I'd guess it would be present for at least one release, so **I** would > expect to see that in both the clang and llvm release notes. Thanks - sounds right to me: https://reviews.llvm.org/rL331056 https://reviews.llvm.org/rL331059 Repository: rC Clang https://reviews.llvm.org/D46135 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40988: Clang-format: add finer-grained options for putting all arguments on one line
russellmcc marked an inline comment as not done. russellmcc added a comment. Okay; I think I've responded to all feedback at this point. Thanks for your patience guiding me through my first contribution to this project. Let me know what else I can do to help get this merged! https://reviews.llvm.org/D40988 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46143: [clang-format/ObjC] Use getIdentifierInfo() instead of tok::identifier
djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. Looks good, thank you. Repository: rC Clang https://reviews.llvm.org/D46143 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46024: [clang-format] Add SpaceBeforeCpp11BracedList option.
rkirsling added a comment. Guidelines page has been updated (https://trac.webkit.org/changeset/231085), though it may take a bit for the website to update. Repository: rC Clang https://reviews.llvm.org/D46024 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r331052 - [MC] Provide default value for IsResolved.
Nit: The comment just repeats what the line below it says. Does this have an observable effect? If so, should this have a test? On Fri, Apr 27, 2018 at 12:11 PM, Nirav Dave via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: niravd > Date: Fri Apr 27 09:11:24 2018 > New Revision: 331052 > > URL: http://llvm.org/viewvc/llvm-project?rev=331052&view=rev > Log: > [MC] Provide default value for IsResolved. > > Modified: > cfe/trunk/tools/driver/cc1as_main.cpp > > Modified: cfe/trunk/tools/driver/cc1as_main.cpp > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/ > driver/cc1as_main.cpp?rev=331052&r1=331051&r2=331052&view=diff > > == > --- cfe/trunk/tools/driver/cc1as_main.cpp (original) > +++ cfe/trunk/tools/driver/cc1as_main.cpp Fri Apr 27 09:11:24 2018 > @@ -435,6 +435,9 @@ static bool ExecuteAssembler(AssemblerIn > Str.get()->InitSections(Opts.NoExecStack); >} > > + // Use Assembler information for parsing. > + Str->setUseAssemblerInfoForParsing(true); > + >bool Failed = false; > >std::unique_ptr Parser( > > > ___ > cfe-commits mailing list > cfe-commits@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits > ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46159: [clang-tidy] Add a flag to enable alpha checkers
dcoughlin added a comment. I'm curious about the use case for this, since I don't think it ever makes sense to run all the alpha checks at once. These checks are typically a work in progress (they're not finished yet) and often have false positives that haven't been fixed yet. Often they don't handle all facets of the language and can crash on unhandled constructs. While it makes sense to enable a specific alpha check (or even a set of related alpha checks) when developing these checks or when evaluating whether they are ready to graduate into non-alpha it seems to me that running all of them isn't useful, ever. There is a real cost to this. People often don't know what 'alpha' means and so when they run all the alpha checks they get an incorrect perspective of the quality of the analyzer. They don't realize that the alpha checks are purely used as a mechanism to support in-tree incremental development. Without a strong rationale for this flag, I would be very opposed it since (as we have seen in the past) it leads to a poor user experience. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D46159 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46159: [clang-tidy] Add a flag to enable alpha checkers
lebedev.ri added a comment. In https://reviews.llvm.org/D46159#1081366, @dcoughlin wrote: > ... Note that it is completely off by default, and is not listed in documentation, `clang-tidy --help`, so one would have to know of the existence of that hidden flag first, and only then when that flag is passed, those alpha checks could be enabled. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D46159 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r331063 - [Modules][ObjC] ASTReader should add protocols for class extensions
Author: bruno
Date: Fri Apr 27 11:01:23 2018
New Revision: 331063
URL: http://llvm.org/viewvc/llvm-project?rev=331063&view=rev
Log:
[Modules][ObjC] ASTReader should add protocols for class extensions
During deserialization clang is currently missing the merging of
protocols into the canonical interface for the class extension.
This merging only currently happens during parsing and should also
be considered during deserialization.
rdar://problem/38724303
Added:
cfe/trunk/test/Modules/Inputs/class-extension/
cfe/trunk/test/Modules/Inputs/class-extension/a-private.h
cfe/trunk/test/Modules/Inputs/class-extension/a-proto.h
cfe/trunk/test/Modules/Inputs/class-extension/a.h
cfe/trunk/test/Modules/Inputs/class-extension/module.modulemap
cfe/trunk/test/Modules/class-extension-protocol.m
Modified:
cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
Modified: cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReaderDecl.cpp?rev=331063&r1=331062&r2=331063&view=diff
==
--- cfe/trunk/lib/Serialization/ASTReaderDecl.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTReaderDecl.cpp Fri Apr 27 11:01:23 2018
@@ -1205,6 +1205,12 @@ void ASTDeclReader::VisitObjCCategoryDec
ProtoLocs.push_back(ReadSourceLocation());
CD->setProtocolList(ProtoRefs.data(), NumProtoRefs, ProtoLocs.data(),
Reader.getContext());
+
+ // Protocols in the class extension belong to the class.
+ if (NumProtoRefs > 0 && CD->ClassInterface && CD->IsClassExtension())
+CD->ClassInterface->mergeClassExtensionProtocolList(
+(ObjCProtocolDecl *const *)ProtoRefs.data(), NumProtoRefs,
+Reader.getContext());
}
void ASTDeclReader::VisitObjCCompatibleAliasDecl(ObjCCompatibleAliasDecl *CAD)
{
Added: cfe/trunk/test/Modules/Inputs/class-extension/a-private.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/class-extension/a-private.h?rev=331063&view=auto
==
--- cfe/trunk/test/Modules/Inputs/class-extension/a-private.h (added)
+++ cfe/trunk/test/Modules/Inputs/class-extension/a-private.h Fri Apr 27
11:01:23 2018
@@ -0,0 +1,5 @@
+#import "a.h"
+#import "a-proto.h"
+
+@interface A ()
+@end
Added: cfe/trunk/test/Modules/Inputs/class-extension/a-proto.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/class-extension/a-proto.h?rev=331063&view=auto
==
--- cfe/trunk/test/Modules/Inputs/class-extension/a-proto.h (added)
+++ cfe/trunk/test/Modules/Inputs/class-extension/a-proto.h Fri Apr 27 11:01:23
2018
@@ -0,0 +1,7 @@
+@protocol NSObject
+@end
+
+@protocol AProto
+@property (nonatomic, readwrite, assign) int p0;
+@property (nonatomic, readwrite, assign) int p1;
+@end
Added: cfe/trunk/test/Modules/Inputs/class-extension/a.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/class-extension/a.h?rev=331063&view=auto
==
--- cfe/trunk/test/Modules/Inputs/class-extension/a.h (added)
+++ cfe/trunk/test/Modules/Inputs/class-extension/a.h Fri Apr 27 11:01:23 2018
@@ -0,0 +1,5 @@
+@interface NSObject
+@end
+
+@interface A : NSObject
+@end
Added: cfe/trunk/test/Modules/Inputs/class-extension/module.modulemap
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/class-extension/module.modulemap?rev=331063&view=auto
==
--- cfe/trunk/test/Modules/Inputs/class-extension/module.modulemap (added)
+++ cfe/trunk/test/Modules/Inputs/class-extension/module.modulemap Fri Apr 27
11:01:23 2018
@@ -0,0 +1,11 @@
+
+module A {
+ header "a.h"
+ header "a-proto.h"
+ export *
+}
+
+module AP {
+ header "a-private.h"
+ export *
+}
Added: cfe/trunk/test/Modules/class-extension-protocol.m
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/class-extension-protocol.m?rev=331063&view=auto
==
--- cfe/trunk/test/Modules/class-extension-protocol.m (added)
+++ cfe/trunk/test/Modules/class-extension-protocol.m Fri Apr 27 11:01:23 2018
@@ -0,0 +1,9 @@
+// RUN: rm -rf %t.cache
+// RUN: %clang_cc1 %s -fsyntax-only -fmodules -fimplicit-module-maps
-fmodules-cache-path=%t.cache -I%S/Inputs/class-extension -verify
+// expected-no-diagnostics
+
+#import "a-private.h"
+
+int foo(A *X) {
+ return X.p0 + X.p1;
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46159: [clang-tidy] Add a flag to enable alpha checkers
dcoughlin added a comment.
In https://reviews.llvm.org/D46159#1081371, @lebedev.ri wrote:
> Note that it is completely off by default, and is not listed in
> documentation, `clang-tidy --help`,
> so one would have to know of the existence of that hidden flag first, and
> only then when that flag is passed, those alpha checks could be enabled.
All it takes is one stack overflow comment or blog post recommending the flag
and then we're back where we started. I'm really worried about well-intentioned
("running more checks means higher-quality code, so let's run ALL the checks")
people enabling this option and having a bad experience.
Can you explain what the benefit of this flag is? How do you envision it being
used?
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D46159
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46159: [clang-tidy] Add a flag to enable alpha checkers
lebedev.ri added a comment.
In https://reviews.llvm.org/D46159#1081392, @dcoughlin wrote:
> In https://reviews.llvm.org/D46159#1081371, @lebedev.ri wrote:
>
> > Note that it is completely off by default, and is not listed in
> > documentation, `clang-tidy --help`,
> > so one would have to know of the existence of that hidden flag first, and
> > only then when that flag is passed, those alpha checks could be enabled.
>
>
> All it takes is one stack overflow comment or blog post recommending the flag
> and then we're back where we started. I'm really worried about
> well-intentioned ("running more checks means higher-quality code, so let's
> run ALL the checks") people enabling this option and having a bad experience.
>
> Can you explain what the benefit of this flag is? How do you envision it
> being used?
What about `scan-build` proper?
Those alpha checks are always present in all builds, one always can pass
`-enable-checker alpha.clone.CloneChecker`.
I would understand if there was a cmake-time switch to build those, but there
isn't one..
Also, it is *much* easier to use `clang-tidy` rather than clang-analyzer,
because you can run clang-tidy on an existing compilation database,
while in `scan-build`'s case, you need whole new build. It's cumbersome.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D46159
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34331: func.wrap.func.con: Unset function before destroying anything
vsapsai added a comment. It is agreed that it is untenable to support reentrancy for `std::function` assignment operators and I'm not trying to achieve that. Here we restrict reentrancy only to `std::function::operator=(nullptr_t)`. @mclow.lists where should the tests go if the standard specifies the functionality as implementation-defined? > Except where explicitly specified in this standard, it is > implementation-defined which functions in the Standard C++ library may be > recursively reentered. [reentrancy] p17.6.5.8 Repository: rCXX libc++ https://reviews.llvm.org/D34331 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45932: [clang-tidy][modernize-raw-string-literal] Don't replace upper ASCII with raw literals
zinovy.nis added a comment. Aaron, any comments for the new revision? https://reviews.llvm.org/D45932 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44934: [analyzer] Improve the modeling of `memset()`.
a.sidorin added a comment.
Hi Henry. I had a quick look at the patch, here are some remarks.
Comment at: lib/StaticAnalyzer/Checkers/CStringChecker.cpp:2092
+ // 'invalidateRegions()', which will remove the pair
+ // in CStringLength map. So calls 'InvalidateBuffer()' after getting
+ // old string length and before setting the new string length.
"So we call"?
Comment at: lib/StaticAnalyzer/Checkers/CStringChecker.cpp:2100
+std::tie(StateNullChar, StateNonNullChar) =
+assumeZero(C, State, CharVal, Ctx.UnsignedCharTy);
+
I think we can use the argument type here (which is int).
One more question. Could we use `SVal::isZeroConstant()` here instead? Do we
need the path-sensitivity for the value or we can just check if the value is a
constant zero?
Comment at: lib/StaticAnalyzer/Checkers/CStringChecker.cpp:2103
+if (StateNullChar && !StateNonNullChar) {
+ // If the value of the second arguement of 'memset()' is zero, set the
+ // string length of destination buffer to 0 directly.
"argument"
Comment at: lib/StaticAnalyzer/Checkers/CStringChecker.cpp:2127
+ /*IsSourceBuffer*/ false, Size);
+ }
+
Could you please factor this chunk to a function? It makes the method too big.
Comment at: lib/StaticAnalyzer/Core/ProgramState.cpp:148
+ ProgramStateRef NewState = makeWithStore(NewStore);
+ return Mgr.getOwningEngine()
+ ? Mgr.getOwningEngine()->processRegionChange(NewState, R, LCtx)
Do clients of `overwriteRegion` really need to call checkers?
Comment at: lib/StaticAnalyzer/Core/RegionStore.cpp:446
+ // default binding.
+ StoreRef OverwriteRegion(Store store, const MemRegion *R, SVal V) override {
+RegionBindingsRef B = getRegionBindings(store);
LLVM naming conventions say that function names should start with small letter.
Comment at: lib/StaticAnalyzer/Core/RegionStore.cpp:1718
-llvm_unreachable("Unknown default value");
+return val;
}
Could you please explain what is the reason of this change? Do we get new value
kinds?
Comment at: lib/StaticAnalyzer/Core/Store.cpp:72
+StoreRef StoreManager::OverwriteRegion(Store store, const MemRegion *R, SVal
V) {
+ return StoreRef(store, *this);
Could we make this method pure virtual?
Repository:
rC Clang
https://reviews.llvm.org/D44934
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r331067 - [clang-format/ObjC] Use getIdentifierInfo() instead of tok::identifier
Author: benhamilton
Date: Fri Apr 27 11:51:12 2018
New Revision: 331067
URL: http://llvm.org/viewvc/llvm-project?rev=331067&view=rev
Log:
[clang-format/ObjC] Use getIdentifierInfo() instead of tok::identifier
Summary:
Previously, we checked tokens for `tok::identifier` to see if they
were identifiers inside an Objective-C selector.
However, this missed C++ keywords like `new` and `delete`.
To fix this, this diff uses `getIdentifierInfo()` to find
identifiers or keywords inside Objective-C selectors.
Test Plan: New tests added. Ran tests with:
% make -j16 FormatTests && ./tools/clang/unittests/Format/FormatTests
Reviewers: djasper, jolesiak
Reviewed By: djasper
Subscribers: klimek, cfe-commits
Differential Revision: https://reviews.llvm.org/D46143
Modified:
cfe/trunk/lib/Format/TokenAnnotator.cpp
cfe/trunk/unittests/Format/FormatTestObjC.cpp
Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=331067&r1=331066&r2=331067&view=diff
==
--- cfe/trunk/lib/Format/TokenAnnotator.cpp (original)
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp Fri Apr 27 11:51:12 2018
@@ -25,6 +25,21 @@ namespace format {
namespace {
+/// \brief Returns \c true if the token can be used as an identifier in
+/// an Objective-C \c @selector, \c false otherwise.
+///
+/// Because getFormattingLangOpts() always lexes source code as
+/// Objective-C++, C++ keywords like \c new and \c delete are
+/// lexed as tok::kw_*, not tok::identifier, even for Objective-C.
+///
+/// For Objective-C and Objective-C++, both identifiers and keywords
+/// are valid inside @selector(...) (or a macro which
+/// invokes @selector(...)). So, we allow treat any identifier or
+/// keyword as a potential Objective-C selector component.
+static bool canBeObjCSelectorComponent(const FormatToken &Tok) {
+ return Tok.Tok.getIdentifierInfo() != nullptr;
+}
+
/// \brief A parser that gathers additional information about tokens.
///
/// The \c TokenAnnotator tries to match parenthesis and square brakets and
@@ -703,9 +718,10 @@ private:
Tok->Type = TT_CtorInitializerColon;
else
Tok->Type = TT_InheritanceColon;
- } else if (Tok->Previous->is(tok::identifier) && Tok->Next &&
+ } else if (canBeObjCSelectorComponent(*Tok->Previous) && Tok->Next &&
(Tok->Next->isOneOf(tok::r_paren, tok::comma) ||
- Tok->Next->startsSequence(tok::identifier, tok::colon))) {
+ (canBeObjCSelectorComponent(*Tok->Next) && Tok->Next->Next &&
+ Tok->Next->Next->is(tok::colon {
// This handles a special macro in ObjC code where selectors including
// the colon are passed as macro arguments.
Tok->Type = TT_ObjCMethodExpr;
@@ -1346,7 +1362,7 @@ private:
TT_LeadingJavaAnnotation)) {
Current.Type = Current.Previous->Type;
}
-} else if (Current.isOneOf(tok::identifier, tok::kw_new) &&
+} else if (canBeObjCSelectorComponent(Current) &&
// FIXME(bug 36976): ObjC return types shouldn't use
TT_CastRParen.
Current.Previous && Current.Previous->is(TT_CastRParen) &&
Current.Previous->MatchingParen &&
@@ -2650,7 +2666,7 @@ bool TokenAnnotator::spaceRequiredBefore
if (Line.Type == LT_ObjCMethodDecl) {
if (Left.is(TT_ObjCMethodSpecifier))
return true;
-if (Left.is(tok::r_paren) && Right.isOneOf(tok::identifier, tok::kw_new))
+if (Left.is(tok::r_paren) && canBeObjCSelectorComponent(Right))
// Don't space between ')' and or ')' and 'new'. 'new' is not a
// keyword in Objective-C, and '+ (instancetype)new;' is a standard class
// method declaration.
@@ -3128,6 +3144,7 @@ void TokenAnnotator::printDebugInfo(cons
for (unsigned i = 0, e = Tok->FakeLParens.size(); i != e; ++i)
llvm::errs() << Tok->FakeLParens[i] << "/";
llvm::errs() << " FakeRParens=" << Tok->FakeRParens;
+llvm::errs() << " II=" << Tok->Tok.getIdentifierInfo();
llvm::errs() << " Text='" << Tok->TokenText << "'\n";
if (!Tok->Next)
assert(Tok == Line.Last);
Modified: cfe/trunk/unittests/Format/FormatTestObjC.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTestObjC.cpp?rev=331067&r1=331066&r2=331067&view=diff
==
--- cfe/trunk/unittests/Format/FormatTestObjC.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTestObjC.cpp Fri Apr 27 11:51:12 2018
@@ -945,7 +945,7 @@ TEST_F(FormatTestObjC, ObjCForIn) {
" }]) {\n}");
}
-TEST_F(FormatTestObjC, ObjCNew) {
+TEST_F(FormatTestObjC, ObjCCxxKeywords) {
verifyFormat("+ (instancetype)new {\n"
" return nil;\n"
"}\n");
@@ -954,6 +954,17 @@ T
[PATCH] D46143: [clang-format/ObjC] Use getIdentifierInfo() instead of tok::identifier
This revision was automatically updated to reflect the committed changes.
Closed by commit rC331067: [clang-format/ObjC] Use getIdentifierInfo() instead
of tok::identifier (authored by benhamilton, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D46143?vs=144357&id=144377#toc
Repository:
rC Clang
https://reviews.llvm.org/D46143
Files:
lib/Format/TokenAnnotator.cpp
unittests/Format/FormatTestObjC.cpp
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -25,6 +25,21 @@
namespace {
+/// \brief Returns \c true if the token can be used as an identifier in
+/// an Objective-C \c @selector, \c false otherwise.
+///
+/// Because getFormattingLangOpts() always lexes source code as
+/// Objective-C++, C++ keywords like \c new and \c delete are
+/// lexed as tok::kw_*, not tok::identifier, even for Objective-C.
+///
+/// For Objective-C and Objective-C++, both identifiers and keywords
+/// are valid inside @selector(...) (or a macro which
+/// invokes @selector(...)). So, we allow treat any identifier or
+/// keyword as a potential Objective-C selector component.
+static bool canBeObjCSelectorComponent(const FormatToken &Tok) {
+ return Tok.Tok.getIdentifierInfo() != nullptr;
+}
+
/// \brief A parser that gathers additional information about tokens.
///
/// The \c TokenAnnotator tries to match parenthesis and square brakets and
@@ -703,9 +718,10 @@
Tok->Type = TT_CtorInitializerColon;
else
Tok->Type = TT_InheritanceColon;
- } else if (Tok->Previous->is(tok::identifier) && Tok->Next &&
+ } else if (canBeObjCSelectorComponent(*Tok->Previous) && Tok->Next &&
(Tok->Next->isOneOf(tok::r_paren, tok::comma) ||
- Tok->Next->startsSequence(tok::identifier, tok::colon))) {
+ (canBeObjCSelectorComponent(*Tok->Next) && Tok->Next->Next &&
+ Tok->Next->Next->is(tok::colon {
// This handles a special macro in ObjC code where selectors including
// the colon are passed as macro arguments.
Tok->Type = TT_ObjCMethodExpr;
@@ -1346,7 +1362,7 @@
TT_LeadingJavaAnnotation)) {
Current.Type = Current.Previous->Type;
}
-} else if (Current.isOneOf(tok::identifier, tok::kw_new) &&
+} else if (canBeObjCSelectorComponent(Current) &&
// FIXME(bug 36976): ObjC return types shouldn't use
TT_CastRParen.
Current.Previous && Current.Previous->is(TT_CastRParen) &&
Current.Previous->MatchingParen &&
@@ -2650,7 +2666,7 @@
if (Line.Type == LT_ObjCMethodDecl) {
if (Left.is(TT_ObjCMethodSpecifier))
return true;
-if (Left.is(tok::r_paren) && Right.isOneOf(tok::identifier, tok::kw_new))
+if (Left.is(tok::r_paren) && canBeObjCSelectorComponent(Right))
// Don't space between ')' and or ')' and 'new'. 'new' is not a
// keyword in Objective-C, and '+ (instancetype)new;' is a standard class
// method declaration.
@@ -3128,6 +3144,7 @@
for (unsigned i = 0, e = Tok->FakeLParens.size(); i != e; ++i)
llvm::errs() << Tok->FakeLParens[i] << "/";
llvm::errs() << " FakeRParens=" << Tok->FakeRParens;
+llvm::errs() << " II=" << Tok->Tok.getIdentifierInfo();
llvm::errs() << " Text='" << Tok->TokenText << "'\n";
if (!Tok->Next)
assert(Tok == Line.Last);
Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -945,15 +945,26 @@
" }]) {\n}");
}
-TEST_F(FormatTestObjC, ObjCNew) {
+TEST_F(FormatTestObjC, ObjCCxxKeywords) {
verifyFormat("+ (instancetype)new {\n"
" return nil;\n"
"}\n");
verifyFormat("+ (instancetype)myNew {\n"
" return [self new];\n"
"}\n");
verifyFormat("SEL NewSelector(void) { return @selector(new); }\n");
verifyFormat("SEL MacroSelector(void) { return MACRO(new); }\n");
+ verifyFormat("+ (instancetype)delete {\n"
+ " return nil;\n"
+ "}\n");
+ verifyFormat("+ (instancetype)myDelete {\n"
+ " return [self delete];\n"
+ "}\n");
+ verifyFormat("SEL DeleteSelector(void) { return @selector(delete); }\n");
+ verifyFormat("SEL MacroSelector(void) { return MACRO(delete); }\n");
+ verifyFormat("MACRO(new:)\n");
+ verifyFormat("MACRO(delete:)\n");
+ verifyFormat("foo = @{MACRO(new:) : MACRO(delete:)}\n");
}
TEST_F(FormatTestObjC, ObjCLiterals) {
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -25,6 +25,21 @@
namespace {
+/// \brief Returns \c
[PATCH] D46159: [clang-tidy] Add a flag to enable alpha checkers
NoQ added inline comments. Comment at: clang-tidy/ClangTidy.cpp:373-376 // FIXME: Remove this option once clang's cfg-temporary-dtors option defaults // to true. AnalyzerOptions->Config["cfg-temporary-dtors"] = Context.getOptions().AnalyzeTemporaryDtors ? "true" : "false"; Btw this is already enabled by default. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D46159 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46035: [clangd] Fix unicode handling, using UTF-16 where LSP requires it.
benhamilton added inline comments. Comment at: clang-tools-extra/trunk/clangd/SourceCode.cpp:38 +// 0xxx is ASCII, handled above. 10xxx is a trailing byte, invalid here. +// 1xxx is not valid UTF-8 at all. Assert because it's probably our bug. +assert((UTF8Length >= 2 && UTF8Length <= 4) && This is user input, right? Have we actually checked for valid UTF-8, or do we just assume it's valid? If not, it seems like an assertion is not the right check, but we should reject it when we're reading the input. Repository: rL LLVM https://reviews.llvm.org/D46035 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r331069 - s/LLVM_ON_WIN32/_WIN32/, clang
Author: nico
Date: Fri Apr 27 12:11:14 2018
New Revision: 331069
URL: http://llvm.org/viewvc/llvm-project?rev=331069&view=rev
Log:
s/LLVM_ON_WIN32/_WIN32/, clang
LLVM_ON_WIN32 is set exactly with MSVC and MinGW (but not Cygwin) in
HandleLLVMOptions.cmake, which is where _WIN32 defined too. Just use the
default macro instead of a reinvented one.
See thread "Replacing LLVM_ON_WIN32 with just _WIN32" on llvm-dev and cfe-dev.
No intended behavior change.
Modified:
cfe/trunk/examples/clang-interpreter/Manager.h
cfe/trunk/lib/Basic/FileManager.cpp
cfe/trunk/lib/Basic/VirtualFileSystem.cpp
cfe/trunk/lib/Driver/ToolChain.cpp
cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
cfe/trunk/lib/Driver/ToolChains/MSVC.cpp
cfe/trunk/lib/Driver/ToolChains/PS4CPU.cpp
cfe/trunk/lib/Frontend/InitHeaderSearch.cpp
cfe/trunk/lib/Frontend/PrecompiledPreamble.cpp
cfe/trunk/lib/Lex/PPDirectives.cpp
cfe/trunk/tools/driver/driver.cpp
cfe/trunk/tools/libclang/CIndex.cpp
cfe/trunk/tools/libclang/CIndexer.cpp
cfe/trunk/unittests/ASTMatchers/ASTMatchersInternalTest.cpp
cfe/trunk/unittests/Basic/FileManagerTest.cpp
cfe/trunk/unittests/Driver/ToolChainTest.cpp
cfe/trunk/unittests/Tooling/RefactoringTest.cpp
cfe/trunk/unittests/Tooling/ToolingTest.cpp
Modified: cfe/trunk/examples/clang-interpreter/Manager.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/examples/clang-interpreter/Manager.h?rev=331069&r1=331068&r2=331069&view=diff
==
--- cfe/trunk/examples/clang-interpreter/Manager.h (original)
+++ cfe/trunk/examples/clang-interpreter/Manager.h Fri Apr 27 12:11:14 2018
@@ -12,7 +12,7 @@
#include "llvm/ExecutionEngine/SectionMemoryManager.h"
-#if defined(LLVM_ON_WIN32) && defined(_WIN64)
+#if defined(_WIN32) && defined(_WIN64)
#define CLANG_INTERPRETER_COFF_FORMAT
#define CLANG_INTERPRETER_WIN_EXCEPTIONS
#endif
Modified: cfe/trunk/lib/Basic/FileManager.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/FileManager.cpp?rev=331069&r1=331068&r2=331069&view=diff
==
--- cfe/trunk/lib/Basic/FileManager.cpp (original)
+++ cfe/trunk/lib/Basic/FileManager.cpp Fri Apr 27 12:11:14 2018
@@ -157,7 +157,7 @@ const DirectoryEntry *FileManager::getDi
DirName != llvm::sys::path::root_path(DirName) &&
llvm::sys::path::is_separator(DirName.back()))
DirName = DirName.substr(0, DirName.size()-1);
-#ifdef LLVM_ON_WIN32
+#ifdef _WIN32
// Fixing a problem with "clang C:test.c" on Windows.
// Stat("C:") does not recognize "C:" as a valid directory
std::string DirNameStr;
Modified: cfe/trunk/lib/Basic/VirtualFileSystem.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/VirtualFileSystem.cpp?rev=331069&r1=331068&r2=331069&view=diff
==
--- cfe/trunk/lib/Basic/VirtualFileSystem.cpp (original)
+++ cfe/trunk/lib/Basic/VirtualFileSystem.cpp Fri Apr 27 12:11:14 2018
@@ -967,7 +967,7 @@ class RedirectingFileSystem : public vfs
/// "." and "./" in their paths. FIXME: some unittests currently fail on
/// win32 when using remove_dots and remove_leading_dotslash on paths.
bool UseCanonicalizedPaths =
-#ifdef LLVM_ON_WIN32
+#ifdef _WIN32
false;
#else
true;
@@ -1560,7 +1560,7 @@ ErrorOr RedirectingFileSystem::
ErrorOr
RedirectingFileSystem::lookupPath(sys::path::const_iterator Start,
sys::path::const_iterator End, Entry *From) {
-#ifndef LLVM_ON_WIN32
+#ifndef _WIN32
assert(!isTraversalComponent(*Start) &&
!isTraversalComponent(From->getName()) &&
"Paths should not contain traversal components");
Modified: cfe/trunk/lib/Driver/ToolChain.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChain.cpp?rev=331069&r1=331068&r2=331069&view=diff
==
--- cfe/trunk/lib/Driver/ToolChain.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChain.cpp Fri Apr 27 12:11:14 2018
@@ -166,7 +166,7 @@ static const DriverSuffix *FindDriverSuf
/// present and lower-casing the string on Windows.
static std::string normalizeProgramName(llvm::StringRef Argv0) {
std::string ProgName = llvm::sys::path::stem(Argv0);
-#ifdef LLVM_ON_WIN32
+#ifdef _WIN32
// Transform to lowercase for case insensitive file systems.
std::transform(ProgName.begin(), ProgName.end(), ProgName.begin(),
::tolower);
#endif
Modified: cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp?rev=331069&r1=331068&r2=331069&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/CommonA
[PATCH] D46206: [clang-tidy] Add Apple prefix acronyms to objc-property-declaration
benhamilton created this revision. benhamilton added reviewers: Wizard, hokein. Herald added subscribers: cfe-commits, xazax.hun, klimek. This adds a few common Apple first-party API prefixes as acronyms to `objc-property-declaration`. Here's a list showing where these come from: http://nshipster.com/namespacing/ Test Plan: New tests added. Ran tests with: % make -j16 check-clang-tools Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D46206 Files: clang-tidy/objc/PropertyDeclarationCheck.cpp test/clang-tidy/objc-property-declaration.m Index: test/clang-tidy/objc-property-declaration.m === --- test/clang-tidy/objc-property-declaration.m +++ test/clang-tidy/objc-property-declaration.m @@ -19,6 +19,8 @@ @property(strong, nonatomic) NSString *VCsPluralToAdd; @property(assign, nonatomic) int centerX; @property(assign, nonatomic) int enable2GBackgroundFetch; +@property(assign, nonatomic) int shouldUseCFPreferences; +@property(assign, nonatomic) int enableGLAcceleration; @end @interface Foo (Bar) Index: clang-tidy/objc/PropertyDeclarationCheck.cpp === --- clang-tidy/objc/PropertyDeclarationCheck.cpp +++ clang-tidy/objc/PropertyDeclarationCheck.cpp @@ -45,11 +45,17 @@ "ARGB", "ASCII", "BGRA", +"CA", +"CF", +"CG", +"CI", +"CV", "CMYK", "DNS", "FPS", "FTP", "GIF", +"GL", "GPS", "GUID", "HD", @@ -65,6 +71,7 @@ "LZW", "MDNS", "MIDI", +"NS", "OS", "PDF", "PIN", @@ -81,6 +88,7 @@ "RPC", "RTF", "RTL", +"SC", "SDK", "SSO", "TCP", Index: test/clang-tidy/objc-property-declaration.m === --- test/clang-tidy/objc-property-declaration.m +++ test/clang-tidy/objc-property-declaration.m @@ -19,6 +19,8 @@ @property(strong, nonatomic) NSString *VCsPluralToAdd; @property(assign, nonatomic) int centerX; @property(assign, nonatomic) int enable2GBackgroundFetch; +@property(assign, nonatomic) int shouldUseCFPreferences; +@property(assign, nonatomic) int enableGLAcceleration; @end @interface Foo (Bar) Index: clang-tidy/objc/PropertyDeclarationCheck.cpp === --- clang-tidy/objc/PropertyDeclarationCheck.cpp +++ clang-tidy/objc/PropertyDeclarationCheck.cpp @@ -45,11 +45,17 @@ "ARGB", "ASCII", "BGRA", +"CA", +"CF", +"CG", +"CI", +"CV", "CMYK", "DNS", "FPS", "FTP", "GIF", +"GL", "GPS", "GUID", "HD", @@ -65,6 +71,7 @@ "LZW", "MDNS", "MIDI", +"NS", "OS", "PDF", "PIN", @@ -81,6 +88,7 @@ "RPC", "RTF", "RTL", +"SC", "SDK", "SSO", "TCP", ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46159: [clang-tidy] Add a flag to enable alpha checkers
NoQ added a comment. Am i understanding correctly that the new `-enable-alpha-checks` flag doesn't enable alpha checks, but it only allows enabling them with a separate command? Also wanted to mention that developers who are interested in running analyzer alpha checkers over a compilation database also have an option of trying to use `scan-build-py` which has support for both generating compilation databases from arbitrary build systems and running over existing compilation databases. This tool is also not recommended for actual users due to a number of regressions from the existing scan-build, but it might get the job done when it comes to developing and testing the analyzer itself. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D46159 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46159: [clang-tidy] Add a flag to enable alpha checkers
lebedev.ri added a comment. In https://reviews.llvm.org/D46159#1081559, @NoQ wrote: > Am i understanding correctly that the new `-enable-alpha-checks` flag doesn't > enable alpha checks, but it only allows enabling them with a separate command? Yes, absolutely! Same with my followup https://reviews.llvm.org/D46188. Perhaps something like `-allow-enabling-alpha-checks` would be a better name, but it is kinda long. > Also wanted to mention that developers who are interested in running analyzer > alpha checkers over a compilation database also have an option of trying to > use `scan-build-py` which has support for both generating compilation > databases from arbitrary build systems and running over existing compilation > databases. Good to know, i think this is the first time i'm hearing of it. > This tool is also not recommended for actual users due to a number of > regressions from the existing scan-build, but it might get the job done when > it comes to developing and testing the analyzer itself. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D46159 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r331077 - Revert r329698 (and r329702).
Author: nico
Date: Fri Apr 27 13:29:57 2018
New Revision: 331077
URL: http://llvm.org/viewvc/llvm-project?rev=331077&view=rev
Log:
Revert r329698 (and r329702).
Speculative. ClangMoveTests started failing on
http://lab.llvm.org:8011/builders/clang-x64-ninja-win7/builds/9958
after this change. I can't reproduce on my machine, let's see
if it was due to this change.
Modified:
cfe/trunk/lib/Basic/FileManager.cpp
cfe/trunk/lib/Frontend/ModuleDependencyCollector.cpp
Modified: cfe/trunk/lib/Basic/FileManager.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/FileManager.cpp?rev=331077&r1=331076&r2=331077&view=diff
==
--- cfe/trunk/lib/Basic/FileManager.cpp (original)
+++ cfe/trunk/lib/Basic/FileManager.cpp Fri Apr 27 13:29:57 2018
@@ -534,9 +534,23 @@ StringRef FileManager::getCanonicalName(
StringRef CanonicalName(Dir->getName());
- SmallString<256> CanonicalNameBuf;
- if (!llvm::sys::fs::real_path(Dir->getName(), CanonicalNameBuf))
+#ifdef LLVM_ON_UNIX
+ char CanonicalNameBuf[PATH_MAX];
+ if (realpath(Dir->getName().str().c_str(), CanonicalNameBuf))
CanonicalName = StringRef(CanonicalNameBuf).copy(CanonicalNameStorage);
+#else
+ SmallString<256> CanonicalNameBuf(CanonicalName);
+ llvm::sys::fs::make_absolute(CanonicalNameBuf);
+ llvm::sys::path::native(CanonicalNameBuf);
+ // We've run into needing to remove '..' here in the wild though, so
+ // remove it.
+ // On Windows, symlinks are significantly less prevalent, so removing
+ // '..' is pretty safe.
+ // Ideally we'd have an equivalent of `realpath` and could implement
+ // sys::fs::canonical across all the platforms.
+ llvm::sys::path::remove_dots(CanonicalNameBuf, /* remove_dot_dot */ true);
+ CanonicalName = StringRef(CanonicalNameBuf).copy(CanonicalNameStorage);
+#endif
CanonicalDirNames.insert(std::make_pair(Dir, CanonicalName));
return CanonicalName;
Modified: cfe/trunk/lib/Frontend/ModuleDependencyCollector.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/ModuleDependencyCollector.cpp?rev=331077&r1=331076&r2=331077&view=diff
==
--- cfe/trunk/lib/Frontend/ModuleDependencyCollector.cpp (original)
+++ cfe/trunk/lib/Frontend/ModuleDependencyCollector.cpp Fri Apr 27 13:29:57
2018
@@ -97,6 +97,24 @@ struct ModuleDependencyMMCallbacks : pub
}
+// TODO: move this to Support/Path.h and check for HAVE_REALPATH?
+static bool real_path(StringRef SrcPath, SmallVectorImpl &RealPath) {
+#ifdef LLVM_ON_UNIX
+ char CanonicalPath[PATH_MAX];
+
+ // TODO: emit a warning in case this fails...?
+ if (!realpath(SrcPath.str().c_str(), CanonicalPath))
+return false;
+
+ SmallString<256> RPath(CanonicalPath);
+ RealPath.swap(RPath);
+ return true;
+#else
+ // FIXME: Add support for systems without realpath.
+ return false;
+#endif
+}
+
void ModuleDependencyCollector::attachToASTReader(ASTReader &R) {
R.addListener(llvm::make_unique(*this));
}
@@ -111,7 +129,7 @@ void ModuleDependencyCollector::attachTo
static bool isCaseSensitivePath(StringRef Path) {
SmallString<256> TmpDest = Path, UpperDest, RealDest;
// Remove component traversals, links, etc.
- if (llvm::sys::fs::real_path(Path, TmpDest))
+ if (!real_path(Path, TmpDest))
return true; // Current default value in vfs.yaml
Path = TmpDest;
@@ -121,7 +139,7 @@ static bool isCaseSensitivePath(StringRe
// already expects when sensitivity isn't setup.
for (auto &C : Path)
UpperDest.push_back(toUppercase(C));
- if (!llvm::sys::fs::real_path(UpperDest, RealDest) && Path.equals(RealDest))
+ if (real_path(UpperDest, RealDest) && Path.equals(RealDest))
return false;
return true;
}
@@ -171,7 +189,7 @@ bool ModuleDependencyCollector::getRealP
// Computing the real path is expensive, cache the search through the
// parent path directory.
if (DirWithSymLink == SymLinkMap.end()) {
-if (llvm::sys::fs::real_path(Dir, RealPath))
+if (!real_path(Dir, RealPath))
return false;
SymLinkMap[Dir] = RealPath.str();
} else {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46148: [CUDA] Added -f[no-]cuda-short-ptr option
bkramer accepted this revision. bkramer added a comment. This revision is now accepted and ready to land. lgtm https://reviews.llvm.org/D46148 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46159: [clang-tidy] Add a flag to enable alpha checkers
pfultz2 added a comment. > I'm curious about the use case for this, since I don't think it ever makes > sense to run all the alpha checks at once. These checks are typically a work > in progress (they're not finished yet) and often have false positives that > haven't been fixed yet. We want to run several of the alpha checks in our codebase, and as we find bugs and FPs we can then contribute patches to fix them. Since most of our repos use compile_commands.json, it easier to do this in clang tidy instead of scan-build or clang driver. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D46159 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46159: [clang-tidy] Add a flag to enable alpha checkers
pfultz2 added a comment. > Am i understanding correctly that the new -enable-alpha-checks flag doesn't > enable alpha checks, but it only allows enabling them with a separate command? Yes, it enables them to be selected by clang tidy. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D46159 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r331089 - [analyzer] ObjCAutoreleaseWrite: Support a few more APIs and fix warning text.
Author: dergachev
Date: Fri Apr 27 15:00:51 2018
New Revision: 331089
URL: http://llvm.org/viewvc/llvm-project?rev=331089&view=rev
Log:
[analyzer] ObjCAutoreleaseWrite: Support a few more APIs and fix warning text.
API list and improved warning text composed by Devin Coughlin.
Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/ObjCAutoreleaseWriteChecker.cpp
cfe/trunk/test/Analysis/autoreleasewritechecker_test.m
Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/ObjCAutoreleaseWriteChecker.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/ObjCAutoreleaseWriteChecker.cpp?rev=331089&r1=331088&r2=331089&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/ObjCAutoreleaseWriteChecker.cpp
(original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/ObjCAutoreleaseWriteChecker.cpp Fri
Apr 27 15:00:51 2018
@@ -51,18 +51,43 @@ public:
BugReporter &BR) const;
private:
std::vector SelectorsWithAutoreleasingPool = {
+ // Common to NSArray, NSSet, NSOrderedSet
"enumerateObjectsUsingBlock:",
- "enumerateKeysAndObjectsUsingBlock:",
- "enumerateKeysAndObjectsWithOptions:usingBlock:",
"enumerateObjectsWithOptions:usingBlock:",
+
+ // Common to NSArray and NSOrderedSet
"enumerateObjectsAtIndexes:options:usingBlock:",
+ "indexOfObjectAtIndexes:options:passingTest:",
+ "indexesOfObjectsAtIndexes:options:passingTest:",
+ "indexOfObjectPassingTest:",
+ "indexOfObjectWithOptions:passingTest:",
+ "indexesOfObjectsPassingTest:",
+ "indexesOfObjectsWithOptions:passingTest:",
+
+ // NSDictionary
+ "enumerateKeysAndObjectsUsingBlock:",
+ "enumerateKeysAndObjectsWithOptions:usingBlock:",
+ "keysOfEntriesPassingTest:",
+ "keysOfEntriesWithOptions:passingTest:",
+
+ // NSSet
+ "objectsPassingTest:",
+ "objectsWithOptions:passingTest:",
+ "enumerateIndexPathsWithOptions:usingBlock:",
+
+ // NSIndexSet
"enumerateIndexesWithOptions:usingBlock:",
"enumerateIndexesUsingBlock:",
"enumerateIndexesInRange:options:usingBlock:",
"enumerateRangesUsingBlock:",
"enumerateRangesWithOptions:usingBlock:",
- "enumerateRangesInRange:options:usingBlock:"
- "objectWithOptions:passingTest:",
+ "enumerateRangesInRange:options:usingBlock:",
+ "indexPassingTest:",
+ "indexesPassingTest:",
+ "indexWithOptions:passingTest:",
+ "indexesWithOptions:passingTest:",
+ "indexInRange:options:passingTest:",
+ "indexesInRange:options:passingTest:"
};
std::vector FunctionsWithAutoreleasingPool = {
@@ -95,9 +120,9 @@ static void emitDiagnostics(BoundNodes &
assert(SW);
BR.EmitBasicReport(
ADC->getDecl(), Checker,
- /*Name=*/"Writing into auto-releasing variable from a different queue",
+ /*Name=*/"Write to autoreleasing out parameter inside autorelease pool",
/*Category=*/"Memory",
- (llvm::Twine("Writing into an auto-releasing out parameter inside ") +
+ (llvm::Twine("Write to autoreleasing out parameter inside ") +
"autorelease pool that may exit before " + Name + " returns; consider "
"writing first to a strong local variable declared outside of the
block")
.str(),
Modified: cfe/trunk/test/Analysis/autoreleasewritechecker_test.m
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/autoreleasewritechecker_test.m?rev=331089&r1=331088&r2=331089&view=diff
==
--- cfe/trunk/test/Analysis/autoreleasewritechecker_test.m (original)
+++ cfe/trunk/test/Analysis/autoreleasewritechecker_test.m Fri Apr 27 15:00:51
2018
@@ -4,6 +4,7 @@
typedef signed char BOOL;
+#define YES ((BOOL)1)
@protocol NSObject - (BOOL)isEqual:(id)object; @end
@interface NSObject {}
+(id)alloc;
@@ -14,6 +15,8 @@ typedef signed char BOOL;
@end
typedef int NSZone;
typedef int NSCoder;
+typedef unsigned long NSUInteger;
+
@protocol NSCopying - (id)copyWithZone:(NSZone *)zone; @end
@protocol NSCoding - (void)encodeWithCoder:(NSCoder *)aCoder; @end
@interface NSError : NSObject {}
@@ -23,8 +26,27 @@ typedef int NSCoder;
typedef int dispatch_semaphore_t;
typedef void (^block_t)();
+typedef enum {
+ NSEnumerationConcurrent = (1UL << 0),
+ NSEnumerationReverse = (1UL << 1)
+} NSEnumerationOptions;
+
@interface NSArray
-- (void) enumerateObjectsUsingBlock:(block_t)block;
+- (void)enumerateObjectsUsingBlock:(block_t)block;
+@end
+
+@interface NSSet
+- (void)objectsPassingTest:(block_t)block;
+@end
+
+@interface NSDictionary
+- (void)enumerateKeysAndObjectsUsingBlock:(block_t)block;
+@end
+
+@interface NSIndexSet
+- (void)indexesPassingTest:(block_t)block;
+- (NSUInteger)indexWithOptions:(NSEnumerationOptions)opts
+ passingTest:(BOOL (^)(NSUInteger idx, BOOL *stop))predicate;
r331093 - Fix diag-format test to not care about what cl.exe is on path
Author: rnk
Date: Fri Apr 27 15:32:21 2018
New Revision: 331093
URL: http://llvm.org/viewvc/llvm-project?rev=331093&view=rev
Log:
Fix diag-format test to not care about what cl.exe is on path
Modified:
cfe/trunk/test/Misc/diag-format.c
Modified: cfe/trunk/test/Misc/diag-format.c
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Misc/diag-format.c?rev=331093&r1=331092&r2=331093&view=diff
==
--- cfe/trunk/test/Misc/diag-format.c (original)
+++ cfe/trunk/test/Misc/diag-format.c Fri Apr 27 15:32:21 2018
@@ -37,7 +37,7 @@
// DEFAULT: {{.*}}:36:8: warning: extra tokens at end of #endif directive
[-Wextra-tokens]
// MSVC2010: {{.*}}(36,7) : warning: extra tokens at end of #endif directive
[-Wextra-tokens]
// MSVC2013: {{.*}}(36,8) : warning: extra tokens at end of #endif directive
[-Wextra-tokens]
-// MSVC: {{.*}}(36,8){{ ?}}: warning: extra tokens at end of #endif directive
[-Wextra-tokens]
+// MSVC: {{.*\(36,[78]\) ?}}: warning: extra tokens at end of #endif directive
[-Wextra-tokens]
// MSVC2015: {{.*}}(36,8): warning: extra tokens at end of #endif directive
[-Wextra-tokens]
// VI: {{.*}} +36:8: warning: extra tokens at end of #endif directive
[-Wextra-tokens]
// MSVC2015_ORIG: {{.*}}(36): warning: extra tokens at end of #endif directive
[-Wextra-tokens]
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r331056 - [docs] add -ffp-cast-overflow-workaround to the release notes
On 27 April 2018 at 09:21, Sanjay Patel via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: spatel > Date: Fri Apr 27 09:21:22 2018 > New Revision: 331056 > > URL: http://llvm.org/viewvc/llvm-project?rev=331056&view=rev > Log: > [docs] add -ffp-cast-overflow-workaround to the release notes > > This option was added with: > D46135 > rL331041 > ...copying the text from UsersManual.rst for more exposure. > > > Modified: > cfe/trunk/docs/ReleaseNotes.rst > > Modified: cfe/trunk/docs/ReleaseNotes.rst > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ > ReleaseNotes.rst?rev=331056&r1=331055&r2=331056&view=diff > > == > --- cfe/trunk/docs/ReleaseNotes.rst (original) > +++ cfe/trunk/docs/ReleaseNotes.rst Fri Apr 27 09:21:22 2018 > @@ -83,6 +83,15 @@ Non-comprehensive list of changes in thi > New Compiler Flags > -- > > +- :option:`-ffp-cast-overflow-workaround` and > + :option:`-fnofp-cast-overflow-workaround` > Shouldn't this be -fno-fp-cast-overflow-workaround? Also, our convention for flags that define undefined behavior is `-fno-strict-*`, so perhaps this should be `-fno-strict-fp-cast-overflow`? > + enable (disable) a workaround for code that casts floating-point values > to > + integers and back to floating-point. If the floating-point value is not > + representable in the intermediate integer type, the code is incorrect > + according to the language standard. I find this hard to read: I initially misread "the code is incorrect according to the language standard" as meaning "Clang will generate code that is incorrect according to the language standard". I think what you mean here is "the code has undefined behavior according to the language standard, and Clang will not guarantee any particular result. This flag causes the behavior to be defined to match the overflowing behavior of the target's native float-to-int conversion instructions." > This flag will attempt to generate code > + as if the result of an overflowing conversion matches the overflowing > behavior > + of a target's native float-to-int conversion instructions. > + > - ... > > Deprecated Compiler Flags > > > ___ > cfe-commits mailing list > cfe-commits@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits > ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46218: PR37275 packed attribute should not apply to base classes
rsmith created this revision.
rsmith added a reviewer: rjmccall.
Clang incorrectly applies the packed attribute to base classes. Per GCC's
documentation and as can be observed from its behavior, packed only applies to
members, not base classes.
This change is conditioned behind `-fclang-abi-compat` so that an ABI break can
be avoided by users if desired.
Repository:
rC Clang
https://reviews.llvm.org/D46218
Files:
lib/AST/RecordLayoutBuilder.cpp
test/CodeGenCXX/alignment.cpp
test/SemaCXX/class-layout.cpp
Index: test/SemaCXX/class-layout.cpp
===
--- test/SemaCXX/class-layout.cpp
+++ test/SemaCXX/class-layout.cpp
@@ -1,5 +1,6 @@
// RUN: %clang_cc1 -triple x86_64-unknown-unknown %s -fsyntax-only -verify -std=c++98 -Wno-inaccessible-base
// RUN: %clang_cc1 -triple x86_64-unknown-unknown %s -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown %s -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base -fclang-abi-compat=6 -DCLANG_ABI_COMPAT=6
// expected-no-diagnostics
#define SA(n, p) int a##n[(p) ? 1 : -1]
@@ -570,3 +571,19 @@
SA(0, sizeof(might_use_tail_padding) == 80);
}
} // namespace PR16537
+
+namespace PR37275 {
+ struct X { char c; };
+
+ struct A { int n; };
+ _Static_assert(_Alignof(A) == _Alignof(int), "");
+
+ struct __attribute__((packed)) B : X, A {};
+#if defined(CLANG_ABI_COMPAT) && CLANG_ABI_COMPAT <= 6
+ _Static_assert(_Alignof(B) == 1, "");
+ _Static_assert(__builtin_offsetof(B, n) == 1, "");
+#else
+ _Static_assert(_Alignof(B) == _Alignof(int), "");
+ _Static_assert(__builtin_offsetof(B, n) == 4, "");
+#endif
+}
Index: test/CodeGenCXX/alignment.cpp
===
--- test/CodeGenCXX/alignment.cpp
+++ test/CodeGenCXX/alignment.cpp
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 %s -emit-llvm -o - -triple=x86_64-apple-darwin10 | FileCheck %s
+// RUN: %clang_cc1 %s -emit-llvm -o - -triple=x86_64-apple-darwin10 | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-NOCOMPAT
+// RUN: %clang_cc1 %s -emit-llvm -o - -triple=x86_64-apple-darwin10 -fclang-abi-compat=6.0 | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-V6COMPAT
extern int int_source();
extern void int_sink(int x);
@@ -54,19 +55,22 @@
// CHECK: [[B_P:%.*]] = bitcast i8* [[T1]] to [[B]]*
// CHECK: [[FIELD_P:%.*]] = bitcast [[B]]* [[B_P]] to i8*
// CHECK: [[TRUNC:%.*]] = trunc i32 [[CALL]] to i8
-// CHECK: [[OLD_VALUE:%.*]] = load i8, i8* [[FIELD_P]], align 2
+// CHECK-V6COMPAT: [[OLD_VALUE:%.*]] = load i8, i8* [[FIELD_P]], align 2
+// CHECK-NOCOMPAT: [[OLD_VALUE:%.*]] = load i8, i8* [[FIELD_P]], align 4
// CHECK: [[T0:%.*]] = and i8 [[TRUNC]], 3
// CHECK: [[T1:%.*]] = and i8 [[OLD_VALUE]], -4
// CHECK: [[T2:%.*]] = or i8 [[T1]], [[T0]]
-// CHECK: store i8 [[T2]], i8* [[FIELD_P]], align 2
+// CHECK-V6COMPAT: store i8 [[T2]], i8* [[FIELD_P]], align 2
+// CHECK-NOCOMPAT: store i8 [[T2]], i8* [[FIELD_P]], align 4
c.onebit = int_source();
// CHECK: [[C_P:%.*]] = load [[C]]*, [[C]]**
// CHECK: [[T0:%.*]] = bitcast [[C]]* [[C_P]] to i8*
// CHECK: [[T1:%.*]] = getelementptr inbounds i8, i8* [[T0]], i64 8
// CHECK: [[B_P:%.*]] = bitcast i8* [[T1]] to [[B]]*
// CHECK: [[FIELD_P:%.*]] = bitcast [[B]]* [[B_P]] to i8*
-// CHECK: [[VALUE:%.*]] = load i8, i8* [[FIELD_P]], align 2
+// CHECK-V6COMPAT: [[VALUE:%.*]] = load i8, i8* [[FIELD_P]], align 2
+// CHECK-NOCOMPAT: [[VALUE:%.*]] = load i8, i8* [[FIELD_P]], align 4
// CHECK: [[T0:%.*]] = shl i8 [[VALUE]], 6
// CHECK: [[T1:%.*]] = ashr i8 [[T0]], 6
// CHECK: [[T2:%.*]] = sext i8 [[T1]] to i32
@@ -83,19 +87,22 @@
// CHECK: [[B_P:%.*]] = bitcast i8* [[T1]] to [[B]]*
// CHECK: [[FIELD_P:%.*]] = bitcast [[B]]* [[B_P]] to i8*
// CHECK: [[TRUNC:%.*]] = trunc i32 [[CALL]] to i8
-// CHECK: [[OLD_VALUE:%.*]] = load i8, i8* [[FIELD_P]], align 2
+// CHECK-V6COMPAT: [[OLD_VALUE:%.*]] = load i8, i8* [[FIELD_P]], align 2
+// CHECK-NOCOMPAT: [[OLD_VALUE:%.*]] = load i8, i8* [[FIELD_P]], align 4
// CHECK: [[T0:%.*]] = and i8 [[TRUNC]], 3
// CHECK: [[T1:%.*]] = and i8 [[OLD_VALUE]], -4
// CHECK: [[T2:%.*]] = or i8 [[T1]], [[T0]]
-// CHECK: store i8 [[T2]], i8* [[FIELD_P]], align 2
+// CHECK-V6COMPAT: store i8 [[T2]], i8* [[FIELD_P]], align 2
+// CHECK-NOCOMPAT: store i8 [[T2]], i8* [[FIELD_P]], align 4
c->onebit = int_source();
// CHECK: [[C_P:%.*]] = load [[C:%.*]]*, [[C]]**
// CHECK: [[T0:%.*]] = bitcast [[C]]* [[C_P]] to i8*
// CHECK: [[T1:%.*]] = getelementptr inbounds i8, i8* [[T0]], i64 8
// CHECK: [[B_P:%.*]] = bitcast i8* [[T1]] to [[B:%.*]]*
// CHECK: [[FIELD_P:%.*]] = bitcast [[B]]* [[B_P]] to i8*
-// CHECK: [[VALUE:%.*]] = load i8, i8* [[FIELD_P]], align 2
+// CHECK-V6COMPAT: [[VALUE:%.*]] = load i8
[PATCH] D46218: PR37275 packed attribute should not apply to base classes
rsmith updated this revision to Diff 144415.
rsmith added a comment.
Added release notes.
Repository:
rC Clang
https://reviews.llvm.org/D46218
Files:
docs/ReleaseNotes.rst
lib/AST/RecordLayoutBuilder.cpp
test/CodeGenCXX/alignment.cpp
test/SemaCXX/class-layout.cpp
Index: test/SemaCXX/class-layout.cpp
===
--- test/SemaCXX/class-layout.cpp
+++ test/SemaCXX/class-layout.cpp
@@ -1,5 +1,6 @@
// RUN: %clang_cc1 -triple x86_64-unknown-unknown %s -fsyntax-only -verify -std=c++98 -Wno-inaccessible-base
// RUN: %clang_cc1 -triple x86_64-unknown-unknown %s -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown %s -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base -fclang-abi-compat=6 -DCLANG_ABI_COMPAT=6
// expected-no-diagnostics
#define SA(n, p) int a##n[(p) ? 1 : -1]
@@ -570,3 +571,19 @@
SA(0, sizeof(might_use_tail_padding) == 80);
}
} // namespace PR16537
+
+namespace PR37275 {
+ struct X { char c; };
+
+ struct A { int n; };
+ _Static_assert(_Alignof(A) == _Alignof(int), "");
+
+ struct __attribute__((packed)) B : X, A {};
+#if defined(CLANG_ABI_COMPAT) && CLANG_ABI_COMPAT <= 6
+ _Static_assert(_Alignof(B) == 1, "");
+ _Static_assert(__builtin_offsetof(B, n) == 1, "");
+#else
+ _Static_assert(_Alignof(B) == _Alignof(int), "");
+ _Static_assert(__builtin_offsetof(B, n) == 4, "");
+#endif
+}
Index: test/CodeGenCXX/alignment.cpp
===
--- test/CodeGenCXX/alignment.cpp
+++ test/CodeGenCXX/alignment.cpp
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 %s -emit-llvm -o - -triple=x86_64-apple-darwin10 | FileCheck %s
+// RUN: %clang_cc1 %s -emit-llvm -o - -triple=x86_64-apple-darwin10 | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-NOCOMPAT
+// RUN: %clang_cc1 %s -emit-llvm -o - -triple=x86_64-apple-darwin10 -fclang-abi-compat=6.0 | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-V6COMPAT
extern int int_source();
extern void int_sink(int x);
@@ -54,19 +55,22 @@
// CHECK: [[B_P:%.*]] = bitcast i8* [[T1]] to [[B]]*
// CHECK: [[FIELD_P:%.*]] = bitcast [[B]]* [[B_P]] to i8*
// CHECK: [[TRUNC:%.*]] = trunc i32 [[CALL]] to i8
-// CHECK: [[OLD_VALUE:%.*]] = load i8, i8* [[FIELD_P]], align 2
+// CHECK-V6COMPAT: [[OLD_VALUE:%.*]] = load i8, i8* [[FIELD_P]], align 2
+// CHECK-NOCOMPAT: [[OLD_VALUE:%.*]] = load i8, i8* [[FIELD_P]], align 4
// CHECK: [[T0:%.*]] = and i8 [[TRUNC]], 3
// CHECK: [[T1:%.*]] = and i8 [[OLD_VALUE]], -4
// CHECK: [[T2:%.*]] = or i8 [[T1]], [[T0]]
-// CHECK: store i8 [[T2]], i8* [[FIELD_P]], align 2
+// CHECK-V6COMPAT: store i8 [[T2]], i8* [[FIELD_P]], align 2
+// CHECK-NOCOMPAT: store i8 [[T2]], i8* [[FIELD_P]], align 4
c.onebit = int_source();
// CHECK: [[C_P:%.*]] = load [[C]]*, [[C]]**
// CHECK: [[T0:%.*]] = bitcast [[C]]* [[C_P]] to i8*
// CHECK: [[T1:%.*]] = getelementptr inbounds i8, i8* [[T0]], i64 8
// CHECK: [[B_P:%.*]] = bitcast i8* [[T1]] to [[B]]*
// CHECK: [[FIELD_P:%.*]] = bitcast [[B]]* [[B_P]] to i8*
-// CHECK: [[VALUE:%.*]] = load i8, i8* [[FIELD_P]], align 2
+// CHECK-V6COMPAT: [[VALUE:%.*]] = load i8, i8* [[FIELD_P]], align 2
+// CHECK-NOCOMPAT: [[VALUE:%.*]] = load i8, i8* [[FIELD_P]], align 4
// CHECK: [[T0:%.*]] = shl i8 [[VALUE]], 6
// CHECK: [[T1:%.*]] = ashr i8 [[T0]], 6
// CHECK: [[T2:%.*]] = sext i8 [[T1]] to i32
@@ -83,19 +87,22 @@
// CHECK: [[B_P:%.*]] = bitcast i8* [[T1]] to [[B]]*
// CHECK: [[FIELD_P:%.*]] = bitcast [[B]]* [[B_P]] to i8*
// CHECK: [[TRUNC:%.*]] = trunc i32 [[CALL]] to i8
-// CHECK: [[OLD_VALUE:%.*]] = load i8, i8* [[FIELD_P]], align 2
+// CHECK-V6COMPAT: [[OLD_VALUE:%.*]] = load i8, i8* [[FIELD_P]], align 2
+// CHECK-NOCOMPAT: [[OLD_VALUE:%.*]] = load i8, i8* [[FIELD_P]], align 4
// CHECK: [[T0:%.*]] = and i8 [[TRUNC]], 3
// CHECK: [[T1:%.*]] = and i8 [[OLD_VALUE]], -4
// CHECK: [[T2:%.*]] = or i8 [[T1]], [[T0]]
-// CHECK: store i8 [[T2]], i8* [[FIELD_P]], align 2
+// CHECK-V6COMPAT: store i8 [[T2]], i8* [[FIELD_P]], align 2
+// CHECK-NOCOMPAT: store i8 [[T2]], i8* [[FIELD_P]], align 4
c->onebit = int_source();
// CHECK: [[C_P:%.*]] = load [[C:%.*]]*, [[C]]**
// CHECK: [[T0:%.*]] = bitcast [[C]]* [[C_P]] to i8*
// CHECK: [[T1:%.*]] = getelementptr inbounds i8, i8* [[T0]], i64 8
// CHECK: [[B_P:%.*]] = bitcast i8* [[T1]] to [[B:%.*]]*
// CHECK: [[FIELD_P:%.*]] = bitcast [[B]]* [[B_P]] to i8*
-// CHECK: [[VALUE:%.*]] = load i8, i8* [[FIELD_P]], align 2
+// CHECK-V6COMPAT: [[VALUE:%.*]] = load i8, i8* [[FIELD_P]], align 2
+// CHECK-NOCOMPAT: [[VALUE:%.*]] = load i8, i8* [[FIELD_P]], align 4
// CHECK: [[T0:%.*]] = shl i8 [[VALUE]], 6
// CHECK: [[T1:%.*]] = ashr i8 [[T0]], 6
// CHECK: [[T2:%.*]] = sext i8 [[T1]] to i32
@@
[PATCH] D46190: For an ODR declaration, set the 'Used' bit on its associated declarations.
rsmith requested changes to this revision. rsmith added a comment. This revision now requires changes to proceed. No, absolutely not. This would be insanely expensive, and marks entirely unrelated things "used". The right thing to do here is to consistently track both the named declaration (the one found by name lookup, which might be a using-declaration) and the ultimately selected declaration (which might be the target of the using-declaration, or if that declaration is a template, a specialization of that template), and pass both to `MarkAnyDeclUsed`. Then we should mark the named declaration as `Referenced` and the selected declaration as `Used`. (The "unused using-declaration" warning should be based on the `Referenced` bit, since it's not meaningful to odr-use a using-declaration.) Repository: rC Clang https://reviews.llvm.org/D46190 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r331056 - [docs] add -ffp-cast-overflow-workaround to the release notes
Missing dash corrected at r331057. I can improve the doc wording, but let's settle on the flag name first, and I'll try to get it all fixed up in one shot. So far we have these candidates: 1. -ffp-cast-overflow-workaround 2. -fstrict-fp-trunc-semantics 3. -fstrict-fp-cast-overflow I don't have a strong opinion here, but on 2nd reading, it does seem like a 'strict' flag fits better with existing options. On Fri, Apr 27, 2018 at 4:41 PM, Richard Smith wrote: > On 27 April 2018 at 09:21, Sanjay Patel via cfe-commits < > cfe-commits@lists.llvm.org> wrote: > >> Author: spatel >> Date: Fri Apr 27 09:21:22 2018 >> New Revision: 331056 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=331056&view=rev >> Log: >> [docs] add -ffp-cast-overflow-workaround to the release notes >> >> This option was added with: >> D46135 >> rL331041 >> ...copying the text from UsersManual.rst for more exposure. >> >> >> Modified: >> cfe/trunk/docs/ReleaseNotes.rst >> >> Modified: cfe/trunk/docs/ReleaseNotes.rst >> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ReleaseNo >> tes.rst?rev=331056&r1=331055&r2=331056&view=diff >> >> == >> --- cfe/trunk/docs/ReleaseNotes.rst (original) >> +++ cfe/trunk/docs/ReleaseNotes.rst Fri Apr 27 09:21:22 2018 >> @@ -83,6 +83,15 @@ Non-comprehensive list of changes in thi >> New Compiler Flags >> -- >> >> +- :option:`-ffp-cast-overflow-workaround` and >> + :option:`-fnofp-cast-overflow-workaround` >> > > Shouldn't this be -fno-fp-cast-overflow-workaround? > > Also, our convention for flags that define undefined behavior is > `-fno-strict-*`, so perhaps this should be `-fno-strict-fp-cast-overflow`? > > >> + enable (disable) a workaround for code that casts floating-point >> values to >> + integers and back to floating-point. If the floating-point value is not >> + representable in the intermediate integer type, the code is incorrect >> + according to the language standard. > > > I find this hard to read: I initially misread "the code is incorrect > according to the language standard" as meaning "Clang will generate code > that is incorrect according to the language standard". I think what you > mean here is "the code has undefined behavior according to the language > standard, and Clang will not guarantee any particular result. This flag > causes the behavior to be defined to match the overflowing behavior of the > target's native float-to-int conversion instructions." > > >> This flag will attempt to generate code >> + as if the result of an overflowing conversion matches the overflowing >> behavior >> + of a target's native float-to-int conversion instructions. >> + >> - ... >> >> Deprecated Compiler Flags >> >> >> ___ >> cfe-commits mailing list >> cfe-commits@lists.llvm.org >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >> > > ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46056: Move _LIBCPP_ENABLE_CXX17_REMOVED_UNEXPECTED_FUNCTIONS macro to build system
rsmith accepted this revision. rsmith added a comment. This revision is now accepted and ready to land. This looks right to me. Repository: rCXXA libc++abi https://reviews.llvm.org/D46056 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46165: [Modules] Handle ObjC/C ODR-like semantics for EnumConstantDecl
rsmith accepted this revision. rsmith added inline comments. This revision is now accepted and ready to land. Comment at: lib/Serialization/ASTReaderDecl.cpp:2568-2570 // ODR-based merging is only performed in C++. In C, identically-named things // in different translation units are not redeclarations (but may still have + // compatible types). However, clang is able to merge tag types with ODR-like I think this comment would benefit from more rewriting, since we've basically decided that we do want to apply the ODR merging rule to C. Repository: rC Clang https://reviews.llvm.org/D46165 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46148: [CUDA] Added -f[no-]cuda-short-ptr option
tra updated this revision to Diff 144419.
tra added a subscriber: arsenm.
tra added a comment.
Do not use subtarget feature to control codegen of short pointers.
Instead we need to add a field to TargetOptions which is the only way to pass
this info to NVPTXTargetInfo constructor where we calculate DataLayout.
https://reviews.llvm.org/D46148
Files:
clang/include/clang/Basic/TargetOptions.h
clang/include/clang/Driver/Options.td
clang/lib/Basic/Targets/NVPTX.cpp
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Driver/ToolChains/Cuda.cpp
clang/lib/Frontend/CompilerInvocation.cpp
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -2910,6 +2910,8 @@
Opts.Triple = llvm::sys::getDefaultTargetTriple();
Opts.OpenCLExtensionsAsWritten = Args.getAllArgValues(OPT_cl_ext_EQ);
Opts.ForceEnableInt128 = Args.hasArg(OPT_fforce_enable_int128);
+ Opts.NVPTXUseShortPointers = Args.hasFlag(
+ options::OPT_fcuda_short_ptr, options::OPT_fno_cuda_short_ptr, false);
}
bool CompilerInvocation::CreateFromArgs(CompilerInvocation &Res,
Index: clang/lib/Driver/ToolChains/Cuda.cpp
===
--- clang/lib/Driver/ToolChains/Cuda.cpp
+++ clang/lib/Driver/ToolChains/Cuda.cpp
@@ -635,8 +635,10 @@
// CUDA-9.0 uses new instructions that are only available in PTX6.0+
PtxFeature = "+ptx60";
}
- CC1Args.push_back("-target-feature");
- CC1Args.push_back(PtxFeature);
+ CC1Args.append({"-target-feature", PtxFeature});
+ if (DriverArgs.hasFlag(options::OPT_fcuda_short_ptr,
+ options::OPT_fno_cuda_short_ptr, false))
+CC1Args.append({"-mllvm", "--nvptx-short-ptr"});
if (DeviceOffloadingKind == Action::OFK_OpenMP) {
SmallVector LibraryPaths;
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4697,6 +4697,9 @@
if (Args.hasFlag(options::OPT_fcuda_rdc, options::OPT_fno_cuda_rdc, false))
CmdArgs.push_back("-fcuda-rdc");
+if (Args.hasFlag(options::OPT_fcuda_short_ptr,
+ options::OPT_fno_cuda_short_ptr, false))
+ CmdArgs.push_back("-fcuda-short-ptr");
}
// OpenMP offloading device jobs take the argument -fopenmp-host-ir-file-path
Index: clang/lib/Basic/Targets/NVPTX.cpp
===
--- clang/lib/Basic/Targets/NVPTX.cpp
+++ clang/lib/Basic/Targets/NVPTX.cpp
@@ -68,6 +68,9 @@
if (TargetPointerWidth == 32)
resetDataLayout("e-p:32:32-i64:64-i128:128-v16:16-v32:32-n16:32:64");
+ else if (Opts.NVPTXUseShortPointers)
+resetDataLayout(
+
"e-p3:32:32-p4:32:32-p5:32:32-i64:64-i128:128-v16:16-v32:32-n16:32:64");
else
resetDataLayout("e-i64:64-i128:128-v16:16-v32:32-n16:32:64");
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -579,6 +579,9 @@
def fcuda_rdc : Flag<["-"], "fcuda-rdc">, Flags<[CC1Option]>,
HelpText<"Generate relocatable device code, also known as separate
compilation mode.">;
def fno_cuda_rdc : Flag<["-"], "fno-cuda-rdc">;
+def fcuda_short_ptr : Flag<["-"], "fcuda-short-ptr">, Flags<[CC1Option]>,
+ HelpText<"Use 32-bit pointers for accessing const/local/shared address
spaces.">;
+def fno_cuda_short_ptr : Flag<["-"], "fno-cuda-short-ptr">;
def dA : Flag<["-"], "dA">, Group;
def dD : Flag<["-"], "dD">, Group, Flags<[CC1Option]>,
HelpText<"Print macro definitions in -E mode in addition to normal output">;
Index: clang/include/clang/Basic/TargetOptions.h
===
--- clang/include/clang/Basic/TargetOptions.h
+++ clang/include/clang/Basic/TargetOptions.h
@@ -63,6 +63,10 @@
/// \brief If given, enables support for __int128_t and __uint128_t types.
bool ForceEnableInt128 = false;
+
+ /// \brief If enabled, use 32-bit pointers for accessing const/local/shared
+ /// address space.
+ bool NVPTXUseShortPointers = false;
};
} // end namespace clang
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -2910,6 +2910,8 @@
Opts.Triple = llvm::sys::getDefaultTargetTriple();
Opts.OpenCLExtensionsAsWritten = Args.getAllArgValues(OPT_cl_ext_EQ);
Opts.ForceEnableInt128 = Args.hasArg(OPT_fforce_enable_int128);
+ Opts.NVPTXUseShortPointers = Args.hasFlag(
+ options::OPT_fcuda_short_ptr, options::OPT_fno_cuda_short_ptr, false);
}
bool CompilerInvocation::CreateFromArgs(Compil
Re: r331056 - [docs] add -ffp-cast-overflow-workaround to the release notes
On 27 April 2018 at 16:07, Sanjay Patel via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Missing dash corrected at r331057. I can improve the doc wording, but > let's settle on the flag name first, and I'll try to get it all fixed up in > one shot. > > So far we have these candidates: > 1. -ffp-cast-overflow-workaround > 2. -fstrict-fp-trunc-semantics > 3. -fstrict-fp-cast-overflow > > I don't have a strong opinion here, but on 2nd reading, it does seem like > a 'strict' flag fits better with existing options. > The corresponding UBSan check is called -fsanitize=float-cast-overflow, so maybe -fno-strict-float-cast-overflow would be the most consistent name? > On Fri, Apr 27, 2018 at 4:41 PM, Richard Smith > wrote: > >> On 27 April 2018 at 09:21, Sanjay Patel via cfe-commits < >> cfe-commits@lists.llvm.org> wrote: >> >>> Author: spatel >>> Date: Fri Apr 27 09:21:22 2018 >>> New Revision: 331056 >>> >>> URL: http://llvm.org/viewvc/llvm-project?rev=331056&view=rev >>> Log: >>> [docs] add -ffp-cast-overflow-workaround to the release notes >>> >>> This option was added with: >>> D46135 >>> rL331041 >>> ...copying the text from UsersManual.rst for more exposure. >>> >>> >>> Modified: >>> cfe/trunk/docs/ReleaseNotes.rst >>> >>> Modified: cfe/trunk/docs/ReleaseNotes.rst >>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ReleaseNo >>> tes.rst?rev=331056&r1=331055&r2=331056&view=diff >>> >>> == >>> --- cfe/trunk/docs/ReleaseNotes.rst (original) >>> +++ cfe/trunk/docs/ReleaseNotes.rst Fri Apr 27 09:21:22 2018 >>> @@ -83,6 +83,15 @@ Non-comprehensive list of changes in thi >>> New Compiler Flags >>> -- >>> >>> +- :option:`-ffp-cast-overflow-workaround` and >>> + :option:`-fnofp-cast-overflow-workaround` >>> >> >> Shouldn't this be -fno-fp-cast-overflow-workaround? >> >> Also, our convention for flags that define undefined behavior is >> `-fno-strict-*`, so perhaps this should be `-fno-strict-fp-cast-overflow` >> ? >> >> >>> + enable (disable) a workaround for code that casts floating-point >>> values to >>> + integers and back to floating-point. If the floating-point value is >>> not >>> + representable in the intermediate integer type, the code is incorrect >>> + according to the language standard. >> >> >> I find this hard to read: I initially misread "the code is incorrect >> according to the language standard" as meaning "Clang will generate code >> that is incorrect according to the language standard". I think what you >> mean here is "the code has undefined behavior according to the language >> standard, and Clang will not guarantee any particular result. This flag >> causes the behavior to be defined to match the overflowing behavior of the >> target's native float-to-int conversion instructions." >> >> >>> This flag will attempt to generate code >>> + as if the result of an overflowing conversion matches the overflowing >>> behavior >>> + of a target's native float-to-int conversion instructions. >>> + >>> - ... >>> >>> Deprecated Compiler Flags >>> >>> >>> ___ >>> cfe-commits mailing list >>> cfe-commits@lists.llvm.org >>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >>> >> >> > > ___ > cfe-commits mailing list > cfe-commits@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits > > ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
