[PATCH] D52290: [driver][mips] Adjust target triple accordingly to provided ABI name
This revision was automatically updated to reflect the committed changes.
Closed by commit rL343169: [driver][mips] Adjust target triple accordingly to
provided ABI name (authored by atanasyan, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D52290?vs=167091=167237#toc
Repository:
rL LLVM
https://reviews.llvm.org/D52290
Files:
cfe/trunk/lib/Driver/Driver.cpp
cfe/trunk/test/Driver/mips-abi.c
Index: cfe/trunk/test/Driver/mips-abi.c
===
--- cfe/trunk/test/Driver/mips-abi.c
+++ cfe/trunk/test/Driver/mips-abi.c
@@ -162,3 +162,28 @@
// RUN:-march=unknown 2>&1 \
// RUN: | FileCheck -check-prefix=MIPS-ARCH-UNKNOWN %s
// MIPS-ARCH-UNKNOWN: error: unknown target CPU 'unknown'
+
+// Check adjusting of target triple accordingly to `-mabi` option.
+// RUN: %clang -target mips64-linux-gnu -mabi=32 -### %s 2>&1 \
+// RUN: | FileCheck -check-prefix=TARGET-O32 %s
+// TARGET-O32: "-triple" "mips-unknown-linux-gnu"
+// TARGET-O32: "-target-cpu" "mips32r2"
+// TARGET-O32: "-target-abi" "o32"
+// TARGET-O32: ld{{.*}}"
+// TARGET-O32: "-m" "elf32btsmip"
+
+// RUN: %clang -target mips-linux-gnu -mabi=n32 -### %s 2>&1 \
+// RUN: | FileCheck -check-prefix=TARGET-N32 %s
+// TARGET-N32: "-triple" "mips64-unknown-linux-gnu"
+// TARGET-N32: "-target-cpu" "mips64r2"
+// TARGET-N32: "-target-abi" "n32"
+// TARGET-N32: ld{{.*}}"
+// TARGET-N32: "-m" "elf32btsmipn32"
+
+// RUN: %clang -target mips-linux-gnu -mabi=64 -### %s 2>&1 \
+// RUN: | FileCheck -check-prefix=TARGET-N64 %s
+// TARGET-N64: "-triple" "mips64-unknown-linux-gnu"
+// TARGET-N64: "-target-cpu" "mips64r2"
+// TARGET-N64: "-target-abi" "n64"
+// TARGET-N64: ld{{.*}}"
+// TARGET-N64: "-m" "elf64btsmip"
Index: cfe/trunk/lib/Driver/Driver.cpp
===
--- cfe/trunk/lib/Driver/Driver.cpp
+++ cfe/trunk/lib/Driver/Driver.cpp
@@ -481,6 +481,16 @@
Target.setVendorName("intel");
}
+ // If target is MIPS adjust the target triple
+ // accordingly to provided ABI name.
+ A = Args.getLastArg(options::OPT_mabi_EQ);
+ if (A && Target.isMIPS())
+Target = llvm::StringSwitch(A->getValue())
+ .Case("32", Target.get32BitArchVariant())
+ .Case("n32", Target.get64BitArchVariant())
+ .Case("64", Target.get64BitArchVariant())
+ .Default(Target);
+
return Target;
}
Index: cfe/trunk/test/Driver/mips-abi.c
===
--- cfe/trunk/test/Driver/mips-abi.c
+++ cfe/trunk/test/Driver/mips-abi.c
@@ -162,3 +162,28 @@
// RUN:-march=unknown 2>&1 \
// RUN: | FileCheck -check-prefix=MIPS-ARCH-UNKNOWN %s
// MIPS-ARCH-UNKNOWN: error: unknown target CPU 'unknown'
+
+// Check adjusting of target triple accordingly to `-mabi` option.
+// RUN: %clang -target mips64-linux-gnu -mabi=32 -### %s 2>&1 \
+// RUN: | FileCheck -check-prefix=TARGET-O32 %s
+// TARGET-O32: "-triple" "mips-unknown-linux-gnu"
+// TARGET-O32: "-target-cpu" "mips32r2"
+// TARGET-O32: "-target-abi" "o32"
+// TARGET-O32: ld{{.*}}"
+// TARGET-O32: "-m" "elf32btsmip"
+
+// RUN: %clang -target mips-linux-gnu -mabi=n32 -### %s 2>&1 \
+// RUN: | FileCheck -check-prefix=TARGET-N32 %s
+// TARGET-N32: "-triple" "mips64-unknown-linux-gnu"
+// TARGET-N32: "-target-cpu" "mips64r2"
+// TARGET-N32: "-target-abi" "n32"
+// TARGET-N32: ld{{.*}}"
+// TARGET-N32: "-m" "elf32btsmipn32"
+
+// RUN: %clang -target mips-linux-gnu -mabi=64 -### %s 2>&1 \
+// RUN: | FileCheck -check-prefix=TARGET-N64 %s
+// TARGET-N64: "-triple" "mips64-unknown-linux-gnu"
+// TARGET-N64: "-target-cpu" "mips64r2"
+// TARGET-N64: "-target-abi" "n64"
+// TARGET-N64: ld{{.*}}"
+// TARGET-N64: "-m" "elf64btsmip"
Index: cfe/trunk/lib/Driver/Driver.cpp
===
--- cfe/trunk/lib/Driver/Driver.cpp
+++ cfe/trunk/lib/Driver/Driver.cpp
@@ -481,6 +481,16 @@
Target.setVendorName("intel");
}
+ // If target is MIPS adjust the target triple
+ // accordingly to provided ABI name.
+ A = Args.getLastArg(options::OPT_mabi_EQ);
+ if (A && Target.isMIPS())
+Target = llvm::StringSwitch(A->getValue())
+ .Case("32", Target.get32BitArchVariant())
+ .Case("n32", Target.get64BitArchVariant())
+ .Case("64", Target.get64BitArchVariant())
+ .Default(Target);
+
return Target;
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r343169 - [driver][mips] Adjust target triple accordingly to provided ABI name
Author: atanasyan
Date: Wed Sep 26 22:04:50 2018
New Revision: 343169
URL: http://llvm.org/viewvc/llvm-project?rev=343169=rev
Log:
[driver][mips] Adjust target triple accordingly to provided ABI name
Explicitly selected MIPS ABI using the `-mabi` option implies
corresponding target triple. For 'O32' ABI it's a 32-bit target triple
like `mips-linux-gnu`. For 'N32' and 'N64' ABIs it's a 64-bit target
triple like `mips64-linux-gnu`. This patch adjusts target triple
accordingly these rules like we do for pseudo-target flags '-m64',
'-m32' etc already.
Differential revision: https://reviews.llvm.org/D52290
Modified:
cfe/trunk/lib/Driver/Driver.cpp
cfe/trunk/test/Driver/mips-abi.c
Modified: cfe/trunk/lib/Driver/Driver.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Driver.cpp?rev=343169=343168=343169=diff
==
--- cfe/trunk/lib/Driver/Driver.cpp (original)
+++ cfe/trunk/lib/Driver/Driver.cpp Wed Sep 26 22:04:50 2018
@@ -481,6 +481,16 @@ static llvm::Triple computeTargetTriple(
Target.setVendorName("intel");
}
+ // If target is MIPS adjust the target triple
+ // accordingly to provided ABI name.
+ A = Args.getLastArg(options::OPT_mabi_EQ);
+ if (A && Target.isMIPS())
+Target = llvm::StringSwitch(A->getValue())
+ .Case("32", Target.get32BitArchVariant())
+ .Case("n32", Target.get64BitArchVariant())
+ .Case("64", Target.get64BitArchVariant())
+ .Default(Target);
+
return Target;
}
Modified: cfe/trunk/test/Driver/mips-abi.c
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/mips-abi.c?rev=343169=343168=343169=diff
==
--- cfe/trunk/test/Driver/mips-abi.c (original)
+++ cfe/trunk/test/Driver/mips-abi.c Wed Sep 26 22:04:50 2018
@@ -162,3 +162,28 @@
// RUN:-march=unknown 2>&1 \
// RUN: | FileCheck -check-prefix=MIPS-ARCH-UNKNOWN %s
// MIPS-ARCH-UNKNOWN: error: unknown target CPU 'unknown'
+
+// Check adjusting of target triple accordingly to `-mabi` option.
+// RUN: %clang -target mips64-linux-gnu -mabi=32 -### %s 2>&1 \
+// RUN: | FileCheck -check-prefix=TARGET-O32 %s
+// TARGET-O32: "-triple" "mips-unknown-linux-gnu"
+// TARGET-O32: "-target-cpu" "mips32r2"
+// TARGET-O32: "-target-abi" "o32"
+// TARGET-O32: ld{{.*}}"
+// TARGET-O32: "-m" "elf32btsmip"
+
+// RUN: %clang -target mips-linux-gnu -mabi=n32 -### %s 2>&1 \
+// RUN: | FileCheck -check-prefix=TARGET-N32 %s
+// TARGET-N32: "-triple" "mips64-unknown-linux-gnu"
+// TARGET-N32: "-target-cpu" "mips64r2"
+// TARGET-N32: "-target-abi" "n32"
+// TARGET-N32: ld{{.*}}"
+// TARGET-N32: "-m" "elf32btsmipn32"
+
+// RUN: %clang -target mips-linux-gnu -mabi=64 -### %s 2>&1 \
+// RUN: | FileCheck -check-prefix=TARGET-N64 %s
+// TARGET-N64: "-triple" "mips64-unknown-linux-gnu"
+// TARGET-N64: "-target-cpu" "mips64r2"
+// TARGET-N64: "-target-abi" "n64"
+// TARGET-N64: ld{{.*}}"
+// TARGET-N64: "-m" "elf64btsmip"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52290: [driver][mips] Adjust target triple accordingly to provided ABI name
atanasyan added a comment. Thanks for review. Repository: rC Clang https://reviews.llvm.org/D52290 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r343168 - [clang-tidy] Add dependency to clangAnalysis after rC343160
Author: maskray Date: Wed Sep 26 21:23:24 2018 New Revision: 343168 URL: http://llvm.org/viewvc/llvm-project?rev=343168=rev Log: [clang-tidy] Add dependency to clangAnalysis after rC343160 Modified: clang-tools-extra/trunk/clang-tidy/mpi/CMakeLists.txt Modified: clang-tools-extra/trunk/clang-tidy/mpi/CMakeLists.txt URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/mpi/CMakeLists.txt?rev=343168=343167=343168=diff == --- clang-tools-extra/trunk/clang-tidy/mpi/CMakeLists.txt (original) +++ clang-tools-extra/trunk/clang-tidy/mpi/CMakeLists.txt Wed Sep 26 21:23:24 2018 @@ -6,6 +6,7 @@ add_clang_library(clangTidyMPIModule TypeMismatchCheck.cpp LINK_LIBS + clangAnalysis clangAST clangASTMatchers clangBasic ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r343166 - llvm::sort(C.begin(), C.end()) -> llvm::sort(C)
Author: maskray
Date: Wed Sep 26 21:19:29 2018
New Revision: 343166
URL: http://llvm.org/viewvc/llvm-project?rev=343166=rev
Log:
llvm::sort(C.begin(), C.end()) -> llvm::sort(C)
The convenience wrapper in STLExtras is available since rL342102.
Modified:
clang-tools-extra/trunk/clang-tidy/readability/MagicNumbersCheck.cpp
Modified: clang-tools-extra/trunk/clang-tidy/readability/MagicNumbersCheck.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/readability/MagicNumbersCheck.cpp?rev=343166=343165=343166=diff
==
--- clang-tools-extra/trunk/clang-tidy/readability/MagicNumbersCheck.cpp
(original)
+++ clang-tools-extra/trunk/clang-tidy/readability/MagicNumbersCheck.cpp Wed
Sep 26 21:19:29 2018
@@ -66,7 +66,7 @@ MagicNumbersCheck::MagicNumbersCheck(Str
IgnoredIntegerValues.resize(IgnoredIntegerValuesInput.size());
llvm::transform(IgnoredIntegerValuesInput, IgnoredIntegerValues.begin(),
[](const std::string ) { return std::stoll(Value); });
- llvm::sort(IgnoredIntegerValues.begin(), IgnoredIntegerValues.end());
+ llvm::sort(IgnoredIntegerValues);
if (!IgnoreAllFloatingPointValues) {
// Process the set of ignored floating point values.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D10833: Retrieve BinaryOperator::getOpcode and BinaryOperator::getOpcodeStr via libclang and its python interface
kent08ian added a comment. Herald added a subscriber: dexonsmith. In https://reviews.llvm.org/D10833#1109371, @kent08ian wrote: > In https://reviews.llvm.org/D10833#970906, @milianw wrote: > > > still looks good to me. can someone else please review and commit this? > > > Ping Any updates on this? :) Repository: rC Clang https://reviews.llvm.org/D10833 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47687: [Sema] Missing -Wlogical-op-parentheses warnings in macros (PR18971)
Higuoxing added a comment. Ping. Now, this patch could give fix-it note on parentheses in macros. https://reviews.llvm.org/D47687 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52583: [analyzer] [NFC] Move the code for dumping the program point to ProgramPoint
This revision was automatically updated to reflect the committed changes.
Closed by commit rC343160: [analyzer] [NFC] Move the code for dumping the
program point to ProgramPoint (authored by george.karpenkov, committed by ).
Herald added a subscriber: cfe-commits.
Changed prior to commit:
https://reviews.llvm.org/D52583?vs=167229=167233#toc
Repository:
rC Clang
https://reviews.llvm.org/D52583
Files:
include/clang/Analysis/ProgramPoint.h
lib/Analysis/ProgramPoint.cpp
lib/StaticAnalyzer/Core/ExprEngine.cpp
Index: include/clang/Analysis/ProgramPoint.h
===
--- include/clang/Analysis/ProgramPoint.h
+++ include/clang/Analysis/ProgramPoint.h
@@ -215,6 +215,12 @@
ID.AddPointer(getTag());
}
+ void print(StringRef CR, llvm::raw_ostream ) const;
+
+ LLVM_DUMP_METHOD void dump() const {
+return print(/*CR=*/"\n", llvm::errs());
+ }
+
static ProgramPoint getProgramPoint(const Stmt *S, ProgramPoint::Kind K,
const LocationContext *LC,
const ProgramPointTag *tag);
Index: lib/Analysis/ProgramPoint.cpp
===
--- lib/Analysis/ProgramPoint.cpp
+++ lib/Analysis/ProgramPoint.cpp
@@ -43,6 +43,177 @@
}
}
+static void printLocation(raw_ostream , SourceLocation SLoc,
+ const SourceManager ,
+ StringRef CR,
+ StringRef Postfix) {
+ if (SLoc.isFileID()) {
+Out << CR << "line=" << SM.getExpansionLineNumber(SLoc)
+<< " col=" << SM.getExpansionColumnNumber(SLoc) << Postfix;
+ }
+}
+
+void ProgramPoint::print(StringRef CR, llvm::raw_ostream ) const {
+ const ASTContext =
+ getLocationContext()->getAnalysisDeclContext()->getASTContext();
+ const SourceManager = Context.getSourceManager();
+ switch (getKind()) {
+ case ProgramPoint::BlockEntranceKind:
+Out << "Block Entrance: B"
+<< castAs().getBlock()->getBlockID();
+break;
+
+ case ProgramPoint::FunctionExitKind: {
+auto FEP = getAs();
+Out << "Function Exit: B" << FEP->getBlock()->getBlockID();
+if (const ReturnStmt *RS = FEP->getStmt()) {
+ Out << CR << " Return: S" << RS->getID(Context) << CR;
+ RS->printPretty(Out, /*helper=*/nullptr, Context.getPrintingPolicy(),
+ /*Indentation=*/2, /*NewlineSymbol=*/CR);
+}
+break;
+ }
+ case ProgramPoint::BlockExitKind:
+assert(false);
+break;
+
+ case ProgramPoint::CallEnterKind:
+Out << "CallEnter";
+break;
+
+ case ProgramPoint::CallExitBeginKind:
+Out << "CallExitBegin";
+break;
+
+ case ProgramPoint::CallExitEndKind:
+Out << "CallExitEnd";
+break;
+
+ case ProgramPoint::PostStmtPurgeDeadSymbolsKind:
+Out << "PostStmtPurgeDeadSymbols";
+break;
+
+ case ProgramPoint::PreStmtPurgeDeadSymbolsKind:
+Out << "PreStmtPurgeDeadSymbols";
+break;
+
+ case ProgramPoint::EpsilonKind:
+Out << "Epsilon Point";
+break;
+
+ case ProgramPoint::LoopExitKind: {
+LoopExit LE = castAs();
+Out << "LoopExit: " << LE.getLoopStmt()->getStmtClassName();
+break;
+ }
+
+ case ProgramPoint::PreImplicitCallKind: {
+ImplicitCallPoint PC = castAs();
+Out << "PreCall: ";
+PC.getDecl()->print(Out, Context.getLangOpts());
+printLocation(Out, PC.getLocation(), SM, CR, /*Postfix=*/CR);
+break;
+ }
+
+ case ProgramPoint::PostImplicitCallKind: {
+ImplicitCallPoint PC = castAs();
+Out << "PostCall: ";
+PC.getDecl()->print(Out, Context.getLangOpts());
+printLocation(Out, PC.getLocation(), SM, CR, /*Postfix=*/CR);
+break;
+ }
+
+ case ProgramPoint::PostInitializerKind: {
+Out << "PostInitializer: ";
+const CXXCtorInitializer *Init = castAs().getInitializer();
+if (const FieldDecl *FD = Init->getAnyMember())
+ Out << *FD;
+else {
+ QualType Ty = Init->getTypeSourceInfo()->getType();
+ Ty = Ty.getLocalUnqualifiedType();
+ Ty.print(Out, Context.getLangOpts());
+}
+break;
+ }
+
+ case ProgramPoint::BlockEdgeKind: {
+const BlockEdge = castAs();
+Out << "Edge: (B" << E.getSrc()->getBlockID() << ", B"
+<< E.getDst()->getBlockID() << ')';
+
+if (const Stmt *T = E.getSrc()->getTerminator()) {
+ SourceLocation SLoc = T->getBeginLoc();
+
+ Out << "\\|Terminator: ";
+ E.getSrc()->printTerminator(Out, Context.getLangOpts());
+ printLocation(Out, SLoc, SM, CR, /*Postfix=*/"");
+
+ if (isa(T)) {
+const Stmt *Label = E.getDst()->getLabel();
+
+if (Label) {
+ if (const auto *C = dyn_cast(Label)) {
+Out << CR << "case ";
+if (C->getLHS())
+ C->getLHS()->printPretty(
+ Out, nullptr, Context.getPrintingPolicy(),
+ /*Indentation=*/0, /*NewlineSymbol=*/CR);
+
+if (const Stmt *RHS =
[PATCH] D52519: [analyzer] [NFC] Heavy refactoring of trackNullOrUndefValue
This revision was automatically updated to reflect the committed changes.
Closed by commit rL343159: [analyzer] [NFC] Heavy refactoring of
trackNullOrUndefValue (authored by george.karpenkov, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D52519?vs=166990=167232#toc
Repository:
rL LLVM
https://reviews.llvm.org/D52519
Files:
cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
Index: cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
===
--- cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
@@ -363,17 +363,13 @@
/// \param N A node "downstream" from the evaluation of the statement.
/// \param S The statement whose value is null or undefined.
/// \param R The bug report to which visitors should be attached.
-/// \param IsArg Whether the statement is an argument to an inlined function.
-/// If this is the case, \p N \em must be the CallEnter node for
-/// the function.
/// \param EnableNullFPSuppression Whether we should employ false positive
/// suppression (inlined defensive checks, returned null).
///
/// \return Whether or not the function was able to add visitors for this
/// statement. Note that returning \c true does not actually imply
/// that any visitors were added.
bool trackNullOrUndefValue(const ExplodedNode *N, const Stmt *S, BugReport ,
- bool IsArg = false,
bool EnableNullFPSuppression = true);
const Expr *getDerefExpr(const Stmt *S);
Index: cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -888,8 +888,7 @@
RetE = RetE->IgnoreParenCasts();
// If we're returning 0, we should track where that 0 came from.
-bugreporter::trackNullOrUndefValue(N, RetE, BR, /*IsArg*/ false,
- EnableNullFPSuppression);
+bugreporter::trackNullOrUndefValue(N, RetE, BR, EnableNullFPSuppression);
// Build an appropriate message based on the return value.
SmallString<64> Msg;
@@ -984,7 +983,7 @@
if (!State->isNull(*ArgV).isConstrainedTrue())
continue;
- if (bugreporter::trackNullOrUndefValue(N, ArgE, BR, /*IsArg=*/true,
+ if (bugreporter::trackNullOrUndefValue(N, ArgE, BR,
EnableNullFPSuppression))
ShouldInvalidate = false;
@@ -1264,7 +1263,7 @@
V.getAs() || V.getAs()) {
if (!IsParam)
InitE = InitE->IgnoreParenCasts();
- bugreporter::trackNullOrUndefValue(StoreSite, InitE, BR, IsParam,
+ bugreporter::trackNullOrUndefValue(StoreSite, InitE, BR,
EnableNullFPSuppression);
}
ReturnVisitor::addVisitorIfNecessary(StoreSite, InitE->IgnoreParenCasts(),
@@ -1516,6 +1515,8 @@
return nullptr;
}
+/// \return A subexpression of {@code Ex} which represents the
+/// expression-of-interest.
static const Expr *peelOffOuterExpr(const Expr *Ex,
const ExplodedNode *N) {
Ex = Ex->IgnoreParenCasts();
@@ -1560,125 +1561,73 @@
if (const Expr *SubEx = peelOffPointerArithmetic(BO))
return peelOffOuterExpr(SubEx, N);
- if (auto *UO = dyn_cast(Ex))
+ if (auto *UO = dyn_cast(Ex)) {
if (UO->getOpcode() == UO_LNot)
return peelOffOuterExpr(UO->getSubExpr(), N);
- return Ex;
-}
+// FIXME: There's a hack in our Store implementation that always computes
+// field offsets around null pointers as if they are always equal to 0.
+// The idea here is to report accesses to fields as null dereferences
+// even though the pointer value that's being dereferenced is actually
+// the offset of the field rather than exactly 0.
+// See the FIXME in StoreManager's getLValueFieldOrIvar() method.
+// This code interacts heavily with this hack; otherwise the value
+// would not be null at all for most fields, so we'd be unable to track it.
+if (UO->getOpcode() == UO_AddrOf && UO->getSubExpr()->isLValue())
+ if (const Expr *DerefEx = bugreporter::getDerefExpr(UO->getSubExpr()))
+return peelOffOuterExpr(DerefEx, N);
+ }
-/// Walk through nodes until we get one that matches the statement exactly.
-/// Alternately, if we hit a known lvalue for the statement, we know we've
-/// gone too far (though we can likely track the lvalue better anyway).
-static const ExplodedNode* findNodeForStatement(const ExplodedNode *N,
-
r343160 - [analyzer] [NFC] Move the code for dumping the program point to ProgramPoint
Author: george.karpenkov
Date: Wed Sep 26 18:46:18 2018
New Revision: 343160
URL: http://llvm.org/viewvc/llvm-project?rev=343160=rev
Log:
[analyzer] [NFC] Move the code for dumping the program point to ProgramPoint
So we can dump them outside of viewing the exploded grpah.
Differential Revision: https://reviews.llvm.org/D52583
Modified:
cfe/trunk/include/clang/Analysis/ProgramPoint.h
cfe/trunk/lib/Analysis/ProgramPoint.cpp
cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
Modified: cfe/trunk/include/clang/Analysis/ProgramPoint.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/ProgramPoint.h?rev=343160=343159=343160=diff
==
--- cfe/trunk/include/clang/Analysis/ProgramPoint.h (original)
+++ cfe/trunk/include/clang/Analysis/ProgramPoint.h Wed Sep 26 18:46:18 2018
@@ -215,6 +215,12 @@ public:
ID.AddPointer(getTag());
}
+ void print(StringRef CR, llvm::raw_ostream ) const;
+
+ LLVM_DUMP_METHOD void dump() const {
+return print(/*CR=*/"\n", llvm::errs());
+ }
+
static ProgramPoint getProgramPoint(const Stmt *S, ProgramPoint::Kind K,
const LocationContext *LC,
const ProgramPointTag *tag);
Modified: cfe/trunk/lib/Analysis/ProgramPoint.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/ProgramPoint.cpp?rev=343160=343159=343160=diff
==
--- cfe/trunk/lib/Analysis/ProgramPoint.cpp (original)
+++ cfe/trunk/lib/Analysis/ProgramPoint.cpp Wed Sep 26 18:46:18 2018
@@ -43,6 +43,177 @@ ProgramPoint ProgramPoint::getProgramPoi
}
}
+static void printLocation(raw_ostream , SourceLocation SLoc,
+ const SourceManager ,
+ StringRef CR,
+ StringRef Postfix) {
+ if (SLoc.isFileID()) {
+Out << CR << "line=" << SM.getExpansionLineNumber(SLoc)
+<< " col=" << SM.getExpansionColumnNumber(SLoc) << Postfix;
+ }
+}
+
+void ProgramPoint::print(StringRef CR, llvm::raw_ostream ) const {
+ const ASTContext =
+ getLocationContext()->getAnalysisDeclContext()->getASTContext();
+ const SourceManager = Context.getSourceManager();
+ switch (getKind()) {
+ case ProgramPoint::BlockEntranceKind:
+Out << "Block Entrance: B"
+<< castAs().getBlock()->getBlockID();
+break;
+
+ case ProgramPoint::FunctionExitKind: {
+auto FEP = getAs();
+Out << "Function Exit: B" << FEP->getBlock()->getBlockID();
+if (const ReturnStmt *RS = FEP->getStmt()) {
+ Out << CR << " Return: S" << RS->getID(Context) << CR;
+ RS->printPretty(Out, /*helper=*/nullptr, Context.getPrintingPolicy(),
+ /*Indentation=*/2, /*NewlineSymbol=*/CR);
+}
+break;
+ }
+ case ProgramPoint::BlockExitKind:
+assert(false);
+break;
+
+ case ProgramPoint::CallEnterKind:
+Out << "CallEnter";
+break;
+
+ case ProgramPoint::CallExitBeginKind:
+Out << "CallExitBegin";
+break;
+
+ case ProgramPoint::CallExitEndKind:
+Out << "CallExitEnd";
+break;
+
+ case ProgramPoint::PostStmtPurgeDeadSymbolsKind:
+Out << "PostStmtPurgeDeadSymbols";
+break;
+
+ case ProgramPoint::PreStmtPurgeDeadSymbolsKind:
+Out << "PreStmtPurgeDeadSymbols";
+break;
+
+ case ProgramPoint::EpsilonKind:
+Out << "Epsilon Point";
+break;
+
+ case ProgramPoint::LoopExitKind: {
+LoopExit LE = castAs();
+Out << "LoopExit: " << LE.getLoopStmt()->getStmtClassName();
+break;
+ }
+
+ case ProgramPoint::PreImplicitCallKind: {
+ImplicitCallPoint PC = castAs();
+Out << "PreCall: ";
+PC.getDecl()->print(Out, Context.getLangOpts());
+printLocation(Out, PC.getLocation(), SM, CR, /*Postfix=*/CR);
+break;
+ }
+
+ case ProgramPoint::PostImplicitCallKind: {
+ImplicitCallPoint PC = castAs();
+Out << "PostCall: ";
+PC.getDecl()->print(Out, Context.getLangOpts());
+printLocation(Out, PC.getLocation(), SM, CR, /*Postfix=*/CR);
+break;
+ }
+
+ case ProgramPoint::PostInitializerKind: {
+Out << "PostInitializer: ";
+const CXXCtorInitializer *Init =
castAs().getInitializer();
+if (const FieldDecl *FD = Init->getAnyMember())
+ Out << *FD;
+else {
+ QualType Ty = Init->getTypeSourceInfo()->getType();
+ Ty = Ty.getLocalUnqualifiedType();
+ Ty.print(Out, Context.getLangOpts());
+}
+break;
+ }
+
+ case ProgramPoint::BlockEdgeKind: {
+const BlockEdge = castAs();
+Out << "Edge: (B" << E.getSrc()->getBlockID() << ", B"
+<< E.getDst()->getBlockID() << ')';
+
+if (const Stmt *T = E.getSrc()->getTerminator()) {
+ SourceLocation SLoc = T->getBeginLoc();
+
+ Out << "\\|Terminator: ";
+ E.getSrc()->printTerminator(Out, Context.getLangOpts());
+ printLocation(Out, SLoc, SM, CR, /*Postfix=*/"");
+
+
[PATCH] D52519: [analyzer] [NFC] Heavy refactoring of trackNullOrUndefValue
This revision was automatically updated to reflect the committed changes.
Closed by commit rC343159: [analyzer] [NFC] Heavy refactoring of
trackNullOrUndefValue (authored by george.karpenkov, committed by ).
Herald added a subscriber: cfe-commits.
Changed prior to commit:
https://reviews.llvm.org/D52519?vs=166990=167231#toc
Repository:
rC Clang
https://reviews.llvm.org/D52519
Files:
include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
Index: lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
===
--- lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -888,8 +888,7 @@
RetE = RetE->IgnoreParenCasts();
// If we're returning 0, we should track where that 0 came from.
-bugreporter::trackNullOrUndefValue(N, RetE, BR, /*IsArg*/ false,
- EnableNullFPSuppression);
+bugreporter::trackNullOrUndefValue(N, RetE, BR, EnableNullFPSuppression);
// Build an appropriate message based on the return value.
SmallString<64> Msg;
@@ -984,7 +983,7 @@
if (!State->isNull(*ArgV).isConstrainedTrue())
continue;
- if (bugreporter::trackNullOrUndefValue(N, ArgE, BR, /*IsArg=*/true,
+ if (bugreporter::trackNullOrUndefValue(N, ArgE, BR,
EnableNullFPSuppression))
ShouldInvalidate = false;
@@ -1264,7 +1263,7 @@
V.getAs() || V.getAs()) {
if (!IsParam)
InitE = InitE->IgnoreParenCasts();
- bugreporter::trackNullOrUndefValue(StoreSite, InitE, BR, IsParam,
+ bugreporter::trackNullOrUndefValue(StoreSite, InitE, BR,
EnableNullFPSuppression);
}
ReturnVisitor::addVisitorIfNecessary(StoreSite, InitE->IgnoreParenCasts(),
@@ -1516,6 +1515,8 @@
return nullptr;
}
+/// \return A subexpression of {@code Ex} which represents the
+/// expression-of-interest.
static const Expr *peelOffOuterExpr(const Expr *Ex,
const ExplodedNode *N) {
Ex = Ex->IgnoreParenCasts();
@@ -1560,125 +1561,73 @@
if (const Expr *SubEx = peelOffPointerArithmetic(BO))
return peelOffOuterExpr(SubEx, N);
- if (auto *UO = dyn_cast(Ex))
+ if (auto *UO = dyn_cast(Ex)) {
if (UO->getOpcode() == UO_LNot)
return peelOffOuterExpr(UO->getSubExpr(), N);
- return Ex;
-}
+// FIXME: There's a hack in our Store implementation that always computes
+// field offsets around null pointers as if they are always equal to 0.
+// The idea here is to report accesses to fields as null dereferences
+// even though the pointer value that's being dereferenced is actually
+// the offset of the field rather than exactly 0.
+// See the FIXME in StoreManager's getLValueFieldOrIvar() method.
+// This code interacts heavily with this hack; otherwise the value
+// would not be null at all for most fields, so we'd be unable to track it.
+if (UO->getOpcode() == UO_AddrOf && UO->getSubExpr()->isLValue())
+ if (const Expr *DerefEx = bugreporter::getDerefExpr(UO->getSubExpr()))
+return peelOffOuterExpr(DerefEx, N);
+ }
-/// Walk through nodes until we get one that matches the statement exactly.
-/// Alternately, if we hit a known lvalue for the statement, we know we've
-/// gone too far (though we can likely track the lvalue better anyway).
-static const ExplodedNode* findNodeForStatement(const ExplodedNode *N,
-const Stmt *S,
-const Expr *Inner) {
- do {
-const ProgramPoint = N->getLocation();
-if (auto ps = pp.getAs()) {
- if (ps->getStmt() == S || ps->getStmt() == Inner)
-break;
-} else if (auto CEE = pp.getAs()) {
- if (CEE->getCalleeContext()->getCallSite() == S ||
- CEE->getCalleeContext()->getCallSite() == Inner)
-break;
-}
-N = N->getFirstPred();
- } while (N);
- return N;
+ return Ex;
}
/// Find the ExplodedNode where the lvalue (the value of 'Ex')
/// was computed.
static const ExplodedNode* findNodeForExpression(const ExplodedNode *N,
-const Expr *Inner) {
+ const Expr *Inner) {
while (N) {
-if (auto P = N->getLocation().getAs()) {
- if (P->getStmt() == Inner)
-break;
-}
+if (PathDiagnosticLocation::getStmt(N) == Inner)
+ return N;
N = N->getFirstPred();
}
- assert(N && "Unable to find the lvalue node.");
return N;
}
-/// Performing operator `&' on an lvalue expression is essentially a no-op.
-/// Then, if we are taking addresses of fields or elements, these are also
-/// unlikely to matter.
-static const Expr* peelOfOuterAddrOf(const Expr* Ex) {
- Ex = Ex->IgnoreParenCasts();
-
- // FIXME: There's a hack
r343159 - [analyzer] [NFC] Heavy refactoring of trackNullOrUndefValue
Author: george.karpenkov
Date: Wed Sep 26 18:45:57 2018
New Revision: 343159
URL: http://llvm.org/viewvc/llvm-project?rev=343159=rev
Log:
[analyzer] [NFC] Heavy refactoring of trackNullOrUndefValue
Differential Revision: https://reviews.llvm.org/D52519
Modified:
cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
Modified:
cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h?rev=343159=343158=343159=diff
==
---
cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
(original)
+++
cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
Wed Sep 26 18:45:57 2018
@@ -363,9 +363,6 @@ namespace bugreporter {
/// \param N A node "downstream" from the evaluation of the statement.
/// \param S The statement whose value is null or undefined.
/// \param R The bug report to which visitors should be attached.
-/// \param IsArg Whether the statement is an argument to an inlined function.
-/// If this is the case, \p N \em must be the CallEnter node for
-/// the function.
/// \param EnableNullFPSuppression Whether we should employ false positive
/// suppression (inlined defensive checks, returned null).
///
@@ -373,7 +370,6 @@ namespace bugreporter {
/// statement. Note that returning \c true does not actually imply
/// that any visitors were added.
bool trackNullOrUndefValue(const ExplodedNode *N, const Stmt *S, BugReport ,
- bool IsArg = false,
bool EnableNullFPSuppression = true);
const Expr *getDerefExpr(const Stmt *S);
Modified: cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp?rev=343159=343158=343159=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp Wed Sep 26
18:45:57 2018
@@ -888,8 +888,7 @@ public:
RetE = RetE->IgnoreParenCasts();
// If we're returning 0, we should track where that 0 came from.
-bugreporter::trackNullOrUndefValue(N, RetE, BR, /*IsArg*/ false,
- EnableNullFPSuppression);
+bugreporter::trackNullOrUndefValue(N, RetE, BR, EnableNullFPSuppression);
// Build an appropriate message based on the return value.
SmallString<64> Msg;
@@ -984,7 +983,7 @@ public:
if (!State->isNull(*ArgV).isConstrainedTrue())
continue;
- if (bugreporter::trackNullOrUndefValue(N, ArgE, BR, /*IsArg=*/true,
+ if (bugreporter::trackNullOrUndefValue(N, ArgE, BR,
EnableNullFPSuppression))
ShouldInvalidate = false;
@@ -1264,7 +1263,7 @@ FindLastStoreBRVisitor::VisitNode(const
V.getAs() || V.getAs()) {
if (!IsParam)
InitE = InitE->IgnoreParenCasts();
- bugreporter::trackNullOrUndefValue(StoreSite, InitE, BR, IsParam,
+ bugreporter::trackNullOrUndefValue(StoreSite, InitE, BR,
EnableNullFPSuppression);
}
ReturnVisitor::addVisitorIfNecessary(StoreSite, InitE->IgnoreParenCasts(),
@@ -1516,6 +1515,8 @@ static const MemRegion *getLocationRegio
return nullptr;
}
+/// \return A subexpression of {@code Ex} which represents the
+/// expression-of-interest.
static const Expr *peelOffOuterExpr(const Expr *Ex,
const ExplodedNode *N) {
Ex = Ex->IgnoreParenCasts();
@@ -1560,125 +1561,73 @@ static const Expr *peelOffOuterExpr(cons
if (const Expr *SubEx = peelOffPointerArithmetic(BO))
return peelOffOuterExpr(SubEx, N);
- if (auto *UO = dyn_cast(Ex))
+ if (auto *UO = dyn_cast(Ex)) {
if (UO->getOpcode() == UO_LNot)
return peelOffOuterExpr(UO->getSubExpr(), N);
- return Ex;
-}
+// FIXME: There's a hack in our Store implementation that always computes
+// field offsets around null pointers as if they are always equal to 0.
+// The idea here is to report accesses to fields as null dereferences
+// even though the pointer value that's being dereferenced is actually
+// the offset of the field rather than exactly 0.
+// See the FIXME in StoreManager's getLValueFieldOrIvar() method.
+// This code interacts heavily with this hack; otherwise the value
+// would not be null at all for most fields, so we'd be unable to track it.
+if (UO->getOpcode() == UO_AddrOf && UO->getSubExpr()->isLValue())
+ if (const Expr *DerefEx =
[PATCH] D52586: [X86] Add the movbe instruction intrinsics from icc.
craig.topper created this revision.
craig.topper added reviewers: spatel, RKSimon.
These intrinsics exist in icc. They can be found on the Intel Intrinsics Guide
website.
All the backend support is in place to pattern match a load+bswap or a
bswap+store pattern to the MOVBE instructions. So we just need to get the
frontend to emit the correct IR. The pointer arguments in icc are declared as
void so I had to jump through a packed struct to forcing a specific alignment
on the load/store. Same trick we use in the unaligned vector load/store
intrinsics
https://reviews.llvm.org/D52586
Files:
lib/Basic/Targets/X86.cpp
lib/Headers/immintrin.h
test/CodeGen/movbe-builtins.c
Index: test/CodeGen/movbe-builtins.c
===
--- /dev/null
+++ test/CodeGen/movbe-builtins.c
@@ -0,0 +1,49 @@
+// RUN: %clang_cc1 -ffreestanding %s -triple=x86_64-apple-darwin -target-feature +movbe -emit-llvm -o - | FileCheck %s --check-prefixes=CHECK,CHECK-X64
+// RUN: %clang_cc1 -ffreestanding %s -triple=i686-apple-darwin -target-feature +movbe -emit-llvm -o - | FileCheck %s
+
+
+#include
+
+short test_loadbe_i16(const short *P) {
+ // CHECK-LABEL: @test_loadbe_i16
+ // CHECK: [[LOAD:%.*]] = load i16, i16* %{{.*}}, align 1
+ // CHECK: call i16 @llvm.bswap.i16(i16 [[LOAD]])
+ return _loadbe_i16(P);
+}
+
+void test_storebe_i16(short *P, short D) {
+ // CHECK-LABEL: @test_storebe_i16
+ // CHECK: [[DATA:%.*]] = call i16 @llvm.bswap.i16(i16 %{{.*}})
+ // CHECK: store i16 [[DATA]], i16* %{{.*}}, align 1
+ _storebe_i16(P, D);
+}
+
+int test_loadbe_i32(const int *P) {
+ // CHECK-LABEL: @test_loadbe_i32
+ // CHECK: [[LOAD:%.*]] = load i32, i32* %{{.*}}, align 1
+ // CHECK: call i32 @llvm.bswap.i32(i32 [[LOAD]])
+ return _loadbe_i32(P);
+}
+
+void test_storebe_i32(int *P, int D) {
+ // CHECK-LABEL: @test_storebe_i32
+ // CHECK: [[DATA:%.*]] = call i32 @llvm.bswap.i32(i32 %{{.*}})
+ // CHECK: store i32 [[DATA]], i32* %{{.*}}, align 1
+ _storebe_i32(P, D);
+}
+
+#ifdef __x86_64__
+long long test_loadbe_i64(const long long *P) {
+ // CHECK-X64-LABEL: @test_loadbe_i64
+ // CHECK-X64: [[LOAD:%.*]] = load i64, i64* %{{.*}}, align 1
+ // CHECK-X64: call i64 @llvm.bswap.i64(i64 [[LOAD]])
+ return _loadbe_i64(P);
+}
+
+void test_storebe_i64(long long *P, long long D) {
+ // CHECK-X64-LABEL: @test_storebe_i64
+ // CHECK-X64: [[DATA:%.*]] = call i64 @llvm.bswap.i64(i64 %{{.*}})
+ // CHECK-X64: store i64 [[DATA]], i64* %{{.*}}, align 1
+ _storebe_i64(P, D);
+}
+#endif
Index: lib/Headers/immintrin.h
===
--- lib/Headers/immintrin.h
+++ lib/Headers/immintrin.h
@@ -306,6 +306,58 @@
#endif
#endif /* __FSGSBASE__ */
+#if !defined(_MSC_VER) || __has_feature(modules) || defined(__MOVBE__)
+static __inline__ short __attribute__((__always_inline__, __nodebug__, __target__("movbe")))
+_loadbe_i16(void const * __P) {
+ struct __loadu_i16 {
+short __v;
+ } __attribute__((__packed__, __may_alias__));
+ return __builtin_bswap16(((struct __loadu_i16*)__P)->__v);
+}
+
+static __inline__ void __attribute__((__always_inline__, __nodebug__, __target__("movbe")))
+_storebe_i16(void * __P, short __D) {
+ struct __storeu_i16 {
+short __v;
+ } __attribute__((__packed__, __may_alias__));
+ ((struct __storeu_i16*)__P)->__v = __builtin_bswap16(__D);
+}
+
+static __inline__ int __attribute__((__always_inline__, __nodebug__, __target__("movbe")))
+_loadbe_i32(void const * __P) {
+ struct __loadu_i32 {
+int __v;
+ } __attribute__((__packed__, __may_alias__));
+ return __builtin_bswap32(((struct __loadu_i32*)__P)->__v);
+}
+
+static __inline__ void __attribute__((__always_inline__, __nodebug__, __target__("movbe")))
+_storebe_i32(void * __P, int __D) {
+ struct __storeu_i32 {
+int __v;
+ } __attribute__((__packed__, __may_alias__));
+ ((struct __storeu_i32*)__P)->__v = __builtin_bswap32(__D);
+}
+
+#ifdef __x86_64__
+static __inline__ long long __attribute__((__always_inline__, __nodebug__, __target__("movbe")))
+_loadbe_i64(void const * __P) {
+ struct __loadu_i64 {
+long long __v;
+ } __attribute__((__packed__, __may_alias__));
+ return __builtin_bswap64(((struct __loadu_i64*)__P)->__v);
+}
+
+static __inline__ void __attribute__((__always_inline__, __nodebug__, __target__("movbe")))
+_storebe_i64(void * __P, long long __D) {
+ struct __storeu_i64 {
+long long __v;
+ } __attribute__((__packed__, __may_alias__));
+ ((struct __storeu_i64*)__P)->__v = __builtin_bswap64(__D);
+}
+#endif
+#endif /* __MOVEBE */
+
#if !defined(_MSC_VER) || __has_feature(modules) || defined(__RTM__)
#include
#include
Index: lib/Basic/Targets/X86.cpp
===
--- lib/Basic/Targets/X86.cpp
+++ lib/Basic/Targets/X86.cpp
@@ -1081,6 +1081,9 @@
if (HasMWAITX)
Builder.defineMacro("__MWAITX__");
+ if (HasMOVBE)
+
[PATCH] D52585: [analyzer] [testing] Pass through an extra argument for specifying extra analyzer options
This revision was automatically updated to reflect the committed changes.
Closed by commit rC343158: [analyzer] [testing] Pass through an extra argument
for specifying extra… (authored by george.karpenkov, committed by ).
Herald added a subscriber: cfe-commits.
Changed prior to commit:
https://reviews.llvm.org/D52585?vs=167226=167227#toc
Repository:
rC Clang
https://reviews.llvm.org/D52585
Files:
utils/analyzer/SATestBuild.py
Index: utils/analyzer/SATestBuild.py
===
--- utils/analyzer/SATestBuild.py
+++ utils/analyzer/SATestBuild.py
@@ -255,7 +255,7 @@
sys.exit(1)
-def runScanBuild(Dir, SBOutputDir, PBuildLogFile):
+def runScanBuild(Args, Dir, SBOutputDir, PBuildLogFile):
"""
Build the project with scan-build by reading in the commands and
prefixing them with the scan-build options.
@@ -281,9 +281,11 @@
("stable-report-filename", "true"),
("serialize-stats", "true"),
]
-
-SBOptions += "-analyzer-config '%s' " % (
-",".join("%s=%s" % (key, value) for (key, value) in AnalyzerConfig))
+AnalyzerConfigSerialized = ",".join(
+"%s=%s" % (key, value) for (key, value) in AnalyzerConfig)
+if Args.extra_args:
+AnalyzerConfigSerialized += "," + Args.extra_args
+SBOptions += "-analyzer-config '%s' " % AnalyzerConfigSerialized
# Always use ccc-analyze to ensure that we can locate the failures
# directory.
@@ -407,7 +409,7 @@
check_call(RmCommand, shell=True)
-def buildProject(Dir, SBOutputDir, ProjectBuildMode, IsReferenceBuild):
+def buildProject(Args, Dir, SBOutputDir, ProjectBuildMode, IsReferenceBuild):
TBegin = time.time()
BuildLogPath = getBuildLogPath(SBOutputDir)
@@ -431,7 +433,7 @@
if (ProjectBuildMode == 1):
downloadAndPatch(Dir, PBuildLogFile)
runCleanupScript(Dir, PBuildLogFile)
-runScanBuild(Dir, SBOutputDir, PBuildLogFile)
+runScanBuild(Args, Dir, SBOutputDir, PBuildLogFile)
else:
runAnalyzePreprocessed(Dir, SBOutputDir, ProjectBuildMode)
@@ -628,12 +630,13 @@
class TestProjectThread(threading.Thread):
-def __init__(self, TasksQueue, ResultsDiffer, FailureFlag):
+def __init__(self, Args, TasksQueue, ResultsDiffer, FailureFlag):
"""
:param ResultsDiffer: Used to signify that results differ from
the canonical ones.
:param FailureFlag: Used to signify a failure during the run.
"""
+self.Args = Args
self.TasksQueue = TasksQueue
self.ResultsDiffer = ResultsDiffer
self.FailureFlag = FailureFlag
@@ -649,15 +652,15 @@
Logger = logging.getLogger(ProjArgs[0])
Local.stdout = StreamToLogger(Logger, logging.INFO)
Local.stderr = StreamToLogger(Logger, logging.ERROR)
-if not testProject(*ProjArgs):
+if not testProject(Args, *ProjArgs):
self.ResultsDiffer.set()
self.TasksQueue.task_done()
except:
self.FailureFlag.set()
raise
-def testProject(ID, ProjectBuildMode, IsReferenceBuild=False, Strictness=0):
+def testProject(Args, ID, ProjectBuildMode, IsReferenceBuild=False, Strictness=0):
"""
Test a given project.
:return TestsPassed: Whether tests have passed according
@@ -675,7 +678,7 @@
RelOutputDir = getSBOutputDirName(IsReferenceBuild)
SBOutputDir = os.path.join(Dir, RelOutputDir)
-buildProject(Dir, SBOutputDir, ProjectBuildMode, IsReferenceBuild)
+buildProject(Args, Dir, SBOutputDir, ProjectBuildMode, IsReferenceBuild)
checkBuild(SBOutputDir)
@@ -719,17 +722,17 @@
" (single file), 1 (project), or 2(single file c++11)."
raise Exception()
-def singleThreadedTestAll(ProjectsToTest):
+def singleThreadedTestAll(Args, ProjectsToTest):
"""
Run all projects.
:return: whether tests have passed.
"""
Success = True
for ProjArgs in ProjectsToTest:
-Success &= testProject(*ProjArgs)
+Success &= testProject(Args, *ProjArgs)
return Success
-def multiThreadedTestAll(ProjectsToTest, Jobs):
+def multiThreadedTestAll(Args, ProjectsToTest, Jobs):
"""
Run each project in a separate thread.
@@ -747,7 +750,7 @@
FailureFlag = threading.Event()
for i in range(Jobs):
-T = TestProjectThread(TasksQueue, ResultsDiffer, FailureFlag)
+T = TestProjectThread(Args, TasksQueue, ResultsDiffer, FailureFlag)
T.start()
# Required to handle Ctrl-C gracefully.
@@ -772,9 +775,9 @@
Args.regenerate,
Args.strictness))
if Args.jobs <= 1:
-return singleThreadedTestAll(ProjectsToTest)
+return singleThreadedTestAll(Args, ProjectsToTest)
else:
-
r343158 - [analyzer] [testing] Pass through an extra argument for specifying extra analyzer options
Author: george.karpenkov
Date: Wed Sep 26 18:10:59 2018
New Revision: 343158
URL: http://llvm.org/viewvc/llvm-project?rev=343158=rev
Log:
[analyzer] [testing] Pass through an extra argument for specifying extra
analyzer options
Differential Revision: https://reviews.llvm.org/D52585
Modified:
cfe/trunk/utils/analyzer/SATestBuild.py
Modified: cfe/trunk/utils/analyzer/SATestBuild.py
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/utils/analyzer/SATestBuild.py?rev=343158=343157=343158=diff
==
--- cfe/trunk/utils/analyzer/SATestBuild.py (original)
+++ cfe/trunk/utils/analyzer/SATestBuild.py Wed Sep 26 18:10:59 2018
@@ -255,7 +255,7 @@ def applyPatch(Dir, PBuildLogFile):
sys.exit(1)
-def runScanBuild(Dir, SBOutputDir, PBuildLogFile):
+def runScanBuild(Args, Dir, SBOutputDir, PBuildLogFile):
"""
Build the project with scan-build by reading in the commands and
prefixing them with the scan-build options.
@@ -281,9 +281,11 @@ def runScanBuild(Dir, SBOutputDir, PBuil
("stable-report-filename", "true"),
("serialize-stats", "true"),
]
-
-SBOptions += "-analyzer-config '%s' " % (
-",".join("%s=%s" % (key, value) for (key, value) in AnalyzerConfig))
+AnalyzerConfigSerialized = ",".join(
+"%s=%s" % (key, value) for (key, value) in AnalyzerConfig)
+if Args.extra_args:
+AnalyzerConfigSerialized += "," + Args.extra_args
+SBOptions += "-analyzer-config '%s' " % AnalyzerConfigSerialized
# Always use ccc-analyze to ensure that we can locate the failures
# directory.
@@ -407,7 +409,7 @@ def removeLogFile(SBOutputDir):
check_call(RmCommand, shell=True)
-def buildProject(Dir, SBOutputDir, ProjectBuildMode, IsReferenceBuild):
+def buildProject(Args, Dir, SBOutputDir, ProjectBuildMode, IsReferenceBuild):
TBegin = time.time()
BuildLogPath = getBuildLogPath(SBOutputDir)
@@ -431,7 +433,7 @@ def buildProject(Dir, SBOutputDir, Proje
if (ProjectBuildMode == 1):
downloadAndPatch(Dir, PBuildLogFile)
runCleanupScript(Dir, PBuildLogFile)
-runScanBuild(Dir, SBOutputDir, PBuildLogFile)
+runScanBuild(Args, Dir, SBOutputDir, PBuildLogFile)
else:
runAnalyzePreprocessed(Dir, SBOutputDir, ProjectBuildMode)
@@ -628,12 +630,13 @@ def cleanupReferenceResults(SBOutputDir)
class TestProjectThread(threading.Thread):
-def __init__(self, TasksQueue, ResultsDiffer, FailureFlag):
+def __init__(self, Args, TasksQueue, ResultsDiffer, FailureFlag):
"""
:param ResultsDiffer: Used to signify that results differ from
the canonical ones.
:param FailureFlag: Used to signify a failure during the run.
"""
+self.Args = Args
self.TasksQueue = TasksQueue
self.ResultsDiffer = ResultsDiffer
self.FailureFlag = FailureFlag
@@ -649,7 +652,7 @@ class TestProjectThread(threading.Thread
Logger = logging.getLogger(ProjArgs[0])
Local.stdout = StreamToLogger(Logger, logging.INFO)
Local.stderr = StreamToLogger(Logger, logging.ERROR)
-if not testProject(*ProjArgs):
+if not testProject(Args, *ProjArgs):
self.ResultsDiffer.set()
self.TasksQueue.task_done()
except:
@@ -657,7 +660,7 @@ class TestProjectThread(threading.Thread
raise
-def testProject(ID, ProjectBuildMode, IsReferenceBuild=False, Strictness=0):
+def testProject(Args, ID, ProjectBuildMode, IsReferenceBuild=False,
Strictness=0):
"""
Test a given project.
:return TestsPassed: Whether tests have passed according
@@ -675,7 +678,7 @@ def testProject(ID, ProjectBuildMode, Is
RelOutputDir = getSBOutputDirName(IsReferenceBuild)
SBOutputDir = os.path.join(Dir, RelOutputDir)
-buildProject(Dir, SBOutputDir, ProjectBuildMode, IsReferenceBuild)
+buildProject(Args, Dir, SBOutputDir, ProjectBuildMode, IsReferenceBuild)
checkBuild(SBOutputDir)
@@ -719,17 +722,17 @@ def validateProjectFile(PMapFile):
" (single file), 1 (project), or 2(single file c++11)."
raise Exception()
-def singleThreadedTestAll(ProjectsToTest):
+def singleThreadedTestAll(Args, ProjectsToTest):
"""
Run all projects.
:return: whether tests have passed.
"""
Success = True
for ProjArgs in ProjectsToTest:
-Success &= testProject(*ProjArgs)
+Success &= testProject(Args, *ProjArgs)
return Success
-def multiThreadedTestAll(ProjectsToTest, Jobs):
+def multiThreadedTestAll(Args, ProjectsToTest, Jobs):
"""
Run each project in a separate thread.
@@ -747,7 +750,7 @@ def multiThreadedTestAll(ProjectsToTest,
FailureFlag = threading.Event()
for i in range(Jobs):
-T =
[PATCH] D52527: [clang-format] fix Bug 38686: add AfterCaseLabel to BraceWrapping
owenpan updated this revision to Diff 167225.
owenpan added a comment.
Updated ClangFormatStyleOptions.rst.
Repository:
rC Clang
https://reviews.llvm.org/D52527
Files:
docs/ClangFormatStyleOptions.rst
include/clang/Format/Format.h
lib/Format/Format.cpp
lib/Format/UnwrappedLineParser.cpp
unittests/Format/FormatTest.cpp
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -1069,6 +1069,7 @@
Style.IndentCaseLabels = true;
Style.AllowShortBlocksOnASingleLine = false;
Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+ Style.BraceWrapping.AfterCaseLabel = true;
Style.BraceWrapping.AfterControlStatement = true;
EXPECT_EQ("switch (n)\n"
"{\n"
@@ -1090,6 +1091,27 @@
" }\n"
"}",
Style));
+ Style.BraceWrapping.AfterCaseLabel = false;
+ EXPECT_EQ("switch (n)\n"
+"{\n"
+" case 0: {\n"
+"return false;\n"
+" }\n"
+" default: {\n"
+"return true;\n"
+" }\n"
+"}",
+format("switch (n) {\n"
+ " case 0:\n"
+ " {\n"
+ "return false;\n"
+ " }\n"
+ " default:\n"
+ " {\n"
+ "return true;\n"
+ " }\n"
+ "}",
+ Style));
}
TEST_F(FormatTest, CaseRanges) {
@@ -1243,6 +1265,7 @@
Style));
Style.AllowShortCaseLabelsOnASingleLine = true;
Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+ Style.BraceWrapping.AfterCaseLabel = true;
Style.BraceWrapping.AfterControlStatement = true;
EXPECT_EQ("switch (n)\n"
"{\n"
@@ -10845,6 +10868,7 @@
CHECK_PARSE_BOOL(SpaceBeforeInheritanceColon);
CHECK_PARSE_BOOL(SpaceBeforeRangeBasedForLoopColon);
+ CHECK_PARSE_NESTED_BOOL(BraceWrapping, AfterCaseLabel);
CHECK_PARSE_NESTED_BOOL(BraceWrapping, AfterClass);
CHECK_PARSE_NESTED_BOOL(BraceWrapping, AfterControlStatement);
CHECK_PARSE_NESTED_BOOL(BraceWrapping, AfterEnum);
Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -173,10 +173,16 @@
public:
CompoundStatementIndenter(UnwrappedLineParser *Parser,
const FormatStyle , unsigned )
+ : CompoundStatementIndenter(Parser, LineLevel,
+ Style.BraceWrapping.AfterControlStatement,
+ Style.BraceWrapping.IndentBraces) {
+ }
+ CompoundStatementIndenter(UnwrappedLineParser *Parser, unsigned ,
+bool WrapeBrace, bool IndentBrace)
: LineLevel(LineLevel), OldLineLevel(LineLevel) {
-if (Style.BraceWrapping.AfterControlStatement)
+if (WrapeBrace)
Parser->addUnwrappedLine();
-if (Style.BraceWrapping.IndentBraces)
+if (IndentBrace)
++LineLevel;
}
~CompoundStatementIndenter() { LineLevel = OldLineLevel; }
@@ -1888,7 +1894,9 @@
if (Line->Level > 1 || (!Line->InPPDirective && Line->Level > 0))
--Line->Level;
if (CommentsBeforeNextToken.empty() && FormatTok->Tok.is(tok::l_brace)) {
-CompoundStatementIndenter Indenter(this, Style, Line->Level);
+CompoundStatementIndenter Indenter(this, Line->Level,
+ Style.BraceWrapping.AfterCaseLabel,
+ Style.BraceWrapping.IndentBraces);
parseBlock(/*MustBeDeclaration=*/false);
if (FormatTok->Tok.is(tok::kw_break)) {
if (Style.BraceWrapping.AfterControlStatement)
Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -476,6 +476,7 @@
template <> struct MappingTraits {
static void mapping(IO , FormatStyle::BraceWrappingFlags ) {
+IO.mapOptional("AfterCaseLabel", Wrapping.AfterCaseLabel);
IO.mapOptional("AfterClass", Wrapping.AfterClass);
IO.mapOptional("AfterControlStatement", Wrapping.AfterControlStatement);
IO.mapOptional("AfterEnum", Wrapping.AfterEnum);
@@ -568,7 +569,7 @@
if (Style.BreakBeforeBraces == FormatStyle::BS_Custom)
return Style;
FormatStyle Expanded = Style;
- Expanded.BraceWrapping = {false, false, false, false, false,
+ Expanded.BraceWrapping = {false, false, false, false, false, false,
false, false, false, false, false,
false, false, true, true, true};
switch (Style.BreakBeforeBraces) {
@@ -593,6 +594,7 @@
Expanded.BraceWrapping.BeforeElse = true;
break;
case FormatStyle::BS_Allman:
+Expanded.BraceWrapping.AfterCaseLabel = true;
[PATCH] D50901: [clang][ubsan] Split Implicit Integer Truncation Sanitizer into unsigned and signed checks
vitalybuka added inline comments.
Comment at: lib/CodeGen/CGExprScalar.cpp:305
enum ImplicitConversionCheckKind : unsigned char {
-ICCK_IntegerTruncation = 0,
+ICCK_IntegerTruncation = 0, // Legacy, no longer used.
+ICCK_UnsignedIntegerTruncation = 1,
lebedev.ri wrote:
> vitalybuka wrote:
> > why do you need to keep it?
> *Here* - for consistency with the compiler-rt part.
>
> There - what about mismatch in the used clang version
> (which still only produced the `(ImplicitConversionCheckKind)0`), and
> compiler-rt version
> (which has `(ImplicitConversionCheckKind)1` and
> `(ImplicitConversionCheckKind)2`)?
> Is it 100.00% guaranteed not to happen? I'm not so sure.
I don't think we try support mismatched versions of clang and compiler-rt
Repository:
rC Clang
https://reviews.llvm.org/D50901
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52581: [AST] Revert mangling changes from r339428
smeenai created this revision.
smeenai added reviewers: compnerd, rjmccall, theraven, DHowett-MSFT.
As discussed in https://reviews.llvm.org/D50144, we want Obj-C classes
to have the same mangling as C++ structs, to support headers like the
following:
#ifdef __OBJC__
@class I;
#else
struct I;
#endif
void f(I *);
since the header can be used from both C++ and Obj-C++ TUs, and we want
a consistent mangling across the two to prevent link errors. Itanium
mangles both the same way, and so should the MS ABI.
The main concern with having the same mangling for C++ structs and Obj-C
classes was that we want to treat them differently for the purposes of
exception handling, e.g. we don't want a C++ catch statement for a
struct to be able to catch an Obj-C class with the same name as the
struct. We can accomplish this by mangling Obj-C class names differently
in their RTTI, which I'll do in https://reviews.llvm.org/D47233. I would
have done the same for the GNUstep RTTI here, except I don't actually
see the code for that anywhere, and no tests seem to break either, so I
believe it's not upstreamed yet.
Repository:
rC Clang
https://reviews.llvm.org/D52581
Files:
lib/AST/MicrosoftMangle.cpp
test/CodeGenObjCXX/arc-marker-funclet.mm
test/CodeGenObjCXX/microsoft-abi-arc-param-order.mm
test/CodeGenObjCXX/msabi-objc-extensions.mm
test/CodeGenObjCXX/msabi-objc-types.mm
Index: test/CodeGenObjCXX/msabi-objc-types.mm
===
--- test/CodeGenObjCXX/msabi-objc-types.mm
+++ test/CodeGenObjCXX/msabi-objc-types.mm
@@ -3,166 +3,166 @@
@class I;
id kid;
-// CHECK: @"?kid@@3PAU.objc_object@@A" = dso_local global
+// CHECK: @"?kid@@3PAUobjc_object@@A" = dso_local global
Class klass;
-// CHECK: @"?klass@@3PAU.objc_class@@A" = dso_local global
+// CHECK: @"?klass@@3PAUobjc_class@@A" = dso_local global
I *kI;
-// CHECK: @"?kI@@3PAU.objc_cls_I@@A" = dso_local global
+// CHECK: @"?kI@@3PAUI@@A" = dso_local global
void f(I *) {}
-// CHECK-LABEL: "?f@@YAXPAU.objc_cls_I@@@Z"
+// CHECK-LABEL: "?f@@YAXPAUI@@@Z"
void f(const I *) {}
-// CHECK-LABEL: "?f@@YAXPBU.objc_cls_I@@@Z"
+// CHECK-LABEL: "?f@@YAXPBUI@@@Z"
void f(I &) {}
-// CHECK-LABEL: "?f@@YAXAAU.objc_cls_I@@@Z"
+// CHECK-LABEL: "?f@@YAXAAUI@@@Z"
void f(const I &) {}
-// CHECK-LABEL: "?f@@YAXABU.objc_cls_I@@@Z"
+// CHECK-LABEL: "?f@@YAXABUI@@@Z"
void f(const I &&) {}
-// CHECK-LABEL: "?f@@YAX$$QBU.objc_cls_I@@@Z"
+// CHECK-LABEL: "?f@@YAX$$QBUI@@@Z"
void g(id) {}
-// CHECK-LABEL: "?g@@YAXPAU.objc_object@@@Z"
+// CHECK-LABEL: "?g@@YAXPAUobjc_object@@@Z"
void g(id &) {}
-// CHECK-LABEL: "?g@@YAXAAPAU.objc_object@@@Z"
+// CHECK-LABEL: "?g@@YAXAAPAUobjc_object@@@Z"
void g(const id &) {}
-// CHECK-LABEL: "?g@@YAXABQAU.objc_object@@@Z"
+// CHECK-LABEL: "?g@@YAXABQAUobjc_object@@@Z"
void g(id &&) {}
-// CHECK-LABEL: "?g@@YAX$$QAPAU.objc_object@@@Z"
+// CHECK-LABEL: "?g@@YAX$$QAPAUobjc_object@@@Z"
void h(Class) {}
-// CHECK-LABEL: "?h@@YAXPAU.objc_class@@@Z"
+// CHECK-LABEL: "?h@@YAXPAUobjc_class@@@Z"
void h(Class &) {}
-// CHECK-LABEL: "?h@@YAXAAPAU.objc_class@@@Z"
+// CHECK-LABEL: "?h@@YAXAAPAUobjc_class@@@Z"
void h(const Class &) {}
-// CHECK-LABEL: "?h@@YAXABQAU.objc_class@@@Z"
+// CHECK-LABEL: "?h@@YAXABQAUobjc_class@@@Z"
void h(Class &&) {}
-// CHECK-LABEL: "?h@@YAX$$QAPAU.objc_class@@@Z"
+// CHECK-LABEL: "?h@@YAX$$QAPAUobjc_class@@@Z"
I *i() { return nullptr; }
-// CHECK-LABEL: "?i@@YAPAU.objc_cls_I@@XZ"
+// CHECK-LABEL: "?i@@YAPAUI@@XZ"
const I *j() { return nullptr; }
-// CHECK-LABEL: "?j@@YAPBU.objc_cls_I@@XZ"
+// CHECK-LABEL: "?j@@YAPBUI@@XZ"
I () { return *kI; }
-// CHECK-LABEL: "?k@@YAAAU.objc_cls_I@@XZ"
+// CHECK-LABEL: "?k@@YAAAUI@@XZ"
const I () { return *kI; }
-// CHECK-LABEL: "?l@@YAABU.objc_cls_I@@XZ"
+// CHECK-LABEL: "?l@@YAABUI@@XZ"
void m(const id) {}
-// CHECK-LABEL: "?m@@YAXQAU.objc_object@@@Z"
+// CHECK-LABEL: "?m@@YAXQAUobjc_object@@@Z"
void m(const I *) {}
-// CHECK-LABEL: "?m@@YAXPBU.objc_cls_I@@@Z"
+// CHECK-LABEL: "?m@@YAXPBUI@@@Z"
void n(SEL) {}
-// CHECK-LABEL: "?n@@YAXPAU.objc_selector@@@Z"
+// CHECK-LABEL: "?n@@YAXPAUobjc_selector@@@Z"
void n(SEL *) {}
-// CHECK-LABEL: "?n@@YAXPAPAU.objc_selector@@@Z"
+// CHECK-LABEL: "?n@@YAXPAPAUobjc_selector@@@Z"
void n(const SEL *) {}
-// CHECK-LABEL: "?n@@YAXPBQAU.objc_selector@@@Z"
+// CHECK-LABEL: "?n@@YAXPBQAUobjc_selector@@@Z"
void n(SEL &) {}
-// CHECK-LABEL: "?n@@YAXAAPAU.objc_selector@@@Z"
+// CHECK-LABEL: "?n@@YAXAAPAUobjc_selector@@@Z"
void n(const SEL &) {}
-// CHECK-LABEL: "?n@@YAXABQAU.objc_selector@@@Z"
+// CHECK-LABEL: "?n@@YAXABQAUobjc_selector@@@Z"
void n(SEL &&) {}
-// CHECK-LABEL: "?n@@YAX$$QAPAU.objc_selector@@@Z"
+// CHECK-LABEL: "?n@@YAX$$QAPAUobjc_selector@@@Z"
struct __declspec(dllexport) s {
struct s =(const struct s &) = delete;
void m(I *) {}
- // CHECK-LABEL: "?m@s@@QAAXPAU.objc_cls_I@@@Z"
+ // CHECK-LABEL:
[PATCH] D52443: Thread safety analysis: Examine constructor arguments
aaron.ballman added a comment.
This generally looks sensible to me.
Comment at: lib/Analysis/ThreadSafety.cpp:1970
+ // There can be default arguments, so we stop when one iterator is at end().
+ for (auto Arg = ArgBegin; Param != Params.end() && Arg != ArgEnd;
+ ++Param, ++Arg) {
How should this interact with variadic functions? Either ones that use `...`
with a C varargs function, or ones that use parameter packs in C++. (I realize
this is basically existing code, but the question remains.)
Comment at: lib/Analysis/ThreadSafety.cpp:2050
+ } else {
+ExamineCallArguments(D, Exp->arg_begin(), Exp->arg_end(), false);
}
Can you add a comment for the bool argument? e.g., `/*OperatorFun*/false`
Repository:
rC Clang
https://reviews.llvm.org/D52443
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r343152 - Remove trailing space in rC343150
Author: maskray
Date: Wed Sep 26 16:47:00 2018
New Revision: 343152
URL: http://llvm.org/viewvc/llvm-project?rev=343152=rev
Log:
Remove trailing space in rC343150
Modified:
cfe/trunk/include/clang/Sema/Lookup.h
Modified: cfe/trunk/include/clang/Sema/Lookup.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Lookup.h?rev=343152=343151=343152=diff
==
--- cfe/trunk/include/clang/Sema/Lookup.h (original)
+++ cfe/trunk/include/clang/Sema/Lookup.h Wed Sep 26 16:47:00 2018
@@ -711,7 +711,7 @@ private:
LookupResultKind ResultKind = NotFound;
// ill-defined unless ambiguous. Still need to be initialized it will be
// copied/moved.
- AmbiguityKind Ambiguity = {};
+ AmbiguityKind Ambiguity = {};
UnresolvedSet<8> Decls;
CXXBasePaths *Paths = nullptr;
CXXRecordDecl *NamingClass = nullptr;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r343150 - Init LookupResult::AmbiguityKind
Author: vitalybuka
Date: Wed Sep 26 15:58:53 2018
New Revision: 343150
URL: http://llvm.org/viewvc/llvm-project?rev=343150=rev
Log:
Init LookupResult::AmbiguityKind
We don't expect useful value there unless it's "ambiguous".
However we use read it for copying and moving, so we need either init the field
add login to avoid reading invalid values. Such reads trigger ubsan errors.
Modified:
cfe/trunk/include/clang/Sema/Lookup.h
Modified: cfe/trunk/include/clang/Sema/Lookup.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Lookup.h?rev=343150=343149=343150=diff
==
--- cfe/trunk/include/clang/Sema/Lookup.h (original)
+++ cfe/trunk/include/clang/Sema/Lookup.h Wed Sep 26 15:58:53 2018
@@ -709,7 +709,9 @@ private:
// Results.
LookupResultKind ResultKind = NotFound;
- AmbiguityKind Ambiguity; // ill-defined unless ambiguous
+ // ill-defined unless ambiguous. Still need to be initialized it will be
+ // copied/moved.
+ AmbiguityKind Ambiguity = {};
UnresolvedSet<8> Decls;
CXXBasePaths *Paths = nullptr;
CXXRecordDecl *NamingClass = nullptr;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r343148 - [DebugInfo] Generate debug information for labels.
Author: hsiangkai
Date: Wed Sep 26 15:18:45 2018
New Revision: 343148
URL: http://llvm.org/viewvc/llvm-project?rev=343148=rev
Log:
[DebugInfo] Generate debug information for labels.
Generate DILabel metadata and call llvm.dbg.label after label
statement to associate the metadata with the label.
After fixing PR37395.
After fixing problems in LiveDebugVariables.
After fixing NULL symbol problems in AddressPool when enabling
split-dwarf-file.
Differential Revision: https://reviews.llvm.org/D45045
Added:
cfe/trunk/test/CodeGen/debug-label-inline.c
cfe/trunk/test/CodeGen/debug-label.c
Modified:
cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
cfe/trunk/lib/CodeGen/CGDebugInfo.h
cfe/trunk/lib/CodeGen/CGStmt.cpp
Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=343148=343147=343148=diff
==
--- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Wed Sep 26 15:18:45 2018
@@ -3767,6 +3767,32 @@ CGDebugInfo::EmitDeclareOfAutoVariable(c
return EmitDeclare(VD, Storage, llvm::None, Builder);
}
+void CGDebugInfo::EmitLabel(const LabelDecl *D, CGBuilderTy ) {
+ assert(DebugKind >= codegenoptions::LimitedDebugInfo);
+ assert(!LexicalBlockStack.empty() && "Region stack mismatch, stack empty!");
+
+ if (D->hasAttr())
+return;
+
+ auto *Scope = cast(LexicalBlockStack.back());
+ llvm::DIFile *Unit = getOrCreateFile(D->getLocation());
+
+ // Get location information.
+ unsigned Line = getLineNumber(D->getLocation());
+ unsigned Column = getColumnNumber(D->getLocation());
+
+ StringRef Name = D->getName();
+
+ // Create the descriptor for the label.
+ auto *L =
+ DBuilder.createLabel(Scope, Name, Unit, Line,
CGM.getLangOpts().Optimize);
+
+ // Insert an llvm.dbg.label into the current block.
+ DBuilder.insertLabel(L,
+ llvm::DebugLoc::get(Line, Column, Scope, CurInlinedAt),
+ Builder.GetInsertBlock());
+}
+
llvm::DIType *CGDebugInfo::CreateSelfType(const QualType ,
llvm::DIType *Ty) {
llvm::DIType *CachedTy = getTypeOrNull(QualTy);
Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.h?rev=343148=343147=343148=diff
==
--- cfe/trunk/lib/CodeGen/CGDebugInfo.h (original)
+++ cfe/trunk/lib/CodeGen/CGDebugInfo.h Wed Sep 26 15:18:45 2018
@@ -415,6 +415,9 @@ public:
llvm::Value *AI,
CGBuilderTy );
+ /// Emit call to \c llvm.dbg.label for an label.
+ void EmitLabel(const LabelDecl *D, CGBuilderTy );
+
/// Emit call to \c llvm.dbg.declare for an imported variable
/// declaration in a block.
void EmitDeclareOfBlockDeclRefVariable(
Modified: cfe/trunk/lib/CodeGen/CGStmt.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGStmt.cpp?rev=343148=343147=343148=diff
==
--- cfe/trunk/lib/CodeGen/CGStmt.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGStmt.cpp Wed Sep 26 15:18:45 2018
@@ -531,6 +531,16 @@ void CodeGenFunction::EmitLabel(const La
}
EmitBlock(Dest.getBlock());
+
+ // Emit debug info for labels.
+ if (CGDebugInfo *DI = getDebugInfo()) {
+if (CGM.getCodeGenOpts().getDebugInfo() >=
+codegenoptions::LimitedDebugInfo) {
+ DI->setLocation(D->getLocation());
+ DI->EmitLabel(D, Builder);
+}
+ }
+
incrementProfileCounter(D->getStmt());
}
Added: cfe/trunk/test/CodeGen/debug-label-inline.c
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/debug-label-inline.c?rev=343148=auto
==
--- cfe/trunk/test/CodeGen/debug-label-inline.c (added)
+++ cfe/trunk/test/CodeGen/debug-label-inline.c Wed Sep 26 15:18:45 2018
@@ -0,0 +1,28 @@
+// This test will test the correctness of generating DILabel and
+// llvm.dbg.label when the label is in inlined functions.
+//
+// RUN: %clang_cc1 -O2 %s -o - -emit-llvm -debug-info-kind=limited | FileCheck
%s
+inline int f1(int a, int b) {
+ int sum;
+
+top:
+ sum = a + b;
+ return sum;
+}
+
+extern int ga, gb;
+
+int f2(void) {
+ int result;
+
+ result = f1(ga, gb);
+ // CHECK: call void @llvm.dbg.label(metadata [[LABEL_METADATA:!.*]]), !dbg
[[LABEL_LOCATION:!.*]]
+
+ return result;
+}
+
+// CHECK: distinct !DISubprogram(name: "f1", {{.*}}, retainedNodes:
[[ELEMENTS:!.*]])
+// CHECK: [[ELEMENTS]] = !{{{.*}}, [[LABEL_METADATA]]}
+// CHECK: [[LABEL_METADATA]] = !DILabel({{.*}}, name: "top", {{.*}}, line: 8)
+// CHECK: [[INLINEDAT:!.*]] = distinct !DILocation(line: 18,
+// CHECK: [[LABEL_LOCATION]] =
[PATCH] D52576: llvm::sort(C.begin(), C.end(), ...) -> llvm::sort(C, ...)
This revision was automatically updated to reflect the committed changes.
Closed by commit rL343147: llvm::sort(C.begin(), C.end(), ...) -
llvm::sort(C, ...) (authored by MaskRay, committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
https://reviews.llvm.org/D52576
Files:
cfe/trunk/lib/AST/ItaniumMangle.cpp
cfe/trunk/lib/AST/VTableBuilder.cpp
cfe/trunk/lib/Analysis/LiveVariables.cpp
cfe/trunk/lib/Basic/VirtualFileSystem.cpp
cfe/trunk/lib/CodeGen/CGBlocks.cpp
cfe/trunk/lib/CodeGen/CGObjCGNU.cpp
cfe/trunk/lib/CodeGen/CGVTables.cpp
cfe/trunk/lib/CodeGen/CodeGenModule.cpp
cfe/trunk/lib/CodeGen/TargetInfo.cpp
cfe/trunk/lib/Driver/Driver.cpp
cfe/trunk/lib/Driver/XRayArgs.cpp
cfe/trunk/lib/Format/FormatTokenLexer.cpp
cfe/trunk/lib/Format/WhitespaceManager.cpp
cfe/trunk/lib/Frontend/DiagnosticRenderer.cpp
cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp
cfe/trunk/lib/Sema/SemaDeclAttr.cpp
cfe/trunk/lib/Sema/SemaLookup.cpp
cfe/trunk/lib/Sema/SemaOverload.cpp
cfe/trunk/lib/Sema/SemaType.cpp
cfe/trunk/lib/Serialization/ASTReader.cpp
cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
cfe/trunk/lib/Serialization/ASTWriter.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/PaddingChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp
cfe/trunk/lib/StaticAnalyzer/Core/CheckerRegistry.cpp
cfe/trunk/lib/Tooling/ASTDiff/ASTDiff.cpp
cfe/trunk/lib/Tooling/Core/Replacement.cpp
cfe/trunk/lib/Tooling/InterpolatingCompilationDatabase.cpp
cfe/trunk/tools/diagtool/DiagTool.cpp
cfe/trunk/tools/libclang/CIndex.cpp
cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp
cfe/trunk/utils/TableGen/ClangDiagnosticsEmitter.cpp
cfe/trunk/utils/TableGen/ClangOptionDocEmitter.cpp
cfe/trunk/utils/TableGen/NeonEmitter.cpp
Index: cfe/trunk/lib/Frontend/DiagnosticRenderer.cpp
===
--- cfe/trunk/lib/Frontend/DiagnosticRenderer.cpp
+++ cfe/trunk/lib/Frontend/DiagnosticRenderer.cpp
@@ -337,8 +337,8 @@
SmallVector EndArgExpansions;
getMacroArgExpansionFileIDs(Begin, BeginArgExpansions, /*IsBegin=*/true, SM);
getMacroArgExpansionFileIDs(End, EndArgExpansions, /*IsBegin=*/false, SM);
- llvm::sort(BeginArgExpansions.begin(), BeginArgExpansions.end());
- llvm::sort(EndArgExpansions.begin(), EndArgExpansions.end());
+ llvm::sort(BeginArgExpansions);
+ llvm::sort(EndArgExpansions);
std::set_intersection(BeginArgExpansions.begin(), BeginArgExpansions.end(),
EndArgExpansions.begin(), EndArgExpansions.end(),
std::back_inserter(CommonArgExpansions));
Index: cfe/trunk/lib/Serialization/ASTWriter.cpp
===
--- cfe/trunk/lib/Serialization/ASTWriter.cpp
+++ cfe/trunk/lib/Serialization/ASTWriter.cpp
@@ -2495,8 +2495,7 @@
MacroIdentifiers.push_back(Id.second);
// Sort the set of macro definitions that need to be serialized by the
// name of the macro, to provide a stable ordering.
- llvm::sort(MacroIdentifiers.begin(), MacroIdentifiers.end(),
- llvm::less_ptr());
+ llvm::sort(MacroIdentifiers, llvm::less_ptr());
// Emit the macro directives as a list and associate the offset with the
// identifier they belong to.
@@ -3230,8 +3229,7 @@
SmallVector, 64> SortedFileDeclIDs(
FileDeclIDs.begin(), FileDeclIDs.end());
- llvm::sort(SortedFileDeclIDs.begin(), SortedFileDeclIDs.end(),
- llvm::less_first());
+ llvm::sort(SortedFileDeclIDs, llvm::less_first());
// Join the vectors of DeclIDs from all files.
SmallVector FileGroupedDeclIDs;
@@ -3737,7 +3735,7 @@
IIs.push_back(ID.second);
// Sort the identifiers lexicographically before getting them references so
// that their order is stable.
-llvm::sort(IIs.begin(), IIs.end(), llvm::less_ptr());
+llvm::sort(IIs, llvm::less_ptr());
for (const IdentifierInfo *II : IIs)
if (Trait.isInterestingNonMacroIdentifier(II))
getIdentifierRef(II);
@@ -4035,7 +4033,7 @@
}
// Sort the names into a stable order.
- llvm::sort(Names.begin(), Names.end());
+ llvm::sort(Names);
if (auto *D = dyn_cast(DC)) {
// We need to establish an ordering of constructor and conversion function
@@ -4172,7 +4170,7 @@
std::make_pair(Entry.first, Entry.second.getLookupResult()));
}
-llvm::sort(LookupResults.begin(), LookupResults.end(), llvm::less_first());
+llvm::sort(LookupResults, llvm::less_first());
for (auto : LookupResults) {
DeclarationName Name = NameAndResult.first;
DeclContext::lookup_result Result = NameAndResult.second;
@@ -4875,7 +4873,7 @@
IIs.push_back(II);
}
// Sort the identifiers to visit based on their name.
-llvm::sort(IIs.begin(), IIs.end(), llvm::less_ptr());
+llvm::sort(IIs, llvm::less_ptr());
for (const IdentifierInfo *II : IIs) {
for
r343147 - llvm::sort(C.begin(), C.end(), ...) -> llvm::sort(C, ...)
Author: maskray
Date: Wed Sep 26 15:16:28 2018
New Revision: 343147
URL: http://llvm.org/viewvc/llvm-project?rev=343147=rev
Log:
llvm::sort(C.begin(), C.end(), ...) -> llvm::sort(C, ...)
Summary: The convenience wrapper in STLExtras is available since rL342102.
Reviewers: rsmith, #clang, dblaikie
Reviewed By: rsmith, #clang
Subscribers: mgrang, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D52576
Modified:
cfe/trunk/lib/AST/ItaniumMangle.cpp
cfe/trunk/lib/AST/VTableBuilder.cpp
cfe/trunk/lib/Analysis/LiveVariables.cpp
cfe/trunk/lib/Basic/VirtualFileSystem.cpp
cfe/trunk/lib/CodeGen/CGBlocks.cpp
cfe/trunk/lib/CodeGen/CGObjCGNU.cpp
cfe/trunk/lib/CodeGen/CGVTables.cpp
cfe/trunk/lib/CodeGen/CodeGenModule.cpp
cfe/trunk/lib/CodeGen/TargetInfo.cpp
cfe/trunk/lib/Driver/Driver.cpp
cfe/trunk/lib/Driver/XRayArgs.cpp
cfe/trunk/lib/Format/FormatTokenLexer.cpp
cfe/trunk/lib/Format/WhitespaceManager.cpp
cfe/trunk/lib/Frontend/DiagnosticRenderer.cpp
cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp
cfe/trunk/lib/Sema/SemaDeclAttr.cpp
cfe/trunk/lib/Sema/SemaLookup.cpp
cfe/trunk/lib/Sema/SemaOverload.cpp
cfe/trunk/lib/Sema/SemaType.cpp
cfe/trunk/lib/Serialization/ASTReader.cpp
cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
cfe/trunk/lib/Serialization/ASTWriter.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/PaddingChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp
cfe/trunk/lib/StaticAnalyzer/Core/CheckerRegistry.cpp
cfe/trunk/lib/Tooling/ASTDiff/ASTDiff.cpp
cfe/trunk/lib/Tooling/Core/Replacement.cpp
cfe/trunk/lib/Tooling/InterpolatingCompilationDatabase.cpp
cfe/trunk/tools/diagtool/DiagTool.cpp
cfe/trunk/tools/libclang/CIndex.cpp
cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp
cfe/trunk/utils/TableGen/ClangDiagnosticsEmitter.cpp
cfe/trunk/utils/TableGen/ClangOptionDocEmitter.cpp
cfe/trunk/utils/TableGen/NeonEmitter.cpp
Modified: cfe/trunk/lib/AST/ItaniumMangle.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ItaniumMangle.cpp?rev=343147=343146=343147=diff
==
--- cfe/trunk/lib/AST/ItaniumMangle.cpp (original)
+++ cfe/trunk/lib/AST/ItaniumMangle.cpp Wed Sep 26 15:16:28 2018
@@ -323,7 +323,7 @@ class CXXNameMangler {
AdditionalAbiTags->end());
}
- llvm::sort(TagList.begin(), TagList.end());
+ llvm::sort(TagList);
TagList.erase(std::unique(TagList.begin(), TagList.end()),
TagList.end());
writeSortedUniqueAbiTags(Out, TagList);
@@ -339,7 +339,7 @@ class CXXNameMangler {
}
const AbiTagList () {
- llvm::sort(UsedAbiTags.begin(), UsedAbiTags.end());
+ llvm::sort(UsedAbiTags);
UsedAbiTags.erase(std::unique(UsedAbiTags.begin(), UsedAbiTags.end()),
UsedAbiTags.end());
return UsedAbiTags;
Modified: cfe/trunk/lib/AST/VTableBuilder.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/VTableBuilder.cpp?rev=343147=343146=343147=diff
==
--- cfe/trunk/lib/AST/VTableBuilder.cpp (original)
+++ cfe/trunk/lib/AST/VTableBuilder.cpp Wed Sep 26 15:16:28 2018
@@ -2105,8 +2105,7 @@ void ItaniumVTableBuilder::dumpLayout(ra
const CXXMethodDecl *MD = I.second;
ThunkInfoVectorTy ThunksVector = Thunks[MD];
- llvm::sort(ThunksVector.begin(), ThunksVector.end(),
- [](const ThunkInfo , const ThunkInfo ) {
+ llvm::sort(ThunksVector, [](const ThunkInfo , const ThunkInfo ) {
assert(LHS.Method == nullptr && RHS.Method == nullptr);
return std::tie(LHS.This, LHS.Return) < std::tie(RHS.This, RHS.Return);
});
@@ -3345,8 +3344,7 @@ static bool rebucketPaths(VPtrInfoVector
PathsSorted.reserve(Paths.size());
for (auto& P : Paths)
PathsSorted.push_back(*P);
- llvm::sort(PathsSorted.begin(), PathsSorted.end(),
- [](const VPtrInfo , const VPtrInfo ) {
+ llvm::sort(PathsSorted, [](const VPtrInfo , const VPtrInfo ) {
return LHS.MangledPath < RHS.MangledPath;
});
bool Changed = false;
Modified: cfe/trunk/lib/Analysis/LiveVariables.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/LiveVariables.cpp?rev=343147=343146=343147=diff
==
--- cfe/trunk/lib/Analysis/LiveVariables.cpp (original)
+++ cfe/trunk/lib/Analysis/LiveVariables.cpp Wed Sep 26 15:16:28 2018
@@ -597,7 +597,7 @@ void LiveVariablesImpl::dumpBlockLivenes
it != ei; ++it) {
vec.push_back(it->first);
}
- llvm::sort(vec.begin(), vec.end(), [](const CFGBlock *A, const CFGBlock *B) {
+ llvm::sort(vec, [](const CFGBlock *A, const CFGBlock *B) {
return A->getBlockID() < B->getBlockID();
});
@@ -617,10 +617,9 @@ void
[PATCH] D52576: llvm::sort(C.begin(), C.end(), ...) -> llvm::sort(C, ...)
MaskRay updated this revision to Diff 167206.
MaskRay added a comment.
Reflow nearby statements as rsmith@ requested:
git diff -U0 --no-color 'HEAD^' |
~/llvm/tools/clang/tools/clang-format/clang-format-diff.py -i -p1
Repository:
rC Clang
https://reviews.llvm.org/D52576
Files:
lib/AST/ItaniumMangle.cpp
lib/AST/VTableBuilder.cpp
lib/Analysis/LiveVariables.cpp
lib/Basic/VirtualFileSystem.cpp
lib/CodeGen/CGBlocks.cpp
lib/CodeGen/CGObjCGNU.cpp
lib/CodeGen/CGVTables.cpp
lib/CodeGen/CodeGenModule.cpp
lib/CodeGen/TargetInfo.cpp
lib/Driver/Driver.cpp
lib/Driver/XRayArgs.cpp
lib/Format/FormatTokenLexer.cpp
lib/Format/WhitespaceManager.cpp
lib/Frontend/DiagnosticRenderer.cpp
lib/Sema/AnalysisBasedWarnings.cpp
lib/Sema/SemaDeclAttr.cpp
lib/Sema/SemaLookup.cpp
lib/Sema/SemaOverload.cpp
lib/Sema/SemaType.cpp
lib/Serialization/ASTReader.cpp
lib/Serialization/ASTReaderDecl.cpp
lib/Serialization/ASTWriter.cpp
lib/StaticAnalyzer/Checkers/PaddingChecker.cpp
lib/StaticAnalyzer/Core/BugReporter.cpp
lib/StaticAnalyzer/Core/CheckerRegistry.cpp
lib/Tooling/ASTDiff/ASTDiff.cpp
lib/Tooling/Core/Replacement.cpp
lib/Tooling/InterpolatingCompilationDatabase.cpp
tools/diagtool/DiagTool.cpp
tools/libclang/CIndex.cpp
unittests/Basic/VirtualFileSystemTest.cpp
utils/TableGen/ClangDiagnosticsEmitter.cpp
utils/TableGen/ClangOptionDocEmitter.cpp
utils/TableGen/NeonEmitter.cpp
Index: utils/TableGen/NeonEmitter.cpp
===
--- utils/TableGen/NeonEmitter.cpp
+++ utils/TableGen/NeonEmitter.cpp
@@ -2020,7 +2020,7 @@
}
}
- llvm::sort(NewTypeSpecs.begin(), NewTypeSpecs.end());
+ llvm::sort(NewTypeSpecs);
NewTypeSpecs.erase(std::unique(NewTypeSpecs.begin(), NewTypeSpecs.end()),
NewTypeSpecs.end());
auto = IntrinsicMap[Name];
Index: utils/TableGen/ClangOptionDocEmitter.cpp
===
--- utils/TableGen/ClangOptionDocEmitter.cpp
+++ utils/TableGen/ClangOptionDocEmitter.cpp
@@ -111,25 +111,25 @@
auto DocumentationForOption = [&](Record *R) -> DocumentedOption {
auto = Aliases[R];
-llvm::sort(A.begin(), A.end(), CompareByName);
+llvm::sort(A, CompareByName);
return {R, std::move(A)};
};
std::function DocumentationForGroup =
[&](Record *R) -> Documentation {
Documentation D;
auto = GroupsInGroup[R];
-llvm::sort(Groups.begin(), Groups.end(), CompareByLocation);
+llvm::sort(Groups, CompareByLocation);
for (Record *G : Groups) {
D.Groups.emplace_back();
D.Groups.back().Group = G;
Documentation = D.Groups.back();
Base = DocumentationForGroup(G);
}
auto = OptionsInGroup[R];
-llvm::sort(Options.begin(), Options.end(), CompareByName);
+llvm::sort(Options, CompareByName);
for (Record *O : Options)
D.Options.push_back(DocumentationForOption(O));
Index: utils/TableGen/ClangDiagnosticsEmitter.cpp
===
--- utils/TableGen/ClangDiagnosticsEmitter.cpp
+++ utils/TableGen/ClangDiagnosticsEmitter.cpp
@@ -208,9 +208,9 @@
E = SortedGroups.end();
I != E; ++I) {
MutableArrayRef GroupDiags = (*I)->DiagsInGroup;
-llvm::sort(GroupDiags.begin(), GroupDiags.end(), beforeThanCompare);
+llvm::sort(GroupDiags, beforeThanCompare);
}
- llvm::sort(SortedGroups.begin(), SortedGroups.end(), beforeThanCompareGroups);
+ llvm::sort(SortedGroups, beforeThanCompareGroups);
// Warn about the same group being used anonymously in multiple places.
for (SmallVectorImpl::const_iterator I = SortedGroups.begin(),
@@ -1595,10 +1595,10 @@
Index.push_back(RecordIndexElement(R));
}
- llvm::sort(Index.begin(), Index.end(),
+ llvm::sort(Index,
[](const RecordIndexElement , const RecordIndexElement ) {
return Lhs.Name < Rhs.Name;
-});
+ });
for (unsigned i = 0, e = Index.size(); i != e; ++i) {
const RecordIndexElement = Index[i];
@@ -1694,7 +1694,7 @@
std::vector DiagGroups =
Records.getAllDerivedDefinitions("DiagGroup");
- llvm::sort(DiagGroups.begin(), DiagGroups.end(), diagGroupBeforeByName);
+ llvm::sort(DiagGroups, diagGroupBeforeByName);
DiagGroupParentMap DGParentMap(Records);
@@ -1713,10 +1713,8 @@
DiagsInPedanticSet.end());
RecordVec GroupsInPedantic(GroupsInPedanticSet.begin(),
GroupsInPedanticSet.end());
-llvm::sort(DiagsInPedantic.begin(), DiagsInPedantic.end(),
- beforeThanCompare);
-llvm::sort(GroupsInPedantic.begin(), GroupsInPedantic.end(),
- beforeThanCompare);
+llvm::sort(DiagsInPedantic, beforeThanCompare);
+llvm::sort(GroupsInPedantic, beforeThanCompare);
[PATCH] D52578: Thread safety analysis: Allow scoped releasing of capabilities
aaronpuchert added a comment. See the bug for further discussion. I'm not sure if we want to have this, but if the pattern is used more widely it might make sense. It blows up the code a bit, although I hope that https://reviews.llvm.org/D51187 might reduce it again. Repository: rC Clang https://reviews.llvm.org/D52578 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52576: llvm::sort(C.begin(), C.end(), ...) -> llvm::sort(C, ...)
rsmith accepted this revision. rsmith added a comment. This revision is now accepted and ready to land. Thanks for the cleanup! Maybe consider running `clang-format-diff` on this? Comment at: lib/Sema/SemaOverload.cpp:10813-10814 - llvm::sort(Cands.begin(), Cands.end(), + llvm::sort(Cands, CompareTemplateSpecCandidatesForDisplay(S)); Reflow. Comment at: lib/Serialization/ASTReader.cpp:9193-9194 // potentially invalidating the original order. Sort it again. -llvm::sort(Comments.begin(), Comments.end(), +llvm::sort(Comments, BeforeThanCompare(SourceMgr)); Context.Comments.addDeserializedComments(Comments); Reflow if this fits on the previous line. Comment at: lib/Serialization/ASTWriter.cpp:2498-2499 // name of the macro, to provide a stable ordering. - llvm::sort(MacroIdentifiers.begin(), MacroIdentifiers.end(), + llvm::sort(MacroIdentifiers, llvm::less_ptr()); Reflow. Comment at: lib/Serialization/ASTWriter.cpp:3233-3234 FileDeclIDs.begin(), FileDeclIDs.end()); - llvm::sort(SortedFileDeclIDs.begin(), SortedFileDeclIDs.end(), + llvm::sort(SortedFileDeclIDs, llvm::less_first()); Reflow. Comment at: lib/StaticAnalyzer/Core/BugReporter.cpp:2389-2390 // Sort the error paths from longest to shortest. - llvm::sort(ReportNodes.begin(), ReportNodes.end(), + llvm::sort(ReportNodes, PriorityCompare(PriorityMap)); } Reflow if this fits on the previous line. Comment at: utils/TableGen/ClangDiagnosticsEmitter.cpp:1716-1719 +llvm::sort(DiagsInPedantic, beforeThanCompare); -llvm::sort(GroupsInPedantic.begin(), GroupsInPedantic.end(), +llvm::sort(GroupsInPedantic, beforeThanCompare); Reflow these lines. Repository: rC Clang https://reviews.llvm.org/D52576 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52578: Thread safety analysis: Allow scoped releasing of capabilities
aaronpuchert created this revision.
aaronpuchert added reviewers: aaron.ballman, delesley.
Herald added a subscriber: cfe-commits.
The pattern is problematic with C++ exceptions, and not as widespread as
scoped locks, but it's still used by some, for example Chromium.
We are a bit stricter here at join points, patterns that are allowed for
scoped locks aren't allowed here. That could still be changed in the
future, but I'd argue we should only relax this if people ask for it.
Fixes PR36162.
Repository:
rC Clang
https://reviews.llvm.org/D52578
Files:
lib/Analysis/ThreadSafety.cpp
test/SemaCXX/warn-thread-safety-analysis.cpp
Index: test/SemaCXX/warn-thread-safety-analysis.cpp
===
--- test/SemaCXX/warn-thread-safety-analysis.cpp
+++ test/SemaCXX/warn-thread-safety-analysis.cpp
@@ -2755,6 +2755,85 @@
} // end namespace RelockableScopedLock
+namespace ScopedUnlock {
+
+class SCOPED_LOCKABLE MutexUnlock {
+public:
+ MutexUnlock(Mutex *mu) EXCLUSIVE_UNLOCK_FUNCTION(mu);
+ ~MutexUnlock() EXCLUSIVE_UNLOCK_FUNCTION();
+
+ void Lock() EXCLUSIVE_UNLOCK_FUNCTION();
+ void Unlock() EXCLUSIVE_LOCK_FUNCTION();
+};
+
+Mutex mu;
+int x GUARDED_BY(mu);
+bool c;
+
+void simple() EXCLUSIVE_LOCKS_REQUIRED(mu) {
+ x = 1;
+ MutexUnlock scope();
+ x = 2; // expected-warning {{writing variable 'x' requires holding mutex 'mu' exclusively}}
+}
+
+void innerUnlock() {
+ MutexLock outer();
+ if (x == 0) {
+MutexUnlock inner();
+x = 1; // expected-warning {{writing variable 'x' requires holding mutex 'mu' exclusively}}
+ }
+ x = 2;
+}
+
+void manual() EXCLUSIVE_LOCKS_REQUIRED(mu) {
+ MutexUnlock scope();
+ scope.Lock();
+ x = 2;
+ scope.Unlock();
+ x = 3; // expected-warning {{writing variable 'x' requires holding mutex 'mu' exclusively}}
+}
+
+void join() EXCLUSIVE_LOCKS_REQUIRED(mu) {
+ MutexUnlock scope();
+ if (c) {
+scope.Lock(); // expected-note{{mutex acquired here}}
+ }
+ // expected-warning@+1{{mutex 'mu' is not held on every path through here}}
+ scope.Lock();
+}
+
+void doubleLock() EXCLUSIVE_LOCKS_REQUIRED(mu) {
+ MutexUnlock scope();
+ scope.Lock();
+ scope.Lock(); // expected-warning {{acquiring mutex 'mu' that is already held}}
+}
+
+void doubleUnlock() EXCLUSIVE_LOCKS_REQUIRED(mu) {
+ MutexUnlock scope();
+ scope.Unlock(); // expected-warning {{releasing mutex 'mu' that was not held}}
+}
+
+class SCOPED_LOCKABLE MutexLockUnlock {
+public:
+ MutexLockUnlock(Mutex *mu1, Mutex *mu2) EXCLUSIVE_UNLOCK_FUNCTION(mu1) EXCLUSIVE_LOCK_FUNCTION(mu2);
+ ~MutexLockUnlock() EXCLUSIVE_UNLOCK_FUNCTION();
+
+ void Release() EXCLUSIVE_UNLOCK_FUNCTION();
+ void Acquire() EXCLUSIVE_LOCK_FUNCTION();
+};
+
+Mutex other;
+void fn() EXCLUSIVE_LOCKS_REQUIRED(other);
+
+void lockUnlock() EXCLUSIVE_LOCKS_REQUIRED(mu) {
+ MutexLockUnlock scope(, );
+ fn();
+ x = 1; // expected-warning {{writing variable 'x' requires holding mutex 'mu' exclusively}}
+}
+
+} // end namespace ScopedUnlock
+
+
namespace TrylockFunctionTest {
class Foo {
Index: lib/Analysis/ThreadSafety.cpp
===
--- lib/Analysis/ThreadSafety.cpp
+++ lib/Analysis/ThreadSafety.cpp
@@ -41,6 +41,7 @@
#include "llvm/ADT/DenseMap.h"
#include "llvm/ADT/ImmutableMap.h"
#include "llvm/ADT/Optional.h"
+#include "llvm/ADT/PointerIntPair.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/StringRef.h"
@@ -889,45 +890,74 @@
class ScopedLockableFactEntry : public FactEntry {
private:
- SmallVector UnderlyingMutexes;
+ enum UnderlyingCapabilityKind {
+UCK_Acquired, ///< Any kind of acquired capability.
+UCK_ReleasedShared,///< Shared capability that was released.
+UCK_ReleasedExclusive, ///< Exclusive capability that was released.
+ };
+
+ using UnderlyingCapability =
+ llvm::PointerIntPair;
+
+ SmallVector UnderlyingMutexes;
public:
ScopedLockableFactEntry(const CapabilityExpr , SourceLocation Loc,
- const CapExprSet , const CapExprSet )
+ const CapExprSet , const CapExprSet ,
+ const CapExprSet , const CapExprSet )
: FactEntry(CE, LK_Exclusive, Loc, false) {
for (const auto : Excl)
- UnderlyingMutexes.push_back(M.sexpr());
+ UnderlyingMutexes.emplace_back(M.sexpr(), UCK_Acquired);
for (const auto : Shrd)
- UnderlyingMutexes.push_back(M.sexpr());
+ UnderlyingMutexes.emplace_back(M.sexpr(), UCK_Acquired);
+for (const auto : ExclRel)
+ UnderlyingMutexes.emplace_back(M.sexpr(), UCK_ReleasedExclusive);
+for (const auto : ShrdRel)
+ UnderlyingMutexes.emplace_back(M.sexpr(), UCK_ReleasedExclusive);
}
void
handleRemovalFromIntersection(const FactSet , FactManager ,
SourceLocation JoinLoc, LockErrorKind LEK,
[PATCH] D52290: [driver][mips] Adjust target triple accordingly to provided ABI name
rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. Lgtm, thanks! Repository: rC Clang https://reviews.llvm.org/D52290 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52576: llvm::sort(C.begin(), C.end(), ...) -> llvm::sort(C, ...)
MaskRay created this revision.
MaskRay added a reviewer: rsmith.
Herald added subscribers: cfe-commits, kadircet, arphaman, mgrang.
The convenience wrapper in STLExtras is available since
https://reviews.llvm.org/rL342102.
Repository:
rC Clang
https://reviews.llvm.org/D52576
Files:
lib/AST/ItaniumMangle.cpp
lib/AST/VTableBuilder.cpp
lib/Analysis/LiveVariables.cpp
lib/Basic/VirtualFileSystem.cpp
lib/CodeGen/CGBlocks.cpp
lib/CodeGen/CGObjCGNU.cpp
lib/CodeGen/CGVTables.cpp
lib/CodeGen/CodeGenModule.cpp
lib/CodeGen/TargetInfo.cpp
lib/Driver/Driver.cpp
lib/Driver/XRayArgs.cpp
lib/Format/FormatTokenLexer.cpp
lib/Format/WhitespaceManager.cpp
lib/Frontend/DiagnosticRenderer.cpp
lib/Sema/AnalysisBasedWarnings.cpp
lib/Sema/SemaDeclAttr.cpp
lib/Sema/SemaLookup.cpp
lib/Sema/SemaOverload.cpp
lib/Sema/SemaType.cpp
lib/Serialization/ASTReader.cpp
lib/Serialization/ASTReaderDecl.cpp
lib/Serialization/ASTWriter.cpp
lib/StaticAnalyzer/Checkers/PaddingChecker.cpp
lib/StaticAnalyzer/Core/BugReporter.cpp
lib/StaticAnalyzer/Core/CheckerRegistry.cpp
lib/Tooling/ASTDiff/ASTDiff.cpp
lib/Tooling/Core/Replacement.cpp
lib/Tooling/InterpolatingCompilationDatabase.cpp
tools/diagtool/DiagTool.cpp
tools/libclang/CIndex.cpp
unittests/Basic/VirtualFileSystemTest.cpp
utils/TableGen/ClangDiagnosticsEmitter.cpp
utils/TableGen/ClangOptionDocEmitter.cpp
utils/TableGen/NeonEmitter.cpp
Index: utils/TableGen/NeonEmitter.cpp
===
--- utils/TableGen/NeonEmitter.cpp
+++ utils/TableGen/NeonEmitter.cpp
@@ -2020,7 +2020,7 @@
}
}
- llvm::sort(NewTypeSpecs.begin(), NewTypeSpecs.end());
+ llvm::sort(NewTypeSpecs);
NewTypeSpecs.erase(std::unique(NewTypeSpecs.begin(), NewTypeSpecs.end()),
NewTypeSpecs.end());
auto = IntrinsicMap[Name];
Index: utils/TableGen/ClangOptionDocEmitter.cpp
===
--- utils/TableGen/ClangOptionDocEmitter.cpp
+++ utils/TableGen/ClangOptionDocEmitter.cpp
@@ -111,25 +111,25 @@
auto DocumentationForOption = [&](Record *R) -> DocumentedOption {
auto = Aliases[R];
-llvm::sort(A.begin(), A.end(), CompareByName);
+llvm::sort(A, CompareByName);
return {R, std::move(A)};
};
std::function DocumentationForGroup =
[&](Record *R) -> Documentation {
Documentation D;
auto = GroupsInGroup[R];
-llvm::sort(Groups.begin(), Groups.end(), CompareByLocation);
+llvm::sort(Groups, CompareByLocation);
for (Record *G : Groups) {
D.Groups.emplace_back();
D.Groups.back().Group = G;
Documentation = D.Groups.back();
Base = DocumentationForGroup(G);
}
auto = OptionsInGroup[R];
-llvm::sort(Options.begin(), Options.end(), CompareByName);
+llvm::sort(Options, CompareByName);
for (Record *O : Options)
D.Options.push_back(DocumentationForOption(O));
Index: utils/TableGen/ClangDiagnosticsEmitter.cpp
===
--- utils/TableGen/ClangDiagnosticsEmitter.cpp
+++ utils/TableGen/ClangDiagnosticsEmitter.cpp
@@ -208,9 +208,9 @@
E = SortedGroups.end();
I != E; ++I) {
MutableArrayRef GroupDiags = (*I)->DiagsInGroup;
-llvm::sort(GroupDiags.begin(), GroupDiags.end(), beforeThanCompare);
+llvm::sort(GroupDiags, beforeThanCompare);
}
- llvm::sort(SortedGroups.begin(), SortedGroups.end(), beforeThanCompareGroups);
+ llvm::sort(SortedGroups, beforeThanCompareGroups);
// Warn about the same group being used anonymously in multiple places.
for (SmallVectorImpl::const_iterator I = SortedGroups.begin(),
@@ -1595,7 +1595,7 @@
Index.push_back(RecordIndexElement(R));
}
- llvm::sort(Index.begin(), Index.end(),
+ llvm::sort(Index,
[](const RecordIndexElement , const RecordIndexElement ) {
return Lhs.Name < Rhs.Name;
});
@@ -1694,7 +1694,7 @@
std::vector DiagGroups =
Records.getAllDerivedDefinitions("DiagGroup");
- llvm::sort(DiagGroups.begin(), DiagGroups.end(), diagGroupBeforeByName);
+ llvm::sort(DiagGroups, diagGroupBeforeByName);
DiagGroupParentMap DGParentMap(Records);
@@ -1713,9 +1713,9 @@
DiagsInPedanticSet.end());
RecordVec GroupsInPedantic(GroupsInPedanticSet.begin(),
GroupsInPedanticSet.end());
-llvm::sort(DiagsInPedantic.begin(), DiagsInPedantic.end(),
+llvm::sort(DiagsInPedantic,
beforeThanCompare);
-llvm::sort(GroupsInPedantic.begin(), GroupsInPedantic.end(),
+llvm::sort(GroupsInPedantic,
beforeThanCompare);
PedDiags.DiagsInGroup.insert(PedDiags.DiagsInGroup.end(),
DiagsInPedantic.begin(),
Index: unittests/Basic/VirtualFileSystemTest.cpp
Re: r338385 - [RISCV] Add driver for riscv32-unknown-elf baremetal target
I put a patch up to fix this here: https://reviews.llvm.org/D52574 Please take these warnings more seriously next time. It's the committer's responsibility to fix these warnings [1], and they often identify legitimate issues in the patch. Thanks, Erik [1]: https://llvm.org/docs/DeveloperPolicy.html#quality On 8/17/18 8:40 AM, Nico Weber via cfe-commits wrote: It's two weeks later and I'm still seeing this warning. Any news? On Fri, Aug 3, 2018 at 9:29 AM Dávid Bolvanský mailto:david.bolvan...@gmail.com>> wrote: Such filename fix could be part of https://reviews.llvm.org/D50246 pi 3. 8. 2018 o 15:17 Nico Weber mailto:tha...@chromium.org>> napísal(a): I'm getting this warning from the mac linker after this commit: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/libtool: warning same member name (libclangDriver.RISCV.o) in output file used for input files: obj/clang/lib/Driver/ToolChains/Arch/libclangDriver.RISCV.o and: obj/clang/lib/Driver/ToolChains/libclangDriver.RISCV.o (due to use of basename, truncation, blank padding or duplicate input files) Could we rename the file to fix that warning? On Tue, Jul 31, 2018 at 10:40 AM David Bolvansky via cfe-commits mailto:cfe-commits@lists.llvm.org>> wrote: Author: xbolva00 Date: Tue Jul 31 07:21:46 2018 New Revision: 338385 URL: http://llvm.org/viewvc/llvm-project?rev=338385=rev Log: [RISCV] Add driver for riscv32-unknown-elf baremetal target Summary: This patch adds a driver for the baremetal RISC-V target (i.e. riscv32-unknown-elf). For reference, D39963 added basic target info and added support for riscv32-linux-unknown-elf. Patch by: asb (Alex Bradbury) Reviewers: efriedma, phosek, apazos, espindola, mgrang Reviewed By: mgrang Subscribers: jrtc27, rogfer01, MartinMosbeck, brucehoult, the_o, rkruppe, emaste, mgorny, arichardson, rbar, johnrusso, simoncook, jordy.potman.lists, sabuasal, niosHD, kito-cheng, shiva0217, zzheng, edward-jones, mgrang, cfe-commits Differential Revision: https://reviews.llvm.org/D46822 Added: cfe/trunk/lib/Driver/ToolChains/RISCV.cpp cfe/trunk/lib/Driver/ToolChains/RISCV.h cfe/trunk/test/Driver/Inputs/basic_riscv32_tree/ cfe/trunk/test/Driver/Inputs/basic_riscv32_tree/bin/ cfe/trunk/test/Driver/Inputs/basic_riscv32_tree/bin/riscv32-unknown-elf-ld (with props) cfe/trunk/test/Driver/Inputs/basic_riscv32_tree/lib/ cfe/trunk/test/Driver/Inputs/basic_riscv32_tree/lib/gcc/ cfe/trunk/test/Driver/Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/ cfe/trunk/test/Driver/Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1/ cfe/trunk/test/Driver/Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1/crtbegin.o cfe/trunk/test/Driver/Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1/crtend.o cfe/trunk/test/Driver/Inputs/basic_riscv32_tree/riscv32-unknown-elf/ cfe/trunk/test/Driver/Inputs/basic_riscv32_tree/riscv32-unknown-elf/include/ cfe/trunk/test/Driver/Inputs/basic_riscv32_tree/riscv32-unknown-elf/include/c++/ cfe/trunk/test/Driver/Inputs/basic_riscv32_tree/riscv32-unknown-elf/include/c++/8.0.1/ cfe/trunk/test/Driver/Inputs/basic_riscv32_tree/riscv32-unknown-elf/include/c++/8.0.1/.keep cfe/trunk/test/Driver/Inputs/basic_riscv32_tree/riscv32-unknown-elf/lib/ cfe/trunk/test/Driver/Inputs/basic_riscv32_tree/riscv32-unknown-elf/lib/crt0.o Modified: cfe/trunk/lib/Driver/CMakeLists.txt cfe/trunk/lib/Driver/Driver.cpp cfe/trunk/lib/Driver/ToolChains/Gnu.cpp cfe/trunk/test/Driver/riscv32-toolchain.c Modified: cfe/trunk/lib/Driver/CMakeLists.txt URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/CMakeLists.txt?rev=338385=338384=338385=diff == --- cfe/trunk/lib/Driver/CMakeLists.txt (original) +++ cfe/trunk/lib/Driver/CMakeLists.txt Tue Jul 31 07:21:46 2018 @@ -57,6 +57,7 @@ add_clang_library(clangDriver ToolChains/NetBSD.cpp ToolChains/OpenBSD.cpp ToolChains/PS4CPU.cpp + ToolChains/RISCV.cpp ToolChains/Solaris.cpp ToolChains/TCE.cpp ToolChains/WebAssembly.cpp Modified: cfe/trunk/lib/Driver/Driver.cpp
[PATCH] D52574: NFC: Fix some darwin linker warnings introduced in r338385
erik.pilkington created this revision. erik.pilkington added reviewers: xbolva00, kristina, asb. Herald added subscribers: kadircet, jocewei, PkmX, dexonsmith, the_o, brucehoult, MartinMosbeck, rogfer01, mgrang, edward-jones, zzheng, jrtc27, ioeric, niosHD, sabuasal, apazos, simoncook, johnrusso, rbar, ilya-biryukov, mgorny. The darwin linker was complaining about Toolchains/RISCV.cpp and Toolchains/Arch/RISCV.cpp had the same name. Fix is to just rename Toolchains/RISCV.cpp to Toolchains/RISCVToolchain.cpp. This was introduced by https://reviews.llvm.org/D50246. /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/libtool: warning same member name (RISCV.cpp.o) in output file used for input files: tools/clang/lib/Driver/CMakeFiles/clangDriver.dir/ToolChains/Arch/RISCV.cpp.o and: tools/clang/lib/Driver/CMakeFiles/clangDriver.dir/ToolChains/RISCV.cpp.o (due to use of basename, truncation, blank padding or duplicate input files) rdar://44535427 Repository: rC Clang https://reviews.llvm.org/D52574 Files: clang/lib/Driver/CMakeLists.txt clang/lib/Driver/Driver.cpp clang/lib/Driver/ToolChains/RISCV.cpp clang/lib/Driver/ToolChains/RISCV.h clang/lib/Driver/ToolChains/RISCVToolchain.cpp clang/lib/Driver/ToolChains/RISCVToolchain.h Index: clang/lib/Driver/ToolChains/RISCVToolchain.h === --- clang/lib/Driver/ToolChains/RISCVToolchain.h +++ clang/lib/Driver/ToolChains/RISCVToolchain.h @@ -1,14 +1,14 @@ -//===--- RISCV.h - RISCV ToolChain Implementations --*- C++ -*-===// +//===--- RISCVToolchain.h - RISCV ToolChain Implementations -*- C++ -*-===// // // The LLVM Compiler Infrastructure // // This file is distributed under the University of Illinois Open Source // License. See LICENSE.TXT for details. // //===--===// -#ifndef LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_RISCV_H -#define LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_RISCV_H +#ifndef LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_RISCVTOOLCHAIN_H +#define LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_RISCVTOOLCHAIN_H #include "Gnu.h" #include "clang/Driver/ToolChain.h" Index: clang/lib/Driver/ToolChains/RISCVToolchain.cpp === --- clang/lib/Driver/ToolChains/RISCVToolchain.cpp +++ clang/lib/Driver/ToolChains/RISCVToolchain.cpp @@ -1,13 +1,13 @@ -//===--- RISCV.cpp - RISCV ToolChain Implementations *- C++ -*-===// +//===--- RISCVToolchain.cpp - RISCV ToolChain Implementations ---*- C++ -*-===// // // The LLVM Compiler Infrastructure // // This file is distributed under the University of Illinois Open Source // License. See LICENSE.TXT for details. // //===--===// -#include "RISCV.h" +#include "RISCVToolchain.h" #include "CommonArgs.h" #include "InputInfo.h" #include "clang/Driver/Compilation.h" Index: clang/lib/Driver/Driver.cpp === --- clang/lib/Driver/Driver.cpp +++ clang/lib/Driver/Driver.cpp @@ -37,7 +37,7 @@ #include "ToolChains/NetBSD.h" #include "ToolChains/OpenBSD.h" #include "ToolChains/PS4CPU.h" -#include "ToolChains/RISCV.h" +#include "ToolChains/RISCVToolchain.h" #include "ToolChains/Solaris.h" #include "ToolChains/TCE.h" #include "ToolChains/WebAssembly.h" Index: clang/lib/Driver/CMakeLists.txt === --- clang/lib/Driver/CMakeLists.txt +++ clang/lib/Driver/CMakeLists.txt @@ -57,7 +57,7 @@ ToolChains/NetBSD.cpp ToolChains/OpenBSD.cpp ToolChains/PS4CPU.cpp - ToolChains/RISCV.cpp + ToolChains/RISCVToolchain.cpp ToolChains/Solaris.cpp ToolChains/TCE.cpp ToolChains/WebAssembly.cpp Index: clang/lib/Driver/ToolChains/RISCVToolchain.h === --- clang/lib/Driver/ToolChains/RISCVToolchain.h +++ clang/lib/Driver/ToolChains/RISCVToolchain.h @@ -1,14 +1,14 @@ -//===--- RISCV.h - RISCV ToolChain Implementations --*- C++ -*-===// +//===--- RISCVToolchain.h - RISCV ToolChain Implementations -*- C++ -*-===// // // The LLVM Compiler Infrastructure // // This file is distributed under the University of Illinois Open Source // License. See LICENSE.TXT for details. // //===--===// -#ifndef LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_RISCV_H -#define LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_RISCV_H +#ifndef LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_RISCVTOOLCHAIN_H +#define LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_RISCVTOOLCHAIN_H #include "Gnu.h" #include "clang/Driver/ToolChain.h" Index: clang/lib/Driver/ToolChains/RISCVToolchain.cpp
[PATCH] D52290: [driver][mips] Adjust target triple accordingly to provided ABI name
atanasyan added inline comments.
Comment at: lib/Driver/Driver.cpp:492
+ .Case("64", Target.get64BitArchVariant())
+ .Default(Target);
+
rnk wrote:
> We should emit a diagnostic for invalid -mabi=values.
We already do that:
$ clang -target mips-linux-gnu -c test.c -mabi=xxx
error: unknown target ABI 'xxx'
Repository:
rC Clang
https://reviews.llvm.org/D52290
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52571: [Sema] Handle __va_start for Windows/ARM64 in the same way as for ARM
rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm Repository: rC Clang https://reviews.llvm.org/D52571 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52571: [Sema] Handle __va_start for Windows/ARM64 in the same way as for ARM
mstorsjo created this revision.
mstorsjo added reviewers: dmajor, mgrang, ssijaric, rnk, compnerd.
Herald added subscribers: chrib, kristof.beyls.
This fixes PR39090.
Repository:
rC Clang
https://reviews.llvm.org/D52571
Files:
lib/Sema/SemaChecking.cpp
test/SemaCXX/microsoft-varargs.cpp
Index: test/SemaCXX/microsoft-varargs.cpp
===
--- test/SemaCXX/microsoft-varargs.cpp
+++ test/SemaCXX/microsoft-varargs.cpp
@@ -1,4 +1,5 @@
// RUN: %clang_cc1 -triple thumbv7-windows -fms-compatibility -fsyntax-only %s
-verify
+// RUN: %clang_cc1 -triple aarch64-windows -fms-compatibility -fsyntax-only %s
-verify
// expected-no-diagnostics
extern "C" {
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -929,6 +929,7 @@
break;
case Builtin::BI__va_start: {
switch (Context.getTargetInfo().getTriple().getArch()) {
+case llvm::Triple::aarch64:
case llvm::Triple::arm:
case llvm::Triple::thumb:
if (SemaBuiltinVAStartARMMicrosoft(TheCall))
Index: test/SemaCXX/microsoft-varargs.cpp
===
--- test/SemaCXX/microsoft-varargs.cpp
+++ test/SemaCXX/microsoft-varargs.cpp
@@ -1,4 +1,5 @@
// RUN: %clang_cc1 -triple thumbv7-windows -fms-compatibility -fsyntax-only %s -verify
+// RUN: %clang_cc1 -triple aarch64-windows -fms-compatibility -fsyntax-only %s -verify
// expected-no-diagnostics
extern "C" {
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -929,6 +929,7 @@
break;
case Builtin::BI__va_start: {
switch (Context.getTargetInfo().getTriple().getArch()) {
+case llvm::Triple::aarch64:
case llvm::Triple::arm:
case llvm::Triple::thumb:
if (SemaBuiltinVAStartARMMicrosoft(TheCall))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51686: [OpenMP] Improve search for libomptarget-nvptx
Hahnfeld added inline comments. Comment at: lib/Driver/ToolChains/Cuda.cpp:665 +// Add path to lib / lib64 folder. +SmallString<256> DefaultLibPath = ABataev wrote: > You're changing the order of the lookup for the paths. Is this intended? If > so, you need the test for this. Yes, that's explained in the summary. I'll try to see if there is a way to test this, even if we can't expect libomptarget-nvptx to be built together with Clang. At the moment I think only `LIBRARY_PATH` is tested, I'm adding some for the new `--libomptarget-nvptx-path`. Repository: rC Clang https://reviews.llvm.org/D51686 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51686: [OpenMP] Improve search for libomptarget-nvptx
ABataev added inline comments.
Comment at: lib/Driver/ToolChains/Cuda.cpp:515
+ if (Arg *A = Args.getLastArg(options::OPT_libomptarget_nvptx_path_EQ))
+CmdArgs.push_back(Args.MakeArgString(Twine("-L") + A->getValue()));
+
`const Arg *A`
Comment at: lib/Driver/ToolChains/Cuda.cpp:651
+
+if (Arg *A =
DriverArgs.getLastArg(options::OPT_libomptarget_nvptx_path_EQ))
+ LibraryPaths.push_back(A->getValue());
`const Arg *`
Comment at: lib/Driver/ToolChains/Cuda.cpp:665
+// Add path to lib / lib64 folder.
+SmallString<256> DefaultLibPath =
You're changing the order of the lookup for the paths. Is this intended? If so,
you need the test for this.
Repository:
rC Clang
https://reviews.llvm.org/D51686
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52538: [MinGW] Allow using ASan
mstorsjo added a comment. Also, I notice that ubsan also is built for windows, but in the MSVC toolchain, only asan (and libfuzzer) can be enabled, not ubsan. Is there something else missing for ubsan, or what's the matter with that one? Repository: rC Clang https://reviews.llvm.org/D52538 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50294: [Driver] Use -gdwarf-3 by default for FreeBSD
MaskRay added a comment. In https://reviews.llvm.org/D50294#1245454, @emaste wrote: > I'm using this change: > https://github.com/emaste/freebsd/commit/1c3deab6d518feb1a7e88de5b342a139e4022a21 > > In FreeBSD 12 and later we use Clang, lld, and ELF Tool Chain. (We still have > gas and objdump from the outdated binutils 2.17.50.) Will you upstream this commit (I can abandon this one)? I created this revision because I was learning clangDriver... I had a vague and probably incorrect impression that some folks said kgdb or dtrace or whatever might not support DWARF 3 or 4. Repository: rC Clang https://reviews.llvm.org/D50294 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51686: [OpenMP] Improve search for libomptarget-nvptx
Hahnfeld added a comment. Ping Repository: rC Clang https://reviews.llvm.org/D51686 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52434: [OpenMP] Make default schedules for NVPTX target regions in SPMD mode achieve coalescing
ABataev added inline comments.
Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:9199
+ OpenMPDistScheduleClauseKind *ScheduleKind, llvm::Value *) const {
+ return;
+}
Remove `return;`, it is not required
Comment at: lib/CodeGen/CGOpenMPRuntimeNVPTX.h:345
+ void setDefaultDistScheduleAndChunk(CodeGenFunction ,
+ OpenMPDistScheduleClauseKind *ScheduleKind,
+ llvm::Value *) const override;
Modify it to be the reference rather than the pointer.
Repository:
rC Clang
https://reviews.llvm.org/D52434
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52434: [OpenMP] Make default schedules for NVPTX target regions in SPMD mode achieve coalescing
gtbercea updated this revision to Diff 167172.
gtbercea edited the summary of this revision.
gtbercea added a comment.
Only change default schedule for distribute directive.
Repository:
rC Clang
https://reviews.llvm.org/D52434
Files:
lib/CodeGen/CGOpenMPRuntime.cpp
lib/CodeGen/CGOpenMPRuntime.h
lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
lib/CodeGen/CGOpenMPRuntimeNVPTX.h
lib/CodeGen/CGStmtOpenMP.cpp
test/OpenMP/nvptx_target_teams_distribute_parallel_for_codegen.cpp
test/OpenMP/nvptx_target_teams_distribute_parallel_for_simd_codegen.cpp
Index: test/OpenMP/nvptx_target_teams_distribute_parallel_for_simd_codegen.cpp
===
--- test/OpenMP/nvptx_target_teams_distribute_parallel_for_simd_codegen.cpp
+++ test/OpenMP/nvptx_target_teams_distribute_parallel_for_simd_codegen.cpp
@@ -33,7 +33,7 @@
l = i;
}
- #pragma omp target teams distribute parallel for simd map(tofrom: aa) num_teams(M) thread_limit(64)
+ #pragma omp target teams distribute parallel for simd map(tofrom: aa) num_teams(M) thread_limit(64)
for(int i = 0; i < n; i++) {
aa[i] += 1;
}
@@ -82,7 +82,7 @@
// CHECK-LABEL: define {{.*}}void {{@__omp_offloading_.+}}(
// CHECK-DAG: [[THREAD_LIMIT:%.+]] = call i32 @llvm.nvvm.read.ptx.sreg.ntid.x()
// CHECK: call void @__kmpc_spmd_kernel_init(i32 [[THREAD_LIMIT]], i16 0, i16 0)
-// CHECK: call void @__kmpc_for_static_init_4({{.+}}, {{.+}}, {{.+}} 92,
+// CHECK: call void @__kmpc_for_static_init_4({{.+}}, {{.+}}, {{.+}} 91,
// CHECK: {{call|invoke}} void [[OUTL2:@.+]](
// CHECK: call void @__kmpc_for_static_fini(
// CHECK: call void @__kmpc_spmd_kernel_deinit()
@@ -96,7 +96,7 @@
// CHECK-LABEL: define {{.*}}void {{@__omp_offloading_.+}}(
// CHECK-DAG: [[THREAD_LIMIT:%.+]] = call i32 @llvm.nvvm.read.ptx.sreg.ntid.x()
// CHECK: call void @__kmpc_spmd_kernel_init(i32 [[THREAD_LIMIT]], i16 0, i16 0)
-// CHECK: call void @__kmpc_for_static_init_4({{.+}}, {{.+}}, {{.+}} 92,
+// CHECK: call void @__kmpc_for_static_init_4({{.+}}, {{.+}}, {{.+}} 91,
// CHECK: {{call|invoke}} void [[OUTL3:@.+]](
// CHECK: call void @__kmpc_for_static_fini(
// CHECK: call void @__kmpc_spmd_kernel_deinit()
@@ -112,7 +112,7 @@
// CHECK-DAG: [[THREAD_LIMIT:%.+]] = call i32 @llvm.nvvm.read.ptx.sreg.ntid.x()
// CHECK: call void @__kmpc_spmd_kernel_init(i32 [[THREAD_LIMIT]], i16 0, i16 0)
// CHECK: store {{.+}} 99, {{.+}}* [[COMB_UB:%.+]], align
-// CHECK: call void @__kmpc_for_static_init_4({{.+}}, {{.+}}, {{.+}} 92, {{.+}}, {{.+}}, {{.+}}* [[COMB_UB]],
+// CHECK: call void @__kmpc_for_static_init_4({{.+}}, {{.+}}, {{.+}} 91, {{.+}}, {{.+}}, {{.+}}* [[COMB_UB]],
// CHECK: {{call|invoke}} void [[OUTL4:@.+]](
// CHECK: call void @__kmpc_for_static_fini(
// CHECK: call void @__kmpc_spmd_kernel_deinit()
Index: test/OpenMP/nvptx_target_teams_distribute_parallel_for_codegen.cpp
===
--- test/OpenMP/nvptx_target_teams_distribute_parallel_for_codegen.cpp
+++ test/OpenMP/nvptx_target_teams_distribute_parallel_for_codegen.cpp
@@ -35,7 +35,7 @@
l = i;
}
- #pragma omp target teams distribute parallel for map(tofrom: aa) num_teams(M) thread_limit(64)
+#pragma omp target teams distribute parallel for map(tofrom: aa) num_teams(M) thread_limit(64)
for(int i = 0; i < n; i++) {
aa[i] += 1;
}
@@ -87,7 +87,7 @@
// CHECK-LABEL: define {{.*}}void {{@__omp_offloading_.+}}(
// CHECK-DAG: [[THREAD_LIMIT:%.+]] = call i32 @llvm.nvvm.read.ptx.sreg.ntid.x()
// CHECK: call void @__kmpc_spmd_kernel_init(i32 [[THREAD_LIMIT]], i16 0, i16 0)
-// CHECK: call void @__kmpc_for_static_init_4({{.+}}, {{.+}}, {{.+}} 92,
+// CHECK: call void @__kmpc_for_static_init_4({{.+}}, {{.+}}, {{.+}} 91,
// CHECK: {{call|invoke}} void [[OUTL2:@.+]](
// CHECK: call void @__kmpc_for_static_fini(
// CHECK: call void @__kmpc_spmd_kernel_deinit()
@@ -101,7 +101,7 @@
// CHECK-LABEL: define {{.*}}void {{@__omp_offloading_.+}}(
// CHECK-DAG: [[THREAD_LIMIT:%.+]] = call i32 @llvm.nvvm.read.ptx.sreg.ntid.x()
// CHECK: call void @__kmpc_spmd_kernel_init(i32 [[THREAD_LIMIT]], i16 0, i16 0)
-// CHECK: call void @__kmpc_for_static_init_4({{.+}}, {{.+}}, {{.+}} 92,
+// CHECK: call void @__kmpc_for_static_init_4({{.+}}, {{.+}}, {{.+}} 91,
// CHECK: {{call|invoke}} void [[OUTL3:@.+]](
// CHECK: call void @__kmpc_for_static_fini(
// CHECK: call void @__kmpc_spmd_kernel_deinit()
@@ -117,7 +117,7 @@
// CHECK-DAG: [[THREAD_LIMIT:%.+]] = call i32 @llvm.nvvm.read.ptx.sreg.ntid.x()
// CHECK: call void @__kmpc_spmd_kernel_init(i32 [[THREAD_LIMIT]], i16 0, i16 0)
// CHECK: store {{.+}} 99, {{.+}}* [[COMB_UB:%.+]], align
-// CHECK: call void @__kmpc_for_static_init_4({{.+}}, {{.+}}, {{.+}} 92, {{.+}}, {{.+}}, {{.+}}* [[COMB_UB]],
+// CHECK: call void @__kmpc_for_static_init_4({{.+}}, {{.+}}, {{.+}} 91, {{.+}}, {{.+}}, {{.+}}* [[COMB_UB]],
// CHECK: {{call|invoke}} void [[OUTL4:@.+]](
// CHECK: call void
[PATCH] D52552: [clang-tidy] Flag Classes Inheriting From Structs
lebedev.ri added inline comments. Comment at: test/clang-tidy/misc-class-inherit-from-struct.cpp:13 +}; + +class C lebedev.ri wrote: > Missing cases: > * struct inheriting from struct > * Different inheritance visibility: > * You only check the default visibility > * What if class explicitly-`public`ly inherits from `struct`? > * What if class explicitly-`private`ly inherits from `struct`? > * What if class explicitly-`protected`ly inherits from `struct`? > * Same for `struct` inheriting from struct? Also, i have only thought about it, there are no tests about what happens if you (say, publicly) inherit from `struct`, that does not have anything `public`. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52552 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r343131 - P1008R1 Classes with user-declared constructors are never aggregates in
Author: rsmith
Date: Wed Sep 26 12:00:16 2018
New Revision: 343131
URL: http://llvm.org/viewvc/llvm-project?rev=343131=rev
Log:
P1008R1 Classes with user-declared constructors are never aggregates in
C++20.
Added:
cfe/trunk/test/SemaCXX/cxx2a-compat.cpp
cfe/trunk/test/SemaCXX/cxx2a-initializer-aggregates.cpp
Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/AST/DeclCXX.cpp
cfe/trunk/lib/Sema/SemaInit.cpp
cfe/trunk/www/cxx_status.html
Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=343131=343130=343131=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Wed Sep 26 12:00:16
2018
@@ -1872,6 +1872,9 @@ def err_reference_bind_init_list : Error
def err_init_list_bad_dest_type : Error<
"%select{|non-aggregate }0type %1 cannot be initialized with an initializer "
"list">;
+def warn_cxx2a_compat_aggregate_init_with_ctors : Warning<
+ "aggregate initialization of type %0 with user-declared constructors "
+ "is incompatible with C++2a">, DefaultIgnore, InGroup;
def err_reference_bind_to_bitfield : Error<
"%select{non-const|volatile}0 reference cannot bind to "
Modified: cfe/trunk/lib/AST/DeclCXX.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclCXX.cpp?rev=343131=343130=343131=diff
==
--- cfe/trunk/lib/AST/DeclCXX.cpp (original)
+++ cfe/trunk/lib/AST/DeclCXX.cpp Wed Sep 26 12:00:16 2018
@@ -731,9 +731,14 @@ void CXXRecordDecl::addedMember(Decl *D)
}
// C++11 [dcl.init.aggr]p1: DR1518
-// An aggregate is an array or a class with no user-provided, explicit,
or
-// inherited constructors
-if (Constructor->isUserProvided() || Constructor->isExplicit())
+// An aggregate is an array or a class with no user-provided [or]
+// explicit [...] constructors
+// C++20 [dcl.init.aggr]p1:
+// An aggregate is an array or a class with no user-declared [...]
+// constructors
+if (getASTContext().getLangOpts().CPlusPlus2a
+? !Constructor->isImplicit()
+: (Constructor->isUserProvided() || Constructor->isExplicit()))
data().Aggregate = false;
}
Modified: cfe/trunk/lib/Sema/SemaInit.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaInit.cpp?rev=343131=343130=343131=diff
==
--- cfe/trunk/lib/Sema/SemaInit.cpp (original)
+++ cfe/trunk/lib/Sema/SemaInit.cpp Wed Sep 26 12:00:16 2018
@@ -964,6 +964,14 @@ void InitListChecker::CheckImplicitInitL
StructuredSubobjectInitList->getEndLoc()),
"}");
}
+
+// Warn if this type won't be an aggregate in future versions of C++.
+auto *CXXRD = T->getAsCXXRecordDecl();
+if (CXXRD && CXXRD->hasUserDeclaredConstructor()) {
+ SemaRef.Diag(StructuredSubobjectInitList->getBeginLoc(),
+ diag::warn_cxx2a_compat_aggregate_init_with_ctors)
+ << StructuredSubobjectInitList->getSourceRange() << T;
+}
}
}
@@ -1106,9 +1114,30 @@ void InitListChecker::CheckExplicitInitL
}
}
- if (!VerifyOnly && T->isScalarType() &&
- IList->getNumInits() == 1 && !isa(IList->getInit(0)))
-warnBracedScalarInit(SemaRef, Entity, IList->getSourceRange());
+ if (!VerifyOnly) {
+if (T->isScalarType() && IList->getNumInits() == 1 &&
+!isa(IList->getInit(0)))
+ warnBracedScalarInit(SemaRef, Entity, IList->getSourceRange());
+
+// Warn if this is a class type that won't be an aggregate in future
+// versions of C++.
+auto *CXXRD = T->getAsCXXRecordDecl();
+if (CXXRD && CXXRD->hasUserDeclaredConstructor()) {
+ // Don't warn if there's an equivalent default constructor that would be
+ // used instead.
+ bool HasEquivCtor = false;
+ if (IList->getNumInits() == 0) {
+auto *CD = SemaRef.LookupDefaultConstructor(CXXRD);
+HasEquivCtor = CD && !CD->isDeleted();
+ }
+
+ if (!HasEquivCtor) {
+SemaRef.Diag(IList->getBeginLoc(),
+ diag::warn_cxx2a_compat_aggregate_init_with_ctors)
+<< IList->getSourceRange() << T;
+ }
+}
+ }
}
void InitListChecker::CheckListElementTypes(const InitializedEntity ,
Added: cfe/trunk/test/SemaCXX/cxx2a-compat.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/cxx2a-compat.cpp?rev=343131=auto
==
--- cfe/trunk/test/SemaCXX/cxx2a-compat.cpp (added)
+++ cfe/trunk/test/SemaCXX/cxx2a-compat.cpp Wed Sep 26 12:00:16 2018
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -fsyntax-only
[PATCH] D52290: [driver][mips] Adjust target triple accordingly to provided ABI name
rnk added inline comments.
Comment at: lib/Driver/Driver.cpp:492
+ .Case("64", Target.get64BitArchVariant())
+ .Default(Target);
+
We should emit a diagnostic for invalid -mabi=values.
Repository:
rC Clang
https://reviews.llvm.org/D52290
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49864: [clang-tidy] The script clang-tidy-diff.py doesn't accept 'pass by' options (--)
janosimas added inline comments.
Comment at: clang-tidy/tool/clang-tidy-diff.py:123-130
if args.fix:
command.append('-fix')
if args.checks != '':
command.append('-checks=' + quote + args.checks + quote)
if args.quiet:
command.append('-quiet')
if args.build_path is not None:
janosimas wrote:
> alexfh wrote:
> > If we make the script leave out the `--` flag, we should stop forwarding
> > these flags and the `extra_arg(_before)?` below. Otherwise it's too
> > confusing (should one place -fix before `--` or after? what about
> > `-warnings-as-errors`?).
> >
> > Please also update the usage example at the top.
> What about keep the current `--` behavior and add a new flag
> `-extra-tidy-flags` ?
`-extra-tidy-arg` to maintain consistency.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D49864
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49864: [clang-tidy] The script clang-tidy-diff.py doesn't accept 'pass by' options (--)
janosimas requested review of this revision.
janosimas added inline comments.
Comment at: clang-tidy/tool/clang-tidy-diff.py:123-130
if args.fix:
command.append('-fix')
if args.checks != '':
command.append('-checks=' + quote + args.checks + quote)
if args.quiet:
command.append('-quiet')
if args.build_path is not None:
alexfh wrote:
> If we make the script leave out the `--` flag, we should stop forwarding
> these flags and the `extra_arg(_before)?` below. Otherwise it's too confusing
> (should one place -fix before `--` or after? what about
> `-warnings-as-errors`?).
>
> Please also update the usage example at the top.
What about keep the current `--` behavior and add a new flag
`-extra-tidy-flags` ?
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D49864
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52552: [clang-tidy] Flag Classes Inheriting From Structs
Eugene.Zelenko added inline comments.
Comment at: docs/ReleaseNotes.rst:109
+- New :doc:`misc-class-inherit-from-struct
+ ` check.
Please use alphabetical order for new checks list.
Comment at: docs/clang-tidy/checks/misc-class-inherit-from-struct.rst:6
+
+Finds instances of classes inheriting from structs. Structs are meant for
+storing data and are often used to maintain C compatibility. Having a class
aaron.ballman wrote:
> I don't agree with the predicate here -- structs aren't just used for C
> compatibility (look at as an example). They're also useful when
> all members need to be public, which is exactly the situation you claim may
> be unintentional.
Please make first statement same as in Release Notes.
Comment at: docs/clang-tidy/checks/misc-class-inherit-from-struct.rst:15
+ struct A {};
+ class B: public A {}; //throws an error, members of A might be
+//unintentionally exposed
I think warning is more correct term then error for Clang-tidy diagnistics.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52552
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52530: [analyzer] scan-build: if --status-bugs is passed, don't forget about the exit status of the actual build
george.karpenkov added a comment. @jroelofs Thanks, but in general code owners would need to take a look as well. @lebedev.ri The code looks great, thanks! Repository: rC Clang https://reviews.llvm.org/D52530 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D52499: [clang-cl] Make /Gs imply default stack probes, not /Gs0 (PR39074)
Sounds great, thanks! On Wed, Sep 26, 2018 at 8:33 AM Hans Wennborg via Phabricator via cfe-commits wrote: > hans added inline comments. > > > > Comment at: include/clang/Driver/CLCompatOptions.td:94 > +def : CLFlag<"Gs">, HelpText<"Use stack probes (default)">, > + Alias, AliasArgs<["4096"]>; > def _SLASH_Gs : CLJoined<"Gs">, > > thakis wrote: > > > https://docs.microsoft.com/en-us/cpp/build/reference/gs-control-stack-checking-calls?view=vs-2017 > still claims that /Gs is /Gs0 though. Is that just wrong? > https://bugs.llvm.org/show_bug.cgi?id=39074#c2 suggests it is. Should we > ask bruce to file a doc bug? > > > > And since this flag is supposed to get the default behavior, should it > be a CLIgnoredFlag instead of duplicating the 4096 somewhat redundantly? > I've submitted feedback on the document page: > https://github.com/MicrosoftDocs/cpp-docs/issues/445 > > By not ignoring it, we enable using /Gs to override a previous e.g. /Gs0 > flag, which I think is the correct thing to do. > > > Repository: > rL LLVM > > https://reviews.llvm.org/D52499 > > > > ___ > cfe-commits mailing list > cfe-commits@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits > ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52392: [X86] For lzcnt/tzcnt intrinsics use cttz/ctlz intrinsics with zero_undef flag set to false.
This revision was automatically updated to reflect the committed changes.
Closed by commit rC343126: [X86] For lzcnt/tzcnt intrinsics use cttz/ctlz
intrinsics with zero_undef flag… (authored by ctopper, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D52392?vs=166606=167159#toc
Repository:
rC Clang
https://reviews.llvm.org/D52392
Files:
include/clang/Basic/BuiltinsX86.def
include/clang/Basic/BuiltinsX86_64.def
lib/CodeGen/CGBuiltin.cpp
lib/Headers/bmiintrin.h
lib/Headers/lzcntintrin.h
test/CodeGen/bmi-builtins.c
test/CodeGen/lzcnt-builtins.c
Index: include/clang/Basic/BuiltinsX86.def
===
--- include/clang/Basic/BuiltinsX86.def
+++ include/clang/Basic/BuiltinsX86.def
@@ -727,8 +727,14 @@
TARGET_BUILTIN(__builtin_ia32_rdseed16_step, "UiUs*", "n", "rdseed")
TARGET_BUILTIN(__builtin_ia32_rdseed32_step, "UiUi*", "n", "rdseed")
+// LZCNT
+TARGET_BUILTIN(__builtin_ia32_lzcnt_u16, "UsUs", "nc", "lzcnt")
+TARGET_BUILTIN(__builtin_ia32_lzcnt_u32, "UiUi", "nc", "lzcnt")
+
// BMI
TARGET_BUILTIN(__builtin_ia32_bextr_u32, "UiUiUi", "nc", "bmi")
+TARGET_BUILTIN(__builtin_ia32_tzcnt_u16, "UsUs", "nc", "")
+TARGET_BUILTIN(__builtin_ia32_tzcnt_u32, "UiUi", "nc", "")
// BMI2
TARGET_BUILTIN(__builtin_ia32_bzhi_si, "UiUiUi", "nc", "bmi2")
Index: include/clang/Basic/BuiltinsX86_64.def
===
--- include/clang/Basic/BuiltinsX86_64.def
+++ include/clang/Basic/BuiltinsX86_64.def
@@ -81,7 +81,9 @@
TARGET_BUILTIN(__builtin_ia32_subborrow_u64, "UcUcULLiULLiULLi*", "n", "")
TARGET_BUILTIN(__builtin_ia32_rdrand64_step, "UiULLi*", "n", "rdrnd")
TARGET_BUILTIN(__builtin_ia32_rdseed64_step, "UiULLi*", "n", "rdseed")
+TARGET_BUILTIN(__builtin_ia32_lzcnt_u64, "ULLiULLi", "nc", "lzcnt")
TARGET_BUILTIN(__builtin_ia32_bextr_u64, "ULLiULLiULLi", "nc", "bmi")
+TARGET_BUILTIN(__builtin_ia32_tzcnt_u64, "ULLiULLi", "nc", "")
TARGET_BUILTIN(__builtin_ia32_bzhi_di, "ULLiULLiULLi", "nc", "bmi2")
TARGET_BUILTIN(__builtin_ia32_pdep_di, "ULLiULLiULLi", "nc", "bmi2")
TARGET_BUILTIN(__builtin_ia32_pext_di, "ULLiULLiULLi", "nc", "bmi2")
Index: test/CodeGen/bmi-builtins.c
===
--- test/CodeGen/bmi-builtins.c
+++ test/CodeGen/bmi-builtins.c
@@ -15,9 +15,7 @@
unsigned short test__tzcnt_u16(unsigned short __X) {
// CHECK-LABEL: test__tzcnt_u16
- // CHECK: zext i16 %{{.*}} to i32
- // CHECK: icmp ne i32 %{{.*}}, 0
- // CHECK: i16 @llvm.cttz.i16(i16 %{{.*}}, i1 true)
+ // CHECK: i16 @llvm.cttz.i16(i16 %{{.*}}, i1 false)
return __tzcnt_u16(__X);
}
@@ -57,15 +55,13 @@
unsigned int test__tzcnt_u32(unsigned int __X) {
// CHECK-LABEL: test__tzcnt_u32
- // CHECK: icmp ne i32 %{{.*}}, 0
- // CHECK: i32 @llvm.cttz.i32(i32 %{{.*}}, i1 true)
+ // CHECK: i32 @llvm.cttz.i32(i32 %{{.*}}, i1 false)
return __tzcnt_u32(__X);
}
int test_mm_tzcnt_32(unsigned int __X) {
// CHECK-LABEL: test_mm_tzcnt_32
- // CHECK: icmp ne i32 %{{.*}}, 0
- // CHECK: i32 @llvm.cttz.i32(i32 %{{.*}}, i1 true)
+ // CHECK: i32 @llvm.cttz.i32(i32 %{{.*}}, i1 false)
return _mm_tzcnt_32(__X);
}
@@ -105,25 +101,21 @@
unsigned long long test__tzcnt_u64(unsigned long long __X) {
// CHECK-LABEL: test__tzcnt_u64
- // CHECK: icmp ne i64 %{{.*}}, 0
- // CHECK: i64 @llvm.cttz.i64(i64 %{{.*}}, i1 true)
+ // CHECK: i64 @llvm.cttz.i64(i64 %{{.*}}, i1 false)
return __tzcnt_u64(__X);
}
long long test_mm_tzcnt_64(unsigned long long __X) {
// CHECK-LABEL: test_mm_tzcnt_64
- // CHECK: icmp ne i64 %{{.*}}, 0
- // CHECK: i64 @llvm.cttz.i64(i64 %{{.*}}, i1 true)
+ // CHECK: i64 @llvm.cttz.i64(i64 %{{.*}}, i1 false)
return _mm_tzcnt_64(__X);
}
// Intel intrinsics
unsigned short test_tzcnt_u16(unsigned short __X) {
// CHECK-LABEL: test_tzcnt_u16
- // CHECK: zext i16 %{{.*}} to i32
- // CHECK: icmp ne i32 %{{.*}}, 0
- // CHECK: i16 @llvm.cttz.i16(i16 %{{.*}}, i1 true)
+ // CHECK: i16 @llvm.cttz.i16(i16 %{{.*}}, i1 false)
return _tzcnt_u16(__X);
}
@@ -168,8 +160,7 @@
unsigned int test_tzcnt_u32(unsigned int __X) {
// CHECK-LABEL: test_tzcnt_u32
- // CHECK: icmp ne i32 %{{.*}}, 0
- // CHECK: i32 @llvm.cttz.i32(i32 %{{.*}}, i1 true)
+ // CHECK: i32 @llvm.cttz.i32(i32 %{{.*}}, i1 false)
return _tzcnt_u32(__X);
}
@@ -215,7 +206,6 @@
unsigned long long test_tzcnt_u64(unsigned long long __X) {
// CHECK-LABEL: test_tzcnt_u64
- // CHECK: icmp ne i64 %{{.*}}, 0
- // CHECK: i64 @llvm.cttz.i64(i64 %{{.*}}, i1 true)
+ // CHECK: i64 @llvm.cttz.i64(i64 %{{.*}}, i1 false)
return _tzcnt_u64(__X);
}
Index: test/CodeGen/lzcnt-builtins.c
===
--- test/CodeGen/lzcnt-builtins.c
+++ test/CodeGen/lzcnt-builtins.c
@@ -5,30 +5,30 @@
unsigned short test__lzcnt16(unsigned short __X)
{
- // CHECK: @llvm.ctlz.i16
+ // CHECK:
r343126 - [X86] For lzcnt/tzcnt intrinsics use cttz/ctlz intrinsics with zero_undef flag set to false.
Author: ctopper
Date: Wed Sep 26 10:01:44 2018
New Revision: 343126
URL: http://llvm.org/viewvc/llvm-project?rev=343126=rev
Log:
[X86] For lzcnt/tzcnt intrinsics use cttz/ctlz intrinsics with zero_undef flag
set to false.
Previously we used a select and the zero_undef=true intrinsic. In -O2 this
pattern will get optimized to zero_undef=false. But in -O0 this optimization
won't happen. This results in a compare and cmov being wrapped around a
tzcnt/lzcnt instruction.
By using the zero_undef=false intrinsic directly without the select, we can
improve the -O0 codegen to just an lzcnt/tzcnt instruction.
Differential Revision: https://reviews.llvm.org/D52392
Modified:
cfe/trunk/include/clang/Basic/BuiltinsX86.def
cfe/trunk/include/clang/Basic/BuiltinsX86_64.def
cfe/trunk/lib/CodeGen/CGBuiltin.cpp
cfe/trunk/lib/Headers/bmiintrin.h
cfe/trunk/lib/Headers/lzcntintrin.h
cfe/trunk/test/CodeGen/bmi-builtins.c
cfe/trunk/test/CodeGen/lzcnt-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=343126=343125=343126=diff
==
--- cfe/trunk/include/clang/Basic/BuiltinsX86.def (original)
+++ cfe/trunk/include/clang/Basic/BuiltinsX86.def Wed Sep 26 10:01:44 2018
@@ -727,8 +727,14 @@ TARGET_BUILTIN(__builtin_ia32_subborrow_
TARGET_BUILTIN(__builtin_ia32_rdseed16_step, "UiUs*", "n", "rdseed")
TARGET_BUILTIN(__builtin_ia32_rdseed32_step, "UiUi*", "n", "rdseed")
+// LZCNT
+TARGET_BUILTIN(__builtin_ia32_lzcnt_u16, "UsUs", "nc", "lzcnt")
+TARGET_BUILTIN(__builtin_ia32_lzcnt_u32, "UiUi", "nc", "lzcnt")
+
// BMI
TARGET_BUILTIN(__builtin_ia32_bextr_u32, "UiUiUi", "nc", "bmi")
+TARGET_BUILTIN(__builtin_ia32_tzcnt_u16, "UsUs", "nc", "")
+TARGET_BUILTIN(__builtin_ia32_tzcnt_u32, "UiUi", "nc", "")
// BMI2
TARGET_BUILTIN(__builtin_ia32_bzhi_si, "UiUiUi", "nc", "bmi2")
Modified: cfe/trunk/include/clang/Basic/BuiltinsX86_64.def
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/BuiltinsX86_64.def?rev=343126=343125=343126=diff
==
--- cfe/trunk/include/clang/Basic/BuiltinsX86_64.def (original)
+++ cfe/trunk/include/clang/Basic/BuiltinsX86_64.def Wed Sep 26 10:01:44 2018
@@ -81,7 +81,9 @@ TARGET_BUILTIN(__builtin_ia32_addcarry_u
TARGET_BUILTIN(__builtin_ia32_subborrow_u64, "UcUcULLiULLiULLi*", "n", "")
TARGET_BUILTIN(__builtin_ia32_rdrand64_step, "UiULLi*", "n", "rdrnd")
TARGET_BUILTIN(__builtin_ia32_rdseed64_step, "UiULLi*", "n", "rdseed")
+TARGET_BUILTIN(__builtin_ia32_lzcnt_u64, "ULLiULLi", "nc", "lzcnt")
TARGET_BUILTIN(__builtin_ia32_bextr_u64, "ULLiULLiULLi", "nc", "bmi")
+TARGET_BUILTIN(__builtin_ia32_tzcnt_u64, "ULLiULLi", "nc", "")
TARGET_BUILTIN(__builtin_ia32_bzhi_di, "ULLiULLiULLi", "nc", "bmi2")
TARGET_BUILTIN(__builtin_ia32_pdep_di, "ULLiULLiULLi", "nc", "bmi2")
TARGET_BUILTIN(__builtin_ia32_pext_di, "ULLiULLiULLi", "nc", "bmi2")
Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBuiltin.cpp?rev=343126=343125=343126=diff
==
--- cfe/trunk/lib/CodeGen/CGBuiltin.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp Wed Sep 26 10:01:44 2018
@@ -9164,6 +9164,18 @@ Value *CodeGenFunction::EmitX86BuiltinEx
Ops[0]);
return Builder.CreateExtractValue(Call, 0);
}
+ case X86::BI__builtin_ia32_lzcnt_u16:
+ case X86::BI__builtin_ia32_lzcnt_u32:
+ case X86::BI__builtin_ia32_lzcnt_u64: {
+Value *F = CGM.getIntrinsic(Intrinsic::ctlz, Ops[0]->getType());
+return Builder.CreateCall(F, {Ops[0], Builder.getInt1(false)});
+ }
+ case X86::BI__builtin_ia32_tzcnt_u16:
+ case X86::BI__builtin_ia32_tzcnt_u32:
+ case X86::BI__builtin_ia32_tzcnt_u64: {
+Value *F = CGM.getIntrinsic(Intrinsic::cttz, Ops[0]->getType());
+return Builder.CreateCall(F, {Ops[0], Builder.getInt1(false)});
+ }
case X86::BI__builtin_ia32_undef128:
case X86::BI__builtin_ia32_undef256:
case X86::BI__builtin_ia32_undef512:
Modified: cfe/trunk/lib/Headers/bmiintrin.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/bmiintrin.h?rev=343126=343125=343126=diff
==
--- cfe/trunk/lib/Headers/bmiintrin.h (original)
+++ cfe/trunk/lib/Headers/bmiintrin.h Wed Sep 26 10:01:44 2018
@@ -62,7 +62,7 @@
static __inline__ unsigned short __RELAXED_FN_ATTRS
__tzcnt_u16(unsigned short __X)
{
- return __X ? __builtin_ctzs(__X) : 16;
+ return __builtin_ia32_tzcnt_u16(__X);
}
/// Performs a bitwise AND of the second operand with the one's
@@ -196,7 +196,7 @@ __blsr_u32(unsigned int __X)
static __inline__ unsigned int __RELAXED_FN_ATTRS
__tzcnt_u32(unsigned int __X)
{
-
[PATCH] D52536: clang-format: [JS] conditional types.
krasimir accepted this revision. krasimir added inline comments. This revision is now accepted and ready to land. Comment at: unittests/Format/FormatTestJS.cpp:2315 + " never) extends((k: infer I) => void) ? I " + ": never;"); +} Add a comment that this formatting of the type expression is not yet clear (esp. I dislike the last line). Repository: rC Clang https://reviews.llvm.org/D52536 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52552: [clang-tidy] Flag Classes Inheriting From Structs
aaron.ballman added a comment. This strikes me as likely being a very chatty diagnostic. For instance, it will trigger on harmless code that uses empty base class optimization (in addition to other concerns pointed out by @lebedev.ri). It seems like the real concern here is unintentional information disclosure, but I don't see a good heuristic for determining that something is "unintentional" or not. I'd be curious to know how frequently this triggers on some large C++ code bases. Comment at: docs/clang-tidy/checks/misc-class-inherit-from-struct.rst:6-10 +Finds instances of classes inheriting from structs. Structs are meant for +storing data and are often used to maintain C compatibility. Having a class +inherit from them can lead to confusion with what type of object is being +dealt with. Additionally, the default public nature of struct members can +lead to unintentional exposure of members. I don't agree with the predicate here -- structs aren't just used for C compatibility (look at as an example). They're also useful when all members need to be public, which is exactly the situation you claim may be unintentional. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52552 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen(), size() or equal length
aaron.ballman added inline comments.
Comment at: clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp:1038
+
+ SmallString<128> NewAddNullTermExprStr;
+ NewAddNullTermExprStr = "\n";
JonasToth wrote:
> whisperity wrote:
> > aaron.ballman wrote:
> > > This should be done using a `Twine`.
> > Can you please give an example of how to well-approach this? We had issues
> > with using Twines on the stack and then later on running into weird
> > undefined behaviour. The documentation is not clear to a lot of our
> > colleagues on where the `Twine`'s usage barriers are... (Asking this not
> > just for @Charusso but also for a few more colleagues, @baloghadamsoftware
> > e.g.)
> naive approach:
>
> ```
> std::string New =
> Twine("\n").concat(SpaceBeforeStmtStr).concat(exprToStr(..))...).str();
> ```
> I think retrieving a `SmallString` is not supported. Please note, that
> `Twine` does support taking integer and floats directly without prior
> conversion.
> You can use `operator+` instead of `concat` too.
>
> Did this help or did you have more specific issues?
The important bit is -- don't try to store a Twine (locally or otherwise). It's
fine as a parameter in a function, but otherwise you should try to use it only
as part of an expression. e.g., `(Twine("foo") + "bar" + baz).str()` is a good
pattern to get used to.
https://reviews.llvm.org/D45050
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52281: [clang-tidy] Add modernize check to use std::invoke in generic code
aaron.ballman added inline comments.
Comment at: clang-tidy/modernize/ReplaceGenericFunctorCallCheck.cpp:70
+const auto *Paren = dyn_cast(MFunctor->getCallee());
+const auto *BinOp = dyn_cast(Paren->getSubExpr());
+
How do you know `Paren` won't be null? If it cannot be null, please use
`cast<>` instead, otherwise, you should be checking for null before
dereferencing.
Comment at: clang-tidy/modernize/ReplaceGenericFunctorCallCheck.cpp:85
+
+const std::string Param = ObjName + ", " + FuncName;
+diagnose(MFunctor, Param, OriginalParams);
Perhaps use a `Twine` in the `diagnose()` call for this.
Comment at: clang-tidy/modernize/ReplaceGenericFunctorCallCheck.cpp:94-96
+ const Twine = Twine("::std::invoke(") + Param +
+ (Functor->getNumArgs() == 0 ? "" : ", ") +
+ OriginalParams;
This is dangerous (the intermediary temps will be destroyed and you will be
referencing dangling memory) -- you should lower it into the call to
`CreateReplacement()`.
Comment at: clang-tidy/modernize/ReplaceGenericFunctorCallCheck.cpp:98
+ diag(Functor->getExprLoc(),
+ "Use ::std::invoke to invoke type dependent callable objects.")
+ << FixItHint::CreateReplacement(Functor->getSourceRange(),
Replace.str());
Diagnostics should not be complete sentences, so Use -> use and drop the full
stop.
Comment at: test/clang-tidy/modernize-replace-generic-functor-call.cpp:23-25
+template
+void func2(T func) {
+ func(1);
Can you add a test case that demonstrates this works well in the presence of
std::forward? This is a common pattern:
```
template
void f(Callable&& C, Args&&... As) {
std::forward(C)(std::forward(As)...);
}
```
This could be converted into:
```
template
void f(Callable&& C, Args&&... As) {
std::invoke(C, As...);
}
```
https://reviews.llvm.org/D52281
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52547: Tell whether file/folder for include completions.
ilya-biryukov added a comment. A drive-by comment. Would it be cleaner to pass this information from clang? Relying on completion label seems shaky. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52547 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51741: [coro]Pass rvalue reference for named local variable to return_value
modocache accepted this revision. modocache added a comment. This revision is now accepted and ready to land. This is great, thanks! Sorry for letting it languish. I defer to @GorNishanov, but I don't see why this couldn't go in now and if there're any edge cases I'm missing we can address those in another patch. I pulled this down and played around with it, and everything seemed OK to me. I had one nit about formatting, but other than that this is good to go. Do you have commit access? If not, let me know if I can land this for you. Comment at: lib/Sema/SemaCoroutine.cpp:851 + ExprResult MoveResult = + this->PerformMoveOrCopyInitialization(Entity, NRVOCandidate, E->getType(), E); + if (MoveResult.get()) nit: When I run clang-format on this patch, it suggests the following here: ``` ExprResult MoveResult = this->PerformMoveOrCopyInitialization( Entity, NRVOCandidate, E->getType(), E); ``` Splitting it up as above makes sure this line stays within the 80-character-per-line limit. Repository: rC Clang https://reviews.llvm.org/D51741 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [clang-tools-extra] r342730 - [clangd] Remember to serialize symbol origin in YAML.
> The downside is probably that all symbols in yaml would have the same
origin, which is a bit redundant but doesn't seem to matter much.
All my comments boil down to this one, but the argument is not only
redundant - it does not make sense design-wise to store an origin in the
serialized static index: the origin is static because the symbol is in the
static index, any other value means we did something wrong.
Including origin would make sense if our serialization was used as wire
transfer format, but that's not the case - it's only used for serializing
into the file index.
Also don't think it's not that important, so feel free to keep as is.
On Wed, Sep 26, 2018 at 2:25 PM Eric Liu wrote:
> I think it depends on how you interpret the origin. You could tie the
> origin to the index or to the producer of the symbol. The current model
> seems to be the later (origin is a configuration in SymbolCollector.
> MergeIndex manipulates the origin, but I would rather treat that as a
> special case). And I think it's conceptually simpler and more generic
> (origin can be more than dynamic vs static). The downside is probably that
> all symbols in yaml would have the same origin, which is a bit redundant
> but doesn't seem to matter much.
>
> Both models have their pros and cons, but I don't think it's worth
> spending much time figuring out which one is better when it seems like a
> non-issue.
>
> On Tue, Sep 25, 2018 at 6:58 PM Ilya Biryukov
> wrote:
>
>>
>>
>> Eric Liu schrieb am Di., 25. Sep. 2018, 01:22:
>>
>>> On Tue, Sep 25, 2018 at 6:34 AM Ilya Biryukov
>>> wrote:
>>>
Why would we want to serialize the origin?
>>> We only serialize and deserialize for the static index, it does not seem
to be useful to serialize origin in that scenario.
>>> We serialize Origin because it's a property of Symbol? The origin for
>>> the current YAML symbols is defaulted to "unknown".
>>>
>> My view would be that it's a property of a symbol living in memory, but
>> not the serialized one. E.g. all symbols read from the yaml index should
>> have the static origin. If we store an origin, we allow loading symbols
>> with non-static origin, it's hard to see how that would be useful.
>>
>>
>>
>>
>>> It's true that we *currently* only serialize symbols from static index,
>>> but there is nothing preventing us from doing it for other indexes with
>>> different origins. You could probably override origins when loading static
>>> index, but that doesn't seem to work in general.
>>>
>> The origin seems to be exactly the field that is set and manipulated by
>> index implementations, but they all have the knowledge on what their origin
>> is or how to combine origins of subindexes.
>>
>> Again, it seems there are two classes hiding in symbol. All other fields
>> provide useful information about C++ semantics of the symbol, while origin
>> provides some traceability when combining indexes. The former is something
>> we need to serialize, the latter is something we can infer when
>> deserializing.
>>
>> If we will have a use case for serializing the latter entity(with origin)
>> for any reason, we might add that separately.
>>
>> Not sure if it's worth the trouble changing it, just wanted to point out
>> that storing the origin for each symbol in the yaml index for the purpose
>> of indicating that the symbol is in the yaml index seems to be a bit off.
>>
>>
>>
>>> I checked this in without review as I thought this was a trivial fix.
>>> The binary serialization also serializes the Origin field.
>>>
>> LG to be consistent with binary serialization, the same question applies
>> to binary serialization.
>>
>> Again, the change itself seems fine, just nitpicking on whether putting
>> origin into a symbol at that level makes sense.
>>
>>
>>
Am I missing something?
>>>
On Fri, Sep 21, 2018 at 3:06 PM Eric Liu via cfe-commits <
cfe-commits@lists.llvm.org> wrote:
> Author: ioeric
> Date: Fri Sep 21 06:04:57 2018
> New Revision: 342730
>
> URL: http://llvm.org/viewvc/llvm-project?rev=342730=rev
> Log:
> [clangd] Remember to serialize symbol origin in YAML.
>
> Modified:
> clang-tools-extra/trunk/clangd/index/SymbolYAML.cpp
> clang-tools-extra/trunk/unittests/clangd/SerializationTests.cpp
>
> Modified: clang-tools-extra/trunk/clangd/index/SymbolYAML.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/SymbolYAML.cpp?rev=342730=342729=342730=diff
>
> ==
> --- clang-tools-extra/trunk/clangd/index/SymbolYAML.cpp (original)
> +++ clang-tools-extra/trunk/clangd/index/SymbolYAML.cpp Fri Sep 21
> 06:04:57 2018
> @@ -27,6 +27,7 @@ namespace yaml {
>
> using clang::clangd::Symbol;
> using clang::clangd::SymbolID;
> +using clang::clangd::SymbolOrigin;
> using clang::clangd::SymbolLocation;
[PATCH] D52503: [clangd] Fix bugs with incorrect memory estimate report
This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE343117: [clangd] Fix bugs with incorrect memory estimate
report (authored by omtcyfz, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D52503?vs=167079=167143#toc
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52503
Files:
clangd/index/Serialization.cpp
clangd/index/dex/Dex.cpp
clangd/index/dex/PostingList.h
Index: clangd/index/Serialization.cpp
===
--- clangd/index/Serialization.cpp
+++ clangd/index/Serialization.cpp
@@ -6,8 +6,10 @@
// License. See LICENSE.TXT for details.
//
//===--===//
+
#include "Serialization.h"
#include "Index.h"
+#include "Logger.h"
#include "RIFF.h"
#include "Trace.h"
#include "dex/Dex.h"
@@ -433,8 +435,12 @@
}
trace::Span Tracer("BuildIndex");
- return UseDex ? dex::Dex::build(std::move(Symbols), URISchemes)
-: MemIndex::build(std::move(Symbols), std::move(Refs));
+ auto Index = UseDex ? dex::Dex::build(std::move(Symbols), URISchemes)
+ : MemIndex::build(std::move(Symbols), std::move(Refs));
+ vlog("Loaded {0} from {1} with estimated memory usage {2}",
+ UseDex ? "Dex" : "MemIndex", SymbolFilename,
+ Index->estimateMemoryUsage());
+ return Index;
}
} // namespace clangd
Index: clangd/index/dex/Dex.cpp
===
--- clangd/index/dex/Dex.cpp
+++ clangd/index/dex/Dex.cpp
@@ -130,9 +130,6 @@
for (const auto : TempInvertedIndex)
InvertedIndex.insert(
{TokenToPostingList.first, PostingList(TokenToPostingList.second)});
-
- vlog("Built Dex with estimated memory usage {0} bytes.",
- estimateMemoryUsage());
}
/// Constructs iterators over tokens extracted from the query and exhausts it
@@ -248,8 +245,8 @@
Bytes += SymbolQuality.size() * sizeof(float);
Bytes += LookupTable.getMemorySize();
Bytes += InvertedIndex.getMemorySize();
- for (const auto : InvertedIndex)
-Bytes += P.second.bytes();
+ for (const auto : InvertedIndex)
+Bytes += TokenToPostingList.second.bytes();
return Bytes + BackingDataSize;
}
Index: clangd/index/dex/PostingList.h
===
--- clangd/index/dex/PostingList.h
+++ clangd/index/dex/PostingList.h
@@ -66,10 +66,8 @@
/// go through the chunks and decompress them on-the-fly when necessary.
std::unique_ptr iterator() const;
- /// Returns in-memory size.
- size_t bytes() const {
-return sizeof(Chunk) + Chunks.capacity() * sizeof(Chunk);
- }
+ /// Returns in-memory size of external storage.
+ size_t bytes() const { return Chunks.capacity() * sizeof(Chunk); }
private:
const std::vector Chunks;
Index: clangd/index/Serialization.cpp
===
--- clangd/index/Serialization.cpp
+++ clangd/index/Serialization.cpp
@@ -6,8 +6,10 @@
// License. See LICENSE.TXT for details.
//
//===--===//
+
#include "Serialization.h"
#include "Index.h"
+#include "Logger.h"
#include "RIFF.h"
#include "Trace.h"
#include "dex/Dex.h"
@@ -433,8 +435,12 @@
}
trace::Span Tracer("BuildIndex");
- return UseDex ? dex::Dex::build(std::move(Symbols), URISchemes)
-: MemIndex::build(std::move(Symbols), std::move(Refs));
+ auto Index = UseDex ? dex::Dex::build(std::move(Symbols), URISchemes)
+ : MemIndex::build(std::move(Symbols), std::move(Refs));
+ vlog("Loaded {0} from {1} with estimated memory usage {2}",
+ UseDex ? "Dex" : "MemIndex", SymbolFilename,
+ Index->estimateMemoryUsage());
+ return Index;
}
} // namespace clangd
Index: clangd/index/dex/Dex.cpp
===
--- clangd/index/dex/Dex.cpp
+++ clangd/index/dex/Dex.cpp
@@ -130,9 +130,6 @@
for (const auto : TempInvertedIndex)
InvertedIndex.insert(
{TokenToPostingList.first, PostingList(TokenToPostingList.second)});
-
- vlog("Built Dex with estimated memory usage {0} bytes.",
- estimateMemoryUsage());
}
/// Constructs iterators over tokens extracted from the query and exhausts it
@@ -248,8 +245,8 @@
Bytes += SymbolQuality.size() * sizeof(float);
Bytes += LookupTable.getMemorySize();
Bytes += InvertedIndex.getMemorySize();
- for (const auto : InvertedIndex)
-Bytes += P.second.bytes();
+ for (const auto : InvertedIndex)
+Bytes += TokenToPostingList.second.bytes();
return Bytes + BackingDataSize;
}
Index: clangd/index/dex/PostingList.h
===
--- clangd/index/dex/PostingList.h
+++ clangd/index/dex/PostingList.h
@@ -66,10 +66,8 @@
/// go through the chunks
[clang-tools-extra] r343117 - [clangd] Fix bugs with incorrect memory estimate report
Author: omtcyfz
Date: Wed Sep 26 08:06:23 2018
New Revision: 343117
URL: http://llvm.org/viewvc/llvm-project?rev=343117=rev
Log:
[clangd] Fix bugs with incorrect memory estimate report
* With the current implementation, `sizeof(std::vector)` is added
twice to the `Dex` memory estimate which is incorrect
* `Dex` logs memory usage estimation before `BackingDataSize` is set and
hence the log report excludes size of the external `SymbolSlab` which is
coupled with `Dex` instance
Reviewed By: ioeric
Differential Revision: https://reviews.llvm.org/D52503
Modified:
clang-tools-extra/trunk/clangd/index/Serialization.cpp
clang-tools-extra/trunk/clangd/index/dex/Dex.cpp
clang-tools-extra/trunk/clangd/index/dex/PostingList.h
Modified: clang-tools-extra/trunk/clangd/index/Serialization.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Serialization.cpp?rev=343117=343116=343117=diff
==
--- clang-tools-extra/trunk/clangd/index/Serialization.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/Serialization.cpp Wed Sep 26 08:06:23
2018
@@ -6,8 +6,10 @@
// License. See LICENSE.TXT for details.
//
//===--===//
+
#include "Serialization.h"
#include "Index.h"
+#include "Logger.h"
#include "RIFF.h"
#include "Trace.h"
#include "dex/Dex.h"
@@ -433,8 +435,12 @@ std::unique_ptr loadIndex(l
}
trace::Span Tracer("BuildIndex");
- return UseDex ? dex::Dex::build(std::move(Symbols), URISchemes)
-: MemIndex::build(std::move(Symbols), std::move(Refs));
+ auto Index = UseDex ? dex::Dex::build(std::move(Symbols), URISchemes)
+ : MemIndex::build(std::move(Symbols), std::move(Refs));
+ vlog("Loaded {0} from {1} with estimated memory usage {2}",
+ UseDex ? "Dex" : "MemIndex", SymbolFilename,
+ Index->estimateMemoryUsage());
+ return Index;
}
} // namespace clangd
Modified: clang-tools-extra/trunk/clangd/index/dex/Dex.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/dex/Dex.cpp?rev=343117=343116=343117=diff
==
--- clang-tools-extra/trunk/clangd/index/dex/Dex.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/dex/Dex.cpp Wed Sep 26 08:06:23 2018
@@ -130,9 +130,6 @@ void Dex::buildIndex() {
for (const auto : TempInvertedIndex)
InvertedIndex.insert(
{TokenToPostingList.first, PostingList(TokenToPostingList.second)});
-
- vlog("Built Dex with estimated memory usage {0} bytes.",
- estimateMemoryUsage());
}
/// Constructs iterators over tokens extracted from the query and exhausts it
@@ -248,8 +245,8 @@ size_t Dex::estimateMemoryUsage() const
Bytes += SymbolQuality.size() * sizeof(float);
Bytes += LookupTable.getMemorySize();
Bytes += InvertedIndex.getMemorySize();
- for (const auto : InvertedIndex)
-Bytes += P.second.bytes();
+ for (const auto : InvertedIndex)
+Bytes += TokenToPostingList.second.bytes();
return Bytes + BackingDataSize;
}
Modified: clang-tools-extra/trunk/clangd/index/dex/PostingList.h
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/dex/PostingList.h?rev=343117=343116=343117=diff
==
--- clang-tools-extra/trunk/clangd/index/dex/PostingList.h (original)
+++ clang-tools-extra/trunk/clangd/index/dex/PostingList.h Wed Sep 26 08:06:23
2018
@@ -66,10 +66,8 @@ public:
/// go through the chunks and decompress them on-the-fly when necessary.
std::unique_ptr iterator() const;
- /// Returns in-memory size.
- size_t bytes() const {
-return sizeof(Chunk) + Chunks.capacity() * sizeof(Chunk);
- }
+ /// Returns in-memory size of external storage.
+ size_t bytes() const { return Chunks.capacity() * sizeof(Chunk); }
private:
const std::vector Chunks;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52554: [WIP] [clangd] Tests for special methods code-completion
jkorous created this revision.
jkorous added reviewers: sammccall, ilya-biryukov.
jkorous added a project: clang-tools-extra.
Herald added subscribers: cfe-commits, kadircet, arphaman, dexonsmith, MaskRay,
ioeric.
Created in order to check we agree on what are the requirements and would later
write patches against these.
Currently these are failing:
[ FAILED ] CompletionTestNoExplicitMembers.Struct
[ FAILED ] CompletionTestNoExplicitMembers.StructTemplate
[ FAILED ]
CompletionTestNoExplicitMembers.ExplicitStructTemplateSpecialization
[ FAILED ] CompletionTestMethodDeclared.ExplicitStructTemplateSpecialization
[ FAILED ] CompletionTestSpecialMethodsDeclared.Struct
[ FAILED ] CompletionTestSpecialMethodsDeclared.StructTemplate
[ FAILED ]
CompletionTestSpecialMethodsDeclared.ExplicitStructTemplateSpecialization
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52554
Files:
clangd/CodeCompleteTests.cpp
Index: clangd/CodeCompleteTests.cpp
===
--- clangd/CodeCompleteTests.cpp
+++ clangd/CodeCompleteTests.cpp
@@ -2040,6 +2040,336 @@
}
}
+TEST(CompletionTestNoExplicitMembers, Struct) {
+ clangd::CodeCompleteOptions Opts;
+ Opts.Limit = 1;
+ auto ResultsDot = completions(R"cpp(
+ struct foo {};
+ void bar() {
+foo a;
+a.^
+)cpp",
+/*IndexSymbols=*/{}, Opts);
+
+ EXPECT_TRUE(ResultsDot.Completions.empty());
+
+ auto ResultsArrow = completions(R"cpp(
+ struct foo {};
+ void bar() {
+foo* b;
+b->^
+)cpp",
+/*IndexSymbols=*/{}, Opts);
+
+ EXPECT_TRUE(ResultsArrow.Completions.empty());
+
+ auto ResultsQualified = completions(R"cpp(
+ struct foo {};
+ foo::^
+)cpp",
+/*IndexSymbols=*/{}, Opts);
+
+ EXPECT_TRUE(ResultsQualified.Completions.empty());
+}
+
+TEST(CompletionTestNoExplicitMembers, StructTemplate) {
+ clangd::CodeCompleteOptions Opts;
+ Opts.Limit = 1;
+ auto ResultsDot = completions(R"cpp(
+ template struct foo {};
+ void bar() {
+foo a;
+a.^
+)cpp",
+/*IndexSymbols=*/{}, Opts);
+
+ EXPECT_TRUE(ResultsDot.Completions.empty());
+
+ auto ResultsArrow = completions(R"cpp(
+ template struct foo {};
+ void bar() {
+foo* b;
+b->^
+)cpp",
+/*IndexSymbols=*/{}, Opts);
+
+ EXPECT_TRUE(ResultsArrow.Completions.empty());
+
+ auto ResultsQualified = completions(R"cpp(
+ template struct foo {};
+ foo::^
+)cpp",
+/*IndexSymbols=*/{}, Opts);
+
+ EXPECT_TRUE(ResultsQualified.Completions.empty());
+}
+
+TEST(CompletionTestNoExplicitMembers, ExplicitStructTemplateSpecialization) {
+ clangd::CodeCompleteOptions Opts;
+ Opts.Limit = 1;
+ auto ResultsDot = completions(R"cpp(
+ template struct foo {}; template<> struct foo {};
+ void bar() {
+foo a;
+a.^
+)cpp",
+/*IndexSymbols=*/{}, Opts);
+
+ EXPECT_TRUE(ResultsDot.Completions.empty());
+
+ auto ResultsArrow = completions(R"cpp(
+ template struct foo {}; template<> struct foo {};
+ void bar() {
+foo* b;
+b->^
+)cpp",
+/*IndexSymbols=*/{}, Opts);
+
+ EXPECT_TRUE(ResultsArrow.Completions.empty());
+
+ auto ResultsQualified = completions(R"cpp(
+ template struct foo {}; template<> struct foo {};
+ foo::^
+)cpp",
+/*IndexSymbols=*/{}, Opts);
+
+ EXPECT_TRUE(ResultsQualified.Completions.empty());
+}
+
+TEST(CompletionTestMethodDeclared, Struct) {
+ clangd::CodeCompleteOptions Opts;
+ Opts.Limit = 1;
+ auto ResultsDot = completions(R"cpp(
+ struct foo { void foomethod(); };
+ void bar() {
+foo a;
+a.^
+)cpp",
+/*IndexSymbols=*/{}, Opts);
+
+ ASSERT_TRUE(ResultsDot.Completions.size() == 1);
+ EXPECT_TRUE(ResultsDot.Completions.front().Name == "foomethod");
+
+ auto ResultsArrow = completions(R"cpp(
+ struct foo { void foomethod(); };
+ void bar() {
+foo* b;
+b->^
+)cpp",
+/*IndexSymbols=*/{}, Opts);
+
+ ASSERT_TRUE(ResultsArrow.Completions.size() == 1);
+ EXPECT_TRUE(ResultsArrow.Completions.front().Name == "foomethod");
+
+ auto ResultsQualified = completions(R"cpp(
+ struct foo { void foomethod(); };
+ foo::^
+)cpp",
+/*IndexSymbols=*/{}, Opts);
+
+ ASSERT_TRUE(ResultsQualified.Completions.size() == 1);
+ EXPECT_TRUE(ResultsQualified.Completions.front().Name == "foomethod");
+
+ auto ResultsQualifiedStatic = completions(R"cpp(
+ struct foo { static void foomethod(); };
+ void bar() {
+foo::^
+)cpp",
+/*IndexSymbols=*/{}, Opts);
+
+ ASSERT_TRUE(ResultsQualifiedStatic.Completions.size() == 1);
+ EXPECT_TRUE(ResultsQualifiedStatic.Completions.front().Name == "foomethod");
+}
+
+TEST(CompletionTestMethodDeclared, StructTemplate) {
+ clangd::CodeCompleteOptions Opts;
+ Opts.Limit = 1;
+ auto ResultsDot = completions(R"cpp(
+ template struct foo { void foomethod(); };
+
[PATCH] D52545: [docs] Update PostingList string representation format
This revision was automatically updated to reflect the committed changes.
Closed by commit rL343116: [docs] Update PostingList string representation
format (authored by omtcyfz, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D52545?vs=167138=167141#toc
Repository:
rL LLVM
https://reviews.llvm.org/D52545
Files:
clang-tools-extra/trunk/clangd/index/dex/Iterator.h
Index: clang-tools-extra/trunk/clangd/index/dex/Iterator.h
===
--- clang-tools-extra/trunk/clangd/index/dex/Iterator.h
+++ clang-tools-extra/trunk/clangd/index/dex/Iterator.h
@@ -94,9 +94,8 @@
///
/// Where Type is the iterator type representation: "&" for And, "|" for Or,
/// ChildN is N-th iterator child. Raw iterators over PostingList are
- /// represented as "[ID1, ID2, ..., {IDN}, ... END]" where IDN is N-th
- /// PostingList entry and the element which is pointed to by the PostingList
- /// iterator is enclosed in {} braces.
+ /// represented as "[... CurID ...]" where CurID is the current PostingList
+ /// entry being inspected.
friend llvm::raw_ostream <<(llvm::raw_ostream ,
const Iterator ) {
return Iterator.dump(OS);
Index: clang-tools-extra/trunk/clangd/index/dex/Iterator.h
===
--- clang-tools-extra/trunk/clangd/index/dex/Iterator.h
+++ clang-tools-extra/trunk/clangd/index/dex/Iterator.h
@@ -94,9 +94,8 @@
///
/// Where Type is the iterator type representation: "&" for And, "|" for Or,
/// ChildN is N-th iterator child. Raw iterators over PostingList are
- /// represented as "[ID1, ID2, ..., {IDN}, ... END]" where IDN is N-th
- /// PostingList entry and the element which is pointed to by the PostingList
- /// iterator is enclosed in {} braces.
+ /// represented as "[... CurID ...]" where CurID is the current PostingList
+ /// entry being inspected.
friend llvm::raw_ostream <<(llvm::raw_ostream ,
const Iterator ) {
return Iterator.dump(OS);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r343116 - [docs] Update PostingList string representation format
Author: omtcyfz
Date: Wed Sep 26 07:59:49 2018
New Revision: 343116
URL: http://llvm.org/viewvc/llvm-project?rev=343116=rev
Log:
[docs] Update PostingList string representation format
Because `PostingList` objects are compressed, it is now impossible to
see elements other than the current one and the documentation doesn't
match implementation anymore.
Reviewed By: ioeric
Differential Revision: https://reviews.llvm.org/D52545
Modified:
clang-tools-extra/trunk/clangd/index/dex/Iterator.h
Modified: clang-tools-extra/trunk/clangd/index/dex/Iterator.h
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/dex/Iterator.h?rev=343116=343115=343116=diff
==
--- clang-tools-extra/trunk/clangd/index/dex/Iterator.h (original)
+++ clang-tools-extra/trunk/clangd/index/dex/Iterator.h Wed Sep 26 07:59:49 2018
@@ -94,9 +94,8 @@ public:
///
/// Where Type is the iterator type representation: "&" for And, "|" for Or,
/// ChildN is N-th iterator child. Raw iterators over PostingList are
- /// represented as "[ID1, ID2, ..., {IDN}, ... END]" where IDN is N-th
- /// PostingList entry and the element which is pointed to by the PostingList
- /// iterator is enclosed in {} braces.
+ /// represented as "[... CurID ...]" where CurID is the current PostingList
+ /// entry being inspected.
friend llvm::raw_ostream <<(llvm::raw_ostream ,
const Iterator ) {
return Iterator.dump(OS);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52545: [docs] Update PostingList string representation format
kbobyrev updated this revision to Diff 167138.
kbobyrev added a comment.
Simplify the documentation format.
https://reviews.llvm.org/D52545
Files:
clang-tools-extra/clangd/index/dex/Iterator.h
Index: clang-tools-extra/clangd/index/dex/Iterator.h
===
--- clang-tools-extra/clangd/index/dex/Iterator.h
+++ clang-tools-extra/clangd/index/dex/Iterator.h
@@ -94,9 +94,8 @@
///
/// Where Type is the iterator type representation: "&" for And, "|" for Or,
/// ChildN is N-th iterator child. Raw iterators over PostingList are
- /// represented as "[ID1, ID2, ..., {IDN}, ... END]" where IDN is N-th
- /// PostingList entry and the element which is pointed to by the PostingList
- /// iterator is enclosed in {} braces.
+ /// represented as "[... CurID ...]" where CurID is the current PostingList
+ /// entry being inspected.
friend llvm::raw_ostream <<(llvm::raw_ostream ,
const Iterator ) {
return Iterator.dump(OS);
Index: clang-tools-extra/clangd/index/dex/Iterator.h
===
--- clang-tools-extra/clangd/index/dex/Iterator.h
+++ clang-tools-extra/clangd/index/dex/Iterator.h
@@ -94,9 +94,8 @@
///
/// Where Type is the iterator type representation: "&" for And, "|" for Or,
/// ChildN is N-th iterator child. Raw iterators over PostingList are
- /// represented as "[ID1, ID2, ..., {IDN}, ... END]" where IDN is N-th
- /// PostingList entry and the element which is pointed to by the PostingList
- /// iterator is enclosed in {} braces.
+ /// represented as "[... CurID ...]" where CurID is the current PostingList
+ /// entry being inspected.
friend llvm::raw_ostream <<(llvm::raw_ostream ,
const Iterator ) {
return Iterator.dump(OS);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52545: [docs] Update PostingList string representation format
ioeric added inline comments.
Comment at: clang-tools-extra/clangd/index/dex/Iterator.h:97
/// ChildN is N-th iterator child. Raw iterators over PostingList are
- /// represented as "[ID1, ID2, ..., {IDN}, ... END]" where IDN is N-th
- /// PostingList entry and the element which is pointed to by the PostingList
- /// iterator is enclosed in {} braces.
+ /// represented as "[(...)? (IDN | END) (...)?]" where IDN is N-th
+ /// PostingList entry.
nit: `[(...)? (IDN | END) (...)?]` is accurate but can be really confusing... I
think it's okay to be a bit inaccurate with just `[ ... CurID ... ]`.
https://reviews.llvm.org/D52545
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52552: [clang-tidy] Flag Classes Inheriting From Structs
lebedev.ri added a comment.
Please upload patches with full context.
Please do follow coding style, clang-format your patches.
Would it make sense to not catch-all all the `class : struct`, but consider the
explicitly specified inheritance visibility?
Comment at: clang-tidy/misc/ClassInheritFromStructCheck.cpp:21
+void ClassInheritFromStructCheck::registerMatchers(MatchFinder *Finder) {
+
Finder->addMatcher(cxxRecordDecl(isClass(),unless(isImplicit()),isDerivedFrom(cxxRecordDecl(isStruct(.bind("class"),
this);
+}
Please linewrap to 80 chars.
Comment at: test/clang-tidy/misc-class-inherit-from-struct.cpp:13
+};
+
+class C
Missing cases:
* struct inheriting from struct
* Different inheritance visibility:
* You only check the default visibility
* What if class explicitly-`public`ly inherits from `struct`?
* What if class explicitly-`private`ly inherits from `struct`?
* What if class explicitly-`protected`ly inherits from `struct`?
* Same for `struct` inheriting from struct?
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52552
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52364: [clangd] Initial supoprt for cross-namespace global code completion.
ioeric added inline comments.
Comment at: clangd/index/Index.h:430
///
- /// The global scope is "", a top level scope is "foo::", etc.
+ /// The global scope is "", a top level scope is "foo::", etc. "*" is
+ /// wildcard.
sammccall wrote:
> I'm not a big fan of this magic value, it seems too easy to forget to handle.
> Especially since we have a lot of existing code that touches this interface,
> and we may forget to check some of it.
>
> I suggest making this a separate boolean field `AnyScope`, with a comment
> that scopes explicitly listed will be ranked higher.
> We can probably also remove the "If this is empty" special case from `Scopes`
> now too, as the old behavior can be achieved by setting `Scopes = {};
> AnyScope = true;`
sounds good.
> We can probably also remove the "If this is empty" special case from Scopes
> now too, as the old behavior can be achieved by setting Scopes = {}; AnyScope
> = true;
I think this is a good idea. Unfortunately, there seem to be many tests that
rely on the old behavior. I'll add a FIXME and do it in followup patch.
Comment at: clangd/index/Index.h:432
+ /// wildcard.
+ /// FIXME: support assigning different weight to each scope.
std::vector Scopes;
sammccall wrote:
> May not want a heavyweight API with explicit weights...
>
> I think what we really **need** here is the ability to weight
> `clang::clangd::` > `clang::` > `` when you're in the scope of namespace
> clangd.
>
> We could just say something like "the primary scope should come first" and do
> some FileDistance-like penalizing of the others...
Changed the wording of `FIXME`.
> I think what we really need here is the ability to weight clang::clangd:: >
> clang:: > `` when you're in the scope of namespace clangd.
It's unclear what this would mean for scopes from `using-namespace` directives.
But we can revisit when we get there.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52364
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52364: [clangd] Initial supoprt for cross-namespace global code completion.
ioeric updated this revision to Diff 167135.
ioeric marked 14 inline comments as done.
ioeric added a comment.
- address review comments
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52364
Files:
clangd/CodeComplete.cpp
clangd/CodeComplete.h
clangd/index/Index.h
clangd/index/MemIndex.cpp
clangd/index/dex/Dex.cpp
clangd/tool/ClangdMain.cpp
unittests/clangd/CodeCompleteTests.cpp
unittests/clangd/DexTests.cpp
Index: unittests/clangd/DexTests.cpp
===
--- unittests/clangd/DexTests.cpp
+++ unittests/clangd/DexTests.cpp
@@ -554,6 +554,17 @@
EXPECT_THAT(match(*I, Req), UnorderedElementsAre("a::y1"));
}
+TEST(DexTest, WildcardScope) {
+ auto I =
+ Dex::build(generateSymbols({"a::y1", "a::b::y2", "c::y3"}), URISchemes);
+ FuzzyFindRequest Req;
+ Req.Query = "y";
+ Req.Scopes = {"a::"};
+ Req.AnyScope = true;
+ EXPECT_THAT(match(*I, Req),
+ UnorderedElementsAre("a::y1", "a::b::y2", "c::y3"));
+}
+
TEST(DexTest, IgnoreCases) {
auto I = Dex::build(generateSymbols({"ns::ABC", "ns::abc"}), URISchemes);
FuzzyFindRequest Req;
Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -2040,6 +2040,58 @@
}
}
+TEST(CompletionTest, NoAllScopesCompletionWhenQualified) {
+ clangd::CodeCompleteOptions Opts = {};
+ Opts.AllScopes = true;
+
+ auto Results = completions(
+ R"cpp(
+void f() { na::Clangd^ }
+ )cpp",
+ {cls("na::ClangdA"), cls("nx::ClangdX"), cls("Clangd3")}, Opts);
+ EXPECT_THAT(Results.Completions,
+ UnorderedElementsAre(
+ AllOf(Qualifier(""), Scope("na::"), Named("ClangdA";
+}
+
+TEST(CompletionTest, AllScopesCompletion) {
+ clangd::CodeCompleteOptions Opts = {};
+ Opts.AllScopes = true;
+
+ auto Results = completions(
+ R"cpp(
+namespace na {
+void f() { Clangd^ }
+}
+ )cpp",
+ {cls("nx::Clangd1"), cls("ny::Clangd2"), cls("Clangd3"),
+ cls("na::nb::Clangd4")},
+ Opts);
+ EXPECT_THAT(
+ Results.Completions,
+ UnorderedElementsAre(AllOf(Qualifier("nx::"), Named("Clangd1")),
+ AllOf(Qualifier("ny::"), Named("Clangd2")),
+ AllOf(Qualifier(""), Scope(""), Named("Clangd3")),
+ AllOf(Qualifier("nb::"), Named("Clangd4";
+}
+
+TEST(CompletionTest, NoQualifierIfShadowed) {
+ clangd::CodeCompleteOptions Opts = {};
+ Opts.AllScopes = true;
+
+ auto Results = completions(R"cpp(
+namespace nx { class Clangd1 {}; }
+using nx::Clangd1;
+void f() { Clangd^ }
+ )cpp",
+ {cls("nx::Clangd1"), cls("nx::Clangd2")}, Opts);
+ // Although Clangd1 is from another namespace, Sema tells us it's in-scope and
+ // needs no qualifier.
+ EXPECT_THAT(Results.Completions,
+ UnorderedElementsAre(AllOf(Qualifier(""), Named("Clangd1")),
+ AllOf(Qualifier("nx::"), Named("Clangd2";
+}
+
} // namespace
} // namespace clangd
} // namespace clang
Index: clangd/tool/ClangdMain.cpp
===
--- clangd/tool/ClangdMain.cpp
+++ clangd/tool/ClangdMain.cpp
@@ -136,6 +136,15 @@
"enabled separatedly."),
llvm::cl::init(true), llvm::cl::Hidden);
+static llvm::cl::opt AllScopesCompletion(
+"all-scopes-completion",
+llvm::cl::desc(
+"If set to true, code completion will include index symbols that are "
+"not defined in the scopes (e.g. "
+"namespaces) visible from the code completion point. Such completions "
+"can insert scope qualifiers."),
+llvm::cl::init(false), llvm::cl::Hidden);
+
static llvm::cl::opt
ShowOrigins("debug-origin",
llvm::cl::desc("Show origins of completion items"),
@@ -304,6 +313,7 @@
}
CCOpts.SpeculativeIndexRequest = Opts.StaticIndex;
CCOpts.EnableFunctionArgSnippets = EnableFunctionArgSnippets;
+ CCOpts.AllScopes = AllScopesCompletion;
// Initialize and run ClangdLSPServer.
ClangdLSPServer LSPServer(
Index: clangd/index/dex/Dex.cpp
===
--- clangd/index/dex/Dex.cpp
+++ clangd/index/dex/Dex.cpp
@@ -12,6 +12,8 @@
#include "FuzzyMatch.h"
#include "Logger.h"
#include "Quality.h"
+#include "index/Index.h"
+#include "index/dex/Iterator.h"
#include "llvm/ADT/StringSet.h"
#include
#include
@@ -166,6 +168,9 @@
if (It != InvertedIndex.end())
ScopeIterators.push_back(It->second.iterator());
}
+ if (Req.AnyScope)
+ScopeIterators.push_back(createBoost(createTrue(Symbols.size()), 0.2));
+
// Add OR iterator for scopes if there are any Scope Iterators.
if (!ScopeIterators.empty())
[PATCH] D52552: [clang-tidy] Flag Classes Inheriting From Structs
DennisMelamed created this revision.
DennisMelamed added reviewers: alexfh, aaron.ballman, hokein, JonasToth.
Herald added subscribers: xazax.hun, mgorny.
[clang-tidy] Flag Classes Inheriting From Structs
Added a check which flags cases of a class inheriting from a struct, which can
cause unintended scope issues since struct members are public by default.
Patch by Dennis Melamed
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52552
Files:
clang-tidy/misc/CMakeLists.txt
clang-tidy/misc/ClassInheritFromStructCheck.cpp
clang-tidy/misc/ClassInheritFromStructCheck.h
clang-tidy/misc/MiscTidyModule.cpp
docs/ReleaseNotes.rst
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/misc-class-inherit-from-struct.rst
test/clang-tidy/misc-class-inherit-from-struct.cpp
Index: test/clang-tidy/misc-class-inherit-from-struct.cpp
===
--- /dev/null
+++ test/clang-tidy/misc-class-inherit-from-struct.cpp
@@ -0,0 +1,22 @@
+// RUN: %check_clang_tidy %s misc-class-inherit-from-struct %t
+
+struct A
+{
+int a;
+};
+
+class B:A
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: classes should not inherit from structs [misc-class-inherit-from-struct]
+{
+int b;
+};
+
+class C
+{
+int c;
+};
+
+class D:C
+{
+int d;
+};
Index: docs/clang-tidy/checks/misc-class-inherit-from-struct.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/misc-class-inherit-from-struct.rst
@@ -0,0 +1,18 @@
+.. title:: clang-tidy - misc-class-inherit-from-struct
+
+misc-class-inherit-from-struct
+==
+
+Finds instances of classes inheriting from structs. Structs are meant for
+storing data and are often used to maintain C compatibility. Having a class
+inherit from them can lead to confusion with what type of object is being
+dealt with. Additionally, the default public nature of struct members can
+lead to unintentional exposure of members.
+
+.. code-block:: c++
+
+ struct A {};
+ class B: public A {}; //throws an error, members of A might be
+//unintentionally exposed
+ class C {};
+ class D: public C {}; //fine
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -159,6 +159,7 @@
llvm-include-order
llvm-namespace-comment
llvm-twine-local
+ misc-class-inherit-from-struct
misc-definitions-in-headers
misc-misplaced-const
misc-new-delete-overloads
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -106,6 +106,13 @@
Detects usage of magic numbers, numbers that are used as literals instead of
introduced via constants or symbols.
+- New :doc:`misc-class-inherit-from-struct
+ ` check.
+
+ Detects instances of classes inheriting from structs as that might lead to
+ unintended member access issues.
+
+
Improvements to include-fixer
-
Index: clang-tidy/misc/MiscTidyModule.cpp
===
--- clang-tidy/misc/MiscTidyModule.cpp
+++ clang-tidy/misc/MiscTidyModule.cpp
@@ -10,6 +10,7 @@
#include "../ClangTidy.h"
#include "../ClangTidyModule.h"
#include "../ClangTidyModuleRegistry.h"
+#include "ClassInheritFromStructCheck.h"
#include "DefinitionsInHeadersCheck.h"
#include "MisplacedConstCheck.h"
#include "NewDeleteOverloadsCheck.h"
@@ -30,6 +31,8 @@
class MiscModule : public ClangTidyModule {
public:
void addCheckFactories(ClangTidyCheckFactories ) override {
+CheckFactories.registerCheck(
+"misc-class-inherit-from-struct");
CheckFactories.registerCheck(
"misc-definitions-in-headers");
CheckFactories.registerCheck("misc-misplaced-const");
Index: clang-tidy/misc/ClassInheritFromStructCheck.h
===
--- /dev/null
+++ clang-tidy/misc/ClassInheritFromStructCheck.h
@@ -0,0 +1,35 @@
+//===--- ClassInheritFromStructCheck.h - clang-tidy--*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_CLASSINHERITFROMSTRUCTCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_CLASSINHERITFROMSTRUCTCHECK_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+/// Matches non-implicit classes which inherit from structs
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/misc-class-inherit-from-struct.html
+class ClassInheritFromStructCheck :
[PATCH] D52491: [ARM/AArch64][v8.5A] Add Armv8.5-A target
This revision was automatically updated to reflect the committed changes.
Closed by commit rC343111: [ARM/AArch64][v8.5A] Add Armv8.5-A target (authored
by olista01, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D52491?vs=166908=167130#toc
Repository:
rC Clang
https://reviews.llvm.org/D52491
Files:
lib/Basic/Targets/ARM.cpp
test/Driver/aarch64-cpus.c
test/Driver/arm-cortex-cpus.c
test/Preprocessor/arm-target-features.c
Index: test/Driver/aarch64-cpus.c
===
--- test/Driver/aarch64-cpus.c
+++ test/Driver/aarch64-cpus.c
@@ -542,6 +542,25 @@
// RUN: %clang -target aarch64 -march=armv8.4-a+nofp16+fp16fml -### -c %s 2>&1 | FileCheck -check-prefix=GENERICV84A-NO-FP16-FP16FML %s
// GENERICV84A-NO-FP16-FP16FML: "-target-feature" "+fp16fml" "-target-feature" "+fullfp16"
+// RUN: %clang -target aarch64 -march=armv8.5a -### -c %s 2>&1 | FileCheck -check-prefix=GENERICV85A %s
+// RUN: %clang -target aarch64 -march=armv8.5-a -### -c %s 2>&1 | FileCheck -check-prefix=GENERICV85A %s
+// RUN: %clang -target aarch64 -mlittle-endian -march=armv8.5a -### -c %s 2>&1 | FileCheck -check-prefix=GENERICV85A %s
+// RUN: %clang -target aarch64 -mlittle-endian -march=armv8.5-a -### -c %s 2>&1 | FileCheck -check-prefix=GENERICV85A %s
+// RUN: %clang -target aarch64_be -mlittle-endian -march=armv8.5a -### -c %s 2>&1 | FileCheck -check-prefix=GENERICV85A %s
+// RUN: %clang -target aarch64_be -mlittle-endian -march=armv8.5-a -### -c %s 2>&1 | FileCheck -check-prefix=GENERICV85A %s
+// GENERICV85A: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-cpu" "generic" "-target-feature" "+neon" "-target-feature" "+v8.5a"
+
+// RUN: %clang -target aarch64_be -march=armv8.5a -### -c %s 2>&1 | FileCheck -check-prefix=GENERICV85A-BE %s
+// RUN: %clang -target aarch64_be -march=armv8.5-a -### -c %s 2>&1 | FileCheck -check-prefix=GENERICV85A-BE %s
+// RUN: %clang -target aarch64 -mbig-endian -march=armv8.5a -### -c %s 2>&1 | FileCheck -check-prefix=GENERICV85A-BE %s
+// RUN: %clang -target aarch64 -mbig-endian -march=armv8.5-a -### -c %s 2>&1 | FileCheck -check-prefix=GENERICV85A-BE %s
+// RUN: %clang -target aarch64_be -mbig-endian -march=armv8.5a -### -c %s 2>&1 | FileCheck -check-prefix=GENERICV85A-BE %s
+// RUN: %clang -target aarch64_be -mbig-endian -march=armv8.5-a -### -c %s 2>&1 | FileCheck -check-prefix=GENERICV85A-BE %s
+// GENERICV85A-BE: "-cc1"{{.*}} "-triple" "aarch64_be{{.*}}" "-target-cpu" "generic" "-target-feature" "+neon" "-target-feature" "+v8.5a"
+
+// RUN: %clang -target aarch64 -march=armv8.5-a+fp16 -### -c %s 2>&1 | FileCheck -check-prefix=GENERICV85A-FP16 %s
+// GENERICV85A-FP16: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-cpu" "generic" "-target-feature" "+neon" "-target-feature" "+v8.5a" "-target-feature" "+fullfp16"
+
// fullfp16 is off by default for v8a, feature must not be mentioned
// RUN: %clang -target aarch64 -march=armv8a -### -c %s 2>&1 | FileCheck -check-prefix=V82ANOFP16 -check-prefix=GENERIC %s
// RUN: %clang -target aarch64 -march=armv8-a -### -c %s 2>&1 | FileCheck -check-prefix=V82ANOFP16 -check-prefix=GENERIC %s
Index: test/Driver/arm-cortex-cpus.c
===
--- test/Driver/arm-cortex-cpus.c
+++ test/Driver/arm-cortex-cpus.c
@@ -318,6 +318,23 @@
// RUN: %clang -target arm -march=armebv8.4-a -mbig-endian -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-BE-V84A %s
// CHECK-BE-V84A: "-cc1"{{.*}} "-triple" "armebv8.4{{.*}}" "-target-cpu" "generic"
+// RUN: %clang -target armv8.5a -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-V85A %s
+// RUN: %clang -target arm -march=armv8.5a -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-V85A %s
+// RUN: %clang -target arm -march=armv8.5-a -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-V85A %s
+// RUN: %clang -target arm -march=armv8.5a -mlittle-endian -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-V85A %s
+// RUN: %clang -target armv8.5a -mlittle-endian -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-V85A %s
+// RUN: %clang -target arm -march=armv8.5a -mlittle-endian -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-V85A %s
+// RUN: %clang -target arm -mlittle-endian -march=armv8.5-a -mlittle-endian -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-V85A %s
+// CHECK-V85A: "-cc1"{{.*}} "-triple" "armv8.5{{.*}}" "-target-cpu" "generic"
+
+// RUN: %clang -target armebv8.5a -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-BE-V85A %s
+// RUN: %clang -target armv8.5a -mbig-endian -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-BE-V85A %s
+// RUN: %clang -target armeb -march=armebv8.5a -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-BE-V85A %s
+// RUN: %clang -target armeb -march=armebv8.5-a -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-BE-V85A %s
+// RUN: %clang -target arm -march=armebv8.5a -mbig-endian -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-BE-V85A %s
+// RUN: %clang -target arm -march=armebv8.5-a
r343111 - [ARM/AArch64][v8.5A] Add Armv8.5-A target
Author: olista01
Date: Wed Sep 26 07:20:29 2018
New Revision: 343111
URL: http://llvm.org/viewvc/llvm-project?rev=343111=rev
Log:
[ARM/AArch64][v8.5A] Add Armv8.5-A target
This patch allows targetting Armv8.5-A from Clang. Most of the
implementation is in TargetParser, so this is mostly just adding tests.
Patch by Pablo Barrio!
Differential revision: https://reviews.llvm.org/D52491
Modified:
cfe/trunk/lib/Basic/Targets/ARM.cpp
cfe/trunk/test/Driver/aarch64-cpus.c
cfe/trunk/test/Driver/arm-cortex-cpus.c
cfe/trunk/test/Preprocessor/arm-target-features.c
Modified: cfe/trunk/lib/Basic/Targets/ARM.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets/ARM.cpp?rev=343111=343110=343111=diff
==
--- cfe/trunk/lib/Basic/Targets/ARM.cpp (original)
+++ cfe/trunk/lib/Basic/Targets/ARM.cpp Wed Sep 26 07:20:29 2018
@@ -189,6 +189,8 @@ StringRef ARMTargetInfo::getCPUAttr() co
return "8_3A";
case llvm::ARM::ArchKind::ARMV8_4A:
return "8_4A";
+ case llvm::ARM::ArchKind::ARMV8_5A:
+return "8_5A";
case llvm::ARM::ArchKind::ARMV8MBaseline:
return "8M_BASE";
case llvm::ARM::ArchKind::ARMV8MMainline:
Modified: cfe/trunk/test/Driver/aarch64-cpus.c
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/aarch64-cpus.c?rev=343111=343110=343111=diff
==
--- cfe/trunk/test/Driver/aarch64-cpus.c (original)
+++ cfe/trunk/test/Driver/aarch64-cpus.c Wed Sep 26 07:20:29 2018
@@ -542,6 +542,25 @@
// RUN: %clang -target aarch64 -march=armv8.4-a+nofp16+fp16fml -### -c %s 2>&1
| FileCheck -check-prefix=GENERICV84A-NO-FP16-FP16FML %s
// GENERICV84A-NO-FP16-FP16FML: "-target-feature" "+fp16fml" "-target-feature"
"+fullfp16"
+// RUN: %clang -target aarch64 -march=armv8.5a -### -c %s 2>&1 | FileCheck
-check-prefix=GENERICV85A %s
+// RUN: %clang -target aarch64 -march=armv8.5-a -### -c %s 2>&1 | FileCheck
-check-prefix=GENERICV85A %s
+// RUN: %clang -target aarch64 -mlittle-endian -march=armv8.5a -### -c %s 2>&1
| FileCheck -check-prefix=GENERICV85A %s
+// RUN: %clang -target aarch64 -mlittle-endian -march=armv8.5-a -### -c %s
2>&1 | FileCheck -check-prefix=GENERICV85A %s
+// RUN: %clang -target aarch64_be -mlittle-endian -march=armv8.5a -### -c %s
2>&1 | FileCheck -check-prefix=GENERICV85A %s
+// RUN: %clang -target aarch64_be -mlittle-endian -march=armv8.5-a -### -c %s
2>&1 | FileCheck -check-prefix=GENERICV85A %s
+// GENERICV85A: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-cpu" "generic"
"-target-feature" "+neon" "-target-feature" "+v8.5a"
+
+// RUN: %clang -target aarch64_be -march=armv8.5a -### -c %s 2>&1 | FileCheck
-check-prefix=GENERICV85A-BE %s
+// RUN: %clang -target aarch64_be -march=armv8.5-a -### -c %s 2>&1 | FileCheck
-check-prefix=GENERICV85A-BE %s
+// RUN: %clang -target aarch64 -mbig-endian -march=armv8.5a -### -c %s 2>&1 |
FileCheck -check-prefix=GENERICV85A-BE %s
+// RUN: %clang -target aarch64 -mbig-endian -march=armv8.5-a -### -c %s 2>&1 |
FileCheck -check-prefix=GENERICV85A-BE %s
+// RUN: %clang -target aarch64_be -mbig-endian -march=armv8.5a -### -c %s 2>&1
| FileCheck -check-prefix=GENERICV85A-BE %s
+// RUN: %clang -target aarch64_be -mbig-endian -march=armv8.5-a -### -c %s
2>&1 | FileCheck -check-prefix=GENERICV85A-BE %s
+// GENERICV85A-BE: "-cc1"{{.*}} "-triple" "aarch64_be{{.*}}" "-target-cpu"
"generic" "-target-feature" "+neon" "-target-feature" "+v8.5a"
+
+// RUN: %clang -target aarch64 -march=armv8.5-a+fp16 -### -c %s 2>&1 |
FileCheck -check-prefix=GENERICV85A-FP16 %s
+// GENERICV85A-FP16: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-cpu"
"generic" "-target-feature" "+neon" "-target-feature" "+v8.5a"
"-target-feature" "+fullfp16"
+
// fullfp16 is off by default for v8a, feature must not be mentioned
// RUN: %clang -target aarch64 -march=armv8a -### -c %s 2>&1 | FileCheck
-check-prefix=V82ANOFP16 -check-prefix=GENERIC %s
// RUN: %clang -target aarch64 -march=armv8-a -### -c %s 2>&1 | FileCheck
-check-prefix=V82ANOFP16 -check-prefix=GENERIC %s
Modified: cfe/trunk/test/Driver/arm-cortex-cpus.c
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/arm-cortex-cpus.c?rev=343111=343110=343111=diff
==
--- cfe/trunk/test/Driver/arm-cortex-cpus.c (original)
+++ cfe/trunk/test/Driver/arm-cortex-cpus.c Wed Sep 26 07:20:29 2018
@@ -318,6 +318,23 @@
// RUN: %clang -target arm -march=armebv8.4-a -mbig-endian -### -c %s 2>&1 |
FileCheck -check-prefix=CHECK-BE-V84A %s
// CHECK-BE-V84A: "-cc1"{{.*}} "-triple" "armebv8.4{{.*}}" "-target-cpu"
"generic"
+// RUN: %clang -target armv8.5a -### -c %s 2>&1 | FileCheck
-check-prefix=CHECK-V85A %s
+// RUN: %clang -target arm -march=armv8.5a -### -c %s 2>&1 | FileCheck
-check-prefix=CHECK-V85A %s
+// RUN: %clang -target arm -march=armv8.5-a
[PATCH] D52286: [Intrinsic] Signed Saturation Intirnsic
ebevhan added inline comments. Comment at: lib/CodeGen/SelectionDAG/LegalizeDAG.cpp:1119 + case ISD::SSAT: +// Target legalization checked here? +Action = TargetLowering::Expand; leonardchan wrote: > ebevhan wrote: > > leonardchan wrote: > > > ebevhan wrote: > > > > leonardchan wrote: > > > > > @ebevhan Do you know if this is where checking if this intrinsic is > > > > > supported by the target should be? > > > > Yes, this would be it. For nodes like ssat and fixed-point ops, you > > > > could ask a custom target hook, maybe > > > > `TLI.getScaledOperationAction(, , )` to > > > > determine the legality of a given node. > > > > > > > > When type-legalizing you also need to handle things for specific nodes > > > > as well, since things behave differently when you legalize > > > > operands/results on different nodes. > > > > > > > > (Of course, this all depends on whether we do the legalization here or > > > > in IR instead.) > > > Made the default action `Expand` for this in `TargetLoweringBase` and > > > targets can override this themselves. > > > > > > I think ssat may not need a custom hook since I don't think it requires > > > anything extra like fixsmul if it has to be expanded. But I see what you > > > mean now by how different nodes may require different steps for > > > legalization. > > You don't need an extra hook to determine anything extra for expansion, but > > you do need the hook to determine if it's legal in the first place. As I > > mentioned in another comment, an i16 ssat with a saturating width of 8 > > might be legal, but an i16 ssat with saturating width 12 might not. The > > former must not be expanded, but the latter must. > Got it. Just to clarify though, legalizations regarding the scale would only > apply for our mul/div intrinsics right? Operations like saturation and sat > add/sub which don't care about the scale do not need this hook right, since > it would also be useful for these operations to work on regular integers? Saturating add/sub won't need anything, but I still think saturation requires some extra check to determine if an operation with a specific saturating bitwidth should be legal or expanded. As I mentioned, a saturation with a type might be legal for one saturating bitwidth but not for another. Repository: rL LLVM https://reviews.llvm.org/D52286 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52360: [clang-tidy] Fix for performance-unnecessary-value-param, performance-unnecessary-copy-initialization and performance-for-range-copy
flx added a comment. In https://reviews.llvm.org/D52360#1246472, @baloghadamsoftware wrote: > In https://reviews.llvm.org/D52360#1246443, @danilaml wrote: > > > Would that also skip checks for something like `shared_ptr`? > > > Yes, everything ending on `pointer`, `ptr`, `reference` or `ref`, first > letter case insensitive. std::shared_ptr should not be blacklisted. It is not free to copy it: It incurs two atomic operations and two branches. Users should blacklist it if they know they don't care about this or not use the check. While it looks weird for the check to suggest to pass std::shared_ptr by reference it is correct. A better change would be to just pass the raw pointer in this case: std::shared_ptr p; PassByRawPointer(p.get()); But this would require function signature change that breaks callers outside of the translation unit. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52360 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52315: [clang-tidy] Fix for performance-unnecessary-value-param, performance-unnecessary-copy-initialization and performance-for-range-copy
lebedev.ri added a comment. In https://reviews.llvm.org/D52315#1245747, @dblaikie wrote: > ... I think this has been attempted to be superseded by https://reviews.llvm.org/D52360, although that has the same basic issue. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52315 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52360: [clang-tidy] Fix for performance-unnecessary-value-param, performance-unnecessary-copy-initialization and performance-for-range-copy
lebedev.ri added a comment. In https://reviews.llvm.org/D52360#1246474, @baloghadamsoftware wrote: > In https://reviews.llvm.org/D52360#1246452, @lebedev.ri wrote: > > > I disagree, it **must** not have false-negatives by default. > > > What kind of false-negatives could be caused by smart pointers or references? > Sane people do not name a type pointer or reference if they are not. Such > types must never be passed by reference. Few people will use the check if > they have to set tons of options for the basic expected behavior. For example > in `CodeChecker` this check is disabled by default and is only enabled in the > `Sensitive` profile. Ok, so let's entertain the current patch. Now, some time passes, and someone either discovers that the check does not warn on some code that one would have thought it should warn. Shitty code happens. Not everyone is sane in their naming choices. More importantly, there isn't a common coding style naming guidelines everyone **has to** follow. Someone somewhere will surely hit this. When he does, how does one get the check to warn on that? Or other side of the coin, how does one get the check //not// to warn on some custom type that does not match those hardcoded regexes? I don't disagree that this has false-positives. But this is **really** sensitive to the type names. It is not a good idea to hardcode this. //Please// follow pre-existing practice of //configurable// type white/blacklists. $ clang-tidy -checks=\* -dump-config | grep -i "types$" -A1 - key: cert-msc32-c.DisallowedSeedTypes value: 'time_t,std::time_t' - key: cert-msc51-cpp.DisallowedSeedTypes value: 'time_t,std::time_t' -- - key: google-runtime-references.WhiteListTypes value: '' -- - key: hicpp-use-emplace.TupleTypes value: '::std::pair;::std::tuple' -- - key: modernize-use-emplace.TupleTypes value: '::std::pair;::std::tuple' -- - key: readability-simplify-subscript-expr.Types value: '::std::basic_string;::std::basic_string_view;::std::vector;::std::array' Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52360 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52503: [clangd] Fix bugs with incorrect memory estimate report
kbobyrev added inline comments. Comment at: clang-tools-extra/clangd/index/dex/Dex.cpp:251 Bytes += InvertedIndex.getMemorySize(); - for (const auto : InvertedIndex) -Bytes += P.second.bytes(); + for (const auto : InvertedIndex) +Bytes += TokenToPostingList.first.Data.size() + ioeric wrote: > kbobyrev wrote: > > ioeric wrote: > > > kbobyrev wrote: > > > > ioeric wrote: > > > > > Would `InvertedIndex.getMemorySize()` be a better estimate? > > > > IIUC this is only precise for the objects which do not make any > > > > allocations (e.g. `std::vector` and `std::string` memory estimate would > > > > not be "correct"). > > > > > > > > From the documentation > > > > > > > > > This is just the raw memory used by DenseMap. If entries are pointers > > > > > to objects, the size of the referenced objects are not included. > > > > > > > > I also have `Bytes += InvertedIndex.getMemorySize();` above, so the > > > > purpose of this code would be to take into account the size of these > > > > "reference objects". > > > > > > > > However, since `PostingList::bytes()` also returns underlying > > > > `std::vector` size, this code will probably add these `std::vector` > > > > objects size twice (the first one was in > > > > `InvertedIndex.getMemorySize()`). I should fix that. > > > > `If entries are pointers to objects, the size of the referenced objects > > > > are not included.` > > > This applies to *pointers* to objects, but the `PostingList`s in > > > InvertedList are not pointerd? So it seems to me that > > > `InvertedIndex.getMemorySize()` covers everything in the `InvertedIndex`. > > > One way to verify is probably check the size calculated with loop against > > > `InvertedIndex.getMemorySize()`. > > As discussed offline, pointers and references are an example of objects > > which would be incorrectly measured by `DesneMap::getMemorySize()`, but > > `std::vector` and `std::string` are pointers in a way because they also > > just store a pointer to the underlying storage which is hidden from > > `DenseMap::getMemorySize()`; thus, it would be more precise to use the > > proposed scheme for memory estimation. > I see. I was expecting DenseMap to maintain an arena which could provide > memory usage of all owned data, but it seems to only consider > `sizeof(KV-Pair)`... > > If the posting lists (vectors) are not covered, shouldn't we also also add > the size of tokens (string) here? Yes, I was also adding `TokenToPostingList.first.Data.size()` to the size estimate in the previous revision snapshot, but as @sammccall pointed out, there are multiple string implementations and one of them (IIUC this one is currently pushed in the C++ Standard) utilizes Small Object Optimization (i.e. it behaves like `llvm::SmallVector` rather than `std::vector`). Therefore, it does not perform external allocations before some limit is reached. I don't know whether it is possible to know whether the external allocation happened or not and the summary of our discussion with Sam was that we should just omit this. https://reviews.llvm.org/D52503 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52549: [VFS] Add a VFS wrapper which avoids opening nonexistent files by reading dirs.
sammccall added a comment. As noted, this is really early stage, but would appreciate design feedback (I think both of you have spent some time fighting VFS by now...) Repository: rC Clang https://reviews.llvm.org/D52549 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52549: [VFS] Add a VFS wrapper which avoids opening nonexistent files by reading dirs.
sammccall updated this revision to Diff 167126.
sammccall added a comment.
remove commented code
Repository:
rC Clang
https://reviews.llvm.org/D52549
Files:
include/clang/Basic/VirtualFileSystem.h
lib/Basic/AvoidStatsVFS.cpp
lib/Basic/CMakeLists.txt
lib/Basic/VirtualFileSystem.cpp
Index: lib/Basic/VirtualFileSystem.cpp
===
--- lib/Basic/VirtualFileSystem.cpp
+++ lib/Basic/VirtualFileSystem.cpp
@@ -303,11 +303,6 @@
return llvm::sys::fs::real_path(Path, Output);
}
-IntrusiveRefCntPtr vfs::getRealFileSystem() {
- static IntrusiveRefCntPtr FS = new RealFileSystem();
- return FS;
-}
-
namespace {
class RealFSDirIter : public clang::vfs::detail::DirIterImpl {
@@ -2141,3 +2136,75 @@
return *this;
}
+
+namespace {
+// Log operation counts.
+class StatFS : public FileSystem {
+public:
+ StatFS(const char *Name, llvm::IntrusiveRefCntPtr FS)
+ : Name(Name), FS(std::move(FS)) {
+llvm::errs() << "created statfs " << Name << "\n";
+ }
+
+ llvm::ErrorOr status(const Twine ) override {
+auto Ret = FS->status(Path);
+bump(Ret ? StatusOK : StatusErr);
+return Ret;
+ }
+
+ llvm::ErrorOr>
+ openFileForRead(const Twine ) override {
+auto Ret = FS->openFileForRead(Path);
+bump(Ret ? OpenOK : OpenErr);
+return Ret;
+ }
+
+ directory_iterator dir_begin(const Twine , std::error_code ) override {
+bump(DirBegin);
+return FS->dir_begin(Dir, EC);
+ }
+
+ virtual std::error_code setCurrentWorkingDirectory(const Twine ) override {
+bump(SetCWD);
+return FS->setCurrentWorkingDirectory(Path);
+ }
+
+ virtual llvm::ErrorOr getCurrentWorkingDirectory() const override {
+bump(GetCWD);
+return FS->getCurrentWorkingDirectory();
+ }
+
+ virtual std::error_code
+ getRealPath(const Twine , SmallVectorImpl ) const override {
+bump(GetRealPath);
+return FS->getRealPath(Path, Output);
+ }
+
+private:
+ void bump(std::atomic ) const {
+++I;
+if (++All % 1 == 0) {
+ llvm::errs() << "== FILESYSTEM " << Name << " ==\n"
+ << "Status: " << StatusOK << "+" << StatusErr << "\n"
+ << "Open: " << OpenOK << "+" << OpenErr << "\n"
+ << "Dir: " << DirBegin << "\n"
+ << "GetRealPath: " << GetRealPath << "\n"
+ << "===\n";
+}
+ }
+
+ const char* Name;
+ llvm::IntrusiveRefCntPtr FS;
+ mutable std::atomic StatusOK = {0}, StatusErr = {0}, OpenOK = {0},
+OpenErr = {0}, DirBegin = {0}, GetRealPath = {0},
+SetCWD = {0}, GetCWD = {0};
+ mutable std::atomic All= {0};
+};
+
+} // namespace
+
+IntrusiveRefCntPtr vfs::getRealFileSystem() {
+ static IntrusiveRefCntPtr FS = new StatFS(
+ "outer", avoidStats(new StatFS("Real", new RealFileSystem())).release());
+ return FS;
+}
Index: lib/Basic/CMakeLists.txt
===
--- lib/Basic/CMakeLists.txt
+++ lib/Basic/CMakeLists.txt
@@ -46,6 +46,7 @@
add_clang_library(clangBasic
Attributes.cpp
+ AvoidStatsVFS.cpp
Builtins.cpp
CharInfo.cpp
Cuda.cpp
Index: lib/Basic/AvoidStatsVFS.cpp
===
--- /dev/null
+++ lib/Basic/AvoidStatsVFS.cpp
@@ -0,0 +1,296 @@
+//===- AvoidStatsVFS.cpp - Implements a stat-reducing VFS wrapper -===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+// We implement a VFS wrapper that uses directory listings to prune status() and
+// open() operations for files that *do not exist*.
+//
+// These operations are common in clang because of include paths.
+// With an include path of {A, B, ...}, an #include directive results
+// in attempts to open or stat {A/foo/bar, B/foo/bar, ...}.
+//
+// This is expensive because this typically takes one syscall per path, so we
+// have O(NumIncludes * IncludePathLen) syscalls.
+//
+// To optimize this, we can list parent directories such as A/.
+// If A only contains {config.h}, attempts to open A/foo/bar, A/foo/baz, A/qux
+// can all fail instantly.
+// Listing a directory tends to be a single syscall, and in practice most
+// missing files can be recognized by looking at the same few directories.
+// In practice the number of syscalls is O(NumIncludes + IncludePathLen).
+//
+// Our constant factor is higher, but this improves performance for large TUs
+// with many include paths.
+//
+//===--===//
+
+#include "clang/Basic/VirtualFileSystem.h"
+#include "llvm/ADT/StringSet.h"
+#include "llvm/Support/Path.h"
+#include
+
+namespace clang {
+namespace vfs {
+namespace {
+using namespace
[PATCH] D52549: [VFS] Add a VFS wrapper which avoids opening nonexistent files by reading dirs.
sammccall created this revision.
sammccall added reviewers: ioeric, ilya-biryukov.
Herald added subscribers: cfe-commits, jfb, mgorny.
This is intended to reduce the number of syscalls when building large
translation units that have long include paths.
This is a WIP for early feedback:
- obviously it needs tests
- it will not be on by default
- the logging and extra instrumentation is temporary to help me tune it
Particularly interested in how to express this without hundreds of
nested switch statements.
Repository:
rC Clang
https://reviews.llvm.org/D52549
Files:
include/clang/Basic/VirtualFileSystem.h
lib/Basic/AvoidStatsVFS.cpp
lib/Basic/CMakeLists.txt
lib/Basic/VirtualFileSystem.cpp
Index: lib/Basic/VirtualFileSystem.cpp
===
--- lib/Basic/VirtualFileSystem.cpp
+++ lib/Basic/VirtualFileSystem.cpp
@@ -303,11 +303,6 @@
return llvm::sys::fs::real_path(Path, Output);
}
-IntrusiveRefCntPtr vfs::getRealFileSystem() {
- static IntrusiveRefCntPtr FS = new RealFileSystem();
- return FS;
-}
-
namespace {
class RealFSDirIter : public clang::vfs::detail::DirIterImpl {
@@ -2141,3 +2136,289 @@
return *this;
}
+
+namespace {
+class StatFS : public FileSystem {
+public:
+ StatFS(const char *Name, llvm::IntrusiveRefCntPtr FS)
+ : Name(Name), FS(std::move(FS)) {
+llvm::errs() << "created statfs " << Name << "\n";
+ }
+
+ llvm::ErrorOr status(const Twine ) override {
+auto Ret = FS->status(Path);
+bump(Ret ? StatusOK : StatusErr);
+return Ret;
+ }
+
+ llvm::ErrorOr>
+ openFileForRead(const Twine ) override {
+auto Ret = FS->openFileForRead(Path);
+bump(Ret ? OpenOK : OpenErr);
+return Ret;
+ }
+
+ directory_iterator dir_begin(const Twine , std::error_code ) override {
+bump(DirBegin);
+return FS->dir_begin(Dir, EC);
+ }
+
+ virtual std::error_code setCurrentWorkingDirectory(const Twine ) override {
+bump(SetCWD);
+return FS->setCurrentWorkingDirectory(Path);
+ }
+
+ virtual llvm::ErrorOr getCurrentWorkingDirectory() const override {
+bump(GetCWD);
+return FS->getCurrentWorkingDirectory();
+ }
+
+ virtual std::error_code
+ getRealPath(const Twine , SmallVectorImpl ) const override {
+bump(GetRealPath);
+return FS->getRealPath(Path, Output);
+ }
+
+private:
+ void bump(std::atomic ) const {
+++I;
+if (++All % 1 == 0) {
+ llvm::errs() << "== FILESYSTEM " << Name << " ==\n"
+ << "Status: " << StatusOK << "+" << StatusErr << "\n"
+ << "Open: " << OpenOK << "+" << OpenErr << "\n"
+ << "Dir: " << DirBegin << "\n"
+ << "GetRealPath: " << GetRealPath << "\n"
+ << "===\n";
+}
+ }
+
+ const char* Name;
+ llvm::IntrusiveRefCntPtr FS;
+ mutable std::atomic StatusOK = {0}, StatusErr = {0}, OpenOK = {0},
+OpenErr = {0}, DirBegin = {0}, GetRealPath = {0},
+SetCWD = {0}, GetCWD = {0};
+ mutable std::atomic All= {0};
+};
+
+#if 0
+class StatLessFS : public FileSystem {
+ enum State : uint8_t {
+NoInfo, // Initial state.
+DirKnownChildren, // Directory, all children's existence is in the cache.
+DirTooBig, // Directory, known to be too big to enumerate.
+DirUnknownChildren, // Directory, but children are not listed.
+File, // Known to be a file.
+Missing,// Known to not exist.
+MissingOrDir, // May be a directory, or may not exist.
+MissingOrFile // May be a file, or may not exist.
+ };
+ mutable llvm::StringMap Cache;
+ mutable std::mutex Mu;
+public:
+ StatLessFS(llvm::IntrusiveRefCntPtr FS) : FS(std::move(FS)) {
+llvm::errs() << "created statlessfs \n";
+ }
+
+ bool mayBeFile(StringRef Path, bool AllowDir) const {
+llvm::errs() << "Could " << Path << " be a file"
+ << (AllowDir ? " or dir" : "") << "?\n";
+auto Parent = llvm::sys::path::parent_path(Path);
+if (!Parent.empty())
+ fillCache(Parent);
+std::lock_guard Lock(Mu);
+switch (Cache.lookup(Path)) {
+case NoInfo:
+ break;
+case File:
+case MissingOrFile:
+ llvm::errs() << "Cache says maybe\n";
+ return true;
+case Missing:
+ llvm::errs() << "Cache says no\n";
+ return false;
+case DirKnownChildren:
+case DirUnknownChildren:
+case DirTooBig:
+case MissingOrDir:
+ return AllowDir;
+ llvm::errs() << "Cache says " << (AllowDir ? "maybedir" : "no,dir")
+ << "\n";
+ return false;
+}
+if (Parent.empty())
+ return true;
+// We don't know anything about the actual file. Consider the parent...
+llvm::errs() << "Cache says nothing. parent is " << int(Cache.lookup(Parent)) << "\n";
+return Cache.lookup(Parent) == DirTooBig;
+ }
+
+ // Populate cache enough to answer questions
[PATCH] D52421: [Sema] Diagnose parameter names that shadow inherited field names
lebedev.ri accepted this revision. lebedev.ri added a comment. This revision is now accepted and ready to land. This looks good to me, but you probably want to wait for someone else to review this, too. https://reviews.llvm.org/D52421 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52334: [clang-tidy] Build it even without static analyzer
JonasToth added a comment. Please wait for explicit approval from @alexfh Am 26.09.2018 um 13:05 schrieb Stephen Kelly via Phabricator: > steveire added a comment. > > Can I go ahead and merge this now? > > Repository: > > rCTE Clang Tools Extra > > https://reviews.llvm.org/D52334 Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52334 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen(), size() or equal length
JonasToth added inline comments.
Comment at: clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp:1038
+
+ SmallString<128> NewAddNullTermExprStr;
+ NewAddNullTermExprStr = "\n";
whisperity wrote:
> aaron.ballman wrote:
> > This should be done using a `Twine`.
> Can you please give an example of how to well-approach this? We had issues
> with using Twines on the stack and then later on running into weird undefined
> behaviour. The documentation is not clear to a lot of our colleagues on where
> the `Twine`'s usage barriers are... (Asking this not just for @Charusso but
> also for a few more colleagues, @baloghadamsoftware e.g.)
naive approach:
```
std::string New =
Twine("\n").concat(SpaceBeforeStmtStr).concat(exprToStr(..))...).str();
```
I think retrieving a `SmallString` is not supported. Please note, that `Twine`
does support taking integer and floats directly without prior conversion.
You can use `operator+` instead of `concat` too.
Did this help or did you have more specific issues?
https://reviews.llvm.org/D45050
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52419: [clangd] Cache FS stat() calls when building preamble.
ioeric added inline comments.
Comment at: clangd/ClangdUnit.cpp:339
+llvm::ErrorOr status(const Twine ) override {
+ auto I = StatCache.find(Path.str());
+ return (I != StatCache.end()) ? I->getValue() : FS->status(Path);
ilya-biryukov wrote:
> ioeric wrote:
> > ilya-biryukov wrote:
> > > We store absolute paths, but Path can be relative here.
> > This is because preamble stores absolute paths. `Path` should be an path
> > from preamble. Added documentation.
> It's true that preamble stores and checks absolute paths, however clang can
> later access the same file using a relative path.
> Calling makeAbsolute here shouldn't hurt and would obviously make it work in
> both cases.
It would "obviously work"... until you have symlinks and tricks to handle
symlinks ;)
Comment at: clangd/CodeComplete.cpp:1028
+ IntrusiveRefCntPtr VFS = Input.VFS;
+ if (Input.PreambleStatCache)
ilya-biryukov wrote:
> sammccall wrote:
> > ioeric wrote:
> > > ilya-biryukov wrote:
> > > > It feels like we could apply this caching one level higher, on the
> > > > TUScheduler level when creating the filesystem. It makes sense not only
> > > > for code completion, but also for all other features, including
> > > > rebuilds of ASTs that were washed out of the cache.
> > > > WDYT?
> > > It sounds like a reasonable thing to do. I took a quick look, and it
> > > seemed that this would probably require some refactoring/redesign on
> > > TUScheduler - it doesn't know about the VFS?
> > >
> > > I think it shouldn't be hard to do this case by case. For example, code
> > > completion and signature help will be covered in this patch. And it
> > > should be pretty easy to make AST rebuild use the cache as well?
> > > It sounds like a reasonable thing to do. I took a quick look, and it
> > > seemed that this would probably require some refactoring/redesign on
> > > TUScheduler - it doesn't know about the VFS?
> >
> > Right, this is a bit of a mess...
> > Currently there are features that access the FS "directly". This includes
> > CodeComplete which runs its own compile action, as well as things like
> > switchSourceHeader or format. These invoke FSProvider.
> > Then there are features that build an AST (which may need file accesses),
> > and then may implicitly read files by querying the AST (the FS is attached
> > to the FileManager or something). These use the FS obtained from the
> > FSProvider **in the relevant addDocument call**.
> > There's no fundamental reason we can't have both (access FS directly and
> > use the AST) in which case we'll be using both filesystems together...
> >
> > In the near term, making the TUScheduler-managed AST rebuild use the cache
> > stored in the preamble seems like a nice win. (As you alluded to I don't
> > think this change is in TUScheduler itself, rather buildAST?)
> My idea would be to create and pass the updated **cached** FS into both
> `buildAST` and `codeComplete` higher up the call chain. This would allow
> localizing the code that creates caching VFSes in one file (TUScheduler.h or
> similar).
>
> The advantage of receiving the patched-up FS in `buildAST` and `codeComplete`
> is that they don't have to care about this trick, making the implementation
> simpler.
> The fewer functions across the codebase have to apply the trick the better,
> e.g. this would make sure we won't accidentally forget it to apply it when
> implementing a new feature.
Added the cache support to `buildAST` as we thinks it's beneficial (haven't
profiled this though).
Currently, the trick is applied in two places (`semaCodeComplete` in
CodeComplete and `buildAST` in ClangdUnit), and I'm not sure how much this
would grow in the near future. It's also unclear to me whether baking this into
`TUScheduler` would be less work in the long run. I would suggest revisiting
this when we have more evidence.
Comment at: clangd/FS.cpp:29
+PreambleFileStatusCache::lookup(llvm::StringRef File) const {
+ auto I = StatCache.find(File);
+ if (I != StatCache.end())
sammccall wrote:
> lock
After a second thought, I'm wondering if the mutex is necessary, if the cache
is only updated during preamble build in a single thread. The cache would also
remain the same in preamble reuses.
Comment at: clangd/FS.cpp:48
+return File;
+ if (auto S = File->get()->status())
+StatCache.update(getUnderlyingFS(), std::move(*S));
sammccall wrote:
> I'm not sure I get this: AFAICT (at least on linux) the status is never
> available on a newly opened file, so this will always be a stat() call, so
> we're just doing the work eagerly and caching it for later. Isn't this just
> moving the work around?
>
> I'm sure you've verified this is important somehow, but a comment explaining
> how would be much appreciated :-)
Files opened via
[PATCH] D52419: [clangd] Cache FS stat() calls when building preamble.
ioeric updated this revision to Diff 167124.
ioeric marked 7 inline comments as done.
ioeric added a comment.
- address review comments
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52419
Files:
clangd/CMakeLists.txt
clangd/ClangdServer.cpp
clangd/ClangdUnit.cpp
clangd/ClangdUnit.h
clangd/CodeComplete.cpp
clangd/CodeComplete.h
clangd/FS.cpp
clangd/FS.h
unittests/clangd/CMakeLists.txt
unittests/clangd/ClangdTests.cpp
unittests/clangd/FSTests.cpp
unittests/clangd/TestFS.cpp
Index: unittests/clangd/TestFS.cpp
===
--- unittests/clangd/TestFS.cpp
+++ unittests/clangd/TestFS.cpp
@@ -23,6 +23,7 @@
llvm::StringMap const ) {
IntrusiveRefCntPtr MemFS(
new vfs::InMemoryFileSystem);
+ MemFS->setCurrentWorkingDirectory(testRoot());
for (auto : Files) {
StringRef File = FileAndContents.first();
MemFS->addFile(
Index: unittests/clangd/FSTests.cpp
===
--- /dev/null
+++ unittests/clangd/FSTests.cpp
@@ -0,0 +1,46 @@
+//===-- FSTests.cpp - File system related tests -*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "FS.h"
+#include "TestFS.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace clangd {
+namespace {
+
+TEST(FSTests, PreambleStatusCache) {
+ llvm::StringMap Files;
+ Files["x"] = "";
+ Files["y"] = "";
+ auto FS = buildTestFS(Files);
+ FS->setCurrentWorkingDirectory(testRoot());
+
+ PreambleFileStatusCache StatCache;
+ auto ProduceFS = StatCache.getProducingFS(FS);
+ EXPECT_TRUE(ProduceFS->openFileForRead("x"));
+ EXPECT_TRUE(ProduceFS->status("y"));
+
+ EXPECT_TRUE(StatCache.lookup(testPath("x")).hasValue());
+ EXPECT_TRUE(StatCache.lookup(testPath("y")).hasValue());
+
+ vfs::Status S("fake", llvm::sys::fs::UniqueID(0, 0),
+std::chrono::system_clock::now(), 0, 0, 1024,
+llvm::sys::fs::file_type::regular_file, llvm::sys::fs::all_all);
+ StatCache.update(*FS, S);
+ auto ConsumeFS = StatCache.getConsumingFS(FS);
+ auto Cached = ConsumeFS->status(testPath("fake"));
+ EXPECT_TRUE(Cached);
+ EXPECT_EQ(Cached->getName(), S.getName());
+}
+
+} // namespace
+} // namespace clangd
+} // namespace clang
Index: unittests/clangd/ClangdTests.cpp
===
--- unittests/clangd/ClangdTests.cpp
+++ unittests/clangd/ClangdTests.cpp
@@ -963,6 +963,71 @@
Field(::Name, "baz")));
}
+// Check that running code completion doesn't stat() a bunch of files from the
+// preamble again. (They should be using the preamble's stat-cache)
+TEST(ClangdTests, PreambleVFSStatCache) {
+ class ListenStatsFSProvider : public FileSystemProvider {
+ public:
+ListenStatsFSProvider(llvm::StringMap )
+: CountStats(CountStats) {}
+
+IntrusiveRefCntPtr getFileSystem() override {
+ class ListenStatVFS : public vfs::ProxyFileSystem {
+ public:
+ListenStatVFS(IntrusiveRefCntPtr FS,
+ llvm::StringMap )
+: ProxyFileSystem(std::move(FS)), CountStats(CountStats) {}
+
+llvm::ErrorOr>
+openFileForRead(const Twine ) override {
+ ++CountStats[llvm::sys::path::filename(Path.str())];
+ return ProxyFileSystem::openFileForRead(Path);
+}
+llvm::ErrorOr status(const Twine ) override {
+ ++CountStats[llvm::sys::path::filename(Path.str())];
+ return ProxyFileSystem::status(Path);
+}
+
+ private:
+llvm::StringMap
+ };
+
+ return IntrusiveRefCntPtr(
+ new ListenStatVFS(buildTestFS(Files), CountStats));
+}
+
+// If relative paths are used, they are resolved with testPath().
+llvm::StringMap Files;
+llvm::StringMap
+ };
+
+ llvm::StringMap CountStats;
+ ListenStatsFSProvider FS(CountStats);
+ ErrorCheckingDiagConsumer DiagConsumer;
+ MockCompilationDatabase CDB;
+ ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
+
+ auto SourcePath = testPath("foo.cpp");
+ auto HeaderPath = testPath("foo.h");
+ FS.Files[HeaderPath] = "struct TestSym {};";
+ Annotations Code(R"cpp(
+#include "foo.h"
+
+int main() {
+ TestSy^
+})cpp");
+
+ runAddDocument(Server, SourcePath, Code.code());
+
+ EXPECT_EQ(CountStats["foo.h"], 1u);
+ auto Completions = cantFail(runCodeComplete(Server, SourcePath, Code.point(),
+ clangd::CodeCompleteOptions()))
+ .Completions;
+ EXPECT_EQ(CountStats["foo.h"], 1u);
+ EXPECT_THAT(Completions,
+
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
hans added a comment.
The static local stuff still makes me nervous. How much does Chromium break if
we just ignore the problem? And how does -fvisibility-inlines-hidden handle the
issue?
Also, Sema already has a check for static locals in C inline functions:
$ echo "inline void f() { static int x; }" | bin/clang -c -x c -
:1:19: warning: non-constant static local variable in inline function
may be different in different files [-Wstatic-local-in-inline]
inline void f() { static int x; }
^
could we reuse that check somehow for this case with static locals in dllexport
inline methods?
Comment at: clang/lib/Sema/SemaDecl.cpp:12624
+ isa(D)) {
+CXXMethodDecl *MD = dyn_cast(D);
+CXXRecordDecl *Class = MD->getParent();
Hmm, now we're adding an AST walk over all inline methods which probably slows
us down a bit. Not sure I have any better ideas though.
In any case, ActOnFinishInlineFunctionDef needs a comment explaining why it's
doing this.
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:5702
+!getLangOpts().DllExportInlines &&
+Class->getTemplateSpecializationKind() !=
TSK_ExplicitInstantiationDeclaration &&
+Class->getTemplateSpecializationKind() !=
TSK_ExplicitInstantiationDefinition) {
What worries me is that now we have logic in two different places about
inheriting the dll attribute.
The new place in ActOnFinishInlineFunctionDef doesn't have the same checks
(checking for MS ABI and the template specialization kind) which means the
logic for when to inherit is already a little out of sync...
https://reviews.llvm.org/D51340
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52547: Tell whether file/folder for include completions.
kadircet created this revision.
kadircet added a reviewer: sammccall.
Herald added subscribers: cfe-commits, arphaman, jkorous, ioeric, ilya-biryukov.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52547
Files:
clangd/CodeComplete.cpp
clangd/Protocol.h
unittests/clangd/CodeCompleteTests.cpp
Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -2073,6 +2073,27 @@
}
}
+TEST(CompletionTest, IncludedCompletionKinds) {
+ MockFSProvider FS;
+ MockCompilationDatabase CDB;
+ std::string Subdir = testPath("sub");
+ std::string SearchDirArg = (llvm::Twine("-I") + Subdir).str();
+ CDB.ExtraClangFlags = {SearchDirArg.c_str()};
+ std::string BarHeader = testPath("sub/bar.h");
+ FS.Files[BarHeader] = "";
+ IgnoreDiagnostics DiagConsumer;
+ ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
+ auto Results = completions(Server,
+ R"cpp(
+#include "^"
+ )cpp"
+ );
+ EXPECT_THAT(Results.Completions,
+ AllOf(Has("sub/", CompletionItemKind::Folder),
+Has("bar.h\"", CompletionItemKind::File)));
+}
+
+
} // namespace
} // namespace clangd
} // namespace clang
Index: clangd/Protocol.h
===
--- clangd/Protocol.h
+++ clangd/Protocol.h
@@ -704,6 +704,7 @@
Color = 16,
File = 17,
Reference = 18,
+ Folder = 19,
};
/// Defines whether the insert text in a completion item should be interpreted
Index: clangd/CodeComplete.cpp
===
--- clangd/CodeComplete.cpp
+++ clangd/CodeComplete.cpp
@@ -349,6 +349,9 @@
}
Completion.Kind = toCompletionItemKind(
C.SemaResult->Kind, C.SemaResult->Declaration, ContextKind);
+ if (Completion.Kind == CompletionItemKind::File &&
+ Completion.Name.back() == '/')
+Completion.Kind = CompletionItemKind::Folder;
for (const auto : C.SemaResult->FixIts) {
Completion.FixIts.push_back(
toTextEdit(FixIt, ASTCtx.getSourceManager(),
ASTCtx.getLangOpts()));
Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -2073,6 +2073,27 @@
}
}
+TEST(CompletionTest, IncludedCompletionKinds) {
+ MockFSProvider FS;
+ MockCompilationDatabase CDB;
+ std::string Subdir = testPath("sub");
+ std::string SearchDirArg = (llvm::Twine("-I") + Subdir).str();
+ CDB.ExtraClangFlags = {SearchDirArg.c_str()};
+ std::string BarHeader = testPath("sub/bar.h");
+ FS.Files[BarHeader] = "";
+ IgnoreDiagnostics DiagConsumer;
+ ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
+ auto Results = completions(Server,
+ R"cpp(
+#include "^"
+ )cpp"
+ );
+ EXPECT_THAT(Results.Completions,
+ AllOf(Has("sub/", CompletionItemKind::Folder),
+Has("bar.h\"", CompletionItemKind::File)));
+}
+
+
} // namespace
} // namespace clangd
} // namespace clang
Index: clangd/Protocol.h
===
--- clangd/Protocol.h
+++ clangd/Protocol.h
@@ -704,6 +704,7 @@
Color = 16,
File = 17,
Reference = 18,
+ Folder = 19,
};
/// Defines whether the insert text in a completion item should be interpreted
Index: clangd/CodeComplete.cpp
===
--- clangd/CodeComplete.cpp
+++ clangd/CodeComplete.cpp
@@ -349,6 +349,9 @@
}
Completion.Kind = toCompletionItemKind(
C.SemaResult->Kind, C.SemaResult->Declaration, ContextKind);
+ if (Completion.Kind == CompletionItemKind::File &&
+ Completion.Name.back() == '/')
+Completion.Kind = CompletionItemKind::Folder;
for (const auto : C.SemaResult->FixIts) {
Completion.FixIts.push_back(
toTextEdit(FixIt, ASTCtx.getSourceManager(), ASTCtx.getLangOpts()));
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52533: [test] Use --sysroot instead of -B in print-multi-directory.c
chrib added a reviewer: jroelofs. chrib added a comment. Hi Martin, maybe just a NIT, use --sysroot= rather than just --sysroot for consistency with other tests. Otherwise looks good to me, thanks (adding Jonathan as I'm not sure I can accept) Repository: rC Clang https://reviews.llvm.org/D52533 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52530: [analyzer] scan-build: if --status-bugs is passed, don't forget about the exit status of the actual build
This revision was automatically updated to reflect the committed changes.
Closed by commit rC343105: [analyzer] scan-build: if --status-bugs is passed,
dont forget about the exit… (authored by lebedevri, committed by ).
Repository:
rC Clang
https://reviews.llvm.org/D52530
Files:
tools/scan-build/bin/scan-build
Index: tools/scan-build/bin/scan-build
===
--- tools/scan-build/bin/scan-build
+++ tools/scan-build/bin/scan-build
@@ -1207,7 +1207,7 @@
By default, the exit status of scan-build is the same as the executed build
command. Specifying this option causes the exit status of scan-build to be 1
- if it found potential bugs and 0 otherwise.
+ if it found potential bugs and the exit status of the build itself
otherwise.
--exclude
@@ -1908,7 +1908,7 @@
if ($Options{ExitStatusFoundBugs}) {
exit 1 if ($NumBugs > 0);
- exit 0;
+ exit $ExitStatus;
}
}
}
Index: tools/scan-build/bin/scan-build
===
--- tools/scan-build/bin/scan-build
+++ tools/scan-build/bin/scan-build
@@ -1207,7 +1207,7 @@
By default, the exit status of scan-build is the same as the executed build
command. Specifying this option causes the exit status of scan-build to be 1
- if it found potential bugs and 0 otherwise.
+ if it found potential bugs and the exit status of the build itself otherwise.
--exclude
@@ -1908,7 +1908,7 @@
if ($Options{ExitStatusFoundBugs}) {
exit 1 if ($NumBugs > 0);
- exit 0;
+ exit $ExitStatus;
}
}
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52530: [analyzer] scan-build: if --status-bugs is passed, don't forget about the exit status of the actual build
lebedev.ri added a comment. In https://reviews.llvm.org/D52530#1246459, @jroelofs wrote: > LGTM Thank you for the speedy review! Repository: rC Clang https://reviews.llvm.org/D52530 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r343105 - [analyzer] scan-build: if --status-bugs is passed, don't forget about the exit status of the actual build
Author: lebedevri
Date: Wed Sep 26 06:08:44 2018
New Revision: 343105
URL: http://llvm.org/viewvc/llvm-project?rev=343105=rev
Log:
[analyzer] scan-build: if --status-bugs is passed, don't forget about the exit
status of the actual build
Summary:
This has been bothering me for a while, but only now i have actually looked
into this.
I'm using one CI job for static analysis - clang static analyzers as compilers
+ clang-tidy via cmake.
And i'd like for the build to fail if at least one of those finds issues.
If clang-tidy finds issues, it will fail the build since the warnings-as-errors
is set.
If static analyzer finds anything, since --status-bugs is set, it will fail the
build.
But if clang-tidy find anything, but static analyzer does not, the build
succeeds :/
Reviewers: sylvestre.ledru, alexfh, jroelofs, ygribov, george.karpenkov,
krememek
Reviewed By: jroelofs
Subscribers: xazax.hun, szepet, a.sidorin, mikhail.ramalho, Szelethus,
cfe-commits
Differential Revision: https://reviews.llvm.org/D52530
Modified:
cfe/trunk/tools/scan-build/bin/scan-build
Modified: cfe/trunk/tools/scan-build/bin/scan-build
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/scan-build/bin/scan-build?rev=343105=343104=343105=diff
==
--- cfe/trunk/tools/scan-build/bin/scan-build (original)
+++ cfe/trunk/tools/scan-build/bin/scan-build Wed Sep 26 06:08:44 2018
@@ -1207,7 +1207,7 @@ OPTIONS:
By default, the exit status of scan-build is the same as the executed build
command. Specifying this option causes the exit status of scan-build to be 1
- if it found potential bugs and 0 otherwise.
+ if it found potential bugs and the exit status of the build itself
otherwise.
--exclude
@@ -1908,7 +1908,7 @@ if (defined $Options{OutputFormat}) {
if ($Options{ExitStatusFoundBugs}) {
exit 1 if ($NumBugs > 0);
- exit 0;
+ exit $ExitStatus;
}
}
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52390: [analyzer] Add StackSizeChecker to StaticAnalyzer
Szelethus added inline comments.
Comment at:
include/clang/StaticAnalyzer/Checkers/StackUsageMeasuringVisitor.h:102-111
+ bool hasEmptyFlaggedUsageStored(const Stmt *S) const {
+auto iter = StmtSubtreeUsages.find(S);
+return iter != StmtSubtreeUsages.end() &&
+ iter->second == Usage::EmptyFlagged;
+ }
+ bool hasEmptyFlaggedUsageStored(const Decl *D) const {
+auto iter = DeclSubtreeUsages.find(D);
whisperity wrote:
> Szelethus wrote:
> > Would `isEmptyFlagged` be a better method name?
> Then calling this method would mean answering this question: "A Stmt/Decl is
> empty flagged?" This is a "What?" moment.
Right. And it doesn't actually describe what the function does.
`hasEmptyFlaggedSubDecl`?
Comment at: lib/StaticAnalyzer/Checkers/StackSizeChecker.cpp:25
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+#include
+
whisperity wrote:
> Szelethus wrote:
> > Don't include standard stream libraries:
> > https://llvm.org/docs/CodingStandards.html#include-iostream-is-forbidden
> > Note that using the other stream headers (`` for example) is not
> > problematic in this regard
Oh, right, sorry about that. In either case, every other checker uses LLVM
streams, so I guess `llvm::raw_*_ostream` are the preferred stream classes
anyways.
Comment at: lib/StaticAnalyzer/Checkers/StackSizeChecker.cpp:41
+: public Checker {
+ mutable std::unique_ptr StackOverflowBugType;
+
whisperity wrote:
> Szelethus wrote:
> > I think you don't need to make this `mutable` anymore, you can just
> > initialize it in the constructor.
> `generateError` is const-qualified but writes this.
But I'm not sure that it should. At least, when I wrote my checker, I didn't
have to:
https://github.com/llvm-mirror/clang/blob/master/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp#L166
Repository:
rC Clang
https://reviews.llvm.org/D52390
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52360: [clang-tidy] Fix for performance-unnecessary-value-param, performance-unnecessary-copy-initialization and performance-for-range-copy
baloghadamsoftware added a comment. In https://reviews.llvm.org/D52360#1246452, @lebedev.ri wrote: > I disagree, it **must** not have false-negatives by default. What kind of false-negatives could be caused by smart pointers or references? Sane people do not name a type pointer or reference if they are not. Such types must never be passed by reference. Few people will use the check if they have to set tons of options for the basic expected behavior. For example in `CodeChecker` this check is disabled by default and is only enabled in the `Sensitive` profile. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52360 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52360: [clang-tidy] Fix for performance-unnecessary-value-param, performance-unnecessary-copy-initialization and performance-for-range-copy
baloghadamsoftware added a comment. In https://reviews.llvm.org/D52360#1246443, @danilaml wrote: > Would that also skip checks for something like `shared_ptr`? Yes, everything ending on `pointer`, `ptr`, `reference` or `ref`, first letter case insensitive. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52360 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52545: [docs] Update PostingList string representation format
kbobyrev created this revision.
kbobyrev added reviewers: ioeric, sammccall.
kbobyrev added a project: clang-tools-extra.
Herald added subscribers: kadircet, arphaman, jkorous.
Because `PostingList` objects are compressed, it is now impossible to see
elements other than the current one and the documentation doesn't match
implementation anymore.
https://reviews.llvm.org/D52545
Files:
clang-tools-extra/clangd/index/dex/Iterator.h
Index: clang-tools-extra/clangd/index/dex/Iterator.h
===
--- clang-tools-extra/clangd/index/dex/Iterator.h
+++ clang-tools-extra/clangd/index/dex/Iterator.h
@@ -94,9 +94,8 @@
///
/// Where Type is the iterator type representation: "&" for And, "|" for Or,
/// ChildN is N-th iterator child. Raw iterators over PostingList are
- /// represented as "[ID1, ID2, ..., {IDN}, ... END]" where IDN is N-th
- /// PostingList entry and the element which is pointed to by the PostingList
- /// iterator is enclosed in {} braces.
+ /// represented as "[(...)? (IDN | END) (...)?]" where IDN is N-th
+ /// PostingList entry.
friend llvm::raw_ostream <<(llvm::raw_ostream ,
const Iterator ) {
return Iterator.dump(OS);
Index: clang-tools-extra/clangd/index/dex/Iterator.h
===
--- clang-tools-extra/clangd/index/dex/Iterator.h
+++ clang-tools-extra/clangd/index/dex/Iterator.h
@@ -94,9 +94,8 @@
///
/// Where Type is the iterator type representation: "&" for And, "|" for Or,
/// ChildN is N-th iterator child. Raw iterators over PostingList are
- /// represented as "[ID1, ID2, ..., {IDN}, ... END]" where IDN is N-th
- /// PostingList entry and the element which is pointed to by the PostingList
- /// iterator is enclosed in {} braces.
+ /// represented as "[(...)? (IDN | END) (...)?]" where IDN is N-th
+ /// PostingList entry.
friend llvm::raw_ostream <<(llvm::raw_ostream ,
const Iterator ) {
return Iterator.dump(OS);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52530: [analyzer] scan-build: if --status-bugs is passed, don't forget about the exit status of the actual build
jroelofs accepted this revision. jroelofs added a comment. This revision is now accepted and ready to land. LGTM Repository: rC Clang https://reviews.llvm.org/D52530 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52499: [clang-cl] Make /Gs imply default stack probes, not /Gs0 (PR39074)
hans added inline comments. Comment at: include/clang/Driver/CLCompatOptions.td:94 +def : CLFlag<"Gs">, HelpText<"Use stack probes (default)">, + Alias, AliasArgs<["4096"]>; def _SLASH_Gs : CLJoined<"Gs">, thakis wrote: > https://docs.microsoft.com/en-us/cpp/build/reference/gs-control-stack-checking-calls?view=vs-2017 > still claims that /Gs is /Gs0 though. Is that just wrong? > https://bugs.llvm.org/show_bug.cgi?id=39074#c2 suggests it is. Should we ask > bruce to file a doc bug? > > And since this flag is supposed to get the default behavior, should it be a > CLIgnoredFlag instead of duplicating the 4096 somewhat redundantly? I've submitted feedback on the document page: https://github.com/MicrosoftDocs/cpp-docs/issues/445 By not ignoring it, we enable using /Gs to override a previous e.g. /Gs0 flag, which I think is the correct thing to do. Repository: rL LLVM https://reviews.llvm.org/D52499 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52499: [clang-cl] Make /Gs imply default stack probes, not /Gs0 (PR39074)
thakis added a comment. Thanks! lgtm. Comment at: include/clang/Driver/CLCompatOptions.td:94 +def : CLFlag<"Gs">, HelpText<"Use stack probes (default)">, + Alias, AliasArgs<["4096"]>; def _SLASH_Gs : CLJoined<"Gs">, https://docs.microsoft.com/en-us/cpp/build/reference/gs-control-stack-checking-calls?view=vs-2017 still claims that /Gs is /Gs0 though. Is that just wrong? https://bugs.llvm.org/show_bug.cgi?id=39074#c2 suggests it is. Should we ask bruce to file a doc bug? And since this flag is supposed to get the default behavior, should it be a CLIgnoredFlag instead of duplicating the 4096 somewhat redundantly? Repository: rL LLVM https://reviews.llvm.org/D52499 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [clang-tools-extra] r342730 - [clangd] Remember to serialize symbol origin in YAML.
I think it depends on how you interpret the origin. You could tie the
origin to the index or to the producer of the symbol. The current model
seems to be the later (origin is a configuration in SymbolCollector.
MergeIndex manipulates the origin, but I would rather treat that as a
special case). And I think it's conceptually simpler and more generic
(origin can be more than dynamic vs static). The downside is probably that
all symbols in yaml would have the same origin, which is a bit redundant
but doesn't seem to matter much.
Both models have their pros and cons, but I don't think it's worth spending
much time figuring out which one is better when it seems like a non-issue.
On Tue, Sep 25, 2018 at 6:58 PM Ilya Biryukov wrote:
>
>
> Eric Liu schrieb am Di., 25. Sep. 2018, 01:22:
>
>> On Tue, Sep 25, 2018 at 6:34 AM Ilya Biryukov
>> wrote:
>>
>>> Why would we want to serialize the origin?
>>>
>> We only serialize and deserialize for the static index, it does not seem
>>> to be useful to serialize origin in that scenario.
>>>
>> We serialize Origin because it's a property of Symbol? The origin for the
>> current YAML symbols is defaulted to "unknown".
>>
> My view would be that it's a property of a symbol living in memory, but
> not the serialized one. E.g. all symbols read from the yaml index should
> have the static origin. If we store an origin, we allow loading symbols
> with non-static origin, it's hard to see how that would be useful.
>
>
>
>
>> It's true that we *currently* only serialize symbols from static index,
>> but there is nothing preventing us from doing it for other indexes with
>> different origins. You could probably override origins when loading static
>> index, but that doesn't seem to work in general.
>>
> The origin seems to be exactly the field that is set and manipulated by
> index implementations, but they all have the knowledge on what their origin
> is or how to combine origins of subindexes.
>
> Again, it seems there are two classes hiding in symbol. All other fields
> provide useful information about C++ semantics of the symbol, while origin
> provides some traceability when combining indexes. The former is something
> we need to serialize, the latter is something we can infer when
> deserializing.
>
> If we will have a use case for serializing the latter entity(with origin)
> for any reason, we might add that separately.
>
> Not sure if it's worth the trouble changing it, just wanted to point out
> that storing the origin for each symbol in the yaml index for the purpose
> of indicating that the symbol is in the yaml index seems to be a bit off.
>
>
>
>> I checked this in without review as I thought this was a trivial fix. The
>> binary serialization also serializes the Origin field.
>>
> LG to be consistent with binary serialization, the same question applies
> to binary serialization.
>
> Again, the change itself seems fine, just nitpicking on whether putting
> origin into a symbol at that level makes sense.
>
>
>
>>> Am I missing something?
>>>
>>
>>> On Fri, Sep 21, 2018 at 3:06 PM Eric Liu via cfe-commits <
>>> cfe-commits@lists.llvm.org> wrote:
>>>
Author: ioeric
Date: Fri Sep 21 06:04:57 2018
New Revision: 342730
URL: http://llvm.org/viewvc/llvm-project?rev=342730=rev
Log:
[clangd] Remember to serialize symbol origin in YAML.
Modified:
clang-tools-extra/trunk/clangd/index/SymbolYAML.cpp
clang-tools-extra/trunk/unittests/clangd/SerializationTests.cpp
Modified: clang-tools-extra/trunk/clangd/index/SymbolYAML.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/SymbolYAML.cpp?rev=342730=342729=342730=diff
==
--- clang-tools-extra/trunk/clangd/index/SymbolYAML.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/SymbolYAML.cpp Fri Sep 21
06:04:57 2018
@@ -27,6 +27,7 @@ namespace yaml {
using clang::clangd::Symbol;
using clang::clangd::SymbolID;
+using clang::clangd::SymbolOrigin;
using clang::clangd::SymbolLocation;
using clang::index::SymbolInfo;
using clang::index::SymbolKind;
@@ -65,6 +66,17 @@ struct NormalizedSymbolFlag {
uint8_t Flag = 0;
};
+struct NormalizedSymbolOrigin {
+ NormalizedSymbolOrigin(IO &) {}
+ NormalizedSymbolOrigin(IO &, SymbolOrigin O) {
+Origin = static_cast(O);
+ }
+
+ SymbolOrigin denormalize(IO &) { return
static_cast(Origin); }
+
+ uint8_t Origin = 0;
+};
+
template <> struct MappingTraits {
static void mapping(IO , SymbolLocation::Position ) {
IO.mapRequired("Line", Value.Line);
@@ -102,6 +114,8 @@ template <> struct MappingTraits
MappingNormalization NSymbolID(IO,
Sym.ID);
MappingNormalization
NSymbolFlag(
IO,
[PATCH] D52360: [clang-tidy] Fix for performance-unnecessary-value-param, performance-unnecessary-copy-initialization and performance-for-range-copy
lebedev.ri added a comment. In https://reviews.llvm.org/D52360#1246440, @baloghadamsoftware wrote: > Config option is a good idea but it must not be empty by default. The checker > must ignore all pointer and references by default based on their names. I disagree, it **must** not have false-negatives by default. It **may** have false-positives, that are controllable via the options. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52360 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
