[PATCH] D70157: Align branches within 32-Byte boundary

[LuoYuanke via Phabricator via cfe-commits]

LuoYuanke added a comment.

In D70157#1773180 , @reames wrote:

> Recording something so I don't forget it when we get back to the prefix 
> padding version.  The write up on the bundle align mode stuff mentions a 
> concerning memory overhead for the feature.  Since the basic implementation 
> techniques are similar, we need to make sure we assess the memory overhead of 
> the prefix padding implementation.  See 
> https://www.chromium.org/nativeclient/pnacl/aligned-bundling-support-in-llvm 
> for context.  I don't believe this is likely to be an issue for the nop 
> padding variant.


From the doc of 
https://www.chromium.org/nativeclient/pnacl/aligned-bundling-support-in-llvm 
the ".bundle_align_mode" ensure each instruction that following the directive 
don't cross the alignment boundary and ensure the first instruction following 
the directive be aligned,  but in this patch we only require branch (jcc, jmp, 
...) instruction don't cross the alignment boundary. Another remind, this patch 
avoid branch instruction hit the alignment boundary, and bundle syntax doesn't 
support that (see section 2.1 of 
https://www.intel.com/content/dam/support/us/en/documents/processors/mitigations-jump-conditional-code-erratum.pdf).
 So I don't think bundle syntax fit current requirement perfectly.


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

https://reviews.llvm.org/D70157



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

[PATCH] D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior

[Michele Scandale via Phabricator via cfe-commits]

michele.scandale added a comment.

I've noticed you removed the change for `CompilerInvocation.cpp` about the 
initialization of the codegen option `NoTrappingMath`. Was that an accident?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62731



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

[PATCH] D71159: [Attr] Move ParsedTargetAttr out of the TargetAttr class

[Craig Topper via Phabricator via cfe-commits]

craig.topper marked an inline comment as done.
craig.topper added inline comments.



Comment at: clang/include/clang/AST/Attr.h:359
 }
 }  // end namespace clang
 

There's an ending namespace comment here and  there's no namespace starting 
since the end of my change. And I assume Attrs.inc doesn't leave a dangling 
namespace?


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

https://reviews.llvm.org/D71159



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

[PATCH] D71154: Driver: Don't look for libc++ headers in the install directory on Android.

[Peter Collingbourne via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG198fbcb81749: Driver: Dont look for libc++ headers in 
the install directory on Android. (authored by pcc).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71154

Files:
  clang/lib/Driver/ToolChains/Linux.cpp
  clang/test/Driver/android-no-installed-libcxx.cpp
  clang/test/Driver/stdlibxx-isystem.cpp

Index: clang/test/Driver/stdlibxx-isystem.cpp
===
--- clang/test/Driver/stdlibxx-isystem.cpp
+++ clang/test/Driver/stdlibxx-isystem.cpp
@@ -6,7 +6,7 @@
 // By default, we should search for libc++ next to the driver.
 // RUN: mkdir -p %t/bin
 // RUN: mkdir -p %t/include/c++/v1
-// RUN: %clang -target aarch64-linux-android -ccc-install-dir %t/bin \
+// RUN: %clang -target aarch64-linux-gnu -ccc-install-dir %t/bin \
 // RUN:   -stdlib=libc++ -fsyntax-only %s -### 2>&1 | \
 // RUN:   FileCheck -check-prefix=LIBCXX %s
 // RUN: %clang -target x86_64-apple-darwin -ccc-install-dir %t/bin \
@@ -16,7 +16,7 @@
 // LIBCXX: "-internal-isystem" "[[INSTALLDIR]]/../include/c++/v1"
 
 // Passing -stdlib++-isystem should suppress the default search.
-// RUN: %clang -target aarch64-linux-android -ccc-install-dir %t/bin \
+// RUN: %clang -target aarch64-linux-gnu -ccc-install-dir %t/bin \
 // RUN:   -stdlib++-isystem /tmp/foo -stdlib++-isystem /tmp/bar -stdlib=libc++ \
 // RUN:   -fsyntax-only %s -### 2>&1 | FileCheck -check-prefix=NODEFAULT %s
 // RUN: %clang -target x86_64-apple-darwin -ccc-install-dir %t/bin \
@@ -26,7 +26,7 @@
 // NODEFAULT-NOT: "-internal-isystem" "[[INSTALLDIR]]/../include/c++/v1"
 
 // And we should add it as an -internal-isystem.
-// RUN: %clang -target aarch64-linux-android -ccc-install-dir %t/bin \
+// RUN: %clang -target aarch64-linux-gnu -ccc-install-dir %t/bin \
 // RUN:   -stdlib++-isystem /tmp/foo -stdlib++-isystem /tmp/bar -stdlib=libc++ \
 // RUN:   -fsyntax-only %s -### 2>&1 | FileCheck -check-prefix=INCPATH %s
 // RUN: %clang -target x86_64-apple-darwin -ccc-install-dir %t/bin \
@@ -35,7 +35,7 @@
 // INCPATH: "-internal-isystem" "/tmp/foo" "-internal-isystem" "/tmp/bar"
 
 // We shouldn't pass the -stdlib++-isystem to cc1.
-// RUN: %clang -target aarch64-linux-android -ccc-install-dir %t/bin \
+// RUN: %clang -target aarch64-linux-gnu -ccc-install-dir %t/bin \
 // RUN:   -stdlib++-isystem /tmp -stdlib=libc++ -fsyntax-only %s -### 2>&1 | \
 // RUN:   FileCheck -check-prefix=NOCC1 %s
 // RUN: %clang -target x86_64-apple-darwin -ccc-install-dir %t/bin \
@@ -44,7 +44,7 @@
 // NOCC1-NOT: "-stdlib++-isystem" "/tmp"
 
 // It should respect -nostdinc++.
-// RUN: %clang -target aarch64-linux-android -ccc-install-dir %t/bin \
+// RUN: %clang -target aarch64-linux-gnu -ccc-install-dir %t/bin \
 // RUN:   -stdlib++-isystem /tmp/foo -stdlib++-isystem /tmp/bar -nostdinc++ \
 // RUN:   -fsyntax-only %s -### 2>&1 | FileCheck -check-prefix=NOSTDINCXX %s
 // RUN: %clang -target x86_64-apple-darwin -ccc-install-dir %t/bin \
Index: clang/test/Driver/android-no-installed-libcxx.cpp
===
--- /dev/null
+++ clang/test/Driver/android-no-installed-libcxx.cpp
@@ -0,0 +1,8 @@
+// Check that we don't find the libc++ in the installation directory when
+// targeting Android.
+
+// RUN: mkdir -p %t/bin
+// RUN: mkdir -p %t/include/c++/v1
+// RUN: %clang -target aarch64-linux-android -ccc-install-dir %t/bin \
+// RUN:   -stdlib=libc++ -fsyntax-only %s -### 2>&1 | FileCheck %s
+// CHECK-NOT: "-internal-isystem" "{{.*}}v1"
Index: clang/lib/Driver/ToolChains/Linux.cpp
===
--- clang/lib/Driver/ToolChains/Linux.cpp
+++ clang/lib/Driver/ToolChains/Linux.cpp
@@ -888,20 +888,25 @@
 void Linux::addLibCxxIncludePaths(const llvm::opt::ArgList ,
   llvm::opt::ArgStringList ) const {
   const std::string& SysRoot = computeSysRoot();
-  const std::string LibCXXIncludePathCandidates[] = {
-  DetectLibcxxIncludePath(getVFS(), getDriver().Dir + "/../include/c++"),
-  // If this is a development, non-installed, clang, libcxx will
-  // not be found at ../include/c++ but it likely to be found at
-  // one of the following two locations:
-  DetectLibcxxIncludePath(getVFS(), SysRoot + "/usr/local/include/c++"),
-  DetectLibcxxIncludePath(getVFS(), SysRoot + "/usr/include/c++") };
-  for (const auto  : LibCXXIncludePathCandidates) {
+  auto AddIncludePath = [&](std::string Path) {
+std::string IncludePath = DetectLibcxxIncludePath(getVFS(), Path);
 if (IncludePath.empty() || !getVFS().exists(IncludePath))
-  continue;
-// Use the first candidate that exists.
+  return false;
 addSystemInclude(DriverArgs, CC1Args, IncludePath);
+return true;
+  };
+  // Android never uses the libc++ headers installed 

[clang] 198fbcb - Driver: Don't look for libc++ headers in the install directory on Android.

[Peter Collingbourne via cfe-commits]

Author: Peter Collingbourne
Date: 2019-12-06T18:24:23-08:00
New Revision: 198fbcb817492ff45946e3f7517de15e8cdf0607

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

LOG: Driver: Don't look for libc++ headers in the install directory on Android.

The NDK uses a separate set of libc++ headers in the sysroot. Any headers
in the installation directory are not going to work on Android, not least
because they use a different name for the inline namespace (std::__1 instead
of std::__ndk1).

This effectively makes it impossible to produce a single toolchain that is
capable of targeting both Android and another platform that expects libc++
headers to be installed in the installation directory, such as Mac.

In order to allow this scenario to work, stop looking for headers in the
install directory on Android.

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

Added: 
clang/test/Driver/android-no-installed-libcxx.cpp

Modified: 
clang/lib/Driver/ToolChains/Linux.cpp
clang/test/Driver/stdlibxx-isystem.cpp

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/Linux.cpp 
b/clang/lib/Driver/ToolChains/Linux.cpp
index 736a2d435ca5..adacd705d831 100644
--- a/clang/lib/Driver/ToolChains/Linux.cpp
+++ b/clang/lib/Driver/ToolChains/Linux.cpp
@@ -888,20 +888,25 @@ static std::string 
DetectLibcxxIncludePath(llvm::vfs::FileSystem ,
 void Linux::addLibCxxIncludePaths(const llvm::opt::ArgList ,
   llvm::opt::ArgStringList ) const {
   const std::string& SysRoot = computeSysRoot();
-  const std::string LibCXXIncludePathCandidates[] = {
-  DetectLibcxxIncludePath(getVFS(), getDriver().Dir + "/../include/c++"),
-  // If this is a development, non-installed, clang, libcxx will
-  // not be found at ../include/c++ but it likely to be found at
-  // one of the following two locations:
-  DetectLibcxxIncludePath(getVFS(), SysRoot + "/usr/local/include/c++"),
-  DetectLibcxxIncludePath(getVFS(), SysRoot + "/usr/include/c++") };
-  for (const auto  : LibCXXIncludePathCandidates) {
+  auto AddIncludePath = [&](std::string Path) {
+std::string IncludePath = DetectLibcxxIncludePath(getVFS(), Path);
 if (IncludePath.empty() || !getVFS().exists(IncludePath))
-  continue;
-// Use the first candidate that exists.
+  return false;
 addSystemInclude(DriverArgs, CC1Args, IncludePath);
+return true;
+  };
+  // Android never uses the libc++ headers installed alongside the toolchain,
+  // which are generally incompatible with the NDK libraries anyway.
+  if (!getTriple().isAndroid())
+if (AddIncludePath(getDriver().Dir + "/../include/c++"))
+  return;
+  // If this is a development, non-installed, clang, libcxx will
+  // not be found at ../include/c++ but it likely to be found at
+  // one of the following two locations:
+  if (AddIncludePath(SysRoot + "/usr/local/include/c++"))
+return;
+  if (AddIncludePath(SysRoot + "/usr/include/c++"))
 return;
-  }
 }
 
 void Linux::addLibStdCxxIncludePaths(const llvm::opt::ArgList ,

diff  --git a/clang/test/Driver/android-no-installed-libcxx.cpp 
b/clang/test/Driver/android-no-installed-libcxx.cpp
new file mode 100644
index ..b6f4227c7dc1
--- /dev/null
+++ b/clang/test/Driver/android-no-installed-libcxx.cpp
@@ -0,0 +1,8 @@
+// Check that we don't find the libc++ in the installation directory when
+// targeting Android.
+
+// RUN: mkdir -p %t/bin
+// RUN: mkdir -p %t/include/c++/v1
+// RUN: %clang -target aarch64-linux-android -ccc-install-dir %t/bin \
+// RUN:   -stdlib=libc++ -fsyntax-only %s -### 2>&1 | FileCheck %s
+// CHECK-NOT: "-internal-isystem" "{{.*}}v1"

diff  --git a/clang/test/Driver/stdlibxx-isystem.cpp 
b/clang/test/Driver/stdlibxx-isystem.cpp
index cf7535ff423e..827cdf9a7c71 100644
--- a/clang/test/Driver/stdlibxx-isystem.cpp
+++ b/clang/test/Driver/stdlibxx-isystem.cpp
@@ -6,7 +6,7 @@
 // By default, we should search for libc++ next to the driver.
 // RUN: mkdir -p %t/bin
 // RUN: mkdir -p %t/include/c++/v1
-// RUN: %clang -target aarch64-linux-android -ccc-install-dir %t/bin \
+// RUN: %clang -target aarch64-linux-gnu -ccc-install-dir %t/bin \
 // RUN:   -stdlib=libc++ -fsyntax-only %s -### 2>&1 | \
 // RUN:   FileCheck -check-prefix=LIBCXX %s
 // RUN: %clang -target x86_64-apple-darwin -ccc-install-dir %t/bin \
@@ -16,7 +16,7 @@
 // LIBCXX: "-internal-isystem" "[[INSTALLDIR]]/../include/c++/v1"
 
 // Passing -stdlib++-isystem should suppress the default search.
-// RUN: %clang -target aarch64-linux-android -ccc-install-dir %t/bin \
+// RUN: %clang -target aarch64-linux-gnu -ccc-install-dir %t/bin \
 // RUN:   -stdlib++-isystem /tmp/foo -stdlib++-isystem /tmp/bar -stdlib=libc++ 
\
 // RUN:   -fsyntax-only %s -### 2>&1 | FileCheck 

[PATCH] D71159: [Attr] Move ParsedTargetAttr out of the TargetAttr class

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

Should this be in a namespace? Can't tell from the header file, is it at least 
in clang?


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

https://reviews.llvm.org/D71159



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

[PATCH] D70469: [attributes] [analyzer] Add handle related attributes

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

xazax.hun updated this revision to Diff 232681.
xazax.hun marked 7 inline comments as done.
xazax.hun added a comment.

- Make the annotation acceptable on both types and declarations.
- Fix some TODOs.
- Teach TypePrinter to print the annotations.
- Address review comments.


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

https://reviews.llvm.org/D70469

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/AST/TypePrinter.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/Sema/attr-handles.cpp

Index: clang/test/Sema/attr-handles.cpp
===
--- /dev/null
+++ clang/test/Sema/attr-handles.cpp
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1  -fsyntax-only -verify %s
+
+// Decl annotations.
+void f(int *a __attribute__((acquire_handle)));
+void (*fp)(int handle [[clang::use_handle]]);
+auto lambda = [](int handle [[clang::use_handle]]){};
+void g(int a __attribute__((acquire_handle))); // expected-error {{attribute only applies to output parameters}}
+void h(int *a __attribute__((acquire_handle(1; // expected-error {{'acquire_handle' attribute takes no arguments}}
+__attribute__((release_handle)) int i(); // expected-warning {{'release_handle' attribute only applies to parameters}}
+__attribute__((use_handle)) int j(); // expected-warning {{'use_handle' attribute only applies to parameters}}
+int a __attribute__((acquire_handle)); // expected-warning {{'acquire_handle' attribute only applies to functions, function pointers, and parameters}}
+
+// Type annotations.
+void ft(int __attribute__((acquire_handle)) * a);
+void (*fpt)(int __attribute__((use_handle)) handle);
+auto lambdat = [](int __attribute__((use_handle)) handle) ->
+int __attribute__((acquire_handle)) { return 0; };
+void gt(int __attribute__((acquire_handle)) a); // expected-error {{attribute only applies to output parameters}}
+void ht(int __attribute__((acquire_handle(1))) *a); // expected-error {{'acquire_handle' attribute takes no arguments}}
+int it() __attribute__((release_handle)); // expected-warning {{'release_handle' only applies to non-function types; type here is 'int ()'}}
+int jt() __attribute__((use_handle)); // expected-warning {{'use_handle' only applies to non-function types; type here is 'int ()'}}
+int __attribute__((acquire_handle)) at; // expected-warning {{'acquire_handle' attribute only applies to functions, function pointers, and parameters}}
\ No newline at end of file
Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -9,6 +9,7 @@
 // CHECK-NEXT: AMDGPUWavesPerEU (SubjectMatchRule_function)
 // CHECK-NEXT: AVRSignal (SubjectMatchRule_function)
 // CHECK-NEXT: AbiTag (SubjectMatchRule_record_not_is_union, SubjectMatchRule_variable, SubjectMatchRule_function, SubjectMatchRule_namespace)
+// CHECK-NEXT: AcquireHandle (SubjectMatchRule_hasType_functionType, SubjectMatchRule_variable_is_parameter)
 // CHECK-NEXT: Alias (SubjectMatchRule_function, SubjectMatchRule_variable_is_global)
 // CHECK-NEXT: AlignValue (SubjectMatchRule_variable, SubjectMatchRule_type_alias)
 // CHECK-NEXT: AllocSize (SubjectMatchRule_function)
@@ -128,6 +129,7 @@
 // CHECK-NEXT: ParamTypestate (SubjectMatchRule_variable_is_parameter)
 // CHECK-NEXT: PassObjectSize (SubjectMatchRule_variable_is_parameter)
 // CHECK-NEXT: Pointer (SubjectMatchRule_record_not_is_union)
+// CHECK-NEXT: ReleaseHandle (SubjectMatchRule_variable_is_parameter)
 // CHECK-NEXT: RenderScriptKernel (SubjectMatchRule_function)
 // CHECK-NEXT: ReqdWorkGroupSize (SubjectMatchRule_function)
 // CHECK-NEXT: Restrict (SubjectMatchRule_function)
@@ -145,6 +147,7 @@
 // CHECK-NEXT: Target (SubjectMatchRule_function)
 // CHECK-NEXT: TestTypestate (SubjectMatchRule_function_is_member)
 // CHECK-NEXT: TrivialABI (SubjectMatchRule_record)
+// CHECK-NEXT: UseHandle (SubjectMatchRule_variable_is_parameter)
 // CHECK-NEXT: VecReturn (SubjectMatchRule_record)
 // CHECK-NEXT: VecTypeHint (SubjectMatchRule_function)
 // CHECK-NEXT: WarnUnused (SubjectMatchRule_record)
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -7418,6 +7418,21 @@
  attrKind == ParsedAttr::AT_OpenCLGenericAddressSpace;
 }
 
+template 
+static void handleHandleAttr(TypeProcessingState , QualType ,
+ ParsedAttr ) {
+  if (CurType->isFunctionType()) {
+State.getSema().Diag(Attr.getLoc(), diag::warn_type_attribute_wrong_type)
+<< Attr.getAttrName()->getName() << 3 << CurType;
+ 

[PATCH] D70469: [attributes] [analyzer] Add handle related attributes

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

xazax.hun added inline comments.



Comment at: clang/lib/Sema/SemaType.cpp:7412
+ diag::warn_type_attribute_wrong_type_str)
+<< Attr.getAttrName()->getName() << "non-function" << CurType;
+return;

aaron.ballman wrote:
> You should be able to pass in `Attr` directly instead of 
> `Attr.getAttrName()->getName()`, I believe. Also, I'd prefer the 
> `non-function` be put into the diagnostic text itself with a `%select` if we 
> need to vary it.
Indeed. But only passing Attr would result in duplicated `'`. I'm reusing the 
message, so I ended up not changing this part.


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

https://reviews.llvm.org/D70469



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

[PATCH] D70839: [clang][IFS] Claiming -emit-merged-ifs in clang driver when -c is used.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi abandoned this revision.
plotfi added a comment.

This commit is no good. I got some wrong inspiration from a lot of the code 
claiming random args in the driver. abandoning.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70839



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

[PATCH] D70973: [OPENMP50]Treat context selectors as expressions, not just strings.

[Alexey Bataev via Phabricator via cfe-commits]

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



Comment at: clang/include/clang/Basic/DiagnosticParseKinds.td:1230
-  "unknown '%0' device kind trait in the 'device' context selector set, 
expected"
-  " one of 'host', 'nohost', 'cpu', 'gpu' or 'fpga'">;
 

jdoerfert wrote:
> ABataev wrote:
> > jdoerfert wrote:
> > > ABataev wrote:
> > > > jdoerfert wrote:
> > > > > ABataev wrote:
> > > > > > jdoerfert wrote:
> > > > > > > I would have expected this error to be still accurate, maybe with 
> > > > > > > the addition ", or quoted versions thereof".
> > > > > > Currently, we could emit it only in codegen phase to avoid double 
> > > > > > converting from expression to string. Will emit it there.
> > > > > Why can't we emit this error if the user writes 
> > > > > `device={kind(gpu)}` anymore? Even `device={kind("gpu")}` 
> > > > > should be diagnosable early.
> > > > The main problem here is the conversion and expression evaluation. We 
> > > > convert the data from the expression to strings at the codegen phase. I 
> > > > don't want to do the same thing for the second time in Sema just for 
> > > > diagnostic.
> > > We have to convert it during sema already, actually during parsing. I'm 
> > > working on declare variant begin/end support right now (part of TR8) 
> > > which needs the information during parsing.
> > Hmmm, it is where we need to parse the code only if the context matches the 
> > trait? Ok, will rework to convert it in sema too. Seems to me, we'll need 
> > to match the context in both, codegen and sema. Will try to move the 
> > matcher somewhere to avoid duplication in sema and codege.
> I put my patch up later tonight or tomorrow. It moves stuff around, we need 
> to coordinate somehow.
I still won't work on it until Monday. So, if you have a good solution for the 
matcher, go ahead.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70973



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

[PATCH] D70520: [WebAssembly] Add new `export_name` clang attribute for controlling wasm export names

[Sam Clegg via Phabricator via cfe-commits]

sbc100 added a comment.

Ping .. I think this is good to go now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70520



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

[PATCH] D71159: [Attr] Move ParsedTargetAttr out of the TargetAttr class

[Craig Topper via Phabricator via cfe-commits]

craig.topper created this revision.
craig.topper added reviewers: rnk, echristo, erichkeane.

Need to forward declare it in ASTContext.h for D68627 
, so it can't be a nested struct.


https://reviews.llvm.org/D71159

Files:
  clang/include/clang/AST/Attr.h
  clang/include/clang/Basic/Attr.td
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/CodeGen/TargetInfo.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp

Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -3046,7 +3046,7 @@
   return Diag(LiteralLoc, diag::warn_unsupported_target_attribute)
  << Unsupported << None << Str;
 
-  TargetAttr::ParsedTargetAttr ParsedAttrs = TargetAttr::parse(AttrStr);
+  ParsedTargetAttr ParsedAttrs = TargetAttr::parse(AttrStr);
 
   if (!ParsedAttrs.Architecture.empty() &&
   !Context.getTargetInfo().isValidCPUName(ParsedAttrs.Architecture))
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -9741,7 +9741,7 @@
 static bool CheckMultiVersionValue(Sema , const FunctionDecl *FD) {
   const auto *TA = FD->getAttr();
   assert(TA && "MultiVersion Candidate requires a target attribute");
-  TargetAttr::ParsedTargetAttr ParseInfo = TA->parse();
+  ParsedTargetAttr ParseInfo = TA->parse();
   const TargetInfo  = S.Context.getTargetInfo();
   enum ErrType { Feature = 0, Architecture = 1 };
 
@@ -9992,7 +9992,7 @@
 bool , NamedDecl *, bool ,
 LookupResult ) {
   const auto *OldTA = OldFD->getAttr();
-  TargetAttr::ParsedTargetAttr NewParsed = NewTA->parse();
+  ParsedTargetAttr NewParsed = NewTA->parse();
   // Sort order doesn't matter, it just needs to be consistent.
   llvm::sort(NewParsed.Features);
 
@@ -10036,8 +10036,7 @@
 return true;
   }
 
-  TargetAttr::ParsedTargetAttr OldParsed =
-  OldTA->parse(std::less());
+  ParsedTargetAttr OldParsed = OldTA->parse(std::less());
 
   if (OldParsed == NewParsed) {
 S.Diag(NewFD->getLocation(), diag::err_multiversion_duplicate);
@@ -10090,7 +10089,7 @@
 return true;
   }
 
-  TargetAttr::ParsedTargetAttr NewParsed;
+  ParsedTargetAttr NewParsed;
   if (NewTA) {
 NewParsed = NewTA->parse();
 llvm::sort(NewParsed.Features);
@@ -10117,8 +10116,7 @@
 return false;
   }
 
-  TargetAttr::ParsedTargetAttr CurParsed =
-  CurTA->parse(std::less());
+  ParsedTargetAttr CurParsed = CurTA->parse(std::less());
   if (CurParsed == NewParsed) {
 S.Diag(NewFD->getLocation(), diag::err_multiversion_duplicate);
 S.Diag(CurFD->getLocation(), diag::note_previous_declaration);
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -5061,7 +5061,7 @@
 CodeGenOptions::SignReturnAddressKeyValue Key = CGM.getCodeGenOpts().getSignReturnAddressKey();
 bool BranchTargetEnforcement = CGM.getCodeGenOpts().BranchTargetEnforcement;
 if (const auto *TA = FD->getAttr()) {
-  TargetAttr::ParsedTargetAttr Attr = TA->parse();
+  ParsedTargetAttr Attr = TA->parse();
   if (!Attr.BranchProtection.empty()) {
 TargetInfo::BranchProtectionInfo BPI;
 StringRef Error;
Index: clang/lib/CodeGen/CodeGenModule.h
===
--- clang/lib/CodeGen/CodeGenModule.h
+++ clang/lib/CodeGen/CodeGenModule.h
@@ -1152,7 +1152,7 @@
 
   /// Parses the target attributes passed in, and returns only the ones that are
   /// valid feature names.
-  TargetAttr::ParsedTargetAttr filterFunctionTargetAttrs(const TargetAttr *TD);
+  ParsedTargetAttr filterFunctionTargetAttrs(const TargetAttr *TD);
 
   // Fills in the supplied string map with the set of target features for the
   // passed in function.
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -957,7 +957,7 @@
 
   Out << '.';
   const TargetInfo  = CGM.getTarget();
-  TargetAttr::ParsedTargetAttr Info =
+  ParsedTargetAttr Info =
   Attr->parse([](StringRef LHS, StringRef RHS) {
 // Multiversioning doesn't allow "no-${feature}", so we can
 // only have "+" prefixes here.
@@ -1678,7 +1678,7 @@
 // get and parse the target attribute so we can get the cpu for
 // the function.
 if (TD) {
-  TargetAttr::ParsedTargetAttr ParsedAttr = TD->parse();
+  ParsedTargetAttr ParsedAttr = TD->parse();
   if (ParsedAttr.Architecture != "" &&
   getTarget().isValidCPUName(ParsedAttr.Architecture))
 TargetCPU = 

[PATCH] D71154: Driver: Don't look for libc++ headers in the install directory on Android.

[Dan Albert via Phabricator via cfe-commits]

danalbert accepted this revision.
danalbert added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/test/Driver/android-no-installed-libcxx.cpp:8
+// RUN:   -stdlib=libc++ -fsyntax-only %s -### 2>&1 | FileCheck %s
+// CHECK-NOT: "-internal-isystem" "{{.*}}v1"

pcc wrote:
> danalbert wrote:
> > This looks like it should be failing. The NDK headers are installed to 
> > `/sysroot/usr/include/c++/v1`. Presumably it's passing because the 
> > trivial sysroot in the test directory is not complete enough?
> > 
> > Will need a more specific CHECK-NOT here, and also a CHECK to make sure the 
> > correct directory is being searched.
> The `%t/include/c++/v1` path here represents the install directory, not the 
> sysroot. Note that the test isn't passing a `-sysroot` flag.
> 
> We already have a test that the correct directory is being searched, see 
> `test/Driver/android-ndk-standalone.cpp`.
Ah, I'd forgotten that the tests required an explicit `--sysroot` flag. That's 
not required for a typical NDK clang call, but it is here. Never mind then, 
this LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71154



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

[PATCH] D70973: [OPENMP50]Treat context selectors as expressions, not just strings.

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticParseKinds.td:1230
-  "unknown '%0' device kind trait in the 'device' context selector set, 
expected"
-  " one of 'host', 'nohost', 'cpu', 'gpu' or 'fpga'">;
 

ABataev wrote:
> jdoerfert wrote:
> > ABataev wrote:
> > > jdoerfert wrote:
> > > > ABataev wrote:
> > > > > jdoerfert wrote:
> > > > > > I would have expected this error to be still accurate, maybe with 
> > > > > > the addition ", or quoted versions thereof".
> > > > > Currently, we could emit it only in codegen phase to avoid double 
> > > > > converting from expression to string. Will emit it there.
> > > > Why can't we emit this error if the user writes 
> > > > `device={kind(gpu)}` anymore? Even `device={kind("gpu")}` 
> > > > should be diagnosable early.
> > > The main problem here is the conversion and expression evaluation. We 
> > > convert the data from the expression to strings at the codegen phase. I 
> > > don't want to do the same thing for the second time in Sema just for 
> > > diagnostic.
> > We have to convert it during sema already, actually during parsing. I'm 
> > working on declare variant begin/end support right now (part of TR8) which 
> > needs the information during parsing.
> Hmmm, it is where we need to parse the code only if the context matches the 
> trait? Ok, will rework to convert it in sema too. Seems to me, we'll need to 
> match the context in both, codegen and sema. Will try to move the matcher 
> somewhere to avoid duplication in sema and codege.
I put my patch up later tonight or tomorrow. It moves stuff around, we need to 
coordinate somehow.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70973



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

[PATCH] D71154: Driver: Don't look for libc++ headers in the install directory on Android.

[Peter Collingbourne via Phabricator via cfe-commits]

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



Comment at: clang/test/Driver/android-no-installed-libcxx.cpp:8
+// RUN:   -stdlib=libc++ -fsyntax-only %s -### 2>&1 | FileCheck %s
+// CHECK-NOT: "-internal-isystem" "{{.*}}v1"

danalbert wrote:
> This looks like it should be failing. The NDK headers are installed to 
> `/sysroot/usr/include/c++/v1`. Presumably it's passing because the 
> trivial sysroot in the test directory is not complete enough?
> 
> Will need a more specific CHECK-NOT here, and also a CHECK to make sure the 
> correct directory is being searched.
The `%t/include/c++/v1` path here represents the install directory, not the 
sysroot. Note that the test isn't passing a `-sysroot` flag.

We already have a test that the correct directory is being searched, see 
`test/Driver/android-ndk-standalone.cpp`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71154



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

[PATCH] D71154: Driver: Don't look for libc++ headers in the install directory on Android.

[Dan Albert via Phabricator via cfe-commits]

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



Comment at: clang/test/Driver/android-no-installed-libcxx.cpp:8
+// RUN:   -stdlib=libc++ -fsyntax-only %s -### 2>&1 | FileCheck %s
+// CHECK-NOT: "-internal-isystem" "{{.*}}v1"

This looks like it should be failing. The NDK headers are installed to 
`/sysroot/usr/include/c++/v1`. Presumably it's passing because the trivial 
sysroot in the test directory is not complete enough?

Will need a more specific CHECK-NOT here, and also a CHECK to make sure the 
correct directory is being searched.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71154



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

[PATCH] D71152: [analyzer] Keep track of escaped locals.

[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/lib/StaticAnalyzer/Core/ProgramState.cpp:48-61
+namespace {
+struct EscapedLocals{};
+} // namespace
+
+template <>
+struct ProgramStateTrait :
+  public ProgramStatePartialTrait> {

NoQ wrote:
> Wait, you are preventing direct access anyway by putting this stuff into the 
> .cpp file.
> 
> In this case i think you can safely use the `REGISTER_...` macros.
Yeah, initially I wanted to hide this trait but at this point I have no idea 
how to do that. If I get the layering right, ExprEngine is using ProgramState, 
and ProgramState should not reference ExprEngine. So if I want to use a trait 
in both it should be in ProgramState. The reason why I also need to touch 
ProgramState, because the list of invalidated regions are readily available 
there. Probably it is possible to somehow get that list in ExprEngine, but I 
think that would be way more complicated than the current solution.


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

https://reviews.llvm.org/D71152



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

[PATCH] D71154: Driver: Don't look for libc++ headers in the install directory on Android.

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

merge_guards_bot added a comment.

Build result: pass - 60573 tests passed, 0 failed and 726 were skipped.

Log files: console-log.txt 
,
 CMakeCache.txt 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71154



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

[clang] 5253d91 - [c++20] Determine whether a defaulted comparison should be deleted or

[Richard Smith via cfe-commits]

Author: Richard Smith
Date: 2019-12-06T16:32:48-08:00
New Revision: 5253d9138eb31252594f5e14133df731551839c7

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

LOG: [c++20] Determine whether a defaulted comparison should be deleted or
constexpr.

Added: 
clang/test/CXX/class/class.compare/class.compare.default/p3.cpp
clang/test/CXX/class/class.compare/class.eq/p2.cpp
clang/test/CXX/class/class.compare/class.rel/p2.cpp
clang/test/CXX/class/class.compare/class.spaceship/p1.cpp

Modified: 
clang/include/clang/AST/ComparisonCategories.h
clang/include/clang/Basic/DiagnosticGroups.td
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/include/clang/Sema/Overload.h
clang/include/clang/Sema/Sema.h
clang/lib/Sema/SemaDeclCXX.cpp
clang/lib/Sema/SemaExpr.cpp
clang/lib/Sema/SemaOverload.cpp
clang/test/CXX/class/class.compare/class.compare.default/p2.cpp
clang/test/CXX/class/class.compare/class.eq/p1.cpp
clang/test/CXX/class/class.compare/class.rel/p1.cpp
clang/www/cxx_status.html

Removed: 




diff  --git a/clang/include/clang/AST/ComparisonCategories.h 
b/clang/include/clang/AST/ComparisonCategories.h
index 9d591cc81495..3dd1db1f7e9b 100644
--- a/clang/include/clang/AST/ComparisonCategories.h
+++ b/clang/include/clang/AST/ComparisonCategories.h
@@ -221,7 +221,6 @@ class ComparisonCategories {
 return const_cast(This.lookupInfo(Kind));
   }
 
-private:
   const ComparisonCategoryInfo *lookupInfoForType(QualType Ty) const;
 
 private:

diff  --git a/clang/include/clang/Basic/DiagnosticGroups.td 
b/clang/include/clang/Basic/DiagnosticGroups.td
index e999ba10a003..6cb1a4e0700d 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -113,7 +113,7 @@ def UndefinedVarTemplate : 
DiagGroup<"undefined-var-template">;
 def UndefinedFuncTemplate : DiagGroup<"undefined-func-template">;
 def MissingNoEscape : DiagGroup<"missing-noescape">;
 
-def DefaultedComparison : DiagGroup<"defaulted-comparison">;
+def DefaultedFunctionDeleted : DiagGroup<"defaulted-function-deleted">;
 def DeleteIncomplete : DiagGroup<"delete-incomplete">;
 def DeleteNonAbstractNonVirtualDtor : 
DiagGroup<"delete-non-abstract-non-virtual-dtor">;
 def DeleteAbstractNonVirtualDtor : 
DiagGroup<"delete-abstract-non-virtual-dtor">;

diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index a2e4cf51232b..b6586bedd5c8 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -4067,7 +4067,10 @@ def err_ovl_deleted_oper : Error<
   "overload resolution selected deleted operator '%0'">;
 def err_ovl_deleted_special_oper : Error<
   "object of type %0 cannot be %select{constructed|copied|moved|assigned|"
-  "assigned|destroyed}1 because its %sub{select_special_member_kind}1 is 
implicitly deleted">;
+  "assigned|destroyed}1 because its %sub{select_special_member_kind}1 is "
+  "implicitly deleted">;
+def err_ovl_deleted_comparison : Error<
+  "object of type %0 cannot be compared because its %1 is implicitly deleted">;
 def err_ovl_rewrite_equalequal_not_bool : Error<
   "return type %0 of selected 'operator==' function for rewritten "
   "'%1' comparison is not 'bool'">;
@@ -8152,7 +8155,7 @@ def err_incorrect_defaulted_consteval : Error<
   "cannot be consteval because implicit definition is not constexpr">;
 def warn_defaulted_method_deleted : Warning<
   "explicitly defaulted %sub{select_special_member_kind}0 is implicitly "
-  "deleted">, InGroup>;
+  "deleted">, InGroup;
 def err_out_of_line_default_deletes : Error<
   "defaulting this %sub{select_special_member_kind}0 "
   "would delete it after its first declaration">;
@@ -8194,21 +8197,42 @@ def err_defaulted_comparison_non_const : Error<
 def err_defaulted_comparison_return_type_not_bool : Error<
   "return type for defaulted %sub{select_defaulted_comparison_kind}0 "
   "must be 'bool', not %1">;
-def err_defaulted_comparison_reference_member : Error<
-  "cannot default %0 in class %1 with reference member">;
-def ext_defaulted_comparison_reference_member : ExtWarn<
-  "ISO C++2a does not allow defaulting %0 in class %1 with reference member">,
-  InGroup;
-def note_reference_member : Note<"reference member %0 declared here">;
-def err_defaulted_comparison_union : Error<
-  "cannot default %0 in %select{union-like class|union}1 %2">;
-def ext_defaulted_comparison_union : ExtWarn<
-  "ISO C++2a does not allow defaulting %0 in "
-  "%select{union-like class|union}1 %2">, InGroup;
-def ext_defaulted_comparison_empty_union : ExtWarn<
-  "ISO C++2a does not allow defaulting %0 in "
-  "%select{union-like class|union}1 %2 despite it having 

[PATCH] D71155: [analyzer] CERT: StrChecker: 30.c

[Csaba Dabis via Phabricator via cfe-commits]

Charusso created this revision.
Charusso added a reviewer: NoQ.
Charusso added a project: clang.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus, 
mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun.
Charusso added a comment.
Charusso added a parent revision: D71033: [analyzer] CERT: StrChecker: 32.c.

Examples generated by CodeChecker from the `curl` project:
F10986729: str30-c.tar.gz 


This checker is implemented based on the following rule:
https://wiki.sei.cmu.edu/confluence/display/c/STR30-C.+Do+not+attempt+to+modify+string+literals

It warns on misusing the following functions: `strpbrk()`, `strchr()`,
`strrchr()`, `strstr()`, `memchr()`.


Repository:
  rC Clang

https://reviews.llvm.org/D71155

Files:
  clang/docs/analyzer/checkers.rst
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
  clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/cert/StrChecker.cpp
  clang/test/Analysis/cert/str30-c-notes.cpp
  clang/test/Analysis/cert/str30-c.cpp

Index: clang/test/Analysis/cert/str30-c.cpp
===
--- /dev/null
+++ clang/test/Analysis/cert/str30-c.cpp
@@ -0,0 +1,59 @@
+// RUN: %clang_analyze_cc1 \
+// RUN:  -analyzer-checker=core,unix,alpha.security.cert.str.30.c \
+// RUN:  -verify %s
+
+// See the examples on the page of STR30-C:
+// https://wiki.sei.cmu.edu/confluence/display/c/STR30-C.+Do+not+attempt+to+modify+string+literals
+
+#include "../Inputs/system-header-simulator.h"
+
+#define EOF -1
+typedef __SIZE_TYPE__ size_t;
+typedef __typeof__((char*)0-(char*)0) ptrdiff_t;
+
+void free(void *memblock);
+void *malloc(size_t size);
+
+char *strrchr(const char *str, int c);
+int puts(const char *str);
+
+namespace test_strrchr_bad {
+const char *get_dirname(const char *pathname) {
+  char *slash;
+  slash = strrchr(pathname, '/');
+  if (slash) {
+*slash = '\0'; /* Undefined behavior */
+// expected-warning@-1 {{'slash' is pointing to a constant string}}
+  }
+  return pathname;
+}
+
+int main(void) {
+  puts(get_dirname(__FILE__));
+  return 0;
+}
+} // namespace test_strrchr_bad
+
+namespace test_strrchr_good {
+char *get_dirname(const char *pathname, char *dirname, size_t size) {
+  const char *slash;
+  slash = strrchr(pathname, '/');
+  if (slash) {
+ptrdiff_t slash_idx = slash - pathname;
+if ((size_t)slash_idx < size) {
+  memcpy(dirname, pathname, slash_idx);
+  dirname[slash_idx] = '\0'; 
+  return dirname;
+}
+  }
+  return 0;
+}
+
+int main(void) {
+  char dirname[260];
+  if (get_dirname(__FILE__, dirname, sizeof(dirname))) {
+puts(dirname);
+  }
+  return 0;
+}
+} // namespace test_strrchr_good
Index: clang/test/Analysis/cert/str30-c-notes.cpp
===
--- /dev/null
+++ clang/test/Analysis/cert/str30-c-notes.cpp
@@ -0,0 +1,44 @@
+// RUN: %clang_analyze_cc1 \
+// RUN:  -analyzer-checker=core,unix,alpha.security.cert.str.30.c \
+// RUN:  -analyzer-output=text -verify %s
+
+// See the examples on the page of STR30-C:
+// https://wiki.sei.cmu.edu/confluence/display/c/STR30-C.+Do+not+attempt+to+modify+string+literals
+
+#include "../Inputs/system-header-simulator.h"
+
+#define EOF -1
+typedef __SIZE_TYPE__ size_t;
+
+void free(void *memblock);
+void *malloc(size_t size);
+
+char *strrchr(const char *str, int c);
+int puts(const char *str);
+
+namespace test_strrchr_bad {
+const char *get_dirname(const char *pathname) {
+  char *slash;
+  slash = strrchr(pathname, '/');
+  // expected-note@-1 {{'strrchr' returns a pointer to the constant 'pathname'}}
+  
+  slash = strrchr(slash, '/');
+  // expected-note@-1 {{'strrchr' returns a pointer to the constant 'pathname'}}
+
+  if (slash) {
+// expected-note@-1 {{Assuming 'slash' is non-null}}
+// expected-note@-2 {{Taking true branch}}
+
+*slash = '\0'; /* Undefined behavior */
+// expected-note@-1 {{'slash' is pointing to a constant string}}
+// expected-warning@-2 {{'slash' is pointing to a constant string}}
+  }
+  return pathname;
+}
+
+int main(void) {
+  puts(get_dirname(__FILE__));
+  // expected-note@-1 {{Calling 'get_dirname'}}
+  return 0;
+}
+} // namespace test_strrchr_bad
Index: clang/lib/StaticAnalyzer/Checkers/cert/StrChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/cert/StrChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/cert/StrChecker.cpp
@@ -12,6 +12,9 @@
 //  https://wiki.sei.cmu.edu/confluence/pages/viewpage.action?pageId=87152038
 //
 //  This checker is a base checker which consist of the following checkers:
+//  - '30.c'
+//  https://wiki.sei.cmu.edu/confluence/display/c/STR30-C.+Do+not+attempt+to+modify+string+literals
+//
 //  - '31.c'
 //  

[PATCH] D71155: [analyzer] CERT: StrChecker: 30.c

[Csaba Dabis via Phabricator via cfe-commits]

Charusso added a comment.

Examples generated by CodeChecker from the `curl` project:
F10986729: str30-c.tar.gz 


Repository:
  rC Clang

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

https://reviews.llvm.org/D71155



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

[PATCH] D70520: [WebAssembly] Add new `export_name` clang attribute for controlling wasm export names

[Sam Clegg via Phabricator via cfe-commits]

sbc100 updated this revision to Diff 232673.
sbc100 added a comment.

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70520

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/lib/CodeGen/TargetInfo.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGen/wasm-export-name.c
  lld/test/wasm/export-name.ll
  lld/wasm/InputChunks.h
  lld/wasm/Writer.cpp
  llvm/include/llvm/BinaryFormat/Wasm.h
  llvm/include/llvm/MC/MCSymbolWasm.h
  llvm/include/llvm/Object/Wasm.h
  llvm/lib/MC/WasmObjectWriter.cpp
  llvm/lib/Object/WasmObjectFile.cpp
  llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp
  llvm/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyTargetStreamer.cpp
  llvm/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyTargetStreamer.h
  llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp
  llvm/test/CodeGen/WebAssembly/export-name.ll
  llvm/test/MC/WebAssembly/export-name.s

Index: llvm/test/MC/WebAssembly/export-name.s
===
--- /dev/null
+++ llvm/test/MC/WebAssembly/export-name.s
@@ -0,0 +1,26 @@
+# RUN: llvm-mc -triple=wasm32-unknown-unknown < %s | FileCheck %s
+# Check that it also comiled to object for format.
+# RUN: llvm-mc -triple=wasm32-unknown-unknown -filetype=obj -o - < %s | obj2yaml | FileCheck -check-prefix=CHECK-OBJ %s
+
+foo:
+.globl foo
+.functype foo () -> ()
+.export_name foo, bar
+end_function
+
+# CHECK: .export_name foo, bar
+
+# CHECK-OBJ:- Type:EXPORT
+# CHECK-OBJ-NEXT: Exports:
+# CHECK-OBJ-NEXT:   - Name:bar
+# CHECK-OBJ-NEXT: Kind:FUNCTION
+# CHECK-OBJ-NEXT: Index:   0
+
+# CHECK-OBJ:  Name:linking
+# CHECK-OBJ-NEXT: Version: 2
+# CHECK-OBJ-NEXT: SymbolTable:
+# CHECK-OBJ-NEXT:   - Index:   0
+# CHECK-OBJ-NEXT: Kind:FUNCTION
+# CHECK-OBJ-NEXT: Name:foo
+# CHECK-OBJ-NEXT: Flags:   [ EXPORTED ]
+# CHECK-OBJ-NEXT: Function:0
Index: llvm/test/CodeGen/WebAssembly/export-name.ll
===
--- /dev/null
+++ llvm/test/CodeGen/WebAssembly/export-name.ll
@@ -0,0 +1,17 @@
+; RUN: llc < %s -asm-verbose=false -wasm-keep-registers | FileCheck %s
+
+target datalayout = "e-m:e-p:32:32-i64:64-n32:64-S128"
+target triple = "wasm32-unknown-unknown"
+
+define void @test() #0 {
+  ret void
+}
+
+declare void @test2() #1
+
+
+attributes #0 = { "wasm-export-name"="foo" }
+attributes #1 = { "wasm-export-name"="bar" }
+
+; CHECK: .export_name test, foo
+; CHECK: .export_name test2, bar
Index: llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp
===
--- llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp
+++ llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp
@@ -96,8 +96,11 @@
   }
 
   for (const auto  : M) {
+if (F.isIntrinsic())
+  continue;
+
 // Emit function type info for all undefined functions
-if (F.isDeclarationForLinker() && !F.isIntrinsic()) {
+if (F.isDeclarationForLinker()) {
   SmallVector Results;
   SmallVector Params;
   computeSignatureVTs(F.getFunctionType(), F, TM, Params, Results);
@@ -130,6 +133,13 @@
 getTargetStreamer()->emitImportName(Sym, Name);
   }
 }
+
+if (F.hasFnAttribute("wasm-export-name")) {
+  auto *Sym = cast(getSymbol());
+  StringRef Name = F.getFnAttribute("wasm-export-name").getValueAsString();
+  Sym->setExportName(Name);
+  getTargetStreamer()->emitExportName(Sym, Name);
+}
   }
 
   for (const auto  : M.globals()) {
Index: llvm/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyTargetStreamer.h
===
--- llvm/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyTargetStreamer.h
+++ llvm/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyTargetStreamer.h
@@ -48,6 +48,9 @@
   /// .import_name
   virtual void emitImportName(const MCSymbolWasm *Sym,
   StringRef ImportName) = 0;
+  /// .export_name
+  virtual void emitExportName(const MCSymbolWasm *Sym,
+  StringRef ExportName) = 0;
 
 protected:
   void emitValueType(wasm::ValType Type);
@@ -68,6 +71,7 @@
   void emitEventType(const MCSymbolWasm *Sym) override;
   void emitImportModule(const MCSymbolWasm *Sym, StringRef ImportModule) override;
   void emitImportName(const MCSymbolWasm *Sym, StringRef ImportName) override;
+  void emitExportName(const MCSymbolWasm *Sym, StringRef ExportName) override;
 };
 
 /// This part is for Wasm object output
@@ -85,6 +89,8 @@
 StringRef ImportModule) override {}
   void emitImportName(const MCSymbolWasm *Sym,
   StringRef 

[PATCH] D71033: [analyzer] CERT: StrChecker: 32.c

[Csaba Dabis via Phabricator via cfe-commits]

Charusso updated this revision to Diff 232672.
Charusso edited the summary of this revision.
Charusso added a comment.

- Add docs.
- Move to alpha.


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

https://reviews.llvm.org/D71033

Files:
  clang/docs/analyzer/checkers.rst
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicSize.h
  clang/lib/StaticAnalyzer/Checkers/cert/StrChecker.cpp
  clang/lib/StaticAnalyzer/Core/DynamicSize.cpp
  clang/test/Analysis/Inputs/system-header-simulator.h
  clang/test/Analysis/cert/str32-c-notes.cpp
  clang/test/Analysis/cert/str32-c.cpp
  clang/test/Analysis/malloc.c

Index: clang/test/Analysis/malloc.c
===
--- clang/test/Analysis/malloc.c
+++ clang/test/Analysis/malloc.c
@@ -9,21 +9,6 @@
 
 void clang_analyzer_eval(int);
 
-// Without -fms-compatibility, wchar_t isn't a builtin type. MSVC defines
-// _WCHAR_T_DEFINED if wchar_t is available. Microsoft recommends that you use
-// the builtin type: "Using the typedef version can cause portability
-// problems", but we're ok here because we're not actually running anything.
-// Also of note is this cryptic warning: "The wchar_t type is not supported
-// when you compile C code".
-//
-// See the docs for more:
-// https://msdn.microsoft.com/en-us/library/dh8che7s.aspx
-#if !defined(_WCHAR_T_DEFINED)
-// "Microsoft implements wchar_t as a two-byte unsigned value"
-typedef unsigned short wchar_t;
-#define _WCHAR_T_DEFINED
-#endif // !defined(_WCHAR_T_DEFINED)
-
 typedef __typeof(sizeof(int)) size_t;
 void *malloc(size_t);
 void *alloca(size_t);
Index: clang/test/Analysis/cert/str32-c.cpp
===
--- /dev/null
+++ clang/test/Analysis/cert/str32-c.cpp
@@ -0,0 +1,88 @@
+// RUN: %clang_analyze_cc1 \
+// RUN:  -analyzer-checker=core,unix,alpha.security.cert.str.32.c \
+// RUN:  -verify %s
+
+// See the examples on the page of STR32-C:
+// https://wiki.sei.cmu.edu/confluence/display/c/STR32-C.+Do+not+pass+a+non-null-terminated+character+sequence+to+a+library+function+that+expects+a+string
+
+#include "../Inputs/system-header-simulator.h"
+
+void *realloc(void *memblock, size_t size);
+
+namespace test_strncpy_bad {
+enum { STR_SIZE = 32 };
+
+size_t func(const char *source) {
+  char c_str[STR_SIZE];
+  size_t ret = 0;
+
+  if (source) {
+c_str[sizeof(c_str) - 1] = '\0';
+strncpy(c_str, source, sizeof(c_str));
+ret = strlen(c_str);
+// expected-warning@-1 {{'c_str' is not null-terminated}}
+  }
+  return ret;
+}
+} // namespace test_strncpy_bad
+
+namespace test_strncpy_good {
+enum { STR_SIZE = 32 };
+
+size_t func(const char *src) {
+  char c_str[STR_SIZE];
+  size_t ret = 0;
+
+  if (src) {
+strncpy(c_str, src, sizeof(c_str) - 1);
+c_str[sizeof(c_str) - 1] = '\0';
+ret = strlen(c_str);
+  }
+  return ret;
+}
+} // namespace test_strncpy_good
+
+namespace test_realloc_bad {
+wchar_t *cur_msg = NULL;
+size_t cur_msg_size = 1024;
+size_t cur_msg_len = 0;
+
+void lessen_memory_usage(void) {
+  if (cur_msg == NULL)
+return;
+
+  size_t temp_size = cur_msg_size / 2 + 1;
+  wchar_t *temp = (wchar_t *)realloc(cur_msg, temp_size * sizeof(wchar_t));
+  /* temp and cur_msg may no longer be null-terminated */
+  if (temp == NULL)
+return;
+
+  cur_msg = temp;
+  cur_msg_size = temp_size;
+  cur_msg_len = wcslen(cur_msg);
+  // expected-warning@-1 {{'cur_msg' is not null-terminated}}
+}
+} // namespace test_realloc_bad
+
+namespace test_realloc_good {
+wchar_t *cur_msg = NULL;
+size_t cur_msg_size = 1024;
+size_t cur_msg_len = 0;
+
+void lessen_memory_usage(void) {
+  if (cur_msg == NULL)
+return;
+
+  size_t temp_size = cur_msg_size / 2 + 1;
+  wchar_t *temp = (wchar_t *)realloc(cur_msg, temp_size * sizeof(wchar_t));
+  /* temp and cur_msg may no longer be null-terminated */
+  if (temp == NULL)
+return;
+
+  cur_msg = temp;
+  /* Properly null-terminate cur_msg */
+  cur_msg[temp_size - 1] = L'\0';
+  cur_msg_size = temp_size;
+  cur_msg_len = wcslen(cur_msg);
+}
+} // namespace test_realloc_good
Index: clang/test/Analysis/cert/str32-c-notes.cpp
===
--- /dev/null
+++ clang/test/Analysis/cert/str32-c-notes.cpp
@@ -0,0 +1,66 @@
+// RUN: %clang_analyze_cc1 \
+// RUN:  -analyzer-checker=core,unix,alpha.security.cert.str.32.c \
+// RUN:  -analyzer-output=text -verify %s
+
+// See the examples on the page of STR32-C:
+// https://wiki.sei.cmu.edu/confluence/display/c/STR32-C.+Do+not+pass+a+non-null-terminated+character+sequence+to+a+library+function+that+expects+a+string
+
+#include "../Inputs/system-header-simulator.h"
+
+void *realloc(void *memblock, size_t size);
+
+namespace test_strncpy_bad {
+enum { STR_SIZE = 32 };
+
+size_t func(const char *source) {
+  char c_str[STR_SIZE];
+  // expected-note@-1 {{'c_str' initialized here}}
+ 

[PATCH] D71152: [analyzer] Keep track of escaped locals.

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

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

- Add cleanup.


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

https://reviews.llvm.org/D71152

Files:
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
  clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
  clang/lib/StaticAnalyzer/Core/ProgramState.cpp
  clang/test/Analysis/symbol-escape.cpp

Index: clang/test/Analysis/symbol-escape.cpp
===
--- clang/test/Analysis/symbol-escape.cpp
+++ clang/test/Analysis/symbol-escape.cpp
@@ -31,3 +31,13 @@
   return Baz;
 }
 
+void save_ptr(int **);
+void delete_saved();
+
+void store_to_escaped_region() {
+  int *p;
+  save_ptr();
+  p = new int;
+  delete_saved();
+} // no-warning:
+
Index: clang/lib/StaticAnalyzer/Core/ProgramState.cpp
===
--- clang/lib/StaticAnalyzer/Core/ProgramState.cpp
+++ clang/lib/StaticAnalyzer/Core/ProgramState.cpp
@@ -16,6 +16,7 @@
 #include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/DynamicType.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SubEngine.h"
 #include "llvm/Support/raw_ostream.h"
@@ -41,7 +42,13 @@
 Mgr.freeStates.push_back(s);
   }
 }
-}}
+} // namespace ento
+} // namespace clang
+
+void *ProgramStateTrait::GDMIndex() {
+  static int index = 0;
+  return 
+}
 
 ProgramState::ProgramState(ProgramStateManager *mgr, const Environment& env,
  StoreRef st, GenericDataMap gdm)
@@ -209,6 +216,12 @@
   ProgramStateRef newState = makeWithStore(newStore);
 
   if (CausedByPointerEscape) {
+for (const MemRegion *R : Invalidated) {
+  if (!R->hasStackStorage())
+continue;
+  newState = newState->add(R->getBaseRegion());
+}
+
 newState = Eng.notifyCheckersOfPointerEscape(newState, IS,
  TopLevelInvalidated,
  Call,
@@ -642,3 +655,7 @@
   }
   return true;
 }
+
+bool ProgramState::isEscapedLocal(const MemRegion *R) const {
+  return this->contains(R->getBaseRegion());
+}
Index: clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -723,6 +723,12 @@
   SymReaper.markLive(MR);
   }
 
+  EscapedLocals::data_type EscapedRegions = CleanedState->get();
+  for (const MemRegion *MR : EscapedRegions) {
+if (!SymReaper.isLiveRegion(MR))
+  CleanedState = CleanedState->remove(MR);
+  }
+
   getCheckerManager().runCheckersForLiveSymbols(CleanedState, SymReaper);
 
   // Create a state in which dead bindings are removed from the environment
@@ -2680,7 +2686,8 @@
 
 // A value escapes in four possible cases:
 // (1) We are binding to something that is not a memory region.
-// (2) We are binding to a MemRegion that does not have stack storage.
+// (2) We are binding to a MemRegion that does not have stack storage
+// or the stack storage is escaped.
 // (3) We are binding to a top-level parameter region with a non-trivial
 // destructor. We won't see the destructor during analysis, but it's there.
 // (4) We are binding to a MemRegion with stack storage that the store
@@ -2691,7 +2698,7 @@
 
   // Cases (1) and (2).
   const MemRegion *MR = Loc.getAsRegion();
-  if (!MR || !MR->hasStackStorage())
+  if (!MR || !MR->hasStackStorage() || State->isEscapedLocal(MR))
 return escapeValue(State, Val, PSK_EscapeOnBind);
 
   // Case (3).
Index: clang/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
===
--- clang/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
+++ clang/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
@@ -18,16 +18,18 @@
 #include "clang/StaticAnalyzer/Core/PathSensitive/DynamicTypeInfo.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/Environment.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState_Fwd.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/Store.h"
 #include "llvm/ADT/FoldingSet.h"
 #include "llvm/ADT/ImmutableMap.h"
+#include "llvm/ADT/ImmutableSet.h"
 #include "llvm/Support/Allocator.h"
 #include 
 
 namespace llvm {
 class APSInt;
-}
+} // namespace llvm
 
 namespace clang {
 class ASTContext;
@@ -57,6 +59,16 @@
   }
 };
 
+/// Tag for GDM.
+struct EscapedLocals{
+  using data_type = llvm::ImmutableSet;
+};
+template <>
+struct 

[PATCH] D70411: [analyzer] CERT: StrChecker: 31.c

[Csaba Dabis via Phabricator via cfe-commits]

Charusso added a comment.

Please let us measure your examples at first to have a consensus what we would 
like to see from this checker.

In D70411#1769803 , @NoQ wrote:

> In D70411#1769628 , @Charusso wrote:
>
> > F10966837: str31-c.tar.gz 
>
>
> These reports seem to confirm my point. I took a look at first 5 and in all 
> of them the code does in fact ensure that the buffer space is sufficient, but 
> in 5 different ways. You most likely won't be able to enumerate all possible 
> ways in which the user can ensure that the destination buffer size is 
> sufficient.


I believe I can enhance the checker with the help of `NoteTags` and give more 
information for the user why the buffer space is Not sufficient. The size 
property's upper-bound is not that difficult to measure, I think we can 
implement this. For now, let me analyze each report.

---

> F10967274: content_encoding.c_5ae4f30ce29f14441139e7bbe20eeaaa.plist.html 
> 
>  The source string is taken from a global table and therefore has known 
> maximum size.
>  F10967280: imap.c_fd80e0804acd9e7ecb9c2483151625a9.plist.html 
> 
>  The source string is a literal `'*'`.

When you encounter a member that is when the hand-waving begins. I have made a 
dump to see what we have. I have not seen any immediate way to connect the 
dynamic size and the members. For now we have that:
The source region is:
`SymRegion{reg_$36}`
and the allocation's dynamic size is:
`extent_$41{SymRegion{conj_$34{void *, LC7, S161233, #1}}}`

My assumption is that, the size of a member is very arbitrary and we cannot 
track the member's allocation, so we even could drop every of these reports as 
being useless. Because I make this checker alpha I am fine with that assumption 
of members, and we definitely need better ideas.

---

> F10967277: ftp.c_8cb07866de61ef56be82135a4d3a5b7e.plist.html 
> 
>  The source string is an IP address and port, which has a known limit on the 
> number of digits it can have.

The known size does not mean that the string is going to be null-terminated. 
For example the VIM project has one single global macro to serve every 
allocation's size, and that is why the CERT rule made. If the attacker passes a 
10k characters long string and you allocate 10k+1 characters for the `'\0'` you 
can work with the data safely. Of course you would need to check some upper 
bound, but that is another story. The key is, any arbitrary buffer size could 
be a problem. Here:

  954 | size_t addrlen = INET6_ADDRSTRLEN > strlen(string_ftpport) // Assuming 
the condition is true
 ? INET6_ADDRSTRLEN
 : strlen(string_ftpport)

`addrlen` is an arbitrary macro size `INET6_ADDRSTRLEN`, which not necessarily 
could hold `string_ftpport` in `strcpy(addr, string_ftpport);`. As a 
human-being you know the most appropriate size, yes. But the program could be 
ill-formed very easily so we warn on the arbitrary-length path. Here you do not 
see something like `addr[INET6_ADDRSTRLEN - 1] = '\0'` so when the string is 
read you cannot be sure whether it is null-terminated. If line 954 would be 
evaluated to false, allocating `calloc(addrlen + 1, 1)` so when `addrlen` is 
`strlen(string_ftpport)` is fine of course, that is the appropriate size to 
store `string_ftpport`.

---

> F10967295: tftp.c_eee38be8d783d25f2e733fa3740d13fc.plist.html 
> 
>  There's an if-statement that checks that the storage size is sufficient.

Well, this is the most suspicious report of all the time. Let me reconstruct 
what is going on:

  513 | sbytes += tftp_option_add(state, sbytes,
  (char *)state->spacket.data + sbytes, // 
warning
  TFTP_OPTION_TSIZE);

My guess was that to change the `check::PostCall` to `check::PreCall`, but if I 
remember right the same report occurred. Here we return from the function call 
`tftp_option_add`, then we warn on its argument which already has been passed 
to the call. We should not warn when the array is passed to a function which we 
already have finished reading from. I have not seen any immediate solution, so 
I count this as an internal ~~bug~~ feature.

---

> F10967300: tool_dirhie.c_87dd00a845a927c9b8ed587c6b314af1.plist.html 
> 
>  The source string is a token (obtained via `strtok`) of a string that has 
> the same size as the destination buffer.

Let me simplify the report as there is no such feature:

  111 | outlen = strlen(outfile);
  112 | outdup = strdup(outfile);
  116 | dirbuildup = malloc(outlen + 1);
  
  125 | tempdir = strtok(outdup, PATH_DELIMITERS);
  136 | if(outdup == tempdir)
  138 |   strcpy(dirbuildup, tempdir);

Here we see both 

[PATCH] D70411: [analyzer] CERT: StrChecker: 31.c

[Csaba Dabis via Phabricator via cfe-commits]

Charusso updated this revision to Diff 232671.
Charusso marked 10 inline comments as done.
Charusso added a comment.

- Make the checker alpha.
- Add docs.
- Copy-paste @NoQ's test.


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

https://reviews.llvm.org/D70411

Files:
  clang/docs/analyzer/checkers.rst
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
  clang/include/clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicSizeInfo.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymExpr.h
  clang/lib/StaticAnalyzer/Checkers/AllocationState.h
  clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
  clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/cert/StrChecker.cpp
  clang/lib/StaticAnalyzer/Core/CommonBugCategories.cpp
  clang/test/Analysis/Inputs/system-header-simulator.h
  clang/test/Analysis/cert/str31-c-fp-suppression.cpp
  clang/test/Analysis/cert/str31-c-notes.cpp
  clang/test/Analysis/cert/str31-c.cpp

Index: clang/test/Analysis/cert/str31-c.cpp
===
--- /dev/null
+++ clang/test/Analysis/cert/str31-c.cpp
@@ -0,0 +1,205 @@
+// RUN: %clang_analyze_cc1 \
+// RUN:  -analyzer-checker=core,unix,alpha.security.cert.str.31.c \
+// RUN:  -verify %s
+
+// See the examples on the page of STR31-C:
+// https://wiki.sei.cmu.edu/confluence/display/c/STR31-C.+Guarantee+that+storage+for+strings+has+sufficient+space+for+character+data+and+the+null+terminator
+
+#include "../Inputs/system-header-simulator.h"
+
+#define EOF -1
+typedef __SIZE_TYPE__ size_t;
+
+void free(void *memblock);
+void *malloc(size_t size);
+
+void do_something(char *buffer);
+
+namespace test_gets_bad {
+#define BUFFER_SIZE 1024
+
+void func(void) {
+  char buf[BUFFER_SIZE];
+  if (gets(buf)) {
+do_something(buf);
+// expected-warning@-1 {{'buf' is not null-terminated}}
+  }
+}
+} // namespace test_gets_bad
+
+namespace test_gets_good {
+enum { BUFFERSIZE = 32 };
+
+void func(void) {
+  char buff[BUFFERSIZE];
+
+  if (fgets(buff, sizeof(buff), stdin)) {
+do_something(buff);
+  }
+}
+} // namespace test_gets_good
+
+namespace test_sprintf_bad {
+void func(const char *name) {
+  char buf[128];
+  sprintf(buf, "%s.txt", name);
+
+  do_something(buf);
+  // expected-warning@-1 {{'buf' is not null-terminated}}
+}
+} // namespace test_sprintf_bad
+
+namespace test_sprintf_good {
+void func(const char *name) {
+  char buff[128];
+  snprintf(buff, sizeof(buff), "%s.txt", name);
+
+  do_something(buff);
+}
+} // namespace test_sprintf_good
+
+namespace test_fscanf_bad {
+enum { BUF_LENGTH = 1024 };
+
+void get_data(void) {
+  char buf[BUF_LENGTH];
+  if (fscanf(stdin, "%s", buf)) {
+do_something(buf);
+// expected-warning@-1 {{'buf' is not null-terminated}}
+  }
+}
+} // namespace test_fscanf_bad
+
+namespace test_fscanf_good {
+enum { BUF_LENGTH = 1024 };
+
+void get_data(void) {
+  char buff[BUF_LENGTH];
+  if (fscanf(stdin, "%1023s", buff)) {
+do_something(buff);
+  }
+}
+} // namespace test_fscanf_good
+
+namespace test_strcpy_bad {
+int main(int argc, char *argv[]) {
+  const char *const name = (argc && argv[0]) ? argv[0] : "";
+  char prog_name[128];
+  strcpy(prog_name, name);
+
+  do_something(prog_name);
+  // expected-warning@-1 {{'prog_name' is not null-terminated}}
+
+  return 0;
+}
+
+void func(void) {
+  char buff[256];
+  char *editor = getenv("EDITOR");
+  if (editor != NULL) {
+strcpy(buff, editor);
+
+do_something(buff);
+// expected-warning@-1 {{'buff' is not null-terminated}}
+  }
+}
+} // namespace test_strcpy_bad
+
+namespace test_strcpy_good {
+int main(int argc, char *argv[]) {
+  const char *const name = (argc && argv[0]) ? argv[0] : "";
+  char *prog_name2 = (char *)malloc(strlen(name) + 1);
+  if (prog_name2 != NULL) {
+strcpy(prog_name2, name);
+  }
+
+  do_something(prog_name2);
+
+  free(prog_name2);
+  return 0;
+}
+
+void func(void) {
+  char *buff2;
+  char *editor = getenv("EDITOR");
+  if (editor != NULL) {
+size_t len = strlen(editor) + 1;
+buff2 = (char *)malloc(len);
+if (buff2 != NULL) {
+  strcpy(buff2, editor);
+}
+
+do_something(buff2);
+free(buff2);
+  }
+}
+} // namespace test_strcpy_good
+
+//===--===//
+// The following are from the rule's page which we do not handle yet.
+//===--===//
+
+namespace test_loop_index_bad {
+void copy(size_t n, char src[n], char dest[n]) {
+  size_t i;
+
+  for (i = 0; src[i] && (i < n); ++i) {
+dest[i] = src[i];
+  }
+  dest[i] = '\0';
+}
+} // namespace test_loop_index_bad
+
+namespace test_loop_index_good {
+void copy(size_t n, char src[n], char dest[n]) {
+  size_t i;
+
+  for (i = 0; src[i] && (i < n - 1); ++i) {
+dest[i] = src[i];
+  }
+  dest[i] = '\0';
+}
+} // 

[PATCH] D71154: Driver: Don't look for libc++ headers in the install directory on Android.

[Peter Collingbourne via Phabricator via cfe-commits]

pcc created this revision.
pcc added reviewers: danalbert, srhines.
Herald added a reviewer: EricWF.
Herald added a subscriber: ldionne.
Herald added a project: clang.

The NDK uses a separate set of libc++ headers in the sysroot. Any headers
in the installation directory are not going to work on Android, not least
because they use a different name for the inline namespace (std::__1 instead
of std::__ndk1).

This effectively makes it impossible to produce a single toolchain that is
capable of targeting both Android and another platform that expects libc++
headers to be installed in the installation directory, such as Mac.

In order to allow this scenario to work, stop looking for headers in the
install directory on Android.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71154

Files:
  clang/lib/Driver/ToolChains/Linux.cpp
  clang/test/Driver/android-no-installed-libcxx.cpp
  clang/test/Driver/stdlibxx-isystem.cpp

Index: clang/test/Driver/stdlibxx-isystem.cpp
===
--- clang/test/Driver/stdlibxx-isystem.cpp
+++ clang/test/Driver/stdlibxx-isystem.cpp
@@ -6,7 +6,7 @@
 // By default, we should search for libc++ next to the driver.
 // RUN: mkdir -p %t/bin
 // RUN: mkdir -p %t/include/c++/v1
-// RUN: %clang -target aarch64-linux-android -ccc-install-dir %t/bin \
+// RUN: %clang -target aarch64-linux-gnu -ccc-install-dir %t/bin \
 // RUN:   -stdlib=libc++ -fsyntax-only %s -### 2>&1 | \
 // RUN:   FileCheck -check-prefix=LIBCXX %s
 // RUN: %clang -target x86_64-apple-darwin -ccc-install-dir %t/bin \
@@ -16,7 +16,7 @@
 // LIBCXX: "-internal-isystem" "[[INSTALLDIR]]/../include/c++/v1"
 
 // Passing -stdlib++-isystem should suppress the default search.
-// RUN: %clang -target aarch64-linux-android -ccc-install-dir %t/bin \
+// RUN: %clang -target aarch64-linux-gnu -ccc-install-dir %t/bin \
 // RUN:   -stdlib++-isystem /tmp/foo -stdlib++-isystem /tmp/bar -stdlib=libc++ \
 // RUN:   -fsyntax-only %s -### 2>&1 | FileCheck -check-prefix=NODEFAULT %s
 // RUN: %clang -target x86_64-apple-darwin -ccc-install-dir %t/bin \
@@ -26,7 +26,7 @@
 // NODEFAULT-NOT: "-internal-isystem" "[[INSTALLDIR]]/../include/c++/v1"
 
 // And we should add it as an -internal-isystem.
-// RUN: %clang -target aarch64-linux-android -ccc-install-dir %t/bin \
+// RUN: %clang -target aarch64-linux-gnu -ccc-install-dir %t/bin \
 // RUN:   -stdlib++-isystem /tmp/foo -stdlib++-isystem /tmp/bar -stdlib=libc++ \
 // RUN:   -fsyntax-only %s -### 2>&1 | FileCheck -check-prefix=INCPATH %s
 // RUN: %clang -target x86_64-apple-darwin -ccc-install-dir %t/bin \
@@ -35,7 +35,7 @@
 // INCPATH: "-internal-isystem" "/tmp/foo" "-internal-isystem" "/tmp/bar"
 
 // We shouldn't pass the -stdlib++-isystem to cc1.
-// RUN: %clang -target aarch64-linux-android -ccc-install-dir %t/bin \
+// RUN: %clang -target aarch64-linux-gnu -ccc-install-dir %t/bin \
 // RUN:   -stdlib++-isystem /tmp -stdlib=libc++ -fsyntax-only %s -### 2>&1 | \
 // RUN:   FileCheck -check-prefix=NOCC1 %s
 // RUN: %clang -target x86_64-apple-darwin -ccc-install-dir %t/bin \
@@ -44,7 +44,7 @@
 // NOCC1-NOT: "-stdlib++-isystem" "/tmp"
 
 // It should respect -nostdinc++.
-// RUN: %clang -target aarch64-linux-android -ccc-install-dir %t/bin \
+// RUN: %clang -target aarch64-linux-gnu -ccc-install-dir %t/bin \
 // RUN:   -stdlib++-isystem /tmp/foo -stdlib++-isystem /tmp/bar -nostdinc++ \
 // RUN:   -fsyntax-only %s -### 2>&1 | FileCheck -check-prefix=NOSTDINCXX %s
 // RUN: %clang -target x86_64-apple-darwin -ccc-install-dir %t/bin \
Index: clang/test/Driver/android-no-installed-libcxx.cpp
===
--- /dev/null
+++ clang/test/Driver/android-no-installed-libcxx.cpp
@@ -0,0 +1,8 @@
+// Check that we don't find the libc++ in the installation directory when
+// targeting Android.
+
+// RUN: mkdir -p %t/bin
+// RUN: mkdir -p %t/include/c++/v1
+// RUN: %clang -target aarch64-linux-android -ccc-install-dir %t/bin \
+// RUN:   -stdlib=libc++ -fsyntax-only %s -### 2>&1 | FileCheck %s
+// CHECK-NOT: "-internal-isystem" "{{.*}}v1"
Index: clang/lib/Driver/ToolChains/Linux.cpp
===
--- clang/lib/Driver/ToolChains/Linux.cpp
+++ clang/lib/Driver/ToolChains/Linux.cpp
@@ -888,20 +888,25 @@
 void Linux::addLibCxxIncludePaths(const llvm::opt::ArgList ,
   llvm::opt::ArgStringList ) const {
   const std::string& SysRoot = computeSysRoot();
-  const std::string LibCXXIncludePathCandidates[] = {
-  DetectLibcxxIncludePath(getVFS(), getDriver().Dir + "/../include/c++"),
-  // If this is a development, non-installed, clang, libcxx will
-  // not be found at ../include/c++ but it likely to be found at
-  // one of the following two locations:
-  DetectLibcxxIncludePath(getVFS(), SysRoot + "/usr/local/include/c++"),
-  DetectLibcxxIncludePath(getVFS(), SysRoot + 

[PATCH] D54406: Add matchDynamic convenience functions

[Stephen Kelly via Phabricator via cfe-commits]

steveire closed this revision.
steveire added a comment.
Herald added a project: clang.

Committed in 2e8dc8590d8b412 



Repository:
  rC Clang

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

https://reviews.llvm.org/D54406



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

[clang] 2e8dc85 - Add matchDynamic convenience functions

[Stephen Kelly via cfe-commits]

Author: Stephen Kelly
Date: 2019-12-07T00:01:29Z
New Revision: 2e8dc8590d8b412a131b127e7aa4aad0d7fc7fa0

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

LOG: Add matchDynamic convenience functions

Summary: These correspond to the existing match() free functions.

Reviewers: aaron.ballman

Subscribers: cfe-commits

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

Added: 


Modified: 
clang/include/clang/ASTMatchers/ASTMatchFinder.h
clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp

Removed: 




diff  --git a/clang/include/clang/ASTMatchers/ASTMatchFinder.h 
b/clang/include/clang/ASTMatchers/ASTMatchFinder.h
index 4c8dd87cecbb..f8160d552c0d 100644
--- a/clang/include/clang/ASTMatchers/ASTMatchFinder.h
+++ b/clang/include/clang/ASTMatchers/ASTMatchFinder.h
@@ -309,6 +309,33 @@ match(MatcherT Matcher, ASTContext ) {
   return std::move(Callback.Nodes);
 }
 
+inline SmallVector
+matchDynamic(internal::DynTypedMatcher Matcher,
+ const ast_type_traits::DynTypedNode , ASTContext ) {
+  internal::CollectMatchesCallback Callback;
+  MatchFinder Finder;
+  Finder.addDynamicMatcher(Matcher, );
+  Finder.match(Node, Context);
+  return std::move(Callback.Nodes);
+}
+
+template 
+SmallVector matchDynamic(internal::DynTypedMatcher Matcher,
+const NodeT ,
+ASTContext ) {
+  return matchDynamic(Matcher, ast_type_traits::DynTypedNode::create(Node),
+  Context);
+}
+
+inline SmallVector
+matchDynamic(internal::DynTypedMatcher Matcher, ASTContext ) {
+  internal::CollectMatchesCallback Callback;
+  MatchFinder Finder;
+  Finder.addDynamicMatcher(Matcher, );
+  Finder.matchAST(Context);
+  return std::move(Callback.Nodes);
+}
+
 } // end namespace ast_matchers
 } // end namespace clang
 

diff  --git a/clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp 
b/clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
index 4c557cfe833f..02514f77afd3 100644
--- a/clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
+++ b/clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
@@ -1827,5 +1827,29 @@ void x(int x) {
   EXPECT_TRUE(notMatchesWithOpenMP(Source4, Matcher));
 }
 
+TEST(MatchFinderAPI, matchesDynamic) {
+
+  std::string SourceCode = "struct A { void f() {} };";
+  auto Matcher = functionDecl(isDefinition()).bind("method");
+
+  auto astUnit = tooling::buildASTFromCode(SourceCode);
+
+  auto GlobalBoundNodes = matchDynamic(Matcher, astUnit->getASTContext());
+
+  EXPECT_EQ(GlobalBoundNodes.size(), 1u);
+  EXPECT_EQ(GlobalBoundNodes[0].getMap().size(), 1u);
+
+  auto GlobalMethodNode = 
GlobalBoundNodes[0].getNodeAs("method");
+  EXPECT_TRUE(GlobalMethodNode != nullptr);
+
+  auto MethodBoundNodes =
+  matchDynamic(Matcher, *GlobalMethodNode, astUnit->getASTContext());
+  EXPECT_EQ(MethodBoundNodes.size(), 1u);
+  EXPECT_EQ(MethodBoundNodes[0].getMap().size(), 1u);
+
+  auto MethodNode = MethodBoundNodes[0].getNodeAs("method");
+  EXPECT_EQ(MethodNode, GlobalMethodNode);
+}
+
 } // namespace ast_matchers
 } // namespace clang



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

[PATCH] D71152: [analyzer] Keep track of escaped locals.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/ProgramState.cpp:48-61
+namespace {
+struct EscapedLocals{};
+} // namespace
+
+template <>
+struct ProgramStateTrait :
+  public ProgramStatePartialTrait> {

Wait, you are preventing direct access anyway by putting this stuff into the 
.cpp file.

In this case i think you can safely use the `REGISTER_...` macros.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71152



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

[PATCH] D70973: [OPENMP50]Treat context selectors as expressions, not just strings.

[Alexey Bataev via Phabricator via cfe-commits]

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



Comment at: clang/include/clang/Basic/DiagnosticParseKinds.td:1230
-  "unknown '%0' device kind trait in the 'device' context selector set, 
expected"
-  " one of 'host', 'nohost', 'cpu', 'gpu' or 'fpga'">;
 

jdoerfert wrote:
> ABataev wrote:
> > jdoerfert wrote:
> > > ABataev wrote:
> > > > jdoerfert wrote:
> > > > > I would have expected this error to be still accurate, maybe with the 
> > > > > addition ", or quoted versions thereof".
> > > > Currently, we could emit it only in codegen phase to avoid double 
> > > > converting from expression to string. Will emit it there.
> > > Why can't we emit this error if the user writes `device={kind(gpu)}` 
> > > anymore? Even `device={kind("gpu")}` should be diagnosable early.
> > The main problem here is the conversion and expression evaluation. We 
> > convert the data from the expression to strings at the codegen phase. I 
> > don't want to do the same thing for the second time in Sema just for 
> > diagnostic.
> We have to convert it during sema already, actually during parsing. I'm 
> working on declare variant begin/end support right now (part of TR8) which 
> needs the information during parsing.
Hmmm, it is where we need to parse the code only if the context matches the 
trait? Ok, will rework to convert it in sema too. Seems to me, we'll need to 
match the context in both, codegen and sema. Will try to move the matcher 
somewhere to avoid duplication in sema and codege.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70973



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

[PATCH] D71142: [Sema] Validate large bitfields

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

rsmith added a comment.

It seems fine and reasonable to reject this as a violation of an implementation 
limit. Can we actually support the full range 2^64 bits range, or would it be 
wiser to pick a smaller limit? (There's at least no point in allowing 
bit-widths that would mean the size of the bitfield doesn't fit in `size_t`, 
which means no more than 35 bits for a 32-bit target.)




Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:5186-5189
+def warn_bitfield_width_exceeds_maximum_width: Error<
+  "width of bit-field %0 doesn't fit in a 64 bit unsigned integer">;
+def warn_anon_bitfield_width_exceeds_maximum_width : Error<
+  "width of anonymous bit-field doesn't fit in a 64 bit unsigned integer">;

aaron.ballman wrote:
> I feel like this situation should be an error rather than a warning -- what 
> could the code possibly have meant?
The name of the diagnostic and the kind of diagnostic should agree. Currently 
we have `warn_` vs `Error<`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71142



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

[PATCH] D71152: [analyzer] Keep track of escaped locals.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

I've been hiding these internal traits in ExprEngine lately but i don't have a 
strong preference. I don't see any immediate need to expose this info to the 
checkers, nor do i see a need to actively hide it.

And, yeah, needs dead region cleanup :)




Comment at: clang/lib/StaticAnalyzer/Core/ProgramState.cpp:229-233
+for (const MemRegion *R : Invalidated) {
+  if (!R->hasStackStorage())
+continue;
+  newState = newState->add(R);
+}

Ok, so how do we want to deal with non-base regions? I think for the purposes 
of escaping we try to stick to per-base-region granularity (cf. the C-style 
linked list example). This is also going to be nice because the amount of 
regions we'll have to track is going to be more limited and there's going to be 
no problem of representing the same region in multiple different ways.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71152



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

[PATCH] D71152: [analyzer] Keep track of escaped locals.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Hiding the trait in `ExprEngine` has the additional benefit of disallowing 
*direct* access to the trait - it can only be accessed through getters. It also 
doesn't prevent us from exposing the trait to checkers as a method on 
`ProgramState` in the future, because `ProgramStateManager` has access to 
`SubEngine`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71152



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

[PATCH] D68627: [Sema][X86] Consider target attribute into the checks in validateOutputSize and validateInputSize.

[Reid Kleckner via Phabricator via cfe-commits]

rnk added inline comments.



Comment at: clang/include/clang/AST/ASTContext.h:2826
+  /// valid feature names.
+  TargetAttr::ParsedTargetAttr
+  filterFunctionTargetAttrs(const TargetAttr *TD) const;

I reverted this because it requires TargetAttr to be complete here, which I 
have been working for a few weeks to avoid. If you pull ParsedTargetAttr out of 
the TargetAttr class, then you can forward declare it, and reland. Sorry I 
didn't see that in review, this commit raced with 
rG60573ae6fe509b618dc6a2c5c55d921bccd77608, which removes the Attr.h include in 
ASTContext.h.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68627



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

[PATCH] D62056: Use ASTDumper to dump the AST from clang-query

[Stephen Kelly via Phabricator via cfe-commits]

steveire closed this revision.
steveire added a comment.

Committed in b22d8ae7f436bfe63 
. I don't 
know why this review didn't automatically update.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D62056



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

[clang] eff08f4 - Revert "[Sema][X86] Consider target attribute into the checks in validateOutputSize and validateInputSize."

[Reid Kleckner via cfe-commits]

Author: Reid Kleckner
Date: 2019-12-06T15:42:14-08:00
New Revision: eff08f40976e177923fe95759917e59375458f71

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

LOG: Revert "[Sema][X86] Consider target attribute into the checks in 
validateOutputSize and validateInputSize."

This reverts commit e1578fd2b79fe5af5f80c0c166a8abd0f816c022.

It introduces a dependency on Attr.h which I am removing from
ASTContext.h.

Added: 


Modified: 
clang/include/clang/AST/ASTContext.h
clang/include/clang/Basic/TargetInfo.h
clang/lib/AST/ASTContext.cpp
clang/lib/Basic/Targets/X86.cpp
clang/lib/Basic/Targets/X86.h
clang/lib/CodeGen/CodeGenFunction.cpp
clang/lib/CodeGen/CodeGenModule.cpp
clang/lib/CodeGen/CodeGenModule.h
clang/lib/Sema/SemaStmtAsm.cpp
clang/test/CodeGen/x86_32-inline-asm.c

Removed: 




diff  --git a/clang/include/clang/AST/ASTContext.h 
b/clang/include/clang/AST/ASTContext.h
index 6f49dc02be07..024aa1cb021e 100644
--- a/clang/include/clang/AST/ASTContext.h
+++ b/clang/include/clang/AST/ASTContext.h
@@ -95,7 +95,6 @@ class CXXRecordDecl;
 class DiagnosticsEngine;
 class Expr;
 class FixedPointSemantics;
-class GlobalDecl;
 class MangleContext;
 class MangleNumberingContext;
 class MaterializeTemporaryExpr;
@@ -2821,16 +2820,6 @@ class ASTContext : public RefCountedBase {
   /// PredefinedExpr to cache evaluated results.
   StringLiteral *getPredefinedStringLiteralFromCache(StringRef Key) const;
 
-  /// Parses the target attributes passed in, and returns only the ones that 
are
-  /// valid feature names.
-  TargetAttr::ParsedTargetAttr
-  filterFunctionTargetAttrs(const TargetAttr *TD) const;
-
-  void getFunctionFeatureMap(llvm::StringMap ,
- const FunctionDecl *) const;
-  void getFunctionFeatureMap(llvm::StringMap ,
- GlobalDecl GD) const;
-
   
//======//
   //Statistics
   
//======//

diff  --git a/clang/include/clang/Basic/TargetInfo.h 
b/clang/include/clang/Basic/TargetInfo.h
index ad863fa1edbd..33cecdadc686 100644
--- a/clang/include/clang/Basic/TargetInfo.h
+++ b/clang/include/clang/Basic/TargetInfo.h
@@ -945,14 +945,12 @@ class TargetInfo : public virtual TransferrableTargetInfo,
   bool validateInputConstraint(MutableArrayRef 
OutputConstraints,
ConstraintInfo ) const;
 
-  virtual bool validateOutputSize(const llvm::StringMap ,
-  StringRef /*Constraint*/,
+  virtual bool validateOutputSize(StringRef /*Constraint*/,
   unsigned /*Size*/) const {
 return true;
   }
 
-  virtual bool validateInputSize(const llvm::StringMap ,
- StringRef /*Constraint*/,
+  virtual bool validateInputSize(StringRef /*Constraint*/,
  unsigned /*Size*/) const {
 return true;
   }

diff  --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index ad61a5a56bc3..4fd7e20dac84 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -10779,66 +10779,3 @@ QualType 
ASTContext::getCorrespondingSignedFixedPointType(QualType Ty) const {
 llvm_unreachable("Unexpected unsigned fixed point type");
   }
 }
-
-TargetAttr::ParsedTargetAttr
-ASTContext::filterFunctionTargetAttrs(const TargetAttr *TD) const {
-  assert(TD != nullptr);
-  TargetAttr::ParsedTargetAttr ParsedAttr = TD->parse();
-
-  ParsedAttr.Features.erase(
-  llvm::remove_if(ParsedAttr.Features,
-  [&](const std::string ) {
-return !Target->isValidFeatureName(
-StringRef{Feat}.substr(1));
-  }),
-  ParsedAttr.Features.end());
-  return ParsedAttr;
-}
-
-void ASTContext::getFunctionFeatureMap(llvm::StringMap ,
-   const FunctionDecl *FD) const {
-  if (FD)
-getFunctionFeatureMap(FeatureMap, GlobalDecl().getWithDecl(FD));
-  else
-Target->initFeatureMap(FeatureMap, getDiagnostics(),
-   Target->getTargetOpts().CPU,
-   Target->getTargetOpts().Features);
-}
-
-// Fills in the supplied string map with the set of target features for the
-// passed in function.
-void ASTContext::getFunctionFeatureMap(llvm::StringMap ,
-   GlobalDecl GD) const {
-  StringRef TargetCPU = Target->getTargetOpts().CPU;
-  const FunctionDecl *FD = GD.getDecl()->getAsFunction();
-  if (const auto *TD = FD->getAttr()) {
-TargetAttr::ParsedTargetAttr ParsedAttr = filterFunctionTargetAttrs(TD);
-

[clang-tools-extra] b22d8ae - Use ASTDumper to dump the AST from clang-query

[Stephen Kelly via cfe-commits]

Author: Stephen Kelly
Date: 2019-12-06T23:38:56Z
New Revision: b22d8ae7f436bfe63b28ceddea743071a6601eb1

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

LOG: Use ASTDumper to dump the AST from clang-query

Summary:
This way, the output is not limited by the various API differences
between the dump() member functions.  For example, all dumps are now in
color, while that used to be the case only for Decls and Stmts, but not
Types.

Additionally, while DynTypedNode::dump (which was used up to now) was
limited to dumping only Decls, Stmts and Types, this makes clang-query
support everything ASTNodeTraverser supports.

Reviewers: aaron.ballman

Subscribers: cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang-tools-extra/clang-query/Query.cpp

Removed: 




diff  --git a/clang-tools-extra/clang-query/Query.cpp 
b/clang-tools-extra/clang-query/Query.cpp
index eea0e7766d3e..675fd968f46e 100644
--- a/clang-tools-extra/clang-query/Query.cpp
+++ b/clang-tools-extra/clang-query/Query.cpp
@@ -8,6 +8,7 @@
 
 #include "Query.h"
 #include "QuerySession.h"
+#include "clang/AST/ASTDumper.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/Frontend/ASTUnit.h"
 #include "clang/Frontend/TextDiagnostic.h"
@@ -128,7 +129,11 @@ bool MatchQuery::run(llvm::raw_ostream , QuerySession 
) const {
 }
 if (QS.DetailedASTOutput) {
   OS << "Binding for \"" << BI->first << "\":\n";
-  BI->second.dump(OS, AST->getSourceManager());
+  const ASTContext  = AST->getASTContext();
+  const SourceManager  = Ctx.getSourceManager();
+  ASTDumper Dumper(OS, (), ,
+SM.getDiagnostics().getShowColors(), Ctx.getPrintingPolicy());
+  Dumper.Visit(BI->second);
   OS << "\n";
 }
   }



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

[PATCH] D71082: Allow system header to provide their own implementation of some builtin

[serge via Phabricator via cfe-commits]

serge-sans-paille updated this revision to Diff 232667.
serge-sans-paille added a comment.

Fix interaction with fortify warnings (cc @efriedma)

Also I've checked interactions with the test suite I wrote here 
https://github.com/serge-sans-paille/fortify-test-suite/ and clang now passes 
all the dynamic check, and only fails a few static checks, which is a great 
improvement. I still need to double check all the examples pointed out by 
@george.burgess.iv .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71082

Files:
  clang/include/clang/AST/Decl.h
  clang/lib/AST/Decl.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGen/memcpy-nobuitin.c
  clang/test/CodeGen/memcpy-nobuitin.inc


Index: clang/test/CodeGen/memcpy-nobuitin.inc
===
--- /dev/null
+++ clang/test/CodeGen/memcpy-nobuitin.inc
@@ -0,0 +1,12 @@
+#include 
+extern void *memcpy(void *dest, void const *from, size_t n);
+
+#ifdef WITH_DECL
+inline void *memcpy(void *dest, void const *from, size_t n) {
+  char const *ifrom = from;
+  char *idest = dest;
+  while (n--)
+*idest++ = *ifrom++;
+  return dest;
+}
+#endif
Index: clang/test/CodeGen/memcpy-nobuitin.c
===
--- /dev/null
+++ clang/test/CodeGen/memcpy-nobuitin.c
@@ -0,0 +1,8 @@
+// RUN: %clang -S -emit-llvm -o- %s -isystem %S -DWITH_DECL | FileCheck 
--check-prefix=CHECK-WITH-DECL %s
+// RUN: %clang -S -emit-llvm -o- %s -isystem %S -UWITH_DECL | FileCheck 
--check-prefix=CHECK-NO-DECL %s
+// CHECK-WITH-DECL-NOT: @llvm.memcpy
+// CHECK-NO-DECL: @llvm.memcpy
+#include 
+void test(void *dest, void const *from, size_t n) {
+  memcpy(dest, from, n);
+}
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -1831,6 +1831,11 @@
   else if (const auto *SA = FD->getAttr())
  F->setSection(SA->getName());
 
+  if (FD->isReplaceableSystemFunction()) {
+F->addAttribute(llvm::AttributeList::FunctionIndex,
+llvm::Attribute::NoBuiltin);
+  }
+
   if (FD->isReplaceableGlobalAllocationFunction()) {
 // A replaceable global allocation function does not act like a builtin by
 // default, only if it is invoked by a new-expression or delete-expression.
Index: clang/lib/AST/Decl.cpp
===
--- clang/lib/AST/Decl.cpp
+++ clang/lib/AST/Decl.cpp
@@ -3003,6 +3003,15 @@
   return Params == FPT->getNumParams();
 }
 
+bool FunctionDecl::isReplaceableSystemFunction() const {
+  FunctionDecl const *Definition;
+  if (hasBody(Definition)) {
+const SourceManager  = getASTContext().getSourceManager();
+return SM.isInSystemHeader(Definition->getLocation());
+  }
+  return false;
+}
+
 bool FunctionDecl::isDestroyingOperatorDelete() const {
   // C++ P0722:
   //   Within a class C, a single object deallocation function with signature
@@ -3165,6 +3174,12 @@
   // function. Determine whether it actually refers to the C library
   // function or whether it just has the same name.
 
+  // If a system-level body was provided, use it instead of the intrinsic. Some
+  // C library do that to implement fortified version.
+  if (!ConsiderWrapperFunctions && isReplaceableSystemFunction()) {
+return 0;
+  }
+
   // If this is a static function, it's not a builtin.
   if (!ConsiderWrapperFunctions && getStorageClass() == SC_Static)
 return 0;
Index: clang/include/clang/AST/Decl.h
===
--- clang/include/clang/AST/Decl.h
+++ clang/include/clang/AST/Decl.h
@@ -2272,6 +2272,9 @@
   /// true through IsAligned.
   bool isReplaceableGlobalAllocationFunction(bool *IsAligned = nullptr) const;
 
+  /// Determine if this function provides the implementation of a system 
Builtin
+  bool isReplaceableSystemFunction() const;
+
   /// Determine whether this is a destroying operator delete.
   bool isDestroyingOperatorDelete() const;
 


Index: clang/test/CodeGen/memcpy-nobuitin.inc
===
--- /dev/null
+++ clang/test/CodeGen/memcpy-nobuitin.inc
@@ -0,0 +1,12 @@
+#include 
+extern void *memcpy(void *dest, void const *from, size_t n);
+
+#ifdef WITH_DECL
+inline void *memcpy(void *dest, void const *from, size_t n) {
+  char const *ifrom = from;
+  char *idest = dest;
+  while (n--)
+*idest++ = *ifrom++;
+  return dest;
+}
+#endif
Index: clang/test/CodeGen/memcpy-nobuitin.c
===
--- /dev/null
+++ clang/test/CodeGen/memcpy-nobuitin.c
@@ -0,0 +1,8 @@
+// RUN: %clang -S -emit-llvm -o- %s -isystem %S -DWITH_DECL | FileCheck --check-prefix=CHECK-WITH-DECL %s
+// RUN: %clang -S -emit-llvm -o- %s -isystem %S 

[PATCH] D70973: [OPENMP50]Treat context selectors as expressions, not just strings.

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticParseKinds.td:1230
-  "unknown '%0' device kind trait in the 'device' context selector set, 
expected"
-  " one of 'host', 'nohost', 'cpu', 'gpu' or 'fpga'">;
 

ABataev wrote:
> jdoerfert wrote:
> > ABataev wrote:
> > > jdoerfert wrote:
> > > > I would have expected this error to be still accurate, maybe with the 
> > > > addition ", or quoted versions thereof".
> > > Currently, we could emit it only in codegen phase to avoid double 
> > > converting from expression to string. Will emit it there.
> > Why can't we emit this error if the user writes `device={kind(gpu)}` 
> > anymore? Even `device={kind("gpu")}` should be diagnosable early.
> The main problem here is the conversion and expression evaluation. We convert 
> the data from the expression to strings at the codegen phase. I don't want to 
> do the same thing for the second time in Sema just for diagnostic.
We have to convert it during sema already, actually during parsing. I'm working 
on declare variant begin/end support right now (part of TR8) which needs the 
information during parsing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70973



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

[clang] e1578fd - [Sema][X86] Consider target attribute into the checks in validateOutputSize and validateInputSize.

[Craig Topper via cfe-commits]

Author: Craig Topper
Date: 2019-12-06T15:30:59-08:00
New Revision: e1578fd2b79fe5af5f80c0c166a8abd0f816c022

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

LOG: [Sema][X86] Consider target attribute into the checks in 
validateOutputSize and validateInputSize.

The validateOutputSize and validateInputSize need to check whether
AVX or AVX512 are enabled. But this can be affected by the
target attribute so we need to factor that in.

This patch copies some of the code from CodeGen to create an
appropriate feature map that we can pass to the function. Probably
need some refactoring here to share more code with Codegen. Is
there a good place to do that? Also need to support the cpu_specific
attribute as well.

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

Added: 


Modified: 
clang/include/clang/AST/ASTContext.h
clang/include/clang/Basic/TargetInfo.h
clang/lib/AST/ASTContext.cpp
clang/lib/Basic/Targets/X86.cpp
clang/lib/Basic/Targets/X86.h
clang/lib/CodeGen/CodeGenFunction.cpp
clang/lib/CodeGen/CodeGenModule.cpp
clang/lib/CodeGen/CodeGenModule.h
clang/lib/Sema/SemaStmtAsm.cpp
clang/test/CodeGen/x86_32-inline-asm.c

Removed: 




diff  --git a/clang/include/clang/AST/ASTContext.h 
b/clang/include/clang/AST/ASTContext.h
index 024aa1cb021e..6f49dc02be07 100644
--- a/clang/include/clang/AST/ASTContext.h
+++ b/clang/include/clang/AST/ASTContext.h
@@ -95,6 +95,7 @@ class CXXRecordDecl;
 class DiagnosticsEngine;
 class Expr;
 class FixedPointSemantics;
+class GlobalDecl;
 class MangleContext;
 class MangleNumberingContext;
 class MaterializeTemporaryExpr;
@@ -2820,6 +2821,16 @@ class ASTContext : public RefCountedBase {
   /// PredefinedExpr to cache evaluated results.
   StringLiteral *getPredefinedStringLiteralFromCache(StringRef Key) const;
 
+  /// Parses the target attributes passed in, and returns only the ones that 
are
+  /// valid feature names.
+  TargetAttr::ParsedTargetAttr
+  filterFunctionTargetAttrs(const TargetAttr *TD) const;
+
+  void getFunctionFeatureMap(llvm::StringMap ,
+ const FunctionDecl *) const;
+  void getFunctionFeatureMap(llvm::StringMap ,
+ GlobalDecl GD) const;
+
   
//======//
   //Statistics
   
//======//

diff  --git a/clang/include/clang/Basic/TargetInfo.h 
b/clang/include/clang/Basic/TargetInfo.h
index 33cecdadc686..ad863fa1edbd 100644
--- a/clang/include/clang/Basic/TargetInfo.h
+++ b/clang/include/clang/Basic/TargetInfo.h
@@ -945,12 +945,14 @@ class TargetInfo : public virtual TransferrableTargetInfo,
   bool validateInputConstraint(MutableArrayRef 
OutputConstraints,
ConstraintInfo ) const;
 
-  virtual bool validateOutputSize(StringRef /*Constraint*/,
+  virtual bool validateOutputSize(const llvm::StringMap ,
+  StringRef /*Constraint*/,
   unsigned /*Size*/) const {
 return true;
   }
 
-  virtual bool validateInputSize(StringRef /*Constraint*/,
+  virtual bool validateInputSize(const llvm::StringMap ,
+ StringRef /*Constraint*/,
  unsigned /*Size*/) const {
 return true;
   }

diff  --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index 4fd7e20dac84..ad61a5a56bc3 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -10779,3 +10779,66 @@ QualType 
ASTContext::getCorrespondingSignedFixedPointType(QualType Ty) const {
 llvm_unreachable("Unexpected unsigned fixed point type");
   }
 }
+
+TargetAttr::ParsedTargetAttr
+ASTContext::filterFunctionTargetAttrs(const TargetAttr *TD) const {
+  assert(TD != nullptr);
+  TargetAttr::ParsedTargetAttr ParsedAttr = TD->parse();
+
+  ParsedAttr.Features.erase(
+  llvm::remove_if(ParsedAttr.Features,
+  [&](const std::string ) {
+return !Target->isValidFeatureName(
+StringRef{Feat}.substr(1));
+  }),
+  ParsedAttr.Features.end());
+  return ParsedAttr;
+}
+
+void ASTContext::getFunctionFeatureMap(llvm::StringMap ,
+   const FunctionDecl *FD) const {
+  if (FD)
+getFunctionFeatureMap(FeatureMap, GlobalDecl().getWithDecl(FD));
+  else
+Target->initFeatureMap(FeatureMap, getDiagnostics(),
+   Target->getTargetOpts().CPU,
+   Target->getTargetOpts().Features);
+}
+
+// Fills in the supplied string map with the set of target features for the
+// passed in 

[PATCH] D68627: [Sema][X86] Consider target attribute into the checks in validateOutputSize and validateInputSize.

[Craig Topper via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGe1578fd2b79f: [Sema][X86] Consider target attribute into the 
checks in validateOutputSize and… (authored by craig.topper).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68627

Files:
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/Basic/TargetInfo.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/Basic/Targets/X86.cpp
  clang/lib/Basic/Targets/X86.h
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/Sema/SemaStmtAsm.cpp
  clang/test/CodeGen/x86_32-inline-asm.c

Index: clang/test/CodeGen/x86_32-inline-asm.c
===
--- clang/test/CodeGen/x86_32-inline-asm.c
+++ clang/test/CodeGen/x86_32-inline-asm.c
@@ -70,3 +70,35 @@
   __asm__ volatile("foo1 %0" : "=x" (val256)); // expected-error {{invalid output size for constraint '=x'}}
 #endif
 }
+
+int __attribute__((__target__("sse"))) _func2() {
+  __asm__ volatile("foo1 %0" : : "x" (val128)); // No error.
+  __asm__ volatile("foo1 %0" : "=x" (val128));  // No error.
+#ifdef __AVX__
+  __asm__ volatile("foo1 %0" : : "x" (val256)); // No error.
+  __asm__ volatile("foo1 %0" : "=x" (val256));  // No error.
+#else
+  __asm__ volatile("foo1 %0" : : "x" (val256)); // expected-error {{invalid input size for constraint 'x'}}
+  __asm__ volatile("foo1 %0" : "=x" (val256)); // expected-error {{invalid output size for constraint '=x'}}
+#endif
+  __asm__ volatile("foo1 %0" : : "x" (val512)); // expected-error {{invalid input size for constraint 'x'}}
+  __asm__ volatile("foo1 %0" : "=x" (val512)); // expected-error {{invalid output size for constraint '=x'}}
+}
+
+int __attribute__((__target__("avx"))) _func3() {
+  __asm__ volatile("foo1 %0" : : "x" (val128)); // No error.
+  __asm__ volatile("foo1 %0" : "=x" (val128));  // No error.
+  __asm__ volatile("foo1 %0" : : "x" (val256)); // No error.
+  __asm__ volatile("foo1 %0" : "=x" (val256));  // No error.
+  __asm__ volatile("foo1 %0" : : "x" (val512)); // expected-error {{invalid input size for constraint 'x'}}
+  __asm__ volatile("foo1 %0" : "=x" (val512)); // expected-error {{invalid output size for constraint '=x'}}
+}
+
+int __attribute__((__target__("avx512f"))) _func4() {
+  __asm__ volatile("foo1 %0" : : "x" (val128)); // No error.
+  __asm__ volatile("foo1 %0" : "=x" (val128));  // No error.
+  __asm__ volatile("foo1 %0" : : "x" (val256)); // No error.
+  __asm__ volatile("foo1 %0" : "=x" (val256));  // No error.
+  __asm__ volatile("foo1 %0" : : "x" (val512)); // No error.
+  __asm__ volatile("foo1 %0" : "=x" (val512)); // No error.
+}
Index: clang/lib/Sema/SemaStmtAsm.cpp
===
--- clang/lib/Sema/SemaStmtAsm.cpp
+++ clang/lib/Sema/SemaStmtAsm.cpp
@@ -11,6 +11,7 @@
 //===--===//
 
 #include "clang/AST/ExprCXX.h"
+#include "clang/AST/GlobalDecl.h"
 #include "clang/AST/RecordLayout.h"
 #include "clang/AST/TypeLoc.h"
 #include "clang/Basic/TargetInfo.h"
@@ -255,6 +256,10 @@
   // The parser verifies that there is a string literal here.
   assert(AsmString->isAscii());
 
+  FunctionDecl *FD = dyn_cast(getCurLexicalContext());
+  llvm::StringMap FeatureMap;
+  Context.getFunctionFeatureMap(FeatureMap, FD);
+
   for (unsigned i = 0; i != NumOutputs; i++) {
 StringLiteral *Literal = Constraints[i];
 assert(Literal->isAscii());
@@ -325,8 +330,8 @@
 }
 
 unsigned Size = Context.getTypeSize(OutputExpr->getType());
-if (!Context.getTargetInfo().validateOutputSize(Literal->getString(),
-Size)) {
+if (!Context.getTargetInfo().validateOutputSize(
+FeatureMap, Literal->getString(), Size)) {
   targetDiag(OutputExpr->getBeginLoc(), diag::err_asm_invalid_output_size)
   << Info.getConstraintStr();
   return new (Context)
@@ -427,8 +432,8 @@
 return StmtError();
 
 unsigned Size = Context.getTypeSize(Ty);
-if (!Context.getTargetInfo().validateInputSize(Literal->getString(),
-   Size))
+if (!Context.getTargetInfo().validateInputSize(FeatureMap,
+   Literal->getString(), Size))
   return StmtResult(
   targetDiag(InputExpr->getBeginLoc(), diag::err_asm_invalid_input_size)
   << Info.getConstraintStr());
Index: clang/lib/CodeGen/CodeGenModule.h
===
--- clang/lib/CodeGen/CodeGenModule.h
+++ clang/lib/CodeGen/CodeGenModule.h
@@ -1150,14 +1150,6 @@
   /// It's up to you to ensure that this is safe.
   void AddDefaultFnAttrs(llvm::Function );
 
-  /// Parses the target attributes passed in, and 

[PATCH] D71152: [analyzer] Keep track of escaped locals.

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

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

So some of the questions that came to my mind:

1. Should this be exposed to the checkers?
2. Where should we actually have this trait registered? ProgramState? 
ExprEngine? Both of them need access, and due to layering I feel like it cannot 
be in ExprEngine but I might be wrong.


This is working progress but I wanted to share early to discuss the direction.

This looks relatively painless so far, but note that the locals are not cleaned 
up from the state just yet.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71152

Files:
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
  clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
  clang/lib/StaticAnalyzer/Core/ProgramState.cpp
  clang/test/Analysis/symbol-escape.cpp

Index: clang/test/Analysis/symbol-escape.cpp
===
--- clang/test/Analysis/symbol-escape.cpp
+++ clang/test/Analysis/symbol-escape.cpp
@@ -31,3 +31,13 @@
   return Baz;
 }
 
+void save_ptr(int **);
+void delete_saved();
+
+void store_to_escaped_region() {
+  int *p;
+  save_ptr();
+  p = new int;
+  delete_saved();
+} // no-warning:
+
Index: clang/lib/StaticAnalyzer/Core/ProgramState.cpp
===
--- clang/lib/StaticAnalyzer/Core/ProgramState.cpp
+++ clang/lib/StaticAnalyzer/Core/ProgramState.cpp
@@ -16,6 +16,7 @@
 #include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/DynamicType.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SubEngine.h"
 #include "llvm/Support/raw_ostream.h"
@@ -41,7 +42,23 @@
 Mgr.freeStates.push_back(s);
   }
 }
-}}
+} // namespace ento
+} // namespace clang
+
+namespace {
+struct EscapedLocals{};
+} // namespace
+
+template <>
+struct ProgramStateTrait :
+  public ProgramStatePartialTrait> {
+  static void *GDMIndex();
+};
+
+void *ProgramStateTrait::GDMIndex() {
+  static int index = 0;
+  return 
+}
 
 ProgramState::ProgramState(ProgramStateManager *mgr, const Environment& env,
  StoreRef st, GenericDataMap gdm)
@@ -209,6 +226,12 @@
   ProgramStateRef newState = makeWithStore(newStore);
 
   if (CausedByPointerEscape) {
+for (const MemRegion *R : Invalidated) {
+  if (!R->hasStackStorage())
+continue;
+  newState = newState->add(R);
+}
+
 newState = Eng.notifyCheckersOfPointerEscape(newState, IS,
  TopLevelInvalidated,
  Call,
@@ -642,3 +665,7 @@
   }
   return true;
 }
+
+bool ProgramState::isEscapedLocal(const MemRegion *R) const {
+  return this->contains(R);
+}
Index: clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -2680,7 +2680,8 @@
 
 // A value escapes in four possible cases:
 // (1) We are binding to something that is not a memory region.
-// (2) We are binding to a MemRegion that does not have stack storage.
+// (2) We are binding to a MemRegion that does not have stack storage
+// or the stack storage is escaped.
 // (3) We are binding to a top-level parameter region with a non-trivial
 // destructor. We won't see the destructor during analysis, but it's there.
 // (4) We are binding to a MemRegion with stack storage that the store
@@ -2691,7 +2692,7 @@
 
   // Cases (1) and (2).
   const MemRegion *MR = Loc.getAsRegion();
-  if (!MR || !MR->hasStackStorage())
+  if (!MR || !MR->hasStackStorage() || State->isEscapedLocal(MR))
 return escapeValue(State, Val, PSK_EscapeOnBind);
 
   // Case (3).
Index: clang/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
===
--- clang/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
+++ clang/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
@@ -345,6 +345,9 @@
   /// a value of such type.
   SVal getSValAsScalarOrLoc(const MemRegion *R) const;
 
+  /// TODO
+  bool isEscapedLocal(const MemRegion *R) const;
+
   using region_iterator = const MemRegion **;
 
   /// Visits the symbols reachable from the given SVal using the provided
@@ -872,8 +875,8 @@
   bool scan(const SymExpr *sym);
 };
 
-} // end ento namespace
+} // namespace ento
 
-} // end clang namespace
+} // namespace clang
 
 #endif

[clang-tools-extra] 60573ae - Remove Expr.h include from ASTContext.h, NFC

[Reid Kleckner via cfe-commits]

Author: Reid Kleckner
Date: 2019-12-06T15:30:49-08:00
New Revision: 60573ae6fe509b618dc6a2c5c55d921bccd77608

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

LOG: Remove Expr.h include from ASTContext.h, NFC

ASTContext.h is popular, prune its includes. Expr.h brings in Attr.h,
which is also expensive.

Move BlockVarCopyInit to Expr.h to accomplish this.

Added: 


Modified: 
clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
clang/include/clang/AST/ASTContext.h
clang/include/clang/AST/ASTFwd.h
clang/include/clang/AST/ASTTypeTraits.h
clang/include/clang/AST/Expr.h
clang/include/clang/AST/TypeLoc.h
clang/include/clang/StaticAnalyzer/Checkers/SValExplainer.h
clang/lib/AST/ASTContext.cpp
clang/lib/AST/ASTTypeTraits.cpp
clang/lib/AST/Decl.cpp
clang/lib/AST/DeclCXX.cpp
clang/lib/AST/ExprConstant.cpp
clang/lib/AST/TypeLoc.cpp
clang/lib/Analysis/CloneDetection.cpp
clang/lib/Index/IndexDecl.cpp
clang/lib/Index/IndexSymbol.cpp
clang/lib/Index/IndexingContext.cpp
clang/lib/Index/USRGeneration.cpp
clang/lib/Serialization/ASTWriterDecl.cpp
clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp
clang/lib/StaticAnalyzer/Checkers/MacOSXAPIChecker.cpp
clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp
clang/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
clang/lib/StaticAnalyzer/Core/CallEvent.cpp
clang/lib/Tooling/Refactoring/ASTSelectionRequirements.cpp

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp 
b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
index 288919862e2d..12e5fed4d631 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -19,6 +19,7 @@
 #include "ClangTidyOptions.h"
 #include "GlobList.h"
 #include "clang/AST/ASTDiagnostic.h"
+#include "clang/AST/Attr.h"
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/DiagnosticOptions.h"
 #include "clang/Frontend/DiagnosticRenderer.h"

diff  --git a/clang/include/clang/AST/ASTContext.h 
b/clang/include/clang/AST/ASTContext.h
index 4ba54fa51885..024aa1cb021e 100644
--- a/clang/include/clang/AST/ASTContext.h
+++ b/clang/include/clang/AST/ASTContext.h
@@ -22,7 +22,6 @@
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclBase.h"
 #include "clang/AST/DeclarationName.h"
-#include "clang/AST/Expr.h"
 #include "clang/AST/ExternalASTSource.h"
 #include "clang/AST/NestedNameSpecifier.h"
 #include "clang/AST/PrettyPrinter.h"
@@ -124,6 +123,7 @@ class UnresolvedSetIterator;
 class UsingShadowDecl;
 class VarTemplateDecl;
 class VTableContextBase;
+struct BlockVarCopyInit;
 
 namespace Builtin {
 
@@ -158,22 +158,6 @@ struct TypeInfo {
 /// Holds long-lived AST nodes (such as types and decls) that can be
 /// referred to throughout the semantic analysis of a file.
 class ASTContext : public RefCountedBase {
-public:
-  /// Copy initialization expr of a __block variable and a boolean flag that
-  /// indicates whether the expression can throw.
-  struct BlockVarCopyInit {
-BlockVarCopyInit() = default;
-BlockVarCopyInit(Expr *CopyExpr, bool CanThrow)
-: ExprAndFlag(CopyExpr, CanThrow) {}
-void setExprAndFlag(Expr *CopyExpr, bool CanThrow) {
-  ExprAndFlag.setPointerAndInt(CopyExpr, CanThrow);
-}
-Expr *getCopyExpr() const { return ExprAndFlag.getPointer(); }
-bool canThrow() const { return ExprAndFlag.getInt(); }
-llvm::PointerIntPair ExprAndFlag;
-  };
-
-private:
   friend class NestedNameSpecifier;
 
   mutable SmallVector Types;

diff  --git a/clang/include/clang/AST/ASTFwd.h 
b/clang/include/clang/AST/ASTFwd.h
index 25c321485443..5a891817b336 100644
--- a/clang/include/clang/AST/ASTFwd.h
+++ b/clang/include/clang/AST/ASTFwd.h
@@ -26,6 +26,10 @@ class Type;
 #define TYPE(DERIVED, BASE) class DERIVED##Type;
 #include "clang/AST/TypeNodes.inc"
 class CXXCtorInitializer;
+class OMPClause;
+#define OPENMP_CLAUSE(KIND, CLASSNAME) class CLASSNAME;
+#include "clang/Basic/OpenMPKinds.def"
+
 
 } // end namespace clang
 

diff  --git a/clang/include/clang/AST/ASTTypeTraits.h 
b/clang/include/clang/AST/ASTTypeTraits.h
index dd4ead2f0c2b..3e2f4162e131 100644
--- a/clang/include/clang/AST/ASTTypeTraits.h
+++ b/clang/include/clang/AST/ASTTypeTraits.h
@@ -16,10 +16,7 @@
 #define LLVM_CLANG_AST_ASTTYPETRAITS_H
 
 #include "clang/AST/ASTFwd.h"
-#include "clang/AST/Decl.h"
 #include "clang/AST/NestedNameSpecifier.h"
-#include "clang/AST/OpenMPClause.h"
-#include "clang/AST/Stmt.h"
 #include "clang/AST/TemplateBase.h"
 #include "clang/AST/TypeLoc.h"
 #include "clang/Basic/LLVM.h"

diff  --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h
index 

[PATCH] D71152: [analyzer] Keep track of escaped locals.

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

xazax.hun added a comment.

So some of the questions that came to my mind:

1. Should this be exposed to the checkers?
2. Where should we actually have this trait registered? ProgramState? 
ExprEngine? Both of them need access, and due to layering I feel like it cannot 
be in ExprEngine but I might be wrong.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71152



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

[PATCH] D61837: Make it possible control matcher traversal kind with ASTContext

[Stephen Kelly via Phabricator via cfe-commits]

steveire added a comment.

I tried using DenseMap, but couldn't make it compile. I stuck with std::map for 
now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61837



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

[PATCH] D70877: [WebAssebmly][MC] Support .import_name/.import_field asm directives

[Sam Clegg via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGb4f4e370b59a: [WebAssebmly][MC] Support 
.import_name/.import_field asm directives (authored by sbc100).

Changed prior to commit:
  https://reviews.llvm.org/D70877?vs=231619=232660#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70877

Files:
  clang/include/clang/Basic/AttrDocs.td
  lld/test/wasm/import-name.ll
  lld/test/wasm/import-names.ll
  llvm/include/llvm/MC/MCSymbolWasm.h
  llvm/lib/MC/WasmObjectWriter.cpp
  llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp
  llvm/test/MC/WebAssembly/import-module.ll
  llvm/test/MC/WebAssembly/import-module.s

Index: llvm/test/MC/WebAssembly/import-module.s
===
--- /dev/null
+++ llvm/test/MC/WebAssembly/import-module.s
@@ -0,0 +1,33 @@
+# RUN: llvm-mc -triple=wasm32 < %s | FileCheck %s -check-prefix=CHECK-ASM
+# RUN: llvm-mc -triple=wasm32 -filetype=obj -o - < %s | obj2yaml | FileCheck %s
+
+test:
+  .functype test () -> ()
+  call  foo
+  call  plain
+  end_function
+
+  .functype foo () -> ()
+  .functype plain () -> ()
+  .import_module  foo, bar
+  .import_name  foo, qux
+
+# CHECK-ASM: .import_module  foo, bar
+# CHECK-ASM: .import_name  foo, qux
+
+# CHECK:- Type:IMPORT
+# CHECK-NEXT: Imports:
+# CHECK:- Module:  bar
+# CHECK-NEXT: Field:   qux
+# CHECK-NEXT: Kind:FUNCTION
+
+# CHECK:- Module:  env
+# CHECK-NEXT: Field:   plain
+# CHECK-NEXT: Kind:FUNCTION
+
+# CHECK:- Type:CUSTOM
+# CHECK:  Name:foo
+# CHECK-NEXT: Flags:   [ UNDEFINED, EXPLICIT_NAME ]
+
+# CHECK:  Name:plain
+# CHECK-NEXT: Flags:   [ UNDEFINED ]
Index: llvm/test/MC/WebAssembly/import-module.ll
===
--- llvm/test/MC/WebAssembly/import-module.ll
+++ /dev/null
@@ -1,31 +0,0 @@
-; RUN: llc -filetype=obj %s -o - | obj2yaml | FileCheck %s
-
-target triple = "wasm32-unknown-unknown"
-
-define void @test() {
-  call void @foo()
-  call void @plain()
-  ret void
-}
-
-declare void @foo() #0
-declare void @plain()
-
-attributes #0 = { "wasm-import-module"="bar" "wasm-import-name"="qux" }
-
-; CHECK:- Type:IMPORT
-; CHECK-NEXT: Imports:
-; CHECK:- Module:  bar
-; CHECK-NEXT: Field:   qux
-; CHECK-NEXT: Kind:FUNCTION
-
-; CHECK:- Module:  env
-; CHECK-NEXT: Field:   plain
-; CHECK-NEXT: Kind:FUNCTION
-
-; CHECK:- Type:CUSTOM
-; CHECK:  Name:foo
-; CHECK-NEXT: Flags:   [ UNDEFINED, EXPLICIT_NAME ]
-
-; CHECK:  Name:plain
-; CHECK-NEXT: Flags:   [ UNDEFINED ]
Index: llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp
===
--- llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp
+++ llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp
@@ -712,6 +712,30 @@
   return expect(AsmToken::EndOfStatement, "EOL");
 }
 
+if (DirectiveID.getString() == ".import_module") {
+  auto SymName = expectIdent();
+  if (SymName.empty())
+return true;
+  if (expect(AsmToken::Comma, ","))
+return true;
+  auto ImportModule = expectIdent();
+  auto WasmSym = cast(Ctx.getOrCreateSymbol(SymName));
+  WasmSym->setImportModule(ImportModule);
+  TOut.emitImportModule(WasmSym, ImportModule);
+}
+
+if (DirectiveID.getString() == ".import_name") {
+  auto SymName = expectIdent();
+  if (SymName.empty())
+return true;
+  if (expect(AsmToken::Comma, ","))
+return true;
+  auto ImportName = expectIdent();
+  auto WasmSym = cast(Ctx.getOrCreateSymbol(SymName));
+  WasmSym->setImportName(ImportName);
+  TOut.emitImportName(WasmSym, ImportName);
+}
+
 if (DirectiveID.getString() == ".eventtype") {
   auto SymName = expectIdent();
   if (SymName.empty())
Index: llvm/lib/MC/WasmObjectWriter.cpp
===
--- llvm/lib/MC/WasmObjectWriter.cpp
+++ llvm/lib/MC/WasmObjectWriter.cpp
@@ -1452,7 +1452,7 @@
 Flags |= wasm::WASM_SYMBOL_EXPORTED;
   }
 }
-if (WS.getName() != WS.getImportName())
+if (WS.hasImportName())
   Flags |= wasm::WASM_SYMBOL_EXPLICIT_NAME;
 
 wasm::WasmSymbolInfo Info;
Index: llvm/include/llvm/MC/MCSymbolWasm.h
===
--- llvm/include/llvm/MC/MCSymbolWasm.h
+++ 

[PATCH] D61837: Make it possible control matcher traversal kind with ASTContext

[Stephen Kelly via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG0a717d5b5d31: Make it possible control matcher traversal 
kind with ASTContext (authored by stephenkelly).

Changed prior to commit:
  https://reviews.llvm.org/D61837?vs=230678=232661#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61837

Files:
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/AST/ASTNodeTraverser.h
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/include/clang/ASTMatchers/ASTMatchersInternal.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/ASTMatchers/ASTMatchFinder.cpp
  clang/lib/ASTMatchers/ASTMatchersInternal.cpp
  clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

Index: clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
@@ -1595,6 +1595,91 @@
   notMatches("class C {}; C a = C();", varDecl(has(cxxConstructExpr();
 }
 
+TEST(Traversal, traverseMatcher) {
+
+  StringRef VarDeclCode = R"cpp(
+void foo()
+{
+  int i = 3.0;
+}
+)cpp";
+
+  auto Matcher = varDecl(hasInitializer(floatLiteral()));
+
+  EXPECT_TRUE(
+  notMatches(VarDeclCode, traverse(ast_type_traits::TK_AsIs, Matcher)));
+  EXPECT_TRUE(
+  matches(VarDeclCode,
+  traverse(ast_type_traits::TK_IgnoreImplicitCastsAndParentheses,
+   Matcher)));
+}
+
+TEST(Traversal, traverseMatcherNesting) {
+
+  StringRef Code = R"cpp(
+float bar(int i)
+{
+  return i;
+}
+
+void foo()
+{
+  bar(bar(3.0));
+}
+)cpp";
+
+  EXPECT_TRUE(matches(
+  Code,
+  traverse(ast_type_traits::TK_IgnoreImplicitCastsAndParentheses,
+   callExpr(has(callExpr(traverse(
+   ast_type_traits::TK_AsIs,
+   callExpr(has(implicitCastExpr(has(floatLiteral(;
+}
+
+TEST(Traversal, traverseMatcherThroughImplicit) {
+  StringRef Code = R"cpp(
+struct S {
+  S(int x);
+};
+
+void constructImplicit() {
+  int a = 8;
+  S s(a);
+}
+  )cpp";
+
+  auto Matcher = traverse(ast_type_traits::TK_IgnoreImplicitCastsAndParentheses,
+  implicitCastExpr());
+
+  // Verfiy that it does not segfault
+  EXPECT_FALSE(matches(Code, Matcher));
+}
+
+TEST(Traversal, traverseMatcherThroughMemoization) {
+
+  StringRef Code = R"cpp(
+void foo()
+{
+  int i = 3.0;
+}
+  )cpp";
+
+  auto Matcher = varDecl(hasInitializer(floatLiteral()));
+
+  // Matchers such as hasDescendant memoize their result regarding AST
+  // nodes. In the matcher below, the first use of hasDescendant(Matcher)
+  // fails, and the use of it inside the traverse() matcher should pass
+  // causing the overall matcher to be a true match.
+  // This test verifies that the first false result is not re-used, which
+  // would cause the overall matcher to be incorrectly false.
+
+  EXPECT_TRUE(matches(
+  Code, functionDecl(anyOf(
+hasDescendant(Matcher),
+traverse(ast_type_traits::TK_IgnoreImplicitCastsAndParentheses,
+ functionDecl(hasDescendant(Matcher)));
+}
+
 TEST(IgnoringImpCasts, MatchesImpCasts) {
   // This test checks that ignoringImpCasts matches when implicit casts are
   // present and its inner matcher alone does not match.
Index: clang/lib/ASTMatchers/ASTMatchersInternal.cpp
===
--- clang/lib/ASTMatchers/ASTMatchersInternal.cpp
+++ clang/lib/ASTMatchers/ASTMatchersInternal.cpp
@@ -189,6 +189,14 @@
   llvm_unreachable("Invalid Op value.");
 }
 
+DynTypedMatcher DynTypedMatcher::constructRestrictedWrapper(
+const DynTypedMatcher ,
+ast_type_traits::ASTNodeKind RestrictKind) {
+  DynTypedMatcher Copy = InnerMatcher;
+  Copy.RestrictKind = RestrictKind;
+  return Copy;
+}
+
 DynTypedMatcher DynTypedMatcher::trueMatcher(
 ast_type_traits::ASTNodeKind NodeKind) {
   return DynTypedMatcher(NodeKind, NodeKind, &*TrueMatcherInstance);
@@ -211,8 +219,13 @@
 bool DynTypedMatcher::matches(const ast_type_traits::DynTypedNode ,
   ASTMatchFinder *Finder,
   BoundNodesTreeBuilder *Builder) const {
-  if (RestrictKind.isBaseOf(DynNode.getNodeKind()) &&
-  Implementation->dynMatches(DynNode, Finder, Builder)) {
+  TraversalKindScope RAII(Finder->getASTContext(),
+  Implementation->TraversalKind());
+
+  auto N = Finder->getASTContext().traverseIgnored(DynNode);
+
+  if (RestrictKind.isBaseOf(N.getNodeKind()) &&
+  Implementation->dynMatches(N, Finder, Builder)) {
 return true;
   }
   // Delete all bindings when a matcher does not match.
@@ -225,8 +238,13 @@
 bool DynTypedMatcher::matchesNoKindCheck(
 const ast_type_traits::DynTypedNode , ASTMatchFinder *Finder,
 

[clang] 0a717d5 - Make it possible control matcher traversal kind with ASTContext

[Stephen Kelly via cfe-commits]

Author: Stephen Kelly
Date: 2019-12-06T23:11:32Z
New Revision: 0a717d5b5d31fc2d5bc98ca695031fb09e65beb0

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

LOG: Make it possible control matcher traversal kind with ASTContext

Summary:
This will eventually allow traversal of an AST while ignoring invisible
AST nodes.  Currently it depends on the available enum values for
TraversalKinds.  That can be extended to ignore all invisible nodes in
the future.

Reviewers: klimek, aaron.ballman

Subscribers: cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang/include/clang/AST/ASTContext.h
clang/include/clang/AST/ASTNodeTraverser.h
clang/include/clang/ASTMatchers/ASTMatchers.h
clang/include/clang/ASTMatchers/ASTMatchersInternal.h
clang/lib/AST/ASTContext.cpp
clang/lib/ASTMatchers/ASTMatchFinder.cpp
clang/lib/ASTMatchers/ASTMatchersInternal.cpp
clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

Removed: 




diff  --git a/clang/include/clang/AST/ASTContext.h 
b/clang/include/clang/AST/ASTContext.h
index a0484509fa4a..4ba54fa51885 100644
--- a/clang/include/clang/AST/ASTContext.h
+++ b/clang/include/clang/AST/ASTContext.h
@@ -569,7 +569,17 @@ class ASTContext : public RefCountedBase {
   clang::PrintingPolicy PrintingPolicy;
   std::unique_ptr InterpContext;
 
+  ast_type_traits::TraversalKind Traversal = ast_type_traits::TK_AsIs;
+
 public:
+  ast_type_traits::TraversalKind getTraversalKind() const { return Traversal; }
+  void setTraversalKind(ast_type_traits::TraversalKind TK) { Traversal = TK; }
+
+  const Expr *traverseIgnored(const Expr *E) const;
+  Expr *traverseIgnored(Expr *E) const;
+  ast_type_traits::DynTypedNode
+  traverseIgnored(const ast_type_traits::DynTypedNode ) const;
+
   IdentifierTable 
   SelectorTable 
   Builtin::Context 
@@ -2996,7 +3006,7 @@ OPT_LIST(V)
 
   std::vector TraversalScope;
   class ParentMap;
-  std::unique_ptr Parents;
+  std::map> Parents;
 
   std::unique_ptr VTContext;
 
@@ -3040,6 +3050,22 @@ inline Selector GetUnarySelector(StringRef name, 
ASTContext ) {
   return Ctx.Selectors.getSelector(1, );
 }
 
+class TraversalKindScope {
+  ASTContext 
+  ast_type_traits::TraversalKind TK = ast_type_traits::TK_AsIs;
+
+public:
+  TraversalKindScope(ASTContext ,
+ llvm::Optional ScopeTK)
+  : Ctx(Ctx) {
+TK = Ctx.getTraversalKind();
+if (ScopeTK)
+  Ctx.setTraversalKind(*ScopeTK);
+  }
+
+  ~TraversalKindScope() { Ctx.setTraversalKind(TK); }
+};
+
 } // namespace clang
 
 // operator new and delete aren't allowed inside namespaces.

diff  --git a/clang/include/clang/AST/ASTNodeTraverser.h 
b/clang/include/clang/AST/ASTNodeTraverser.h
index ed9fc14aba42..b2e6d9e9c5e4 100644
--- a/clang/include/clang/AST/ASTNodeTraverser.h
+++ b/clang/include/clang/AST/ASTNodeTraverser.h
@@ -65,6 +65,9 @@ class ASTNodeTraverser
   /// not already been loaded.
   bool Deserialize = false;
 
+  ast_type_traits::TraversalKind Traversal =
+  ast_type_traits::TraversalKind::TK_AsIs;
+
   NodeDelegateType () {
 return getDerived().doGetNodeDelegate();
   }
@@ -74,6 +77,8 @@ class ASTNodeTraverser
   void setDeserialize(bool D) { Deserialize = D; }
   bool getDeserialize() const { return Deserialize; }
 
+  void SetTraversalKind(ast_type_traits::TraversalKind TK) { Traversal = TK; }
+
   void Visit(const Decl *D) {
 getNodeDelegate().AddChild([=] {
   getNodeDelegate().Visit(D);
@@ -97,8 +102,20 @@ class ASTNodeTraverser
 });
   }
 
-  void Visit(const Stmt *S, StringRef Label = {}) {
+  void Visit(const Stmt *Node, StringRef Label = {}) {
 getNodeDelegate().AddChild(Label, [=] {
+  const Stmt *S = Node;
+
+  if (auto *E = dyn_cast_or_null(S)) {
+switch (Traversal) {
+case ast_type_traits::TK_AsIs:
+  break;
+case ast_type_traits::TK_IgnoreImplicitCastsAndParentheses:
+  S = E->IgnoreParenImpCasts();
+  break;
+}
+  }
+
   getNodeDelegate().Visit(S);
 
   if (!S) {

diff  --git a/clang/include/clang/ASTMatchers/ASTMatchers.h 
b/clang/include/clang/ASTMatchers/ASTMatchers.h
index 3a5d0c08337e..608454631556 100644
--- a/clang/include/clang/ASTMatchers/ASTMatchers.h
+++ b/clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -689,6 +689,31 @@ AST_POLYMORPHIC_MATCHER_P(
  Builder);
 }
 
+/// Causes all nested matchers to be matched with the specified traversal kind.
+///
+/// Given
+/// \code
+///   void foo()
+///   {
+///   int i = 3.0;
+///   }
+/// \endcode
+/// The matcher
+/// \code
+///   traverse(ast_type_traits::TK_IgnoreImplicitCastsAndParentheses,
+/// varDecl(hasInitializer(floatLiteral().bind("init")))
+///   )

[PATCH] D71091: Make sure that the implicit arguments for direct methods have been setup

[Alex Lorenz via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGf3efd6957474: [ObjC] Make sure that the implicit arguments 
for direct methods have been setup (authored by arphaman).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71091

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGObjCMac.cpp
  clang/lib/Sema/SemaDeclObjC.cpp
  clang/lib/Serialization/ASTReaderDecl.cpp
  clang/lib/Serialization/ASTWriterDecl.cpp
  clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
  clang/test/CodeGenObjC/direct-method.m

Index: clang/test/CodeGenObjC/direct-method.m
===
--- clang/test/CodeGenObjC/direct-method.m
+++ clang/test/CodeGenObjC/direct-method.m
@@ -11,9 +11,17 @@
 
 __attribute__((objc_root_class))
 @interface Root
+- (int)getInt __attribute__((objc_direct));
+@property(direct, readonly) int intProperty;
+@property(direct, readonly) int intProperty2;
 @end
 
 @implementation Root
+// CHECK-LABEL: define hidden i32 @"\01-[Root intProperty2]"
+- (int)intProperty2 {
+  return 42;
+}
+
 // CHECK-LABEL: define hidden i32 @"\01-[Root getInt]"(
 - (int)getInt __attribute__((objc_direct)) {
   // loading parameters
@@ -152,6 +160,7 @@
 }
 
 @end
+// CHECK-LABEL: define hidden i32 @"\01-[Root intProperty]"
 
 @interface Foo : Root {
   id __strong _cause_cxx_destruct;
@@ -173,3 +182,11 @@
 // CHECK-LABEL: define hidden void @"\01-[Foo setGetDynamic_setDirect:]"(
 // CHECK-LABEL: define internal void @"\01-[Foo .cxx_destruct]"(
 @end
+
+int useRoot(Root *r) {
+  // CHECK-LABEL: define i32 @useRoot
+  // CHECK: %{{[^ ]*}} = call i32 bitcast {{.*}} @"\01-[Root getInt]"
+  // CHECK: %{{[^ ]*}} = call i32 bitcast {{.*}} @"\01-[Root intProperty]"
+  // CHECK: %{{[^ ]*}} = call i32 bitcast {{.*}} @"\01-[Root intProperty2]"
+  return [r getInt] + [r intProperty] + [r intProperty2];
+}
Index: clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
===
--- clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
+++ clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
@@ -722,13 +722,6 @@
 } else if (const auto *OCD = dyn_cast(DC)) {
   OS << OCD->getClassInterface()->getName() << '('
  << OCD->getName() << ')';
-} else if (isa(DC)) {
-  // We can extract the type of the class from the self pointer.
-  if (ImplicitParamDecl *SelfDecl = OMD->getSelfDecl()) {
-QualType ClassTy =
-cast(SelfDecl->getType())->getPointeeType();
-ClassTy.print(OS, PrintingPolicy(LangOptions()));
-  }
 }
 OS << ' ' << OMD->getSelector().getAsString() << ']';
 
Index: clang/lib/Serialization/ASTWriterDecl.cpp
===
--- clang/lib/Serialization/ASTWriterDecl.cpp
+++ clang/lib/Serialization/ASTWriterDecl.cpp
@@ -664,14 +664,13 @@
   VisitNamedDecl(D);
   // FIXME: convert to LazyStmtPtr?
   // Unlike C/C++, method bodies will never be in header files.
-  bool HasBodyStuff = D->getBody() != nullptr ||
-  D->getSelfDecl() != nullptr || D->getCmdDecl() != nullptr;
+  bool HasBodyStuff = D->getBody() != nullptr;
   Record.push_back(HasBodyStuff);
   if (HasBodyStuff) {
 Record.AddStmt(D->getBody());
-Record.AddDeclRef(D->getSelfDecl());
-Record.AddDeclRef(D->getCmdDecl());
   }
+  Record.AddDeclRef(D->getSelfDecl());
+  Record.AddDeclRef(D->getCmdDecl());
   Record.push_back(D->isInstanceMethod());
   Record.push_back(D->isVariadic());
   Record.push_back(D->isPropertyAccessor());
Index: clang/lib/Serialization/ASTReaderDecl.cpp
===
--- clang/lib/Serialization/ASTReaderDecl.cpp
+++ clang/lib/Serialization/ASTReaderDecl.cpp
@@ -1017,9 +1017,9 @@
 // definitions rarely show up in headers.
 Reader.PendingBodies[MD] = GetCurrentCursorOffset();
 HasPendingBody = true;
-MD->setSelfDecl(ReadDeclAs());
-MD->setCmdDecl(ReadDeclAs());
   }
+  MD->setSelfDecl(ReadDeclAs());
+  MD->setCmdDecl(ReadDeclAs());
   MD->setInstanceMethod(Record.readInt());
   MD->setVariadic(Record.readInt());
   MD->setPropertyAccessor(Record.readInt());
Index: clang/lib/Sema/SemaDeclObjC.cpp
===
--- clang/lib/Sema/SemaDeclObjC.cpp
+++ clang/lib/Sema/SemaDeclObjC.cpp
@@ -4869,6 +4869,9 @@
 }
   }
 
+  // Insert the invisible arguments, self and _cmd!
+  ObjCMethod->createImplicitParams(Context, ObjCMethod->getClassInterface());
+
   ActOnDocumentableDecl(ObjCMethod);
 
   return ObjCMethod;
Index: clang/lib/CodeGen/CGObjCMac.cpp
===
--- clang/lib/CodeGen/CGObjCMac.cpp
+++ clang/lib/CodeGen/CGObjCMac.cpp
@@ -4027,7 +4027,7 @@
 llvm::Function *
 

[clang] f3efd69 - [ObjC] Make sure that the implicit arguments for direct methods have been setup

[Alex Lorenz via cfe-commits]

Author: Alex Lorenz
Date: 2019-12-06T14:28:28-08:00
New Revision: f3efd6957474bfd3b9b232ac6e4b3608174c3b79

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

LOG: [ObjC] Make sure that the implicit arguments for direct methods have been 
setup

This commit sets the Self and Imp declarations for ObjC method declarations,
in addition to the definitions. It also fixes
a bunch of code in clang that had wrong assumptions about when getSelfDecl() 
would be set:

- CGDebugInfo::getObjCMethodName and AnalysisConsumer::getFunctionName would 
assume that it was
  set for method declarations part of a protocol, which they never were,
  and that self would be a Class type, which it isn't as it is id for a 
protocol.

Also use the Canonical Decl to index the set of Direct methods so that
when calls and implementations interleave, the same llvm::Function is
used and the same symbol name emitted.

Radar-Id: rdar://problem/57661767

Patch by: Pierre Habouzit

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

Added: 


Modified: 
clang/lib/CodeGen/CGDebugInfo.cpp
clang/lib/CodeGen/CGObjCMac.cpp
clang/lib/Sema/SemaDeclObjC.cpp
clang/lib/Serialization/ASTReaderDecl.cpp
clang/lib/Serialization/ASTWriterDecl.cpp
clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
clang/test/CodeGenObjC/direct-method.m

Removed: 




diff  --git a/clang/lib/CodeGen/CGDebugInfo.cpp 
b/clang/lib/CodeGen/CGDebugInfo.cpp
index 8d6406c02773..7ea3c0850fe2 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -292,13 +292,6 @@ StringRef CGDebugInfo::getObjCMethodName(const 
ObjCMethodDecl *OMD) {
 }
   } else if (const auto *OCD = dyn_cast(DC)) {
 OS << OCD->getClassInterface()->getName() << '(' << OCD->getName() << ')';
-  } else if (isa(DC)) {
-// We can extract the type of the class from the self pointer.
-if (ImplicitParamDecl *SelfDecl = OMD->getSelfDecl()) {
-  QualType ClassTy =
-  cast(SelfDecl->getType())->getPointeeType();
-  ClassTy.print(OS, PrintingPolicy(LangOptions()));
-}
   }
   OS << ' ' << OMD->getSelector().getAsString() << ']';
 

diff  --git a/clang/lib/CodeGen/CGObjCMac.cpp b/clang/lib/CodeGen/CGObjCMac.cpp
index 775e3406da7e..5bd04bc88b75 100644
--- a/clang/lib/CodeGen/CGObjCMac.cpp
+++ b/clang/lib/CodeGen/CGObjCMac.cpp
@@ -4027,7 +4027,7 @@ llvm::Function *CGObjCCommonMac::GenerateMethod(const 
ObjCMethodDecl *OMD,
 llvm::Function *
 CGObjCCommonMac::GenerateDirectMethod(const ObjCMethodDecl *OMD,
   const ObjCContainerDecl *CD) {
-  auto I = DirectMethodDefinitions.find(OMD);
+  auto I = DirectMethodDefinitions.find(OMD->getCanonicalDecl());
   if (I != DirectMethodDefinitions.end())
 return I->second;
 
@@ -4040,7 +4040,7 @@ CGObjCCommonMac::GenerateDirectMethod(const 
ObjCMethodDecl *OMD,
   llvm::Function *Method =
   llvm::Function::Create(MethodTy, llvm::GlobalValue::ExternalLinkage,
  Name.str(), ());
-  DirectMethodDefinitions.insert(std::make_pair(OMD, Method));
+  DirectMethodDefinitions.insert(std::make_pair(OMD->getCanonicalDecl(), 
Method));
 
   return Method;
 }

diff  --git a/clang/lib/Sema/SemaDeclObjC.cpp b/clang/lib/Sema/SemaDeclObjC.cpp
index e84dc47a1ee1..70c04571ac67 100644
--- a/clang/lib/Sema/SemaDeclObjC.cpp
+++ b/clang/lib/Sema/SemaDeclObjC.cpp
@@ -4869,6 +4869,9 @@ Decl *Sema::ActOnMethodDeclaration(
 }
   }
 
+  // Insert the invisible arguments, self and _cmd!
+  ObjCMethod->createImplicitParams(Context, ObjCMethod->getClassInterface());
+
   ActOnDocumentableDecl(ObjCMethod);
 
   return ObjCMethod;

diff  --git a/clang/lib/Serialization/ASTReaderDecl.cpp 
b/clang/lib/Serialization/ASTReaderDecl.cpp
index d989f46c4ab4..8990379069b3 100644
--- a/clang/lib/Serialization/ASTReaderDecl.cpp
+++ b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -1017,9 +1017,9 @@ void ASTDeclReader::VisitObjCMethodDecl(ObjCMethodDecl 
*MD) {
 // definitions rarely show up in headers.
 Reader.PendingBodies[MD] = GetCurrentCursorOffset();
 HasPendingBody = true;
-MD->setSelfDecl(ReadDeclAs());
-MD->setCmdDecl(ReadDeclAs());
   }
+  MD->setSelfDecl(ReadDeclAs());
+  MD->setCmdDecl(ReadDeclAs());
   MD->setInstanceMethod(Record.readInt());
   MD->setVariadic(Record.readInt());
   MD->setPropertyAccessor(Record.readInt());

diff  --git a/clang/lib/Serialization/ASTWriterDecl.cpp 
b/clang/lib/Serialization/ASTWriterDecl.cpp
index 38eb64e52e4a..eaf2c5458be7 100644
--- a/clang/lib/Serialization/ASTWriterDecl.cpp
+++ b/clang/lib/Serialization/ASTWriterDecl.cpp
@@ -664,14 +664,13 @@ void ASTDeclWriter::VisitObjCMethodDecl(ObjCMethodDecl 
*D) {
   VisitNamedDecl(D);
   // FIXME: convert to LazyStmtPtr?
   // 

[PATCH] D71091: Make sure that the implicit arguments for direct methods have been setup

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added a comment.

In D71091#1773494 , @MadCoder wrote:

> @liuliu I wouldn't be surprised this addresses your issues if snapchat is 
> using PCH


This also impacts modules, so builds that use could've been affected.


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

https://reviews.llvm.org/D71091



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

[PATCH] D71091: Make sure that the implicit arguments for direct methods have been setup

[Alex Lorenz via Phabricator via cfe-commits]

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

LGTM, thanks


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

https://reviews.llvm.org/D71091



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

[PATCH] D71091: Make sure that the implicit arguments for direct methods have been setup

[Pierre Habouzit via Phabricator via cfe-commits]

MadCoder added inline comments.



Comment at: clang/lib/Serialization/ASTWriterDecl.cpp:674
   }
   Record.push_back(D->isInstanceMethod());
   Record.push_back(D->isVariadic());

arphaman wrote:
> @MadCoder Given the fact that you're now setting SelfDecl and CmdDecl for 
> declarations, they need to be serialized properly. I would suggest the 
> following change to the writer:
> 
> ```
> bool HasBody = D->getBody() != nullptr;
> Record.push_back(HasBody);
> if (HasBody)
>   Record.AddStmt(D->getBody());
> Record.AddDeclRef(D->getSelfDecl());
> Record.AddDeclRef(D-> getCmdDecl());
> ```
> 
> The AST reader should be updated accordingly.
hah was the source of the last etst failure I was chasing, thanks!


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

https://reviews.llvm.org/D71091



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

[PATCH] D71091: Make sure that the implicit arguments for direct methods have been setup

[Pierre Habouzit via Phabricator via cfe-commits]

MadCoder updated this revision to Diff 232644.
MadCoder marked an inline comment as done.
MadCoder added a comment.

@liuliu I wouldn't be surprised this addresses your issues if snapchat is using 
PCH


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

https://reviews.llvm.org/D71091

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGObjCMac.cpp
  clang/lib/Sema/SemaDeclObjC.cpp
  clang/lib/Serialization/ASTReaderDecl.cpp
  clang/lib/Serialization/ASTWriterDecl.cpp
  clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
  clang/test/CodeGenObjC/direct-method.m

Index: clang/test/CodeGenObjC/direct-method.m
===
--- clang/test/CodeGenObjC/direct-method.m
+++ clang/test/CodeGenObjC/direct-method.m
@@ -11,9 +11,17 @@
 
 __attribute__((objc_root_class))
 @interface Root
+- (int)getInt __attribute__((objc_direct));
+@property(direct, readonly) int intProperty;
+@property(direct, readonly) int intProperty2;
 @end
 
 @implementation Root
+// CHECK-LABEL: define hidden i32 @"\01-[Root intProperty2]"
+- (int)intProperty2 {
+  return 42;
+}
+
 // CHECK-LABEL: define hidden i32 @"\01-[Root getInt]"(
 - (int)getInt __attribute__((objc_direct)) {
   // loading parameters
@@ -152,6 +160,7 @@
 }
 
 @end
+// CHECK-LABEL: define hidden i32 @"\01-[Root intProperty]"
 
 @interface Foo : Root {
   id __strong _cause_cxx_destruct;
@@ -173,3 +182,11 @@
 // CHECK-LABEL: define hidden void @"\01-[Foo setGetDynamic_setDirect:]"(
 // CHECK-LABEL: define internal void @"\01-[Foo .cxx_destruct]"(
 @end
+
+int useRoot(Root *r) {
+  // CHECK-LABEL: define i32 @useRoot
+  // CHECK: %{{[^ ]*}} = call i32 bitcast {{.*}} @"\01-[Root getInt]"
+  // CHECK: %{{[^ ]*}} = call i32 bitcast {{.*}} @"\01-[Root intProperty]"
+  // CHECK: %{{[^ ]*}} = call i32 bitcast {{.*}} @"\01-[Root intProperty2]"
+  return [r getInt] + [r intProperty] + [r intProperty2];
+}
Index: clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
===
--- clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
+++ clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
@@ -722,13 +722,6 @@
 } else if (const auto *OCD = dyn_cast(DC)) {
   OS << OCD->getClassInterface()->getName() << '('
  << OCD->getName() << ')';
-} else if (isa(DC)) {
-  // We can extract the type of the class from the self pointer.
-  if (ImplicitParamDecl *SelfDecl = OMD->getSelfDecl()) {
-QualType ClassTy =
-cast(SelfDecl->getType())->getPointeeType();
-ClassTy.print(OS, PrintingPolicy(LangOptions()));
-  }
 }
 OS << ' ' << OMD->getSelector().getAsString() << ']';
 
Index: clang/lib/Serialization/ASTWriterDecl.cpp
===
--- clang/lib/Serialization/ASTWriterDecl.cpp
+++ clang/lib/Serialization/ASTWriterDecl.cpp
@@ -664,14 +664,13 @@
   VisitNamedDecl(D);
   // FIXME: convert to LazyStmtPtr?
   // Unlike C/C++, method bodies will never be in header files.
-  bool HasBodyStuff = D->getBody() != nullptr ||
-  D->getSelfDecl() != nullptr || D->getCmdDecl() != nullptr;
+  bool HasBodyStuff = D->getBody() != nullptr;
   Record.push_back(HasBodyStuff);
   if (HasBodyStuff) {
 Record.AddStmt(D->getBody());
-Record.AddDeclRef(D->getSelfDecl());
-Record.AddDeclRef(D->getCmdDecl());
   }
+  Record.AddDeclRef(D->getSelfDecl());
+  Record.AddDeclRef(D->getCmdDecl());
   Record.push_back(D->isInstanceMethod());
   Record.push_back(D->isVariadic());
   Record.push_back(D->isPropertyAccessor());
Index: clang/lib/Serialization/ASTReaderDecl.cpp
===
--- clang/lib/Serialization/ASTReaderDecl.cpp
+++ clang/lib/Serialization/ASTReaderDecl.cpp
@@ -1017,9 +1017,9 @@
 // definitions rarely show up in headers.
 Reader.PendingBodies[MD] = GetCurrentCursorOffset();
 HasPendingBody = true;
-MD->setSelfDecl(ReadDeclAs());
-MD->setCmdDecl(ReadDeclAs());
   }
+  MD->setSelfDecl(ReadDeclAs());
+  MD->setCmdDecl(ReadDeclAs());
   MD->setInstanceMethod(Record.readInt());
   MD->setVariadic(Record.readInt());
   MD->setPropertyAccessor(Record.readInt());
Index: clang/lib/Sema/SemaDeclObjC.cpp
===
--- clang/lib/Sema/SemaDeclObjC.cpp
+++ clang/lib/Sema/SemaDeclObjC.cpp
@@ -4869,6 +4869,9 @@
 }
   }
 
+  // Insert the invisible arguments, self and _cmd!
+  ObjCMethod->createImplicitParams(Context, ObjCMethod->getClassInterface());
+
   ActOnDocumentableDecl(ObjCMethod);
 
   return ObjCMethod;
Index: clang/lib/CodeGen/CGObjCMac.cpp
===
--- clang/lib/CodeGen/CGObjCMac.cpp
+++ clang/lib/CodeGen/CGObjCMac.cpp
@@ -4027,7 +4027,7 @@
 llvm::Function *
 CGObjCCommonMac::GenerateDirectMethod(const ObjCMethodDecl *OMD,
 

[PATCH] D71091: Make sure that the implicit arguments for direct methods have been setup

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added inline comments.



Comment at: clang/lib/Serialization/ASTWriterDecl.cpp:674
   }
   Record.push_back(D->isInstanceMethod());
   Record.push_back(D->isVariadic());

@MadCoder Given the fact that you're now setting SelfDecl and CmdDecl for 
declarations, they need to be serialized properly. I would suggest the 
following change to the writer:

```
bool HasBody = D->getBody() != nullptr;
Record.push_back(HasBody);
if (HasBody)
  Record.AddStmt(D->getBody());
Record.AddDeclRef(D->getSelfDecl());
Record.AddDeclRef(D-> getCmdDecl());
```

The AST reader should be updated accordingly.


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

https://reviews.llvm.org/D71091



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

[PATCH] D70469: [attributes] [analyzer] Add handle related attributes

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D70469#1766084 , @xazax.hun wrote:

> @aaron.ballman
>
> Type attributes are definitely more flexible as in they can be applied in 
> more contexts. These attributes, however, make no sense outside of a function 
> declaration, or maybe a type alias.


Function pointers are a primary case for what I was thinking of, and those are 
neither a function declaration nor a type alias.

> So I feel like we could argue on both sides. I made a minimal version of the 
> attribute so it can potentially unblock dependent revisions (the static 
> analyzer changes) and plan to add more diagnostics in follow-up patches. What 
> do you think?

Thank you! I think this is a more flexible way forward.

In D70469#1770041 , @xazax.hun wrote:

> In the meantime, I realized lambdas are actually not that important. Lambdas 
> are only usable in C++, and in C++ one should prefer to use move only RAII 
> wrapper for handles.


Generally yes, but thinking of things like file descriptors (which are also 
handles), it seems like overkill to expect someone to wrap those in a separate 
type.

> This reduces the problem into a use after move and we already have a check 
> for that. These annotations are more important for C, where we do not have 
> language features to mitigate these problems. The function pointer argument 
> is still valid though.

Okay, that's a fair point.




Comment at: clang/include/clang/Basic/Attr.td:3464
+
+def AcquireHandle : TypeAttr {
+  let Spellings = [Clang<"acquire_handle">];

These probably should be `DeclOrTypeAttr` because they can be applied to a 
parameter (which is a declaration) or a type.



Comment at: clang/include/clang/Basic/AttrDocs.td:4569
+
+def HandleDocs : DocumentationCategory<"Handle Attributes"> {
+  let Content = [{

The docs should clarify when they say "function" that they mean the function 
type rather than the function declaration.



Comment at: clang/lib/Sema/SemaType.cpp:7412
+ diag::warn_type_attribute_wrong_type_str)
+<< Attr.getAttrName()->getName() << "non-function" << CurType;
+return;

You should be able to pass in `Attr` directly instead of 
`Attr.getAttrName()->getName()`, I believe. Also, I'd prefer the `non-function` 
be put into the diagnostic text itself with a `%select` if we need to vary it.



Comment at: clang/test/Sema/attr-handles.cpp:7
+int __attribute__((acquire_handle)) { return 0; };
+void g(int __attribute__((acquire_handle)) a); // TODO: diagnose this. The 
acquire attribute only makes sense for outputs.
+void h(int __attribute__((acquire_handle(1))) *a); // expected-error 
{{'acquire_handle' attribute takes no arguments}}

Are you planning to implement the TODOs as part of this patch?


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

https://reviews.llvm.org/D70469



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

[PATCH] D71142: [Sema] Validate large bitfields

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:5186-5189
+def warn_bitfield_width_exceeds_maximum_width: Error<
+  "width of bit-field %0 doesn't fit in a 64 bit unsigned integer">;
+def warn_anon_bitfield_width_exceeds_maximum_width : Error<
+  "width of anonymous bit-field doesn't fit in a 64 bit unsigned integer">;

I feel like this situation should be an error rather than a warning -- what 
could the code possibly have meant?



Comment at: clang/lib/Sema/SemaDecl.cpp:15845
 
+// Don't accept too wide bit-fields they will cause assertion failures
+// when used.

too wide -> too-wide
bit-fields they -> bit-fields, they


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71142



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

[PATCH] D70804: [Frontend] Allow OpenMP offloading to aarch64

[Alexey Bataev via Phabricator via cfe-commits]

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

LG


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70804



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

[PATCH] D71091: Make sure that the implicit arguments for direct methods have been setup

[Pierre Habouzit via Phabricator via cfe-commits]

MadCoder updated this revision to Diff 232641.
MadCoder marked an inline comment as done.
MadCoder added a comment.

woops a bogus hunk went in


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

https://reviews.llvm.org/D71091

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGObjCMac.cpp
  clang/lib/Sema/SemaDeclObjC.cpp
  clang/lib/Serialization/ASTWriterDecl.cpp
  clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
  clang/test/CodeGenObjC/direct-method.m

Index: clang/test/CodeGenObjC/direct-method.m
===
--- clang/test/CodeGenObjC/direct-method.m
+++ clang/test/CodeGenObjC/direct-method.m
@@ -11,9 +11,17 @@
 
 __attribute__((objc_root_class))
 @interface Root
+- (int)getInt __attribute__((objc_direct));
+@property(direct, readonly) int intProperty;
+@property(direct, readonly) int intProperty2;
 @end
 
 @implementation Root
+// CHECK-LABEL: define hidden i32 @"\01-[Root intProperty2]"
+- (int)intProperty2 {
+  return 42;
+}
+
 // CHECK-LABEL: define hidden i32 @"\01-[Root getInt]"(
 - (int)getInt __attribute__((objc_direct)) {
   // loading parameters
@@ -152,6 +160,7 @@
 }
 
 @end
+// CHECK-LABEL: define hidden i32 @"\01-[Root intProperty]"
 
 @interface Foo : Root {
   id __strong _cause_cxx_destruct;
@@ -173,3 +182,11 @@
 // CHECK-LABEL: define hidden void @"\01-[Foo setGetDynamic_setDirect:]"(
 // CHECK-LABEL: define internal void @"\01-[Foo .cxx_destruct]"(
 @end
+
+int useRoot(Root *r) {
+  // CHECK-LABEL: define i32 @useRoot
+  // CHECK: %{{[^ ]*}} = call i32 bitcast {{.*}} @"\01-[Root getInt]"
+  // CHECK: %{{[^ ]*}} = call i32 bitcast {{.*}} @"\01-[Root intProperty]"
+  // CHECK: %{{[^ ]*}} = call i32 bitcast {{.*}} @"\01-[Root intProperty2]"
+  return [r getInt] + [r intProperty] + [r intProperty2];
+}
Index: clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
===
--- clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
+++ clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
@@ -722,13 +722,6 @@
 } else if (const auto *OCD = dyn_cast(DC)) {
   OS << OCD->getClassInterface()->getName() << '('
  << OCD->getName() << ')';
-} else if (isa(DC)) {
-  // We can extract the type of the class from the self pointer.
-  if (ImplicitParamDecl *SelfDecl = OMD->getSelfDecl()) {
-QualType ClassTy =
-cast(SelfDecl->getType())->getPointeeType();
-ClassTy.print(OS, PrintingPolicy(LangOptions()));
-  }
 }
 OS << ' ' << OMD->getSelector().getAsString() << ']';
 
Index: clang/lib/Serialization/ASTWriterDecl.cpp
===
--- clang/lib/Serialization/ASTWriterDecl.cpp
+++ clang/lib/Serialization/ASTWriterDecl.cpp
@@ -664,8 +664,7 @@
   VisitNamedDecl(D);
   // FIXME: convert to LazyStmtPtr?
   // Unlike C/C++, method bodies will never be in header files.
-  bool HasBodyStuff = D->getBody() != nullptr ||
-  D->getSelfDecl() != nullptr || D->getCmdDecl() != nullptr;
+  bool HasBodyStuff = D->getBody() != nullptr;
   Record.push_back(HasBodyStuff);
   if (HasBodyStuff) {
 Record.AddStmt(D->getBody());
Index: clang/lib/Sema/SemaDeclObjC.cpp
===
--- clang/lib/Sema/SemaDeclObjC.cpp
+++ clang/lib/Sema/SemaDeclObjC.cpp
@@ -4869,6 +4869,9 @@
 }
   }
 
+  // Insert the invisible arguments, self and _cmd!
+  ObjCMethod->createImplicitParams(Context, ObjCMethod->getClassInterface());
+
   ActOnDocumentableDecl(ObjCMethod);
 
   return ObjCMethod;
Index: clang/lib/CodeGen/CGObjCMac.cpp
===
--- clang/lib/CodeGen/CGObjCMac.cpp
+++ clang/lib/CodeGen/CGObjCMac.cpp
@@ -4027,7 +4027,7 @@
 llvm::Function *
 CGObjCCommonMac::GenerateDirectMethod(const ObjCMethodDecl *OMD,
   const ObjCContainerDecl *CD) {
-  auto I = DirectMethodDefinitions.find(OMD);
+  auto I = DirectMethodDefinitions.find(OMD->getCanonicalDecl());
   if (I != DirectMethodDefinitions.end())
 return I->second;
 
@@ -4040,7 +4040,7 @@
   llvm::Function *Method =
   llvm::Function::Create(MethodTy, llvm::GlobalValue::ExternalLinkage,
  Name.str(), ());
-  DirectMethodDefinitions.insert(std::make_pair(OMD, Method));
+  DirectMethodDefinitions.insert(std::make_pair(OMD->getCanonicalDecl(), Method));
 
   return Method;
 }
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -292,13 +292,6 @@
 }
   } else if (const auto *OCD = dyn_cast(DC)) {
 OS << OCD->getClassInterface()->getName() << '(' << OCD->getName() << ')';
-  } else if (isa(DC)) {
-// We can extract the type of the class from the self pointer.
-

[PATCH] D71091: Make sure that the implicit arguments for direct methods have been setup

[Pierre Habouzit via Phabricator via cfe-commits]

MadCoder added inline comments.



Comment at: clang/lib/Sema/SemaDeclObjC.cpp:4839
 
+  ObjCMethod->createImplicitParams(Context, ObjCMethod->getClassInterface());
+

MadCoder wrote:
> liuliu wrote:
> > Yeah, I applied this change. As I said, for my trivial example it does 
> > work. But for my more thorough example with @property (direct) etc, it 
> > doesn't. I can dig into more later today or Monday. I just got everything 
> > work (with another more hackier fix) and would first to evaluate this as a 
> > viable path first before commit more time to debug this.
> > 
> > We probably can also just have a follow-up fix for that, depends on if 
> > reviewers thought this is blocking or not. I personally think not.
> Huh I could have seen why property methods would have an issue but they seem 
> to have gotten their `direct`ness properly anyway (I'll add them to the CG 
> test for good measure while I debug the tests I broke).
> 
> so I'll need a reduced test case from you I can't seem to find an easy way to 
> break it indeed.
I just added a test using a direct property with both a synthesized and a 
manual one, and it seems to work for me (?)


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

https://reviews.llvm.org/D71091



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

[PATCH] D71080: [NFC] Separate getLastArgIntValue to Basic

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl updated this revision to Diff 232639.
yaxunl marked 2 inline comments as done.
yaxunl added a comment.

revised by Artem's comments


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

https://reviews.llvm.org/D71080

Files:
  clang/include/clang/Basic/OptionUtils.h
  clang/include/clang/Frontend/Utils.h
  clang/lib/Basic/CMakeLists.txt
  clang/lib/Basic/OptionUtils.cpp
  clang/lib/Frontend/CompilerInvocation.cpp

Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -3682,35 +3682,8 @@
   return llvm::APInt(64, code).toString(36, /*Signed=*/false);
 }
 
-template
-static IntTy getLastArgIntValueImpl(const ArgList , OptSpecifier Id,
-IntTy Default,
-DiagnosticsEngine *Diags) {
-  IntTy Res = Default;
-  if (Arg *A = Args.getLastArg(Id)) {
-if (StringRef(A->getValue()).getAsInteger(10, Res)) {
-  if (Diags)
-Diags->Report(diag::err_drv_invalid_int_value) << A->getAsString(Args)
-   << A->getValue();
-}
-  }
-  return Res;
-}
-
 namespace clang {
 
-// Declared in clang/Frontend/Utils.h.
-int getLastArgIntValue(const ArgList , OptSpecifier Id, int Default,
-   DiagnosticsEngine *Diags) {
-  return getLastArgIntValueImpl(Args, Id, Default, Diags);
-}
-
-uint64_t getLastArgUInt64Value(const ArgList , OptSpecifier Id,
-   uint64_t Default,
-   DiagnosticsEngine *Diags) {
-  return getLastArgIntValueImpl(Args, Id, Default, Diags);
-}
-
 IntrusiveRefCntPtr
 createVFSFromCompilerInvocation(const CompilerInvocation ,
 DiagnosticsEngine ) {
Index: clang/lib/Basic/OptionUtils.cpp
===
--- /dev/null
+++ clang/lib/Basic/OptionUtils.cpp
@@ -0,0 +1,47 @@
+//===--- OptionUtils.cpp - Utilities for command line arguments ---===//
+//
+// 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
+//
+//===--===//
+
+#include "clang/Basic/OptionUtils.h"
+#include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/DiagnosticDriver.h"
+#include "llvm/Option/ArgList.h"
+
+using namespace clang;
+using namespace llvm::opt;
+
+namespace {
+template 
+IntTy getLastArgIntValueImpl(const ArgList , OptSpecifier Id,
+ IntTy Default, DiagnosticsEngine *Diags,
+ unsigned Base) {
+  IntTy Res = Default;
+  if (Arg *A = Args.getLastArg(Id)) {
+if (StringRef(A->getValue()).getAsInteger(Base, Res)) {
+  if (Diags)
+Diags->Report(diag::err_drv_invalid_int_value)
+<< A->getAsString(Args) << A->getValue();
+}
+  }
+  return Res;
+}
+} // namespace
+
+namespace clang {
+
+int getLastArgIntValue(const ArgList , OptSpecifier Id, int Default,
+   DiagnosticsEngine *Diags, unsigned Base) {
+  return getLastArgIntValueImpl(Args, Id, Default, Diags, Base);
+}
+
+uint64_t getLastArgUInt64Value(const ArgList , OptSpecifier Id,
+   uint64_t Default, DiagnosticsEngine *Diags,
+   unsigned Base) {
+  return getLastArgIntValueImpl(Args, Id, Default, Diags, Base);
+}
+
+} // namespace clang
Index: clang/lib/Basic/CMakeLists.txt
===
--- clang/lib/Basic/CMakeLists.txt
+++ clang/lib/Basic/CMakeLists.txt
@@ -1,6 +1,7 @@
 set(LLVM_LINK_COMPONENTS
   Core
   MC
+  Option
   Support
   )
 
@@ -55,6 +56,7 @@
   ObjCRuntime.cpp
   OpenMPKinds.cpp
   OperatorPrecedence.cpp
+  OptionUtils.cpp
   SanitizerBlacklist.cpp
   SanitizerSpecialCaseList.cpp
   Sanitizers.cpp
Index: clang/include/clang/Frontend/Utils.h
===
--- clang/include/clang/Frontend/Utils.h
+++ clang/include/clang/Frontend/Utils.h
@@ -15,6 +15,7 @@
 
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/LLVM.h"
+#include "clang/Basic/OptionUtils.h"
 #include "clang/Frontend/DependencyOutputOptions.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/IntrusiveRefCntPtr.h"
@@ -34,12 +35,6 @@
 
 class Triple;
 
-namespace opt {
-
-class ArgList;
-
-} // namespace opt
-
 } // namespace llvm
 
 namespace clang {
@@ -230,29 +225,6 @@
 bool ShouldRecoverOnErrors = false,
 std::vector *CC1Args = nullptr);
 
-/// Return the value of the last argument as an integer, or a default. If Diags
-/// is non-null, emits an error if the argument is given, but non-integral.
-int getLastArgIntValue(const llvm::opt::ArgList ,

[clang] 040c39d - [analyzer] Fix false positive on introspection of a block's internal layout.

[Artem Dergachev via cfe-commits]

Author: Artem Dergachev
Date: 2019-12-06T13:24:20-08:00
New Revision: 040c39d50fb9c60de9020caf86e1a1fccfd6f861

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

LOG: [analyzer] Fix false positive on introspection of a block's internal 
layout.

When implementation of the block runtime is available, we should not
warn that block layout fields are uninitialized simply because they're
on the stack.

Added: 


Modified: 
clang/lib/StaticAnalyzer/Core/RegionStore.cpp
clang/test/Analysis/blocks.m

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Core/RegionStore.cpp 
b/clang/lib/StaticAnalyzer/Core/RegionStore.cpp
index 5d2ef59e2d66..4797f564a837 100644
--- a/clang/lib/StaticAnalyzer/Core/RegionStore.cpp
+++ b/clang/lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -1951,7 +1951,8 @@ 
RegionStoreManager::getBindingForFieldOrElementCommon(RegionBindingsConstRef B,
 if (hasSymbolicIndex)
   return UnknownVal();
 
-if (!hasPartialLazyBinding)
+// Additionally allow introspection of a block's internal layout.
+if (!hasPartialLazyBinding && !isa(R->getBaseRegion()))
   return UndefinedVal();
   }
 

diff  --git a/clang/test/Analysis/blocks.m b/clang/test/Analysis/blocks.m
index 98d0f8a2ebaa..a21a605ffa61 100644
--- a/clang/test/Analysis/blocks.m
+++ b/clang/test/Analysis/blocks.m
@@ -47,6 +47,10 @@ - (id)initWithFormat:(NSString *)format 
arguments:(va_list)argList __attribute__
 aslclient asl_open(const char *ident, const char *facility, uint32_t opts);
 int asl_log(aslclient asl, aslmsg msg, int level, const char *format, ...) 
__attribute__((__format__ (__printf__, 4, 5)));
 
+struct Block_layout {
+  int flags;
+};
+
 
//===--===//
 // Begin actual test cases.
 
//===--===//
@@ -241,3 +245,8 @@ void call_block_with_fewer_arguments() {
   b(); // expected-warning {{Block taking 1 argument is called with fewer (0)}}
 }
 #endif
+
+int getBlockFlags() {
+  int x = 0;
+  return ((struct Block_layout *)^{ (void)x; })->flags; // no-warning
+}



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

[PATCH] D71091: Make sure that the implicit arguments for direct methods have been setup

[Pierre Habouzit via Phabricator via cfe-commits]

MadCoder updated this revision to Diff 232638.

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

https://reviews.llvm.org/D71091

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGObjCMac.cpp
  clang/lib/Sema/SemaDeclObjC.cpp
  clang/lib/Serialization/ASTWriterDecl.cpp
  clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
  clang/test/CodeGenObjC/direct-method.m

Index: clang/test/CodeGenObjC/direct-method.m
===
--- clang/test/CodeGenObjC/direct-method.m
+++ clang/test/CodeGenObjC/direct-method.m
@@ -11,9 +11,17 @@
 
 __attribute__((objc_root_class))
 @interface Root
+- (int)getInt __attribute__((objc_direct));
+@property(direct, readonly) int intProperty;
+@property(direct, readonly) int intProperty2;
 @end
 
 @implementation Root
+// CHECK-LABEL: define hidden i32 @"\01-[Root intProperty2]"
+- (int)intProperty2 {
+  return 42;
+}
+
 // CHECK-LABEL: define hidden i32 @"\01-[Root getInt]"(
 - (int)getInt __attribute__((objc_direct)) {
   // loading parameters
@@ -152,6 +160,7 @@
 }
 
 @end
+// CHECK-LABEL: define hidden i32 @"\01-[Root intProperty]"
 
 @interface Foo : Root {
   id __strong _cause_cxx_destruct;
@@ -173,3 +182,11 @@
 // CHECK-LABEL: define hidden void @"\01-[Foo setGetDynamic_setDirect:]"(
 // CHECK-LABEL: define internal void @"\01-[Foo .cxx_destruct]"(
 @end
+
+int useRoot(Root *r) {
+  // CHECK-LABEL: define i32 @useRoot
+  // CHECK: %{{[^ ]*}} = call i32 bitcast {{.*}} @"\01-[Root getInt]"
+  // CHECK: %{{[^ ]*}} = call i32 bitcast {{.*}} @"\01-[Root intProperty]"
+  // CHECK: %{{[^ ]*}} = call i32 bitcast {{.*}} @"\01-[Root intProperty2]"
+  return [r getInt] + [r intProperty] + [r intProperty2];
+}
Index: clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
===
--- clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
+++ clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
@@ -722,13 +722,6 @@
 } else if (const auto *OCD = dyn_cast(DC)) {
   OS << OCD->getClassInterface()->getName() << '('
  << OCD->getName() << ')';
-} else if (isa(DC)) {
-  // We can extract the type of the class from the self pointer.
-  if (ImplicitParamDecl *SelfDecl = OMD->getSelfDecl()) {
-QualType ClassTy =
-cast(SelfDecl->getType())->getPointeeType();
-ClassTy.print(OS, PrintingPolicy(LangOptions()));
-  }
 }
 OS << ' ' << OMD->getSelector().getAsString() << ']';
 
Index: clang/lib/Serialization/ASTWriterDecl.cpp
===
--- clang/lib/Serialization/ASTWriterDecl.cpp
+++ clang/lib/Serialization/ASTWriterDecl.cpp
@@ -664,8 +664,7 @@
   VisitNamedDecl(D);
   // FIXME: convert to LazyStmtPtr?
   // Unlike C/C++, method bodies will never be in header files.
-  bool HasBodyStuff = D->getBody() != nullptr ||
-  D->getSelfDecl() != nullptr || D->getCmdDecl() != nullptr;
+  bool HasBodyStuff = D->getBody() != nullptr;
   Record.push_back(HasBodyStuff);
   if (HasBodyStuff) {
 Record.AddStmt(D->getBody());
Index: clang/lib/Sema/SemaDeclObjC.cpp
===
--- clang/lib/Sema/SemaDeclObjC.cpp
+++ clang/lib/Sema/SemaDeclObjC.cpp
@@ -383,9 +383,6 @@
   // Create Decl objects for each parameter, entrring them in the scope for
   // binding to their use.
 
-  // Insert the invisible arguments, self and _cmd!
-  MDecl->createImplicitParams(Context, MDecl->getClassInterface());
-
   PushOnScopeChains(MDecl->getSelfDecl(), FnBodyScope);
   PushOnScopeChains(MDecl->getCmdDecl(), FnBodyScope);
 
@@ -4869,6 +4866,9 @@
 }
   }
 
+  // Insert the invisible arguments, self and _cmd!
+  ObjCMethod->createImplicitParams(Context, ObjCMethod->getClassInterface());
+
   ActOnDocumentableDecl(ObjCMethod);
 
   return ObjCMethod;
Index: clang/lib/CodeGen/CGObjCMac.cpp
===
--- clang/lib/CodeGen/CGObjCMac.cpp
+++ clang/lib/CodeGen/CGObjCMac.cpp
@@ -4027,7 +4027,7 @@
 llvm::Function *
 CGObjCCommonMac::GenerateDirectMethod(const ObjCMethodDecl *OMD,
   const ObjCContainerDecl *CD) {
-  auto I = DirectMethodDefinitions.find(OMD);
+  auto I = DirectMethodDefinitions.find(OMD->getCanonicalDecl());
   if (I != DirectMethodDefinitions.end())
 return I->second;
 
@@ -4040,7 +4040,7 @@
   llvm::Function *Method =
   llvm::Function::Create(MethodTy, llvm::GlobalValue::ExternalLinkage,
  Name.str(), ());
-  DirectMethodDefinitions.insert(std::make_pair(OMD, Method));
+  DirectMethodDefinitions.insert(std::make_pair(OMD->getCanonicalDecl(), Method));
 
   return Method;
 }
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ 

[PATCH] D70877: [WebAssebmly][MC] Support .import_name/.import_field asm directives

[Sam Clegg via Phabricator via cfe-commits]

sbc100 added a comment.

Ping ..


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70877



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

[PATCH] D71080: [NFC] Separate getLastArgIntValue to Basic

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl marked 5 inline comments as done.
yaxunl added inline comments.



Comment at: clang/include/clang/Basic/OptionUtils.h:24
+
+class ArgList;
+

tra wrote:
> What are the rules on using LLVM headers here? Can we just include 
> llvm/Option/ArgList.h instead?
In the API llvm::opt::ArgList is used as reference, therefore it only needs a 
forward declaration. llvm::opt::OptSpecifier is passed by value, therefore it 
needs header. OptSpecifier is a small class, therefore it is not efficient to 
pass by reference.



Comment at: clang/include/clang/Basic/OptionUtils.h:46
+   DiagnosticsEngine *Diags = nullptr,
+   unsigned Base = 10);
+

tra wrote:
> Same question as before -- does it have to be `10`?
> `0` would be a more reasonable default for general use. IMO we care about the 
> value, but not so much about the form. I.e. is there a reason not to allow 
> 0xf, for instance, if that works for the user?
> 
will do. StringRef will do radix autosensing for that.



Comment at: clang/lib/Basic/CMakeLists.txt:1
 set(LLVM_LINK_COMPONENTS
   Core

tra wrote:
> I think now that you're using ArgList, you need to depend on LLVM's 
> LLVMOption library.
> As is you're likely to run into build issues if shared libraries are enabled.
will do


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

https://reviews.llvm.org/D71080



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

[PATCH] D70553: [clang-apply-replacements] Add command line option to overwrite readonly files.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D70553#1773046 , @zturner wrote:

> In D70553#1757862 , @aaron.ballman 
> wrote:
>
> > Can you add a test case for this functionality?
>
>
> The reason there's no test case is because it seems like that wouldn't really 
> be any different than testing the functionality of `llvm::sys::fs`, which 
> should already be handled at the llvm/Support layer.  There's not really any 
> interesting logic here.  I can still do one if you think it's important but 
> that was my reasoning anyway.


I have a slight preference for a test because it's a user-facing feature and we 
want to be alerted if we ever regress it. The unit tests in llvm/Support will 
catch if we break the underlying get/set permissions, but not the user feature 
(though it would be hard to break this user feature except through get/set 
permissions, lol). If it's hard to test for some reason, I don't insist on it.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D70553



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

[PATCH] D70973: [OPENMP50]Treat context selectors as expressions, not just strings.

[Alexey Bataev via Phabricator via cfe-commits]

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



Comment at: clang/include/clang/Basic/DiagnosticParseKinds.td:1230
-  "unknown '%0' device kind trait in the 'device' context selector set, 
expected"
-  " one of 'host', 'nohost', 'cpu', 'gpu' or 'fpga'">;
 

jdoerfert wrote:
> ABataev wrote:
> > jdoerfert wrote:
> > > I would have expected this error to be still accurate, maybe with the 
> > > addition ", or quoted versions thereof".
> > Currently, we could emit it only in codegen phase to avoid double 
> > converting from expression to string. Will emit it there.
> Why can't we emit this error if the user writes `device={kind(gpu)}` 
> anymore? Even `device={kind("gpu")}` should be diagnosable early.
The main problem here is the conversion and expression evaluation. We convert 
the data from the expression to strings at the codegen phase. I don't want to 
do the same thing for the second time in Sema just for diagnostic.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70973



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

[PATCH] D71141: [Wdocumentation] Use C++14 deprecated attribute

[Mark de Wever via Phabricator via cfe-commits]

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



Comment at: clang/lib/AST/CommentSema.cpp:693
+StringRef AttributeSpelling =
+CPlusPlus14 ? "[[deprecated]]" : "__attribute__((deprecated))";
 if (PP) {

aaron.ballman wrote:
> Mordante wrote:
> > aaron.ballman wrote:
> > > This attribute also exists with this spelling in C2x, FWIW.
> > True, but unless I'm mistaken `CPlusPlus17` and `CPlusPlus2a` also include 
> > `CPlusPlus14`. Do you prefer a different name for the Boolean?
> I'm talking about C2x, not C++2a. The name for the variable is fine, but we 
> should prefer `[[deprecated]]` in C2x mode to `__attribute__((deprecated))`.
> 
> I think the correct predicate is: 
> `getLangOpts().DoubleSquareBracketAttributes` -- if the user says they want 
> to use double-square bracket attributes, we should probably prefer them to 
> GNU-style attributes.
Ah sorry I misread. I'll have a look at C2x. Thanks for the information.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71141



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

[PATCH] D71049: Remove implicit conversion that promotes half to other larger precision types for fp classification builtins

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Thank you for the patch -- can you add some test cases?




Comment at: clang/lib/Sema/SemaChecking.cpp:5835-5836
+"is the only expected cast here");
+Cast->setSubExpr(nullptr);
+TheCall->setArg(NumArgs-1, CastArg);
+  }

I think this should be combined with the code above. Something like:
```
bool IgnoreCast = false;
if (CastArg-> is Float) {
  assert(stuff);
  IgnoreCast = true;
} else if (CastArg-> is Half) {
  assert(stuff);
  IgnoreCast = true;
}

if (IgnoreCast) {
  Cast->setSubExpr(nullptr);
  TheCall->setArg(NumArgs - 1, CastArg);
}
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71049



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

[PATCH] D71141: [Wdocumentation] Use C++14 deprecated attribute

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/AST/CommentSema.cpp:693
+StringRef AttributeSpelling =
+CPlusPlus14 ? "[[deprecated]]" : "__attribute__((deprecated))";
 if (PP) {

Mordante wrote:
> aaron.ballman wrote:
> > This attribute also exists with this spelling in C2x, FWIW.
> True, but unless I'm mistaken `CPlusPlus17` and `CPlusPlus2a` also include 
> `CPlusPlus14`. Do you prefer a different name for the Boolean?
I'm talking about C2x, not C++2a. The name for the variable is fine, but we 
should prefer `[[deprecated]]` in C2x mode to `__attribute__((deprecated))`.

I think the correct predicate is: `getLangOpts().DoubleSquareBracketAttributes` 
-- if the user says they want to use double-square bracket attributes, we 
should probably prefer them to GNU-style attributes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71141



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

[PATCH] D71005: [AST] Enable expression of OpenCL language address spaces an attribute

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D71005#1773042 , @bader wrote:

> @aaron.ballman, thank you for fixing the problem with documentation 
> generation.


No problem!

> With regards to attributes naming question, let me check if I get it right.
> 
> 1. You suggest checking with Khronos if they want to adopt 
> `[[opencl::private]]` attribute names in OpenCL kernel language standard. 
> Right? @Anastasia, can you help with this?

That's correct.

> 2. If Khronos leaves this decision to implementers, are current names (e.g. 
> `[[clang::opencl_private]]`) look okay or we should consider other options?

If they want to leave it to implementers, I think the names you have now are 
fine.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71005



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

[PATCH] D70973: [OPENMP50]Treat context selectors as expressions, not just strings.

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticParseKinds.td:1230
-  "unknown '%0' device kind trait in the 'device' context selector set, 
expected"
-  " one of 'host', 'nohost', 'cpu', 'gpu' or 'fpga'">;
 

ABataev wrote:
> jdoerfert wrote:
> > I would have expected this error to be still accurate, maybe with the 
> > addition ", or quoted versions thereof".
> Currently, we could emit it only in codegen phase to avoid double converting 
> from expression to string. Will emit it there.
Why can't we emit this error if the user writes `device={kind(gpu)}` 
anymore? Even `device={kind("gpu")}` should be diagnosable early.



Comment at: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:424
+if (ER.isUsable())
+  Values.push_back(ER);
   }

ABataev wrote:
> jdoerfert wrote:
> > When would ER not be usable here? Is there a valid use case or should we 
> > assert it is?
> If the instantiation is failed, the error message is emitted and `ER` is set 
> to `ExprError`, which is not usable.
Thx.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70973



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

[clang-tools-extra] a7bdab2 - [clang-tidy] Pass -faligned-allocation on the compiler command line to

[Akira Hatanaka via cfe-commits]

Author: Akira Hatanaka
Date: 2019-12-06T12:29:21-08:00
New Revision: a7bdab2e9d59ba0fdf06390f4ddadfd00fe50f2e

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

LOG: [clang-tidy] Pass -faligned-allocation on the compiler command line to
fix compile error

The test was failing when run on OSes older than MacOSX10.14 because
aligned deallocation functions are unavailable on older OSes.

rdar://problem/57706710

Added: 


Modified: 
clang-tools-extra/test/clang-tidy/checkers/cert-mem57-cpp-cpp17.cpp

Removed: 




diff  --git 
a/clang-tools-extra/test/clang-tidy/checkers/cert-mem57-cpp-cpp17.cpp 
b/clang-tools-extra/test/clang-tidy/checkers/cert-mem57-cpp-cpp17.cpp
index f2c1cb48d9d3..d393b229bda1 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/cert-mem57-cpp-cpp17.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/cert-mem57-cpp-cpp17.cpp
@@ -1,6 +1,6 @@
 // RUN: %check_clang_tidy %s -std=c++14 cert-mem57-cpp %t
-// RUN: clang-tidy --extra-arg='-std=c++17' -checks='-*,cert-mem57-cpp' 
--warnings-as-errors='*' %s
-// RUN: clang-tidy --extra-arg='-std=c++2a' -checks='-*,cert-mem57-cpp' 
--warnings-as-errors='*' %s
+// RUN: clang-tidy --extra-arg='-std=c++17' --extra-arg='-faligned-allocation' 
-checks='-*,cert-mem57-cpp' --warnings-as-errors='*' %s
+// RUN: clang-tidy --extra-arg='-std=c++2a' --extra-arg='-faligned-allocation' 
-checks='-*,cert-mem57-cpp' --warnings-as-errors='*' %s
 
 struct alignas(128) Vector {
   char Elems[128];



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

[PATCH] D71091: Make sure that the implicit arguments for direct methods have been setup

[Pierre Habouzit via Phabricator via cfe-commits]

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



Comment at: clang/lib/Sema/SemaDeclObjC.cpp:4839
 
+  ObjCMethod->createImplicitParams(Context, ObjCMethod->getClassInterface());
+

liuliu wrote:
> Yeah, I applied this change. As I said, for my trivial example it does work. 
> But for my more thorough example with @property (direct) etc, it doesn't. I 
> can dig into more later today or Monday. I just got everything work (with 
> another more hackier fix) and would first to evaluate this as a viable path 
> first before commit more time to debug this.
> 
> We probably can also just have a follow-up fix for that, depends on if 
> reviewers thought this is blocking or not. I personally think not.
Huh I could have seen why property methods would have an issue but they seem to 
have gotten their `direct`ness properly anyway (I'll add them to the CG test 
for good measure while I debug the tests I broke).

so I'll need a reduced test case from you I can't seem to find an easy way to 
break it indeed.


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

https://reviews.llvm.org/D71091



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

[PATCH] D71141: [Wdocumentation] Use C++14 deprecated attribute

[Mark de Wever via Phabricator via cfe-commits]

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



Comment at: clang/lib/AST/CommentSema.cpp:693
+StringRef AttributeSpelling =
+CPlusPlus14 ? "[[deprecated]]" : "__attribute__((deprecated))";
 if (PP) {

aaron.ballman wrote:
> This attribute also exists with this spelling in C2x, FWIW.
True, but unless I'm mistaken `CPlusPlus17` and `CPlusPlus2a` also include 
`CPlusPlus14`. Do you prefer a different name for the Boolean?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71141



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

[PATCH] D71082: Allow system header to provide their own implementation of some builtin

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

It looks like this patch accidentally suppresses certain warnings when 
_FORTIFY_SOURCE is enabled.  For example, Sema::CheckMemaccessArguments only 
gets called for recognized builtin functions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71082



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

[PATCH] D71142: [Sema] Validate large bitfields

[Mark de Wever via Phabricator via cfe-commits]

Mordante created this revision.
Mordante added reviewers: rsmith, majnemer, aaron.ballman.
Mordante added a project: clang.

Note: I'm not entirely happy with the text of the diagnostics and I'm open to 
suggestions.

Fixes PR23505: Large bitfield: Assertion `getActiveBits() <= 64 && "Too many 
bits for uint64_t"' failed


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71142

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDecl.cpp
  clang/test/Sema/bitfield.c


Index: clang/test/Sema/bitfield.c
===
--- clang/test/Sema/bitfield.c
+++ clang/test/Sema/bitfield.c
@@ -28,6 +28,12 @@
 
   _Bool : 2;   // expected-error {{width of anonymous bit-field (2 bits) 
exceeds width of its type (1 bit)}}
   _Bool h : 5; // expected-error {{width of bit-field 'h' (5 bits) exceeds 
width of its type (1 bit)}}
+
+  // PR23505
+  _Bool : 1 + (unsigned __int128)0x;   // expected-error 
{{width of anonymous bit-field doesn't fit in a 64 bit unsigned integer}}
+  _Bool i : 1 + (unsigned __int128)0x; // expected-error 
{{width of bit-field 'i' doesn't fit in a 64 bit unsigned integer}}
+  int : 1 + (unsigned __int128)0x; // expected-error 
{{width of anonymous bit-field doesn't fit in a 64 bit unsigned integer}}
+  int j : 1 + (unsigned __int128)0x;   // expected-error 
{{width of bit-field 'j' doesn't fit in a 64 bit unsigned integer}}
 };
 
 struct b {unsigned x : 2;} x;
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -15842,6 +15842,17 @@
 uint64_t TypeWidth = Context.getIntWidth(FieldTy);
 bool BitfieldIsOverwide = Value.ugt(TypeWidth);
 
+// Don't accept too wide bit-fields they will cause assertion failures
+// when used.
+if (BitfieldIsOverwide && Value.ugt(UINT64_MAX)) {
+  if (FieldName)
+return Diag(FieldLoc, diag::warn_bitfield_width_exceeds_maximum_width)
+   << FieldName;
+
+  return Diag(FieldLoc,
+  diag::warn_anon_bitfield_width_exceeds_maximum_width);
+}
+
 // Over-wide bitfields are an error in C or when using the MSVC bitfield
 // ABI.
 bool CStdConstraintViolation =
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -5183,6 +5183,10 @@
   "number of elements must be either one or match the size of the vector">;
 
 // Used by C++ which allows bit-fields that are wider than the type.
+def warn_bitfield_width_exceeds_maximum_width: Error<
+  "width of bit-field %0 doesn't fit in a 64 bit unsigned integer">;
+def warn_anon_bitfield_width_exceeds_maximum_width : Error<
+  "width of anonymous bit-field doesn't fit in a 64 bit unsigned integer">;
 def warn_bitfield_width_exceeds_type_width: Warning<
   "width of bit-field %0 (%1 bits) exceeds the width of its type; value will "
   "be truncated to %2 bit%s2">, InGroup;


Index: clang/test/Sema/bitfield.c
===
--- clang/test/Sema/bitfield.c
+++ clang/test/Sema/bitfield.c
@@ -28,6 +28,12 @@
 
   _Bool : 2;   // expected-error {{width of anonymous bit-field (2 bits) exceeds width of its type (1 bit)}}
   _Bool h : 5; // expected-error {{width of bit-field 'h' (5 bits) exceeds width of its type (1 bit)}}
+
+  // PR23505
+  _Bool : 1 + (unsigned __int128)0x;   // expected-error {{width of anonymous bit-field doesn't fit in a 64 bit unsigned integer}}
+  _Bool i : 1 + (unsigned __int128)0x; // expected-error {{width of bit-field 'i' doesn't fit in a 64 bit unsigned integer}}
+  int : 1 + (unsigned __int128)0x; // expected-error {{width of anonymous bit-field doesn't fit in a 64 bit unsigned integer}}
+  int j : 1 + (unsigned __int128)0x;   // expected-error {{width of bit-field 'j' doesn't fit in a 64 bit unsigned integer}}
 };
 
 struct b {unsigned x : 2;} x;
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -15842,6 +15842,17 @@
 uint64_t TypeWidth = Context.getIntWidth(FieldTy);
 bool BitfieldIsOverwide = Value.ugt(TypeWidth);
 
+// Don't accept too wide bit-fields they will cause assertion failures
+// when used.
+if (BitfieldIsOverwide && Value.ugt(UINT64_MAX)) {
+  if (FieldName)
+return Diag(FieldLoc, diag::warn_bitfield_width_exceeds_maximum_width)
+   << FieldName;
+
+  return Diag(FieldLoc,
+  diag::warn_anon_bitfield_width_exceeds_maximum_width);
+}
+
 // Over-wide bitfields are an error 

[clang] 779a180 - [OPENMP50]Add if clause in distribute simd directive.

[Alexey Bataev via cfe-commits]

Author: Alexey Bataev
Date: 2019-12-06T14:49:49-05:00
New Revision: 779a180d964bf362f26f4c493db749cbbae550c5

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

LOG: [OPENMP50]Add if clause in distribute simd directive.

According to OpenMP 5.0, if clause can be used in for simd directive. If
condition in the if clause if false, the non-vectorized version of the
loop must be executed.

Added: 
clang/test/OpenMP/distribute_simd_if_messages.cpp

Modified: 
clang/include/clang/Basic/OpenMPKinds.def
clang/lib/Basic/OpenMPKinds.cpp
clang/lib/CodeGen/CGStmtOpenMP.cpp
clang/lib/Sema/SemaOpenMP.cpp
clang/test/OpenMP/distribute_simd_ast_print.cpp
clang/test/OpenMP/distribute_simd_codegen.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/OpenMPKinds.def 
b/clang/include/clang/Basic/OpenMPKinds.def
index d2bfae4e60c1..6ab35cafc7ed 100644
--- a/clang/include/clang/Basic/OpenMPKinds.def
+++ b/clang/include/clang/Basic/OpenMPKinds.def
@@ -872,6 +872,7 @@ OPENMP_DISTRIBUTE_SIMD_CLAUSE(safelen)
 OPENMP_DISTRIBUTE_SIMD_CLAUSE(simdlen)
 OPENMP_DISTRIBUTE_SIMD_CLAUSE(reduction)
 OPENMP_DISTRIBUTE_SIMD_CLAUSE(allocate)
+OPENMP_DISTRIBUTE_SIMD_CLAUSE(if)
 
 // Clauses allowed for OpenMP directive 'target parallel for simd'.
 OPENMP_TARGET_PARALLEL_FOR_SIMD_CLAUSE(if)

diff  --git a/clang/lib/Basic/OpenMPKinds.cpp b/clang/lib/Basic/OpenMPKinds.cpp
index d95850fc7963..c075deb01a18 100644
--- a/clang/lib/Basic/OpenMPKinds.cpp
+++ b/clang/lib/Basic/OpenMPKinds.cpp
@@ -790,6 +790,8 @@ bool clang::isAllowedClauseForDirective(OpenMPDirectiveKind 
DKind,
 }
 break;
   case OMPD_distribute_simd:
+if (OpenMPVersion < 50 && CKind == OMPC_if)
+  return false;
 switch (CKind) {
 #define OPENMP_DISTRIBUTE_SIMD_CLAUSE(Name)
\
   case OMPC_##Name:
\

diff  --git a/clang/lib/CodeGen/CGStmtOpenMP.cpp 
b/clang/lib/CodeGen/CGStmtOpenMP.cpp
index a5190007406d..6f3d6d859357 100644
--- a/clang/lib/CodeGen/CGStmtOpenMP.cpp
+++ b/clang/lib/CodeGen/CGStmtOpenMP.cpp
@@ -3605,8 +3605,6 @@ void CodeGenFunction::EmitOMPDistributeLoop(const 
OMPLoopDirective ,
   if (RT.isStaticNonchunked(ScheduleKind,
 /* Chunked */ Chunk != nullptr) ||
   StaticChunked) {
-if (isOpenMPSimdDirective(S.getDirectiveKind()))
-  EmitOMPSimdInit(S, /*IsMonotonic=*/true);
 CGOpenMPRuntime::StaticRTInput StaticInit(
 IVSize, IVSigned, /* Ordered = */ false, IL.getAddress(*this),
 LB.getAddress(*this), UB.getAddress(*this), ST.getAddress(*this),
@@ -3656,18 +3654,28 @@ void CodeGenFunction::EmitOMPDistributeLoop(const 
OMPLoopDirective ,
 //   IV = LB;
 // }
 //
-EmitOMPInnerLoop(S, LoopScope.requiresCleanups(), Cond, IncExpr,
- [, LoopExit, ](CodeGenFunction ) {
-   CodeGenLoop(CGF, S, LoopExit);
- },
- [, StaticChunked](CodeGenFunction ) {
-   if (StaticChunked) {
- 
CGF.EmitIgnoredExpr(S.getCombinedNextLowerBound());
- 
CGF.EmitIgnoredExpr(S.getCombinedNextUpperBound());
- 
CGF.EmitIgnoredExpr(S.getCombinedEnsureUpperBound());
- CGF.EmitIgnoredExpr(S.getCombinedInit());
-   }
- });
+emitCommonSimdLoop(
+*this, S,
+[](CodeGenFunction , PrePostActionTy &) {
+  if (isOpenMPSimdDirective(S.getDirectiveKind()))
+CGF.EmitOMPSimdInit(S, /*IsMonotonic=*/true);
+},
+[, , Cond, IncExpr, LoopExit, ,
+ StaticChunked](CodeGenFunction , PrePostActionTy &) {
+  CGF.EmitOMPInnerLoop(
+  S, LoopScope.requiresCleanups(), Cond, IncExpr,
+  [, LoopExit, ](CodeGenFunction ) {
+CodeGenLoop(CGF, S, LoopExit);
+  },
+  [, StaticChunked](CodeGenFunction ) {
+if (StaticChunked) {
+  CGF.EmitIgnoredExpr(S.getCombinedNextLowerBound());
+  CGF.EmitIgnoredExpr(S.getCombinedNextUpperBound());
+  CGF.EmitIgnoredExpr(S.getCombinedEnsureUpperBound());
+  CGF.EmitIgnoredExpr(S.getCombinedInit());
+}
+  });
+});
 EmitBlock(LoopExit.getBlock());
 // Tell the runtime we are done.
 RT.emitForStaticFinish(*this, S.getBeginLoc(), S.getDirectiveKind());

diff  --git 

[PATCH] D71134: [OpenMP] Require trivially copyable type for mapping

[Alexey Bataev via Phabricator via cfe-commits]

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

LG


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71134



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

[PATCH] D70804: [Frontend] Allow OpenMP offloading to aarch64

[Bryan Chan via Phabricator via cfe-commits]

bryanpkc updated this revision to Diff 232627.
bryanpkc marked 2 inline comments as done.
bryanpkc added a comment.

Removed unrelated changes from this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70804

Files:
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/OpenMP/openmp_offload_registration.cpp


Index: clang/test/OpenMP/openmp_offload_registration.cpp
===
--- clang/test/OpenMP/openmp_offload_registration.cpp
+++ clang/test/OpenMP/openmp_offload_registration.cpp
@@ -1,5 +1,6 @@
-// Test for offload registration code for two targets
+// Test offload registration for two targets, and test offload target 
validation.
 // RUN: %clang_cc1 -verify -fopenmp -x c -triple x86_64-unknown-linux-gnu 
-fopenmp-targets=x86_64-pc-linux-gnu,powerpc64le-ibm-linux-gnu -emit-llvm %s -o 
- | FileCheck %s
+// RUN: %clang_cc1 -verify -fopenmp -x c -triple x86_64-unknown-linux-gnu 
-fopenmp-targets=aarch64-unknown-linux-gnu -emit-llvm %s -o - | FileCheck %s
 // expected-no-diagnostics
 
 void foo() {
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -3070,7 +3070,8 @@
   llvm::Triple TT(A->getValue(i));
 
   if (TT.getArch() == llvm::Triple::UnknownArch ||
-  !(TT.getArch() == llvm::Triple::ppc ||
+  !(TT.getArch() == llvm::Triple::aarch64 ||
+TT.getArch() == llvm::Triple::ppc ||
 TT.getArch() == llvm::Triple::ppc64 ||
 TT.getArch() == llvm::Triple::ppc64le ||
 TT.getArch() == llvm::Triple::nvptx ||


Index: clang/test/OpenMP/openmp_offload_registration.cpp
===
--- clang/test/OpenMP/openmp_offload_registration.cpp
+++ clang/test/OpenMP/openmp_offload_registration.cpp
@@ -1,5 +1,6 @@
-// Test for offload registration code for two targets
+// Test offload registration for two targets, and test offload target validation.
 // RUN: %clang_cc1 -verify -fopenmp -x c -triple x86_64-unknown-linux-gnu -fopenmp-targets=x86_64-pc-linux-gnu,powerpc64le-ibm-linux-gnu -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -verify -fopenmp -x c -triple x86_64-unknown-linux-gnu -fopenmp-targets=aarch64-unknown-linux-gnu -emit-llvm %s -o - | FileCheck %s
 // expected-no-diagnostics
 
 void foo() {
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -3070,7 +3070,8 @@
   llvm::Triple TT(A->getValue(i));
 
   if (TT.getArch() == llvm::Triple::UnknownArch ||
-  !(TT.getArch() == llvm::Triple::ppc ||
+  !(TT.getArch() == llvm::Triple::aarch64 ||
+TT.getArch() == llvm::Triple::ppc ||
 TT.getArch() == llvm::Triple::ppc64 ||
 TT.getArch() == llvm::Triple::ppc64le ||
 TT.getArch() == llvm::Triple::nvptx ||
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71141: [Wdocumentation] Use C++14 deprecated attribute

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/AST/CommentSema.cpp:693
+StringRef AttributeSpelling =
+CPlusPlus14 ? "[[deprecated]]" : "__attribute__((deprecated))";
 if (PP) {

This attribute also exists with this spelling in C2x, FWIW.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71141



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

[PATCH] D71134: [OpenMP] Require trivially copyable type for mapping

[Jonas Hahnfeld via Phabricator via cfe-commits]

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



Comment at: clang/lib/Sema/SemaOpenMP.cpp:14917
   if (FullCheck && !SemaRef.CurContext->isDependentContext() &&
-  !QTy.isTrivialType(SemaRef.Context))
+  !QTy.isTriviallyCopyableType(SemaRef.Context))
 SemaRef.Diag(SL, diag::warn_omp_non_trivial_type_mapped) << QTy << SR;

ABataev wrote:
> Need to add a check for defaulted destructor here, `isTriviallyCopyableType` 
> does not include this check.
As I said in the summary, this is a prerequisite for being trivially copyable 
and as such checked by `CXXRecordDecl::isTriviallyCopyable()` (line `if 
(!hasTrivialDestructor()) return false;`).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71134



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

[PATCH] D71091: Make sure that the implicit arguments for direct methods have been setup

[Liu Liu via Phabricator via cfe-commits]

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



Comment at: clang/lib/Sema/SemaDeclObjC.cpp:4839
 
+  ObjCMethod->createImplicitParams(Context, ObjCMethod->getClassInterface());
+

Yeah, I applied this change. As I said, for my trivial example it does work. 
But for my more thorough example with @property (direct) etc, it doesn't. I can 
dig into more later today or Monday. I just got everything work (with another 
more hackier fix) and would first to evaluate this as a viable path first 
before commit more time to debug this.

We probably can also just have a follow-up fix for that, depends on if 
reviewers thought this is blocking or not. I personally think not.


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

https://reviews.llvm.org/D71091



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

[PATCH] D71141: [Wdocumentation] Use C++14 deprecated attribute

[Mark de Wever via Phabricator via cfe-commits]

Mordante created this revision.
Mordante added a reviewer: gribozavr2.
Mordante added a project: clang.

This replaces the non-standard `__attribute__((deprecated))` with the standard 
`[[deprecated]]` when compiling in C++14 mode.

Discovered while looking at https://bugs.llvm.org/show_bug.cgi?id=43753

Depends: D71140 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71141

Files:
  clang/lib/AST/CommentSema.cpp
  clang/test/Sema/warn-documentation-fixits.cpp
  clang/test/Sema/warn-documentation.cpp

Index: clang/test/Sema/warn-documentation.cpp
===
--- clang/test/Sema/warn-documentation.cpp
+++ clang/test/Sema/warn-documentation.cpp
@@ -612,6 +612,12 @@
 /// \deprecated Bbb
 void test_deprecated_1(int a) __attribute__((deprecated));
 
+#if __cplusplus >= 201402L
+/// Aaa
+/// \deprecated Bbb
+[[deprecated]] void test_deprecated_no_warning_std14(int a);
+#endif
+
 // We don't want \deprecated to warn about empty paragraph.  It is fine to use
 // \deprecated by itself without explanations.
 
Index: clang/test/Sema/warn-documentation-fixits.cpp
===
--- clang/test/Sema/warn-documentation-fixits.cpp
+++ clang/test/Sema/warn-documentation-fixits.cpp
@@ -1,6 +1,6 @@
 // RUN: %clang_cc1 -fsyntax-only -Wdocumentation -Wdocumentation-pedantic -fcomment-block-commands=foobar -verify %s
-// RUN: %clang_cc1 -std=c++11 -fsyntax-only -Wdocumentation -Wdocumentation-pedantic -fcomment-block-commands=foobar -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
-// RUN: %clang_cc1 -std=c++14 -fsyntax-only -Wdocumentation -Wdocumentation-pedantic -fcomment-block-commands=foobar -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck --check-prefixes=CHECK,CHECK14 %s
+// RUN  %clang_cc1 -std=c++11 -fsyntax-only -Wdocumentation -Wdocumentation-pedantic -fcomment-block-commands=foobar -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck -DATTRIBUTE="__attribute__((deprecated))" %s
+// RUN: %clang_cc1 -std=c++14 -fsyntax-only -Wdocumentation -Wdocumentation-pedantic -fcomment-block-commands=foobar -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck --check-prefixes=CHECK,CHECK14 -DATTRIBUTE="[[deprecated]]" %s
 
 // expected-warning@+1 {{parameter 'ZZ' not found in the function declaration}} expected-note@+1 {{did you mean 'a'?}}
 /// \param ZZ Blah blah.
@@ -96,6 +96,14 @@
 /// \deprecated
 void test_deprecated_9(int a);
 
+#if __cplusplus >= 201402L
+#define ATTRIBUTE_DEPRECATED [[deprecated]]
+
+// expected-warning@+1 {{declaration is marked with '\deprecated' command but does not have a deprecation attribute}} expected-note@+2 {{add a deprecation attribute to the declaration to silence this warning}}
+/// \deprecated
+void test_deprecated_10(int a);
+#endif
+
 // rdar://12381408
 // expected-warning@+2  {{unknown command tag name 'retur'; did you mean 'return'?}}
 /// \brief testing fixit
@@ -119,16 +127,17 @@
 // CHECK: fix-it:"{{.*}}":{10:12-10:15}:"aaa"
 // CHECK: fix-it:"{{.*}}":{14:13-14:23}:"T"
 // CHECK: fix-it:"{{.*}}":{19:13-19:18}:"SomeTy"
-// CHECK: fix-it:"{{.*}}":{26:1-26:1}:"__attribute__((deprecated)) "
-// CHECK: fix-it:"{{.*}}":{30:1-30:1}:"__attribute__((deprecated)) "
-// CHECK: fix-it:"{{.*}}":{35:3-35:3}:"__attribute__((deprecated)) "
-// CHECK: fix-it:"{{.*}}":{39:3-39:3}:"__attribute__((deprecated)) "
-// CHECK: fix-it:"{{.*}}":{47:3-47:3}:"__attribute__((deprecated)) "
-// CHECK: fix-it:"{{.*}}":{51:3-51:3}:"__attribute__((deprecated)) "
-// CHECK: fix-it:"{{.*}}":{76:3-76:3}:"__attribute__((deprecated)) "
-// CHECK: fix-it:"{{.*}}":{81:3-81:3}:"__attribute__((deprecated)) "
-// CHECK14: fix-it:"{{.*}}":{87:3-87:3}:"__attribute__((deprecated)) "
+// CHECK: fix-it:"{{.*}}":{26:1-26:1}:"[[ATTRIBUTE]] "
+// CHECK: fix-it:"{{.*}}":{30:1-30:1}:"[[ATTRIBUTE]] "
+// CHECK: fix-it:"{{.*}}":{35:3-35:3}:"[[ATTRIBUTE]] "
+// CHECK: fix-it:"{{.*}}":{39:3-39:3}:"[[ATTRIBUTE]] "
+// CHECK: fix-it:"{{.*}}":{47:3-47:3}:"[[ATTRIBUTE]] "
+// CHECK: fix-it:"{{.*}}":{51:3-51:3}:"[[ATTRIBUTE]] "
+// CHECK: fix-it:"{{.*}}":{76:3-76:3}:"[[ATTRIBUTE]] "
+// CHECK: fix-it:"{{.*}}":{81:3-81:3}:"[[ATTRIBUTE]] "
+// CHECK14: fix-it:"{{.*}}":{87:3-87:3}:"[[ATTRIBUTE]] "
 // CHECK: fix-it:"{{.*}}":{97:1-97:1}:"MY_ATTR_DEPRECATED "
-// CHECK: fix-it:"{{.*}}":{102:6-102:11}:"return"
-// CHECK: fix-it:"{{.*}}":{106:6-106:11}:"foobar"
-// CHECK: fix-it:"{{.*}}":{115:6-115:12}:"endcode"
+// CHECK14: fix-it:"{{.*}}":{104:1-104:1}:"ATTRIBUTE_DEPRECATED "
+// CHECK: fix-it:"{{.*}}":{110:6-110:11}:"return"
+// CHECK: fix-it:"{{.*}}":{114:6-114:11}:"foobar"
+// CHECK: fix-it:"{{.*}}":{123:6-123:12}:"endcode"
Index: clang/lib/AST/CommentSema.cpp
===
--- clang/lib/AST/CommentSema.cpp
+++ clang/lib/AST/CommentSema.cpp
@@ -688,17 +688,33 @@
 FD->doesThisDeclarationHaveABody())
   

[PATCH] D71140: [Wdocumentation] Properly place deprecated attribute

[Mark de Wever via Phabricator via cfe-commits]

Mordante created this revision.
Mordante added a reviewer: gribozavr2.
Mordante added a project: clang.

It is now placed before the function:

- allows to replace `__attribute__((deprecated))` with `[[deprecated]]`.
- required for trailing returns.

Fixes bug: https://bugs.llvm.org/show_bug.cgi?id=43753


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71140

Files:
  clang/lib/AST/CommentSema.cpp
  clang/test/Sema/warn-documentation-fixits.cpp
  clang/test/Sema/warn-documentation.cpp

Index: clang/test/Sema/warn-documentation.cpp
===
--- clang/test/Sema/warn-documentation.cpp
+++ clang/test/Sema/warn-documentation.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -std=c++11 -fsyntax-only -Wdocumentation -Wdocumentation-pedantic -verify %s
+// RUN: %clang_cc1 -std=c++14 -fsyntax-only -Wdocumentation -Wdocumentation-pedantic -verify %s
 
 // This file contains lots of corner cases, so ensure that XML we generate is not invalid.
 // RUN: c-index-test -test-load-source all -comments-xml-schema=%S/../../bindings/xml/comment-xml-schema.rng %s | FileCheck %s -check-prefix=WRONG
@@ -643,6 +644,44 @@
 template
 void test_deprecated_7(T aaa);
 
+class PR43753 {
+  // expected-warning@+2 {{declaration is marked with '\deprecated' command but does not have a deprecation attribute}}
+  // expected-note@+2 {{add a deprecation attribute to the declaration to silence this warning}}
+  /// \deprecated
+  static void test_deprecated_static();
+
+  // expected-warning@+2 {{declaration is marked with '\deprecated' command but does not have a deprecation attribute}}
+  // expected-note@+2 {{add a deprecation attribute to the declaration to silence this warning}}
+  /// \deprecated
+  static auto test_deprecated_static_trailing_return() -> int;
+
+#if __cplusplus >= 201402L
+  // expected-warning@+2 {{declaration is marked with '\deprecated' command but does not have a deprecation attribute}}
+  // expected-note@+2 {{add a deprecation attribute to the declaration to silence this warning}}
+  /// \deprecated
+  static decltype(auto) test_deprecated_static_decltype_auto() { return 1; }
+#endif
+
+  // expected-warning@+2 {{declaration is marked with '\deprecated' command but does not have a deprecation attribute}}
+  // expected-note@+2 {{add a deprecation attribute to the declaration to silence this warning}}
+  /// \deprecated
+  void test_deprecated_const() const;
+
+  // expected-warning@+2 {{declaration is marked with '\deprecated' command but does not have a deprecation attribute}}
+  // expected-note@+2 {{add a deprecation attribute to the declaration to silence this warning}}
+  /// \deprecated
+  auto test_deprecated_trailing_return() -> int;
+
+#if __cplusplus >= 201402L
+  // expected-warning@+2 {{declaration is marked with '\deprecated' command but does not have a deprecation attribute}}
+  // expected-note@+2 {{add a deprecation attribute to the declaration to silence this warning}}
+  /// \deprecated
+  decltype(auto) test_deprecated_decltype_auto() const { return a; }
+
+private:
+  int a{0};
+#endif
+};
 
 // rdar://12397511
 // expected-note@+2 {{previous command '\headerfile' here}}
Index: clang/test/Sema/warn-documentation-fixits.cpp
===
--- clang/test/Sema/warn-documentation-fixits.cpp
+++ clang/test/Sema/warn-documentation-fixits.cpp
@@ -1,5 +1,6 @@
 // RUN: %clang_cc1 -fsyntax-only -Wdocumentation -Wdocumentation-pedantic -fcomment-block-commands=foobar -verify %s
-// RUN: %clang_cc1 -fsyntax-only -Wdocumentation -Wdocumentation-pedantic -fcomment-block-commands=foobar -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -Wdocumentation -Wdocumentation-pedantic -fcomment-block-commands=foobar -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -std=c++14 -fsyntax-only -Wdocumentation -Wdocumentation-pedantic -fcomment-block-commands=foobar -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck --check-prefixes=CHECK,CHECK14 %s
 
 // expected-warning@+1 {{parameter 'ZZ' not found in the function declaration}} expected-note@+1 {{did you mean 'a'?}}
 /// \param ZZ Blah blah.
@@ -51,6 +52,44 @@
   }
 };
 
+class PR43753 {
+  // expected-warning@+2 {{declaration is marked with '\deprecated' command but does not have a deprecation attribute}}
+  // expected-note@+2 {{add a deprecation attribute to the declaration to silence this warning}}
+  /// \deprecated
+  static void test_deprecated_static();
+
+  // expected-warning@+2 {{declaration is marked with '\deprecated' command but does not have a deprecation attribute}}
+  // expected-note@+2 {{add a deprecation attribute to the declaration to silence this warning}}
+  /// \deprecated
+  static auto test_deprecated_static_trailing_return() -> int;
+
+#if __cplusplus >= 201402L
+  // expected-warning@+2 {{declaration is marked with '\deprecated' command 

[PATCH] D71139: [Wdocumentation] Use the command marker

[Mark de Wever via Phabricator via cfe-commits]

Mordante created this revision.
Mordante added a reviewer: gribozavr2.
Mordante added a project: clang.

Use the proper marker for `-Wdocumentation-deprecated-sync` instead of 
hard-coded the backslash.

Discovered while looking at https://bugs.llvm.org/show_bug.cgi?id=43753


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71139

Files:
  clang/include/clang/Basic/DiagnosticCommentKinds.td
  clang/lib/AST/CommentSema.cpp
  clang/test/Sema/warn-documentation.cpp


Index: clang/test/Sema/warn-documentation.cpp
===
--- clang/test/Sema/warn-documentation.cpp
+++ clang/test/Sema/warn-documentation.cpp
@@ -631,9 +631,9 @@
 /// \deprecated
 void test_deprecated_5(int a);
 
-// expected-warning@+2 {{declaration is marked with '\deprecated' command but 
does not have a deprecation attribute}} expected-note@+3 {{add a deprecation 
attribute to the declaration to silence this warning}}
+// expected-warning@+2 {{declaration is marked with '@deprecated' command but 
does not have a deprecation attribute}} expected-note@+3 {{add a deprecation 
attribute to the declaration to silence this warning}}
 /// Aaa
-/// \deprecated
+/// @deprecated
 void test_deprecated_6(int a) {
 }
 
Index: clang/lib/AST/CommentSema.cpp
===
--- clang/lib/AST/CommentSema.cpp
+++ clang/lib/AST/CommentSema.cpp
@@ -676,9 +676,8 @@
   D->hasAttr())
 return;
 
-  Diag(Command->getLocation(),
-   diag::warn_doc_deprecated_not_sync)
-<< Command->getSourceRange();
+  Diag(Command->getLocation(), diag::warn_doc_deprecated_not_sync)
+  << Command->getSourceRange() << Command->getCommandMarker();
 
   // Try to emit a fixit with a deprecation attribute.
   if (const FunctionDecl *FD = dyn_cast(D)) {
Index: clang/include/clang/Basic/DiagnosticCommentKinds.td
===
--- clang/include/clang/Basic/DiagnosticCommentKinds.td
+++ clang/include/clang/Basic/DiagnosticCommentKinds.td
@@ -146,8 +146,8 @@
 // \deprecated command
 
 def warn_doc_deprecated_not_sync : Warning<
-  "declaration is marked with '\\deprecated' command but does not have "
-  "a deprecation attribute">,
+  "declaration is marked with '%select{\\|@}0deprecated' command but does "
+  "not have a deprecation attribute">,
   InGroup, DefaultIgnore;
 
 def note_add_deprecation_attr : Note<


Index: clang/test/Sema/warn-documentation.cpp
===
--- clang/test/Sema/warn-documentation.cpp
+++ clang/test/Sema/warn-documentation.cpp
@@ -631,9 +631,9 @@
 /// \deprecated
 void test_deprecated_5(int a);
 
-// expected-warning@+2 {{declaration is marked with '\deprecated' command but does not have a deprecation attribute}} expected-note@+3 {{add a deprecation attribute to the declaration to silence this warning}}
+// expected-warning@+2 {{declaration is marked with '@deprecated' command but does not have a deprecation attribute}} expected-note@+3 {{add a deprecation attribute to the declaration to silence this warning}}
 /// Aaa
-/// \deprecated
+/// @deprecated
 void test_deprecated_6(int a) {
 }
 
Index: clang/lib/AST/CommentSema.cpp
===
--- clang/lib/AST/CommentSema.cpp
+++ clang/lib/AST/CommentSema.cpp
@@ -676,9 +676,8 @@
   D->hasAttr())
 return;
 
-  Diag(Command->getLocation(),
-   diag::warn_doc_deprecated_not_sync)
-<< Command->getSourceRange();
+  Diag(Command->getLocation(), diag::warn_doc_deprecated_not_sync)
+  << Command->getSourceRange() << Command->getCommandMarker();
 
   // Try to emit a fixit with a deprecation attribute.
   if (const FunctionDecl *FD = dyn_cast(D)) {
Index: clang/include/clang/Basic/DiagnosticCommentKinds.td
===
--- clang/include/clang/Basic/DiagnosticCommentKinds.td
+++ clang/include/clang/Basic/DiagnosticCommentKinds.td
@@ -146,8 +146,8 @@
 // \deprecated command
 
 def warn_doc_deprecated_not_sync : Warning<
-  "declaration is marked with '\\deprecated' command but does not have "
-  "a deprecation attribute">,
+  "declaration is marked with '%select{\\|@}0deprecated' command but does "
+  "not have a deprecation attribute">,
   InGroup, DefaultIgnore;
 
 def note_add_deprecation_attr : Note<
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70804: [Frontend] Allow OpenMP offloading to aarch64

[Bryan Chan via Phabricator via cfe-commits]

bryanpkc marked 6 inline comments as done.
bryanpkc added inline comments.



Comment at: clang/test/OpenMP/openmp_offload_registration.cpp:3-5
+// RUN: %clang_cc1 -verify -fopenmp -x c -triple x86_64-unknown-linux-gnu 
-fopenmp-targets=aarch64-unknown-linux-gnu,powerpc64-ibm-linux-gnu -emit-llvm 
%s -o - | FileCheck %s
+// RUN: %clang_cc1 -verify -fopenmp -x c -triple x86_64-unknown-linux-gnu 
-fopenmp-targets=i686-pclinux-gnu,powerpc-ibm-linux-gnu -emit-llvm %s -o - | 
FileCheck %s
+// RUN: %clang_cc1 -verify -fopenmp -x c -triple x86_64-unknown-linux-gnu 
-fopenmp-targets=nvptx-nvidia-cuda,nvptx64-nvidia-cuda -emit-llvm %s -o - | 
FileCheck %s

ABataev wrote:
> Do not add tests targets other than aarch64
OK, thank you.



Comment at: clang/test/OpenMP/target_messages.cpp:4
+
+// RUN: not %clang_cc1 -fopenmp -std=c++11 -fopenmp-targets=aaa-bbb-ccc-ddd -o 
- %s 2>&1 | FileCheck %s
 // CHECK: error: OpenMP target is invalid: 'aaa-bbb-ccc-ddd'

ABataev wrote:
> bryanpkc wrote:
> > ABataev wrote:
> > > Why do you need the changes in this file?
> > The clean-up is not strictly necessary. I came across this file when I was 
> > looking for the right file to add the tests, and thought it would be better 
> > to group similar tests (and their corresponding CHECK patterns) together.
> Do not do this in this patch, these changes are not related to the patch 
> itself.
OK, I will remove these changes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70804



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

[PATCH] D71091: Make sure that the implicit arguments for direct methods have been setup

[Pierre Habouzit via Phabricator via cfe-commits]

MadCoder added a comment.

In D71091#1773066 , @liuliu wrote:

> With this latest fix applied on top of 
> b220662a45c8067a2ae485ae34c1138d93506df9 
> , in our 
> company's internal code, I still encounter the crash.
>
>   Showing All Messages
>   /Users/liuliu/Snapchat/Dev/phantom/3.   
> Libraries/Storage/DocObject/Implementations/SCSQLiteDocObjectContext/Tests/Generated/SCTestMainEntityChangeRequest.mm:338:25:
>  LLVM IR generation of compound statement ('{}')
>   /Users/liuliu/Snapchat/Dev/phantom/0  clang-10 
> 0x00010b9edf0c llvm::sys::PrintStackTrace(llvm::raw_ostream&) + 60
>   1  clang-10 0x00010b9ee4c9 
> PrintStackTraceSignalHandler(void*) + 25
>   /Users/liuliu/Snapchat/Dev/phantom/2  clang-10 
> 0x00010b9ec086 llvm::sys::RunSignalHandlers() + 118
>   3  clang-10 0x00010b9f1e6c SignalHandler(int) + 252
>   4  libsystem_platform.dylib 0x7fff65d8eb5d _sigtramp + 29
>   5  libsystem_platform.dylib 0x7ffee7e9b790 _sigtramp + 
> 18446744071596723280
>   /Users/liuliu/Snapchat/Dev/phantom/6  clang-10 
> 0x00010bfdff14 
> clang::CodeGen::CodeGenTypes::arrangeObjCMethodDeclaration(clang::ObjCMethodDecl
>  const*) + 52
>   /Users/liuliu/Snapchat/Dev/phantom/7  clang-10 
> 0x00010c1f6bcd (anonymous 
> namespace)::CGObjCCommonMac::GenerateDirectMethod(clang::ObjCMethodDecl 
> const*, clang::ObjCContainerDecl const*) + 285
>   /Users/liuliu/Snapchat/Dev/phantom/8  clang-10 
> 0x00010c1f63e9 (anonymous 
> namespace)::CGObjCCommonMac::EmitMessageSend(clang::CodeGen::CodeGenFunction&,
>  clang::CodeGen::ReturnValueSlot, clang::QualType, clang::Selector, 
> llvm::Value*, clang::QualType, bool, clang::CodeGen::CallArgList const&, 
> clang::ObjCMethodDecl const*, clang::ObjCInterfaceDecl const*, (anonymous 
> namespace)::ObjCCommonTypesHelper const&) + 1401
>   /Users/liuliu/Snapchat/Dev/phantom/9  clang-10 
> 0x00010c205b14 (anonymous 
> namespace)::CGObjCNonFragileABIMac::GenerateMessageSend(clang::CodeGen::CodeGenFunction&,
>  clang::CodeGen::ReturnValueSlot, clang::QualType, clang::Selector, 
> llvm::Value*, clang::CodeGen::CallArgList const&, clang::ObjCInterfaceDecl 
> const*, clang::ObjCMethodDecl const*) + 644
>
>
> It indeed won't crash any more in trivial examples. I need to have some other 
> time to reduce my local example to a reasonable size.


This seems like the problem I just fixed, did you use the latest patch? because 
the problem was that we didn't bother to emit the implicit arguments for just 
prototypes before as it was only used by CodeGen of the implementations. This 
patch is supposed to add it to all declarations now, so I fail to understand 
how it can be missing anywhere ?


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

https://reviews.llvm.org/D71091



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

[PATCH] D71091: Make sure that the implicit arguments for direct methods have been setup

[Alex Lorenz via Phabricator via cfe-commits]

arphaman requested changes to this revision.
arphaman added a comment.
This revision now requires changes to proceed.

@MadCoder unfortunately this change causes several test failures when 
`check-clang` runs:

  Clang :: Analysis/analyzeOneFunction.m
  Clang :: Index/c-index-api-loadTU-test.m
  Clang :: SemaObjC/dealloc.m

Please fix them before committing this.


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

https://reviews.llvm.org/D71091



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

[clang] dbd1129 - Stop checking whether std::strong_* has ::equivalent members.

[Richard Smith via cfe-commits]

Author: Richard Smith
Date: 2019-12-06T11:35:41-08:00
New Revision: dbd112972416f48f7e5b117e7a14b6e4b4d38146

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

LOG: Stop checking whether std::strong_* has ::equivalent members.

Any attempt to use these would be a bug, so we shouldn't even look for
them.

Added: 


Modified: 
clang/lib/AST/ComparisonCategories.cpp
clang/test/SemaCXX/std-compare-cxx2a.cpp

Removed: 




diff  --git a/clang/lib/AST/ComparisonCategories.cpp 
b/clang/lib/AST/ComparisonCategories.cpp
index ee4c1b0443a3..813b728b 100644
--- a/clang/lib/AST/ComparisonCategories.cpp
+++ b/clang/lib/AST/ComparisonCategories.cpp
@@ -190,19 +190,16 @@ 
ComparisonCategories::getPossibleResultsForType(ComparisonCategoryType Type) {
   using CCT = ComparisonCategoryType;
   using CCR = ComparisonCategoryResult;
   std::vector Values;
-  Values.reserve(6);
-  Values.push_back(CCR::Equivalent);
+  Values.reserve(4);
   bool IsStrong = (Type == CCT::StrongEquality || Type == CCT::StrongOrdering);
   if (IsStrong)
-Values.push_back(CCR::Equal);
+  Values.push_back(IsStrong ? CCR::Equal : CCR::Equivalent);
   if (Type == CCT::StrongOrdering || Type == CCT::WeakOrdering ||
   Type == CCT::PartialOrdering) {
 Values.push_back(CCR::Less);
 Values.push_back(CCR::Greater);
   } else {
-Values.push_back(CCR::Nonequivalent);
-if (IsStrong)
-  Values.push_back(CCR::Nonequal);
+Values.push_back(IsStrong ? CCR::Nonequal : CCR::Nonequivalent);
   }
   if (Type == CCT::PartialOrdering)
 Values.push_back(CCR::Unordered);

diff  --git a/clang/test/SemaCXX/std-compare-cxx2a.cpp 
b/clang/test/SemaCXX/std-compare-cxx2a.cpp
index 6746fb480e62..941c4faeb7ff 100644
--- a/clang/test/SemaCXX/std-compare-cxx2a.cpp
+++ b/clang/test/SemaCXX/std-compare-cxx2a.cpp
@@ -27,7 +27,7 @@ struct partial_ordering {
 } // namespace std
 
 auto missing_member_test() {
-  // expected-error@+1 {{standard library implementation of 
'std::partial_ordering' is not supported; member 'equivalent' is missing}}
+  // expected-error@+1 {{standard library implementation of 
'std::partial_ordering' is not supported; member 'less' is missing}}
   return (1.0 <=> 1.0);
 }
 
@@ -35,13 +35,13 @@ namespace std {
 inline namespace __1 {
 struct strong_ordering {
   long long value;
-  static const strong_ordering equivalent; // expected-note {{declared here}}
+  static const strong_ordering equal; // expected-note {{declared here}}
 };
 } // namespace __1
 } // namespace std
 
 auto test_non_constexpr_var() {
-  // expected-error@+1 {{standard library implementation of 
'std::strong_ordering' is not supported; member 'equivalent' does not have 
expected form}}
+  // expected-error@+1 {{standard library implementation of 
'std::strong_ordering' is not supported; member 'equal' does not have expected 
form}}
   return (1 <=> 0);
 }
 



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

[PATCH] D49091: Warn about usage of __has_include/__has_include_next in macro expansions

[Tor Arne Vestbø via Phabricator via cfe-commits]

torarnv added a comment.

I'm picking this up on the Qt side and removing our wrapper macro. Does this 
apply to other cases as well? The standard mentions `_­_­has_­cpp_­attribute`, 
but what about `__has_builtin`, `__has_attribute`,  and `__has_include_next`?


Repository:
  rC Clang

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

https://reviews.llvm.org/D49091



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

[PATCH] D70157: Align branches within 32-Byte boundary

[Philip Reames via Phabricator via cfe-commits]

reames added a comment.

Recording something so I don't forget it when we get back to the prefix padding 
version.  The write up on the bundle align mode stuff mentions a concerning 
memory overhead for the feature.  Since the basic implementation techniques are 
similar, we need to make sure we assess the memory overhead of the prefix 
padding implementation.  See 
https://www.chromium.org/nativeclient/pnacl/aligned-bundling-support-in-llvm 
for context.  I don't believe this is likely to be an issue for the nop padding 
variant.


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

https://reviews.llvm.org/D70157



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

[PATCH] D70849: [AST] Traverse the class type loc inside the member pointer type loc.

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

We also hit this, and I have a repro here, but it depends on the Chromium Blink 
GC plugin:
https://bugs.chromium.org/p/chromium/issues/detail?id=1031274#c4
I think you just need an RAV client that walks over some particular AST node to 
trigger the assert. I assume there is nothing special about the Blink GC plugin 
here.

Thanks again Sterling for the revert, more bots are green this morning as a 
result.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70849



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

[PATCH] D71134: [OpenMP] Require trivially copyable type for mapping

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:14917
   if (FullCheck && !SemaRef.CurContext->isDependentContext() &&
-  !QTy.isTrivialType(SemaRef.Context))
+  !QTy.isTriviallyCopyableType(SemaRef.Context))
 SemaRef.Diag(SL, diag::warn_omp_non_trivial_type_mapped) << QTy << SR;

Need to add a check for defaulted destructor here, `isTriviallyCopyableType` 
does not include this check.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71134



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

[PATCH] D71080: [NFC] Separate getLastArgIntValue to Basic

[Artem Belevich via Phabricator via cfe-commits]

tra added inline comments.



Comment at: clang/include/clang/Basic/OptionUtils.h:24
+
+class ArgList;
+

What are the rules on using LLVM headers here? Can we just include 
llvm/Option/ArgList.h instead?



Comment at: clang/include/clang/Basic/OptionUtils.h:46
+   DiagnosticsEngine *Diags = nullptr,
+   unsigned Base = 10);
+

Same question as before -- does it have to be `10`?
`0` would be a more reasonable default for general use. IMO we care about the 
value, but not so much about the form. I.e. is there a reason not to allow 0xf, 
for instance, if that works for the user?




Comment at: clang/lib/Basic/CMakeLists.txt:1
 set(LLVM_LINK_COMPONENTS
   Core

I think now that you're using ArgList, you need to depend on LLVM's LLVMOption 
library.
As is you're likely to run into build issues if shared libraries are enabled.


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

https://reviews.llvm.org/D71080



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

[PATCH] D71134: [OpenMP] Require trivially copyable type for mapping

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld created this revision.
Hahnfeld added reviewers: ABataev, jdoerfert.
Herald added subscribers: cfe-commits, guansong.
Herald added a project: clang.

A trivially copyable type provides a trivial copy constructor and a trivial
copy assignment operator. This is enough for the runtime to memcpy the data
to the device. Additionally there must be no virtual functions or virtual
base classes and the destructor is guaranteed to be trivial, ie performs
no action.
The runtime does not require trivial default constructors because on alloc
the memory is undefined. Thus, weaken the warning to be only issued if the
mapped type is not trivially copyable.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71134

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/distribute_firstprivate_messages.cpp
  clang/test/OpenMP/distribute_parallel_for_firstprivate_messages.cpp
  clang/test/OpenMP/distribute_parallel_for_lastprivate_messages.cpp
  clang/test/OpenMP/distribute_parallel_for_private_messages.cpp
  clang/test/OpenMP/distribute_parallel_for_reduction_messages.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_aligned_messages.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_private_messages.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_shared_messages.cpp
  clang/test/OpenMP/distribute_simd_aligned_messages.cpp
  clang/test/OpenMP/distribute_simd_firstprivate_messages.cpp
  clang/test/OpenMP/distribute_simd_lastprivate_messages.cpp
  clang/test/OpenMP/distribute_simd_linear_messages.cpp
  clang/test/OpenMP/distribute_simd_loop_messages.cpp
  clang/test/OpenMP/distribute_simd_private_messages.cpp
  clang/test/OpenMP/distribute_simd_reduction_messages.cpp
  clang/test/OpenMP/target_firstprivate_messages.cpp
  clang/test/OpenMP/target_parallel_for_private_messages.cpp
  clang/test/OpenMP/target_parallel_for_simd_private_messages.cpp
  clang/test/OpenMP/target_private_messages.cpp
  clang/test/OpenMP/target_simd_private_messages.cpp
  clang/test/OpenMP/target_teams_distribute_firstprivate_messages.cpp
  clang/test/OpenMP/teams_distribute_loop_messages.cpp
  clang/test/OpenMP/teams_distribute_parallel_for_loop_messages.cpp
  clang/test/OpenMP/teams_distribute_parallel_for_simd_aligned_messages.cpp
  clang/test/OpenMP/teams_distribute_parallel_for_simd_linear_messages.cpp
  clang/test/OpenMP/teams_distribute_parallel_for_simd_loop_messages.cpp
  clang/test/OpenMP/teams_distribute_simd_aligned_messages.cpp
  clang/test/OpenMP/teams_distribute_simd_linear_messages.cpp
  clang/test/OpenMP/teams_distribute_simd_loop_messages.cpp

Index: clang/test/OpenMP/teams_distribute_simd_loop_messages.cpp
===
--- clang/test/OpenMP/teams_distribute_simd_loop_messages.cpp
+++ clang/test/OpenMP/teams_distribute_simd_loop_messages.cpp
@@ -416,7 +416,7 @@
   Iter0 begin0, end0;
 #pragma omp target
 #pragma omp teams distribute simd
-  for (GoodIter I = begin; I < end; ++I) // expected-warning 2 {{Non-trivial type 'GoodIter' is mapped, only trivial types are guaranteed to be mapped correctly}}
+  for (GoodIter I = begin; I < end; ++I) // expected-warning 2 {{Type 'GoodIter' is not trivially copyable and not guaranteed to be mapped correctly}}
 ++I;
 #pragma omp target
 #pragma omp teams distribute simd
@@ -425,31 +425,31 @@
 ++I;
 #pragma omp target
 #pragma omp teams distribute simd
-  for (GoodIter I = begin; I >= end; --I) // expected-warning 2 {{Non-trivial type 'GoodIter' is mapped, only trivial types are guaranteed to be mapped correctly}}
+  for (GoodIter I = begin; I >= end; --I) // expected-warning 2 {{Type 'GoodIter' is not trivially copyable and not guaranteed to be mapped correctly}}
 ++I;
 #pragma omp target
 #pragma omp teams distribute simd
 // expected-warning@+1 {{initialization clause of OpenMP for loop is not in canonical form ('var = init' or 'T var = init')}}
-  for (GoodIter I(begin); I < end; ++I) // expected-warning 2 {{Non-trivial type 'GoodIter' is mapped, only trivial types are guaranteed to be mapped correctly}}
+  for (GoodIter I(begin); I < end; ++I) // expected-warning 2 {{Type 'GoodIter' is not trivially copyable and not guaranteed to be mapped correctly}}
 ++I;
 #pragma omp target
 #pragma omp teams distribute simd
 // expected-warning@+1 {{initialization clause of OpenMP for loop is not in canonical form ('var = init' or 'T var = init')}}
-  for (GoodIter I(nullptr); I < end; ++I) // expected-warning {{Non-trivial type 'GoodIter' is mapped, only trivial types are guaranteed to be mapped correctly}}
+  for (GoodIter I(nullptr); I < end; ++I) // expected-warning {{Type 'GoodIter' is not trivially copyable and not guaranteed to be mapped correctly}}
 ++I;
 #pragma omp target
 #pragma omp teams distribute simd
 // expected-warning@+1 {{initialization clause of OpenMP for loop is not in canonical form ('var = init' or 'T var = init')}}
-  

[PATCH] D71133: [OpenCL] Add ExtVectorElementExpr constant evaluation (PR42387)

[Sven van Haastregt via Phabricator via cfe-commits]

svenvh created this revision.
svenvh added a reviewer: Anastasia.
Herald added a subscriber: yaxunl.

Add constexpr evaluation for ExtVectorElementExpr nodes by evaluating
the underlying vector expression.

Add basic folding too, for the case that Evaluate does not return an
LValue.


https://reviews.llvm.org/D71133

Files:
  clang/lib/AST/ExprConstant.cpp
  clang/test/CodeGenOpenCLCXX/constexpr.cl


Index: clang/test/CodeGenOpenCLCXX/constexpr.cl
===
--- clang/test/CodeGenOpenCLCXX/constexpr.cl
+++ clang/test/CodeGenOpenCLCXX/constexpr.cl
@@ -1,5 +1,8 @@
 // RUN: %clang_cc1 %s -triple spir-unknown-unknown -cl-std=clc++ -O0 
-emit-llvm -o - | FileCheck %s
 
+typedef int int2 __attribute__((ext_vector_type(2)));
+typedef int int4 __attribute__((ext_vector_type(4)));
+
 struct Storage final {
   constexpr const float& operator[](const int index) const noexcept {
 return InternalStorage[index];
@@ -24,3 +27,28 @@
 kernel void foo(global float *x) {
   *x = FloatConstant;
 }
+
+// Test evaluation of constant vectors.
+// CHECK-LABEL: define spir_kernel void @vecEval
+// CHECK: store i32 3
+// CHECK: store <2 x i32> , <2 x i32>
+
+const int oneElt = int4(3).x;
+const int2 twoElts = (int4)(11, 22, 33, 44).yz;
+
+kernel void vecEval(global int *x, global int2 *y) {
+  *x = oneElt;
+  *y = twoElts;
+}
+
+// Test evaluation of vectors initialized through a constexpr function.
+// CHECK-LABEL: define spir_kernel void @vecEval2
+// CHECK: store <2 x i32>
+constexpr int2 addOne(int2 x) {
+  return (int2)(x.x + 1, x.y + 1);
+}
+const int2 fromConstexprFunc = addOne(int2(2));
+
+kernel void vecEval2(global int2 *x) {
+  *x = fromConstexprFunc;
+}
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -7049,6 +7049,31 @@
DerivedSuccess(Result, E);
   }
 
+  bool VisitExtVectorElementExpr(const ExtVectorElementExpr *E) {
+APValue Val;
+if (!Evaluate(Val, Info, E->getBase()))
+  return false;
+
+if (Val.isVector()) {
+  SmallVector Indices;
+  E->getEncodedElementAccess(Indices);
+  if (Indices.size() == 1) {
+// Return scalar.
+return DerivedSuccess(Val.getVectorElt(Indices[0]), E);
+  } else {
+// Construct new APValue vector.
+SmallVector Elts;
+for (unsigned I = 0; I < Indices.size(); ++I) {
+  Elts.push_back(Val.getVectorElt(Indices[I]));
+}
+APValue VecResult(Elts.data(), Indices.size());
+return DerivedSuccess(VecResult, E);
+  }
+}
+
+return false;
+  }
+
   bool VisitCastExpr(const CastExpr *E) {
 switch (E->getCastKind()) {
 default:


Index: clang/test/CodeGenOpenCLCXX/constexpr.cl
===
--- clang/test/CodeGenOpenCLCXX/constexpr.cl
+++ clang/test/CodeGenOpenCLCXX/constexpr.cl
@@ -1,5 +1,8 @@
 // RUN: %clang_cc1 %s -triple spir-unknown-unknown -cl-std=clc++ -O0 -emit-llvm -o - | FileCheck %s
 
+typedef int int2 __attribute__((ext_vector_type(2)));
+typedef int int4 __attribute__((ext_vector_type(4)));
+
 struct Storage final {
   constexpr const float& operator[](const int index) const noexcept {
 return InternalStorage[index];
@@ -24,3 +27,28 @@
 kernel void foo(global float *x) {
   *x = FloatConstant;
 }
+
+// Test evaluation of constant vectors.
+// CHECK-LABEL: define spir_kernel void @vecEval
+// CHECK: store i32 3
+// CHECK: store <2 x i32> , <2 x i32>
+
+const int oneElt = int4(3).x;
+const int2 twoElts = (int4)(11, 22, 33, 44).yz;
+
+kernel void vecEval(global int *x, global int2 *y) {
+  *x = oneElt;
+  *y = twoElts;
+}
+
+// Test evaluation of vectors initialized through a constexpr function.
+// CHECK-LABEL: define spir_kernel void @vecEval2
+// CHECK: store <2 x i32>
+constexpr int2 addOne(int2 x) {
+  return (int2)(x.x + 1, x.y + 1);
+}
+const int2 fromConstexprFunc = addOne(int2(2));
+
+kernel void vecEval2(global int2 *x) {
+  *x = fromConstexprFunc;
+}
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -7049,6 +7049,31 @@
DerivedSuccess(Result, E);
   }
 
+  bool VisitExtVectorElementExpr(const ExtVectorElementExpr *E) {
+APValue Val;
+if (!Evaluate(Val, Info, E->getBase()))
+  return false;
+
+if (Val.isVector()) {
+  SmallVector Indices;
+  E->getEncodedElementAccess(Indices);
+  if (Indices.size() == 1) {
+// Return scalar.
+return DerivedSuccess(Val.getVectorElt(Indices[0]), E);
+  } else {
+// Construct new APValue vector.
+SmallVector Elts;
+for (unsigned I = 0; I < Indices.size(); ++I) {
+  Elts.push_back(Val.getVectorElt(Indices[I]));
+}
+APValue VecResult(Elts.data(), 

[PATCH] D70424: clang/AMDGPU: Fix default for frame-pointer attribute

[Matt Arsenault via Phabricator via cfe-commits]

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

2cc11941a2e88236e0b4842229454ae6d85142cd 



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

https://reviews.llvm.org/D70424



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

[clang] 2cc1194 - clang/AMDGPU: Fix default for frame-pointer attribute

[Matt Arsenault via cfe-commits]

Author: Matt Arsenault
Date: 2019-12-07T00:09:10+05:30
New Revision: 2cc11941a2e88236e0b4842229454ae6d85142cd

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

LOG: clang/AMDGPU: Fix default for frame-pointer attribute

Enabling optimization should allow frame pointer elimination.

Added: 
clang/test/Driver/frame-pointer-elim.cl

Modified: 
clang/lib/Driver/ToolChains/Clang.cpp

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index 5ebf36f13ce9..02a365fa4969 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -538,6 +538,8 @@ static bool useFramePointerForTargetByDefault(const ArgList 
,
   case llvm::Triple::ppc64le:
   case llvm::Triple::riscv32:
   case llvm::Triple::riscv64:
+  case llvm::Triple::amdgcn:
+  case llvm::Triple::r600:
 return !areOptimizationsEnabled(Args);
   default:
 break;

diff  --git a/clang/test/Driver/frame-pointer-elim.cl 
b/clang/test/Driver/frame-pointer-elim.cl
new file mode 100644
index ..c469d10a64be
--- /dev/null
+++ b/clang/test/Driver/frame-pointer-elim.cl
@@ -0,0 +1,8 @@
+// RUN: %clang -target amdgcn-amd-amdhsa -### -S -O3 %s -o %t.s 2>&1 | 
FileCheck -check-prefix=CHECKNONE %s
+// RUN: %clang -target amdgcn-amd-amdhsa -### -S -O3 -fno-omit-frame-pointer 
%s -o %t.s 2>&1 | FileCheck -check-prefix=CHECKALL %s
+// RUN: %clang -target amdgcn-amd-amdhsa -### -S %s -o %t.s 2>&1 | FileCheck 
-check-prefix=CHECKALL %s
+// RUN: %clang -target amdgcn-amd-amdhsa -### -S -O0 %s -o %t.s 2>&1 | 
FileCheck -check-prefix=CHECKALL %s
+// RUN: %clang -target amdgcn-amd-amdhsa -### -S -cl-opt-disable %s -o %t.s 
2>&1 | FileCheck -check-prefix=CHECKALL %s
+
+// CHECKNONE: -mframe-pointer=none
+// CHECKALL: -mframe-pointer=all



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