r312436 - Driver; extract target specific option application (NFC)

2017-09-02 Thread Saleem Abdulrasool via cfe-commits 
Author: compnerd
Date: Sat Sep  2 21:47:00 2017
New Revision: 312436

URL: http://llvm.org/viewvc/llvm-project?rev=312436=rev
Log:
Driver; extract target specific option application (NFC)

Extract the target specific option application.  This is a huge switch
which was inlined into the `ConstructJob` option which adds a large
amount of code to the already large function.  Extract it to simply
reduce the line count.  NFC

Modified:
cfe/trunk/lib/Driver/ToolChains/Clang.cpp
cfe/trunk/lib/Driver/ToolChains/Clang.h

Modified: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Clang.cpp?rev=312436=312435=312436=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp Sat Sep  2 21:47:00 2017
@@ -1360,6 +1360,77 @@ void Clang::AddARMTargetArgs(const llvm:
 CmdArgs.push_back("-no-implicit-float");
 }
 
+void Clang::RenderTargetOptions(const llvm::Triple ,
+const ArgList , bool KernelOrKext,
+ArgStringList ) const {
+  const ToolChain  = getToolChain();
+
+  // Add the target features
+  getTargetFeatures(TC, EffectiveTriple, Args, CmdArgs, false);
+
+  // Add target specific flags.
+  switch (TC.getArch()) {
+  default:
+break;
+
+  case llvm::Triple::arm:
+  case llvm::Triple::armeb:
+  case llvm::Triple::thumb:
+  case llvm::Triple::thumbeb:
+// Use the effective triple, which takes into account the deployment 
target.
+AddARMTargetArgs(EffectiveTriple, Args, CmdArgs, KernelOrKext);
+CmdArgs.push_back("-fallow-half-arguments-and-returns");
+break;
+
+  case llvm::Triple::aarch64:
+  case llvm::Triple::aarch64_be:
+AddAArch64TargetArgs(Args, CmdArgs);
+CmdArgs.push_back("-fallow-half-arguments-and-returns");
+break;
+
+  case llvm::Triple::mips:
+  case llvm::Triple::mipsel:
+  case llvm::Triple::mips64:
+  case llvm::Triple::mips64el:
+AddMIPSTargetArgs(Args, CmdArgs);
+break;
+
+  case llvm::Triple::ppc:
+  case llvm::Triple::ppc64:
+  case llvm::Triple::ppc64le:
+AddPPCTargetArgs(Args, CmdArgs);
+break;
+
+  case llvm::Triple::sparc:
+  case llvm::Triple::sparcel:
+  case llvm::Triple::sparcv9:
+AddSparcTargetArgs(Args, CmdArgs);
+break;
+
+  case llvm::Triple::systemz:
+AddSystemZTargetArgs(Args, CmdArgs);
+break;
+
+  case llvm::Triple::x86:
+  case llvm::Triple::x86_64:
+AddX86TargetArgs(Args, CmdArgs);
+break;
+
+  case llvm::Triple::lanai:
+AddLanaiTargetArgs(Args, CmdArgs);
+break;
+
+  case llvm::Triple::hexagon:
+AddHexagonTargetArgs(Args, CmdArgs);
+break;
+
+  case llvm::Triple::wasm32:
+  case llvm::Triple::wasm64:
+AddWebAssemblyTargetArgs(Args, CmdArgs);
+break;
+  }
+}
+
 void Clang::AddAArch64TargetArgs(const ArgList ,
  ArgStringList ) const {
   const llvm::Triple  = getToolChain().getEffectiveTriple();
@@ -3374,68 +3445,7 @@ void Clang::ConstructJob(Compilation ,
 CmdArgs.push_back(Args.MakeArgString(CPU));
   }
 
-  // Add the target features
-  getTargetFeatures(getToolChain(), Triple, Args, CmdArgs, false);
-
-  // Add target specific flags.
-  switch (getToolChain().getArch()) {
-  default:
-break;
-
-  case llvm::Triple::arm:
-  case llvm::Triple::armeb:
-  case llvm::Triple::thumb:
-  case llvm::Triple::thumbeb:
-// Use the effective triple, which takes into account the deployment 
target.
-AddARMTargetArgs(Triple, Args, CmdArgs, KernelOrKext);
-break;
-
-  case llvm::Triple::aarch64:
-  case llvm::Triple::aarch64_be:
-AddAArch64TargetArgs(Args, CmdArgs);
-break;
-
-  case llvm::Triple::mips:
-  case llvm::Triple::mipsel:
-  case llvm::Triple::mips64:
-  case llvm::Triple::mips64el:
-AddMIPSTargetArgs(Args, CmdArgs);
-break;
-
-  case llvm::Triple::ppc:
-  case llvm::Triple::ppc64:
-  case llvm::Triple::ppc64le:
-AddPPCTargetArgs(Args, CmdArgs);
-break;
-
-  case llvm::Triple::sparc:
-  case llvm::Triple::sparcel:
-  case llvm::Triple::sparcv9:
-AddSparcTargetArgs(Args, CmdArgs);
-break;
-
-  case llvm::Triple::systemz:
-AddSystemZTargetArgs(Args, CmdArgs);
-break;
-
-  case llvm::Triple::x86:
-  case llvm::Triple::x86_64:
-AddX86TargetArgs(Args, CmdArgs);
-break;
-
-  case llvm::Triple::lanai:
-AddLanaiTargetArgs(Args, CmdArgs);
-break;
-
-  case llvm::Triple::hexagon:
-AddHexagonTargetArgs(Args, CmdArgs);
-break;
-
-  case llvm::Triple::wasm32:
-  case llvm::Triple::wasm64:
-AddWebAssemblyTargetArgs(Args, CmdArgs);
-break;
-  }
+  RenderTargetOptions(Triple, Args, KernelOrKext, CmdArgs);
 
   // These two are potentially updated by AddClangCLArgs.
   codegenoptions::DebugInfoKind DebugInfoKind = codegenoptions::NoDebugInfo;
@@ -3930,20 +3940,6 @@ void Clang::ConstructJob(Compilation ,

r312435 - Driver: extract debugging related options (NFC)

2017-09-02 Thread Saleem Abdulrasool via cfe-commits 
Author: compnerd
Date: Sat Sep  2 21:46:59 2017
New Revision: 312435

URL: http://llvm.org/viewvc/llvm-project?rev=312435=rev
Log:
Driver: extract debugging related options (NFC)

Out-of-line the logic for selecting the debug information handling.
This is still split across the new function and partially inline in the
job construction.  This is needed since the split portion attempts to
record the "-cc1" arguments.  This needs to be the very last item to
ensure that all the flags are recorded.  NFC.

Modified:
cfe/trunk/lib/Driver/ToolChains/Clang.cpp

Modified: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Clang.cpp?rev=312435=312434=312435=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp Sat Sep  2 21:46:59 2017
@@ -2758,6 +2758,170 @@ static void RenderDiagnosticsOptions(con
 CmdArgs.push_back("-fno-spell-checking");
 }
 
+static void RenderDebugOptions(const ToolChain , const Driver ,
+   const llvm::Triple , const ArgList ,
+   bool EmitCodeView, bool IsWindowsMSVC,
+   ArgStringList ,
+   codegenoptions::DebugInfoKind ,
+   const Arg *) {
+  bool IsPS4CPU = T.isPS4CPU();
+
+  if (Args.hasFlag(options::OPT_fdebug_info_for_profiling,
+   options::OPT_fno_debug_info_for_profiling, false))
+CmdArgs.push_back("-fdebug-info-for-profiling");
+
+  // The 'g' groups options involve a somewhat intricate sequence of decisions
+  // about what to pass from the driver to the frontend, but by the time they
+  // reach cc1 they've been factored into three well-defined orthogonal 
choices:
+  //  * what level of debug info to generate
+  //  * what dwarf version to write
+  //  * what debugger tuning to use
+  // This avoids having to monkey around further in cc1 other than to disable
+  // codeview if not running in a Windows environment. Perhaps even that
+  // decision should be made in the driver as well though.
+  unsigned DWARFVersion = 0;
+  llvm::DebuggerKind DebuggerTuning = TC.getDefaultDebuggerTuning();
+
+  bool SplitDWARFInlining =
+  Args.hasFlag(options::OPT_fsplit_dwarf_inlining,
+   options::OPT_fno_split_dwarf_inlining, true);
+
+  Args.ClaimAllArgs(options::OPT_g_Group);
+
+  SplitDWARFArg = Args.getLastArg(options::OPT_gsplit_dwarf);
+
+  if (const Arg *A = Args.getLastArg(options::OPT_g_Group)) {
+// If the last option explicitly specified a debug-info level, use it.
+if (A->getOption().matches(options::OPT_gN_Group)) {
+  DebugInfoKind = DebugLevelToInfoKind(*A);
+  // If you say "-gsplit-dwarf -gline-tables-only", -gsplit-dwarf loses.
+  // But -gsplit-dwarf is not a g_group option, hence we have to check the
+  // order explicitly. If -gsplit-dwarf wins, we fix DebugInfoKind later.
+  // This gets a bit more complicated if you've disabled inline info in the
+  // skeleton CUs (SplitDWARFInlining) - then there's value in composing
+  // split-dwarf and line-tables-only, so let those compose naturally in
+  // that case.
+  // And if you just turned off debug info, (-gsplit-dwarf -g0) - do that.
+  if (SplitDWARFArg) {
+if (A->getIndex() > SplitDWARFArg->getIndex()) {
+  if (DebugInfoKind == codegenoptions::NoDebugInfo ||
+  (DebugInfoKind == codegenoptions::DebugLineTablesOnly &&
+   SplitDWARFInlining))
+SplitDWARFArg = nullptr;
+} else if (SplitDWARFInlining)
+  DebugInfoKind = codegenoptions::NoDebugInfo;
+  }
+} else {
+  // For any other 'g' option, use Limited.
+  DebugInfoKind = codegenoptions::LimitedDebugInfo;
+}
+  }
+
+  // If a debugger tuning argument appeared, remember it.
+  if (const Arg *A =
+  Args.getLastArg(options::OPT_gTune_Group, options::OPT_ggdbN_Group)) 
{
+if (A->getOption().matches(options::OPT_glldb))
+  DebuggerTuning = llvm::DebuggerKind::LLDB;
+else if (A->getOption().matches(options::OPT_gsce))
+  DebuggerTuning = llvm::DebuggerKind::SCE;
+else
+  DebuggerTuning = llvm::DebuggerKind::GDB;
+  }
+
+  // If a -gdwarf argument appeared, remember it.
+  if (const Arg *A =
+  Args.getLastArg(options::OPT_gdwarf_2, options::OPT_gdwarf_3,
+  options::OPT_gdwarf_4, options::OPT_gdwarf_5))
+DWARFVersion = DwarfVersionNum(A->getSpelling());
+
+  // Forward -gcodeview. EmitCodeView might have been set by CL-compatibility
+  // argument parsing.
+  if (Args.hasArg(options::OPT_gcodeview) || EmitCodeView) {
+// DWARFVersion remains at 0 if no explicit choice was made.
+CmdArgs.push_back("-gcodeview");
+  } else if (DWARFVersion == 0 &&
+ DebugInfoKind !=

r312434 - Driver: move `-mfpmath` into FP Options (NFC)

2017-09-02 Thread Saleem Abdulrasool via cfe-commits 
Author: compnerd
Date: Sat Sep  2 21:46:57 2017
New Revision: 312434

URL: http://llvm.org/viewvc/llvm-project?rev=312434=rev
Log:
Driver: move `-mfpmath` into FP Options (NFC)

Move the `-mfpmath` handling with the rest of the floating point
optimization flags.

Modified:
cfe/trunk/lib/Driver/ToolChains/Clang.cpp

Modified: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Clang.cpp?rev=312434=312433=312434=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp Sat Sep  2 21:46:57 2017
@@ -2125,6 +2125,11 @@ static void RenderFloatingPointOptions(c
   // Handle __FINITE_MATH_ONLY__ similarly.
   if (!HonorINFs && !HonorNaNs)
 CmdArgs.push_back("-ffinite-math-only");
+
+  if (const Arg *A = Args.getLastArg(options::OPT_mfpmath_EQ)) {
+CmdArgs.push_back("-mfpmath");
+CmdArgs.push_back(A->getValue());
+  }
 }
 
 static void RenderAnalyzerOptions(const ArgList , ArgStringList ,
@@ -3206,11 +3211,6 @@ void Clang::ConstructJob(Compilation ,
 CmdArgs.push_back(Args.MakeArgString(CPU));
   }
 
-  if (const Arg *A = Args.getLastArg(options::OPT_mfpmath_EQ)) {
-CmdArgs.push_back("-mfpmath");
-CmdArgs.push_back(A->getValue());
-  }
-
   // Add the target features
   getTargetFeatures(getToolChain(), Triple, Args, CmdArgs, false);
 


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D36562: [Bitfield] Make the bitfield a separate location if it has width of legal integer type and its bit offset is naturally aligned for the type

2017-09-02 Thread Hal Finkel via cfe-commits 


On 08/22/2017 10:56 PM, Wei Mi via llvm-commits wrote:

On Tue, Aug 22, 2017 at 7:03 PM, Xinliang David Li  wrote:


On Tue, Aug 22, 2017 at 6:37 PM, Chandler Carruth via Phabricator
 wrote:

chandlerc added a comment.

I'm really not a fan of the degree of complexity and subtlety that this
introduces into the frontend, all to allow particular backend optimizations.

I feel like this is Clang working around a fundamental deficiency in LLVM
and we should instead find a way to fix this in LLVM itself.

As has been pointed out before, user code can synthesize large integers
that small bit sequences are extracted from, and Clang and LLVM should
handle those just as well as actual bitfields.

Can we see how far we can push the LLVM side before we add complexity to
Clang here? I understand that there remain challenges to LLVM's stuff, but I
don't think those challenges make *all* of the LLVM improvements off the
table, I don't think we've exhausted all ways of improving the LLVM changes
being proposed, and I think we should still land all of those and
re-evaluate how important these issues are when all of that is in place.


The main challenge of doing  this in LLVM is that inter-procedural analysis
(and possibly cross module) is needed (for store forwarding issues).

Wei, perhaps you can provide concrete test case to illustrate the issue so
that reviewers have a good understanding.

David

Here is a runable testcase:
 1.cc 
class A {
public:
   unsigned long f1:2;
   unsigned long f2:6;
   unsigned long f3:8;
   unsigned long f4:4;
};
A a;
unsigned long b;
unsigned long N = 10;

__attribute__((noinline))
void foo() {
   a.f3 = 3;
}

__attribute__((noinline))
void goo() {
   b = a.f3;
}

int main() {
   unsigned long i;
   for (i = 0; i < N; i++) {
 foo();
 goo();
   }
}

Now trunk takes about twice running time compared with trunk + this
patch. That is because trunk shrinks the store of a.f3 in foo (Done by
DagCombiner) but not shrink the load of a.f3 in goo, so store
forwarding will be blocked.


I can confirm that this is true on Haswell and also on an POWER8. 
Interestingly, on a POWER7, the performance is the same either way (on 
the good side). I ran the test case as presented and where I replaced f3 
with a non-bitfield unsigned char member. Thinking that the POWER7 
result might be because it's big-Endian, I flipped the order of the 
fields, and found that the version where f3 is not a bitfield is faster 
than otherwise, but only by 12.5%.


Why, in this case, don't we shrink the load? It seems like we should 
(and it seems like a straightforward case).


Thanks again,
Hal



The testcases shows the potential problem of store shrinking. Before
we decide to do store shrinking, we need to know all the related loads
will be shrunk,  and that requires IPA analysis. Otherwise, when load
shrinking was blocked for some difficult case (Like the instcombine
case described in
https://www.mail-archive.com/cfe-commits@lists.llvm.org/msg65085.html),
performance regression will happen.

Wei.





Repository:
   rL LLVM

https://reviews.llvm.org/D36562




___
llvm-commits mailing list
llvm-comm...@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits


--
Hal Finkel
Lead, Compiler Technology and Programming Languages
Leadership Computing Facility
Argonne National Laboratory

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37413: [X86][MS-InlineAsm] Extended support for variables / identifiers on memory / immediate expressions

2017-09-02 Thread coby via Phabricator via cfe-commits 
coby updated this revision to Diff 113659.

Repository:
  rL LLVM

https://reviews.llvm.org/D37413

Files:
  include/clang/Parse/Parser.h
  include/clang/Sema/Sema.h
  lib/Parse/ParseStmtAsm.cpp
  lib/Sema/SemaStmtAsm.cpp

Index: lib/Parse/ParseStmtAsm.cpp
===
--- lib/Parse/ParseStmtAsm.cpp
+++ lib/Parse/ParseStmtAsm.cpp
@@ -54,17 +54,17 @@
 assert(AsmToks.size() == AsmTokOffsets.size());
   }
 
-  void *LookupInlineAsmIdentifier(StringRef ,
-  llvm::InlineAsmIdentifierInfo ,
-  bool IsUnevaluatedContext) override {
+  void LookupInlineAsmIdentifier(StringRef ,
+ llvm::InlineAsmIdentifierInfo ,
+ bool IsUnevaluatedContext) override {
 // Collect the desired tokens.
 SmallVector LineToks;
 const Token *FirstOrigToken = nullptr;
 findTokensForString(LineBuf, LineToks, FirstOrigToken);
 
 unsigned NumConsumedToks;
 ExprResult Result = TheParser.ParseMSAsmIdentifier(
-LineToks, NumConsumedToks, , IsUnevaluatedContext);
+LineToks, NumConsumedToks, IsUnevaluatedContext);
 
 // If we consumed the entire line, tell MC that.
 // Also do this if we consumed nothing as a way of reporting failure.
@@ -89,9 +89,10 @@
   LineBuf = LineBuf.substr(0, TotalOffset);
 }
 
-// Initialize the "decl" with the lookup result.
-Info.OpDecl = static_cast(Result.get());
-return Info.OpDecl;
+// Initialize Info with the lookup result.
+if (!Result.isUsable())
+  return;
+TheParser.getActions().FillInlineAsmIdentifierInfo(Result.get(), Info);
   }
 
   StringRef LookupInlineAsmLabel(StringRef Identifier, llvm::SourceMgr ,
@@ -178,16 +179,9 @@
 }
 
 /// Parse an identifier in an MS-style inline assembly block.
-///
-/// \param CastInfo - a void* so that we don't have to teach Parser.h
-///   about the actual type.
 ExprResult Parser::ParseMSAsmIdentifier(llvm::SmallVectorImpl ,
 unsigned ,
-void *CastInfo,
 bool IsUnevaluatedContext) {
-  llvm::InlineAsmIdentifierInfo  =
-  *(llvm::InlineAsmIdentifierInfo *)CastInfo;
-
   // Push a fake token on the end so that we don't overrun the token
   // stream.  We use ';' because it expression-parsing should never
   // overrun it.
@@ -227,7 +221,7 @@
  /*AllowDeductionGuide=*/false,
  /*ObjectType=*/nullptr, TemplateKWLoc, Id);
 // Perform the lookup.
-Result = Actions.LookupInlineAsmIdentifier(SS, TemplateKWLoc, Id, Info,
+Result = Actions.LookupInlineAsmIdentifier(SS, TemplateKWLoc, Id,
IsUnevaluatedContext);
   }
   // While the next two tokens are 'period' 'identifier', repeatedly parse it as
@@ -241,7 +235,7 @@
 IdentifierInfo *Id = Tok.getIdentifierInfo();
 ConsumeToken(); // Consume the identifier.
 Result = Actions.LookupInlineAsmVarDeclField(Result.get(), Id->getName(),
- Info, Tok.getLocation());
+ Tok.getLocation());
   }
 
   // Figure out how many tokens we are into LineToks.
Index: lib/Sema/SemaStmtAsm.cpp
===
--- lib/Sema/SemaStmtAsm.cpp
+++ lib/Sema/SemaStmtAsm.cpp
@@ -48,10 +48,10 @@
   if (E != E2 && E2->isLValue()) {
 if (!S.getLangOpts().HeinousExtensions)
   S.Diag(E2->getLocStart(), diag::err_invalid_asm_cast_lvalue)
-<< E->getSourceRange();
+  << E->getSourceRange();
 else
   S.Diag(E2->getLocStart(), diag::warn_invalid_asm_cast_lvalue)
-<< E->getSourceRange();
+  << E->getSourceRange();
 // Accept, even if we emitted an error diagnostic.
 return false;
   }
@@ -62,11 +62,13 @@
 
 /// isOperandMentioned - Return true if the specified operand # is mentioned
 /// anywhere in the decomposed asm string.
-static bool isOperandMentioned(unsigned OpNo,
- ArrayRef AsmStrPieces) {
+static bool
+isOperandMentioned(unsigned OpNo,
+   ArrayRef AsmStrPieces) {
   for (unsigned p = 0, e = AsmStrPieces.size(); p != e; ++p) {
 const GCCAsmStmt::AsmStringPiece  = AsmStrPieces[p];
-if (!Piece.isOperand()) continue;
+if (!Piece.isOperand())
+  continue;
 
 // If this is a reference to the input and if the input was the smaller
 // one, then we have to reject this asm.
@@ -605,23 +607,32 @@
   return NS;
 }
 
-static void fillInlineAsmTypeInfo(const ASTContext , QualType T,
-  llvm::InlineAsmIdentifierInfo ) {
-  // Compute the type size (and array length if applicable?).
-  Info.Type = Info.Size = Context.getTypeSizeInChars(T).getQuantity();

[PATCH] D37101: [clangd] [WIP] Add support for snippet completions

2017-09-02 Thread Raoul Wols via Phabricator via cfe-commits 
rwols updated this revision to Diff 113657.
rwols added a comment.

- Adjust SnippetCompletionItemCollector such that it only sets insertTextFormat 
to InsertTextFormat::Snippet when it's actually needed (i.e. we encounter a 
CK_Placeholder chunk).
- Fix failing tests in test/clangd/{completion.test,authority-less-uri.test} 
because of the new changes.
- Add new tests in test/clangd/completion-snippet.test that tests the snippet 
functionality.


https://reviews.llvm.org/D37101

Files:
  clangd/ClangdLSPServer.cpp
  clangd/ClangdLSPServer.h
  clangd/ClangdServer.cpp
  clangd/ClangdServer.h
  clangd/ClangdUnit.cpp
  clangd/ClangdUnit.h
  clangd/Protocol.cpp
  clangd/Protocol.h
  clangd/tool/ClangdMain.cpp
  test/clangd/authority-less-uri.test
  test/clangd/completion-snippet.test
  test/clangd/completion.test

Index: test/clangd/completion.test
===
--- test/clangd/completion.test
+++ test/clangd/completion.test
@@ -16,25 +16,25 @@
 # nondeterministic, so we check regardless of order.
 #
 # CHECK: {"jsonrpc":"2.0","id":1,"result":[
-# CHECK-DAG: {"label":"a","kind":5,"detail":"int","sortText":"00035a","filterText":"a","insertText":"a"}
-# CHECK-DAG: {"label":"bb","kind":5,"detail":"int","sortText":"00035bb","filterText":"bb","insertText":"bb"}
-# CHECK-DAG: {"label":"ccc","kind":5,"detail":"int","sortText":"00035ccc","filterText":"ccc","insertText":"ccc"}
-# CHECK-DAG: {"label":"operator=(const fake &)","kind":2,"detail":"fake &","sortText":"00034operator=","filterText":"operator=","insertText":"operator="}
-# CHECK-DAG: {"label":"~fake()","kind":4,"detail":"void","sortText":"00034~fake","filterText":"~fake","insertText":"~fake"}
-# CHECK-DAG: {"label":"f(int i, const float f) const","kind":2,"detail":"int","sortText":"00035f","filterText":"f","insertText":"f"}
+# CHECK-DAG: {"label":"a","kind":5,"detail":"int","sortText":"00035a","filterText":"a","insertText":"a","insertTextFormat":1}
+# CHECK-DAG: {"label":"bb","kind":5,"detail":"int","sortText":"00035bb","filterText":"bb","insertText":"bb","insertTextFormat":1}
+# CHECK-DAG: {"label":"ccc","kind":5,"detail":"int","sortText":"00035ccc","filterText":"ccc","insertText":"ccc","insertTextFormat":1}
+# CHECK-DAG: {"label":"operator=(const fake &)","kind":2,"detail":"fake &","sortText":"00034operator=","filterText":"operator=","insertText":"operator=","insertTextFormat":1}
+# CHECK-DAG: {"label":"~fake()","kind":4,"detail":"void","sortText":"00034~fake","filterText":"~fake","insertText":"~fake","insertTextFormat":1}
+# CHECK-DAG: {"label":"f(int i, const float f) const","kind":2,"detail":"int","sortText":"00035f","filterText":"f","insertText":"f","insertTextFormat":1}
 # CHECK: ]}
 Content-Length: 148
 
 {"jsonrpc":"2.0","id":2,"method":"textDocument/completion","params":{"textDocument":{"uri":"file:///main.cpp"},"position":{"line":3,"character":5}}}
 # Repeat the completion request, expect the same results.
 #
 # CHECK: {"jsonrpc":"2.0","id":2,"result":[
-# CHECK-DAG: {"label":"a","kind":5,"detail":"int","sortText":"00035a","filterText":"a","insertText":"a"}
-# CHECK-DAG: {"label":"bb","kind":5,"detail":"int","sortText":"00035bb","filterText":"bb","insertText":"bb"}
-# CHECK-DAG: {"label":"ccc","kind":5,"detail":"int","sortText":"00035ccc","filterText":"ccc","insertText":"ccc"}
-# CHECK-DAG: {"label":"operator=(const fake &)","kind":2,"detail":"fake &","sortText":"00034operator=","filterText":"operator=","insertText":"operator="}
-# CHECK-DAG: {"label":"~fake()","kind":4,"detail":"void","sortText":"00034~fake","filterText":"~fake","insertText":"~fake"}
-# CHECK-DAG: {"label":"f(int i, const float f) const","kind":2,"detail":"int","sortText":"00035f","filterText":"f","insertText":"f"}
+# CHECK-DAG: {"label":"a","kind":5,"detail":"int","sortText":"00035a","filterText":"a","insertText":"a","insertTextFormat":1}
+# CHECK-DAG: {"label":"bb","kind":5,"detail":"int","sortText":"00035bb","filterText":"bb","insertText":"bb","insertTextFormat":1}
+# CHECK-DAG: {"label":"ccc","kind":5,"detail":"int","sortText":"00035ccc","filterText":"ccc","insertText":"ccc","insertTextFormat":1}
+# CHECK-DAG: {"label":"operator=(const fake &)","kind":2,"detail":"fake &","sortText":"00034operator=","filterText":"operator=","insertText":"operator=","insertTextFormat":1}
+# CHECK-DAG: {"label":"~fake()","kind":4,"detail":"void","sortText":"00034~fake","filterText":"~fake","insertText":"~fake","insertTextFormat":1}
+# CHECK-DAG: {"label":"f(int i, const float f) const","kind":2,"detail":"int","sortText":"00035f","filterText":"f","insertText":"f","insertTextFormat":1}
 # CHECK: ]}
 # Update the source file and check for completions again.
 Content-Length: 226
@@ -47,7 +47,7 @@
 # Repeat the completion request, expect the same results.
 #
 # CHECK: {"jsonrpc":"2.0","id":3,"result":[
-# CHECK-DAG: {"label":"func()","kind":2,"detail":"int (*)(int, int)","sortText":"00034func","filterText":"func","insertText":"func"}
+# CHECK-DAG:

[PATCH] D37182: [libcxx] Special visibility macros for the experimental library

2017-09-02 Thread Hamza Sood via Phabricator via cfe-commits 
hamzasood added a comment.

Thanks. Could someone commit this for me please?




Comment at: include/experimental/__config:57
 
+#if defined(_LIBCPP_OBJECT_FORMAT_COFF)
+# define _LIBCPPX_TYPE_VIS

compnerd wrote:
> hamzasood wrote:
> > smeenai wrote:
> > > I think it would make more sense to make these macros empty on all 
> > > platforms, not just Windows. It's true that they'll only cause link 
> > > errors on Windows (since you'll attempt to dllimport functions from a 
> > > static library), but on ELF and Mach-O, the visibility annotations would 
> > > cause these functions to be exported from whatever library 
> > > c++experimental gets linked into, which is probably not desirable either.
> > > 
> > > The exception (if you'll pardon the pun) is `_LIBCPPX_EXCEPTION_ABI`, 
> > > which will still need to expand to at least 
> > > `__type_visibility__((default))` for non-COFF in order for throwing and 
> > > catching those types to work correctly.
> > I might be mistaken, but I think the regular libc++ library still uses 
> > visibility annotations when it's being built as a static library on 
> > non-Windows platforms. So I figured it'd be best to match that behaviour.
> It's not about matching that behaviour, libc++ is built shared, this is built 
> static.  The static link marked default visibility would make the functions 
> be re-exported.  However, if I'm not mistaken, this already happens today, so 
> fixing that in a follow-up is fine as the status quo is maintained.
But isn't it possible to build libc++ as static instead of shared? 
(LIBCXX_ENABLE_STATIC=YES and LIBCXX_ENABLE_SHARED=NO).
I'm pretty sure that in that case, the visibility annotations are left active.


https://reviews.llvm.org/D37182



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33826: [clang-tidy] avoid pointer cast to more strict alignment check

2017-09-02 Thread Eniko Donatella Toth via Phabricator via cfe-commits 
NorenaLeonetti updated this revision to Diff 113654.
NorenaLeonetti added a comment.
Herald added a subscriber: JDevlieghere.

Added a test for C++ style cast and modified the docs.


Repository:
  rL LLVM

https://reviews.llvm.org/D33826

Files:
  clang-tidy/cert/AvoidPointerCastToMoreStrictAlignmentCheck.cpp
  clang-tidy/cert/AvoidPointerCastToMoreStrictAlignmentCheck.h
  clang-tidy/cert/CERTTidyModule.cpp
  clang-tidy/cert/CMakeLists.txt
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/cert-exp36-c.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/cert-avoid-pointer-cast-to-more-strict-alignment.c
  test/clang-tidy/cert-avoid-pointer-cast-to-more-strict-alignment.cpp

Index: test/clang-tidy/cert-avoid-pointer-cast-to-more-strict-alignment.cpp
===
--- /dev/null
+++ test/clang-tidy/cert-avoid-pointer-cast-to-more-strict-alignment.cpp
@@ -0,0 +1,7 @@
+// RUN: %check_clang_tidy %s cert-exp36-c %t
+
+void function(void) {
+  char c = 'x';
+  int *ip = reinterpret_cast();
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: do not cast pointers into more strictly aligned pointer types [cert-exp36-c] 
+}
Index: test/clang-tidy/cert-avoid-pointer-cast-to-more-strict-alignment.c
===
--- /dev/null
+++ test/clang-tidy/cert-avoid-pointer-cast-to-more-strict-alignment.c
@@ -0,0 +1,45 @@
+// RUN: %check_clang_tidy %s cert-exp36-c %t -- -- -std=c11
+
+void function(void) {
+  char c = 'x';
+  int *ip = (int *)
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: do not cast pointers into more strictly aligned pointer types [cert-exp36-c] 
+}
+
+struct foo_header {
+  int len;
+};
+ 
+void function2(char *data, unsigned offset) {
+  struct foo_header *tmp;
+  struct foo_header header;
+ 
+  tmp = (struct foo_header *)(data + offset);
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: do not cast pointers into more strictly aligned pointer types [cert-exp36-c] 
+}
+
+
+// Something that does not trigger the check:
+
+struct w;
+
+void function3(struct w *v) {
+  int *ip = (int *)v;
+  struct w *u = (struct w *)ip;
+}
+
+struct x {
+   _Alignas(int) char c;
+};
+
+void function4(void) {
+  struct x c = {'x'};
+  int *ip = (int *)
+}
+
+// FIXME: we do not want a warning for this
+void function5(void) {
+  _Alignas(int) char c = 'x';
+  int *ip = (int *)
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: do not cast pointers into more strictly aligned pointer types [cert-exp36-c]
+}
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -20,6 +20,7 @@
bugprone-integer-division
bugprone-suspicious-memset-usage
bugprone-undefined-memory-manipulation
+   cert-exp36-c
cert-dcl03-c (redirects to misc-static-assert) 
cert-dcl21-cpp
cert-dcl50-cpp
Index: docs/clang-tidy/checks/cert-exp36-c.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/cert-exp36-c.rst
@@ -0,0 +1,45 @@
+.. title:: clang-tidy - cert-exp36-c
+
+cert-exp36-c
+
+
+This check will give a warning if a pointer value is converted to
+a pointer type that is more strictly aligned than the referenced type.
+ 
+This check is the same as `-Wcast-align`, except it checks for `reinterpret_cast` as well.
+
+ Here's an example:
+ 
+ .. code-block:: c
+ 
+char c = 'x';
+int *ip = (int *)
+// warning: do not cast pointers into more strictly aligned pointer types
+ 
+ This check does not completely include warnings for types with explicitly
+ specified alignment, this remains a possible future extension.
+
+ See the example:
+
+  .. code-block:: c
+
+// Works fine:
+struct x {
+  _Alignas(int) char c;
+};
+
+void function3(void) {
+  struct x c = {'x'};
+  int *ip = (int *)
+}
+
+// Won't work:
+void function4(void) {
+  _Alignas(int) char c = 'x';
+  int *ip = (int *)
+  // the check will give a warning for this
+}
+
+ This check corresponds to the CERT C Coding Standard rule
+ `EXP36-C. Do not cast pointers into more strictly aligned pointer types
+`_.
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -56,6 +56,10 @@
 
 Improvements to clang-tidy
 --
+- New `cert-exp36-c
+  `_ check
+
+  Checks if a pointer value is casted to a more stricter alignment.
 
 - Renamed checks to use correct term "implicit conversion" instead of "implicit
   cast" and modified messages and option names accordingly:
@@ -143,7 +147,7 @@
   option.
 
 - Added alias

[PATCH] D37182: [libcxx] Special visibility macros for the experimental library

2017-09-02 Thread Saleem Abdulrasool via Phabricator via cfe-commits 
compnerd accepted this revision.
compnerd added inline comments.
This revision is now accepted and ready to land.



Comment at: include/experimental/__config:57
 
+#if defined(_LIBCPP_OBJECT_FORMAT_COFF)
+# define _LIBCPPX_TYPE_VIS

hamzasood wrote:
> smeenai wrote:
> > I think it would make more sense to make these macros empty on all 
> > platforms, not just Windows. It's true that they'll only cause link errors 
> > on Windows (since you'll attempt to dllimport functions from a static 
> > library), but on ELF and Mach-O, the visibility annotations would cause 
> > these functions to be exported from whatever library c++experimental gets 
> > linked into, which is probably not desirable either.
> > 
> > The exception (if you'll pardon the pun) is `_LIBCPPX_EXCEPTION_ABI`, which 
> > will still need to expand to at least `__type_visibility__((default))` for 
> > non-COFF in order for throwing and catching those types to work correctly.
> I might be mistaken, but I think the regular libc++ library still uses 
> visibility annotations when it's being built as a static library on 
> non-Windows platforms. So I figured it'd be best to match that behaviour.
It's not about matching that behaviour, libc++ is built shared, this is built 
static.  The static link marked default visibility would make the functions be 
re-exported.  However, if I'm not mistaken, this already happens today, so 
fixing that in a follow-up is fine as the status quo is maintained.


https://reviews.llvm.org/D37182



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37282: clangd: Tolerate additional headers

2017-09-02 Thread Ben Jackson via Phabricator via cfe-commits 
puremourning added a comment.

So I think this is ready now. Anything more you need from me?




Comment at: clangd/JSONRPCDispatcher.cpp:138
bool ) {
+  unsigned long long ContentLength = 0;
   while (In.good()) {

puremourning wrote:
> ilya-biryukov wrote:
> > Maybe we could move that variable into loop body by splitting the loop body 
> > into multiple parts?
> > Having it outside a loop makes it harder to comprehend the parsing code.
> > Rewriting to something like this would arguably make it easier to read:
> > ```
> > while (In.good()) {
> >   // Read first line
> >   //
> >   llvm::StringRef LineRef = /*...*/;
> >   // Skip comments
> >   if (LineRef.startsWith("#"))
> > continue;
> >   
> >   // Read HTTP headers here, reading multiple lines right inside the loop.
> >   unsigned ContentLength = 0;
> >   // 
> > 
> > 
> >   // Read ContentLength bytes of a message here.
> >   std::vector JSON;
> >   //
> > }
> > ```
> I'll take a look, sure. Something like...
> 
> ```
> while ( the input stream is good )
> {
>   set len to 0
>   while ( the input stream is good )
>   {
> read a line into header
> if the line is a comment, read another line (continue the inner loop !)
> 
> if header is Content-Length, store the length in len
> otherwise, if header is empty, we're no longer reading headers, break out 
> of the inner loop
>   }
> 
>   if the input stream is not good, break out of the loop
> 
>   if len is 0, reset the outer loop
> 
>   read len bytes from the stream, 
>   if we read len bytes ok, parse JSON and dispatch to the handlers
> }
> ```
done.



Comment at: clangd/JSONRPCDispatcher.cpp:163
+if (LineRef.consume_front("Content-Length: ")) {
+  llvm::getAsUnsignedInteger(LineRef.trim(), 0, ContentLength);
+  continue;

ilya-biryukov wrote:
> Maybe log an error if `Content-Length` is specified twice?
OK, though I suspect this would only happen in the tests :)



Comment at: clangd/JSONRPCDispatcher.cpp:167
+  // It's another header, ignore it.
   continue;
+} else {

ilya-biryukov wrote:
> Maybe log the skipped header?
I think this would just be noise for conforming clients which send the 
(legitimate) Content-Type header. We could of course special case this, but I'm 
not sure the extra logging would be all that useful.


https://reviews.llvm.org/D37282



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33825: [clang-tidy] signal handler must be plain old function check

2017-09-02 Thread Eniko Donatella Toth via Phabricator via cfe-commits 
NorenaLeonetti updated this revision to Diff 113652.

Repository:
  rL LLVM

https://reviews.llvm.org/D33825

Files:
  clang-tidy/cert/CERTTidyModule.cpp
  clang-tidy/cert/CMakeLists.txt
  clang-tidy/cert/SignalHandlerMustBePlainOldFunctionCheck.cpp
  clang-tidy/cert/SignalHandlerMustBePlainOldFunctionCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/cert-msc54-cpp.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/cert-signal-handler-must-be-plain-old-function.cpp

Index: test/clang-tidy/cert-signal-handler-must-be-plain-old-function.cpp
===
--- /dev/null
+++ test/clang-tidy/cert-signal-handler-must-be-plain-old-function.cpp
@@ -0,0 +1,134 @@
+// RUN: %check_clang_tidy %s cert-msc54-cpp %t -- -- -std=c++11
+
+namespace std {
+extern "C" using signalhandler = void(int);
+int signal(int sig, signalhandler *func);
+}
+
+#define SIG_ERR -1
+#define SIGTERM 15
+
+static void sig_handler(int sig) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: signal handlers must be 'extern "C"' [cert-msc54-cpp]
+// CHECK-MESSAGES: :[[@LINE+3]]:39: note: given as a signal parameter here
+
+void install_signal_handler() {
+  if (SIG_ERR == std::signal(SIGTERM, sig_handler))
+return;
+}
+
+extern "C" void cpp_signal_handler(int sig) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: do not use C++ constructs in signal handlers [cert-msc54-cpp]
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: note: C++ construct used here
+  throw "error message";
+}
+
+void install_cpp_signal_handler() {
+  if (SIG_ERR == std::signal(SIGTERM, cpp_signal_handler))
+return;
+}
+
+void indirect_recursion();
+
+void cpp_like(){
+  try {
+cpp_signal_handler(SIG_ERR);
+  } catch(...) {
+// handle error
+  }
+}
+
+void no_body();
+
+void recursive_function() {
+  indirect_recursion();
+  cpp_like();
+  no_body();
+  recursive_function();
+}
+
+void indirect_recursion() {
+  recursive_function();
+}
+
+extern "C" void signal_handler_with_cpp_function_call(int sig) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: do not call functions with C++ constructs in signal handlers [cert-msc54-cpp]
+  // CHECK-MESSAGES: :[[@LINE-11]]:3: note: function called here
+  // CHECK-MESSAGES: :[[@LINE-23]]:3: note: C++ construct used here
+  recursive_function();
+}
+
+void install_signal_handler_with_cpp_function_call() {
+  if (SIG_ERR == std::signal(SIGTERM, signal_handler_with_cpp_function_call))
+return;
+}
+
+class TestClass {
+public:
+  static void static_function() {}
+};
+
+extern "C" void signal_handler_with_cpp_static_method_call(int sig) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: do not call functions with C++ constructs in signal handlers [cert-msc54-cpp]
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: note: function called here
+  TestClass::static_function();
+}
+
+void install_signal_handler_with_cpp_static_method_call() {
+  if (SIG_ERR == std::signal(SIGTERM, signal_handler_with_cpp_static_method_call))
+return;
+}
+
+
+// Something that does not trigger the check:
+
+extern "C" void c_sig_handler(int sig) {
+#define cbrt(X) _Generic((X), long double \
+ : cbrtl, default \
+ : cbrt)(X)
+  auto char c = '1';
+  extern _Thread_local double _Complex d;
+  static const unsigned long int i = sizeof(float);
+  register short s;
+  void f();
+  volatile struct c_struct {
+enum e {};
+union u;
+  };
+  typedef bool boolean;
+label:
+  switch (c) {
+  case ('1'):
+break;
+  default:
+d = 1.2;
+  }
+  goto label;
+  for (; i < 42;) {
+if (d == 1.2)
+  continue;
+else
+  return;
+  }
+  do {
+_Atomic int v = _Alignof(char);
+_Static_assert(42 == 42, "True");
+  } while (c == 42);
+}
+
+void install_c_sig_handler() {
+  if (SIG_ERR == std::signal(SIGTERM, c_sig_handler)) {
+// Handle error
+  }
+}
+
+extern "C" void signal_handler_with_function_pointer(int sig) {
+  void (*funp) (void);
+  funp = _like;
+  funp();
+}
+
+void install_signal_handler_with_function_pointer() {
+  if (SIG_ERR == std::signal(SIGTERM, signal_handler_with_function_pointer))
+return;
+}
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -37,6 +37,7 @@
cert-flp30-c
cert-msc30-c (redirects to cert-msc50-cpp) 
cert-msc50-cpp
+   cert-msc54-cpp
cert-oop11-cpp (redirects to misc-move-constructor-init) 
cppcoreguidelines-c-copy-assignment-signature
cppcoreguidelines-interfaces-global-init
Index: docs/clang-tidy/checks/cert-msc54-cpp.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/cert-msc54-cpp.rst
@@ -0,0 +1,34 @@
+.. title:: clang-tidy - cert-msc54-cpp
+
+cert-msc54-cpp
+==
+
+This check will give a warning if a signal handler is not defined
+as an `extern "C"` function or if the

[PATCH] D33825: [clang-tidy] signal handler must be plain old function check

2017-09-02 Thread Johann Klähn via Phabricator via cfe-commits 
jklaehn added inline comments.



Comment at: docs/ReleaseNotes.rst:62
 
+<<< 2f301f50187ede4b9b8c7456ac4a67b9f7418004
 - Renamed checks to use correct term "implicit conversion" instead of "implicit

You missed a conflict marker here (and below in line 149).


Repository:
  rL LLVM

https://reviews.llvm.org/D33825



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33825: [clang-tidy] signal handler must be plain old function check

2017-09-02 Thread Eniko Donatella Toth via Phabricator via cfe-commits 
NorenaLeonetti updated this revision to Diff 113651.
Herald added a subscriber: JDevlieghere.

Repository:
  rL LLVM

https://reviews.llvm.org/D33825

Files:
  clang-tidy/cert/CERTTidyModule.cpp
  clang-tidy/cert/CMakeLists.txt
  clang-tidy/cert/SignalHandlerMustBePlainOldFunctionCheck.cpp
  clang-tidy/cert/SignalHandlerMustBePlainOldFunctionCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/cert-msc54-cpp.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/cert-signal-handler-must-be-plain-old-function.cpp

Index: test/clang-tidy/cert-signal-handler-must-be-plain-old-function.cpp
===
--- /dev/null
+++ test/clang-tidy/cert-signal-handler-must-be-plain-old-function.cpp
@@ -0,0 +1,134 @@
+// RUN: %check_clang_tidy %s cert-msc54-cpp %t -- -- -std=c++11
+
+namespace std {
+extern "C" using signalhandler = void(int);
+int signal(int sig, signalhandler *func);
+}
+
+#define SIG_ERR -1
+#define SIGTERM 15
+
+static void sig_handler(int sig) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: signal handlers must be 'extern "C"' [cert-msc54-cpp]
+// CHECK-MESSAGES: :[[@LINE+3]]:39: note: given as a signal parameter here
+
+void install_signal_handler() {
+  if (SIG_ERR == std::signal(SIGTERM, sig_handler))
+return;
+}
+
+extern "C" void cpp_signal_handler(int sig) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: do not use C++ constructs in signal handlers [cert-msc54-cpp]
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: note: C++ construct used here
+  throw "error message";
+}
+
+void install_cpp_signal_handler() {
+  if (SIG_ERR == std::signal(SIGTERM, cpp_signal_handler))
+return;
+}
+
+void indirect_recursion();
+
+void cpp_like(){
+  try {
+cpp_signal_handler(SIG_ERR);
+  } catch(...) {
+// handle error
+  }
+}
+
+void no_body();
+
+void recursive_function() {
+  indirect_recursion();
+  cpp_like();
+  no_body();
+  recursive_function();
+}
+
+void indirect_recursion() {
+  recursive_function();
+}
+
+extern "C" void signal_handler_with_cpp_function_call(int sig) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: do not call functions with C++ constructs in signal handlers [cert-msc54-cpp]
+  // CHECK-MESSAGES: :[[@LINE-11]]:3: note: function called here
+  // CHECK-MESSAGES: :[[@LINE-23]]:3: note: C++ construct used here
+  recursive_function();
+}
+
+void install_signal_handler_with_cpp_function_call() {
+  if (SIG_ERR == std::signal(SIGTERM, signal_handler_with_cpp_function_call))
+return;
+}
+
+class TestClass {
+public:
+  static void static_function() {}
+};
+
+extern "C" void signal_handler_with_cpp_static_method_call(int sig) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: do not call functions with C++ constructs in signal handlers [cert-msc54-cpp]
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: note: function called here
+  TestClass::static_function();
+}
+
+void install_signal_handler_with_cpp_static_method_call() {
+  if (SIG_ERR == std::signal(SIGTERM, signal_handler_with_cpp_static_method_call))
+return;
+}
+
+
+// Something that does not trigger the check:
+
+extern "C" void c_sig_handler(int sig) {
+#define cbrt(X) _Generic((X), long double \
+ : cbrtl, default \
+ : cbrt)(X)
+  auto char c = '1';
+  extern _Thread_local double _Complex d;
+  static const unsigned long int i = sizeof(float);
+  register short s;
+  void f();
+  volatile struct c_struct {
+enum e {};
+union u;
+  };
+  typedef bool boolean;
+label:
+  switch (c) {
+  case ('1'):
+break;
+  default:
+d = 1.2;
+  }
+  goto label;
+  for (; i < 42;) {
+if (d == 1.2)
+  continue;
+else
+  return;
+  }
+  do {
+_Atomic int v = _Alignof(char);
+_Static_assert(42 == 42, "True");
+  } while (c == 42);
+}
+
+void install_c_sig_handler() {
+  if (SIG_ERR == std::signal(SIGTERM, c_sig_handler)) {
+// Handle error
+  }
+}
+
+extern "C" void signal_handler_with_function_pointer(int sig) {
+  void (*funp) (void);
+  funp = _like;
+  funp();
+}
+
+void install_signal_handler_with_function_pointer() {
+  if (SIG_ERR == std::signal(SIGTERM, signal_handler_with_function_pointer))
+return;
+}
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -37,6 +37,7 @@
cert-flp30-c
cert-msc30-c (redirects to cert-msc50-cpp) 
cert-msc50-cpp
+   cert-msc54-cpp
cert-oop11-cpp (redirects to misc-move-constructor-init) 
cppcoreguidelines-c-copy-assignment-signature
cppcoreguidelines-interfaces-global-init
Index: docs/clang-tidy/checks/cert-msc54-cpp.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/cert-msc54-cpp.rst
@@ -0,0 +1,34 @@
+.. title:: clang-tidy - cert-msc54-cpp
+
+cert-msc54-cpp
+==
+
+This check will give a warning if a signal handler is not defined
+as

[PATCH] D33825: [clang-tidy] signal handler must be plain old function check

2017-09-02 Thread Eniko Donatella Toth via Phabricator via cfe-commits 
NorenaLeonetti marked 16 inline comments as done.
NorenaLeonetti added a comment.

In https://reviews.llvm.org/D33825#772688, @aaron.ballman wrote:

> This check is an interesting one. The rules around what is signal safe are 
> changing for C++17 to be a bit more lenient than what the rules are for 
> C++14. CERT's rule is written against C++14, and so the current behavior 
> matches the rule wording. However, the *intent* of the rule is to ensure that 
> only signal-safe functionality is used from a signal handler, and so from 
> that perspective, I can imagine a user compiling for C++17 to want the 
> relaxed rules to still comply with CERT's wording. What do you think?


I think your concerns are right. I've checked the upcoming changes in the C++17 
standard draft and I'll add some logic to the code to have those relaxed rules 
at least partly fulfilled.
For now, I uploaded the changes required for this check.


Repository:
  rL LLVM

https://reviews.llvm.org/D33825



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37101: [clangd] [WIP] Add support for snippet completions

2017-09-02 Thread Raoul Wols via Phabricator via cfe-commits 
rwols marked 4 inline comments as done.
rwols added inline comments.



Comment at: clangd/ClangdUnit.cpp:419
+assert(item.detail.empty() && "Unexpected extraneous CK_ResultType");
+Item.detail = Chunk.Text;
+break;

ilya-biryukov wrote:
> Won't that assertion fail with function return types? Let's add a test for 
> that.
> 
> ```
> int (*foo(int a))(float);
> ```
I'll make sure to add a test case for this!


https://reviews.llvm.org/D37101



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37101: [clangd] [WIP] Add support for snippet completions

2017-09-02 Thread Raoul Wols via Phabricator via cfe-commits 
rwols added a comment.

Forgot to mention I've also made sure the `filterText` field is taken care of 
now (both for snippet and for plaintext items).


https://reviews.llvm.org/D37101



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37101: [clangd] [WIP] Add support for snippet completions

2017-09-02 Thread Raoul Wols via Phabricator via cfe-commits 
rwols updated this revision to Diff 113650.
rwols marked 3 inline comments as done.
rwols added a comment.

- Split up the CompletionItemsCollector into two classes called 
PlainTextCompletionItemsCollector and SnippetCompletionItemsCollector to allow 
collecting both types of items.
- Add a command-line setting to clangd called "--enable-snippets" that, when 
set, will make clangd use the SnippetCompletionItemsCollector. The default is 
to use the PlainTextCompletionItemsCollector.
- Enable "code pattern" completions when "--enable-snippets" is true.
- Do not add a space in the completion label when we encounter a 
CK_VerticalSpace in the SnippetCompletionItemsCollector. A space looks weird.

I will add tests now.


https://reviews.llvm.org/D37101

Files:
  clangd/ClangdLSPServer.cpp
  clangd/ClangdLSPServer.h
  clangd/ClangdServer.cpp
  clangd/ClangdServer.h
  clangd/ClangdUnit.cpp
  clangd/ClangdUnit.h
  clangd/Protocol.cpp
  clangd/Protocol.h
  clangd/tool/ClangdMain.cpp

Index: clangd/tool/ClangdMain.cpp
===
--- clangd/tool/ClangdMain.cpp
+++ clangd/tool/ClangdMain.cpp
@@ -26,6 +26,13 @@
llvm::cl::desc("Number of async workers used by clangd"),
llvm::cl::init(getDefaultAsyncThreadsCount()));
 
+// FIXME: Make snippets the default once the client landscape has adjusted.
+static llvm::cl::opt EnableSnippets(
+"enable-snippets",
+llvm::cl::desc("Present completions with embedded snippets instead of "
+   "plaintext completions"),
+llvm::cl::init(false));
+
 static llvm::cl::opt RunSynchronously(
 "run-synchronously",
 llvm::cl::desc("Parse on main thread. If set, -j is ignored"),
@@ -61,6 +68,7 @@
   if (!ResourceDir.empty())
 ResourceDirRef = ResourceDir;
 
-  ClangdLSPServer LSPServer(Out, WorkerThreadsCount, ResourceDirRef);
+  ClangdLSPServer LSPServer(Out, WorkerThreadsCount, EnableSnippets,
+ResourceDirRef);
   LSPServer.run(std::cin);
 }
Index: clangd/Protocol.h
===
--- clangd/Protocol.h
+++ clangd/Protocol.h
@@ -389,6 +389,9 @@
   /// themselves.
   std::vector additionalTextEdits;
 
+  CompletionItem() = default;
+  CompletionItem(const InsertTextFormat Format);
+
   // TODO(krasimir): The following optional fields defined by the language
   // server protocol are unsupported:
   //
Index: clangd/Protocol.cpp
===
--- clangd/Protocol.cpp
+++ clangd/Protocol.cpp
@@ -705,6 +705,9 @@
   return Result;
 }
 
+CompletionItem::CompletionItem(const InsertTextFormat Format)
+: insertTextFormat(Format) {}
+
 std::string CompletionItem::unparse(const CompletionItem ) {
   std::string Result = "{";
   llvm::raw_string_ostream Os(Result);
@@ -724,8 +727,8 @@
   if (!CI.insertText.empty())
 Os << R"("insertText":")" << llvm::yaml::escape(CI.insertText) << R"(",)";
   if (CI.insertTextFormat != InsertTextFormat::Missing) {
-Os << R"("insertTextFormat":")" << static_cast(CI.insertTextFormat)
-   << R"(",)";
+Os << R"("insertTextFormat":)" << static_cast(CI.insertTextFormat)
+   << R"(,)";
   }
   if (CI.textEdit)
 Os << R"("textEdit":)" << TextEdit::unparse(*CI.textEdit) << ',';
Index: clangd/ClangdUnit.h
===
--- clangd/ClangdUnit.h
+++ clangd/ClangdUnit.h
@@ -253,7 +253,8 @@
 codeComplete(PathRef FileName, tooling::CompileCommand Command,
  PrecompiledPreamble const *Preamble, StringRef Contents,
  Position Pos, IntrusiveRefCntPtr VFS,
- std::shared_ptr PCHs);
+ std::shared_ptr PCHs,
+ const bool SnippetCompletions);
 
 /// Get definition of symbol at a specified \p Pos.
 std::vector findDefinitions(ParsedAST , Position Pos);
Index: clangd/ClangdUnit.cpp
===
--- clangd/ClangdUnit.cpp
+++ clangd/ClangdUnit.cpp
@@ -272,68 +272,277 @@
   }
 }
 
+std::string SnippetEscape(const llvm::StringRef Text) {
+  std::string Result;
+  Result.reserve(Text.size()); // Assume '$', '}' and '\\' are rare.
+  for (const auto Character : Text) {
+if (Character == '$' || Character == '}' || Character == '\\')
+  Result.push_back('\\');
+Result.push_back(Character);
+  }
+  return Result;
+}
+
 class CompletionItemsCollector : public CodeCompleteConsumer {
-  std::vector *Items;
-  std::shared_ptr Allocator;
-  CodeCompletionTUInfo CCTUInfo;
 
 public:
-  CompletionItemsCollector(std::vector *Items,
-   const CodeCompleteOptions )
+  CompletionItemsCollector(const CodeCompleteOptions ,
+   std::vector )
   : CodeCompleteConsumer(CodeCompleteOpts, /*OutputIsBinary=*/false),
 Items(Items),
 Allocator(std::make_shared()),