[PATCH] D40381: Parse concept definition

[Saar Raz via Phabricator via cfe-commits]

saar.raz added a comment.

@rsmith?


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D40381/new/

https://reviews.llvm.org/D40381



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

r360559 - [c++20] P1064R0: Allow virtual function calls in constant expression

[Richard Smith via cfe-commits]

Author: rsmith
Date: Mon May 13 00:42:10 2019
New Revision: 360559

URL: http://llvm.org/viewvc/llvm-project?rev=360559&view=rev
Log:
[c++20] P1064R0: Allow virtual function calls in constant expression
evaluation.

Modified:
cfe/trunk/include/clang/AST/DeclCXX.h
cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/AST/DeclCXX.cpp
cfe/trunk/lib/AST/ExprConstant.cpp
cfe/trunk/lib/Sema/SemaDeclCXX.cpp
cfe/trunk/lib/Sema/SemaTemplateInstantiate.cpp
cfe/trunk/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp
cfe/trunk/test/CXX/drs/dr18xx.cpp
cfe/trunk/test/CXX/drs/dr6xx.cpp
cfe/trunk/test/SemaCXX/constant-expression-cxx2a.cpp
cfe/trunk/test/SemaCXX/cxx17-compat.cpp
cfe/trunk/www/cxx_status.html

Modified: cfe/trunk/include/clang/AST/DeclCXX.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclCXX.h?rev=360559&r1=360558&r2=360559&view=diff
==
--- cfe/trunk/include/clang/AST/DeclCXX.h (original)
+++ cfe/trunk/include/clang/AST/DeclCXX.h Mon May 13 00:42:10 2019
@@ -2298,6 +2298,17 @@ public:
   ->getCorrespondingMethodInClass(RD, MayBeBase);
   }
 
+  /// Find if \p RD declares a function that overrides this function, and if 
so,
+  /// return it. Does not search base classes.
+  CXXMethodDecl *getCorrespondingMethodDeclaredInClass(const CXXRecordDecl *RD,
+   bool MayBeBase = false);
+  const CXXMethodDecl *
+  getCorrespondingMethodDeclaredInClass(const CXXRecordDecl *RD,
+bool MayBeBase = false) const {
+return const_cast(this)
+->getCorrespondingMethodDeclaredInClass(RD, MayBeBase);
+  }
+
   // Implement isa/cast/dyncast/etc.
   static bool classof(const Decl *D) { return classofKind(D->getKind()); }
   static bool classofKind(Kind K) {

Modified: cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td?rev=360559&r1=360558&r2=360559&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td Mon May 13 00:42:10 2019
@@ -32,6 +32,8 @@ def note_constexpr_no_return : Note<
   "control reached end of constexpr function">;
 def note_constexpr_virtual_call : Note<
   "cannot evaluate call to virtual function in a constant expression">;
+def note_constexpr_pure_virtual_call : Note<
+  "pure virtual function %q0 called">;
 def note_constexpr_virtual_base : Note<
   "cannot construct object of type %0 with virtual base class "
   "in a constant expression">;

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=360559&r1=360558&r2=360559&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Mon May 13 00:42:10 
2019
@@ -2314,6 +2314,9 @@ def err_constexpr_redecl_mismatch : Erro
   "%select{non-constexpr declaration of %0 follows constexpr declaration"
   "|constexpr declaration of %0 follows non-constexpr declaration}1">;
 def err_constexpr_virtual : Error<"virtual function cannot be constexpr">;
+def warn_cxx17_compat_constexpr_virtual : Warning<
+  "virtual constexpr functions are incompatible with "
+  "C++ standards before C++2a">, InGroup, DefaultIgnore;
 def err_constexpr_virtual_base : Error<
   "constexpr %select{member function|constructor}0 not allowed in "
   "%select{struct|interface|class}1 with virtual base "

Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=360559&r1=360558&r2=360559&view=diff
==
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Mon May 13 00:42:10 2019
@@ -5984,8 +5984,8 @@ public:
 
   /// MarkVirtualMembersReferenced - Will mark all members of the given
   /// CXXRecordDecl referenced.
-  void MarkVirtualMembersReferenced(SourceLocation Loc,
-const CXXRecordDecl *RD);
+  void MarkVirtualMembersReferenced(SourceLocation Loc, const CXXRecordDecl 
*RD,
+bool ConstexprOnly = false);
 
   /// Define all of the vtables that have been used in this
   /// translation unit and reference any virtual members used by those

Modified: cfe/trunk/lib/AST/DeclCXX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclCXX.cpp?rev=360559&r1=360558&r2=360559&vi

[PATCH] D60953: [clangd] Respect clang-tidy suppression comments

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Thanks, this looks nice. Main ideas here:

- it'd be nice to get rid of the subclassing in the diag consumer if we can
- i'm not sure the logic around LastErrorWasIgnored is completely accurate
- can we add a unit test for this? The existing clang-tidy diag tests are in 
`unittests/DiagnosticsTests.cpp`




Comment at: clang-tools-extra/clangd/ClangdUnit.cpp:243
+// future).
+class ClangdDiagnosticConsumer : public StoreDiags {
+public:

Thanks, this is much cleaner.

I think we don't need the subclass at all though - just a filter function here, 
and call setFilter after setting up clang-tidy?



Comment at: clang-tools-extra/clangd/ClangdUnit.cpp:338
   Check->registerMatchers(&CTFinder);
 }
   }

I believe you can just call setFilter at the end of this block



Comment at: clang-tools-extra/clangd/Diagnostics.cpp:309
 
   if (!LangOpts || !Info.hasSourceManager()) {
 IgnoreDiagnostics::log(DiagLevel, Info);

do we need to set LastErrorWasIgnored here (in the case that it's not a note)?

I guess it's pretty unlikely that e.g. a problem with the command-line will get 
a note that has a source location...



Comment at: clang-tools-extra/clangd/Diagnostics.cpp:314
 
+  if (LastErrorWasIgnored && DiagLevel == DiagnosticsEngine::Note)
+return;

Can you add a comment here, about notes attached to suppressed errors? This 
code is under-commented (sorry), but I think it would aid understanding.



Comment at: clang-tools-extra/clangd/Diagnostics.cpp:319
 
+  if (Filter && !Filter(DiagLevel, Info, InsideMainFile)) {
+LastErrorWasIgnored = true;

If I read the code right, we're now applying the filter to a note (whose 
primary diagnostic was ignored), which we shouldn't. We're also updating the 
LastErrorWasIgnored flag, which we shouldn't for notes.

I think we might prefer to merge the new logic with the bit below the lambdas, 
which already looks at note/non-note status.

e.g.

```
if (!isNote(...)) {
  flushLastDiag();
  LastErrorWasIgnored = !Filter(...);
  if (LastErrorWasIgnored)
return;
  ...
} else {
  // Handle a note...
  if (LastErrorWasIgnored)
return;
  ...
}



Comment at: clang-tools-extra/clangd/Diagnostics.h:118
+  std::function;
   /// If set, possibly adds fixes for diagnostics using \p Fixer.

nit: IsInsideMainFile is information already inside the diagnostic.

Diagnostics.cpp does compute this information (in an unneccesarily verbose way 
IMO) but I'd prefer not to expose that in this public interface - the filter 
function can recompute.



Comment at: clang-tools-extra/clangd/Diagnostics.h:122
+  /// If set, ignore diagnostics for which \p Filter returns false.
+  void filterDiagnostics(DiagFilter Filter) { this->Filter = Filter; }
 

I know I suggested this name, but just an idea...

The difficulty with `filter` is it's unclear whether `true` means "this passes 
the filter" or "this is filtered out". It may be clearer to invert the sense 
and call this `suppress`. 

Anyway, totally up to you, can definitely live with filter.



Comment at: clang-tools-extra/clangd/Diagnostics.h:132
   llvm::Optional LastDiag;
+  bool LastErrorWasIgnored = false;
 };

I think we're talking about last non-note diagnostic, right? Warnings count 
here I think.

At the risk of being verbose, maybe `LastPrimaryDiagnosticWasIgnored`?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60953/new/

https://reviews.llvm.org/D60953



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

r360560 - PR41854: Don't assert when constant-evaluating a member function call on an invalid designator.

[Richard Smith via cfe-commits]

Author: rsmith
Date: Mon May 13 00:51:29 2019
New Revision: 360560

URL: http://llvm.org/viewvc/llvm-project?rev=360560&view=rev
Log:
PR41854: Don't assert when constant-evaluating a member function call on an 
invalid designator.

Modified:
cfe/trunk/lib/AST/ExprConstant.cpp
cfe/trunk/test/SemaCXX/constant-expression-cxx11.cpp

Modified: cfe/trunk/lib/AST/ExprConstant.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=360560&r1=360559&r2=360560&view=diff
==
--- cfe/trunk/lib/AST/ExprConstant.cpp (original)
+++ cfe/trunk/lib/AST/ExprConstant.cpp Mon May 13 00:51:29 2019
@@ -4537,6 +4537,9 @@ const AccessKinds CheckMemberCallThisPoi
 /// either within its lifetime or in its period of construction or destruction.
 static bool checkMemberCallThisPointer(EvalInfo &Info, const Expr *E,
const LValue &This) {
+  if (This.Designator.Invalid)
+return false;
+
   CompleteObject Obj =
   findCompleteObject(Info, E, AK_MemberCall, This, QualType());
 

Modified: cfe/trunk/test/SemaCXX/constant-expression-cxx11.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/constant-expression-cxx11.cpp?rev=360560&r1=360559&r2=360560&view=diff
==
--- cfe/trunk/test/SemaCXX/constant-expression-cxx11.cpp (original)
+++ cfe/trunk/test/SemaCXX/constant-expression-cxx11.cpp Mon May 13 00:51:29 
2019
@@ -2284,3 +2284,11 @@ namespace PR40430 {
   };
   static_assert(S().foo() == 'f', "");
 }
+
+namespace PR41854 {
+  struct e { operator int(); };
+  struct f { e c; };
+  int a;
+  f &d = reinterpret_cast(a);
+  unsigned b = d.c;
+}


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

r360563 - PR41845: Detect and reject mismatched inner/outer pack expansion sizes

[Richard Smith via cfe-commits]

Author: rsmith
Date: Mon May 13 01:31:14 2019
New Revision: 360563

URL: http://llvm.org/viewvc/llvm-project?rev=360563&view=rev
Log:
PR41845: Detect and reject mismatched inner/outer pack expansion sizes
in fold expressions rather than crashing.

Modified:
cfe/trunk/include/clang/AST/ExprCXX.h
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/Sema/SemaTemplateVariadic.cpp
cfe/trunk/lib/Sema/TreeTransform.h
cfe/trunk/lib/Serialization/ASTReaderStmt.cpp
cfe/trunk/lib/Serialization/ASTWriterStmt.cpp
cfe/trunk/test/SemaTemplate/cxx1z-fold-expressions.cpp

Modified: cfe/trunk/include/clang/AST/ExprCXX.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ExprCXX.h?rev=360563&r1=360562&r2=360563&view=diff
==
--- cfe/trunk/include/clang/AST/ExprCXX.h (original)
+++ cfe/trunk/include/clang/AST/ExprCXX.h Mon May 13 01:31:14 2019
@@ -4436,18 +4436,21 @@ class CXXFoldExpr : public Expr {
   SourceLocation LParenLoc;
   SourceLocation EllipsisLoc;
   SourceLocation RParenLoc;
+  // When 0, the number of expansions is not known. Otherwise, this is one more
+  // than the number of expansions.
+  unsigned NumExpansions;
   Stmt *SubExprs[2];
   BinaryOperatorKind Opcode;
 
 public:
   CXXFoldExpr(QualType T, SourceLocation LParenLoc, Expr *LHS,
   BinaryOperatorKind Opcode, SourceLocation EllipsisLoc, Expr *RHS,
-  SourceLocation RParenLoc)
+  SourceLocation RParenLoc, Optional NumExpansions)
   : Expr(CXXFoldExprClass, T, VK_RValue, OK_Ordinary,
  /*Dependent*/ true, true, true,
  /*ContainsUnexpandedParameterPack*/ false),
 LParenLoc(LParenLoc), EllipsisLoc(EllipsisLoc), RParenLoc(RParenLoc),
-Opcode(Opcode) {
+NumExpansions(NumExpansions ? *NumExpansions + 1 : 0), Opcode(Opcode) {
 SubExprs[0] = LHS;
 SubExprs[1] = RHS;
   }
@@ -4474,6 +4477,12 @@ public:
   SourceLocation getEllipsisLoc() const { return EllipsisLoc; }
   BinaryOperatorKind getOperator() const { return Opcode; }
 
+  Optional getNumExpansions() const {
+if (NumExpansions)
+  return NumExpansions - 1;
+return None;
+  }
+
   SourceLocation getBeginLoc() const LLVM_READONLY { return LParenLoc; }
 
   SourceLocation getEndLoc() const LLVM_READONLY { return RParenLoc; }

Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=360563&r1=360562&r2=360563&view=diff
==
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Mon May 13 01:31:14 2019
@@ -5204,7 +5204,8 @@ public:
   ExprResult BuildCXXFoldExpr(SourceLocation LParenLoc, Expr *LHS,
   BinaryOperatorKind Operator,
   SourceLocation EllipsisLoc, Expr *RHS,
-  SourceLocation RParenLoc);
+  SourceLocation RParenLoc,
+  Optional NumExpansions);
   ExprResult BuildEmptyCXXFoldExpr(SourceLocation EllipsisLoc,
BinaryOperatorKind Operator);
 

Modified: cfe/trunk/lib/Sema/SemaTemplateVariadic.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplateVariadic.cpp?rev=360563&r1=360562&r2=360563&view=diff
==
--- cfe/trunk/lib/Sema/SemaTemplateVariadic.cpp (original)
+++ cfe/trunk/lib/Sema/SemaTemplateVariadic.cpp Mon May 13 01:31:14 2019
@@ -1176,15 +1176,18 @@ ExprResult Sema::ActOnCXXFoldExpr(Source
   }
 
   BinaryOperatorKind Opc = ConvertTokenKindToBinaryOpcode(Operator);
-  return BuildCXXFoldExpr(LParenLoc, LHS, Opc, EllipsisLoc, RHS, RParenLoc);
+  return BuildCXXFoldExpr(LParenLoc, LHS, Opc, EllipsisLoc, RHS, RParenLoc,
+  None);
 }
 
 ExprResult Sema::BuildCXXFoldExpr(SourceLocation LParenLoc, Expr *LHS,
   BinaryOperatorKind Operator,
   SourceLocation EllipsisLoc, Expr *RHS,
-  SourceLocation RParenLoc) {
+  SourceLocation RParenLoc,
+  Optional NumExpansions) {
   return new (Context) CXXFoldExpr(Context.DependentTy, LParenLoc, LHS,
-   Operator, EllipsisLoc, RHS, RParenLoc);
+   Operator, EllipsisLoc, RHS, RParenLoc,
+   NumExpansions);
 }
 
 ExprResult Sema::BuildEmptyCXXFoldExpr(SourceLocation EllipsisLoc,

Modified: cfe/trunk/lib/Sema/TreeTransform.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/TreeTransform.h?rev=360563&r1=360562&r2=360563&view=diff
==

[PATCH] D58033: Add option for emitting dbg info for call site parameters

[Djordje Todorovic via Phabricator via cfe-commits]

djtodoro updated this revision to Diff 199213.
djtodoro edited the summary of this revision.
djtodoro added a comment.

-Add an input in test/CodeGenCXX/dbg-info-all-calls-described.cpp
-Rename the option


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58033/new/

https://reviews.llvm.org/D58033

Files:
  include/clang/Basic/CodeGenOptions.def
  include/clang/Driver/CC1Options.td
  lib/CodeGen/BackendUtil.cpp
  lib/CodeGen/CGDebugInfo.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGenCXX/dbg-info-all-calls-described.cpp


Index: test/CodeGenCXX/dbg-info-all-calls-described.cpp
===
--- test/CodeGenCXX/dbg-info-all-calls-described.cpp
+++ test/CodeGenCXX/dbg-info-all-calls-described.cpp
@@ -15,6 +15,13 @@
 // RUN: | FileCheck %s -check-prefix=HAS-ATTR \
 // RUN: -implicit-check-not=DISubprogram 
-implicit-check-not=DIFlagAllCallsDescribed
 
+// Supported: DWARF4 + GDB tuning by using '-femit-debug-entry-values'
+// RUN: %clang_cc1 -femit-debug-entry-values -emit-llvm %s -o - \
+// RUN:   -O1 -disable-llvm-passes -debugger-tuning=gdb \
+// RUN:   -debug-info-kind=standalone -dwarf-version=4 \
+// RUN: | FileCheck %s -check-prefix=HAS-ATTR \
+// RUN: -implicit-check-not=DIFlagAllCallsDescribed
+
 // Supported: DWARF4 + LLDB tuning, -O1, line-tables only DI
 // RUN: %clang_cc1 -emit-llvm -triple %itanium_abi_triple %s -o - \
 // RUN:   -O1 -disable-llvm-passes \
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -746,6 +746,7 @@
 
   Opts.DisableLLVMPasses = Args.hasArg(OPT_disable_llvm_passes);
   Opts.DisableLifetimeMarkers = Args.hasArg(OPT_disable_lifetimemarkers);
+  Opts.EnableDebugEntryValues = Args.hasArg(OPT_femit_debug_entry_values);
   Opts.DisableO0ImplyOptNone = Args.hasArg(OPT_disable_O0_optnone);
   Opts.DisableRedZone = Args.hasArg(OPT_disable_red_zone);
   Opts.IndirectTlsSegRefs = Args.hasArg(OPT_mno_tls_direct_seg_refs);
Index: lib/CodeGen/CGDebugInfo.cpp
===
--- lib/CodeGen/CGDebugInfo.cpp
+++ lib/CodeGen/CGDebugInfo.cpp
@@ -4558,7 +4558,10 @@
   // were part of DWARF v4.
   bool SupportsDWARFv4Ext =
   CGM.getCodeGenOpts().DwarfVersion == 4 &&
-  CGM.getCodeGenOpts().getDebuggerTuning() == llvm::DebuggerKind::LLDB;
+  (CGM.getCodeGenOpts().getDebuggerTuning() == llvm::DebuggerKind::LLDB ||
+   (CGM.getCodeGenOpts().EnableDebugEntryValues &&
+   CGM.getCodeGenOpts().getDebuggerTuning() == llvm::DebuggerKind::GDB));
+
   if (!SupportsDWARFv4Ext && CGM.getCodeGenOpts().DwarfVersion < 5)
 return llvm::DINode::FlagZero;
 
Index: lib/CodeGen/BackendUtil.cpp
===
--- lib/CodeGen/BackendUtil.cpp
+++ lib/CodeGen/BackendUtil.cpp
@@ -463,6 +463,7 @@
   Options.DebuggerTuning = CodeGenOpts.getDebuggerTuning();
   Options.EmitStackSizeSection = CodeGenOpts.StackSizeSection;
   Options.EmitAddrsig = CodeGenOpts.Addrsig;
+  Options.EnableDebugEntryValues = CodeGenOpts.EnableDebugEntryValues;
 
   if (CodeGenOpts.getSplitDwarfMode() != CodeGenOptions::NoFission)
 Options.MCOptions.SplitDwarfFile = CodeGenOpts.SplitDwarfFile;
Index: include/clang/Driver/CC1Options.td
===
--- include/clang/Driver/CC1Options.td
+++ include/clang/Driver/CC1Options.td
@@ -364,6 +364,8 @@
 def fno_lto_unit: Flag<["-"], "fno-lto-unit">;
 def fthin_link_bitcode_EQ : Joined<["-"], "fthin-link-bitcode=">,
 HelpText<"Write minimized bitcode to  for the ThinLTO thin link 
only">;
+def femit_debug_entry_values : Flag<["-"], "femit-debug-entry-values">,
+HelpText<"Enables debug info about call site parameter's entry values">;
 def fdebug_pass_manager : Flag<["-"], "fdebug-pass-manager">,
 HelpText<"Prints debug information for the new pass manager">;
 def fno_debug_pass_manager : Flag<["-"], "fno-debug-pass-manager">,
Index: include/clang/Basic/CodeGenOptions.def
===
--- include/clang/Basic/CodeGenOptions.def
+++ include/clang/Basic/CodeGenOptions.def
@@ -61,6 +61,7 @@
 CODEGENOPT(DebugPassManager, 1, 0) ///< Prints debug information for the new
///< pass manager.
 CODEGENOPT(DisableRedZone, 1, 0) ///< Set when -mno-red-zone is enabled.
+CODEGENOPT(EnableDebugEntryValues, 1, 0) ///< Emit call site parameter dbg info
 CODEGENOPT(IndirectTlsSegRefs, 1, 0) ///< Set when -mno-tls-direct-seg-refs
  ///< is specified.
 CODEGENOPT(DisableTailCalls  , 1, 0) ///< Do not emit tail calls.


Index: test/CodeGenCXX/dbg-info-all-calls-described.cpp
===
--- test/CodeGenCXX/dbg-info-all-calls-describe

[PATCH] D58035: [clang/DIVar] Emit flag for params that have unchanged values

[Djordje Todorovic via Phabricator via cfe-commits]

djtodoro updated this revision to Diff 199214.
djtodoro added a comment.

-Rebase


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58035/new/

https://reviews.llvm.org/D58035

Files:
  lib/CodeGen/CGDebugInfo.cpp
  lib/CodeGen/CGDebugInfo.h


Index: lib/CodeGen/CGDebugInfo.h
===
--- lib/CodeGen/CGDebugInfo.h
+++ lib/CodeGen/CGDebugInfo.h
@@ -133,6 +133,7 @@
 
   llvm::DenseMap DIFileCache;
   llvm::DenseMap SPCache;
+  llvm::DenseMap ParmCache;
   /// Cache declarations relevant to DW_TAG_imported_declarations (C++
   /// using declarations) that aren't covered by other more specific caches.
   llvm::DenseMap DeclCache;
Index: lib/CodeGen/CGDebugInfo.cpp
===
--- lib/CodeGen/CGDebugInfo.cpp
+++ lib/CodeGen/CGDebugInfo.cpp
@@ -18,6 +18,7 @@
 #include "CodeGenFunction.h"
 #include "CodeGenModule.h"
 #include "ConstantEmitter.h"
+#include "clang/Analysis/Analyses/ExprMutationAnalyzer.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/DeclFriend.h"
 #include "clang/AST/DeclObjC.h"
@@ -3880,6 +3881,11 @@
  llvm::DebugLoc::get(Line, Column, Scope, 
CurInlinedAt),
  Builder.GetInsertBlock());
 
+  if (ArgNo) {
+auto *PD = dyn_cast(VD);
+ParmCache[PD].reset(D);
+  }
+
   return D;
 }
 
@@ -4526,6 +4532,25 @@
 if (auto MD = TypeCache[RT])
   DBuilder.retainType(cast(MD));
 
+  if (CGM.getCodeGenOpts().EnableDebugEntryValues &&
+  CGM.getLangOpts().Optimize) {
+for (auto &SP : DeclCache) {
+  auto *D = SP.first;
+  if (const auto *FD = dyn_cast_or_null(D)) {
+const Stmt *FuncBody = (*FD).getBody();
+for (auto Parm : FD->parameters()) {
+  ExprMutationAnalyzer FuncAnalyzer(*FuncBody, CGM.getContext());
+  if (!FuncAnalyzer.isMutated(Parm)) {
+auto I = ParmCache.find(Parm);
+if (I != ParmCache.end()) {
+  auto *DIParm = cast(I->second);
+  DIParm->setIsNotModified();
+}
+  }
+}
+  }
+}
+  }
   DBuilder.finalize();
 }
 


Index: lib/CodeGen/CGDebugInfo.h
===
--- lib/CodeGen/CGDebugInfo.h
+++ lib/CodeGen/CGDebugInfo.h
@@ -133,6 +133,7 @@
 
   llvm::DenseMap DIFileCache;
   llvm::DenseMap SPCache;
+  llvm::DenseMap ParmCache;
   /// Cache declarations relevant to DW_TAG_imported_declarations (C++
   /// using declarations) that aren't covered by other more specific caches.
   llvm::DenseMap DeclCache;
Index: lib/CodeGen/CGDebugInfo.cpp
===
--- lib/CodeGen/CGDebugInfo.cpp
+++ lib/CodeGen/CGDebugInfo.cpp
@@ -18,6 +18,7 @@
 #include "CodeGenFunction.h"
 #include "CodeGenModule.h"
 #include "ConstantEmitter.h"
+#include "clang/Analysis/Analyses/ExprMutationAnalyzer.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/DeclFriend.h"
 #include "clang/AST/DeclObjC.h"
@@ -3880,6 +3881,11 @@
  llvm::DebugLoc::get(Line, Column, Scope, CurInlinedAt),
  Builder.GetInsertBlock());
 
+  if (ArgNo) {
+auto *PD = dyn_cast(VD);
+ParmCache[PD].reset(D);
+  }
+
   return D;
 }
 
@@ -4526,6 +4532,25 @@
 if (auto MD = TypeCache[RT])
   DBuilder.retainType(cast(MD));
 
+  if (CGM.getCodeGenOpts().EnableDebugEntryValues &&
+  CGM.getLangOpts().Optimize) {
+for (auto &SP : DeclCache) {
+  auto *D = SP.first;
+  if (const auto *FD = dyn_cast_or_null(D)) {
+const Stmt *FuncBody = (*FD).getBody();
+for (auto Parm : FD->parameters()) {
+  ExprMutationAnalyzer FuncAnalyzer(*FuncBody, CGM.getContext());
+  if (!FuncAnalyzer.isMutated(Parm)) {
+auto I = ParmCache.find(Parm);
+if (I != ParmCache.end()) {
+  auto *DIParm = cast(I->second);
+  DIParm->setIsNotModified();
+}
+  }
+}
+  }
+}
+  }
   DBuilder.finalize();
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61841: [clangd] Respect WarningsAsErrors configuration for clang-tidy

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clang-tools-extra/clangd/ClangdUnit.cpp:346
+  std::string CheckName = CTContext->getCheckName(Info.getID());
+  if (!CheckName.empty() && WarningAsErrorFilter->contains(CheckName)) {
+Level = DiagnosticsEngine::Error;

Why can't we call `CTContext->treatAsError()` here?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61841/new/

https://reviews.llvm.org/D61841



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

[PATCH] D61790: [C++20] add consteval specifier

[Gabor Marton via Phabricator via cfe-commits]

martong resigned from this revision.
martong added a comment.
Herald added a subscriber: rnkovacs.

ASTImporter.cpp looks good to me.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61790/new/

https://reviews.llvm.org/D61790



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

[PATCH] D61786: [ASTImporter] Separate unittest files

[Gabor Marton via Phabricator via cfe-commits]

martong marked 8 inline comments as done.
martong added a comment.

Thanks for the review Alexei!




Comment at: clang/unittests/AST/ASTImporterGenericRedeclTest.cpp:20
+
+// FIXME put these structs and the tests rely on them into their own separate
+// test file!

a_sidorin wrote:
> Is this FIXME obsolete now?
Yes, that's correct, thank you for finding it!



Comment at: clang/unittests/AST/ASTImporterVisibilityTest.cpp:49
+::testing::WithParamInterface>;
+// Fixture to test the redecl chain of Decls with the same visibility.  Gtest
+// makes it possible to have either value-parameterized or type-parameterized

a_sidorin wrote:
> Double spaces in the end of sentences look a bit strange.
Ok I removed them. (Note, this is the result of some autoformatting option in 
vim, could not discover which specific setting causes the insertion of double 
spaces.)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61786/new/

https://reviews.llvm.org/D61786



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

[PATCH] D61786: [ASTImporter] Separate unittest files

[Gabor Marton via Phabricator via cfe-commits]

martong updated this revision to Diff 199225.
martong marked 2 inline comments as done.
martong added a comment.

- Address Alexei's comments


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61786/new/

https://reviews.llvm.org/D61786

Files:
  clang/unittests/AST/ASTImporterFixtures.cpp
  clang/unittests/AST/ASTImporterFixtures.h
  clang/unittests/AST/ASTImporterGenericRedeclTest.cpp
  clang/unittests/AST/ASTImporterTest.cpp
  clang/unittests/AST/ASTImporterVisibilityTest.cpp
  clang/unittests/AST/CMakeLists.txt

Index: clang/unittests/AST/CMakeLists.txt
===
--- clang/unittests/AST/CMakeLists.txt
+++ clang/unittests/AST/CMakeLists.txt
@@ -8,7 +8,10 @@
 
 add_clang_unittest(ASTTests
   ASTContextParentMapTest.cpp
+  ASTImporterFixtures.cpp
   ASTImporterTest.cpp
+  ASTImporterGenericRedeclTest.cpp
+  ASTImporterVisibilityTest.cpp
   ASTTypeTraitsTest.cpp
   ASTVectorTest.cpp
   CommentLexer.cpp
Index: clang/unittests/AST/ASTImporterVisibilityTest.cpp
===
--- /dev/null
+++ clang/unittests/AST/ASTImporterVisibilityTest.cpp
@@ -0,0 +1,219 @@
+//===- unittest/AST/ASTImporterTest.cpp - AST node import test ===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// Type-parameterized tests for the correct import of Decls with different
+// visibility.
+//
+//===--===//
+
+// Define this to have ::testing::Combine available.
+// FIXME: Better solution for this?
+#define GTEST_HAS_COMBINE 1
+
+#include "ASTImporterFixtures.h"
+
+namespace clang {
+namespace ast_matchers {
+
+using internal::BindableMatcher;
+
+// Type parameters for type-parameterized test fixtures.
+struct GetFunPattern {
+  using DeclTy = FunctionDecl;
+  BindableMatcher operator()() { return functionDecl(hasName("f")); }
+};
+struct GetVarPattern {
+  using DeclTy = VarDecl;
+  BindableMatcher operator()() { return varDecl(hasName("v")); }
+};
+
+// Values for the value-parameterized test fixtures.
+// FunctionDecl:
+const auto *ExternF = "void f();";
+const auto *StaticF = "static void f();";
+const auto *AnonF = "namespace { void f(); }";
+// VarDecl:
+const auto *ExternV = "extern int v;";
+const auto *StaticV = "static int v;";
+const auto *AnonV = "namespace { extern int v; }";
+
+// First value in tuple: Compile options.
+// Second value in tuple: Source code to be used in the test.
+using ImportVisibilityChainParams =
+::testing::WithParamInterface>;
+// Fixture to test the redecl chain of Decls with the same visibility. Gtest
+// makes it possible to have either value-parameterized or type-parameterized
+// fixtures. However, we cannot have both value- and type-parameterized test
+// fixtures. This is a value-parameterized test fixture in the gtest sense. We
+// intend to mimic gtest's type-parameters via the PatternFactory template
+// parameter. We manually instantiate the different tests with the each types.
+template 
+class ImportVisibilityChain
+: public ASTImporterTestBase, public ImportVisibilityChainParams {
+protected:
+  using DeclTy = typename PatternFactory::DeclTy;
+  ArgVector getExtraArgs() const override { return std::get<0>(GetParam()); }
+  std::string getCode() const { return std::get<1>(GetParam()); }
+  BindableMatcher getPattern() const { return PatternFactory()(); }
+
+  // Type-parameterized test.
+  void TypedTest_ImportChain() {
+std::string Code = getCode() + getCode();
+auto Pattern = getPattern();
+
+TranslationUnitDecl *FromTu = getTuDecl(Code, Lang_CXX, "input0.cc");
+
+auto *FromD0 = FirstDeclMatcher().match(FromTu, Pattern);
+auto *FromD1 = LastDeclMatcher().match(FromTu, Pattern);
+
+auto *ToD0 = Import(FromD0, Lang_CXX);
+auto *ToD1 = Import(FromD1, Lang_CXX);
+
+EXPECT_TRUE(ToD0);
+ASSERT_TRUE(ToD1);
+EXPECT_NE(ToD0, ToD1);
+EXPECT_EQ(ToD1->getPreviousDecl(), ToD0);
+  }
+};
+
+// Manual instantiation of the fixture with each type.
+using ImportFunctionsVisibilityChain = ImportVisibilityChain;
+using ImportVariablesVisibilityChain = ImportVisibilityChain;
+// Value-parameterized test for the first type.
+TEST_P(ImportFunctionsVisibilityChain, ImportChain) {
+  TypedTest_ImportChain();
+}
+// Value-parameterized test for the second type.
+TEST_P(ImportVariablesVisibilityChain, ImportChain) {
+  TypedTest_ImportChain();
+}
+
+// Automatic instantiation of the value-parameterized tests.
+INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportFunctionsVisibilityChain,
+::testing::Combine(
+   DefaultTestValuesForRunOp

r360572 - [ASTImporter] Separate unittest files

[Gabor Marton via cfe-commits]

Author: martong
Date: Mon May 13 03:06:25 2019
New Revision: 360572

URL: http://llvm.org/viewvc/llvm-project?rev=360572&view=rev
Log:
[ASTImporter] Separate unittest files

Summary:
Move generic redecl chain tests and visibility tests into their own
separate test files.

Reviewers: a_sidorin, a.sidorin, shafik

Subscribers: mgorny, rnkovacs, dkrupp, Szelethus, gamesh411, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D61786

Added:
cfe/trunk/unittests/AST/ASTImporterFixtures.cpp
cfe/trunk/unittests/AST/ASTImporterFixtures.h
cfe/trunk/unittests/AST/ASTImporterGenericRedeclTest.cpp
cfe/trunk/unittests/AST/ASTImporterVisibilityTest.cpp
Modified:
cfe/trunk/unittests/AST/ASTImporterTest.cpp
cfe/trunk/unittests/AST/CMakeLists.txt

Added: cfe/trunk/unittests/AST/ASTImporterFixtures.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/AST/ASTImporterFixtures.cpp?rev=360572&view=auto
==
--- cfe/trunk/unittests/AST/ASTImporterFixtures.cpp (added)
+++ cfe/trunk/unittests/AST/ASTImporterFixtures.cpp Mon May 13 03:06:25 2019
@@ -0,0 +1,210 @@
+//===- unittest/AST/ASTImporterFixtures.cpp - AST unit test support 
---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+/// \file
+/// Implementation of fixture classes for testing the ASTImporter.
+//
+//===--===//
+
+#include "ASTImporterFixtures.h"
+
+#include "clang/AST/ASTImporter.h"
+#include "clang/AST/ASTImporterLookupTable.h"
+#include "clang/Frontend/ASTUnit.h"
+#include "clang/Tooling/Tooling.h"
+
+namespace clang {
+namespace ast_matchers {
+
+void createVirtualFileIfNeeded(ASTUnit *ToAST, StringRef FileName,
+   std::unique_ptr &&Buffer) {
+  assert(ToAST);
+  ASTContext &ToCtx = ToAST->getASTContext();
+  auto *OFS = static_cast(
+  &ToCtx.getSourceManager().getFileManager().getVirtualFileSystem());
+  auto *MFS = static_cast(
+  OFS->overlays_begin()->get());
+  MFS->addFile(FileName, 0, std::move(Buffer));
+}
+
+void createVirtualFileIfNeeded(ASTUnit *ToAST, StringRef FileName,
+   StringRef Code) {
+  return createVirtualFileIfNeeded(ToAST, FileName,
+   llvm::MemoryBuffer::getMemBuffer(Code));
+}
+
+ASTImporterTestBase::TU::TU(StringRef Code, StringRef FileName, ArgVector Args,
+ImporterConstructor C)
+: Code(Code), FileName(FileName),
+  Unit(tooling::buildASTFromCodeWithArgs(this->Code, Args, 
this->FileName)),
+  TUDecl(Unit->getASTContext().getTranslationUnitDecl()), Creator(C) {
+  Unit->enableSourceFileDiagnostics();
+
+  // If the test doesn't need a specific ASTImporter, we just create a
+  // normal ASTImporter with it.
+  if (!Creator)
+Creator = [](ASTContext &ToContext, FileManager &ToFileManager,
+ ASTContext &FromContext, FileManager &FromFileManager,
+ bool MinimalImport, ASTImporterLookupTable *LookupTable) {
+  return new ASTImporter(ToContext, ToFileManager, FromContext,
+ FromFileManager, MinimalImport, LookupTable);
+};
+}
+
+ASTImporterTestBase::TU::~TU() {}
+
+void ASTImporterTestBase::TU::lazyInitImporter(
+ASTImporterLookupTable &LookupTable, ASTUnit *ToAST) {
+  assert(ToAST);
+  if (!Importer)
+Importer.reset(Creator(ToAST->getASTContext(), ToAST->getFileManager(),
+   Unit->getASTContext(), Unit->getFileManager(), 
false,
+   &LookupTable));
+  assert(&ToAST->getASTContext() == &Importer->getToContext());
+  createVirtualFileIfNeeded(ToAST, FileName, Code);
+}
+
+Decl *ASTImporterTestBase::TU::import(ASTImporterLookupTable &LookupTable,
+  ASTUnit *ToAST, Decl *FromDecl) {
+  lazyInitImporter(LookupTable, ToAST);
+  if (auto ImportedOrErr = Importer->Import_New(FromDecl))
+return *ImportedOrErr;
+  else {
+llvm::consumeError(ImportedOrErr.takeError());
+return nullptr;
+  }
+}
+
+QualType ASTImporterTestBase::TU::import(ASTImporterLookupTable &LookupTable,
+ ASTUnit *ToAST, QualType FromType) {
+  lazyInitImporter(LookupTable, ToAST);
+  if (auto ImportedOrErr = Importer->Import_New(FromType))
+return *ImportedOrErr;
+  else {
+llvm::consumeError(ImportedOrErr.takeError());
+return QualType{};
+  }
+}
+
+void ASTImporterTestBase::lazyInitLookupTable(TranslationUnitDecl *ToTU) {
+  assert(ToTU);
+  if (!LookupTablePtr)
+LookupTablePtr = llvm::make_unique(*ToTU);
+}
+
+void ASTImporterTestBase::lazyInitToAST(Langu

[PATCH] D61786: [ASTImporter] Separate unittest files

[Gabor Marton via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC360572: [ASTImporter] Separate unittest files (authored by 
martong, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D61786?vs=199225&id=199226#toc

Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61786/new/

https://reviews.llvm.org/D61786

Files:
  unittests/AST/ASTImporterFixtures.cpp
  unittests/AST/ASTImporterFixtures.h
  unittests/AST/ASTImporterGenericRedeclTest.cpp
  unittests/AST/ASTImporterTest.cpp
  unittests/AST/ASTImporterVisibilityTest.cpp
  unittests/AST/CMakeLists.txt

Index: unittests/AST/ASTImporterGenericRedeclTest.cpp
===
--- unittests/AST/ASTImporterGenericRedeclTest.cpp
+++ unittests/AST/ASTImporterGenericRedeclTest.cpp
@@ -0,0 +1,577 @@
+//===- unittest/AST/ASTImporterTest.cpp - AST node import test ===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// Type-parameterized tests for the correct import of redecl chains.
+//
+//===--===//
+
+#include "ASTImporterFixtures.h"
+
+namespace clang {
+namespace ast_matchers {
+
+using internal::BindableMatcher;
+
+struct Function {
+  using DeclTy = FunctionDecl;
+  static constexpr auto *Prototype = "void X();";
+  static constexpr auto *Definition = "void X() {}";
+  BindableMatcher getPattern() {
+return functionDecl(hasName("X"), unless(isImplicit()));
+  }
+};
+
+struct Class {
+  using DeclTy = CXXRecordDecl;
+  static constexpr auto *Prototype = "class X;";
+  static constexpr auto *Definition = "class X {};";
+  BindableMatcher getPattern() {
+return cxxRecordDecl(hasName("X"), unless(isImplicit()));
+  }
+};
+
+struct Variable {
+  using DeclTy = VarDecl;
+  static constexpr auto *Prototype = "extern int X;";
+  static constexpr auto *Definition = "int X;";
+  BindableMatcher getPattern() { return varDecl(hasName("X")); }
+};
+
+struct FunctionTemplate {
+  using DeclTy = FunctionTemplateDecl;
+  static constexpr auto *Prototype = "template  void X();";
+  static constexpr auto *Definition =
+  R"(
+  template  void X() {};
+  // Explicit instantiation is a must because of -fdelayed-template-parsing:
+  template void X();
+  )";
+  BindableMatcher getPattern() {
+return functionTemplateDecl(hasName("X"), unless(isImplicit()));
+  }
+};
+
+struct ClassTemplate {
+  using DeclTy = ClassTemplateDecl;
+  static constexpr auto *Prototype = "template  class X;";
+  static constexpr auto *Definition = "template  class X {};";
+  BindableMatcher getPattern() {
+return classTemplateDecl(hasName("X"), unless(isImplicit()));
+  }
+};
+
+struct FunctionTemplateSpec {
+  using DeclTy = FunctionDecl;
+  static constexpr auto *Prototype =
+  R"(
+  // Proto of the primary template.
+  template 
+  void X();
+  // Proto of the specialization.
+  template <>
+  void X();
+  )";
+  static constexpr auto *Definition =
+  R"(
+  // Proto of the primary template.
+  template 
+  void X();
+  // Specialization and definition.
+  template <>
+  void X() {}
+  )";
+  BindableMatcher getPattern() {
+return functionDecl(hasName("X"), isExplicitTemplateSpecialization());
+  }
+};
+
+struct ClassTemplateSpec {
+  using DeclTy = ClassTemplateSpecializationDecl;
+  static constexpr auto *Prototype =
+  R"(
+template  class X;
+template <> class X;
+)";
+  static constexpr auto *Definition =
+  R"(
+template  class X;
+template <> class X {};
+)";
+  BindableMatcher getPattern() {
+return classTemplateSpecializationDecl(hasName("X"), unless(isImplicit()));
+  }
+};
+
+template 
+struct RedeclChain : ASTImporterOptionSpecificTestBase {
+
+  using DeclTy = typename TypeParam::DeclTy;
+  std::string getPrototype() { return TypeParam::Prototype; }
+  std::string getDefinition() { return TypeParam::Definition; }
+  BindableMatcher getPattern() const { return TypeParam().getPattern(); }
+
+  void CheckPreviousDecl(Decl *Prev, Decl *Current) {
+ASSERT_NE(Prev, Current);
+ASSERT_EQ(&Prev->getASTContext(), &Current->getASTContext());
+EXPECT_EQ(Prev->getCanonicalDecl(), Current->getCanonicalDecl());
+
+// Templates.
+if (auto *PrevT = dyn_cast(Prev)) {
+  EXPECT_EQ(Current->getPreviousDecl(), Prev);
+  auto *CurrentT = cast(Current);
+  ASSERT_TRUE(PrevT->getTemplatedDecl());
+  ASSERT_TRUE(CurrentT->getTemplatedDecl());
+  EXPECT_EQ(CurrentT->getTemplatedDecl()->getPreviousDecl(),
+PrevT->getTemplatedDecl());
+  return;
+}
+
+// Speciali

[PATCH] D61849: Do not list enabled checks when -quiet is given to clang-tidy.

[Sven Panne via Phabricator via cfe-commits]

svenpanne created this revision.
svenpanne added a reviewer: alexfh.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

When clang-tidy is given the -quiet flag, do not output the potentially 
hundreds of enabled check names at the beginning.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D61849

Files:
  clang-tools-extra/clang-tidy/tool/run-clang-tidy.py


Index: clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
===
--- clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
+++ clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
@@ -245,7 +245,12 @@
 if args.checks:
   invocation.append('-checks=' + args.checks)
 invocation.append('-')
-subprocess.check_call(invocation)
+if args.quiet:
+  # Even with -quiet we still want to check if we can call clang-tidy.
+  with open(os.devnull, 'w') as dev_null:
+subprocess.check_call(invocation, stdout=dev_null)
+else:
+  subprocess.check_call(invocation)
   except:
 print("Unable to run clang-tidy.", file=sys.stderr)
 sys.exit(1)


Index: clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
===
--- clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
+++ clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
@@ -245,7 +245,12 @@
 if args.checks:
   invocation.append('-checks=' + args.checks)
 invocation.append('-')
-subprocess.check_call(invocation)
+if args.quiet:
+  # Even with -quiet we still want to check if we can call clang-tidy.
+  with open(os.devnull, 'w') as dev_null:
+subprocess.check_call(invocation, stdout=dev_null)
+else:
+  subprocess.check_call(invocation)
   except:
 print("Unable to run clang-tidy.", file=sys.stderr)
 sys.exit(1)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61850: Removed superfluous and slightly annoying newlines in run-clang-tidy's output.

[Sven Panne via Phabricator via cfe-commits]

svenpanne created this revision.
svenpanne added a reviewer: alexfh.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

The output of clang-tidy itself already has enough newlines, so the resulting 
output is more in line with the usual compiler output.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D61850

Files:
  clang-tools-extra/clang-tidy/tool/run-clang-tidy.py


Index: clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
===
--- clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
+++ clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
@@ -170,10 +170,10 @@
 if proc.returncode != 0:
   failed_files.append(name)
 with lock:
-  sys.stdout.write(' '.join(invocation) + '\n' + output.decode('utf-8') + 
'\n')
+  sys.stdout.write(' '.join(invocation) + '\n' + output.decode('utf-8'))
   if len(err) > 0:
 sys.stdout.flush()
-sys.stderr.write(err.decode('utf-8') + '\n')
+sys.stderr.write(err.decode('utf-8'))
 queue.task_done()
 
 


Index: clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
===
--- clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
+++ clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
@@ -170,10 +170,10 @@
 if proc.returncode != 0:
   failed_files.append(name)
 with lock:
-  sys.stdout.write(' '.join(invocation) + '\n' + output.decode('utf-8') + '\n')
+  sys.stdout.write(' '.join(invocation) + '\n' + output.decode('utf-8'))
   if len(err) > 0:
 sys.stdout.flush()
-sys.stderr.write(err.decode('utf-8') + '\n')
+sys.stderr.write(err.decode('utf-8'))
 queue.task_done()
 
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59885: [Lex] Allow to consume tokens while preprocessing

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov marked 2 inline comments as done.
ilya-biryukov added a comment.

I'll try to explore bringing the overhead down.
The fact that `CachingLex` is happening at `LexLevel==1` might help, thanks for 
pointing that out!




Comment at: clang/lib/Lex/PPCaching.cpp:64
   ExitCachingLexMode();
-  Lex(Result);
+  Lex(Result, Report);
 

rsmith wrote:
> This change seems redundant: `LexLevel` is always `1` here, so this token 
> would never be reported anyway. And with that gone I think you can remove the 
> `Report` parameter entirely.
It is reported by the caller of `CachingLex` (the one that has `LexLevel == 0`) 
and it needs to know where the reported token comes from.



Comment at: clang/lib/Lex/Preprocessor.cpp:892
   do {
 switch (CurLexerKind) {
 case CLK_Lexer:

rsmith wrote:
> Doesn't this always set `Report` to the same value as `IsNewToken`? (The only 
> case we set `Report` to `false` is when we call `CachingLex` and it sets 
> `IsNewToken` to `false`, and `CachingLex` can't be recursively called twice, 
> so its recursive call to `Lex` can't set `Report` to `false`..)
We can have `IsNewToken = true && Report = false` when the recursive call to 
`Lex()` inside `CachingLex()` sets `Report = CurTokenLexer->isMacroExpansion()`.
This happens when we call `CachingLex` and the resulting token comes from a 
token stream for out-of-order parsing.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59885/new/

https://reviews.llvm.org/D59885



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

[PATCH] D60748: Fix i386 struct and union parameter alignment

[Wei Xiao via Phabricator via cfe-commits]

wxiao3 updated this revision to Diff 199232.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60748/new/

https://reviews.llvm.org/D60748

Files:
  lib/CodeGen/TargetInfo.cpp
  test/CodeGen/x86_32-align-linux.c
  test/CodeGen/x86_32-arguments-linux.c

Index: test/CodeGen/x86_32-arguments-linux.c
===
--- test/CodeGen/x86_32-arguments-linux.c
+++ test/CodeGen/x86_32-arguments-linux.c
@@ -3,21 +3,21 @@
 
 // CHECK-LABEL: define void @f56(
 // CHECK: i8 signext %a0, %struct.s56_0* byval align 4 %a1,
-// CHECK: i64 %a2.coerce, %struct.s56_1* byval align 4,
-// CHECK: <1 x double> %a4, %struct.s56_2* byval align 4,
-// CHECK: <4 x i32> %a6, %struct.s56_3* byval align 4,
-// CHECK: <2 x double> %a8, %struct.s56_4* byval align 4,
-// CHECK: <8 x i32> %a10, %struct.s56_5* byval align 4,
-// CHECK: <4 x double> %a12, %struct.s56_6* byval align 4)
+// CHECK: i64 %a2.coerce, %struct.s56_1* byval align 8 %a3,
+// CHECK: <1 x double> %a4, %struct.s56_2* byval align 8 %a5,
+// CHECK: <4 x i32> %a6, %struct.s56_3* byval align 16 %a7,
+// CHECK: <2 x double> %a8, %struct.s56_4* byval align 16 %a9,
+// CHECK: <8 x i32> %a10, %struct.s56_5* byval align 32 %a11,
+// CHECK: <4 x double> %a12, %struct.s56_6* byval align 32 %a13)
 
 // CHECK: call void (i32, ...) @f56_0(i32 1,
 // CHECK: i32 %{{.*}}, %struct.s56_0* byval align 4 %{{[^ ]*}},
-// CHECK: i64 %{{[^ ]*}}, %struct.s56_1* byval align 4 %{{[^ ]*}},
-// CHECK: <1 x double> %{{[^ ]*}}, %struct.s56_2* byval align 4 %{{[^ ]*}},
-// CHECK: <4 x i32> %{{[^ ]*}}, %struct.s56_3* byval align 4 %{{[^ ]*}},
-// CHECK: <2 x double> %{{[^ ]*}}, %struct.s56_4* byval align 4 %{{[^ ]*}},
-// CHECK: <8 x i32> %{{[^ ]*}}, %struct.s56_5* byval align 4 %{{[^ ]*}},
-// CHECK: <4 x double> %{{[^ ]*}}, %struct.s56_6* byval align 4 %{{[^ ]*}})
+// CHECK: i64 %{{[^ ]*}}, %struct.s56_1* byval align 8 %{{[^ ]*}},
+// CHECK: <1 x double> %{{[^ ]*}}, %struct.s56_2* byval align 8 %{{[^ ]*}},
+// CHECK: <4 x i32> %{{[^ ]*}}, %struct.s56_3* byval align 16 %{{[^ ]*}},
+// CHECK: <2 x double> %{{[^ ]*}}, %struct.s56_4* byval align 16 %{{[^ ]*}},
+// CHECK: <8 x i32> %{{[^ ]*}}, %struct.s56_5* byval align 32 %{{[^ ]*}},
+// CHECK: <4 x double> %{{[^ ]*}}, %struct.s56_6* byval align 32 %{{[^ ]*}})
 // CHECK: }
 //
 //  [i386] clang misaligns long double in structures
Index: test/CodeGen/x86_32-align-linux.c
===
--- /dev/null
+++ test/CodeGen/x86_32-align-linux.c
@@ -0,0 +1,25 @@
+// RUN: %clang_cc1 -w -fblocks -ffreestanding -triple i386-pc-linux-gnu -emit-llvm -o %t %s
+// RUN: FileCheck < %t %s
+
+#include 
+
+typedef union {
+int d[4];
+__m128 m;
+} M128;
+
+extern void foo(int, ...);
+
+M128 a;
+
+// CHECK-LABEL: define void @test
+// CHECK: entry:
+// CHECK: call void (i32, ...) @foo(i32 1, %union.M128* byval align 16
+// CHECK: call void (i32, ...) @foo(i32 1, <4 x float>
+
+void test(void)
+{
+  foo(1, a);
+  foo(1, a.m);
+}
+
Index: lib/CodeGen/TargetInfo.cpp
===
--- lib/CodeGen/TargetInfo.cpp
+++ lib/CodeGen/TargetInfo.cpp
@@ -1010,6 +1010,7 @@
   bool IsWin32StructABI;
   bool IsSoftFloatABI;
   bool IsMCUABI;
+  bool IsPS4ABI;
   unsigned DefaultNumRegisterParameters;
 
   static bool isRegisterSize(unsigned Size) {
@@ -1076,6 +1077,7 @@
   IsWin32StructABI(Win32StructABI),
   IsSoftFloatABI(SoftFloatABI),
   IsMCUABI(CGT.getTarget().getTriple().isOSIAMCU()),
+  IsPS4ABI(CGT.getTarget().getTriple().isPS4()),
   DefaultNumRegisterParameters(NumRegisterParameters) {}
 
   bool shouldPassIndirectlyForSwift(ArrayRef scalars,
@@ -1492,18 +1494,18 @@
   if (Align <= MinABIStackAlignInBytes)
 return 0; // Use default alignment.
 
-  // On non-Darwin, the stack type alignment is always 4.
-  if (!IsDarwinVectorABI) {
-// Set explicit alignment, since we may need to realign the top.
+  if (IsDarwinVectorABI) {
+// On Darwin, if the type contains an SSE vector type, the alignment is 16.
+if (Align >= 16 && (isSSEVectorType(getContext(), Ty) ||
+  isRecordWithSSEVectorType(getContext(), Ty)))
+  return 16;
+return MinABIStackAlignInBytes;
+  } else if (IsWin32StructABI || IsPS4ABI) {
 return MinABIStackAlignInBytes;
   }
-
-  // Otherwise, if the type contains an SSE vector type, the alignment is 16.
-  if (Align >= 16 && (isSSEVectorType(getContext(), Ty) ||
-  isRecordWithSSEVectorType(getContext(), Ty)))
-return 16;
-
-  return MinABIStackAlignInBytes;
+  // i386 System V ABI 2.1: Structures and unions assume the alignment of their
+  // most strictly aligned component.
+  return Align;
 }
 
 ABIArgInfo X86_32ABIInfo::getIndirectResult(QualType Ty, bool ByVal,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listi

[PATCH] D60748: Fix i386 struct and union parameter alignment

[Wei Xiao via Phabricator via cfe-commits]

wxiao3 updated this revision to Diff 199231.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60748/new/

https://reviews.llvm.org/D60748

Files:
  lib/CodeGen/TargetInfo.cpp
  test/CodeGen/vector.c
  test/CodeGen/x86_32-arguments-darwin.c
  test/CodeGen/x86_32-arguments-linux.c
  test/CodeGen/x86_32-m64.c

Index: test/CodeGen/x86_32-m64.c
===
--- /dev/null
+++ test/CodeGen/x86_32-m64.c
@@ -0,0 +1,25 @@
+// RUN: %clang_cc1 -w -O2 -fblocks -triple i386-pc-linux-gnu -target-cpu pentium4 -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,LINUX
+// RUN: %clang_cc1 -w -O2 -fblocks -triple i386-apple-darwin9 -target-cpu yonah -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,DARWIN
+// RUN: %clang_cc1 -w -O2 -fblocks -triple i386-pc-elfiamcu -mfloat-abi soft -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,IAMCU
+// RUN: %clang_cc1 -w -O2 -fblocks -triple i386-pc-win32 -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,WIN32
+
+#include 
+__m64 m64;
+void callee(__m64 __m1, __m64 __m2);
+__m64 caller(__m64 __m1, __m64 __m2)
+{
+// LINUX-LABEL: define x86_mmx @caller(x86_mmx %__m1.coerce, x86_mmx %__m2.coerce)
+// LINUX: tail call void @callee(x86_mmx %__m2.coerce, x86_mmx %__m1.coerce)
+// LINUX: ret x86_mmx
+// DARWIN-LABEL: define i64 @caller(i64 %__m1.coerce, i64 %__m2.coerce)
+// DARWIN: tail call void @callee(i64 %__m2.coerce, i64 %__m1.coerce)
+// DARWIN: ret i64
+// IAMCU-LABEL: define <1 x i64> @caller(i64 %__m1.coerce, i64 %__m2.coerce)
+// IAMCU: tail call void @callee(i64 %__m2.coerce, i64 %__m1.coerce)
+// IAMCU: ret <1 x i64>
+// WIN32-LABEL: define dso_local <1 x i64> @caller(i64 %__m1.coerce, i64 %__m2.coerce)
+// WIN32: call void @callee(i64 %__m2.coerce, i64 %__m1.coerce)
+// WIN32: ret <1 x i64>
+  callee(__m2, __m1);
+  return m64;
+}
Index: test/CodeGen/x86_32-arguments-linux.c
===
--- test/CodeGen/x86_32-arguments-linux.c
+++ test/CodeGen/x86_32-arguments-linux.c
@@ -3,7 +3,7 @@
 
 // CHECK-LABEL: define void @f56(
 // CHECK: i8 signext %a0, %struct.s56_0* byval align 4 %a1,
-// CHECK: i64 %a2.coerce, %struct.s56_1* byval align 4,
+// CHECK: x86_mmx %a2.coerce, %struct.s56_1* byval align 4,
 // CHECK: <1 x double> %a4, %struct.s56_2* byval align 4,
 // CHECK: <4 x i32> %a6, %struct.s56_3* byval align 4,
 // CHECK: <2 x double> %a8, %struct.s56_4* byval align 4,
@@ -12,7 +12,7 @@
 
 // CHECK: call void (i32, ...) @f56_0(i32 1,
 // CHECK: i32 %{{.*}}, %struct.s56_0* byval align 4 %{{[^ ]*}},
-// CHECK: i64 %{{[^ ]*}}, %struct.s56_1* byval align 4 %{{[^ ]*}},
+// CHECK: x86_mmx %{{[^ ]*}}, %struct.s56_1* byval align 4 %{{[^ ]*}},
 // CHECK: <1 x double> %{{[^ ]*}}, %struct.s56_2* byval align 4 %{{[^ ]*}},
 // CHECK: <4 x i32> %{{[^ ]*}}, %struct.s56_3* byval align 4 %{{[^ ]*}},
 // CHECK: <2 x double> %{{[^ ]*}}, %struct.s56_4* byval align 4 %{{[^ ]*}},
Index: test/CodeGen/x86_32-arguments-darwin.c
===
--- test/CodeGen/x86_32-arguments-darwin.c
+++ test/CodeGen/x86_32-arguments-darwin.c
@@ -229,7 +229,7 @@
 
 // CHECK-LABEL: define void @f56(
 // CHECK: i8 signext %a0, %struct.s56_0* byval align 4 %a1,
-// CHECK: i64 %a2.coerce, %struct.s56_1* byval align 4,
+// CHECK: x86_mmx %a2.coerce, %struct.s56_1* byval align 4,
 // CHECK: i64 %a4.coerce, %struct.s56_2* byval align 4,
 // CHECK: <4 x i32> %a6, %struct.s56_3* byval align 16 %a7,
 // CHECK: <2 x double> %a8, %struct.s56_4* byval align 16 %a9,
@@ -238,7 +238,7 @@
 
 // CHECK:   call void (i32, ...) @f56_0(i32 1,
 // CHECK: i32 %{{[^ ]*}}, %struct.s56_0* byval align 4 %{{[^ ]*}},
-// CHECK: i64 %{{[^ ]*}}, %struct.s56_1* byval align 4 %{{[^ ]*}},
+// CHECK: x86_mmx %{{[^ ]*}}, %struct.s56_1* byval align 4 %{{[^ ]*}},
 // CHECK: i64 %{{[^ ]*}}, %struct.s56_2* byval align 4 %{{[^ ]*}},
 // CHECK: <4 x i32> %{{[^ ]*}}, %struct.s56_3* byval align 16 %{{[^ ]*}},
 // CHECK: <2 x double> %{{[^ ]*}}, %struct.s56_4* byval align 16 %{{[^ ]*}},
Index: test/CodeGen/vector.c
===
--- test/CodeGen/vector.c
+++ test/CodeGen/vector.c
@@ -78,5 +78,5 @@
   return y;
 }
 
-// CHECK: define void @lax_vector_compare2(<2 x i32>* {{.*sret.*}}, i64 {{.*}}, i64 {{.*}})
+// CHECK: define void @lax_vector_compare2(<2 x i32>* {{.*sret.*}}, i64 {{.*}}, x86_mmx {{.*}})
 // CHECK: icmp eq <2 x i32>
Index: lib/CodeGen/TargetInfo.cpp
===
--- lib/CodeGen/TargetInfo.cpp
+++ lib/CodeGen/TargetInfo.cpp
@@ -915,14 +915,6 @@
: ABIArgInfo::getDirect());
 }
 
-/// IsX86_MMXType - Return true if this is an MMX type.
-bool IsX86_MMXType(llvm::Type *IRType) {
-  // Return true if the type is an MMX type <2 x i32>, <4 x i16>, or <8 x i8>.
-  return IRType->isVectorTy() && IRType->getPrimitiveSizeIn

[PATCH] D59744: Fix i386 ABI "__m64" type bug

[Wei Xiao via Phabricator via cfe-commits]

wxiao3 updated this revision to Diff 199233.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59744/new/

https://reviews.llvm.org/D59744

Files:
  lib/CodeGen/TargetInfo.cpp
  test/CodeGen/vector.c
  test/CodeGen/x86_32-arguments-darwin.c
  test/CodeGen/x86_32-arguments-linux.c
  test/CodeGen/x86_32-m64.c

Index: test/CodeGen/x86_32-m64.c
===
--- /dev/null
+++ test/CodeGen/x86_32-m64.c
@@ -0,0 +1,25 @@
+// RUN: %clang_cc1 -w -O2 -fblocks -triple i386-pc-linux-gnu -target-cpu pentium4 -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,LINUX
+// RUN: %clang_cc1 -w -O2 -fblocks -triple i386-apple-darwin9 -target-cpu yonah -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,DARWIN
+// RUN: %clang_cc1 -w -O2 -fblocks -triple i386-pc-elfiamcu -mfloat-abi soft -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,IAMCU
+// RUN: %clang_cc1 -w -O2 -fblocks -triple i386-pc-win32 -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,WIN32
+
+#include 
+__m64 m64;
+void callee(__m64 __m1, __m64 __m2);
+__m64 caller(__m64 __m1, __m64 __m2)
+{
+// LINUX-LABEL: define x86_mmx @caller(x86_mmx %__m1.coerce, x86_mmx %__m2.coerce)
+// LINUX: tail call void @callee(x86_mmx %__m2.coerce, x86_mmx %__m1.coerce)
+// LINUX: ret x86_mmx
+// DARWIN-LABEL: define i64 @caller(i64 %__m1.coerce, i64 %__m2.coerce)
+// DARWIN: tail call void @callee(i64 %__m2.coerce, i64 %__m1.coerce)
+// DARWIN: ret i64
+// IAMCU-LABEL: define <1 x i64> @caller(i64 %__m1.coerce, i64 %__m2.coerce)
+// IAMCU: tail call void @callee(i64 %__m2.coerce, i64 %__m1.coerce)
+// IAMCU: ret <1 x i64>
+// WIN32-LABEL: define dso_local <1 x i64> @caller(i64 %__m1.coerce, i64 %__m2.coerce)
+// WIN32: call void @callee(i64 %__m2.coerce, i64 %__m1.coerce)
+// WIN32: ret <1 x i64>
+  callee(__m2, __m1);
+  return m64;
+}
Index: test/CodeGen/x86_32-arguments-linux.c
===
--- test/CodeGen/x86_32-arguments-linux.c
+++ test/CodeGen/x86_32-arguments-linux.c
@@ -3,7 +3,7 @@
 
 // CHECK-LABEL: define void @f56(
 // CHECK: i8 signext %a0, %struct.s56_0* byval align 4 %a1,
-// CHECK: i64 %a2.coerce, %struct.s56_1* byval align 4,
+// CHECK: x86_mmx %a2.coerce, %struct.s56_1* byval align 4,
 // CHECK: <1 x double> %a4, %struct.s56_2* byval align 4,
 // CHECK: <4 x i32> %a6, %struct.s56_3* byval align 4,
 // CHECK: <2 x double> %a8, %struct.s56_4* byval align 4,
@@ -12,7 +12,7 @@
 
 // CHECK: call void (i32, ...) @f56_0(i32 1,
 // CHECK: i32 %{{.*}}, %struct.s56_0* byval align 4 %{{[^ ]*}},
-// CHECK: i64 %{{[^ ]*}}, %struct.s56_1* byval align 4 %{{[^ ]*}},
+// CHECK: x86_mmx %{{[^ ]*}}, %struct.s56_1* byval align 4 %{{[^ ]*}},
 // CHECK: <1 x double> %{{[^ ]*}}, %struct.s56_2* byval align 4 %{{[^ ]*}},
 // CHECK: <4 x i32> %{{[^ ]*}}, %struct.s56_3* byval align 4 %{{[^ ]*}},
 // CHECK: <2 x double> %{{[^ ]*}}, %struct.s56_4* byval align 4 %{{[^ ]*}},
Index: test/CodeGen/x86_32-arguments-darwin.c
===
--- test/CodeGen/x86_32-arguments-darwin.c
+++ test/CodeGen/x86_32-arguments-darwin.c
@@ -229,7 +229,7 @@
 
 // CHECK-LABEL: define void @f56(
 // CHECK: i8 signext %a0, %struct.s56_0* byval align 4 %a1,
-// CHECK: i64 %a2.coerce, %struct.s56_1* byval align 4,
+// CHECK: x86_mmx %a2.coerce, %struct.s56_1* byval align 4,
 // CHECK: i64 %a4.coerce, %struct.s56_2* byval align 4,
 // CHECK: <4 x i32> %a6, %struct.s56_3* byval align 16 %a7,
 // CHECK: <2 x double> %a8, %struct.s56_4* byval align 16 %a9,
@@ -238,7 +238,7 @@
 
 // CHECK:   call void (i32, ...) @f56_0(i32 1,
 // CHECK: i32 %{{[^ ]*}}, %struct.s56_0* byval align 4 %{{[^ ]*}},
-// CHECK: i64 %{{[^ ]*}}, %struct.s56_1* byval align 4 %{{[^ ]*}},
+// CHECK: x86_mmx %{{[^ ]*}}, %struct.s56_1* byval align 4 %{{[^ ]*}},
 // CHECK: i64 %{{[^ ]*}}, %struct.s56_2* byval align 4 %{{[^ ]*}},
 // CHECK: <4 x i32> %{{[^ ]*}}, %struct.s56_3* byval align 16 %{{[^ ]*}},
 // CHECK: <2 x double> %{{[^ ]*}}, %struct.s56_4* byval align 16 %{{[^ ]*}},
Index: test/CodeGen/vector.c
===
--- test/CodeGen/vector.c
+++ test/CodeGen/vector.c
@@ -78,5 +78,5 @@
   return y;
 }
 
-// CHECK: define void @lax_vector_compare2(<2 x i32>* {{.*sret.*}}, i64 {{.*}}, i64 {{.*}})
+// CHECK: define void @lax_vector_compare2(<2 x i32>* {{.*sret.*}}, i64 {{.*}}, x86_mmx {{.*}})
 // CHECK: icmp eq <2 x i32>
Index: lib/CodeGen/TargetInfo.cpp
===
--- lib/CodeGen/TargetInfo.cpp
+++ lib/CodeGen/TargetInfo.cpp
@@ -915,14 +915,6 @@
: ABIArgInfo::getDirect());
 }
 
-/// IsX86_MMXType - Return true if this is an MMX type.
-bool IsX86_MMXType(llvm::Type *IRType) {
-  // Return true if the type is an MMX type <2 x i32>, <4 x i16>, or <8 x i8>.
-  return IRType->isVectorTy() && IRType->getPrimitiveSizeIn

[PATCH] D59744: Fix i386 ABI "__m64" type bug

[Wei Xiao via Phabricator via cfe-commits]

wxiao3 marked an inline comment as done.
wxiao3 added inline comments.



Comment at: lib/CodeGen/TargetInfo.cpp:919
 /// IsX86_MMXType - Return true if this is an MMX type.
 bool IsX86_MMXType(llvm::Type *IRType) {
-  // Return true if the type is an MMX type <2 x i32>, <4 x i16>, or <8 x i8>.

rnk wrote:
> wxiao3 wrote:
> > rnk wrote:
> > > I think looking at the LLVM type to decide how something should be passed 
> > > is a bad pattern to follow. We should look at the clang AST to decide how 
> > > things will be passed, not LLVM types. Would that be complicated? Are 
> > > there aggregate types that end up getting passed directly in MMX 
> > > registers?
> > For x86 32 bit target, no aggregate types end up getting passed in MMX 
> > register.
> > The only type passed by MMX is 
> > 
> > > __m64
> > 
> >  which is defined in header file (mmintrin.h):
> > 
> > 
> > ```
> > typedef long long __m64 __attribute__((__vector_size__(8), __aligned__(8)));
> > ```
> > 
> > Yes, it would be good if we define _m64 as a builtin type and handle it in 
> > AST level. But I'm afraid that it won't be a trivial work. Since GCC also 
> > handles __m64 in the same way as Clang currently does, can we just keep 
> > current implementation as it is?
> > 
> That's not quite what I'm suggesting. I'm saying that IsX86_MMXType should 
> take a QualType parameter, and it should check if that qualtype looks like 
> the __m64 vector type, instead of converting the QualType to llvm::Type and 
> then checking if the llvm::Type is a 64-bit vector. Does that seem 
> reasonable? See the code near the call site conditionalized on 
> IsDarwinVectorABI which already has similar logic.
Yes, it's unnecessary to convert QualType to llvm::Type just for the _m64 
vector type checking.
Since It's very simple to check _m64 type based on QualType with 
pre-conditioned type assertion 
```
if (const VectorType *VT = RetTy->getAs())
```
I just remove the utility function: IsX86_MMXType.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59744/new/

https://reviews.llvm.org/D59744



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

[PATCH] D60748: Fix i386 struct and union parameter alignment

[Wei Xiao via Phabricator via cfe-commits]

wxiao3 added a comment.

Any other comments?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60748/new/

https://reviews.llvm.org/D60748



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

[PATCH] D60748: Fix i386 struct and union parameter alignment

[Dimitry Andric via Phabricator via cfe-commits]

dim added subscribers: emaste, dim.
dim added a comment.

Please also exclude FreeBSD from these changes, since we care a lot about 
backwards compatibility, and specifically about alignment requirements.  (We 
have run into many issues in our ports collection where upstream assumes 
everything is 16-byte aligned on i386, which is *NOT* ABI compliant.)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60748/new/

https://reviews.llvm.org/D60748



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

[PATCH] D60748: Fix i386 struct and union parameter alignment

[Dimitry Andric via Phabricator via cfe-commits]

dim added a comment.

In fact, it is probably better to turn the OS check around, e.g. *only* 
increase the alignment for Linux, and nowhere else.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60748/new/

https://reviews.llvm.org/D60748



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

[PATCH] D61851: [clang-tidy] New option for misc-throw-by-value-catch-by-reference

[Balogh, Ádám via Phabricator via cfe-commits]

baloghadamsoftware created this revision.
baloghadamsoftware added reviewers: alexfh, aaron.ballman, lebedev.ri.
baloghadamsoftware added a project: clang-tools-extra.
Herald added subscribers: gamesh411, Szelethus, rnkovacs.
Herald added a project: clang.

Catching trivial objects by value is not dangerous but may be inefficient if 
they are too large. This patch adds an option `WarnOnLargeObject` to the 
checker to also warn if such an object is caught by value. An object is 
considered as "large" if its size is greater than `MaxSize` which is another 
option. Default value is the machine word of the architecture (size of the type 
`size_t`).


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D61851

Files:
  clang-tidy/misc/ThrowByValueCatchByReferenceCheck.cpp
  clang-tidy/misc/ThrowByValueCatchByReferenceCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/misc-throw-by-value-catch-by-reference.rst
  
test/clang-tidy/misc-throw-by-value-catch-by-reference-warn-on-large-object.cpp

Index: test/clang-tidy/misc-throw-by-value-catch-by-reference-warn-on-large-object.cpp
===
--- /dev/null
+++ test/clang-tidy/misc-throw-by-value-catch-by-reference-warn-on-large-object.cpp
@@ -0,0 +1,36 @@
+// RUN: %check_clang_tidy %s misc-throw-by-value-catch-by-reference %t -- -config="{CheckOptions: [{key: misc-throw-by-value-catch-by-reference.WarnOnLargeObject, value: 1}]}" -- -std=c++11 -fcxx-exceptions
+
+struct SmallType {
+  long n;
+};
+
+struct LargeType {
+  long v[255];
+};
+
+void testThrowFunc() {
+  throw SmallType();
+  throw LargeType();
+}
+
+void catchSmall() {
+  try {
+testThrowFunc();
+  } catch (SmallType s) {
+  }
+}
+
+void catchLarge() {
+  try {
+testThrowFunc();
+  } catch (LargeType l) {
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: catch handler catches by value; should catch by reference instead [misc-throw-by-value-catch-by-reference]
+  }
+}
+
+void catchByReference() {
+  try {
+testThrowFunc();
+  } catch (LargeType &l) {
+  }
+}
Index: docs/clang-tidy/checks/misc-throw-by-value-catch-by-reference.rst
===
--- docs/clang-tidy/checks/misc-throw-by-value-catch-by-reference.rst
+++ docs/clang-tidy/checks/misc-throw-by-value-catch-by-reference.rst
@@ -32,3 +32,15 @@
`_.
Default is `1`.
 
+.. option:: WarnOnLargeObject
+
+   Also warns for any large trivial object caught by value. Catching a large
+   object by value is not dangerous but affects the perofrmance negatively. The
+   maximum size of an object allowed to be caught without warning can be set
+   using option `MaxSize`
+   Default is `0`.
+
+.. option:: MaxSize
+
+   Determines the maximum size of an object allowed to be caught without
+   warning. Only applicable if `WarnOnLargeObject` is set to `1`.
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -179,6 +179,11 @@
   but either don't specify it or the clause is specified but with the kind
   other than ``none``, and suggests to use the ``default(none)`` clause.
 
+- New options `WarnOnLargeObject` and `MaxSize` for check
+  :doc:`misc-throw-by-value-catch-by-reference
+  ` check to warn
+  on any large trivial object caught by value.
+
 Improvements to clang-include-fixer
 ---
 
Index: clang-tidy/misc/ThrowByValueCatchByReferenceCheck.h
===
--- clang-tidy/misc/ThrowByValueCatchByReferenceCheck.h
+++ clang-tidy/misc/ThrowByValueCatchByReferenceCheck.h
@@ -41,6 +41,8 @@
   bool isCatchVariable(const DeclRefExpr *declRefExpr);
   bool isFunctionOrCatchVar(const DeclRefExpr *declRefExpr);
   const bool CheckAnonymousTemporaries;
+  const bool WarnOnLargeObject;
+  uint64_t MaxSize; // No `const` bacause we have to set it in two steps.
 };
 
 } // namespace misc
Index: clang-tidy/misc/ThrowByValueCatchByReferenceCheck.cpp
===
--- clang-tidy/misc/ThrowByValueCatchByReferenceCheck.cpp
+++ clang-tidy/misc/ThrowByValueCatchByReferenceCheck.cpp
@@ -20,7 +20,10 @@
 ThrowByValueCatchByReferenceCheck::ThrowByValueCatchByReferenceCheck(
 StringRef Name, ClangTidyContext *Context)
 : ClangTidyCheck(Name, Context),
-  CheckAnonymousTemporaries(Options.get("CheckThrowTemporaries", true)) {}
+  CheckAnonymousTemporaries(Options.get("CheckThrowTemporaries", true)),
+  WarnOnLargeObject(Options.get("WarnOnLargeObject", false)),
+  // Cannot access `ASTContext` from here so set it to an extremal value
+  MaxSize(Options.get("MaxSize", std::numeric_limits::max())) {}
 
 void ThrowByValueCatchByReferenceCheck::registerMatchers(MatchFinder *Finder) {

[PATCH] D61497: [clangd] Introduce a structured hover response

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet marked 16 inline comments as done.
kadircet added a comment.

Thanks for also taking a look at the implementation, it is not complete yet. I 
am rather waiting for a green light on struct itself, so that I can write tests.




Comment at: clang-tools-extra/clangd/XRefs.cpp:726
+llvm::raw_string_ostream OS(*P.Type);
+printSpecifiers(OS, PVD);
+PVD->getType().print(OS, Policy);

sammccall wrote:
> what are the storage class specifiers that are relevant to function 
> parameters?
i suppose only "register" is allowed in the context of a function parameter. 
but doesn't matter now as requested above, leaving specifiers out.



Comment at: clang-tools-extra/clangd/XRefs.cpp:1209
+else {
+  if (TemplateParameters) {
+Output.emplace_back();

sammccall wrote:
> why do we have this no-definition case?
> Please avoid reinventing code to print C++ syntax if possible...
oops, this branch is a leftover :/



Comment at: clang-tools-extra/clangd/XRefs.h:89
+  llvm::Optional> Parameters;
+  /// Set for all template declarations(function, class, variable).
+  llvm::Optional> TemplateParameters;

sammccall wrote:
> nit: just "templates"?
> 
> (e.g. if we hover over a call to std::swap, we might be showing the 
> instantiation rather than the declaration?)
Existing hover behavior is to show declaration, except auto, in that case we 
might show instantiations.

But you are right it is not necessarily only declarations.



Comment at: clang-tools-extra/clangd/XRefs.h:93
+  /// Lower to LSP struct.
+  Hover render() const;
+};

sammccall wrote:
> Even if this struct ends up containing the range, it might be more obvious to 
> have render() produce the `MarkupContent` only, leaving the caller to pull 
> out the range themselves.
> 
> Converting the range isn't closely related to rendering, and I think for 
> cleanest layering/testing you want to return `FormattedString` after rL360151 
> (which doesn't have a `Hover` equivalent).
I don't think caller of this method would have enough context to deduce 
symbol's range so leaving the `Range` in the struct, but changing the output



Comment at: clang-tools-extra/clangd/unittests/XRefsTests.cpp:940
   )cpp",
-  "class std::initializer_list",
+  "template<> class initializer_list {}",
   },

sammccall wrote:
> hmm, this is a bit weird - this uses specialization syntax but there's no 
> actual specialization here right?
> I think the old output without `template<>` is probably better if possible.
The old behavior was inconsistent in the case of auto. We print the decl in all 
cases, but print the type in the case of auto. For example, if you had 
`initializer_list i = {1,2}` instead of `auto i = {1,2}` you would get the 
new response I've provided in this test.

I agree this looks weird in the case of instantiations but I believe it is more 
important to give a consistent look.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61497/new/

https://reviews.llvm.org/D61497



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

[PATCH] D61497: [clangd] Introduce a structured hover response

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 199257.
kadircet marked 2 inline comments as done.
kadircet added a comment.

- Address comments


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61497/new/

https://reviews.llvm.org/D61497

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/XRefs.h
  clang-tools-extra/clangd/unittests/XRefsTests.cpp

Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -937,7 +937,7 @@
   ^auto i = {1,2};
 }
   )cpp",
-  "class std::initializer_list",
+  "template<> class initializer_list {}",
   },
   {
   R"cpp(// User defined conversion to auto
@@ -1012,7 +1012,7 @@
   return Bar();
 }
   )cpp",
-  "struct Bar",
+  "struct Bar {}",
   },
   {
   R"cpp(// trailing return type
@@ -1021,7 +1021,7 @@
   return Bar();
 }
   )cpp",
-  "struct Bar",
+  "struct Bar {}",
   },
   {
   R"cpp(// auto in function return
@@ -1030,7 +1030,7 @@
   return Bar();
 }
   )cpp",
-  "struct Bar",
+  "struct Bar {}",
   },
   {
   R"cpp(// auto& in function return
@@ -1039,7 +1039,7 @@
   return Bar();
 }
   )cpp",
-  "struct Bar",
+  "struct Bar {}",
   },
   {
   R"cpp(// auto* in function return
@@ -1049,7 +1049,7 @@
   return bar;
 }
   )cpp",
-  "struct Bar",
+  "struct Bar {}",
   },
   {
   R"cpp(// const auto& in function return
@@ -1058,7 +1058,7 @@
   return Bar();
 }
   )cpp",
-  "struct Bar",
+  "struct Bar {}",
   },
   {
   R"cpp(// decltype(auto) in function return
@@ -1067,7 +1067,7 @@
   return Bar();
 }
   )cpp",
-  "struct Bar",
+  "struct Bar {}",
   },
   {
   R"cpp(// decltype(auto) reference in function return
@@ -1136,7 +1136,7 @@
   ^decltype(test()) i = test();
 }
   )cpp",
-  "struct Bar",
+  "struct Bar {}",
   },
   {
   R"cpp(// decltype of var with decltype.
@@ -1185,7 +1185,7 @@
 auto AST = TU.build();
 if (auto H = getHover(AST, T.point())) {
   EXPECT_NE("", Test.ExpectedHover) << Test.Input;
-  EXPECT_EQ(H->contents.value, Test.ExpectedHover.str()) << Test.Input;
+  EXPECT_EQ(H->render().value, Test.ExpectedHover.str()) << Test.Input;
 } else
   EXPECT_EQ("", Test.ExpectedHover.str()) << Test.Input;
   }
Index: clang-tools-extra/clangd/XRefs.h
===
--- clang-tools-extra/clangd/XRefs.h
+++ clang-tools-extra/clangd/XRefs.h
@@ -16,7 +16,10 @@
 #include "ClangdUnit.h"
 #include "Protocol.h"
 #include "index/Index.h"
+#include "index/SymbolLocation.h"
+#include "clang/Index/IndexSymbol.h"
 #include "llvm/ADT/Optional.h"
+#include "llvm/Support/raw_ostream.h"
 #include 
 
 namespace clang {
@@ -46,8 +49,63 @@
 std::vector findDocumentHighlights(ParsedAST &AST,
   Position Pos);
 
+/// Contains detailed information about a Symbol. Especially useful when
+/// generating hover responses. It can be rendered as a hover panel, or
+/// embedding clients can use the structured information to provide their own
+/// UI.
+struct HoverInfo {
+  /// Represents parameters of a function, a template or a macro.
+  /// For example:
+  /// - void foo(ParamType Name = DefaultValue)
+  /// - #define FOO(Name)
+  /// - template  class Foo {};
+  struct Param {
+/// The pretty-printed parameter type, e.g. "int", or "typename" (in
+/// TemplateParameters)
+llvm::Optional Type;
+/// None for unnamed parameters.
+llvm::Optional Name;
+/// None if no default is provided.
+llvm::Optional Default;
+  };
+
+  /// For a variable named Bar, declared in clang::clangd::Foo::getFoo the
+  /// following fields will hold:
+  /// - NamespaceScope: clang::clangd::
+  /// - LocalScope: Foo::getFoo::
+  /// - Name: Bar
+
+  /// Contains all of the enclosing namespaces.
+  std::string NamespaceScope;
+  /// Remaining named contexts in symbol's qualified name.
+  std::string LocalScope;
+  /// Name of the symbol, does not contain any "::".
+  std::string Name;
+  llvm::Optional SymRange;
+  /// Scope containing the symbol. e.g, "global namespace", "function x::Y"
+  /// - None for deduced types, e.g "auto", 

r360580 - Revert r360559 "[c++20] P1064R0: Allow virtual function calls in constant expression evaluation."

[Hans Wennborg via cfe-commits]

Author: hans
Date: Mon May 13 06:19:09 2019
New Revision: 360580

URL: http://llvm.org/viewvc/llvm-project?rev=360580&view=rev
Log:
Revert r360559 "[c++20] P1064R0: Allow virtual function calls in constant 
expression evaluation."

This caused Chromium builds to hit the new "can't handle virtual calls with
virtual bases" assert. Reduced repro coming up.

Modified:
cfe/trunk/include/clang/AST/DeclCXX.h
cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/AST/DeclCXX.cpp
cfe/trunk/lib/AST/ExprConstant.cpp
cfe/trunk/lib/Sema/SemaDeclCXX.cpp
cfe/trunk/lib/Sema/SemaTemplateInstantiate.cpp
cfe/trunk/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp
cfe/trunk/test/CXX/drs/dr18xx.cpp
cfe/trunk/test/CXX/drs/dr6xx.cpp
cfe/trunk/test/SemaCXX/constant-expression-cxx2a.cpp
cfe/trunk/test/SemaCXX/cxx17-compat.cpp
cfe/trunk/www/cxx_status.html

Modified: cfe/trunk/include/clang/AST/DeclCXX.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclCXX.h?rev=360580&r1=360579&r2=360580&view=diff
==
--- cfe/trunk/include/clang/AST/DeclCXX.h (original)
+++ cfe/trunk/include/clang/AST/DeclCXX.h Mon May 13 06:19:09 2019
@@ -2298,17 +2298,6 @@ public:
   ->getCorrespondingMethodInClass(RD, MayBeBase);
   }
 
-  /// Find if \p RD declares a function that overrides this function, and if 
so,
-  /// return it. Does not search base classes.
-  CXXMethodDecl *getCorrespondingMethodDeclaredInClass(const CXXRecordDecl *RD,
-   bool MayBeBase = false);
-  const CXXMethodDecl *
-  getCorrespondingMethodDeclaredInClass(const CXXRecordDecl *RD,
-bool MayBeBase = false) const {
-return const_cast(this)
-->getCorrespondingMethodDeclaredInClass(RD, MayBeBase);
-  }
-
   // Implement isa/cast/dyncast/etc.
   static bool classof(const Decl *D) { return classofKind(D->getKind()); }
   static bool classofKind(Kind K) {

Modified: cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td?rev=360580&r1=360579&r2=360580&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td Mon May 13 06:19:09 2019
@@ -32,8 +32,6 @@ def note_constexpr_no_return : Note<
   "control reached end of constexpr function">;
 def note_constexpr_virtual_call : Note<
   "cannot evaluate call to virtual function in a constant expression">;
-def note_constexpr_pure_virtual_call : Note<
-  "pure virtual function %q0 called">;
 def note_constexpr_virtual_base : Note<
   "cannot construct object of type %0 with virtual base class "
   "in a constant expression">;

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=360580&r1=360579&r2=360580&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Mon May 13 06:19:09 
2019
@@ -2314,9 +2314,6 @@ def err_constexpr_redecl_mismatch : Erro
   "%select{non-constexpr declaration of %0 follows constexpr declaration"
   "|constexpr declaration of %0 follows non-constexpr declaration}1">;
 def err_constexpr_virtual : Error<"virtual function cannot be constexpr">;
-def warn_cxx17_compat_constexpr_virtual : Warning<
-  "virtual constexpr functions are incompatible with "
-  "C++ standards before C++2a">, InGroup, DefaultIgnore;
 def err_constexpr_virtual_base : Error<
   "constexpr %select{member function|constructor}0 not allowed in "
   "%select{struct|interface|class}1 with virtual base "

Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=360580&r1=360579&r2=360580&view=diff
==
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Mon May 13 06:19:09 2019
@@ -5985,8 +5985,8 @@ public:
 
   /// MarkVirtualMembersReferenced - Will mark all members of the given
   /// CXXRecordDecl referenced.
-  void MarkVirtualMembersReferenced(SourceLocation Loc, const CXXRecordDecl 
*RD,
-bool ConstexprOnly = false);
+  void MarkVirtualMembersReferenced(SourceLocation Loc,
+const CXXRecordDecl *RD);
 
   /// Define all of the vtables that have been used in this
   /// translation unit and reference any virtual members used by those

Modi

Re: r360559 - [c++20] P1064R0: Allow virtual function calls in constant expression

[Hans Wennborg via cfe-commits]

This caused asserts in Chromium, so I've reverted in r360580. There's
a repro at https://bugs.chromium.org/p/chromium/issues/detail?id=962458#c1,
and I'm working on a reduced version.

From: Richard Smith via cfe-commits 
Date: Mon, May 13, 2019 at 9:39 AM
To: 

> Author: rsmith
> Date: Mon May 13 00:42:10 2019
> New Revision: 360559
>
> URL: http://llvm.org/viewvc/llvm-project?rev=360559&view=rev
> Log:
> [c++20] P1064R0: Allow virtual function calls in constant expression
> evaluation.
>
> Modified:
> cfe/trunk/include/clang/AST/DeclCXX.h
> cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td
> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> cfe/trunk/include/clang/Sema/Sema.h
> cfe/trunk/lib/AST/DeclCXX.cpp
> cfe/trunk/lib/AST/ExprConstant.cpp
> cfe/trunk/lib/Sema/SemaDeclCXX.cpp
> cfe/trunk/lib/Sema/SemaTemplateInstantiate.cpp
> cfe/trunk/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp
> cfe/trunk/test/CXX/drs/dr18xx.cpp
> cfe/trunk/test/CXX/drs/dr6xx.cpp
> cfe/trunk/test/SemaCXX/constant-expression-cxx2a.cpp
> cfe/trunk/test/SemaCXX/cxx17-compat.cpp
> cfe/trunk/www/cxx_status.html
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53866: [Preamble] Stop circular inclusion of main file when building preamble

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

I guess using `Edit Related Object -> Edit Commits` should do the trick.
I'm not sure what the "Lean Into Action" is either.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D53866/new/

https://reviews.llvm.org/D53866



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

[PATCH] D53023: Prototype OpenCL BIFs using Tablegen

[Alexey Sotkin via Phabricator via cfe-commits]

AlexeySotkin added a comment.

LGTM! @joey, any idea when it will be landed?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D53023/new/

https://reviews.llvm.org/D53023



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

[PATCH] D53023: Prototype OpenCL BIFs using Tablegen

[Sven van Haastregt via Phabricator via cfe-commits]

svenvh added a comment.

In D53023#1499942 , @AlexeySotkin 
wrote:

> LGTM! @joey, any idea when it will be landed?


This work is being continued in https://reviews.llvm.org/D60763


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D53023/new/

https://reviews.llvm.org/D53023



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

r360539 - make -ftime-trace also print template arguments

[Lubos Lunak via cfe-commits]

Author: llunak
Date: Sun May 12 03:39:21 2019
New Revision: 360539

URL: http://llvm.org/viewvc/llvm-project?rev=360539&view=rev
Log:
make -ftime-trace also print template arguments

Without this, I get e.g. 'PerformPendingInstantiations' -> 'std::fill',
now I get 'std::fill'.

Differential Revision: https://reviews.llvm.org/D61822

Modified:
cfe/trunk/lib/CodeGen/CodeGenModule.cpp
cfe/trunk/lib/Sema/SemaTemplateInstantiate.cpp
cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp

Modified: cfe/trunk/lib/CodeGen/CodeGenModule.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenModule.cpp?rev=360539&r1=360538&r2=360539&view=diff
==
--- cfe/trunk/lib/CodeGen/CodeGenModule.cpp (original)
+++ cfe/trunk/lib/CodeGen/CodeGenModule.cpp Sun May 12 03:39:21 2019
@@ -2695,8 +2695,13 @@ void CodeGenModule::EmitGlobalDefinition
 if (!shouldEmitFunction(GD))
   return;
 
-llvm::TimeTraceScope TimeScope(
-"CodeGen Function", [&]() { return FD->getQualifiedNameAsString(); });
+llvm::TimeTraceScope TimeScope("CodeGen Function", [&]() {
+  std::string Name;
+  llvm::raw_string_ostream OS(Name);
+  FD->getNameForDiagnostic(OS, getContext().getPrintingPolicy(),
+   /*Qualified=*/true);
+  return Name;
+});
 
 if (const auto *Method = dyn_cast(D)) {
   // Make sure to emit the definition(s) before we emit the thunks.

Modified: cfe/trunk/lib/Sema/SemaTemplateInstantiate.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplateInstantiate.cpp?rev=360539&r1=360538&r2=360539&view=diff
==
--- cfe/trunk/lib/Sema/SemaTemplateInstantiate.cpp (original)
+++ cfe/trunk/lib/Sema/SemaTemplateInstantiate.cpp Sun May 12 03:39:21 2019
@@ -2014,7 +2014,11 @@ Sema::InstantiateClass(SourceLocation Po
 return true;
 
   llvm::TimeTraceScope TimeScope("InstantiateClass", [&]() {
-return Instantiation->getQualifiedNameAsString();
+std::string Name;
+llvm::raw_string_ostream OS(Name);
+Instantiation->getNameForDiagnostic(OS, getPrintingPolicy(),
+/*Qualified=*/true);
+return Name;
   });
 
   Pattern = PatternDef;

Modified: cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp?rev=360539&r1=360538&r2=360539&view=diff
==
--- cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp (original)
+++ cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp Sun May 12 03:39:21 2019
@@ -4156,7 +4156,11 @@ void Sema::InstantiateFunctionDefinition
   }
 
   llvm::TimeTraceScope TimeScope("InstantiateFunction", [&]() {
-return Function->getQualifiedNameAsString();
+std::string Name;
+llvm::raw_string_ostream OS(Name);
+Function->getNameForDiagnostic(OS, getPrintingPolicy(),
+   /*Qualified=*/true);
+return Name;
   });
 
   // If we're performing recursive template instantiation, create our own


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

[PATCH] D61634: [clang/llvm] Allow efficient implementation of libc's memory functions in C/C++

[Guillaume Chatelet via Phabricator via cfe-commits]

gchatelet added a comment.

In D61634#1498376 , @efriedma wrote:

> I still think there are really two things you're trying to accomplish here, 
> which should be handled separately.
>
> 1. Add a function attribute that works like -fno-builtin-memcpy currently 
> does, to prevent the compiler from synthesizing calls to memcpy.
> 2. Provide a convenient way to write a small, fixed-length "memcpy", in C and 
> in LLVM IR, that is always expanded to optimal straight-line code (without 
> any calls or loops).
>
>   These correspond to proposals (1) and (2) from your RFC; I think we need 
> both to arrive at a reasonable final state.
>
>   (The "rep; movs" is a third thing, which I think should also be handled 
> separately, but it sounds like you don't think that's very important.)


Thank you for taking the time to comment, your feedback is highly appreciated.

I understand that your main concern is about conflating two orthogonal 
requirements (namely 1. and 2.) in a single attribute. Is that correct?
From my point of view, the RFC (and this Patch) really is about 1. - because 2. 
can already be achieved with builtins `__builtin_memcpy(dst, src, )`.

My secondary goals in decreasing priority order are:

- do not change the semantic of current builtins,
- create a relatively straightforward LLVM patch that does not touch too much 
code and that is easy review,
- do not add more confusion around `-fno-builtin-*` meaning,
- do not add more builtins or IR intrinsics.

What is the main blocker on your end? What would you suggest so we can move 
forward?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61634/new/

https://reviews.llvm.org/D61634



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

[PATCH] D61765: [OpenMP][Clang][BugFix] Split declares and math functions inclusion.

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

Two small changes and then it is fine with me. @tra ?

1. we need to use ifdef to not define clock
2. Can you switch the include order in 
`test/Headers/nvptx_device_math_functions.cpp`?

P.S. I'm currently at the OpenMP standard meeting to get the OpenMP variants 
fixed.
Once done, we should prioritize the implementation.
Excluding non-math functions in the cuda headers is not perfect...


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61765/new/

https://reviews.llvm.org/D61765



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

Re: r360452 - Replace 'REQUIRES: nozlib' with '!zlib' because we don't need two ways

[David Blaikie via cfe-commits]

What's the practical difference between "UNSUPPORTED: foo" and "REQUIRES:
!foo"? (I see some of the fixes you've made go one way, some the other way)

On Fri, May 10, 2019 at 11:30 AM Paul Robinson via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: probinson
> Date: Fri May 10 11:32:53 2019
> New Revision: 360452
>
> URL: http://llvm.org/viewvc/llvm-project?rev=360452&view=rev
> Log:
> Replace 'REQUIRES: nozlib' with '!zlib' because we don't need two ways
> to say the same thing.
>
> Modified:
> cfe/trunk/test/Driver/nozlibcompress.c
>
> Modified: cfe/trunk/test/Driver/nozlibcompress.c
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/nozlibcompress.c?rev=360452&r1=360451&r2=360452&view=diff
>
> ==
> --- cfe/trunk/test/Driver/nozlibcompress.c (original)
> +++ cfe/trunk/test/Driver/nozlibcompress.c Fri May 10 11:32:53 2019
> @@ -1,4 +1,4 @@
> -// REQUIRES: nozlib
> +// REQUIRES: !zlib
>
>  // RUN: %clang -### -fintegrated-as -gz -c %s 2>&1 | FileCheck %s
> -check-prefix CHECK-WARN
>  // RUN: %clang -### -fintegrated-as -gz=none -c %s 2>&1 | FileCheck
> -allow-empty -check-prefix CHECK-NOWARN %s
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r360559 - [c++20] P1064R0: Allow virtual function calls in constant expression

[Hans Wennborg via cfe-commits]

Here's a creduced repro:

--
class a {};
class b : virtual a {
  virtual bool c(const void *, int);
};
class C : b {
public:
  bool c(const void *, int);
};
int d;
bool e() {
  C f;
  if (f.c(&d, d))
;
}
--

$ clang.bad -cc1 -triple x86_64-unknown-linux-gnu -fsyntax-only -std=c++14 a.cc

The assert goes away if invoking clang with -w.

From: Hans Wennborg 
Date: Mon, May 13, 2019 at 3:16 PM
To: Richard Smith
Cc: cfe-commits

> This caused asserts in Chromium, so I've reverted in r360580. There's
> a repro at https://bugs.chromium.org/p/chromium/issues/detail?id=962458#c1,
> and I'm working on a reduced version.
>
> From: Richard Smith via cfe-commits 
> Date: Mon, May 13, 2019 at 9:39 AM
> To: 
>
> > Author: rsmith
> > Date: Mon May 13 00:42:10 2019
> > New Revision: 360559
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=360559&view=rev
> > Log:
> > [c++20] P1064R0: Allow virtual function calls in constant expression
> > evaluation.
> >
> > Modified:
> > cfe/trunk/include/clang/AST/DeclCXX.h
> > cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td
> > cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> > cfe/trunk/include/clang/Sema/Sema.h
> > cfe/trunk/lib/AST/DeclCXX.cpp
> > cfe/trunk/lib/AST/ExprConstant.cpp
> > cfe/trunk/lib/Sema/SemaDeclCXX.cpp
> > cfe/trunk/lib/Sema/SemaTemplateInstantiate.cpp
> > cfe/trunk/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp
> > cfe/trunk/test/CXX/drs/dr18xx.cpp
> > cfe/trunk/test/CXX/drs/dr6xx.cpp
> > cfe/trunk/test/SemaCXX/constant-expression-cxx2a.cpp
> > cfe/trunk/test/SemaCXX/cxx17-compat.cpp
> > cfe/trunk/www/cxx_status.html
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61724: [clangd] Use AsyncTaskRunner in BackgroundIndex instead of std::thread

[Brennan Vincent via Phabricator via cfe-commits]

umanwizard added inline comments.



Comment at: clang-tools-extra/trunk/clangd/index/Background.cpp:164
   stop();
-  for (auto &Thread : ThreadPool)
-Thread.join();
+  ThreadPool.wait();
 }

This is already called in `~AsyncTaskRunner`.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61724/new/

https://reviews.llvm.org/D61724



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

[PATCH] D61724: [clangd] Use AsyncTaskRunner in BackgroundIndex instead of std::thread

[Brennan Vincent via Phabricator via cfe-commits]

umanwizard added a comment.

Ignore me. I didn't read the previous discussion before commenting.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61724/new/

https://reviews.llvm.org/D61724



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

[PATCH] D61804: Support building shared and static libraries simultaneously

[Tom Stellard via Phabricator via cfe-commits]

tstellar added a comment.

Not sure if this would be relevant for your use case, but it would be really 
nice to have a libCLANG.so with all the C++ symbols, like we do for llvm.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61804/new/

https://reviews.llvm.org/D61804



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

[PATCH] D57858: [analyzer] Add a new frontend flag to display all checker options

[Daniel Krupp via Phabricator via cfe-commits]

dkrupp added a comment.

In D57858#1499414 , @Szelethus wrote:

> In D57858#1498640 , @NoQ wrote:
>
> > So, like, the global picture is as follows. In our case the Driver (i.e., 
> > --analyze) is not much more user facing than frontend flags. It's still 
> > fairly unusable directly, as our Static Analyzer is generally not a 
> > command-line tool. The real user-facing stuff is the GUIs such as 
> > scan-build or CodeChecker. These GUIs decide themselves on what options 
> > they want to expose. For instance, you have a right to decide that 
> > CodeChecker shouldn't support the aggressive mode of the move-checker and 
> > don't expose it as an option in your GUI. In this sense it's not really 
> > useful to provide a centralized `-help` of all user-facing options.
> >
> > But it sounds as if you want to change this situation and provide a single 
> > source of truth on what are the user-facing options. Particular GUIs can 
> > still ignore them, but you don't want to hardcode flags in CodeChecker, but 
> > instead you want to rely on clang to provide the list of supported options 
> > for you and, as a side effect, for scan-build users (if you also add a 
> > scan-build help flag). I'm totally in favor of crystallizing such list of 
> > user-facing flags, and this patch sounds like a good step in that 
> > direction, assuming that non-user-facing options are hidden.
>
>
> That describes my intention quite well :)
>
> > I'm still in favor of hiding alpha checkers (as they are for development 
> > only, much like debug flags; i'd recommend hiding them in the CodeChecker 
> > UI as well)
> > 
> > Now, why do we care about frontend/driver flags when they're unusable by 
> > definition? That's because we have a mental trauma after seeing a few 
> > powerusers actively explore those flags, observe that they don't work, and 
> > then tell everybody that the Analyzer is broken. So there's a threshold, 
> > based on a tiny but painful bit of practical experience, that says that 
> > documentation of developer-only features on llvm.org or in code comments is 
> > fine, but documentation printed by the released binary itself is not fine.
>
> What you say sounds very reasonable. Still, I am kind of hesitant about 
> hiding all alpha checkers: I initially intended to hide only are 
> developer-only checkers (modeling, debug). I guess if we define alpha 
> checkers (as you stated numerous times) as incomplete, under development, are 
> missing half their limbs and crash if you look at them the wrong way, sure, 
> they belong in the developer-only category. But checkers such as mine 
> (UninitializedObjectChecker), for the longest time were very stable, have 
> been enabled by default for our internal projects, despite only recently 
> moving out of alpha.
>
> Then agaaain, if we're that stubborn about alpha checkers, we could might as 
> well dig them out of `-analyzer-checker-help-hidden`, and leave the rest 
> there. Untangling what alpha checkers depend on one another could be solved 
> by making yet another frontend flag that would display checker dependencies, 
> which would be super easy since D54438 , or 
> create an `-analyzer-checker-help-alpha` flag that would display alpha, but 
> not developer-only checkers. @dkrupp @o.gyorgy Do you have any feelings on 
> this?
>
> > and we should probably automatically hide options of checker that are 
> > hidden.
>
> Checker options are a different kind of animal entirely. I think we should 
> definitely let the user fine-tune some modeling checkers, for instance, 
> `unix.DynamicMemoryModeling:Optimistic`, despite us not really wanting to let 
>  anyone (even developers really) mess around whether 
> `unix.DynamicMemoryModeling` should be enabled. While that specific option 
> is, to put it nicely, a little esoteric, making some decisions the analyzer 
> makes less conservative, or limiting state splits to help performance may be 
> desirable in the future.
>
> Let's move the rest of the discussion directly related to hiding checker 
> options to D61839 !


Yes, it would be great if the clang static analyzer would be the ultimate 
source of information with respect to the checkers and checker options. Then we 
would not need to split this info between the front-end (scanbuild, 
codechecker) and the analyzer.

Some alpha checkers are considerably more mature than others and are quite 
usable. In our experience, there are some users who are keen to run these 
checkers on their code and report back any false positives to us. So in this 
sense these are not "developer only" checkers. So I think we should let the 
users list them, read their descriptions and try them out. Some of them will 
come back with useful feedback as to how to improve them further. Some users 
would not care if the checker gives some more false positives 

[PATCH] D61804: Support building shared and static libraries simultaneously

[Chris Bieneman via Phabricator via cfe-commits]

beanz added a comment.

In D61804#1499275 , @winksaville wrote:

> When you say you don't think the build system should do this, what do you 
> mean?


`llvm_add_library` supports both, because there are places where we consciously 
choose to build both. That's not the problem. What I don't think the build 
system should do, is take an unsupported distribution format (which is 
unsupported for technical reasons), and make it supported without resolving the 
technical issues.

> With the change I've made when both are specified the default cmake entity, 
> ${name}, is the static libraries. When neither are specified the default is 
> static or shared depending on BUILD_SHARED_LIBS which is unchanged.

I missed that you were renaming the targets before calling `llvm_add_library` 
so that isn't an issue, but there are still lots of issues here. Like, if you 
specify your new option to generate clang static libraries but don't specify 
`BUILD_SHARED_LIBS` or `LLVM_USE_LLVM_DYLIB` each of your clang libraries will 
get statically linked against its own copy of LLVM, which not only makes your 
libraries big, it also makes them not work. You can't link more than one copy 
of LLVM into a single process memory space safely because of LLVM's use of 
global data.

> The problem is that some people want to use shared libraries and some want 
> static.

Why? I generally think most people don't understand the trade-offs, and I'm not 
sure we should feed into that.

> I'm trying to allow both to be built and ask the Arch Linux people to 
> distribute both.

Arch Linux is in an unsupported state, and your patch isn't the way forward.

Let's rewind.

Why do "some people like shared libraries"? There are usually two reasons 
people cite for linking shared libraries. One is reduced binary distribution 
size because you're reducing duplication of code. The other, is the ability to 
update libraries without updating tools.

The later, is not an issue here. LLVM and Clang's C++ libraries don't provide 
any API or ABI stability guarantees across releases, so there is no way you're 
going to reasonably update LLVM or Clang libraries without rebuilding the 
applications using them.

The former can be valuable, but it comes at a cost. Dynamic resolution of 
symbols slows down process launch time, and dynamic resolution of C++ link-once 
ODRs are exceptionally slow. If you are building a compiler, chances are you 
don't want to slow down your process launch that much. But let's pretend you 
don't care. Your solution still isn't the right way to do this.

Creating component shared libraries results in duplicating all the C++ 
link-once ODRs across each shared module. That will not only slow down your 
process launch, but bloat the size of your binary distribution.

In D61804#1499276 , @winksaville wrote:

> It looks to me that llvm, libunwind and libcxx support building both, so the 
> goal isn't unprecedented,


This is very much an apple and oranges comparison. For one, LLVM does not 
support generating component dylibs and shared libs. It supports generating 
libLLVM, a single dylib containing all of the LLVM components rolled together. 
libunwind, libcxx, libcxxabi, are runtime libraries that are designed to allow 
static linkage, and they don't link LLVM.

I apologize that I missed your thread on the dev list, because that would have 
been a much better place to have this conversation. Having gone back and read 
it now, it sounds to me like what you really want is a clang equivalent of 
libLLVM. That is a reasonable and viable thing to want. This patch 
(https://reviews.llvm.org/P8147) is a first stab. I'm happy to prepare that for 
landing in Clang if that meets your needs, and that is a viable way forward.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61804/new/

https://reviews.llvm.org/D61804



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

[PATCH] D60543: [clang] Update isDerivedFrom to support Objective-C classes 🔍

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D60543#1497576 , @stephanemoore 
wrote:

> I did some digging and I believe there are two approaches that we can take to 
> extend `isDerivedFrom` to support Objective-C classes.
>
> **Option 1: Match on Common Ancestor Declaration Type**:
>  Convert `isDerivedFrom` to match on the common ancestor declaration type, 
> `NamedDecl`, and return `false` for
>  declaration types other than `CXXRecordDecl` and `ObjCInterfaceDecl`.
>
> Advantages:
>  • Largely works in-place without requiring major changes to matchers built 
> on top of `isDerivedFrom`.
>
> Disadvantages:
>  • Allows nonsensical matchers, e.g., `functionDecl(isDerivedFrom("X"))`.
>
> **Option 2: Convert to Polymorphic Matcher**:
>  Convert `isDerivedFrom` to a polymorphic matcher supporting `CXXRecordDecl` 
> and `ObjCInterfaceDecl`.
>
> Advantages:
>  • Restricts usage of `isDerivedFrom` to `CXXRecordDecl` and 
> `ObjCInterfaceDecl`.
>
> Disadvantages:
>  • Potentially requires many or all matchers using `isDerivedFrom` to 
> refactor to adapt to the polymorphic matcher interface.
>
> **Evaluation**
>  I have been going back and forth as to which approach is superior. Option 1 
> promotes source stability at the cost of usability while
>  option 2 seems to promote usability at the cost of source stability. I 
> exported a portrayal of option 1 for consideration as it
>  required less invasive changes. I can see arguments supporting both 
> approaches.
>
> What do you think? Which of the two approaches do you think we should go 
> with? Is there another approach that I have not thought of?


This is a great breakdown, thank you for providing it!

Out of curiosity, how invasive is Option 2 within our own code base? Does that 
option require fixing a lot of code? Are the breakages loud or silent? Is the 
code easy to modify, or are there corner cases where this option becomes 
problematic? I prefer Option 2 because it is a cleaner, more understandable 
design for the matchers. If it turns out that this option causes a hard break 
(rather than silently changing matcher behavior) and the changes needed to get 
old code to compile again are minimal, I think it may be a reasonable choice.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60543/new/

https://reviews.llvm.org/D60543



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

[PATCH] D60543: [clang] Update isDerivedFrom to support Objective-C classes 🔍

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added reviewers: klimek, alexfh.
aaron.ballman added a comment.

Adding some more AST matcher reviewers to see if there are other options that 
we've not considered.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60543/new/

https://reviews.llvm.org/D60543



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

[PATCH] D61858: Make `__is_base_of` more friendly with unions

[Marshall Clow via Phabricator via cfe-commits]

mclow.lists created this revision.
mclow.lists added reviewers: rsmith, t.p.northover, EricWF, ldionne.
mclow.lists added a project: clang.
Herald added a subscriber: dexonsmith.

Unions are never base classes, and never have base classes.
It doesn't matter if they are complete or not. See http://llvm.org/PR41843


https://reviews.llvm.org/D61858

Files:
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/SemaCXX/type-traits.cpp


Index: clang/test/SemaCXX/type-traits.cpp
===
--- clang/test/SemaCXX/type-traits.cpp
+++ clang/test/SemaCXX/type-traits.cpp
@@ -14,6 +14,7 @@
 enum Enum { EV };
 struct POD { Enum e; int i; float f; NonPOD* p; };
 struct Empty {};
+struct IncompleteStruct;
 typedef Empty EmptyAr[10];
 typedef Empty EmptyArNB[];
 typedef Empty EmptyArMB[1][2];
@@ -1915,6 +1916,20 @@
   { int arr[F(__is_base_of(Base, NonderivedTemp))]; }
   { int arr[F(__is_base_of(Base, UndefinedTemp))]; } // expected-error 
{{implicit instantiation of undefined template 'UndefinedTemp'}}
 
+  { int arr[F(__is_base_of(IncompleteUnion, IncompleteUnion))]; }
+  { int arr[F(__is_base_of(Union, IncompleteUnion))]; }
+  { int arr[F(__is_base_of(IncompleteUnion, Union))]; }
+  { int arr[F(__is_base_of(IncompleteStruct, IncompleteUnion))]; }
+  { int arr[F(__is_base_of(IncompleteUnion, IncompleteStruct))]; }
+  { int arr[F(__is_base_of(Empty, IncompleteUnion))]; }
+  { int arr[F(__is_base_of(IncompleteUnion, Empty))]; }
+  { int arr[F(__is_base_of(int, IncompleteUnion))]; }
+  { int arr[F(__is_base_of(IncompleteUnion, int))]; }
+  { int arr[F(__is_base_of(Empty, Union))]; }
+  { int arr[F(__is_base_of(Union, Empty))]; }
+  { int arr[F(__is_base_of(int, Empty))]; }
+  { int arr[F(__is_base_of(Union, int))]; }
+
   isBaseOfT();
   isBaseOfF();
 
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -5118,8 +5118,15 @@
 assert(Self.Context.hasSameUnqualifiedType(LhsT, RhsT)
  == (lhsRecord == rhsRecord));
 
+// Unions are never base classes, and never have base classes.
+// It doesn't matter if they are complete or not. See PR#41843
+if (lhsRecord && lhsRecord->getDecl()->isUnion())
+  return false;
+if (rhsRecord && rhsRecord->getDecl()->isUnion())
+  return false;
+
 if (lhsRecord == rhsRecord)
-  return !lhsRecord->getDecl()->isUnion();
+  return true;
 
 // C++0x [meta.rel]p2:
 //   If Base and Derived are class types and are different types


Index: clang/test/SemaCXX/type-traits.cpp
===
--- clang/test/SemaCXX/type-traits.cpp
+++ clang/test/SemaCXX/type-traits.cpp
@@ -14,6 +14,7 @@
 enum Enum { EV };
 struct POD { Enum e; int i; float f; NonPOD* p; };
 struct Empty {};
+struct IncompleteStruct;
 typedef Empty EmptyAr[10];
 typedef Empty EmptyArNB[];
 typedef Empty EmptyArMB[1][2];
@@ -1915,6 +1916,20 @@
   { int arr[F(__is_base_of(Base, NonderivedTemp))]; }
   { int arr[F(__is_base_of(Base, UndefinedTemp))]; } // expected-error {{implicit instantiation of undefined template 'UndefinedTemp'}}
 
+  { int arr[F(__is_base_of(IncompleteUnion, IncompleteUnion))]; }
+  { int arr[F(__is_base_of(Union, IncompleteUnion))]; }
+  { int arr[F(__is_base_of(IncompleteUnion, Union))]; }
+  { int arr[F(__is_base_of(IncompleteStruct, IncompleteUnion))]; }
+  { int arr[F(__is_base_of(IncompleteUnion, IncompleteStruct))]; }
+  { int arr[F(__is_base_of(Empty, IncompleteUnion))]; }
+  { int arr[F(__is_base_of(IncompleteUnion, Empty))]; }
+  { int arr[F(__is_base_of(int, IncompleteUnion))]; }
+  { int arr[F(__is_base_of(IncompleteUnion, int))]; }
+  { int arr[F(__is_base_of(Empty, Union))]; }
+  { int arr[F(__is_base_of(Union, Empty))]; }
+  { int arr[F(__is_base_of(int, Empty))]; }
+  { int arr[F(__is_base_of(Union, int))]; }
+
   isBaseOfT();
   isBaseOfF();
 
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -5118,8 +5118,15 @@
 assert(Self.Context.hasSameUnqualifiedType(LhsT, RhsT)
  == (lhsRecord == rhsRecord));
 
+// Unions are never base classes, and never have base classes.
+// It doesn't matter if they are complete or not. See PR#41843
+if (lhsRecord && lhsRecord->getDecl()->isUnion())
+  return false;
+if (rhsRecord && rhsRecord->getDecl()->isUnion())
+  return false;
+
 if (lhsRecord == rhsRecord)
-  return !lhsRecord->getDecl()->isUnion();
+  return true;
 
 // C++0x [meta.rel]p2:
 //   If Base and Derived are class types and are different types
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61438: [ASTImporter] Use llvm::Expected and Error in the importer API

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

@aprantl Ping


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61438/new/

https://reviews.llvm.org/D61438



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

[PATCH] D61858: Make `__is_base_of` more friendly with unions

[Marshall Clow via Phabricator via cfe-commits]

mclow.lists added a comment.

Note: There are tabs in `clang/test/SemaCXX/type-traits.cpp` that I didn't 
remove because it would have cluttered up the diff.
I can de-tabify the file when it is committed if people want.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61858/new/

https://reviews.llvm.org/D61858



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

[PATCH] D59798: [analyzer] Add analyzer option to limit the number of imported TUs

[Endre Fülöp via Phabricator via cfe-commits]

gamesh411 updated this revision to Diff 199277.
gamesh411 added a comment.

Apply review suggestions by Xazax


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59798/new/

https://reviews.llvm.org/D59798

Files:
  include/clang/CrossTU/CrossTranslationUnit.h
  include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
  lib/CrossTU/CrossTranslationUnit.cpp
  lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
  test/Analysis/analyzer-config.c
  test/Analysis/ctu-import-threshold.c
  unittests/CrossTU/CrossTranslationUnitTest.cpp

Index: unittests/CrossTU/CrossTranslationUnitTest.cpp
===
--- unittests/CrossTU/CrossTranslationUnitTest.cpp
+++ unittests/CrossTU/CrossTranslationUnitTest.cpp
@@ -7,6 +7,7 @@
 //===--===//
 
 #include "clang/CrossTU/CrossTranslationUnit.h"
+#include "clang/Frontend/CompilerInstance.h"
 #include "clang/AST/ASTConsumer.h"
 #include "clang/Frontend/FrontendAction.h"
 #include "clang/Tooling/Tooling.h"
@@ -70,12 +71,14 @@
 EXPECT_TRUE(llvm::sys::fs::exists(ASTFileName));
 
 // Load the definition from the AST file.
-llvm::Expected NewFDorError =
-CTU.getCrossTUDefinition(FD, "", IndexFileName);
-EXPECT_TRUE((bool)NewFDorError);
-const FunctionDecl *NewFD = *NewFDorError;
+llvm::Expected NewFDorError = handleExpected(
+CTU.getCrossTUDefinition(FD, "", IndexFileName, false),
+[]() { return nullptr; }, [](IndexError &) {});
 
-*Success = NewFD && NewFD->hasBody() && !OrigFDHasBody;
+if (NewFDorError) {
+  const FunctionDecl *NewFD = *NewFDorError;
+  *Success = NewFD && NewFD->hasBody() && !OrigFDHasBody;
+}
   }
 
 private:
@@ -85,26 +88,37 @@
 
 class CTUAction : public clang::ASTFrontendAction {
 public:
-  CTUAction(bool *Success) : Success(Success) {}
+  CTUAction(bool *Success, unsigned OverrideLimit)
+  : Success(Success), OverrideLimit(OverrideLimit) {}
 
 protected:
   std::unique_ptr
   CreateASTConsumer(clang::CompilerInstance &CI, StringRef) override {
+CI.getAnalyzerOpts()->CTUImportThreshold = OverrideLimit;
 return llvm::make_unique(CI, Success);
   }
 
 private:
   bool *Success;
+  const unsigned OverrideLimit;
 };
 
 } // end namespace
 
 TEST(CrossTranslationUnit, CanLoadFunctionDefinition) {
   bool Success = false;
-  EXPECT_TRUE(tooling::runToolOnCode(new CTUAction(&Success), "int f(int);"));
+  EXPECT_TRUE(
+  tooling::runToolOnCode(new CTUAction(&Success, 1u), "int f(int);"));
   EXPECT_TRUE(Success);
 }
 
+TEST(CrossTranslationUnit, RespectsLoadThreshold) {
+  bool Success = false;
+  EXPECT_TRUE(
+  tooling::runToolOnCode(new CTUAction(&Success, 0u), "int f(int);"));
+  EXPECT_FALSE(Success);
+}
+
 TEST(CrossTranslationUnit, IndexFormatCanBeParsed) {
   llvm::StringMap Index;
   Index["a"] = "/b/f1";
Index: test/Analysis/ctu-import-threshold.c
===
--- /dev/null
+++ test/Analysis/ctu-import-threshold.c
@@ -0,0 +1,5 @@
+// Ensure analyzer option 'ctu-import-threshold' is a recognized option.
+//
+// RUN: %clang_cc1 -analyze -analyzer-config ctu-import-threshold=30 -verify %s
+//
+// expected-no-diagnostics
Index: test/Analysis/analyzer-config.c
===
--- test/Analysis/analyzer-config.c
+++ test/Analysis/analyzer-config.c
@@ -20,6 +20,7 @@
 // CHECK-NEXT: cfg-temporary-dtors = true
 // CHECK-NEXT: crosscheck-with-z3 = false
 // CHECK-NEXT: ctu-dir = ""
+// CHECK-NEXT: ctu-import-threshold = 100
 // CHECK-NEXT: ctu-index-name = externalDefMap.txt
 // CHECK-NEXT: display-ctu-progress = false
 // CHECK-NEXT: eagerly-assume = true
@@ -52,4 +53,4 @@
 // CHECK-NEXT: unroll-loops = false
 // CHECK-NEXT: widen-loops = false
 // CHECK-NEXT: [stats]
-// CHECK-NEXT: num-entries = 49
+// CHECK-NEXT: num-entries = 50
Index: lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
===
--- lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
+++ lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
@@ -204,8 +204,8 @@
 Plugins(plugins), Injector(injector), CTU(CI) {
 DigestAnalyzerOptions();
 if (Opts->PrintStats || Opts->ShouldSerializeStats) {
-  AnalyzerTimers = llvm::make_unique(
-  "analyzer", "Analyzer timers");
+  AnalyzerTimers =
+  llvm::make_unique("analyzer", "Analyzer timers");
   TUTotalTimer = llvm::make_unique(
   "time", "Analyzer total time", *AnalyzerTimers);
   llvm::EnableStatistics(/* PrintOnExit= */ false);
Index: lib/CrossTU/CrossTranslationUnit.cpp
===
--- lib/CrossTU/CrossTranslationUnit.cpp
+++ lib/CrossTU/CrossTranslationUnit.cpp
@@ -43,6 +43,8 @@
 STATISTIC(NumTripleMismatch, "The # of triple mismatches");
 STATISTIC(NumLangMi

[PATCH] D59798: [analyzer] Add analyzer option to limit the number of imported TUs

[Endre Fülöp via Phabricator via cfe-commits]

gamesh411 marked an inline comment as done.
gamesh411 added a comment.

I could greatly simplify the API of getCrossTUDefinition by injecting the 
threshold value in the constructor. A minor drawback with this approach is that 
testing code is a bit more complicated, as I had to patch the CompilerInstance 
to override the AnalyzerOption value for CTUImportThreshold. Thanks for the 
suggestion!


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59798/new/

https://reviews.llvm.org/D59798



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

[PATCH] D59798: [analyzer] Add analyzer option to limit the number of imported TUs

[Endre Fülöp via Phabricator via cfe-commits]

gamesh411 updated this revision to Diff 199278.
gamesh411 added a comment.

Revert unnecessary clang-formating of AnalysisConsumer.cpp


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59798/new/

https://reviews.llvm.org/D59798

Files:
  include/clang/CrossTU/CrossTranslationUnit.h
  include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
  lib/CrossTU/CrossTranslationUnit.cpp
  test/Analysis/analyzer-config.c
  test/Analysis/ctu-import-threshold.c
  unittests/CrossTU/CrossTranslationUnitTest.cpp

Index: unittests/CrossTU/CrossTranslationUnitTest.cpp
===
--- unittests/CrossTU/CrossTranslationUnitTest.cpp
+++ unittests/CrossTU/CrossTranslationUnitTest.cpp
@@ -7,6 +7,7 @@
 //===--===//
 
 #include "clang/CrossTU/CrossTranslationUnit.h"
+#include "clang/Frontend/CompilerInstance.h"
 #include "clang/AST/ASTConsumer.h"
 #include "clang/Frontend/FrontendAction.h"
 #include "clang/Tooling/Tooling.h"
@@ -70,12 +71,14 @@
 EXPECT_TRUE(llvm::sys::fs::exists(ASTFileName));
 
 // Load the definition from the AST file.
-llvm::Expected NewFDorError =
-CTU.getCrossTUDefinition(FD, "", IndexFileName);
-EXPECT_TRUE((bool)NewFDorError);
-const FunctionDecl *NewFD = *NewFDorError;
+llvm::Expected NewFDorError = handleExpected(
+CTU.getCrossTUDefinition(FD, "", IndexFileName, false),
+[]() { return nullptr; }, [](IndexError &) {});
 
-*Success = NewFD && NewFD->hasBody() && !OrigFDHasBody;
+if (NewFDorError) {
+  const FunctionDecl *NewFD = *NewFDorError;
+  *Success = NewFD && NewFD->hasBody() && !OrigFDHasBody;
+}
   }
 
 private:
@@ -85,26 +88,37 @@
 
 class CTUAction : public clang::ASTFrontendAction {
 public:
-  CTUAction(bool *Success) : Success(Success) {}
+  CTUAction(bool *Success, unsigned OverrideLimit)
+  : Success(Success), OverrideLimit(OverrideLimit) {}
 
 protected:
   std::unique_ptr
   CreateASTConsumer(clang::CompilerInstance &CI, StringRef) override {
+CI.getAnalyzerOpts()->CTUImportThreshold = OverrideLimit;
 return llvm::make_unique(CI, Success);
   }
 
 private:
   bool *Success;
+  const unsigned OverrideLimit;
 };
 
 } // end namespace
 
 TEST(CrossTranslationUnit, CanLoadFunctionDefinition) {
   bool Success = false;
-  EXPECT_TRUE(tooling::runToolOnCode(new CTUAction(&Success), "int f(int);"));
+  EXPECT_TRUE(
+  tooling::runToolOnCode(new CTUAction(&Success, 1u), "int f(int);"));
   EXPECT_TRUE(Success);
 }
 
+TEST(CrossTranslationUnit, RespectsLoadThreshold) {
+  bool Success = false;
+  EXPECT_TRUE(
+  tooling::runToolOnCode(new CTUAction(&Success, 0u), "int f(int);"));
+  EXPECT_FALSE(Success);
+}
+
 TEST(CrossTranslationUnit, IndexFormatCanBeParsed) {
   llvm::StringMap Index;
   Index["a"] = "/b/f1";
Index: test/Analysis/ctu-import-threshold.c
===
--- /dev/null
+++ test/Analysis/ctu-import-threshold.c
@@ -0,0 +1,5 @@
+// Ensure analyzer option 'ctu-import-threshold' is a recognized option.
+//
+// RUN: %clang_cc1 -analyze -analyzer-config ctu-import-threshold=30 -verify %s
+//
+// expected-no-diagnostics
Index: test/Analysis/analyzer-config.c
===
--- test/Analysis/analyzer-config.c
+++ test/Analysis/analyzer-config.c
@@ -20,6 +20,7 @@
 // CHECK-NEXT: cfg-temporary-dtors = true
 // CHECK-NEXT: crosscheck-with-z3 = false
 // CHECK-NEXT: ctu-dir = ""
+// CHECK-NEXT: ctu-import-threshold = 100
 // CHECK-NEXT: ctu-index-name = externalDefMap.txt
 // CHECK-NEXT: display-ctu-progress = false
 // CHECK-NEXT: eagerly-assume = true
@@ -52,4 +53,4 @@
 // CHECK-NEXT: unroll-loops = false
 // CHECK-NEXT: widen-loops = false
 // CHECK-NEXT: [stats]
-// CHECK-NEXT: num-entries = 49
+// CHECK-NEXT: num-entries = 50
Index: lib/CrossTU/CrossTranslationUnit.cpp
===
--- lib/CrossTU/CrossTranslationUnit.cpp
+++ lib/CrossTU/CrossTranslationUnit.cpp
@@ -43,6 +43,8 @@
 STATISTIC(NumTripleMismatch, "The # of triple mismatches");
 STATISTIC(NumLangMismatch, "The # of language mismatches");
 STATISTIC(NumLangDialectMismatch, "The # of language dialect mismatches");
+STATISTIC(NumASTLoadThresholdReached,
+  "The # of ASTs not loaded because of threshold");
 
 // Same as Triple's equality operator, but we check a field only if that is
 // known in both instances.
@@ -102,6 +104,8 @@
   return "Language mismatch";
 case index_error_code::lang_dialect_mismatch:
   return "Language dialect mismatch";
+case index_error_code::load_threshold_reached:
+  return "Load threshold reached";
 }
 llvm_unreachable("Unrecognized index_error_code.");
   }
@@ -180,7 +184,8 @@
 }
 
 CrossTranslationUnitContext::CrossTranslationUnitContext(CompilerInstance &CI)
-: CI(C

[PATCH] D60974: Clang IFSO driver action.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi added a comment.

@compnerd Any other test cases you'd like before the first drop? @jakehehrlich 
@rupprecht Do things here look good to you guys? Both formats are marked 
experimental, I plan to work on a merge action next.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60974/new/

https://reviews.llvm.org/D60974



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

[PATCH] D61861: DeleteNullPointerCheck now deletes until the end brace of the condition

[Jonathan Camilleri via Phabricator via cfe-commits]

J-Camilleri created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Bug Fix for #35811


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D61861

Files:
  clang-tools-extra/clang-tidy/readability/DeleteNullPointerCheck.cpp
  clang-tools-extra/test/clang-tidy/readability-delete-null-pointer.cpp


Index: clang-tools-extra/test/clang-tidy/readability-delete-null-pointer.cpp
===
--- clang-tools-extra/test/clang-tidy/readability-delete-null-pointer.cpp
+++ clang-tools-extra/test/clang-tidy/readability-delete-null-pointer.cpp
@@ -6,7 +6,7 @@
   int *p = 0;
 
   // #1
-  if (p) { // #2
+  if (p /**/) { // #2
 delete p;
   } // #3
   // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: 'if' statement is unnecessary; 
deleting null pointer has no effect [readability-delete-null-pointer]
Index: clang-tools-extra/clang-tidy/readability/DeleteNullPointerCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/DeleteNullPointerCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/DeleteNullPointerCheck.cpp
@@ -7,6 +7,7 @@
 
//===--===//
 
 #include "DeleteNullPointerCheck.h"
+#include "../utils/LexerUtils.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/Lex/Lexer.h"
@@ -62,9 +63,11 @@
 
   Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange(
   IfWithDelete->getBeginLoc(),
-  Lexer::getLocForEndOfToken(IfWithDelete->getCond()->getEndLoc(), 0,
- *Result.SourceManager,
- Result.Context->getLangOpts(;
+  utils::lexer::getPreviousToken(IfWithDelete->getThen()->getBeginLoc(),
+ *Result.SourceManager,
+ Result.Context->getLangOpts())
+  .getLocation()));
+
   if (Compound) {
 Diag << FixItHint::CreateRemoval(
 CharSourceRange::getTokenRange(Compound->getLBracLoc()));


Index: clang-tools-extra/test/clang-tidy/readability-delete-null-pointer.cpp
===
--- clang-tools-extra/test/clang-tidy/readability-delete-null-pointer.cpp
+++ clang-tools-extra/test/clang-tidy/readability-delete-null-pointer.cpp
@@ -6,7 +6,7 @@
   int *p = 0;
 
   // #1
-  if (p) { // #2
+  if (p /**/) { // #2
 delete p;
   } // #3
   // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: 'if' statement is unnecessary; deleting null pointer has no effect [readability-delete-null-pointer]
Index: clang-tools-extra/clang-tidy/readability/DeleteNullPointerCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/DeleteNullPointerCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/DeleteNullPointerCheck.cpp
@@ -7,6 +7,7 @@
 //===--===//
 
 #include "DeleteNullPointerCheck.h"
+#include "../utils/LexerUtils.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/Lex/Lexer.h"
@@ -62,9 +63,11 @@
 
   Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange(
   IfWithDelete->getBeginLoc(),
-  Lexer::getLocForEndOfToken(IfWithDelete->getCond()->getEndLoc(), 0,
- *Result.SourceManager,
- Result.Context->getLangOpts(;
+  utils::lexer::getPreviousToken(IfWithDelete->getThen()->getBeginLoc(),
+ *Result.SourceManager,
+ Result.Context->getLangOpts())
+  .getLocation()));
+
   if (Compound) {
 Diag << FixItHint::CreateRemoval(
 CharSourceRange::getTokenRange(Compound->getLBracLoc()));
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59465: [analyzer] Add a test plugin for checker option handling

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus added a comment.

Ping^2


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59465/new/

https://reviews.llvm.org/D59465



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

[PATCH] D59464: [analyzer] Add an example plugin for checker dependency handling

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus added a comment.

Ping^3


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59464/new/

https://reviews.llvm.org/D59464



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

[PATCH] D58035: [clang/DIVar] Emit flag for params that have unchanged values

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added inline comments.



Comment at: lib/CodeGen/CGDebugInfo.cpp:3885
+  if (ArgNo) {
+auto *PD = dyn_cast(VD);
+ParmCache[PD].reset(D);

A `dyn_cast` followed by an unconditional use seems strange. I would expect 
either a `cast` or an `if (PD)` check.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58035/new/

https://reviews.llvm.org/D58035



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

[PATCH] D61865: [clangd] improve help message for limit-results

[Brennan Vincent via Phabricator via cfe-commits]

umanwizard created this revision.
umanwizard added a reviewer: clang-tools-extra.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay, 
ilya-biryukov.
Herald added a project: clang.

Make it clear that the default is 100.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D61865

Files:
  clang-tools-extra/clangd/tool/ClangdMain.cpp


Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -117,7 +117,7 @@
 static llvm::cl::opt LimitResults(
 "limit-results",
 llvm::cl::desc("Limit the number of results returned by clangd. "
-   "0 means no limit."),
+   "0 means no limit. (default=100)"),
 llvm::cl::init(100));
 
 static llvm::cl::opt RunSynchronously(


Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -117,7 +117,7 @@
 static llvm::cl::opt LimitResults(
 "limit-results",
 llvm::cl::desc("Limit the number of results returned by clangd. "
-   "0 means no limit."),
+   "0 means no limit. (default=100)"),
 llvm::cl::init(100));
 
 static llvm::cl::opt RunSynchronously(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61765: [OpenMP][Clang][BugFix] Split declares and math functions inclusion.

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

In D61765#1499957 , @jdoerfert wrote:

> Two small changes and then it is fine with me. @tra ?


LGTM in general. I would still like to confirm that the changes work with 
libc++.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61765/new/

https://reviews.llvm.org/D61765



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

Re: r360559 - [c++20] P1064R0: Allow virtual function calls in constant expression

[Richard Smith via cfe-commits]

Thanks for the revert and the reduced test case.

On Mon, 13 May 2019, 07:50 Hans Wennborg via cfe-commits, <
cfe-commits@lists.llvm.org> wrote:

> Here's a creduced repro:
>
> --
> class a {};
> class b : virtual a {
>   virtual bool c(const void *, int);
> };
> class C : b {
> public:
>   bool c(const void *, int);
> };
> int d;
> bool e() {
>   C f;
>   if (f.c(&d, d))
> ;
> }
> --
>
> $ clang.bad -cc1 -triple x86_64-unknown-linux-gnu -fsyntax-only -std=c++14
> a.cc
>
> The assert goes away if invoking clang with -w.
>
> From: Hans Wennborg 
> Date: Mon, May 13, 2019 at 3:16 PM
> To: Richard Smith
> Cc: cfe-commits
>
> > This caused asserts in Chromium, so I've reverted in r360580. There's
> > a repro at
> https://bugs.chromium.org/p/chromium/issues/detail?id=962458#c1,
> > and I'm working on a reduced version.
> >
> > From: Richard Smith via cfe-commits 
> > Date: Mon, May 13, 2019 at 9:39 AM
> > To: 
> >
> > > Author: rsmith
> > > Date: Mon May 13 00:42:10 2019
> > > New Revision: 360559
> > >
> > > URL: http://llvm.org/viewvc/llvm-project?rev=360559&view=rev
> > > Log:
> > > [c++20] P1064R0: Allow virtual function calls in constant expression
> > > evaluation.
> > >
> > > Modified:
> > > cfe/trunk/include/clang/AST/DeclCXX.h
> > > cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td
> > > cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> > > cfe/trunk/include/clang/Sema/Sema.h
> > > cfe/trunk/lib/AST/DeclCXX.cpp
> > > cfe/trunk/lib/AST/ExprConstant.cpp
> > > cfe/trunk/lib/Sema/SemaDeclCXX.cpp
> > > cfe/trunk/lib/Sema/SemaTemplateInstantiate.cpp
> > > cfe/trunk/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp
> > > cfe/trunk/test/CXX/drs/dr18xx.cpp
> > > cfe/trunk/test/CXX/drs/dr6xx.cpp
> > > cfe/trunk/test/SemaCXX/constant-expression-cxx2a.cpp
> > > cfe/trunk/test/SemaCXX/cxx17-compat.cpp
> > > cfe/trunk/www/cxx_status.html
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D58920: [Modules][PR39287] Consolidate multiple std's

[Brian Gesiak via Phabricator via cfe-commits]

modocache added a comment.

Thanks @rsmith for the guidance here! I appreciate it very much. One snag I ran 
into after following your suggestion, though, is that when I modify 
`ASTDeclReader::findExisting` to return Sema's existing implicit std namespace, 
I run into an assertion later on, when the decl chain is being marked as 
incomplete. That is, the following patch (debug output included):

  diff --git a/lib/Serialization/ASTReaderDecl.cpp 
b/lib/Serialization/ASTReaderDecl.cpp
  index 32bd82d077..9d447952e1 100644
  --- a/lib/Serialization/ASTReaderDecl.cpp
  +++ b/lib/Serialization/ASTReaderDecl.cpp
  @@ -47,6 +47,7 @@
   #include "clang/Basic/SourceLocation.h"
   #include "clang/Basic/Specifiers.h"
   #include "clang/Sema/IdentifierResolver.h"
  +#include "clang/Sema/Sema.h"
   #include "clang/Serialization/ASTBitCodes.h"
   #include "clang/Serialization/ASTReader.h"
   #include "clang/Serialization/ContinuousRangeMap.h"
  @@ -3401,6 +3402,22 @@ ASTDeclReader::FindExistingResult 
ASTDeclReader::findExisting(NamedDecl *D) {
 return FindExistingResult(Reader, D, Existing, AnonymousDeclNumber,
   TypedefNameForLinkage);
   }
  +if (isa(D) && D->getName() == "std") {
  +  llvm::outs() << "Found std namespace: ";
  +  D->dump();
  +  llvm::outs() << "Merging into Sema std namespace: ";
  +  Sema *S = Reader.getSema();
  +  S->getStdNamespace()->dump();
  +  NamedDecl *Existing =
  +  getDeclForMerging(S->getStdNamespace(), TypedefNameForLinkage);
  +  llvm::outs() << "Found Existing: ";
  +  Existing->dump();
  +  if (isSameEntity(Existing, D)) {
  +llvm::outs() << "isSameEntity is true\n";
  +return FindExistingResult(Reader, D, Existing, AnonymousDeclNumber,
  +  TypedefNameForLinkage);
  +  }
  +}
 } else {
   // Not in a mergeable context.
   return FindExistingResult(Reader);

Results in the expected output, along with an unexpected assert:

  Found std namespace: NamespaceDecl 0x55a391530910 
 col:11 imported in a.h std
  Merging into Sema std namespace: NamespaceDecl 0x55a391501ec8 <>  implicit std
  Found Existing: NamespaceDecl 0x55a391501ec8 <>  
implicit std
  isSameEntity is true
  clang-9: ../include/llvm/ADT/PointerUnion.h:135: T llvm::PointerUnion::get() const [with T = clang::LazyGenerationalUpdatePtr; 
PT1 = llvm::PointerUnion; PT2 = 
clang::LazyGenerationalUpdatePtr]: Assertion `is() && 
"Invalid accessor called"' failed.
  Stack dump:
  0.  Program arguments: 
/home/modocache/Source/llvm/git/dev/llvm/build/bin/clang-9 -cc1 -triple 
x86_64-unknown-linux-gnu -emit-obj -mrelax-all -disable-free -main-file-name 
mod-virtual-destructor-bug.cpp -mrelocation-model static -mthread-model posix 
-mdisable-fp-elim -fmath-errno -masm-verbose -mconstructor-aliases 
-munwind-tables -fuse-init-array -target-cpu x86-64 -dwarf-column-info 
-debugger-tuning=gdb -resource-dir 
/home/modocache/Source/llvm/git/dev/llvm/build/lib/clang/9.0.0 -I 
mod-virtual-destructor-bug -internal-isystem 
/usr/lib/gcc/x86_64-linux-gnu/8/../../../../include/c++/8 -internal-isystem 
/usr/lib/gcc/x86_64-linux-gnu/8/../../../../include/x86_64-linux-gnu/c++/8 
-internal-isystem 
/usr/lib/gcc/x86_64-linux-gnu/8/../../../../include/x86_64-linux-gnu/c++/8 
-internal-isystem 
/usr/lib/gcc/x86_64-linux-gnu/8/../../../../include/c++/8/backward 
-internal-isystem /usr/local/include -internal-isystem 
/home/modocache/Source/llvm/git/dev/llvm/build/lib/clang/9.0.0/include 
-internal-externc-isystem /usr/include/x86_64-linux-gnu 
-internal-externc-isystem /include -internal-externc-isystem /usr/include 
-std=c++17 -fdeprecated-macro -fdebug-compilation-dir 
/home/modocache/Source/tmp/mod -ferror-limit 19 -fmessage-length 195 -fmodules 
-fimplicit-module-maps -fmodules-cache-path=mod-virtual-destructor-cache 
-fmodules-validate-system-headers -fobjc-runtime=gcc -fcxx-exceptions 
-fexceptions -fdiagnostics-show-option -fcolor-diagnostics -o 
/tmp/mod-virtual-destructor-bug-0da92f.o -x c++ mod-virtual-destructor-bug.cpp 
-faddrsig
  1.  mod-virtual-destructor-bug.cpp:10:17: current parser token 'class'
   #0 0x55a388bfaaa5 llvm::sys::PrintStackTrace(llvm::raw_ostream&) 
/home/modocache/Source/llvm/git/dev/llvm/build/../lib/Support/Unix/Signals.inc:494:22
   #1 0x55a388bfab38 PrintStackTraceSignalHandler(void*) 
/home/modocache/Source/llvm/git/dev/llvm/build/../lib/Support/Unix/Signals.inc:558:1
   #2 0x55a388bf8b32 llvm::sys::RunSignalHandlers() 
/home/modocache/Source/llvm/git/dev/llvm/build/../lib/Support/Signals.cpp:68:20
   #3 0x55a388bfa4fb SignalHandler(int) 
/home/modocache/Source/llvm/git/dev/llvm/build/../lib/Support/Unix/Signals.inc:357:1
   #4 0x7f909ab86dd0 __restore_rt 
(/lib/x86_64-linux-gnu/libpthread.so.0+0x12dd0)
   #5 0x7f909a249077 raise 
/build/glibc-B9XfQf/glibc-2.28/signal/../sysdeps/unix/sysv/linux/raise.c:51:1
   #6 

r360607 - [clang][ASTContext] Call setAttached for comments attached to a declaration

[Jan Korous via cfe-commits]

Author: jkorous
Date: Mon May 13 10:52:09 2019
New Revision: 360607

URL: http://llvm.org/viewvc/llvm-project?rev=360607&view=rev
Log:
[clang][ASTContext] Call setAttached for comments attached to a declaration

This is a bug affecting performance when compiling with -Wdocumentation.

In Sema::ActOnDocumentable we're checking whether there are any comments 
unattached to declaration at the end of comment list whenever we encounter new 
documentable declaration.
Since this property of RawComment was never set we were trying to find comments 
every time and that involves at least a couple expensive SourceLocation 
decompositions.

Differential Revision: https://reviews.llvm.org/D61538

Modified:
cfe/trunk/lib/AST/ASTContext.cpp

Modified: cfe/trunk/lib/AST/ASTContext.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTContext.cpp?rev=360607&r1=360606&r2=360607&view=diff
==
--- cfe/trunk/lib/AST/ASTContext.cpp (original)
+++ cfe/trunk/lib/AST/ASTContext.cpp Mon May 13 10:52:09 2019
@@ -255,6 +255,7 @@ RawComment *ASTContext::getRawCommentFor
 SourceMgr.getLineNumber(DeclLocDecomp.first, DeclLocDecomp.second)
   == SourceMgr.getLineNumber(CommentBeginDecomp.first,
  CommentBeginDecomp.second)) {
+  (**Comment).setAttached();
   return *Comment;
 }
   }
@@ -296,6 +297,7 @@ RawComment *ASTContext::getRawCommentFor
   if (Text.find_first_of(";{}#@") != StringRef::npos)
 return nullptr;
 
+  (**Comment).setAttached();
   return *Comment;
 }
 


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

[PATCH] D61851: [clang-tidy] New option for misc-throw-by-value-catch-by-reference

[Torbjörn Klatt via Phabricator via cfe-commits]

torbjoernk added inline comments.



Comment at: 
docs/clang-tidy/checks/misc-throw-by-value-catch-by-reference.rst:35-46
+.. option:: WarnOnLargeObject
+
+   Also warns for any large trivial object caught by value. Catching a large
+   object by value is not dangerous but affects the perofrmance negatively. The
+   maximum size of an object allowed to be caught without warning can be set
+   using option `MaxSize`
+   Default is `0`.

I think it might be worth adding a note on the fallback to `sizeof(size_t)` if 
`MaxSize` is not set or manually set to `std::numeric_limits::max()` 
by the user.


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61851/new/

https://reviews.llvm.org/D61851



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

[PATCH] D61858: Make `__is_base_of` more friendly with unions

[Louis Dionne via Phabricator via cfe-commits]

ldionne added a comment.

Shouldn't we also add unit tests for PR41843 in libc++?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61858/new/

https://reviews.llvm.org/D61858



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

[PATCH] D61861: DeleteNullPointerCheck now deletes until the end brace of the condition

[Mads Ravn via Phabricator via cfe-commits]

madsravn added a comment.

Besides my one comment this looks good to me.




Comment at: 
clang-tools-extra/test/clang-tidy/readability-delete-null-pointer.cpp:9
   // #1
-  if (p) { // #2
+  if (p /**/) { // #2
 delete p;

Would you mind creating a new test instead of changing the existing one? 
Removing tests could create another whole for a bug to fall through.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61861/new/

https://reviews.llvm.org/D61861



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

[PATCH] D61749: [clang-tidy] initial version of readability-static-const-method

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/readability-static-const-method.cpp:209
+void KeepLambdas() {
+  auto F = +[]() { return 0; };
+  auto F2 = []() { return 0; };

mgehre wrote:
> JonasToth wrote:
> > Does it pass here?
> > I looks a bit weird, shouldnt the lambda be called for this to work (this 
> > is the unary + right?)?
> https://stackoverflow.com/questions/18889028/a-positive-lambda-what-sorcery-is-this
Thank you C++ for showing my simple mindedness all the time :D
Its fine then and I will keep it in my mind for future test cases I check 
against ;)



Comment at: 
clang-tools-extra/test/clang-tidy/readability-static-const-method.cpp:312
+return const_cast(this)->get();
+  }
+

mgehre wrote:
> JonasToth wrote:
> > I think more template tests wouldn't hurt. From the other checks experience 
> > I can safely say we got burned a few times :)
> > Instantiating the templates with builtin arrays, pointers, references and 
> > different qualifiers usually produces interesting test cases as well, that 
> > need to be handled properly.
> > 
> > Another thing that comes to mind with templates are overloaded operators.
> > ```
> > template 
> > void bar() {
> >  Foo x1, x2;
> >  Foo y = x1 + x2;
> > }
> > ```
> > Builtins are not changed by `operator+` but that can not be said about 
> > other types in general (maybe with concepts used properly).
> The check only checks templates instantiations (so we will see no template 
> parameters, just ordinary types). The plus here will be be function call in 
> the AST of the instantiation when Foo has an overloaded operator+.
> The current version will never propose to make bar() const when a method on 
> `this` is called.
> I can add the test to show this.
Yes please add a test to show they are not analyzed.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61749/new/

https://reviews.llvm.org/D61749



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

[PATCH] D61849: Do not list enabled checks when -quiet is given to run-clang-tidy.

[Jonas Toth via Phabricator via cfe-commits]

JonasToth accepted this revision.
JonasToth added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61849/new/

https://reviews.llvm.org/D61849



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

[PATCH] D61850: Removed superfluous and slightly annoying newlines in run-clang-tidy's output.

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

I agree with the direction, please ensure that e.g. clang-analyzer-* output 
looks proper as well and `-quiet` does, too.


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61850/new/

https://reviews.llvm.org/D61850



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

[PATCH] D61765: [OpenMP][Clang][BugFix] Split declares and math functions inclusion.

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

gtbercea added a comment.

In D61765#1500233 , @tra wrote:

> In D61765#1499957 , @jdoerfert wrote:
>
> > Two small changes and then it is fine with me. @tra ?
>
>
> LGTM in general. I would still like to confirm that the changes work with 
> libc++.


As soon as libc++ the limits header included in

  __clang_cuda_cmath.h:15
  ``` is not found:

__clang_cuda_cmath.h:15:10: fatal error: 'limits' file not found
#include 

  Not even CUDA works actually so I'm not sure what the best answer to this 
problem is.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61765/new/

https://reviews.llvm.org/D61765



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

[PATCH] D61438: [ASTImporter] Use llvm::Expected and Error in the importer API

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added inline comments.



Comment at: clang/include/clang/AST/ASTImporter.h:203
 /// context, or the import error.
-llvm::Expected Import_New(TypeSourceInfo *FromTSI);
-// FIXME: Remove this version.
-TypeSourceInfo *Import(TypeSourceInfo *FromTSI);
+llvm::Expected Import(TypeSourceInfo *FromTSI);
 

martong wrote:
> aprantl wrote:
> > Wouldn't it make more sense to return `Expected`?
> That would not be consistent with the other changes. In this patch we change 
> the signature of each functions similarly:
> From `T foo(...)` to `Expected foo(...)`.
> Also, `TypeSourceInfo` factory functions in `ASTContext` do return with a 
> pointer. For example:
> ```
>   TypeSourceInfo *CreateTypeSourceInfo(QualType T, unsigned Size = 0) const;
> 
>   /// Allocate a TypeSourceInfo where all locations have been
>   /// initialized to a given location, which defaults to the empty
>   /// location.
>   TypeSourceInfo *
>   getTrivialTypeSourceInfo(QualType T,
>SourceLocation Loc = SourceLocation()) const;
> 
> ```
I believe that LLVM's practice of passing non-null pointers around is bad API 
design because it's never quite clear from looking at an API which pointer 
parameters are nullable, so I was hoping that we could correct some of that at 
least in the cases where we introduce API that return Expected<> objects to 
make it obvious that this is either an error or a valid object. If you that the 
perceived inconsistency weighs stronger than the readability improvements let 
me know and I can reconsider.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61438/new/

https://reviews.llvm.org/D61438



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

[libunwind] r360610 - Add a new LIBUNWIND_WEAK_PTHREAD Cmake option to force

[Sterling Augustine via cfe-commits]

Author: saugustine
Date: Mon May 13 11:45:03 2019
New Revision: 360610

URL: http://llvm.org/viewvc/llvm-project?rev=360610&view=rev
Log:
Add a new LIBUNWIND_WEAK_PTHREAD Cmake option to force
calls into the pthread library use weak symbols.

This option allows libpthread to be a weak dependency rather
than a hard one.

Differential Revision: https://reviews.llvm.org/D60285

Modified:
libunwind/trunk/CMakeLists.txt
libunwind/trunk/src/CMakeLists.txt
libunwind/trunk/src/RWMutex.hpp

Modified: libunwind/trunk/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/libunwind/trunk/CMakeLists.txt?rev=360610&r1=360609&r2=360610&view=diff
==
--- libunwind/trunk/CMakeLists.txt (original)
+++ libunwind/trunk/CMakeLists.txt Mon May 13 11:45:03 2019
@@ -134,6 +134,7 @@ option(LIBUNWIND_ENABLE_STATIC "Build li
 option(LIBUNWIND_ENABLE_CROSS_UNWINDING "Enable cross-platform unwinding 
support." OFF)
 option(LIBUNWIND_ENABLE_ARM_WMMX "Enable unwinding support for ARM WMMX 
registers." OFF)
 option(LIBUNWIND_ENABLE_THREADS "Build libunwind with threading support." ON)
+option(LIBUNWIND_WEAK_PTHREAD_LIB "Use weak references to refer to pthread 
functions." OFF)
 option(LIBUNWIND_USE_COMPILER_RT "Use compiler-rt instead of libgcc" OFF)
 option(LIBUNWIND_INCLUDE_DOCS "Build the libunwind documentation." 
${LLVM_INCLUDE_DOCS})
 

Modified: libunwind/trunk/src/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/CMakeLists.txt?rev=360610&r1=360609&r2=360610&view=diff
==
--- libunwind/trunk/src/CMakeLists.txt (original)
+++ libunwind/trunk/src/CMakeLists.txt Mon May 13 11:45:03 2019
@@ -67,6 +67,7 @@ endif()
 unwind_append_if(libraries LIBUNWIND_HAS_DL_LIB dl)
 if (LIBUNWIND_ENABLE_THREADS)
   unwind_append_if(libraries LIBUNWIND_HAS_PTHREAD_LIB pthread)
+  unwind_append_if(LIBUNWIND_COMPILE_FLAGS LIBUNWIND_WEAK_PTHREAD_LIB 
-DLIBUNWIND_USE_WEAK_PTHREAD=1)
 endif()
 
 # Setup flags.

Modified: libunwind/trunk/src/RWMutex.hpp
URL: 
http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/RWMutex.hpp?rev=360610&r1=360609&r2=360610&view=diff
==
--- libunwind/trunk/src/RWMutex.hpp (original)
+++ libunwind/trunk/src/RWMutex.hpp Mon May 13 11:45:03 2019
@@ -56,17 +56,52 @@ private:
   SRWLOCK _lock = SRWLOCK_INIT;
 };
 
-#else
+#elif !defined(LIBUNWIND_USE_WEAK_PTHREAD)
 
 class _LIBUNWIND_HIDDEN RWMutex {
 public:
-  bool lock_shared() { return pthread_rwlock_rdlock(&_lock) == 0; }
+  bool lock_shared() { return pthread_rwlock_rdlock(&_lock) == 0;  }
   bool unlock_shared() { return pthread_rwlock_unlock(&_lock) == 0; }
   bool lock() { return pthread_rwlock_wrlock(&_lock) == 0; }
   bool unlock() { return pthread_rwlock_unlock(&_lock) == 0; }
 
 private:
   pthread_rwlock_t _lock = PTHREAD_RWLOCK_INITIALIZER;
+};
+
+#else
+
+extern "C" int __attribute__((weak))
+pthread_create(pthread_t *thread, const pthread_attr_t *attr,
+   void *(*start_routine)(void *), void *arg);
+extern "C" int __attribute__((weak))
+pthread_rwlock_rdlock(pthread_rwlock_t *lock);
+extern "C" int __attribute__((weak))
+pthread_rwlock_wrlock(pthread_rwlock_t *lock);
+extern "C" int __attribute__((weak))
+pthread_rwlock_unlock(pthread_rwlock_t *lock);
+
+// Calls to the locking functions are gated on pthread_create, and not the
+// functions themselves, because the data structure should only be locked if
+// another thread has been created. This is what similar libraries do.
+
+class _LIBUNWIND_HIDDEN RWMutex {
+public:
+  bool lock_shared() {
+return !pthread_create || (pthread_rwlock_rdlock(&_lock) == 0);
+  }
+  bool unlock_shared() {
+return !pthread_create || (pthread_rwlock_unlock(&_lock) == 0);
+  }
+  bool lock() {
+return !pthread_create || (pthread_rwlock_wrlock(&_lock) == 0);
+  }
+  bool unlock() {
+return !pthread_create || (pthread_rwlock_unlock(&_lock) == 0);
+  }
+
+private:
+  pthread_rwlock_t _lock = PTHREAD_RWLOCK_INITIALIZER;
 };
 
 #endif


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

[PATCH] D61850: Removed superfluous and slightly annoying newlines in run-clang-tidy's output.

[Sven Panne via Phabricator via cfe-commits]

svenpanne added a comment.

In D61850#1500298 , @JonasToth wrote:

> [...] please ensure that e.g. clang-analyzer-* output looks proper as well 
> and `-quiet` does, too.


OK, are there any (unit) tests which I should run? If yes, how exactly? I have 
a successfully built a fork of https://github.com/llvm/llvm-project via ninja 
locally, but the documentation for the various subprojects is still a bit hard 
to find to me. Is there a ninja target for `clang-tidy`-related tests? Any 
hints to get started would be highly appreciated.

This and the other output-related patches are my first submissions here, and 
they are intentionally simple to get the workflow right. My real intention is 
trying to improve/fix the `readability-identifier-naming` check/fixer, which 
still has a few issues.


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61850/new/

https://reviews.llvm.org/D61850



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

[PATCH] D61858: Make `__is_base_of` more friendly with unions

[Marshall Clow via Phabricator via cfe-commits]

mclow.lists added a comment.

In D61858#1500270 , @ldionne wrote:

> Shouldn't we also add unit tests for PR41843 in libc++?


Yes. But I want to do that later.
After this has landed (and probably wait a week for all the bots that are 
running trunk to update).


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61858/new/

https://reviews.llvm.org/D61858



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

[PATCH] D61858: Make `__is_base_of` more friendly with unions

[Louis Dionne via Phabricator via cfe-commits]

ldionne accepted this revision.
ldionne added a comment.
This revision is now accepted and ready to land.

I'm not a frontend expert, but this looks reasonable to me.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61858/new/

https://reviews.llvm.org/D61858



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

[PATCH] D61765: [OpenMP][Clang][BugFix] Split declares and math functions inclusion.

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

gtbercea updated this revision to Diff 199304.
gtbercea added a comment.

- Exclude clock functions. Reverse inclusion order.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61765/new/

https://reviews.llvm.org/D61765

Files:
  lib/Driver/ToolChains/Clang.cpp
  lib/Headers/CMakeLists.txt
  lib/Headers/__clang_cuda_cmath.h
  lib/Headers/__clang_cuda_device_functions.h
  lib/Headers/__clang_cuda_math_forward_declares.h
  lib/Headers/openmp_wrappers/__clang_openmp_math.h
  lib/Headers/openmp_wrappers/__clang_openmp_math_declares.h
  lib/Headers/openmp_wrappers/cmath
  lib/Headers/openmp_wrappers/math.h
  test/Headers/Inputs/include/cstdlib
  test/Headers/nvptx_device_cmath_functions.c
  test/Headers/nvptx_device_cmath_functions.cpp
  test/Headers/nvptx_device_math_functions.c
  test/Headers/nvptx_device_math_functions.cpp

Index: test/Headers/nvptx_device_math_functions.cpp
===
--- test/Headers/nvptx_device_math_functions.cpp
+++ test/Headers/nvptx_device_math_functions.cpp
@@ -4,8 +4,9 @@
 // REQUIRES: nvptx-registered-target
 
 // RUN: %clang_cc1 -internal-isystem %S/Inputs/include -include math.h -x c++ -fopenmp -triple powerpc64le-unknown-unknown -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm-bc %s -o %t-ppc-host.bc
-// RUN: %clang_cc1 -internal-isystem %S/../../lib/Headers/openmp_wrappers -include __clang_openmp_math.h -internal-isystem %S/../../lib/Headers/openmp_wrappers -include math.h -internal-isystem %S/Inputs/include -include stdlib.h -include limits -x c++ -fopenmp -triple nvptx64-nvidia-cuda -aux-triple powerpc64le-unknown-unknown -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-ppc-host.bc -o - | FileCheck -check-prefix CHECK-YES %s
+// RUN: %clang_cc1 -internal-isystem %S/../../lib/Headers/openmp_wrappers -include __clang_openmp_math_declares.h -internal-isystem %S/../../lib/Headers/openmp_wrappers -include math.h -internal-isystem %S/Inputs/include -include stdlib.h -include limits -include cstdlib -x c++ -fopenmp -triple nvptx64-nvidia-cuda -aux-triple powerpc64le-unknown-unknown -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-ppc-host.bc -o - | FileCheck -check-prefix CHECK-YES %s
 
+#include 
 #include 
 
 void test_sqrt(double a1) {
Index: test/Headers/nvptx_device_math_functions.c
===
--- test/Headers/nvptx_device_math_functions.c
+++ test/Headers/nvptx_device_math_functions.c
@@ -4,7 +4,7 @@
 // REQUIRES: nvptx-registered-target
 
 // RUN: %clang_cc1 -internal-isystem %S/Inputs/include -include math.h -fopenmp -triple powerpc64le-unknown-unknown -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm-bc %s -o %t-ppc-host.bc
-// RUN: %clang_cc1 -internal-isystem %S/../../lib/Headers/openmp_wrappers -include __clang_openmp_math.h -internal-isystem %S/../../lib/Headers/openmp_wrappers -include math.h -fopenmp -triple nvptx64-nvidia-cuda -aux-triple powerpc64le-unknown-unknown -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-ppc-host.bc -o - | FileCheck -check-prefix CHECK-YES %s
+// RUN: %clang_cc1 -internal-isystem %S/../../lib/Headers/openmp_wrappers -include __clang_openmp_math_declares.h -internal-isystem %S/../../lib/Headers/openmp_wrappers -include math.h -fopenmp -triple nvptx64-nvidia-cuda -aux-triple powerpc64le-unknown-unknown -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-ppc-host.bc -o - | FileCheck -check-prefix CHECK-YES %s
 
 #include 
 
Index: test/Headers/nvptx_device_cmath_functions.cpp
===
--- test/Headers/nvptx_device_cmath_functions.cpp
+++ test/Headers/nvptx_device_cmath_functions.cpp
@@ -4,9 +4,10 @@
 // REQUIRES: nvptx-registered-target
 
 // RUN: %clang_cc1 -internal-isystem %S/Inputs/include -include cmath -x c++ -fopenmp -triple powerpc64le-unknown-unknown -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm-bc %s -o %t-ppc-host.bc
-// RUN: %clang_cc1 -internal-isystem %S/../../lib/Headers/openmp_wrappers -include __clang_openmp_math.h -internal-isystem %S/../../lib/Headers/openmp_wrappers -include cmath -internal-isystem %S/Inputs/include -include stdlib.h -x c++ -fopenmp -triple nvptx64-nvidia-cuda -aux-triple powerpc64le-unknown-unknown -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-ppc-host.bc -o - | FileCheck -check-prefix CHECK-YES %s
+// RUN: %clang_cc1 -internal-isystem %S/../../lib/Headers/openmp_wrappers -include __clang_openmp_math_declares.h -internal-isystem %S/../../lib/Headers/openmp_wrappers -include cmath -internal-isystem %S/Inputs/include -include stdlib.h -x c++ -fopenmp -triple nvptx64-nvidia-cuda -aux-triple powerpc64le-unknown-unknown -fopenmp-targets=nvptx64-nvidia-cuda -emit-

[PATCH] D61858: Make `__is_base_of` more friendly with unions

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith accepted this revision.
rsmith added a comment.

Yeah, this seems to match the library wording. (I think it's short-sighted and 
over-fitting -- this is baking into the library specification that the language 
happens to not allow unions to have base classes today -- but this isn't the 
venue for that discussion.)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61858/new/

https://reviews.llvm.org/D61858



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

[clang-tools-extra] r360613 - [clang-tidy] readability-redundant-declaration: fix false positive with C "extern inline"

[Matthias Gehre via cfe-commits]

Author: mgehre
Date: Mon May 13 12:21:57 2019
New Revision: 360613

URL: http://llvm.org/viewvc/llvm-project?rev=360613&view=rev
Log:
[clang-tidy] readability-redundant-declaration: fix false positive with C 
"extern inline"

Summary:
readability-redundant-declaration was diagnosing a redundant declaration
on "extern inline void f();", which is needed in C code to force an external 
definition
of the inline function f. (This is different to how inline behaves in C++).

Reviewers: alexfh, danielmarjamaki

Subscribers: xazax.hun, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D61700

Added:
clang-tools-extra/trunk/test/clang-tidy/readability-redundant-declaration.c
Modified:
clang-tools-extra/trunk/clang-tidy/readability/RedundantDeclarationCheck.cpp

clang-tools-extra/trunk/test/clang-tidy/readability-redundant-declaration.cpp

Modified: 
clang-tools-extra/trunk/clang-tidy/readability/RedundantDeclarationCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/readability/RedundantDeclarationCheck.cpp?rev=360613&r1=360612&r2=360613&view=diff
==
--- 
clang-tools-extra/trunk/clang-tidy/readability/RedundantDeclarationCheck.cpp 
(original)
+++ 
clang-tools-extra/trunk/clang-tidy/readability/RedundantDeclarationCheck.cpp 
Mon May 13 12:21:57 2019
@@ -17,6 +17,10 @@ namespace clang {
 namespace tidy {
 namespace readability {
 
+AST_MATCHER(FunctionDecl, doesDeclarationForceExternallyVisibleDefinition) {
+  return Node.doesDeclarationForceExternallyVisibleDefinition();
+}
+
 RedundantDeclarationCheck::RedundantDeclarationCheck(StringRef Name,
  ClangTidyContext *Context)
 : ClangTidyCheck(Name, Context),
@@ -25,8 +29,10 @@ RedundantDeclarationCheck::RedundantDecl
 void RedundantDeclarationCheck::registerMatchers(MatchFinder *Finder) {
   Finder->addMatcher(
   namedDecl(anyOf(varDecl(unless(isDefinition())),
-  functionDecl(unless(anyOf(isDefinition(), isDefaulted(),
-hasParent(friendDecl()))
+  functionDecl(unless(anyOf(
+  isDefinition(), isDefaulted(),
+  doesDeclarationForceExternallyVisibleDefinition(),
+  hasParent(friendDecl()))
   .bind("Decl"),
   this);
 }

Added: 
clang-tools-extra/trunk/test/clang-tidy/readability-redundant-declaration.c
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/readability-redundant-declaration.c?rev=360613&view=auto
==
--- clang-tools-extra/trunk/test/clang-tidy/readability-redundant-declaration.c 
(added)
+++ clang-tools-extra/trunk/test/clang-tidy/readability-redundant-declaration.c 
Mon May 13 12:21:57 2019
@@ -0,0 +1,31 @@
+// RUN: %check_clang_tidy %s readability-redundant-declaration %t
+
+extern int Xyz;
+extern int Xyz; // Xyz
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: redundant 'Xyz' declaration 
[readability-redundant-declaration]
+// CHECK-FIXES: {{^}}// Xyz{{$}}
+int Xyz = 123;
+
+extern int A;
+extern int A, B;
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: redundant 'A' declaration
+// CHECK-FIXES: {{^}}extern int A, B;{{$}}
+
+extern int Buf[10];
+extern int Buf[10]; // Buf[10]
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: redundant 'Buf' declaration
+// CHECK-FIXES: {{^}}// Buf[10]{{$}}
+
+static int f();
+static int f(); // f
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: redundant 'f' declaration
+// CHECK-FIXES: {{^}}// f{{$}}
+static int f() {}
+
+inline void g() {}
+
+inline void g();
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: redundant 'g' declaration
+
+// OK: Needed to emit an external definition.
+extern inline void g();

Modified: 
clang-tools-extra/trunk/test/clang-tidy/readability-redundant-declaration.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/readability-redundant-declaration.cpp?rev=360613&r1=360612&r2=360613&view=diff
==
--- 
clang-tools-extra/trunk/test/clang-tidy/readability-redundant-declaration.cpp 
(original)
+++ 
clang-tools-extra/trunk/test/clang-tidy/readability-redundant-declaration.cpp 
Mon May 13 12:21:57 2019
@@ -68,3 +68,13 @@ DEFINE(test);
 // CHECK-FIXES: {{^}}DEFINE(test);{{$}}
 
 } // namespace macros
+
+inline void g() {}
+
+inline void g(); // g
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: redundant 'g' declaration
+// CHECK-FIXES: {{^}}// g{{$}}
+
+extern inline void g(); // extern g
+// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: redundant 'g' declaration
+// CHECK-FIXES: {{^}}// extern g{{$}}


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/c

[PATCH] D61700: [clang-tidy] readability-redundant-declaration: fix false positive with C "extern inline"

[Matthias Gehre via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
mgehre marked 2 inline comments as done.
Closed by commit rL360613: [clang-tidy] readability-redundant-declaration: fix 
false positive with C… (authored by mgehre, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D61700?vs=198724&id=199306#toc

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61700/new/

https://reviews.llvm.org/D61700

Files:
  clang-tools-extra/trunk/clang-tidy/readability/RedundantDeclarationCheck.cpp
  clang-tools-extra/trunk/test/clang-tidy/readability-redundant-declaration.c
  clang-tools-extra/trunk/test/clang-tidy/readability-redundant-declaration.cpp


Index: 
clang-tools-extra/trunk/clang-tidy/readability/RedundantDeclarationCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/readability/RedundantDeclarationCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/readability/RedundantDeclarationCheck.cpp
@@ -17,6 +17,10 @@
 namespace tidy {
 namespace readability {
 
+AST_MATCHER(FunctionDecl, doesDeclarationForceExternallyVisibleDefinition) {
+  return Node.doesDeclarationForceExternallyVisibleDefinition();
+}
+
 RedundantDeclarationCheck::RedundantDeclarationCheck(StringRef Name,
  ClangTidyContext *Context)
 : ClangTidyCheck(Name, Context),
@@ -25,8 +29,10 @@
 void RedundantDeclarationCheck::registerMatchers(MatchFinder *Finder) {
   Finder->addMatcher(
   namedDecl(anyOf(varDecl(unless(isDefinition())),
-  functionDecl(unless(anyOf(isDefinition(), isDefaulted(),
-hasParent(friendDecl()))
+  functionDecl(unless(anyOf(
+  isDefinition(), isDefaulted(),
+  doesDeclarationForceExternallyVisibleDefinition(),
+  hasParent(friendDecl()))
   .bind("Decl"),
   this);
 }
Index: 
clang-tools-extra/trunk/test/clang-tidy/readability-redundant-declaration.c
===
--- clang-tools-extra/trunk/test/clang-tidy/readability-redundant-declaration.c
+++ clang-tools-extra/trunk/test/clang-tidy/readability-redundant-declaration.c
@@ -0,0 +1,31 @@
+// RUN: %check_clang_tidy %s readability-redundant-declaration %t
+
+extern int Xyz;
+extern int Xyz; // Xyz
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: redundant 'Xyz' declaration 
[readability-redundant-declaration]
+// CHECK-FIXES: {{^}}// Xyz{{$}}
+int Xyz = 123;
+
+extern int A;
+extern int A, B;
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: redundant 'A' declaration
+// CHECK-FIXES: {{^}}extern int A, B;{{$}}
+
+extern int Buf[10];
+extern int Buf[10]; // Buf[10]
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: redundant 'Buf' declaration
+// CHECK-FIXES: {{^}}// Buf[10]{{$}}
+
+static int f();
+static int f(); // f
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: redundant 'f' declaration
+// CHECK-FIXES: {{^}}// f{{$}}
+static int f() {}
+
+inline void g() {}
+
+inline void g();
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: redundant 'g' declaration
+
+// OK: Needed to emit an external definition.
+extern inline void g();
Index: 
clang-tools-extra/trunk/test/clang-tidy/readability-redundant-declaration.cpp
===
--- 
clang-tools-extra/trunk/test/clang-tidy/readability-redundant-declaration.cpp
+++ 
clang-tools-extra/trunk/test/clang-tidy/readability-redundant-declaration.cpp
@@ -68,3 +68,13 @@
 // CHECK-FIXES: {{^}}DEFINE(test);{{$}}
 
 } // namespace macros
+
+inline void g() {}
+
+inline void g(); // g
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: redundant 'g' declaration
+// CHECK-FIXES: {{^}}// g{{$}}
+
+extern inline void g(); // extern g
+// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: redundant 'g' declaration
+// CHECK-FIXES: {{^}}// extern g{{$}}


Index: clang-tools-extra/trunk/clang-tidy/readability/RedundantDeclarationCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/readability/RedundantDeclarationCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/readability/RedundantDeclarationCheck.cpp
@@ -17,6 +17,10 @@
 namespace tidy {
 namespace readability {
 
+AST_MATCHER(FunctionDecl, doesDeclarationForceExternallyVisibleDefinition) {
+  return Node.doesDeclarationForceExternallyVisibleDefinition();
+}
+
 RedundantDeclarationCheck::RedundantDeclarationCheck(StringRef Name,
  ClangTidyContext *Context)
 : ClangTidyCheck(Name, Context),
@@ -25,8 +29,10 @@
 void RedundantDeclarationCheck::registerMatchers(MatchFinder *Finder) {
   Finder->addMatcher(
   namedDecl(anyOf(varDecl(unless(isDefinition())),
-  f

r360614 - Make more friendly with unions. Reviewed as https://reviews.llvm.org/D61858

[Marshall Clow via cfe-commits]

Author: marshall
Date: Mon May 13 12:29:23 2019
New Revision: 360614

URL: http://llvm.org/viewvc/llvm-project?rev=360614&view=rev
Log:
Make  more friendly with unions. Reviewed as https://reviews.llvm.org/D61858

Modified:
cfe/trunk/lib/Sema/SemaExprCXX.cpp
cfe/trunk/test/SemaCXX/type-traits.cpp

Modified: cfe/trunk/lib/Sema/SemaExprCXX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprCXX.cpp?rev=360614&r1=360613&r2=360614&view=diff
==
--- cfe/trunk/lib/Sema/SemaExprCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExprCXX.cpp Mon May 13 12:29:23 2019
@@ -5118,8 +5118,15 @@ static bool EvaluateBinaryTypeTrait(Sema
 assert(Self.Context.hasSameUnqualifiedType(LhsT, RhsT)
  == (lhsRecord == rhsRecord));
 
+// Unions are never base classes, and never have base classes.
+// It doesn't matter if they are complete or not. See PR#41843
+if (lhsRecord && lhsRecord->getDecl()->isUnion())
+  return false;
+if (rhsRecord && rhsRecord->getDecl()->isUnion())
+  return false;
+
 if (lhsRecord == rhsRecord)
-  return !lhsRecord->getDecl()->isUnion();
+  return true;
 
 // C++0x [meta.rel]p2:
 //   If Base and Derived are class types and are different types

Modified: cfe/trunk/test/SemaCXX/type-traits.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/type-traits.cpp?rev=360614&r1=360613&r2=360614&view=diff
==
--- cfe/trunk/test/SemaCXX/type-traits.cpp (original)
+++ cfe/trunk/test/SemaCXX/type-traits.cpp Mon May 13 12:29:23 2019
@@ -14,6 +14,7 @@ typedef NonPOD NonPODArMB[10][2];
 enum Enum { EV };
 struct POD { Enum e; int i; float f; NonPOD* p; };
 struct Empty {};
+struct IncompleteStruct;
 typedef Empty EmptyAr[10];
 typedef Empty EmptyArNB[];
 typedef Empty EmptyArMB[1][2];
@@ -1915,6 +1916,20 @@ void is_base_of() {
   { int arr[F(__is_base_of(Base, NonderivedTemp))]; }
   { int arr[F(__is_base_of(Base, UndefinedTemp))]; } // expected-error 
{{implicit instantiation of undefined template 'UndefinedTemp'}}
 
+  { int arr[F(__is_base_of(IncompleteUnion, IncompleteUnion))]; }
+  { int arr[F(__is_base_of(Union, IncompleteUnion))]; }
+  { int arr[F(__is_base_of(IncompleteUnion, Union))]; }
+  { int arr[F(__is_base_of(IncompleteStruct, IncompleteUnion))]; }
+  { int arr[F(__is_base_of(IncompleteUnion, IncompleteStruct))]; }
+  { int arr[F(__is_base_of(Empty, IncompleteUnion))]; }
+  { int arr[F(__is_base_of(IncompleteUnion, Empty))]; }
+  { int arr[F(__is_base_of(int, IncompleteUnion))]; }
+  { int arr[F(__is_base_of(IncompleteUnion, int))]; }
+  { int arr[F(__is_base_of(Empty, Union))]; }
+  { int arr[F(__is_base_of(Union, Empty))]; }
+  { int arr[F(__is_base_of(int, Empty))]; }
+  { int arr[F(__is_base_of(Union, int))]; }
+
   isBaseOfT();
   isBaseOfF();
 


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

[PATCH] D61858: Make `__is_base_of` more friendly with unions

[Marshall Clow via Phabricator via cfe-commits]

mclow.lists closed this revision.
mclow.lists added a comment.

Committed as revision 360614


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61858/new/

https://reviews.llvm.org/D61858



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

[PATCH] D24892: [clang-tidy] Add option "LiteralInitializers" to cppcoreguidelines-pro-type-member-init

[Matthias Gehre via Phabricator via cfe-commits]

mgehre added a comment.

Do I wait for @alexfh to turn his red into a green, too?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D24892/new/

https://reviews.llvm.org/D24892



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

[PATCH] D59885: [Lex] Allow to consume tokens while preprocessing

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

Thinking about this some more: distinguishing between "macro expansion" and 
other cases seems like a proxy for "this token came from inside the 
preprocessor / lexer" versus "this token was provided by the user", which is 
also exactly what `IsNewToken` is supposed to capture. I don't think we need 
two separate flags here.

I also suspect that a significant amount of the cost is the additional function 
call on the hot path (the 1-parameter `Lex` wrapper function). I wonder if 
there's something that we could do to remove that from the hot path without 
duplicating the whole `Lex` function. Maybe we could add a flag to `Token`s to 
say that they're a replay of prior tokens, rather than being newly-created 
tokens, and make `TokenLexer` set that flag on tokens it produces if its token 
sequence came from a "replay" context (that's largely the same as your current 
"not a macro expansion" check, but there are probably cases where the 
preprocessor itself generates tokens for which that's wrong; we should have a 
flag on `EnterTokenStream` to indicate which mode we're in). We can then also 
change `CachingLex` to set the flag, and get rid of the `IsNewToken` flag too.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59885/new/

https://reviews.llvm.org/D59885



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

[PATCH] D61619: Make language option `GNUAsm` discoverable with `__has_extension` macro.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

Ping.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61619/new/

https://reviews.llvm.org/D61619



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

[PATCH] D61851: [clang-tidy] New option for misc-throw-by-value-catch-by-reference

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tidy/misc/ThrowByValueCatchByReferenceCheck.cpp:25
+  WarnOnLargeObject(Options.get("WarnOnLargeObject", false)),
+  // Cannot access `ASTContext` from here so set it to an extremal value
+  MaxSize(Options.get("MaxSize", std::numeric_limits::max())) {}

Missing a full stop at the end of the comment.



Comment at: 
docs/clang-tidy/checks/misc-throw-by-value-catch-by-reference.rst:37-40
+   Also warns for any large trivial object caught by value. Catching a large
+   object by value is not dangerous but affects the perofrmance negatively. The
+   maximum size of an object allowed to be caught without warning can be set
+   using option `MaxSize`

large trivial object -> large, trivial object
affects the perofrmance -> affects the performance
using option `MaxSize` -> using the `MaxSize` option.


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61851/new/

https://reviews.llvm.org/D61851



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

[PATCH] D61804: Support building shared and static libraries simultaneously

[Wink Saville via Phabricator via cfe-commits]

winksaville added a comment.

> Arch Linux is in an unsupported state, and your patch isn't the way forward.

OK, I'll filed a bug, https://bugs.archlinux.org/task/62624

> Let's rewind.
> 
> Why do "some people like shared libraries"? There are usually two reasons 
> people cite for linking shared libraries. One is reduced binary distribution 
> size because you're reducing duplication of code. The other, is the ability 
> to update libraries without updating tools.
> 
> The later, is not an issue here. LLVM and Clang's C++ libraries don't provide 
> any API or ABI stability guarantees across releases, so there is no way 
> you're going to reasonably update LLVM or Clang libraries without rebuilding 
> the applications using them.
> 
> The former can be valuable, but it comes at a cost. Dynamic resolution of 
> symbols slows down process launch time, and dynamic resolution of C++ 
> link-once ODRs are exceptionally slow. If you are building a compiler, 
> chances are you don't want to slow down your process launch that much. But 
> let's pretend you don't care. Your solution still isn't the right way to do 
> this.
> 
> Creating component shared libraries results in duplicating all the C++ 
> link-once ODRs across each shared module. That will not only slow down your 
> process launch, but bloat the size of your binary distribution.

Learn something new every day, thanks.

> 
> 
> In D61804#1499276 , @winksaville 
> wrote:
> 
>> It looks to me that llvm, libunwind and libcxx support building both, so the 
>> goal isn't unprecedented,
> 
> 
> This is very much an apple and oranges comparison. For one, LLVM does not 
> support generating component dylibs and shared libs. It supports generating 
> libLLVM, a single dylib containing all of the LLVM components rolled 
> together. libunwind, libcxx, libcxxabi, are runtime libraries that are 
> designed to allow static linkage, and they don't link LLVM.
> 
> I apologize that I missed your thread on the dev list, because that would 
> have been a much better place to have this conversation. Having gone back and 
> read it now, it sounds to me like what you really want is a clang equivalent 
> of libLLVM. That is a reasonable and viable thing to want. This patch 
> (https://reviews.llvm.org/P8147) is a first stab. I'm happy to prepare that 
> for landing in Clang if that meets your needs, and that is a viable way 
> forward.

Yes, as @tstellar also said, creating a single clang ".so" that exported the 
same set of symbols as the static libraries (libclang*.a) would be nice and 
probably what Arch Linux would prefer, especially if it was "officially" 
supported and was generated "simultaneously" with the static libraries.

I applied your patch to master:

  $ git log -1
  commit e5a5b7db3ac71a37af790a416486f653dbff47fb (HEAD -> master, 
origin/master, origin/HEAD)
  Author: anatolik 
  Date:   Sun May 12 19:16:54 2019 +
  
  archrelease: copy trunk to testing-x86_64
  
  git-svn-id: file:///srv/repos/svn-packages/svn@353145 
eb2447ed-0c53-47e4-bac8-5bc4a241df78

And built llvm, clang, lld and compiler-rt using:

  $ cmake ../llvm -G Ninja '-DLLVM_ENABLE_PROJECTS=clang;lld;compiler-rt' 
-DCMAKE_INSTALL_PREFIX=/home/wink/local-clang-shlib-master 
-DCMAKE_BUILD_TYPE=Release -DLLVM_BUILD_LLVM_DYLIB=ON
  $ ninja -j11 -v

I see libclang.so*:

  $ ls build-clang-shlib-master/lib/libclang.so* -al
  lrwxrwxrwx 1 wink users   16 May 13 10:49 
build-clang-shlib-master/lib/libclang.so -> libclang.so.9svn
  -rwxr-xr-x 1 wink users 81573360 May 13 10:49 
build-clang-shlib-master/lib/libclang.so.9
  lrwxrwxrwx 1 wink users   13 May 13 10:49 
build-clang-shlib-master/lib/libclang.so.9svn -> libclang.so.9

But the exported symbols are only the clang C api from libclang.exports:

  $ nm -g --demangle build-clang-shlib-master/lib/libclang.so.9 | grep ' T ' | 
wc -l
  381
  $ wc -l clang/tools/libclang/libclang.exports 
  381 clang/tools/libclang/libclang.exports

If I don't use your patch and build the same way lib/libclang.so.9 is still 
created and with the 381 globals:

  $ cmake ../llvm -G Ninja '-DLLVM_ENABLE_PROJECTS=clang;lld;compiler-rt' 
-DCMAKE_INSTALL_PREFIX=/home/wink/local-master -DCMAKE_BUILD_TYPE=Release 
-DLLVM_BUILD_LLVM_DYLIB=ON
  $ ninja -j11 -v
  $ ls build-master/lib/libclang*so* -al
  lrwxrwxrwx 1 wink users   16 May 13 11:35 build-master/lib/libclang.so -> 
libclang.so.9svn
  -rwxr-xr-x 1 wink users 81573360 May 13 11:35 build-master/lib/libclang.so.9
  lrwxrwxrwx 1 wink users   13 May 13 11:35 
build-master/lib/libclang.so.9svn -> libclang.so.9
  $ nm -g --demangle build-master/lib/libclang.so.9 | grep ' T ' | wc -l
  381

I was assuming the goal of clang-shlib was to export all of the symbols as 
exported by the libclang*.a files,
of so the first stab didn't quite do it.

But one thing it did do was reduce the steps and time ninja needed to complete 
my test
build from 4355 steps for the

[PATCH] D61707: [Preprocessor] Fix crash emitting note with framework location for "file not found" error.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

Ping.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61707/new/

https://reviews.llvm.org/D61707



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

[PATCH] D24892: [clang-tidy] Add option "LiteralInitializers" to cppcoreguidelines-pro-type-member-init

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D24892#1500371 , @mgehre wrote:

> Do I wait for @alexfh to turn his red into a green, too?


I think you covered all of the concerns he raised. If he doesn't give you an LG 
in the next 24 hours, I think you can go ahead and commit. If there are 
remaining issues, we can deal with them post-commit.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D24892/new/

https://reviews.llvm.org/D24892



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

[PATCH] D61619: Make language option `GNUAsm` discoverable with `__has_extension` macro.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM!


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61619/new/

https://reviews.llvm.org/D61619



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

[PATCH] D61809: [BPF] Preserve original struct/union type name/access index and array subscripts

[Yonghong Song via Phabricator via cfe-commits]

yonghong-song added a comment.

@rsmith @eli.friedman Thanks for your comments. I fully agree that it seems 
awkward that we have both GEP and intrinsic generation. I will try to do some 
experiments here to only have intrinsic generation. My only concern is possible 
performance degradation. I will post here once I got concrete implementation 
and numbers.

@rsmith regarding to your concerns about struct size change. Yes, the structure 
size may indeed change. Currently, we plan only to support field fields 
(non-struct/union type with 1/2/4/8 bytes, and strings). These are most common 
use cases. Let me explain a little bit more.

Here, we are targeting the bpf_probe_read 
(https://github.com/torvalds/linux/blob/master/include/uapi/linux/bpf.h#L532-L537)

  int bpf_probe_read(void *dst, u32 size, const void *src)

which tries to copy some data from kernel (src pointer) to the stack of the bpf 
program.

Here, if "src" points to a structure, and user intends to copy the whole 
structure out of kernel, then "size" may need to be different for different 
kernels if different kernels indeed have different structure size. 
But first, in all our use cases, users typically do not read the whole 
structure. Second, if "size" does change, we request users to use multiple 
versioning (using patchable global variables) with possibly different "size" 
values for different kernel versions. 
Note that "size" must be constant for bpf verifier to succeed.
(https://github.com/torvalds/linux/blob/master/kernel/trace/bpf_trace.c#L138-L156)

Just gave another two examples here about reading kernel memories in 
iovisor/bcc. Note that
bcc will transform certain pointer access "b->c" to "bpf_probe_read(&dest, 
size, &b->c)" internally.

The bcc tool biosnoop.py contain several bpf probe read:

  https://github.com/iovisor/bcc/blob/master/tools/biosnoop.py#L110
data.len = req->__data_len;
  https://github.com/iovisor/bcc/blob/master/tools/biosnoop.py#L117
data.len = req->__data_len;
  https://github.com/iovisor/bcc/blob/master/tools/biosnoop.py#L118
data.sector = req->__sector;
  https://github.com/iovisor/bcc/blob/master/tools/biosnoop.py#L120
struct gendisk *rq_disk = req->rq_disk;
  https://github.com/iovisor/bcc/blob/master/tools/biosnoop.py#L121-L122
   bpf_probe_read(&data.disk_name, sizeof(data.disk_name),
   rq_disk->disk_name);

They are all primitive types and strings.

Another example for bcc tool tcpconnect.py

  https://github.com/iovisor/bcc/blob/master/tools/tcpconnect.py#L128
u16 dport = skp->__sk_common.skc_dport;
  https://github.com/iovisor/bcc/blob/master/tools/tcpconnect.py#L136-L137
data4.saddr = skp->__sk_common.skc_rcv_saddr;
data4.daddr = skp->__sk_common.skc_daddr;

.

In summary, right now, the to-be-read kernel memory size (through a specific 
bpf_probe_read() call)
won't change between different kernel versions. So we do not need to handle 
structure allocation size change here.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61809/new/

https://reviews.llvm.org/D61809



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

[PATCH] D24892: [clang-tidy] Add option "LiteralInitializers" to cppcoreguidelines-pro-type-member-init

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added inline comments.



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:66-69
+- Added `UseAssignment` option to `cppcoreguidelines-pro-type-member-init`
+
+  If set to true, the check will provide fix-its with literal initializers
+  (``int i = 0;``) instead of curly braces (``int i{};``).

This is in the wrong category


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D24892/new/

https://reviews.llvm.org/D24892



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

[PATCH] D61804: Support building shared and static libraries simultaneously

[Chris Bieneman via Phabricator via cfe-commits]

beanz added a comment.

My change should not have decreased build time from trunk, nor should it have 
reduced the number of build steps. That patch should generate 
lib/libClang_shared.so, which will export the C++ API. libClang is unchanged 
since changing it would break existing clients of the library.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61804/new/

https://reviews.llvm.org/D61804



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

[PATCH] D58920: [Modules][PR39287] Consolidate multiple std's

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

The problem may well be that you're recursively triggering deserialization of 
the `std` namespace from its own deserialization.

In D58920#1500246 , @modocache wrote:

>   @@ -3401,6 +3402,22 @@ ASTDeclReader::FindExistingResult 
> ASTDeclReader::findExisting(NamedDecl *D) {
>  return FindExistingResult(Reader, D, Existing, 
> AnonymousDeclNumber,
>TypedefNameForLinkage);
>}
>   +if (isa(D) && D->getName() == "std") {
>   +  llvm::outs() << "Found std namespace: ";
>   +  D->dump();
>   +  llvm::outs() << "Merging into Sema std namespace: ";
>   +  Sema *S = Reader.getSema();
>   +  S->getStdNamespace()->dump();
>


You shouldn't call `getStdNamespace()` here, because that will trigger 
deserialization. You should query the `LazyDeclPtr` directly to see if there's 
an already-deserialized `std` namespace.

>   +  NamedDecl *Existing =
>   +  getDeclForMerging(S->getStdNamespace(), TypedefNameForLinkage);

(No need to call this, `std` isn't an anonymous struct.)

>   +  llvm::outs() << "Found Existing: ";
>   +  Existing->dump();
>   +  if (isSameEntity(Existing, D)) {
>   +llvm::outs() << "isSameEntity is true\n";
>   +return FindExistingResult(Reader, D, Existing, AnonymousDeclNumber,
>   +  TypedefNameForLinkage);
>   +  }
>   +}
>  } else {
>// Not in a mergeable context.
>return FindExistingResult(Reader);

You should also change `FindExistingResult::~FindExistingResult` to update 
`Sema::StdNamespace` to point to your newly-deserialized namespace if you 
didn't find a prior declaration of it, so that `Sema::getStdNamespace()` finds 
the deserialized namespace.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58920/new/

https://reviews.llvm.org/D58920



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

[PATCH] D61370: Add a C2x mode and allow attributes in it

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Ping x2.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61370/new/

https://reviews.llvm.org/D61370



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

[PATCH] D61861: DeleteNullPointerCheck now deletes until the end brace of the condition

[Jonathan Camilleri via Phabricator via cfe-commits]

J-Camilleri updated this revision to Diff 199315.
J-Camilleri added a comment.

Added a new unit test and restored the modified test as it was.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61861/new/

https://reviews.llvm.org/D61861

Files:
  clang-tools-extra/clang-tidy/readability/DeleteNullPointerCheck.cpp
  clang-tools-extra/test/clang-tidy/readability-delete-null-pointer.cpp


Index: clang-tools-extra/test/clang-tidy/readability-delete-null-pointer.cpp
===
--- clang-tools-extra/test/clang-tidy/readability-delete-null-pointer.cpp
+++ clang-tools-extra/test/clang-tidy/readability-delete-null-pointer.cpp
@@ -3,6 +3,15 @@
 #define NULL 0
 
 void f() {
+  int *ps = 0;
+  if (ps /**/) // #0
+delete ps;
+  // CHECK-MESSAGES: :[[@LINE-2]]:3: warning: 'if' statement is unnecessary; 
deleting null pointer has no effect [readability-delete-null-pointer]
+
+  // CHECK-FIXES: int *ps = 0;
+  // CHECK-FIXES-NEXT: {{^  }}// #0
+  // CHECK-FIXES-NEXT: delete ps;
+
   int *p = 0;
 
   // #1
Index: clang-tools-extra/clang-tidy/readability/DeleteNullPointerCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/DeleteNullPointerCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/DeleteNullPointerCheck.cpp
@@ -7,6 +7,7 @@
 
//===--===//
 
 #include "DeleteNullPointerCheck.h"
+#include "../utils/LexerUtils.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/Lex/Lexer.h"
@@ -62,9 +63,11 @@
 
   Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange(
   IfWithDelete->getBeginLoc(),
-  Lexer::getLocForEndOfToken(IfWithDelete->getCond()->getEndLoc(), 0,
- *Result.SourceManager,
- Result.Context->getLangOpts(;
+  utils::lexer::getPreviousToken(IfWithDelete->getThen()->getBeginLoc(),
+ *Result.SourceManager,
+ Result.Context->getLangOpts())
+  .getLocation()));
+
   if (Compound) {
 Diag << FixItHint::CreateRemoval(
 CharSourceRange::getTokenRange(Compound->getLBracLoc()));


Index: clang-tools-extra/test/clang-tidy/readability-delete-null-pointer.cpp
===
--- clang-tools-extra/test/clang-tidy/readability-delete-null-pointer.cpp
+++ clang-tools-extra/test/clang-tidy/readability-delete-null-pointer.cpp
@@ -3,6 +3,15 @@
 #define NULL 0
 
 void f() {
+  int *ps = 0;
+  if (ps /**/) // #0
+delete ps;
+  // CHECK-MESSAGES: :[[@LINE-2]]:3: warning: 'if' statement is unnecessary; deleting null pointer has no effect [readability-delete-null-pointer]
+
+  // CHECK-FIXES: int *ps = 0;
+  // CHECK-FIXES-NEXT: {{^  }}// #0
+  // CHECK-FIXES-NEXT: delete ps;
+
   int *p = 0;
 
   // #1
Index: clang-tools-extra/clang-tidy/readability/DeleteNullPointerCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/DeleteNullPointerCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/DeleteNullPointerCheck.cpp
@@ -7,6 +7,7 @@
 //===--===//
 
 #include "DeleteNullPointerCheck.h"
+#include "../utils/LexerUtils.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/Lex/Lexer.h"
@@ -62,9 +63,11 @@
 
   Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange(
   IfWithDelete->getBeginLoc(),
-  Lexer::getLocForEndOfToken(IfWithDelete->getCond()->getEndLoc(), 0,
- *Result.SourceManager,
- Result.Context->getLangOpts(;
+  utils::lexer::getPreviousToken(IfWithDelete->getThen()->getBeginLoc(),
+ *Result.SourceManager,
+ Result.Context->getLangOpts())
+  .getLocation()));
+
   if (Compound) {
 Diag << FixItHint::CreateRemoval(
 CharSourceRange::getTokenRange(Compound->getLBracLoc()));
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61804: Support building shared and static libraries simultaneously

[Wink Saville via Phabricator via cfe-commits]

winksaville added a comment.

In D61804#1500409 , @beanz wrote:

> My change should not have decreased build time from trunk, nor should it have 
> reduced the number of build steps. That patch should generate 
> lib/libClang_shared.so, which will export the C++ API. libClang is unchanged 
> since changing it would break existing clients of the library.


OK, I'll look into what I've done wrong. If you see anything or have any 
guesses please let me know.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61804/new/

https://reviews.llvm.org/D61804



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

[PATCH] D61765: [OpenMP][Clang][BugFix] Split declares and math functions inclusion.

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

In D61765#1500309 , @gtbercea wrote:

> As soon as libc++ the limits header included in
>
>   __clang_cuda_cmath.h:15
>   ``` is not found:
>  
>  
>
>
> __clang_cuda_cmath.h:15:10: fatal error: 'limits' file not found
>  #include 
>
>   Not even CUDA works actually so I'm not sure what the best answer to this 
> problem is.


Could you give me more details on how you've got this error?

If this change breaks CUDA compilation with libc++, that's going to be a 
problem. Currently CUDA and clang's headers we ship do work with both libc++ 
and few versions of libstdc++:
E.g: 
http://lab.llvm.org:8011/builders/clang-cuda-build/builds/33364/steps/ninja%20build%20simple%20CUDA%20tests/logs/stdio


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61765/new/

https://reviews.llvm.org/D61765



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

[PATCH] D61634: [clang/llvm] Allow efficient implementation of libc's memory functions in C/C++

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

My main blocker is that I want to make sure we're moving in the right 
direction: towards LLVM IR with clear semantics, towards straightforward rules 
for writing freestanding C code, and towards solutions which behave 
appropriately for all targets.  There's clearly a problem here that's worth 
solving, but I want to make sure we're adding small features that interact with 
existing features in an obvious way.  The patch as written is adding a new 
attribute that changes the semantics of C and LLVM IR in a subtle way that 
interacts with existing optimizations and lowering in ways that are complex and 
hard to understand.

I don't want to mix general restrictions on calling C library functions, with 
restrictions that apply specifically to memcpy: clang generally works on the 
assumption that a "memcpy" symbol is available in freestanding environments, 
and we don't really want to change that.

With -fno-builtin, specifically, we currently apply the restriction that 
optimizations will not introduce memcpy calls that would not exist with 
optimization disabled.  This is sort of artificial, and and maybe a bit 
confusing, but it seems to work well enough in practice.  gcc does something 
similar.

I don't really want to add an attribute that has a different meaning from 
-fno-builtin.  An attribute that has the same meaning is a lot less 
problematic: it reduces potential confusion, and solves a related problem that 
-fno-builtin currently doesn't really work with LTO, because we don't encode it 
into the IR.

Your current patch is using the "AlwaysInline" flag for 
SelectionDAG::getMemcpy, which forces a memcpy to be lowered to an inline 
implementation.  In general, this flag is only supported for constant-size 
memcpy calls; otherwise, on targets where EmitTargetCodeForMemcpy does not have 
some special lowering, like the x86 "rep;movs", you'll hit an assertion 
failure.  And we don't really want to add an implementation of variable-length 
memcpy for a lot of targets; it's very inefficient on targets which don't have 
unaligned memory access.  You could try to narrowly fix this to only apply 
"AlwaysInline" if the size is a constant integer, but there's a related 
problem: even if a memcpy is constant size in C, optimizations don't promise to 
preserve that, in general.  We could theoretically add such a promise, I guess, 
but that's awkward: it would conditionally apply to llvm.memcpy where the 
parameter is already const, so it's not something we can easily verify.

If we added a new builtin `llvm.memcpy.inline`, or something like that, the end 
result ends up being simpler in many ways. It has obvious rules for both 
generating it and lowering it, which don't depend on attributes in the parent 
function.  We could mark the size as "immarg", so we don't have to add special 
optimization rules for it.   And if we have it, we have a "complete" solution 
for writing memcpy implementations in C: we make `__builtin_memcpy`, or a new 
`__builtin_inline_memcpy`, produce it, and it can be combined with -fno-builtin 
to ensure we don't produce any calls to the "real" memcpy.  The downside of a 
new builtin, of course, is that we now have two functions with similar 
semantics, and potentially have to fix a bunch of places to check for both of 
them.

MemCpyOpt has been mentioned (the pass which transforms memcpy-like loops into 
llvm.memcpy).  If we want it to perform some transform in circumstances where 
the call "memcpy" isn't available, we have to make sure to restrict it based on 
the target: in the worst case, on some targets without unaligned memory access, 
it could just act as a low-quality loop unroller.  This applies no matter how 
the result is actually represented in IR.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61634/new/

https://reviews.llvm.org/D61634



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

[PATCH] D61765: [OpenMP][Clang][BugFix] Split declares and math functions inclusion.

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

gtbercea added a comment.

In D61765#1500446 , @tra wrote:

> In D61765#1500309 , @gtbercea wrote:
>
> > As soon as libc++ the limits header included in
> >
> >   __clang_cuda_cmath.h:15
> >   ``` is not found:
> >  
> >  
> >
> >
> > __clang_cuda_cmath.h:15:10: fatal error: 'limits' file not found
> >  #include 
> >
> >   Not even CUDA works actually so I'm not sure what the best answer to this 
> > problem is.
>
>
> Could you give me more details on how you've got this error?
>
> If this change breaks CUDA compilation with libc++, that's going to be a 
> problem. Currently CUDA and clang's headers we ship do work with both libc++ 
> and few versions of libstdc++:
>  E.g: 
> http://lab.llvm.org:8011/builders/clang-cuda-build/builds/33364/steps/ninja%20build%20simple%20CUDA%20tests/logs/stdio


It's an error on my side, I don't have libc++ installed so trying to use it 
will come up with header not found errors.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61765/new/

https://reviews.llvm.org/D61765



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

[PATCH] D61765: [OpenMP][Clang][BugFix] Split declares and math functions inclusion.

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

gtbercea added a comment.

In D61765#1500446 , @tra wrote:

> In D61765#1500309 , @gtbercea wrote:
>
> > As soon as libc++ the limits header included in
> >
> >   __clang_cuda_cmath.h:15
> >   ``` is not found:
> >  
> >  
> >
> >
> > __clang_cuda_cmath.h:15:10: fatal error: 'limits' file not found
> >  #include 
> >
> >   Not even CUDA works actually so I'm not sure what the best answer to this 
> > problem is.
>
>
> Could you give me more details on how you've got this error?
>
> If this change breaks CUDA compilation with libc++, that's going to be a 
> problem. Currently CUDA and clang's headers we ship do work with both libc++ 
> and few versions of libstdc++:
>  E.g: 
> http://lab.llvm.org:8011/builders/clang-cuda-build/builds/33364/steps/ninja%20build%20simple%20CUDA%20tests/logs/stdio


This won't affect CUDA in any way, all we have added is OpenMP specific.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61765/new/

https://reviews.llvm.org/D61765



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

[PATCH] D60162: [ThinLTO] Support TargetLibraryInfoImpl in the backend

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson updated this revision to Diff 199320.
tejohnson added a comment.
Herald added subscribers: cfe-commits, hiraditya, eraman.
Herald added a project: clang.

Rework using module flags.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60162/new/

https://reviews.llvm.org/D60162

Files:
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGen/nobuiltins.c
  clang/test/CodeGen/svml-calls.ll
  clang/test/CodeGen/thinlto_backend_nobuiltin.ll
  clang/test/CodeGen/thinlto_backend_nobuiltin_memset.ll
  clang/test/CodeGen/veclib-calls.ll
  clang/test/CodeGen/veclib.c
  llvm/lib/LTO/LTOBackend.cpp

Index: llvm/lib/LTO/LTOBackend.cpp
===
--- llvm/lib/LTO/LTOBackend.cpp
+++ llvm/lib/LTO/LTOBackend.cpp
@@ -218,6 +218,34 @@
   // FIXME (davide): verify the output.
 }
 
+static TargetLibraryInfoImpl *createTLII(Module &Mod, TargetMachine *TM) {
+  TargetLibraryInfoImpl *TLII =
+  new TargetLibraryInfoImpl(Triple(TM->getTargetTriple()));
+  if (auto *MD = mdconst::extract_or_null(
+  Mod.getModuleFlag("DisableAllBuiltins"))) {
+if (MD->getZExtValue())
+  TLII->disableAllFunctions();
+  } else if (Metadata *Val = Mod.getModuleFlag("NoBuiltins")) {
+// Disable individual libc/libm calls in TargetLibraryInfo.
+LibFunc F;
+for (const MDOperand &FuncName : cast(Val)->operands())
+  if (TLII->getLibFunc(cast(*FuncName).getString(), F))
+TLII->setUnavailable(F);
+  }
+
+  if (MDString *VL =
+  dyn_cast_or_null(Mod.getModuleFlag("VectorLibrary"))) {
+if (VL->getString() == "Accelerate")
+  TLII->addVectorizableFunctionsFromVecLib(
+  TargetLibraryInfoImpl::Accelerate);
+else if (VL->getString() == "SVML")
+  TLII->addVectorizableFunctionsFromVecLib(TargetLibraryInfoImpl::SVML);
+else
+  llvm_unreachable("Invalid vector library module flag");
+  }
+  return TLII;
+}
+
 static void runNewPMCustomPasses(Module &Mod, TargetMachine *TM,
  std::string PipelineDesc,
  std::string AAPipelineDesc,
@@ -239,6 +267,10 @@
   // Register the AA manager first so that our version is the one used.
   FAM.registerPass([&] { return std::move(AA); });
 
+  std::unique_ptr TLII(createTLII(Mod, TM));
+  FAM.registerPass([&] { return TargetLibraryAnalysis(*TLII); });
+  MAM.registerPass([&] { return TargetLibraryAnalysis(*TLII); });
+
   // Register all the basic analyses with the managers.
   PB.registerModuleAnalyses(MAM);
   PB.registerCGSCCAnalyses(CGAM);
@@ -268,7 +300,7 @@
   passes.add(createTargetTransformInfoWrapperPass(TM->getTargetIRAnalysis()));
 
   PassManagerBuilder PMB;
-  PMB.LibraryInfo = new TargetLibraryInfoImpl(Triple(TM->getTargetTriple()));
+  PMB.LibraryInfo = createTLII(Mod, TM);
   PMB.Inliner = createFunctionInliningPass();
   PMB.ExportSummary = ExportSummary;
   PMB.ImportSummary = ImportSummary;
Index: clang/test/CodeGen/veclib.c
===
--- /dev/null
+++ clang/test/CodeGen/veclib.c
@@ -0,0 +1,21 @@
+// RUN: %clang_cc1 -emit-llvm -fveclib=Accelerate %s -o - | FileCheck %s -check-prefix=ACCELERATE
+// RUN: %clang_cc1 -emit-llvm -fveclib=SVML %s -o - | FileCheck %s -check-prefix=SVML
+// RUN: %clang_cc1 -emit-llvm -fveclib=none %s -o - | FileCheck %s -check-prefix=NONE
+// RUN: %clang_cc1 -emit-llvm %s -o - | FileCheck %s -check-prefix=NONE
+
+// ACCELERATE: !{{[0-9]+}} = !{i32 1, !"VectorLibrary", !"Accelerate"}
+// SVML: !{{[0-9]+}} = !{i32 1, !"VectorLibrary", !"SVML"}
+// NONE-NOT: VectorLibrary
+
+// Now ensure merging gets the expected behavior
+// RUN: %clang -c -flto %s -o %t0.o
+// RUN: %clang -c -flto -fveclib=Accelerate %s -o %t1.o
+// RUN: %clang -c -flto -fveclib=SVML %s -o %t2.o
+// Merge none with -fveclib=Accelerate -> VectorLibrary=Accelerate
+// RUN: llvm-lto %t0.o %t1.o -o %t3.o -save-merged-module
+// RUN: llvm-dis %t3.o.merged.bc -o - | FileCheck %s --check-prefix=ACCELERATE
+// Merge none with -fveclib=SVML -> VectorLibrary=SVML
+// RUN: llvm-lto %t0.o %t2.o -o %t3.o -save-merged-module
+// RUN: llvm-dis %t3.o.merged.bc -o - | FileCheck %s --check-prefix=SVML
+// Merge -fveclib=Accelerate with -fveclib=SVML -> Error
+// RUN: not llvm-lto %t1.o %t2.o -o %t3.o -save-merged-module
Index: clang/test/CodeGen/veclib-calls.ll
===
--- /dev/null
+++ clang/test/CodeGen/veclib-calls.ll
@@ -0,0 +1,38 @@
+; Test to ensure that -fveclib=Accelerate module flag is handled properly in
+; the ThinLTO distributed backend.
+
+; RUN: opt -module-summary -o %t.o %s
+; RUN: llvm-lto -thinlto -o %t %t.o
+; RUN: %clang -target x86_64-unknown-linux-gnu -O3 -o %t2.o -x ir %t.o -c -fthinlto-index=%t.thinlto.bc -save-temps=obj
+; RUN: llvm-dis %t.s.4.opt.bc -o - | FileCheck %s
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target t

[PATCH] D60162: [ThinLTO] Support TargetLibraryInfoImpl in the backend

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson added a comment.

In D60162#1498392 , @tejohnson wrote:

> In D60162#1498288 , @tejohnson wrote:
>
> > Working on this one again as it previously wasn't causing any issues but 
> > just now got exposed in multiple places and started causing correctness 
> > issues. Two questions:
> >
> > 1. What should be the behavior when merging modules e.g. for LTO. I'm 
> > thinking of the following, with multiple module flags to specify different 
> > aspects: a) For the NoBuiltinFuncs I'm thinking of a module flag containing 
> > the list of strings, with AppendUnique merging behavior, so we get a 
> > superset in the merged module b) For the flag to disable all builtins, have 
> > a separate module flag with a Max merge (so we get the most conservative 
> > behavior) c) For the VectorLibrary, not sure - should it be an Error to 
> > merge modules with different libraries?
> > 2. While we get the above hammered out, would it be ok to submit this one 
> > (and companion clang change) to fix the bug (then remove when the module 
> > flags are added)?
>
>
> Weird, phabricator added numbering to all of my bullets, which formatted very 
> differently than intended. Modified the above a bit to hopefully format 
> better (2 high level questions, and some subquestions on the first one).


I went ahead and implemented the above approach, PTAL. I added test cases for 
ensuring that the various clang driver flags set up the module flags properly, 
that merging behaves as expected, and that these options correctly affect LTO 
backends.

Note that this patch now includes both clang and llvm changes. I am going to 
mark the old clang child patch as obsolete.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60162/new/

https://reviews.llvm.org/D60162



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

[PATCH] D60910: [WIP] Dumping the AST to JSON

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman updated this revision to Diff 199316.
aaron.ballman marked 2 inline comments as done.
aaron.ballman added a comment.

Switched to using the JSON streaming interface rather than a custom solution
Updated the test cases for the new formatting


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60910/new/

https://reviews.llvm.org/D60910

Files:
  include/clang/AST/ASTDumperUtils.h
  include/clang/AST/DeclBase.h
  include/clang/AST/JSONNodeDumper.h
  include/clang/Driver/CC1Options.td
  include/clang/Frontend/ASTConsumers.h
  include/clang/Frontend/FrontendOptions.h
  lib/AST/ASTDumper.cpp
  lib/AST/CMakeLists.txt
  lib/AST/JSONNodeDumper.cpp
  lib/Frontend/ASTConsumers.cpp
  lib/Frontend/CompilerInvocation.cpp
  lib/Frontend/FrontendActions.cpp
  test/AST/ast-dump-enum-json.cpp
  test/AST/ast-dump-if-json.cpp
  test/AST/ast-dump-namespace-json.cpp
  tools/clang-check/ClangCheck.cpp
  tools/clang-import-test/clang-import-test.cpp

Index: tools/clang-import-test/clang-import-test.cpp
===
--- tools/clang-import-test/clang-import-test.cpp
+++ tools/clang-import-test/clang-import-test.cpp
@@ -316,8 +316,9 @@
   auto &CG = *static_cast(ASTConsumers.back().get());
 
   if (ShouldDumpAST)
-ASTConsumers.push_back(CreateASTDumper(nullptr /*Dump to stdout.*/,
-   "", true, false, false));
+ASTConsumers.push_back(
+CreateASTDumper(nullptr /*Dump to stdout.*/, "", true, false, false,
+clang::ADOF_Default));
 
   CI.getDiagnosticClient().BeginSourceFile(
   CI.getCompilerInstance().getLangOpts(),
Index: tools/clang-check/ClangCheck.cpp
===
--- tools/clang-check/ClangCheck.cpp
+++ tools/clang-check/ClangCheck.cpp
@@ -134,11 +134,11 @@
 if (ASTList)
   return clang::CreateASTDeclNodeLister();
 if (ASTDump)
-  return clang::CreateASTDumper(nullptr /*Dump to stdout.*/,
-ASTDumpFilter,
+  return clang::CreateASTDumper(nullptr /*Dump to stdout.*/, ASTDumpFilter,
 /*DumpDecls=*/true,
 /*Deserialize=*/false,
-/*DumpLookups=*/false);
+/*DumpLookups=*/false,
+clang::ADOF_Default);
 if (ASTPrint)
   return clang::CreateASTPrinter(nullptr, ASTDumpFilter);
 return llvm::make_unique();
Index: test/AST/ast-dump-namespace-json.cpp
===
--- test/AST/ast-dump-namespace-json.cpp
+++ test/AST/ast-dump-namespace-json.cpp
@@ -0,0 +1,211 @@
+// RUN: %clang_cc1 -triple x86_64-pc-linux -std=c++2a -ast-dump=json %s | FileCheck %s
+
+namespace foo {
+}
+// CHECK: "kind": "NamespaceDecl",
+// CHECK-NEXT: "loc": {
+// CHECK-NEXT: "col": 11,
+// CHECK-NEXT: "file": "{{.*}}",
+// CHECK-NEXT: "line": 3
+// CHECK-NEXT: },
+// CHECK-NEXT: "range": {
+// CHECK-NEXT: "begin": {
+// CHECK-NEXT: "col": 1,
+// CHECK-NEXT: "file": "{{.*}}",
+// CHECK-NEXT: "line": 3
+// CHECK-NEXT: },
+// CHECK-NEXT: "end": {
+// CHECK-NEXT: "col": 1,
+// CHECK-NEXT: "file": "{{.*}}",
+// CHECK-NEXT: "line": 4
+// CHECK-NEXT: }
+// CHECK-NEXT: },
+// CHECK-NEXT: "name": "foo"
+// CHECK-NEXT: },
+
+
+namespace {
+}
+// CHECK: "kind": "NamespaceDecl",
+// CHECK-NEXT: "loc": {
+// CHECK-NEXT: "col": 11,
+// CHECK-NEXT: "file": "{{.*}}",
+// CHECK-NEXT: "line": 27
+// CHECK-NEXT: },
+// CHECK-NEXT: "range": {
+// CHECK-NEXT: "begin": {
+// CHECK-NEXT: "col": 1,
+// CHECK-NEXT: "file": "{{.*}}",
+// CHECK-NEXT: "line": 27
+// CHECK-NEXT: },
+// CHECK-NEXT: "end": {
+// CHECK-NEXT: "col": 1,
+// CHECK-NEXT: "file": "{{.*}}",
+// CHECK-NEXT: "line": 28
+// CHECK-NEXT: }
+// CHECK-NEXT: }
+// CHECK-NEXT: },
+// CHECK-NEXT: {
+// CHECK-NEXT: "id": "0x{{.*}}",
+// CHECK-NEXT: "kind": "UsingDirectiveDecl",
+// CHECK-NEXT: "loc": {},
+// CHECK-NEXT: "range": {
+// CHECK-NEXT: "begin": {
+// CHECK-NEXT: "col": 11,
+// CHECK-NEXT: "file": "{{.*}}",
+// CHECK-NEXT: "line": 27
+// CHECK-NEXT: },
+// CHECK-NEXT: "end": {}
+// CHECK-NEXT: },
+// CHECK-NEXT: "isImplicit": true,
+// CHECK-NEXT: "nominatedNamespace": {
+// CHECK-NEXT: "id": "0x{{.*}}",
+// CHECK-NEXT: "kind": "NamespaceDecl",
+// CHECK-NEXT: "name": ""
+// CHECK-NEXT: }
+// CHECK-NEXT: },
+
+namespace bar {
+inline namespace __1 {
+}
+}
+// CHECK: "kind": "NamespaceDecl",
+// CHECK-NEXT: "loc": {
+// CHECK-NEXT: "col": 11,
+// CHECK-NEXT: "file": "{{.*}}",
+// CHECK-NEXT: "line": 68
+// CHECK-NEXT: },
+// CHECK-NEXT: "range": {
+// CHECK-NEXT: "begin": {
+// CHECK-NEXT: "col": 1,
+// CHECK-NEXT: "file": "{{.*}}",
+// CHECK-NEXT: "line": 68
+// CHECK-NEXT: },
+// CHECK-NEXT: "end": {
+// CHECK-NEXT: "col": 1,
+// CHECK-NEXT: "file": "{{.*}}",
+// CHECK-NEXT: "line": 71
+// CHECK-NEXT: }
+// CHECK-NEXT:

[PATCH] D60163: [ThinLTO] Handle -fno-builtin* options for distributed backends

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson abandoned this revision.
tejohnson added a comment.

The parent revision now includes both clang and llvm side changes.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60163/new/

https://reviews.llvm.org/D60163



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