[PATCH] D73397: [Clang] Enable -fsanitize=leak on Fuchsia targets

[Petr Hosek via Phabricator via cfe-commits]

phosek accepted this revision.
phosek 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/D73397/new/

https://reviews.llvm.org/D73397



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

[PATCH] D73282: Fix debug-info generation for block invocations so that we set the LinkageName instead of the Name

[David Blaikie via Phabricator via cfe-commits]

dblaikie added inline comments.



Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:3659
 // Use llvm function name.
-Name = Fn->getName();
+if (Fn->getName().startswith("___Z"))
+  LinkageName = Fn->getName();

aprantl wrote:
> aprantl wrote:
> > Could you please add a comment that Clang Blocks are generated as raw 
> > llvm::Functions but do have a mangled name and that is handling this case? 
> > Otherwise this would look suspicious.
> Should *all* raw LLVM functions have their name as the linkage name? Perhaps 
> a raw LLVM function should only have a linkage name and no human-readable 
> name?
Seems plausible to me - do we have any data on other types of functions that 
hit this codepath? 


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

https://reviews.llvm.org/D73282



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

[PATCH] D73290: [PowerPC] Add clang -msvr4-struct-return for 32-bit ELF

[Mark Millard via Phabricator via cfe-commits]

markmi added a comment.

I built and installed ports, what built vs. did-not was the same as before 
applying the update, with the same failure details having nothing to do with 
this change. I then ran the FreeBSD kyua tests and the result match what I got 
prior to the update.

These tests are not about gcc/g++ ABI compatibility, but FreeBSD seems to work 
fine when built based on the change.

(FYI, given the ABI incompatibility, I did not try to self-host getting from 
one ABI to the other. I instead installed the updated FreeBSD as a separate 
drive on another system and then put that drive back in its normal system and 
booted from it.)

I'm not aware of anything around that makes for a good, fairly-general gcc/g++ 
ABI compatibility test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73290



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

[PATCH] D72675: [Clang][Driver] Fix -ffast-math/-ffp-contract interaction

[Warren Ristow via Phabricator via cfe-commits]

wristow marked 4 inline comments as done.
wristow added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2792
   if (!MathErrno && AssociativeMath && ReciprocalMath && !SignedZeros &&
-  !TrappingMath)
+  !TrappingMath && !(FPContract.equals("off") || FPContract.equals("on")))
 CmdArgs.push_back("-menable-unsafe-fp-math");

andrew.w.kaylor wrote:
> I think this would read better as "... && !FPContract.equals("off") && 
> !FPContract.equals("on")" The '||' in the middle of all these '&&'s combined 
> with the extra parens from the function call trips me up.
Sounds good.  Will do.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2846
   ReciprocalMath && !SignedZeros && !TrappingMath && !RoundingFPMath) {
-CmdArgs.push_back("-ffast-math");
+if (!(FPContract.equals("off") || FPContract.equals("on")))
+  CmdArgs.push_back("-ffast-math");

andrew.w.kaylor wrote:
> As above, I'd prefer "!off && !on".
As above, will do.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2854
 // Enable -ffp-contract=fast
 CmdArgs.push_back(Args.MakeArgString("-ffp-contract=fast"));
   else

andrew.w.kaylor wrote:
> This is a bit of an oddity in our handling. The FPContract local variable 
> starts out initialized to an empty string. So, what we're saying here is that 
> if you've used all the individual controls to set the rest of the fast math 
> flags, we'll turn on fp-contract=fast also. That seems wrong. If you use 
> "-ffast-math", FPContract will have been set to "fast" so that's not 
> applicable here.
> 
> I believe this means
> 
> ```
> -fno-honor-infinities -fno-honor-nans -fno-math-errno -fassociative-math 
> -freciprocal-math -fno-signed-zeros -fno-trapping-math -fno-rounding-math
> 
> ```
> 
> will imply "-ffp-contract=fast". Even worse:
> 
> ```
> -ffp-contract=off -fno-fast-math -fno-honor-infinities -fno-honor-nans 
> -fno-math-errno -fassociative-math -freciprocal-math -fno-signed-zeros 
> -fno-trapping-math -fno-rounding-math
> 
> ```
> will imply "-ffp-contract=fast" because "-fno-fast-math" resets FPContract to 
> empty.
> 
> I think we should initialize FPContract to whatever we want to be the default 
> (on?) and remove the special case for empty here. Also, -fno-fast-math should 
> either not change FPContract at all or set it to "off". Probably the latter 
> since we're saying -ffast-math includes -ffp-contract=on.
> This is a bit of an oddity in our handling.

Yes it is!

This is certainly getting more complicated than I had originally expected.  I 
feel there isn't a clear spec on what we want in terms of whether FMA should be 
enabled "automatically" at (for example) '-O2', and/or whether it should be 
enabled by `-ffast-math`.  I'm very willing to make whatever change is needed 
here, to meet whatever spec we all ultimately agree on.

So I think this patch should be put on hold until we decide on these wider 
aspects.

One minor note about:
> ... Probably the latter since we're saying -ffast-math includes 
> -ffp-contract=on.

I think that is intended to say "-ffast-math includes -ffp-contract=fast".  The 
semantics of `-ffp-contract=on` are another part that seems unclear (or at 
least, more complicated, since it then gets into the FP_CONTRACT pragma).



Comment at: clang/test/Driver/fast-math.c:196
+// RUN: %clang -### -ffast-math -ffp-contract=fast -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-UNSAFE-MATH %s
+//

andrew.w.kaylor wrote:
> What about "-ffp-contract=off -ffast-math"? The way the code is written that 
> will override the -ffp-contract option. That's probably what we want, though 
> it might be nice to have a warning.
Yes, currently `-fffast-math` will override that earlier `-ffp-contract=off` 
settings.  It's unclear to me whether we're ultimately intending for that to be 
the behavior (because GCC doesn't do that, as @uweigand noted).  I guess this 
is another reason to hold off for a bit, until we figure out the wider spec.


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

https://reviews.llvm.org/D72675



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

[libunwind] c48974f - [libunwind] Set LIBUNWIND_ASM_SOURCES to the ASM source language from C

[Petr Hosek via cfe-commits]

Author: James Nagurne
Date: 2020-01-24T19:16:47-08:00
New Revision: c48974ffd7d1676f79d39d3b1e70f07d3a5e2e44

URL: 
https://github.com/llvm/llvm-project/commit/c48974ffd7d1676f79d39d3b1e70f07d3a5e2e44
DIFF: 
https://github.com/llvm/llvm-project/commit/c48974ffd7d1676f79d39d3b1e70f07d3a5e2e44.diff

LOG: [libunwind] Set LIBUNWIND_ASM_SOURCES to the ASM source language from C

I believe this is an oversight from the import of libunwind into its own
library from libc++abi.

In libc++abi, these files had the .s suffix, which indicates that the file
is a preprocessed assembly source file. This caused problems because the
files rely upon preprocessors to guard target-specific blocks.

To fix this, the CMakeLists file marked these files as C so that the
preprocessor would be run over them, but then the compiler would correctly
identify the files as assembly and compile them as such.

When imported to libunwind, these files were (correctly) renamed with .S
suffixes, which are non-preprocessed assembly. Thus, we no longer need the
C language property.

The benefit here is that the files can now benefit from CMAKE_ASM_FLAGS
rather than CMAKE_C_FLAGS.

Patch By: JamesNagurne

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

Added: 


Modified: 
libunwind/src/CMakeLists.txt

Removed: 




diff  --git a/libunwind/src/CMakeLists.txt b/libunwind/src/CMakeLists.txt
index d02f8a21dd48..01ec4b6de651 100644
--- a/libunwind/src/CMakeLists.txt
+++ b/libunwind/src/CMakeLists.txt
@@ -24,9 +24,6 @@ set(LIBUNWIND_ASM_SOURCES
 UnwindRegistersRestore.S
 UnwindRegistersSave.S
 )
-set_source_files_properties(${LIBUNWIND_ASM_SOURCES}
-PROPERTIES
-  LANGUAGE C)
 
 set(LIBUNWIND_HEADERS
 AddressSpace.hpp



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

[PATCH] D73380: [clang] Annotating C++'s `operator new` with more attributes

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

rsmith added inline comments.



Comment at: clang/lib/Sema/SemaDecl.cpp:14361
+/// attributes are applied to declarations.
+void Sema::AddKnownFunctionAttributesForReplaceableGlobalAllocationFunction(
+FunctionDecl *FD) {

This should all be done by CodeGen, not by injecting source-level attributes.



Comment at: clang/lib/Sema/SemaDecl.cpp:14414-14432
+  // C++2a [basic.stc.dynamic.allocation]p3:
+  //   For an allocation function [...], the pointer returned on a successful
+  //   call shall represent the address of storage that is aligned as follows:
+  //   (3.2),(3.3) Otherwise, [...] the storage is aligned for any object
+  //   that does not have new-extended alignment [...].
+  //
+  // NOTE: we intentionally always manifest this basic alignment, because it is

This is incorrect. The pointer returned by `operator new` is only suitably 
aligned for any object that does not have new-extended alignment **and is of 
the requested size**. And the pointer returned by `operator new[]` is suitably 
aligned for any object **that is no larger than the requested size**. (These 
are both different from the rule for `malloc`, which does behave as you're 
suggesting here.) For example:

Suppose the default new alignment and the largest fundamental alignment are 
both 16, and we try to allocate 12 bytes. Then:

 * `operator new` need only return storage that is 4-byte aligned (because that 
is the largest alignment that can be required by a type `T` with `sizeof(T) == 
12`)
 * `operator new` need only return storage that is 8-byte aligned (because that 
is the largest alignment that can be required by a type `T` with `sizeof(T) <= 
12`)
 * `malloc` must return storage that is 16-byte aligned (because that is the 
largest fundamental alignment)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73380



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

[PATCH] D73388: NFC: Implement AST node skipping in ParentMapContext

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

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

This seems reasonable to me.

If you want to unify something about this and the `Ignore*` functions in 
`Expr`, maybe we could add a "classify" mechanism, so you could ask "what kind 
of node is this?" and get back an enumerated value that indicates whether it's 
semantic / syntactic / whatever. Then we could implement both this and the 
downward-skipping in terms of that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73388



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

[clang] 04f131d - DR1753: Don't permit x.NS::~T() as a pseudo-destructor name.

[Richard Smith via cfe-commits]

Author: Richard Smith
Date: 2020-01-24T18:53:50-08:00
New Revision: 04f131da0b19abff611773c03be9bafb53c753ce

URL: 
https://github.com/llvm/llvm-project/commit/04f131da0b19abff611773c03be9bafb53c753ce
DIFF: 
https://github.com/llvm/llvm-project/commit/04f131da0b19abff611773c03be9bafb53c753ce.diff

LOG: DR1753: Don't permit x.NS::~T() as a pseudo-destructor name.

When used as qualified names, pseudo-destructors are always named as if
they were members of the type, never as members of the namespace
enclosing the type.

Added: 


Modified: 
clang/lib/Parse/ParseExprCXX.cpp
clang/test/CXX/drs/dr17xx.cpp
clang/test/SemaCXX/pseudo-destructors.cpp
clang/www/cxx_dr_status.html

Removed: 




diff  --git a/clang/lib/Parse/ParseExprCXX.cpp 
b/clang/lib/Parse/ParseExprCXX.cpp
index 036eabb94dd7..73d15cbc20c1 100644
--- a/clang/lib/Parse/ParseExprCXX.cpp
+++ b/clang/lib/Parse/ParseExprCXX.cpp
@@ -418,8 +418,7 @@ bool Parser::ParseOptionalCXXScopeSpecifier(CXXScopeSpec 
,
 }
 
 if (Next.is(tok::coloncolon)) {
-  if (CheckForDestructor && GetLookAheadToken(2).is(tok::tilde) &&
-  !Actions.isNonTypeNestedNameSpecifier(getCurScope(), SS, IdInfo)) {
+  if (CheckForDestructor && GetLookAheadToken(2).is(tok::tilde)) {
 *MayBePseudoDestructor = true;
 return false;
   }
@@ -548,7 +547,7 @@ bool Parser::ParseOptionalCXXScopeSpecifier(CXXScopeSpec 
,
   // Even if we didn't see any pieces of a nested-name-specifier, we
   // still check whether there is a tilde in this position, which
   // indicates a potential pseudo-destructor.
-  if (CheckForDestructor && Tok.is(tok::tilde))
+  if (CheckForDestructor && !HasScopeSpecifier && Tok.is(tok::tilde))
 *MayBePseudoDestructor = true;
 
   return false;
@@ -1689,31 +1688,42 @@ ExprResult Parser::ParseCXXUuidof() {
 
 /// Parse a C++ pseudo-destructor expression after the base,
 /// . or -> operator, and nested-name-specifier have already been
-/// parsed.
+/// parsed. We're handling this fragment of the grammar:
+///
+///   postfix-expression: [C++2a expr.post]
+/// postfix-expression . template[opt] id-expression
+/// postfix-expression -> template[opt] id-expression
 ///
-///   postfix-expression: [C++ 5.2]
-/// postfix-expression . pseudo-destructor-name
-/// postfix-expression -> pseudo-destructor-name
+///   id-expression:
+/// qualified-id
+/// unqualified-id
 ///
-///   pseudo-destructor-name:
-/// ::[opt] nested-name-specifier[opt] type-name :: ~type-name
-/// ::[opt] nested-name-specifier template simple-template-id ::
-/// ~type-name
-/// ::[opt] nested-name-specifier[opt] ~type-name
+///   qualified-id:
+/// nested-name-specifier template[opt] unqualified-id
 ///
+///   nested-name-specifier:
+/// type-name ::
+/// decltype-specifier ::FIXME: not implemented, but probably only
+/// allowed in C++ grammar by accident
+/// nested-name-specifier identifier ::
+/// nested-name-specifier template[opt] simple-template-id ::
+/// [...]
+///
+///   unqualified-id:
+/// ~ type-name
+/// ~ decltype-specifier
+/// [...]
+///
+/// ... where the all but the last component of the nested-name-specifier
+/// has already been parsed, and the base expression is not of a non-dependent
+/// class type.
 ExprResult
 Parser::ParseCXXPseudoDestructor(Expr *Base, SourceLocation OpLoc,
  tok::TokenKind OpKind,
  CXXScopeSpec ,
  ParsedType ObjectType) {
-  // We're parsing either a pseudo-destructor-name or a dependent
-  // member access that has the same form as a
-  // pseudo-destructor-name. We parse both in the same way and let
-  // the action model sort them out.
-  //
-  // Note that the ::[opt] nested-name-specifier[opt] has already
-  // been parsed, and if there was a simple-template-id, it has
-  // been coalesced into a template-id annotation token.
+  // If the last component of the (optional) nested-name-specifier is
+  // template[opt] simple-template-id, it has already been annotated.
   UnqualifiedId FirstTypeName;
   SourceLocation CCLoc;
   if (Tok.is(tok::identifier)) {
@@ -1722,14 +1732,13 @@ Parser::ParseCXXPseudoDestructor(Expr *Base, 
SourceLocation OpLoc,
 assert(Tok.is(tok::coloncolon) &&"ParseOptionalCXXScopeSpecifier fail");
 CCLoc = ConsumeToken();
   } else if (Tok.is(tok::annot_template_id)) {
-// FIXME: retrieve TemplateKWLoc from template-id annotation and
-// store it in the pseudo-dtor node (to be used when instantiating it).
 FirstTypeName.setTemplateId(
   (TemplateIdAnnotation 
*)Tok.getAnnotationValue());
 ConsumeAnnotationToken();
 

[PATCH] D73397: [Clang] Enable -fsanitize=leak on Fuchsia targets

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr updated this revision to Diff 240346.
mcgrathr added a comment.

Fixed Android SafeStack case, now passing check-clang.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73397

Files:
  clang/lib/Driver/SanitizerArgs.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Driver/ToolChains/Fuchsia.cpp
  
clang/test/Driver/Inputs/resource_dir_with_per_target_subdir/lib/aarch64-fuchsia/libclang_rt.lsan.a
  
clang/test/Driver/Inputs/resource_dir_with_per_target_subdir/lib/x86_64-fuchsia/libclang_rt.lsan.a
  clang/test/Driver/fuchsia.c

Index: clang/test/Driver/fuchsia.c
===
--- clang/test/Driver/fuchsia.c
+++ clang/test/Driver/fuchsia.c
@@ -174,6 +174,35 @@
 // CHECK-SCUDO-SHARED: "-fsanitize=safe-stack,scudo"
 // CHECK-SCUDO-SHARED: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}x86_64-fuchsia{{/|}}libclang_rt.scudo.so"
 
+// RUN: %clang %s -### --target=aarch64-fuchsia \
+// RUN: -fsanitize=leak 2>&1 \
+// RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
+// RUN: -fuse-ld=lld \
+// RUN: | FileCheck %s -check-prefix=CHECK-LSAN-AARCH64
+// CHECK-LSAN-AARCH64: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
+// CHECK-LSAN-AARCH64: "-fsanitize=leak,shadow-call-stack"
+// CHECK-LSAN-AARCH64: "-pie"
+// CHECK-LSAN-AARCH64: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}aarch64-fuchsia{{/|}}libclang_rt.lsan.a"
+
+// RUN: %clang %s -### --target=x86_64-fuchsia \
+// RUN: -fsanitize=leak 2>&1 \
+// RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
+// RUN: -fuse-ld=lld \
+// RUN: | FileCheck %s -check-prefix=CHECK-LSAN-X86
+// CHECK-LSAN-X86: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
+// CHECK-LSAN-X86: "-fsanitize=leak,safe-stack"
+// CHECK-LSAN-X86: "-pie"
+// CHECK-LSAN-X86: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}x86_64-fuchsia{{/|}}libclang_rt.lsan.a"
+
+// RUN: %clang %s -### --target=aarch64-fuchsia \
+// RUN: -fsanitize=leak -fPIC -shared 2>&1 \
+// RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
+// RUN: -fuse-ld=lld \
+// RUN: | FileCheck %s -check-prefix=CHECK-LSAN-SHARED
+// CHECK-LSAN-SHARED: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
+// CHECK-LSAN-SHARED: "-fsanitize=leak,shadow-call-stack"
+// CHECK-LSAN-SHARED-NOT: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}aarch64-fuchsia{{/|}}libclang_rt.lsan.a"
+
 // RUN: %clang %s -### --target=x86_64-fuchsia \
 // RUN: -fxray-instrument -fxray-modes=xray-basic \
 // RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
Index: clang/lib/Driver/ToolChains/Fuchsia.cpp
===
--- clang/lib/Driver/ToolChains/Fuchsia.cpp
+++ clang/lib/Driver/ToolChains/Fuchsia.cpp
@@ -340,6 +340,7 @@
   Res |= SanitizerKind::PointerSubtract;
   Res |= SanitizerKind::Fuzzer;
   Res |= SanitizerKind::FuzzerNoLink;
+  Res |= SanitizerKind::Leak;
   Res |= SanitizerKind::SafeStack;
   Res |= SanitizerKind::Scudo;
   return Res;
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -642,17 +642,21 @@
 StaticRuntimes.push_back("stats_client");
 
   // Collect static runtimes.
-  if (Args.hasArg(options::OPT_shared) || SanArgs.needsSharedRt()) {
-// Don't link static runtimes into DSOs or if -shared-libasan.
+  if (Args.hasArg(options::OPT_shared)) {
+// Don't link static runtimes into DSOs.
 return;
   }
-  if (SanArgs.needsAsanRt() && SanArgs.linkRuntimes()) {
+
+  // Each static runtime that has a DSO counterpart above is excluded below,
+  // but runtimes that exist only as static are not affected by needsSharedRt.
+
+  if (!SanArgs.needsSharedRt() && SanArgs.needsAsanRt() && SanArgs.linkRuntimes()) {
 StaticRuntimes.push_back("asan");
 if (SanArgs.linkCXXRuntimes())
   StaticRuntimes.push_back("asan_cxx");
   }
 
-  if (SanArgs.needsHwasanRt() && SanArgs.linkRuntimes()) {
+  if (!SanArgs.needsSharedRt() && SanArgs.needsHwasanRt() && SanArgs.linkRuntimes()) {
 StaticRuntimes.push_back("hwasan");
 if (SanArgs.linkCXXRuntimes())
   StaticRuntimes.push_back("hwasan_cxx");
@@ -671,7 +675,7 @@
 if (SanArgs.linkCXXRuntimes())
   StaticRuntimes.push_back("tsan_cxx");
   }
-  if (SanArgs.needsUbsanRt() && SanArgs.linkRuntimes()) {
+  if (!SanArgs.needsSharedRt() && SanArgs.needsUbsanRt() && SanArgs.linkRuntimes()) {
 if (SanArgs.requiresMinimalRuntime()) {
   StaticRuntimes.push_back("ubsan_minimal");
 } else {
@@ -684,18 +688,20 @@
 NonWholeStaticRuntimes.push_back("safestack");
 RequiredSymbols.push_back("__safestack_init");
   }
-  if (SanArgs.needsCfiRt() && SanArgs.linkRuntimes())
-StaticRuntimes.push_back("cfi");
-  if 

[PATCH] D73397: [Clang] Enable -fsanitize=leak on Fuchsia targets

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr created this revision.
mcgrathr added reviewers: phosek, aarongreen, cryptoad, vitalybuka.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This required some fixes to the generic code for two issues:

1. -fsanitize=safe-stack is default on x86_64-fuchsia and is *not* incompatible 
with -fsanitize=leak on Fuchisa

2. -fsanitize=leak and other static-only runtimes must not be omitted under 
-shared-libsan (which is the default on Fuchsia)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D73397

Files:
  clang/lib/Driver/SanitizerArgs.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Driver/ToolChains/Fuchsia.cpp
  clang/test/Driver/fuchsia.c

Index: clang/test/Driver/fuchsia.c
===
--- clang/test/Driver/fuchsia.c
+++ clang/test/Driver/fuchsia.c
@@ -174,6 +174,35 @@
 // CHECK-SCUDO-SHARED: "-fsanitize=safe-stack,scudo"
 // CHECK-SCUDO-SHARED: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}x86_64-fuchsia{{/|}}libclang_rt.scudo.so"
 
+// RUN: %clang %s -### --target=aarch64-fuchsia \
+// RUN: -fsanitize=leak 2>&1 \
+// RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
+// RUN: -fuse-ld=lld \
+// RUN: | FileCheck %s -check-prefix=CHECK-LSAN-AARCH64
+// CHECK-LSAN-AARCH64: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
+// CHECK-LSAN-AARCH64: "-fsanitize=leak,shadow-call-stack"
+// CHECK-LSAN-AARCH64: "-pie"
+// CHECK-LSAN-AARCH64: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}aarch64-fuchsia{{/|}}libclang_rt.lsan-aarch64.a"
+
+// RUN: %clang %s -### --target=x86_64-fuchsia \
+// RUN: -fsanitize=leak 2>&1 \
+// RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
+// RUN: -fuse-ld=lld \
+// RUN: | FileCheck %s -check-prefix=CHECK-LSAN-X86
+// CHECK-LSAN-X86: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
+// CHECK-LSAN-X86: "-fsanitize=leak,safe-stack"
+// CHECK-LSAN-X86: "-pie"
+// CHECK-LSAN-X86: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}x86_64-fuchsia{{/|}}libclang_rt.lsan-x86_64.a"
+
+// RUN: %clang %s -### --target=aarch64-fuchsia \
+// RUN: -fsanitize=leak -fPIC -shared 2>&1 \
+// RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
+// RUN: -fuse-ld=lld \
+// RUN: | FileCheck %s -check-prefix=CHECK-LSAN-SHARED
+// CHECK-LSAN-SHARED: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
+// CHECK-LSAN-SHARED: "-fsanitize=leak,shadow-call-stack"
+// CHECK-LSAN-SHARED-NOT: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}aarch64-fuchsia{{/|}}libclang_rt.lsan-aarch64.a"
+
 // RUN: %clang %s -### --target=x86_64-fuchsia \
 // RUN: -fxray-instrument -fxray-modes=xray-basic \
 // RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
Index: clang/lib/Driver/ToolChains/Fuchsia.cpp
===
--- clang/lib/Driver/ToolChains/Fuchsia.cpp
+++ clang/lib/Driver/ToolChains/Fuchsia.cpp
@@ -340,6 +340,7 @@
   Res |= SanitizerKind::PointerSubtract;
   Res |= SanitizerKind::Fuzzer;
   Res |= SanitizerKind::FuzzerNoLink;
+  Res |= SanitizerKind::Leak;
   Res |= SanitizerKind::SafeStack;
   Res |= SanitizerKind::Scudo;
   return Res;
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -642,17 +642,21 @@
 StaticRuntimes.push_back("stats_client");
 
   // Collect static runtimes.
-  if (Args.hasArg(options::OPT_shared) || SanArgs.needsSharedRt()) {
-// Don't link static runtimes into DSOs or if -shared-libasan.
+  if (Args.hasArg(options::OPT_shared)) {
+// Don't link static runtimes into DSOs.
 return;
   }
-  if (SanArgs.needsAsanRt() && SanArgs.linkRuntimes()) {
+
+  // Each static runtime that has a DSO counterpart above is excluded below,
+  // but runtimes that exist only as static are not affected by needsSharedRt.
+
+  if (!SanArgs.needsSharedRt() && SanArgs.needsAsanRt() && SanArgs.linkRuntimes()) {
 StaticRuntimes.push_back("asan");
 if (SanArgs.linkCXXRuntimes())
   StaticRuntimes.push_back("asan_cxx");
   }
 
-  if (SanArgs.needsHwasanRt() && SanArgs.linkRuntimes()) {
+  if (!SanArgs.needsSharedRt() && SanArgs.needsHwasanRt() && SanArgs.linkRuntimes()) {
 StaticRuntimes.push_back("hwasan");
 if (SanArgs.linkCXXRuntimes())
   StaticRuntimes.push_back("hwasan_cxx");
@@ -671,7 +675,7 @@
 if (SanArgs.linkCXXRuntimes())
   StaticRuntimes.push_back("tsan_cxx");
   }
-  if (SanArgs.needsUbsanRt() && SanArgs.linkRuntimes()) {
+  if (!SanArgs.needsSharedRt() && SanArgs.needsUbsanRt() && SanArgs.linkRuntimes()) {
 if (SanArgs.requiresMinimalRuntime()) {
   StaticRuntimes.push_back("ubsan_minimal");
 } else {
@@ -684,18 +688,20 @@
 NonWholeStaticRuntimes.push_back("safestack");
 RequiredSymbols.push_back("__safestack_init");
   }
-  if 

[clang] 0ebc8e6 - [Sema] Remove unneeded TreeTransform.h includes, NFC

[Reid Kleckner via cfe-commits]

Author: Reid Kleckner
Date: 2020-01-24T17:48:38-08:00
New Revision: 0ebc8e6c42167ba049aef8d73cae7eb7a316c8a1

URL: 
https://github.com/llvm/llvm-project/commit/0ebc8e6c42167ba049aef8d73cae7eb7a316c8a1
DIFF: 
https://github.com/llvm/llvm-project/commit/0ebc8e6c42167ba049aef8d73cae7eb7a316c8a1.diff

LOG: [Sema] Remove unneeded TreeTransform.h includes, NFC

SemaDecl.cpp and SemaType.cpp don't have any TreeTransforms.

Added: 


Modified: 
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaType.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 0bf490336537..d70e85afb465 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -10,7 +10,6 @@
 //
 
//===--===//
 
-#include "TreeTransform.h"
 #include "TypeLocBuilder.h"
 #include "clang/AST/ASTConsumer.h"
 #include "clang/AST/ASTContext.h"

diff  --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp
index 2d4263170740..4dee030a9362 100644
--- a/clang/lib/Sema/SemaType.cpp
+++ b/clang/lib/Sema/SemaType.cpp
@@ -11,7 +11,6 @@
 
//===--===//
 
 #include "TypeLocBuilder.h"
-#include "TreeTransform.h"
 #include "clang/AST/ASTConsumer.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/ASTMutationListener.h"



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

[PATCH] D73385: [Sema] Split availability processing into SemaAvailability.cpp

[Reid Kleckner via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGdd8e0a0a23ba: [Sema] Split availability processing into 
SemaAvailability.cpp (authored by rnk).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73385

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/CMakeLists.txt
  clang/lib/Sema/SemaAvailability.cpp
  clang/lib/Sema/SemaDeclAttr.cpp

Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -,534 +,6 @@
   DD.Triggered = true;
 }
 
-static const AvailabilityAttr *getAttrForPlatform(ASTContext ,
-  const Decl *D) {
-  // Check each AvailabilityAttr to find the one for this platform.
-  for (const auto *A : D->attrs()) {
-if (const auto *Avail = dyn_cast(A)) {
-  // FIXME: this is copied from CheckAvailability. We should try to
-  // de-duplicate.
-
-  // Check if this is an App Extension "platform", and if so chop off
-  // the suffix for matching with the actual platform.
-  StringRef ActualPlatform = Avail->getPlatform()->getName();
-  StringRef RealizedPlatform = ActualPlatform;
-  if (Context.getLangOpts().AppExt) {
-size_t suffix = RealizedPlatform.rfind("_app_extension");
-if (suffix != StringRef::npos)
-  RealizedPlatform = RealizedPlatform.slice(0, suffix);
-  }
-
-  StringRef TargetPlatform = Context.getTargetInfo().getPlatformName();
-
-  // Match the platform name.
-  if (RealizedPlatform == TargetPlatform)
-return Avail;
-}
-  }
-  return nullptr;
-}
-
-/// The diagnostic we should emit for \c D, and the declaration that
-/// originated it, or \c AR_Available.
-///
-/// \param D The declaration to check.
-/// \param Message If non-null, this will be populated with the message from
-/// the availability attribute that is selected.
-/// \param ClassReceiver If we're checking the the method of a class message
-/// send, the class. Otherwise nullptr.
-static std::pair
-ShouldDiagnoseAvailabilityOfDecl(Sema , const NamedDecl *D,
- std::string *Message,
- ObjCInterfaceDecl *ClassReceiver) {
-  AvailabilityResult Result = D->getAvailability(Message);
-
-  // For typedefs, if the typedef declaration appears available look
-  // to the underlying type to see if it is more restrictive.
-  while (const auto *TD = dyn_cast(D)) {
-if (Result == AR_Available) {
-  if (const auto *TT = TD->getUnderlyingType()->getAs()) {
-D = TT->getDecl();
-Result = D->getAvailability(Message);
-continue;
-  }
-}
-break;
-  }
-
-  // Forward class declarations get their attributes from their definition.
-  if (const auto *IDecl = dyn_cast(D)) {
-if (IDecl->getDefinition()) {
-  D = IDecl->getDefinition();
-  Result = D->getAvailability(Message);
-}
-  }
-
-  if (const auto *ECD = dyn_cast(D))
-if (Result == AR_Available) {
-  const DeclContext *DC = ECD->getDeclContext();
-  if (const auto *TheEnumDecl = dyn_cast(DC)) {
-Result = TheEnumDecl->getAvailability(Message);
-D = TheEnumDecl;
-  }
-}
-
-  // For +new, infer availability from -init.
-  if (const auto *MD = dyn_cast(D)) {
-if (S.NSAPIObj && ClassReceiver) {
-  ObjCMethodDecl *Init = ClassReceiver->lookupInstanceMethod(
-  S.NSAPIObj->getInitSelector());
-  if (Init && Result == AR_Available && MD->isClassMethod() &&
-  MD->getSelector() == S.NSAPIObj->getNewSelector() &&
-  MD->definedInNSObject(S.getASTContext())) {
-Result = Init->getAvailability(Message);
-D = Init;
-  }
-}
-  }
-
-  return {Result, D};
-}
-
-
-/// whether we should emit a diagnostic for \c K and \c DeclVersion in
-/// the context of \c Ctx. For example, we should emit an unavailable diagnostic
-/// in a deprecated context, but not the other way around.
-static bool
-ShouldDiagnoseAvailabilityInContext(Sema , AvailabilityResult K,
-VersionTuple DeclVersion, Decl *Ctx,
-const NamedDecl *OffendingDecl) {
-  assert(K != AR_Available && "Expected an unavailable declaration here!");
-
-  // Checks if we should emit the availability diagnostic in the context of C.
-  auto CheckContext = [&](const Decl *C) {
-if (K == AR_NotYetIntroduced) {
-  if (const AvailabilityAttr *AA = getAttrForPlatform(S.Context, C))
-if (AA->getIntroduced() >= DeclVersion)
-  return true;
-} else if (K == AR_Deprecated) {
-  if (C->isDeprecated())
-return true;
-} else if (K == AR_Unavailable) {
-  // It is perfectly fine to refer to an 'unavailable' Objective-C method
-  // when it is 

[clang] dd8e0a0 - [Sema] Split availability processing into SemaAvailability.cpp

[Reid Kleckner via cfe-commits]

Author: Reid Kleckner
Date: 2020-01-24T17:35:39-08:00
New Revision: dd8e0a0a23bab23fee283145c599014bf4b450d3

URL: 
https://github.com/llvm/llvm-project/commit/dd8e0a0a23bab23fee283145c599014bf4b450d3
DIFF: 
https://github.com/llvm/llvm-project/commit/dd8e0a0a23bab23fee283145c599014bf4b450d3.diff

LOG: [Sema] Split availability processing into SemaAvailability.cpp

Reduces compile time of SemaDeclAttr.cpp down to 28s from 50s. The new
TU does a few RecursiveASTVisitor instantiations, so it takes 30s.

Reviewed By: rsmith

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

Added: 
clang/lib/Sema/SemaAvailability.cpp

Modified: 
clang/include/clang/Sema/Sema.h
clang/lib/Sema/CMakeLists.txt
clang/lib/Sema/SemaDeclAttr.cpp

Removed: 




diff  --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 1e9ea0dd9f3d..5ab74e4cd662 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -4424,6 +4424,8 @@ class Sema final {
   /// Issue any -Wunguarded-availability warnings in \c FD
   void DiagnoseUnguardedAvailabilityViolations(Decl *FD);
 
+  void handleDelayedAvailabilityCheck(sema::DelayedDiagnostic , Decl *Ctx);
+
   
//======//
   // Expression Parsing Callbacks: SemaExpr.cpp.
 

diff  --git a/clang/lib/Sema/CMakeLists.txt b/clang/lib/Sema/CMakeLists.txt
index 844abc7ce598..89ea6904de85 100644
--- a/clang/lib/Sema/CMakeLists.txt
+++ b/clang/lib/Sema/CMakeLists.txt
@@ -29,6 +29,7 @@ add_clang_library(clangSema
   Sema.cpp
   SemaAccess.cpp
   SemaAttr.cpp
+  SemaAvailability.cpp
   SemaCXXScopeSpec.cpp
   SemaCast.cpp
   SemaChecking.cpp

diff  --git a/clang/lib/Sema/SemaAvailability.cpp 
b/clang/lib/Sema/SemaAvailability.cpp
new file mode 100644
index ..0c0275d5e90b
--- /dev/null
+++ b/clang/lib/Sema/SemaAvailability.cpp
@@ -0,0 +1,963 @@
+//===--- SemaAvailability.cpp - Availability attribute handling 
---===//
+//
+// 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
+//
+//===--===//
+//
+//  This file processes the availability attribute.
+//
+//===--===//
+
+#include "clang/AST/Attr.h"
+#include "clang/AST/Decl.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/Basic/DiagnosticSema.h"
+#include "clang/Lex/Preprocessor.h"
+#include "clang/Sema/DelayedDiagnostic.h"
+#include "clang/Sema/ScopeInfo.h"
+#include "clang/Sema/Sema.h"
+
+using namespace clang;
+using namespace sema;
+
+static const AvailabilityAttr *getAttrForPlatform(ASTContext ,
+  const Decl *D) {
+  // Check each AvailabilityAttr to find the one for this platform.
+  for (const auto *A : D->attrs()) {
+if (const auto *Avail = dyn_cast(A)) {
+  // FIXME: this is copied from CheckAvailability. We should try to
+  // de-duplicate.
+
+  // Check if this is an App Extension "platform", and if so chop off
+  // the suffix for matching with the actual platform.
+  StringRef ActualPlatform = Avail->getPlatform()->getName();
+  StringRef RealizedPlatform = ActualPlatform;
+  if (Context.getLangOpts().AppExt) {
+size_t suffix = RealizedPlatform.rfind("_app_extension");
+if (suffix != StringRef::npos)
+  RealizedPlatform = RealizedPlatform.slice(0, suffix);
+  }
+
+  StringRef TargetPlatform = Context.getTargetInfo().getPlatformName();
+
+  // Match the platform name.
+  if (RealizedPlatform == TargetPlatform)
+return Avail;
+}
+  }
+  return nullptr;
+}
+
+/// The diagnostic we should emit for \c D, and the declaration that
+/// originated it, or \c AR_Available.
+///
+/// \param D The declaration to check.
+/// \param Message If non-null, this will be populated with the message from
+/// the availability attribute that is selected.
+/// \param ClassReceiver If we're checking the the method of a class message
+/// send, the class. Otherwise nullptr.
+static std::pair
+ShouldDiagnoseAvailabilityOfDecl(Sema , const NamedDecl *D,
+ std::string *Message,
+ ObjCInterfaceDecl *ClassReceiver) {
+  AvailabilityResult Result = D->getAvailability(Message);
+
+  // For typedefs, if the typedef declaration appears available look
+  // to the underlying type to see if it is more restrictive.
+  while (const auto *TD = dyn_cast(D)) {
+if (Result == AR_Available) {
+  if (const auto *TT = TD->getUnderlyingType()->getAs()) {
+D = TT->getDecl();
+Result = D->getAvailability(Message);
+continue;
+  }
+   

[PATCH] D73151: [analyzer] Fix handle leak false positive when the handle dies too early

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun updated this revision to Diff 240337.
xazax.hun added a comment.

- Address review comments.


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

https://reviews.llvm.org/D73151

Files:
  clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp
  clang/test/Analysis/fuchsia_handle.cpp


Index: clang/test/Analysis/fuchsia_handle.cpp
===
--- clang/test/Analysis/fuchsia_handle.cpp
+++ clang/test/Analysis/fuchsia_handle.cpp
@@ -66,6 +66,26 @@
 zx_handle_close(sa);
 }
 
+void handleDieBeforeErrorSymbol01() {
+  zx_handle_t sa, sb;
+  zx_status_t status = zx_channel_create(0, , );
+  if (status < 0)
+return;
+  __builtin_trap();
+}
+
+void handleDieBeforeErrorSymbol02() {
+  zx_handle_t sa, sb;
+  zx_status_t status = zx_channel_create(0, , );
+  // expected-note@-1 {{Handle allocated through 2nd parameter}}
+  if (status == 0) { // expected-note {{Assuming 'status' is equal to 0}}
+ // expected-note@-1 {{Taking true branch}}
+return; // expected-warning {{Potential leak of handle}}
+// expected-note@-1 {{Potential leak of handle}}
+  }
+  __builtin_trap();
+}
+
 void checkNoCrash01() {
   zx_handle_t sa, sb;
   zx_channel_create(0, , );
Index: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp
@@ -149,6 +149,10 @@
   CASE(Kind::Released)
   CASE(Kind::Escaped)
 }
+if (ErrorSym) {
+  OS << " ErrorSym: ";
+  ErrorSym->dumpToStream(OS);
+}
   }
 
   LLVM_DUMP_METHOD void dump() const { dump(llvm::errs()); }
@@ -401,7 +405,13 @@
   SmallVector LeakedSyms;
   HStateMapTy TrackedHandles = State->get();
   for (auto  : TrackedHandles) {
-if (!SymReaper.isDead(CurItem.first))
+SymbolRef ErrorSym = CurItem.second.getErrorSym();
+// Keeping zombie handle symbols. In case the error symbol is dying later
+// than the handle symbol we might produce spurious leak warnings (in case
+// we find out later from the status code that the handle allocation failed
+// in the first place).
+if (!SymReaper.isDead(CurItem.first) ||
+(ErrorSym && !SymReaper.isDead(ErrorSym)))
   continue;
 if (CurItem.second.isAllocated() || CurItem.second.maybeAllocated())
   LeakedSyms.push_back(CurItem.first);


Index: clang/test/Analysis/fuchsia_handle.cpp
===
--- clang/test/Analysis/fuchsia_handle.cpp
+++ clang/test/Analysis/fuchsia_handle.cpp
@@ -66,6 +66,26 @@
 zx_handle_close(sa);
 }
 
+void handleDieBeforeErrorSymbol01() {
+  zx_handle_t sa, sb;
+  zx_status_t status = zx_channel_create(0, , );
+  if (status < 0)
+return;
+  __builtin_trap();
+}
+
+void handleDieBeforeErrorSymbol02() {
+  zx_handle_t sa, sb;
+  zx_status_t status = zx_channel_create(0, , );
+  // expected-note@-1 {{Handle allocated through 2nd parameter}}
+  if (status == 0) { // expected-note {{Assuming 'status' is equal to 0}}
+ // expected-note@-1 {{Taking true branch}}
+return; // expected-warning {{Potential leak of handle}}
+// expected-note@-1 {{Potential leak of handle}}
+  }
+  __builtin_trap();
+}
+
 void checkNoCrash01() {
   zx_handle_t sa, sb;
   zx_channel_create(0, , );
Index: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp
@@ -149,6 +149,10 @@
   CASE(Kind::Released)
   CASE(Kind::Escaped)
 }
+if (ErrorSym) {
+  OS << " ErrorSym: ";
+  ErrorSym->dumpToStream(OS);
+}
   }
 
   LLVM_DUMP_METHOD void dump() const { dump(llvm::errs()); }
@@ -401,7 +405,13 @@
   SmallVector LeakedSyms;
   HStateMapTy TrackedHandles = State->get();
   for (auto  : TrackedHandles) {
-if (!SymReaper.isDead(CurItem.first))
+SymbolRef ErrorSym = CurItem.second.getErrorSym();
+// Keeping zombie handle symbols. In case the error symbol is dying later
+// than the handle symbol we might produce spurious leak warnings (in case
+// we find out later from the status code that the handle allocation failed
+// in the first place).
+if (!SymReaper.isDead(CurItem.first) ||
+(ErrorSym && !SymReaper.isDead(ErrorSym)))
   continue;
 if (CurItem.second.isAllocated() || CurItem.second.maybeAllocated())
   LeakedSyms.push_back(CurItem.first);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D73151: [analyzer] Fix handle leak false positive when the handle dies too early

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun marked 4 inline comments as done.
xazax.hun added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp:133-135
+  static HandleState getWithoutError(HandleState S) {
+return HandleState(S.K, nullptr);
+  }

xazax.hun wrote:
> NoQ wrote:
> > It already makes me mildly uncomfortable that our data is not "normalized", 
> > i.e. you can indicate the Schrödinger state by either adding an error 
> > symbol or changing from `Allocated` to `MaybeAllocated`. Do i understand it 
> > correctly that you're now adding a special state where the handle is still 
> > `MaybeAllocated` but there's no error symbol? Please comment this up.
> Yeah, the reason is that, when the handle is a return value, we have no idea 
> where the error symbol is. This would only happen if someone do not follow 
> the API guidelines (in Fuchsia). I'll add a comment.
Sorry, I was hasty and my head was full of another patch. It has nothing to do 
with returned handles. I used this to control when to let some symbols die. But 
with your suggested approach this factory is no longer needed.


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

https://reviews.llvm.org/D73151



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

[PATCH] D72810: [LifetimeAnalysis] Add support for lifetime annotations on functions

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun updated this revision to Diff 240335.
xazax.hun added a comment.

- Address some review comments.


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

https://reviews.llvm.org/D72810

Files:
  clang/include/clang/AST/Attr.h
  clang/include/clang/AST/LifetimeAttr.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/AST/CMakeLists.txt
  clang/lib/AST/LifetimeAttr.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaInit.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/Sema/attr-psets-annotation.cpp

Index: clang/test/Sema/attr-psets-annotation.cpp
===
--- /dev/null
+++ clang/test/Sema/attr-psets-annotation.cpp
@@ -0,0 +1,135 @@
+// NOT RUN: %clang_cc1 -fsyntax-only -Wlifetime -Wlifetime-dump-contracts -verify %s
+
+namespace gsl {
+struct null_t {
+  template 
+  operator T() const;
+} null;
+
+struct global_t {
+  template 
+  operator T() const;
+} global;
+
+struct invalid_t {
+  template 
+  operator T() const;
+} invalid;
+
+struct return_t {
+  template 
+  operator T() const;
+} Return;
+
+template T &();
+
+template
+struct PointerTraits {
+  using DerefType = decltype(*declval());
+};
+
+struct PSet {
+  template
+  PSet(const T&...);
+};
+
+template 
+auto deref(const T ) -> typename PointerTraits::DerefType;
+
+template 
+bool lifetime(const T , const PSet );
+} // namespace gsl
+
+void basic(int *a, int *b) [[gsl::pre(gsl::lifetime(b, {a}))]];
+// expected-warning@-1 {{Pre { b -> { a }; }}}
+
+void specials(int *a, int *b, int *c)
+[[gsl::pre(gsl::lifetime(a, {gsl::null}))]]
+[[gsl::pre(gsl::lifetime(b, {gsl::global}))]]
+[[gsl::pre(gsl::lifetime(c, {gsl::invalid}))]];
+// expected-warning@-4 {{Pre { a -> { null }; b -> { global }; c -> { invalid }; }}}
+
+void variadic(int *a, int *b, int *c)
+[[gsl::pre(gsl::lifetime(b, {a, c}))]];
+// expected-warning@-2 {{Pre { b -> { a c }; }}}
+
+void variadic_special(int *a, int *b, int *c)
+[[gsl::pre(gsl::lifetime(b, {a, gsl::null}))]];
+// expected-warning@-2 {{Pre { b -> { null a }; }}}
+
+void multiple_annotations(int *a, int *b, int *c)
+[[gsl::pre(gsl::lifetime(b, {a}))]]
+[[gsl::pre(gsl::lifetime(c, {a}))]];
+// expected-warning@-3 {{Pre { b -> { a }; c -> { a }; }}}
+
+void multiple_annotations_chained(int *a, int *b, int *c)
+[[gsl::pre(gsl::lifetime(b, {a}))]]
+[[gsl::pre(gsl::lifetime(c, {b}))]];
+// expected-warning@-3 {{Pre { b -> { a }; c -> { a }; }}}
+
+void deref_ptr(int *a, int *b, int **c)
+[[gsl::pre(gsl::lifetime(gsl::deref(c), {a}))]];
+// expected-warning@-2 {{Pre { *c -> { a }; }}}
+
+void deref_ptr_pointee(int *a, int *b, int **c)
+[[gsl::pre(gsl::lifetime(a, {gsl::deref(c)}))]];
+// expected-warning@-2 {{Pre { a -> { *c }; }}}
+
+void deref_ref(int *a, int *b, int *)
+[[gsl::pre(gsl::lifetime(gsl::deref(c), {a}))]];
+// expected-warning@-2 {{Pre { *c -> { a }; }}}
+
+void deref_ref_pointee(int *a, int *b, int *)
+[[gsl::pre(gsl::lifetime(a, {gsl::deref(c)}))]];
+// expected-warning@-2 {{Pre { a -> { *c }; }}}
+
+struct [[gsl::Owner(void)]] X {
+  void f(X **out)
+  [[gsl::post(gsl::lifetime(gsl::deref(out), {this}))]];
+  // expected-warning@-2 {{Pre { }  Post { *out -> { this }; }}}
+  X *operator+(const X& other)
+  [[gsl::post(gsl::lifetime(gsl::Return, {other}))]];
+  // expected-warning@-2 {{Pre { }  Post { (return value) -> { other }; }}}
+};
+
+template 
+It find(It begin, It end, const T )
+[[gsl::pre(gsl::lifetime(end, {begin}))]]
+[[gsl::post(gsl::lifetime(gsl::Return, {begin}))]];
+// expected-warning@-3 {{Pre { end -> { begin }; }  Post { (return value) -> { begin }; }}}
+
+int *find_nontemp(int *begin, int *end, const int )
+[[gsl::pre(gsl::lifetime(end, {begin}))]]
+[[gsl::post(gsl::lifetime(gsl::Return, {begin}))]];
+// expected-warning@-3 {{Pre { end -> { begin }; }  Post { (return value) -> { begin }; }}}
+
+struct [[gsl::Owner(int)]] MyOwner {
+  int *begin()
+  [[gsl::post(lifetime(gsl::Return, {this}))]];
+  // expected-warning@-2 {{Pre { }  Post { (return value) -> { this }; }}}
+  int *end()
+  [[gsl::post(lifetime(gsl::Return, {this}))]];
+  // expected-warning@-2 {{Pre { }  Post { (return value) -> { this }; }}}
+};
+
+void testGslWarning() {
+  int *res = find(MyOwner{}.begin(), MyOwner{}.end(), 5);
+  // expected-warning@-1 {{object backing the pointer will be destroyed at the end of the full-expression}}
+  (void)res;
+  int *res2 = find_nontemp(MyOwner{}.begin(), MyOwner{}.end(), 5);
+  // expected-warning@-1 {{object backing the pointer will be destroyed at the end of the full-expression}}
+  (void)res2;
+  X x;
+  // TODO: this should work without X annotated as owner.
+  X *xp = x + X{};
+  // expected-warning@-1 {{object backing the pointer will be destroyed at the end of the full-expression}}
+  (void)xp;
+}
+
+// 

[PATCH] D72876: Create a clang-tidy check to warn when -dealloc is implemented inside an ObjC class category.

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore added inline comments.



Comment at: clang-tools-extra/clang-tidy/objc/DeallocInCategoriesCheck.cpp:19
+
+void DeallocInCategoriesCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(

Early return if the language is not an Objective-C variant:
```
  // This check should only be applied to Objective-C sources.
  if (!getLangOpts().ObjC)
return;
```



Comment at: clang-tools-extra/clang-tidy/objc/DeallocInCategoriesCheck.cpp:21
+  Finder->addMatcher(
+  objcMethodDecl(hasName("dealloc"), 
hasDeclContext(objcCategoryImplDecl()))
+  .bind("dealloc"),

stephanemoore wrote:
> Add `isInstanceMethod()` within the `objcMethodDecl`?
Technically, isn't `-dealloc` specific to certain classes, e.g., 
[`NSObject`](https://developer.apple.com/documentation/objectivec/nsobject/1571947-dealloc?language=objc))
 and 
[NSProxy](https://developer.apple.com/documentation/foundation/nsproxy/1589830-dealloc?language=objc)?
 If that is true, we should technically check that the category is on a class 
that is or is derived from a relevant class like `NSObject` or `NSProxy`.


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

https://reviews.llvm.org/D72876



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

[PATCH] D73307: Unique Names for Functions with Internal Linkage

[Mircea Trofin via Phabricator via cfe-commits]

mtrofin added a comment.

In D73307#1839984 , @MaskRay wrote:

> In D73307#1839880 , @mtrofin wrote:
>
> > In D73307#1839829 , @MaskRay wrote:
> >
> > > The code change seems fine, but the test requires some changes. I haven't 
> > > followed Propeller development, but hope someone with profile experience 
> > > can confirm InternalLinkage is the only linkage we need to care about 
> > > (otherwise the option will be a misnomer if we ever extend it) and check 
> > > whether this feature is useful on its own. Does it improve profile 
> > > precision?
> >
> >
> > I can comment on the usefulness aspect: we had an earlier prototype of 
> > this, which we tried on a real-world application benchmark. The binary had 
> > ~10% of local statics exhibiting duplicate names. Ensuring unique names led 
> > to observable differences in the AFDO file (i.e. some of those functions 
> > had profiles that, before, were lost for one of the duplicates, and now 
> > were correctly attributed to the different functions), and a measurable 
> > performance improvement.
>
>
> Thanks! Do you happen to have numbers about the code size increase? Every 
> InternalLinkage function will have a longer time. They may take a significant 
> portion of the string table (.strtab) size. If you strip .strtab, the 
> profiling precision will be lowered to the situation before.


I assume you mean binary size increase. It was 0.8% larger in my case.


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

https://reviews.llvm.org/D73307



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

[PATCH] D72810: [LifetimeAnalysis] Add support for lifetime annotations on functions

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun marked 14 inline comments as done.
xazax.hun added inline comments.



Comment at: clang/include/clang/AST/LifetimeAttr.h:163
+// Maps each annotated entity to a lifetime set.
+using LifetimeContracts = std::map;
+

gribozavr2 wrote:
> Generally, DenseMap and DenseSet are faster. Any reason to not use them 
> instead?
No specific reason other than I am always insecure about the choice. Dense data 
structures are great for small objects and I do not know where the break even 
point is and never really did any benchmarks. Is there some guidelines 
somewhere for what object size should we prefer one over the other?



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:3342
+  InGroup, DefaultIgnore;
+def warn_dump_lifetime_contracts : Warning<"%0">, 
InGroup, DefaultIgnore;
+

gribozavr2 wrote:
> xazax.hun wrote:
> > gribozavr2 wrote:
> > > I think a warning that is a debugging aid is new territory.
> > Do you think a cc1 flag would be sufficient or do you have other 
> > ideas/preferences?
> Yes, I think that would be quite standard (similar to dumping the AST, 
> dumping the CFG etc.)
I started to look into a cc1 flag, but it looks like it needs a bit more 
plumbing I expected as some does not have access to the CompilerInvocation 
object or the FrontendOptions. While it is not too hard to pass an additional 
boolean to Sema I also started to think about the user/developer experience 
(ok, regular users are not expected to do debugging). The advantage of warnings 
are that we get a source location for free, so it is really easy to see where 
the message is originated from. And while it is true that I cannot think of any 
examples of using warnings for debugging the Clang Static Analyzer is full of 
checks that are only dumping debug data as warnings.


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

https://reviews.llvm.org/D72810



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

[PATCH] D73307: Unique Names for Functions with Internal Linkage

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

In D73307#1839880 , @mtrofin wrote:

> In D73307#1839829 , @MaskRay wrote:
>
> > The code change seems fine, but the test requires some changes. I haven't 
> > followed Propeller development, but hope someone with profile experience 
> > can confirm InternalLinkage is the only linkage we need to care about 
> > (otherwise the option will be a misnomer if we ever extend it) and check 
> > whether this feature is useful on its own. Does it improve profile 
> > precision?
>
>
> I can comment on the usefulness aspect: we had an earlier prototype of this, 
> which we tried on a real-world application benchmark. The binary had ~10% of 
> local statics exhibiting duplicate names. Ensuring unique names led to 
> observable differences in the AFDO file (i.e. some of those functions had 
> profiles that, before, were lost for one of the duplicates, and now were 
> correctly attributed to the different functions), and a measurable 
> performance improvement.


Thanks! Do you happen to have numbers about the code size increase? Every 
InternalLinkage function will have a longer time. They may take a significant 
portion of the string table (.strtab) size. If you strip .strtab, the profiling 
precision will be lowered to the situation before.


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

https://reviews.llvm.org/D73307



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

[PATCH] D72876: Create a clang-tidy check to warn when -dealloc is implemented inside an ObjC class category.

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore requested changes to this revision.
stephanemoore added inline comments.
This revision now requires changes to proceed.



Comment at: clang-tools-extra/clang-tidy/objc/DeallocInCategoriesCheck.cpp:21
+  Finder->addMatcher(
+  objcMethodDecl(hasName("dealloc"), 
hasDeclContext(objcCategoryImplDecl()))
+  .bind("dealloc"),

Add `isInstanceMethod()` within the `objcMethodDecl`?



Comment at: clang-tools-extra/clang-tidy/objc/DeallocInCategoriesCheck.h:24
+/// 
http://clang.llvm.org/extra/clang-tidy/checks/objc-dealloc-in-categories.html
+class DeallocInCategoriesCheck : public ClangTidyCheck {
+public:

What do you think of the name `DeallocInCategoryCheck`?



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/objc-dealloc-in-categories.m:23
+
+@implementation Bar (Category)
+- (void)dealloc {

Please add a class method `+dealloc` and make sure that the check does not 
trigger on that.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/objc-dealloc-in-categories.m:35
+// overriding implementation, and should not generate a warning.
+- (void)dealloc;
+@end

Perhaps add an `@implementation` for `Baz` (not a category implementation) that 
contains `-dealloc` to ensure that a declaration in a category with a 
definition in the implementation does not trigger?


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

https://reviews.llvm.org/D72876



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

[PATCH] D72675: [Clang][Driver] Fix -ffast-math/-ffp-contract interaction

[Andy Kaylor via Phabricator via cfe-commits]

andrew.w.kaylor added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2792
   if (!MathErrno && AssociativeMath && ReciprocalMath && !SignedZeros &&
-  !TrappingMath)
+  !TrappingMath && !(FPContract.equals("off") || FPContract.equals("on")))
 CmdArgs.push_back("-menable-unsafe-fp-math");

I think this would read better as "... && !FPContract.equals("off") && 
!FPContract.equals("on")" The '||' in the middle of all these '&&'s combined 
with the extra parens from the function call trips me up.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2846
   ReciprocalMath && !SignedZeros && !TrappingMath && !RoundingFPMath) {
-CmdArgs.push_back("-ffast-math");
+if (!(FPContract.equals("off") || FPContract.equals("on")))
+  CmdArgs.push_back("-ffast-math");

As above, I'd prefer "!off && !on".



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2854
 // Enable -ffp-contract=fast
 CmdArgs.push_back(Args.MakeArgString("-ffp-contract=fast"));
   else

This is a bit of an oddity in our handling. The FPContract local variable 
starts out initialized to an empty string. So, what we're saying here is that 
if you've used all the individual controls to set the rest of the fast math 
flags, we'll turn on fp-contract=fast also. That seems wrong. If you use 
"-ffast-math", FPContract will have been set to "fast" so that's not applicable 
here.

I believe this means

```
-fno-honor-infinities -fno-honor-nans -fno-math-errno -fassociative-math 
-freciprocal-math -fno-signed-zeros -fno-trapping-math -fno-rounding-math

```

will imply "-ffp-contract=fast". Even worse:

```
-ffp-contract=off -fno-fast-math -fno-honor-infinities -fno-honor-nans 
-fno-math-errno -fassociative-math -freciprocal-math -fno-signed-zeros 
-fno-trapping-math -fno-rounding-math

```
will imply "-ffp-contract=fast" because "-fno-fast-math" resets FPContract to 
empty.

I think we should initialize FPContract to whatever we want to be the default 
(on?) and remove the special case for empty here. Also, -fno-fast-math should 
either not change FPContract at all or set it to "off". Probably the latter 
since we're saying -ffast-math includes -ffp-contract=on.



Comment at: clang/test/Driver/fast-math.c:196
+// RUN: %clang -### -ffast-math -ffp-contract=fast -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-UNSAFE-MATH %s
+//

What about "-ffp-contract=off -ffast-math"? The way the code is written that 
will override the -ffp-contract option. That's probably what we want, though it 
might be nice to have a warning.


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

https://reviews.llvm.org/D72675



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

[PATCH] D73376: [analyzer] Add FuchsiaLockChecker and C11LockChecker

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun marked an inline comment as done.
xazax.hun added inline comments.



Comment at: clang/test/Analysis/c11lock.c:1
+// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.unix.C11Lock -verify %s
+

NoQ wrote:
> NoQ wrote:
> > I wouldn't mind `alpha.core` given that these functions belong to the 
> > standard library.
> (i.e., i mean put the checker into `alpha.core`)
> (but, yeah, also please add `core` to the run-line)
You are reading my mind. Actually I wanted to ask where to put this but forgot 
to ask :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73376



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

[PATCH] D73282: Fix debug-info generation for block invocations so that we set the LinkageName instead of the Name

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added inline comments.



Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:3659
 // Use llvm function name.
-Name = Fn->getName();
+if (Fn->getName().startswith("___Z"))
+  LinkageName = Fn->getName();

Could you please add a comment that Clang Blocks are generated as raw 
llvm::Functions but do have a mangled name and that is handling this case? 
Otherwise this would look suspicious.


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

https://reviews.llvm.org/D73282



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

[PATCH] D73282: Fix debug-info generation for block invocations so that we set the LinkageName instead of the Name

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added inline comments.



Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:3659
 // Use llvm function name.
-Name = Fn->getName();
+if (Fn->getName().startswith("___Z"))
+  LinkageName = Fn->getName();

aprantl wrote:
> Could you please add a comment that Clang Blocks are generated as raw 
> llvm::Functions but do have a mangled name and that is handling this case? 
> Otherwise this would look suspicious.
Should *all* raw LLVM functions have their name as the linkage name? Perhaps a 
raw LLVM function should only have a linkage name and no human-readable name?


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

https://reviews.llvm.org/D73282



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

[PATCH] D73307: Unique Names for Functions with Internal Linkage

[Mircea Trofin via Phabricator via cfe-commits]

mtrofin added inline comments.



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:
+  !getModule().getSourceFileName().empty()) {
+llvm::MD5 Md5;
+Md5.update(getModule().getSourceFileName());

Just a thought: md5 is a non-bijective transformation, afaik. How about using 
base64 encoding, with the caveat that we replace + with $ and / with _ (so it 
results in a valid name), and discard padding =

The value being, one can copy/paste that identifier, do the trivial conversion 
back to base64 ($->+, _->/) and get the module name. Useful when debugging, for 
example.


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

https://reviews.llvm.org/D73307



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

[PATCH] D73307: Unique Names for Functions with Internal Linkage

[Mircea Trofin via Phabricator via cfe-commits]

mtrofin added a comment.

In D73307#1839829 , @MaskRay wrote:

> The code change seems fine, but the test requires some changes. I haven't 
> followed Propeller development, but hope someone with profile experience can 
> confirm InternalLinkage is the only linkage we need to care about (otherwise 
> the option will be a misnomer if we ever extend it) and check whether this 
> feature is useful on its own. Does it improve profile precision?


I can comment on the usefulness aspect: we had an earlier prototype of this, 
which we tried on a real-world application benchmark. The binary had ~10% of 
local statics exhibiting duplicate names. Ensuring unique names led to 
observable differences in the AFDO file (i.e. some of those functions had 
profiles that, before, were lost for one of the duplicates, and now were 
correctly attributed to the different functions), and a measurable performance 
improvement.


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

https://reviews.llvm.org/D73307



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

[PATCH] D73028: Extract ExprTraversal class from Expr

[Stephen Kelly via Phabricator via cfe-commits]

steveire added a comment.

In D73028#1839644 , @steveire wrote:

> In D73028#1839572 , @rsmith wrote:
>
> > In D73028#1839494 , @steveire 
> > wrote:
> >
> > > In D73028#1839383 , @rsmith 
> > > wrote:
> > >
> > > >
> > >
> > >
> > > The follow-up is here: https://reviews.llvm.org/D73029 .
> > >
> > > I have the need to change the ascending traversal implemented in 
> > > ASTContext.cpp with a map (populated while descending) to make it skip 
> > > nodes too during parent traversal.
> > >
> > > I didn't want to have descending traversal in Expr.cpp and ascending 
> > > traversal in ASTContext.cpp as they would be likely to go out of sync, so 
> > > moving this implementation here allows extending the purpose in the 
> > > follow-up.
> >
> >
> > The ascending traversal should not be in the AST library at all. If tooling 
> > or static analysis wants this, that's fine, but navigating upwards is not 
> > something the AST supports, and we do not want any part of clang proper 
> > developing a reliance on having a parent map, so we're working on moving 
> > the parent map out of `ASTContext` (with a plan to eventually move it out 
> > of the AST library). See D71313  for 
> > related work in this area.
>
>
> Thanks for the heads-up - I'm trying to reach the conclusion from this 
> information.
>
> Are you saying that
>
> 1. this patch shouldn't be committed, so down-traversal-skipping should 
> remain in Expr.cpp
> 2. D73029  should be changed to implement 
> the up-traversal-skipping directly in ParentMapContext.cpp
>
>   Is that the right conclusion?


Implemented that in https://reviews.llvm.org/D73388

I think this and https://reviews.llvm.org/D73029 can be closed in favor of that 
one.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73028



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

[PATCH] D73388: Implement AST node skipping in ParentMapContext

[Stephen Kelly via Phabricator via cfe-commits]

steveire updated this revision to Diff 240318.
steveire added a comment.

constness


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73388

Files:
  clang/include/clang/AST/ParentMapContext.h
  clang/lib/AST/ParentMapContext.cpp

Index: clang/lib/AST/ParentMapContext.cpp
===
--- clang/lib/AST/ParentMapContext.cpp
+++ clang/lib/AST/ParentMapContext.cpp
@@ -23,7 +23,7 @@
 
 ParentMapContext::~ParentMapContext() = default;
 
-void ParentMapContext::clear() { Parents.clear(); }
+void ParentMapContext::clear() { Parents.reset(); }
 
 const Expr *ParentMapContext::traverseIgnored(const Expr *E) const {
   return traverseIgnored(const_cast(E));
@@ -116,11 +116,79 @@
 }
   }
 
-  DynTypedNodeList getParents(const ast_type_traits::DynTypedNode ) {
-if (Node.getNodeKind().hasPointerIdentity())
-  return getDynNodeFromMap(Node.getMemoizationData(), PointerParents);
+  DynTypedNodeList getParents(ast_type_traits::TraversalKind TK,
+  const ast_type_traits::DynTypedNode ) {
+if (Node.getNodeKind().hasPointerIdentity()) {
+  auto ParentList =
+  getDynNodeFromMap(Node.getMemoizationData(), PointerParents);
+  if (ParentList.size() == 1 &&
+  TK == ast_type_traits::TK_IgnoreUnlessSpelledInSource) {
+const auto *E = ParentList[0].get();
+const auto *Child = Node.get();
+if (E && Child)
+  return AscendIgnoreUnlessSpelledInSource(E, Child);
+  }
+  return ParentList;
+}
 return getDynNodeFromMap(Node, OtherParents);
   }
+
+  ast_type_traits::DynTypedNode
+  AscendIgnoreUnlessSpelledInSource(const Expr *E, const Expr *Child) {
+
+auto ShouldSkip = [](const Expr *E, const Expr *Child) {
+  if (isa(E))
+return true;
+
+  if (isa(E))
+return true;
+
+  if (isa(E))
+return true;
+
+  if (isa(E))
+return true;
+
+  if (isa(E))
+return true;
+
+  if (isa(E))
+return true;
+
+  auto SR = Child->getSourceRange();
+
+  if (const auto *C = dyn_cast(E)) {
+if (C->getSourceRange() == SR || !isa(C))
+  return true;
+  }
+
+  if (const auto *C = dyn_cast(E)) {
+if (C->getSourceRange() == SR)
+  return true;
+  }
+
+  if (const auto *C = dyn_cast(E)) {
+if (C->getSourceRange() == SR)
+  return true;
+  }
+  return false;
+};
+
+while (ShouldSkip(E, Child)) {
+  auto It = PointerParents.find(E);
+  if (It == PointerParents.end())
+break;
+  const auto *S = It->second.dyn_cast();
+  if (!S)
+return getSingleDynTypedNodeFromParentMap(It->second);
+  const auto *P = dyn_cast(S);
+  if (!P)
+return ast_type_traits::DynTypedNode::create(*S);
+  Child = E;
+  E = P;
+}
+return ast_type_traits::DynTypedNode::create(*E);
+  }
 };
 
 /// Template specializations to abstract away from pointers and TypeLocs.
@@ -151,8 +219,7 @@
 class ParentMapContext::ParentMap::ASTVisitor
 : public RecursiveASTVisitor {
 public:
-  ASTVisitor(ParentMap , ParentMapContext )
-  : Map(Map), MapCtx(MapCtx) {}
+  ASTVisitor(ParentMap ) : Map(Map) {}
 
 private:
   friend class RecursiveASTVisitor;
@@ -222,11 +289,8 @@
   }
 
   bool TraverseStmt(Stmt *StmtNode) {
-Stmt *FilteredNode = StmtNode;
-if (auto *ExprNode = dyn_cast_or_null(FilteredNode))
-  FilteredNode = MapCtx.traverseIgnored(ExprNode);
-return TraverseNode(FilteredNode, FilteredNode,
-[&] { return VisitorBase::TraverseStmt(FilteredNode); },
+return TraverseNode(StmtNode, StmtNode,
+[&] { return VisitorBase::TraverseStmt(StmtNode); },
 );
   }
 
@@ -245,21 +309,18 @@
   }
 
   ParentMap 
-  ParentMapContext 
   llvm::SmallVector ParentStack;
 };
 
 ParentMapContext::ParentMap::ParentMap(ASTContext ) {
-  ASTVisitor(*this, Ctx.getParentMapContext()).TraverseAST(Ctx);
+  ASTVisitor(*this).TraverseAST(Ctx);
 }
 
 DynTypedNodeList
 ParentMapContext::getParents(const ast_type_traits::DynTypedNode ) {
-  std::unique_ptr  = Parents[Traversal];
-  if (!P)
+  if (!Parents)
 // We build the parent map for the traversal scope (usually whole TU), as
 // hasAncestor can escape any subtree.
-P = std::make_unique(ASTCtx);
-  return P->getParents(Node);
+Parents = std::make_unique(ASTCtx);
+  return Parents->getParents(getTraversalKind(), Node);
 }
-
Index: clang/include/clang/AST/ParentMapContext.h
===
--- clang/include/clang/AST/ParentMapContext.h
+++ clang/include/clang/AST/ParentMapContext.h
@@ -69,7 +69,7 @@
   ASTContext 
   class ParentMap;
   ast_type_traits::TraversalKind Traversal = ast_type_traits::TK_AsIs;
-  std::map> Parents;
+  std::unique_ptr 

[PATCH] D73388: Implement AST node skipping in ParentMapContext

[Stephen Kelly via Phabricator via cfe-commits]

steveire created this revision.
steveire added reviewers: aaron.ballman, rsmith, rnk.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
steveire updated this revision to Diff 240318.
steveire added a comment.

constness


This allows ASTContext to store only one parent map, rather than storing
an entire parent map for each traversal mode used.

This is therefore a partial revert of commit 0a717d5b 
 (Make it 
possible
control matcher traversal kind with ASTContext, 2019-12-06).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D73388

Files:
  clang/include/clang/AST/ParentMapContext.h
  clang/lib/AST/ParentMapContext.cpp

Index: clang/lib/AST/ParentMapContext.cpp
===
--- clang/lib/AST/ParentMapContext.cpp
+++ clang/lib/AST/ParentMapContext.cpp
@@ -23,7 +23,7 @@
 
 ParentMapContext::~ParentMapContext() = default;
 
-void ParentMapContext::clear() { Parents.clear(); }
+void ParentMapContext::clear() { Parents.reset(); }
 
 const Expr *ParentMapContext::traverseIgnored(const Expr *E) const {
   return traverseIgnored(const_cast(E));
@@ -116,11 +116,79 @@
 }
   }
 
-  DynTypedNodeList getParents(const ast_type_traits::DynTypedNode ) {
-if (Node.getNodeKind().hasPointerIdentity())
-  return getDynNodeFromMap(Node.getMemoizationData(), PointerParents);
+  DynTypedNodeList getParents(ast_type_traits::TraversalKind TK,
+  const ast_type_traits::DynTypedNode ) {
+if (Node.getNodeKind().hasPointerIdentity()) {
+  auto ParentList =
+  getDynNodeFromMap(Node.getMemoizationData(), PointerParents);
+  if (ParentList.size() == 1 &&
+  TK == ast_type_traits::TK_IgnoreUnlessSpelledInSource) {
+const auto *E = ParentList[0].get();
+const auto *Child = Node.get();
+if (E && Child)
+  return AscendIgnoreUnlessSpelledInSource(E, Child);
+  }
+  return ParentList;
+}
 return getDynNodeFromMap(Node, OtherParents);
   }
+
+  ast_type_traits::DynTypedNode
+  AscendIgnoreUnlessSpelledInSource(const Expr *E, const Expr *Child) {
+
+auto ShouldSkip = [](const Expr *E, const Expr *Child) {
+  if (isa(E))
+return true;
+
+  if (isa(E))
+return true;
+
+  if (isa(E))
+return true;
+
+  if (isa(E))
+return true;
+
+  if (isa(E))
+return true;
+
+  if (isa(E))
+return true;
+
+  auto SR = Child->getSourceRange();
+
+  if (const auto *C = dyn_cast(E)) {
+if (C->getSourceRange() == SR || !isa(C))
+  return true;
+  }
+
+  if (const auto *C = dyn_cast(E)) {
+if (C->getSourceRange() == SR)
+  return true;
+  }
+
+  if (const auto *C = dyn_cast(E)) {
+if (C->getSourceRange() == SR)
+  return true;
+  }
+  return false;
+};
+
+while (ShouldSkip(E, Child)) {
+  auto It = PointerParents.find(E);
+  if (It == PointerParents.end())
+break;
+  const auto *S = It->second.dyn_cast();
+  if (!S)
+return getSingleDynTypedNodeFromParentMap(It->second);
+  const auto *P = dyn_cast(S);
+  if (!P)
+return ast_type_traits::DynTypedNode::create(*S);
+  Child = E;
+  E = P;
+}
+return ast_type_traits::DynTypedNode::create(*E);
+  }
 };
 
 /// Template specializations to abstract away from pointers and TypeLocs.
@@ -151,8 +219,7 @@
 class ParentMapContext::ParentMap::ASTVisitor
 : public RecursiveASTVisitor {
 public:
-  ASTVisitor(ParentMap , ParentMapContext )
-  : Map(Map), MapCtx(MapCtx) {}
+  ASTVisitor(ParentMap ) : Map(Map) {}
 
 private:
   friend class RecursiveASTVisitor;
@@ -222,11 +289,8 @@
   }
 
   bool TraverseStmt(Stmt *StmtNode) {
-Stmt *FilteredNode = StmtNode;
-if (auto *ExprNode = dyn_cast_or_null(FilteredNode))
-  FilteredNode = MapCtx.traverseIgnored(ExprNode);
-return TraverseNode(FilteredNode, FilteredNode,
-[&] { return VisitorBase::TraverseStmt(FilteredNode); },
+return TraverseNode(StmtNode, StmtNode,
+[&] { return VisitorBase::TraverseStmt(StmtNode); },
 );
   }
 
@@ -245,21 +309,18 @@
   }
 
   ParentMap 
-  ParentMapContext 
   llvm::SmallVector ParentStack;
 };
 
 ParentMapContext::ParentMap::ParentMap(ASTContext ) {
-  ASTVisitor(*this, Ctx.getParentMapContext()).TraverseAST(Ctx);
+  ASTVisitor(*this).TraverseAST(Ctx);
 }
 
 DynTypedNodeList
 ParentMapContext::getParents(const ast_type_traits::DynTypedNode ) {
-  std::unique_ptr  = Parents[Traversal];
-  if (!P)
+  if (!Parents)
 // We build the parent map for the traversal scope (usually whole TU), as
 // hasAncestor can escape any subtree.
-P = std::make_unique(ASTCtx);
-  return P->getParents(Node);
+Parents = std::make_unique(ASTCtx);
+  return 

[PATCH] D71313: [AST] Split parent map traversal logic into ParentMapContext.h

[Stephen Kelly via Phabricator via cfe-commits]

steveire added a comment.

Should this new class have some tests?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71313



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

[PATCH] D73307: Unique Names for Functions with Internal Linkage

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

The code change seems fine, but the test requires some changes. I haven't 
followed Propeller development, but hope someone with profile experience can 
confirm InternalLinkage is the only linkage we need to care about (otherwise 
the option will be a misnomer if we ever extend it) and check whether this 
feature is useful on its own. Does it improve profile precision?




Comment at: clang/test/CodeGen/unique_internal_funcnames.c:1
+// REQUIRES: x86-registered-target
+

The convention is file-name.c , rather than file_name.c . The latter exists, 
but is a minority.



Comment at: clang/test/CodeGen/unique_internal_funcnames.c:2
+// REQUIRES: x86-registered-target
+
+// RUN: %clang -target x86_64-pc-linux-gnu -S -o - %s | FileCheck %s 
--check-prefix=PLAIN

There should be two tests:

* one in test/Driver/funique-internal-funcnames.c, for 
`-funique-internal-funcnames -fno-unique-internal-funcnames` testing
* the other one should stay here (test/CodeGen), but should be a -S -emit-llvm 
test for IR output.

We don't normally do driver --> assembly testing.



Comment at: clang/test/CodeGen/unique_internal_funcnames.c:3
+
+// RUN: %clang -target x86_64-pc-linux-gnu -S -o - %s | FileCheck %s 
--check-prefix=PLAIN
+// RUN: %clang -target x86_64-pc-linux-gnu -S -funique-internal-funcnames 
-fno-unique-internal-funcnames -o - %s | FileCheck %s --check-prefix=PLAIN

`-target x86_64` should just work (generic ELF). This feature is not tied to 
linux-gnu.


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

https://reviews.llvm.org/D73307



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

[PATCH] D73376: [analyzer] Add FuchsiaLockChecker and C11LockChecker

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: clang/test/Analysis/c11lock.c:1
+// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.unix.C11Lock -verify %s
+

NoQ wrote:
> I wouldn't mind `alpha.core` given that these functions belong to the 
> standard library.
(i.e., i mean put the checker into `alpha.core`)
(but, yeah, also please add `core` to the run-line)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73376



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

[PATCH] D73385: [Sema] Split availability processing into SemaAvailability.cpp

[pre-merge checks [bot] via Phabricator via cfe-commits]

merge_guards_bot added a comment.

{icon times-circle color=red} Unit tests: fail. 62189 tests passed, 1 failed 
and 815 were skipped.

  failed: 
libc++.std/thread/thread_mutex/thread_mutex_requirements/thread_mutex_requirements_mutex/thread_mutex_class/try_lock.pass.cpp

{icon times-circle color=red} clang-tidy: fail. clang-tidy found 0 errors and 3 
warnings 
.
 0 of them are added as review comments below (why? 
).

{icon times-circle color=red} clang-format: fail. Please format your changes 
with clang-format by running `git-clang-format HEAD^` or applying this patch 
.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 


//Pre-merge checks is in beta. Report issue 
.
 Please join beta  or enable 
it for your project 
.//


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73385



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

[PATCH] D73376: [analyzer] Add FuchsiaLockChecker and C11LockChecker

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Yay, C11 as well!!

> If you have any idea how to reduce this boilerplate I'd like to know :)

I don't see any immediate solutions to the boilerplate that don't consist in 
introducing better checker APIs. Eg., we could have introduced a `LazyBugType` 
- a wrapper around `Optional` that'd take description immediately but 
create the actual bug type only once the checker name is known, or we could 
make a `MultiCallDescriptionMap` that'd prevent us from both splitting up the 
map and writing down an extra piece of information on every line. But these are 
big and very much non-K.I.S.S. endeavors.

> I had to add a dummy check that can always be enabled. Otherwise, 
> `CheckerManager::getChecker` would trigger an assertion fail due to 
> attempting to get a checker that was potentially not enabled.

You mean `PthreadLockBase`? I think that's how @Szelethus intended to have such 
checker hierarchies to work.

> I plan to add more tests

Yes please! :D




Comment at: clang/test/Analysis/c11lock.c:1
+// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.unix.C11Lock -verify %s
+

I wouldn't mind `alpha.core` given that these functions belong to the standard 
library.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73376



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

[PATCH] D73307: Unique Names for Functions with Internal Linkage

[Sriraman Tallam via Phabricator via cfe-commits]

tmsriram updated this revision to Diff 240312.
tmsriram added a comment.

Minor changes.  Remove CC1option on "-fno", Remove "$" from unique suffix.


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

https://reviews.llvm.org/D73307

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/unique_internal_funcnames.c

Index: clang/test/CodeGen/unique_internal_funcnames.c
===
--- /dev/null
+++ clang/test/CodeGen/unique_internal_funcnames.c
@@ -0,0 +1,17 @@
+// REQUIRES: x86-registered-target
+
+// RUN: %clang -target x86_64-pc-linux-gnu -S -o - %s | FileCheck %s --check-prefix=PLAIN
+// RUN: %clang -target x86_64-pc-linux-gnu -S -funique-internal-funcnames -fno-unique-internal-funcnames -o - %s | FileCheck %s --check-prefix=PLAIN
+// RUN: %clang -target x86_64-pc-linux-gnu -S -funique-internal-funcnames -o -  %s | FileCheck %s --check-prefix=UNIQUE
+
+static int foo() {
+  return 0;
+}
+
+int (*bar())() {
+  return foo;
+}
+
+// PLAIN: foo:
+// UNIQUE-NOT: foo:
+// UNIQUE: foo.{{[0-9a-f]+}}:
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -964,6 +964,8 @@
   Opts.UniqueSectionNames = Args.hasFlag(OPT_funique_section_names,
  OPT_fno_unique_section_names, true);
 
+  Opts.UniqueInternalFuncNames = Args.hasArg(OPT_funique_internal_funcnames);
+
   Opts.MergeFunctions = Args.hasArg(OPT_fmerge_functions);
 
   Opts.NoUseJumpTables = Args.hasArg(OPT_fno_jump_tables);
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4153,6 +4153,8 @@
 options::OPT_fno_function_sections,
 options::OPT_fdata_sections,
 options::OPT_fno_data_sections,
+options::OPT_funique_internal_funcnames,
+options::OPT_fno_unique_internal_funcnames,
 options::OPT_funique_section_names,
 options::OPT_fno_unique_section_names,
 options::OPT_mrestrict_it,
@@ -4695,6 +4697,10 @@
 options::OPT_fno_unique_section_names, true))
 CmdArgs.push_back("-fno-unique-section-names");
 
+  if (Args.hasFlag(options::OPT_funique_internal_funcnames,
+   options::OPT_fno_unique_internal_funcnames, false))
+CmdArgs.push_back("-funique-internal-funcnames");
+
   Args.AddLastArg(CmdArgs, options::OPT_finstrument_functions,
   options::OPT_finstrument_functions_after_inlining,
   options::OPT_finstrument_function_entry_bare);
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -1101,6 +1101,23 @@
   const auto *ND = cast(GD.getDecl());
   std::string MangledName = getMangledNameImpl(*this, GD, ND);
 
+  // With option -funique-internal-funcnames, functions with internal linkage
+  // should get unique names.  Use "getUniqueModuleId" to get a unique
+  // identifier and this is a best effort.
+  if (getCodeGenOpts().UniqueInternalFuncNames &&
+  dyn_cast(GD.getDecl()) &&
+  getFunctionLinkage(GD) == llvm::GlobalValue::InternalLinkage &&
+  !getModule().getSourceFileName().empty()) {
+llvm::MD5 Md5;
+Md5.update(getModule().getSourceFileName());
+llvm::MD5::MD5Result R;
+Md5.final(R);
+SmallString<32> Str;
+llvm::MD5::stringifyResult(R, Str);
+std::string UniqueSuffix = ("." + Str).str();
+MangledName += UniqueSuffix;
+  }
+
   // Adjust kernel stub mangling as we may need to be able to differentiate
   // them from the kernel itself (e.g., for HIP).
   if (auto *FD = dyn_cast(GD.getDecl()))
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -1952,6 +1952,12 @@
 def fno_unique_section_names : Flag <["-"], "fno-unique-section-names">,
   Group, Flags<[CC1Option]>;
 
+def funique_internal_funcnames : Flag <["-"], "funique-internal-funcnames">,
+  Group, Flags<[CC1Option]>,
+  HelpText<"Uniqueify Internal Linkage Function Names">;
+def fno_unique_internal_funcnames : Flag <["-"], "fno-unique-internal-funcnames">,
+  Group;
+
 def fstrict_return : Flag<["-"], "fstrict-return">, Group,
   Flags<[CC1Option]>,
   HelpText<"Always treat control flow paths that fall off the end of a "
Index: clang/include/clang/Basic/CodeGenOptions.def
===
--- 

[PATCH] D73151: [analyzer] Fix handle leak false positive when the handle dies too early

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun marked 2 inline comments as done.
xazax.hun added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp:133-135
+  static HandleState getWithoutError(HandleState S) {
+return HandleState(S.K, nullptr);
+  }

NoQ wrote:
> It already makes me mildly uncomfortable that our data is not "normalized", 
> i.e. you can indicate the Schrödinger state by either adding an error symbol 
> or changing from `Allocated` to `MaybeAllocated`. Do i understand it 
> correctly that you're now adding a special state where the handle is still 
> `MaybeAllocated` but there's no error symbol? Please comment this up.
Yeah, the reason is that, when the handle is a return value, we have no idea 
where the error symbol is. This would only happen if someone do not follow the 
API guidelines (in Fuchsia). I'll add a comment.



Comment at: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp:415
+if (CurItem.second.getErrorSym())
+  SymReaper.markLive(CurItem.first);
+  }

NoQ wrote:
> I think it should be possible to implement it without `checkLiveSymbols`, 
> because i don't have any reasons to believe that you can find the handle 
> symbol by looking at the error symbol, or vice versa, so i think they aren't 
> supposed to keep each other alive.
> 
> I.e., i think you can implement this by intentionally leaving zombie handles 
> in your state until the respective error symbol dies. After all, you don't 
> rely on any other metadata attached to your handles (eg., range constraints), 
> you only need your map, right?
> 
> I'm open to discussion here. I understand that intentional zombies sound 
> scary, but they also look slightly more correct to me. //"The handle is 
> definitely dead by now, but the successfulness of its birth is still a 'known 
> unknown', so let's delay our warning and keep the corpse frozen until we find 
> out if it was ever born to begin with"// - sounds about right :/ It's 
> probably not a big deal in any case, but i'm curious WDYT.
I see, yeah this makes sense to me. I will try this approach out and report 
back :)


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

https://reviews.llvm.org/D73151



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

[PATCH] D72675: [Clang][Driver] Fix -ffast-math/-ffp-contract interaction

[Warren Ristow via Phabricator via cfe-commits]

wristow updated this revision to Diff 240305.
wristow added a comment.

Update a comment to remove the discussion about enabling the `__FAST_MATH__` 
preprocessor macro.  The handling of the setting of `__FAST_MATH__` is done in 
"clang/lib/Frontend/InitPreprocessor.cpp", and once we decide on the set of 
conditions that enable that definition, we can add an appropriate comment there.


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

https://reviews.llvm.org/D72675

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/fast-math.c


Index: clang/test/Driver/fast-math.c
===
--- clang/test/Driver/fast-math.c
+++ clang/test/Driver/fast-math.c
@@ -180,6 +180,21 @@
 // CHECK-FAST-MATH: "-ffast-math"
 // CHECK-FAST-MATH: "-ffinite-math-only"
 //
+// -ffp-contract=off and -ffp-contract=on must disable the fast-math umbrella,
+// and the unsafe-fp-math umbrella (-ffp-contract=fast leaves them enabled).
+// RUN: %clang -### -ffast-math -ffp-contract=off -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-NO-FAST-MATH %s
+// RUN: %clang -### -ffast-math -ffp-contract=off -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-NO-UNSAFE-MATH %s
+// RUN: %clang -### -ffast-math -ffp-contract=on -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-NO-FAST-MATH %s
+// RUN: %clang -### -ffast-math -ffp-contract=on -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-NO-UNSAFE-MATH %s
+// RUN: %clang -### -ffast-math -ffp-contract=fast -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FAST-MATH %s
+// RUN: %clang -### -ffast-math -ffp-contract=fast -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-UNSAFE-MATH %s
+//
 // RUN: %clang -### -ffast-math -fno-fast-math -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-NO-FAST-MATH %s
 // RUN: %clang -### -ffast-math -fno-finite-math-only -c %s 2>&1 \
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -2785,8 +2785,11 @@
   if (MathErrno)
 CmdArgs.push_back("-fmath-errno");
 
+  // If -ffp-contract=off/on has been specified on the command line, then we
+  // must suppress the emission of -ffast-math and -menable-unsafe-fp-math to
+  // cc1.
   if (!MathErrno && AssociativeMath && ReciprocalMath && !SignedZeros &&
-  !TrappingMath)
+  !TrappingMath && !(FPContract.equals("off") || FPContract.equals("on")))
 CmdArgs.push_back("-menable-unsafe-fp-math");
 
   if (!SignedZeros)
@@ -2831,12 +2834,17 @@
 
   ParseMRecip(D, Args, CmdArgs);
 
-  // -ffast-math enables the __FAST_MATH__ preprocessor macro, but check for 
the
-  // individual features enabled by -ffast-math instead of the option itself as
-  // that's consistent with gcc's behaviour.
+  // -ffast-math acts as an "umbrella", enabling a variety of individual
+  // floating-point transformations.  We check if the appropriate set of those
+  // transformations are enabled, and if they are, we pass that umbrella switch
+  // to cc1.  This also interacts with another "umbrella" switch, -ffp-model.
+  // We handle -ffp-contract somewhat specially here, to produce a warning in
+  // situations where -ffp-model=fast is overridden by the setting of
+  // -ffp-contract.
   if (!HonorINFs && !HonorNaNs && !MathErrno && AssociativeMath &&
   ReciprocalMath && !SignedZeros && !TrappingMath && !RoundingFPMath) {
-CmdArgs.push_back("-ffast-math");
+if (!(FPContract.equals("off") || FPContract.equals("on")))
+  CmdArgs.push_back("-ffast-math");
 if (FPModel.equals("fast")) {
   if (FPContract.equals("fast"))
 // All set, do nothing.


Index: clang/test/Driver/fast-math.c
===
--- clang/test/Driver/fast-math.c
+++ clang/test/Driver/fast-math.c
@@ -180,6 +180,21 @@
 // CHECK-FAST-MATH: "-ffast-math"
 // CHECK-FAST-MATH: "-ffinite-math-only"
 //
+// -ffp-contract=off and -ffp-contract=on must disable the fast-math umbrella,
+// and the unsafe-fp-math umbrella (-ffp-contract=fast leaves them enabled).
+// RUN: %clang -### -ffast-math -ffp-contract=off -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-NO-FAST-MATH %s
+// RUN: %clang -### -ffast-math -ffp-contract=off -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-NO-UNSAFE-MATH %s
+// RUN: %clang -### -ffast-math -ffp-contract=on -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-NO-FAST-MATH %s
+// RUN: %clang -### -ffast-math -ffp-contract=on -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-NO-UNSAFE-MATH %s
+// RUN: %clang -### -ffast-math -ffp-contract=fast -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FAST-MATH %s
+// RUN: %clang -### -ffast-math -ffp-contract=fast -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-UNSAFE-MATH %s
+//
 // RUN: %clang -### 

[PATCH] D73385: [Sema] Split availability processing into SemaAvailability.cpp

[Reid Kleckner via Phabricator via cfe-commits]

rnk created this revision.
rnk added reviewers: arphaman, aaron.ballman, rsmith.
Herald added subscribers: dexonsmith, mgorny.
Herald added a project: clang.

Reduces compile time of SemaDeclAttr.cpp down to 28s from 50s. The new
TU does a few RecursiveASTVisitor instantiations, so it takes 30s.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D73385

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/CMakeLists.txt
  clang/lib/Sema/SemaAvailability.cpp
  clang/lib/Sema/SemaDeclAttr.cpp

Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -,534 +,6 @@
   DD.Triggered = true;
 }
 
-static const AvailabilityAttr *getAttrForPlatform(ASTContext ,
-  const Decl *D) {
-  // Check each AvailabilityAttr to find the one for this platform.
-  for (const auto *A : D->attrs()) {
-if (const auto *Avail = dyn_cast(A)) {
-  // FIXME: this is copied from CheckAvailability. We should try to
-  // de-duplicate.
-
-  // Check if this is an App Extension "platform", and if so chop off
-  // the suffix for matching with the actual platform.
-  StringRef ActualPlatform = Avail->getPlatform()->getName();
-  StringRef RealizedPlatform = ActualPlatform;
-  if (Context.getLangOpts().AppExt) {
-size_t suffix = RealizedPlatform.rfind("_app_extension");
-if (suffix != StringRef::npos)
-  RealizedPlatform = RealizedPlatform.slice(0, suffix);
-  }
-
-  StringRef TargetPlatform = Context.getTargetInfo().getPlatformName();
-
-  // Match the platform name.
-  if (RealizedPlatform == TargetPlatform)
-return Avail;
-}
-  }
-  return nullptr;
-}
-
-/// The diagnostic we should emit for \c D, and the declaration that
-/// originated it, or \c AR_Available.
-///
-/// \param D The declaration to check.
-/// \param Message If non-null, this will be populated with the message from
-/// the availability attribute that is selected.
-/// \param ClassReceiver If we're checking the the method of a class message
-/// send, the class. Otherwise nullptr.
-static std::pair
-ShouldDiagnoseAvailabilityOfDecl(Sema , const NamedDecl *D,
- std::string *Message,
- ObjCInterfaceDecl *ClassReceiver) {
-  AvailabilityResult Result = D->getAvailability(Message);
-
-  // For typedefs, if the typedef declaration appears available look
-  // to the underlying type to see if it is more restrictive.
-  while (const auto *TD = dyn_cast(D)) {
-if (Result == AR_Available) {
-  if (const auto *TT = TD->getUnderlyingType()->getAs()) {
-D = TT->getDecl();
-Result = D->getAvailability(Message);
-continue;
-  }
-}
-break;
-  }
-
-  // Forward class declarations get their attributes from their definition.
-  if (const auto *IDecl = dyn_cast(D)) {
-if (IDecl->getDefinition()) {
-  D = IDecl->getDefinition();
-  Result = D->getAvailability(Message);
-}
-  }
-
-  if (const auto *ECD = dyn_cast(D))
-if (Result == AR_Available) {
-  const DeclContext *DC = ECD->getDeclContext();
-  if (const auto *TheEnumDecl = dyn_cast(DC)) {
-Result = TheEnumDecl->getAvailability(Message);
-D = TheEnumDecl;
-  }
-}
-
-  // For +new, infer availability from -init.
-  if (const auto *MD = dyn_cast(D)) {
-if (S.NSAPIObj && ClassReceiver) {
-  ObjCMethodDecl *Init = ClassReceiver->lookupInstanceMethod(
-  S.NSAPIObj->getInitSelector());
-  if (Init && Result == AR_Available && MD->isClassMethod() &&
-  MD->getSelector() == S.NSAPIObj->getNewSelector() &&
-  MD->definedInNSObject(S.getASTContext())) {
-Result = Init->getAvailability(Message);
-D = Init;
-  }
-}
-  }
-
-  return {Result, D};
-}
-
-
-/// whether we should emit a diagnostic for \c K and \c DeclVersion in
-/// the context of \c Ctx. For example, we should emit an unavailable diagnostic
-/// in a deprecated context, but not the other way around.
-static bool
-ShouldDiagnoseAvailabilityInContext(Sema , AvailabilityResult K,
-VersionTuple DeclVersion, Decl *Ctx,
-const NamedDecl *OffendingDecl) {
-  assert(K != AR_Available && "Expected an unavailable declaration here!");
-
-  // Checks if we should emit the availability diagnostic in the context of C.
-  auto CheckContext = [&](const Decl *C) {
-if (K == AR_NotYetIntroduced) {
-  if (const AvailabilityAttr *AA = getAttrForPlatform(S.Context, C))
-if (AA->getIntroduced() >= DeclVersion)
-  return true;
-} else if (K == AR_Deprecated) {
-  if (C->isDeprecated())
-return true;
-} else if (K == AR_Unavailable) {
-  // It is perfectly fine to refer to an 'unavailable' 

[PATCH] D72675: [Clang][Driver] Fix -ffast-math/-ffp-contract interaction

[Warren Ristow via Phabricator via cfe-commits]

wristow added a comment.

In D72675#1839662 , @andrew.w.kaylor 
wrote:

> In D72675#1839492 , @wristow wrote:
>
> > 1. Should we enable FMA "by default" at (for example) '-O2'?
>
>
> We recently introduced a new option "-ffp-model=[precise|fast|strict], which 
> is supposed to serve as an umbrella option for the most common combination of 
> options. I think our default behavior should be equivalent to 
> -ffp-model=precise, which enables contraction. It currently seems to enable 
> -ffp-contract=fast in the precise model (https://godbolt.org/z/c6w8mJ). I'm 
> not sure that's what it should be doing, but whatever the precise model does 
> should be our default.


That makes sense to me.  So `-ffp-model`/`-ffp-contract` interaction should be 
examined, and possibly adjusted.  If an adjustment is needed, I think it makes 
sense to handle that interaction separately.

I'm going to update this patch to adjust the comment that @spatel and I 
discussed earlier , so it no longer 
implies that `__FAST_MATH__` is defined only when "all of" `-ffast-math` is 
enabled.


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

https://reviews.llvm.org/D72675



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

[PATCH] D73151: [analyzer] Fix handle leak false positive when the handle dies too early

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp:133-135
+  static HandleState getWithoutError(HandleState S) {
+return HandleState(S.K, nullptr);
+  }

It already makes me mildly uncomfortable that our data is not "normalized", 
i.e. you can indicate the Schrödinger state by either adding an error symbol or 
changing from `Allocated` to `MaybeAllocated`. Do i understand it correctly 
that you're now adding a special state where the handle is still 
`MaybeAllocated` but there's no error symbol? Please comment this up.



Comment at: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp:415
+if (CurItem.second.getErrorSym())
+  SymReaper.markLive(CurItem.first);
+  }

I think it should be possible to implement it without `checkLiveSymbols`, 
because i don't have any reasons to believe that you can find the handle symbol 
by looking at the error symbol, or vice versa, so i think they aren't supposed 
to keep each other alive.

I.e., i think you can implement this by intentionally leaving zombie handles in 
your state until the respective error symbol dies. After all, you don't rely on 
any other metadata attached to your handles (eg., range constraints), you only 
need your map, right?

I'm open to discussion here. I understand that intentional zombies sound scary, 
but they also look slightly more correct to me. //"The handle is definitely 
dead by now, but the successfulness of its birth is still a 'known unknown', so 
let's delay our warning and keep the corpse frozen until we find out if it was 
ever born to begin with"// - sounds about right :/ It's probably not a big deal 
in any case, but i'm curious WDYT.



Comment at: clang/test/Analysis/fuchsia_handle.cpp:69
+  zx_status_t status = zx_channel_create(0, , );
+  if (status < 0)
+return;

I'd like to see how do notes look when the warning *is* emitted (i.e., same 
test but replace `status < 0` with `status >= 0`).

We don't have a note for the collapse point of the Schrödinger state, right? (i 
think it was an unfortunate inevitable use case for a bug visitor because you 
can't have tags in `evalAssume`)

That is, how comfortable would it be for the user to see that the leak warning 
is emitted significantly later than the handle is dead? We already aren't great 
at positioning our leak warnings, but it makes it even more unobvious. I guess 
it's probably fine, but i want to see it.


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

https://reviews.llvm.org/D73151



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

[PATCH] D73237: [CUDA] Fix order of memcpy arguments in __shfl_*(<64-bit type>).

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

In D73237#1839216 , @hans wrote:

> In D73237#1837077 , @tra wrote:
>
> > Landed in 
> > https://github.com/llvm/llvm-project/commit/cc14de88da27a8178976972bdc8211c31f7ca9ae
> >  @hans -- can we cherry-pick it into 10?
>
>
> Yes, go ahead and "git cherry-pick -x" it and push to the branch.


Done: 
https://github.com/llvm/llvm-project/commit/0df13627c6a4006de39e5f01d81a338793b0e82b


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73237



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

[clang] 1e487e4 - clang: Only define OBJC_NEW_PROPERTIES when -x objective-c

[Duncan P. N. Exon Smith via cfe-commits]

Author: Duncan P. N. Exon Smith
Date: 2020-01-24T14:55:12-08:00
New Revision: 1e487e4c16821b6de3d651f618274df90bd3fad9

URL: 
https://github.com/llvm/llvm-project/commit/1e487e4c16821b6de3d651f618274df90bd3fad9
DIFF: 
https://github.com/llvm/llvm-project/commit/1e487e4c16821b6de3d651f618274df90bd3fad9.diff

LOG: clang: Only define OBJC_NEW_PROPERTIES when -x objective-c

Since 2009 (in r63846) we've been `#define`-ing OBJC_NEW_PROPERTIES all
the time on Darwin, but this macro only makes sense for `-x objective-c`
and `-x objective-c++`.  Restrict it to those cases (for which there is
already separate logic).

https://reviews.llvm.org/D72970
rdar://problem/10050342

Added: 


Modified: 
clang/lib/Basic/Targets/OSTargets.cpp
clang/test/Preprocessor/init.c

Removed: 




diff  --git a/clang/lib/Basic/Targets/OSTargets.cpp 
b/clang/lib/Basic/Targets/OSTargets.cpp
index d4c64ba8..187944146bde 100644
--- a/clang/lib/Basic/Targets/OSTargets.cpp
+++ b/clang/lib/Basic/Targets/OSTargets.cpp
@@ -25,7 +25,7 @@ void getDarwinDefines(MacroBuilder , const 
LangOptions ,
   Builder.defineMacro("__APPLE_CC__", "6000");
   Builder.defineMacro("__APPLE__");
   Builder.defineMacro("__STDC_NO_THREADS__");
-  Builder.defineMacro("OBJC_NEW_PROPERTIES");
+
   // AddressSanitizer doesn't play well with source fortification, which is on
   // by default on Darwin.
   if (Opts.Sanitize.has(SanitizerKind::Address))

diff  --git a/clang/test/Preprocessor/init.c b/clang/test/Preprocessor/init.c
index e25946304d0f..6c9508e5c634 100644
--- a/clang/test/Preprocessor/init.c
+++ b/clang/test/Preprocessor/init.c
@@ -187,6 +187,7 @@
 //
 //
 // RUN: %clang_cc1 -x objective-c -E -dM < /dev/null | FileCheck 
-match-full-lines -check-prefix OBJC %s
+// RUN: %clang_cc1 -x objective-c++ -E -dM < /dev/null | FileCheck 
-match-full-lines -check-prefix OBJC %s
 //
 // OBJC:#define OBJC_NEW_PROPERTIES 1
 // OBJC:#define __NEXT_RUNTIME__ 1
@@ -9272,6 +9273,7 @@
 // RUN:   -fgnuc-version=4.2.1 -triple=aarch64-apple-macosx10.12 < /dev/null \
 // RUN: | FileCheck -check-prefix=DARWIN %s
 
+// DARWIN-NOT: OBJC_NEW_PROPERTIES
 // DARWIN:#define __STDC_NO_THREADS__ 1
 
 // RUN: %clang_cc1 -triple i386-apple-macosx -ffreestanding -dM -E /dev/null 
-o - | FileCheck -match-full-lines -check-prefix MACOS-32 %s



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

[PATCH] D73307: Unique Names for Functions with Internal Linkage

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/include/clang/Driver/Options.td:1959
+def fno_unique_internal_funcnames : Flag <["-"], 
"fno-unique-internal-funcnames">,
+  Group, Flags<[CC1Option]>;
+

`, Flags<[CC1Option]>` can be deleted.



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:1109
+  dyn_cast(GD.getDecl()) &&
+  this->getFunctionLinkage(GD) == llvm::GlobalValue::InternalLinkage &&
+  !getModule().getSourceFileName().empty()) {

Is `this->` a must?



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:1117
+llvm::MD5::stringifyResult(R, Str);
+std::string UniqueSuffix = (".$" + Str).str();
+MangledName = MangledName + UniqueSuffix;

tmsriram wrote:
> pcc wrote:
> > Why `".$"` and not just `"."`?
> I just wanted to keep it consistent with what getUniqueModuleId does with the 
> Md5 hash.  I can remove it.
Agreed. `"."` seems sufficient.



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:1118
+std::string UniqueSuffix = (".$" + Str).str();
+MangledName = MangledName + UniqueSuffix;
+  }

`MangledName += UniqueSuffix;`



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:966
  OPT_fno_unique_section_names, true);
+  Opts.UniqueInternalFuncNames = Args.hasFlag(
+  OPT_funique_internal_funcnames, OPT_fno_unique_internal_funcnames, 
false);

tmsriram wrote:
> MaskRay wrote:
> > Just `Args.hasArg(OPT_funique_internal_funcnames)`.
> > 
> > `-fno-*` is handled in clangDriver. CC1 does not need to know `-fno-*` if 
> > it defaults to false.
> Are you suggesting I remove the -fno-* flag? The -fno-* is useful  if 
> -funique-internal-funcnames came added to a Makefile ,for example, and this 
> needs to be disabled.  This is similar to say -ffunction-sections.  Please 
> clarify. Thanks.
This file is about the cc1 option, which is not expected to be used by a user.

The driver has handled -fno-, so we don't have to repeat it here. 
-fmerge-functions and -fno-jump-tables are good examples that we don't have to 
add both -ffoo and -fno-foo to cc1 options.


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

https://reviews.llvm.org/D73307



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

[PATCH] D72970: clang: Only define OBJC_NEW_PROPERTIES when -x objective-c

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith closed this revision.
dexonsmith added a comment.

Thanks, committed in 1e487e4c16821b6de3d651f618274df90bd3fad9 
.


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

https://reviews.llvm.org/D72970



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

[PATCH] D73307: Unique Names for Functions with Internal Linkage

[Sriraman Tallam via Phabricator via cfe-commits]

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



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:1117
+llvm::MD5::stringifyResult(R, Str);
+std::string UniqueSuffix = (".$" + Str).str();
+MangledName = MangledName + UniqueSuffix;

pcc wrote:
> Why `".$"` and not just `"."`?
I just wanted to keep it consistent with what getUniqueModuleId does with the 
Md5 hash.  I can remove it.


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

https://reviews.llvm.org/D73307



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

[PATCH] D73307: Unique Names for Functions with Internal Linkage

[Peter Collingbourne via Phabricator via cfe-commits]

pcc added inline comments.



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:1117
+llvm::MD5::stringifyResult(R, Str);
+std::string UniqueSuffix = (".$" + Str).str();
+MangledName = MangledName + UniqueSuffix;

Why `".$"` and not just `"."`?


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

https://reviews.llvm.org/D73307



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

[PATCH] D73322: [WebAssembly] Update bleeding-edge CPU features

[Heejin Ahn via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG65eb11306e92: [WebAssembly] Update bleeding-edge CPU 
features (authored by aheejin).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73322

Files:
  clang/lib/Basic/Targets/WebAssembly.cpp
  clang/test/Preprocessor/wasm-target-features.c
  llvm/lib/Target/WebAssembly/WebAssembly.td
  llvm/test/CodeGen/WebAssembly/target-features.ll


Index: llvm/test/CodeGen/WebAssembly/target-features.ll
===
--- llvm/test/CodeGen/WebAssembly/target-features.ll
+++ llvm/test/CodeGen/WebAssembly/target-features.ll
@@ -83,13 +83,16 @@
 ; SIMD128-NEXT: .int8 7
 ; SIMD128-NEXT: .ascii "simd128"
 
-; +atomics, +mutable-globals, +nontrapping-fptoint, +reference-types, 
+sign-ext,
-; +simd128
-; BLEEDING-EDGE-NEXT: .int8   6
+; +atomics, +bulk-memory, +mutable-globals, +nontrapping-fptoint,
+; +reference-types, +sign-ext, +simd128, +tail-call
+; BLEEDING-EDGE-NEXT: .int8   8
 ; BLEEDING-EDGE-NEXT: .int8   43
 ; BLEEDING-EDGE-NEXT: .int8   7
 ; BLEEDING-EDGE-NEXT: .ascii  "atomics"
 ; BLEEDING-EDGE-NEXT: .int8   43
+; BLEEDING-EDGE-NEXT: .int8   11
+; BLEEDING-EDGE-NEXT: .ascii  "bulk-memory"
+; BLEEDING-EDGE-NEXT: .int8   43
 ; BLEEDING-EDGE-NEXT: .int8   15
 ; BLEEDING-EDGE-NEXT: .ascii  "mutable-globals"
 ; BLEEDING-EDGE-NEXT: .int8   43
@@ -104,5 +107,8 @@
 ; BLEEDING-EDGE-NEXT: .int8   43
 ; BLEEDING-EDGE-NEXT: .int8   7
 ; BLEEDING-EDGE-NEXT: .ascii  "simd128"
+; BLEEDING-EDGE-NEXT: .int8   43
+; BLEEDING-EDGE-NEXT: .int8   9
+; BLEEDING-EDGE-NEXT: .ascii  "tail-call"
 
 ; CHECK-NEXT: .text
Index: llvm/lib/Target/WebAssembly/WebAssembly.td
===
--- llvm/lib/Target/WebAssembly/WebAssembly.td
+++ llvm/lib/Target/WebAssembly/WebAssembly.td
@@ -102,7 +102,8 @@
 def : ProcessorModel<"bleeding-edge", NoSchedModel,
   [FeatureSIMD128, FeatureAtomics,
FeatureNontrappingFPToInt, FeatureSignExt,
-   FeatureMutableGlobals]>;
+   FeatureMutableGlobals, FeatureBulkMemory,
+   FeatureTailCall]>;
 
 
//===--===//
 // Target Declaration
Index: clang/test/Preprocessor/wasm-target-features.c
===
--- clang/test/Preprocessor/wasm-target-features.c
+++ clang/test/Preprocessor/wasm-target-features.c
@@ -134,12 +134,14 @@
 //
 // BLEEDING-EDGE-DAG:#define __wasm_nontrapping_fptoint__ 1{{$}}
 // BLEEDING-EDGE-DAG:#define __wasm_sign_ext__ 1{{$}}
+// BLEEDING-EDGE-DAG:#define __wasm_bulk_memory__ 1{{$}}
 // BLEEDING-EDGE-DAG:#define __wasm_simd128__ 1{{$}}
 // BLEEDING-EDGE-DAG:#define __wasm_atomics__ 1{{$}}
 // BLEEDING-EDGE-DAG:#define __wasm_mutable_globals__ 1{{$}}
+// BLEEDING-EDGE-DAG:#define __wasm_tail_call__ 1{{$}}
 // BLEEDING-EDGE-NOT:#define __wasm_unimplemented_simd128__ 1{{$}}
+// BLEEDING-EDGE-NOT:#define __wasm_exception_handling__ 1{{$}}
 // BLEEDING-EDGE-NOT:#define __wasm_multivalue__ 1{{$}}
-// BLEEDING-EDGE-NOT:#define __wasm_tail_call__ 1{{$}}
 // BLEEDING-EDGE-NOT:#define __wasm_reference_types__ 1{{$}}
 
 // RUN: %clang -E -dM %s -o - 2>&1 \
Index: clang/lib/Basic/Targets/WebAssembly.cpp
===
--- clang/lib/Basic/Targets/WebAssembly.cpp
+++ clang/lib/Basic/Targets/WebAssembly.cpp
@@ -105,8 +105,10 @@
   if (CPU == "bleeding-edge") {
 Features["nontrapping-fptoint"] = true;
 Features["sign-ext"] = true;
+Features["bulk-memory"] = true;
 Features["atomics"] = true;
 Features["mutable-globals"] = true;
+Features["tail-call"] = true;
 setSIMDLevel(Features, SIMD128);
   }
   // Other targets do not consider user-configured features here, but while we


Index: llvm/test/CodeGen/WebAssembly/target-features.ll
===
--- llvm/test/CodeGen/WebAssembly/target-features.ll
+++ llvm/test/CodeGen/WebAssembly/target-features.ll
@@ -83,13 +83,16 @@
 ; SIMD128-NEXT: .int8 7
 ; SIMD128-NEXT: .ascii "simd128"
 
-; +atomics, +mutable-globals, +nontrapping-fptoint, +reference-types, +sign-ext,
-; +simd128
-; BLEEDING-EDGE-NEXT: .int8   6
+; +atomics, +bulk-memory, +mutable-globals, +nontrapping-fptoint,
+; +reference-types, +sign-ext, +simd128, +tail-call
+; BLEEDING-EDGE-NEXT: .int8   8
 ; BLEEDING-EDGE-NEXT: .int8   43
 ; BLEEDING-EDGE-NEXT: .int8   7
 ; BLEEDING-EDGE-NEXT: .ascii  "atomics"
 ; BLEEDING-EDGE-NEXT: .int8   43
+; BLEEDING-EDGE-NEXT: .int8   11
+; BLEEDING-EDGE-NEXT: .ascii  "bulk-memory"
+; BLEEDING-EDGE-NEXT: .int8   43
 ; BLEEDING-EDGE-NEXT: .int8   15
 ; BLEEDING-EDGE-NEXT: .ascii  "mutable-globals"
 ; BLEEDING-EDGE-NEXT: .int8   43
@@ -104,5 

[clang] 65eb113 - [WebAssembly] Update bleeding-edge CPU features

[Heejin Ahn via cfe-commits]

Author: Heejin Ahn
Date: 2020-01-24T14:27:35-08:00
New Revision: 65eb11306e921bb0299100dfc61e79858f903c1b

URL: 
https://github.com/llvm/llvm-project/commit/65eb11306e921bb0299100dfc61e79858f903c1b
DIFF: 
https://github.com/llvm/llvm-project/commit/65eb11306e921bb0299100dfc61e79858f903c1b.diff

LOG: [WebAssembly] Update bleeding-edge CPU features

Summary:
This adds bulk memory and tail call to "bleeding-edge" CPU, since their
implementation in LLVM/clang seems mostly complete.

Reviewers: tlively

Subscribers: dschuff, sbc100, jgravelle-google, sunfish, cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang/lib/Basic/Targets/WebAssembly.cpp
clang/test/Preprocessor/wasm-target-features.c
llvm/lib/Target/WebAssembly/WebAssembly.td
llvm/test/CodeGen/WebAssembly/target-features.ll

Removed: 




diff  --git a/clang/lib/Basic/Targets/WebAssembly.cpp 
b/clang/lib/Basic/Targets/WebAssembly.cpp
index ca07697f6ac8..ef7936762152 100644
--- a/clang/lib/Basic/Targets/WebAssembly.cpp
+++ b/clang/lib/Basic/Targets/WebAssembly.cpp
@@ -105,8 +105,10 @@ bool WebAssemblyTargetInfo::initFeatureMap(
   if (CPU == "bleeding-edge") {
 Features["nontrapping-fptoint"] = true;
 Features["sign-ext"] = true;
+Features["bulk-memory"] = true;
 Features["atomics"] = true;
 Features["mutable-globals"] = true;
+Features["tail-call"] = true;
 setSIMDLevel(Features, SIMD128);
   }
   // Other targets do not consider user-configured features here, but while we

diff  --git a/clang/test/Preprocessor/wasm-target-features.c 
b/clang/test/Preprocessor/wasm-target-features.c
index 45f60e260456..05b4bb49d73b 100644
--- a/clang/test/Preprocessor/wasm-target-features.c
+++ b/clang/test/Preprocessor/wasm-target-features.c
@@ -134,12 +134,14 @@
 //
 // BLEEDING-EDGE-DAG:#define __wasm_nontrapping_fptoint__ 1{{$}}
 // BLEEDING-EDGE-DAG:#define __wasm_sign_ext__ 1{{$}}
+// BLEEDING-EDGE-DAG:#define __wasm_bulk_memory__ 1{{$}}
 // BLEEDING-EDGE-DAG:#define __wasm_simd128__ 1{{$}}
 // BLEEDING-EDGE-DAG:#define __wasm_atomics__ 1{{$}}
 // BLEEDING-EDGE-DAG:#define __wasm_mutable_globals__ 1{{$}}
+// BLEEDING-EDGE-DAG:#define __wasm_tail_call__ 1{{$}}
 // BLEEDING-EDGE-NOT:#define __wasm_unimplemented_simd128__ 1{{$}}
+// BLEEDING-EDGE-NOT:#define __wasm_exception_handling__ 1{{$}}
 // BLEEDING-EDGE-NOT:#define __wasm_multivalue__ 1{{$}}
-// BLEEDING-EDGE-NOT:#define __wasm_tail_call__ 1{{$}}
 // BLEEDING-EDGE-NOT:#define __wasm_reference_types__ 1{{$}}
 
 // RUN: %clang -E -dM %s -o - 2>&1 \

diff  --git a/llvm/lib/Target/WebAssembly/WebAssembly.td 
b/llvm/lib/Target/WebAssembly/WebAssembly.td
index 3a82778e0367..2c18bf2c3abe 100644
--- a/llvm/lib/Target/WebAssembly/WebAssembly.td
+++ b/llvm/lib/Target/WebAssembly/WebAssembly.td
@@ -102,7 +102,8 @@ def : ProcessorModel<"generic", NoSchedModel, []>;
 def : ProcessorModel<"bleeding-edge", NoSchedModel,
   [FeatureSIMD128, FeatureAtomics,
FeatureNontrappingFPToInt, FeatureSignExt,
-   FeatureMutableGlobals]>;
+   FeatureMutableGlobals, FeatureBulkMemory,
+   FeatureTailCall]>;
 
 
//===--===//
 // Target Declaration

diff  --git a/llvm/test/CodeGen/WebAssembly/target-features.ll 
b/llvm/test/CodeGen/WebAssembly/target-features.ll
index ffb414f670be..f479e12a1afa 100644
--- a/llvm/test/CodeGen/WebAssembly/target-features.ll
+++ b/llvm/test/CodeGen/WebAssembly/target-features.ll
@@ -83,13 +83,16 @@ attributes #2 = { "target-features"="+reference-types" }
 ; SIMD128-NEXT: .int8 7
 ; SIMD128-NEXT: .ascii "simd128"
 
-; +atomics, +mutable-globals, +nontrapping-fptoint, +reference-types, 
+sign-ext,
-; +simd128
-; BLEEDING-EDGE-NEXT: .int8   6
+; +atomics, +bulk-memory, +mutable-globals, +nontrapping-fptoint,
+; +reference-types, +sign-ext, +simd128, +tail-call
+; BLEEDING-EDGE-NEXT: .int8   8
 ; BLEEDING-EDGE-NEXT: .int8   43
 ; BLEEDING-EDGE-NEXT: .int8   7
 ; BLEEDING-EDGE-NEXT: .ascii  "atomics"
 ; BLEEDING-EDGE-NEXT: .int8   43
+; BLEEDING-EDGE-NEXT: .int8   11
+; BLEEDING-EDGE-NEXT: .ascii  "bulk-memory"
+; BLEEDING-EDGE-NEXT: .int8   43
 ; BLEEDING-EDGE-NEXT: .int8   15
 ; BLEEDING-EDGE-NEXT: .ascii  "mutable-globals"
 ; BLEEDING-EDGE-NEXT: .int8   43
@@ -104,5 +107,8 @@ attributes #2 = { "target-features"="+reference-types" }
 ; BLEEDING-EDGE-NEXT: .int8   43
 ; BLEEDING-EDGE-NEXT: .int8   7
 ; BLEEDING-EDGE-NEXT: .ascii  "simd128"
+; BLEEDING-EDGE-NEXT: .int8   43
+; BLEEDING-EDGE-NEXT: .int8   9
+; BLEEDING-EDGE-NEXT: .ascii  "tail-call"
 
 ; CHECK-NEXT: .text



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

[PATCH] D73320: [WebAssembly] Add reference types target feature

[Heejin Ahn via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG764f4089e89e: [WebAssembly] Add reference types target 
feature (authored by aheejin).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73320

Files:
  clang/lib/Basic/Targets/WebAssembly.cpp
  clang/lib/Basic/Targets/WebAssembly.h
  clang/test/Preprocessor/wasm-target-features.c
  llvm/lib/Target/WebAssembly/WebAssembly.td
  llvm/lib/Target/WebAssembly/WebAssemblyInstrInfo.td
  llvm/lib/Target/WebAssembly/WebAssemblySubtarget.h
  llvm/test/CodeGen/WebAssembly/reference-types.ll
  llvm/test/CodeGen/WebAssembly/target-features.ll

Index: llvm/test/CodeGen/WebAssembly/target-features.ll
===
--- llvm/test/CodeGen/WebAssembly/target-features.ll
+++ llvm/test/CodeGen/WebAssembly/target-features.ll
@@ -23,8 +23,13 @@
   ret void
 }
 
+define void @fn_reference_types() #2 {
+  ret void
+}
+
 attributes #0 = { "target-features"="+atomics" }
 attributes #1 = { "target-features"="+nontrapping-fptoint" }
+attributes #2 = { "target-features"="+reference-types" }
 
 ; CHECK-LABEL: fn_atomics:
 
@@ -51,29 +56,36 @@
 
 ; CHECK-LABEL: .custom_section.target_features,"",@
 
-; +atomics, +nontrapping-fptoint
-; ATTRS-NEXT: .int8 2
+; +atomics, +nontrapping-fptoint, +reference-types
+; ATTRS-NEXT: .int8 3
 ; ATTRS-NEXT: .int8 43
 ; ATTRS-NEXT: .int8 7
 ; ATTRS-NEXT: .ascii "atomics"
 ; ATTRS-NEXT: .int8 43
 ; ATTRS-NEXT: .int8 19
 ; ATTRS-NEXT: .ascii "nontrapping-fptoint"
+; ATTRS-NEXT: .int8 43
+; ATTRS-NEXT: .int8 15
+; ATTRS-NEXT: .ascii "reference-types"
 
-; +atomics, +simd128
-; SIMD128-NEXT: .int8 3
+; +atomics, +nontrapping-fptoint, +reference-types, +simd128
+; SIMD128-NEXT: .int8 4
 ; SIMD128-NEXT: .int8 43
 ; SIMD128-NEXT: .int8 7
 ; SIMD128-NEXT: .ascii "atomics"
 ; SIMD128-NEXT: .int8 43
 ; SIMD128-NEXT: .int8 19
-; SIMD128-NEXT: .ascii  "nontrapping-fptoint"
+; SIMD128-NEXT: .ascii "nontrapping-fptoint"
+; SIMD128-NEXT: .int8 43
+; SIMD128-NEXT: .int8 15
+; SIMD128-NEXT: .ascii "reference-types"
 ; SIMD128-NEXT: .int8 43
 ; SIMD128-NEXT: .int8 7
 ; SIMD128-NEXT: .ascii "simd128"
 
-; +atomics, +nontrapping-fptoint, +sign-ext, +simd128
-; BLEEDING-EDGE-NEXT: .int8   5
+; +atomics, +mutable-globals, +nontrapping-fptoint, +reference-types, +sign-ext,
+; +simd128
+; BLEEDING-EDGE-NEXT: .int8   6
 ; BLEEDING-EDGE-NEXT: .int8   43
 ; BLEEDING-EDGE-NEXT: .int8   7
 ; BLEEDING-EDGE-NEXT: .ascii  "atomics"
@@ -84,6 +96,9 @@
 ; BLEEDING-EDGE-NEXT: .int8   19
 ; BLEEDING-EDGE-NEXT: .ascii  "nontrapping-fptoint"
 ; BLEEDING-EDGE-NEXT: .int8   43
+; BLEEDING-EDGE-NEXT: .int8   15
+; BLEEDING-EDGE-NEXT: .ascii  "reference-types"
+; BLEEDING-EDGE-NEXT: .int8   43
 ; BLEEDING-EDGE-NEXT: .int8   8
 ; BLEEDING-EDGE-NEXT: .ascii  "sign-ext"
 ; BLEEDING-EDGE-NEXT: .int8   43
Index: llvm/test/CodeGen/WebAssembly/reference-types.ll
===
--- /dev/null
+++ llvm/test/CodeGen/WebAssembly/reference-types.ll
@@ -0,0 +1,14 @@
+; RUN: llc < %s -mattr=+reference-types | FileCheck %s
+
+target datalayout = "e-m:e-p:32:32-i64:64-n32:64-S128"
+target triple = "wasm32-unknown-unknown"
+
+; CHECK-LABEL: reference-types
+define void @reference-types() {
+  ret void
+}
+
+; CHECK:  .int8 1
+; CHECK-NEXT: .int8 43
+; CHECK-NEXT: .int8 15
+; CHECK-NEXT: .ascii "reference-types"
Index: llvm/lib/Target/WebAssembly/WebAssemblySubtarget.h
===
--- llvm/lib/Target/WebAssembly/WebAssemblySubtarget.h
+++ llvm/lib/Target/WebAssembly/WebAssemblySubtarget.h
@@ -47,6 +47,7 @@
   bool HasMultivalue = false;
   bool HasMutableGlobals = false;
   bool HasTailCall = false;
+  bool HasReferenceTypes = false;
 
   /// String name of used CPU.
   std::string CPUString;
@@ -104,6 +105,7 @@
   bool hasMultivalue() const { return HasMultivalue; }
   bool hasMutableGlobals() const { return HasMutableGlobals; }
   bool hasTailCall() const { return HasTailCall; }
+  bool hasReferenceTypes() const { return HasReferenceTypes; }
 
   /// Parses features string setting specified subtarget options. Definition of
   /// function is auto generated by tblgen.
Index: llvm/lib/Target/WebAssembly/WebAssemblyInstrInfo.td
===
--- llvm/lib/Target/WebAssembly/WebAssemblyInstrInfo.td
+++ llvm/lib/Target/WebAssembly/WebAssemblyInstrInfo.td
@@ -62,6 +62,10 @@
 Predicate<"Subtarget->hasBulkMemory()">,
 AssemblerPredicate<"FeatureBulkMemory", "bulk-memory">;
 
+def HasReferenceTypes :
+Predicate<"Subtarget->hasReferenceTypes()">,
+AssemblerPredicate<"FeatureReferenceTypes", "reference-types">;
+
 //===--===//
 // WebAssembly-specific DAG Node Types.
 

[clang] 764f408 - [WebAssembly] Add reference types target feature

[Heejin Ahn via cfe-commits]

Author: Heejin Ahn
Date: 2020-01-24T14:26:27-08:00
New Revision: 764f4089e89e4693b7bb8f1ee18080703ce760dd

URL: 
https://github.com/llvm/llvm-project/commit/764f4089e89e4693b7bb8f1ee18080703ce760dd
DIFF: 
https://github.com/llvm/llvm-project/commit/764f4089e89e4693b7bb8f1ee18080703ce760dd.diff

LOG: [WebAssembly] Add reference types target feature

Summary:
This adds the reference types target feature. This does not enable any
more functionality in LLVM/clang for now, but this is necessary to embed
the info in the target features section, which is used by Binaryen and
Emscripten. It turned out that after D69832 `-fwasm-exceptions` crashed
because we didn't have the reference types target feature.

Reviewers: tlively

Subscribers: dschuff, sbc100, jgravelle-google, hiraditya, sunfish, 
cfe-commits, llvm-commits

Tags: #clang, #llvm

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

Added: 
llvm/test/CodeGen/WebAssembly/reference-types.ll

Modified: 
clang/lib/Basic/Targets/WebAssembly.cpp
clang/lib/Basic/Targets/WebAssembly.h
clang/test/Preprocessor/wasm-target-features.c
llvm/lib/Target/WebAssembly/WebAssembly.td
llvm/lib/Target/WebAssembly/WebAssemblyInstrInfo.td
llvm/lib/Target/WebAssembly/WebAssemblySubtarget.h
llvm/test/CodeGen/WebAssembly/target-features.ll

Removed: 




diff  --git a/clang/lib/Basic/Targets/WebAssembly.cpp 
b/clang/lib/Basic/Targets/WebAssembly.cpp
index b16442b99b62..ca07697f6ac8 100644
--- a/clang/lib/Basic/Targets/WebAssembly.cpp
+++ b/clang/lib/Basic/Targets/WebAssembly.cpp
@@ -45,6 +45,7 @@ bool WebAssemblyTargetInfo::hasFeature(StringRef Feature) 
const {
   .Case("mutable-globals", HasMutableGlobals)
   .Case("multivalue", HasMultivalue)
   .Case("tail-call", HasTailCall)
+  .Case("reference-types", HasReferenceTypes)
   .Default(false);
 }
 
@@ -80,6 +81,8 @@ void WebAssemblyTargetInfo::getTargetDefines(const 
LangOptions ,
 Builder.defineMacro("__wasm_multivalue__");
   if (HasTailCall)
 Builder.defineMacro("__wasm_tail_call__");
+  if (HasReferenceTypes)
+Builder.defineMacro("__wasm_reference_types__");
 }
 
 void WebAssemblyTargetInfo::setSIMDLevel(llvm::StringMap ,
@@ -126,6 +129,8 @@ bool WebAssemblyTargetInfo::initFeatureMap(
 Features["multivalue"] = true;
   if (HasTailCall)
 Features["tail-call"] = true;
+  if (HasReferenceTypes)
+Features["reference-types"] = true;
 
   return TargetInfo::initFeatureMap(Features, Diags, CPU, FeaturesVec);
 }
@@ -213,6 +218,14 @@ bool WebAssemblyTargetInfo::handleTargetFeatures(
   HasTailCall = false;
   continue;
 }
+if (Feature == "+reference-types") {
+  HasReferenceTypes = true;
+  continue;
+}
+if (Feature == "-reference-types") {
+  HasReferenceTypes = false;
+  continue;
+}
 
 Diags.Report(diag::err_opt_not_valid_with_opt)
 << Feature << "-target-feature";

diff  --git a/clang/lib/Basic/Targets/WebAssembly.h 
b/clang/lib/Basic/Targets/WebAssembly.h
index 55d90db267e1..b022b4bb38a0 100644
--- a/clang/lib/Basic/Targets/WebAssembly.h
+++ b/clang/lib/Basic/Targets/WebAssembly.h
@@ -38,6 +38,7 @@ class LLVM_LIBRARY_VISIBILITY WebAssemblyTargetInfo : public 
TargetInfo {
   bool HasMutableGlobals = false;
   bool HasMultivalue = false;
   bool HasTailCall = false;
+  bool HasReferenceTypes = false;
 
 public:
   explicit WebAssemblyTargetInfo(const llvm::Triple , const TargetOptions &)

diff  --git a/clang/test/Preprocessor/wasm-target-features.c 
b/clang/test/Preprocessor/wasm-target-features.c
index 41681123f206..45f60e260456 100644
--- a/clang/test/Preprocessor/wasm-target-features.c
+++ b/clang/test/Preprocessor/wasm-target-features.c
@@ -96,6 +96,15 @@
 // RUN:   | FileCheck %s -check-prefix=TAIL-CALL
 //
 // TAIL-CALL:#define __wasm_tail_call__ 1{{$}}
+//
+// RUN: %clang -E -dM %s -o - 2>&1 \
+// RUN: -target wasm32-unknown-unknown -mreference-types \
+// RUN:   | FileCheck %s -check-prefix=REFERENCE-TYPES
+// RUN: %clang -E -dM %s -o - 2>&1 \
+// RUN: -target wasm64-unknown-unknown -mreference-types \
+// RUN:   | FileCheck %s -check-prefix=REFERENCE-TYPES
+//
+// REFERENCE-TYPES:#define __wasm_reference_types__ 1{{$}}
 
 // RUN: %clang -E -dM %s -o - 2>&1 \
 // RUN: -target wasm32-unknown-unknown -mcpu=mvp \
@@ -114,6 +123,7 @@
 // MVP-NOT:#define __wasm_mutable_globals__
 // MVP-NOT:#define __wasm_multivalue__
 // MVP-NOT:#define __wasm_tail_call__
+// MVP-NOT:#define __wasm_reference_types__
 
 // RUN: %clang -E -dM %s -o - 2>&1 \
 // RUN: -target wasm32-unknown-unknown -mcpu=bleeding-edge \
@@ -130,6 +140,7 @@
 // BLEEDING-EDGE-NOT:#define __wasm_unimplemented_simd128__ 1{{$}}
 // BLEEDING-EDGE-NOT:#define __wasm_multivalue__ 1{{$}}
 // BLEEDING-EDGE-NOT:#define __wasm_tail_call__ 1{{$}}
+// BLEEDING-EDGE-NOT:#define __wasm_reference_types__ 1{{$}}
 
 // RUN: %clang -E -dM %s -o - 

[PATCH] D72675: [Clang][Driver] Fix -ffast-math/-ffp-contract interaction

[Andy Kaylor via Phabricator via cfe-commits]

andrew.w.kaylor added a comment.

In D72675#1839492 , @wristow wrote:

> 1. Should we enable FMA "by default" at (for example) '-O2'?


We recently introduced a new option "-ffp-model=[precise|fast|strict], which is 
supposed to serve as an umbrella option for the most common combination of 
options. I think our default behavior should be equivalent to 
-ffp-model=precise, which enables contraction. It currently seems to enable 
-ffp-contract=fast in the precise model (https://godbolt.org/z/c6w8mJ). I'm not 
sure that's what it should be doing, but whatever the precise model does should 
be our default.


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

https://reviews.llvm.org/D72675



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

[PATCH] D71973: [clang-tidy] Add bugprone-suspicious-memory-comparison check

[Gabor Bencze via Phabricator via cfe-commits]

gbencze added a comment.

ping


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

https://reviews.llvm.org/D71973



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

[PATCH] D73007: [Sema] Avoid Wrange-loop-analysis false positives

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert added a comment.

Here is a proposal: we add two children to `-Wrange-loop-analysis`.

- `-Wrange-loop-construct` warns about possibly unintended constructor calls. 
This might be in `-Wall`. It contains
  - `warn_for_range_copy`: loop variable A of type B creates a copy from type C
  - `warn_for_range_const_reference_copy`: loop variable A  is initialized with 
a value of a different type resulting in a copy
- `-Wrange-loop-bind-ref[erence]` warns about misleading use of reference 
types. This might **not** be in `-Wall`. It contains
  - `warn_for_range_variable_always_copy`: loop variable A is always a copy 
because the range of type B does not return a reference


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73007



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

[PATCH] D73028: Extract ExprTraversal class from Expr

[Stephen Kelly via Phabricator via cfe-commits]

steveire added a comment.

In D73028#1839572 , @rsmith wrote:

> In D73028#1839494 , @steveire wrote:
>
> > In D73028#1839383 , @rsmith wrote:
> >
> > >
> >
> >
> > The follow-up is here: https://reviews.llvm.org/D73029 .
> >
> > I have the need to change the ascending traversal implemented in 
> > ASTContext.cpp with a map (populated while descending) to make it skip 
> > nodes too during parent traversal.
> >
> > I didn't want to have descending traversal in Expr.cpp and ascending 
> > traversal in ASTContext.cpp as they would be likely to go out of sync, so 
> > moving this implementation here allows extending the purpose in the 
> > follow-up.
>
>
> The ascending traversal should not be in the AST library at all. If tooling 
> or static analysis wants this, that's fine, but navigating upwards is not 
> something the AST supports, and we do not want any part of clang proper 
> developing a reliance on having a parent map, so we're working on moving the 
> parent map out of `ASTContext` (with a plan to eventually move it out of the 
> AST library). See D71313  for related work 
> in this area.


Thanks for the heads-up - I'm trying to reach the conclusion from this 
information.

Are you saying that

1. this patch shouldn't be committed, so down-traversal-skipping should remain 
in Expr.cpp
2. D73029  should be changed to implement the 
up-traversal-skipping directly in ParentMapContext.cpp

Is that the tight conclusion?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73028



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

[PATCH] D73380: [clang] Annotating C++'s `operator new` with more attributes

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri marked 2 inline comments as done.
lebedev.ri added a comment.

In D73380#1839603 , @xbolva00 wrote:

> LLVM already infers noalias nonnull for eg. _Znwm so noalias and nonnull info 
> added by clang will not increase power of LLVM. Or?


To be honest, i don't really believe it is LLVM place to deduce attributes on 
standard library functions like these, frontend should be doing that.
Because as noted in patch description, if `noalias` is blindly "deduced" by 
LLVM,
that is a miscompile if `-fno-assume-sane-operator-new` was specificed.

> Alignment info is useful I think.

That is the main motivation, yes.




Comment at: clang/test/CodeGenCXX/builtin-operator-new-delete.cpp:54
 }
-// CHECK: declare noalias i8* @_ZnwmSt11align_val_t(i64, i64) 
[[ATTR_NOBUILTIN:#[^ ]*]]
+// CHECK: declare nonnull i8* @_ZnwmSt11align_val_t(i64, i64) 
[[ATTR_NOBUILTIN:#[^ ]*]]
 // CHECK: declare void @_ZdlPvSt11align_val_t(i8*, i64) 
[[ATTR_NOBUILTIN_NOUNWIND:#[^ ]*]]

xbolva00 wrote:
> We lost noalias here?
See two lines above, it migrated to the callsite.

I'm going to argue that the previous behavior was erroneous:
despite what `-fno-assume-sane-operator-new` is supposed to be doing,
i **suspect** it will magically break with LTO.
(we'd have `declare noalias nonnull i8* @_ZnwmSt11align_val_t(i64, i64)` in one 
TU
and `declare nonnull i8* @_ZnwmSt11align_val_t(i64, i64)` in another - one with 
`noalias`
and one without - how would they be merged? by dropping `noalias`, or keeping 
it?)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73380



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

[PATCH] D71313: [AST] Split parent map traversal logic into ParentMapContext.h

[Reid Kleckner via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG8a81daaa8b58: [AST] Split parent map traversal logic into 
ParentMapContext.h (authored by rnk).

Changed prior to commit:
  https://reviews.llvm.org/D71313?vs=240050=240283#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71313

Files:
  
clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.cpp
  clang-tools-extra/clang-tidy/readability/MakeMemberFunctionConstCheck.cpp
  clang-tools-extra/clang-tidy/utils/ExprSequence.cpp
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/AST/ASTNodeTraverser.h
  clang/include/clang/AST/ParentMapContext.h
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/CMakeLists.txt
  clang/lib/AST/Linkage.h
  clang/lib/AST/ParentMapContext.cpp
  clang/lib/ASTMatchers/ASTMatchFinder.cpp
  clang/lib/ASTMatchers/ASTMatchersInternal.cpp
  clang/lib/CodeGen/CGCall.h
  clang/lib/Tooling/ASTDiff/ASTDiff.cpp
  clang/lib/Tooling/Refactoring/Rename/USRLocFinder.cpp
  lldb/include/lldb/Symbol/TypeSystemClang.h

Index: lldb/include/lldb/Symbol/TypeSystemClang.h
===
--- lldb/include/lldb/Symbol/TypeSystemClang.h
+++ lldb/include/lldb/Symbol/TypeSystemClang.h
@@ -21,6 +21,7 @@
 #include 
 
 #include "clang/AST/ASTContext.h"
+#include "clang/AST/ASTFwd.h"
 #include "clang/AST/TemplateBase.h"
 #include "llvm/ADT/APSInt.h"
 #include "llvm/ADT/SmallVector.h"
Index: clang/lib/Tooling/Refactoring/Rename/USRLocFinder.cpp
===
--- clang/lib/Tooling/Refactoring/Rename/USRLocFinder.cpp
+++ clang/lib/Tooling/Refactoring/Rename/USRLocFinder.cpp
@@ -15,6 +15,7 @@
 
 #include "clang/Tooling/Refactoring/Rename/USRLocFinder.h"
 #include "clang/AST/ASTContext.h"
+#include "clang/AST/ParentMapContext.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/Basic/LLVM.h"
 #include "clang/Basic/SourceLocation.h"
Index: clang/lib/Tooling/ASTDiff/ASTDiff.cpp
===
--- clang/lib/Tooling/ASTDiff/ASTDiff.cpp
+++ clang/lib/Tooling/ASTDiff/ASTDiff.cpp
@@ -12,6 +12,7 @@
 
 #include "clang/Tooling/ASTDiff/ASTDiff.h"
 
+#include "clang/AST/ParentMapContext.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/Lex/Lexer.h"
 #include "llvm/ADT/PriorityQueue.h"
Index: clang/lib/CodeGen/CGCall.h
===
--- clang/lib/CodeGen/CGCall.h
+++ clang/lib/CodeGen/CGCall.h
@@ -16,6 +16,7 @@
 
 #include "CGValue.h"
 #include "EHScopeStack.h"
+#include "clang/AST/ASTFwd.h"
 #include "clang/AST/CanonicalType.h"
 #include "clang/AST/GlobalDecl.h"
 #include "clang/AST/Type.h"
Index: clang/lib/ASTMatchers/ASTMatchersInternal.cpp
===
--- clang/lib/ASTMatchers/ASTMatchersInternal.cpp
+++ clang/lib/ASTMatchers/ASTMatchersInternal.cpp
@@ -15,6 +15,7 @@
 #include "clang/AST/ASTTypeTraits.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclTemplate.h"
+#include "clang/AST/ParentMapContext.h"
 #include "clang/AST/PrettyPrinter.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
 #include "clang/Basic/LLVM.h"
@@ -237,7 +238,8 @@
   TraversalKindScope RAII(Finder->getASTContext(),
   Implementation->TraversalKind());
 
-  auto N = Finder->getASTContext().traverseIgnored(DynNode);
+  auto N =
+  Finder->getASTContext().getParentMapContext().traverseIgnored(DynNode);
 
   if (RestrictKind.isBaseOf(N.getNodeKind()) &&
   Implementation->dynMatches(N, Finder, Builder)) {
@@ -256,7 +258,8 @@
   TraversalKindScope raii(Finder->getASTContext(),
   Implementation->TraversalKind());
 
-  auto N = Finder->getASTContext().traverseIgnored(DynNode);
+  auto N =
+  Finder->getASTContext().getParentMapContext().traverseIgnored(DynNode);
 
   assert(RestrictKind.isBaseOf(N.getNodeKind()));
   if (Implementation->dynMatches(N, Finder, Builder)) {
Index: clang/lib/ASTMatchers/ASTMatchFinder.cpp
===
--- clang/lib/ASTMatchers/ASTMatchFinder.cpp
+++ clang/lib/ASTMatchers/ASTMatchFinder.cpp
@@ -143,11 +143,14 @@
 Stmt *StmtToTraverse = StmtNode;
 if (auto *ExprNode = dyn_cast_or_null(StmtNode)) {
   auto *LambdaNode = dyn_cast_or_null(StmtNode);
-  if (LambdaNode && Finder->getASTContext().getTraversalKind() ==
-  ast_type_traits::TK_IgnoreUnlessSpelledInSource)
+  if (LambdaNode &&
+  Finder->getASTContext().getParentMapContext().getTraversalKind() ==
+  ast_type_traits::TK_IgnoreUnlessSpelledInSource)
 StmtToTraverse = LambdaNode;
   else
-StmtToTraverse 

[clang] 8a81daa - [AST] Split parent map traversal logic into ParentMapContext.h

[Reid Kleckner via cfe-commits]

Author: Reid Kleckner
Date: 2020-01-24T13:42:28-08:00
New Revision: 8a81daaa8b58aeaa192a47c4ce7f94b4d59ce082

URL: 
https://github.com/llvm/llvm-project/commit/8a81daaa8b58aeaa192a47c4ce7f94b4d59ce082
DIFF: 
https://github.com/llvm/llvm-project/commit/8a81daaa8b58aeaa192a47c4ce7f94b4d59ce082.diff

LOG: [AST] Split parent map traversal logic into ParentMapContext.h

The only part of ASTContext.h that requires most AST types to be
complete is the parent map. Nothing in Clang proper uses the ParentMap,
so split it out into its own class. Make ASTContext own the
ParentMapContext so there is still a one-to-one relationship.

After this change, 562 fewer files depend on ASTTypeTraits.h, and 66
fewer depend on TypeLoc.h:
  $ diff -u deps-before.txt deps-after.txt | \
grep '^[-+] ' | sort | uniq -c | sort -nr | less
  562 -../clang/include/clang/AST/ASTTypeTraits.h
  340 +../clang/include/clang/AST/ParentMapContext.h
   66 -../clang/include/clang/AST/TypeLocNodes.def
   66 -../clang/include/clang/AST/TypeLoc.h
   15 -../clang/include/clang/AST/TemplateBase.h
  ...
I computed deps-before.txt and deps-after.txt with `ninja -t deps`.

This removes a common and key dependency on TemplateBase.h and
TypeLoc.h.

This also has the effect of breaking the ParentMap RecursiveASTVisitor
instantiation into its own file, which roughly halves the compilation
time of ASTContext.cpp (29.75s -> 17.66s). The new file takes 13.8s to
compile.

I left behind forwarding methods for getParents(), but clients will need
to include a new header to make them work:
  #include "clang/AST/ParentMapContext.h"

I noticed that this parent map functionality is unfortunately duplicated
in ParentMap.h, which only works for Stmt nodes.

Reviewed By: rsmith

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

Added: 
clang/include/clang/AST/ParentMapContext.h
clang/lib/AST/ParentMapContext.cpp

Modified: 

clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.cpp
clang-tools-extra/clang-tidy/readability/MakeMemberFunctionConstCheck.cpp
clang-tools-extra/clang-tidy/utils/ExprSequence.cpp
clang/include/clang/AST/ASTContext.h
clang/include/clang/AST/ASTNodeTraverser.h
clang/include/clang/ASTMatchers/ASTMatchers.h
clang/include/clang/Sema/Sema.h
clang/lib/AST/ASTContext.cpp
clang/lib/AST/CMakeLists.txt
clang/lib/AST/Linkage.h
clang/lib/ASTMatchers/ASTMatchFinder.cpp
clang/lib/ASTMatchers/ASTMatchersInternal.cpp
clang/lib/CodeGen/CGCall.h
clang/lib/Tooling/ASTDiff/ASTDiff.cpp
clang/lib/Tooling/Refactoring/Rename/USRLocFinder.cpp
lldb/include/lldb/Symbol/TypeSystemClang.h

Removed: 




diff  --git 
a/clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.cpp
 
b/clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.cpp
index 6939ec9b2e07..760073fcaac2 100644
--- 
a/clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.cpp
+++ 
b/clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.cpp
@@ -8,6 +8,7 @@
 
 #include "ProBoundsArrayToPointerDecayCheck.h"
 #include "clang/AST/ASTContext.h"
+#include "clang/AST/ParentMapContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 
 using namespace clang::ast_matchers;
@@ -35,8 +36,7 @@ AST_MATCHER_P(Expr, hasParentIgnoringImpCasts,
   ast_matchers::internal::Matcher, InnerMatcher) {
   const Expr *E = 
   do {
-ASTContext::DynTypedNodeList Parents =
-Finder->getASTContext().getParents(*E);
+DynTypedNodeList Parents = Finder->getASTContext().getParents(*E);
 if (Parents.size() != 1)
   return false;
 E = Parents[0].get();

diff  --git 
a/clang-tools-extra/clang-tidy/readability/MakeMemberFunctionConstCheck.cpp 
b/clang-tools-extra/clang-tidy/readability/MakeMemberFunctionConstCheck.cpp
index aca8d0fe89d8..3f1dcfe803e2 100644
--- a/clang-tools-extra/clang-tidy/readability/MakeMemberFunctionConstCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/MakeMemberFunctionConstCheck.cpp
@@ -8,6 +8,7 @@
 
 #include "MakeMemberFunctionConstCheck.h"
 #include "clang/AST/ASTContext.h"
+#include "clang/AST/ParentMapContext.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 
@@ -57,7 +58,7 @@ class FindUsageOfThis : public 
RecursiveASTVisitor {
   UsageKind Usage = Unused;
 
   template  const T *getParent(const Expr *E) {
-ASTContext::DynTypedNodeList Parents = Ctxt.getParents(*E);
+DynTypedNodeList Parents = Ctxt.getParents(*E);
 if (Parents.size() != 1)
   return nullptr;
 

diff  --git a/clang-tools-extra/clang-tidy/utils/ExprSequence.cpp 
b/clang-tools-extra/clang-tidy/utils/ExprSequence.cpp
index 6ed595e4a0d6..3505f31c92a2 100644
--- a/clang-tools-extra/clang-tidy/utils/ExprSequence.cpp

[PATCH] D73380: [clang] Annotating C++'s `operator new` with more attributes

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 added a comment.

LLVM already infers noalias nonnull for eg. _Znwm so noalias and nonnull info 
added by clang will not increase power of LLVM. Or?

Alignment info is useful I think.




Comment at: clang/test/CodeGenCXX/builtin-operator-new-delete.cpp:54
 }
-// CHECK: declare noalias i8* @_ZnwmSt11align_val_t(i64, i64) 
[[ATTR_NOBUILTIN:#[^ ]*]]
+// CHECK: declare nonnull i8* @_ZnwmSt11align_val_t(i64, i64) 
[[ATTR_NOBUILTIN:#[^ ]*]]
 // CHECK: declare void @_ZdlPvSt11align_val_t(i8*, i64) 
[[ATTR_NOBUILTIN_NOUNWIND:#[^ ]*]]

We lost noalias here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73380



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

[PATCH] D73028: Extract ExprTraversal class from Expr

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D73028#1839572 , @rsmith wrote:

> In D73028#1839494 , @steveire wrote:
>
> > In D73028#1839383 , @rsmith wrote:
> >
> > >
> >
> >
> > The follow-up is here: https://reviews.llvm.org/D73029 .
> >
> > I have the need to change the ascending traversal implemented in 
> > ASTContext.cpp with a map (populated while descending) to make it skip 
> > nodes too during parent traversal.
> >
> > I didn't want to have descending traversal in Expr.cpp and ascending 
> > traversal in ASTContext.cpp as they would be likely to go out of sync, so 
> > moving this implementation here allows extending the purpose in the 
> > follow-up.
>
>
> The ascending traversal should not be in the AST library at all. If tooling 
> or static analysis wants this, that's fine, but navigating upwards is not 
> something the AST supports, and we do not want any part of clang proper 
> developing a reliance on having a parent map, so we're working on moving the 
> parent map out of `ASTContext` (with a plan to eventually move it out of the 
> AST library). See D71313  for related work 
> in this area.


Thank you for pointing this out, I was unaware of that direction.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73028



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

[PATCH] D73380: [clang] Annotating C++'s `operator new` with more attributes

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri created this revision.
lebedev.ri added reviewers: rsandifo, erichkeane, rjmccall, jdoerfert, eugenis.
lebedev.ri added a project: LLVM.
Herald added subscribers: atanasyan, jrtc27.
Herald added a project: clang.
lebedev.ri added a parent revision: D73020: [Sema] Perform call checking when 
building CXXNewExpr.
lebedev.ri edited reviewers, added: rsmith; removed: rsandifo.

Right now we annotate C++'s `operator new` with `noalias` attribute,
which very much is healthy for optimizations.

However as per `[basic.stc.dynamic.allocation]` 
,
there are more promises on global `operator new`, namely:

- non-`std::nothrow_t` `operator new` *never* returns `nullptr`
- global `operator new`-returned pointer is 
`__STDCPP_DEFAULT_NEW_ALIGNMENT__`-aligned
- If `std::align_val_t align` parameter is taken, the pointer will also be 
`align`-aligned

Supplying this information may not cause immediate landslide effects
on any specific benchmarks, but it for sure will be healthy for optimizer
in the sense that the IR will better reflect the guarantees provided in the 
source code.

The caveat is `-fno-assume-sane-operator-new`, which currently prevents 
emitting `noalias`
attribute, and is automatically passed by Sanitizers (PR16386 
) - should it also cover these 
attributes?
The problem is that the flag is back-end-specific, as seen in 
`test/Modules/explicit-build-flags.cpp`.
But while it is okay to add `noalias` metadata in backend, we really should be 
adding at least
the alignment metadata to the AST, since that allows us to perform sema checks 
on it.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D73380

Files:
  clang/include/clang/AST/Decl.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/Decl.cpp
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/AST/ast-dump-stmt-json.cpp
  clang/test/Analysis/new-ctor-malloc.cpp
  clang/test/Analysis/new-ctor-null-throw.cpp
  clang/test/Analysis/new-ctor-null.cpp
  clang/test/CodeGenCXX/align-avx-complete-objects.cpp
  clang/test/CodeGenCXX/arm.cpp
  clang/test/CodeGenCXX/builtin-calling-conv.cpp
  clang/test/CodeGenCXX/builtin-is-constant-evaluated.cpp
  clang/test/CodeGenCXX/builtin-operator-new-delete.cpp
  clang/test/CodeGenCXX/cxx0x-initializer-stdinitializerlist.cpp
  clang/test/CodeGenCXX/cxx11-initializer-array-new.cpp
  clang/test/CodeGenCXX/cxx1z-aligned-allocation.cpp
  clang/test/CodeGenCXX/delete-two-arg.cpp
  clang/test/CodeGenCXX/dllexport.cpp
  clang/test/CodeGenCXX/dllimport.cpp
  clang/test/CodeGenCXX/exceptions.cpp
  clang/test/CodeGenCXX/goto.cpp
  clang/test/CodeGenCXX/microsoft-abi-array-cookies.cpp
  clang/test/CodeGenCXX/mips-size_t-ptrdiff_t.cpp
  clang/test/CodeGenCXX/multi-dim-operator-new.cpp
  clang/test/CodeGenCXX/new-alias.cpp
  clang/test/CodeGenCXX/new-array-init.cpp
  clang/test/CodeGenCXX/new-overflow.cpp
  clang/test/CodeGenCXX/new.cpp
  clang/test/CodeGenCXX/operator-new.cpp
  clang/test/CodeGenCXX/runtime-dllstorage.cpp
  clang/test/CodeGenCXX/static-init.cpp
  clang/test/CodeGenCoroutines/coro-alloc.cpp
  clang/test/CodeGenCoroutines/coro-cleanup.cpp
  clang/test/CodeGenCoroutines/coro-gro-nrvo.cpp
  clang/test/CodeGenCoroutines/coro-gro.cpp
  clang/test/CodeGenCoroutines/coro-return.cpp
  clang/test/CodeGenObjCXX/arc-new-delete.mm
  clang/test/CodeGenObjCXX/copy.mm
  clang/test/SemaCXX/builtin-operator-new-delete.cpp
  clang/test/SemaCXX/new-delete.cpp

Index: clang/test/SemaCXX/new-delete.cpp
===
--- clang/test/SemaCXX/new-delete.cpp
+++ clang/test/SemaCXX/new-delete.cpp
@@ -27,7 +27,7 @@
 
 __attribute__((used))
 inline void *operator new(size_t) { // no warning, due to __attribute__((used))
-  return 0;
+  return 0; // expected-warning {{null returned from function that requires a non-null return value}}
 }
 
 // PR5823
@@ -53,9 +53,9 @@
   pi = ::new int;
   U *pu = new (ps) U;
   V *pv = new (ps) V;
-  
+
   pi = new (S(1.0f, 2)) int;
-  
+
   (void)new int[true];
 
   // PR7147
@@ -329,7 +329,7 @@
 
 void f() {
   (void)new int[10](1, 2); // expected-error {{array 'new' cannot have initialization arguments}}
-  
+
   typedef int T[10];
   (void)new T(1, 2); // expected-error {{array 'new' cannot have initialization arguments}}
 }
@@ -363,7 +363,7 @@
   void operator delete(void* p); // expected-note {{declared private here}}
 };
 
-void test(S1* s1, S2* s2) { 
+void test(S1* s1, S2* s2) {
   delete s1;
   delete s2; // expected-error {{is a private member}}
   (void)new S1();
@@ -397,7 +397,7 @@
 
 // 
 namespace Instantiate {
-  template struct X { 
+  template struct X {
 operator T*();
   };
 
Index: clang/test/SemaCXX/builtin-operator-new-delete.cpp
===
--- 

[PATCH] D73029: Extend ExprTraversal class with acending traversal

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

rsmith added a comment.

We're working on moving the parent map out of `ASTContext` and into something 
specific to tooling; please don't add more dependencies onto it in the AST 
library.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73029



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

[PATCH] D73028: Extract ExprTraversal class from Expr

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

rsmith added a comment.

In D73028#1839494 , @steveire wrote:

> In D73028#1839383 , @rsmith wrote:
>
> >
>
>
> The follow-up is here: https://reviews.llvm.org/D73029 .
>
> I have the need to change the ascending traversal implemented in 
> ASTContext.cpp with a map (populated while descending) to make it skip nodes 
> too during parent traversal.
>
> I didn't want to have descending traversal in Expr.cpp and ascending 
> traversal in ASTContext.cpp as they would be likely to go out of sync, so 
> moving this implementation here allows extending the purpose in the follow-up.


The ascending traversal should not be in the AST library at all. If tooling or 
static analysis wants this, that's fine, but navigating upwards is not 
something the AST supports, and we do not want any part of clang proper 
developing a reliance on having a parent map, so we're working on moving the 
parent map out of `ASTContext` (with a plan to eventually move it out of the 
AST library). See D71313  for related work in 
this area.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73028



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

[PATCH] D72811: [WIP][OPENMP5.0] allow lvalue for motion clause

[Chi Chun Chen via Phabricator via cfe-commits]

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



Comment at: clang/lib/Sema/SemaOpenMP.cpp:15235
+  bool VisitUnaryOperator(UnaryOperator *UO) {
+DerefCnt++;
+CurComponents.emplace_back(UO, nullptr);

ABataev wrote:
> cchen wrote:
> > cchen wrote:
> > > ABataev wrote:
> > > > Need a check that this is a dereference op. Also, maybe allow using an 
> > > > addr_of operation?
> > > is addr_of operation allowed in lvalue?
> > > 
> > > In this code:
> > > ```
> > > int arr[50];
> > > 
> > > #pragma omp target map()
> > >   {}
> > > ```
> > > We now reject `` since `RE->IgnoreParenImpCasts()->isLValue()` return 
> > > false. (RE is the expr of ``)
> > BTW, `RE->isLValue()` also return false in this case.
> I'm not saying that `` must be allowed, but something like `*()`. Of 
> course, just `` is an rvalue.
Got it, thanks for the clarification!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72811



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

[PATCH] D72811: [WIP][OPENMP5.0] allow lvalue for motion clause

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In D72811#1839538 , @cchen wrote:

> In D72811#1837392 , @jdoerfert wrote:
>
> > Thanks for working on this! While you are at it, `*this` is probably one of 
> > the most important ones to test and support.
>
>
> Can anyone tell me how to get `ValueDecl` from `CXXThisExpr`? I cannot find 
> any method return decl in 
> https://clang.llvm.org/doxygen/classclang_1_1CXXThisExpr.html#details.
>  Thanks!


CXXThisExpr is an expression without associated declaration.




Comment at: clang/lib/Sema/SemaOpenMP.cpp:15235
+  bool VisitUnaryOperator(UnaryOperator *UO) {
+DerefCnt++;
+CurComponents.emplace_back(UO, nullptr);

cchen wrote:
> cchen wrote:
> > ABataev wrote:
> > > Need a check that this is a dereference op. Also, maybe allow using an 
> > > addr_of operation?
> > is addr_of operation allowed in lvalue?
> > 
> > In this code:
> > ```
> > int arr[50];
> > 
> > #pragma omp target map()
> >   {}
> > ```
> > We now reject `` since `RE->IgnoreParenImpCasts()->isLValue()` return 
> > false. (RE is the expr of ``)
> BTW, `RE->isLValue()` also return false in this case.
I'm not saying that `` must be allowed, but something like `*()`. Of 
course, just `` is an rvalue.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72811



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

[PATCH] D73300: [clang-tidy] Add library for clang-tidy main function

[Dmitry Polukhin via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG3f8b100e94b5: [clang-tidy] Add library for clang-tidy main 
function (authored by DmitryPolukhin).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73300

Files:
  clang-tools-extra/clang-tidy/tool/CMakeLists.txt
  clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
  clang-tools-extra/clang-tidy/tool/ClangTidyMain.h
  clang-tools-extra/clang-tidy/tool/ClangTidyToolMain.cpp

Index: clang-tools-extra/clang-tidy/tool/ClangTidyToolMain.cpp
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/tool/ClangTidyToolMain.cpp
@@ -0,0 +1,21 @@
+//===--- tools/extra/clang-tidy/ClangTidyToolMain.cpp - Clang tidy tool ---===//
+//
+// 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 This file contains clang-tidy tool entry point main function.
+///
+///  This tool uses the Clang Tooling infrastructure, see
+///http://clang.llvm.org/docs/HowToSetupToolingForLLVM.html
+///  for details on setting it up with LLVM source tree.
+///
+//===--===//
+
+#include "ClangTidyMain.h"
+
+int main(int argc, const char **argv) {
+  return clang::tidy::clangTidyMain(argc, argv);
+}
Index: clang-tools-extra/clang-tidy/tool/ClangTidyMain.h
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/tool/ClangTidyMain.h
@@ -0,0 +1,23 @@
+//===--- tools/extra/clang-tidy/ClangTidyMain.h - Clang tidy tool ---===//
+//
+// 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 This file declares the main function for the clang-tidy tool.
+///
+///  This tool uses the Clang Tooling infrastructure, see
+///http://clang.llvm.org/docs/HowToSetupToolingForLLVM.html
+///  for details on setting it up with LLVM source tree.
+///
+//===--===//
+
+namespace clang {
+namespace tidy {
+
+int clangTidyMain(int argc, const char **argv);
+
+} // namespace tidy
+} // namespace clang
Index: clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
===
--- clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
+++ clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
@@ -14,6 +14,7 @@
 ///
 //===--===//
 
+#include "ClangTidyMain.h"
 #include "../ClangTidy.h"
 #include "../ClangTidyForceLinker.h"
 #include "../GlobList.h"
@@ -327,7 +328,7 @@
   return FS;
 }
 
-static int clangTidyMain(int argc, const char **argv) {
+int clangTidyMain(int argc, const char **argv) {
   llvm::InitLLVM X(argc, argv);
   CommonOptionsParser OptionsParser(argc, argv, ClangTidyCategory,
 cl::ZeroOrMore);
@@ -488,7 +489,3 @@
 
 } // namespace tidy
 } // namespace clang
-
-int main(int argc, const char **argv) {
-  return clang::tidy::clangTidyMain(argc, argv);
-}
Index: clang-tools-extra/clang-tidy/tool/CMakeLists.txt
===
--- clang-tools-extra/clang-tidy/tool/CMakeLists.txt
+++ clang-tools-extra/clang-tidy/tool/CMakeLists.txt
@@ -5,8 +5,24 @@
   support
   )
 
-add_clang_tool(clang-tidy
+# Needed by LLVM's CMake checks because this file defines multiple targets.
+set(LLVM_OPTIONAL_SOURCES ClangTidyMain.cpp ClangTidyToolMain.cpp)
+
+add_clang_library(clangTidyMain
   ClangTidyMain.cpp
+
+  LINK_LIBS
+  clangAST
+  clangASTMatchers
+  clangBasic
+  clangTidy
+  ${ALL_CLANG_TIDY_CHECKS}
+  clangTooling
+  clangToolingCore
+  )
+
+add_clang_tool(clang-tidy
+  ClangTidyToolMain.cpp
   )
 add_dependencies(clang-tidy
   clang-resource-headers
@@ -22,6 +38,7 @@
 target_link_libraries(clang-tidy
   PRIVATE
   clangTidy
+  clangTidyMain
   ${ALL_CLANG_TIDY_CHECKS}
   )
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72811: [WIP][OPENMP5.0] allow lvalue for motion clause

[Chi Chun Chen via Phabricator via cfe-commits]

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



Comment at: clang/lib/Sema/SemaOpenMP.cpp:15235
+  bool VisitUnaryOperator(UnaryOperator *UO) {
+DerefCnt++;
+CurComponents.emplace_back(UO, nullptr);

cchen wrote:
> ABataev wrote:
> > Need a check that this is a dereference op. Also, maybe allow using an 
> > addr_of operation?
> is addr_of operation allowed in lvalue?
> 
> In this code:
> ```
> int arr[50];
> 
> #pragma omp target map()
>   {}
> ```
> We now reject `` since `RE->IgnoreParenImpCasts()->isLValue()` return 
> false. (RE is the expr of ``)
BTW, `RE->isLValue()` also return false in this case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72811



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

[PATCH] D72811: [WIP][OPENMP5.0] allow lvalue for motion clause

[Chi Chun Chen via Phabricator via cfe-commits]

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

In D72811#1837392 , @jdoerfert wrote:

> Thanks for working on this! While you are at it, `*this` is probably one of 
> the most important ones to test and support.


Can anyone tell me how to get `ValueDecl` from `CXXThisExpr`? I cannot find any 
method return decl in 
https://clang.llvm.org/doxygen/classclang_1_1CXXThisExpr.html#details.
Thanks!




Comment at: clang/lib/Sema/SemaOpenMP.cpp:15235
+  bool VisitUnaryOperator(UnaryOperator *UO) {
+DerefCnt++;
+CurComponents.emplace_back(UO, nullptr);

ABataev wrote:
> Need a check that this is a dereference op. Also, maybe allow using an 
> addr_of operation?
is addr_of operation allowed in lvalue?

In this code:
```
int arr[50];

#pragma omp target map()
  {}
```
We now reject `` since `RE->IgnoreParenImpCasts()->isLValue()` return 
false. (RE is the expr of ``)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72811



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

[clang-tools-extra] 3f8b100 - [clang-tidy] Add library for clang-tidy main function

[Dmitry Polukhin via cfe-commits]

Author: Dmitry Polukhin
Date: 2020-01-24T13:00:45-08:00
New Revision: 3f8b100e94b5c848843fa91c9782d9d4df4bb026

URL: 
https://github.com/llvm/llvm-project/commit/3f8b100e94b5c848843fa91c9782d9d4df4bb026
DIFF: 
https://github.com/llvm/llvm-project/commit/3f8b100e94b5c848843fa91c9782d9d4df4bb026.diff

LOG: [clang-tidy] Add library for clang-tidy main function

Summary:
This library allows to create clang-tidy tools with custom checks outside of 
llvm repo
using prebuilt clang release tarball.

Test Plan:
Checked that clang-tidy works as before. New library exists in istall dir.

Reviewers: smeenai, gribozavr, stephanemoore

Subscribers: mgorny, xazax.hun, cfe-commits

Tags: #clang-tools-extra, #clang

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

Added: 
clang-tools-extra/clang-tidy/tool/ClangTidyMain.h
clang-tools-extra/clang-tidy/tool/ClangTidyToolMain.cpp

Modified: 
clang-tools-extra/clang-tidy/tool/CMakeLists.txt
clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/tool/CMakeLists.txt 
b/clang-tools-extra/clang-tidy/tool/CMakeLists.txt
index 073749a7d836..0cd15ddb4653 100644
--- a/clang-tools-extra/clang-tidy/tool/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/tool/CMakeLists.txt
@@ -5,8 +5,24 @@ set(LLVM_LINK_COMPONENTS
   support
   )
 
-add_clang_tool(clang-tidy
+# Needed by LLVM's CMake checks because this file defines multiple targets.
+set(LLVM_OPTIONAL_SOURCES ClangTidyMain.cpp ClangTidyToolMain.cpp)
+
+add_clang_library(clangTidyMain
   ClangTidyMain.cpp
+
+  LINK_LIBS
+  clangAST
+  clangASTMatchers
+  clangBasic
+  clangTidy
+  ${ALL_CLANG_TIDY_CHECKS}
+  clangTooling
+  clangToolingCore
+  )
+
+add_clang_tool(clang-tidy
+  ClangTidyToolMain.cpp
   )
 add_dependencies(clang-tidy
   clang-resource-headers
@@ -22,6 +38,7 @@ clang_target_link_libraries(clang-tidy
 target_link_libraries(clang-tidy
   PRIVATE
   clangTidy
+  clangTidyMain
   ${ALL_CLANG_TIDY_CHECKS}
   )
 

diff  --git a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp 
b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
index ad6182def20d..c6927cc6bd98 100644
--- a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
+++ b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
@@ -14,6 +14,7 @@
 ///
 
//===--===//
 
+#include "ClangTidyMain.h"
 #include "../ClangTidy.h"
 #include "../ClangTidyForceLinker.h"
 #include "../GlobList.h"
@@ -327,7 +328,7 @@ getVfsFromFile(const std::string ,
   return FS;
 }
 
-static int clangTidyMain(int argc, const char **argv) {
+int clangTidyMain(int argc, const char **argv) {
   llvm::InitLLVM X(argc, argv);
   CommonOptionsParser OptionsParser(argc, argv, ClangTidyCategory,
 cl::ZeroOrMore);
@@ -488,7 +489,3 @@ static int clangTidyMain(int argc, const char **argv) {
 
 } // namespace tidy
 } // namespace clang
-
-int main(int argc, const char **argv) {
-  return clang::tidy::clangTidyMain(argc, argv);
-}

diff  --git a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.h 
b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.h
new file mode 100644
index ..f87f84b66aca
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.h
@@ -0,0 +1,23 @@
+//===--- tools/extra/clang-tidy/ClangTidyMain.h - Clang tidy tool ---===//
+//
+// 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 This file declares the main function for the clang-tidy tool.
+///
+///  This tool uses the Clang Tooling infrastructure, see
+///http://clang.llvm.org/docs/HowToSetupToolingForLLVM.html
+///  for details on setting it up with LLVM source tree.
+///
+//===--===//
+
+namespace clang {
+namespace tidy {
+
+int clangTidyMain(int argc, const char **argv);
+
+} // namespace tidy
+} // namespace clang

diff  --git a/clang-tools-extra/clang-tidy/tool/ClangTidyToolMain.cpp 
b/clang-tools-extra/clang-tidy/tool/ClangTidyToolMain.cpp
new file mode 100644
index ..eb7fde7b8e07
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/tool/ClangTidyToolMain.cpp
@@ -0,0 +1,21 @@
+//===--- tools/extra/clang-tidy/ClangTidyToolMain.cpp - Clang tidy tool 
---===//
+//
+// 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 This file contains clang-tidy tool entry point main function.

[PATCH] D71600: PowerPC 32-bit - forces 8 byte lock/lock_free decisions at compiled time

[Dimitry Andric via Phabricator via cfe-commits]

dim added inline comments.



Comment at: clang/lib/AST/ExprConstant.cpp:11180
 
+// Avoid emiting call for runtime decision on PowerPC 32-bit
+// The lock free possibilities on this platform are covered by the lines 

`s/emiting/emitting/`



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71600



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

[PATCH] D73028: Extract ExprTraversal class from Expr

[Stephen Kelly via Phabricator via cfe-commits]

steveire added a comment.

In D73028#1839383 , @rsmith wrote:

> > An addition to the API will be concerned with ascending through the AST in 
> > different traversal modes.
>
> Ascending through the AST is not possibly by design. (For example, we share 
> AST nodes between template instantiations, and statement nodes don't contain 
> parent pointers.) Can you say more about what you're thinking of adding here?


The follow-up is here: https://reviews.llvm.org/D73029 .

I have the need to change the ascending traversal implemented in ASTContext.cpp 
with a map (populated while descending) to make it skip nodes too during parent 
traversal.

I didn't want to have descending traversal in Expr.cpp and ascending traversal 
in ASTContext.cpp as they would be likely to go out of sync, so moving this 
implementation here allows extending the purpose in the follow-up.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73028



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

[PATCH] D72675: [Clang][Driver] Fix -ffast-math/-ffp-contract interaction

[Warren Ristow via Phabricator via cfe-commits]

wristow added a comment.

> A separate question is the interaction of `-ffast-math` with 
> `-ffp-contract=`.  Currently, there is no such interaction whatsoever in GCC: 
> `-ffast-math` does not imply any particular `-ffp-contract=` setting, and 
> vice versa the `-ffp-contract=` setting is not considered at all when 
> defining `__FAST_MATH__`. This seems at least internally consistent.

That's interesting, and as you said, internally consistent behavior in GCC.  I 
think it would be a fine idea for us to do the same thing.

Looking into this point, I see that (ignoring fastmath for the moment) our 
default `-ffp-contract` behavior is different than GCC's.  GCC enables FMA by 
default when optimization is high enough ('-O2') without any special switch 
needed.  For example, taking an architecture that supports FMA (Haswell), GCC 
has the following behavior:

  float test(float a, float b, float c)
  {
// FMA is enabled by default for GCC (on Haswell), so done at -O2:
//gcc -S -O2 -march=haswell test.c  # FMA happens
//$ gcc -S -march=haswell test.c ; egrep 'mul|add' test.s
//vmulss  -12(%rbp), %xmm0, %xmm0
//vaddss  -4(%rbp), %xmm0, %xmm0
//$ gcc -S -O2 -march=haswell test.c ; egrep 'mul|add' test.s
//vfmadd231ss %xmm2, %xmm1, %xmm0
//$
return a + (b * c);
  }

As we'd expect, GCC does disable FMA with `-ffp-contract=off` (this is 
irrespective of whether `-ffast-math` was specified).  Loosely, GCC's behavior 
can be summarized very simply on this point as:
//Suppress FMA when `-ffp-contract=off`.//
(As an aside, GCC's behavior with `-ffp-contract=on` is non-intuitive to me.  
It relates to the FP_CONTRACT pragma, which as far as I can see is ignored by 
GCC.)

In contrast, we do //not// enable FMA by default (via general optimization, 
such as '-O2').  For example:

  $ clang -S -O2 -march=haswell test.c ; egrep 'mul|add' test.s
  vmulss  %xmm2, %xmm1, %xmm1
  vaddss  %xmm0, %xmm1, %xmm0
  .addrsig
  $

I think that whether we want to continue doing that (or instead, enable it at 
'-O2', like GCC does), is a separate issue.  I can see arguments either way.

We do enable FMA with `-ffp-contract=fast`, as desired (and also with 
`-ffp-contract=on`).  And we do "leave it disabled" with `-ffp-contract=off` 
(as expected).

Now, bringing fastmath back into the discussion, we //do// enable FMA with 
`-fffast-math`.  If we decide to continue leaving it disabled by default, then 
enabling it with `-ffast-math` seems perfectly sensible.  (If we decide to 
enable it by default when optimization is high enough, like GCC, then turning 
on `-ffast-math` should not disable it of course.)

The problem I want to address here is that if the compiler is a mode where FMA 
is enabled (whether that's at '-O2' "by default", or whether it's because the 
user turned on `-ffast-math`), then appending `-ffp-contract=off` //should// 
disable FMA.  I think this patch (along with the small change to 
"DAGCombiner.cpp", in an earlier version of this patch) is a reasonable 
approach to solving that.  I'd say this patch/review/discussion has raised two 
additional questions:

1. Under what conditions should `__FAST_MATH__` be defined?
2. Should we enable FMA "by default" at (for example) '-O2'?

I think these additional questions are best addressed separately.  My 2 cents 
are that for (1), mimicking GCC's behavior seems reasonable (although that's 
assuming we don't find out that GCC's `__FAST_MATH__` behavior is a bug).  And 
for (2), I don't have a strong preference.


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

https://reviews.llvm.org/D72675



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

[PATCH] D73028: Extract ExprTraversal class from Expr

[Stephen Kelly via Phabricator via cfe-commits]

steveire updated this revision to Diff 240269.
steveire marked an inline comment as done.
steveire added a comment.

Update


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73028

Files:
  clang/include/clang/AST/Expr.h
  clang/include/clang/AST/ExprTraversal.h
  clang/lib/AST/CMakeLists.txt
  clang/lib/AST/Expr.cpp
  clang/lib/AST/ExprTraversal.cpp

Index: clang/lib/AST/ExprTraversal.cpp
===
--- /dev/null
+++ clang/lib/AST/ExprTraversal.cpp
@@ -0,0 +1,254 @@
+//===--- ExprTraversal.cpp - Simple expression traversal --===//
+//
+// 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
+//
+//===--===//
+//
+// This file implements Expr traversal.
+//
+//===--===//
+
+#include "clang/AST/ExprTraversal.h"
+#include "clang/AST/Expr.h"
+#include "clang/AST/ExprCXX.h"
+
+using namespace clang;
+
+static Expr *IgnoreImpCastsSingleStep(Expr *E) {
+  if (auto *ICE = dyn_cast(E))
+return ICE->getSubExpr();
+
+  if (auto *FE = dyn_cast(E))
+return FE->getSubExpr();
+
+  return E;
+}
+
+static Expr *IgnoreImpCastsExtraSingleStep(Expr *E) {
+  // FIXME: Skip MaterializeTemporaryExpr and SubstNonTypeTemplateParmExpr in
+  // addition to what IgnoreImpCasts() skips to account for the current
+  // behaviour of IgnoreParenImpCasts().
+  Expr *SubE = IgnoreImpCastsSingleStep(E);
+  if (SubE != E)
+return SubE;
+
+  if (auto *MTE = dyn_cast(E))
+return MTE->getSubExpr();
+
+  if (auto *NTTP = dyn_cast(E))
+return NTTP->getReplacement();
+
+  return E;
+}
+
+static Expr *IgnoreCastsSingleStep(Expr *E) {
+  if (auto *CE = dyn_cast(E))
+return CE->getSubExpr();
+
+  if (auto *FE = dyn_cast(E))
+return FE->getSubExpr();
+
+  if (auto *MTE = dyn_cast(E))
+return MTE->getSubExpr();
+
+  if (auto *NTTP = dyn_cast(E))
+return NTTP->getReplacement();
+
+  return E;
+}
+
+static Expr *IgnoreLValueCastsSingleStep(Expr *E) {
+  // Skip what IgnoreCastsSingleStep skips, except that only
+  // lvalue-to-rvalue casts are skipped.
+  if (auto *CE = dyn_cast(E))
+if (CE->getCastKind() != CK_LValueToRValue)
+  return E;
+
+  return IgnoreCastsSingleStep(E);
+}
+
+static Expr *IgnoreBaseCastsSingleStep(Expr *E) {
+  if (auto *CE = dyn_cast(E))
+if (CE->getCastKind() == CK_DerivedToBase ||
+CE->getCastKind() == CK_UncheckedDerivedToBase ||
+CE->getCastKind() == CK_NoOp)
+  return CE->getSubExpr();
+
+  return E;
+}
+
+static Expr *IgnoreImplicitSingleStep(Expr *E) {
+  Expr *SubE = IgnoreImpCastsSingleStep(E);
+  if (SubE != E)
+return SubE;
+
+  if (auto *MTE = dyn_cast(E))
+return MTE->getSubExpr();
+
+  if (auto *BTE = dyn_cast(E))
+return BTE->getSubExpr();
+
+  return E;
+}
+
+static Expr *IgnoreImplicitAsWrittenSingleStep(Expr *E) {
+  if (auto *ICE = dyn_cast(E))
+return ICE->getSubExprAsWritten();
+
+  return IgnoreImplicitSingleStep(E);
+}
+
+static Expr *IgnoreParensOnlySingleStep(Expr *E) {
+  if (auto *PE = dyn_cast(E))
+return PE->getSubExpr();
+  return E;
+}
+
+static Expr *IgnoreParensSingleStep(Expr *E) {
+  if (auto *PE = dyn_cast(E))
+return PE->getSubExpr();
+
+  if (auto *UO = dyn_cast(E)) {
+if (UO->getOpcode() == UO_Extension)
+  return UO->getSubExpr();
+  }
+
+  else if (auto *GSE = dyn_cast(E)) {
+if (!GSE->isResultDependent())
+  return GSE->getResultExpr();
+  }
+
+  else if (auto *CE = dyn_cast(E)) {
+if (!CE->isConditionDependent())
+  return CE->getChosenSubExpr();
+  }
+
+  else if (auto *CE = dyn_cast(E))
+return CE->getSubExpr();
+
+  return E;
+}
+
+static Expr *IgnoreNoopCastsSingleStep(const ASTContext , Expr *E) {
+  if (auto *CE = dyn_cast(E)) {
+// We ignore integer <-> casts that are of the same width, ptr<->ptr and
+// ptr<->int casts of the same width. We also ignore all identity casts.
+Expr *SubExpr = CE->getSubExpr();
+bool IsIdentityCast =
+Ctx.hasSameUnqualifiedType(E->getType(), SubExpr->getType());
+bool IsSameWidthCast =
+(E->getType()->isPointerType() || E->getType()->isIntegralType(Ctx)) &&
+(SubExpr->getType()->isPointerType() ||
+ SubExpr->getType()->isIntegralType(Ctx)) &&
+(Ctx.getTypeSize(E->getType()) == Ctx.getTypeSize(SubExpr->getType()));
+
+if (IsIdentityCast || IsSameWidthCast)
+  return SubExpr;
+  }
+
+  else if (auto *NTTP = dyn_cast(E))
+return NTTP->getReplacement();
+
+  return E;
+}
+
+static Expr *IgnoreExprNodesImpl(Expr *E) { return E; }
+template 
+static Expr *IgnoreExprNodesImpl(Expr *E, FnTy &, FnTys &&... Fns) {
+  return 

[PATCH] D73028: Extract ExprTraversal class from Expr

[Stephen Kelly via Phabricator via cfe-commits]

steveire updated this revision to Diff 240267.
steveire added a comment.

Update for review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73028

Files:
  clang/include/clang/AST/Expr.h
  clang/include/clang/AST/ExprTraversal.h
  clang/lib/AST/CMakeLists.txt
  clang/lib/AST/Expr.cpp
  clang/lib/AST/ExprTraversal.cpp

Index: clang/lib/AST/ExprTraversal.cpp
===
--- /dev/null
+++ clang/lib/AST/ExprTraversal.cpp
@@ -0,0 +1,254 @@
+//===--- ExprTraversal.cpp - Simple expression traversal --===//
+//
+// 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
+//
+//===--===//
+//
+// This file implements Expr traversal.
+//
+//===--===//
+
+#include "clang/AST/ExprTraversal.h"
+#include "clang/AST/Expr.h"
+#include "clang/AST/ExprCXX.h"
+
+using namespace clang;
+
+static Expr *IgnoreImpCastsSingleStep(Expr *E) {
+  if (auto *ICE = dyn_cast(E))
+return ICE->getSubExpr();
+
+  if (auto *FE = dyn_cast(E))
+return FE->getSubExpr();
+
+  return E;
+}
+
+static Expr *IgnoreImpCastsExtraSingleStep(Expr *E) {
+  // FIXME: Skip MaterializeTemporaryExpr and SubstNonTypeTemplateParmExpr in
+  // addition to what IgnoreImpCasts() skips to account for the current
+  // behaviour of IgnoreParenImpCasts().
+  Expr *SubE = IgnoreImpCastsSingleStep(E);
+  if (SubE != E)
+return SubE;
+
+  if (auto *MTE = dyn_cast(E))
+return MTE->getSubExpr();
+
+  if (auto *NTTP = dyn_cast(E))
+return NTTP->getReplacement();
+
+  return E;
+}
+
+static Expr *IgnoreCastsSingleStep(Expr *E) {
+  if (auto *CE = dyn_cast(E))
+return CE->getSubExpr();
+
+  if (auto *FE = dyn_cast(E))
+return FE->getSubExpr();
+
+  if (auto *MTE = dyn_cast(E))
+return MTE->getSubExpr();
+
+  if (auto *NTTP = dyn_cast(E))
+return NTTP->getReplacement();
+
+  return E;
+}
+
+static Expr *IgnoreLValueCastsSingleStep(Expr *E) {
+  // Skip what IgnoreCastsSingleStep skips, except that only
+  // lvalue-to-rvalue casts are skipped.
+  if (auto *CE = dyn_cast(E))
+if (CE->getCastKind() != CK_LValueToRValue)
+  return E;
+
+  return IgnoreCastsSingleStep(E);
+}
+
+static Expr *IgnoreBaseCastsSingleStep(Expr *E) {
+  if (auto *CE = dyn_cast(E))
+if (CE->getCastKind() == CK_DerivedToBase ||
+CE->getCastKind() == CK_UncheckedDerivedToBase ||
+CE->getCastKind() == CK_NoOp)
+  return CE->getSubExpr();
+
+  return E;
+}
+
+static Expr *IgnoreImplicitSingleStep(Expr *E) {
+  Expr *SubE = IgnoreImpCastsSingleStep(E);
+  if (SubE != E)
+return SubE;
+
+  if (auto *MTE = dyn_cast(E))
+return MTE->getSubExpr();
+
+  if (auto *BTE = dyn_cast(E))
+return BTE->getSubExpr();
+
+  return E;
+}
+
+static Expr *IgnoreImplicitAsWrittenSingleStep(Expr *E) {
+  if (auto *ICE = dyn_cast(E))
+return ICE->getSubExprAsWritten();
+
+  return IgnoreImplicitSingleStep(E);
+}
+
+static Expr *IgnoreParensOnlySingleStep(Expr *E) {
+  if (auto *PE = dyn_cast(E))
+return PE->getSubExpr();
+  return E;
+}
+
+static Expr *IgnoreParensSingleStep(Expr *E) {
+  if (auto *PE = dyn_cast(E))
+return PE->getSubExpr();
+
+  if (auto *UO = dyn_cast(E)) {
+if (UO->getOpcode() == UO_Extension)
+  return UO->getSubExpr();
+  }
+
+  else if (auto *GSE = dyn_cast(E)) {
+if (!GSE->isResultDependent())
+  return GSE->getResultExpr();
+  }
+
+  else if (auto *CE = dyn_cast(E)) {
+if (!CE->isConditionDependent())
+  return CE->getChosenSubExpr();
+  }
+
+  else if (auto *CE = dyn_cast(E))
+return CE->getSubExpr();
+
+  return E;
+}
+
+static Expr *IgnoreNoopCastsSingleStep(const ASTContext , Expr *E) {
+  if (auto *CE = dyn_cast(E)) {
+// We ignore integer <-> casts that are of the same width, ptr<->ptr and
+// ptr<->int casts of the same width. We also ignore all identity casts.
+Expr *SubExpr = CE->getSubExpr();
+bool IsIdentityCast =
+Ctx.hasSameUnqualifiedType(E->getType(), SubExpr->getType());
+bool IsSameWidthCast =
+(E->getType()->isPointerType() || E->getType()->isIntegralType(Ctx)) &&
+(SubExpr->getType()->isPointerType() ||
+ SubExpr->getType()->isIntegralType(Ctx)) &&
+(Ctx.getTypeSize(E->getType()) == Ctx.getTypeSize(SubExpr->getType()));
+
+if (IsIdentityCast || IsSameWidthCast)
+  return SubExpr;
+  }
+
+  else if (auto *NTTP = dyn_cast(E))
+return NTTP->getReplacement();
+
+  return E;
+}
+
+static Expr *IgnoreExprNodesImpl(Expr *E) { return E; }
+template 
+static Expr *IgnoreExprNodesImpl(Expr *E, FnTy &, FnTys &&... Fns) {
+  return IgnoreExprNodesImpl(Fn(E), 

[PATCH] D71566: New checks for fortified sprintf

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Continues to LG, do you need me to commit on your behalf?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71566



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

[PATCH] D73376: [analyzer] Add FuchsiaLockChecker and C11LockChecker

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun created this revision.
xazax.hun added reviewers: NoQ, haowei.
xazax.hun added a project: clang.
Herald added subscribers: Charusso, gamesh411, dkrupp, donat.nagy, Szelethus, 
jfb, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware.

Basically, the semantics of C11 and Fuchsia locks are very close to PThreads. 
Most of the work was boilerplate to make sure the correct check name appears in 
the diagnostics. If you have any idea how to reduce this boilerplate I'd like 
to know :)

I had to add a dummy check that can always be enabled. Otherwise, 
`CheckerManager::getChecker` would trigger an assertion fail due to attempting 
to get a checker that was potentially not enabled.

I plan to add more tests but wanted to share early to get some feedback about 
the approach.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D73376

Files:
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
  clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
  clang/test/Analysis/c11lock.c
  clang/test/Analysis/fuchsia_lock.c

Index: clang/test/Analysis/fuchsia_lock.c
===
--- /dev/null
+++ clang/test/Analysis/fuchsia_lock.c
@@ -0,0 +1,31 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=fuchsia.Lock -verify %s
+
+typedef int spin_lock_t;
+typedef int sync_mutex_t;
+typedef int zx_status_t;
+typedef int zx_time_t;
+
+void spin_lock(spin_lock_t *lock);
+void spin_trylock(spin_lock_t *lock);
+void spin_unlock(spin_lock_t *lock);
+void spin_lock_init(spin_lock_t *lock);
+
+void spin_lock_save(spin_lock_t *lock, void *statep,
+int flags);
+
+void spin_unlock_restore(spin_lock_t *lock, void *old_state,
+ int flags);
+
+void sync_mutex_lock(sync_mutex_t* mutex);
+void sync_mutex_lock_with_waiter(sync_mutex_t* mutex);
+zx_status_t sync_mutex_timedlock(sync_mutex_t* mutex, zx_time_t deadline);
+zx_status_t sync_mutex_trylock(sync_mutex_t* mutex);
+void sync_mutex_unlock(sync_mutex_t* mutex);
+
+spin_lock_t mtx1;
+
+void bad1(void)
+{
+	spin_lock();	// no-warning
+	spin_lock();	// expected-warning{{This lock has already been acquired}}
+}
Index: clang/test/Analysis/c11lock.c
===
--- /dev/null
+++ clang/test/Analysis/c11lock.c
@@ -0,0 +1,20 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.unix.C11Lock -verify %s
+
+typedef int mtx_t;
+struct timespec;
+
+int mtx_init(mtx_t *mutex, int type);
+int mtx_lock(mtx_t *mutex);
+int mtx_timedlock(mtx_t *mutex,
+  const struct timespec *time_point);
+int mtx_trylock(mtx_t *mutex);
+int mtx_unlock(mtx_t *mutex);
+int mtx_destroy(mtx_t *mutex);
+
+mtx_t mtx1;
+
+void bad1(void)
+{
+	mtx_lock();	// no-warning
+	mtx_lock();	// expected-warning{{This lock has already been acquired}}
+}
Index: clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
@@ -6,8 +6,11 @@
 //
 //===--===//
 //
-// This defines PthreadLockChecker, a simple lock -> unlock checker.
-// Also handles XNU locks, which behave similarly enough to share code.
+// This file defines:
+//  * PthreadLockChecker, a simple lock -> unlock checker.
+//Which also checks for XNU locks, which behave similarly enough to share
+//code.
+//  * FuchsiaLocksChecker, which is also rather similar.
 //
 //===--===//
 
@@ -15,8 +18,8 @@
 #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
 #include "clang/StaticAnalyzer/Core/Checker.h"
 #include "clang/StaticAnalyzer/Core/CheckerManager.h"
-#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
 
 using namespace clang;
 using namespace ento;
@@ -46,9 +49,7 @@
 return LockState(UnlockedAndPossiblyDestroyed);
   }
 
-  bool operator==(const LockState ) const {
-return K == X.K;
-  }
+  bool operator==(const LockState ) const { return K == X.K; }
 
   bool isLocked() const { return K == Locked; }
   bool isUnlocked() const { return K == Unlocked; }
@@ -60,98 +61,148 @@
 return K == UnlockedAndPossiblyDestroyed;
   }
 
-  void Profile(llvm::FoldingSetNodeID ) const {
-ID.AddInteger(K);
-  }
+  void Profile(llvm::FoldingSetNodeID ) const { ID.AddInteger(K); }
 };
 
-class PthreadLockChecker
-: public Checker {
-  BugType BT_doublelock{this, "Double locking", "Lock checker"},
-  BT_doubleunlock{this, "Double unlocking", "Lock checker"},
-  BT_destroylock{this, "Use destroyed lock", "Lock checker"},
-  BT_initlock{this, "Init invalid lock", "Lock 

[PATCH] D73037: Add a way to set traversal mode in clang-query

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

There should also be a mention of this in the release notes (especially if the 
default behavior winds up changing).




Comment at: clang-tools-extra/clang-query/Query.cpp:51
+"IgnoreImplicitCastsAndParentheses"
+"  Omit implicit casts and parens in matching and dumping.\n"
+"IgnoreUnlessSpelledInSource "

It took me a few tries to understand why there's a leading whitespace here. I'd 
prefer this to be a trailing whitespace on the previous line as in other cases 
unless there's some reason we need all the trailing quotes to line up 
vertically.



Comment at: clang-tools-extra/clang-query/Query.cpp:53
+"IgnoreUnlessSpelledInSource "
+"Omit AST nodes unless spelled in the source.\n"
 "  set output   "

We should document explicitly that this is the default (or document `AsIs` as 
the default if we go that route for now).



Comment at: clang-tools-extra/clang-query/QuerySession.h:29
 DetailedASTOutput(false), BindRoot(true), PrintMatcher(false),
-Terminate(false) {}
+Terminate(false), TK(ast_type_traits::TK_IgnoreUnlessSpelledInSource) 
{}
 

While we want to get here eventually, I think it might make sense to have the 
default be `AsIs` until there's clear agreement that we want the default 
traversal mode in AST matchers to be this way. Basically, clang-query's default 
mode should be the same as the C++ AST matcher default mode.



Comment at: clang-tools-extra/unittests/clang-query/QueryParserTest.cpp:114
+
+  Q = parse("set traversal AsIs");
+  ASSERT_TRUE(isa >(Q));

Can you also add a test for `set traversal ThisShouldNotWork` or something 
along those lines?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73037



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

[clang-tools-extra] 58592f6 - Include for std::abort() in clangd

[Dimitry Andric via cfe-commits]

Author: Dimitry Andric
Date: 2020-01-24T20:52:37+01:00
New Revision: 58592f6c49249293f79698cfcb31dba532e12603

URL: 
https://github.com/llvm/llvm-project/commit/58592f6c49249293f79698cfcb31dba532e12603
DIFF: 
https://github.com/llvm/llvm-project/commit/58592f6c49249293f79698cfcb31dba532e12603.diff

LOG: Include  for std::abort() in clangd

This fixes a "not a member of 'std'" error with e.g. Fedora 32.

Closes: #105

Added: 


Modified: 
clang-tools-extra/clangd/Shutdown.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/Shutdown.cpp 
b/clang-tools-extra/clangd/Shutdown.cpp
index dfea46d8dfeb..36d977570a4f 100644
--- a/clang-tools-extra/clangd/Shutdown.cpp
+++ b/clang-tools-extra/clangd/Shutdown.cpp
@@ -9,6 +9,7 @@
 #include "Shutdown.h"
 
 #include 
+#include 
 #include 
 
 namespace clang {



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

[PATCH] D73028: Extract ExprTraversal class from Expr

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

rsmith added a comment.

> An addition to the API will be concerned with ascending through the AST in 
> different traversal modes.

Ascending through the AST is not possibly by design. (For example, we share AST 
nodes between template instantiations, and statement nodes don't contain parent 
pointers.) Can you say more about what you're thinking of adding here?




Comment at: clang/include/clang/AST/ExprTraversal.h:1
+//===--- Expr.h - Classes for representing expressions --*- C++ 
-*-===//
+//

This needs an update.



Comment at: clang/lib/AST/ExprTraversal.cpp:1
+//===--- ExprTraversal.cpp - Expression AST Node Implementation 
---===//
+//

This header should briefly describe the purpose of this file. ("Simple 
expression traversal"?)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73028



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

[PATCH] D73029: Extend ExprTraversal class with acending traversal

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Test cases?




Comment at: clang/lib/AST/ASTContext.cpp:1066
+if (Node.getNodeKind().hasPointerIdentity()) {
+  auto ParentList =
+  getDynNodeFromMap(Node.getMemoizationData(), PointerParents);

Please spell the type out instead of using `auto`.



Comment at: clang/lib/AST/ASTContext.cpp:1070
+  TK == ast_type_traits::TK_IgnoreUnlessSpelledInSource) {
+const Expr *E = ParentList[0].get();
+const Expr *Child = Node.get();

`const auto *` (same below) because the type is spelled out in the 
initialization.



Comment at: clang/lib/AST/ASTContext.cpp:1086
+break;
+  auto *S = It->second.dyn_cast();
+  if (!S)

`const auto *` so we don't have to infer the constness (same below with `P`).



Comment at: clang/lib/AST/ExprTraversal.cpp:278
+
+  if (auto *C = dyn_cast(E)) {
+if (C->getSourceRange() == SR || !isa(C))

`const auto *` here and elsewhere


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73029



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

[PATCH] D70926: [clang-format] Add option for not breaking line before ObjC params

[Kanglei Fang via Phabricator via cfe-commits]

ghvg1313 updated this revision to Diff 240260.
ghvg1313 marked an inline comment as done.
ghvg1313 added a comment.



- rebase master


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70926

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/Format.cpp
  clang/unittests/Format/FormatTestObjC.cpp

Index: clang/unittests/Format/FormatTestObjC.cpp
===
--- clang/unittests/Format/FormatTestObjC.cpp
+++ clang/unittests/Format/FormatTestObjC.cpp
@@ -1398,6 +1398,37 @@
   "}");
 }
 
+TEST_F(FormatTestObjC, BreakLineBeforeNestedBlockParam) {
+  Style = getGoogleStyle(FormatStyle::LK_ObjC);
+  Style.ObjCBreakBeforeNestedBlockParam = false;
+  Style.ColumnLimit = 0;
+
+  verifyFormat("[self.test1 t:self callback:^(typeof(self) self, NSNumber *u, "
+   "NSNumber *v) {\n"
+   "  u = v;\n"
+   "}]");
+
+  verifyFormat("[self.test1 t:self w:self callback:^(typeof(self) self, "
+   "NSNumber *u, NSNumber *v) {\n"
+   "  u = v;\n"
+   "}]");
+
+  verifyFormat("[self.test1 t:self w:self callback:^(typeof(self) self, "
+   "NSNumber *u, NSNumber *v) {\n"
+   "  u = c;\n"
+   "} w:self callback2:^(typeof(self) self, NSNumber *a, NSNumber "
+   "*b, NSNumber *c) {\n"
+   "  b = c;\n"
+   "}]");
+
+  Style.ColumnLimit = 80;
+  verifyFormat(
+  "[self.test_method a:self b:self\n"
+  "   callback:^(typeof(self) self, NSNumber *u, NSNumber *v) {\n"
+  " u = v;\n"
+  "   }]");
+}
+
 } // end namespace
 } // end namespace format
 } // end namespace clang
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -498,6 +498,8 @@
 IO.mapOptional("NamespaceMacros", Style.NamespaceMacros);
 IO.mapOptional("ObjCBinPackProtocolList", Style.ObjCBinPackProtocolList);
 IO.mapOptional("ObjCBlockIndentWidth", Style.ObjCBlockIndentWidth);
+IO.mapOptional("ObjCBreakBeforeNestedBlockParam",
+   Style.ObjCBreakBeforeNestedBlockParam);
 IO.mapOptional("ObjCSpaceAfterProperty", Style.ObjCSpaceAfterProperty);
 IO.mapOptional("ObjCSpaceBeforeProtocolList",
Style.ObjCSpaceBeforeProtocolList);
@@ -796,6 +798,7 @@
   LLVMStyle.NamespaceIndentation = FormatStyle::NI_None;
   LLVMStyle.ObjCBinPackProtocolList = FormatStyle::BPS_Auto;
   LLVMStyle.ObjCBlockIndentWidth = 2;
+  LLVMStyle.ObjCBreakBeforeNestedBlockParam = true;
   LLVMStyle.ObjCSpaceAfterProperty = false;
   LLVMStyle.ObjCSpaceBeforeProtocolList = true;
   LLVMStyle.PointerAlignment = FormatStyle::PAS_Right;
Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -861,8 +861,10 @@
   // Any break on this level means that the parent level has been broken
   // and we need to avoid bin packing there.
   bool NestedBlockSpecialCase =
-  !Style.isCpp() && Current.is(tok::r_brace) && State.Stack.size() > 1 &&
-  State.Stack[State.Stack.size() - 2].NestedBlockInlined;
+  (!Style.isCpp() && Current.is(tok::r_brace) && State.Stack.size() > 1 &&
+   State.Stack[State.Stack.size() - 2].NestedBlockInlined) ||
+  (Style.Language == FormatStyle::LK_ObjC && Current.is(tok::r_brace) &&
+   State.Stack.size() > 1 && !Style.ObjCBreakBeforeNestedBlockParam);
   if (!NestedBlockSpecialCase)
 for (unsigned i = 0, e = State.Stack.size() - 1; i != e; ++i)
   State.Stack[i].BreakBeforeParameter = true;
@@ -1380,7 +1382,8 @@
   (!BinPackInconclusiveFunctions &&
Current.PackingKind == PPK_Inconclusive)));
 
-if (Current.is(TT_ObjCMethodExpr) && Current.MatchingParen) {
+if (Current.is(TT_ObjCMethodExpr) && Current.MatchingParen &&
+Style.ObjCBreakBeforeNestedBlockParam) {
   if (Style.ColumnLimit) {
 // If this '[' opens an ObjC call, determine whether all parameters fit
 // into one line and put one per line if they don't.
Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -1669,6 +1669,29 @@
   /// ``@property (readonly)`` instead of ``@property(readonly)``.
   bool ObjCSpaceAfterProperty;
 
+  /// Break parameters list into lines when there is nested block
+  /// parameters in a fuction call.
+  /// \code
+  ///   false:
+  ///- (void)_aMethod
+  ///{
+  ///[self.test1 t:self w:self 

[PATCH] D73028: Extract ExprTraversal class from Expr

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

This looks like it's an NFC patch where the only change is to hoist this 
functionality out into its own interface. Is that correct? If so, can you 
please be sure to add NFC to the commit log?




Comment at: clang/include/clang/AST/ExprTraversal.h:9
+//
+//  This file defines the ExprTraversal class.
+//

Some comments explaining the purpose of the file would be appreciated.



Comment at: clang/include/clang/AST/ExprTraversal.h:21
+
+struct ExprTraversal {
+  static Expr *DescendIgnoreImpCasts(Expr *E);

Any particular reason why this is a `struct` rather than a `namespace` given 
that there's no state?



Comment at: clang/include/clang/AST/ExprTraversal.h:22
+struct ExprTraversal {
+  static Expr *DescendIgnoreImpCasts(Expr *E);
+  static Expr *DescendIgnoreCasts(Expr *E);

Should we be adding some documentation comments for these APIs?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73028



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

[PATCH] D70926: [clang-format] Add option for not breaking line before ObjC params

[Kanglei Fang via Phabricator via cfe-commits]

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



Comment at: clang/docs/ReleaseNotes.rst:164
+
+- Clang-format now supports JavaScript null operators.
+

MyDeveloperDay wrote:
> is this line part of this patch?
No, rebased again and got rid of it


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70926



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

[clang] 698d1cd - Make address-space-lambda.cl pass on 32-bit Windows

[Hans Wennborg via cfe-commits]

Author: Hans Wennborg
Date: 2020-01-24T20:35:25+01:00
New Revision: 698d1cd3b8154b3b74423386d3e111e6b756e87a

URL: 
https://github.com/llvm/llvm-project/commit/698d1cd3b8154b3b74423386d3e111e6b756e87a
DIFF: 
https://github.com/llvm/llvm-project/commit/698d1cd3b8154b3b74423386d3e111e6b756e87a.diff

LOG: Make address-space-lambda.cl pass on 32-bit Windows

Member functions will have the thiscall attribute on them.

Added: 


Modified: 
clang/test/SemaOpenCLCXX/address-space-lambda.cl

Removed: 




diff  --git a/clang/test/SemaOpenCLCXX/address-space-lambda.cl 
b/clang/test/SemaOpenCLCXX/address-space-lambda.cl
index eeea71e6353f..e953817442f7 100644
--- a/clang/test/SemaOpenCLCXX/address-space-lambda.cl
+++ b/clang/test/SemaOpenCLCXX/address-space-lambda.cl
@@ -1,16 +1,16 @@
 //RUN: %clang_cc1 %s -cl-std=clc++ -pedantic -ast-dump -verify | FileCheck %s
 
-//CHECK: CXXMethodDecl {{.*}} constexpr operator() 'int (__private int) const 
__generic'
+//CHECK: CXXMethodDecl {{.*}} constexpr operator() 'int (__private int){{.*}} 
const __generic'
 auto glambda = [](auto a) { return a; };
 
 __kernel void test() {
   int i;
-//CHECK: CXXMethodDecl {{.*}} constexpr operator() 'void () const __generic'
+//CHECK: CXXMethodDecl {{.*}} constexpr operator() 'void () {{.*}}const 
__generic'
   auto  llambda = [&]() {i++;};
   llambda();
   glambda(1);
   // Test lambda with default parameters
-//CHECK: CXXMethodDecl {{.*}} constexpr operator() 'void () const __generic'
+//CHECK: CXXMethodDecl {{.*}} constexpr operator() 'void () {{.*}}const 
__generic'
   [&] {i++;} ();
   __constant auto err = [&]() {}; //expected-note{{candidate function not 
viable: 'this' object is in address space '__constant', but method expects 
object in address space '__generic'}}
   err();  //expected-error-re{{no matching function 
for call to object of type '__constant (lambda at {{.*}})'}}
@@ -25,10 +25,10 @@ __kernel void test() {
 }
 
 __kernel void test_qual() {
-//CHECK: |-CXXMethodDecl {{.*}} constexpr operator() 'void () const __private'
+//CHECK: |-CXXMethodDecl {{.*}} constexpr operator() 'void () {{.*}}const 
__private'
   auto priv1 = []() __private {};
   priv1();
-//CHECK: |-CXXMethodDecl {{.*}} constexpr operator() 'void () const __generic'
+//CHECK: |-CXXMethodDecl {{.*}} constexpr operator() 'void () {{.*}}const 
__generic'
   auto priv2 = []() __generic {};
   priv2();
   auto priv3 = []() __global {}; //expected-note{{candidate function not 
viable: 'this' object is in address space '__private', but method expects 
object in address space '__global'}} //expected-note{{conversion candidate of 
type 'void (*)()'}}
@@ -38,7 +38,7 @@ __kernel void test_qual() {
   const1(); //expected-error{{no matching function for call to object of type 
'__constant (lambda at}}
   __constant auto const2 = []() __generic{}; //expected-note{{candidate 
function not viable: 'this' object is in address space '__constant', but method 
expects object in address space '__generic'}} //expected-note{{conversion 
candidate of type 'void (*)()'}}
   const2(); //expected-error{{no matching function for call to object of type 
'__constant (lambda at}}
-//CHECK: |-CXXMethodDecl {{.*}} constexpr operator() 'void () const __constant'
+//CHECK: |-CXXMethodDecl {{.*}} constexpr operator() 'void () {{.*}}const 
__constant'
   __constant auto const3 = []() __constant{};
   const3();
 



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

[PATCH] D73307: Unique Names for Functions with Internal Linkage

[Sriraman Tallam via Phabricator via cfe-commits]

tmsriram updated this revision to Diff 240253.
tmsriram added a comment.

Remove an unnecessary header file inclusion.


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

https://reviews.llvm.org/D73307

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/unique_internal_funcnames.c

Index: clang/test/CodeGen/unique_internal_funcnames.c
===
--- /dev/null
+++ clang/test/CodeGen/unique_internal_funcnames.c
@@ -0,0 +1,17 @@
+// REQUIRES: x86-registered-target
+
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -S -o - < %s | FileCheck %s --check-prefix=PLAIN
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -S -funique-internal-funcnames -fno-unique-internal-funcnames -o - < %s | FileCheck %s --check-prefix=PLAIN
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -S -funique-internal-funcnames -o - < %s | FileCheck %s --check-prefix=UNIQUE
+
+static int foo() {
+  return 0;
+}
+
+int (*bar())() {
+  return foo;
+}
+
+// PLAIN: foo:
+// UNIQUE-NOT: foo:
+// UNIQUE: foo.${{[0-9a-f]+}}:
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -964,6 +964,9 @@
   Opts.UniqueSectionNames = Args.hasFlag(OPT_funique_section_names,
  OPT_fno_unique_section_names, true);
 
+  Opts.UniqueInternalFuncNames = Args.hasFlag(
+  OPT_funique_internal_funcnames, OPT_fno_unique_internal_funcnames, false);
+
   Opts.MergeFunctions = Args.hasArg(OPT_fmerge_functions);
 
   Opts.NoUseJumpTables = Args.hasArg(OPT_fno_jump_tables);
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4153,6 +4153,8 @@
 options::OPT_fno_function_sections,
 options::OPT_fdata_sections,
 options::OPT_fno_data_sections,
+options::OPT_funique_internal_funcnames,
+options::OPT_fno_unique_internal_funcnames,
 options::OPT_funique_section_names,
 options::OPT_fno_unique_section_names,
 options::OPT_mrestrict_it,
@@ -4695,6 +4697,10 @@
 options::OPT_fno_unique_section_names, true))
 CmdArgs.push_back("-fno-unique-section-names");
 
+  if (Args.hasFlag(options::OPT_funique_internal_funcnames,
+   options::OPT_fno_unique_internal_funcnames, false))
+CmdArgs.push_back("-funique-internal-funcnames");
+
   Args.AddLastArg(CmdArgs, options::OPT_finstrument_functions,
   options::OPT_finstrument_functions_after_inlining,
   options::OPT_finstrument_function_entry_bare);
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -1101,6 +1101,23 @@
   const auto *ND = cast(GD.getDecl());
   std::string MangledName = getMangledNameImpl(*this, GD, ND);
 
+  // With option -funique-internal-funcnames, functions with internal linkage
+  // should get unique names.  Use "getUniqueModuleId" to get a unique
+  // identifier and this is a best effort.
+  if (getCodeGenOpts().UniqueInternalFuncNames &&
+  dyn_cast(GD.getDecl()) &&
+  this->getFunctionLinkage(GD) == llvm::GlobalValue::InternalLinkage &&
+  !getModule().getSourceFileName().empty()) {
+llvm::MD5 Md5;
+Md5.update(getModule().getSourceFileName());
+llvm::MD5::MD5Result R;
+Md5.final(R);
+SmallString<32> Str;
+llvm::MD5::stringifyResult(R, Str);
+std::string UniqueSuffix = (".$" + Str).str();
+MangledName = MangledName + UniqueSuffix;
+  }
+
   // Adjust kernel stub mangling as we may need to be able to differentiate
   // them from the kernel itself (e.g., for HIP).
   if (auto *FD = dyn_cast(GD.getDecl()))
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -1952,6 +1952,12 @@
 def fno_unique_section_names : Flag <["-"], "fno-unique-section-names">,
   Group, Flags<[CC1Option]>;
 
+def funique_internal_funcnames : Flag <["-"], "funique-internal-funcnames">,
+  Group, Flags<[CC1Option]>,
+  HelpText<"Uniqueify Internal Linkage Function Names">;
+def fno_unique_internal_funcnames : Flag <["-"], "fno-unique-internal-funcnames">,
+  Group, Flags<[CC1Option]>;
+
 def fstrict_return : Flag<["-"], "fstrict-return">, Group,
   Flags<[CC1Option]>,
   HelpText<"Always treat control flow paths that fall off the end of a "
Index: 

[PATCH] D73307: Unique Names for Functions with Internal Linkage

[Sriraman Tallam via Phabricator via cfe-commits]

tmsriram updated this revision to Diff 240249.
tmsriram added a comment.

Use Hash of Module's source file name directly.


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

https://reviews.llvm.org/D73307

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/unique_internal_funcnames.c

Index: clang/test/CodeGen/unique_internal_funcnames.c
===
--- /dev/null
+++ clang/test/CodeGen/unique_internal_funcnames.c
@@ -0,0 +1,17 @@
+// REQUIRES: x86-registered-target
+
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -S -o - < %s | FileCheck %s --check-prefix=PLAIN
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -S -funique-internal-funcnames -fno-unique-internal-funcnames -o - < %s | FileCheck %s --check-prefix=PLAIN
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -S -funique-internal-funcnames -o - < %s | FileCheck %s --check-prefix=UNIQUE
+
+static int foo() {
+  return 0;
+}
+
+int (*bar())() {
+  return foo;
+}
+
+// PLAIN: foo:
+// UNIQUE-NOT: foo:
+// UNIQUE: foo.${{[0-9a-f]+}}:
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -964,6 +964,9 @@
   Opts.UniqueSectionNames = Args.hasFlag(OPT_funique_section_names,
  OPT_fno_unique_section_names, true);
 
+  Opts.UniqueInternalFuncNames = Args.hasFlag(
+  OPT_funique_internal_funcnames, OPT_fno_unique_internal_funcnames, false);
+
   Opts.MergeFunctions = Args.hasArg(OPT_fmerge_functions);
 
   Opts.NoUseJumpTables = Args.hasArg(OPT_fno_jump_tables);
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4153,6 +4153,8 @@
 options::OPT_fno_function_sections,
 options::OPT_fdata_sections,
 options::OPT_fno_data_sections,
+options::OPT_funique_internal_funcnames,
+options::OPT_fno_unique_internal_funcnames,
 options::OPT_funique_section_names,
 options::OPT_fno_unique_section_names,
 options::OPT_mrestrict_it,
@@ -4695,6 +4697,10 @@
 options::OPT_fno_unique_section_names, true))
 CmdArgs.push_back("-fno-unique-section-names");
 
+  if (Args.hasFlag(options::OPT_funique_internal_funcnames,
+   options::OPT_fno_unique_internal_funcnames, false))
+CmdArgs.push_back("-funique-internal-funcnames");
+
   Args.AddLastArg(CmdArgs, options::OPT_finstrument_functions,
   options::OPT_finstrument_functions_after_inlining,
   options::OPT_finstrument_function_entry_bare);
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -61,6 +61,7 @@
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/MD5.h"
 #include "llvm/Support/TimeProfiler.h"
+#include "llvm/Transforms/Utils/ModuleUtils.h"
 
 using namespace clang;
 using namespace CodeGen;
@@ -1101,6 +1102,23 @@
   const auto *ND = cast(GD.getDecl());
   std::string MangledName = getMangledNameImpl(*this, GD, ND);
 
+  // With option -funique-internal-funcnames, functions with internal linkage
+  // should get unique names.  Use "getUniqueModuleId" to get a unique
+  // identifier and this is a best effort.
+  if (getCodeGenOpts().UniqueInternalFuncNames &&
+  dyn_cast(GD.getDecl()) &&
+  this->getFunctionLinkage(GD) == llvm::GlobalValue::InternalLinkage &&
+  !getModule().getSourceFileName().empty()) {
+llvm::MD5 Md5;
+Md5.update(getModule().getSourceFileName());
+llvm::MD5::MD5Result R;
+Md5.final(R);
+SmallString<32> Str;
+llvm::MD5::stringifyResult(R, Str);
+std::string UniqueSuffix = (".$" + Str).str();
+MangledName = MangledName + UniqueSuffix;
+  }
+
   // Adjust kernel stub mangling as we may need to be able to differentiate
   // them from the kernel itself (e.g., for HIP).
   if (auto *FD = dyn_cast(GD.getDecl()))
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -1952,6 +1952,12 @@
 def fno_unique_section_names : Flag <["-"], "fno-unique-section-names">,
   Group, Flags<[CC1Option]>;
 
+def funique_internal_funcnames : Flag <["-"], "funique-internal-funcnames">,
+  Group, Flags<[CC1Option]>,
+  HelpText<"Uniqueify Internal Linkage Function Names">;
+def fno_unique_internal_funcnames : Flag <["-"], "fno-unique-internal-funcnames">,
+  

[PATCH] D72841: [RFC] Add support for pragma float_control, to control precision and exception behavior at the source level

[Melanie Blower via Phabricator via cfe-commits]

mibintc marked 3 inline comments as done.
mibintc added a comment.

In D72841#1833022 , @sepavloff wrote:

> I don't see tests for correctness of the pragma stack (`pragma 
> float_control(... push)`, `pragma float_control(pop)`). Can you add them?


I added a test case for the stack of float_control, and it's also hitting the 
problem with the function scope not being closed by the right brace. This patch 
is useless until I tackle that problem.  In the test case I put in bogus 
declarations which force the function scope to close.  There are some pragmas 
called "range pragmas" that affect the functions in a source location range in 
the file. I don't know if I could mix that idea with the MS pragma stack that 
I'm using to push/pop the pragma settings, I'm doubtful.




Comment at: clang/lib/Sema/SemaExpr.cpp:13129
 if (FunctionDecl *F = dyn_cast(CurContext)) {
+  // If the expression occurs inside an internal global_var_init_function
+  // then the FunctionDecl is not availble

mibintc wrote:
> sepavloff wrote:
> > mibintc wrote:
> > > sepavloff wrote:
> > > > mibintc wrote:
> > > > > sepavloff wrote:
> > > > > > The standard says that static initializers execute in default FP 
> > > > > > mode.
> > > > > > The standard says ...
> > > > > Are you sure about this one?  Can you please provide the standards 
> > > > > reference so I can study it?
> > > > >> The standard says ...
> > > > > Are you sure about this one? Can you please provide the standards 
> > > > > reference so I can study it?
> > > > 
> > > > http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1570.pdf, F.8.5:
> > > > ```
> > > > ... All computation for initialization of objects that have static or 
> > > > thread storage duration is done (as if) at translation time.
> > > > ```
> > > > F.8.2:
> > > > ```
> > > > During translation the IEC 60559 default modes are in effect:
> > > > — The rounding direction mode is rounding to nearest.
> > > > — The rounding precision mode (if supported) is set so that results are 
> > > > not shortened.
> > > > — Trapping or stopping (if supported) is disabled on all floating-point 
> > > > exceptions.
> > > > ```
> > > Thanks for the pointer to the reference. The desired semantics of the 
> > > pragma may differ from the standard. For example I tried this test case 
> > > with the fp_contract pragma, and the pragma does modify the semantics of 
> > > the floating point expressions in the initializer.  So this issue is 
> > > still a problem in this patch. 
> > > 
> > > // RUN: %clang_cc1 -emit-llvm -o - %s | FileCheck %s
> > > 
> > > #pragma STDC FP_CONTRACT ON
> > > float y();
> > > float d();
> > > class ON {
> > > float z = y() * 1 + d();
> > > // CHECK-LABEL: define {{.*}} void @_ZN2ONC2Ev{{.*}}
> > > //CHECK: llvm.fmuladd.f32{{.*}}
> > > };
> > > ON on;
> > > 
> > > #pragma STDC FP_CONTRACT OFF
> > > class OFF {
> > > float w = y() * 1 + d();
> > > // CHECK-LABEL: define {{.*}} void @_ZN3OFFC2Ev{{.*}}
> > > //CHECK: fmul float
> > > };
> > > OFF off;
> > > 
> > This is an interesting example. However there is no contradiction with the 
> > standard. The standard speaks about floating point environment, which on 
> > most processors are represented by bits of some register(s). The pragma 
> > `STDC FP_CONTRACT` does not refer to the FPEnv, but is an instruction to 
> > the compiler how to generate code, so it affects code generation even in 
> > global var initializers.
> > 
> > What `TBD` here means? Do you think this code may be somehow improved?
> I wasn't yet certain about the interpretation of the pragma on the 
> initializatin expressions. Today I did some testing with ICL and CL and it 
> seems the pragma has no effect on the initialization expressions that occur 
> within constructors in classes at file scope.  So I'll remove the TBD and the 
> current behavior in this patch wrt this question is OK.
I removed the TBD, thanks. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72841



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

[PATCH] D72906: [X86] Improve X86 cmpps/cmppd/cmpss/cmpsd intrinsics with strictfp

[Craig Topper via Phabricator via cfe-commits]

craig.topper added a comment.

In D72906#1838532 , @uweigand wrote:

> In D72906#1837905 , @craig.topper 
> wrote:
>
> > In D72906#1826849 , @uweigand 
> > wrote:
> >
> > > In D72906#1826122 , 
> > > @craig.topper wrote:
> > >
> > > > In D72906#1826061 , @uweigand 
> > > > wrote:
> > > >
> > > > > > The constrained fcmp intrinsics don't allow the TRUE/FALSE 
> > > > > > predicates.
> > > > >
> > > > > Hmm, maybe they should then?   The only reason I didn't add them 
> > > > > initially was that I wasn't sure they were useful for anything; if 
> > > > > they are, it should be straightforward to add them back.
> > > >
> > > >
> > > > What would we lower it to on a target that doesn’t support it natively?
> > >
> > >
> > > Any supported compare (quiet or signaling as appropriate, just so we get 
> > > the correct exceptions), and then ignore the result (and use true/false 
> > > constant result instead)?
> >
> >
> > Sure. Is that something we want to force all targets to have to implement 
> > just to handle this case for X86? Unless we can come up with a generic DAG 
> > combine to pick a valid condition alternate so that the lowering code for 
> > each target doesn't have to deal with it.
>
>
> Hmm, OK.  Given that this X86-specific builtin code seems to be the only 
> place in clang where FCMP_TRUE/FCMP_FALSE can ever get emitted anyway, I 
> guess you might as well implement the workaround (use some other compare to 
> get the exception) right there, just for X86.


I think I'd like to do that as a follow up. This patch as is will at least fix 
the assertion failure and get the signalling behavior correct for the non- 
TRUE/FALSE intrinsics. These are going to be far more common to use so fixing 
those seems like the priority.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72906



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

[PATCH] D72841: [RFC] Add support for pragma float_control, to control precision and exception behavior at the source level

[Melanie Blower via Phabricator via cfe-commits]

mibintc updated this revision to Diff 240245.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72841

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/AST/Stmt.h
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Basic/LangOptions.h
  clang/include/clang/Basic/PragmaKinds.h
  clang/include/clang/Basic/TokenKinds.def
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/Sema.h
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/Parse/ParsePragma.cpp
  clang/lib/Parse/ParseStmt.cpp
  clang/lib/Parse/Parser.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaAttr.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/CodeGen/fp-floatcontrol-class.cpp
  clang/test/CodeGen/fp-floatcontrol-pragma.cpp
  clang/test/CodeGen/fp-floatcontrol-stack.cpp
  clang/test/Parser/fp-floatcontrol-syntax.cpp
  llvm/include/llvm/IR/IRBuilder.h

Index: llvm/include/llvm/IR/IRBuilder.h
===
--- llvm/include/llvm/IR/IRBuilder.h
+++ llvm/include/llvm/IR/IRBuilder.h
@@ -299,10 +299,16 @@
 IRBuilderBase 
 FastMathFlags FMF;
 MDNode *FPMathTag;
+bool IsFPConstrained;
+fp::ExceptionBehavior DefaultConstrainedExcept;
+fp::RoundingMode DefaultConstrainedRounding;
 
   public:
 FastMathFlagGuard(IRBuilderBase )
-: Builder(B), FMF(B.FMF), FPMathTag(B.DefaultFPMathTag) {}
+: Builder(B), FMF(B.FMF), FPMathTag(B.DefaultFPMathTag),
+  IsFPConstrained(B.IsFPConstrained),
+  DefaultConstrainedExcept(B.DefaultConstrainedExcept),
+  DefaultConstrainedRounding(B.DefaultConstrainedRounding) {}
 
 FastMathFlagGuard(const FastMathFlagGuard &) = delete;
 FastMathFlagGuard =(const FastMathFlagGuard &) = delete;
@@ -310,6 +316,9 @@
 ~FastMathFlagGuard() {
   Builder.FMF = FMF;
   Builder.DefaultFPMathTag = FPMathTag;
+  Builder.IsFPConstrained = IsFPConstrained;
+  Builder.DefaultConstrainedExcept = DefaultConstrainedExcept;
+  Builder.DefaultConstrainedRounding = DefaultConstrainedRounding;
 }
   };
 
Index: clang/test/Parser/fp-floatcontrol-syntax.cpp
===
--- /dev/null
+++ clang/test/Parser/fp-floatcontrol-syntax.cpp
@@ -0,0 +1,25 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -DCHECK_ERROR %s
+// XFAIL: *
+
+float function_scope(float a) {
+  return a;
+}
+
+// There seems to be a bug in Actions.CurContext->isTranslationUnit()
+// unless this dummy class is used. FIXME Fix the issue then remove this
+// workaround.
+#define TU_WORKAROUND
+#ifdef TU_WORKAROUND
+class ResetTUScope;
+#endif
+#ifdef CHECK_ERROR
+#  pragma float_control(push)
+#  pragma float_control(pop)
+#  pragma float_control(precise,on,push)
+void check_stack() {
+#pragma float_control(push) // expected-error {{can only appear at file scope}}
+#pragma float_control(pop) // expected-error {{can only appear at file scope}}
+#pragma float_control(precise,on,push) // expected-error {{can only appear at file scope}}
+  return;
+}
+#endif
Index: clang/test/CodeGen/fp-floatcontrol-stack.cpp
===
--- /dev/null
+++ clang/test/CodeGen/fp-floatcontrol-stack.cpp
@@ -0,0 +1,171 @@
+// RUN: %clang_cc1 -DDEFAULT=1 -emit-llvm -o - %s | FileCheck --check-prefix=CHECK-DDEFAULT %s
+// RUN: %clang_cc1 -DEBSTRICT=1 -ffp-exception-behavior=strict -emit-llvm -o - %s | FileCheck --check-prefix=CHECK-DEBSTRICT %s
+
+#define FUN(n) (float z) { return n * z + n; }
+
+float fun_default FUN(1)
+//CHECK-LABEL: define {{.*}} @_Z11fun_defaultf{{.*}}
+#if DEFAULT
+//CHECK-DDEFAULT: fmul float
+//CHECK-DDEFAULT: fadd float
+#endif
+#if EBSTRICT
+// Note that backend wants constrained intrinsics used "everywhere"
+// if they have been requested on the command line, so fp operations
+// will be built with constrained intrinsics and default settings
+// for exception behavior and rounding mode.
+//CHECK-DEBSTRICT: llvm.experimental.constrained.fmul{{.*}}tonearest{{.*}}strict
+#endif
+class ResetScope;
+
+#pragma float_control(except, on, push)
+float exc_on FUN(2)
+//CHECK-LABEL: define {{.*}} @_Z6exc_onf{{.*}}
+#if DEFAULT
+//CHECK-DDEFAULT: llvm.experimental.constrained.fmul{{.*}}
+#endif
+#if EBSTRICT
+//CHECK-DEBSTRICT: llvm.experimental.constrained.fmul{{.*}}tonearest{{.*}}strict
+#endif
+
+class ResetScope;
+#pragma float_control(pop)
+float exc_pop FUN(5)
+//CHECK-LABEL: define {{.*}} @_Z7exc_popf{{.*}}
+#if DEFAULT
+//CHECK-DDEFAULT: fmul float
+//CHECK-DDEFAULT: fadd float
+#endif
+#if EBSTRICT
+//CHECK-DEBSTRICT: llvm.experimental.constrained.fmul{{.*}}tonearest{{.*}}strict
+#endif
+
+class ResetScope;
+#pragma float_control(except, off)
+float exc_off FUN(5)
+//CHECK-LABEL: define {{.*}} 

[PATCH] D72467: Remove "mask" operand from shufflevector.

[Sanjay Patel via Phabricator via cfe-commits]

spatel added a comment.

In D72467#1839172 , @efriedma wrote:

> In D72467#1838591 , @spatel wrote:
>
> > LGTM, but should get a 2nd opinion since I'm not familiar with some of the 
> > parts.
>
>
> Any specific part you're worried about?


I don't know anything about the scalable vector enhancements, but that seems 
covered now. Nuances of the bitcode serialization are beyond me, but I trust 
you've stepped through that. So, no worries. :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72467



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

[PATCH] D71823: Support Swift calling convention for WebAssembly targets

[Derek Schuff via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGc5bd3d07262f: Support Swift calling convention for 
WebAssembly targets (authored by kateinoigakukun, committed by dschuff).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71823

Files:
  clang/lib/Basic/Targets/WebAssembly.h
  llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp


Index: llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
===
--- llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
+++ llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
@@ -672,7 +672,8 @@
  CallConv == CallingConv::PreserveMost ||
  CallConv == CallingConv::PreserveAll ||
  CallConv == CallingConv::CXX_FAST_TLS ||
- CallConv == CallingConv::WASM_EmscriptenInvoke;
+ CallConv == CallingConv::WASM_EmscriptenInvoke ||
+ CallConv == CallingConv::Swift;
 }
 
 SDValue
Index: clang/lib/Basic/Targets/WebAssembly.h
===
--- clang/lib/Basic/Targets/WebAssembly.h
+++ clang/lib/Basic/Targets/WebAssembly.h
@@ -114,6 +114,16 @@
? (IsSigned ? SignedLongLong : UnsignedLongLong)
: TargetInfo::getLeastIntTypeByWidth(BitWidth, IsSigned);
   }
+
+  CallingConvCheckResult checkCallingConvention(CallingConv CC) const override 
{
+switch (CC) {
+case CC_C:
+case CC_Swift:
+  return CCCR_OK;
+default:
+  return CCCR_Warning;
+}
+  }
 };
 class LLVM_LIBRARY_VISIBILITY WebAssembly32TargetInfo
 : public WebAssemblyTargetInfo {


Index: llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
===
--- llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
+++ llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
@@ -672,7 +672,8 @@
  CallConv == CallingConv::PreserveMost ||
  CallConv == CallingConv::PreserveAll ||
  CallConv == CallingConv::CXX_FAST_TLS ||
- CallConv == CallingConv::WASM_EmscriptenInvoke;
+ CallConv == CallingConv::WASM_EmscriptenInvoke ||
+ CallConv == CallingConv::Swift;
 }
 
 SDValue
Index: clang/lib/Basic/Targets/WebAssembly.h
===
--- clang/lib/Basic/Targets/WebAssembly.h
+++ clang/lib/Basic/Targets/WebAssembly.h
@@ -114,6 +114,16 @@
? (IsSigned ? SignedLongLong : UnsignedLongLong)
: TargetInfo::getLeastIntTypeByWidth(BitWidth, IsSigned);
   }
+
+  CallingConvCheckResult checkCallingConvention(CallingConv CC) const override {
+switch (CC) {
+case CC_C:
+case CC_Swift:
+  return CCCR_OK;
+default:
+  return CCCR_Warning;
+}
+  }
 };
 class LLVM_LIBRARY_VISIBILITY WebAssembly32TargetInfo
 : public WebAssemblyTargetInfo {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] c5bd3d0 - Support Swift calling convention for WebAssembly targets

[Derek Schuff via cfe-commits]

Author: Yuta Saito
Date: 2020-01-24T10:30:46-08:00
New Revision: c5bd3d07262ffda5b21576b9e1e2d2dd2e51fb4b

URL: 
https://github.com/llvm/llvm-project/commit/c5bd3d07262ffda5b21576b9e1e2d2dd2e51fb4b
DIFF: 
https://github.com/llvm/llvm-project/commit/c5bd3d07262ffda5b21576b9e1e2d2dd2e51fb4b.diff

LOG: Support Swift calling convention for WebAssembly targets

This adds basic support for the Swift calling convention with WebAssembly
targets.

Reviewed By: dschuff

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

Added: 


Modified: 
clang/lib/Basic/Targets/WebAssembly.h
llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp

Removed: 




diff  --git a/clang/lib/Basic/Targets/WebAssembly.h 
b/clang/lib/Basic/Targets/WebAssembly.h
index 9665156b143f..55d90db267e1 100644
--- a/clang/lib/Basic/Targets/WebAssembly.h
+++ b/clang/lib/Basic/Targets/WebAssembly.h
@@ -114,6 +114,16 @@ class LLVM_LIBRARY_VISIBILITY WebAssemblyTargetInfo : 
public TargetInfo {
? (IsSigned ? SignedLongLong : UnsignedLongLong)
: TargetInfo::getLeastIntTypeByWidth(BitWidth, IsSigned);
   }
+
+  CallingConvCheckResult checkCallingConvention(CallingConv CC) const override 
{
+switch (CC) {
+case CC_C:
+case CC_Swift:
+  return CCCR_OK;
+default:
+  return CCCR_Warning;
+}
+  }
 };
 class LLVM_LIBRARY_VISIBILITY WebAssembly32TargetInfo
 : public WebAssemblyTargetInfo {

diff  --git a/llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp 
b/llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
index e91a9ea03767..12fad047759f 100644
--- a/llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
+++ b/llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
@@ -672,7 +672,8 @@ static bool callingConvSupported(CallingConv::ID CallConv) {
  CallConv == CallingConv::PreserveMost ||
  CallConv == CallingConv::PreserveAll ||
  CallConv == CallingConv::CXX_FAST_TLS ||
- CallConv == CallingConv::WASM_EmscriptenInvoke;
+ CallConv == CallingConv::WASM_EmscriptenInvoke ||
+ CallConv == CallingConv::Swift;
 }
 
 SDValue



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

[PATCH] D73237: [CUDA] Fix order of memcpy arguments in __shfl_*(<64-bit type>).

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

In D73237#1837077 , @tra wrote:

> Landed in 
> https://github.com/llvm/llvm-project/commit/cc14de88da27a8178976972bdc8211c31f7ca9ae
>  @hans -- can we cherry-pick it into 10?


Yes, go ahead and "git cherry-pick -x" it and push to the branch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73237



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

[PATCH] D73007: [Sema] Avoid Wrange-loop-analysis false positives

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

In D73007#1837621 , @rtrieu wrote:

> I'm in favor of splitting the warning into subgroups, then deciding which 
> ones should be in -Wall.  We've done this with other warnings such as the 
> conversion warnings and tautological compare warnings, with only a subset of 
> them in -Wall.


It sounds like we may want this to go to the 10.x branch too, to avoid changing 
the interface too much between releases. So please keep me posted about how 
this progresses.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73007



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

[PATCH] D72222: [Driver][CodeGen] Add -fpatchable-function-entry=N[,0]

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

In D7#1838988 , @mrutland wrote:

> In D7#1837536 , @MaskRay wrote:
>
> >
>
>
>
>
> > For `BTI c` issue, GCC has several releases that do not work with 
> > -mbranch-protection=bti. The Linux kernel has to develop some mechanism to 
> > detect the undesirable placement of `bti c`, if there are 
> > -mbranch-protection=bti users. So I don't think that inconsistency in clang 
> > 10.0.0 with GCC will be a problem.
>
> Speaking as the person implementing the Linux side of things, I think that 
> will be a problem. Kernel-side we want consistency across compilers.
>
> For Linux we were planning to do a very simple build-time test to rule out 
> compilers with the broken behaviour. We would expect functional compilers to 
> have a consistent layout for a given combination of options, and we would 
> consider this layout to be ABI.
>
> The compile time check would compile a trivial test file, e.g.
>
>   void function(void) { }
>
>
> ... with flags:
>
>   -fpatchable-function-entry=2 -mbranch-protection=bti
>
>
> ... and if the function entry point is a NOP, mark that compiler as broken. 
> In practice, this will mean that the kernel build system will disable BTI 
> support for those broken compilers.
>
> Trying to support different layout approaches within Linux will add a fair 
> amount of unnecessary complexity which we cannot contain in one place, and 
> makes it more likely that support for either case will be broken in future.
>
> For the M=0 case, I would prefer to avoid runtime detection unless really 
> necessary.
>
> For the M!=0 case, which I believe Linux will need to use in the near future, 
> I realise that a runtime check will be necessary to detect the absence of a 
> BTI. Regardless, consistent behaviour across compilers will make this 
> significantly simpler and less error-prone.
>
> Thanks,
>  Mark.


When -mbranch-protection=bti is used, due to 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92424 (existing GCC releases have 
the issue), Linux/arch/arm64 has to make more checks to detect the combination.

I changed `nop; nop; bti c` to `bti c; nop; nop` with 2 commits not included in 
LLVM release/10.x (a72d15e37c5e066f597f13a8ba60aff214ac992d 
 and 
9a24488cb67a90f889529987275c5e411ce01dda 
). They 
make Clang's M!=0 placement (.Lpatch; nop; func: bti c; nop) consistent with 
GCC. If @hans allows me to cherry pick them to release/10.x, I'll be happy to 
do that.

For the M=0 case, Clang does (`func: .Lpatch; bti c; nop; nop`), which is 
inconsistent with GCC (`func: bti c; .Lpatch: nop; nop`). I'll be happy to try 
updating the placement of .Lpatch if future M>0 usage does not obsolete M=0 
usage (a proof that the proposed GCC layout is indeed superior, not just for 
simplicity for a not-so-common configuration), otherwise I would feel the clang 
side is just making changes to appease GCC/Linux compatibility, which would 
make me sad. I shall also mention that we are essentially making decisions for 
x86 people's endbr32/endbr64. I hope you can engage in x86 maintainers.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D7



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

[PATCH] D73300: [clang-tidy] Add library for clang-tidy main function

[Dmitry Polukhin via Phabricator via cfe-commits]

DmitryPolukhin updated this revision to Diff 240241.
DmitryPolukhin added a comment.

Added #include "ClangTidyMain.h" in ClangTidyMain.cpp


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73300

Files:
  clang-tools-extra/clang-tidy/tool/CMakeLists.txt
  clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
  clang-tools-extra/clang-tidy/tool/ClangTidyMain.h
  clang-tools-extra/clang-tidy/tool/ClangTidyToolMain.cpp

Index: clang-tools-extra/clang-tidy/tool/ClangTidyToolMain.cpp
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/tool/ClangTidyToolMain.cpp
@@ -0,0 +1,21 @@
+//===--- tools/extra/clang-tidy/ClangTidyToolMain.cpp - Clang tidy tool ---===//
+//
+// 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 This file contains clang-tidy tool entry point main function.
+///
+///  This tool uses the Clang Tooling infrastructure, see
+///http://clang.llvm.org/docs/HowToSetupToolingForLLVM.html
+///  for details on setting it up with LLVM source tree.
+///
+//===--===//
+
+#include "ClangTidyMain.h"
+
+int main(int argc, const char **argv) {
+  return clang::tidy::clangTidyMain(argc, argv);
+}
Index: clang-tools-extra/clang-tidy/tool/ClangTidyMain.h
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/tool/ClangTidyMain.h
@@ -0,0 +1,23 @@
+//===--- tools/extra/clang-tidy/ClangTidyMain.h - Clang tidy tool ---===//
+//
+// 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 This file declares the main function for the clang-tidy tool.
+///
+///  This tool uses the Clang Tooling infrastructure, see
+///http://clang.llvm.org/docs/HowToSetupToolingForLLVM.html
+///  for details on setting it up with LLVM source tree.
+///
+//===--===//
+
+namespace clang {
+namespace tidy {
+
+int clangTidyMain(int argc, const char **argv);
+
+} // namespace tidy
+} // namespace clang
Index: clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
===
--- clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
+++ clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
@@ -14,6 +14,7 @@
 ///
 //===--===//
 
+#include "ClangTidyMain.h"
 #include "../ClangTidy.h"
 #include "../ClangTidyForceLinker.h"
 #include "../GlobList.h"
@@ -327,7 +328,7 @@
   return FS;
 }
 
-static int clangTidyMain(int argc, const char **argv) {
+int clangTidyMain(int argc, const char **argv) {
   llvm::InitLLVM X(argc, argv);
   CommonOptionsParser OptionsParser(argc, argv, ClangTidyCategory,
 cl::ZeroOrMore);
@@ -488,7 +489,3 @@
 
 } // namespace tidy
 } // namespace clang
-
-int main(int argc, const char **argv) {
-  return clang::tidy::clangTidyMain(argc, argv);
-}
Index: clang-tools-extra/clang-tidy/tool/CMakeLists.txt
===
--- clang-tools-extra/clang-tidy/tool/CMakeLists.txt
+++ clang-tools-extra/clang-tidy/tool/CMakeLists.txt
@@ -5,8 +5,24 @@
   support
   )
 
-add_clang_tool(clang-tidy
+# Needed by LLVM's CMake checks because this file defines multiple targets.
+set(LLVM_OPTIONAL_SOURCES ClangTidyMain.cpp ClangTidyToolMain.cpp)
+
+add_clang_library(clangTidyMain
   ClangTidyMain.cpp
+
+  LINK_LIBS
+  clangAST
+  clangASTMatchers
+  clangBasic
+  clangTidy
+  ${ALL_CLANG_TIDY_CHECKS}
+  clangTooling
+  clangToolingCore
+  )
+
+add_clang_tool(clang-tidy
+  ClangTidyToolMain.cpp
   )
 add_dependencies(clang-tidy
   clang-resource-headers
@@ -22,6 +38,7 @@
 target_link_libraries(clang-tidy
   PRIVATE
   clangTidy
+  clangTidyMain
   ${ALL_CLANG_TIDY_CHECKS}
   )
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D73307: Unique Names for Functions with Internal Linkage

[Sriraman Tallam via Phabricator via cfe-commits]

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



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:966
  OPT_fno_unique_section_names, true);
+  Opts.UniqueInternalFuncNames = Args.hasFlag(
+  OPT_funique_internal_funcnames, OPT_fno_unique_internal_funcnames, 
false);

MaskRay wrote:
> Just `Args.hasArg(OPT_funique_internal_funcnames)`.
> 
> `-fno-*` is handled in clangDriver. CC1 does not need to know `-fno-*` if it 
> defaults to false.
Are you suggesting I remove the -fno-* flag? The -fno-* is useful  if 
-funique-internal-funcnames came added to a Makefile ,for example, and this 
needs to be disabled.  This is similar to say -ffunction-sections.  Please 
clarify. Thanks.


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

https://reviews.llvm.org/D73307



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

[PATCH] D71092: [VFS] More consistent support for Windows

[Adrian McCarthy via Phabricator via cfe-commits]

amccarth added a comment.

cc: +thakis, who expressed interest in seeing a fix for the text exempted on 
Windows.


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

https://reviews.llvm.org/D71092



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

[PATCH] D72467: Remove "mask" operand from shufflevector.

[Eli Friedman via Phabricator via cfe-commits]

efriedma marked 5 inline comments as done.
efriedma added a comment.

In D72467#1838591 , @spatel wrote:

> LGTM, but should get a 2nd opinion since I'm not familiar with some of the 
> parts.


Any specific part you're worried about?




Comment at: llvm/include/llvm/IR/Instructions.h:1980
 
+constexpr int UndefMaskElem = -1;
+

sdesmalen wrote:
> nit: do we want to make this a member of ShuffleVectorInst, as this is likely 
> the only context in which it will be used?
ShuffleVectorInst::UndefMaskElem is sort of long. Maybe we don't use it in 
enough places to matter.



Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:3597
 
-  if (MaskV->isNullValue() && VT.isScalableVector()) {
+  if (all_of(Mask, [](int Elem) { return Elem == 0; }) &&
+  VT.isScalableVector()) {

sdesmalen wrote:
> Should this use `m_ZeroMask()` ?
I don't want to make unrelated functional changes in this patch.



Comment at: llvm/lib/IR/ConstantFold.cpp:870
   // Undefined shuffle mask -> undefined value.
-  if (isa(Mask))
+  if (all_of(Mask, [](int Elt) { return Elt == UndefMaskElem; }))
 return UndefValue::get(VectorType::get(EltTy, MaskNumElts));

sdesmalen wrote:
> Is it worth adding a `m_UndefMask` ?
We currently only use this pattern in three places, and I doubt we're going to 
add more.



Comment at: llvm/lib/IR/Constants.cpp:2260
+  Constant *ArgVec[] = { V1, V2 };
+  ConstantExprKeyType Key(Instruction::ShuffleVector, ArgVec, 0, 0, None, 
Mask);
 

sdesmalen wrote:
> nit: is there a reason for dropping `const` here?
Doesn't really matter either way, but we generally don't add const markings to 
local variables just because we can.



Comment at: llvm/lib/IR/ConstantsContext.h:153
cast(C1->getType())->getElementType(),
-   cast(C3->getType())->getElementCount()),
+   Mask.size(), C1->getType()->getVectorIsScalable()),
  Instruction::ShuffleVector,

ctetreau wrote:
> sdesmalen wrote:
> > The number of elements in the result matches the number of elements in the 
> > mask, but if we assume 'scalable' from one of the source vectors, this 
> > means we make the choice that we cannot extract a fixed-width vector from a 
> > scalable vector, e.g.
> > 
> >   shufflevector( %in,  undef, <4 x i32> 
> > )
> > 
> > The LangRef does not explicitly call out this case (and it currently fails 
> > to compile), but it would provide a way to extract a sub-vector from a 
> > scalable vector.
> > If we want to allow this, we'd also need to decide what the meaning would 
> > be of:
> > 
> >   shufflevector( %in,  undef, <4 x i32> 
> > )
> > 
> > which may not be valid if `vscale = 1`.
> > 
> > Alternatively, we could implement this with an additional extract intrinsic 
> > and add the restriction that if the source values are scalable, the mask 
> > must be scalable as well. If so, then we should update the LangRef to 
> > explicitly disallow the above case.
> For the time being, Either this assumption must hold, or a bool must be added 
> that is true if the mask is from a scalable vector.
> 
> There are places in the codebase that assume that the mask has a concrete 
> value known at compile time. These sites are currently the sites of bugs, and 
> the fix is to not try to access the mask if the vector is scalable. We need 
> some way of knowing that the mask is scalable, either by the assumption made 
> here, or a queryable bool value.
I'd prefer to just say the result is scalable if and only if the operand is 
scalable, at least for now.  It's not clear what semantics we want for a fixed 
shuffle of a scalable operand, and allowing scalable splats of fixed vectors 
doesn't really allow expressing anything new.  We can relax it later once we've 
settled on our general approach to scalable shuffles.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72467



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

[PATCH] D73369: [clangd] Simplify "preferred" vs "definition" logic a bit in XRefs AST code.

[pre-merge checks [bot] via Phabricator via cfe-commits]

merge_guards_bot added a comment.

{icon times-circle color=red} Unit tests: fail. 62173 tests passed, 5 failed 
and 815 were skipped.

  failed: libc++.std/language_support/cmp/cmp_partialord/partialord.pass.cpp
  failed: libc++.std/language_support/cmp/cmp_strongeq/cmp.strongeq.pass.cpp
  failed: libc++.std/language_support/cmp/cmp_strongord/strongord.pass.cpp
  failed: libc++.std/language_support/cmp/cmp_weakeq/cmp.weakeq.pass.cpp
  failed: libc++.std/language_support/cmp/cmp_weakord/weakord.pass.cpp

{icon times-circle color=red} clang-tidy: fail. clang-tidy found 0 errors and 1 
warnings 
.
 1 of them are added as review comments below (why? 
).

{icon times-circle color=red} clang-format: fail. Please format your changes 
with clang-format by running `git-clang-format HEAD^` or applying this patch 
.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 


//Pre-merge checks is in beta. Report issue 
.
 Please join beta  or enable 
it for your project 
.//


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73369



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