[PATCH] D45686: [Driver] Clean up tmp files when deleting Compilation objects
dstenb updated this revision to Diff 148791.
dstenb added a comment.
Update patch to include the clang-tools-extra test case originally added in
https://reviews.llvm.org/D47251.
https://reviews.llvm.org/D45686
Files:
cfe/trunk/include/clang/Driver/Compilation.h
cfe/trunk/lib/Driver/Compilation.cpp
cfe/trunk/lib/Driver/Driver.cpp
clang-tools-extra/trunk/test/clang-tidy/pr37091.cpp
Index: clang-tools-extra/trunk/test/clang-tidy/pr37091.cpp
===
--- clang-tools-extra/trunk/test/clang-tidy/pr37091.cpp
+++ clang-tools-extra/trunk/test/clang-tidy/pr37091.cpp
@@ -0,0 +1,10 @@
+// REQUIRES: shell
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+
+// This is a reproducer for PR37091.
+//
+// Verify that no temporary files are left behind by the clang-tidy invocation.
+
+// RUN: env TMPDIR=%t TEMP=%t TMP=%t clang-tidy %s -- --target=mips64
+// RUN: rmdir %t
Index: cfe/trunk/lib/Driver/Driver.cpp
===
--- cfe/trunk/lib/Driver/Driver.cpp
+++ cfe/trunk/lib/Driver/Driver.cpp
@@ -1243,9 +1243,6 @@
// If any of the preprocessing commands failed, clean up and exit.
if (!FailingCommands.empty()) {
-if (!isSaveTempsEnabled())
- C.CleanupFileList(C.getTempFiles(), true);
-
Diag(clang::diag::note_drv_command_failed_diag_msg)
<< "Error generating preprocessed source(s).";
return;
@@ -1362,9 +1359,6 @@
C.ExecuteJobs(C.getJobs(), FailingCommands);
- // Remove temp files.
- C.CleanupFileList(C.getTempFiles());
-
// If the command succeeded, we are done.
if (FailingCommands.empty())
return 0;
Index: cfe/trunk/lib/Driver/Compilation.cpp
===
--- cfe/trunk/lib/Driver/Compilation.cpp
+++ cfe/trunk/lib/Driver/Compilation.cpp
@@ -45,6 +45,11 @@
}
Compilation::~Compilation() {
+ // Remove temporary files. This must be done before arguments are freed, as
+ // the file names might be derived from the input arguments.
+ if (!TheDriver.isSaveTempsEnabled() && !ForceKeepTempFiles)
+CleanupFileList(TempFiles);
+
delete TranslatedArgs;
delete Args;
@@ -245,6 +250,10 @@
AllActions.clear();
Jobs.clear();
+ // Remove temporary files.
+ if (!TheDriver.isSaveTempsEnabled() && !ForceKeepTempFiles)
+CleanupFileList(TempFiles);
+
// Clear temporary/results file lists.
TempFiles.clear();
ResultFiles.clear();
@@ -262,6 +271,9 @@
// Redirect stdout/stderr to /dev/null.
Redirects = {None, {""}, {""}};
+
+ // Temporary files added by diagnostics should be kept.
+ ForceKeepTempFiles = true;
}
StringRef Compilation::getSysRoot() const {
Index: cfe/trunk/include/clang/Driver/Compilation.h
===
--- cfe/trunk/include/clang/Driver/Compilation.h
+++ cfe/trunk/include/clang/Driver/Compilation.h
@@ -122,6 +122,9 @@
/// Whether an error during the parsing of the input args.
bool ContainsError;
+ /// Whether to keep temporary files regardless of -save-temps.
+ bool ForceKeepTempFiles = false;
+
public:
Compilation(const Driver &D, const ToolChain &DefaultToolChain,
llvm::opt::InputArgList *Args,
Index: clang-tools-extra/trunk/test/clang-tidy/pr37091.cpp
===
--- clang-tools-extra/trunk/test/clang-tidy/pr37091.cpp
+++ clang-tools-extra/trunk/test/clang-tidy/pr37091.cpp
@@ -0,0 +1,10 @@
+// REQUIRES: shell
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+
+// This is a reproducer for PR37091.
+//
+// Verify that no temporary files are left behind by the clang-tidy invocation.
+
+// RUN: env TMPDIR=%t TEMP=%t TMP=%t clang-tidy %s -- --target=mips64
+// RUN: rmdir %t
Index: cfe/trunk/lib/Driver/Driver.cpp
===
--- cfe/trunk/lib/Driver/Driver.cpp
+++ cfe/trunk/lib/Driver/Driver.cpp
@@ -1243,9 +1243,6 @@
// If any of the preprocessing commands failed, clean up and exit.
if (!FailingCommands.empty()) {
-if (!isSaveTempsEnabled())
- C.CleanupFileList(C.getTempFiles(), true);
-
Diag(clang::diag::note_drv_command_failed_diag_msg)
<< "Error generating preprocessed source(s).";
return;
@@ -1362,9 +1359,6 @@
C.ExecuteJobs(C.getJobs(), FailingCommands);
- // Remove temp files.
- C.CleanupFileList(C.getTempFiles());
-
// If the command succeeded, we are done.
if (FailingCommands.empty())
return 0;
Index: cfe/trunk/lib/Driver/Compilation.cpp
===
--- cfe/trunk/lib/Driver/Compilation.cpp
+++ cfe/trunk/lib/Driver/Compilation.cpp
@@ -45,6 +45,11 @@
}
Compilation::~Compilation() {
+ // Remove temporary files. This must be done before arguments are freed, as
+ // the file names might be derived from the input arguments.
+ if (!TheDriver.isSaveTempsEnabled
[PATCH] D47251: Add a lit reproducer for PR37091
dstenb abandoned this revision. dstenb added a comment. Merged into https://reviews.llvm.org/D45686. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D47251 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47445: [ASTImporter] Corrected diagnostic client handling in tests.
balazske created this revision.
Herald added subscribers: cfe-commits, martong.
Herald added a reviewer: a.sidorin.
ASTImporter tests may produce source file related warnings, the diagnostic
client should be in correct state to handle it. Added 'beginSourceFile' to set
the client state.
Repository:
rC Clang
https://reviews.llvm.org/D47445
Files:
include/clang/Frontend/ASTUnit.h
lib/Frontend/ASTUnit.cpp
unittests/AST/ASTImporterTest.cpp
Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -98,6 +98,9 @@
ASTContext &FromCtx = FromAST->getASTContext(),
&ToCtx = ToAST->getASTContext();
+ FromAST->beginSourceFile();
+ ToAST->beginSourceFile();
+
ASTImporter Importer(ToCtx, ToAST->getFileManager(),
FromCtx, FromAST->getFileManager(), false);
@@ -172,7 +175,9 @@
: Code(Code), FileName(FileName),
Unit(tooling::buildASTFromCodeWithArgs(this->Code, Args,
this->FileName)),
- TUDecl(Unit->getASTContext().getTranslationUnitDecl()) {}
+ TUDecl(Unit->getASTContext().getTranslationUnitDecl()) {
+ Unit->beginSourceFile();
+}
};
// We may have several From contexts and related translation units. In each
@@ -214,6 +219,7 @@
ToCode = ToSrcCode;
assert(!ToAST);
ToAST = tooling::buildASTFromCodeWithArgs(ToCode, ToArgs, OutputFileName);
+ToAST->beginSourceFile();
ASTContext &FromCtx = FromTU.Unit->getASTContext(),
&ToCtx = ToAST->getASTContext();
@@ -261,6 +267,7 @@
ToCode = ToSrcCode;
assert(!ToAST);
ToAST = tooling::buildASTFromCodeWithArgs(ToCode, ToArgs, OutputFileName);
+ToAST->beginSourceFile();
return ToAST->getASTContext().getTranslationUnitDecl();
}
@@ -274,6 +281,7 @@
// Build the AST from an empty file.
ToAST =
tooling::buildASTFromCodeWithArgs(/*Code=*/"", ToArgs, "empty.cc");
+ ToAST->beginSourceFile();
}
// Create a virtual file in the To Ctx which corresponds to the file from
Index: lib/Frontend/ASTUnit.cpp
===
--- lib/Frontend/ASTUnit.cpp
+++ lib/Frontend/ASTUnit.cpp
@@ -274,6 +274,12 @@
this->PP = std::move(PP);
}
+void ASTUnit::beginSourceFile() {
+ assert(getDiagnostics().getClient() && PP && Ctx &&
+ "Bad context for source file");
+ getDiagnostics().getClient()->BeginSourceFile(Ctx->getLangOpts(), PP.get());
+}
+
/// Determine the set of code-completion contexts in which this
/// declaration should be shown.
static unsigned getDeclShowContexts(const NamedDecl *ND,
Index: include/clang/Frontend/ASTUnit.h
===
--- include/clang/Frontend/ASTUnit.h
+++ include/clang/Frontend/ASTUnit.h
@@ -438,6 +438,8 @@
void setASTContext(ASTContext *ctx) { Ctx = ctx; }
void setPreprocessor(std::shared_ptr pp);
+ void beginSourceFile();
+
bool hasSema() const { return (bool)TheSema; }
Sema &getSema() const {
Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -98,6 +98,9 @@
ASTContext &FromCtx = FromAST->getASTContext(),
&ToCtx = ToAST->getASTContext();
+ FromAST->beginSourceFile();
+ ToAST->beginSourceFile();
+
ASTImporter Importer(ToCtx, ToAST->getFileManager(),
FromCtx, FromAST->getFileManager(), false);
@@ -172,7 +175,9 @@
: Code(Code), FileName(FileName),
Unit(tooling::buildASTFromCodeWithArgs(this->Code, Args,
this->FileName)),
- TUDecl(Unit->getASTContext().getTranslationUnitDecl()) {}
+ TUDecl(Unit->getASTContext().getTranslationUnitDecl()) {
+ Unit->beginSourceFile();
+}
};
// We may have several From contexts and related translation units. In each
@@ -214,6 +219,7 @@
ToCode = ToSrcCode;
assert(!ToAST);
ToAST = tooling::buildASTFromCodeWithArgs(ToCode, ToArgs, OutputFileName);
+ToAST->beginSourceFile();
ASTContext &FromCtx = FromTU.Unit->getASTContext(),
&ToCtx = ToAST->getASTContext();
@@ -261,6 +267,7 @@
ToCode = ToSrcCode;
assert(!ToAST);
ToAST = tooling::buildASTFromCodeWithArgs(ToCode, ToArgs, OutputFileName);
+ToAST->beginSourceFile();
return ToAST->getASTContext().getTranslationUnitDecl();
}
@@ -274,6 +281,7 @@
// Build the AST from an empty file.
ToAST =
tooling::buildASTFromCodeWithArgs(/*Code=*/"", ToArgs, "empty.cc");
+ ToAST->beginSourceFile();
}
// Create a virtual file in the To Ctx which corresponds to the file from
Index: lib/Frontend/ASTUnit.cpp
[PATCH] D47417: [analyzer] Add missing state transition in IteratorChecker
MTC added inline comments. Comment at: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp:399 + + C.addTransition(State); } NoQ wrote: > MTC wrote: > > I have two questions may need @NoQ or @xazax.hun who is more familiar with > > the analyzer engine help to answer. > > > > - `State` may not change at all, do we need a check here like [[ > > https://github.com/llvm-mirror/clang/blob/master/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp#L227 > > | if (state != originalState) ]] > > - A more basic problem is that do we need `originalState = State` trick. > > It seems that `addTransitionImpl()` has a check about same state > > transition, see [[ > > https://github.com/llvm-mirror/clang/blob/master/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h#L339 > > | addTransitionImp() ]]. > > > > Thanks in advance! > > > > > > > > It seems that `addTransitionImpl()` has a check about same state > > transition, see `addTransitionImp()`. > > Yep, you pretty much answered your question. The check in the checker code is > unnecessary. Thanks, NoQ! It seems that `if (state != originalState)` in some checkers is misleading and may need to be cleaned up. Repository: rC Clang https://reviews.llvm.org/D47417 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r333369 - [clangd] Workaround the comments crash, reenable the test.
Author: ibiryukov
Date: Mon May 28 02:54:51 2018
New Revision: 69
URL: http://llvm.org/viewvc/llvm-project?rev=69&view=rev
Log:
[clangd] Workaround the comments crash, reenable the test.
This fix is still quite fragile, the underlying problem is that the
code should not rely on source ranges coming from the preamble to be
correct when reading from the text buffers.
This is probably not possible to achieve in practice, so we would
probably have to keep the contents of old headers around for the
lifetime of the preamble.
Modified:
clang-tools-extra/trunk/clangd/CodeCompletionStrings.cpp
clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
Modified: clang-tools-extra/trunk/clangd/CodeCompletionStrings.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeCompletionStrings.cpp?rev=69&r1=68&r2=69&view=diff
==
--- clang-tools-extra/trunk/clangd/CodeCompletionStrings.cpp (original)
+++ clang-tools-extra/trunk/clangd/CodeCompletionStrings.cpp Mon May 28
02:54:51 2018
@@ -9,6 +9,7 @@
#include "CodeCompletionStrings.h"
#include "clang/AST/ASTContext.h"
+#include "clang/AST/DeclObjC.h"
#include "clang/AST/RawCommentList.h"
#include "clang/Basic/SourceManager.h"
#include
@@ -123,17 +124,32 @@ void processSnippetChunks(const CodeComp
}
}
-std::string getFormattedComment(const ASTContext &Ctx, const RawComment &RC,
-bool CommentsFromHeaders) {
+bool canRequestComment(const ASTContext &Ctx, const NamedDecl &D,
+ bool CommentsFromHeaders) {
+ if (CommentsFromHeaders)
+return true;
auto &SourceMgr = Ctx.getSourceManager();
- // Parsing comments from invalid preamble can lead to crashes. So we only
- // return comments from the main file when doing code completion. For
- // indexing, we still read all the comments.
+ // Accessing comments for decls from invalid preamble can lead to crashes.
+ // So we only return comments from the main file when doing code completion.
+ // For indexing, we still read all the comments.
// FIXME: find a better fix, e.g. store file contents in the preamble or get
// doc comments from the index.
- if (!CommentsFromHeaders && !SourceMgr.isWrittenInMainFile(RC.getLocStart()))
-return "";
- return RC.getFormattedText(Ctx.getSourceManager(), Ctx.getDiagnostics());
+ auto canRequestForDecl = [&](const NamedDecl &D) -> bool {
+for (auto *Redecl : D.redecls()) {
+ auto Loc = SourceMgr.getSpellingLoc(Redecl->getLocation());
+ if (!SourceMgr.isWrittenInMainFile(Loc))
+return false;
+}
+return true;
+ };
+ // First, check the decl itself.
+ if (!canRequestForDecl(D))
+return false;
+ // Completion also returns comments for properties, corresponding to ObjC
+ // methods.
+ const ObjCMethodDecl *M = dyn_cast(&D);
+ const ObjCPropertyDecl *PDecl = M ? M->findPropertyDecl() : nullptr;
+ return !PDecl || canRequestForDecl(*PDecl);
}
} // namespace
@@ -145,26 +161,26 @@ std::string getDocComment(const ASTConte
// get this declaration, so we don't show documentation in that case.
if (Result.Kind != CodeCompletionResult::RK_Declaration)
return "";
- auto Decl = Result.getDeclaration();
- if (!Decl)
+ auto *Decl = Result.getDeclaration();
+ if (!Decl || !canRequestComment(Ctx, *Decl, CommentsFromHeaders))
return "";
const RawComment *RC = getCompletionComment(Ctx, Decl);
if (!RC)
return "";
- return getFormattedComment(Ctx, *RC, CommentsFromHeaders);
+ return RC->getFormattedText(Ctx.getSourceManager(), Ctx.getDiagnostics());
}
std::string
getParameterDocComment(const ASTContext &Ctx,
const CodeCompleteConsumer::OverloadCandidate &Result,
unsigned ArgIndex, bool CommentsFromHeaders) {
- auto Func = Result.getFunction();
- if (!Func)
+ auto *Func = Result.getFunction();
+ if (!Func || !canRequestComment(Ctx, *Func, CommentsFromHeaders))
return "";
const RawComment *RC = getParameterComment(Ctx, Result, ArgIndex);
if (!RC)
return "";
- return getFormattedComment(Ctx, *RC, CommentsFromHeaders);
+ return RC->getFormattedText(Ctx.getSourceManager(), Ctx.getDiagnostics());
}
void getLabelAndInsertText(const CodeCompletionString &CCS, std::string *Label,
Modified: clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp?rev=69&r1=68&r2=69&view=diff
==
--- clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp Mon May 28
02:54:51 2018
@@ -1000,7 +1000,7 @@ TEST(CompletionTest, NoIndexCompletionsI
}
// FIXME: This still crashes under asan. Fix
[PATCH] D45532: [StaticAnalyzer] Checker to find uninitialized fields after a constructor call
Szelethus marked 4 inline comments as done.
Szelethus added a comment.
I decided to mark the inline comments in `isPointerOrReferenceUninit` regarding
the dereferencing loop termination done for now. I left several TODO's in the
function to be fixed later, with many of them depending on one another, so
fixing them all or many at once would probably be the best solution.
I left two conversations "not done" (fatal/nonfatal errors and base classes),
but if I may, do you have any other objections?
Comment at: lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp:223-225
+ ExplodedNode *Node = Context.generateNonFatalErrorNode(Context.getState());
+ if (!Node)
+return;
NoQ wrote:
> Szelethus wrote:
> > NoQ wrote:
> > > I suspect that a fatal error is better here. We don't want the user to
> > > receive duplicate report from other checkers that catch uninitialized
> > > values; just one report is enough.
> > I think that would be a bad idea. For example, this checker shouts so
> > loudly while checking the LLVM project, it would practically halt the
> > analysis of the code, reducing the coverage, which means that checkers
> > other then uninit value checkers would "suffer" from it.
> >
> > However, I also think that having multiple uninit reports for the same
> > object might be good, especially with this checker, as it would be very
> > easy to see where the problem originated from.
> >
> > What do you think?
> Well, i guess that's the reason to not use the checker on LLVM. Regardless of
> fatal/nonfatal warnings, enabling this checker on LLVM regularly would be a
> bad idea because it's unlikely that anybody will be able to fix all the false
> positives to make it usable. And for other projects that don't demonstrate
> many false positives, this shouldn't be a problem.
>
> In order to indicate where the problem originates from, we have our bug
> reporter visitors that try their best to add such info directly to the
> report. In particular, @george.karpenkov's recent `NoStoreFuncVisitor`
> highlights functions in which a variable was //not// initialized but was
> probably expected to be. Not sure if it highlights constructors in its
> current shape, but that's definitely a better way to give this piece of
> information to the user because it doesn't make the user look for a different
> report to understand the current report.
LLVM is a special project in the fact that almost every part of it is so
performance critical, that leaving many fields uninit matters. However, I would
believe that in most projects, only a smaller portion of the code would be like
that.
Suppose that we have a project that also defines a set of ADTs, like an
`std::list`-like container. If that container had a field that would be left
uninit after a ctor call, analysis on every execution path would be halted
which would use an object like that.
My point is, as long as there is no way to tell the analyzer (or the checker)
to ignore certain constructor calls, I think it would be best not to generate a
fatal error.
>Regardless of fatal/nonfatal warnings, enabling this checker on LLVM regularly
>would be a bad idea because it's unlikely that anybody will be able to fix all
>the false positives to make it usable. And for other projects that don't
>demonstrate many false positives, this shouldn't be a problem.
I wouldn't necessarily call them false positives. This checker doesn't look for
bugs, and all reports I checked were correct in the fact that those fields
really were left uninit. They just don't cause any trouble (just yet!).
Comment at: lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp:359-372
+ // Checking bases.
+ const auto *CXXRD = dyn_cast(RD);
+ if (!CXXRD)
+return ContainsUninitField;
+
+ for (const CXXBaseSpecifier BaseSpec : CXXRD->bases()) {
+const auto *Base = BaseSpec.getType()->getAsCXXRecordDecl();
Szelethus wrote:
> NoQ wrote:
> > Szelethus wrote:
> > > NoQ wrote:
> > > > Are there many warnings that will be caused by this but won't be caused
> > > > by conducting the check for the constructor-within-constructor that's
> > > > currently disabled? Not sure - is it even syntactically possible to not
> > > > initialize the base class?
> > > I'm not a 100% sure what you mean. Can you clarify?
> > > >Not sure - is it even syntactically possible to not initialize the base
> > > >class?
> > > If I understand the question correctly, no, as far as I know.
> > You have a check for `isCalledByConstructor()` in order to skip base class
> > constructors but then you check them manually. That's a lot of code, but it
> > seems that the same result could have been achieved by simply skipping the
> > descent into the base class.
> >
> > Same question for class-type fields, actually.
> >Same question for class-type fields, actually.
>
> My problem with that approach is that the same un
[PATCH] D47108: [CodeGenCXX] Add -fforce-emit-vtables
amharc requested changes to this revision.
amharc added inline comments.
This revision now requires changes to proceed.
Comment at: clang/test/CodeGenCXX/vtable-available-externally.cpp:445
+// after the Derived construction.
+// CHECK-FORCE-EMIT-DAG: @_ZTVN6Test187DerivedE = linkonce_odr unnamed_addr
constant {{.*}} @_ZTIN6Test187DerivedE {{.*}} @_ZN6Test184Base3funEv {{.*}}
@_ZN6Test187DerivedD1Ev {{.*}} @_ZN6Test187DerivedD0Ev
+// CHECK-FORCE-EMIT-DAG: define linkonce_odr void @_ZN6Test187DerivedD0Ev
This still uses `_ZN6Test187DerivedD1Ev` instead of `_ZN6Test184BaseD2Ev`,
because the triple you selected is `x86_64-apple-darwin10` and
`-mconstructor-aliases` is off by default on Darwin.
I suggest you either add `-mconstructor-aliases` to the compiler invocation or
change the triple to literally everything else than Darwin (or CUDA).
Preferably the former, as it shows clearly what we want to test here.
Repository:
rL LLVM
https://reviews.llvm.org/D47108
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D35110: [Analyzer] Constraint Manager Negates Difference
baloghadamsoftware updated this revision to Diff 148804.
baloghadamsoftware added a comment.
I still disagree, but I want the review to continue so I did the requested
modifications.
https://reviews.llvm.org/D35110
Files:
lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
lib/StaticAnalyzer/Core/RangedConstraintManager.h
test/Analysis/constraint_manager_negate_difference.c
test/Analysis/ptr-arith.c
Index: test/Analysis/ptr-arith.c
===
--- test/Analysis/ptr-arith.c
+++ test/Analysis/ptr-arith.c
@@ -265,49 +265,26 @@
clang_analyzer_eval((rhs - lhs) > 0); // expected-warning{{TRUE}}
}
-//---
-// False positives
-//---
-
void zero_implies_reversed_equal(int *lhs, int *rhs) {
clang_analyzer_eval((rhs - lhs) == 0); // expected-warning{{UNKNOWN}}
if ((rhs - lhs) == 0) {
-#ifdef ANALYZER_CM_Z3
clang_analyzer_eval(rhs != lhs); // expected-warning{{FALSE}}
clang_analyzer_eval(rhs == lhs); // expected-warning{{TRUE}}
-#else
-clang_analyzer_eval(rhs != lhs); // expected-warning{{UNKNOWN}}
-clang_analyzer_eval(rhs == lhs); // expected-warning{{UNKNOWN}}
-#endif
return;
}
clang_analyzer_eval((rhs - lhs) == 0); // expected-warning{{FALSE}}
-#ifdef ANALYZER_CM_Z3
clang_analyzer_eval(rhs == lhs); // expected-warning{{FALSE}}
clang_analyzer_eval(rhs != lhs); // expected-warning{{TRUE}}
-#else
- clang_analyzer_eval(rhs == lhs); // expected-warning{{UNKNOWN}}
- clang_analyzer_eval(rhs != lhs); // expected-warning{{UNKNOWN}}
-#endif
}
void canonical_equal(int *lhs, int *rhs) {
clang_analyzer_eval(lhs == rhs); // expected-warning{{UNKNOWN}}
if (lhs == rhs) {
-#ifdef ANALYZER_CM_Z3
clang_analyzer_eval(rhs == lhs); // expected-warning{{TRUE}}
-#else
-clang_analyzer_eval(rhs == lhs); // expected-warning{{UNKNOWN}}
-#endif
return;
}
clang_analyzer_eval(lhs == rhs); // expected-warning{{FALSE}}
-
-#ifdef ANALYZER_CM_Z3
clang_analyzer_eval(rhs == lhs); // expected-warning{{FALSE}}
-#else
- clang_analyzer_eval(rhs == lhs); // expected-warning{{UNKNOWN}}
-#endif
}
void compare_element_region_and_base(int *p) {
Index: test/Analysis/constraint_manager_negate_difference.c
===
--- /dev/null
+++ test/Analysis/constraint_manager_negate_difference.c
@@ -0,0 +1,66 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=debug.ExprInspection,core.builtin -analyzer-config aggressive-relational-comparison-simplification=true -verify %s
+
+void clang_analyzer_eval(int);
+
+void exit(int);
+
+#define UINT_MAX (~0U)
+#define INT_MAX (UINT_MAX & (UINT_MAX >> 1))
+
+extern void __assert_fail (__const char *__assertion, __const char *__file,
+unsigned int __line, __const char *__function)
+ __attribute__ ((__noreturn__));
+#define assert(expr) \
+ ((expr) ? (void)(0) : __assert_fail (#expr, __FILE__, __LINE__, __func__))
+
+void assert_in_range(int x) {
+ assert(x <= ((int)INT_MAX / 4));
+ assert(x >= -(((int)INT_MAX) / 4));
+}
+
+void assert_in_range_2(int m, int n) {
+ assert_in_range(m);
+ assert_in_range(n);
+}
+
+void equal(int m, int n) {
+ assert_in_range_2(m, n);
+ if (m != n)
+return;
+ clang_analyzer_eval(n == m); // expected-warning{{TRUE}}
+}
+
+void non_equal(int m, int n) {
+ assert_in_range_2(m, n);
+ if (m == n)
+return;
+ clang_analyzer_eval(n != m); // expected-warning{{TRUE}}
+}
+
+void less_or_equal(int m, int n) {
+ assert_in_range_2(m, n);
+ if (m < n)
+return;
+ clang_analyzer_eval(n <= m); // expected-warning{{TRUE}}
+}
+
+void less(int m, int n) {
+ assert_in_range_2(m, n);
+ if (m <= n)
+return;
+ clang_analyzer_eval(n < m); // expected-warning{{TRUE}}
+}
+
+void greater_or_equal(int m, int n) {
+ assert_in_range_2(m, n);
+ if (m > n)
+return;
+ clang_analyzer_eval(n >= m); // expected-warning{{TRUE}}
+}
+
+void greater(int m, int n) {
+ assert_in_range_2(m, n);
+ if (m >= n)
+return;
+ clang_analyzer_eval(n > m); // expected-warning{{TRUE}}
+}
Index: lib/StaticAnalyzer/Core/RangedConstraintManager.h
===
--- lib/StaticAnalyzer/Core/RangedConstraintManager.h
+++ lib/StaticAnalyzer/Core/RangedConstraintManager.h
@@ -115,6 +115,8 @@
RangeSet Intersect(BasicValueFactory &BV, Factory &F, llvm::APSInt Lower,
llvm::APSInt Upper) const;
+ RangeSet Negate(BasicValueFactory &BV, Factory &F) const;
+
void print(raw_ostream &os) const;
bool operator==(const RangeSet &other) const {
Index: lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
===
--- lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
+++ lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
@@ -174,6 +174,28 @@
return newRanges;
}
+// Turn all [A, B] ranges to [-B, -A]. Turn minimal
[PATCH] D45835: Add new driver mode for dumping compiler options
aaron.ballman marked 3 inline comments as done. aaron.ballman added inline comments. Comment at: test/Frontend/compiler-options-dump.cpp:3 +// RUN: %clang_cc1 -compiler-options-dump -std=c++17 %s -o - | FileCheck %s --check-prefix=CXX17 +// RUN: %clang_cc1 -compiler-options-dump -std=c99 -ffast-math -x c %s -o - | FileCheck %s --check-prefix=C99 + hfinkel wrote: > You don't need -ffast-math here I presume. Correct, I'll drop from the commit (or in the next patch). Comment at: test/Frontend/compiler-options-dump.cpp:15 +// CXX17: "extensions" +// CXX17: "cxx_range_for" : true + hfinkel wrote: > cxx_range_for is both a feature and an extension? Correct. The way we implement __has_feature is to return true only if the -std line supports the feature, and __has_extension will return true if __has_feature returns true or if the extension is enabled. https://reviews.llvm.org/D45835 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r333370 - [clangd] Fix leak sanitizers warnings in clangd
Author: ibiryukov
Date: Mon May 28 05:11:37 2018
New Revision: 70
URL: http://llvm.org/viewvc/llvm-project?rev=70&view=rev
Log:
[clangd] Fix leak sanitizers warnings in clangd
The commit includes two changes:
1. Set DisableFree to false when building the ParsedAST.
This is sane default, since clangd never wants to leak the AST.
2. Make sure CompilerInstance created in code completion is passed to
the FrontendAction::BeginSourceFile call.
We have to do this to make sure the memory buffers of remapped
files are properly freed.
Our tests do not produce any warnings under asan anymore.
The changes are mostly trivial, just moving the code around. So
sending without review.
Modified:
clang-tools-extra/trunk/clangd/ClangdUnit.cpp
clang-tools-extra/trunk/clangd/CodeComplete.cpp
clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
Modified: clang-tools-extra/trunk/clangd/ClangdUnit.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdUnit.cpp?rev=70&r1=69&r2=70&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdUnit.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdUnit.cpp Mon May 28 05:11:37 2018
@@ -153,6 +153,10 @@ ParsedAST::Build(std::unique_ptr Buffer,
std::shared_ptr PCHs,
IntrusiveRefCntPtr VFS) {
+ assert(CI);
+ // Command-line parsing sets DisableFree to true by default, but we don't
want
+ // to leak memory in clangd.
+ CI->getFrontendOpts().DisableFree = false;
const PrecompiledPreamble *PreamblePCH =
Preamble ? &Preamble->Preamble : nullptr;
Modified: clang-tools-extra/trunk/clangd/CodeComplete.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeComplete.cpp?rev=70&r1=69&r2=70&view=diff
==
--- clang-tools-extra/trunk/clangd/CodeComplete.cpp (original)
+++ clang-tools-extra/trunk/clangd/CodeComplete.cpp Mon May 28 05:11:37 2018
@@ -699,26 +699,13 @@ bool semaCodeComplete(std::unique_ptrgetFrontendOpts().DisableFree = false;
+ auto &FrontendOpts = CI->getFrontendOpts();
+ FrontendOpts.DisableFree = false;
+ FrontendOpts.SkipFunctionBodies = true;
CI->getLangOpts()->CommentOpts.ParseAllComments = true;
-
- std::unique_ptr ContentsBuffer =
- llvm::MemoryBuffer::getMemBufferCopy(Input.Contents, Input.FileName);
-
- // The diagnostic options must be set before creating a CompilerInstance.
- CI->getDiagnosticOpts().IgnoreWarnings = true;
- // We reuse the preamble whether it's valid or not. This is a
- // correctness/performance tradeoff: building without a preamble is slow, and
- // completion is latency-sensitive.
- auto Clang = prepareCompilerInstance(
- std::move(CI), Input.Preamble, std::move(ContentsBuffer),
- std::move(Input.PCHs), std::move(Input.VFS), DummyDiagsConsumer);
-
// Disable typo correction in Sema.
- Clang->getLangOpts().SpellChecking = false;
-
- auto &FrontendOpts = Clang->getFrontendOpts();
- FrontendOpts.SkipFunctionBodies = true;
+ CI->getLangOpts()->SpellChecking = false;
+ // Setup code completion.
FrontendOpts.CodeCompleteOpts = Options;
FrontendOpts.CodeCompletionAt.FileName = Input.FileName;
auto Offset = positionToOffset(Input.Contents, Input.Pos);
@@ -731,6 +718,18 @@ bool semaCodeComplete(std::unique_ptr ContentsBuffer =
+ llvm::MemoryBuffer::getMemBufferCopy(Input.Contents, Input.FileName);
+ // The diagnostic options must be set before creating a CompilerInstance.
+ CI->getDiagnosticOpts().IgnoreWarnings = true;
+ // We reuse the preamble whether it's valid or not. This is a
+ // correctness/performance tradeoff: building without a preamble is slow, and
+ // completion is latency-sensitive.
+ // NOTE: we must call BeginSourceFile after prepareCompilerInstance.
Otherwise
+ // the remapped buffers do not get freed.
+ auto Clang = prepareCompilerInstance(
+ std::move(CI), Input.Preamble, std::move(ContentsBuffer),
+ std::move(Input.PCHs), std::move(Input.VFS), DummyDiagsConsumer);
Clang->setCodeCompletionConsumer(Consumer.release());
SyntaxOnlyAction Action;
Modified: clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp?rev=70&r1=69&r2=70&view=diff
==
--- clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp Mon May 28
05:11:37 2018
@@ -18,6 +18,7 @@
#include "TestFS.h"
#include "index/MemIndex.h"
#include "llvm/Support/Error.h"
+#include "llvm/Testing/Support/Error.h"
#include "gmock/gmock.h"
#include "gtest/gtest.h"
@@ -1040,6 +1041,25 @@ TEST(CompletionTest, Documen
[PATCH] D47450: [ASTImporter] Use InjectedClassNameType at import of templated record.
balazske created this revision.
Herald added subscribers: cfe-commits, martong.
Herald added a reviewer: a.sidorin.
At import of a record describing a template set its type to
InjectedClassNameType (instead of RecordType).
Repository:
rC Clang
https://reviews.llvm.org/D47450
Files:
lib/AST/ASTImporter.cpp
test/ASTMerge/injected-class-name-decl-1/Inputs/inject1.cpp
test/ASTMerge/injected-class-name-decl-1/test.cpp
test/ASTMerge/injected-class-name-decl/Inputs/inject1.cpp
test/ASTMerge/injected-class-name-decl/Inputs/inject2.cpp
test/ASTMerge/injected-class-name-decl/test.cpp
Index: test/ASTMerge/injected-class-name-decl/test.cpp
===
--- /dev/null
+++ test/ASTMerge/injected-class-name-decl/test.cpp
@@ -0,0 +1,3 @@
+// RUN: %clang_cc1 -std=c++1z -emit-pch -o %t.ast %S/Inputs/inject1.cpp
+// RUN: %clang_cc1 -std=c++1z -emit-obj -o /dev/null -ast-merge %t.ast %S/Inputs/inject2.cpp
+// expected-no-diagnostics
Index: test/ASTMerge/injected-class-name-decl/Inputs/inject2.cpp
===
--- /dev/null
+++ test/ASTMerge/injected-class-name-decl/Inputs/inject2.cpp
@@ -0,0 +1 @@
+template X C::x;
Index: test/ASTMerge/injected-class-name-decl/Inputs/inject1.cpp
===
--- /dev/null
+++ test/ASTMerge/injected-class-name-decl/Inputs/inject1.cpp
@@ -0,0 +1 @@
+template class C { static X x; };
Index: test/ASTMerge/injected-class-name-decl-1/test.cpp
===
--- /dev/null
+++ test/ASTMerge/injected-class-name-decl-1/test.cpp
@@ -0,0 +1,6 @@
+// Trigger a case when at import of template record and replacing its type
+// with InjectedClassNameType there is a PrevDecl of the record.
+
+// RUN: %clang_cc1 -std=c++1z -emit-pch -o %t.ast %S/Inputs/inject1.cpp
+// RUN: %clang_cc1 -std=c++1z -fsyntax-only -ast-merge %t.ast %s
+// expected-no-diagnostics
Index: test/ASTMerge/injected-class-name-decl-1/Inputs/inject1.cpp
===
--- /dev/null
+++ test/ASTMerge/injected-class-name-decl-1/Inputs/inject1.cpp
@@ -0,0 +1,43 @@
+namespace a {
+template struct b;
+template struct c;
+template using e = d;
+class f;
+} // namespace a
+namespace google {
+namespace protobuf {
+namespace internal {
+class LogMessage {
+ LogMessage &operator<<(const char *);
+};
+} // namespace internal
+} // namespace protobuf
+} // namespace google
+namespace a {
+template class g;
+namespace i {
+struct h;
+template struct F;
+struct G;
+template struct j;
+using k = g;
+template struct l {};
+} // namespace i
+} // namespace a
+namespace a {
+using n = c>;
+template class g : i::F>> {
+ template friend struct i::l;
+};
+} // namespace a
+namespace a {
+using i::G;
+}
+namespace google {
+namespace protobuf {
+namespace internal {
+LogMessage &LogMessage::operator<<(const char *) { return *this; }
+a::f *o;
+} // namespace internal
+} // namespace protobuf
+} // namespace google
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -2128,6 +2128,29 @@
if (!ToDescribed)
return nullptr;
D2CXX->setDescribedClassTemplate(ToDescribed);
+if (!DCXX->isInjectedClassName()) {
+ // In a record describing a template the type should be a
+ // InjectedClassNameType (see Sema::CheckClassTemplate). Update the
+ // previously set type to the correct value here (ToDescribed is not
+ // available at record create).
+ // FIXME: The previous type is cleared but not removed from
+ // ASTContext's internal storage.
+ CXXRecordDecl *Injected = nullptr;
+ for (NamedDecl *Found : D2CXX->noload_lookup(Name)) {
+auto *Record = dyn_cast(Found);
+if (Record && Record->isInjectedClassName()) {
+ Injected = Record;
+ break;
+}
+ }
+ D2CXX->setTypeForDecl(nullptr);
+ Importer.getToContext().getInjectedClassNameType(D2CXX,
+ ToDescribed->getInjectedClassNameSpecialization());
+ if (Injected) {
+Injected->setTypeForDecl(nullptr);
+Importer.getToContext().getTypeDeclType(Injected, D2CXX);
+ }
+}
} else if (MemberSpecializationInfo *MemberInfo =
DCXX->getMemberSpecializationInfo()) {
TemplateSpecializationKind SK =
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r333371 - [clangd] Remove accessors for top-level decls from preamble
Author: ibiryukov
Date: Mon May 28 05:23:17 2018
New Revision: 71
URL: http://llvm.org/viewvc/llvm-project?rev=71&view=rev
Log:
[clangd] Remove accessors for top-level decls from preamble
Summary:
They cause lots of deserialization and are not actually used.
The ParsedAST accessor that previously returned those was renamed from
getTopLevelDecls to getLocalTopLevelDecls in order to avoid
confusion.
This change should considerably improve the speed of findDefinition
and other features that try to find AST nodes, corresponding to the
locations in the source code.
Reviewers: ioeric, sammccall
Reviewed By: sammccall
Subscribers: klimek, mehdi_amini, MaskRay, jkorous, cfe-commits
Differential Revision: https://reviews.llvm.org/D47331
Modified:
clang-tools-extra/trunk/clangd/ClangdUnit.cpp
clang-tools-extra/trunk/clangd/ClangdUnit.h
clang-tools-extra/trunk/clangd/XRefs.cpp
clang-tools-extra/trunk/unittests/clangd/TestTU.cpp
Modified: clang-tools-extra/trunk/clangd/ClangdUnit.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdUnit.cpp?rev=71&r1=70&r2=71&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdUnit.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdUnit.cpp Mon May 28 05:23:17 2018
@@ -90,22 +90,8 @@ public:
CppFilePreambleCallbacks(PathRef File, PreambleParsedCallback ParsedCallback)
: File(File), ParsedCallback(ParsedCallback) {}
- std::vector takeTopLevelDeclIDs() {
-return std::move(TopLevelDeclIDs);
- }
-
std::vector takeInclusions() { return std::move(Inclusions); }
- void AfterPCHEmitted(ASTWriter &Writer) override {
-TopLevelDeclIDs.reserve(TopLevelDecls.size());
-for (Decl *D : TopLevelDecls) {
- // Invalid top-level decls may not have been serialized.
- if (D->isInvalidDecl())
-continue;
- TopLevelDeclIDs.push_back(Writer.getDeclID(D));
-}
- }
-
void AfterExecute(CompilerInstance &CI) override {
if (!ParsedCallback)
return;
@@ -113,14 +99,6 @@ public:
ParsedCallback(File, CI.getASTContext(), CI.getPreprocessorPtr());
}
- void HandleTopLevelDecl(DeclGroupRef DG) override {
-for (Decl *D : DG) {
- if (isa(D))
-continue;
- TopLevelDecls.push_back(D);
-}
- }
-
void BeforeExecute(CompilerInstance &CI) override {
SourceMgr = &CI.getSourceManager();
}
@@ -135,8 +113,6 @@ public:
private:
PathRef File;
PreambleParsedCallback ParsedCallback;
- std::vector TopLevelDecls;
- std::vector TopLevelDeclIDs;
std::vector Inclusions;
SourceManager *SourceMgr = nullptr;
};
@@ -204,28 +180,6 @@ ParsedAST::Build(std::unique_ptr Resolved;
- Resolved.reserve(Preamble->TopLevelDeclIDs.size());
-
- ExternalASTSource &Source = *getASTContext().getExternalSource();
- for (serialization::DeclID TopLevelDecl : Preamble->TopLevelDeclIDs) {
-// Resolve the declaration ID to an actual declaration, possibly
-// deserializing the declaration in the process.
-if (Decl *D = Source.GetExternalDecl(TopLevelDecl))
- Resolved.push_back(D);
- }
-
- TopLevelDecls.reserve(TopLevelDecls.size() +
-Preamble->TopLevelDeclIDs.size());
- TopLevelDecls.insert(TopLevelDecls.begin(), Resolved.begin(),
Resolved.end());
-
- PreambleDeclsDeserialized = true;
-}
-
ParsedAST::ParsedAST(ParsedAST &&Other) = default;
ParsedAST &ParsedAST::operator=(ParsedAST &&Other) = default;
@@ -252,9 +206,8 @@ const Preprocessor &ParsedAST::getPrepro
return Clang->getPreprocessor();
}
-ArrayRef ParsedAST::getTopLevelDecls() {
- ensurePreambleDeclsDeserialized();
- return TopLevelDecls;
+ArrayRef ParsedAST::getLocalTopLevelDecls() {
+ return LocalTopLevelDecls;
}
const std::vector &ParsedAST::getDiagnostics() const { return Diags; }
@@ -264,7 +217,7 @@ std::size_t ParsedAST::getUsedBytes() co
// FIXME(ibiryukov): we do not account for the dynamically allocated part of
// Message and Fixes inside each diagnostic.
return AST.getASTAllocatedMemory() + AST.getSideTableAllocatedMemory() +
- ::getUsedBytes(TopLevelDecls) + ::getUsedBytes(Diags);
+ ::getUsedBytes(LocalTopLevelDecls) + ::getUsedBytes(Diags);
}
const std::vector &ParsedAST::getInclusions() const {
@@ -272,21 +225,19 @@ const std::vector &ParsedAST:
}
PreambleData::PreambleData(PrecompiledPreamble Preamble,
- std::vector TopLevelDeclIDs,
std::vector Diags,
std::vector Inclusions)
-: Preamble(std::move(Preamble)),
- TopLevelDeclIDs(std::move(TopLevelDeclIDs)), Diags(std::move(Diags)),
+: Preamble(std::move(Preamble)), Diags(std::move(Diags)),
Inclusions(std::move(Inclusions)) {}
ParsedAST::ParsedAST(std::shared_ptr Preamble,
std::unique_ptr Clang,
std::unique_ptr Action,
-
[PATCH] D47450: [ASTImporter] Use InjectedClassNameType at import of templated record.
martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. Herald added a subscriber: rnkovacs. LGTM! But let's wait for someone else's review too. @a.sidorin We discovered this error during the CTU analysis of google/protobuf and we could reduce the erroneous C file with c-reduce to the minimal example presented in the test file. Repository: rC Clang https://reviews.llvm.org/D47450 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47331: [clangd] Remove accessors for top-level decls from preamble
This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE71: [clangd] Remove accessors for top-level decls from
preamble (authored by ibiryukov, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D47331?vs=148605&id=148806#toc
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D47331
Files:
clangd/ClangdUnit.cpp
clangd/ClangdUnit.h
clangd/XRefs.cpp
unittests/clangd/TestTU.cpp
Index: unittests/clangd/TestTU.cpp
===
--- unittests/clangd/TestTU.cpp
+++ unittests/clangd/TestTU.cpp
@@ -1,5 +1,4 @@
-//===--- TestTU.cpp - Scratch source files for testing *-
-//C++-*-===//
+//===--- TestTU.cpp - Scratch source files for testing ===//
//
// The LLVM Compiler Infrastructure
//
@@ -73,22 +72,24 @@
}
const NamedDecl &findDecl(ParsedAST &AST, llvm::StringRef QName) {
- const NamedDecl *Result = nullptr;
- for (const Decl *D : AST.getTopLevelDecls()) {
-auto *ND = dyn_cast(D);
-if (!ND || ND->getNameAsString() != QName)
- continue;
-if (Result) {
- ADD_FAILURE() << "Multiple Decls named " << QName;
- assert(false && "QName is not unique");
-}
-Result = ND;
- }
- if (!Result) {
-ADD_FAILURE() << "No Decl named " << QName << " in AST";
-assert(false && "No Decl with QName");
+ llvm::SmallVector Components;
+ QName.split(Components, "::");
+
+ auto &Ctx = AST.getASTContext();
+ auto LookupDecl = [&Ctx](const DeclContext &Scope,
+ llvm::StringRef Name) -> const NamedDecl & {
+auto LookupRes = Scope.lookup(DeclarationName(&Ctx.Idents.get(Name)));
+assert(!LookupRes.empty() && "Lookup failed");
+assert(LookupRes.size() == 1 && "Lookup returned multiple results");
+return *LookupRes.front();
+ };
+
+ const DeclContext *Scope = Ctx.getTranslationUnitDecl();
+ for (auto NameIt = Components.begin(), End = Components.end() - 1;
+ NameIt != End; ++NameIt) {
+Scope = &cast(LookupDecl(*Scope, *NameIt));
}
- return *Result;
+ return LookupDecl(*Scope, Components.back());
}
} // namespace clangd
Index: clangd/ClangdUnit.h
===
--- clangd/ClangdUnit.h
+++ clangd/ClangdUnit.h
@@ -44,12 +44,10 @@
// Stores Preamble and associated data.
struct PreambleData {
- PreambleData(PrecompiledPreamble Preamble,
- std::vector TopLevelDeclIDs,
- std::vector Diags, std::vector Inclusions);
+ PreambleData(PrecompiledPreamble Preamble, std::vector Diags,
+ std::vector Inclusions);
PrecompiledPreamble Preamble;
- std::vector TopLevelDeclIDs;
std::vector Diags;
// Processes like code completions and go-to-definitions will need #include
// information, and their compile action skips preamble range.
@@ -80,17 +78,19 @@
~ParsedAST();
+ /// Note that the returned ast will not contain decls from the preamble that
+ /// were not deserialized during parsing. Clients should expect only decls
+ /// from the main file to be in the AST.
ASTContext &getASTContext();
const ASTContext &getASTContext() const;
Preprocessor &getPreprocessor();
std::shared_ptr getPreprocessorPtr();
const Preprocessor &getPreprocessor() const;
- /// This function returns all top-level decls, including those that come
- /// from Preamble. Decls, coming from Preamble, have to be deserialized, so
- /// this call might be expensive.
- ArrayRef getTopLevelDecls();
+ /// This function returns top-level decls present in the main file of the AST.
+ /// The result does not include the decls that come from the preamble.
+ ArrayRef getLocalTopLevelDecls();
const std::vector &getDiagnostics() const;
@@ -103,11 +103,8 @@
ParsedAST(std::shared_ptr Preamble,
std::unique_ptr Clang,
std::unique_ptr Action,
-std::vector TopLevelDecls, std::vector Diags,
-std::vector Inclusions);
-
-private:
- void ensurePreambleDeclsDeserialized();
+std::vector LocalTopLevelDecls,
+std::vector Diags, std::vector Inclusions);
// In-memory preambles must outlive the AST, it is important that this member
// goes before Clang and Action.
@@ -122,8 +119,9 @@
// Data, stored after parsing.
std::vector Diags;
- std::vector TopLevelDecls;
- bool PreambleDeclsDeserialized;
+ // Top-level decls inside the current file. Not that this does not include
+ // top-level decls from the preamble.
+ std::vector LocalTopLevelDecls;
std::vector Inclusions;
};
Index: clangd/ClangdUnit.cpp
===
--- clangd/ClangdUnit.cpp
+++ clangd/ClangdUnit.cpp
@@ -90,37 +90,15 @@
CppFilePreambleCallbacks(PathRef File, PreambleParsedCallback ParsedCallback)
: File(File), ParsedCallback(ParsedCallback) {}
[PATCH] D47450: [ASTImporter] Use InjectedClassNameType at import of templated record.
martong added a reviewer: r.stahl. martong added a comment. Adding Rafael too as a reviewer, because he has been working also on the ASTImporter recently. Repository: rC Clang https://reviews.llvm.org/D47450 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47445: [ASTImporter] Corrected diagnostic client handling in tests.
martong added reviewers: akyrtzi, ddunbar. martong added a comment. Adding Argyrios and Daniel as a reviewer for ASTUnit related changes. Repository: rC Clang https://reviews.llvm.org/D47445 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47445: [ASTImporter] Corrected diagnostic client handling in tests.
martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. Herald added a subscriber: rnkovacs. LGTM! But let's wait the review of those people who have history in `ASTUnit.cpp`. Repository: rC Clang https://reviews.llvm.org/D47445 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47223: [clangd] Handle enumerators in named, unscoped enums similarly to scoped enums
ilya-biryukov added a comment.
So to keep completion working, maybe we can give add enumerators with full
qualification and have a flag that determines whether the last component of the
qualifier can be ignored?
This would give a simple implementation path for now, and will add a semantic
signal that we need to support enumerators, at least.
For example,
enum En {
A,
};
enum class ScopedEn {
A
};
will produce symbols with
[ { Scope = "En::",
InsideUnscopedEnum=true,
Name="A"},
{ Scope = "ScopedEn::",
InsideUnscopedEnum = false,
Name = "A" } ]
fuzzyFind will have 2 variants:
- For code completion, it will return `En::A` only when queries with `::A`
(e.g. in global scope).
- For workspace symbol, it can return `En::A` for `A` and `En::A`. For `::A` it
can return empty results.
Scoped enums work the same as before. Other case (using decls and inline
namespaces) will also work as before.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D47223
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47445: [ASTImporter] Corrected diagnostic client handling in tests.
a.sidorin accepted this revision. a.sidorin added a comment. Looks good to me, but the approval from AST code owners is required, I think. Repository: rC Clang https://reviews.llvm.org/D47445 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r333373 - [clangd] Remove the outdated comment. NFC
Author: ibiryukov
Date: Mon May 28 05:43:20 2018
New Revision: 73
URL: http://llvm.org/viewvc/llvm-project?rev=73&view=rev
Log:
[clangd] Remove the outdated comment. NFC
Modified:
clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
Modified: clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp?rev=73&r1=72&r2=73&view=diff
==
--- clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp Mon May 28
05:43:20 2018
@@ -1000,7 +1000,6 @@ TEST(CompletionTest, NoIndexCompletionsI
}
}
-// FIXME: This still crashes under asan. Fix it and reenable the test.
TEST(CompletionTest, DocumentationFromChangedFileCrash) {
MockFSProvider FS;
auto FooH = testPath("foo.h");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46476: [HIP] Add action builder for HIP
yaxunl added a comment. ping Any further changes are needed? Thanks. https://reviews.llvm.org/D46476 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47450: [ASTImporter] Use InjectedClassNameType at import of templated record.
r.stahl accepted this revision.
r.stahl added a comment.
LGTM!
I'm not really too confident to approve changes yet, but with a second opinion
it should be fine.
Comment at: lib/AST/ASTImporter.cpp:2139
+ CXXRecordDecl *Injected = nullptr;
+ for (NamedDecl *Found : D2CXX->noload_lookup(Name)) {
+auto *Record = dyn_cast(Found);
The only thing I'm wondering is whether the Decls looked up here are always
known to be already imported.
Repository:
rC Clang
https://reviews.llvm.org/D47450
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44888: [RISCV] Add -mrelax/-mno-relax flags to enable/disable RISCV linker relaxation
asb accepted this revision. asb added a comment. This revision is now accepted and ready to land. Looks good to me, thanks! Repository: rL LLVM https://reviews.llvm.org/D44888 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46823: [analyzer] const init: handle non-explicit cases more accurately
r.stahl updated this revision to Diff 148809.
r.stahl added a comment.
Added FIXME tests.
I know my example didn't exercise your scenario, but it was the only one I
could think of where returning zero would have been straight up wrong. Note
that I default initialized `a` to 3 there, so `sarr` is not just
zero-initialized.
I do not have access yet, but applied today!
https://reviews.llvm.org/D46823
Files:
lib/StaticAnalyzer/Core/RegionStore.cpp
test/Analysis/initialization.c
test/Analysis/initialization.cpp
Index: test/Analysis/initialization.cpp
===
--- test/Analysis/initialization.cpp
+++ test/Analysis/initialization.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -triple i386-apple-darwin10 -analyze -analyzer-checker=core.builtin,debug.ExprInspection -verify %s
+
+void clang_analyzer_eval(int);
+
+struct S {
+ int a = 3;
+};
+S const sarr[2] = {};
+void definit() {
+ int i = 1;
+ // FIXME: Should recognize that it is 3.
+ clang_analyzer_eval(sarr[i].a); // expected-warning{{UNKNOWN}}
+}
+
+int const arr[2][2] = {};
+void arr2init() {
+ int i = 1;
+ // FIXME: Should recognize that it is 0.
+ clang_analyzer_eval(arr[i][0]); // expected-warning{{UNKNOWN}}
+}
Index: test/Analysis/initialization.c
===
--- test/Analysis/initialization.c
+++ test/Analysis/initialization.c
@@ -1,7 +1,28 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core -verify %s
-// expected-no-diagnostics
+// RUN: %clang_cc1 -triple i386-apple-darwin10 -analyze -analyzer-checker=core.builtin,debug.ExprInspection -verify %s
+
+void clang_analyzer_eval(int);
void initbug() {
const union { float a; } u = {};
(void)u.a; // no-crash
}
+
+int const parr[2] = {1};
+void constarr() {
+ int i = 2;
+ clang_analyzer_eval(parr[i]); // expected-warning{{UNDEFINED}}
+ i = 1;
+ clang_analyzer_eval(parr[i] == 0); // expected-warning{{TRUE}}
+ i = -1;
+ clang_analyzer_eval(parr[i]); // expected-warning{{UNDEFINED}}
+}
+
+struct SM {
+ int a;
+ int b;
+};
+const struct SM sm = {.a = 1};
+void multinit() {
+ clang_analyzer_eval(sm.a == 1); // expected-warning{{TRUE}}
+ clang_analyzer_eval(sm.b == 0); // expected-warning{{TRUE}}
+}
Index: lib/StaticAnalyzer/Core/RegionStore.cpp
===
--- lib/StaticAnalyzer/Core/RegionStore.cpp
+++ lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -1638,9 +1638,18 @@
// The array index has to be known.
if (auto CI = R->getIndex().getAs()) {
int64_t i = CI->getValue().getSExtValue();
-// Return unknown value if index is out of bounds.
-if (i < 0 || i >= InitList->getNumInits())
- return UnknownVal();
+// If it is known that the index is out of bounds, we can return
+// an undefined value.
+if (i < 0)
+ return UndefinedVal();
+
+if (auto CAT = Ctx.getAsConstantArrayType(VD->getType()))
+ if (CAT->getSize().sle(i))
+return UndefinedVal();
+
+// If there is a list, but no init, it must be zero.
+if (i >= InitList->getNumInits())
+ return svalBuilder.makeZeroVal(R->getElementType());
if (const Expr *ElemInit = InitList->getInit(i))
if (Optional V = svalBuilder.getConstantVal(ElemInit))
@@ -1715,11 +1724,15 @@
// Either the record variable or the field has to be const qualified.
if (RecordVarTy.isConstQualified() || Ty.isConstQualified())
if (const Expr *Init = VD->getInit())
-if (const auto *InitList = dyn_cast(Init))
- if (Index < InitList->getNumInits())
+if (const auto *InitList = dyn_cast(Init)) {
+ if (Index < InitList->getNumInits()) {
if (const Expr *FieldInit = InitList->getInit(Index))
if (Optional V = svalBuilder.getConstantVal(FieldInit))
return *V;
+ } else {
+return svalBuilder.makeZeroVal(Ty);
+ }
+}
}
return getBindingForFieldOrElementCommon(B, R, Ty);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47450: [ASTImporter] Use InjectedClassNameType at import of templated record.
balazske added inline comments.
Comment at: lib/AST/ASTImporter.cpp:2139
+ CXXRecordDecl *Injected = nullptr;
+ for (NamedDecl *Found : D2CXX->noload_lookup(Name)) {
+auto *Record = dyn_cast(Found);
r.stahl wrote:
> The only thing I'm wondering is whether the Decls looked up here are always
> known to be already imported.
These are in the 'To' context. It may be that the `Injected` is not found here,
probably because not yet imported (in this case the import may be part of a not
completed recursive process).
Repository:
rC Clang
https://reviews.llvm.org/D47450
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47451: [analyzer] Remove the redundant check about same state transition in `ArrayBoundCheckerV2.cpp`.
MTC created this revision. MTC added reviewers: NoQ, xazax.hun, george.karpenkov. Herald added subscribers: cfe-commits, a.sidorin, rnkovacs, szepet. Since the `addTransitionImpl()` has a check about same state transition, there is no need to check it in `ArrayBoundCheckerV2.cpp`. Repository: rC Clang https://reviews.llvm.org/D47451 Files: lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp Index: lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp === --- lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp +++ lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp @@ -125,7 +125,6 @@ // have some flexibility in defining the base region, we can achieve // various levels of conservatism in our buffer overflow checking. ProgramStateRef state = checkerContext.getState(); - ProgramStateRef originalState = state; SValBuilder &svalBuilder = checkerContext.getSValBuilder(); const RegionRawOffsetV2 &rawOffset = @@ -224,8 +223,7 @@ } while (false); - if (state != originalState) -checkerContext.addTransition(state); + checkerContext.addTransition(state); } void ArrayBoundCheckerV2::reportOOB( Index: lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp === --- lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp +++ lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp @@ -125,7 +125,6 @@ // have some flexibility in defining the base region, we can achieve // various levels of conservatism in our buffer overflow checking. ProgramStateRef state = checkerContext.getState(); - ProgramStateRef originalState = state; SValBuilder &svalBuilder = checkerContext.getSValBuilder(); const RegionRawOffsetV2 &rawOffset = @@ -224,8 +223,7 @@ } while (false); - if (state != originalState) -checkerContext.addTransition(state); + checkerContext.addTransition(state); } void ArrayBoundCheckerV2::reportOOB( ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47450: [ASTImporter] Use InjectedClassNameType at import of templated record.
r.stahl added inline comments.
Comment at: lib/AST/ASTImporter.cpp:2139
+ CXXRecordDecl *Injected = nullptr;
+ for (NamedDecl *Found : D2CXX->noload_lookup(Name)) {
+auto *Record = dyn_cast(Found);
balazske wrote:
> r.stahl wrote:
> > The only thing I'm wondering is whether the Decls looked up here are always
> > known to be already imported.
> These are in the 'To' context. It may be that the `Injected` is not found
> here, probably because not yet imported (in this case the import may be part
> of a not completed recursive process).
As far as I understand that corner case could be covered by doing the lookup on
`DCXX` instead and then importing the injected decl. But then you wouldn't find
it if it is only in the To context (if that is possible).
I mean if a user calls ImportDecl in another order specifically. But such a
case is probably really artificial and I'm not sure if it's even makes sense or
is already covered by ImportDeclParts.
It should be fine as it is.
Repository:
rC Clang
https://reviews.llvm.org/D47450
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44568: Fix emission of phony dependency targets when adding extra deps
dstenb updated this revision to Diff 148819.
dstenb marked an inline comment as done.
dstenb added a comment.
Addressed vsapsai's comments.
https://reviews.llvm.org/D44568
Files:
lib/Frontend/DependencyFile.cpp
test/Frontend/dependency-gen-extradeps-phony.c
Index: test/Frontend/dependency-gen-extradeps-phony.c
===
--- /dev/null
+++ test/Frontend/dependency-gen-extradeps-phony.c
@@ -0,0 +1,10 @@
+// RUN: %clang -MM -MP -Xclang -fdepfile-entry=1.extra -Xclang
-fdepfile-entry=2.extra -Xclang -fdepfile-entry=2.extra %s | \
+// RUN: FileCheck %s --implicit-check-not=.c:
+//
+// Verify that phony targets are only created for the extra dependency files,
+// and not the input file.
+
+// CHECK: dependency-gen-extradeps-phony.o: 1.extra 2.extra \
+// CHECK-NEXT: dependency-gen-extradeps-phony.c
+// CHECK: 1.extra:
+// CHECK: 2.extra:
Index: lib/Frontend/DependencyFile.cpp
===
--- lib/Frontend/DependencyFile.cpp
+++ lib/Frontend/DependencyFile.cpp
@@ -163,6 +163,7 @@
bool SeenMissingHeader;
bool IncludeModuleFiles;
DependencyOutputFormat OutputFormat;
+ unsigned InputFileIndex;
private:
bool FileMatchesDepCriteria(const char *Filename,
@@ -177,9 +178,11 @@
AddMissingHeaderDeps(Opts.AddMissingHeaderDeps),
SeenMissingHeader(false),
IncludeModuleFiles(Opts.IncludeModuleFiles),
- OutputFormat(Opts.OutputFormat) {
+ OutputFormat(Opts.OutputFormat),
+ InputFileIndex(0) {
for (const auto &ExtraDep : Opts.ExtraDeps) {
- AddFilename(ExtraDep);
+ if (AddFilename(ExtraDep))
+++InputFileIndex;
}
}
@@ -201,7 +204,7 @@
OutputDependencyFile();
}
- void AddFilename(StringRef Filename);
+ bool AddFilename(StringRef Filename);
bool includeSystemHeaders() const { return IncludeSystemHeaders; }
bool includeModuleFiles() const { return IncludeModuleFiles; }
};
@@ -325,9 +328,12 @@
}
}
-void DFGImpl::AddFilename(StringRef Filename) {
- if (FilesSet.insert(Filename).second)
+bool DFGImpl::AddFilename(StringRef Filename) {
+ if (FilesSet.insert(Filename).second) {
Files.push_back(Filename);
+return true;
+ }
+ return false;
}
/// Print the filename, with escaping or quoting that accommodates the three
@@ -463,8 +469,10 @@
// Create phony targets if requested.
if (PhonyTarget && !Files.empty()) {
-// Skip the first entry, this is always the input file itself.
-for (auto I = Files.begin() + 1, E = Files.end(); I != E; ++I) {
+unsigned Index = 0;
+for (auto I = Files.begin(), E = Files.end(); I != E; ++I) {
+ if (Index++ == InputFileIndex)
+continue;
OS << '\n';
PrintFilename(OS, *I, OutputFormat);
OS << ":\n";
Index: test/Frontend/dependency-gen-extradeps-phony.c
===
--- /dev/null
+++ test/Frontend/dependency-gen-extradeps-phony.c
@@ -0,0 +1,10 @@
+// RUN: %clang -MM -MP -Xclang -fdepfile-entry=1.extra -Xclang -fdepfile-entry=2.extra -Xclang -fdepfile-entry=2.extra %s | \
+// RUN: FileCheck %s --implicit-check-not=.c:
+//
+// Verify that phony targets are only created for the extra dependency files,
+// and not the input file.
+
+// CHECK: dependency-gen-extradeps-phony.o: 1.extra 2.extra \
+// CHECK-NEXT: dependency-gen-extradeps-phony.c
+// CHECK: 1.extra:
+// CHECK: 2.extra:
Index: lib/Frontend/DependencyFile.cpp
===
--- lib/Frontend/DependencyFile.cpp
+++ lib/Frontend/DependencyFile.cpp
@@ -163,6 +163,7 @@
bool SeenMissingHeader;
bool IncludeModuleFiles;
DependencyOutputFormat OutputFormat;
+ unsigned InputFileIndex;
private:
bool FileMatchesDepCriteria(const char *Filename,
@@ -177,9 +178,11 @@
AddMissingHeaderDeps(Opts.AddMissingHeaderDeps),
SeenMissingHeader(false),
IncludeModuleFiles(Opts.IncludeModuleFiles),
- OutputFormat(Opts.OutputFormat) {
+ OutputFormat(Opts.OutputFormat),
+ InputFileIndex(0) {
for (const auto &ExtraDep : Opts.ExtraDeps) {
- AddFilename(ExtraDep);
+ if (AddFilename(ExtraDep))
+++InputFileIndex;
}
}
@@ -201,7 +204,7 @@
OutputDependencyFile();
}
- void AddFilename(StringRef Filename);
+ bool AddFilename(StringRef Filename);
bool includeSystemHeaders() const { return IncludeSystemHeaders; }
bool includeModuleFiles() const { return IncludeModuleFiles; }
};
@@ -325,9 +328,12 @@
}
}
-void DFGImpl::AddFilename(StringRef Filename) {
- if (FilesSet.insert(Filename).second)
+bool DFGImpl::AddFilename(StringRef Filename) {
+ if (FilesSet.insert(Filename).second) {
Files.push_back(Filename);
+return true;
+ }
+ return false;
}
/// Print the filename, with escaping or quoting that accommodates the three
@@ -463,8 +469,10 @@
// Create phony targets i
[PATCH] D47063: [clangd] Keep only a limited number of idle ASTs in memory
ilya-biryukov updated this revision to Diff 148820.
ilya-biryukov marked 2 inline comments as done.
ilya-biryukov added a comment.
- Address review comments.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D47063
Files:
clangd/ClangdUnit.cpp
clangd/ClangdUnit.h
clangd/TUScheduler.cpp
clangd/TUScheduler.h
test/clangd/trace.test
unittests/clangd/FileIndexTests.cpp
Index: unittests/clangd/FileIndexTests.cpp
===
--- unittests/clangd/FileIndexTests.cpp
+++ unittests/clangd/FileIndexTests.cpp
@@ -11,6 +11,7 @@
#include "TestFS.h"
#include "TestTU.h"
#include "index/FileIndex.h"
+#include "clang/Frontend/CompilerInvocation.h"
#include "clang/Frontend/PCHContainerOperations.h"
#include "clang/Lex/Preprocessor.h"
#include "clang/Tooling/CompilationDatabase.h"
@@ -210,15 +211,15 @@
auto FooH = testPath("foo.h");
FileIndex Index;
bool IndexUpdated = false;
- CppFile File("foo.cpp", /*StorePreambleInMemory=*/true,
- std::make_shared(),
- [&Index, &IndexUpdated](PathRef FilePath, ASTContext &Ctx,
- std::shared_ptr PP) {
- EXPECT_FALSE(IndexUpdated)
- << "Expected only a single index update";
- IndexUpdated = true;
- Index.update(FilePath, &Ctx, std::move(PP));
- });
+ ASTBuilder Builder("foo.cpp", /*StorePreambleInMemory=*/true,
+ std::make_shared(),
+ [&Index, &IndexUpdated](PathRef FilePath, ASTContext &Ctx,
+ std::shared_ptr PP) {
+ EXPECT_FALSE(IndexUpdated)
+ << "Expected only a single index update";
+ IndexUpdated = true;
+ Index.update(FilePath, &Ctx, std::move(PP));
+ });
// Preparse ParseInputs.
ParseInputs PI;
@@ -243,7 +244,9 @@
)cpp";
// Rebuild the file.
- File.rebuild(std::move(PI));
+ auto CI = Builder.buildCompilerInvocation(PI);
+ Builder.buildPreamble(*CI, /*OldPreamble=*/nullptr, tooling::CompileCommand(),
+PI);
ASSERT_TRUE(IndexUpdated);
// Check the index contains symbols from the preamble, but not from the main
Index: test/clangd/trace.test
===
--- test/clangd/trace.test
+++ test/clangd/trace.test
@@ -8,14 +8,14 @@
# CHECK: "args": {
# CHECK: "File": "{{.*(/|\\)}}foo.c"
# CHECK: },
-# CHECK: "name": "Preamble",
+# CHECK: "name": "BuildPreamble",
# CHECK: "ph": "X",
# CHECK: }
# CHECK: {
# CHECK: "args": {
# CHECK: "File": "{{.*(/|\\)}}foo.c"
# CHECK: },
-# CHECK: "name": "Build",
+# CHECK: "name": "BuildAST",
# CHECK: "ph": "X",
# CHECK: }
# CHECK: },
Index: clangd/TUScheduler.h
===
--- clangd/TUScheduler.h
+++ clangd/TUScheduler.h
@@ -42,6 +42,15 @@
/// within a bounded amount of time.
};
+/// Configuration of the AST retention policy. This only covers retention of
+/// *idle* ASTs. If queue has operations requiring the AST, they might be
+/// kept in memory.
+struct ASTRetentionParams {
+ /// Maximum number of ASTs to be retained in memory when there are no pending
+ /// requests for them.
+ unsigned MaxRetainedASTs = 3;
+};
+
/// Handles running tasks for ClangdServer and managing the resources (e.g.,
/// preambles and ASTs) for opened files.
/// TUScheduler is not thread-safe, only one thread should be providing updates
@@ -53,7 +62,8 @@
public:
TUScheduler(unsigned AsyncThreadsCount, bool StorePreamblesInMemory,
PreambleParsedCallback PreambleCallback,
- std::chrono::steady_clock::duration UpdateDebounce);
+ std::chrono::steady_clock::duration UpdateDebounce,
+ ASTRetentionParams RetentionConfig = {});
~TUScheduler();
/// Returns estimated memory usage for each of the currently open files.
@@ -99,11 +109,18 @@
/// This class stores per-file data in the Files map.
struct FileData;
+public:
+ /// Responsible for retaining and rebuilding idle ASTs. An implementation is
+ /// an LRU cache.
+ class ASTCache;
+
+private:
const bool StorePreamblesInMemory;
const std::shared_ptr PCHOps;
const PreambleParsedCallback PreambleCallback;
Semaphore Barrier;
llvm::StringMap> Files;
+ std::unique_ptr IdleASTs;
// None when running tasks synchronously and non-None when running tasks
// asynchronously.
llvm::Optional PreambleTasks;
Index: clangd/TUScheduler.cpp
===
--- clangd/TUScheduler.cpp
+++ clangd/TUScheduler.cpp
@@ -45,16 +45,88 @@
#include "TUScheduler.h"
#include "Logger.h"
#include "Trace.h"
+#include "clang/Frontend/CompilerInvocation.h"
#include "clang
[PATCH] D44568: Fix emission of phony dependency targets when adding extra deps
dstenb added inline comments. Comment at: test/Frontend/dependency-gen-extradeps-phony.c:6-7 + +// CHECK: dependency-gen-extradeps-phony.o: 1.extra 2.extra +// CHECK-NOT: .c: +// CHECK: 1.extra: vsapsai wrote: > I think it would be better to have a check > > // CHECK: dependency-gen-extradeps-phony.c > > Because you expect it to be there and it's not that simple to notice the > colon in `.c:`, so it's not immediately clear how CHECK-NOT is applied here. Do you mean replace the two checks with that? Isn't there a risk that that would match with `dependency-gen-extradeps-phony.c:`, which the not-checks would not pick up then? I added a CHECK-NEXT check for the input file so that we match that whole dependency entry at least. Comment at: test/Frontend/dependency-gen-extradeps-phony.c:9-11 +// CHECK-NOT: .c: +// CHECK: 2.extra: +// CHECK-NOT: .c: vsapsai wrote: > For these repeated CHECK-NOT please consider using `FileCheck > --implicit-check-not`. In this case it's not that important as the test is > small but it can still help to capture your intention more clearly. I'll add that in the next patch (and I'll keep that in mind for future changes). Thanks! https://reviews.llvm.org/D44568 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[libcxx] r333376 - Mark the template deduction tests as UNSUPPORTED on clang 5, because it deduces the wrong type.
Author: marshall Date: Mon May 28 08:42:47 2018 New Revision: 76 URL: http://llvm.org/viewvc/llvm-project?rev=76&view=rev Log: Mark the template deduction tests as UNSUPPORTED on clang 5, because it deduces the wrong type. Modified: libcxx/trunk/test/std/containers/container.adaptors/queue/queue.cons/deduct.pass.cpp libcxx/trunk/test/std/containers/container.adaptors/stack/stack.cons/deduct.pass.cpp Modified: libcxx/trunk/test/std/containers/container.adaptors/queue/queue.cons/deduct.pass.cpp URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/containers/container.adaptors/queue/queue.cons/deduct.pass.cpp?rev=76&r1=75&r2=76&view=diff == --- libcxx/trunk/test/std/containers/container.adaptors/queue/queue.cons/deduct.pass.cpp (original) +++ libcxx/trunk/test/std/containers/container.adaptors/queue/queue.cons/deduct.pass.cpp Mon May 28 08:42:47 2018 @@ -9,8 +9,10 @@ // // UNSUPPORTED: c++98, c++03, c++11, c++14 +// UNSUPPORTED: clang-5 // UNSUPPORTED: libcpp-no-deduction-guides - +// Clang 5 will generate bad implicit deduction guides +// Specifically, for the copy constructor. // template // queue(Container) -> queue; Modified: libcxx/trunk/test/std/containers/container.adaptors/stack/stack.cons/deduct.pass.cpp URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/containers/container.adaptors/stack/stack.cons/deduct.pass.cpp?rev=76&r1=75&r2=76&view=diff == --- libcxx/trunk/test/std/containers/container.adaptors/stack/stack.cons/deduct.pass.cpp (original) +++ libcxx/trunk/test/std/containers/container.adaptors/stack/stack.cons/deduct.pass.cpp Mon May 28 08:42:47 2018 @@ -9,7 +9,10 @@ // // UNSUPPORTED: c++98, c++03, c++11, c++14 +// UNSUPPORTED: clang-5 // UNSUPPORTED: libcpp-no-deduction-guides +// Clang 5 will generate bad implicit deduction guides +// Specifically, for the copy constructor. // template ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47063: [clangd] Keep only a limited number of idle ASTs in memory
ilya-biryukov added inline comments. Comment at: clangd/TUScheduler.cpp:71 + + /// Update the function used to compute the value. + void update(std::function()> ComputeF); sammccall wrote: > ilya-biryukov wrote: > > sammccall wrote: > > > I think I understand this more as "updates the value" but the value is > > > lazy... > > > > > > I find this API somewhat hard to follow, maybe just because it's > > > unfamiliar. > > > I've mostly seen cache APIs look like one of: > > > 1. `Cache(function Compute)`, `Value Cache::get(Input)` > > > 2. `void Cache::put(Key, Value)`, `Optional Cache::get(Key)` > > > 3. `Handle Cache::put(Value)`, `Optional Handle::get()` > > > > > > This one is `Slot Cache::create()`, `void Slot::update(function > > > LazyV)`, `Value Slot::get()`. > > > > > > It's similar-ish to 3, but has 3 nontrivial operations instead of 2, and > > > the slot object is externally mutable instead of immutable, so it seems > > > more complex. What does it buy us in exchange? > > > > > > (1 & 2 work well with a natural key or inputs that are easy to compare, > > > which we don't particularly have) > > As discussed offline, now we have a simpler version that keeps > > `unique_ptr`s to idle ASTs and the clients are responsible for building the > > ASTs. > > Note that it's not a "cache" per se, so we might want a different name for > > that. > > @sammccall, you suggested to call it a pool, I find it reasonable. Should > > we name it `ASTPool` instead of `ASTCache`? > I think the name is actually fine, it's still mostly a cache. > It does have things in common with a pool, but unrelated consumers can't > share a resource, so I think that name is at least as misleading. SG, let's leave as is. Comment at: clangd/TUScheduler.cpp:94 +Lock.unlock(); +ForCleanup.reset(); + } sammccall wrote: > this line isn't actually needed right? It isn't. But it makes an important operation (destructor of the AST) explicit, so I'd still keep it. Comment at: clangd/TUScheduler.cpp:342 +if (!AST) + return Action(llvm::make_error( + "invalid AST", llvm::errc::invalid_argument)); sammccall wrote: > This failure doesn't get cached, correct? That's bad for performance. > > But if we think this is always a clangd bug, it's probably fine. (and > certainly simplifies things) Thanks, good catch! I somehow missed it, since at some point the failure **was** cached. I think we should cache it. Failing ASTs is a clangd bug, of course. However, they might be hard to fix if it's something inside clang, so I believe we should handle the failures gracefully in that case. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D47063 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47063: [clangd] Keep only a limited number of idle ASTs in memory
ilya-biryukov updated this revision to Diff 148822.
ilya-biryukov added a comment.
- Remove ASTBuilder, make everything a helper function
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D47063
Files:
clangd/ClangdUnit.cpp
clangd/ClangdUnit.h
clangd/TUScheduler.cpp
clangd/TUScheduler.h
test/clangd/trace.test
unittests/clangd/FileIndexTests.cpp
Index: unittests/clangd/FileIndexTests.cpp
===
--- unittests/clangd/FileIndexTests.cpp
+++ unittests/clangd/FileIndexTests.cpp
@@ -11,6 +11,7 @@
#include "TestFS.h"
#include "TestTU.h"
#include "index/FileIndex.h"
+#include "clang/Frontend/CompilerInvocation.h"
#include "clang/Frontend/PCHContainerOperations.h"
#include "clang/Lex/Preprocessor.h"
#include "clang/Tooling/CompilationDatabase.h"
@@ -208,18 +209,6 @@
TEST(FileIndexTest, RebuildWithPreamble) {
auto FooCpp = testPath("foo.cpp");
auto FooH = testPath("foo.h");
- FileIndex Index;
- bool IndexUpdated = false;
- CppFile File("foo.cpp", /*StorePreambleInMemory=*/true,
- std::make_shared(),
- [&Index, &IndexUpdated](PathRef FilePath, ASTContext &Ctx,
- std::shared_ptr PP) {
- EXPECT_FALSE(IndexUpdated)
- << "Expected only a single index update";
- IndexUpdated = true;
- Index.update(FilePath, &Ctx, std::move(PP));
- });
-
// Preparse ParseInputs.
ParseInputs PI;
PI.CompileCommand.Directory = testRoot();
@@ -243,7 +232,19 @@
)cpp";
// Rebuild the file.
- File.rebuild(std::move(PI));
+ auto CI = buildCompilerInvocation(PI);
+
+ FileIndex Index;
+ bool IndexUpdated = false;
+ buildPreamble(
+ FooCpp, *CI, /*OldPreamble=*/nullptr, tooling::CompileCommand(), PI,
+ std::make_shared(), /*StoreInMemory=*/true,
+ [&Index, &IndexUpdated](PathRef FilePath, ASTContext &Ctx,
+ std::shared_ptr PP) {
+EXPECT_FALSE(IndexUpdated) << "Expected only a single index update";
+IndexUpdated = true;
+Index.update(FilePath, &Ctx, std::move(PP));
+ });
ASSERT_TRUE(IndexUpdated);
// Check the index contains symbols from the preamble, but not from the main
Index: test/clangd/trace.test
===
--- test/clangd/trace.test
+++ test/clangd/trace.test
@@ -8,14 +8,14 @@
# CHECK: "args": {
# CHECK: "File": "{{.*(/|\\)}}foo.c"
# CHECK: },
-# CHECK: "name": "Preamble",
+# CHECK: "name": "BuildPreamble",
# CHECK: "ph": "X",
# CHECK: }
# CHECK: {
# CHECK: "args": {
# CHECK: "File": "{{.*(/|\\)}}foo.c"
# CHECK: },
-# CHECK: "name": "Build",
+# CHECK: "name": "BuildAST",
# CHECK: "ph": "X",
# CHECK: }
# CHECK: },
Index: clangd/TUScheduler.h
===
--- clangd/TUScheduler.h
+++ clangd/TUScheduler.h
@@ -42,6 +42,15 @@
/// within a bounded amount of time.
};
+/// Configuration of the AST retention policy. This only covers retention of
+/// *idle* ASTs. If queue has operations requiring the AST, they might be
+/// kept in memory.
+struct ASTRetentionParams {
+ /// Maximum number of ASTs to be retained in memory when there are no pending
+ /// requests for them.
+ unsigned MaxRetainedASTs = 3;
+};
+
/// Handles running tasks for ClangdServer and managing the resources (e.g.,
/// preambles and ASTs) for opened files.
/// TUScheduler is not thread-safe, only one thread should be providing updates
@@ -53,7 +62,8 @@
public:
TUScheduler(unsigned AsyncThreadsCount, bool StorePreamblesInMemory,
PreambleParsedCallback PreambleCallback,
- std::chrono::steady_clock::duration UpdateDebounce);
+ std::chrono::steady_clock::duration UpdateDebounce,
+ ASTRetentionParams RetentionConfig = {});
~TUScheduler();
/// Returns estimated memory usage for each of the currently open files.
@@ -99,11 +109,18 @@
/// This class stores per-file data in the Files map.
struct FileData;
+public:
+ /// Responsible for retaining and rebuilding idle ASTs. An implementation is
+ /// an LRU cache.
+ class ASTCache;
+
+private:
const bool StorePreamblesInMemory;
const std::shared_ptr PCHOps;
const PreambleParsedCallback PreambleCallback;
Semaphore Barrier;
llvm::StringMap> Files;
+ std::unique_ptr IdleASTs;
// None when running tasks synchronously and non-None when running tasks
// asynchronously.
llvm::Optional PreambleTasks;
Index: clangd/TUScheduler.cpp
===
--- clangd/TUScheduler.cpp
+++ clangd/TUScheduler.cpp
@@ -45,16 +45,88 @@
#include "TUScheduler.h"
#include "Logger.h"
#include "Trace.h"
+#include "clang/Frontend/CompilerInvocation.h"
#include "clang/Frontend/PCHContainerOperations.h"
+#inc
[PATCH] D47063: [clangd] Keep only a limited number of idle ASTs in memory
ilya-biryukov updated this revision to Diff 148824.
ilya-biryukov marked an inline comment as done.
ilya-biryukov added a comment.
- s/Params/Policy
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D47063
Files:
clangd/ClangdUnit.cpp
clangd/ClangdUnit.h
clangd/TUScheduler.cpp
clangd/TUScheduler.h
test/clangd/trace.test
unittests/clangd/FileIndexTests.cpp
Index: unittests/clangd/FileIndexTests.cpp
===
--- unittests/clangd/FileIndexTests.cpp
+++ unittests/clangd/FileIndexTests.cpp
@@ -11,6 +11,7 @@
#include "TestFS.h"
#include "TestTU.h"
#include "index/FileIndex.h"
+#include "clang/Frontend/CompilerInvocation.h"
#include "clang/Frontend/PCHContainerOperations.h"
#include "clang/Lex/Preprocessor.h"
#include "clang/Tooling/CompilationDatabase.h"
@@ -208,18 +209,6 @@
TEST(FileIndexTest, RebuildWithPreamble) {
auto FooCpp = testPath("foo.cpp");
auto FooH = testPath("foo.h");
- FileIndex Index;
- bool IndexUpdated = false;
- CppFile File("foo.cpp", /*StorePreambleInMemory=*/true,
- std::make_shared(),
- [&Index, &IndexUpdated](PathRef FilePath, ASTContext &Ctx,
- std::shared_ptr PP) {
- EXPECT_FALSE(IndexUpdated)
- << "Expected only a single index update";
- IndexUpdated = true;
- Index.update(FilePath, &Ctx, std::move(PP));
- });
-
// Preparse ParseInputs.
ParseInputs PI;
PI.CompileCommand.Directory = testRoot();
@@ -243,7 +232,19 @@
)cpp";
// Rebuild the file.
- File.rebuild(std::move(PI));
+ auto CI = buildCompilerInvocation(PI);
+
+ FileIndex Index;
+ bool IndexUpdated = false;
+ buildPreamble(
+ FooCpp, *CI, /*OldPreamble=*/nullptr, tooling::CompileCommand(), PI,
+ std::make_shared(), /*StoreInMemory=*/true,
+ [&Index, &IndexUpdated](PathRef FilePath, ASTContext &Ctx,
+ std::shared_ptr PP) {
+EXPECT_FALSE(IndexUpdated) << "Expected only a single index update";
+IndexUpdated = true;
+Index.update(FilePath, &Ctx, std::move(PP));
+ });
ASSERT_TRUE(IndexUpdated);
// Check the index contains symbols from the preamble, but not from the main
Index: test/clangd/trace.test
===
--- test/clangd/trace.test
+++ test/clangd/trace.test
@@ -8,14 +8,14 @@
# CHECK: "args": {
# CHECK: "File": "{{.*(/|\\)}}foo.c"
# CHECK: },
-# CHECK: "name": "Preamble",
+# CHECK: "name": "BuildPreamble",
# CHECK: "ph": "X",
# CHECK: }
# CHECK: {
# CHECK: "args": {
# CHECK: "File": "{{.*(/|\\)}}foo.c"
# CHECK: },
-# CHECK: "name": "Build",
+# CHECK: "name": "BuildAST",
# CHECK: "ph": "X",
# CHECK: }
# CHECK: },
Index: clangd/TUScheduler.h
===
--- clangd/TUScheduler.h
+++ clangd/TUScheduler.h
@@ -42,6 +42,15 @@
/// within a bounded amount of time.
};
+/// Configuration of the AST retention policy. This only covers retention of
+/// *idle* ASTs. If queue has operations requiring the AST, they might be
+/// kept in memory.
+struct ASTRetentionPolicy {
+ /// Maximum number of ASTs to be retained in memory when there are no pending
+ /// requests for them.
+ unsigned MaxRetainedASTs = 3;
+};
+
/// Handles running tasks for ClangdServer and managing the resources (e.g.,
/// preambles and ASTs) for opened files.
/// TUScheduler is not thread-safe, only one thread should be providing updates
@@ -53,7 +62,8 @@
public:
TUScheduler(unsigned AsyncThreadsCount, bool StorePreamblesInMemory,
PreambleParsedCallback PreambleCallback,
- std::chrono::steady_clock::duration UpdateDebounce);
+ std::chrono::steady_clock::duration UpdateDebounce,
+ ASTRetentionPolicy RetentionPolicy = {});
~TUScheduler();
/// Returns estimated memory usage for each of the currently open files.
@@ -99,11 +109,18 @@
/// This class stores per-file data in the Files map.
struct FileData;
+public:
+ /// Responsible for retaining and rebuilding idle ASTs. An implementation is
+ /// an LRU cache.
+ class ASTCache;
+
+private:
const bool StorePreamblesInMemory;
const std::shared_ptr PCHOps;
const PreambleParsedCallback PreambleCallback;
Semaphore Barrier;
llvm::StringMap> Files;
+ std::unique_ptr IdleASTs;
// None when running tasks synchronously and non-None when running tasks
// asynchronously.
llvm::Optional PreambleTasks;
Index: clangd/TUScheduler.cpp
===
--- clangd/TUScheduler.cpp
+++ clangd/TUScheduler.cpp
@@ -45,16 +45,88 @@
#include "TUScheduler.h"
#include "Logger.h"
#include "Trace.h"
+#include "clang/Frontend/CompilerInvocation.h"
#include "clang/Frontend/PCHContainerOperatio
[PATCH] D47063: [clangd] Keep only a limited number of idle ASTs in memory
ilya-biryukov added a comment. There are still a few nits I haven't addressed, but the other big change is now there: removing ASTBuilder, replacing it with free-standing functions. I'd say I liked the previous code better, since the use sites were a bit smaller and the functions have ridiculously many params now. But all of that seems manageable at this point. @sammccall, let me know what you think. And I'll address the nits that are left tomorrow. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D47063 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47135: [analyzer] A checker for dangling internal buffer pointers in C++
rnkovacs updated this revision to Diff 148827.
rnkovacs added a comment.
Added a check for `UnknownVal` and two FIXMEs (one for the `OriginExpr` and one
for the new `CheckKind`).
https://reviews.llvm.org/D47135
Files:
include/clang/StaticAnalyzer/Checkers/Checkers.td
lib/StaticAnalyzer/Checkers/AllocationState.h
lib/StaticAnalyzer/Checkers/CMakeLists.txt
lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp
lib/StaticAnalyzer/Checkers/MallocChecker.cpp
test/Analysis/dangling-internal-buffer.cpp
Index: test/Analysis/dangling-internal-buffer.cpp
===
--- /dev/null
+++ test/Analysis/dangling-internal-buffer.cpp
@@ -0,0 +1,71 @@
+//RUN: %clang_analyze_cc1 -analyzer-checker=alpha.cplusplus.DanglingInternalBuffer %s -analyzer-output=text -verify
+
+namespace std {
+
+template< typename CharT >
+class basic_string {
+public:
+ ~basic_string();
+ const CharT *c_str();
+};
+
+typedef basic_string string;
+typedef basic_string wstring;
+typedef basic_string u16string;
+typedef basic_string u32string;
+
+} // end namespace std
+
+void consume(const char *) {}
+void consume(const wchar_t *) {}
+void consume(const char16_t *) {}
+void consume(const char32_t *) {}
+
+void deref_after_scope_char() {
+ const char *c;
+ {
+std::string s;
+c = s.c_str();
+ }
+ consume(c); // expected-warning {{Use of memory after it is freed}}
+ // expected-note@-1 {{Use of memory after it is freed}}
+}
+
+void deref_after_scope_wchar_t() {
+ const wchar_t *w;
+ {
+std::wstring ws;
+w = ws.c_str();
+ }
+ consume(w); // expected-warning {{Use of memory after it is freed}}
+ // expected-note@-1 {{Use of memory after it is freed}}
+}
+
+void deref_after_scope_char16_t() {
+ const char16_t *c16;
+ {
+std::u16string s16;
+c16 = s16.c_str();
+ }
+ consume(c16); // expected-warning {{Use of memory after it is freed}}
+ // expected-note@-1 {{Use of memory after it is freed}}
+}
+
+void deref_after_scope_char32_t() {
+ const char32_t *c32;
+ {
+std::u32string s32;
+c32 = s32.c_str();
+ }
+ consume(c32); // expected-warning {{Use of memory after it is freed}}
+ // expected-note@-1 {{Use of memory after it is freed}}
+}
+
+void deref_after_scope_ok() {
+ const char *c;
+ std::string s;
+ {
+c = s.c_str();
+ }
+ consume(c); // no-warning
+}
Index: lib/StaticAnalyzer/Checkers/MallocChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -30,6 +30,7 @@
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/SmallString.h"
#include "llvm/ADT/StringExtras.h"
+#include "AllocationState.h"
#include
#include
@@ -45,7 +46,8 @@
AF_CXXNew,
AF_CXXNewArray,
AF_IfNameIndex,
- AF_Alloca
+ AF_Alloca,
+ AF_InternalBuffer
};
class RefState {
@@ -1467,6 +1469,7 @@
case AF_CXXNew: os << "'new'"; return;
case AF_CXXNewArray: os << "'new[]'"; return;
case AF_IfNameIndex: os << "'if_nameindex()'"; return;
+case AF_InternalBuffer: os << "container-specific allocator"; return;
case AF_Alloca:
case AF_None: llvm_unreachable("not a deallocation expression");
}
@@ -1479,6 +1482,7 @@
case AF_CXXNew: os << "'delete'"; return;
case AF_CXXNewArray: os << "'delete[]'"; return;
case AF_IfNameIndex: os << "'if_freenameindex()'"; return;
+case AF_InternalBuffer: os << "container-specific deallocator"; return;
case AF_Alloca:
case AF_None: llvm_unreachable("suspicious argument");
}
@@ -1653,7 +1657,9 @@
return Optional();
}
case AF_CXXNew:
- case AF_CXXNewArray: {
+ case AF_CXXNewArray:
+ // FIXME: Add new CheckKind for AF_InternalBuffer.
+ case AF_InternalBuffer: {
if (IsALeakCheck) {
if (ChecksEnabled[CK_NewDeleteLeaksChecker])
return CK_NewDeleteLeaksChecker;
@@ -2991,6 +2997,20 @@
}
}
+namespace clang {
+namespace ento {
+namespace allocation_state {
+
+ProgramStateRef
+markReleased(ProgramStateRef State, SymbolRef Sym, const Expr *Origin) {
+ AllocationFamily Family = AF_InternalBuffer;
+ return State->set(Sym, RefState::getReleased(Family, Origin));
+}
+
+} // end namespace allocation_state
+} // end namespace ento
+} // end namespace clang
+
void ento::registerNewDeleteLeaksChecker(CheckerManager &mgr) {
registerCStringCheckerBasic(mgr);
MallocChecker *checker = mgr.registerChecker();
Index: lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp
===
--- /dev/null
+++ lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp
@@ -0,0 +1,88 @@
+//=== DanglingInternalBufferChecker.cpp ---*- C++ -*--//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+/
[PATCH] D45517: [analyzer] False positive refutation with Z3
mikhail.ramalho updated this revision to Diff 148828.
mikhail.ramalho retitled this revision from "[analyzer] WIP: False positive
refutation with Z3" to "[analyzer] False positive refutation with Z3".
mikhail.ramalho added a comment.
Added test cases and updated the analyzer-config tests with the new crosscheck
flag.
Currently, there is one test failing that does not fail when building without
the crosscheck:
llvm/tools/clang/test/Driver/response-file.c:18:10: error: expected string
not found in input
// LONG: extern int it_works;
^
:1:1: note: scanning from here
clang version 7.0.0 (trunk 52) (llvm/trunk 74)
^
:8:3: note: possible intended match here
Selected GCC installation: /usr/lib/gcc/x86_64-redhat-linux/6.4.1
^
https://reviews.llvm.org/D45517
Files:
include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
lib/StaticAnalyzer/Core/BugReporter.cpp
lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
lib/StaticAnalyzer/Core/ProgramState.cpp
lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
test/Analysis/analyzer-config.c
test/Analysis/analyzer-config.cpp
test/Analysis/z3-crosscheck.c
Index: test/Analysis/z3-crosscheck.c
===
--- /dev/null
+++ test/Analysis/z3-crosscheck.c
@@ -0,0 +1,41 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,unix.Malloc,debug.ExprInspection -analyzer-config crosscheck-with-z3=true -verify %s
+// REQUIRES: z3
+// expected-no-diagnostics
+
+int foo(int x)
+{
+ int *z = 0;
+ if ((x & 1) && ((x & 1) ^ 1))
+ return *z; // no-warning
+ return 0;
+}
+
+void g(int d);
+
+void f(int *a, int *b) {
+ int c = 5;
+ if ((a - b) == 0)
+c = 0;
+ if (a != b)
+g(3 / c); // no-warning
+}
+
+_Bool nondet_bool();
+
+void h(int d) {
+ int x, y, k, z = 1;
+ // FIXME: Should warn about 'k' being a garbage value
+ while (z < k) { // no-warning
+z = 2 * z;
+ }
+}
+
+void i() {
+ _Bool c = nondet_bool();
+ if (c) {
+h(1);
+ } else {
+h(2);
+ }
+}
+
Index: test/Analysis/analyzer-config.cpp
===
--- test/Analysis/analyzer-config.cpp
+++ test/Analysis/analyzer-config.cpp
@@ -32,6 +32,7 @@
// CHECK-NEXT: cfg-rich-constructors = true
// CHECK-NEXT: cfg-scopes = false
// CHECK-NEXT: cfg-temporary-dtors = true
+// CHECK-NEXT: crosscheck-with-z3 = false
// CHECK-NEXT: experimental-enable-naive-ctu-analysis = false
// CHECK-NEXT: exploration_strategy = unexplored_first_queue
// CHECK-NEXT: faux-bodies = true
@@ -50,4 +51,4 @@
// CHECK-NEXT: unroll-loops = false
// CHECK-NEXT: widen-loops = false
// CHECK-NEXT: [stats]
-// CHECK-NEXT: num-entries = 30
+// CHECK-NEXT: num-entries = 31
Index: test/Analysis/analyzer-config.c
===
--- test/Analysis/analyzer-config.c
+++ test/Analysis/analyzer-config.c
@@ -18,6 +18,7 @@
// CHECK-NEXT: cfg-rich-constructors = true
// CHECK-NEXT: cfg-scopes = false
// CHECK-NEXT: cfg-temporary-dtors = true
+// CHECK-NEXT: crosscheck-with-z3 = false
// CHECK-NEXT: exploration_strategy = unexplored_first_queue
// CHECK-NEXT: faux-bodies = true
// CHECK-NEXT: graph-trim-interval = 1000
@@ -35,4 +36,4 @@
// CHECK-NEXT: unroll-loops = false
// CHECK-NEXT: widen-loops = false
// CHECK-NEXT: [stats]
-// CHECK-NEXT: num-entries = 23
+// CHECK-NEXT: num-entries = 24
Index: lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
===
--- lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
+++ lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
@@ -7,6 +7,7 @@
//
//===--===//
+#include "RangedConstraintManager.h"
#include "clang/Basic/TargetInfo.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
@@ -915,6 +916,13 @@
void print(ProgramStateRef St, raw_ostream &Out, const char *nl,
const char *sep) override;
+ void reset() override;
+
+ bool isModelFeasible() override;
+
+ void addRangeConstraints(ConstraintRangeTy PrevCR, ConstraintRangeTy SuccCR,
+ bool OnlyPurged) override;
+
//===--===//
// Implementation for interface from SimpleConstraintManager.
//===--===//
@@ -1235,6 +1243,58 @@
return State->set(CZ);
}
+void Z3ConstraintManager::reset() { Solver.reset(); }
+
+bool Z3ConstraintManager::isModelFeasible() {
+ return Solver.check() != Z3_L_FALSE;
+}
+
+void Z3C
[PATCH] D43341: [clang-doc] Implement reducer portion of the frontend framework
ioeric added inline comments.
Comment at: clang-doc/BitcodeReader.cpp:553
+
+#define READINFO(INFO)
\
+ {
\
Convert this to a template function?
Comment at: clang-doc/Reducer.cpp:19
+
+std::unique_ptr reduceInfos(std::vector> &Values) {
+ return mergeInfos(Values);
Now that merging is implemented in the info library, `reduceInfos` doesn't seem
to be necessary.
I'd suggest moving `writeInfo` closer to the bitcode writer library and get
rid of the Reducer.h/cpp.
Comment at: clang-doc/Reducer.cpp:23
+
+bool writeInfo(Info *I, SmallVectorImpl &Buffer) {
+ llvm::BitstreamWriter Stream(Buffer);
I think a more canonical form to pass in an output buffer is via
`llvm::raw_ostream`, and then callers can use `llvm::raw_string_ostream`.
Comment at: clang-doc/Representation.cpp:30
+
+#define ASSERT_MERGEABLE
\
+ assert(IT == Other.IT && (USR == EmptySID || USR == Other.USR))
Macros are generally discouraged. Could you make this a function in `Info` e.g.
`Info::mergeable(const Info &Other)`. And sub-classes can simply can
`asssert(mergeable(Other))`.
Comment at: clang-doc/Representation.cpp:59
+ case InfoType::IT_default:
+llvm::errs() << "Unexpected info type in index.\n";
+return nullptr;
Use `llvm::Expected>` (e.g.
`llvm::make_error(...)`) for error handling and let callers decide
how to handle the error (e.g. `llvm::errs() << llvm::toString(Err)`).
Comment at: clang-doc/Representation.h:146
+protected:
+ bool mergeBase(Info &&I);
};
juliehockett wrote:
> ioeric wrote:
> > It's a bit awkward that users have to dispatch from info types to the
> > corresponding `merge` function (as in `Reducer.cpp`). I think it would make
> > users' life easier if the library handles the dispatching.
> >
> > I wonder if something like the following would be better:
> > ```
> > struct Info {
> > std::unique_ptr merge(const Indo& LHS, const Info& RHS);
> > };
> > // A standalone function.
> > std::unique_ptr mergeInfo(const Info &LHS, const Info& RHS) {
> > // merge base info.
> > ...
> > // merge subclass infos.
> > assert(LHS.IT == RHS.IT); // or return nullptr
> > switch (LHS.IT) {
> >...
> > return Namespace::merge(LHS, RHS);
> > }
> > }
> >
> > struct NamespaceInfo : public Info {
> > std::unique_ptr merge(LHS, RHS);
> > };
> >
> > ```
> >
> > The unhandled switch case warning in compiler would help you catch
> > unimplemented `merge` when new info types are added.
> Sort of addressed in this update. There's an issue with where we allocate the
> return pointer, because we need to know the type of info at allocation time
> -- let me know if what's here now is too far off of what you were suggesting.
Thanks!
I thin what you have is also fine, as long as it's easy to maintain when adding
new info types.
Comment at: clang-doc/Representation.h:216
+// A standalone function to call to merge a vector of infos into one.
+std::unique_ptr mergeInfos(std::vector> &Value);
Please elaborate a bit on the contract e.g. all values should have the same
type; otherweise ...
Comment at: clang-doc/Representation.h:217
+// A standalone function to call to merge a vector of infos into one.
+std::unique_ptr mergeInfos(std::vector> &Value);
+
nit: s/Value/Values/ or Infos
Comment at: clang-doc/tool/ClangDocMain.cpp:181
+doc::writeInfo(I.get(), Buffer);
+ if (DumpResultToFile("bc", Group.getKey() + ".bc", Buffer))
+return 1;
juliehockett wrote:
> ioeric wrote:
> > (Sorry that I might be missing context here.)
> >
> > Could you please explain the incentive for dumping each info group to one
> > bc file? If the key identifies a symbol (e.g. USR), wouldn't this result in
> > a bitcode file being created for each symbol? This doesn't seem very
> > scalable.
> Yes, it would. This is mostly for debugging, since there's not really any
> tools outside the clang system that would actually want/be able to use this
> information.
Ok, is there any plan to convert intermediate result to final result and output
in a more accessible format? If so, please add a `FIXME` somewhere just to be
clearer.
Comment at: clang-doc/tool/ClangDocMain.cpp:176
+Group.getValue().clear();
+Group.getValue().emplace_back(std::move(Reduced));
+
Rather then replace the values in `MapOutput`, it would probably be clearer if
you store the reduced infos into a different container e.g. `Reduced`.
Comment at: docs/ReleaseNotes.rst:61
r333379 - [coroutines] Pass implicit object parameter to promise ctor (fix BUG37604)
Author: gornishanov
Date: Mon May 28 11:08:47 2018
New Revision: 79
URL: http://llvm.org/viewvc/llvm-project?rev=79&view=rev
Log:
[coroutines] Pass implicit object parameter to promise ctor (fix BUG37604)
Summary:
Complete the implementation of p0914r1.
Implicit object parameter should be passed to a promise constructor.
Fixes: https://bugs.llvm.org/show_bug.cgi?id=37604
Reviewers: modocache, rsmith, lewissbaker
Reviewed By: modocache
Subscribers: cfe-commits, EricWF
Differential Revision: https://reviews.llvm.org/D47454
Modified:
cfe/trunk/lib/Sema/SemaCoroutine.cpp
cfe/trunk/test/CodeGenCoroutines/coro-params.cpp
cfe/trunk/test/SemaCXX/coroutines.cpp
Modified: cfe/trunk/lib/Sema/SemaCoroutine.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaCoroutine.cpp?rev=79&r1=78&r2=79&view=diff
==
--- cfe/trunk/lib/Sema/SemaCoroutine.cpp (original)
+++ cfe/trunk/lib/Sema/SemaCoroutine.cpp Mon May 28 11:08:47 2018
@@ -510,6 +510,20 @@ VarDecl *Sema::buildCoroutinePromise(Sou
// Build a list of arguments, based on the coroutine functions arguments,
// that will be passed to the promise type's constructor.
llvm::SmallVector CtorArgExprs;
+
+ // Add implicit object parameter.
+ if (auto *MD = dyn_cast(FD)) {
+if (MD->isInstance() && !isLambdaCallOperator(MD)) {
+ ExprResult ThisExpr = ActOnCXXThis(Loc);
+ if (ThisExpr.isInvalid())
+return nullptr;
+ ThisExpr = CreateBuiltinUnaryOp(Loc, UO_Deref, ThisExpr.get());
+ if (ThisExpr.isInvalid())
+return nullptr;
+ CtorArgExprs.push_back(ThisExpr.get());
+}
+ }
+
auto &Moves = ScopeInfo->CoroutineParameterMoves;
for (auto *PD : FD->parameters()) {
if (PD->getType()->isDependentType())
Modified: cfe/trunk/test/CodeGenCoroutines/coro-params.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCoroutines/coro-params.cpp?rev=79&r1=78&r2=79&view=diff
==
--- cfe/trunk/test/CodeGenCoroutines/coro-params.cpp (original)
+++ cfe/trunk/test/CodeGenCoroutines/coro-params.cpp Mon May 28 11:08:47 2018
@@ -156,3 +156,28 @@ void coroutine_matching_promise_construc
// CHECK: invoke void
@_ZNSt12experimental16coroutine_traitsIJv28promise_matching_constructorifdEE12promise_typeC1ES1_ifd(%"struct.std::experimental::coroutine_traits::promise_type"* %__promise,
i32 %[[INT]], float %[[FLOAT]], double %[[DOUBLE]])
co_return;
}
+
+struct some_class;
+
+struct method {};
+
+template struct std::experimental::coroutine_traits {
+ struct promise_type {
+promise_type(some_class&, float);
+method get_return_object();
+suspend_always initial_suspend();
+suspend_always final_suspend();
+void return_void();
+void unhandled_exception();
+ };
+};
+
+struct some_class {
+ method good_coroutine_calls_custom_constructor(float);
+};
+
+// CHECK-LABEL: define void
@_ZN10some_class39good_coroutine_calls_custom_constructorEf(%struct.some_class*
+method some_class::good_coroutine_calls_custom_constructor(float) {
+ // CHECK: invoke void
@_ZNSt12experimental16coroutine_traitsIJ6methodR10some_classfEE12promise_typeC1ES3_f(%"struct.std::experimental::coroutine_traits::promise_type"* %__promise, %struct.some_class*
dereferenceable(1) %{{.+}}, float
+ co_return;
+}
Modified: cfe/trunk/test/SemaCXX/coroutines.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/coroutines.cpp?rev=79&r1=78&r2=79&view=diff
==
--- cfe/trunk/test/SemaCXX/coroutines.cpp (original)
+++ cfe/trunk/test/SemaCXX/coroutines.cpp Mon May 28 11:08:47 2018
@@ -831,7 +831,7 @@ struct std::experimental::coroutine_trai
};
};
-extern "C" int f(mismatch_gro_type_tag1) {
+extern "C" int f(mismatch_gro_type_tag1) {
// expected-error@-1 {{cannot initialize return object of type 'int' with an
rvalue of type 'void'}}
co_return; //expected-note {{function is a coroutine due to use of
'co_return' here}}
}
@@ -848,7 +848,7 @@ struct std::experimental::coroutine_trai
};
};
-extern "C" int f(mismatch_gro_type_tag2) {
+extern "C" int f(mismatch_gro_type_tag2) {
// expected-error@-1 {{cannot initialize return object of type 'int' with an
lvalue of type 'void *'}}
co_return; //expected-note {{function is a coroutine due to use of
'co_return' here}}
}
@@ -866,7 +866,7 @@ struct std::experimental::coroutine_trai
};
};
-extern "C" int f(mismatch_gro_type_tag3) {
+extern "C" int f(mismatch_gro_type_tag3) {
// expected-error@-1 {{cannot initialize return object of type 'int' with an
rvalue of type 'void'}}
co_return; //expected-note {{function is a coroutine due to use of
'co_return' here}}
}
@@ -885,7 +885,7 @@ struct std::experimental::coroutine_trai
};
};
-extern "C" int
r333375 - Introduces --stats-only option to only show changes in statistics.
Author: mramalho
Date: Mon May 28 08:40:39 2018
New Revision: 75
URL: http://llvm.org/viewvc/llvm-project?rev=75&view=rev
Log:
Introduces --stats-only option to only show changes in statistics.
Modified:
cfe/trunk/utils/analyzer/CmpRuns.py
Modified: cfe/trunk/utils/analyzer/CmpRuns.py
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/utils/analyzer/CmpRuns.py?rev=75&r1=74&r2=75&view=diff
==
--- cfe/trunk/utils/analyzer/CmpRuns.py (original)
+++ cfe/trunk/utils/analyzer/CmpRuns.py Mon May 28 08:40:39 2018
@@ -26,12 +26,25 @@ Usage:
"""
-import sys
-import os
-import plistlib
+from collections import defaultdict
+
from math import log
from optparse import OptionParser
+import json
+import os
+import plistlib
+import re
+import sys
+
+STATS_REGEXP = re.compile(r"Statistics: (\{.+\})", re.MULTILINE | re.DOTALL)
+class Colors:
+"""
+Color for terminal highlight.
+"""
+RED = '\x1b[2;30;41m'
+GREEN = '\x1b[6;30;42m'
+CLEAR = '\x1b[0m'
# Information about analysis run:
# path - the analysis output directory
@@ -120,12 +133,16 @@ class AnalysisRun:
# Cumulative list of all diagnostics from all the reports.
self.diagnostics = []
self.clang_version = None
+self.stats = []
def getClangVersion(self):
return self.clang_version
def readSingleFile(self, p, deleteEmpty):
data = plistlib.readPlist(p)
+if 'statistics' in data:
+self.stats.append(json.loads(data['statistics']))
+data.pop('statistics')
# We want to retrieve the clang version even if there are no
# reports. Assume that all reports were created using the same
@@ -264,12 +281,53 @@ def compareResults(A, B, opts):
return res
+def deriveStats(results):
+# Assume all keys are the same in each statistics bucket.
+combined_data = defaultdict(list)
+for stat in results.stats:
+for key, value in stat.iteritems():
+combined_data[key].append(value)
+combined_stats = {}
+for key, values in combined_data.iteritems():
+combined_stats[str(key)] = {
+"max": max(values),
+"min": min(values),
+"mean": sum(values) / len(values),
+"median": sorted(values)[len(values) / 2],
+"total": sum(values)
+}
+return combined_stats
+
+
+def compareStats(resultsA, resultsB):
+statsA = deriveStats(resultsA)
+statsB = deriveStats(resultsB)
+keys = sorted(statsA.keys())
+for key in keys:
+print key
+for kkey in statsA[key]:
+valA = float(statsA[key][kkey])
+valB = float(statsB[key][kkey])
+report = "%.3f -> %.3f" % (valA, valB)
+# Only apply highlighting when writing to TTY and it's not Windows
+if sys.stdout.isatty() and os.name != 'nt':
+if valA != 0:
+ ratio = (valB - valA) / valB
+ if ratio < -0.2:
+ report = Colors.GREEN + report + Colors.CLEAR
+ elif ratio > 0.2:
+ report = Colors.RED + report + Colors.CLEAR
+print "\t %s %s" % (kkey, report)
def dumpScanBuildResultsDiff(dirA, dirB, opts, deleteEmpty=True,
Stdout=sys.stdout):
# Load the run results.
resultsA = loadResults(dirA, opts, opts.rootA, deleteEmpty)
resultsB = loadResults(dirB, opts, opts.rootB, deleteEmpty)
+if resultsA.stats:
+compareStats(resultsA, resultsB)
+if opts.stats_only:
+return
# Open the verbose log, if given.
if opts.verboseLog:
@@ -339,6 +397,8 @@ def generate_option_parser():
default=False,
help="Show histogram of absolute paths differences. \
Requires matplotlib")
+parser.add_option("--stats-only", action="store_true", dest="stats_only",
+ default=False, help="Only show statistics on reports")
return parser
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44954: [clangd] Add "member" symbols to the index
malaperle added inline comments.
Comment at: clangd/index/SymbolCollector.cpp:158
+ translationUnitDecl(), namespaceDecl(), linkageSpecDecl(), recordDecl(),
+ enumDecl(), objcProtocolDecl(), objcInterfaceDecl(), objcCategoryDecl(),
+ objcCategoryImplDecl(), objcImplementationDecl()));
ioeric wrote:
> (Disclaimer: I don't know obj-c...)
>
> It seems that some objc contexts here are good for global code completion. If
> we want to support objc symbols, it might also make sense to properly set
> `SupportGlobalCompletion` for them.
Sounds good. Maybe it would be OK to do this in another (small) patch? I also
know next to nothing about obj-c :)
Comment at: unittests/clangd/CodeCompleteTests.cpp:741
+TEST(CompletionTest, Enums) {
+ EXPECT_THAT(completions(R"cpp(
ioeric wrote:
> It's not clear to me what the following tests (`Enums`, `AnonymousNamespace`,
> `InMainFile`) are testing. Do they test code completion or symbol collector?
> If these test symbol collection, they should be moved int
> SymbolCollectorTest.cpp
They are testing that code completion works as intended regardless of how
symbol collector is implemented. It's similar to our previous discussion in
D44882 about "black box testing". I can remove them but it seems odd to me to
not have code completion level tests for all cases because we assume that it
will behave a certain way at the symbol collection and querying levels.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D44954
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44954: [clangd] Add "member" symbols to the index
malaperle updated this revision to Diff 148834.
malaperle marked 6 inline comments as done.
malaperle added a comment.
Address comments.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D44954
Files:
clangd/CodeComplete.cpp
clangd/index/Index.h
clangd/index/MemIndex.cpp
clangd/index/SymbolCollector.cpp
clangd/index/SymbolYAML.cpp
unittests/clangd/CodeCompleteTests.cpp
unittests/clangd/FileIndexTests.cpp
unittests/clangd/FindSymbolsTests.cpp
unittests/clangd/SymbolCollectorTests.cpp
Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -67,6 +67,9 @@
Pos.end.character);
}
MATCHER_P(Refs, R, "") { return int(arg.References) == R; }
+MATCHER_P(Global, SupportGlobalCompletion, "") {
+ return arg.SupportGlobalCompletion == SupportGlobalCompletion;
+}
namespace clang {
namespace clangd {
@@ -132,9 +135,11 @@
CollectorOpts, PragmaHandler.get());
std::vector Args = {
-"symbol_collector", "-fsyntax-only", "-xc++", "-std=c++11",
-"-include", TestHeaderName, TestFileName};
+"symbol_collector", "-fsyntax-only", "-xc++",
+"-std=c++11", "-include", TestHeaderName};
Args.insert(Args.end(), ExtraArgs.begin(), ExtraArgs.end());
+Args.push_back(TestFileName);
+
tooling::ToolInvocation Invocation(
Args,
Factory->create(), Files.get(),
@@ -163,8 +168,20 @@
TEST_F(SymbolCollectorTest, CollectSymbols) {
const std::string Header = R"(
class Foo {
+ Foo() {}
+ Foo(int a) {}
void f();
+ friend void f1();
+ friend class Friend;
+ Foo& operator=(const Foo&);
+ ~Foo();
+ class Nested {
+ void f();
+ };
+};
+class Friend {
};
+
void f1();
inline void f2() {}
static const int KInt = 2;
@@ -198,25 +215,79 @@
} // namespace foo
)";
runSymbolCollector(Header, /*Main=*/"");
- EXPECT_THAT(Symbols,
- UnorderedElementsAreArray(
- {QName("Foo"), QName("f1"), QName("f2"), QName("KInt"),
- QName("kStr"), QName("foo"), QName("foo::bar"),
- QName("foo::int32"), QName("foo::int32_t"), QName("foo::v1"),
- QName("foo::bar::v2"), QName("foo::baz")}));
+ EXPECT_THAT(Symbols, UnorderedElementsAreArray(
+ {AllOf(QName("Foo"), Global(true)),
+AllOf(QName("Foo::Foo"), Global(false)),
+AllOf(QName("Foo::Foo"), Global(false)),
+AllOf(QName("Foo::f"), Global(false)),
+AllOf(QName("Foo::~Foo"), Global(false)),
+AllOf(QName("Foo::operator="), Global(false)),
+AllOf(QName("Foo::Nested"), Global(false)),
+AllOf(QName("Foo::Nested::f"), Global(false)),
+
+AllOf(QName("Friend"), Global(true)),
+AllOf(QName("f1"), Global(true)),
+AllOf(QName("f2"), Global(true)),
+AllOf(QName("KInt"), Global(true)),
+AllOf(QName("kStr"), Global(true)),
+AllOf(QName("foo"), Global(true)),
+AllOf(QName("foo::bar"), Global(true)),
+AllOf(QName("foo::int32"), Global(true)),
+AllOf(QName("foo::int32_t"), Global(true)),
+AllOf(QName("foo::v1"), Global(true)),
+AllOf(QName("foo::bar::v2"), Global(true)),
+AllOf(QName("foo::baz"), Global(true))}));
}
TEST_F(SymbolCollectorTest, Template) {
Annotations Header(R"(
// Template is indexed, specialization and instantiation is not.
-template struct [[Tmpl]] {T x = 0;};
+template struct [[Tmpl]] {T $xdecl[[x]] = 0;};
template <> struct Tmpl {};
extern template struct Tmpl;
template struct Tmpl;
)");
runSymbolCollector(Header.code(), /*Main=*/"");
- EXPECT_THAT(Symbols, UnorderedElementsAreArray({AllOf(
- QName("Tmpl"), DeclRange(Header.range()))}));
+ EXPECT_THAT(Symbols,
+ UnorderedElementsAreArray(
+ {AllOf(QName("Tmpl"), DeclRange(Header.range())),
+ AllOf(QName("Tmpl::x"), DeclRange(Header.range("xdecl")))}));
+}
+
+TEST_F(SymbolCollectorTest, ObjCSymbols) {
+ const std::string Header = R"(
+@interface Person
+- (void)someMethodName:(void*)name1 lastName:(void*)lName;
+@end
+
+@implementation Person
+- (void)someMethodName:(void*)name1 lastName:(void*)lName{
+ int foo;
+ ^(int param){ int bar; };
+}
+@end
+
+@interface Person (MyCategor
[PATCH] D44954: [clangd] Add "member" symbols to the index
malaperle added inline comments. Comment at: unittests/clangd/SymbolCollectorTests.cpp:141 Args.insert(Args.end(), ExtraArgs.begin(), ExtraArgs.end()); +Args.push_back(TestFileName); + This allows to override the "-xc++" with something else, i.e. -xobjective-c++ Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D44954 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47454: [coroutines] Pass implicit object parameter to promise ctor (fix BUG37604)
GorNishanov created this revision.
GorNishanov added reviewers: modocache, rsmith, lewissbaker.
Herald added a subscriber: EricWF.
Complete the implementation of p0914r1.
Implicit object parameter should be passed to a promise constructor.
Fixes: https://bugs.llvm.org/show_bug.cgi?id=37604
https://reviews.llvm.org/D47454
Files:
lib/Sema/SemaCoroutine.cpp
test/CodeGenCoroutines/coro-params.cpp
test/SemaCXX/coroutines.cpp
Index: test/SemaCXX/coroutines.cpp
===
--- test/SemaCXX/coroutines.cpp
+++ test/SemaCXX/coroutines.cpp
@@ -831,7 +831,7 @@
};
};
-extern "C" int f(mismatch_gro_type_tag1) {
+extern "C" int f(mismatch_gro_type_tag1) {
// expected-error@-1 {{cannot initialize return object of type 'int' with an rvalue of type 'void'}}
co_return; //expected-note {{function is a coroutine due to use of 'co_return' here}}
}
@@ -848,7 +848,7 @@
};
};
-extern "C" int f(mismatch_gro_type_tag2) {
+extern "C" int f(mismatch_gro_type_tag2) {
// expected-error@-1 {{cannot initialize return object of type 'int' with an lvalue of type 'void *'}}
co_return; //expected-note {{function is a coroutine due to use of 'co_return' here}}
}
@@ -866,7 +866,7 @@
};
};
-extern "C" int f(mismatch_gro_type_tag3) {
+extern "C" int f(mismatch_gro_type_tag3) {
// expected-error@-1 {{cannot initialize return object of type 'int' with an rvalue of type 'void'}}
co_return; //expected-note {{function is a coroutine due to use of 'co_return' here}}
}
@@ -885,7 +885,7 @@
};
};
-extern "C" int f(mismatch_gro_type_tag4) {
+extern "C" int f(mismatch_gro_type_tag4) {
// expected-error@-1 {{cannot initialize return object of type 'int' with an rvalue of type 'char *'}}
co_return; //expected-note {{function is a coroutine due to use of 'co_return' here}}
}
@@ -1246,7 +1246,10 @@
co_return;
}
+struct some_class;
+
struct good_promise_custom_constructor {
+ good_promise_custom_constructor(some_class&, float, int);
good_promise_custom_constructor(double, float, int);
good_promise_custom_constructor() = delete;
coro get_return_object();
@@ -1261,9 +1264,20 @@
co_return;
}
+struct some_class {
+ coro
+ good_coroutine_calls_custom_constructor(float, int) {
+co_return;
+ }
+ coro
+ static good_coroutine_calls_custom_constructor(double, float, int) {
+co_return;
+ }
+};
+
struct bad_promise_no_matching_constructor {
bad_promise_no_matching_constructor(int, int, int);
- // expected-note@+1 {{'bad_promise_no_matching_constructor' has been explicitly marked deleted here}}
+ // expected-note@+1 2 {{'bad_promise_no_matching_constructor' has been explicitly marked deleted here}}
bad_promise_no_matching_constructor() = delete;
coro get_return_object();
suspend_always initial_suspend();
@@ -1278,6 +1292,14 @@
co_return;
}
+struct some_class2 {
+coro
+bad_coroutine_calls_with_no_matching_constructor(int, int, int) {
+ // expected-error@-1 {{call to deleted constructor}}
+ co_return;
+}
+};
+
} // namespace CoroHandleMemberFunctionTest
class awaitable_no_unused_warn {
Index: test/CodeGenCoroutines/coro-params.cpp
===
--- test/CodeGenCoroutines/coro-params.cpp
+++ test/CodeGenCoroutines/coro-params.cpp
@@ -156,3 +156,28 @@
// CHECK: invoke void @_ZNSt12experimental16coroutine_traitsIJv28promise_matching_constructorifdEE12promise_typeC1ES1_ifd(%"struct.std::experimental::coroutine_traits::promise_type"* %__promise, i32 %[[INT]], float %[[FLOAT]], double %[[DOUBLE]])
co_return;
}
+
+struct some_class;
+
+struct method {};
+
+template struct std::experimental::coroutine_traits {
+ struct promise_type {
+promise_type(some_class&, float);
+method get_return_object();
+suspend_always initial_suspend();
+suspend_always final_suspend();
+void return_void();
+void unhandled_exception();
+ };
+};
+
+struct some_class {
+ method good_coroutine_calls_custom_constructor(float);
+};
+
+// CHECK-LABEL: define void @_ZN10some_class39good_coroutine_calls_custom_constructorEf(%struct.some_class*
+method some_class::good_coroutine_calls_custom_constructor(float) {
+ // CHECK: invoke void @_ZNSt12experimental16coroutine_traitsIJ6methodR10some_classfEE12promise_typeC1ES3_f(%"struct.std::experimental::coroutine_traits::promise_type"* %__promise, %struct.some_class* dereferenceable(1) %{{.+}}, float
+ co_return;
+}
Index: lib/Sema/SemaCoroutine.cpp
===
--- lib/Sema/SemaCoroutine.cpp
+++ lib/Sema/SemaCoroutine.cpp
@@ -510,6 +510,20 @@
// Build a list of arguments, based on the coroutine functions arguments,
// that will be passed to the promise type's constructor.
llvm::SmallVector CtorArgExprs;
+
+ // Add implicit object parameter.
+ if (auto *MD = dyn_cast(FD)) {
+if (MD->isInstance() && !isLambdaCallOperator(MD)) {
+
[PATCH] D47454: [coroutines] Pass implicit object parameter to promise ctor (fix BUG37604)
This revision was automatically updated to reflect the committed changes.
Closed by commit rL79: [coroutines] Pass implicit object parameter to
promise ctor (fix BUG37604) (authored by GorNishanov, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D47454?vs=148831&id=148833#toc
Repository:
rL LLVM
https://reviews.llvm.org/D47454
Files:
cfe/trunk/lib/Sema/SemaCoroutine.cpp
cfe/trunk/test/CodeGenCoroutines/coro-params.cpp
cfe/trunk/test/SemaCXX/coroutines.cpp
Index: cfe/trunk/test/SemaCXX/coroutines.cpp
===
--- cfe/trunk/test/SemaCXX/coroutines.cpp
+++ cfe/trunk/test/SemaCXX/coroutines.cpp
@@ -831,7 +831,7 @@
};
};
-extern "C" int f(mismatch_gro_type_tag1) {
+extern "C" int f(mismatch_gro_type_tag1) {
// expected-error@-1 {{cannot initialize return object of type 'int' with an rvalue of type 'void'}}
co_return; //expected-note {{function is a coroutine due to use of 'co_return' here}}
}
@@ -848,7 +848,7 @@
};
};
-extern "C" int f(mismatch_gro_type_tag2) {
+extern "C" int f(mismatch_gro_type_tag2) {
// expected-error@-1 {{cannot initialize return object of type 'int' with an lvalue of type 'void *'}}
co_return; //expected-note {{function is a coroutine due to use of 'co_return' here}}
}
@@ -866,7 +866,7 @@
};
};
-extern "C" int f(mismatch_gro_type_tag3) {
+extern "C" int f(mismatch_gro_type_tag3) {
// expected-error@-1 {{cannot initialize return object of type 'int' with an rvalue of type 'void'}}
co_return; //expected-note {{function is a coroutine due to use of 'co_return' here}}
}
@@ -885,7 +885,7 @@
};
};
-extern "C" int f(mismatch_gro_type_tag4) {
+extern "C" int f(mismatch_gro_type_tag4) {
// expected-error@-1 {{cannot initialize return object of type 'int' with an rvalue of type 'char *'}}
co_return; //expected-note {{function is a coroutine due to use of 'co_return' here}}
}
@@ -1246,7 +1246,10 @@
co_return;
}
+struct some_class;
+
struct good_promise_custom_constructor {
+ good_promise_custom_constructor(some_class&, float, int);
good_promise_custom_constructor(double, float, int);
good_promise_custom_constructor() = delete;
coro get_return_object();
@@ -1261,9 +1264,20 @@
co_return;
}
+struct some_class {
+ coro
+ good_coroutine_calls_custom_constructor(float, int) {
+co_return;
+ }
+ coro
+ static good_coroutine_calls_custom_constructor(double, float, int) {
+co_return;
+ }
+};
+
struct bad_promise_no_matching_constructor {
bad_promise_no_matching_constructor(int, int, int);
- // expected-note@+1 {{'bad_promise_no_matching_constructor' has been explicitly marked deleted here}}
+ // expected-note@+1 2 {{'bad_promise_no_matching_constructor' has been explicitly marked deleted here}}
bad_promise_no_matching_constructor() = delete;
coro get_return_object();
suspend_always initial_suspend();
@@ -1278,6 +1292,14 @@
co_return;
}
+struct some_class2 {
+coro
+bad_coroutine_calls_with_no_matching_constructor(int, int, int) {
+ // expected-error@-1 {{call to deleted constructor}}
+ co_return;
+}
+};
+
} // namespace CoroHandleMemberFunctionTest
class awaitable_no_unused_warn {
Index: cfe/trunk/test/CodeGenCoroutines/coro-params.cpp
===
--- cfe/trunk/test/CodeGenCoroutines/coro-params.cpp
+++ cfe/trunk/test/CodeGenCoroutines/coro-params.cpp
@@ -156,3 +156,28 @@
// CHECK: invoke void @_ZNSt12experimental16coroutine_traitsIJv28promise_matching_constructorifdEE12promise_typeC1ES1_ifd(%"struct.std::experimental::coroutine_traits::promise_type"* %__promise, i32 %[[INT]], float %[[FLOAT]], double %[[DOUBLE]])
co_return;
}
+
+struct some_class;
+
+struct method {};
+
+template struct std::experimental::coroutine_traits {
+ struct promise_type {
+promise_type(some_class&, float);
+method get_return_object();
+suspend_always initial_suspend();
+suspend_always final_suspend();
+void return_void();
+void unhandled_exception();
+ };
+};
+
+struct some_class {
+ method good_coroutine_calls_custom_constructor(float);
+};
+
+// CHECK-LABEL: define void @_ZN10some_class39good_coroutine_calls_custom_constructorEf(%struct.some_class*
+method some_class::good_coroutine_calls_custom_constructor(float) {
+ // CHECK: invoke void @_ZNSt12experimental16coroutine_traitsIJ6methodR10some_classfEE12promise_typeC1ES3_f(%"struct.std::experimental::coroutine_traits::promise_type"* %__promise, %struct.some_class* dereferenceable(1) %{{.+}}, float
+ co_return;
+}
Index: cfe/trunk/lib/Sema/SemaCoroutine.cpp
===
--- cfe/trunk/lib/Sema/SemaCoroutine.cpp
+++ cfe/trunk/lib/Sema/SemaCoroutine.cpp
@@ -510,6 +510,20 @@
// Build a list of arguments, based on the coroutine functions arguments,
// that will be passed to the promi
[PATCH] D47454: [coroutines] Pass implicit object parameter to promise ctor (fix BUG37604)
modocache accepted this revision. modocache added a comment. This revision is now accepted and ready to land. Great! Thanks @GorNishanov! https://reviews.llvm.org/D47454 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44480: [Sema] Don't skip function bodies with 'auto' without trailing return type
ilya-biryukov updated this revision to Diff 148835.
ilya-biryukov marked 2 inline comments as done.
ilya-biryukov added a comment.
- Address review comments.
- Added one more test.
Repository:
rC Clang
https://reviews.llvm.org/D44480
Files:
lib/Sema/SemaDecl.cpp
test/CodeCompletion/crash-skipped-bodies-template-inst.cpp
test/CodeCompletion/skip-auto-funcs.cpp
Index: test/CodeCompletion/skip-auto-funcs.cpp
===
--- /dev/null
+++ test/CodeCompletion/skip-auto-funcs.cpp
@@ -0,0 +1,43 @@
+// We run clang in completion mode to force skipping of function bodies and
+// check if the function bodies were skipped by observing the warnings that
+// clang produces.
+// RUN: not %clang_cc1 -fsyntax-only -code-completion-at=%s:42:1 %s -o - 2>&1 | FileCheck %s
+template
+auto not_skipped() {
+ int x;
+ if (x = 10) {}
+ // Check that this function is skipped.
+ // CHECK: 8:9: warning: using the result of an assignment as a condition without parentheses
+ return 1;
+}
+
+template
+auto lambda_not_skipped = []() {
+ int x;
+ if (x = 10) {}
+ // Check that this function is skipped.
+ // CHECK: 17:9: warning: using the result of an assignment as a condition without parentheses
+ return 1;
+}
+
+template
+auto skipped() -> T {
+ int x;
+ if (x = 10) {}
+ // Check that this function is skipped.
+ // CHECK-NOT: 26:9: warning: using the result of an assignment as a condition without parentheses
+ return 1;
+};
+
+auto lambda_skipped = []() -> int {
+ int x;
+ if (x = 10) {}
+ // This could potentially be skipped, but it isn't at the moment.
+ // CHECK: 34:9: warning: using the result of an assignment as a condition without parentheses
+ return 1;
+};
+
+int test() {
+ int complete_in_this_function;
+ // CHECK: COMPLETION: complete_in_this_function
+}
Index: test/CodeCompletion/crash-skipped-bodies-template-inst.cpp
===
--- /dev/null
+++ test/CodeCompletion/crash-skipped-bodies-template-inst.cpp
@@ -0,0 +1,27 @@
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:24:5 %s -o - 2>&1 | FileCheck %s
+template
+auto make_func() {
+ struct impl {
+impl* func() {
+ int x;
+ if (x = 10) {}
+ // Check that body of this function is actually skipped.
+ // CHECK-NOT: crash-skipped-bodies-template-inst.cpp:7:{{[0-9]+}}: warning: using the result of an assignment as a condition without parentheses
+ return this;
+}
+ };
+
+ int x;
+ if (x = 10) {}
+ // Check that this function is not skipped.
+ // CHECK: crash-skipped-bodies-template-inst.cpp:15:9: warning: using the result of an assignment as a condition without parentheses
+ return impl();
+}
+
+void foo() {
+ []() {
+make_func();
+m
+// CHECK: COMPLETION: make_func : [#auto#]make_func<<#class T#>>()
+ };
+}
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -12658,9 +12658,15 @@
// rest of the file.
// We cannot skip the body of a function with an undeduced return type,
// because any callers of that function need to know the type.
- if (const FunctionDecl *FD = D->getAsFunction())
-if (FD->isConstexpr() || FD->getReturnType()->isUndeducedType())
+ if (const FunctionDecl *FD = D->getAsFunction()) {
+if (FD->isConstexpr())
return false;
+// We can't simply call Type::isUndeducedType here, because inside template
+// auto can be deduced to a dependent type, which is not considered
+// "undeduced".
+if (FD->getReturnType()->getContainedDeducedType())
+ return false;
+ }
return Consumer.shouldSkipFunctionBody(D);
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[libcxx] r333381 - Fix up the final bits of breakage due to clang v5 generating bad implicit template deduction guides - specifically for copy-ctors
Author: marshall
Date: Mon May 28 12:20:21 2018
New Revision: 81
URL: http://llvm.org/viewvc/llvm-project?rev=81&view=rev
Log:
Fix up the final bits of breakage due to clang v5 generating bad implicit
template deduction guides - specifically for copy-ctors
Modified:
libcxx/trunk/test/std/containers/sequences/array/array.cons/deduct.pass.cpp
libcxx/trunk/test/std/utilities/optional/optional.object/optional.object.ctor/deduct.fail.cpp
libcxx/trunk/test/std/utilities/optional/optional.object/optional.object.ctor/deduct.pass.cpp
Modified:
libcxx/trunk/test/std/containers/sequences/array/array.cons/deduct.pass.cpp
URL:
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/containers/sequences/array/array.cons/deduct.pass.cpp?rev=81&r1=80&r2=81&view=diff
==
--- libcxx/trunk/test/std/containers/sequences/array/array.cons/deduct.pass.cpp
(original)
+++ libcxx/trunk/test/std/containers/sequences/array/array.cons/deduct.pass.cpp
Mon May 28 12:20:21 2018
@@ -9,7 +9,10 @@
//
// UNSUPPORTED: c++98, c++03, c++11, c++14
+// UNSUPPORTED: clang-5
// UNSUPPORTED: libcpp-no-deduction-guides
+// Clang 5 will generate bad implicit deduction guides
+// Specifically, for the copy constructor.
// template
@@ -51,8 +54,6 @@ int main()
}
// Test the implicit deduction guides
-// FIXME broken: no matching constructor
-#if 0
{
std::array source = {4.0, 5.0};
std::array arr(source); // array(array)
@@ -61,5 +62,4 @@ int main()
assert(arr[0] == 4.0);
assert(arr[1] == 5.0);
}
-#endif
}
Modified:
libcxx/trunk/test/std/utilities/optional/optional.object/optional.object.ctor/deduct.fail.cpp
URL:
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/utilities/optional/optional.object/optional.object.ctor/deduct.fail.cpp?rev=81&r1=80&r2=81&view=diff
==
---
libcxx/trunk/test/std/utilities/optional/optional.object/optional.object.ctor/deduct.fail.cpp
(original)
+++
libcxx/trunk/test/std/utilities/optional/optional.object/optional.object.ctor/deduct.fail.cpp
Mon May 28 12:20:21 2018
@@ -9,7 +9,10 @@
//
// UNSUPPORTED: c++98, c++03, c++11, c++14
+// UNSUPPORTED: clang-5
// UNSUPPORTED: libcpp-no-deduction-guides
+// Clang 5 will generate bad implicit deduction guides
+// Specifically, for the copy constructor.
// template
@@ -28,7 +31,12 @@ int main()
// Test the implicit deduction guides
{
// optional()
-std::optional opt; // expected-error {{declaration of variable 'opt'
with deduced type 'std::optional' requires an initializer}}
+std::optional opt; // expected-error-re declaration of variable
'opt' with deduced type 'std::optional' requires an initializer|no viable
constructor or deduction guide for deduction of template arguments of
'optional'
+// clang-6 gives a bogus error here:
+// declaration of variable 'opt' with deduced type 'std::optional'
requires an initializer
+// clang-7 (and later) give a better message:
+// no viable constructor or deduction guide for deduction of template
arguments of 'optional'
+// So we check for one or the other.
}
{
Modified:
libcxx/trunk/test/std/utilities/optional/optional.object/optional.object.ctor/deduct.pass.cpp
URL:
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/utilities/optional/optional.object/optional.object.ctor/deduct.pass.cpp?rev=81&r1=80&r2=81&view=diff
==
---
libcxx/trunk/test/std/utilities/optional/optional.object/optional.object.ctor/deduct.pass.cpp
(original)
+++
libcxx/trunk/test/std/utilities/optional/optional.object/optional.object.ctor/deduct.pass.cpp
Mon May 28 12:20:21 2018
@@ -9,7 +9,10 @@
//
// UNSUPPORTED: c++98, c++03, c++11, c++14
+// UNSUPPORTED: clang-5
// UNSUPPORTED: libcpp-no-deduction-guides
+// Clang 5 will generate bad implicit deduction guides
+// Specifically, for the copy constructor.
// template
@@ -40,18 +43,12 @@ int main()
}
// Test the implicit deduction guides
-
{
-// optional(const optional &);
- // FIXME clang and GCC disagree about this!
- // clang thinks opt is optional>, GCC thinks it's
optional.
-#if 0
+// optional(optional);
std::optional source('A');
std::optional opt(source);
-static_assert(std::is_same_v>>, "");
+static_assert(std::is_same_v>, "");
assert(static_cast(opt) == static_cast(source));
assert(*opt == *source);
-#endif
}
-
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44480: [Sema] Don't skip function bodies with 'auto' without trailing return type
ilya-biryukov added a comment.
Sorry for delays with this patch, I somehow missed the email notification about
it.
> Expounding on my concerns a bit -- I'm worried about all the other places
> calling isUndeducedType() and whether they're also at risk and thus a better
> fix is to change isUndeducedType() to pay attention to the language mode.
I thought semantics of `isUndeducedType()` is actually what's most callers
(e.g. diagnostics) want. That is "the type is undeduced **and** not dependent".
But the name can definitely bring some confusion, so there might be other
places that misinterpreted it.
@rsmith, it would be great if you could take another look when you have time.
Comment at: lib/Sema/SemaDecl.cpp:12609-12610
return false;
+// We can't simply call Type::isUndeducedType here, because it returns
+// false on C++14's auto return type without trailing return type.
+DeducedType *DT = FD->getReturnType()->getContainedDeducedType();
rsmith wrote:
> I don't think this comment captures the problem. Rather, I think the problem
> is that in a template we deduce the `auto` to a dependent type, so it's not
> considered "undeduced" at the point when we ask this question.
Thanks! That definitely explains the problem in a proper manner.
Comment at: lib/Sema/SemaDecl.cpp:12612-12613
+DeducedType *DT = FD->getReturnType()->getContainedDeducedType();
+if (DT && DT->getDeducedType().isNull())
+ return false;
+ }
rsmith wrote:
> Is there any need to check whether the type has been deduced here? It seems
> simpler (and equivalent) to turn off skipping for functions whose return type
> involves a deduced type regardless of whether deduction has been performed
> already (which it hasn't).
Thanks! I misinterpreted the deduced type's semantics. I thought it also
appears in the following case:
`auto foo() -> int {}`
But it doesn't.
Repository:
rC Clang
https://reviews.llvm.org/D44480
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44568: Fix emission of phony dependency targets when adding extra deps
vsapsai accepted this revision. vsapsai added a comment. This revision is now accepted and ready to land. Looks good to me. Just watch the build bots in case some of them are strict with warnings and require `(void)AddFilename(Filename)`. As for the case when the input file is also an extra dependency, I think we can ignore that for now because extra dependencies are supposed to be sanitizer blacklists. Even if that changes in the future, I expect extra dependencies to stay orthogonal to the input. Comment at: test/Frontend/dependency-gen-extradeps-phony.c:6-7 + +// CHECK: dependency-gen-extradeps-phony.o: 1.extra 2.extra +// CHECK-NOT: .c: +// CHECK: 1.extra: dstenb wrote: > vsapsai wrote: > > I think it would be better to have a check > > > > // CHECK: dependency-gen-extradeps-phony.c > > > > Because you expect it to be there and it's not that simple to notice the > > colon in `.c:`, so it's not immediately clear how CHECK-NOT is applied here. > Do you mean replace the two checks with that? Isn't there a risk that that > would match with `dependency-gen-extradeps-phony.c:`, which the not-checks > would not pick up then? > > I added a CHECK-NEXT check for the input file so that we match that whole > dependency entry at least. You are right, you need to be careful to make sure that we match .c file only as a dependency and not as a target. Good solution with CHECK-NEXT. Now I'm happy with the test as expected output is clearly visible and we protect from undesired output. https://reviews.llvm.org/D44568 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r333141 - Use zeroinitializer for (trailing zero portion of) large array initializers
Probably nice to mention in the commit message what the fix was (& if/where
there was there a test added for it?) so readers don't have to try to
eyeball diff this commit against the otherone.
On Wed, May 23, 2018 at 4:45 PM Richard Smith via cfe-commits <
cfe-commits@lists.llvm.org> wrote:
> Author: rsmith
> Date: Wed May 23 16:41:38 2018
> New Revision: 333141
>
> URL: http://llvm.org/viewvc/llvm-project?rev=333141&view=rev
> Log:
> Use zeroinitializer for (trailing zero portion of) large array initializers
> more reliably.
>
> This re-commits r333044 with a fix for PR37560.
>
> Modified:
> cfe/trunk/lib/CodeGen/CGExprConstant.cpp
> cfe/trunk/lib/Sema/SemaInit.cpp
> cfe/trunk/test/CodeGen/init.c
> cfe/trunk/test/CodeGenCXX/cxx11-initializer-aggregate.cpp
> cfe/trunk/test/SemaCXX/aggregate-initialization.cpp
>
> Modified: cfe/trunk/lib/CodeGen/CGExprConstant.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExprConstant.cpp?rev=333141&r1=333140&r2=333141&view=diff
>
> ==
> --- cfe/trunk/lib/CodeGen/CGExprConstant.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CGExprConstant.cpp Wed May 23 16:41:38 2018
> @@ -635,6 +635,60 @@ static ConstantAddress tryEmitGlobalComp
>return ConstantAddress(GV, Align);
> }
>
> +static llvm::Constant *
> +EmitArrayConstant(CodeGenModule &CGM, const ConstantArrayType *DestType,
> + llvm::Type *CommonElementType, unsigned ArrayBound,
> + SmallVectorImpl &Elements,
> + llvm::Constant *Filler) {
> + // Figure out how long the initial prefix of non-zero elements is.
> + unsigned NonzeroLength = ArrayBound;
> + if (Elements.size() < NonzeroLength && Filler->isNullValue())
> +NonzeroLength = Elements.size();
> + if (NonzeroLength == Elements.size()) {
> +while (NonzeroLength > 0 && Elements[NonzeroLength -
> 1]->isNullValue())
> + --NonzeroLength;
> + }
> +
> + if (NonzeroLength == 0) {
> +return llvm::ConstantAggregateZero::get(
> +CGM.getTypes().ConvertType(QualType(DestType, 0)));
> + }
> +
> + // Add a zeroinitializer array filler if we have lots of trailing
> zeroes.
> + unsigned TrailingZeroes = ArrayBound - NonzeroLength;
> + if (TrailingZeroes >= 8) {
> +assert(Elements.size() >= NonzeroLength &&
> + "missing initializer for non-zero element");
> +Elements.resize(NonzeroLength + 1);
> +auto *FillerType =
> +CommonElementType
> +? CommonElementType
> +: CGM.getTypes().ConvertType(DestType->getElementType());
> +FillerType = llvm::ArrayType::get(FillerType, TrailingZeroes);
> +Elements.back() = llvm::ConstantAggregateZero::get(FillerType);
> +CommonElementType = nullptr;
> + } else if (Elements.size() != ArrayBound) {
> +// Otherwise pad to the right size with the filler if necessary.
> +Elements.resize(ArrayBound, Filler);
> +if (Filler->getType() != CommonElementType)
> + CommonElementType = nullptr;
> + }
> +
> + // If all elements have the same type, just emit an array constant.
> + if (CommonElementType)
> +return llvm::ConstantArray::get(
> +llvm::ArrayType::get(CommonElementType, ArrayBound), Elements);
> +
> + // We have mixed types. Use a packed struct.
> + llvm::SmallVector Types;
> + Types.reserve(Elements.size());
> + for (llvm::Constant *Elt : Elements)
> +Types.push_back(Elt->getType());
> + llvm::StructType *SType =
> + llvm::StructType::get(CGM.getLLVMContext(), Types, true);
> + return llvm::ConstantStruct::get(SType, Elements);
> +}
> +
> /// This class only needs to handle two cases:
> /// 1) Literals (this is used by APValue emission to emit literals).
> /// 2) Arrays, structs and unions (outside C++11 mode, we don't currently
> @@ -832,68 +886,47 @@ public:
>}
>
>llvm::Constant *EmitArrayInitialization(InitListExpr *ILE, QualType T) {
> -llvm::ArrayType *AType =
> -cast(ConvertType(ILE->getType()));
> -llvm::Type *ElemTy = AType->getElementType();
> +auto *CAT = CGM.getContext().getAsConstantArrayType(ILE->getType());
> +assert(CAT && "can't emit array init for non-constant-bound array");
> unsigned NumInitElements = ILE->getNumInits();
> -unsigned NumElements = AType->getNumElements();
> +unsigned NumElements = CAT->getSize().getZExtValue();
>
> // Initialising an array requires us to automatically
> // initialise any elements that have not been initialised explicitly
> unsigned NumInitableElts = std::min(NumInitElements, NumElements);
>
> -QualType EltType =
> CGM.getContext().getAsArrayType(T)->getElementType();
> +QualType EltType = CAT->getElementType();
>
> // Initialize remaining array elements.
> -llvm::Constant *fillC;
> -if (Expr *filler = ILE->getArrayFiller())
> +llvm::Constant *fillC = nullptr;
> +if (Expr *filler = ILE->getArrayFiller(
[PATCH] D47108: [CodeGenCXX] Add -fforce-emit-vtables
Prazek updated this revision to Diff 148839.
Prazek marked an inline comment as done.
Prazek added a comment.
fixed test
Repository:
rL LLVM
https://reviews.llvm.org/D47108
Files:
clang/docs/ClangCommandLineReference.rst
clang/docs/ReleaseNotes.rst
clang/docs/UsersManual.rst
clang/include/clang/Basic/LangOptions.def
clang/include/clang/Driver/Options.td
clang/include/clang/Frontend/CodeGenOptions.def
clang/lib/CodeGen/ItaniumCXXABI.cpp
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/test/CodeGenCXX/vtable-available-externally.cpp
Index: clang/test/CodeGenCXX/vtable-available-externally.cpp
===
--- clang/test/CodeGenCXX/vtable-available-externally.cpp
+++ clang/test/CodeGenCXX/vtable-available-externally.cpp
@@ -1,5 +1,6 @@
// RUN: %clang_cc1 %s -I%S -triple=x86_64-apple-darwin10 -std=c++98 -emit-llvm -o %t
// RUN: %clang_cc1 %s -I%S -triple=x86_64-apple-darwin10 -std=c++98 -O2 -disable-llvm-passes -emit-llvm -o %t.opt
+// RUN: %clang_cc1 %s -I%S -triple=x86_64-apple-darwin10 -std=c++98 -O2 -disable-llvm-passes -emit-llvm -o %t.vtable -fforce-emit-vtables -fstrict-vtable-pointers -mconstructor-aliases
// RUN: FileCheck --check-prefix=CHECK-TEST1 %s < %t
// RUN: FileCheck --check-prefix=CHECK-TEST2 %s < %t
// RUN: FileCheck --check-prefix=CHECK-TEST5 %s < %t
@@ -13,10 +14,13 @@
// RUN: FileCheck --check-prefix=CHECK-TEST15 %s < %t.opt
// RUN: FileCheck --check-prefix=CHECK-TEST16 %s < %t.opt
// RUN: FileCheck --check-prefix=CHECK-TEST17 %s < %t.opt
+// RUN: FileCheck --check-prefix=CHECK-FORCE-EMIT %s < %t.vtable
+
#include
// CHECK-TEST1: @_ZTVN5Test11AE = external unnamed_addr constant
+// CHECK-FORCE-EMIT-DAG: @_ZTVN5Test11AE = available_externally unnamed_addr constant
namespace Test1 {
struct A {
@@ -213,14 +217,16 @@
// because A's key function is defined here, vtable is generated in this TU
// CHECK-TEST10-DAG: @_ZTVN6Test101AE = unnamed_addr constant
+// CHECK-FORCE-EMIT-DAG: @_ZTVN6Test101AE = unnamed_addr constant
struct A {
virtual void foo();
virtual void bar();
};
void A::foo() {}
// Because key function is inline we will generate vtable as linkonce_odr.
// CHECK-TEST10-DAG: @_ZTVN6Test101DE = linkonce_odr unnamed_addr constant
+// CHECK-FORCE-EMIT-DAG: @_ZTVN6Test101DE = linkonce_odr unnamed_addr constant
struct D : A {
void bar();
};
@@ -237,14 +243,17 @@
// can't guarantee that we will be able to refer to bar from name
// so (at the moment) we can't emit vtable available_externally.
// CHECK-TEST10-DAG: @_ZTVN6Test101CE = external unnamed_addr constant
+// CHECK-FORCE-EMIT-DAG: @_ZTVN6Test101CE = available_externally unnamed_addr constant
struct C : A {
void bar() {} // defined in body - not key function
virtual inline void gar(); // inline in body - not key function
virtual void car();
};
// no key function, vtable will be generated everywhere it will be used
// CHECK-TEST10-DAG: @_ZTVN6Test101EE = linkonce_odr unnamed_addr constant
+// CHECK-FORCE-EMIT-DAG: @_ZTVN6Test101EE = linkonce_odr unnamed_addr constant
+
struct E : A {};
void g(A& a) {
@@ -298,11 +307,13 @@
namespace Test12 {
// CHECK-TEST12: @_ZTVN6Test121AE = external unnamed_addr constant
+// CHECK-FORCE-EMIT-DAG: @_ZTVN6Test121AE = available_externally unnamed_addr constant
struct A {
virtual void foo();
virtual ~A() {}
};
// CHECK-TEST12: @_ZTVN6Test121BE = external unnamed_addr constant
+// CHECK-FORCE-EMIT-DAG: @_ZTVN6Test121BE = available_externally unnamed_addr constant
struct B : A {
void foo();
};
@@ -319,6 +330,9 @@
// CHECK-TEST13-DAG: @_ZTVN6Test131AE = available_externally unnamed_addr constant
// CHECK-TEST13-DAG: @_ZTVN6Test131BE = external unnamed_addr constant
+// CHECK-FORCE-EMIT-DAG: @_ZTVN6Test131AE = available_externally unnamed_addr constant
+// CHECK-FORCE-EMIT-DAG: @_ZTVN6Test131BE = available_externally unnamed_addr constant
+
struct A {
virtual ~A();
};
@@ -371,6 +385,8 @@
// generate available_externally vtable for it.
// CHECK-TEST16-DAG: @_ZTVN6Test161SE = external unnamed_addr constant
// CHECK-TEST16-DAG: @_ZTVN6Test162S2E = available_externally
+// CHECK-FORCE-EMIT-DAG: @_ZTVN6Test161SE = external unnamed_addr constant
+// CHECK-FORCE-EMIT-DAG: @_ZTVN6Test162S2E = available_externally
struct S {
__attribute__((visibility("hidden"))) virtual void doStuff();
@@ -395,6 +411,10 @@
// This test checks if we emit vtables opportunistically.
// CHECK-TEST17-DAG: @_ZTVN6Test171AE = available_externally
// CHECK-TEST17-DAG: @_ZTVN6Test171BE = external
+// CHECK-FORCE-EMIT-DAG: @_ZTVN6Test171AE = available_externally
+// CHECK-FORCE-EMIT-DAG: @_ZTVN6Test171BE = available_externally
+// CHECK-FORCE-EMIT-DAG: define linkonce_odr void @_ZN6Test171BD2Ev(
+// CHECK-FORCE-EMIT-DAG: define linkonce_odr void @_ZN6Test171BD0Ev(
struct A {
[PATCH] D45517: [analyzer] False positive refutation with Z3
george.karpenkov added a comment.
Please resubmit with -U999 diff flag (or using arc)
Comment at:
include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h:362
+class FalsePositiveRefutationBRVisitor final
+: public BugReporterVisitorImpl {
Can we have the whole class inside the `.cpp` file? It's annoying to recompile
half of the analyzer when an internal implementation detail changes
Comment at:
include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h:365
+ // Flag first node as seen by the visitor.
+ bool FirstNodeVisited = true;
+
I'm really not convinced we need this boolean field
Comment at:
include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h:21
#include "clang/StaticAnalyzer/Core/PathSensitive/SymExpr.h"
+#include "llvm/ADT/ImmutableMap.h"
#include "llvm/ADT/Optional.h"
NB: diff should be resubmitted with -U999, as phabricator shows "context not
available"
Comment at: lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp:1261
+
+ for (ConstraintRangeTy::iterator I = CR.begin(), E = CR.end(); I != E; ++I) {
+SymbolRef Sym = I.getKey();
xbolva00 wrote:
> for (auto I : CR)?
@mikhail.ramalho yes please do fix this one
https://reviews.llvm.org/D45517
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[libcxx] r333384 - LWG 2969 "polymorphic_allocator::construct() shouldn't pass resource()"
Author: ericwf
Date: Mon May 28 17:08:47 2018
New Revision: 84
URL: http://llvm.org/viewvc/llvm-project?rev=84&view=rev
Log:
LWG 2969 "polymorphic_allocator::construct() shouldn't pass resource()"
Patch from Arthur O'Dwyer.
In the TS, `uses_allocator` construction for `pair` tried to use an allocator
type of `memory_resource*`, which is incorrect because `memory_resource*` is
not an allocator type. LWG 2969 fixed it to use `polymorphic_allocator` as the
allocator type instead.
https://wg21.link/lwg2969
(D47090 included this in ``; at Eric's request, I've split
this out into its own patch applied to the existing
`` instead.)
Reviewed as https://reviews.llvm.org/D47109
Added:
libcxx/trunk/test/std/experimental/memory/memory.polymorphic.allocator.class/memory.polymorphic.allocator.mem/construct_piecewise_pair_evil.pass.cpp
Modified:
libcxx/trunk/include/experimental/memory_resource
libcxx/trunk/test/libcxx/experimental/memory/memory.polymorphic.allocator.class/memory.polymorphic.allocator.mem/construct_piecewise_pair.pass.cpp
libcxx/trunk/test/std/experimental/memory/memory.polymorphic.allocator.class/memory.polymorphic.allocator.mem/construct_pair_const_lvalue_pair.pass.cpp
libcxx/trunk/test/std/experimental/memory/memory.polymorphic.allocator.class/memory.polymorphic.allocator.mem/construct_pair_rvalue.pass.cpp
libcxx/trunk/test/std/experimental/memory/memory.polymorphic.allocator.class/memory.polymorphic.allocator.mem/construct_pair_values.pass.cpp
libcxx/trunk/test/std/experimental/memory/memory.polymorphic.allocator.class/memory.polymorphic.allocator.mem/construct_piecewise_pair.pass.cpp
libcxx/trunk/test/std/experimental/memory/memory.polymorphic.allocator.class/memory.polymorphic.allocator.mem/construct_types.pass.cpp
libcxx/trunk/test/support/test_memory_resource.hpp
Modified: libcxx/trunk/include/experimental/memory_resource
URL:
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/experimental/memory_resource?rev=84&r1=83&r2=84&view=diff
==
--- libcxx/trunk/include/experimental/memory_resource (original)
+++ libcxx/trunk/include/experimental/memory_resource Mon May 28 17:08:47 2018
@@ -206,7 +206,7 @@ public:
void construct(_Tp* __p, _Ts &&... __args)
{
_VSTD_LFTS::__lfts_user_alloc_construct(
-__p, resource(), _VSTD::forward<_Ts>(__args)...
+__p, *this, _VSTD::forward<_Ts>(__args)...
);
}
@@ -218,14 +218,14 @@ public:
::new ((void*)__p) pair<_T1, _T2>(piecewise_construct
, __transform_tuple(
typename __lfts_uses_alloc_ctor<
- _T1, memory_resource*, _Args1...
+ _T1, polymorphic_allocator&, _Args1...
>::type()
, _VSTD::move(__x)
, typename __make_tuple_indices::type{}
)
, __transform_tuple(
typename __lfts_uses_alloc_ctor<
- _T2, memory_resource*, _Args2...
+ _T2, polymorphic_allocator&, _Args2...
>::type()
, _VSTD::move(__y)
, typename __make_tuple_indices::type{}
@@ -289,23 +289,23 @@ private:
template
_LIBCPP_INLINE_VISIBILITY
-tuple
+tuple
__transform_tuple(integral_constant, tuple<_Args...> && __t,
- __tuple_indices<_Idx...>) const
+ __tuple_indices<_Idx...>)
{
-using _Tup = tuple;
-return _Tup(allocator_arg, resource(),
+using _Tup = tuple;
+return _Tup(allocator_arg, *this,
_VSTD::get<_Idx>(_VSTD::move(__t))...);
}
template
_LIBCPP_INLINE_VISIBILITY
-tuple<_Args&&..., memory_resource*>
+tuple<_Args&&..., polymorphic_allocator&>
__transform_tuple(integral_constant, tuple<_Args...> && __t,
- __tuple_indices<_Idx...>) const
+ __tuple_indices<_Idx...>)
{
-using _Tup = tuple<_Args&&..., memory_resource*>;
-return _Tup(_VSTD::get<_Idx>(_VSTD::move(__t))..., resource());
+using _Tup = tuple<_Args&&..., polymorphic_allocator&>;
+return _Tup(_VSTD::get<_Idx>(_VSTD::move(__t))..., *this);
}
_LIBCPP_INLINE_VISIBILITY
Modified:
libcxx/trunk/test/libcxx/experimental/memory/memory.polymorphic.allocator.class/memory.polymorphic.allocator.mem/construct_piecewise_pair.pass.cpp
URL:
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/libcxx/experimental/memory/memory.polymorphic.allocator.class/memory.polymorphic.allocator.mem/construct_piecewise_pair.pass.cpp?rev=84&r1=83&r2=84&view=diff
==
---
libcxx/trunk/test/libcxx/experimental/memory/memory.polymorphic.allocator.class/memory.polymorphic.allocator.mem/construct_piecewise_pair.pass.cpp
(origina
[PATCH] D47109: LWG 2969 "polymorphic_allocator::construct() shouldn't pass resource()"
EricWF closed this revision. EricWF added a comment. Committed as r84. Repository: rCXX libc++ https://reviews.llvm.org/D47109 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47111: : Implement monotonic_buffer_resource.
EricWF added a comment.
FYI I have a full implementation of this laying around as
https://reviews.llvm.org/D27402 (https://reviews.llvm.org/D27402). But I have
never taken the time to resolve merge conflicts.
Feel free to steal any of the tests if they're still relevant.
Comment at: include/experimental/memory_resource:429
+size_t __capacity_;
+size_t __alignment_;
+size_t __used_;
I can't imagine we'll need more than 1 byte to represent the alignment.
Comment at: include/experimental/memory_resource:474
+protected:
+void* do_allocate(size_t __bytes, size_t __alignment);
+
Lets add `override` to these functions.
Comment at: src/experimental/memory_resource.cpp:217
+{
+if (void *result = try_allocate_from_chunk(&__original_, bytes, align)) {
+return result;
Drop the braces for conditionals and loops with single statements.
Repository:
rCXX libc++
https://reviews.llvm.org/D47111
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44888: [RISCV] Add -mrelax/-mno-relax flags to enable/disable RISCV linker relaxation
This revision was automatically updated to reflect the committed changes.
Closed by commit rL85: [RISCV] Add -mrelax/-mno-relax flags to
enable/disable RISCV linker relaxation (authored by shiva, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D44888?vs=148548&id=148840#toc
Repository:
rL LLVM
https://reviews.llvm.org/D44888
Files:
cfe/trunk/include/clang/Driver/Options.td
cfe/trunk/lib/Driver/ToolChains/Arch/RISCV.cpp
cfe/trunk/test/Driver/riscv-features.c
Index: cfe/trunk/include/clang/Driver/Options.td
===
--- cfe/trunk/include/clang/Driver/Options.td
+++ cfe/trunk/include/clang/Driver/Options.td
@@ -151,6 +151,8 @@
Group, DocName<"WebAssembly">;
def m_x86_Features_Group : OptionGroup<"">,
Group, Flags<[CoreOption]>, DocName<"X86">;
+def m_riscv_Features_Group : OptionGroup<"">,
+ Group, DocName<"RISCV">;
def m_libc_Group : OptionGroup<"">, Group,
Flags<[HelpHidden]>;
@@ -1947,6 +1949,11 @@
def mno_soft_float : Flag<["-"], "mno-soft-float">, Group;
def mno_stackrealign : Flag<["-"], "mno-stackrealign">, Group;
+def mrelax : Flag<["-"], "mrelax">, Group,
+ HelpText<"Enable linker relaxation">;
+def mno_relax : Flag<["-"], "mno-relax">, Group,
+ HelpText<"Disable linker relaxation">;
+
def munaligned_access : Flag<["-"], "munaligned-access">,
Group,
HelpText<"Allow memory accesses to be unaligned (AArch32/AArch64 only)">;
def mno_unaligned_access : Flag<["-"], "mno-unaligned-access">,
Group,
Index: cfe/trunk/test/Driver/riscv-features.c
===
--- cfe/trunk/test/Driver/riscv-features.c
+++ cfe/trunk/test/Driver/riscv-features.c
@@ -2,3 +2,12 @@
// RUN: %clang -target riscv64-unknown-elf -### %s -fsyntax-only 2>&1 |
FileCheck %s
// CHECK: fno-signed-char
+
+// RUN: %clang -target riscv32-unknown-elf -### %s -mrelax 2>&1 | FileCheck %s
-check-prefix=RELAX
+// RUN: %clang -target riscv32-unknown-elf -### %s -mno-relax 2>&1 | FileCheck
%s -check-prefix=NO-RELAX
+// RUN: %clang -target riscv32-unknown-elf -### %s 2>&1 | FileCheck %s
-check-prefix=DEFAULT
+
+// RELAX: "-target-feature" "+relax"
+// NO-RELAX: "-target-feature" "-relax"
+// DEFAULT-NOT: "-target-feature" "+relax"
+// DEFAULT-NOT: "-target-feature" "-relax"
Index: cfe/trunk/lib/Driver/ToolChains/Arch/RISCV.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Arch/RISCV.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Arch/RISCV.cpp
@@ -15,6 +15,7 @@
#include "llvm/Option/ArgList.h"
#include "llvm/Support/TargetParser.h"
#include "llvm/Support/raw_ostream.h"
+#include "ToolChains/CommonArgs.h"
using namespace clang::driver;
using namespace clang::driver::tools;
@@ -363,6 +364,10 @@
// Handle all other types of extensions.
getExtensionFeatures(D, Args, Features, MArch, OtherExts);
}
+
+ // Now add any that the user explicitly requested on the command line,
+ // which may override the defaults.
+ handleTargetFeaturesGroup(Args, Features,
options::OPT_m_riscv_Features_Group);
}
StringRef riscv::getRISCVABI(const ArgList &Args, const llvm::Triple &Triple) {
Index: cfe/trunk/include/clang/Driver/Options.td
===
--- cfe/trunk/include/clang/Driver/Options.td
+++ cfe/trunk/include/clang/Driver/Options.td
@@ -151,6 +151,8 @@
Group, DocName<"WebAssembly">;
def m_x86_Features_Group : OptionGroup<"">,
Group, Flags<[CoreOption]>, DocName<"X86">;
+def m_riscv_Features_Group : OptionGroup<"">,
+ Group, DocName<"RISCV">;
def m_libc_Group : OptionGroup<"">, Group,
Flags<[HelpHidden]>;
@@ -1947,6 +1949,11 @@
def mno_soft_float : Flag<["-"], "mno-soft-float">, Group;
def mno_stackrealign : Flag<["-"], "mno-stackrealign">, Group;
+def mrelax : Flag<["-"], "mrelax">, Group,
+ HelpText<"Enable linker relaxation">;
+def mno_relax : Flag<["-"], "mno-relax">, Group,
+ HelpText<"Disable linker relaxation">;
+
def munaligned_access : Flag<["-"], "munaligned-access">, Group,
HelpText<"Allow memory accesses to be unaligned (AArch32/AArch64 only)">;
def mno_unaligned_access : Flag<["-"], "mno-unaligned-access">, Group,
Index: cfe/trunk/test/Driver/riscv-features.c
===
--- cfe/trunk/test/Driver/riscv-features.c
+++ cfe/trunk/test/Driver/riscv-features.c
@@ -2,3 +2,12 @@
// RUN: %clang -target riscv64-unknown-elf -### %s -fsyntax-only 2>&1 | FileCheck %s
// CHECK: fno-signed-char
+
+// RUN: %clang -target riscv32-unknown-elf -### %s -mrelax 2>&1 | FileCheck %s -check-prefix=RELAX
+// RUN: %clang -target riscv32-unknown-elf -### %s -mno-relax 2>&1 | FileCheck %s -check-pr
[PATCH] D47341: [Sema] Disable creating new delayed typos while correcting existing.
vsapsai updated this revision to Diff 148841.
vsapsai added a comment.
- Fix only infinite loop and don't touch the assertion failure.
New commit message should be:
[Sema] Fix infinite typo correction loop.
NumTypos guard value ~0U doesn't prevent from creating new delayed typos. When
you create new delayed typos during typo correction, value ~0U wraps around to
0. When NumTypos is 0 we can miss some typos and treat an expression as it can
be typo-corrected. But if the expression is still invalid after correction, we
can get stuck in infinite loop trying to correct it.
Fix by not using value ~0U so that NumTypos correctly reflects the number of
typos.
rdar://problem/38642201
https://reviews.llvm.org/D47341
Files:
clang/lib/Sema/SemaExprCXX.cpp
clang/test/Sema/typo-correction.c
Index: clang/test/Sema/typo-correction.c
===
--- clang/test/Sema/typo-correction.c
+++ clang/test/Sema/typo-correction.c
@@ -87,3 +87,16 @@
void overloadable_callexpr(int arg) {
func_overloadable(ar); //expected-error{{use of undeclared identifier}}
}
+
+// rdar://problem/38642201
+struct rdar38642201 {
+ int fieldName;
+};
+
+void rdar38642201_callee(int x, int y);
+void rdar38642201_caller() {
+ struct rdar38642201 structVar;
+ rdar38642201_callee(
+ structVar1.fieldName1.member1, //expected-error{{use of undeclared
identifier 'structVar1'}}
+ structVar2.fieldName2.member2); //expected-error{{use of undeclared
identifier 'structVar2'}}
+}
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -7668,12 +7668,8 @@
if (E && !ExprEvalContexts.empty() && ExprEvalContexts.back().NumTypos &&
(E->isTypeDependent() || E->isValueDependent() ||
E->isInstantiationDependent())) {
-auto TyposInContext = ExprEvalContexts.back().NumTypos;
-assert(TyposInContext < ~0U && "Recursive call of
CorrectDelayedTyposInExpr");
-ExprEvalContexts.back().NumTypos = ~0U;
auto TyposResolved = DelayedTypos.size();
auto Result = TransformTypos(*this, InitDecl, Filter).Transform(E);
-ExprEvalContexts.back().NumTypos = TyposInContext;
TyposResolved -= DelayedTypos.size();
if (Result.isInvalid() || Result.get() != E) {
ExprEvalContexts.back().NumTypos -= TyposResolved;
Index: clang/test/Sema/typo-correction.c
===
--- clang/test/Sema/typo-correction.c
+++ clang/test/Sema/typo-correction.c
@@ -87,3 +87,16 @@
void overloadable_callexpr(int arg) {
func_overloadable(ar); //expected-error{{use of undeclared identifier}}
}
+
+// rdar://problem/38642201
+struct rdar38642201 {
+ int fieldName;
+};
+
+void rdar38642201_callee(int x, int y);
+void rdar38642201_caller() {
+ struct rdar38642201 structVar;
+ rdar38642201_callee(
+ structVar1.fieldName1.member1, //expected-error{{use of undeclared identifier 'structVar1'}}
+ structVar2.fieldName2.member2); //expected-error{{use of undeclared identifier 'structVar2'}}
+}
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -7668,12 +7668,8 @@
if (E && !ExprEvalContexts.empty() && ExprEvalContexts.back().NumTypos &&
(E->isTypeDependent() || E->isValueDependent() ||
E->isInstantiationDependent())) {
-auto TyposInContext = ExprEvalContexts.back().NumTypos;
-assert(TyposInContext < ~0U && "Recursive call of CorrectDelayedTyposInExpr");
-ExprEvalContexts.back().NumTypos = ~0U;
auto TyposResolved = DelayedTypos.size();
auto Result = TransformTypos(*this, InitDecl, Filter).Transform(E);
-ExprEvalContexts.back().NumTypos = TyposInContext;
TyposResolved -= DelayedTypos.size();
if (Result.isInvalid() || Result.get() != E) {
ExprEvalContexts.back().NumTypos -= TyposResolved;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47341: [Sema] Disable creating new delayed typos while correcting existing.
vsapsai added a comment. Some debugging details that can be useful but too specific for the commit message. When we have infinite loop, the steps are: 1. Detect typos `structVar1`, `structVar2` 2. Correct typos `structVar1`, `structVar2` 1. Replace `structVar1` with `structVar`. Add corresponding object to `TransformTypos::TypoExprs`. 2. Detect typo `fieldName1`. `NumTypos` becomes 0. 3. Try to `CorrectDelayedTyposInExpr` for `fieldName1` TypoExpr. As there are no typos, no expression transformation. 4. Replace `structVar2` with `structVar`. Add corresponding object to `TransformTypos::TypoExprs`. 5. Detect typo `fieldName2`. `NumTypos` becomes 1. 1. Replace `fieldName2` with `fieldName` 6. Figure out that `structVar.fieldName.member2` is invalid, CheckAndAdvanceTypoExprCorrectionStreams 3. Try different TypoCorrection for `structVar1`. As it is empty, the correction fails. 4. CheckAndAdvanceTypoExprCorrectionStreams. Reset correction stream for `structVar1` and keep going with typo correction because `structVar2` typo correction stream isn't finished. 5. Try different TypoCorrection for `structVar1`. 1. Replace `structVar1` with `structVar` 2. Detect typo `fieldName1` 1. Replace `fieldName1` with `fieldName` 3. Figure out that `structVar.fieldName.member1` is invalid, CheckAndAdvanceTypoExprCorrectionStreams 6. Try different TypoCorrection for `structVar1`. As it is empty, the correction fails. 7. CheckAndAdvanceTypoExprCorrectionStreams over and over again. After my fix the steps are: 1. Detect typos `structVar1`, `structVar2` 2. Correct typos `structVar1`, `structVar2` 1. Replace `structVar1` with `structVar`. Add corresponding object to `TransformTypos::TypoExprs`. 2. Detect typo `fieldName1`. `NumTypos` becomes 3. 1. Replace `fieldName1` with `fieldName` 3. Figure out that `structVar.fieldName.member1` is invalid, CheckAndAdvanceTypoExprCorrectionStreams 3. Try different TypoCorrection for `structVar1`. As it is empty, the correction fails. 4. All available typo corrections were tried because `TransformTypos::TypoExprs` contains only `structVar1`. Complete the typo correction. https://reviews.llvm.org/D47341 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47111: : Implement monotonic_buffer_resource.
Quuxplusone added inline comments.
Comment at: include/experimental/memory_resource:429
+size_t __capacity_;
+size_t __alignment_;
+size_t __used_;
EricWF wrote:
> I can't imagine we'll need more than 1 byte to represent the alignment.
Even assuming for the sake of argument that that's right, it wouldn't buy us
anything here because of padding, would it?
At the moment, `__alignment_` needs to have enough range to store the `align`
parameter passed to `monotonic_buffer_resource::do_allocate`. In an SSE world
we should not blithely assume that `align < 256`. We //could// store
`lg(align)` in a single byte since `align` is always a power of two and less
than 2^64 — but then we're back to the first argument, which is that it'll be
padded to 8 bytes anyway so what do we gain.
Comment at: include/experimental/memory_resource:474
+protected:
+void* do_allocate(size_t __bytes, size_t __alignment);
+
EricWF wrote:
> Lets add `override` to these functions.
I grepped for "override" in `include/` and saw exactly one (accidental?) use in
`experimental/filesystem`, so I thought probably libc++ had a policy not to use
it for portability reasons. I'll add it throughout, but would like someone to
explicitly confirm that
(A) it's okay to use in this header and wouldn't need to be guarded with a
`_LIBCPP_OVERRIDE` macro or anything
(B) Arthur should //not// bother trying to add it to any //other// headers, not
even "for consistency"
Comment at: src/experimental/memory_resource.cpp:217
+{
+if (void *result = try_allocate_from_chunk(&__original_, bytes, align)) {
+return result;
EricWF wrote:
> Drop the braces for conditionals and loops with single statements.
*shiver* but okay.
Comment at: src/experimental/memory_resource.cpp:237
+
+void *result = __res_->allocate(aligned_capacity, align);
+__monotonic_buffer_header *header = (__monotonic_buffer_header *)((char
*)result + aligned_capacity - header_size);
For reference, here is where we ask the upstream for a block aligned according
to the user's `align`.
It occurs to me that the upstream might not be able to satisfy such a request
(actually, `new_delete_resource` works that way for me because libc++ doesn't
support aligned new and delete on OSX), which would make the upstream throw
`bad_alloc`. We handle this by passing along the exception. We //could//
conceivably handle it by retrying with
```
aligned_capacity += align;
__res_->allocate(aligned_capacity, alignof(max_align_t));
header->__alignment_ = alignof(max_align_t);
```
but I'm not sure that that's any of `monotonic_buffer_resource`'s business.
Happy to make the patch if you think it ought to be.
Repository:
rCXX libc++
https://reviews.llvm.org/D47111
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47111: : Implement monotonic_buffer_resource.
EricWF added inline comments. Comment at: include/experimental/memory_resource:428 +__monotonic_buffer_header *__next_; +size_t __capacity_; +size_t __alignment_; Can't we determine the capacity in most cases simply using `static_cast(this) - static_cast(__start_)`? Comment at: include/experimental/memory_resource:429 +size_t __capacity_; +size_t __alignment_; +size_t __used_; Quuxplusone wrote: > EricWF wrote: > > I can't imagine we'll need more than 1 byte to represent the alignment. > Even assuming for the sake of argument that that's right, it wouldn't buy us > anything here because of padding, would it? > > At the moment, `__alignment_` needs to have enough range to store the `align` > parameter passed to `monotonic_buffer_resource::do_allocate`. In an SSE world > we should not blithely assume that `align < 256`. We //could// store > `lg(align)` in a single byte since `align` is always a power of two and less > than 2^64 — but then we're back to the first argument, which is that it'll be > padded to 8 bytes anyway so what do we gain. > Even assuming for the sake of argument that that's right, it wouldn't buy us > anything here because of padding, would it? It could. (A) we don't actually need to include the types trailing padding when tail allocating it as a part of a buffer. and less importantly (B) I'm not sure all ABI's require the trailing padding of a type be included when that type is a data member of another type (I might just be wrong on this). I'm also looking into other ways of improving the way your implementation packs data, which may cause this to be beneficial. Comment at: include/experimental/memory_resource:474 +protected: +void* do_allocate(size_t __bytes, size_t __alignment); + Quuxplusone wrote: > EricWF wrote: > > Lets add `override` to these functions. > I grepped for "override" in `include/` and saw exactly one (accidental?) use > in `experimental/filesystem`, so I thought probably libc++ had a policy not > to use it for portability reasons. I'll add it throughout, but would like > someone to explicitly confirm that > > (A) it's okay to use in this header and wouldn't need to be guarded with a > `_LIBCPP_OVERRIDE` macro or anything > > (B) Arthur should //not// bother trying to add it to any //other// headers, > not even "for consistency" The header already assumes a full C++11 implementation (it uses `std::tuple`), the `override` keyword will bi available. No need for a special macro. Comment at: test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic_buffer.pass.cpp:1 +//===--===// +// Instead of testing the whole of `monotonic_buffer` in a single file, this test should be broken up into separate files. Roughly one per function, or when it makes sense, one per subsection for the lowest level of heading (For example a single test for all constructors under `memory.buffer.ctor` ). Repository: rCXX libc++ https://reviews.llvm.org/D47111 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47111: : Implement monotonic_buffer_resource.
EricWF added inline comments. Comment at: src/experimental/memory_resource.cpp:237 + +void *result = __res_->allocate(aligned_capacity, align); +__monotonic_buffer_header *header = (__monotonic_buffer_header *)((char *)result + aligned_capacity - header_size); Quuxplusone wrote: > For reference, here is where we ask the upstream for a block aligned > according to the user's `align`. > It occurs to me that the upstream might not be able to satisfy such a request > (actually, `new_delete_resource` works that way for me because libc++ doesn't > support aligned new and delete on OSX), which would make the upstream throw > `bad_alloc`. We handle this by passing along the exception. We //could// > conceivably handle it by retrying with > ``` > aligned_capacity += align; > __res_->allocate(aligned_capacity, alignof(max_align_t)); > header->__alignment_ = alignof(max_align_t); > ``` > but I'm not sure that that's any of `monotonic_buffer_resource`'s business. > Happy to make the patch if you think it ought to be. I was initially thinking of storing `lg(align)`. Repository: rCXX libc++ https://reviews.llvm.org/D47111 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r333387 - [X86] Merge the 3 different flavors of masked vpermi2var/vpermt2var builtins to a single version without masking. Use select builtins with appropriate operand instead.
Author: ctopper Date: Mon May 28 20:26:38 2018 New Revision: 87 URL: http://llvm.org/viewvc/llvm-project?rev=87&view=rev Log: [X86] Merge the 3 different flavors of masked vpermi2var/vpermt2var builtins to a single version without masking. Use select builtins with appropriate operand instead. Modified: cfe/trunk/include/clang/Basic/BuiltinsX86.def cfe/trunk/lib/Headers/avx512bwintrin.h cfe/trunk/lib/Headers/avx512fintrin.h cfe/trunk/lib/Headers/avx512vbmiintrin.h cfe/trunk/lib/Headers/avx512vbmivlintrin.h cfe/trunk/lib/Headers/avx512vlbwintrin.h cfe/trunk/lib/Headers/avx512vlintrin.h cfe/trunk/test/CodeGen/avx512bw-builtins.c cfe/trunk/test/CodeGen/avx512f-builtins.c cfe/trunk/test/CodeGen/avx512vbmi-builtins.c cfe/trunk/test/CodeGen/avx512vbmivl-builtin.c cfe/trunk/test/CodeGen/avx512vl-builtins.c cfe/trunk/test/CodeGen/avx512vlbw-builtins.c Modified: cfe/trunk/include/clang/Basic/BuiltinsX86.def URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/BuiltinsX86.def?rev=87&r1=86&r2=87&view=diff == --- cfe/trunk/include/clang/Basic/BuiltinsX86.def (original) +++ cfe/trunk/include/clang/Basic/BuiltinsX86.def Mon May 28 20:26:38 2018 @@ -969,10 +969,6 @@ TARGET_BUILTIN(__builtin_ia32_storeupd51 TARGET_BUILTIN(__builtin_ia32_storeapd512_mask, "vV8d*V8dUc", "n", "avx512f") TARGET_BUILTIN(__builtin_ia32_storeups512_mask, "vf*V16fUs", "n", "avx512f") TARGET_BUILTIN(__builtin_ia32_storeaps512_mask, "vV16f*V16fUs", "n", "avx512f") -TARGET_BUILTIN(__builtin_ia32_vpermt2vard512_mask, "V16iV16iV16iV16iUs", "nc", "avx512f") -TARGET_BUILTIN(__builtin_ia32_vpermt2varq512_mask, "V8LLiV8LLiV8LLiV8LLiUc", "nc", "avx512f") -TARGET_BUILTIN(__builtin_ia32_vpermt2varps512_mask, "V16fV16iV16fV16fUs", "nc", "avx512f") -TARGET_BUILTIN(__builtin_ia32_vpermt2varpd512_mask, "V8dV8LLiV8dV8dUc", "nc", "avx512f") TARGET_BUILTIN(__builtin_ia32_vpdpbusd128_mask, "V4iV4iV4iV4iUc", "nc", "avx512vl,avx512vnni") TARGET_BUILTIN(__builtin_ia32_vpdpbusd256_mask, "V8iV8iV8iV8iUc", "nc", "avx512vl,avx512vnni") @@ -1092,10 +1088,6 @@ TARGET_BUILTIN(__builtin_ia32_psubsw512_ TARGET_BUILTIN(__builtin_ia32_psubusb512_mask, "V64cV64cV64cV64cULLi", "nc", "avx512bw") TARGET_BUILTIN(__builtin_ia32_psubusw512_mask, "V32sV32sV32sV32sUi", "nc", "avx512bw") -TARGET_BUILTIN(__builtin_ia32_vpermi2varhi512_mask, "V32sV32sV32sV32sUi", "nc", "avx512bw") -TARGET_BUILTIN(__builtin_ia32_vpermt2varhi512_mask, "V32sV32sV32sV32sUi", "nc", "avx512bw") -TARGET_BUILTIN(__builtin_ia32_vpermt2varhi512_maskz, "V32sV32sV32sV32sUi", "nc", "avx512bw") - TARGET_BUILTIN(__builtin_ia32_vpconflictdi_128_mask, "V2LLiV2LLiV2LLiUc", "nc", "avx512cd,avx512vl") TARGET_BUILTIN(__builtin_ia32_vpconflictdi_256_mask, "V4LLiV4LLiV4LLiUc", "nc", "avx512cd,avx512vl") TARGET_BUILTIN(__builtin_ia32_vpconflictsi_128_mask, "V4iV4iV4iUc", "nc", "avx512cd,avx512vl") @@ -1123,13 +1115,6 @@ TARGET_BUILTIN(__builtin_ia32_vpshufbitq TARGET_BUILTIN(__builtin_ia32_vpshufbitqmb256_mask, "UiV32cV32cUi", "nc", "avx512vl,avx512bitalg") TARGET_BUILTIN(__builtin_ia32_vpshufbitqmb512_mask, "ULLiV64cV64cULLi", "nc", "avx512bitalg") -TARGET_BUILTIN(__builtin_ia32_vpermi2varhi128_mask, "V8sV8sV8sV8sUc", "nc", "avx512vl,avx512bw") -TARGET_BUILTIN(__builtin_ia32_vpermi2varhi256_mask, "V16sV16sV16sV16sUs", "nc", "avx512vl,avx512bw") -TARGET_BUILTIN(__builtin_ia32_vpermt2varhi128_mask, "V8sV8sV8sV8sUc", "nc", "avx512vl,avx512bw") -TARGET_BUILTIN(__builtin_ia32_vpermt2varhi128_maskz, "V8sV8sV8sV8sUc", "nc", "avx512vl,avx512bw") -TARGET_BUILTIN(__builtin_ia32_vpermt2varhi256_mask, "V16sV16sV16sV16sUs", "nc", "avx512vl,avx512bw") -TARGET_BUILTIN(__builtin_ia32_vpermt2varhi256_maskz, "V16sV16sV16sV16sUs", "nc", "avx512vl,avx512bw") - TARGET_BUILTIN(__builtin_ia32_pmulhrsw512, "V32sV32sV32s", "nc", "avx512bw") TARGET_BUILTIN(__builtin_ia32_pmulhuw512, "V32sV32sV32s", "nc", "avx512bw") TARGET_BUILTIN(__builtin_ia32_pmulhw512, "V32sV32sV32s", "nc", "avx512bw") @@ -1266,30 +1251,24 @@ TARGET_BUILTIN(__builtin_ia32_scattersiv TARGET_BUILTIN(__builtin_ia32_scattersiv8sf, "vf*UcV8iV8fIi", "n", "avx512vl") TARGET_BUILTIN(__builtin_ia32_scattersiv8si, "vi*UcV8iV8iIi", "n", "avx512vl") -TARGET_BUILTIN(__builtin_ia32_vpermi2vard128_mask, "V4iV4iV4iV4iUc", "nc", "avx512vl") -TARGET_BUILTIN(__builtin_ia32_vpermi2vard256_mask, "V8iV8iV8iV8iUc", "nc", "avx512vl") -TARGET_BUILTIN(__builtin_ia32_vpermi2varpd128_mask, "V2dV2dV2LLiV2dUc", "nc", "avx512vl") -TARGET_BUILTIN(__builtin_ia32_vpermi2varpd256_mask, "V4dV4dV4LLiV4dUc", "nc", "avx512vl") -TARGET_BUILTIN(__builtin_ia32_vpermi2varps128_mask, "V4fV4fV4iV4fUc", "nc", "avx512vl") -TARGET_BUILTIN(__builtin_ia32_vpermi2varps256_mask, "V8fV8fV8iV8fUc", "nc", "avx512vl") -TARGET_BUILTIN(__builtin_ia32_vpermi2varq128_mask, "V2LLiV2LLiV2LLiV2LLiUc", "nc", "avx512vl") -TARGET
[PATCH] D47111: : Implement monotonic_buffer_resource.
Quuxplusone updated this revision to Diff 148843.
Quuxplusone added a comment.
Add `override`; disintegrate the unit test; adopt some tests from Eric's
https://reviews.llvm.org/D27402.
Also fix one QOI issue discovered by Eric's tests: if the user passes an
`initial_size` to the constructor, then they are //probably// intending that
our first upstream-allocation be big enough to serve at least `initial_size`
1-byte allocations (or one `initial_size`-byte allocation with a suitably small
alignment). This is explicitly //not// mandated by the Standard (it merely
requires that our next upstream-allocation be of size at least `initial_size`),
but it's probably a healthy choice.
Repository:
rCXX libc++
https://reviews.llvm.org/D47111
Files:
include/experimental/memory_resource
src/experimental/memory_resource.cpp
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.ctor/copy_move.pass.cpp
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.ctor/with_default_resource.pass.cpp
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.ctor/without_buffer.pass.cpp
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_deallocate.pass.cpp
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_exception_safety.pass.cpp
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_from_initial_buffer.pass.cpp
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_from_underaligned_buffer.pass.cpp
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_from_zero_sized_buffer.pass.cpp
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_in_geometric_progression.pass.cpp
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_overaligned_request.pass.cpp
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/equality.pass.cpp
test/std/experimental/memory/memory.resource.monotonic.buffer/nothing_to_do.pass.cpp
Index: test/std/experimental/memory/memory.resource.monotonic.buffer/nothing_to_do.pass.cpp
===
--- /dev/null
+++ test/std/experimental/memory/memory.resource.monotonic.buffer/nothing_to_do.pass.cpp
@@ -0,0 +1,13 @@
+// -*- C++ -*-
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+int main()
+{
+}
Index: test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/equality.pass.cpp
===
--- /dev/null
+++ test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/equality.pass.cpp
@@ -0,0 +1,61 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+// UNSUPPORTED: c++98, c++03
+
+//
+
+// class monotonic_buffer_resource
+
+#include
+#include
+#include
+#include
+
+struct assert_on_compare : public std::experimental::pmr::memory_resource
+{
+protected:
+virtual void * do_allocate(size_t, size_t)
+{ assert(false); }
+
+virtual void do_deallocate(void *, size_t, size_t)
+{ assert(false); }
+
+virtual bool do_is_equal(std::experimental::pmr::memory_resource const &) const noexcept
+{ assert(false); }
+};
+
+int main()
+{
+// Same object
+{
+std::experimental::pmr::monotonic_buffer_resource r1;
+std::experimental::pmr::monotonic_buffer_resource r2;
+assert(r1 == r1);
+assert(r1 != r2);
+
+std::experimental::pmr::memory_resource & p1 = r1;
+std::experimental::pmr::memory_resource & p2 = r2;
+assert(p1 == p1);
+assert(p1 != p2);
+assert(p1 == r1);
+assert(r1 == p1);
+assert(p1 != r2);
+assert(r2 != p1);
+}
+// Different types
+{
+std::experimental::pmr::monotonic_buffer_resource mono1;
+std::experimental::pmr::memory_resource & r1 = mono1;
+assert_on_compare c;
+std::experimental::pmr::memory_resource & r2 = c;
+assert(r1 != r2);
+assert(!(r1 == r2));
+}
+}
Index: test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffe
[PATCH] D45045: [DebugInfo] Generate debug information for labels.
HsiangKai added a comment. In https://reviews.llvm.org/D45045#1092697, @hans wrote: > This broke the Chromium build. I've uploaded a reproducer at > https://bugs.chromium.org/p/chromium/issues/detail?id=841170#c1 > > I'm guessing maybe a Clang bootstrap with debug info might also reproduce the > problem, but I haven't tried that. > > Reverted in r331861. https://reviews.llvm.org/D46738 should fix the bug. Repository: rL LLVM https://reviews.llvm.org/D45045 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47111: : Implement monotonic_buffer_resource.
Quuxplusone updated this revision to Diff 148845.
Quuxplusone added a comment.
Refactor to remove unused fields from the header structs.
Before this change, `sizeof(monotonic_buffer_resource) == 56` and the overhead
per allocated chunk was `40` bytes.
After this change, `sizeof(monotonic_buffer_resource) == 48` and the overhead
per allocated chunk is `24` bytes.
Eric suggests replacing `size_t __align_` with `uint8_t __log2_align_`. I'm
amenable to this idea, but I'd want to know what's the best way to compute the
log2 of a user-supplied number.
Repository:
rCXX libc++
https://reviews.llvm.org/D47111
Files:
include/experimental/memory_resource
src/experimental/memory_resource.cpp
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.ctor/copy_move.pass.cpp
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.ctor/with_default_resource.pass.cpp
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.ctor/without_buffer.pass.cpp
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_deallocate.pass.cpp
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_exception_safety.pass.cpp
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_from_initial_buffer.pass.cpp
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_from_underaligned_buffer.pass.cpp
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_from_zero_sized_buffer.pass.cpp
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_in_geometric_progression.pass.cpp
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_overaligned_request.pass.cpp
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/equality.pass.cpp
test/std/experimental/memory/memory.resource.monotonic.buffer/nothing_to_do.pass.cpp
Index: test/std/experimental/memory/memory.resource.monotonic.buffer/nothing_to_do.pass.cpp
===
--- /dev/null
+++ test/std/experimental/memory/memory.resource.monotonic.buffer/nothing_to_do.pass.cpp
@@ -0,0 +1,13 @@
+// -*- C++ -*-
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+int main()
+{
+}
Index: test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/equality.pass.cpp
===
--- /dev/null
+++ test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/equality.pass.cpp
@@ -0,0 +1,61 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+// UNSUPPORTED: c++98, c++03
+
+//
+
+// class monotonic_buffer_resource
+
+#include
+#include
+#include
+#include
+
+struct assert_on_compare : public std::experimental::pmr::memory_resource
+{
+protected:
+virtual void * do_allocate(size_t, size_t)
+{ assert(false); }
+
+virtual void do_deallocate(void *, size_t, size_t)
+{ assert(false); }
+
+virtual bool do_is_equal(std::experimental::pmr::memory_resource const &) const noexcept
+{ assert(false); }
+};
+
+int main()
+{
+// Same object
+{
+std::experimental::pmr::monotonic_buffer_resource r1;
+std::experimental::pmr::monotonic_buffer_resource r2;
+assert(r1 == r1);
+assert(r1 != r2);
+
+std::experimental::pmr::memory_resource & p1 = r1;
+std::experimental::pmr::memory_resource & p2 = r2;
+assert(p1 == p1);
+assert(p1 != p2);
+assert(p1 == r1);
+assert(r1 == p1);
+assert(p1 != r2);
+assert(r2 != p1);
+}
+// Different types
+{
+std::experimental::pmr::monotonic_buffer_resource mono1;
+std::experimental::pmr::memory_resource & r1 = mono1;
+assert_on_compare c;
+std::experimental::pmr::memory_resource & r2 = c;
+assert(r1 != r2);
+assert(!(r1 == r2));
+}
+}
Index: test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_overaligned_request.pass.cpp
===
--- /dev/null
+++ te
