[PATCH] D70366: Add new 'flatten' LLVM attribute to fix clang's 'flatten' function attribute

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

Thank you for working on this!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70366



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

[PATCH] D69990: Populate CUDA flags on FreeBSD too, as many other toolchains do.

[Dimitry Andric via Phabricator via cfe-commits]

dim edited reviewers, added: dim, emaste; removed: clang.
dim added a comment.

I'll work on a test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69990



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

[PATCH] D70196: [clang-include-fixer] Skip .rc files when finding symbols

[Kim Gräsman via Phabricator via cfe-commits]

kimgr added a comment.

Oh, I see. I thought `rc.exe` was the one and only implementation. Fair enough, 
this is probably the lesser of two evils :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70196



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

[PATCH] D70274: [clang][IFS] Driver pipeline change for clang-ifs: generate interface stubs after standard pipeline.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi updated this revision to Diff 229747.
plotfi marked an inline comment as done.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70274

Files:
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/InterfaceStubs.cpp
  clang/lib/Driver/Types.cpp
  clang/test/InterfaceStubs/driver-test.c
  clang/test/InterfaceStubs/driver-test2.c
  clang/test/InterfaceStubs/windows.cpp

Index: clang/test/InterfaceStubs/windows.cpp
===
--- clang/test/InterfaceStubs/windows.cpp
+++ clang/test/InterfaceStubs/windows.cpp
@@ -1,6 +1,7 @@
 // REQUIRES: x86-registered-target
 // RUN: %clang_cc1 -triple x86_64-windows-msvc -o - %s -emit-interface-stubs | FileCheck -check-prefix=CHECK-CC1 %s
-// RUN: %clang -target x86_64-windows-msvc -o - %s -emit-interface-stubs -emit-merged-ifs | FileCheck -check-prefix=CHECK-IFS %s
+// RUN: %clang -target x86_64-windows-msvc -o - %s -emit-interface-stubs -emit-merged-ifs -S | FileCheck -check-prefix=CHECK-IFS %s
+// note: -S is added here to prevent clang from invoking link.exe
 
 // CHECK-CC1: Symbols:
 // CHECK-CC1-NEXT: ?helloWindowsMsvc@@YAHXZ
Index: clang/test/InterfaceStubs/driver-test2.c
===
--- /dev/null
+++ clang/test/InterfaceStubs/driver-test2.c
@@ -0,0 +1,14 @@
+// REQUIRES: x86-registered-target
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -c -emit-interface-stubs \
+// RUN:   %s %S/object.c %S/weak.cpp
+// RUN: %clang -emit-interface-stubs -emit-merged-ifs \
+// RUN:   driver-test2.o object.o weak.o -S -o - | FileCheck %s
+
+// CHECK-DAG: data
+// CHECK-DAG: bar
+// CHECK-DAG: strongFunc
+// CHECK-DAG: weakFunc
+
+int bar(int a) { return a; }
+int main() { return 0; }
Index: clang/test/InterfaceStubs/driver-test.c
===
--- clang/test/InterfaceStubs/driver-test.c
+++ clang/test/InterfaceStubs/driver-test.c
@@ -1,11 +1,13 @@
 // REQUIRES: x86-registered-target
 
-// RUN: %clang -target x86_64-unknown-linux-gnu -x c -o %t1.so -emit-interface-stubs %s %S/object.c %S/weak.cpp && \
-// RUN: llvm-nm %t1.so 2>&1 | FileCheck --check-prefix=CHECK-IFS %s
+// RUN: %clang -target x86_64-unknown-linux-gnu -x c -o %t1 -emit-interface-stubs %s %S/object.c %S/weak.cpp
+// RUN: llvm-nm %t1   2>&1 | FileCheck %s
+// RUN: llvm-nm %t1.ifso 2>&1 | FileCheck %s
 
-// CHECK-IFS-DAG: data
-// CHECK-IFS-DAG: foo
-// CHECK-IFS-DAG: strongFunc
-// CHECK-IFS-DAG: weakFunc
+// CHECK-DAG: data
+// CHECK-DAG: foo
+// CHECK-DAG: strongFunc
+// CHECK-DAG: weakFunc
 
 int foo(int bar) { return 42 + 1844; }
+int main() { return foo(23); }
Index: clang/lib/Driver/Types.cpp
===
--- clang/lib/Driver/Types.cpp
+++ clang/lib/Driver/Types.cpp
@@ -330,22 +330,6 @@
 llvm::copy_if(PhaseList, std::back_inserter(P),
   [](phases::ID Phase) { return Phase <= phases::Precompile; });
 
-  // Treat Interface Stubs like its own compilation mode.
-  else if (DAL.getLastArg(options::OPT_emit_interface_stubs)) {
-llvm::SmallVector IfsModePhaseList;
-llvm::SmallVector  = PhaseList;
-phases::ID LastPhase = phases::IfsMerge;
-if (Id != types::TY_IFS) {
-  if (DAL.hasArg(options::OPT_c))
-LastPhase = phases::Compile;
-  PL = IfsModePhaseList;
-  types::getCompilationPhases(types::TY_IFS_CPP, PL);
-}
-llvm::copy_if(PL, std::back_inserter(P), [&](phases::ID Phase) {
-  return Phase <= LastPhase;
-});
-  }
-
   // -{fsyntax-only,-analyze,emit-ast} only run up to the compiler.
   else if (DAL.getLastArg(options::OPT_fsyntax_only) ||
DAL.getLastArg(options::OPT_print_supported_cpus) ||
Index: clang/lib/Driver/ToolChains/InterfaceStubs.cpp
===
--- clang/lib/Driver/ToolChains/InterfaceStubs.cpp
+++ clang/lib/Driver/ToolChains/InterfaceStubs.cpp
@@ -9,6 +9,7 @@
 #include "InterfaceStubs.h"
 #include "CommonArgs.h"
 #include "clang/Driver/Compilation.h"
+#include "llvm/Support/Path.h"
 
 namespace clang {
 namespace driver {
@@ -21,13 +22,36 @@
   std::string Merger = getToolChain().GetProgramPath(getShortName());
   llvm::opt::ArgStringList CmdArgs;
   CmdArgs.push_back("-action");
-  CmdArgs.push_back(Args.getLastArg(options::OPT_emit_merged_ifs)
-? "write-ifs"
-: "write-bin");
+  const bool WriteBin = !Args.getLastArg(options::OPT_emit_merged_ifs);
+  CmdArgs.push_back(WriteBin ? "write-bin" : "write-ifs");
   CmdArgs.push_back("-o");
-  CmdArgs.push_back(Output.getFilename());
-  for (const auto  : Inputs)
-CmdArgs.push_back(Input.getFilename());
+
+  // Normally we want to write to a side-car file ending in ".ifso" so for
+  // example if `clang -emit-interface-stubs -shared -o libhello.so` were
+  

[PATCH] D70274: [clang][IFS] Driver pipeline change for clang-ifs: generate interface stubs after standard pipeline.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi updated this revision to Diff 229745.
plotfi marked 2 inline comments as done.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70274

Files:
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/InterfaceStubs.cpp
  clang/lib/Driver/Types.cpp
  clang/test/InterfaceStubs/driver-test.c
  clang/test/InterfaceStubs/driver-test2.c
  clang/test/InterfaceStubs/windows.cpp

Index: clang/test/InterfaceStubs/windows.cpp
===
--- clang/test/InterfaceStubs/windows.cpp
+++ clang/test/InterfaceStubs/windows.cpp
@@ -1,6 +1,7 @@
 // REQUIRES: x86-registered-target
 // RUN: %clang_cc1 -triple x86_64-windows-msvc -o - %s -emit-interface-stubs | FileCheck -check-prefix=CHECK-CC1 %s
-// RUN: %clang -target x86_64-windows-msvc -o - %s -emit-interface-stubs -emit-merged-ifs | FileCheck -check-prefix=CHECK-IFS %s
+// RUN: %clang -target x86_64-windows-msvc -o - %s -emit-interface-stubs -emit-merged-ifs -S | FileCheck -check-prefix=CHECK-IFS %s
+// note: -S is added here to prevent clang from invoking link.exe
 
 // CHECK-CC1: Symbols:
 // CHECK-CC1-NEXT: ?helloWindowsMsvc@@YAHXZ
Index: clang/test/InterfaceStubs/driver-test2.c
===
--- /dev/null
+++ clang/test/InterfaceStubs/driver-test2.c
@@ -0,0 +1,14 @@
+// REQUIRES: x86-registered-target
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -c -emit-interface-stubs \
+// RUN:   %s %S/object.c %S/weak.cpp
+// RUN: %clang -emit-interface-stubs -emit-merged-ifs \
+// RUN:   driver-test2.o object.o weak.o -S -o - | FileCheck %s
+
+// CHECK-DAG: data
+// CHECK-DAG: bar
+// CHECK-DAG: strongFunc
+// CHECK-DAG: weakFunc
+
+int bar(int a) { return a; }
+int main() { return 0; }
Index: clang/test/InterfaceStubs/driver-test.c
===
--- clang/test/InterfaceStubs/driver-test.c
+++ clang/test/InterfaceStubs/driver-test.c
@@ -1,11 +1,13 @@
 // REQUIRES: x86-registered-target
 
-// RUN: %clang -target x86_64-unknown-linux-gnu -x c -o %t1.so -emit-interface-stubs %s %S/object.c %S/weak.cpp && \
-// RUN: llvm-nm %t1.so 2>&1 | FileCheck --check-prefix=CHECK-IFS %s
+// RUN: %clang -target x86_64-unknown-linux-gnu -x c -o %t1 -emit-interface-stubs %s %S/object.c %S/weak.cpp
+// RUN: llvm-nm %t1   2>&1 | FileCheck %s
+// RUN: llvm-nm %t1.ifso 2>&1 | FileCheck %s
 
-// CHECK-IFS-DAG: data
-// CHECK-IFS-DAG: foo
-// CHECK-IFS-DAG: strongFunc
-// CHECK-IFS-DAG: weakFunc
+// CHECK-DAG: data
+// CHECK-DAG: foo
+// CHECK-DAG: strongFunc
+// CHECK-DAG: weakFunc
 
 int foo(int bar) { return 42 + 1844; }
+int main() { return foo(23); }
Index: clang/lib/Driver/Types.cpp
===
--- clang/lib/Driver/Types.cpp
+++ clang/lib/Driver/Types.cpp
@@ -330,22 +330,6 @@
 llvm::copy_if(PhaseList, std::back_inserter(P),
   [](phases::ID Phase) { return Phase <= phases::Precompile; });
 
-  // Treat Interface Stubs like its own compilation mode.
-  else if (DAL.getLastArg(options::OPT_emit_interface_stubs)) {
-llvm::SmallVector IfsModePhaseList;
-llvm::SmallVector  = PhaseList;
-phases::ID LastPhase = phases::IfsMerge;
-if (Id != types::TY_IFS) {
-  if (DAL.hasArg(options::OPT_c))
-LastPhase = phases::Compile;
-  PL = IfsModePhaseList;
-  types::getCompilationPhases(types::TY_IFS_CPP, PL);
-}
-llvm::copy_if(PL, std::back_inserter(P), [&](phases::ID Phase) {
-  return Phase <= LastPhase;
-});
-  }
-
   // -{fsyntax-only,-analyze,emit-ast} only run up to the compiler.
   else if (DAL.getLastArg(options::OPT_fsyntax_only) ||
DAL.getLastArg(options::OPT_print_supported_cpus) ||
Index: clang/lib/Driver/ToolChains/InterfaceStubs.cpp
===
--- clang/lib/Driver/ToolChains/InterfaceStubs.cpp
+++ clang/lib/Driver/ToolChains/InterfaceStubs.cpp
@@ -9,6 +9,7 @@
 #include "InterfaceStubs.h"
 #include "CommonArgs.h"
 #include "clang/Driver/Compilation.h"
+#include "llvm/Support/Path.h"
 
 namespace clang {
 namespace driver {
@@ -21,13 +22,36 @@
   std::string Merger = getToolChain().GetProgramPath(getShortName());
   llvm::opt::ArgStringList CmdArgs;
   CmdArgs.push_back("-action");
-  CmdArgs.push_back(Args.getLastArg(options::OPT_emit_merged_ifs)
-? "write-ifs"
-: "write-bin");
+  const bool WriteBin = !Args.getLastArg(options::OPT_emit_merged_ifs);
+  CmdArgs.push_back(WriteBin ? "write-bin" : "write-ifs");
   CmdArgs.push_back("-o");
-  CmdArgs.push_back(Output.getFilename());
-  for (const auto  : Inputs)
-CmdArgs.push_back(Input.getFilename());
+
+  // Normally we want to write to a side-car file ending in ".ifso" so for
+  // example if `clang -emit-interface-stubs -shared -o libhello.so` were
+  

[PATCH] D70368: [clang-tidy] Rewrite modernize-avoid-bind check

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added a comment.

Please describe changes in documentation and Release Notes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70368



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

[PATCH] D69979: clang: Guess at some platform FTZ/DAZ default settings

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added a comment.

In D69979#1746043 , @spatel wrote:

> In D69979#1740294 , @arsenm wrote:
>
> > In D69979#1738099 , @craig.topper 
> > wrote:
> >
> > > I checked Redhat 7.4 that's on the server I'm using for work. And I had a 
> > > coworker check his Ubuntu 18.04 system with this program. And both 
> > > systems printed 1f80 as the value of MXCSR which shows FTZ and DAZ are 
> > > both 0. Are you seeing something different?
> > >
> > >   #include 
> > >   #include 
> > >  
> > >   int main() {
> > > int csr = _mm_getcsr();
> > > printf("%x\n", csr);
> > > return 0;
> > >   }
> > >
> >
> >
> > I see the value as 1f80. However the test program I wrote suggests the 
> > default is to flush (and what the comments in bug 34994 suggest?):
>
>
> Is the test program attached somewhere? 
>  Bug 34994 (https://bugs.llvm.org/show_bug.cgi?id=34994) was limited to 
> changing cases where we are running in some kind of loose-FP environment 
> (otherwise, we would not be generating a sqrt estimate sequence at all). In 
> the default (IEEE-compliant) environment, x86 would use a full-precision sqrt 
> instruction or make a call to libm sqrt.


I just posted the test I wrote here: https://github.com/arsenm/subnormal_test


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

https://reviews.llvm.org/D69979



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

[PATCH] D69991: Implement __attribute__((objc_direct)), __attribute__((objc_direct_members))

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Okay.  There's actually a good reason to emit the selector reference as close 
to the call as possible, but I agree that we shouldn't do that in this patch.


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

https://reviews.llvm.org/D69991



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

[PATCH] D70366: Add new 'flatten' LLVM attribute to fix clang's 'flatten' function attribute

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added inline comments.



Comment at: llvm/docs/LangRef.rst:1428
 can prove that the call/invoke cannot call a convergent function.
+``flatten``
+This attribute is similar to ``alwaysinline``, but applies recursively to

It's not obvious to me what the flatten name means. flatteninline? 
recursive_alwaysinline? Something else?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70366



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

[PATCH] D69991: Implement __attribute__((objc_direct)), __attribute__((objc_direct_members))

[Pierre Habouzit via Phabricator via cfe-commits]

MadCoder updated this revision to Diff 229743.
MadCoder added a comment.

Diff against previous is:

  diff
  diff --git a/clang/lib/CodeGen/CGObjCMac.cpp b/clang/lib/CodeGen/CGObjCMac.cpp
  index 814f67787dd..775e3406da7 100644
  --- a/clang/lib/CodeGen/CGObjCMac.cpp
  +++ b/clang/lib/CodeGen/CGObjCMac.cpp
  @@ -2159,21 +2159,23 @@ 
CGObjCCommonMac::EmitMessageSend(CodeGen::CodeGenFunction ,
const ObjCInterfaceDecl *ClassReceiver,
const ObjCCommonTypesHelper ) {
 CodeGenTypes  = CGM.getTypes();
  -  CallArgList ActualArgs;
  -  if (!IsSuper)
  -Arg0 = CGF.Builder.CreateBitCast(Arg0, ObjCTypes.ObjectPtrTy);
  -  ActualArgs.add(RValue::get(Arg0), Arg0Ty);
  +  auto selTy = CGF.getContext().getObjCSelType();
  +  llvm::Value *SelValue;
  +
 if (Method && Method->isDirectMethod()) {
   // Direct methods will synthesize the proper `_cmd` internally,
   // so just don't bother with setting the `_cmd` argument.
   assert(!IsSuper);
  -auto selTy = CGF.getContext().getObjCSelType();
  -auto undefSel = llvm::UndefValue::get(Types.ConvertType(selTy));
  -ActualArgs.add(RValue::get(undefSel), selTy);
  +SelValue = llvm::UndefValue::get(Types.ConvertType(selTy));
 } else {
  -ActualArgs.add(RValue::get(GetSelector(CGF, Sel)),
  -   CGF.getContext().getObjCSelType());
  +SelValue = GetSelector(CGF, Sel);
 }
  +
  +  CallArgList ActualArgs;
  +  if (!IsSuper)
  +Arg0 = CGF.Builder.CreateBitCast(Arg0, ObjCTypes.ObjectPtrTy);
  +  ActualArgs.add(RValue::get(Arg0), Arg0Ty);
  +  ActualArgs.add(RValue::get(SelValue), selTy);
 ActualArgs.addFrom(CallArgs);
  
 // If we're calling a method, use the formal signature.
  @@ -7622,7 +7624,7 @@ Address 
CGObjCNonFragileABIMac::EmitSelectorAddr(CodeGenFunction ,
 llvm::ConstantExpr::getBitCast(GetMethodVarName(Sel),
ObjCTypes.SelectorPtrTy);
   std::string SectionName =
  -GetSectionName("__objc_selrefs", "literal_pointers");
  +GetSectionName("__objc_selrefs", "literal_pointers,no_dead_strip");
   Entry = new llvm::GlobalVariable(
   CGM.getModule(), ObjCTypes.SelectorPtrTy, false,
   getLinkageTypeForObjCMetadata(CGM, SectionName), Casted,

Basically, I emit the selref before emiting Arg0 as it used to, to avoid having 
to change all the tests.
I also put the "no_dead_strip" on selector sections back, as it's a remnant 
from an old iteration and this change is not needed for this per se (if we mean 
to do it it should be its own change).


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

https://reviews.llvm.org/D69991

Files:
  clang/include/clang/AST/DeclObjC.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/ObjCRuntime.h
  clang/include/clang/Sema/DeclSpec.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/DeclObjC.cpp
  clang/lib/AST/DeclPrinter.cpp
  clang/lib/AST/JSONNodeDumper.cpp
  clang/lib/AST/TextNodeDumper.cpp
  clang/lib/CodeGen/CGObjC.cpp
  clang/lib/CodeGen/CGObjCGNU.cpp
  clang/lib/CodeGen/CGObjCMac.cpp
  clang/lib/CodeGen/CGObjCRuntime.h
  clang/lib/Parse/ParseObjc.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaDeclObjC.cpp
  clang/lib/Sema/SemaExprObjC.cpp
  clang/lib/Sema/SemaObjCProperty.cpp
  clang/test/CodeGenObjC/direct-method.m
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/SemaObjC/method-direct-properties.m
  clang/test/SemaObjC/method-direct.m

Index: clang/test/SemaObjC/method-direct.m
===
--- /dev/null
+++ clang/test/SemaObjC/method-direct.m
@@ -0,0 +1,148 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Wselector-type-mismatch %s
+
+@protocol Proto
+- (void)protoMethod;  // expected-note {{previous declaration is here}}
++ (void)classProtoMethod; // expected-note {{previous declaration is here}}
+@end
+
+@protocol ProtoDirectFail
+- (void)protoMethod __attribute__((objc_direct));  // expected-error {{'objc_direct' attribute cannot be applied to methods declared in an Objective-C protocol}}
++ (void)classProtoMethod __attribute__((objc_direct)); // expected-error {{'objc_direct' attribute cannot be applied to methods declared in an Objective-C protocol}}
+@end
+
+__attribute__((objc_root_class))
+@interface Root
+- (void)rootRegular;  // expected-note {{previous declaration is here}}
++ (void)classRootRegular; // expected-note {{previous declaration is here}}
+- (void)rootDirect __attribute__((objc_direct));  // expected-note {{previous declaration is here}};
++ (void)classRootDirect __attribute__((objc_direct)); // expected-note {{previous declaration is here}};
+- (void)otherRootDirect 

[PATCH] D69991: Implement __attribute__((objc_direct)), __attribute__((objc_direct_members))

[Pierre Habouzit via Phabricator via cfe-commits]

MadCoder added a comment.

when running the full test-suite before sending the patch, and it broke tests 
because some loads are now ordered differently :(
yay.

so I need to either make sure the CG is done in the same order again, or to fix 
the test expectations.


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

https://reviews.llvm.org/D69991



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

[PATCH] D70368: [clang-tidy] Rewrite modernize-avoid-bind check

[Zachary Turner via Phabricator via cfe-commits]

zturner created this revision.
zturner added reviewers: aaron.ballman, dblaikie, jbcoe, NoQ.
Herald added a subscriber: xazax.hun.

This represents largely a full re-write of modernize-avoid-bind, adding 
significant new functionality in the process.  In particular:

1. Both boost::bind and std::bind are now supported
2. Function objects are supported in addition to functions
3. Member functions are supported
4. Nested calls are supported using capture-init syntax
5. `std::ref()` and `boost::ref()` are now recognized, and will capture by 
reference.
6. Rather than capturing with a global `=`, we now build up an individual 
capture list that is both necessary and sufficient for the call.
7. Fixits are supported in a much larger variety of scenarios than before.

All previous tests pass under the re-write, but a large number of new tests 
have been added as well.

I don't know who the best person to review this is, so I've put a couple of 
people on here.  Feel free to re-direct if there's someone better.


https://reviews.llvm.org/D70368

Files:
  clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.cpp
  clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.h
  clang-tools-extra/test/clang-tidy/checkers/modernize-avoid-bind.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/modernize-avoid-bind.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/modernize-avoid-bind.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize-avoid-bind.cpp
@@ -8,75 +8,51 @@
 template 
 bind_rt bind(Fp &&, Arguments &&...);
 }
+
+template 
+T ref(T );
 }
 
-int add(int x, int y) { return x + y; }
+namespace boost {
+template 
+class bind_rt {};
 
-void f() {
-  auto clj = std::bind(add, 2, 2);
-  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind [modernize-avoid-bind]
-  // CHECK-FIXES: auto clj = [] { return add(2, 2); };
-}
+template 
+bind_rt bind(const Fp &, Arguments...);
+} // namespace boost
 
-void g() {
-  int x = 2;
-  int y = 2;
-  auto clj = std::bind(add, x, y);
-  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind
-  // CHECK-FIXES: auto clj = [=] { return add(x, y); };
-}
+namespace C {
+int add(int x, int y) { return x + y; }
+} // namespace C
 
-struct placeholder {};
-placeholder _1;
-placeholder _2;
+struct Foo {
+  static int add(int x, int y) { return x + y; }
+};
 
-void h() {
-  int x = 2;
-  auto clj = std::bind(add, x, _1);
-  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind
-  // CHECK-FIXES: auto clj = [=](auto && arg1) { return add(x, arg1); };
-}
+struct D {
+  D() = default;
+  void operator()(int x, int y) const {}
 
-struct A;
-struct B;
-bool ABTest(const A &, const B &);
+  void MemberFunction(int x) {}
 
-void i() {
-  auto BATest = std::bind(ABTest, _2, _1);
-  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: prefer a lambda to std::bind
-  // CHECK-FIXES: auto BATest = [](auto && arg1, auto && arg2) { return ABTest(arg2, arg1); };
-}
+  static D *create();
+};
 
-void j() {
-  auto clj = std::bind(add, 2, 2, 2);
-  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind
-  // No fix is applied for argument mismatches.
-  // CHECK-FIXES: auto clj = std::bind(add, 2, 2, 2);
-}
+struct F {
+  F(int x) {}
+  ~F() {}
 
-void k() {
-  auto clj = std::bind(add, _1, _1);
-  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind
-  // No fix is applied for reused placeholders.
-  // CHECK-FIXES: auto clj = std::bind(add, _1, _1);
-}
+  int get() { return 42; }
+};
 
-void m() {
-  auto clj = std::bind(add, 1, add(2, 5));
-  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind
-  // No fix is applied for nested calls.
-  // CHECK-FIXES: auto clj = std::bind(add, 1, add(2, 5));
-}
+void UseF(F);
 
-namespace C {
-  int add(int x, int y){ return x + y; }
-}
+struct placeholder {};
+placeholder _1;
+placeholder _2;
 
-void n() {
-  auto clj = std::bind(C::add, 1, 1);
-  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind
-  // CHECK-FIXES: auto clj = [] { return C::add(1, 1); };
-}
+int add(int x, int y) { return x + y; }
+int addThree(int x, int y, int z) { return x + y + z; }
 
 // Let's fake a minimal std::function-like facility.
 namespace std {
@@ -114,10 +90,188 @@
   void Reset(std::function);
 };
 
-void test(Thing *t) {
+int GlobalVariable = 42;
+
+struct TestCaptureByValueStruct {
+  int MemberVariable;
+  static int StaticMemberVariable;
+  F MemberStruct;
+
+  void testCaptureByValue(int Param, F f) {
+int x = 3;
+int y = 4;
+auto AAA = std::bind(add, x, y);
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind [modernize-avoid-bind]
+// CHECK-FIXES: auto AAA = [x, y] { return add(x, y); };
+
+// When the captured variable is repeated, it should only appear in the capture list once.
+auto BBB = std::bind(add, x, x);
+

[PATCH] D69872: Improve modernize-avoid-bind to support more types of expressions

[Zachary Turner via Phabricator via cfe-commits]

zturner abandoned this revision.
zturner added a comment.

I have a more comprehensive version of this patch that I'll upload separately.


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

https://reviews.llvm.org/D69872



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

[PATCH] D70366: Add new 'flatten' LLVM attribute to fix clang's 'flatten' function attribute

[LevitatingLion via Phabricator via cfe-commits]

LevitatingLion created this revision.
LevitatingLion added reviewers: jdoerfert, pcc, chandlerc.
LevitatingLion added projects: LLVM, clang.
Herald added subscribers: dexonsmith, steven_wu, haicheng, hiraditya, eraman, 
nhaehnle, jvesely, mehdi_amini, arsenm.

This adds a new 'flatten' attribute, which works like 'always_inline' but 
applies recursively to inlined call sites. The addition was briefly discussed 
on the mailing list: 
http://lists.llvm.org/pipermail/llvm-dev/2019-November/136514.html

This patch also contains changes to clang, so that it uses the new LLVM 
attribute on functions marked with the clang attribute 'flatten'. Previously, 
clang marked all calls in such functions with 'always_inline'; in effect, only 
the first level of calls was inlined.

Currently this patch fails the '/llvm/test/Bitcode/highLevelStructure.3.2.ll' 
test. llvm-dis seems to be unable to correctly decode attributes stored in the 
bitcode when the new attribute is added, although other attributes don't seem 
to have required any handling of this problem, see 
https://reviews.llvm.org/D62766 or https://reviews.llvm.org/D49165. I 
speculated that's because this is the 65th attribute, so a bitmask indicating 
all attributes doesn't fit in 64 bit anymore.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70366

Files:
  clang/lib/CodeGen/CGCall.cpp
  llvm/docs/BitCodeFormat.rst
  llvm/docs/LangRef.rst
  llvm/include/llvm/Bitcode/LLVMBitCodes.h
  llvm/include/llvm/IR/Attributes.td
  llvm/lib/Analysis/InlineCost.cpp
  llvm/lib/AsmParser/LLLexer.cpp
  llvm/lib/AsmParser/LLParser.cpp
  llvm/lib/AsmParser/LLToken.h
  llvm/lib/Bitcode/Reader/BitcodeReader.cpp
  llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
  llvm/lib/IR/Attributes.cpp
  llvm/lib/IR/Verifier.cpp
  llvm/lib/Target/AMDGPU/AMDGPUInline.cpp
  llvm/lib/Target/Hexagon/HexagonLoopIdiomRecognition.cpp
  llvm/lib/Transforms/IPO/AlwaysInliner.cpp
  llvm/lib/Transforms/IPO/ForceFunctionAttrs.cpp
  llvm/lib/Transforms/IPO/HotColdSplitting.cpp
  llvm/lib/Transforms/IPO/Inliner.cpp
  llvm/lib/Transforms/IPO/PartialInlining.cpp
  llvm/lib/Transforms/IPO/SyntheticCountsPropagation.cpp
  llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
  llvm/lib/Transforms/Utils/CodeExtractor.cpp
  llvm/test/Transforms/Inline/flatten.ll

Index: llvm/test/Transforms/Inline/flatten.ll
===
--- /dev/null
+++ llvm/test/Transforms/Inline/flatten.ll
@@ -0,0 +1,29 @@
+; RUN: opt < %s -inline -S | FileCheck %s
+
+declare void @ext()
+
+define void @should_inline2() {
+  call void @ext()
+  ret void
+}
+
+define void @should_inline() {
+  call void @should_inline2()
+  ret void
+}
+
+define void @should_not_inline() noinline {
+  call void @ext()
+  ret void
+}
+
+; CHECK: @flattened() {
+define void @flattened() {
+; CHECK-NEXT; @ext() #1
+  call void @should_inline() flatten
+; CHECK-NEXT; @should_not_inline() #1
+  call void @should_not_inline() flatten
+  ret void
+}
+
+; CHECK: attributes #1 = { flatten }
Index: llvm/lib/Transforms/Utils/CodeExtractor.cpp
===
--- llvm/lib/Transforms/Utils/CodeExtractor.cpp
+++ llvm/lib/Transforms/Utils/CodeExtractor.cpp
@@ -888,6 +888,7 @@
   // Those attributes should be safe to propagate to the extracted function.
   case Attribute::AlwaysInline:
   case Attribute::Cold:
+  case Attribute::Flatten:
   case Attribute::NoRecurse:
   case Attribute::InlineHint:
   case Attribute::MinSize:
Index: llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
===
--- llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
+++ llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
@@ -697,7 +697,8 @@
   // have its address taken. Doing so would create an undefined external ref to
   // the function, which would fail to link.
   if (HasAvailableExternallyLinkage &&
-  F->hasFnAttribute(Attribute::AlwaysInline))
+  (F->hasFnAttribute(Attribute::AlwaysInline) ||
+   F->hasFnAttribute(Attribute::Flatten)))
 return false;
 
   // Prohibit function address recording if the function is both internal and
Index: llvm/lib/Transforms/IPO/SyntheticCountsPropagation.cpp
===
--- llvm/lib/Transforms/IPO/SyntheticCountsPropagation.cpp
+++ llvm/lib/Transforms/IPO/SyntheticCountsPropagation.cpp
@@ -77,6 +77,7 @@
 if (F.isDeclaration())
   continue;
 if (F.hasFnAttribute(Attribute::AlwaysInline) ||
+F.hasFnAttribute(Attribute::Flatten) ||
 F.hasFnAttribute(Attribute::InlineHint)) {
   // Use a higher value for inline functions to account for the fact that
   // these are usually beneficial to inline.
Index: llvm/lib/Transforms/IPO/PartialInlining.cpp
===
--- 

[PATCH] D70274: [clang][IFS] Driver pipeline change for clang-ifs: generate interface stubs after standard pipeline.

[Saleem Abdulrasool via Phabricator via cfe-commits]

compnerd added inline comments.



Comment at: clang/lib/Driver/Driver.cpp:3506
+  types::ID InputType = I.first;
+  const Arg *InputArg = I.second;
+

plotfi wrote:
> compnerd wrote:
> > structured bindings ... so much.
> I don't understand? 
C++17's structured bindings would be nice:

```
for (const auto &[Type, Arg] : Inputs)
```



Comment at: clang/lib/Driver/Driver.cpp:3536
+
+llvm_unreachable(
+"IFS Pipeline can only consist of Compile followed by IfsMerge.");

I think that `switch`ing on the `Phase` will make this more clear.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70274



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

[PATCH] D49466: Initial implementation of -fmacro-prefix-map and -ffile-prefix-map

[Dan McGregor via Phabricator via cfe-commits]

dankm marked 2 inline comments as done.
dankm added inline comments.



Comment at: llvm/include/llvm/Support/Path.h:172
+/// @param style The path separator style
+/// @param strict Strict prefix path checking
+/// @result true if \a Path begins with OldPrefix

Lekensteyn wrote:
> "strict checking" is ambiguous on its own. What about something like:
> 
> If strict is true, a directory separator following \a OldPrefix will also 
> be stripped. Otherwise, directory separators will only be matched and 
> stripped when present in \a OldPrefix.
> 
> Or whatever semantics you would like to assign to "strict mode".
Thanks. I like your wording, and I've used it in the incoming patch.



Comment at: llvm/include/llvm/Support/Path.h:181
+  return replace_path_prefix(Path, OldPrefix, NewPrefix, style, strict);
+}
 

Lekensteyn wrote:
> Why have a variant with the parameters swapped, is it common in LLVM to have 
> such convenience wrappers?
> 
> Why not require callers to pass `Style::native` whenever they want to modify 
> "strict"?
Works for me. I did that to make my call sites somewhat more readable. It'll be 
changed too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D49466



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

[PATCH] D70274: [clang][IFS] Driver pipeline change for clang-ifs: generate interface stubs after standard pipeline.

[Puyan Lotfi via Phabricator via cfe-commits]

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



Comment at: clang/lib/Driver/Driver.cpp:3500
+  PL = CompilePhaseList;
+}
+

compnerd wrote:
> Do you use the `PhaseList` again?  Why not `erase_if`?
We tried this in getCompilationPhases. It didn't work very well. 



Comment at: clang/lib/Driver/Driver.cpp:3506
+  types::ID InputType = I.first;
+  const Arg *InputArg = I.second;
+

compnerd wrote:
> structured bindings ... so much.
I don't understand? 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70274



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

[PATCH] D70274: [clang][IFS] Driver pipeline change for clang-ifs: generate interface stubs after standard pipeline.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi updated this revision to Diff 229731.
plotfi marked 7 inline comments as done.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70274

Files:
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/InterfaceStubs.cpp
  clang/lib/Driver/Types.cpp
  clang/test/InterfaceStubs/driver-test.c
  clang/test/InterfaceStubs/driver-test2.c
  clang/test/InterfaceStubs/windows.cpp

Index: clang/test/InterfaceStubs/windows.cpp
===
--- clang/test/InterfaceStubs/windows.cpp
+++ clang/test/InterfaceStubs/windows.cpp
@@ -1,6 +1,7 @@
 // REQUIRES: x86-registered-target
 // RUN: %clang_cc1 -triple x86_64-windows-msvc -o - %s -emit-interface-stubs | FileCheck -check-prefix=CHECK-CC1 %s
-// RUN: %clang -target x86_64-windows-msvc -o - %s -emit-interface-stubs -emit-merged-ifs | FileCheck -check-prefix=CHECK-IFS %s
+// RUN: %clang -target x86_64-windows-msvc -o - %s -emit-interface-stubs -emit-merged-ifs -S | FileCheck -check-prefix=CHECK-IFS %s
+// note: -S is added here to prevent clang from invoking link.exe
 
 // CHECK-CC1: Symbols:
 // CHECK-CC1-NEXT: ?helloWindowsMsvc@@YAHXZ
Index: clang/test/InterfaceStubs/driver-test2.c
===
--- /dev/null
+++ clang/test/InterfaceStubs/driver-test2.c
@@ -0,0 +1,14 @@
+// REQUIRES: x86-registered-target
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -c -emit-interface-stubs \
+// RUN:   %s %S/object.c %S/weak.cpp
+// RUN: %clang -emit-interface-stubs -emit-merged-ifs \
+// RUN:   driver-test2.o object.o weak.o -S -o - | FileCheck %s
+
+// CHECK-DAG: data
+// CHECK-DAG: bar
+// CHECK-DAG: strongFunc
+// CHECK-DAG: weakFunc
+
+int bar(int a) { return a; }
+int main() { return 0; }
Index: clang/test/InterfaceStubs/driver-test.c
===
--- clang/test/InterfaceStubs/driver-test.c
+++ clang/test/InterfaceStubs/driver-test.c
@@ -1,11 +1,13 @@
 // REQUIRES: x86-registered-target
 
-// RUN: %clang -target x86_64-unknown-linux-gnu -x c -o %t1.so -emit-interface-stubs %s %S/object.c %S/weak.cpp && \
-// RUN: llvm-nm %t1.so 2>&1 | FileCheck --check-prefix=CHECK-IFS %s
+// RUN: %clang -target x86_64-unknown-linux-gnu -x c -o %t1 -emit-interface-stubs %s %S/object.c %S/weak.cpp
+// RUN: llvm-nm %t1   2>&1 | FileCheck %s
+// RUN: llvm-nm %t1.ifso 2>&1 | FileCheck %s
 
-// CHECK-IFS-DAG: data
-// CHECK-IFS-DAG: foo
-// CHECK-IFS-DAG: strongFunc
-// CHECK-IFS-DAG: weakFunc
+// CHECK-DAG: data
+// CHECK-DAG: foo
+// CHECK-DAG: strongFunc
+// CHECK-DAG: weakFunc
 
 int foo(int bar) { return 42 + 1844; }
+int main() { return foo(23); }
Index: clang/lib/Driver/Types.cpp
===
--- clang/lib/Driver/Types.cpp
+++ clang/lib/Driver/Types.cpp
@@ -330,22 +330,6 @@
 llvm::copy_if(PhaseList, std::back_inserter(P),
   [](phases::ID Phase) { return Phase <= phases::Precompile; });
 
-  // Treat Interface Stubs like its own compilation mode.
-  else if (DAL.getLastArg(options::OPT_emit_interface_stubs)) {
-llvm::SmallVector IfsModePhaseList;
-llvm::SmallVector  = PhaseList;
-phases::ID LastPhase = phases::IfsMerge;
-if (Id != types::TY_IFS) {
-  if (DAL.hasArg(options::OPT_c))
-LastPhase = phases::Compile;
-  PL = IfsModePhaseList;
-  types::getCompilationPhases(types::TY_IFS_CPP, PL);
-}
-llvm::copy_if(PL, std::back_inserter(P), [&](phases::ID Phase) {
-  return Phase <= LastPhase;
-});
-  }
-
   // -{fsyntax-only,-analyze,emit-ast} only run up to the compiler.
   else if (DAL.getLastArg(options::OPT_fsyntax_only) ||
DAL.getLastArg(options::OPT_print_supported_cpus) ||
Index: clang/lib/Driver/ToolChains/InterfaceStubs.cpp
===
--- clang/lib/Driver/ToolChains/InterfaceStubs.cpp
+++ clang/lib/Driver/ToolChains/InterfaceStubs.cpp
@@ -9,6 +9,7 @@
 #include "InterfaceStubs.h"
 #include "CommonArgs.h"
 #include "clang/Driver/Compilation.h"
+#include "llvm/Support/Path.h"
 
 namespace clang {
 namespace driver {
@@ -21,13 +22,36 @@
   std::string Merger = getToolChain().GetProgramPath(getShortName());
   llvm::opt::ArgStringList CmdArgs;
   CmdArgs.push_back("-action");
-  CmdArgs.push_back(Args.getLastArg(options::OPT_emit_merged_ifs)
-? "write-ifs"
-: "write-bin");
+  const bool WriteBin = !Args.getLastArg(options::OPT_emit_merged_ifs);
+  CmdArgs.push_back(WriteBin ? "write-bin" : "write-ifs");
   CmdArgs.push_back("-o");
-  CmdArgs.push_back(Output.getFilename());
-  for (const auto  : Inputs)
-CmdArgs.push_back(Input.getFilename());
+
+  // Normally we want to write to a side-car file ending in ".ifso" so for
+  // example if `clang -emit-interface-stubs -shared -o libhello.so` were
+  

[PATCH] D70340: Add a key method to Sema to optimize debug info size

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

In D70340#1748975 , @rnk wrote:

> In D70340#1748712 , @
>
> >
>
>
> I guess I was thinking about enabling this only in +asserts builds, so we pay 
> zero overhead in release builds. I was also thinking that if we do implement 
> the "constructor is key for class debug info" flag in the near term, this 
> becomes obsolete. But it's not that much code churn, and it reduces DWARF 
> size with GCC. I guess we could land it after all. :)


With the overhead being the cost of a single vtable with one entry? Or is there 
more?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70340



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

[PATCH] D70196: [clang-include-fixer] Skip .rc files when finding symbols

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

We already have llvm-rc.exe which one could plausibly use to compile rc files, 
and I didn't like `binary.lower().endswith('rc.exe')` as a test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70196



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

[PATCH] D68362: [libunwind][RISCV] Add 64-bit RISC-V support

[Mitchell Horne via Phabricator via cfe-commits]

mhorne added reviewers: compnerd, phosek.
mhorne marked 2 inline comments as done.
mhorne added a comment.

Add some libunwind contributors for additional review.




Comment at: libunwind/src/Registers.hpp:3756
+inline double Registers_riscv::getFloatRegister(int regNum) const {
+#ifdef __riscv_float_abi_double
+  assert(validFloatRegister(regNum));

lenary wrote:
> mhorne wrote:
> > luismarques wrote:
> > > luismarques wrote:
> > > > lenary wrote:
> > > > > mhorne wrote:
> > > > > > lenary wrote:
> > > > > > > Is this an ABI or an architecture issue? I'm not sure what other 
> > > > > > > libunwind "backends" do for similar cases.
> > > > > > > 
> > > > > > > The difference is, if I compile libunwind with `-march=rv64g 
> > > > > > > -mabi=lp64`, `__riscv_float_abi_double` is not defined (because 
> > > > > > > you're using a soft-float ABI), but `__riscv_flen == 64` (because 
> > > > > > > the machine does have hardware floating-point registers).
> > > > > > > 
> > > > > > > I'm not sure what the intended behaviour of libunwind is in this 
> > > > > > > case.
> > > > > > > 
> > > > > > > `__riscv_float_abi_double` implies `__riscv_flen >= 64`.
> > > > > > An ABI issue, in my opinion. The unwind frame will always contain 
> > > > > > space for the float registers, but accessing them should be 
> > > > > > disallowed for soft-float configurations as the intent of 
> > > > > > soft-float is that the FPRs will not be used at all. I'd say there 
> > > > > > is precedent for this in the MIPS implementation, since it checks 
> > > > > > for `defined(__mips_hard_float) && __mips_fpr == 64`.
> > > > > I had a discussion with @asb about this. The ISA vs ABI issue in 
> > > > > RISC-V is complex. The TL;DR is we both think you need to be using 
> > > > > `__riscv_flen == 64` here.
> > > > > 
> > > > > The reason for this is that if you compile with `-march=rv64imfd` but 
> > > > > `-mabi=lp64`, the architecture still has floating point registers, it 
> > > > > just does not use the floating-point calling convention. This means 
> > > > > there are still `D`-extension instructions in the stream of 
> > > > > instructions, just that "No floating-point registers, if present, are 
> > > > > preserved across calls." (see [[ 
> > > > > https://github.com/riscv/riscv-elf-psabi-doc/blob/master/riscv-elf.md#integer-calling-convention
> > > > >  | psABI Integer Calling Convention ]]) This effectively means that 
> > > > > with this combination, `f0-f31` are treated exactly the same as 
> > > > > `t0-t6`, and so should be able to be restored when unwinding. It is 
> > > > > not necessarily the case that with a soft float ABI, `f0-f31` are not 
> > > > > used at all. This is similar to ARM's `soft` vs `softfp` calling 
> > > > > conventions.
> > > > > 
> > > > > The expectation is that if you are compiling your programs with a 
> > > > > specific `-march`, then you should be compiling your runtime 
> > > > > libraries with the same `-march`. Eventually there should be enough 
> > > > > details in the ELF file to allow you to ensure both `-march` and 
> > > > > `-mabi` match when linking programs, but support for this is not 
> > > > > widespread.
> > > > A soft-float *ABI* doesn't mean that FPRs aren't used at all, it means 
> > > > that floating-point arguments aren't passed in the floating-point 
> > > > registers. From a quick Google search I got the impression that 
> > > > `__mips_hard_float` was used for a mips softfloat target (i.e. without 
> > > > hardware floating-point support, not for a soft-float ABI), so that's 
> > > > probably not a comparable example.
> > > I just saw @lenary's reply. I agree with his more detailed analysis.
> > Thanks Sam and Luis for the detailed replies.
> > 
> > I definitely agree with you that `__riscv_flen == 64` is the more 
> > appropriate check. But now I'm reconsidering if a floating point check is 
> > needed at all. By adding it are we not preventing access to the FPRs for 
> > cross/remote unwinding?
> Cross-compiling across RISC-V architectures is very complex. Sadly, using 
> only the target triple is not enough, and nor is matching the ABI, because 
> the architecture is so extensible.
> 
> In all of these cases, we expect end users to explicitly compile their 
> required libraries with the correct `-march`/`-mabi` for the RISC-V platform 
> they are using. If they are cross-compiling, then that means the whole 
> sysroot, compiler runtime (libgcc or compiler-rt), and libc should be 
> compiled with an explicitly-set `-march`/`-mabi`. If they do this, then there 
> will be no issues with our code. Importantly, this should still largely work 
> for multilib builds, where there are multiple march/mabi combinations that 
> libraries are compiled for.
> 
> This is not optimal from the point-of-view of someone developing for lots of 
> disparate RISC-V targets (like compiler developers), but should be ok for 
> developers developing for single 

[PATCH] D69585: PerformPendingInstatiations() already in the PCH

[Luboš Luňák via Phabricator via cfe-commits]

llunak updated this revision to Diff 229722.
llunak added a comment.

It turns out that this patch may introduce unwanted changes, specifically it 
can cause err_specialization_after_instantiation if the specialization is done 
in a source file but needed already by code in the PCH. But this seems to be 
rare (I've encountered it only after building the Skia library with 
put-everything-in-the-PCH), is easy to avoid (PCHs often need little tweaks 
anyway) and the performance gain is very much worth it.

Still, this means there should be an option to enable this. Done in this 
version of the patch. I don't know how to handle tests for it, pretty much all 
the tests pass as said above, but it seems a bit silly to duplicate all the 
PCH-related ones to also use the flag.


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

https://reviews.llvm.org/D69585

Files:
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/CC1Options.td
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Sema/Sema.cpp


Index: clang/lib/Sema/Sema.cpp
===
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -981,6 +981,14 @@
  LateParsedInstantiations.begin(),
  LateParsedInstantiations.end());
 LateParsedInstantiations.clear();
+
+// FIXME: Instantiating implicit templates already in the PCH breaks some
+// OpenMP-specific code paths, see https://reviews.llvm.org/D69585 .
+if(LangOpts.PCHInstantiateTemplates && !LangOpts.OpenMP) {
+  llvm::TimeTraceScope TimeScope("PerformPendingInstantiations",
+ StringRef(""));
+  PerformPendingInstantiations();
+}
   }
 
   DiagnoseUnterminatedPragmaPack();
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -3223,6 +3223,7 @@
 
   Opts.CompleteMemberPointers = Args.hasArg(OPT_fcomplete_member_pointers);
   Opts.BuildingPCHWithObjectFile = Args.hasArg(OPT_building_pch_with_obj);
+  Opts.PCHInstantiateTemplates = Args.hasArg(OPT_pch_instantiate_templates);
 }
 
 static bool isStrictlyPreprocessorAction(frontend::ActionKind Action) {
Index: clang/include/clang/Driver/CC1Options.td
===
--- clang/include/clang/Driver/CC1Options.td
+++ clang/include/clang/Driver/CC1Options.td
@@ -672,6 +672,8 @@
   HelpText<"Disable inclusion of timestamp in precompiled headers">;
 def building_pch_with_obj : Flag<["-"], "building-pch-with-obj">,
   HelpText<"This compilation is part of building a PCH with corresponding 
object file.">;
+def pch_instantiate_templates : Flag<["-"], "pch-instantiate-templates">,
+  HelpText<"Perform pending template instantiations already while building the 
PCH.">;
 
 def aligned_alloc_unavailable : Flag<["-"], "faligned-alloc-unavailable">,
   HelpText<"Aligned allocation/deallocation functions are unavailable">;
Index: clang/include/clang/Basic/LangOptions.def
===
--- clang/include/clang/Basic/LangOptions.def
+++ clang/include/clang/Basic/LangOptions.def
@@ -160,6 +160,7 @@
 BENIGN_LANGOPT(CompilingPCH, 1, 0, "building a pch")
 BENIGN_LANGOPT(BuildingPCHWithObjectFile, 1, 0, "building a pch which has a 
corresponding object file")
 BENIGN_LANGOPT(CacheGeneratedPCH, 1, 0, "cache generated PCH files in memory")
+BENIGN_LANGOPT(PCHInstantiateTemplates, 1, 0, "performing pending template 
instantiations already while building a pch")
 COMPATIBLE_LANGOPT(ModulesDeclUse, 1, 0, "require declaration of module 
uses")
 BENIGN_LANGOPT(ModulesSearchAll  , 1, 1, "searching even non-imported modules 
to find unresolved references")
 COMPATIBLE_LANGOPT(ModulesStrictDeclUse, 1, 0, "requiring declaration of 
module uses and all headers to be in modules")


Index: clang/lib/Sema/Sema.cpp
===
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -981,6 +981,14 @@
  LateParsedInstantiations.begin(),
  LateParsedInstantiations.end());
 LateParsedInstantiations.clear();
+
+// FIXME: Instantiating implicit templates already in the PCH breaks some
+// OpenMP-specific code paths, see https://reviews.llvm.org/D69585 .
+if(LangOpts.PCHInstantiateTemplates && !LangOpts.OpenMP) {
+  llvm::TimeTraceScope TimeScope("PerformPendingInstantiations",
+ StringRef(""));
+  PerformPendingInstantiations();
+}
   }
 
   DiagnoseUnterminatedPragmaPack();
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- 

[PATCH] D70349: [Attr] Fix `-ast-print` for `asm` attribute

[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!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70349



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

[PATCH] D69990: Populate CUDA flags on FreeBSD too, as many other toolchains do.

[Ed Maste via Phabricator via cfe-commits]

emaste added a comment.

Should have a test added


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69990



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

[PATCH] D64305: [clangd] Add path mappings functionality

[William Wagner via Phabricator via cfe-commits]

wwagner19 marked 9 inline comments as done and an inline comment as not done.
wwagner19 added inline comments.



Comment at: clang-tools-extra/clangd/PathMapping.cpp:153
+// Converts \pPath to a posix-style absolute, i.e. if it's a windows path
+// then the backward-slash notation will be converted to forward slash
+llvm::Expected parsePath(llvm::StringRef Path) {

sammccall wrote:
> "posix-style" doesn't really describe representing `c:\foo` as `/c:/foo`. 
> That's really *just* a URI thing AFAIK.
> 
> Something like "Converts a unix/windows path to the path part of a file URI".
> But in that case, I think the implementation is just 
> `URI::createFile(Path).body()`. Does that pass tests?
Oh I did not realize `createFile` was a thing, however looking at the way it's 
implemented now, won't that throw an assert if a non-absolute path is passed 
in? If so, is that desirable at all?

IIUC, if I were to use that API, then wouldn't it make more sense for it to 
return an `llvm::Expected`? If we want to consolidate the logic to one place, 
I'd be happy to try and refactor the signature.



Comment at: clang-tools-extra/clangd/PathMapping.cpp:214
+  llvm::SmallString<256> MappedPath(Uri->body());
+  llvm::sys::path::replace_path_prefix(MappedPath, From, To);
+  auto MappedUri = URI(Uri->scheme(), Uri->authority(), 
MappedPath.c_str());

sammccall wrote:
> Sorry, I know I suggested replace_path_prefix, but now that the mappings 
> consist of paths in their URL form, I think the old string::replace version 
> is what you want :-/
Will do, I like string replace better anyway! 

I'm not a huge fan of the `mappingMatches` function, and would prefer a simple 
string `startswith(from)`, but the only way I may see that working is by 
appending "/" to all the path mappings internally - which would prevent `/foo` 
matching `/foo-bar` - but appending a "/" would break directory-based file 
URIs, I believe.



Comment at: clang-tools-extra/clangd/PathMapping.h:56
 /// untouched.
-llvm::json::Value doPathMapping(const llvm::json::Value ,
-bool IsIncoming, const PathMappings );
+void applyPathMappings(llvm::json::Value , bool IsIncoming,
+   const PathMappings );

sammccall wrote:
> nit: the sense of the bool is pretty arbitrary here, prefer a two value enum?
> 
> e.g. `enum class PathMapping::Direction { ServerToClient, ClientToServer }`
> 
> (Reusing the "server" and "client" concept rather than adding 
> "incoming/outgoing" seems a little simpler, though up to you)
much much better that way, thanks


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

https://reviews.llvm.org/D64305



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

[PATCH] D64305: [clangd] Add path mappings functionality

[William Wagner via Phabricator via cfe-commits]

wwagner19 updated this revision to Diff 229721.
wwagner19 marked 5 inline comments as done.
wwagner19 added a comment.

Thanks for the second review Sam, I addressed most of your comments, notably:

- Changed the bool IsIncoming to an enum
- Fixed the "doxygen" comments,
- Removed some redundant incudes/variables
- Switched `replace_path_prefix` to string replace


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

https://reviews.llvm.org/D64305

Files:
  clang-tools-extra/clangd/PathMapping.cpp
  clang-tools-extra/clangd/PathMapping.h
  clang-tools-extra/clangd/tool/ClangdMain.cpp
  clang-tools-extra/clangd/unittests/PathMappingTests.cpp

Index: clang-tools-extra/clangd/unittests/PathMappingTests.cpp
===
--- clang-tools-extra/clangd/unittests/PathMappingTests.cpp
+++ clang-tools-extra/clangd/unittests/PathMappingTests.cpp
@@ -82,12 +82,11 @@
 }
 
 bool mapsProperly(llvm::StringRef Orig, llvm::StringRef Expected,
-  llvm::StringRef RawMappings, bool IsIncoming) {
+  llvm::StringRef RawMappings, PathMapping::Direction Dir) {
   llvm::Expected Mappings = parsePathMappings(RawMappings);
   if (!Mappings)
 return false;
-  llvm::Optional MappedPath =
-  doPathMapping(Orig, IsIncoming, *Mappings);
+  llvm::Optional MappedPath = doPathMapping(Orig, Dir, *Mappings);
   std::string Actual = MappedPath ? *MappedPath : Orig.str();
   EXPECT_STREQ(Expected.str().c_str(), Actual.c_str());
   return Expected == Actual;
@@ -95,48 +94,54 @@
 
 TEST(DoPathMappingTests, PreservesOriginal) {
   // Preserves original path when no mapping
-  EXPECT_TRUE(mapsProperly("file:///home", "file:///home", "", true));
+  EXPECT_TRUE(mapsProperly("file:///home", "file:///home", "",
+   PathMapping::Direction::ClientToServer));
 }
 
 TEST(DoPathMappingTests, UsesFirstMatch) {
   EXPECT_TRUE(mapsProperly("file:///home/foo.cpp", "file:///workarea1/foo.cpp",
-   "/home=/workarea1,/home=/workarea2", true));
+   "/home=/workarea1,/home=/workarea2",
+   PathMapping::Direction::ClientToServer));
 }
 
 TEST(DoPathMappingTests, IgnoresSubstrings) {
   // Doesn't map substrings that aren't a proper path prefix
   EXPECT_TRUE(mapsProperly("file://home/foo-bar.cpp", "file://home/foo-bar.cpp",
-   "/home/foo=/home/bar", true));
+   "/home/foo=/home/bar",
+   PathMapping::Direction::ClientToServer));
 }
 
 TEST(DoPathMappingTests, MapsOutgoingPaths) {
   // When IsIncoming is false (i.e.a  response), map the other way
   EXPECT_TRUE(mapsProperly("file:///workarea/foo.cpp", "file:///home/foo.cpp",
-   "/home=/workarea", false));
+   "/home=/workarea",
+   PathMapping::Direction::ServerToClient));
 }
 
 TEST(DoPathMappingTests, OnlyMapFileUris) {
   EXPECT_TRUE(mapsProperly("test:///home/foo.cpp", "test:///home/foo.cpp",
-   "/home=/workarea", true));
+   "/home=/workarea",
+   PathMapping::Direction::ClientToServer));
 }
 
 TEST(DoPathMappingTests, RespectsCaseSensitivity) {
   EXPECT_TRUE(mapsProperly("file:///HOME/foo.cpp", "file:///HOME/foo.cpp",
-   "/home=/workarea", true));
+   "/home=/workarea",
+   PathMapping::Direction::ClientToServer));
 }
 
 TEST(DoPathMappingTests, MapsWindowsPaths) {
   // Maps windows properly
   EXPECT_TRUE(mapsProperly("file:///C:/home/foo.cpp",
-   "file:///C:/workarea/foo.cpp", "C:/home=C:/workarea",
-   true));
+   "file:///C:/workarea/foo.cpp", R"(C:\home=C:\workarea)",
+   PathMapping::Direction::ClientToServer));
 }
 
 TEST(DoPathMappingTests, MapsWindowsUnixInterop) {
   // Path mappings with a windows-style client path and unix-style server path
-  EXPECT_TRUE(mapsProperly("file:///C:/home/foo.cpp",
-   "file:///C:/workarea/foo.cpp",
-   "C:/home=/C:/workarea", true));
+  EXPECT_TRUE(mapsProperly(
+  "file:///C:/home/foo.cpp", "file:///workarea/foo.cpp",
+  R"(C:\home=/workarea)", PathMapping::Direction::ClientToServer));
 }
 
 TEST(ApplyPathMappingTests, PreservesOriginalParams) {
@@ -147,7 +152,7 @@
   ASSERT_TRUE(bool(Params));
   llvm::json::Value ExpectedParams = *Params;
   PathMappings Mappings;
-  applyPathMappings(*Params, /*IsIncoming=*/true, Mappings);
+  applyPathMappings(*Params, PathMapping::Direction::ClientToServer, Mappings);
   EXPECT_EQ(*Params, ExpectedParams);
 }
 
@@ -163,7 +168,7 @@
   })");
   auto Mappings = parsePathMappings("/home=/workarea");
   ASSERT_TRUE(bool(Params) && bool(ExpectedParams) && bool(Mappings));
-  

[PATCH] D70340: Add a key method to Sema to optimize debug info size

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

In D70340#1748712 , @thakis wrote:

> I don't see any reason not to do this. What's there to discuss? I'm probably 
> missing something obvious.


I guess I was thinking about enabling this only in +asserts builds, so we pay 
zero overhead in release builds. I was also thinking that if we do implement 
the "constructor is key for class debug info" flag in the near term, this 
becomes obsolete. But it's not that much code churn, and it reduces DWARF size 
with GCC. I guess we could land it after all. :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70340



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

[PATCH] D70052: [clang-tidy] Add misc-mutating-copy check

[Gabor Bencze via Phabricator via cfe-commits]

gbencze updated this revision to Diff 229720.
gbencze added a comment.

Moved check to CERT module. 
Added warnings for calling mutating member functions.


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

https://reviews.llvm.org/D70052

Files:
  clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
  clang-tools-extra/clang-tidy/cert/CMakeLists.txt
  clang-tools-extra/clang-tidy/cert/MutatingCopyCheck.cpp
  clang-tools-extra/clang-tidy/cert/MutatingCopyCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/cert-oop58-cpp.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/cert-oop58-cpp.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cert-oop58-cpp.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cert-oop58-cpp.cpp
@@ -0,0 +1,149 @@
+// RUN: %check_clang_tidy %s cert-oop58-cpp %t -std=c++11-or-later
+
+// Example test cases from CERT rule
+// https://wiki.sei.cmu.edu/confluence/display/cplusplus/OOP58-CPP.+Copy+operations+must+not+mutate+the+source+object
+namespace test_mutating_noncompliant_example {
+class A {
+  mutable int m;
+
+public:
+  A() : m(0) {}
+  explicit A(int m) : m(m) {}
+
+  A(const A ) : m(other.m) {
+other.m = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: mutating copied object
+  }
+
+  A =(const A ) {
+if ( != this) {
+  m = other.m;
+  other.m = 0;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: mutating copied object
+}
+return *this;
+  }
+
+  int get_m() const { return m; }
+};
+} // namespace test_mutating_noncompliant_example
+
+namespace test_mutating_compliant_example {
+class B {
+  int m;
+
+public:
+  B() : m(0) {}
+  explicit B(int m) : m(m) {}
+
+  B(const B ) : m(other.m) {}
+  B(B &) : m(other.m) {
+other.m = 0; //no-warning: mutation allowed in move constructor
+  }
+
+  B =(const B ) {
+if ( != this) {
+  m = other.m;
+}
+return *this;
+  }
+
+  B =(B &) {
+m = other.m;
+other.m = 0; //no-warning: mutation allowed in move assignment operator
+return *this;
+  }
+
+  int get_m() const { return m; }
+};
+} // namespace test_mutating_compliant_example
+
+namespace test_mutating_pointer {
+class C {
+  C *ptr;
+  int value;
+
+  C();
+  C(C ) {
+other = {};
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: mutating copied object
+other.ptr = nullptr;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: mutating copied object
+other.value = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: mutating copied object
+
+// no-warning: mutating a pointee is allowed
+other.ptr->value = 0;
+*other.ptr = {};
+  }
+};
+} // namespace test_mutating_pointer
+
+namespace test_mutating_indirect_member {
+struct S {
+  int x;
+};
+
+class D {
+  S s;
+  D(D ) {
+other.s = {};
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: mutating copied object
+other.s.x = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: mutating copied object
+  }
+};
+} // namespace test_mutating_indirect_member
+
+namespace test_mutating_other_object {
+class E {
+  E();
+  E(E ) {
+E tmp;
+// no-warning: mutating an object that is not the source is allowed
+tmp = {};
+  }
+};
+} // namespace test_mutating_other_object
+
+namespace test_mutating_member_function {
+class F {
+  int a;
+
+public:
+  void bad_func() { a = 12; }
+  void fine_func() const;
+  void fine_func_2(int x) { x = 5; }
+  void questionable_func();
+
+  F(F ) : a(other.a) {
+this->bad_func(); // no-warning: mutating this is allowed
+
+other.bad_func();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: call mutates copied object
+
+other.fine_func();
+other.fine_func_2(42);
+other.questionable_func();
+  }
+};
+} // namespace test_mutating_member_function
+
+namespace test_mutating_function_on_nested_object {
+struct S {
+  int x;
+  void mutate(int y) {
+x = y;
+  }
+};
+
+class G {
+  S s;
+  G(G ) {
+s.mutate(0); // no-warning: mutating this is allowed
+
+other.s.mutate(0);
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: call mutates copied object
+  }
+};
+} // namespace test_mutating_function_on_nested_object
\ No newline at end of file
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -105,6 +105,7 @@
cert-msc51-cpp
cert-oop11-cpp (redirects to performance-move-constructor-init) 
cert-oop54-cpp (redirects to bugprone-unhandled-self-assignment) 
+   cert-oop58-cpp
clang-analyzer-core.CallAndMessage (redirects to https://clang.llvm.org/docs/analyzer/checkers) 
clang-analyzer-core.DivideZero (redirects to https://clang.llvm.org/docs/analyzer/checkers) 
clang-analyzer-core.DynamicTypePropagation
Index: 

Re: [PATCH] D70144: clang-tidy: modernize-use-equals-default avoid adding redundant semicolons

[MyDeveloper Day via cfe-commits]

We could resolve this using a separate TokenAnalyzer pass similar to what
we are doing here  https://reviews.llvm.org/D69764, such a  would let us
look at the last token on the previous line or the next token in the next
time to ensure we don't remove legal cases like

for(;
;
)

Adding a DoubleSemiFixer type class would allow clang-format to resolve
such issue where it see erroneous double ";;" not just were the
replacements have got confused.

I'm happy to look into that if that would resolve your issues?

On Sat, Nov 16, 2019 at 11:03 PM Jonas Toth via Phabricator via cfe-commits
 wrote:

> JonasToth added a comment.
>
> Hmm. I think this is fine, even though its not perfect.
> @aaron.ballman wdyt?
>
>
> Repository:
>   rCTE Clang Tools Extra
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D70144/new/
>
> https://reviews.llvm.org/D70144
>
>
>
> ___
> 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] D69360: [NFC] Refactor representation of materialized temporaries

[Tyker via Phabricator via cfe-commits]

Tyker updated this revision to Diff 229718.
Tyker added a comment.
Herald added a reviewer: deadalnix.
Herald added subscribers: llvm-commits, ormris, hiraditya.
Herald added a project: LLVM.

fixe the issue causing buildbot failure.

node were visited multiple time by the RecursiveASTVisitor in some cases.


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

https://reviews.llvm.org/D69360

Files:
  clang-tools-extra/clang-tidy/abseil/StrCatAppendCheck.cpp
  clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.cpp
  clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.cpp
  clang-tools-extra/clang-tidy/performance/ImplicitConversionInLoopCheck.cpp
  clang-tools-extra/clang-tidy/readability/NonConstParameterCheck.cpp
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/AST/DeclCXX.h
  clang/include/clang/AST/ExprCXX.h
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/include/clang/Basic/DeclNodes.td
  clang/include/clang/Sema/Template.h
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/DeclBase.cpp
  clang/lib/AST/DeclCXX.cpp
  clang/lib/AST/Expr.cpp
  clang/lib/AST/ExprCXX.cpp
  clang/lib/AST/ExprConstant.cpp
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/StmtPrinter.cpp
  clang/lib/Analysis/CFG.cpp
  clang/lib/Analysis/Consumed.cpp
  clang/lib/Analysis/ThreadSafetyCommon.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGExprAgg.cpp
  clang/lib/CodeGen/CGExprConstant.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Sema/JumpDiagnostics.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaInit.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTCommon.cpp
  clang/lib/Serialization/ASTReaderDecl.cpp
  clang/lib/Serialization/ASTReaderStmt.cpp
  clang/lib/Serialization/ASTWriterDecl.cpp
  clang/lib/Serialization/ASTWriterStmt.cpp
  clang/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
  clang/test/CodeGenCXX/debug-info-template-align.cpp
  clang/tools/libclang/CIndex.cpp
  llvm/include/llvm-c/DebugInfo.h
  llvm/include/llvm/IR/DIBuilder.h
  llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
  llvm/lib/IR/DIBuilder.cpp
  llvm/lib/IR/DebugInfo.cpp
  llvm/test/DebugInfo/X86/debug-info-template-align.ll
  llvm/tools/llvm-c-test/debuginfo.c

Index: llvm/tools/llvm-c-test/debuginfo.c
===
--- llvm/tools/llvm-c-test/debuginfo.c
+++ llvm/tools/llvm-c-test/debuginfo.c
@@ -69,7 +69,7 @@
   LLVMMetadataRef Int64Ty =
   LLVMDIBuilderCreateBasicType(DIB, "Int64", 5, 64, 0, LLVMDIFlagZero);
   LLVMMetadataRef Int64TypeDef =
-LLVMDIBuilderCreateTypedef(DIB, Int64Ty, "int64_t", 7, File, 42, File);
+  LLVMDIBuilderCreateTypedef(DIB, Int64Ty, "int64_t", 7, File, 42, File, 0);
 
   LLVMMetadataRef GlobalVarValueExpr =
   LLVMDIBuilderCreateConstantValueExpression(DIB, 0);
Index: llvm/test/DebugInfo/X86/debug-info-template-align.ll
===
--- /dev/null
+++ llvm/test/DebugInfo/X86/debug-info-template-align.ll
@@ -0,0 +1,63 @@
+; RUN: llc %s -filetype=obj -o - | llvm-dwarfdump -v - | FileCheck %s
+
+; C++ source to regenerate:
+
+;typedef char  __attribute__((__aligned__(64))) alchar;
+
+;int main(){
+;alchar newChar;
+;}
+; $ clang++ -O0 -g -gdwarf-5 debug-info-template-align.cpp -c
+
+; CHECK: .debug_abbrev contents:
+
+; CHECK: [5] DW_TAG_typedef  DW_CHILDREN_no
+; CHECK:  DW_AT_alignment DW_FORM_udata
+
+; CHECK: .debug_info contents:
+
+;CHECK: DW_TAG_typedef [5]
+;CHECK: DW_AT_name {{.*}} "alchar"
+;CHECK-NEXT: DW_AT_alignment [DW_FORM_udata] (64)
+
+
+; ModuleID = '/dir/test.cpp'
+source_filename = "/dir/test.cpp"
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+; Function Attrs: noinline norecurse nounwind optnone uwtable
+define dso_local i32 @main() #0 !dbg !7 {
+entry:
+  %newChar = alloca i8, align 64
+  call void @llvm.dbg.declare(metadata i8* %newChar, metadata !12, metadata !DIExpression()), !dbg !15
+  ret i32 0, !dbg !16
+}
+
+; Function Attrs: nounwind readnone speculatable willreturn
+declare void @llvm.dbg.declare(metadata, metadata, metadata) #1
+
+attributes #0 = { noinline norecurse nounwind optnone uwtable }
+attributes #1 = { nounwind readnone speculatable willreturn }
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3, !4, !5}
+!llvm.ident = !{!6}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !1, producer: "clang version 10.0.0 ", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, 

[PATCH] D70359: [clangd] Show values of more expressions on hover

[liu hui via Phabricator via cfe-commits]

lh123 added inline comments.



Comment at: clang-tools-extra/clangd/Hover.cpp:423
+  //  - certain expressions (sizeof etc)
+  //  - built-in types
 }

I think we should also support hover on literal.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70359



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