date:20200903

[PATCH] D85473: [Clang] Add option to allow marking pass-by-value args as noalias.

2020-09-03 Thread John McCall via Phabricator via cfe-commits

rjmccall added a comment.

Is it acceptable to leave this as a -cc1 option while we're pursuing this with 
the language committee?  Do we have any intent to pursue this with the language 
committee?




Comment at: clang/include/clang/Basic/LangOptions.def:372
+LANGOPT(PassByValueIsNoAlias, 1, 0, "assumption that by-value parameters do "
+"not alias any other values")
 

This should be a code-gen option, I think.  I can't really imagine how this 
would affect language processing.



Comment at: clang/include/clang/Driver/Options.td:4287-4290
+def fpass_by_value_noalias: Flag<["-"], "fpass-by-value-noalias">,
+  HelpText<"Allows assuming no references to passed by value escape before "
+   "transferring execution to the called function. Note that this "
+   "does not hold for C++">;

rsmith wrote:
> This should be in `Group`.
The "Note" clause seems to muddy more than it clarifies.  Maybe "has no effect 
on non-trivially-copyable classes in C++"?  Or add proper documentation 
somewhere instead of trying to jam this into the help text.



Comment at: clang/lib/CodeGen/CGCall.cpp:2198
+// reference to the underlying object. Mark it accordingly.
+Attrs.addAttribute(llvm::Attribute::NoAlias);
+

This definitely can't be added unconditionally to all types; you need to rule 
out non-trivial C++ class types, as well as types with ObjC weak references.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85473

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

[PATCH] D86921: [FPEnv] Partially implement #pragma STDC FENV_ROUND

2020-09-03 Thread John McCall via Phabricator via cfe-commits

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86921

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

[PATCH] D85596: [Docs] Fix --print-supported-cpus option rendering

2020-09-03 Thread Travis Finkenauer via Phabricator via cfe-commits

tmfink accepted this revision.
tmfink added a comment.

@nickdesaulniers, could you merge my patch? I don't have commit access.
Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85596

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

[PATCH] D80344: [Windows SEH]: HARDWARE EXCEPTION HANDLING (MSVC -EHa) - Part 1

2020-09-03 Thread John McCall via Phabricator via cfe-commits

rjmccall added a comment.

I've made some brief comments about the code, but I think I have much larger 
concerns here.

The first is whether LLVM really intends to satisfy the constraints necessary 
to make these exceptions work, which I don't think you've gotten clear 
consensus about at all.  Unfortunately, llvm-dev is not an effective place to 
gather this kind of consensus.  For a change of this magnitude, I think you 
need to be proactively gathering a group of interested parties to sort out 
whether this is something we should support, not just expecting people to reply 
to an RFC.

The second is that I think the concept of block-level tracking state / control 
flow is potentially really problematic for LLVM, and the entire design here 
seems contingent on it.  Again, this is something we need to ensure consensus 
on, not something we can really just sign off on in code review.

The third is that I'm not really convinced that the way we handle cleanups in 
Clang today is sufficiently "atomic" in design to accommodate this.  I need to 
think about it.




Comment at: clang/include/clang/Basic/LangOptions.def:131
 LANGOPT(CXXExceptions , 1, 0, "C++ exceptions")
+LANGOPT(EHAsynch  , 1, 0, "C/C++ EH Asynch exceptions")
 LANGOPT(DWARFExceptions   , 1, 0, "dwarf exception handling")

This is usually shortened as "async", but I'm not sure why it's being shortened 
at all.  Also, hardware faults are synchronous.  Also, it applies in all 
language modes.



Comment at: clang/lib/CodeGen/CGCleanup.cpp:769
+// Under -EHa, invoke eha_scope_end() to mark scope end before dtor
+bool IsEHa = getLangOpts().EHAsynch && !Scope.isLifetimeMarker();
+const EHPersonality  = EHPersonality::get(*this);

Readers of this code shouldn't have to remember what cl.exe command-line flags 
mean.  Please call this something like `RequiresSEHScopeEnd`.



Comment at: clang/lib/CodeGen/CGCleanup.cpp:783
+  EmitSehTryScopeEnd();
+}
 

Please put braces around the outer `if`, and please be consistent about braces 
for the inner clauses.



Comment at: clang/lib/CodeGen/CGCleanup.cpp:1305
+static void EmitSehEHaScope(CodeGenFunction ,
+llvm::FunctionCallee ) {
+  llvm::BasicBlock *InvokeDest = CGF.getInvokeDest();

I think you should take the callee by value.  And please rename this to not use 
"EHa".



Comment at: clang/lib/CodeGen/CGDecl.cpp:2008
+  EmitSehCppScopeBegin();
+  }
+

This seems like a pretty random place to do this.  Presumably this need to be 
tied to basically any scope with EH protection?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80344

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

[PATCH] D86049: RFC: Implement optional exportable wrapper function generation for objc_direct methods.

2020-09-03 Thread John McCall via Phabricator via cfe-commits

rjmccall added a comment.

We just talk about it.  I agree with Nathan that we shouldn't just add this as 
a short-term hack; we should design the ABI right and then do what we want.

I think these are basically all the ABI questions:

- Is there a good reason to preserve signature compatibility with normal ObjC 
methods, or should we just drop the selector argument on direct methods?
- Should we do the null test on the caller side or the callee side?
- In direct class methods, is the caller's responsibility or the callee's to 
ensure that `self` is initialized?

For the first, I think dropping the argument is fine.

For the second, I'm less certain.

For the third, making it the callee's responsibility is generally better for 
code size but harder to optimize.  But we only have the code-size cost on the 
caller side when materializing from a global, and that's actually something we 
can also eliminate as redundant.  So maybe the best approach is to introduce a 
weak+hidden thunk that we use when making a call to a direct class method on a 
specific class, and we have that thunk do the entire materialization sequence.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86049

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

[PATCH] D86841: [clang] Add noprogress attribute deduction for infinite loops

2020-09-03 Thread Atmn Patel via Phabricator via cfe-commits

atmnpatel updated this revision to Diff 289849.
atmnpatel added a comment.

In Summary:

- I changed the llvm loop metadata name to `mustprogress`, to indicate that 
loops with this attribute are required to have side-effects.
- The `mayprogress` function attribute is applied to functions where an 
infinite loop is found with a constant conditional.
- The `mustprogress` attribute is applied to loops within a `mayprogress` 
function that do not have a constant conditional.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86841

Files:
  clang/lib/CodeGen/CGLoopInfo.cpp
  clang/lib/CodeGen/CGLoopInfo.h
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/test/CodeGen/attr-noprogress.c
  clang/test/CodeGen/attr-noprogress.cpp
  clang/test/CodeGenCXX/fno-unroll-loops-metadata.cpp

Index: clang/test/CodeGenCXX/fno-unroll-loops-metadata.cpp
===
--- clang/test/CodeGenCXX/fno-unroll-loops-metadata.cpp
+++ clang/test/CodeGenCXX/fno-unroll-loops-metadata.cpp
@@ -4,7 +4,7 @@
 // RUN: %clang_cc1 -triple x86_64-apple-darwin -std=c++11 -emit-llvm -o - %s -O3 -disable-llvm-optzns -fno-unroll-loops | FileCheck --check-prefix=UNROLL_DISABLED_MD %s
 // RUN: %clang_cc1 -triple x86_64-apple-darwin -std=c++11 -emit-llvm -o - %s -O3 -disable-llvm-optzns | FileCheck --check-prefix=NO_UNROLL_MD %s
 
-// NO_UNROLL_MD-NOT: llvm.loop
+// NO_UNROLL_MD-NOT: llvm.loop.unroll.disable
 
 // Verify unroll.disable metadata is added to while loop with -fno-unroll-loops
 // and optlevel > 0.
Index: clang/test/CodeGen/attr-noprogress.cpp
===
--- /dev/null
+++ clang/test/CodeGen/attr-noprogress.cpp
@@ -0,0 +1,188 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py UTC_ARGS: --check-attributes
+// RUN: %clang_cc1 -S -emit-llvm %s -o - | FileCheck %s
+
+int a = 0;
+int b = 0;
+
+// CHECK: Function Attrs: noinline nounwind optnone mayprogress
+// CHECK-LABEL: @_Z2f1v(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:br label [[FOR_COND:%.*]]
+// CHECK:   for.cond:
+// CHECK: br i1 true, label [[FOR_BODY:%.*]], label [[FOR_END:%.*]]
+// CHECK:   for.body:
+// CHECK: br label [[FOR_COND]]
+// CHECK:   for.end:
+// CHECK: ret void
+//
+void f1() {
+  for (; 1;) {
+  }
+}
+
+// CHECK: Function Attrs: noinline nounwind optnone
+// CHECK-LABEL: @_Z2f2v(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:br label [[FOR_COND:%.*]]
+// CHECK:   for.cond:
+// CHECK: [[TMP0:%.*]] = load i32, i32* @a, align 4
+// CHECK: [[TMP1:%.*]] = load i32, i32* @b, align 4
+// CHECK: [[CMP:%.*]] = icmp eq i32 [[TMP0]], [[TMP1]]
+// CHECK: br i1 [[CMP]], label [[FOR_BODY:%.*]], label [[FOR_END:%.*]]
+// CHECK:   for.body:
+// CHECK: br label [[FOR_COND]]
+// CHECK:   for.end:
+// CHECK: ret void
+//
+void f2() {
+  for (; a == b;) {
+  }
+}
+
+// CHECK: Function Attrs: noinline nounwind optnone mayprogress
+// CHECK-LABEL: @_Z1Fv(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:br label [[FOR_COND:%.*]]
+// CHECK:   for.cond:
+// CHECK: br i1 true, label [[FOR_BODY:%.*]], label [[FOR_END:%.*]]
+// CHECK:   for.body:
+// CHECK: br label [[FOR_COND]]
+// CHECK:   for.end:
+// CHECK: br label [[FOR_COND1:%.*]]
+// CHECK:   for.cond1:
+// CHECK: [[TMP0:%.*]] = load i32, i32* @a, align 4
+// CHECK: [[TMP1:%.*]] = load i32, i32* @b, align 4
+// CHECK: [[CMP:%.*]] = icmp eq i32 [[TMP0]], [[TMP1]]
+// CHECK: br i1 [[CMP]], label [[FOR_BODY2:%.*]], label [[FOR_END3:%.*]]
+// CHECK:   for.body2:
+// CHECK: br label [[FOR_COND1]], !llvm.loop [[LOOP2:!.*]]
+// CHECK:   for.end3:
+// CHECK: ret void
+//
+void F() {
+  for (; 1;) {
+  }
+  for (; a == b;) {
+  }
+}
+
+// CHECK: Function Attrs: noinline nounwind optnone mayprogress
+// CHECK-LABEL: @_Z2w1v(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:br label [[WHILE_BODY:%.*]]
+// CHECK:   while.body:
+// CHECK: br label [[WHILE_BODY]]
+//
+void w1() {
+  while (1) {
+  }
+}
+
+// CHECK: Function Attrs: noinline nounwind optnone
+// CHECK-LABEL: @_Z2w2v(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:br label [[WHILE_COND:%.*]]
+// CHECK:   while.cond:
+// CHECK: [[TMP0:%.*]] = load i32, i32* @a, align 4
+// CHECK: [[TMP1:%.*]] = load i32, i32* @b, align 4
+// CHECK: [[CMP:%.*]] = icmp eq i32 [[TMP0]], [[TMP1]]
+// CHECK: br i1 [[CMP]], label [[WHILE_BODY:%.*]], label [[WHILE_END:%.*]]
+// CHECK:   while.body:
+// CHECK: br label [[WHILE_COND]]
+// CHECK:   while.end:
+// CHECK: ret void
+//
+void w2() {
+  while (a == b) {
+  }
+}
+
+// CHECK: Function Attrs: noinline nounwind optnone mayprogress

[PATCH] D86049: RFC: Implement optional exportable wrapper function generation for objc_direct methods.

2020-09-03 Thread Puyan Lotfi via Phabricator via cfe-commits

plotfi added a comment.

In D86049#2253705 , @plotfi wrote:

> I've updated the diff to reflect the alternate non-wrapper exposure. This way 
> requires the sanitizing of the exported objc_direct method name.

@rjmccall Is there a process for discussing ABI issues and considerations for 
something like this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86049

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

[PATCH] D84364: [CUDA][HIP] Defer overloading resolution diagnostics for host device functions

2020-09-03 Thread Yaxun Liu via Phabricator via cfe-commits

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

In D84364#2255572 , @tra wrote:

> LGTM.
>
> Nice!
>
> To sum it up --  the patch introduces  `-fgpu-defer-diag` flag which allows 
> deferring overload resolution diagnostics, if overload set included 
> candidates from both sides.
>
> We may be deferring cases when we don't have to (e.g. `df()->()callee2()` 
> should've errored out right away, even during host compilation, as there's no 
> way it could ever be valid), but this approach is a good starting point. It 
> affects only the interesting subset of diags, is not enabled by default, and 
> can be refined further, if necessary.

Thanks Artem. Would you please also review https://reviews.llvm.org/D84362 
since this patch depends on that. Thanks.




Comment at: clang/include/clang/Basic/LangOptions.def:244
 LANGOPT(GPUMaxThreadsPerBlock, 32, 256, "default max threads per block for 
kernel launch bounds for HIP")
+LANGOPT(GPUDeferDiag, 1, 0, "defer all semantic diagnostic messages in host 
device functions for CUDA/HIP")
 

tra wrote:
> The description does not seem to reflect reality. IIUIC only overload-related 
> diagnostic messages will be deferred.
will fix when committing


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

https://reviews.llvm.org/D84364

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

[PATCH] D87118: Add an explicit toggle for the static analyzer in clang-tidy

2020-09-03 Thread Nico Weber via Phabricator via cfe-commits

thakis created this revision.
thakis added a reviewer: hans.
Herald added subscribers: llvm-commits, ASDenysPetrov, dkrupp, donat.nagy, 
Szelethus, a.sidorin, baloghadamsoftware, mgorny.
Herald added a project: LLVM.
thakis requested review of this revision.

Instead of using CLANG_ENABLE_STATIC_ANALYZER for use of the
static analyzer in both clang and clang-tidy, add a second
toggle CLANG_TIDY_ENABLE_STATIC_ANALYZER.

This allows enabling the static analyzer in clang-tidy while
disabling it in clang.


https://reviews.llvm.org/D87118

Files:
  clang-tools-extra/CMakeLists.txt
  clang-tools-extra/clang-tidy/CMakeLists.txt
  clang-tools-extra/clang-tidy/ClangTidy.cpp
  clang-tools-extra/clang-tidy/ClangTidyForceLinker.h
  clang-tools-extra/clang-tidy/clang-tidy-config.h.cmake
  clang-tools-extra/docs/clang-tidy/Contributing.rst
  clang-tools-extra/test/CMakeLists.txt
  clang-tools-extra/test/lit.cfg.py
  clang-tools-extra/test/lit.site.cfg.py.in
  clang/CMakeLists.txt
  clang/cmake/caches/Android.cmake
  clang/lib/CMakeLists.txt
  llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/BUILD.gn
  llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/enable.gni
  llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/tool/BUILD.gn
  llvm/utils/gn/secondary/clang-tools-extra/test/BUILD.gn

Index: llvm/utils/gn/secondary/clang-tools-extra/test/BUILD.gn
===
--- llvm/utils/gn/secondary/clang-tools-extra/test/BUILD.gn
+++ llvm/utils/gn/secondary/clang-tools-extra/test/BUILD.gn
@@ -1,3 +1,4 @@
+import("//clang-tools-extra/clang-tidy/enable.gni")
 import("//clang/lib/StaticAnalyzer/Frontend/enable.gni")
 import("//clang/tools/libclang/include_clang_tools_extra.gni")
 import("//llvm/triples.gni")
@@ -38,10 +39,10 @@
 "Python3_EXECUTABLE=$python_path",
   ]
 
-  if (clang_enable_static_analyzer) {
-extra_values += [ "CLANG_ENABLE_STATIC_ANALYZER=1" ]
+  if (clang_tidy_enable_static_analyzer) {
+extra_values += [ "CLANG_TIDY_ENABLE_STATIC_ANALYZER=1" ]
   } else {
-extra_values += [ "CLANG_ENABLE_STATIC_ANALYZER=0" ]
+extra_values += [ "CLANG_TIDY_ENABLE_STATIC_ANALYZER=0" ]
   }
 
   if (libclang_include_clang_tools_extra) {
Index: llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/tool/BUILD.gn
===
--- llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/tool/BUILD.gn
+++ llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/tool/BUILD.gn
@@ -3,6 +3,7 @@
   deps = [
 "//clang-tools-extra/clang-tidy",
 "//clang-tools-extra/clang-tidy:all-checks",
+"//clang-tools-extra/clang-tidy:clang-tidy-config",
 "//clang/lib/AST",
 "//clang/lib/ASTMatchers",
 "//clang/lib/Basic",
Index: llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/enable.gni
===
--- /dev/null
+++ llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/enable.gni
@@ -0,0 +1,4 @@
+declare_args() {
+  # Whether to include the static analyzer in the clang-tidy binary.
+  clang_tidy_enable_static_analyzer = true
+}
Index: llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/BUILD.gn
===
--- llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/BUILD.gn
+++ llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/BUILD.gn
@@ -1,9 +1,32 @@
 import("//clang/lib/StaticAnalyzer/Frontend/enable.gni")
+import("//llvm/utils/gn/build/write_cmake_config.gni")
+import("enable.gni")
+
+config("clang-tidy-config_Config") {
+  visibility = [ ":clang-tidy-config" ]
+  include_dirs = [ "$target_gen_dir" ]
+}
+
+write_cmake_config("clang-tidy-config") {
+  input = "clang-tidy-config.h.cmake"
+  output = "$target_gen_dir/clang-tidy-config.h"
+  values = []
+
+  if (clang_tidy_enable_static_analyzer) {
+values += [ "CLANG_TIDY_ENABLE_STATIC_ANALYZER=1" ]
+  } else {
+values += [ "CLANG_TIDY_ENABLE_STATIC_ANALYZER=" ]
+  }
+
+  # Let targets depending on this find the generated file.
+  public_configs = [ ":clang-tidy-config_Config" ]
+}
 
 static_library("clang-tidy") {
   output_name = "clangTidy"
   configs += [ "//llvm/utils/gn/build:clang_code" ]
   deps = [
+":clang-tidy-config",
 "//clang/include/clang/StaticAnalyzer/Checkers",
 "//clang/lib/AST",
 "//clang/lib/ASTMatchers",
@@ -19,7 +42,7 @@
 "//llvm/lib/Support",
   ]
 
-  if (clang_enable_static_analyzer) {
+  if (clang_tidy_enable_static_analyzer) {
 deps += [
   "//clang/lib/StaticAnalyzer/Core",
   "//clang/lib/StaticAnalyzer/Frontend",
@@ -63,7 +86,7 @@
 "//clang-tools-extra/clang-tidy/readability",
 "//clang-tools-extra/clang-tidy/zircon",
   ]
-  if (clang_enable_static_analyzer) {
+  if (clang_tidy_enable_static_analyzer) {
 deps += [ "//clang-tools-extra/clang-tidy/mpi" ]
   }
 }
Index: clang/lib/CMakeLists.txt
===

[PATCH] D84604: Thread safety analysis: Consider global variables in scope

2020-09-03 Thread Aaron Puchert via Phabricator via cfe-commits

aaronpuchert updated this revision to Diff 289833.
aaronpuchert marked 2 inline comments as done.
aaronpuchert added a comment.

Add tests with qualified names, let tests rely on shadowing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84604

Files:
  clang/lib/Analysis/ThreadSafety.cpp
  clang/test/SemaCXX/warn-thread-safety-analysis.cpp
  clang/test/SemaCXX/warn-thread-safety-negative.cpp


Index: clang/test/SemaCXX/warn-thread-safety-negative.cpp
===
--- clang/test/SemaCXX/warn-thread-safety-negative.cpp
+++ clang/test/SemaCXX/warn-thread-safety-negative.cpp
@@ -81,6 +81,35 @@
 
 }  // end namespace SimpleTest
 
+Mutex globalMutex;
+
+namespace ScopeTest {
+
+void f() EXCLUSIVE_LOCKS_REQUIRED(!globalMutex);
+void fq() EXCLUSIVE_LOCKS_REQUIRED(!::globalMutex);
+
+namespace ns {
+  Mutex globalMutex;
+  void f() EXCLUSIVE_LOCKS_REQUIRED(!globalMutex);
+  void fq() EXCLUSIVE_LOCKS_REQUIRED(!ns::globalMutex);
+}
+
+void testGlobals() EXCLUSIVE_LOCKS_REQUIRED(!ns::globalMutex) {
+  f(); // expected-warning {{calling function 'f' requires negative 
capability '!globalMutex'}}
+  fq();// expected-warning {{calling function 'fq' requires negative 
capability '!globalMutex'}}
+  ns::f();
+  ns::fq();
+}
+
+void testNamespaceGlobals() EXCLUSIVE_LOCKS_REQUIRED(!globalMutex) {
+  f();
+  fq();
+  ns::f();  // expected-warning {{calling function 'f' requires negative 
capability '!globalMutex'}}
+  ns::fq(); // expected-warning {{calling function 'fq' requires negative 
capability '!globalMutex'}}
+}
+
+}  // end namespace ScopeTest
+
 namespace DoubleAttribute {
 
 struct Foo {
Index: clang/test/SemaCXX/warn-thread-safety-analysis.cpp
===
--- clang/test/SemaCXX/warn-thread-safety-analysis.cpp
+++ clang/test/SemaCXX/warn-thread-safety-analysis.cpp
@@ -5036,7 +5036,8 @@
 }
 
 extern const char *deque_log_msg(void) 
__attribute__((requires_capability(Logger)));
-void logger_entry(void) __attribute__((requires_capability(Logger))) {
+void logger_entry(void) __attribute__((requires_capability(Logger)))
+__attribute__((requires_capability(!FlightControl))) {
   const char *msg;
 
   while ((msg = deque_log_msg())) {
@@ -5044,13 +5045,13 @@
   }
 }
 
-void spawn_fake_logger_thread(void) {
+void spawn_fake_logger_thread(void) 
__attribute__((requires_capability(!FlightControl))) {
   acquire(Logger);
   logger_entry();
   release(Logger);
 }
 
-int main(void) {
+int main(void) __attribute__((requires_capability(!FlightControl))) {
   spawn_fake_flight_control_thread();
   spawn_fake_logger_thread();
 
Index: clang/lib/Analysis/ThreadSafety.cpp
===
--- clang/lib/Analysis/ThreadSafety.cpp
+++ clang/lib/Analysis/ThreadSafety.cpp
@@ -1266,13 +1266,22 @@
 }
 
 bool ThreadSafetyAnalyzer::inCurrentScope(const CapabilityExpr ) {
-  if (!CurrentMethod)
+  const threadSafety::til::SExpr *SExp = CapE.sexpr();
+  assert(SExp && "Null expressions should be ignored");
+
+  // Global variables are always in scope.
+  if (isa(SExp))
+return true;
+
+  // Members are in scope from methods of the same class.
+  if (const auto *P = dyn_cast(SExp)) {
+if (!CurrentMethod)
   return false;
-  if (const auto *P = dyn_cast_or_null(CapE.sexpr())) {
 const auto *VD = P->clangDecl();
 if (VD)
   return VD->getDeclContext() == CurrentMethod->getDeclContext();
   }
+
   return false;
 }
 


Index: clang/test/SemaCXX/warn-thread-safety-negative.cpp
===
--- clang/test/SemaCXX/warn-thread-safety-negative.cpp
+++ clang/test/SemaCXX/warn-thread-safety-negative.cpp
@@ -81,6 +81,35 @@
 
 }  // end namespace SimpleTest
 
+Mutex globalMutex;
+
+namespace ScopeTest {
+
+void f() EXCLUSIVE_LOCKS_REQUIRED(!globalMutex);
+void fq() EXCLUSIVE_LOCKS_REQUIRED(!::globalMutex);
+
+namespace ns {
+  Mutex globalMutex;
+  void f() EXCLUSIVE_LOCKS_REQUIRED(!globalMutex);
+  void fq() EXCLUSIVE_LOCKS_REQUIRED(!ns::globalMutex);
+}
+
+void testGlobals() EXCLUSIVE_LOCKS_REQUIRED(!ns::globalMutex) {
+  f(); // expected-warning {{calling function 'f' requires negative capability '!globalMutex'}}
+  fq();// expected-warning {{calling function 'fq' requires negative capability '!globalMutex'}}
+  ns::f();
+  ns::fq();
+}
+
+void testNamespaceGlobals() EXCLUSIVE_LOCKS_REQUIRED(!globalMutex) {
+  f();
+  fq();
+  ns::f();  // expected-warning {{calling function 'f' requires negative capability '!globalMutex'}}
+  ns::fq(); // expected-warning {{calling function 'fq' requires negative capability '!globalMutex'}}
+}
+
+}  // end namespace ScopeTest
+
 namespace DoubleAttribute {
 
 struct Foo {
Index: clang/test/SemaCXX/warn-thread-safety-analysis.cpp

[PATCH] D84604: Thread safety analysis: Consider global variables in scope

2020-09-03 Thread Aaron Puchert via Phabricator via cfe-commits

aaronpuchert added inline comments.



Comment at: clang/test/SemaCXX/warn-thread-safety-negative.cpp:87
+Mutex globalMutex;
+void f() EXCLUSIVE_LOCKS_REQUIRED(!globalMutex);
+

aaron.ballman wrote:
> Can you add a test that uses `!::globalMutex`? I'd like to verify that lookup 
> rules are honored for naming the mutex, so more complex examples with name 
> hiding would also be useful.
Just noticed that we're in namespace `ScopeTest` here, I'll probably need to 
move that mutex out first. For name hiding I can just name both mutexes the 
same, I think.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84604

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

[PATCH] D84364: [CUDA][HIP] Defer overloading resolution diagnostics for host device functions

2020-09-03 Thread Artem Belevich via Phabricator via cfe-commits

tra added a comment.

LGTM.

Nice!

To sum it up --  the patch introduces  `-fgpu-defer-diag` flag which allows 
deferring overload resolution diagnostics, if overload set included candidates 
from both sides.

We may be deferring cases when we don't have to (e.g. `df()->()callee2()` 
should've errored out right away, even during host compilation, as there's no 
way it could ever be valid), but this approach is a good starting point. It 
affects only the interesting subset of diags, is not enabled by default, and 
can be refined further, if necessary.




Comment at: clang/include/clang/Basic/LangOptions.def:244
 LANGOPT(GPUMaxThreadsPerBlock, 32, 256, "default max threads per block for 
kernel launch bounds for HIP")
+LANGOPT(GPUDeferDiag, 1, 0, "defer all semantic diagnostic messages in host 
device functions for CUDA/HIP")
 

The description does not seem to reflect reality. IIUIC only overload-related 
diagnostic messages will be deferred.


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

https://reviews.llvm.org/D84364

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

[libunwind] 673484b - [libunwind] Minor SJLJ config cleanup. NFCI.

2020-09-03 Thread Ryan Prichard via cfe-commits


Author: Ryan Prichard
Date: 2020-09-03T15:59:45-07:00
New Revision: 673484b34189b1bccf73a2ec96968092bc8a26a7

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

LOG: [libunwind] Minor SJLJ config cleanup. NFCI.

Simplify:

defined(__ARM_DWARF_EH__) || !defined(__arm__)

to:

!defined(_LIBUNWIND_ARM_EHABI)

A later patch benefits from the simplicity. This change will result in
the two DWARF macros being defined when __USING_SJLJ_EXCEPTIONS__ is
defined, but:

 * That's already the case with the __APPLE__ and _WIN32 clauses.
 * That's also already the case with other architectures.
 * With __USING_SJLJ_EXCEPTIONS__, most of the unwinder is #ifdef'ed
   away.

Generally, when __USING_SJLJ_EXCEPTIONS__ is defined, most of the
libunwind code is removed by the preprocessor. e.g. None of the hpp
files are included, and almost all of the .c and .cpp files are defined
away, except in Unwind-sjlj.c. Unwind_AppleExtras.cpp is an exception
because it includes two hpp files, which it doesn't use. Remove the
unneeded includes for consistency with the general rule.

Reviewed By: steven_wu

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

Added: 


Modified: 
libunwind/src/Unwind_AppleExtras.cpp
libunwind/src/config.h

Removed: 




diff  --git a/libunwind/src/Unwind_AppleExtras.cpp 
b/libunwind/src/Unwind_AppleExtras.cpp
index 1d9948aced35..e3d41ca2b4e9 100644
--- a/libunwind/src/Unwind_AppleExtras.cpp
+++ b/libunwind/src/Unwind_AppleExtras.cpp
@@ -8,8 +8,6 @@
 
//===--===//
 
 #include "config.h"
-#include "AddressSpace.hpp"
-#include "DwarfParser.hpp"
 
 
 // static linker symbols to prevent wrong two level namespace for _Unwind 
symbols

diff  --git a/libunwind/src/config.h b/libunwind/src/config.h
index 2014b8cb77ab..fd177dd7338c 100644
--- a/libunwind/src/config.h
+++ b/libunwind/src/config.h
@@ -18,6 +18,8 @@
 #include 
 #include 
 
+#include <__libunwind_config.h>
+
 // Platform specific configuration defines.
 #ifdef __APPLE__
   #if defined(FOR_DYLD)
@@ -33,7 +35,7 @@
 #define _LIBUNWIND_SUPPORT_DWARF_UNWIND 1
   #endif
 #else
-  #if defined(__ARM_DWARF_EH__) || !defined(__arm__)
+  #if !defined(_LIBUNWIND_ARM_EHABI)
 #define _LIBUNWIND_SUPPORT_DWARF_UNWIND 1
 #define _LIBUNWIND_SUPPORT_DWARF_INDEX 1
   #endif
@@ -81,6 +83,8 @@
 #error Unsupported target
 #endif
 
+// Apple/armv7k defaults to DWARF/Compact unwinding, but its libunwind also
+// needs to include the SJLJ APIs.
 #if (defined(__APPLE__) && defined(__arm__)) || 
defined(__USING_SJLJ_EXCEPTIONS__)
 #define _LIBUNWIND_BUILD_SJLJ_APIS
 #endif



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

[PATCH] D87066: Thread safety analysis: Improve documentation for scoped capabilities

2020-09-03 Thread Aaron Puchert via Phabricator via cfe-commits

aaronpuchert updated this revision to Diff 289826.
aaronpuchert marked an inline comment as done.
aaronpuchert added a comment.

- More detailed description how scoped capabilities work.
- Make the comment wording more consistent with existing comments and the 
previous explanation.
- Properly implement the destructor in the presence of an `Unlock()` function: 
we need to keep track of the status then.
- Add try-acquire functions on scoped lock.
- Fix compiler errors.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87066

Files:
  clang/docs/ThreadSafetyAnalysis.rst


Index: clang/docs/ThreadSafetyAnalysis.rst
===
--- clang/docs/ThreadSafetyAnalysis.rst
+++ clang/docs/ThreadSafetyAnalysis.rst
@@ -402,6 +402,13 @@
 and destructor refer to the capability via different names; see the
 ``MutexLocker`` class in :ref:`mutexheader`, below.
 
+Scoped capabilities are treated as capabilities that are implicitly acquired
+on construction and released on destruction. They are associated with
+the set of (regular) capabilities named in thread safety attributes on the
+constructor. Acquire-type attributes on other member functions are treated as
+applying to that set of associated capabilities, while ``RELEASE`` implies that
+a function releases all associated capabilities in whatever mode they're held.
+
 
 TRY_ACQUIRE(, ...), TRY_ACQUIRE_SHARED(, ...)
 -
@@ -886,19 +893,78 @@
 const Mutex& operator!() const { return *this; }
   };
 
+  // Tag types for selecting a constructor.
+  struct adopt_lock_t {} inline constexpr adopt_lock = {};
+  struct defer_lock_t {} inline constexpr defer_lock = {};
+  struct shared_lock_t {} inline constexpr shared_lock = {};
 
   // MutexLocker is an RAII class that acquires a mutex in its constructor, and
   // releases it in its destructor.
   class SCOPED_CAPABILITY MutexLocker {
   private:
 Mutex* mut;
+bool locked;
 
   public:
-MutexLocker(Mutex *mu) ACQUIRE(mu) : mut(mu) {
+// Acquire mu, implicitly acquire *this and associate it with mu.
+MutexLocker(Mutex *mu) ACQUIRE(mu) : mut(mu), locked(true) {
   mu->Lock();
 }
+
+// Assume mu is held, implicitly acquire *this and associate it with mu.
+MutexLocker(Mutex *mu, adopt_lock_t) REQUIRES(mu) : mut(mu), locked(true) 
{}
+
+// Acquire mu in shared mode, implicitly acquire *this and connect to mu.
+MutexLocker(Mutex *mu, shared_lock_t) ACQUIRE_SHARED(mu) : mut(mu), 
locked(true) {
+  mu->ReaderLock();
+}
+
+// Assume mu is held in shared mode, implicitly acquire *this and 
associate it with mu.
+MutexLocker(Mutex *mu, adopt_lock_t, shared_lock_t) REQUIRES_SHARED(mu)
+  : mut(mu), locked(true) {}
+
+// Assume mu is not held, implicitly acquire *this and associate it with 
mu.
+MutexLocker(Mutex *mu, defer_lock_t) EXCLUDES(mu) : mut(mu), locked(false) 
{}
+
+// Release *this and all associated mutexes, if they are still held.
+// There is no warning if the scope was already unlocked before.
 ~MutexLocker() RELEASE() {
+  if (locked)
+mut->GenericUnlock();
+}
+
+// Acquire all associated mutexes exclusively.
+void Lock() ACQUIRE() {
+  mut->Lock();
+  locked = true;
+}
+
+// Try to acquire all associated mutexes exclusively.
+bool TryLock() TRY_ACQUIRE(true) {
+  return locked = mut->TryLock();
+}
+
+// Acquire all associated mutexes in shared mode.
+void ReaderLock() ACQUIRE_SHARED() {
+  mut->ReaderLock();
+  locked = true;
+}
+
+// Try to acquire all associated mutexes in shared mode.
+bool ReaderTryLock() TRY_ACQUIRE_SHARED(true) {
+  return locked = mut->ReaderTryLock();
+}
+
+// Release all associated mutexes. Warn on double unlock.
+void Unlock() RELEASE() {
   mut->Unlock();
+  locked = false;
+}
+
+// Release all associated mutexes. Warn on double unlock.
+void ReaderUnlock() RELEASE() {
+  mut->ReaderUnlock();
+  locked = false;
 }
   };
 


Index: clang/docs/ThreadSafetyAnalysis.rst
===
--- clang/docs/ThreadSafetyAnalysis.rst
+++ clang/docs/ThreadSafetyAnalysis.rst
@@ -402,6 +402,13 @@
 and destructor refer to the capability via different names; see the
 ``MutexLocker`` class in :ref:`mutexheader`, below.
 
+Scoped capabilities are treated as capabilities that are implicitly acquired
+on construction and released on destruction. They are associated with
+the set of (regular) capabilities named in thread safety attributes on the
+constructor. Acquire-type attributes on other member functions are treated as
+applying to that set of associated capabilities, while ``RELEASE`` implies that
+a function releases all associated capabilities in whatever

[PATCH] D87062: [DebugInfo] Add size to class declarations in debug info.

2020-09-03 Thread Amy Huang via Phabricator via cfe-commits

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGaaf1a96408b1: [DebugInfo] Add size to class declarations in 
debug info. (authored by akhuang).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87062

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/test/CodeGenCXX/debug-info-class.cpp


Index: clang/test/CodeGenCXX/debug-info-class.cpp
===
--- clang/test/CodeGenCXX/debug-info-class.cpp
+++ clang/test/CodeGenCXX/debug-info-class.cpp
@@ -136,7 +136,7 @@
 // CHECK: [[C_DTOR]] = !DISubprogram(name: "~C"
 
 // CHECK: [[D:![0-9]+]] = !DICompositeType(tag: DW_TAG_structure_type, name: 
"D"
-// CHECK-NOT:  size:
+// CHECK-SAME: size:
 // CHECK-SAME: DIFlagFwdDecl
 // CHECK-NOT:  identifier:
 // CHECK-SAME: ){{$}}
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -1031,6 +1031,10 @@
   uint64_t Size = 0;
   uint32_t Align = 0;
 
+  const RecordDecl *D = RD->getDefinition();
+  if (D && D->isCompleteDefinition())
+Size = CGM.getContext().getTypeSize(Ty);
+
   llvm::DINode::DIFlags Flags = llvm::DINode::FlagFwdDecl;
 
   // Add flag to nontrivial forward declarations. To be consistent with MSVC,


Index: clang/test/CodeGenCXX/debug-info-class.cpp
===
--- clang/test/CodeGenCXX/debug-info-class.cpp
+++ clang/test/CodeGenCXX/debug-info-class.cpp
@@ -136,7 +136,7 @@
 // CHECK: [[C_DTOR]] = !DISubprogram(name: "~C"
 
 // CHECK: [[D:![0-9]+]] = !DICompositeType(tag: DW_TAG_structure_type, name: "D"
-// CHECK-NOT:  size:
+// CHECK-SAME: size:
 // CHECK-SAME: DIFlagFwdDecl
 // CHECK-NOT:  identifier:
 // CHECK-SAME: ){{$}}
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -1031,6 +1031,10 @@
   uint64_t Size = 0;
   uint32_t Align = 0;
 
+  const RecordDecl *D = RD->getDefinition();
+  if (D && D->isCompleteDefinition())
+Size = CGM.getContext().getTypeSize(Ty);
+
   llvm::DINode::DIFlags Flags = llvm::DINode::FlagFwdDecl;
 
   // Add flag to nontrivial forward declarations. To be consistent with MSVC,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] aaf1a96 - [DebugInfo] Add size to class declarations in debug info.

2020-09-03 Thread Amy Huang via cfe-commits


Author: Amy Huang
Date: 2020-09-03T15:42:27-07:00
New Revision: aaf1a96408b1587b5fb80a3a7c424348cb09e577

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

LOG: [DebugInfo] Add size to class declarations in debug info.

This adds the size to forward declared class DITypes, if the size is known.

Fixes an issue where we determine whether to emit fragments based on the
type size, so fragments would sometimes be incorrectly emitted if there
was no size.

Bug: https://bugs.llvm.org/show_bug.cgi?id=47338

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

Added: 


Modified: 
clang/lib/CodeGen/CGDebugInfo.cpp
clang/test/CodeGenCXX/debug-info-class.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGDebugInfo.cpp 
b/clang/lib/CodeGen/CGDebugInfo.cpp
index 8a85a24910e4..1fdb6814c7bd 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -1031,6 +1031,10 @@ CGDebugInfo::getOrCreateRecordFwdDecl(const RecordType 
*Ty,
   uint64_t Size = 0;
   uint32_t Align = 0;
 
+  const RecordDecl *D = RD->getDefinition();
+  if (D && D->isCompleteDefinition())
+Size = CGM.getContext().getTypeSize(Ty);
+
   llvm::DINode::DIFlags Flags = llvm::DINode::FlagFwdDecl;
 
   // Add flag to nontrivial forward declarations. To be consistent with MSVC,

diff  --git a/clang/test/CodeGenCXX/debug-info-class.cpp 
b/clang/test/CodeGenCXX/debug-info-class.cpp
index 94d5a0f1f082..e000532b8c3b 100644
--- a/clang/test/CodeGenCXX/debug-info-class.cpp
+++ b/clang/test/CodeGenCXX/debug-info-class.cpp
@@ -136,7 +136,7 @@ int main(int argc, char **argv) {
 // CHECK: [[C_DTOR]] = !DISubprogram(name: "~C"
 
 // CHECK: [[D:![0-9]+]] = !DICompositeType(tag: DW_TAG_structure_type, name: 
"D"
-// CHECK-NOT:  size:
+// CHECK-SAME: size:
 // CHECK-SAME: DIFlagFwdDecl
 // CHECK-NOT:  identifier:
 // CHECK-SAME: ){{$}}



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

[PATCH] D86999: getClangStripDependencyFileAdjuster(): Do not remove -M args when using MSVC cl driver

2020-09-03 Thread Shivanshu Goyal via Phabricator via cfe-commits

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

Note that I do not have commit access and this change will have to be committed 
by someone else on my behalf. Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86999

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

[PATCH] D87062: [DebugInfo] Add size to class declarations in debug info.

2020-09-03 Thread Amy Huang via Phabricator via cfe-commits

akhuang added inline comments.



Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:1035
+  const RecordDecl *D = RD->getDefinition();
+  if (D && D->isCompleteDefinition())
+Size = CGM.getContext().getTypeSize(Ty);

akhuang wrote:
> dblaikie wrote:
> > When is a definition not a complete definition? (sounds like a line from 
> > Alice in Wonderland... but I'm genuinely curious/not sure I understand)
> ha, good question, I copied this without really looking at it. I am also not 
> sure when a definition would not be a complete definition.
Oh, I guess incomplete definition means it's in the process of being defined.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87062

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

[clang] 052dbe2 - Remove unused and dangerous overload of PerformImplicitConversion.

2020-09-03 Thread Richard Smith via cfe-commits


Author: Richard Smith
Date: 2020-09-03T15:35:12-07:00
New Revision: 052dbe226cb3540c77cf0b3dc4a51a4ab7726b55

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

LOG: Remove unused and dangerous overload of PerformImplicitConversion.

Previously we had two overloads where the only real difference beyond
parameter order was whether a reference parameter is const, where one
overload treated the reference parameter as an in-parameter and the
other treated it as an out-parameter!

Added: 


Modified: 
clang/include/clang/Sema/Sema.h
clang/lib/Sema/SemaOverload.cpp

Removed: 




diff  --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 174b424bb996..ec449d6dd6be 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -11224,10 +11224,6 @@ class Sema final {
   ExprResult PerformImplicitConversion(Expr *From, QualType ToType,
AssignmentAction Action,
bool AllowExplicit = false);
-  ExprResult PerformImplicitConversion(Expr *From, QualType ToType,
-   AssignmentAction Action,
-   bool AllowExplicit,
-   ImplicitConversionSequence& ICS);
   ExprResult PerformImplicitConversion(Expr *From, QualType ToType,
const ImplicitConversionSequence& ICS,
AssignmentAction Action,

diff  --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index 21a9ad04d500..71341e5688fe 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -1494,17 +1494,9 @@ Sema::TryImplicitConversion(Expr *From, QualType ToType,
 /// converted expression. Flavor is the kind of conversion we're
 /// performing, used in the error message. If @p AllowExplicit,
 /// explicit user-defined conversions are permitted.
-ExprResult
-Sema::PerformImplicitConversion(Expr *From, QualType ToType,
-AssignmentAction Action, bool AllowExplicit) {
-  ImplicitConversionSequence ICS;
-  return PerformImplicitConversion(From, ToType, Action, AllowExplicit, ICS);
-}
-
-ExprResult
-Sema::PerformImplicitConversion(Expr *From, QualType ToType,
-AssignmentAction Action, bool AllowExplicit,
-ImplicitConversionSequence& ICS) {
+ExprResult Sema::PerformImplicitConversion(Expr *From, QualType ToType,
+   AssignmentAction Action,
+   bool AllowExplicit) {
   if (checkPlaceholderForOverload(*this, From))
 return ExprError();
 
@@ -1515,13 +1507,13 @@ Sema::PerformImplicitConversion(Expr *From, QualType 
ToType,
   if (getLangOpts().ObjC)
 CheckObjCBridgeRelatedConversions(From->getBeginLoc(), ToType,
   From->getType(), From);
-  ICS = ::TryImplicitConversion(*this, From, ToType,
-/*SuppressUserConversions=*/false,
-AllowExplicit ? AllowedExplicit::All
-  : AllowedExplicit::None,
-/*InOverloadResolution=*/false,
-/*CStyle=*/false, AllowObjCWritebackConversion,
-/*AllowObjCConversionOnExplicit=*/false);
+  ImplicitConversionSequence ICS = ::TryImplicitConversion(
+  *this, From, ToType,
+  /*SuppressUserConversions=*/false,
+  AllowExplicit ? AllowedExplicit::All : AllowedExplicit::None,
+  /*InOverloadResolution=*/false,
+  /*CStyle=*/false, AllowObjCWritebackConversion,
+  /*AllowObjCConversionOnExplicit=*/false);
   return PerformImplicitConversion(From, ToType, ICS, Action);
 }
 



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

[clang] e6393ee - Canonicalize declaration pointers when forming APValues.

2020-09-03 Thread Richard Smith via cfe-commits


Author: Richard Smith
Date: 2020-09-03T15:35:12-07:00
New Revision: e6393ee813178e9d3306b8e3c6949a4f32f8a2cb

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

LOG: Canonicalize declaration pointers when forming APValues.

References to different declarations of the same entity aren't different
values, so shouldn't have different representations.

Added: 


Modified: 
clang/include/clang/AST/APValue.h
clang/lib/AST/APValue.cpp
clang/lib/AST/ExprConstant.cpp
clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p9.cpp
clang/test/OpenMP/ordered_messages.cpp

Removed: 




diff  --git a/clang/include/clang/AST/APValue.h 
b/clang/include/clang/AST/APValue.h
index 87e4bd7f84c1..485e6c2602cf 100644
--- a/clang/include/clang/AST/APValue.h
+++ b/clang/include/clang/AST/APValue.h
@@ -174,6 +174,7 @@ class APValue {
   return !(LHS == RHS);
 }
 friend llvm::hash_code hash_value(const LValueBase );
+friend struct llvm::DenseMapInfo;
 
   private:
 PtrTy Ptr;
@@ -201,8 +202,7 @@ class APValue {
 
   public:
 LValuePathEntry() : Value() {}
-LValuePathEntry(BaseOrMemberType BaseOrMember)
-: Value{reinterpret_cast(BaseOrMember.getOpaqueValue())} {}
+LValuePathEntry(BaseOrMemberType BaseOrMember);
 static LValuePathEntry ArrayIndex(uint64_t Index) {
   LValuePathEntry Result;
   Result.Value = Index;

diff  --git a/clang/lib/AST/APValue.cpp b/clang/lib/AST/APValue.cpp
index 2a8834b4db0c..7531229654cf 100644
--- a/clang/lib/AST/APValue.cpp
+++ b/clang/lib/AST/APValue.cpp
@@ -38,7 +38,7 @@ static_assert(
 "Type is insufficiently aligned");
 
 APValue::LValueBase::LValueBase(const ValueDecl *P, unsigned I, unsigned V)
-: Ptr(P), Local{I, V} {}
+: Ptr(P ? cast(P->getCanonicalDecl()) : nullptr), Local{I, V} {}
 APValue::LValueBase::LValueBase(const Expr *P, unsigned I, unsigned V)
 : Ptr(P), Local{I, V} {}
 
@@ -82,13 +82,19 @@ bool operator==(const APValue::LValueBase ,
 const APValue::LValueBase ) {
   if (LHS.Ptr != RHS.Ptr)
 return false;
-  if (LHS.is())
+  if (LHS.is() || LHS.is())
 return true;
   return LHS.Local.CallIndex == RHS.Local.CallIndex &&
  LHS.Local.Version == RHS.Local.Version;
 }
 }
 
+APValue::LValuePathEntry::LValuePathEntry(BaseOrMemberType BaseOrMember) {
+  if (const Decl *D = BaseOrMember.getPointer())
+BaseOrMember.setPointer(D->getCanonicalDecl());
+  Value = reinterpret_cast(BaseOrMember.getOpaqueValue());
+}
+
 namespace {
   struct LVBase {
 APValue::LValueBase Base;
@@ -113,14 +119,16 @@ APValue::LValueBase::operator bool () const {
 
 clang::APValue::LValueBase
 llvm::DenseMapInfo::getEmptyKey() {
-  return clang::APValue::LValueBase(
-  DenseMapInfo::getEmptyKey());
+  clang::APValue::LValueBase B;
+  B.Ptr = DenseMapInfo::getEmptyKey();
+  return B;
 }
 
 clang::APValue::LValueBase
 llvm::DenseMapInfo::getTombstoneKey() {
-  return clang::APValue::LValueBase(
-  DenseMapInfo::getTombstoneKey());
+  clang::APValue::LValueBase B;
+  B.Ptr = DenseMapInfo::getTombstoneKey();
+  return B;
 }
 
 namespace clang {
@@ -757,8 +765,10 @@ void APValue::MakeMemberPointer(const ValueDecl *Member, 
bool IsDerivedMember,
   assert(isAbsent() && "Bad state change");
   MemberPointerData *MPD = new ((void*)(char*)Data.buffer) MemberPointerData;
   Kind = MemberPointer;
-  MPD->MemberAndIsDerivedMember.setPointer(Member);
+  MPD->MemberAndIsDerivedMember.setPointer(
+  Member ? cast(Member->getCanonicalDecl()) : nullptr);
   MPD->MemberAndIsDerivedMember.setInt(IsDerivedMember);
   MPD->resizePath(Path.size());
-  memcpy(MPD->getPath(), Path.data(), Path.size()*sizeof(const 
CXXRecordDecl*));
+  for (unsigned I = 0; I != Path.size(); ++I)
+MPD->getPath()[I] = Path[I]->getCanonicalDecl();
 }

diff  --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index e8f132dd4803..8e43b62662ee 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -1978,18 +1978,11 @@ static bool HasSameBase(const LValue , const LValue 
) {
 return false;
 
   if (A.getLValueBase().getOpaqueValue() !=
-  B.getLValueBase().getOpaqueValue()) {
-const Decl *ADecl = GetLValueBaseDecl(A);
-if (!ADecl)
-  return false;
-const Decl *BDecl = GetLValueBaseDecl(B);
-if (!BDecl || ADecl->getCanonicalDecl() != BDecl->getCanonicalDecl())
-  return false;
-  }
+  B.getLValueBase().getOpaqueValue())
+return false;
 
-  return IsGlobalLValue(A.getLValueBase()) ||
- (A.getLValueCallIndex() == B.getLValueCallIndex() &&
-  A.getLValueVersion() == B.getLValueVersion());
+  return A.getLValueCallIndex() == B.getLValueCallIndex() &&
+ A.getLValueVersion() == B.getLValueVersion();
 }

[PATCH] D86841: [clang] Add noprogress attribute deduction for infinite loops

2020-09-03 Thread Atmn Patel via Phabricator via cfe-commits

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



Comment at: clang/lib/CodeGen/CGLoopInfo.h:211
 llvm::ArrayRef Attrs, const llvm::DebugLoc ,
-const llvm::DebugLoc );
+const llvm::DebugLoc , const bool NoProgress = false);
 

aaron.ballman wrote:
> I'd drop the top-level `const` on the declaration of `NoProgress` (that's not 
> a style we typically use in the project).
My bad, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86841

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

[PATCH] D86841: [clang] Add noprogress attribute deduction for infinite loops

2020-09-03 Thread Atmn Patel via Phabricator via cfe-commits

atmnpatel updated this revision to Diff 289821.
atmnpatel added a comment.
Herald added a subscriber: zzheng.

Renamed llvm loop metadata, changed deduction rules.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86841

Files:
  clang/lib/CodeGen/CGLoopInfo.cpp
  clang/lib/CodeGen/CGLoopInfo.h
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/test/CodeGen/attr-noprogress.c
  clang/test/CodeGen/attr-noprogress.cpp
  clang/test/CodeGenCXX/fno-unroll-loops-metadata.cpp

Index: clang/test/CodeGenCXX/fno-unroll-loops-metadata.cpp
===
--- clang/test/CodeGenCXX/fno-unroll-loops-metadata.cpp
+++ clang/test/CodeGenCXX/fno-unroll-loops-metadata.cpp
@@ -4,7 +4,7 @@
 // RUN: %clang_cc1 -triple x86_64-apple-darwin -std=c++11 -emit-llvm -o - %s -O3 -disable-llvm-optzns -fno-unroll-loops | FileCheck --check-prefix=UNROLL_DISABLED_MD %s
 // RUN: %clang_cc1 -triple x86_64-apple-darwin -std=c++11 -emit-llvm -o - %s -O3 -disable-llvm-optzns | FileCheck --check-prefix=NO_UNROLL_MD %s
 
-// NO_UNROLL_MD-NOT: llvm.loop
+// NO_UNROLL_MD-NOT: llvm.loop.unroll.disable
 
 // Verify unroll.disable metadata is added to while loop with -fno-unroll-loops
 // and optlevel > 0.
Index: clang/test/CodeGen/attr-noprogress.cpp
===
--- /dev/null
+++ clang/test/CodeGen/attr-noprogress.cpp
@@ -0,0 +1,188 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py UTC_ARGS: --check-attributes
+// RUN: %clang_cc1 -S -emit-llvm %s -o - | FileCheck %s
+
+int a = 0;
+int b = 0;
+
+// CHECK: Function Attrs: noinline nounwind optnone noprogress
+// CHECK-LABEL: @_Z2f1v(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:br label [[FOR_COND:%.*]]
+// CHECK:   for.cond:
+// CHECK: br i1 true, label [[FOR_BODY:%.*]], label [[FOR_END:%.*]]
+// CHECK:   for.body:
+// CHECK: br label [[FOR_COND]]
+// CHECK:   for.end:
+// CHECK: ret void
+//
+void f1() {
+  for (; 1;) {
+  }
+}
+
+// CHECK: Function Attrs: noinline nounwind optnone
+// CHECK-LABEL: @_Z2f2v(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:br label [[FOR_COND:%.*]]
+// CHECK:   for.cond:
+// CHECK: [[TMP0:%.*]] = load i32, i32* @a, align 4
+// CHECK: [[TMP1:%.*]] = load i32, i32* @b, align 4
+// CHECK: [[CMP:%.*]] = icmp eq i32 [[TMP0]], [[TMP1]]
+// CHECK: br i1 [[CMP]], label [[FOR_BODY:%.*]], label [[FOR_END:%.*]]
+// CHECK:   for.body:
+// CHECK: br label [[FOR_COND]]
+// CHECK:   for.end:
+// CHECK: ret void
+//
+void f2() {
+  for (; a == b;) {
+  }
+}
+
+// CHECK: Function Attrs: noinline nounwind optnone noprogress
+// CHECK-LABEL: @_Z1Fv(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:br label [[FOR_COND:%.*]]
+// CHECK:   for.cond:
+// CHECK: br i1 true, label [[FOR_BODY:%.*]], label [[FOR_END:%.*]]
+// CHECK:   for.body:
+// CHECK: br label [[FOR_COND]]
+// CHECK:   for.end:
+// CHECK: br label [[FOR_COND1:%.*]]
+// CHECK:   for.cond1:
+// CHECK: [[TMP0:%.*]] = load i32, i32* @a, align 4
+// CHECK: [[TMP1:%.*]] = load i32, i32* @b, align 4
+// CHECK: [[CMP:%.*]] = icmp eq i32 [[TMP0]], [[TMP1]]
+// CHECK: br i1 [[CMP]], label [[FOR_BODY2:%.*]], label [[FOR_END3:%.*]]
+// CHECK:   for.body2:
+// CHECK: br label [[FOR_COND1]], !llvm.loop [[LOOP2:!.*]]
+// CHECK:   for.end3:
+// CHECK: ret void
+//
+void F() {
+  for (; 1;) {
+  }
+  for (; a == b;) {
+  }
+}
+
+// CHECK: Function Attrs: noinline nounwind optnone noprogress
+// CHECK-LABEL: @_Z2w1v(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:br label [[WHILE_BODY:%.*]]
+// CHECK:   while.body:
+// CHECK: br label [[WHILE_BODY]]
+//
+void w1() {
+  while (1) {
+  }
+}
+
+// CHECK: Function Attrs: noinline nounwind optnone
+// CHECK-LABEL: @_Z2w2v(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:br label [[WHILE_COND:%.*]]
+// CHECK:   while.cond:
+// CHECK: [[TMP0:%.*]] = load i32, i32* @a, align 4
+// CHECK: [[TMP1:%.*]] = load i32, i32* @b, align 4
+// CHECK: [[CMP:%.*]] = icmp eq i32 [[TMP0]], [[TMP1]]
+// CHECK: br i1 [[CMP]], label [[WHILE_BODY:%.*]], label [[WHILE_END:%.*]]
+// CHECK:   while.body:
+// CHECK: br label [[WHILE_COND]]
+// CHECK:   while.end:
+// CHECK: ret void
+//
+void w2() {
+  while (a == b) {
+  }
+}
+
+// CHECK: Function Attrs: noinline nounwind optnone noprogress
+// CHECK-LABEL: @_Z1Wv(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:br label [[WHILE_COND:%.*]]
+// CHECK:   while.cond:
+// CHECK: [[TMP0:%.*]] = load i32, i32* @a, align 4
+// CHECK: [[TMP1:%.*]] = load i32, i32* @b, align 4
+// CHECK: [[CMP:%.*]] = icmp eq i32 [[TMP0]], [[TMP1]]
+//

[PATCH] D82118: [clang][module] Improve incomplete-umbrella warning

2020-09-03 Thread Zixu Wang via Phabricator via cfe-commits

zixuw added a comment.

Okay so I think I traced down to the root problem: the diagnostic message 
prints out fine because `TextDiagnosticPrinter` uses `FullSourceLoc`, which 
actually embeds an appropriate `SourceManager` within it:

  TextDiag->emitDiagnostic(
  FullSourceLoc(Info.getLocation(), Info.getSourceManager()), Level,
  DiagMessageStream.str(), Info.getRanges(), Info.getFixItHints());

However, `FixItHint` is handled by a different path: a `FixItHint` only stores 
a trivial location (a `SourceRange`) which does not embed a `SourceManager`. 
And when a `FixItRewriter` constructs, it initializes a bunch of helper objects 
(Editor, Rewriter, Commit, etc.) using the current `SourceManager`. When trying 
to commit a fix-it hint, the helper objects use their (const ref to) 
`SourceManager`, which is used by the main compilation, to resolve the plain 
location in the `FixItHint`, which is generated in the new compiler instance 
for building a module. Therefore the location resolution is wrong.
To fix this, we can make `FixItHint` use `FullSourceLoc` instead, and also 
modify all relevant parties (`FixItRewriter`, `EditedSource`, `Rewriter`, 
`Commit`, ...) to use the embedded `SourceManager` instead of having a const 
ref to one initialized `SourceManager`.
My main concern is that I'm not very familiar with all the rewrite facilities, 
and changing `EditedSource` etc. might affect other users of the facilities.
What do you think? @arphaman @bruno @vsapsai


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82118

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

[PATCH] D86796: [Sema] Address-space sensitive index check for unbounded arrays

2020-09-03 Thread Chris Hamilton via Phabricator via cfe-commits

chrish_ericsson_atx updated this revision to Diff 289819.
chrish_ericsson_atx added a comment.

NC push did not resolve failed test.  Rebased in hopes that whatever has 
broken the build has been resolved in the intervening commits.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86796

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaChecking.cpp
  clang/test/Sema/const-eval.c
  clang/test/Sema/unbounded-array-bounds.c
  clang/test/SemaCXX/constant-expression-cxx1y.cpp

Index: clang/test/SemaCXX/constant-expression-cxx1y.cpp
===
--- clang/test/SemaCXX/constant-expression-cxx1y.cpp
+++ clang/test/SemaCXX/constant-expression-cxx1y.cpp
@@ -1018,8 +1018,9 @@
 }
 
 constexpr void PR28739(int n) { // expected-error {{never produces a constant}}
-  int *p = 
+  int *p =   // expected-note {{declared here}}
   p += (__int128)(unsigned long)-1; // expected-note {{cannot refer to element 18446744073709551615 of non-array object in a constant expression}}
+  // expected-warning@-1 {{refers past the last possible element}}
 }
 
 constexpr void Void(int n) {
Index: clang/test/Sema/unbounded-array-bounds.c
===
--- /dev/null
+++ clang/test/Sema/unbounded-array-bounds.c
@@ -0,0 +1,70 @@
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -fsyntax-only %s 2>&1 | FileCheck --check-prefix=CHECK-X86-ADDR64 %s  \
+// RUN:  --implicit-check-not 'past the last possible element'
+// RUN: %clang_cc1 -triple i386-pc-linux-gnu   -fsyntax-only %s 2>&1 | FileCheck --check-prefix=CHECK-I386-ADDR32 %s \
+// RUN:  --implicit-check-not 'past the last possible element'
+// RUN: %clang_cc1 -triple avr-pc-linux-gnu-fsyntax-only %s 2>&1 | FileCheck --check-prefix=CHECK-AVR-ADDR16 %s  \
+// RUN:  --implicit-check-not 'past the last possible element'
+
+struct S {
+  long long a;
+  char b;
+  long long c;
+  short d;
+};
+
+struct S s[];
+
+void f1() {
+  ++s[3].a;
+  ++s[7073650413200313099].b;
+  // CHECK-X86-ADDR64:  :[[@LINE-1]]:5: warning: {{.*}} past the last possible element {{.*}} in 64-bit {{.*}} (max possible 576460752303423488 elements)
+  // CHECK-I386-ADDR32: :[[@LINE-2]]:5: warning: {{.*}} past the last possible element {{.*}} in 32-bit {{.*}} (max possible 178956970 elements)
+  // CHECK-AVR-ADDR16:  :[[@LINE-3]]:5: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 3276 elements)
+  ++s[7073650].c;
+  // CHECK-AVR-ADDR16:  :[[@LINE-1]]:5: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 3276 elements)
+}
+
+long long ll[];
+
+void f2() {
+  ++ll[3];
+  ++ll[2705843009213693952];
+  // CHECK-X86-ADDR64:  :[[@LINE-1]]:5: warning: {{.*}} past the last possible element {{.*}} in 64-bit {{.*}} (max possible 2305843009213693952 elements)
+  // CHECK-I386-ADDR32: :[[@LINE-2]]:5: warning: {{.*}} past the last possible element {{.*}} in 32-bit {{.*}} (max possible 536870912 elements)
+  // CHECK-AVR-ADDR16:  :[[@LINE-3]]:5: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 8192 elements)
+  ++ll[847073650];
+  // CHECK-I386-ADDR32: :[[@LINE-1]]:5: warning: {{.*}} past the last possible element {{.*}} in 32-bit {{.*}} (max possible 536870912 elements)
+  // CHECK-AVR-ADDR16:  :[[@LINE-2]]:5: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 8192 elements)
+}
+
+void f3(struct S p[]) {
+  ++p[3].a;
+  ++p[7073650413200313099].b;
+  // CHECK-X86-ADDR64:  :[[@LINE-1]]:5: warning: {{.*}} past the last possible element {{.*}} in 64-bit {{.*}} (max possible 576460752303423488 elements)
+  // CHECK-I386-ADDR32: :[[@LINE-2]]:5: warning: {{.*}} past the last possible element {{.*}} in 32-bit {{.*}} (max possible 178956970 elements)
+  // CHECK-AVR-ADDR16:  :[[@LINE-3]]:5: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 3276 elements)
+  ++p[7073650].c;
+  // CHECK-AVR-ADDR16:  :[[@LINE-1]]:5: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 3276 elements)
+}
+
+void f4(struct S *p) {
+  p += 3;
+  p += 7073650413200313099;
+  // CHECK-X86-ADDR64:  :[[@LINE-1]]:3: warning: {{.*}} past the last possible element {{.*}} in 64-bit {{.*}} (max possible 576460752303423488 elements)
+  // CHECK-I386-ADDR32: :[[@LINE-2]]:3: warning: {{.*}} past the last possible element {{.*}} in 32-bit {{.*}} (max possible 178956970 elements)
+  // CHECK-AVR-ADDR16:  :[[@LINE-3]]:3: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 3276 elements)
+  p += 7073650;
+  // CHECK-AVR-ADDR16:  :[[@LINE-1]]:3: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 3276 elements)
+}
+
+struct BQ {
+  struct S

[PATCH] D85408: Let -basic-block-sections=labels emit basicblock metadata in a new .bb_addr_map section, instead of emitting special unary-encoded symbols.

2020-09-03 Thread Fangrui Song via Phabricator via cfe-commits

MaskRay added inline comments.



Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:1047
+  getObjFileLowering().getBBAddrMapSection(*MF.getSection());
+  if (!BBAddrMapSection)
+return;

rahmanl wrote:
> MaskRay wrote:
> > BBAddrMapSection is always non-null. Delete the if.
> I believe we return null for non-ELF environment (Please refer to 
> MCObjectFileInfo::getBBAddrMapSection).
In that case emitBBAddrMapSection will not be called?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85408

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

[PATCH] D84364: [CUDA][HIP] Defer overloading resolution diagnostics for host device functions

2020-09-03 Thread Yaxun Liu via Phabricator via cfe-commits

yaxunl added a comment.

In D84364#2201336 , @tra wrote:

> In D84364#2176091 , @yaxunl wrote:
>
>> I added a `Deferrable` bit to the diagnostics which can be specified in td 
>> files. This can be added to individual diagnostic defs or added to a bunch 
>> of diagnostic defs all together.
>>
>> This field is used to control whether a diagnostic message can be deferred.
>
> This may be a case of "too much, but not enough". It will be unnecessary for 
> most of the diagnostics we have. Overload resolution is likely to be the 
> primary beneficiary, inline asm and exceptions may be two other classes, but 
> I can't think of anything else ATM. 
> At the same time it may not be enough, because we also need to take into 
> account where and when particular diagnostic is emitted. I.e. the same 
> diagnostic may need to be postponed when we emit it from CUDA code, yet we 
> may want to *not* postpone it if it's in the code which has nothing to do 
> with CUDA. E.g. C++ code which has oveloading-related error in an inline 
> function which would not be codegen'ed. I would expect such error to be 
> reported as it would be if the same function was compiled in plain C++ mode.

I added a heuristics based rule for deferring overloading resolution related 
diagnostics. Basically clang will check if there are wrong-sided candidates 
when an overloading resolution error happens. This seems to be able to capture 
most of the host-ness related diagnostics which we want to defer whereas 
without affecting the diagnostics that are unrelated to host-ness.

>> For now I enabled this bit for the overloading resolution diagnostics since 
>> these have been seen as false alarms in host device functions. We could let 
>> more diagnostics be deferrable if we see it is necessary.
>
>
>
>> I just saw bugzilla bug https://bugs.llvm.org/show_bug.cgi?id=46922
>> my patch https://reviews.llvm.org/D77954 is supposed to fix this issue. 
>> However since implicit host device functions often cause overloading 
>> resolution diagnostics on the device side which are not deferred, my patch 
>> caused regressions and was reverted several times. Currently it was still 
>> reverted.
>>
>> I think to fix this issue we need to make overloading resolution diagnostics 
>> deferrable.
>
> I'm missing something here. How would deferred diagnostics help with the 
> issue in the bug 46922? The HD function there is codegen'ed on both sides, so 
> the only difference postponing would make is that we'd emit the diagnostic a 
> bit later.
> Do you mean that postponed diags patch is not the fix, but rather a 
> prerequisite for the overload resolution changes patch?

Right. We need to favor same sided candidates for host device functions to fix 
that issue, the obstacle to implement the correct host/device based overloading 
resolution is that implicit host device function causes diagnostics in device 
compilation, which should be deferred but currently are not. With this patch we 
may be able to fix the overloading resolution issue properly.


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

https://reviews.llvm.org/D84364

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

[PATCH] D84364: [CUDA][HIP] Defer overloading resolution diagnostics for host device functions

2020-09-03 Thread Yaxun Liu via Phabricator via cfe-commits

yaxunl updated this revision to Diff 289814.
yaxunl added a comment.

Defer overload resolution diags only if there are wrong-sided candidates.


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

https://reviews.llvm.org/D84364

Files:
  clang/include/clang/Basic/Diagnostic.td
  clang/include/clang/Basic/DiagnosticAST.h
  clang/include/clang/Basic/DiagnosticAnalysis.h
  clang/include/clang/Basic/DiagnosticComment.h
  clang/include/clang/Basic/DiagnosticCrossTU.h
  clang/include/clang/Basic/DiagnosticDriver.h
  clang/include/clang/Basic/DiagnosticFrontend.h
  clang/include/clang/Basic/DiagnosticIDs.h
  clang/include/clang/Basic/DiagnosticLex.h
  clang/include/clang/Basic/DiagnosticParse.h
  clang/include/clang/Basic/DiagnosticRefactoring.h
  clang/include/clang/Basic/DiagnosticSema.h
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/DiagnosticSerialization.h
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/Options.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Basic/DiagnosticIDs.cpp
  clang/lib/Driver/ToolChains/Cuda.cpp
  clang/lib/Driver/ToolChains/HIP.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Sema/AnalysisBasedWarnings.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaAttr.cpp
  clang/lib/Sema/SemaCUDA.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExprObjC.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/SemaSYCL.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaStmtAsm.cpp
  clang/lib/Sema/SemaTemplateInstantiate.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/lib/Sema/SemaTemplateVariadic.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/SemaCUDA/deferred-oeverload.cu
  clang/test/TableGen/DiagnosticBase.inc
  clang/test/TableGen/deferred-diag.td
  clang/tools/diagtool/DiagnosticNames.cpp
  clang/utils/TableGen/ClangDiagnosticsEmitter.cpp

Index: clang/utils/TableGen/ClangDiagnosticsEmitter.cpp
===
--- clang/utils/TableGen/ClangDiagnosticsEmitter.cpp
+++ clang/utils/TableGen/ClangDiagnosticsEmitter.cpp
@@ -1294,6 +1294,11 @@
 else
   OS << ", false";
 
+if (R.getValueAsBit("Deferrable"))
+  OS << ", true";
+else
+  OS << ", false";
+
 // Category number.
 OS << ", " << CategoryIDs.getID(getDiagnosticCategory(, DGParentMap));
 OS << ")\n";
Index: clang/tools/diagtool/DiagnosticNames.cpp
===
--- clang/tools/diagtool/DiagnosticNames.cpp
+++ clang/tools/diagtool/DiagnosticNames.cpp
@@ -28,7 +28,7 @@
 // out of sync easily?
 static const DiagnosticRecord BuiltinDiagnosticsByID[] = {
 #define DIAG(ENUM,CLASS,DEFAULT_MAPPING,DESC,GROUP,   \
- SFINAE,NOWERROR,SHOWINSYSHEADER,CATEGORY)\
+ SFINAE,NOWERROR,SHOWINSYSHEADER,DEFER,CATEGORY)\
   { #ENUM, diag::ENUM, STR_SIZE(#ENUM, uint8_t) },
 #include "clang/Basic/DiagnosticCommonKinds.inc"
 #include "clang/Basic/DiagnosticCrossTUKinds.inc"
Index: clang/test/TableGen/deferred-diag.td
===
--- /dev/null
+++ clang/test/TableGen/deferred-diag.td
@@ -0,0 +1,27 @@
+// RUN: clang-tblgen -gen-clang-diags-defs -I%S %s -o - 2>&1 | \
+// RUN:FileCheck --strict-whitespace %s
+include "DiagnosticBase.inc"
+
+// Test usage of Deferrable and NonDeferrable in diagnostics.
+
+def test_default : Error<"This error is non-deferrable by default">;
+// CHECK-DAG: DIAG(test_default, {{.*}}SFINAE_SubstitutionFailure, false, true, false, 0)
+
+def test_deferrable : Error<"This error is deferrable">, Deferrable;
+// CHECK-DAG: DIAG(test_deferrable, {{.*}} SFINAE_SubstitutionFailure, false, true, true, 0)
+
+def test_non_deferrable : Error<"This error is non-deferrable">, NonDeferrable;
+// CHECK-DAG: DIAG(test_non_deferrable, {{.*}} SFINAE_SubstitutionFailure, false, true, false, 0)
+
+let Deferrable = 1 in {
+
+def test_let : Error<"This error is deferrable by let">;
+// CHECK-DAG: DIAG(test_let, {{.*}} SFINAE_SubstitutionFailure, false, true, true, 0)
+
+// Make sure TextSubstitution is allowed in the let Deferrable block.
+def textsub : TextSubstitution<"%select{text1|text2}0">;
+
+def test_let2 : Error<"This error is deferrable by let %sub{textsub}0">;
+// CHECK-DAG: DIAG(test_let2, {{.*}} SFINAE_SubstitutionFailure, false, true, true, 0)
+
+}
\ No newline at end of file
Index: clang/test/TableGen/DiagnosticBase.inc
===
--- clang/test/TableGen/DiagnosticBase.inc
+++ clang/test/TableGen/DiagnosticBase.inc
@@ -45,6 +45,7 @@
   // diagnostics
   string Component = "";
   string CategoryName = "";
+  bit Deferrable = 0;
 }
 
 // Diagnostic Categories.  These can be applied to groups or individual
@@ -75,6 +76,7 @@
   bitAccessControl = 0;
   bitWarningNoWerror = 0;

[PATCH] D86796: [Sema] Address-space sensitive index check for unbounded arrays

2020-09-03 Thread Chris Hamilton via Phabricator via cfe-commits

chrish_ericsson_atx updated this revision to Diff 289815.
chrish_ericsson_atx added a comment.

NC.  Pushing null change in hopes of re-triggering testing.

Unit test that failed is low-level LLVM test, which doesn't even exercise the 
code I've changed here, so I'm assuming it's a spurious failure in the CI
machinery.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86796

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaChecking.cpp
  clang/test/Sema/const-eval.c
  clang/test/Sema/unbounded-array-bounds.c
  clang/test/SemaCXX/constant-expression-cxx1y.cpp

Index: clang/test/SemaCXX/constant-expression-cxx1y.cpp
===
--- clang/test/SemaCXX/constant-expression-cxx1y.cpp
+++ clang/test/SemaCXX/constant-expression-cxx1y.cpp
@@ -1018,8 +1018,9 @@
 }
 
 constexpr void PR28739(int n) { // expected-error {{never produces a constant}}
-  int *p = 
+  int *p =   // expected-note {{declared here}}
   p += (__int128)(unsigned long)-1; // expected-note {{cannot refer to element 18446744073709551615 of non-array object in a constant expression}}
+  // expected-warning@-1 {{refers past the last possible element}}
 }
 
 constexpr void Void(int n) {
Index: clang/test/Sema/unbounded-array-bounds.c
===
--- /dev/null
+++ clang/test/Sema/unbounded-array-bounds.c
@@ -0,0 +1,70 @@
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -fsyntax-only %s 2>&1 | FileCheck --check-prefix=CHECK-X86-ADDR64 %s  \
+// RUN:  --implicit-check-not 'past the last possible element'
+// RUN: %clang_cc1 -triple i386-pc-linux-gnu   -fsyntax-only %s 2>&1 | FileCheck --check-prefix=CHECK-I386-ADDR32 %s \
+// RUN:  --implicit-check-not 'past the last possible element'
+// RUN: %clang_cc1 -triple avr-pc-linux-gnu-fsyntax-only %s 2>&1 | FileCheck --check-prefix=CHECK-AVR-ADDR16 %s  \
+// RUN:  --implicit-check-not 'past the last possible element'
+
+struct S {
+  long long a;
+  char b;
+  long long c;
+  short d;
+};
+
+struct S s[];
+
+void f1() {
+  ++s[3].a;
+  ++s[7073650413200313099].b;
+  // CHECK-X86-ADDR64:  :[[@LINE-1]]:5: warning: {{.*}} past the last possible element {{.*}} in 64-bit {{.*}} (max possible 576460752303423488 elements)
+  // CHECK-I386-ADDR32: :[[@LINE-2]]:5: warning: {{.*}} past the last possible element {{.*}} in 32-bit {{.*}} (max possible 178956970 elements)
+  // CHECK-AVR-ADDR16:  :[[@LINE-3]]:5: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 3276 elements)
+  ++s[7073650].c;
+  // CHECK-AVR-ADDR16:  :[[@LINE-1]]:5: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 3276 elements)
+}
+
+long long ll[];
+
+void f2() {
+  ++ll[3];
+  ++ll[2705843009213693952];
+  // CHECK-X86-ADDR64:  :[[@LINE-1]]:5: warning: {{.*}} past the last possible element {{.*}} in 64-bit {{.*}} (max possible 2305843009213693952 elements)
+  // CHECK-I386-ADDR32: :[[@LINE-2]]:5: warning: {{.*}} past the last possible element {{.*}} in 32-bit {{.*}} (max possible 536870912 elements)
+  // CHECK-AVR-ADDR16:  :[[@LINE-3]]:5: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 8192 elements)
+  ++ll[847073650];
+  // CHECK-I386-ADDR32: :[[@LINE-1]]:5: warning: {{.*}} past the last possible element {{.*}} in 32-bit {{.*}} (max possible 536870912 elements)
+  // CHECK-AVR-ADDR16:  :[[@LINE-2]]:5: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 8192 elements)
+}
+
+void f3(struct S p[]) {
+  ++p[3].a;
+  ++p[7073650413200313099].b;
+  // CHECK-X86-ADDR64:  :[[@LINE-1]]:5: warning: {{.*}} past the last possible element {{.*}} in 64-bit {{.*}} (max possible 576460752303423488 elements)
+  // CHECK-I386-ADDR32: :[[@LINE-2]]:5: warning: {{.*}} past the last possible element {{.*}} in 32-bit {{.*}} (max possible 178956970 elements)
+  // CHECK-AVR-ADDR16:  :[[@LINE-3]]:5: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 3276 elements)
+  ++p[7073650].c;
+  // CHECK-AVR-ADDR16:  :[[@LINE-1]]:5: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 3276 elements)
+}
+
+void f4(struct S *p) {
+  p += 3;
+  p += 7073650413200313099;
+  // CHECK-X86-ADDR64:  :[[@LINE-1]]:3: warning: {{.*}} past the last possible element {{.*}} in 64-bit {{.*}} (max possible 576460752303423488 elements)
+  // CHECK-I386-ADDR32: :[[@LINE-2]]:3: warning: {{.*}} past the last possible element {{.*}} in 32-bit {{.*}} (max possible 178956970 elements)
+  // CHECK-AVR-ADDR16:  :[[@LINE-3]]:3: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 3276 elements)
+  p += 7073650;
+  // CHECK-AVR-ADDR16:  :[[@LINE-1]]:3: warning: {{.*}} past the last possible element

[PATCH] D87047: [clang] Add command line options for the Machine Function Splitter.

2020-09-03 Thread Snehasish Kumar via Phabricator via cfe-commits

snehasish added a comment.

Thanks for the comments! PTAL.

In D87047#2252982 , @davidxl wrote:

> For x86 target, should it be turned on when -fprofile-use= option is 
> specified unless -fno-split-machine-function is specified?

I don't think we are there yet to turn it on for x86 profile builds. There is 
at least one unresolved issue with DebugInfo (D85085 
); once we have tested this some more 
internally we can revisit and make it the default.

> Also is it worth to give a warning if the option is specified but PGO is not 
> on?

Added a warning if no profile is specified but this option is turned on.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87047

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

[clang] 0f1be87 - [Sema] Fix a -Warc-repeated-use-of-weak false-positive by only calling CheckPlaceholderExpr once

2020-09-03 Thread Erik Pilkington via cfe-commits


Author: Erik Pilkington
Date: 2020-09-03T16:56:35-04:00
New Revision: 0f1be87e294751a0941f1d9b7785ebf4d8072149

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

LOG: [Sema] Fix a -Warc-repeated-use-of-weak false-positive by only calling 
CheckPlaceholderExpr once

Previously, this code discarded the result of CheckPlaceholderExpr for
non-matrix subexpressions. Not only is this wasteful, but it was creating a
Warc-repeated-use-of-weak false-positive on the attached testcase, since the
discarded expression was still registered as a use of the weak property.

rdar://66162246

Differential revision: https://reviews.llvm.org/D87102

Added: 


Modified: 
clang/lib/Sema/SemaExpr.cpp
clang/test/SemaObjC/arc-repeated-weak.mm

Removed: 




diff  --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 450185788537..cd71ce70c70e 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -4595,8 +4595,8 @@ Sema::ActOnArraySubscriptExpr(Scope *S, Expr *base, 
SourceLocation lbLoc,
 << SourceRange(base->getBeginLoc(), rbLoc);
 return ExprError();
   }
-  // If the base is either a MatrixSubscriptExpr or a matrix type, try to 
create
-  // a new MatrixSubscriptExpr.
+  // If the base is a MatrixSubscriptExpr, try to create a new
+  // MatrixSubscriptExpr.
   auto *matSubscriptE = dyn_cast(base);
   if (matSubscriptE) {
 if (CheckAndReportCommaError(idx))
@@ -4607,34 +4607,13 @@ Sema::ActOnArraySubscriptExpr(Scope *S, Expr *base, 
SourceLocation lbLoc,
 return CreateBuiltinMatrixSubscriptExpr(
 matSubscriptE->getBase(), matSubscriptE->getRowIdx(), idx, rbLoc);
   }
-  Expr *matrixBase = base;
-  bool IsMSPropertySubscript = isMSPropertySubscriptExpr(*this, base);
-  if (!IsMSPropertySubscript) {
-ExprResult result = CheckPlaceholderExpr(base);
-if (!result.isInvalid())
-  matrixBase = result.get();
-  }
-  if (matrixBase->getType()->isMatrixType()) {
-if (CheckAndReportCommaError(idx))
-  return ExprError();
-
-return CreateBuiltinMatrixSubscriptExpr(matrixBase, idx, nullptr, rbLoc);
-  }
-
-  // A comma-expression as the index is deprecated in C++2a onwards.
-  if (getLangOpts().CPlusPlus20 &&
-  ((isa(idx) && cast(idx)->isCommaOp()) ||
-   (isa(idx) &&
-cast(idx)->getOperator() == OO_Comma))) {
-Diag(idx->getExprLoc(), diag::warn_deprecated_comma_subscript)
-  << SourceRange(base->getBeginLoc(), rbLoc);
-  }
 
   // Handle any non-overload placeholder types in the base and index
   // expressions.  We can't handle overloads here because the other
   // operand might be an overloadable type, in which case the overload
   // resolution for the operator overload should get the first crack
   // at the overload.
+  bool IsMSPropertySubscript = false;
   if (base->getType()->isNonOverloadPlaceholderType()) {
 IsMSPropertySubscript = isMSPropertySubscriptExpr(*this, base);
 if (!IsMSPropertySubscript) {
@@ -4644,6 +4623,24 @@ Sema::ActOnArraySubscriptExpr(Scope *S, Expr *base, 
SourceLocation lbLoc,
   base = result.get();
 }
   }
+
+  // If the base is a matrix type, try to create a new MatrixSubscriptExpr.
+  if (base->getType()->isMatrixType()) {
+if (CheckAndReportCommaError(idx))
+  return ExprError();
+
+return CreateBuiltinMatrixSubscriptExpr(base, idx, nullptr, rbLoc);
+  }
+
+  // A comma-expression as the index is deprecated in C++2a onwards.
+  if (getLangOpts().CPlusPlus20 &&
+  ((isa(idx) && cast(idx)->isCommaOp()) ||
+   (isa(idx) &&
+cast(idx)->getOperator() == OO_Comma))) {
+Diag(idx->getExprLoc(), diag::warn_deprecated_comma_subscript)
+<< SourceRange(base->getBeginLoc(), rbLoc);
+  }
+
   if (idx->getType()->isNonOverloadPlaceholderType()) {
 ExprResult result = CheckPlaceholderExpr(idx);
 if (result.isInvalid()) return ExprError();

diff  --git a/clang/test/SemaObjC/arc-repeated-weak.mm 
b/clang/test/SemaObjC/arc-repeated-weak.mm
index 4eec4d2fe69c..90388598c7b8 100644
--- a/clang/test/SemaObjC/arc-repeated-weak.mm
+++ b/clang/test/SemaObjC/arc-repeated-weak.mm
@@ -485,3 +485,17 @@ void foo1() {
 
 @class NSString;
 static NSString* const kGlobal = @"";
+
+@interface NSDictionary
+- (id)objectForKeyedSubscript:(id)key;
+@end
+
+@interface WeakProp
+@property (weak) NSDictionary *nd;
+@end
+
+@implementation WeakProp
+-(void)m {
+  (void)self.nd[@""]; // no warning
+}
+@end



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

[PATCH] D87102: [Sema] Fix a -Warc-repeated-use-of-weak false-positive by only calling CheckPlaceholderExpr once

2020-09-03 Thread Erik Pilkington via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG0f1be87e2947: [Sema] Fix a -Warc-repeated-use-of-weak 
false-positive by only calling… (authored by erik.pilkington).
Herald added a project: clang.

Changed prior to commit:
  https://reviews.llvm.org/D87102?vs=289764=289806#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87102

Files:
  clang/lib/Sema/SemaExpr.cpp
  clang/test/SemaObjC/arc-repeated-weak.mm


Index: clang/test/SemaObjC/arc-repeated-weak.mm
===
--- clang/test/SemaObjC/arc-repeated-weak.mm
+++ clang/test/SemaObjC/arc-repeated-weak.mm
@@ -485,3 +485,17 @@
 
 @class NSString;
 static NSString* const kGlobal = @"";
+
+@interface NSDictionary
+- (id)objectForKeyedSubscript:(id)key;
+@end
+
+@interface WeakProp
+@property (weak) NSDictionary *nd;
+@end
+
+@implementation WeakProp
+-(void)m {
+  (void)self.nd[@""]; // no warning
+}
+@end
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -4595,8 +4595,8 @@
 << SourceRange(base->getBeginLoc(), rbLoc);
 return ExprError();
   }
-  // If the base is either a MatrixSubscriptExpr or a matrix type, try to 
create
-  // a new MatrixSubscriptExpr.
+  // If the base is a MatrixSubscriptExpr, try to create a new
+  // MatrixSubscriptExpr.
   auto *matSubscriptE = dyn_cast(base);
   if (matSubscriptE) {
 if (CheckAndReportCommaError(idx))
@@ -4607,34 +4607,13 @@
 return CreateBuiltinMatrixSubscriptExpr(
 matSubscriptE->getBase(), matSubscriptE->getRowIdx(), idx, rbLoc);
   }
-  Expr *matrixBase = base;
-  bool IsMSPropertySubscript = isMSPropertySubscriptExpr(*this, base);
-  if (!IsMSPropertySubscript) {
-ExprResult result = CheckPlaceholderExpr(base);
-if (!result.isInvalid())
-  matrixBase = result.get();
-  }
-  if (matrixBase->getType()->isMatrixType()) {
-if (CheckAndReportCommaError(idx))
-  return ExprError();
-
-return CreateBuiltinMatrixSubscriptExpr(matrixBase, idx, nullptr, rbLoc);
-  }
-
-  // A comma-expression as the index is deprecated in C++2a onwards.
-  if (getLangOpts().CPlusPlus20 &&
-  ((isa(idx) && cast(idx)->isCommaOp()) ||
-   (isa(idx) &&
-cast(idx)->getOperator() == OO_Comma))) {
-Diag(idx->getExprLoc(), diag::warn_deprecated_comma_subscript)
-  << SourceRange(base->getBeginLoc(), rbLoc);
-  }
 
   // Handle any non-overload placeholder types in the base and index
   // expressions.  We can't handle overloads here because the other
   // operand might be an overloadable type, in which case the overload
   // resolution for the operator overload should get the first crack
   // at the overload.
+  bool IsMSPropertySubscript = false;
   if (base->getType()->isNonOverloadPlaceholderType()) {
 IsMSPropertySubscript = isMSPropertySubscriptExpr(*this, base);
 if (!IsMSPropertySubscript) {
@@ -4644,6 +4623,24 @@
   base = result.get();
 }
   }
+
+  // If the base is a matrix type, try to create a new MatrixSubscriptExpr.
+  if (base->getType()->isMatrixType()) {
+if (CheckAndReportCommaError(idx))
+  return ExprError();
+
+return CreateBuiltinMatrixSubscriptExpr(base, idx, nullptr, rbLoc);
+  }
+
+  // A comma-expression as the index is deprecated in C++2a onwards.
+  if (getLangOpts().CPlusPlus20 &&
+  ((isa(idx) && cast(idx)->isCommaOp()) ||
+   (isa(idx) &&
+cast(idx)->getOperator() == OO_Comma))) {
+Diag(idx->getExprLoc(), diag::warn_deprecated_comma_subscript)
+<< SourceRange(base->getBeginLoc(), rbLoc);
+  }
+
   if (idx->getType()->isNonOverloadPlaceholderType()) {
 ExprResult result = CheckPlaceholderExpr(idx);
 if (result.isInvalid()) return ExprError();


Index: clang/test/SemaObjC/arc-repeated-weak.mm
===
--- clang/test/SemaObjC/arc-repeated-weak.mm
+++ clang/test/SemaObjC/arc-repeated-weak.mm
@@ -485,3 +485,17 @@
 
 @class NSString;
 static NSString* const kGlobal = @"";
+
+@interface NSDictionary
+- (id)objectForKeyedSubscript:(id)key;
+@end
+
+@interface WeakProp
+@property (weak) NSDictionary *nd;
+@end
+
+@implementation WeakProp
+-(void)m {
+  (void)self.nd[@""]; // no warning
+}
+@end
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -4595,8 +4595,8 @@
 << SourceRange(base->getBeginLoc(), rbLoc);
 return ExprError();
   }
-  // If the base is either a MatrixSubscriptExpr or a matrix type, try to create
-  // a new MatrixSubscriptExpr.
+  // If the base is a MatrixSubscriptExpr, try to create a new
+  // MatrixSubscriptExpr.
   auto *matSubscriptE = dyn_cast(base);
   if

[PATCH] D87047: [clang] Add command line options for the Machine Function Splitter.

2020-09-03 Thread Snehasish Kumar via Phabricator via cfe-commits

snehasish updated this revision to Diff 289805.
snehasish added a comment.

Add warning when option is enabled without profile.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87047

Files:
  clang/docs/ClangCommandLineReference.rst
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/split-machine-functions.c
  clang/test/Driver/fsplit-machine-functions.c

Index: clang/test/Driver/fsplit-machine-functions.c
===
--- /dev/null
+++ clang/test/Driver/fsplit-machine-functions.c
@@ -0,0 +1,4 @@
+// RUN: %clang -### -fsplit-machine-functions %s -c 2>&1 | FileCheck -check-prefix=CHECK-OPT %s
+// RUN: %clang -### -fsplit-machine-functions -fno-split-machine-functions %s -c 2>&1 | FileCheck -check-prefix=CHECK-NOOPT %s
+// CHECK-OPT: "-fsplit-machine-functions"
+// CHECK-NOOPT-NOT: "-fsplit-machine-functions"
Index: clang/test/CodeGen/split-machine-functions.c
===
--- /dev/null
+++ clang/test/CodeGen/split-machine-functions.c
@@ -0,0 +1,34 @@
+// REQUIRES: x86-registered-target
+
+// RUN: echo "foo"> %t.proftext
+// RUN: echo "# Func Hash:"   >> %t.proftext
+// RUN: echo "11262309905">> %t.proftext
+// RUN: echo "# Num Counters:">> %t.proftext
+// RUN: echo "2"  >> %t.proftext
+// RUN: echo "# Counter Values:"  >> %t.proftext
+// RUN: echo "100">> %t.proftext
+// RUN: echo "0"  >> %t.proftext
+// RUN: llvm-profdata merge -o %t.profdata %t.proftext
+// RUN: %clang_cc1 -triple x86_64 -O3 -S -fprofile-instrument-use-path=%t.profdata -fsplit-machine-functions -o - < %s | FileCheck %s
+
+__attribute__((noinline)) int foo(int argc) {
+  if (argc % 2 == 0) {
+exit(argc);
+  } else {
+return argc + 1;
+  }
+}
+
+int main(int argc, char *argv[]) {
+  int total = 0;
+  for (int i = 0; i < 100; ++i) {
+total += foo(argc);
+  }
+  printf("%d\n", total);
+}
+
+// CHECK: .section .text.hot.,"ax",@progbits
+// CHECK: foo:
+// CHECK: section .text.unlikely.foo,"ax",@progbits
+// CHECK: foo.cold:
+// CHECK: callq exit@PLT
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -998,6 +998,8 @@
   Opts.UniqueInternalLinkageNames =
   Args.hasArg(OPT_funique_internal_linkage_names);
 
+  Opts.SplitMachineFunctions = Args.hasArg(OPT_fsplit_machine_functions);
+
   Opts.MergeFunctions = Args.hasArg(OPT_fmerge_functions);
 
   Opts.NoUseJumpTables = Args.hasArg(OPT_fno_jump_tables);
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4255,6 +4255,8 @@
 options::OPT_fno_unique_section_names,
 options::OPT_funique_basic_block_section_names,
 options::OPT_fno_unique_basic_block_section_names,
+options::OPT_fsplit_machine_functions,
+options::OPT_fno_split_machine_functions,
 options::OPT_mrestrict_it,
 options::OPT_mno_restrict_it,
 options::OPT_mstackrealign,
@@ -4905,6 +4907,10 @@
options::OPT_fno_unique_basic_block_section_names, false))
 CmdArgs.push_back("-funique-basic-block-section-names");
 
+  if (Args.hasFlag(options::OPT_fsplit_machine_functions,
+   options::OPT_fno_split_machine_functions, false))
+CmdArgs.push_back("-fsplit-machine-functions");
+
   Args.AddLastArg(CmdArgs, options::OPT_finstrument_functions,
   options::OPT_finstrument_functions_after_inlining,
   options::OPT_finstrument_function_entry_bare);
Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -514,6 +514,13 @@
   Options.BBSectionsFuncListBuf = std::move(*MBOrErr);
   }
 
+  if (CodeGenOpts.SplitMachineFunctions) {
+if (CodeGenOpts.getProfileUse() != CodeGenOptions::ProfileNone)
+  Options.EnableMachineFunctionSplitter = true;
+else
+  Diags.Report(diag::warn_fe_machine_function_splitter_missing_profile);
+  }
+
   Options.FunctionSections = CodeGenOpts.FunctionSections;
   Options.DataSections = CodeGenOpts.DataSections;
   Options.UniqueSectionNames = CodeGenOpts.UniqueSectionNames;
Index: clang/include/clang/Driver/Options.td
===
---

[PATCH] D86790: [FE] Use preferred alignment instead of ABI alignment for complete object when applicable

2020-09-03 Thread James Y Knight via Phabricator via cfe-commits

jyknight added a comment.

Do you have open questions on whether some callsites passing "false" here, 
should be switched to true? Given what's here, I would say that it definitely 
does not makes sense to add this parameter everywhere.

So, for getting something committed: please send a new review which makes only 
the required changes, splitting by logical part of the code. E.g. one change to 
fix the new/delete alignment, one for the global-variable alignment, and so on 
if there are others.




Comment at: clang/lib/CodeGen/CGAtomic.cpp:814
+  std::tie(sizeChars, alignChars) = getContext().getTypeInfoInChars(
+  AtomicTy, true /* NeedsPreferredAlignment */);
   uint64_t Size = sizeChars.getQuantity();

This is wrong.



Comment at: clang/lib/StaticAnalyzer/Checkers/PaddingChecker.cpp:252
+  std::tie(RetVal.Size, RetVal.Align) = Ctx.getTypeInfoInChars(
+  FD->getType(), true /* NeedsPreferredAlignment */);
   assert(llvm::isPowerOf2_64(RetVal.Align.getQuantity()));

Not sure if this is right, since it's looking at individual fields in the 
struct?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86790

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

[PATCH] D86660: Modifying ImportDeclContext(...) to ensure that we also handle the case when the FieldDecl is an ArrayType whose ElementType is a RecordDecl

2020-09-03 Thread Shafik Yaghmour via Phabricator via cfe-commits

shafik marked an inline comment as done.
shafik added a subscriber: rsmith.
shafik added a comment.

@martong I have been experimenting w/ how we pass around `ImportDefinitionKind` 
and pushing it through to more places does not help. I also tried defaulting 
most locations to `IDK_Everything` to experiment to see if maybe I was missing 
some crucial point and no luck. So while it seemed like a good direction it has 
not worked out, unless I missed something in your idea.

So my current approach may be the best we have until we can do some larger 
refactoring.

I had explored trying to fix this from the codegen and sema side of things but 
after discussing this w/ @rsmith there is no simple fix there that would not 
require some large refactoring on the LLDB side.


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

https://reviews.llvm.org/D86660

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

[PATCH] D54943: [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

2020-09-03 Thread Alexander Lanin via Phabricator via cfe-commits

AlexanderLanin added a comment.

Observed behavior:

Change: `for(string_view token : split_into_views(file_content, " \t\r\n"))` 
--> `for(string_view const token : split_into_views(file_content, " \t\r\n"))`.
Note: `std::vector split_into_views(string_view input, const char* 
separators);`
Problem: Now it triggers `error: loop variable 'token' of type 'const 
nonstd::sv_lite::string_view' (aka 'const basic_string_view') creates a 
copy from type 'const nonstd::sv_lite::string_view' 
[-Werror,-Wrange-loop-analysis]`
Fixed manually by making it `for(std::string_view& const token : 
split_into_views(file_content, " \t\r\n"))`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D54943

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

[PATCH] D86369: [Sema][MSVC] warn at dynamic_cast when /GR- is given

2020-09-03 Thread Zequan Wu via Phabricator via cfe-commits

zequanwu updated this revision to Diff 289802.
zequanwu added a comment.

address comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86369

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaCast.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/SemaCXX/ms_no_dynamic_cast.cpp
  clang/test/SemaCXX/no_dynamic_cast.cpp

Index: clang/test/SemaCXX/no_dynamic_cast.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/no_dynamic_cast.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 %s -fno-rtti-data -fsyntax-only -verify
+
+namespace std {
+struct type_info {};
+} // namespace std
+class B {
+public:
+  virtual ~B() = default;
+};
+
+class D1 : public B {
+public:
+  ~D1() = default;
+};
+
+void f() {
+  B *b = new D1();
+  auto d = dynamic_cast(b); // expected-warning{{dynamic_cast will not work since RTTI data is disabled by -fno-rtti-data}}
+  (void)typeid(int);  // expected-warning{{typeid will not work since RTTI data is disabled by -fno-rtti-data}}
+}
Index: clang/test/SemaCXX/ms_no_dynamic_cast.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/ms_no_dynamic_cast.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 %s -fdiagnostics-format msvc -fno-rtti-data -fsyntax-only -verify
+
+namespace std {
+struct type_info {};
+} // namespace std
+class B {
+public:
+  virtual ~B() = default;
+};
+
+class D1 : public B {
+public:
+  ~D1() = default;
+};
+
+void f() {
+  B *b = new D1();
+  auto d = dynamic_cast(b); // expected-warning{{dynamic_cast will not work since RTTI data is disabled by /GR-}}
+  (void)typeid(int);  // expected-warning{{typeid will not work since RTTI data is disabled by /GR-}}
+}
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -646,6 +646,12 @@
 return ExprError(Diag(OpLoc, diag::err_no_typeid_with_fno_rtti));
   }
 
+  // Warns when typeid is used with RTTI data disabled.
+  if (!getLangOpts().RTTIData)
+Diag(OpLoc, diag::warn_no_typeid_with_rtti_disabled)
+<< (getDiagnostics().getDiagnosticOptions().getFormat() ==
+DiagnosticOptions::MSVC);
+
   QualType TypeInfoType = Context.getTypeDeclType(CXXTypeInfoDecl);
 
   if (isType) {
Index: clang/lib/Sema/SemaCast.cpp
===
--- clang/lib/Sema/SemaCast.cpp
+++ clang/lib/Sema/SemaCast.cpp
@@ -890,6 +890,18 @@
 return;
   }
 
+  // Warns when dynamic_cast is used with RTTI data disabled.
+  if (!Self.getLangOpts().RTTIData) {
+bool MicrosoftABI =
+Self.getASTContext().getTargetInfo().getCXXABI().isMicrosoft();
+bool isClangCL = Self.getDiagnostics().getDiagnosticOptions().getFormat() ==
+ DiagnosticOptions::MSVC;
+if (MicrosoftABI || !DestPointee->isVoidType())
+  Self.Diag(OpRange.getBegin(),
+diag::warn_no_dynamic_cast_with_rtti_disabled)
+  << isClangCL;
+  }
+
   // Done. Everything else is run-time checks.
   Kind = CK_Dynamic;
 }
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -7433,6 +7433,12 @@
   "use of typeid requires -frtti">;
 def err_no_dynamic_cast_with_fno_rtti : Error<
   "use of dynamic_cast requires -frtti">;
+def warn_no_dynamic_cast_with_rtti_disabled: Warning<
+  "dynamic_cast will not work since RTTI data is disabled by " 
+  "%select{-fno-rtti-data|/GR-}0">, InGroup;
+def warn_no_typeid_with_rtti_disabled: Warning<
+  "typeid will not work since RTTI data is disabled by "
+  "%select{-fno-rtti-data|/GR-}0">, InGroup;
 
 def err_cannot_form_pointer_to_member_of_reference_type : Error<
   "cannot form a pointer-to-member to member %0 of reference type %1">;
Index: clang/include/clang/Basic/DiagnosticGroups.td
===
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -1229,3 +1229,5 @@
 }
 
 def WebAssemblyExceptionSpec : DiagGroup<"wasm-exception-spec">;
+
+def RTTI : DiagGroup<"rtti">;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D87102: [Sema] Fix a -Warc-repeated-use-of-weak false-positive by only calling CheckPlaceholderExpr once

2020-09-03 Thread Florian Hahn via Phabricator via cfe-commits

fhahn accepted this revision.
fhahn added a comment.

LGTM, thanks for addressing this! Please wait a bit with committing, in case 
there are additional comments.




Comment at: clang/lib/Sema/SemaExpr.cpp:4598
   }
   // If the base is either a MatrixSubscriptExpr or a matrix type, try to 
create
   // a new MatrixSubscriptExpr.

I think it would be good to split the comment into 2. The case below is `base 
is a MatrixSubscriptExpr`, and further down we handle `base is a matrix type`.


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

https://reviews.llvm.org/D87102

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

[PATCH] D87102: [Sema] Fix a -Warc-repeated-use-of-weak false-positive by only calling CheckPlaceholderExpr once

2020-09-03 Thread John McCall via Phabricator via cfe-commits

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

LGTM


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

https://reviews.llvm.org/D87102

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

[PATCH] D86999: getClangStripDependencyFileAdjuster(): Do not remove -M args when using MSVC cl driver

2020-09-03 Thread Shivanshu Goyal via Phabricator via cfe-commits

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



Comment at: clang/lib/Tooling/ArgumentsAdjusters.cpp:117
+  // do not remove those when using the cl driver.
+  bool IsDependencyFileArg;
+  if (Arg.startswith("/showIncludes") || Arg.startswith("-showIncludes"))

hans wrote:
> shivanshu3 wrote:
> > hans wrote:
> > > Instead of a bool and if below, I'd suggest if-statements and early 
> > > continues instead. Breaking up the old if-statement is nice though, and 
> > > maybe the comment from above about what flags to ignore could be moved 
> > > down here to those if statements. For example:
> > > 
> > > 
> > > ```
> > > // -M flags blah blah
> > > if (Arg.startswith("-M") && !UsingClDriver)
> > >   continue;
> > > // MSVC flags blah blah
> > > if (Arg.startswith("/showIncludes") || Arg.startswith("-showIncludes"))
> > >   continue;
> > > AdjustedArgs.push_back(Args[i]);
> > > ```
> > I realized that with my original change, we would skip the next argument 
> > under cl driver mode when -MT was used, which would be wrong. The next 
> > argument should only be skipped when `IsDependencyFileArg` is true. So I 
> > think it might be cleaner to keep that extra boolean so the code is easy to 
> > read and understand. What do you think?
> I think having the boolean variable is still more confusing (it adds more 
> state to keep track of) than just handling the cases with if-statements and 
> early continue.
> 
> How about:
> 
> ```
> // -M flags that take an arg..
> if (!UsingClDriver && (Arg == "-MF" || Arg == "-MT" || Arg == "-MQ")) {
>   ++i;
>   continue;
> }
> // -M flags blah blah
> if (!UsingClDriver && Arg.startswith("-M"))
>   continue;
> // MSVC flags blah blah
> if (Arg.startswith("/showIncludes") || Arg.startswith("-showIncludes"))
>   continue;
> 
> AdjustedArgs.push_back(Args[i]);
> ```
> 
> ?
Yup I agree that looks better :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86999

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

[PATCH] D86999: getClangStripDependencyFileAdjuster(): Do not remove -M args when using MSVC cl driver

2020-09-03 Thread Shivanshu Goyal via Phabricator via cfe-commits

shivanshu3 updated this revision to Diff 289793.
shivanshu3 added a comment.

- Remove the bool `IsDependencyFileArg` in the implementation of 
`getClangStripDependencyFileAdjuster()` to make it simpler.
- Add an extra argument after -MT in the test case to ensure we do not strip 
arguments after -MT when using the cl driver mode.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86999

Files:
  clang/lib/Tooling/ArgumentsAdjusters.cpp
  clang/unittests/Tooling/ToolingTest.cpp

Index: clang/unittests/Tooling/ToolingTest.cpp
===
--- clang/unittests/Tooling/ToolingTest.cpp
+++ clang/unittests/Tooling/ToolingTest.cpp
@@ -563,6 +563,40 @@
   EXPECT_TRUE(HasFlag("-c"));
 }
 
+// Check getClangStripDependencyFileAdjuster doesn't strip args when using the
+// MSVC cl.exe driver
+TEST(ClangToolTest, StripDependencyFileAdjusterMsvc) {
+  FixedCompilationDatabase Compilations(
+  "/", {"--driver-mode=cl", "-MD", "-MDd", "-MT", "-O1", "-MTd", "-MP"});
+
+  ClangTool Tool(Compilations, std::vector(1, "/a.cc"));
+  Tool.mapVirtualFile("/a.cc", "void a() {}");
+
+  std::unique_ptr Action(
+  newFrontendActionFactory());
+
+  CommandLineArguments FinalArgs;
+  ArgumentsAdjuster CheckFlagsAdjuster =
+  [](const CommandLineArguments , StringRef /*unused*/) {
+FinalArgs = Args;
+return Args;
+  };
+  Tool.clearArgumentsAdjusters();
+  Tool.appendArgumentsAdjuster(getClangStripDependencyFileAdjuster());
+  Tool.appendArgumentsAdjuster(CheckFlagsAdjuster);
+  Tool.run(Action.get());
+
+  auto HasFlag = [](const std::string ) {
+return llvm::find(FinalArgs, Flag) != FinalArgs.end();
+  };
+  EXPECT_TRUE(HasFlag("-MD"));
+  EXPECT_TRUE(HasFlag("-MDd"));
+  EXPECT_TRUE(HasFlag("-MT"));
+  EXPECT_TRUE(HasFlag("-O1"));
+  EXPECT_TRUE(HasFlag("-MTd"));
+  EXPECT_TRUE(HasFlag("-MP"));
+}
+
 // Check getClangStripPluginsAdjuster strips plugin related args.
 TEST(ClangToolTest, StripPluginsAdjuster) {
   FixedCompilationDatabase Compilations(
Index: clang/lib/Tooling/ArgumentsAdjusters.cpp
===
--- clang/lib/Tooling/ArgumentsAdjusters.cpp
+++ clang/lib/Tooling/ArgumentsAdjusters.cpp
@@ -21,6 +21,16 @@
 namespace clang {
 namespace tooling {
 
+static StringRef getDriverMode(const CommandLineArguments ) {
+  for (const auto  : Args) {
+StringRef ArgRef = Arg;
+if (ArgRef.consume_front("--driver-mode=")) {
+  return ArgRef;
+}
+  }
+  return StringRef();
+}
+
 /// Add -fsyntax-only option and drop options that triggers output generation.
 ArgumentsAdjuster getClangSyntaxOnlyAdjuster() {
   return [](const CommandLineArguments , StringRef /*unused*/) {
@@ -93,20 +103,28 @@
 
 ArgumentsAdjuster getClangStripDependencyFileAdjuster() {
   return [](const CommandLineArguments , StringRef /*unused*/) {
+auto UsingClDriver = (getDriverMode(Args) == "cl");
+
 CommandLineArguments AdjustedArgs;
 for (size_t i = 0, e = Args.size(); i < e; ++i) {
   StringRef Arg = Args[i];
-  // All dependency-file options begin with -M. These include -MM,
-  // -MF, -MG, -MP, -MT, -MQ, -MD, and -MMD.
-  if (!Arg.startswith("-M") && !Arg.startswith("/showIncludes") &&
-  !Arg.startswith("-showIncludes")) {
-AdjustedArgs.push_back(Args[i]);
+
+  // These flags take an argument: -MX foo. Skip the next argument also.
+  if (!UsingClDriver && (Arg == "-MF" || Arg == "-MT" || Arg == "-MQ")) {
+++i;
 continue;
   }
+  // When not using the cl driver mode, dependency file generation options
+  // begin with -M. These include -MM, -MF, -MG, -MP, -MT, -MQ, -MD, and
+  // -MMD.
+  if (!UsingClDriver && Arg.startswith("-M"))
+continue;
+  // Under MSVC's cl driver mode, dependency file generation is controlled
+  // using /showIncludes
+  if (Arg.startswith("/showIncludes") || Arg.startswith("-showIncludes"))
+continue;
 
-  if (Arg == "-MF" || Arg == "-MT" || Arg == "-MQ")
-// These flags take an argument: -MX foo. Skip the next argument also.
-++i;
+  AdjustedArgs.push_back(Args[i]);
 }
 return AdjustedArgs;
   };
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D87101: [X86] Update SSE/AVX ABS intrinsics to emit llvm.abs.* (PR46851)

2020-09-03 Thread Nikita Popov via Phabricator via cfe-commits

nikic added a comment.

I think our abs intrinsic support is already sufficient to start canonicalizing 
to the intrinsic variant (the same is not true for min/max intrinsics). We 
might want to make the InstCombine change before this one, to make sure we 
don't lose CSE opportunities between the intrinsic and the expanded forms. 
Though if that's not a concern for these vector intrinsics, then this LG to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87101

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

[PATCH] D86369: [Sema][MSVC] warn at dynamic_cast when /GR- is given

2020-09-03 Thread Hans Wennborg via Phabricator via cfe-commits

hans added inline comments.



Comment at: clang/lib/Sema/SemaCast.cpp:897
+  DiagnosticOptions::MSVC;
+if (isMSVC || !DestPointee->isVoidType())
+  Self.Diag(OpRange.getBegin(),

zequanwu wrote:
> hans wrote:
> > I don't think the if-statement is necessary. I thought we concluded we want 
> > to warn even for void*? Also note that "isMSVC" here is only checking the 
> > driver mode, i.e. just the user interface to the compiler. The user could 
> > still be compiling in MSVC mode.
> My bad. I thought you meant to use the previous version, so I reverted this 
> region.
> Like we concluded, we want to warn even for void*. This only applies to 
> clang-cl, not clang, right? This is the purpose of this if. If it's in 
> clang-cl, warn regardless of destination type. If it's in clang, don't warn 
> for void*, like line 887 which doesn't emit error for void* destination type. 
> 
> If RTTI is false, RTTIData is also false 
> (https://github.com/llvm/llvm-project/blob/master/clang/lib/Frontend/CompilerInvocation.cpp#L2870).
>  There is a test 
> https://github.com/llvm/llvm-project/blob/master/clang/test/SemaCXX/no-rtti.cpp#L28,
>  which should not be warned, right?
> Like we concluded, we want to warn even for void*. This only applies to 
> clang-cl, not clang, right?

Oh, right, we need to do the "even for void*" part only when using the 
Microsoft ABI. It's not about clang or clang-cl, those are just different user 
interfaces really, but about what platform (processor, abi, etc.) the compiler 
is targeting. It's possible to target Windows with regular clang, not just 
clang-cl.

Inside Sema, you can check Context.getTargetInfo().getCXXABI().isMicrosoft() to 
see if the microsoft abi is being targeted.

Apologies for my confusing comments here.



Comment at: clang/test/SemaCXX/ms_no_dynamic_cast.cpp:19
+  auto d = dynamic_cast(b); // expected-warning{{dynamic_cast will not 
work since RTTI data is disabled by /GR-}}
+  (void)typeid(int);  // expected-warning{{typeid will not work 
since RTTI data is disabled by /GR-}}
+}

zequanwu wrote:
> hans wrote:
> > Is the cast to void necessary? Same comment for the next file.
> Yes, to suppress the warning of unused expression.
Ah, okay. Didn't realize that warning was on by default :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86369

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

[PATCH] D86508: [clang] improve GCC-compat for C90 __builtin_ functions

2020-09-03 Thread Richard Smith - zygoloid via Phabricator via cfe-commits

rsmith added a comment.

It seems to me that adding new `__builtin_*` functions for GCC compatibility 
and adding new `LIBBUILTIN`s are unrelated changes, and it might be clearer to 
split them up into two commits.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86508

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

[PATCH] D86508: [clang] improve GCC-compat for C90 __builtin_ functions

2020-09-03 Thread Richard Smith - zygoloid via Phabricator via cfe-commits

rsmith added inline comments.



Comment at: clang/docs/LanguageExtensions.rst:2370-2398
+* ``bcmp``
 * ``memchr``
-* ``memcmp`` (and its deprecated BSD / POSIX alias ``bcmp``)
+* ``memcmp``
+* ``memcpy``
+* ``memmove``
+* ``memset``
+* ``strcat``

aaron.ballman wrote:
> Can you mention the deprecation issue here?
Do we really provide constant evaluation support for all of these functions?



Comment at: clang/docs/LanguageExtensions.rst:2381
+* ``strcspn``
+* ``strerror``
 * ``strlen``

Do we really provide `__builtin_strerror`? I don't see it below.



Comment at: clang/docs/LanguageExtensions.rst:2425-2435
-Clang provides constant expression evaluation support for builtin forms of the
-following functions from the C standard library headers
- and :
-
-* ``memcpy``
-* ``memmove``
-* ``wmemcpy``

You've moved this from the section where we describe the constant evaluation 
rules for these functions to a section where we describe the constant 
evaluation rules for string functions; those rules are quite different.



Comment at: clang/docs/LanguageExtensions.rst:2427-2428
+
+Clang provides support for builtins forms of the following functions from the C
+standard library header :
+

We should say how these "builtin forms" are named and what they're for: that we 
provide a `__builtin_`-prefixed version of each of these functions that has the 
same signature and semantics as the function from the C standard library but 
doesn't require a header to be included.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86508

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

[PATCH] D87103: [test] Use %t instead of %T to remove race conditions between config-file3.c and target-override.c

2020-09-03 Thread Fangrui Song via Phabricator via cfe-commits

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG6e09722b27ed: [test] Use %t instead of %T to remove race 
conditions between config-file3.c… (authored by MaskRay).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87103

Files:
  clang/test/Driver/config-file3.c
  clang/test/Driver/target-override.c

Index: clang/test/Driver/target-override.c
===
--- clang/test/Driver/target-override.c
+++ clang/test/Driver/target-override.c
@@ -1,16 +1,15 @@
 // REQUIRES: shell
 // REQUIRES: x86-registered-target
 
-// RUN: rm -rf %T/testbin
-// RUN: mkdir -p %T/testbin
-// RUN: ln -s %clang %T/testbin/i386-clang
+// RUN: rm -rf %t && mkdir %t
+// RUN: ln -s %clang %t/i386-clang
 
 // Check if invocation of "foo-clang" adds option "-target foo".
 //
-// RUN: %T/testbin/i386-clang -c -no-canonical-prefixes %s -### 2>&1 | FileCheck -check-prefix CHECK-TG1 %s
+// RUN: %t/i386-clang -c -no-canonical-prefixes %s -### 2>&1 | FileCheck -check-prefix CHECK-TG1 %s
 // CHECK-TG1: Target: i386
 
 // Check if invocation of "foo-clang -target bar" overrides option "-target foo".
 //
-// RUN: %T/testbin/i386-clang -c -no-canonical-prefixes -target x86_64 %s -### 2>&1 | FileCheck -check-prefix CHECK-TG2 %s
+// RUN: %t/i386-clang -c -no-canonical-prefixes -target x86_64 %s -### 2>&1 | FileCheck -check-prefix CHECK-TG2 %s
 // CHECK-TG2: Target: x86_64
Index: clang/test/Driver/config-file3.c
===
--- clang/test/Driver/config-file3.c
+++ clang/test/Driver/config-file3.c
@@ -1,14 +1,15 @@
 // REQUIRES: shell
 // REQUIRES: x86-registered-target
 
+// RUN: rm -rf %t && mkdir %t
+
 //--- If config file is specified by relative path (workdir/cfg-s2), it is searched for by that path.
+
+// RUN: mkdir -p %t/workdir/subdir
+// RUN: echo "@subdir/cfg-s2" > %t/workdir/cfg-1
+// RUN: echo "-Wundefined-var-template" > %t/workdir/subdir/cfg-s2
 //
-// RUN: mkdir -p %T/workdir
-// RUN: echo "@subdir/cfg-s2" > %T/workdir/cfg-1
-// RUN: mkdir -p %T/workdir/subdir
-// RUN: echo "-Wundefined-var-template" > %T/workdir/subdir/cfg-s2
-//
-// RUN: ( cd %T && %clang --config workdir/cfg-1 -c %s -### 2>&1 | FileCheck %s -check-prefix CHECK-REL )
+// RUN: ( cd %t && %clang --config workdir/cfg-1 -c %s -### 2>&1 | FileCheck %s -check-prefix CHECK-REL )
 //
 // CHECK-REL: Configuration file: {{.*}}/workdir/cfg-1
 // CHECK-REL: -Wundefined-var-template
@@ -16,12 +17,11 @@
 
 //--- Invocation qqq-clang-g++ tries to find config file qqq-clang-g++.cfg first.
 //
-// RUN: rm -rf %T/testdmode
-// RUN: mkdir -p %T/testdmode
-// RUN: ln -s %clang %T/testdmode/qqq-clang-g++
-// RUN: echo "-Wundefined-func-template" > %T/testdmode/qqq-clang-g++.cfg
-// RUN: echo "-Werror" > %T/testdmode/qqq.cfg
-// RUN: %T/testdmode/qqq-clang-g++ --config-system-dir= --config-user-dir= -c -no-canonical-prefixes %s -### 2>&1 | FileCheck %s -check-prefix FULL-NAME
+// RUN: mkdir %t/testdmode
+// RUN: ln -s %clang %t/testdmode/qqq-clang-g++
+// RUN: echo "-Wundefined-func-template" > %t/testdmode/qqq-clang-g++.cfg
+// RUN: echo "-Werror" > %t/testdmode/qqq.cfg
+// RUN: %t/testdmode/qqq-clang-g++ --config-system-dir= --config-user-dir= -c -no-canonical-prefixes %s -### 2>&1 | FileCheck %s -check-prefix FULL-NAME
 //
 // FULL-NAME: Configuration file: {{.*}}/testdmode/qqq-clang-g++.cfg
 // FULL-NAME: -Wundefined-func-template
@@ -31,20 +31,20 @@
 // (As the clang executable and symlink are in different directories, this
 // requires specifying the path via --config-*-dir= though.)
 //
-// RUN: %T/testdmode/qqq-clang-g++ --config-system-dir= --config-user-dir=%T/testdmode -c %s -### 2>&1 | FileCheck %s -check-prefix SYMLINK
+// RUN: %t/testdmode/qqq-clang-g++ --config-system-dir= --config-user-dir=%t/testdmode -c %s -### 2>&1 | FileCheck %s -check-prefix SYMLINK
 //
 // SYMLINK: Configuration file: {{.*}}/testdmode/qqq-clang-g++.cfg
 //
 //--- File specified by --config overrides config inferred from clang executable.
 //
-// RUN: %T/testdmode/qqq-clang-g++ --config-system-dir=%S/Inputs/config --config-user-dir= --config i386-qqq -c -no-canonical-prefixes %s -### 2>&1 | FileCheck %s -check-prefix CHECK-EXPLICIT
+// RUN: %t/testdmode/qqq-clang-g++ --config-system-dir=%S/Inputs/config --config-user-dir= --config i386-qqq -c -no-canonical-prefixes %s -### 2>&1 | FileCheck %s -check-prefix CHECK-EXPLICIT
 //
 // CHECK-EXPLICIT: Configuration file: {{.*}}/Inputs/config/i386-qqq.cfg
 //
 //--- Invocation qqq-clang-g++ tries to find config file qqq.cfg if qqq-clang-g++.cfg is not found.
 //
-// RUN: rm %T/testdmode/qqq-clang-g++.cfg
-// RUN: %T/testdmode/qqq-clang-g++ --config-system-dir= --config-user-dir= -c -no-canonical-prefixes %s -### 2>&1 | FileCheck %s -check-prefix SHORT-NAME
+// RUN: rm

[clang] 6e09722 - [test] Use %t instead of %T to remove race conditions between config-file3.c and target-override.c

2020-09-03 Thread Fangrui Song via cfe-commits


Author: Fangrui Song
Date: 2020-09-03T12:28:53-07:00
New Revision: 6e09722b27ed4d48dfc668b0efc2aed88d701ebf

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

LOG: [test] Use %t instead of %T to remove race conditions between 
config-file3.c and target-override.c

Both tests operate on `%T/testbin`. If the two tests run concurrently,
one may fail.

This is likely the root cause of flaky failures reported by
https://lists.llvm.org/pipermail/llvm-dev/2020-September/144781.html

https://llvm.org/docs/CommandGuide/lit.html says:

`%T parent directory of %t (not unique, deprecated, do not use)`

Reviewed By: dblaikie

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

Added: 


Modified: 
clang/test/Driver/config-file3.c
clang/test/Driver/target-override.c

Removed: 




diff  --git a/clang/test/Driver/config-file3.c 
b/clang/test/Driver/config-file3.c
index 148646c2ebbf..fc5c286553ad 100644
--- a/clang/test/Driver/config-file3.c
+++ b/clang/test/Driver/config-file3.c
@@ -1,14 +1,15 @@
 // REQUIRES: shell
 // REQUIRES: x86-registered-target
 
+// RUN: rm -rf %t && mkdir %t
+
 //--- If config file is specified by relative path (workdir/cfg-s2), it is 
searched for by that path.
+
+// RUN: mkdir -p %t/workdir/subdir
+// RUN: echo "@subdir/cfg-s2" > %t/workdir/cfg-1
+// RUN: echo "-Wundefined-var-template" > %t/workdir/subdir/cfg-s2
 //
-// RUN: mkdir -p %T/workdir
-// RUN: echo "@subdir/cfg-s2" > %T/workdir/cfg-1
-// RUN: mkdir -p %T/workdir/subdir
-// RUN: echo "-Wundefined-var-template" > %T/workdir/subdir/cfg-s2
-//
-// RUN: ( cd %T && %clang --config workdir/cfg-1 -c %s -### 2>&1 | FileCheck 
%s -check-prefix CHECK-REL )
+// RUN: ( cd %t && %clang --config workdir/cfg-1 -c %s -### 2>&1 | FileCheck 
%s -check-prefix CHECK-REL )
 //
 // CHECK-REL: Configuration file: {{.*}}/workdir/cfg-1
 // CHECK-REL: -Wundefined-var-template
@@ -16,12 +17,11 @@
 
 //--- Invocation qqq-clang-g++ tries to find config file qqq-clang-g++.cfg 
first.
 //
-// RUN: rm -rf %T/testdmode
-// RUN: mkdir -p %T/testdmode
-// RUN: ln -s %clang %T/testdmode/qqq-clang-g++
-// RUN: echo "-Wundefined-func-template" > %T/testdmode/qqq-clang-g++.cfg
-// RUN: echo "-Werror" > %T/testdmode/qqq.cfg
-// RUN: %T/testdmode/qqq-clang-g++ --config-system-dir= --config-user-dir= -c 
-no-canonical-prefixes %s -### 2>&1 | FileCheck %s -check-prefix FULL-NAME
+// RUN: mkdir %t/testdmode
+// RUN: ln -s %clang %t/testdmode/qqq-clang-g++
+// RUN: echo "-Wundefined-func-template" > %t/testdmode/qqq-clang-g++.cfg
+// RUN: echo "-Werror" > %t/testdmode/qqq.cfg
+// RUN: %t/testdmode/qqq-clang-g++ --config-system-dir= --config-user-dir= -c 
-no-canonical-prefixes %s -### 2>&1 | FileCheck %s -check-prefix FULL-NAME
 //
 // FULL-NAME: Configuration file: {{.*}}/testdmode/qqq-clang-g++.cfg
 // FULL-NAME: -Wundefined-func-template
@@ -31,20 +31,20 @@
 // (As the clang executable and symlink are in 
diff erent directories, this
 // requires specifying the path via --config-*-dir= though.)
 //
-// RUN: %T/testdmode/qqq-clang-g++ --config-system-dir= 
--config-user-dir=%T/testdmode -c %s -### 2>&1 | FileCheck %s -check-prefix 
SYMLINK
+// RUN: %t/testdmode/qqq-clang-g++ --config-system-dir= 
--config-user-dir=%t/testdmode -c %s -### 2>&1 | FileCheck %s -check-prefix 
SYMLINK
 //
 // SYMLINK: Configuration file: {{.*}}/testdmode/qqq-clang-g++.cfg
 //
 //--- File specified by --config overrides config inferred from clang 
executable.
 //
-// RUN: %T/testdmode/qqq-clang-g++ --config-system-dir=%S/Inputs/config 
--config-user-dir= --config i386-qqq -c -no-canonical-prefixes %s -### 2>&1 | 
FileCheck %s -check-prefix CHECK-EXPLICIT
+// RUN: %t/testdmode/qqq-clang-g++ --config-system-dir=%S/Inputs/config 
--config-user-dir= --config i386-qqq -c -no-canonical-prefixes %s -### 2>&1 | 
FileCheck %s -check-prefix CHECK-EXPLICIT
 //
 // CHECK-EXPLICIT: Configuration file: {{.*}}/Inputs/config/i386-qqq.cfg
 //
 //--- Invocation qqq-clang-g++ tries to find config file qqq.cfg if 
qqq-clang-g++.cfg is not found.
 //
-// RUN: rm %T/testdmode/qqq-clang-g++.cfg
-// RUN: %T/testdmode/qqq-clang-g++ --config-system-dir= --config-user-dir= -c 
-no-canonical-prefixes %s -### 2>&1 | FileCheck %s -check-prefix SHORT-NAME
+// RUN: rm %t/testdmode/qqq-clang-g++.cfg
+// RUN: %t/testdmode/qqq-clang-g++ --config-system-dir= --config-user-dir= -c 
-no-canonical-prefixes %s -### 2>&1 | FileCheck %s -check-prefix SHORT-NAME
 //
 // SHORT-NAME: Configuration file: {{.*}}/testdmode/qqq.cfg
 // SHORT-NAME: -Werror
@@ -53,11 +53,10 @@
 
 //--- Config files are searched for in binary directory as well.
 //
-// RUN: rm -rf %T/testbin
-// RUN: mkdir -p %T/testbin
-// RUN: ln -s %clang %T/testbin/clang
-// RUN: echo "-Werror" > %T/testbin/aaa.cfg
-// RUN:

[PATCH] D86796: [Sema] Address-space sensitive index check for unbounded arrays

2020-09-03 Thread Chris Hamilton via Phabricator via cfe-commits

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



Comment at: clang/lib/Sema/SemaChecking.cpp:13966
   if (index.isUnsigned() || !index.isNegative()) {
-// It is possible that the type of the base expression after
-// IgnoreParenCasts is incomplete, even though the type of the base
-// expression before IgnoreParenCasts is complete (see PR39746 for an
-// example). In this case we have no information about whether the array
-// access exceeds the array bounds. However we can still diagnose an array
-// access which precedes the array bounds.
-if (BaseType->isIncompleteType())
-  return;
+if (isUnboundedArray) {
+  const auto  = getASTContext();

chrish_ericsson_atx wrote:
> ebevhan wrote:
> > chrish_ericsson_atx wrote:
> > > ebevhan wrote:
> > > > It might simplify the patch to move this condition out of the tree and 
> > > > just early return for the other case. That is:
> > > > 
> > > > ```
> > > > if (isUnboundedArray) {
> > > >   if (!(index.isUnsigned() || !index.isNegative()))
> > > > return;
> > > > 
> > > >   ...
> > > >   return;
> > > > }
> > > > 
> > > > if (index.isUnsigned() ...
> > > > ```
> > > There's a bit more code (starting at line 14094 in this patch set) that 
> > > applies in all cases, so an early return here would prevent the "Array 
> > > declared here" note from being generated.
> > Ah, the note.
> > 
> > I wonder if it wouldn't be cleaner (and avoid indenting the entire block) 
> > if that was just duplicated.
> Hard to say which is cleaner -- it's a tradeoff.  Nesting level would be 
> shallower if that code was duplicated, but then again, the duplication 
> increases the chance of an incomplete fix should an issue be discovered in 
> that code.  Overall code would be slightly longer, as well (adding about 16 
> lines of code, but removing only 4).   To me, the current strategy feels more 
> surgical, but I'll change it if you feel more strongly about it than I do.   
> Please re-ping if you do.
I reconsidered and made the changes you suggested.  Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86796

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

[PATCH] D86796: [Sema] Address-space sensitive index check for unbounded arrays

2020-09-03 Thread Chris Hamilton via Phabricator via cfe-commits

chrish_ericsson_atx updated this revision to Diff 289785.
chrish_ericsson_atx added a comment.

Refactored as ebevhan suggested to simplify patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86796

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaChecking.cpp
  clang/test/Sema/const-eval.c
  clang/test/Sema/unbounded-array-bounds.c
  clang/test/SemaCXX/constant-expression-cxx1y.cpp

Index: clang/test/SemaCXX/constant-expression-cxx1y.cpp
===
--- clang/test/SemaCXX/constant-expression-cxx1y.cpp
+++ clang/test/SemaCXX/constant-expression-cxx1y.cpp
@@ -1018,8 +1018,9 @@
 }
 
 constexpr void PR28739(int n) { // expected-error {{never produces a constant}}
-  int *p = 
+  int *p =   // expected-note {{declared here}}
   p += (__int128)(unsigned long)-1; // expected-note {{cannot refer to element 18446744073709551615 of non-array object in a constant expression}}
+  // expected-warning@-1 {{refers past the last possible element}}
 }
 
 constexpr void Void(int n) {
Index: clang/test/Sema/unbounded-array-bounds.c
===
--- /dev/null
+++ clang/test/Sema/unbounded-array-bounds.c
@@ -0,0 +1,70 @@
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -fsyntax-only %s 2>&1 | FileCheck --check-prefix=CHECK-X86-ADDR64 %s  \
+// RUN:  --implicit-check-not 'past the last possible element'
+// RUN: %clang_cc1 -triple i386-pc-linux-gnu   -fsyntax-only %s 2>&1 | FileCheck --check-prefix=CHECK-I386-ADDR32 %s \
+// RUN:  --implicit-check-not 'past the last possible element'
+// RUN: %clang_cc1 -triple avr-pc-linux-gnu-fsyntax-only %s 2>&1 | FileCheck --check-prefix=CHECK-AVR-ADDR16 %s  \
+// RUN:  --implicit-check-not 'past the last possible element'
+
+struct S {
+  long long a;
+  char b;
+  long long c;
+  short d;
+};
+
+struct S s[];
+
+void f1() {
+  ++s[3].a;
+  ++s[7073650413200313099].b;
+  // CHECK-X86-ADDR64:  :[[@LINE-1]]:5: warning: {{.*}} past the last possible element {{.*}} in 64-bit {{.*}} (max possible 576460752303423488 elements)
+  // CHECK-I386-ADDR32: :[[@LINE-2]]:5: warning: {{.*}} past the last possible element {{.*}} in 32-bit {{.*}} (max possible 178956970 elements)
+  // CHECK-AVR-ADDR16:  :[[@LINE-3]]:5: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 3276 elements)
+  ++s[7073650].c;
+  // CHECK-AVR-ADDR16:  :[[@LINE-1]]:5: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 3276 elements)
+}
+
+long long ll[];
+
+void f2() {
+  ++ll[3];
+  ++ll[2705843009213693952];
+  // CHECK-X86-ADDR64:  :[[@LINE-1]]:5: warning: {{.*}} past the last possible element {{.*}} in 64-bit {{.*}} (max possible 2305843009213693952 elements)
+  // CHECK-I386-ADDR32: :[[@LINE-2]]:5: warning: {{.*}} past the last possible element {{.*}} in 32-bit {{.*}} (max possible 536870912 elements)
+  // CHECK-AVR-ADDR16:  :[[@LINE-3]]:5: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 8192 elements)
+  ++ll[847073650];
+  // CHECK-I386-ADDR32: :[[@LINE-1]]:5: warning: {{.*}} past the last possible element {{.*}} in 32-bit {{.*}} (max possible 536870912 elements)
+  // CHECK-AVR-ADDR16:  :[[@LINE-2]]:5: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 8192 elements)
+}
+
+void f3(struct S p[]) {
+  ++p[3].a;
+  ++p[7073650413200313099].b;
+  // CHECK-X86-ADDR64:  :[[@LINE-1]]:5: warning: {{.*}} past the last possible element {{.*}} in 64-bit {{.*}} (max possible 576460752303423488 elements)
+  // CHECK-I386-ADDR32: :[[@LINE-2]]:5: warning: {{.*}} past the last possible element {{.*}} in 32-bit {{.*}} (max possible 178956970 elements)
+  // CHECK-AVR-ADDR16:  :[[@LINE-3]]:5: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 3276 elements)
+  ++p[7073650].c;
+  // CHECK-AVR-ADDR16:  :[[@LINE-1]]:5: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 3276 elements)
+}
+
+void f4(struct S *p) {
+  p += 3;
+  p += 7073650413200313099;
+  // CHECK-X86-ADDR64:  :[[@LINE-1]]:3: warning: {{.*}} past the last possible element {{.*}} in 64-bit {{.*}} (max possible 576460752303423488 elements)
+  // CHECK-I386-ADDR32: :[[@LINE-2]]:3: warning: {{.*}} past the last possible element {{.*}} in 32-bit {{.*}} (max possible 178956970 elements)
+  // CHECK-AVR-ADDR16:  :[[@LINE-3]]:3: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 3276 elements)
+  p += 7073650;
+  // CHECK-AVR-ADDR16:  :[[@LINE-1]]:3: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 3276 elements)
+}
+
+struct BQ {
+  struct S bigblock[3276];
+};
+
+struct BQ bq[];
+
+void f5() {
+  ++bq[0].bigblock[0].a;
+

[PATCH] D86790: [FE] Use preferred alignment instead of ABI alignment for complete object when applicable

2020-09-03 Thread Xiangling Liao via Phabricator via cfe-commits

Xiangling_L added a comment.

ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86790

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

[PATCH] D87103: [test] Use %t instead of %T to remove race conditions between config-file3.c and target-override.c

2020-09-03 Thread David Blaikie via Phabricator via cfe-commits

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

Looks good - thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87103

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

[PATCH] D87062: [DebugInfo] Add size to class declarations in debug info.

2020-09-03 Thread David Blaikie via Phabricator via cfe-commits

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

Looks good, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87062

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

[PATCH] D80344: [Windows SEH]: HARDWARE EXCEPTION HANDLING (MSVC -EHa) - Part 1

2020-09-03 Thread Roman Lebedev via Phabricator via cfe-commits

lebedev.ri added a comment.

In D80344#2255090 , @asmith wrote:

> Yes there was an RFC and discussion and several requests for comments.
> http://lists.llvm.org/pipermail/llvm-dev/2020-March/140541.html

It is probably good to state so in the patch's description.
But as far as i can tell the disscussion died down without an explicit 
acceptance?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80344

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

LLVM buildmaster will be updated and restarted tonight

2020-09-03 Thread Galina Kistanova via cfe-commits

 Hello everyone,

LLVM buildmaster will be updated and restarted after 6PM PST today.

Thanks

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

[PATCH] D80344: [Windows SEH]: HARDWARE EXCEPTION HANDLING (MSVC -EHa) - Part 1

2020-09-03 Thread Aaron Smith via Phabricator via cfe-commits

asmith added a comment.

Yes there was an RFC and discussion and several requests for comments.
http://lists.llvm.org/pipermail/llvm-dev/2020-March/140541.html


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80344

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

[PATCH] D80344: [Windows SEH]: HARDWARE EXCEPTION HANDLING (MSVC -EHa) - Part 1

2020-09-03 Thread Roman Lebedev via Phabricator via cfe-commits

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

I feel like this may be trying to do too many things at once.
Was there an RFC?
There are several patches here:

1. langref
2. llvm side of the patch (maybe should be in the previous patch)
3. `SideEntry` changes - missing astdump changes, serialization/deserialization
4. the rest of the owl




Comment at: clang/lib/CodeGen/CGCleanup.cpp:777
+
+  // mark EHa scope end for fall-through flow
+  if (IsEHa && getInvokeDest())

Here and elsewhere, the comments start from Capital Letter


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80344

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

[PATCH] D87031: [libTooling] Provide overloads of `rewriteDescendants` that operate directly on an AST node.

2020-09-03 Thread Dmitri Gribenko via Phabricator via cfe-commits

gribozavr2 added inline comments.



Comment at: clang/include/clang/Tooling/Transformer/RewriteRule.h:388
+llvm::Expected>
+rewriteDescendants(const Decl , RewriteRule Rule,
+   const ast_matchers::MatchFinder::MatchResult );

ymandel wrote:
> gribozavr2 wrote:
> > I agree that these functions provide very useful functionality, however I 
> > feel uneasy about putting a function that returns an `EditGenerator` and a 
> > function that actually executes the refactoring into the same overload set. 
> > The first is a part of a DSL, the second is a regular function. Do you 
> > think this is a problem worth solving?
> > 
> > IDK what exactly to suggest though. A namespace for DSL functions like we 
> > have in AST matchers?
> That's a really good point. These new functions are definitely a part of an 
> (implicit) lower-level library that improves manipulation of the AST 
> directly.  We don't have any appropriate library yet for this -- SourceCode 
> is in this spirit but even simpler than RewriteRule, which in fact depends on 
> it.
> 
> For now,  I'll put these in the `detail` which is (in some sense) the 
> collection of low level functions for which we need to solve this same 
> problem. WDYT? (I wasn't quite clear about the comparison with ast matchers, 
> since both the matchers DSL and the `match` functions (on which this is 
> loosely based) are in the same namespace).
> I wasn't quite clear about the comparison with ast matchers, since both the 
> matchers DSL and the match functions (on which this is loosely based) are in 
> the same namespace

Oh right -- I mistakenly thought that `clang::ast_matchers` only contains the 
DSL. I think your choice to put functions into `detail` looks good.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87031

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

[PATCH] D87103: [test] Use %t instead of %T to remove race conditions between config-file3.c and target-override.c

2020-09-03 Thread Fangrui Song via Phabricator via cfe-commits

MaskRay created this revision.
MaskRay added reviewers: dblaikie, nemanjai.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
MaskRay requested review of this revision.

Both tests operate on `%T/testbin`. If the two tests run concurrently,
one may fail.

This is likely the root cause of flaky failures reported by
https://lists.llvm.org/pipermail/llvm-dev/2020-September/144781.html

https://llvm.org/docs/CommandGuide/lit.html says:

`%T parent directory of %t (not unique, deprecated, do not use)`


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D87103

Files:
  clang/test/Driver/config-file3.c
  clang/test/Driver/target-override.c

Index: clang/test/Driver/target-override.c
===
--- clang/test/Driver/target-override.c
+++ clang/test/Driver/target-override.c
@@ -1,16 +1,15 @@
 // REQUIRES: shell
 // REQUIRES: x86-registered-target
 
-// RUN: rm -rf %T/testbin
-// RUN: mkdir -p %T/testbin
-// RUN: ln -s %clang %T/testbin/i386-clang
+// RUN: rm -rf %t && mkdir %t
+// RUN: ln -s %clang %t/i386-clang
 
 // Check if invocation of "foo-clang" adds option "-target foo".
 //
-// RUN: %T/testbin/i386-clang -c -no-canonical-prefixes %s -### 2>&1 | FileCheck -check-prefix CHECK-TG1 %s
+// RUN: %t/i386-clang -c -no-canonical-prefixes %s -### 2>&1 | FileCheck -check-prefix CHECK-TG1 %s
 // CHECK-TG1: Target: i386
 
 // Check if invocation of "foo-clang -target bar" overrides option "-target foo".
 //
-// RUN: %T/testbin/i386-clang -c -no-canonical-prefixes -target x86_64 %s -### 2>&1 | FileCheck -check-prefix CHECK-TG2 %s
+// RUN: %t/i386-clang -c -no-canonical-prefixes -target x86_64 %s -### 2>&1 | FileCheck -check-prefix CHECK-TG2 %s
 // CHECK-TG2: Target: x86_64
Index: clang/test/Driver/config-file3.c
===
--- clang/test/Driver/config-file3.c
+++ clang/test/Driver/config-file3.c
@@ -1,14 +1,15 @@
 // REQUIRES: shell
 // REQUIRES: x86-registered-target
 
+// RUN: rm -rf %t && mkdir %t
+
 //--- If config file is specified by relative path (workdir/cfg-s2), it is searched for by that path.
+
+// RUN: mkdir -p %t/workdir/subdir
+// RUN: echo "@subdir/cfg-s2" > %t/workdir/cfg-1
+// RUN: echo "-Wundefined-var-template" > %t/workdir/subdir/cfg-s2
 //
-// RUN: mkdir -p %T/workdir
-// RUN: echo "@subdir/cfg-s2" > %T/workdir/cfg-1
-// RUN: mkdir -p %T/workdir/subdir
-// RUN: echo "-Wundefined-var-template" > %T/workdir/subdir/cfg-s2
-//
-// RUN: ( cd %T && %clang --config workdir/cfg-1 -c %s -### 2>&1 | FileCheck %s -check-prefix CHECK-REL )
+// RUN: ( cd %t && %clang --config workdir/cfg-1 -c %s -### 2>&1 | FileCheck %s -check-prefix CHECK-REL )
 //
 // CHECK-REL: Configuration file: {{.*}}/workdir/cfg-1
 // CHECK-REL: -Wundefined-var-template
@@ -16,12 +17,11 @@
 
 //--- Invocation qqq-clang-g++ tries to find config file qqq-clang-g++.cfg first.
 //
-// RUN: rm -rf %T/testdmode
-// RUN: mkdir -p %T/testdmode
-// RUN: ln -s %clang %T/testdmode/qqq-clang-g++
-// RUN: echo "-Wundefined-func-template" > %T/testdmode/qqq-clang-g++.cfg
-// RUN: echo "-Werror" > %T/testdmode/qqq.cfg
-// RUN: %T/testdmode/qqq-clang-g++ --config-system-dir= --config-user-dir= -c -no-canonical-prefixes %s -### 2>&1 | FileCheck %s -check-prefix FULL-NAME
+// RUN: mkdir %t/testdmode
+// RUN: ln -s %clang %t/testdmode/qqq-clang-g++
+// RUN: echo "-Wundefined-func-template" > %t/testdmode/qqq-clang-g++.cfg
+// RUN: echo "-Werror" > %t/testdmode/qqq.cfg
+// RUN: %t/testdmode/qqq-clang-g++ --config-system-dir= --config-user-dir= -c -no-canonical-prefixes %s -### 2>&1 | FileCheck %s -check-prefix FULL-NAME
 //
 // FULL-NAME: Configuration file: {{.*}}/testdmode/qqq-clang-g++.cfg
 // FULL-NAME: -Wundefined-func-template
@@ -31,20 +31,20 @@
 // (As the clang executable and symlink are in different directories, this
 // requires specifying the path via --config-*-dir= though.)
 //
-// RUN: %T/testdmode/qqq-clang-g++ --config-system-dir= --config-user-dir=%T/testdmode -c %s -### 2>&1 | FileCheck %s -check-prefix SYMLINK
+// RUN: %t/testdmode/qqq-clang-g++ --config-system-dir= --config-user-dir=%t/testdmode -c %s -### 2>&1 | FileCheck %s -check-prefix SYMLINK
 //
 // SYMLINK: Configuration file: {{.*}}/testdmode/qqq-clang-g++.cfg
 //
 //--- File specified by --config overrides config inferred from clang executable.
 //
-// RUN: %T/testdmode/qqq-clang-g++ --config-system-dir=%S/Inputs/config --config-user-dir= --config i386-qqq -c -no-canonical-prefixes %s -### 2>&1 | FileCheck %s -check-prefix CHECK-EXPLICIT
+// RUN: %t/testdmode/qqq-clang-g++ --config-system-dir=%S/Inputs/config --config-user-dir= --config i386-qqq -c -no-canonical-prefixes %s -### 2>&1 | FileCheck %s -check-prefix CHECK-EXPLICIT
 //
 // CHECK-EXPLICIT: Configuration file: {{.*}}/Inputs/config/i386-qqq.cfg
 //
 //--- Invocation qqq-clang-g++ tries to find config file qqq.cfg if qqq-clang-g++.cfg is not found.
 //
-// RUN: rm

[PATCH] D86369: [Sema][MSVC] warn at dynamic_cast when /GR- is given

2020-09-03 Thread Zequan Wu via Phabricator via cfe-commits

zequanwu updated this revision to Diff 289769.
zequanwu marked an inline comment as done.
zequanwu added a comment.

address comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86369

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaCast.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/SemaCXX/ms_no_dynamic_cast.cpp
  clang/test/SemaCXX/no_dynamic_cast.cpp

Index: clang/test/SemaCXX/no_dynamic_cast.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/no_dynamic_cast.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 %s -fno-rtti-data -fsyntax-only -verify
+
+namespace std {
+struct type_info {};
+} // namespace std
+class B {
+public:
+  virtual ~B() = default;
+};
+
+class D1 : public B {
+public:
+  ~D1() = default;
+};
+
+void f() {
+  B *b = new D1();
+  auto d = dynamic_cast(b); // expected-warning{{dynamic_cast will not work since RTTI data is disabled by -fno-rtti-data}}
+  (void)typeid(int);  // expected-warning{{typeid will not work since RTTI data is disabled by -fno-rtti-data}}
+}
Index: clang/test/SemaCXX/ms_no_dynamic_cast.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/ms_no_dynamic_cast.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 %s -fdiagnostics-format msvc -fno-rtti-data -fsyntax-only -verify
+
+namespace std {
+struct type_info {};
+} // namespace std
+class B {
+public:
+  virtual ~B() = default;
+};
+
+class D1 : public B {
+public:
+  ~D1() = default;
+};
+
+void f() {
+  B *b = new D1();
+  auto d = dynamic_cast(b); // expected-warning{{dynamic_cast will not work since RTTI data is disabled by /GR-}}
+  (void)typeid(int);  // expected-warning{{typeid will not work since RTTI data is disabled by /GR-}}
+}
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -646,6 +646,12 @@
 return ExprError(Diag(OpLoc, diag::err_no_typeid_with_fno_rtti));
   }
 
+  // Warns when typeid is used with RTTI data disabled.
+  if (!getLangOpts().RTTIData)
+Diag(OpLoc, diag::warn_no_typeid_with_rtti_disabled)
+<< (getDiagnostics().getDiagnosticOptions().getFormat() ==
+DiagnosticOptions::MSVC);
+
   QualType TypeInfoType = Context.getTypeDeclType(CXXTypeInfoDecl);
 
   if (isType) {
Index: clang/lib/Sema/SemaCast.cpp
===
--- clang/lib/Sema/SemaCast.cpp
+++ clang/lib/Sema/SemaCast.cpp
@@ -890,6 +890,16 @@
 return;
   }
 
+  // Warns when dynamic_cast is used with RTTI data disabled.
+  if (!Self.getLangOpts().RTTIData) {
+bool isClangCL = Self.getDiagnostics().getDiagnosticOptions().getFormat() ==
+ DiagnosticOptions::MSVC;
+if (isClangCL || !DestPointee->isVoidType())
+  Self.Diag(OpRange.getBegin(),
+diag::warn_no_dynamic_cast_with_rtti_disabled)
+  << isClangCL;
+  }
+
   // Done. Everything else is run-time checks.
   Kind = CK_Dynamic;
 }
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -7433,6 +7433,12 @@
   "use of typeid requires -frtti">;
 def err_no_dynamic_cast_with_fno_rtti : Error<
   "use of dynamic_cast requires -frtti">;
+def warn_no_dynamic_cast_with_rtti_disabled: Warning<
+  "dynamic_cast will not work since RTTI data is disabled by " 
+  "%select{-fno-rtti-data|/GR-}0">, InGroup;
+def warn_no_typeid_with_rtti_disabled: Warning<
+  "typeid will not work since RTTI data is disabled by "
+  "%select{-fno-rtti-data|/GR-}0">, InGroup;
 
 def err_cannot_form_pointer_to_member_of_reference_type : Error<
   "cannot form a pointer-to-member to member %0 of reference type %1">;
Index: clang/include/clang/Basic/DiagnosticGroups.td
===
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -1229,3 +1229,5 @@
 }
 
 def WebAssemblyExceptionSpec : DiagGroup<"wasm-exception-spec">;
+
+def RTTI : DiagGroup<"rtti">;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D86369: [Sema][MSVC] warn at dynamic_cast when /GR- is given

2020-09-03 Thread Zequan Wu via Phabricator via cfe-commits

zequanwu added inline comments.



Comment at: clang/lib/Sema/SemaCast.cpp:897
+  DiagnosticOptions::MSVC;
+if (isMSVC || !DestPointee->isVoidType())
+  Self.Diag(OpRange.getBegin(),

hans wrote:
> I don't think the if-statement is necessary. I thought we concluded we want 
> to warn even for void*? Also note that "isMSVC" here is only checking the 
> driver mode, i.e. just the user interface to the compiler. The user could 
> still be compiling in MSVC mode.
My bad. I thought you meant to use the previous version, so I reverted this 
region.
Like we concluded, we want to warn even for void*. This only applies to 
clang-cl, not clang, right? This is the purpose of this if. If it's in 
clang-cl, warn regardless of destination type. If it's in clang, don't warn for 
void*, like line 887 which doesn't emit error for void* destination type. 

If RTTI is false, RTTIData is also false 
(https://github.com/llvm/llvm-project/blob/master/clang/lib/Frontend/CompilerInvocation.cpp#L2870).
 There is a test 
https://github.com/llvm/llvm-project/blob/master/clang/test/SemaCXX/no-rtti.cpp#L28,
 which should not be warned, right?



Comment at: clang/test/SemaCXX/ms_no_dynamic_cast.cpp:19
+  auto d = dynamic_cast(b); // expected-warning{{dynamic_cast will not 
work since RTTI data is disabled by /GR-}}
+  (void)typeid(int);  // expected-warning{{typeid will not work 
since RTTI data is disabled by /GR-}}
+}

hans wrote:
> Is the cast to void necessary? Same comment for the next file.
Yes, to suppress the warning of unused expression.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86369

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

[PATCH] D87095: [Triple][MachO] Define "arm64e", an AArch64 subarch for Pointer Auth.

2020-09-03 Thread Ahmed Bougacha via Phabricator via cfe-commits

ab added a comment.

In D87095#2255010 , @pcc wrote:

> But I was wondering: shouldn't this be the *last* patch? I was imagining that 
> you would first upstream support for the `-fptrauth-*` flags, and then at the 
> end you would add an `arm64e` subarch that turns them on by default. 
> Otherwise the upstream compiler will temporarily produce ABI-incompatible 
> objects when targeting `arm64e`.

Good point, you're right.  ABI compatibility has obviously been a major issue 
as we keep iterating on this, so we added the concept of ptrauth ABI versions 
to the arm64e mach-o cpusubtype.  With this patch, upstream clang produces 
"unversioned" binaries, which are rejected on macOS11/iOS14.  Once we're done 
upstreaming, I'll make the final patch to match the ABI version.  In the 
meantime, that means upstream can't produce anything that runs on macOS, which 
should avoid this problem (and lets us use arm64e checks in the remaining 
patches, though I can obviously extract that to have the patches done the other 
way)

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87095

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

[PATCH] D85408: Let -basic-block-sections=labels emit basicblock metadata in a new .bb_addr_map section, instead of emitting special unary-encoded symbols.

2020-09-03 Thread Rahman Lavaee via Phabricator via cfe-commits

rahmanl added inline comments.



Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:1047
+  getObjFileLowering().getBBAddrMapSection(*MF.getSection());
+  if (!BBAddrMapSection)
+return;

MaskRay wrote:
> BBAddrMapSection is always non-null. Delete the if.
I believe we return null for non-ELF environment (Please refer to 
MCObjectFileInfo::getBBAddrMapSection).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85408

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

[PATCH] D87062: [DebugInfo] Add size to class declarations in debug info.

2020-09-03 Thread Amy Huang via Phabricator via cfe-commits

akhuang updated this revision to Diff 289768.
akhuang added a comment.

rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87062

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/test/CodeGenCXX/debug-info-class.cpp


Index: clang/test/CodeGenCXX/debug-info-class.cpp
===
--- clang/test/CodeGenCXX/debug-info-class.cpp
+++ clang/test/CodeGenCXX/debug-info-class.cpp
@@ -136,7 +136,7 @@
 // CHECK: [[C_DTOR]] = !DISubprogram(name: "~C"
 
 // CHECK: [[D:![0-9]+]] = !DICompositeType(tag: DW_TAG_structure_type, name: 
"D"
-// CHECK-NOT:  size:
+// CHECK-SAME: size:
 // CHECK-SAME: DIFlagFwdDecl
 // CHECK-NOT:  identifier:
 // CHECK-SAME: ){{$}}
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -1031,6 +1031,10 @@
   uint64_t Size = 0;
   uint32_t Align = 0;
 
+  const RecordDecl *D = RD->getDefinition();
+  if (D && D->isCompleteDefinition())
+Size = CGM.getContext().getTypeSize(Ty);
+
   llvm::DINode::DIFlags Flags = llvm::DINode::FlagFwdDecl;
 
   // Add flag to nontrivial forward declarations. To be consistent with MSVC,


Index: clang/test/CodeGenCXX/debug-info-class.cpp
===
--- clang/test/CodeGenCXX/debug-info-class.cpp
+++ clang/test/CodeGenCXX/debug-info-class.cpp
@@ -136,7 +136,7 @@
 // CHECK: [[C_DTOR]] = !DISubprogram(name: "~C"
 
 // CHECK: [[D:![0-9]+]] = !DICompositeType(tag: DW_TAG_structure_type, name: "D"
-// CHECK-NOT:  size:
+// CHECK-SAME: size:
 // CHECK-SAME: DIFlagFwdDecl
 // CHECK-NOT:  identifier:
 // CHECK-SAME: ){{$}}
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -1031,6 +1031,10 @@
   uint64_t Size = 0;
   uint32_t Align = 0;
 
+  const RecordDecl *D = RD->getDefinition();
+  if (D && D->isCompleteDefinition())
+Size = CGM.getContext().getTypeSize(Ty);
+
   llvm::DINode::DIFlags Flags = llvm::DINode::FlagFwdDecl;
 
   // Add flag to nontrivial forward declarations. To be consistent with MSVC,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D86993: Document Clang's expectations of the C standard library.

2020-09-03 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added inline comments.



Comment at: clang/docs/Toolchain.rst:289
+
+C standard library implementations that do not guarantee these properties are
+incompatible with Clang and LLVM (and with several other major compilers).

While I think it's good that we're documenting this, it is really troubling 
that Clang community's perspective is "we can't work with a conforming C 
standard library" without filing any DRs to WG14 about why we cannot conform, 
especially if other major compilers are in a similar boat. Do you have any 
interest in filing a DR to see if we can change the C standard?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86993

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

[PATCH] D85408: Let -basic-block-sections=labels emit basicblock metadata in a new .bb_addr_map section, instead of emitting special unary-encoded symbols.

2020-09-03 Thread Rahman Lavaee via Phabricator via cfe-commits

rahmanl updated this revision to Diff 289765.
rahmanl marked 5 inline comments as done.
rahmanl added a comment.

- Address +MaskRay's comments:
- Change the check prefix simply to "CHECK" for basic-block-sections-labels.ll
- Change the triple to x86_64 for this test.
- nits.
- Remove the "-LABEL" feature from basic-block-sections-labels.ll and use 
precise BB indices.
- Remove the assertion in emitBBAddrMapSection.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85408

Files:
  clang/docs/UsersManual.rst
  clang/test/CodeGen/basic-block-sections.c
  llvm/include/llvm/CodeGen/AsmPrinter.h
  llvm/include/llvm/CodeGen/MachineFunction.h
  llvm/include/llvm/MC/MCObjectFileInfo.h
  llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
  llvm/lib/CodeGen/BasicBlockSections.cpp
  llvm/lib/CodeGen/MIRParser/MIRParser.cpp
  llvm/lib/CodeGen/MachineBasicBlock.cpp
  llvm/lib/CodeGen/MachineFunction.cpp
  llvm/lib/MC/MCObjectFileInfo.cpp
  llvm/test/CodeGen/X86/basic-block-sections-labels-functions-sections.ll
  llvm/test/CodeGen/X86/basic-block-sections-labels.ll

Index: llvm/test/CodeGen/X86/basic-block-sections-labels.ll
===
--- llvm/test/CodeGen/X86/basic-block-sections-labels.ll
+++ llvm/test/CodeGen/X86/basic-block-sections-labels.ll
@@ -1,23 +1,24 @@
 ; Check the basic block sections labels option
-; RUN: llc < %s -mtriple=x86_64-pc-linux -function-sections -basic-block-sections=labels | FileCheck %s -check-prefix=LINUX-LABELS
+; RUN: llc < %s -mtriple=x86_64 -function-sections -basic-block-sections=labels | FileCheck %s -check-prefix=CHECK
 
-define void @_Z3bazb(i1 zeroext) {
-  %2 = alloca i8, align 1
-  %3 = zext i1 %0 to i8
-  store i8 %3, i8* %2, align 1
-  %4 = load i8, i8* %2, align 1
-  %5 = trunc i8 %4 to i1
-  br i1 %5, label %6, label %8
+define void @_Z3bazb(i1 zeroext) personality i32 (...)* @__gxx_personality_v0 {
+  br i1 %0, label %2, label %7
 
-6:; preds = %1
-  %7 = call i32 @_Z3barv()
-  br label %10
+2:
+  %3 = invoke i32 @_Z3barv()
+  to label %7 unwind label %5
+  br label %9
 
-8:; preds = %1
-  %9 = call i32 @_Z3foov()
-  br label %10
+5:
+  landingpad { i8*, i32 }
+  catch i8* null
+  br label %9
 
-10:   ; preds = %8, %6
+7:
+  %8 = call i32 @_Z3foov()
+  br label %9
+
+9:
   ret void
 }
 
@@ -25,9 +26,31 @@
 
 declare i32 @_Z3foov() #1
 
-; LINUX-LABELS: .section
-; LINUX-LABELS: _Z3bazb:
-; LINUX-LABELS-NOT: .section
-; LINUX-LABELS: r.BB._Z3bazb:
-; LINUX-LABELS-NOT: .section
-; LINUX-LABELS: rr.BB._Z3bazb:
+declare i32 @__gxx_personality_v0(...)
+
+; CHECK-LABEL:	_Z3bazb:
+; CHECK-LABEL:	.Lfunc_begin0:
+; CHECK-LABEL:	.LBB_END0_0:
+; CHECK-LABEL:	.LBB0_1:
+; CHECK-LABEL:	.LBB_END0_1:
+; CHECK-LABEL:	.LBB0_2:
+; CHECK-LABEL:	.LBB_END0_2:
+; CHECK-LABEL:	.LBB0_3:
+; CHECK-LABEL:	.LBB_END0_3:
+; CHECK-LABEL:	.Lfunc_end0:
+
+; CHECK:	.section	.bb_addr_map,"o",@progbits,.text
+; CHECK-NEXT:	.quad	.Lfunc_begin0
+; CHECK-NEXT:	.byte	4
+; CHECK-NEXT:	.uleb128 .Lfunc_begin0-.Lfunc_begin0
+; CHECK-NEXT:	.uleb128 .LBB_END0_0-.Lfunc_begin0
+; CHECK-NEXT:	.byte	0
+; CHECK-NEXT:	.uleb128 .LBB0_1-.Lfunc_begin0
+; CHECK-NEXT:	.uleb128 .LBB_END0_1-.LBB0_1
+; CHECK-NEXT:	.byte	0
+; CHECK-NEXT:	.uleb128 .LBB0_2-.Lfunc_begin0
+; CHECK-NEXT:	.uleb128 .LBB_END0_2-.LBB0_2
+; CHECK-NEXT:	.byte	1
+; CHECK-NEXT:	.uleb128 .LBB0_3-.Lfunc_begin0
+; CHECK-NEXT:	.uleb128 .LBB_END0_3-.LBB0_3
+; CHECK-NEXT:	.byte	5
Index: llvm/test/CodeGen/X86/basic-block-sections-labels-functions-sections.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/basic-block-sections-labels-functions-sections.ll
@@ -0,0 +1,35 @@
+; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu -function-sections -basic-block-sections=labels | FileCheck %s
+
+$_Z4fooTIiET_v = comdat any
+
+define dso_local i32 @_Z3barv() {
+  ret i32 0
+}
+;; Check we add SHF_LINK_ORDER for .bb_addr_map and link it with the corresponding .text sections.
+; CHECK:		.section .text._Z3barv,"ax",@progbits
+; CHECK-LABEL:	_Z3barv:
+; CHECK-NEXT:	[[BAR_BEGIN:.Lfunc_begin[0-9]+]]:
+; CHECK:		.section .bb_addr_map,"o",@progbits,.text._Z3barv{{$}}
+; CHECK-NEXT:		.quad [[BAR_BEGIN]]
+
+
+define dso_local i32 @_Z3foov() {
+  %1 = call i32 @_Z4fooTIiET_v()
+  ret i32 %1
+}
+; CHECK:		.section .text._Z3foov,"ax",@progbits
+; CHECK-LABEL:	_Z3foov:
+; CHECK-NEXT:	[[FOO_BEGIN:.Lfunc_begin[0-9]+]]:
+; CHECK:		.section  .bb_addr_map,"o",@progbits,.text._Z3foov{{$}}
+; CHECK-NEXT:		.quad [[FOO_BEGIN]]
+
+
+define linkonce_odr dso_local i32 @_Z4fooTIiET_v() comdat {
+  ret i32 0
+}
+;; Check we add .bb_addr_map section to a COMDAT group with the corresponding .text section if such a COMDAT exists.
+; CHECK:		.section

[PATCH] D87095: [Triple][MachO] Define "arm64e", an AArch64 subarch for Pointer Auth.

2020-09-03 Thread Peter Collingbourne via Phabricator via cfe-commits

pcc added a comment.

Hi, thanks for getting started on upstreaming this!

But I was wondering: shouldn't this be the *last* patch? I was imagining that 
you would first upstream support for the `-fptrauth-*` flags, and then at the 
end you would add an `arm64e` subarch that turns them on by default. Otherwise 
the upstream compiler will temporarily produce ABI-incompatible objects when 
targeting `arm64e`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87095

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

[PATCH] D87102: [Sema] Fix a -Warc-repeated-use-of-weak false-positive by only calling CheckPlaceholderExpr once

2020-09-03 Thread Erik Pilkington via Phabricator via cfe-commits

erik.pilkington created this revision.
erik.pilkington added reviewers: rjmccall, ahatanak, fhahn.
Herald added subscribers: ributzka, dexonsmith, jkorous.
erik.pilkington requested review of this revision.

Previously, this code discarded the result of CheckPlaceholderExpr for 
non-matrix subexpressions. Not only is this wasteful, but it was creating a 
Warc-repeated-use-of-weak false-positive on the attached testcase, since the 
discarded expression was still registered as a use of the weak property. (This 
was introduced in D76791 )

rdar://66162246


https://reviews.llvm.org/D87102

Files:
  clang/lib/Sema/SemaExpr.cpp
  clang/test/SemaObjC/arc-repeated-weak.mm


Index: clang/test/SemaObjC/arc-repeated-weak.mm
===
--- clang/test/SemaObjC/arc-repeated-weak.mm
+++ clang/test/SemaObjC/arc-repeated-weak.mm
@@ -485,3 +485,17 @@
 
 @class NSString;
 static NSString* const kGlobal = @"";
+
+@interface NSDictionary
+- (id)objectForKeyedSubscript:(id)key;
+@end
+
+@interface WeakProp
+@property (weak) NSDictionary *nd;
+@end
+
+@implementation WeakProp
+-(void)m {
+  (void)self.nd[@""]; // no warning
+}
+@end
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -4607,34 +4607,13 @@
 return CreateBuiltinMatrixSubscriptExpr(
 matSubscriptE->getBase(), matSubscriptE->getRowIdx(), idx, rbLoc);
   }
-  Expr *matrixBase = base;
-  bool IsMSPropertySubscript = isMSPropertySubscriptExpr(*this, base);
-  if (!IsMSPropertySubscript) {
-ExprResult result = CheckPlaceholderExpr(base);
-if (!result.isInvalid())
-  matrixBase = result.get();
-  }
-  if (matrixBase->getType()->isMatrixType()) {
-if (CheckAndReportCommaError(idx))
-  return ExprError();
-
-return CreateBuiltinMatrixSubscriptExpr(matrixBase, idx, nullptr, rbLoc);
-  }
-
-  // A comma-expression as the index is deprecated in C++2a onwards.
-  if (getLangOpts().CPlusPlus20 &&
-  ((isa(idx) && cast(idx)->isCommaOp()) ||
-   (isa(idx) &&
-cast(idx)->getOperator() == OO_Comma))) {
-Diag(idx->getExprLoc(), diag::warn_deprecated_comma_subscript)
-  << SourceRange(base->getBeginLoc(), rbLoc);
-  }
 
   // Handle any non-overload placeholder types in the base and index
   // expressions.  We can't handle overloads here because the other
   // operand might be an overloadable type, in which case the overload
   // resolution for the operator overload should get the first crack
   // at the overload.
+  bool IsMSPropertySubscript = false;
   if (base->getType()->isNonOverloadPlaceholderType()) {
 IsMSPropertySubscript = isMSPropertySubscriptExpr(*this, base);
 if (!IsMSPropertySubscript) {
@@ -4644,6 +4623,23 @@
   base = result.get();
 }
   }
+
+  if (base->getType()->isMatrixType()) {
+if (CheckAndReportCommaError(idx))
+  return ExprError();
+
+return CreateBuiltinMatrixSubscriptExpr(base, idx, nullptr, rbLoc);
+  }
+
+  // A comma-expression as the index is deprecated in C++2a onwards.
+  if (getLangOpts().CPlusPlus20 &&
+  ((isa(idx) && cast(idx)->isCommaOp()) ||
+   (isa(idx) &&
+cast(idx)->getOperator() == OO_Comma))) {
+Diag(idx->getExprLoc(), diag::warn_deprecated_comma_subscript)
+<< SourceRange(base->getBeginLoc(), rbLoc);
+  }
+
   if (idx->getType()->isNonOverloadPlaceholderType()) {
 ExprResult result = CheckPlaceholderExpr(idx);
 if (result.isInvalid()) return ExprError();


Index: clang/test/SemaObjC/arc-repeated-weak.mm
===
--- clang/test/SemaObjC/arc-repeated-weak.mm
+++ clang/test/SemaObjC/arc-repeated-weak.mm
@@ -485,3 +485,17 @@
 
 @class NSString;
 static NSString* const kGlobal = @"";
+
+@interface NSDictionary
+- (id)objectForKeyedSubscript:(id)key;
+@end
+
+@interface WeakProp
+@property (weak) NSDictionary *nd;
+@end
+
+@implementation WeakProp
+-(void)m {
+  (void)self.nd[@""]; // no warning
+}
+@end
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -4607,34 +4607,13 @@
 return CreateBuiltinMatrixSubscriptExpr(
 matSubscriptE->getBase(), matSubscriptE->getRowIdx(), idx, rbLoc);
   }
-  Expr *matrixBase = base;
-  bool IsMSPropertySubscript = isMSPropertySubscriptExpr(*this, base);
-  if (!IsMSPropertySubscript) {
-ExprResult result = CheckPlaceholderExpr(base);
-if (!result.isInvalid())
-  matrixBase = result.get();
-  }
-  if (matrixBase->getType()->isMatrixType()) {
-if (CheckAndReportCommaError(idx))
-  return ExprError();
-
-return CreateBuiltinMatrixSubscriptExpr(matrixBase, idx, nullptr, rbLoc);
-  }
-
-  // A comma-expression as the index is deprecated in C++2a onwards.
-  if

[PATCH] D86993: Document Clang's expectations of the C standard library.

2020-09-03 Thread John McCall via Phabricator via cfe-commits

rjmccall added a subscriber: t.p.northover.
rjmccall added a comment.

@t.p.northover says it's complicated.  `memcpy`, `memmove`, `memset`, and 
`bzero` are (I think) the only ones that LLVM will potentially synthesize from 
nothing and therefore need to be present even in freestanding builds; that's 
probably okay for us to guarantee.  That's probably also a good place to 
document the supported way to write those functions in libraries (just 
`-fno-builtin`, IIRC?).

In hosted builds, we should document that we assume the existence of the 
standard C library for the target platform, potentially including non-standard 
functions like `exp10`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86993

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

[PATCH] D87101: [X86] Update SSE/AVX ABS intrinsics to emit llvm.abs.* (PR46851)

2020-09-03 Thread Simon Pilgrim via Phabricator via cfe-commits

RKSimon created this revision.
RKSimon added reviewers: craig.topper, spatel, nikic, lebedev.ri.
Herald added projects: clang, LLVM.
Herald added a subscriber: llvm-commits.
RKSimon requested review of this revision.

We're now getting close to having the necessary analysis/combines etc. for the 
new generic llvm.abs.* intrinsics.

This patch updates the SSE/AVX ABS intrinsics to emit the generic equivalents 
instead of the icmp+sub+select code pattern.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D87101

Files:
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/avx2-builtins.c
  clang/test/CodeGen/avx512bw-builtins.c
  clang/test/CodeGen/avx512f-builtins.c
  clang/test/CodeGen/avx512vl-builtins.c
  clang/test/CodeGen/avx512vlbw-builtins.c
  clang/test/CodeGen/ssse3-builtins.c
  llvm/test/CodeGen/X86/avx2-intrinsics-fast-isel.ll
  llvm/test/CodeGen/X86/ssse3-intrinsics-fast-isel.ll

Index: llvm/test/CodeGen/X86/ssse3-intrinsics-fast-isel.ll
===
--- llvm/test/CodeGen/X86/ssse3-intrinsics-fast-isel.ll
+++ llvm/test/CodeGen/X86/ssse3-intrinsics-fast-isel.ll
@@ -19,13 +19,11 @@
 ; AVX-NEXT:vpabsb %xmm0, %xmm0
 ; AVX-NEXT:ret{{[l|q]}}
   %arg = bitcast <2 x i64> %a0 to <16 x i8>
-  %sub = sub <16 x i8> zeroinitializer, %arg
-  %cmp = icmp sgt <16 x i8> %arg, zeroinitializer
-  %sel = select <16 x i1> %cmp, <16 x i8> %arg, <16 x i8> %sub
-  %res = bitcast <16 x i8> %sel to <2 x i64>
+  %abs = call <16 x i8> @llvm.abs.v16i8(<16 x i8> %arg, i1 false)
+  %res = bitcast <16 x i8> %abs to <2 x i64>
   ret <2 x i64> %res
 }
-declare <16 x i8> @llvm.x86.ssse3.pabs.b.128(<16 x i8>) nounwind readnone
+declare <16 x i8> @llvm.abs.v16i8(<16 x i8>, i1) nounwind readnone
 
 define <2 x i64> @test_mm_abs_epi16(<2 x i64> %a0) {
 ; SSE-LABEL: test_mm_abs_epi16:
@@ -38,13 +36,11 @@
 ; AVX-NEXT:vpabsw %xmm0, %xmm0
 ; AVX-NEXT:ret{{[l|q]}}
   %arg = bitcast <2 x i64> %a0 to <8 x i16>
-  %sub = sub <8 x i16> zeroinitializer, %arg
-  %cmp = icmp sgt <8 x i16> %arg, zeroinitializer
-  %sel = select <8 x i1> %cmp, <8 x i16> %arg, <8 x i16> %sub
-  %res = bitcast <8 x i16> %sel to <2 x i64>
+  %abs = call <8 x i16> @llvm.abs.v8i16(<8 x i16> %arg, i1 false)
+  %res = bitcast <8 x i16> %abs to <2 x i64>
   ret <2 x i64> %res
 }
-declare <8 x i16> @llvm.x86.ssse3.pabs.w.128(<8 x i16>) nounwind readnone
+declare <8 x i16> @llvm.abs.v8i16(<8 x i16>, i1) nounwind readnone
 
 define <2 x i64> @test_mm_abs_epi32(<2 x i64> %a0) {
 ; SSE-LABEL: test_mm_abs_epi32:
@@ -57,13 +53,11 @@
 ; AVX-NEXT:vpabsd %xmm0, %xmm0
 ; AVX-NEXT:ret{{[l|q]}}
   %arg = bitcast <2 x i64> %a0 to <4 x i32>
-  %sub = sub <4 x i32> zeroinitializer, %arg
-  %cmp = icmp sgt <4 x i32> %arg, zeroinitializer
-  %sel = select <4 x i1> %cmp, <4 x i32> %arg, <4 x i32> %sub
-  %res = bitcast <4 x i32> %sel to <2 x i64>
+  %abs = call <4 x i32> @llvm.abs.v4i32(<4 x i32> %arg, i1 false)
+  %res = bitcast <4 x i32> %abs to <2 x i64>
   ret <2 x i64> %res
 }
-declare <4 x i32> @llvm.x86.ssse3.pabs.d.128(<4 x i32>) nounwind readnone
+declare <4 x i32> @llvm.abs.v4i32(<4 x i32>, i1) nounwind readnone
 
 define <2 x i64> @test_mm_alignr_epi8(<2 x i64> %a0, <2 x i64> %a1) {
 ; SSE-LABEL: test_mm_alignr_epi8:
Index: llvm/test/CodeGen/X86/avx2-intrinsics-fast-isel.ll
===
--- llvm/test/CodeGen/X86/avx2-intrinsics-fast-isel.ll
+++ llvm/test/CodeGen/X86/avx2-intrinsics-fast-isel.ll
@@ -10,13 +10,11 @@
 ; CHECK-NEXT:vpabsb %ymm0, %ymm0
 ; CHECK-NEXT:ret{{[l|q]}}
   %arg = bitcast <4 x i64> %a0 to <32 x i8>
-  %sub = sub <32 x i8> zeroinitializer, %arg
-  %cmp = icmp sgt <32 x i8> %arg, zeroinitializer
-  %sel = select <32 x i1> %cmp, <32 x i8> %arg, <32 x i8> %sub
-  %res = bitcast <32 x i8> %sel to <4 x i64>
+  %abs = call <32 x i8> @llvm.abs.v32i8(<32 x i8> %arg, i1 false)
+  %res = bitcast <32 x i8> %abs to <4 x i64>
   ret <4 x i64> %res
 }
-declare <32 x i8> @llvm.x86.avx2.pabs.b(<32 x i8>) nounwind readnone
+declare <32 x i8> @llvm.abs.v32i8(<32 x i8>, i1) nounwind readnone
 
 define <4 x i64> @test_mm256_abs_epi16(<4 x i64> %a0) {
 ; CHECK-LABEL: test_mm256_abs_epi16:
@@ -24,13 +22,11 @@
 ; CHECK-NEXT:vpabsw %ymm0, %ymm0
 ; CHECK-NEXT:ret{{[l|q]}}
   %arg = bitcast <4 x i64> %a0 to <16 x i16>
-  %sub = sub <16 x i16> zeroinitializer, %arg
-  %cmp = icmp sgt <16 x i16> %arg, zeroinitializer
-  %sel = select <16 x i1> %cmp, <16 x i16> %arg, <16 x i16> %sub
-  %res = bitcast <16 x i16> %sel to <4 x i64>
+  %abs = call <16 x i16> @llvm.abs.v16i16(<16 x i16> %arg, i1 false)
+  %res = bitcast <16 x i16> %abs to <4 x i64>
   ret <4 x i64> %res
 }
-declare <16 x i16> @llvm.x86.avx2.pabs.w(<16 x i16>) nounwind readnone
+declare <16 x i16> @llvm.abs.v16i16(<16 x i16>, i1) nounwind readnone
 
 define <4 x i64> @test_mm256_abs_epi32(<4 x i64> %a0) {
 ; CHECK-LABEL: test_mm256_abs_epi32:
@@ -38,13 +34,11 @@

[PATCH] D82727: [PowerPC] Implement Vector Expand Mask builtins in LLVM/Clang

2020-09-03 Thread Albion Fung via Phabricator via cfe-commits

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

Minor nit, okay if changed for commit


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82727

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

[PATCH] D77491: [Sema] Introduce BuiltinAttr, per-declaration builtin-ness

2020-09-03 Thread John McCall via Phabricator via cfe-commits

rjmccall added a comment.

I didn't see the specific example, sorry.  I agree that my description is more 
applicable to builtins in the `__builtin` namespace, which describes most of 
the builtins with custom typechecking.  Is the problem just `__va_start`?

If we have to treat all declarations as builtins for the custom-typechecking 
builtins just to land this patch, I don't think that's the worst result in the 
world, and we can incrementally go from there.  `__va_start` actually has a 
signature, it just effectively has optional arguments, which is something we 
could definitely teach the builtins database and signature-matcher about.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77491

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

[PATCH] D82727: [PowerPC] Implement Vector Expand Mask builtins in LLVM/Clang

2020-09-03 Thread Albion Fung via Phabricator via cfe-commits

Conanap added inline comments.



Comment at: llvm/lib/Target/PowerPC/PPCInstrPrefix.td:993
+ [(set v16i8:$vD, 
(int_ppc_altivec_vexpandbm
+  v16i8:$vB))]>;
   def VEXPANDHM : VXForm_RD5_XO5_RS5<1602, 1, (outs vrrc:$vD), (ins vrrc:$vB),

Nit: Please make this indentation inline with the others


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82727

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

[PATCH] D86796: [Sema] Address-space sensitive index check for unbounded arrays

2020-09-03 Thread Chris Hamilton via Phabricator via cfe-commits

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



Comment at: clang/lib/Sema/SemaChecking.cpp:13966
   if (index.isUnsigned() || !index.isNegative()) {
-// It is possible that the type of the base expression after
-// IgnoreParenCasts is incomplete, even though the type of the base
-// expression before IgnoreParenCasts is complete (see PR39746 for an
-// example). In this case we have no information about whether the array
-// access exceeds the array bounds. However we can still diagnose an array
-// access which precedes the array bounds.
-if (BaseType->isIncompleteType())
-  return;
+if (isUnboundedArray) {
+  const auto  = getASTContext();

ebevhan wrote:
> chrish_ericsson_atx wrote:
> > ebevhan wrote:
> > > It might simplify the patch to move this condition out of the tree and 
> > > just early return for the other case. That is:
> > > 
> > > ```
> > > if (isUnboundedArray) {
> > >   if (!(index.isUnsigned() || !index.isNegative()))
> > > return;
> > > 
> > >   ...
> > >   return;
> > > }
> > > 
> > > if (index.isUnsigned() ...
> > > ```
> > There's a bit more code (starting at line 14094 in this patch set) that 
> > applies in all cases, so an early return here would prevent the "Array 
> > declared here" note from being generated.
> Ah, the note.
> 
> I wonder if it wouldn't be cleaner (and avoid indenting the entire block) if 
> that was just duplicated.
Hard to say which is cleaner -- it's a tradeoff.  Nesting level would be 
shallower if that code was duplicated, but then again, the duplication 
increases the chance of an incomplete fix should an issue be discovered in that 
code.  Overall code would be slightly longer, as well (adding about 16 lines of 
code, but removing only 4).   To me, the current strategy feels more surgical, 
but I'll change it if you feel more strongly about it than I do.   Please 
re-ping if you do.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86796

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

[PATCH] D86290: Move all fields of '-cc1' option related classes into def file databases

2020-09-03 Thread Daniel Grumberg via Phabricator via cfe-commits

dang added a comment.

Yes of course. This is a prerequisite for some other changes that are waiting 
to land and need polishing. Thanks for doing the revert. I will investigate the 
failures and recommit it when appropriate.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86290

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

[PATCH] D82726: [PowerPC] Implement Vector Count Mask Bits builtins in LLVM/Clang

2020-09-03 Thread Amy Kwan via Phabricator via cfe-commits

amyk added a comment.

Ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82726

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

[PATCH] D82725: [PowerPC] Implement Move to VSR Mask builtins in LLVM/Clang

2020-09-03 Thread Amy Kwan via Phabricator via cfe-commits

amyk added a comment.

Ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82725

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

[PATCH] D86796: [Sema] Address-space sensitive index check for unbounded arrays

2020-09-03 Thread Chris Hamilton via Phabricator via cfe-commits

chrish_ericsson_atx updated this revision to Diff 289753.
chrish_ericsson_atx marked 2 inline comments as done.
chrish_ericsson_atx added a comment.

Addressed reviewer feedback.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86796

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaChecking.cpp
  clang/test/Sema/const-eval.c
  clang/test/Sema/unbounded-array-bounds.c
  clang/test/SemaCXX/constant-expression-cxx1y.cpp

Index: clang/test/SemaCXX/constant-expression-cxx1y.cpp
===
--- clang/test/SemaCXX/constant-expression-cxx1y.cpp
+++ clang/test/SemaCXX/constant-expression-cxx1y.cpp
@@ -1018,8 +1018,9 @@
 }
 
 constexpr void PR28739(int n) { // expected-error {{never produces a constant}}
-  int *p = 
+  int *p =   // expected-note {{declared here}}
   p += (__int128)(unsigned long)-1; // expected-note {{cannot refer to element 18446744073709551615 of non-array object in a constant expression}}
+  // expected-warning@-1 {{refers past the last possible element}}
 }
 
 constexpr void Void(int n) {
Index: clang/test/Sema/unbounded-array-bounds.c
===
--- /dev/null
+++ clang/test/Sema/unbounded-array-bounds.c
@@ -0,0 +1,70 @@
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -fsyntax-only %s 2>&1 | FileCheck --check-prefix=CHECK-X86-ADDR64 %s  \
+// RUN:  --implicit-check-not 'past the last possible element'
+// RUN: %clang_cc1 -triple i386-pc-linux-gnu   -fsyntax-only %s 2>&1 | FileCheck --check-prefix=CHECK-I386-ADDR32 %s \
+// RUN:  --implicit-check-not 'past the last possible element'
+// RUN: %clang_cc1 -triple avr-pc-linux-gnu-fsyntax-only %s 2>&1 | FileCheck --check-prefix=CHECK-AVR-ADDR16 %s  \
+// RUN:  --implicit-check-not 'past the last possible element'
+
+struct S {
+  long long a;
+  char b;
+  long long c;
+  short d;
+};
+
+struct S s[];
+
+void f1() {
+  ++s[3].a;
+  ++s[7073650413200313099].b;
+  // CHECK-X86-ADDR64:  :[[@LINE-1]]:5: warning: {{.*}} past the last possible element {{.*}} in 64-bit {{.*}} (max possible 576460752303423488 elements)
+  // CHECK-I386-ADDR32: :[[@LINE-2]]:5: warning: {{.*}} past the last possible element {{.*}} in 32-bit {{.*}} (max possible 178956970 elements)
+  // CHECK-AVR-ADDR16:  :[[@LINE-3]]:5: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 3276 elements)
+  ++s[7073650].c;
+  // CHECK-AVR-ADDR16:  :[[@LINE-1]]:5: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 3276 elements)
+}
+
+long long ll[];
+
+void f2() {
+  ++ll[3];
+  ++ll[2705843009213693952];
+  // CHECK-X86-ADDR64:  :[[@LINE-1]]:5: warning: {{.*}} past the last possible element {{.*}} in 64-bit {{.*}} (max possible 2305843009213693952 elements)
+  // CHECK-I386-ADDR32: :[[@LINE-2]]:5: warning: {{.*}} past the last possible element {{.*}} in 32-bit {{.*}} (max possible 536870912 elements)
+  // CHECK-AVR-ADDR16:  :[[@LINE-3]]:5: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 8192 elements)
+  ++ll[847073650];
+  // CHECK-I386-ADDR32: :[[@LINE-1]]:5: warning: {{.*}} past the last possible element {{.*}} in 32-bit {{.*}} (max possible 536870912 elements)
+  // CHECK-AVR-ADDR16:  :[[@LINE-2]]:5: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 8192 elements)
+}
+
+void f3(struct S p[]) {
+  ++p[3].a;
+  ++p[7073650413200313099].b;
+  // CHECK-X86-ADDR64:  :[[@LINE-1]]:5: warning: {{.*}} past the last possible element {{.*}} in 64-bit {{.*}} (max possible 576460752303423488 elements)
+  // CHECK-I386-ADDR32: :[[@LINE-2]]:5: warning: {{.*}} past the last possible element {{.*}} in 32-bit {{.*}} (max possible 178956970 elements)
+  // CHECK-AVR-ADDR16:  :[[@LINE-3]]:5: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 3276 elements)
+  ++p[7073650].c;
+  // CHECK-AVR-ADDR16:  :[[@LINE-1]]:5: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 3276 elements)
+}
+
+void f4(struct S *p) {
+  p += 3;
+  p += 7073650413200313099;
+  // CHECK-X86-ADDR64:  :[[@LINE-1]]:3: warning: {{.*}} past the last possible element {{.*}} in 64-bit {{.*}} (max possible 576460752303423488 elements)
+  // CHECK-I386-ADDR32: :[[@LINE-2]]:3: warning: {{.*}} past the last possible element {{.*}} in 32-bit {{.*}} (max possible 178956970 elements)
+  // CHECK-AVR-ADDR16:  :[[@LINE-3]]:3: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 3276 elements)
+  p += 7073650;
+  // CHECK-AVR-ADDR16:  :[[@LINE-1]]:3: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 3276 elements)
+}
+
+struct BQ {
+  struct S bigblock[3276];
+};
+
+struct BQ bq[];
+
+void f5() {
+

[PATCH] D87097: [analyzer][StdLibraryFunctionsChecker] Do not match based on the restrict qualifier in C++

2020-09-03 Thread Gabor Marton via Phabricator via cfe-commits

martong created this revision.
martong added reviewers: balazske, steakhal, Szelethus, NoQ, vsavchenko, 
baloghadamsoftware, gamesh411.
Herald added subscribers: cfe-commits, ASDenysPetrov, Charusso, dkrupp, 
donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet, xazax.hun, whisperity.
Herald added a project: clang.
martong requested review of this revision.

The "restrict" keyword is illegal in C++, however, many libc
implementations use the "__restrict" compiler intrinsic in functions
prototypes. The "__restrict" keyword qualifies a type as a restricted type
even in C++.
In case of any non-C99 languages, we don't want to match based on the
restrict qualifier because we cannot know if the given libc implementation
qualifies the paramter type or not.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D87097

Files:
  clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
  clang/test/Analysis/std-c-library-functions-restrict.c
  clang/test/Analysis/std-c-library-functions-restrict.cpp

Index: clang/test/Analysis/std-c-library-functions-restrict.cpp
===
--- /dev/null
+++ clang/test/Analysis/std-c-library-functions-restrict.cpp
@@ -0,0 +1,25 @@
+// RUN: %clang_analyze_cc1 %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=apiModeling.StdCLibraryFunctions \
+// RUN:   -analyzer-checker=debug.StdCLibraryFunctionsTester \
+// RUN:   -analyzer-config apiModeling.StdCLibraryFunctions:DisplayLoadedSummaries=true \
+// RUN:   -triple i686-unknown-linux 2>&1 | FileCheck %s
+
+// The signatures for these functions are the same and they specify their
+// parameter with the restrict qualifier. In C++, however, we are more
+// indulgent and we do not match based on this qualifier. Thus, the given
+// signature should match for both of the declarations below, i.e the summary
+// should be loaded for both of them.
+void __test_restrict_param_0(void *p);
+void __test_restrict_param_1(void *__restrict p);
+// The below declaration is illegal, "restrict" is not a keyword in C++.
+// void __test_restrict_param_2(void *restrict p);
+
+// CHECK: Loaded summary for: void __test_restrict_param_0(void *p)
+// CHECK: Loaded summary for: void __test_restrict_param_1(void *__restrict p)
+
+// Must have at least one call expression to initialize the summary map.
+int bar(void);
+void foo() {
+  bar();
+}
Index: clang/test/Analysis/std-c-library-functions-restrict.c
===
--- /dev/null
+++ clang/test/Analysis/std-c-library-functions-restrict.c
@@ -0,0 +1,24 @@
+// RUN: %clang_analyze_cc1 %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=apiModeling.StdCLibraryFunctions \
+// RUN:   -analyzer-checker=debug.StdCLibraryFunctionsTester \
+// RUN:   -analyzer-config apiModeling.StdCLibraryFunctions:DisplayLoadedSummaries=true \
+// RUN:   -triple i686-unknown-linux 2>&1 | FileCheck %s
+
+// The signatures for these functions are the same and they specify their
+// parameter with the restrict qualifier. In C the signature should match only
+// if the restrict qualifier is there on the parameter. Thus, the summary
+// should be loaded for the last two declarations only.
+void __test_restrict_param_0(void *p);
+void __test_restrict_param_1(void *__restrict p);
+void __test_restrict_param_2(void *restrict p);
+
+// CHECK-NOT: Loaded summary for: void __test_restrict_param_0
+// CHECK: Loaded summary for: void __test_restrict_param_1(void *restrict p)
+// CHECK: Loaded summary for: void __test_restrict_param_2(void *restrict p)
+
+// Must have at least one call expression to initialize the summary map.
+int bar(void);
+void foo() {
+  bar();
+}
Index: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -744,21 +744,38 @@
 bool StdLibraryFunctionsChecker::Signature::matches(
 const FunctionDecl *FD) const {
   assert(!isInvalid());
-  // Check number of arguments:
+  // Check the number of arguments.
   if (FD->param_size() != ArgTys.size())
 return false;
 
-  // Check return type.
-  if (!isIrrelevant(RetTy))
-if (RetTy != FD->getReturnType().getCanonicalType())
+  // The "restrict" keyword is illegal in C++, however, many libc
+  // implementations use the "__restrict" compiler intrinsic in functions
+  // prototypes. The "__restrict" keyword qualifies a type as a restricted type
+  // even in C++.
+  // In case of any non-C99 languages, we don't want to match based on the
+  // restrict qualifier because we cannot know if the given libc implementation
+  // qualifies the paramter type or not.
+  auto RemoveRestrict = [](QualType T) {
+if (!FD->getASTContext().getLangOpts().C99)
+  T.removeLocalRestrict();
+

[PATCH] D87095: [Triple][MachO] Define "arm64e", an AArch64 subarch for Pointer Auth.

2020-09-03 Thread Ahmed Bougacha via Phabricator via cfe-commits

ab created this revision.
Herald added subscribers: cfe-commits, danielkiss, cmtice, rupprecht, 
dexonsmith, steven_wu, hiraditya, kristof.beyls, arichardson.
Herald added a reviewer: jhenderson.
Herald added a reviewer: MaskRay.
Herald added projects: clang, LLVM.
ab requested review of this revision.
Herald added a subscriber: ormris.

This is the first of many Pointer Authentication-related patches we're happy to 
finally upstream =]
For a high-level overview, see our llvm-dev RFC: 
http://lists.llvm.org/pipermail/llvm-dev/2019-October/136091.html, as well as 
the devmtg talk we did at the same time last year.
For concrete code that builds on this, see the staging PR in 
apple/llvm-project: https://github.com/apple/llvm-project/pull/14.  Though 
we've made changes downstream since then, the general concepts and added 
constructs are mostly identical.

Per commit message:

  This also teaches MachO writers/readers about the MachO cpu subtype,
  beyond the minimal subtype reader support present at the moment.
  
  This also defines a preprocessor macro to allow users to distinguish
  __arm64__ from __arm64e__.
  
  arm64e defaults to an "apple-a12" CPU, which supports v8.3a, allowing
  pointer-authentication codegen.
  It also currently defaults to ios14+ and macos11+.

If it helps, we can obviously split this patch further, but I feel like this 
all belongs together, as the core boilerplate needed for new (darwin) targets.

Note that this adds a `Triple::isArm64e()`, which is a bit of a departure from 
prior subarches, in part because we check it more often.  The obvious 
alternative would be to compare `Triple::getArchName()` with "arm64e";  I'm 
fine with either.   However, I would like to preserve the "arm64e" naming, as 
we refer to that extensively in Darwin-land.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D87095

Files:
  clang/lib/Basic/Targets/AArch64.cpp
  clang/lib/Driver/ToolChain.cpp
  clang/lib/Driver/ToolChains/Arch/AArch64.cpp
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/test/Driver/aarch64-cpus.c
  clang/test/Driver/arclite-link.c
  clang/test/Driver/target-triple-deployment.c
  clang/test/Preprocessor/arm64e.c
  llvm/include/llvm/ADT/Triple.h
  llvm/lib/BinaryFormat/MachO.cpp
  llvm/lib/LTO/LTOCodeGenerator.cpp
  llvm/lib/LTO/LTOModule.cpp
  llvm/lib/Object/MachOObjectFile.cpp
  llvm/lib/Support/ARMTargetParser.cpp
  llvm/lib/Support/Triple.cpp
  llvm/lib/Target/AArch64/AArch64TargetMachine.cpp
  llvm/lib/Target/AArch64/MCTargetDesc/AArch64MCTargetDesc.cpp
  llvm/test/MC/AArch64/arm64e-subtype.s
  llvm/test/MC/AArch64/arm64e.s
  llvm/test/MC/MachO/AArch64/arm-darwin-version-min-load-command.s
  llvm/test/tools/llvm-dwarfdump/AArch64/arm64e.ll
  llvm/test/tools/llvm-objdump/MachO/universal-arm64.test
  llvm/test/tools/llvm-readobj/macho-arm64e.test
  llvm/unittests/ADT/TripleTest.cpp
  llvm/utils/UpdateTestChecks/asm.py

Index: llvm/utils/UpdateTestChecks/asm.py
===
--- llvm/utils/UpdateTestChecks/asm.py
+++ llvm/utils/UpdateTestChecks/asm.py
@@ -335,6 +335,7 @@
   'amdgcn': (scrub_asm_amdgpu, ASM_FUNCTION_AMDGPU_RE),
   'arm': (scrub_asm_arm_eabi, ASM_FUNCTION_ARM_RE),
   'arm64': (scrub_asm_arm_eabi, ASM_FUNCTION_AARCH64_RE),
+  'arm64e': (scrub_asm_arm_eabi, ASM_FUNCTION_AARCH64_DARWIN_RE),
   'arm64-apple-ios': (scrub_asm_arm_eabi, ASM_FUNCTION_AARCH64_DARWIN_RE),
   'armv7-apple-ios' : (scrub_asm_arm_eabi, ASM_FUNCTION_ARM_IOS_RE),
   'armv7-apple-darwin': (scrub_asm_arm_eabi, ASM_FUNCTION_ARM_DARWIN_RE),
Index: llvm/unittests/ADT/TripleTest.cpp
===
--- llvm/unittests/ADT/TripleTest.cpp
+++ llvm/unittests/ADT/TripleTest.cpp
@@ -1590,5 +1590,10 @@
 Triple T = Triple("aarch64_be");
 EXPECT_EQ(Triple::aarch64_be, T.getArch());
   }
+  {
+Triple T = Triple("arm64e");
+EXPECT_EQ(Triple::aarch64, T.getArch());
+EXPECT_EQ(Triple::AArch64SubArch_arm64e, T.getSubArch());
+  }
 }
 } // end anonymous namespace
Index: llvm/test/tools/llvm-readobj/macho-arm64e.test
===
--- /dev/null
+++ llvm/test/tools/llvm-readobj/macho-arm64e.test
@@ -0,0 +1,17 @@
+# RUN: yaml2obj %s -o %t.o
+# RUN: llvm-readobj -h %t.o | FileCheck %s
+
+# CHECK: Magic: Magic64 (0xFEEDFACF)
+# CHECK: CpuType: Arm64 (0x10C)
+# CHECK: CpuSubType: CPU_SUBTYPE_ARM64E (0x2)
+
+--- !mach-o
+FileHeader:
+  magic:   0xFEEDFACF
+  cputype: 0x010C
+  cpusubtype:  0x0002
+  filetype:0x0001
+  ncmds:   0
+  sizeofcmds:  0
+  flags:   0x
+  reserved:0x
Index: llvm/test/tools/llvm-objdump/MachO/universal-arm64.test
===
--- llvm/test/tools/llvm-objdump/MachO/universal-arm64.test
+++

[clang] c9239b2 - [Analyzer][docs][NFC] Fix typo in code example

2020-09-03 Thread Jan Korous via cfe-commits


Author: Jan Korous
Date: 2020-09-03T09:28:34-07:00
New Revision: c9239b2bf5f00b58aaa431955f24013e0cada0a3

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

LOG: [Analyzer][docs][NFC] Fix typo in code example

Added: 


Modified: 
clang/docs/analyzer/checkers.rst

Removed: 




diff  --git a/clang/docs/analyzer/checkers.rst 
b/clang/docs/analyzer/checkers.rst
index 3b378f735ebc..7a294f916bcf 100644
--- a/clang/docs/analyzer/checkers.rst
+++ b/clang/docs/analyzer/checkers.rst
@@ -1747,7 +1747,7 @@ Check for integer to enumeration casts that could result 
in undefined values.
  void foo() {
TestEnum t = static_cast(-1);
// warn: the value provided to the cast expression is not in
-the valid range of values for the enum
+   //   the valid range of values for the enum
 
 .. _alpha-cplusplus-InvalidatedIterator:
 



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

[PATCH] D87066: Thread safety analysis: Improve documentation for scoped capabilities

2020-09-03 Thread Aaron Puchert via Phabricator via cfe-commits

aaronpuchert added inline comments.



Comment at: clang/docs/ThreadSafetyAnalysis.rst:908
+// Assumes mu is held, implicitly acquires *this and connects it to mu.
+MutexLocker(t_mutex , adopt_lock_t) REQUIRES(mu) : mut(mu) {}
+

`s/t_mutex/Mutex/g`



Comment at: clang/docs/ThreadSafetyAnalysis.rst:922
+// Releases *this and all underlying capabilities, if they are still held.
+// There is no warning on double unlock.
 ~MutexLocker() RELEASE() {

aaron.ballman wrote:
> This makes it sound like we're missing a diagnostic, but IIRC, this is 
> intended behavior. Maybe we want to say there is purposefully no warning on 
> double unlock and why? Or add a FIXME if I'm remembering wrong and this isn't 
> intended behavior.
It is intentional (we have special handling for the destructor), and I also 
think it makes sense. I just wanted to clarify the difference between this and 
`Unlock`: calling `Unlock` twice is not allowed, but calling `Unlock` and then 
(implicitly) the destructor is allowed.

I could rephrase this as "There is no warning if the scope was already unlocked 
before." Could also add an "intentional" or "deliberate", but since it's 
documented I think the reader can infer that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87066

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

[PATCH] D77491: [Sema] Introduce BuiltinAttr, per-declaration builtin-ness

2020-09-03 Thread Raul Tambre via Phabricator via cfe-commits

tambre added a comment.

In D77491#2254065 , @rjmccall wrote:

> The builtins with custom type-checking are all true intrinsics like 
> `__builtin_operator_new` and so on.  They really can't be validly declared by 
> the user program.  The thing that seems most likely to avoid random compiler 
> crashes would be to either forbid explicit declarations of them or treat 
> those as no longer being builtins.  If we need to maintain compatibility with 
> people making custom declarations, we would need to always treat them as 
> builtins and run the risk of crashing if someone declares one with a bad 
> signature.  But I don't think it's unfair of us to break those people; that 
> is seriously not reasonable user behavior.
>
> It's possible that custom declarations are people trying to create 
> compatibility shims for compilers that don't provide these as builtins.  
> Those people should be guarding their custom declarations, preferably with 
> `__has_builtin`.

I fully agree.

However, I believe you forget to account for the example that I brought up.
In particular, MSVC's header `vadefs.h` includes a declaration of 
`__va_start()`, which would cause almost any code including standard headers to 
fail to compile on Windows.
This issue isn't isolated to some old MSVC version, as the declaration is still 
there in the latest Visual Studio Preview version 14.28.29213.

How about turning it into an error only on non-Windows?
Though keeping that as a followup might be even better, as this will probably 
be merged into 11.0.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77491

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

[PATCH] D87080: [AST] Reduce the size of TemplateArgumentLocInfo.

2020-09-03 Thread Sam McCall via Phabricator via cfe-commits

sammccall added a comment.

Nit on the commit message: I think this is TemplateArgumentLoc 48 -> 32 bytes, 
and DynTypedNode 56 -> 40 bytes.




Comment at: clang/include/clang/AST/TemplateBase.h:415
 
-public:
-  constexpr TemplateArgumentLocInfo() : Template({nullptr, nullptr, 0, 0}) {}
+  llvm::PointerIntPair PointerAndKind;
 

Can you use PointerUnion, and save even more of the 
boilerplate?



Comment at: clang/include/clang/AST/TemplateBase.h:429
+  auto *T = getTemplate();
+  T->Ctx->Deallocate(T);
+}

this is a no-op, and thus not worth stashing a pointer to Ctx for!

It also doesn't delete T, and it's probably best to do that even if it's 
(currently) a no-op



Comment at: clang/include/clang/AST/TemplateBase.h:429
+  auto *T = getTemplate();
+  T->Ctx->Deallocate(T);
+}

sammccall wrote:
> this is a no-op, and thus not worth stashing a pointer to Ctx for!
> 
> It also doesn't delete T, and it's probably best to do that even if it's 
> (currently) a no-op
If you're going to destroy T in the destructor, then you can't have trivial 
copies, as the second one is pointing at deallocated memory.
(Well, apart from the fact that deallocation does nothing).

So I think we probably either want:
 - allocation on ASTContext, trivial copies, no deallocation
 - allocation on heap, copies reallocate
 - allocation on heap using shared_ptr
 - copies disallowed (but I think we rely on them being available)



Comment at: clang/include/clang/AST/TemplateBase.h:443
   SourceLocation EllipsisLoc) {
-Template.Qualifier = QualifierLoc.getNestedNameSpecifier();
-Template.QualifierLocData = QualifierLoc.getOpaqueData();
-Template.TemplateNameLoc = TemplateNameLoc.getRawEncoding();
-Template.EllipsisLoc = EllipsisLoc.getRawEncoding();
+T *Template = Ctx.Allocate();
+Template->Qualifier = QualifierLoc.getNestedNameSpecifier();

this is not "heap allocation" in the usual sense - allocating in the context's 
slab allocator is cheaper but can't be deallocated.

Not sure how this compares to simply `new T` instead.

If not, then as with delete, I'd prefer `new (Ctx) T` (I think it's correct 
without as long as T is trivial, but less obvious)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87080

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

[PATCH] D86796: [Sema] Address-space sensitive index check for unbounded arrays

2020-09-03 Thread Chris Hamilton via Phabricator via cfe-commits

chrish_ericsson_atx marked 3 inline comments as done.
chrish_ericsson_atx added inline comments.



Comment at: clang/lib/Sema/SemaChecking.cpp:13981
+bool overflow;
+llvm::APInt product(index);
+product += 1;

ebevhan wrote:
> What if index is wider than AddrBits, but the active bits are fewer? Then you 
> might miss out on triggering the overflow case in the multiplication.
Line 13984 checks for active bits of product being less than AddrBits, which is 
the same case (since product, by definition, has same width as index).  So I 
think this is covered.  If I've misunderstood, please re-ping.



Comment at: clang/lib/Sema/SemaChecking.cpp:13992
+  MaxElems =
+  MaxElems.zext(std::max(AddrBits << 1, apElemBytes.getBitWidth()));
+  MaxElems += 1;

ebevhan wrote:
> Should this not be AddrBits + 1 if you want to add 1 below?
AddrBits + 1 would also work.  I chose AddrBits << 1 figuring that the width 
would end up being a nice clean power of 2, but that's not necessarily true.  
Functionally, the effect of either approach is identical.  I suppose + 1 is a 
bit more obvious, though.  I'll make this change.



Comment at: clang/lib/Sema/SemaChecking.cpp:13993
+  MaxElems.zext(std::max(AddrBits << 1, apElemBytes.getBitWidth()));
+  MaxElems += 1;
+  if (MaxElems.getBitWidth() < apElemBytes.getBitWidth())

ebevhan wrote:
> Though, why is the +1 here? Isn't this already the maximum number of elements?
Initial value of MaxElems is APInt::getMaxValue(AddrBits), which is the index 
of the last addressable CharUnit in the address space.  Adding 1 makes it the 
total number of addressable CharUnits in the address space, which is what we 
want as the numerator for computing total number of elements of a given size 
that will fit in that address space.




Comment at: clang/lib/Sema/SemaChecking.cpp:13994-13997
+  if (MaxElems.getBitWidth() < apElemBytes.getBitWidth())
+MaxElems = MaxElems.zext(apElemBytes.getBitWidth());
+  else if (apElemBytes.getBitWidth() < MaxElems.getBitWidth())
+apElemBytes = apElemBytes.zext(MaxElems.getBitWidth());

ebevhan wrote:
> MaxElems should already be at a sufficient width here because of the earlier 
> max. You can probably just do apElemBytes = 
> apElemBytes.zextOrTrunc(MaxElems.getBitWidth())?
Yep -- you are right.  index and apElemBytes are already guaranteed to have 
equal width (>= AddrBits), and MaxElems is guaranteed to have the same width or 
double width.  So the single zext of apElemBytes will do fine.  Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86796

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

[PATCH] D86874: [analyzer] Fix ArrayBoundCheckerV2 false positive regarding size_t indexer

2020-09-03 Thread Balázs Benics via Phabricator via cfe-commits

steakhal added a comment.

In D86874#inline-803844 , 
@martong wrote:

> I really feel that this check would have a better place in the implementation 
> of `eval`. This seems really counter-intuitive to do this stuff at the 
> Checker's level. Is there any reason why we can't do this in `eval`?
> `evalBinOpNN` could return with Unknown, and the state should remain 
> unchanged. Am I missing something?

Ah, sort of.
To make everything clear, I have to provide examples, building-blocks, 
reasoning and stuff, so please don't be mad if it's too long.
**I hope you have a warm cup of tee to read through this - seriously - you will 
need that!**

Diving in
-

It is really important to make a clear distinction between evaluating an 
expression according to the semantics of the //meta-language// or of the 
//object-language//, because we might get different answers for the same 
expression.

In fact, `evalBinOpNN` evaluates expressions according to the semantics of the 
//object-language//.

In our context, meta-language is //mathematics//, and the //object-language// 
is the semantics of the abstract machine of the c/c++ language.
In mathematics, there is no such thing as overflow or value ranges, unlike in 
C++.

Let's see an example:
Assuming that `x` is in range `[1,UINT_MAX]`.
`x + 1` will be in range `[2,ULL_MAX+1]` in the //meta-language//.
`x + 1` will be in range `[0,0]u[2,UINT_MAX]` or in `[2,UINT_MAX+1]` weather 
the type of `x` is capable representing the (+1) value and the overflow is 
well-defined or not.

Another valuable comment is that non-contiguous ranges (range sets) are not 
really useful.
Knowing that `x` is in range `[0,0]u[2,UINT_MAX]` doesn't help much if you want 
to prove that:
`x < 5` holds for all possible interpretations of `x`.
We can not disproof that either.

So, overflow/underflow can really screw the value ranges, preventing us from 
evaluating expressions over relational operators.
Which is technically accurate, but not satisfiable in all cases - like in the 
checker `ArrayBoundCheckerV2`.

---

Before describing why is it so problematic in this checker, I should give an 
overview, how this checker works.

Overview of the logic of the ArrayBoundCheckerV2

The checker checks `Location` accesses (aka. pointer dereferences).
We construct the `RegionRawOffsetV2` object of the access (Which consists of 
the //base region//, and the symbolic //byte offset// expression of the access).

Then we check, whether we access an element //preceding// the **first valid 
index** of the //base// memory region.
In other words, we check if the //byte offset// symbolic expression is **less 
then** 0:

- If YES:   Report that we access an out-of-bounds element.
- If NO:Infer the range constraints on the symbol and add the constraint to 
make this array access valid.
- If MAYBE: Infer and constrain, just as you would do in the previous case.

Then we check, whether we access an element //exceeding// the **last valid 
index** of the memory region.
In other words, we check if the //byte offset// symbolic expression is greater 
then or equal to the //extent// of the region:

- If YES:   Report the bug...
- If NO:Infer & constrain...
- If MAYBE: Infer & constrain...

However, in the checker, we don't check `byte offset < 0` directly.
We //simplify// the left part of the `byte offset < 0` inequality by 
substracting/dividing both sides with the constant `C`, if the `byte offset` is 
a `SymintExpr` of `SymExpr OP C` over the plus or multiplication operator (OP).
We do this //simplification// recursively.
In the end, we will have a //symbolic root// (call it `RootSym`) expression and 
a `C'` constant over the right-hand side of the original relational operator.
So, after the //simplification// process we get the `RootSym < C'` inequality, 
which we check.

This //simplification// is the //infer// operation in the pseudo-code.
And the //constrain// step is where we assume that the negation of `RootSym < 
C'` holds.

**SPOILER**: This //simplification// process is only **valid** using the 
semantics of the //meta-language//.

ElementRegions
--

Pointer values, which describe a particular element of a memory region, can get 
quite complex.
Even more complex, when we reinterpret cast a pointer to a different type.
In such cases, we might wrap the `SubRegion` symbol within an `ElementRegion` 
with the given target type and offset `0`.
Eg:

  void foo(int x) {
int buf[10];
char *p = (char*)(buf + 2) + 3;
^-- 2*sizeof(int) in bytes which is 8.
*p = 42; // Not UB.
int *q = (int*)p; // unaligned pointer!
*q = 5; // UB.

char *r = (char*)(buf + x) + 3;
*r = 66; // Might be safe, if x has a value to make it so.
  }

RegionRawOffsetV2
-

In the previous example `p` would have the `{buf,11 S64b,char}` 
`SymbolRegionVal`

[PATCH] D87031: [libTooling] Provide overloads of `rewriteDescendants` that operate directly on an AST node.

2020-09-03 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rGd4f390313129: [libTooling] Provide overloads of 
`rewriteDescendants` that operate directly on… (authored by ymandel).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87031

Files:
  clang/include/clang/Tooling/Transformer/RewriteRule.h
  clang/lib/Tooling/Transformer/RewriteRule.cpp
  clang/unittests/Tooling/TransformerTest.cpp

Index: clang/unittests/Tooling/TransformerTest.cpp
===
--- clang/unittests/Tooling/TransformerTest.cpp
+++ clang/unittests/Tooling/TransformerTest.cpp
@@ -25,6 +25,7 @@
 using ::testing::IsEmpty;
 using transformer::cat;
 using transformer::changeTo;
+using transformer::rewriteDescendants;
 using transformer::RewriteRule;
 
 constexpr char KHeaderContents[] = R"cc(
@@ -568,6 +569,88 @@
   EXPECT_EQ(ErrorCount, 1);
 }
 
+//
+// We include one test per typed overload. We don't test extensively since that
+// is already covered by the tests above.
+//
+
+TEST_F(TransformerTest, RewriteDescendantsTypedStmt) {
+  // Add an unrelated definition to the header that also has a variable named
+  // "x", to test that the rewrite is limited to the scope we intend.
+  appendToHeader(R"cc(int g(int x) { return x; })cc");
+  std::string Input =
+  "int f(int x) { int y = x; { int z = x * x; } return x; }";
+  std::string Expected =
+  "int f(int x) { int y = 3; { int z = 3 * 3; } return 3; }";
+  auto InlineX =
+  makeRule(declRefExpr(to(varDecl(hasName("x", changeTo(cat("3")));
+  testRule(makeRule(functionDecl(hasName("f"), hasBody(stmt().bind("body"))),
+[](const MatchFinder::MatchResult ) {
+  const auto *Node = R.Nodes.getNodeAs("body");
+  assert(Node != nullptr && "body must be bound");
+  return transformer::detail::rewriteDescendants(
+  *Node, InlineX, R);
+}),
+   Input, Expected);
+}
+
+TEST_F(TransformerTest, RewriteDescendantsTypedDecl) {
+  std::string Input =
+  "int f(int x) { int y = x; { int z = x * x; } return x; }";
+  std::string Expected =
+  "int f(int x) { int y = 3; { int z = 3 * 3; } return 3; }";
+  auto InlineX =
+  makeRule(declRefExpr(to(varDecl(hasName("x", changeTo(cat("3")));
+  testRule(makeRule(functionDecl(hasName("f")).bind("fun"),
+[](const MatchFinder::MatchResult ) {
+  const auto *Node = R.Nodes.getNodeAs("fun");
+  assert(Node != nullptr && "fun must be bound");
+  return transformer::detail::rewriteDescendants(
+  *Node, InlineX, R);
+}),
+   Input, Expected);
+}
+
+TEST_F(TransformerTest, RewriteDescendantsTypedTypeLoc) {
+  std::string Input = "int f(int *x) { return *x; }";
+  std::string Expected = "int f(char *x) { return *x; }";
+  auto IntToChar =
+  makeRule(typeLoc(loc(qualType(isInteger(), builtinType(.bind("loc"),
+   changeTo(cat("char")));
+  testRule(
+  makeRule(
+  functionDecl(
+  hasName("f"),
+  hasParameter(0, varDecl(hasTypeLoc(typeLoc().bind("parmType"),
+  [](const MatchFinder::MatchResult ) {
+const auto *Node = R.Nodes.getNodeAs("parmType");
+assert(Node != nullptr && "parmType must be bound");
+return transformer::detail::rewriteDescendants(*Node, IntToChar, R);
+  }),
+  Input, Expected);
+}
+
+TEST_F(TransformerTest, RewriteDescendantsTypedDynTyped) {
+  // Add an unrelated definition to the header that also has a variable named
+  // "x", to test that the rewrite is limited to the scope we intend.
+  appendToHeader(R"cc(int g(int x) { return x; })cc");
+  std::string Input =
+  "int f(int x) { int y = x; { int z = x * x; } return x; }";
+  std::string Expected =
+  "int f(int x) { int y = 3; { int z = 3 * 3; } return 3; }";
+  auto InlineX =
+  makeRule(declRefExpr(to(varDecl(hasName("x", changeTo(cat("3")));
+  testRule(
+  makeRule(functionDecl(hasName("f"), hasBody(stmt().bind("body"))),
+   [](const MatchFinder::MatchResult ) {
+ auto It = R.Nodes.getMap().find("body");
+ assert(It != R.Nodes.getMap().end() && "body must be bound");
+ return transformer::detail::rewriteDescendants(It->second,
+InlineX, R);
+   }),
+  Input, Expected);
+}
+
 TEST_F(TransformerTest, InsertBeforeEdit) {
   std::string Input = R"cc(
 int f() {
Index: clang/lib/Tooling/Transformer/RewriteRule.cpp
===
--- clang/lib/Tooling/Transformer/RewriteRule.cpp
+++

[clang] d4f3903 - [libTooling] Provide overloads of `rewriteDescendants` that operate directly on an AST node.

2020-09-03 Thread Yitzhak Mandelbaum via cfe-commits


Author: Yitzhak Mandelbaum
Date: 2020-09-03T14:39:50Z
New Revision: d4f3903131292d36b3bc22c28798b8e9dae20af6

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

LOG: [libTooling] Provide overloads of `rewriteDescendants` that operate 
directly on an AST node.

The new overloads apply directly to a node, like the
`clang::ast_matchers::match` functions, Rather than generating an
`EditGenerator` combinator.

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

Added: 


Modified: 
clang/include/clang/Tooling/Transformer/RewriteRule.h
clang/lib/Tooling/Transformer/RewriteRule.cpp
clang/unittests/Tooling/TransformerTest.cpp

Removed: 




diff  --git a/clang/include/clang/Tooling/Transformer/RewriteRule.h 
b/clang/include/clang/Tooling/Transformer/RewriteRule.h
index 9700d1ff539d..4bdcc8d5c329 100644
--- a/clang/include/clang/Tooling/Transformer/RewriteRule.h
+++ b/clang/include/clang/Tooling/Transformer/RewriteRule.h
@@ -380,6 +380,38 @@ EditGenerator rewriteDescendants(std::string NodeId, 
RewriteRule Rule);
 // RewriteRule API.  Recast them as such.  Or, just declare these functions
 // public and well-supported and move them out of `detail`.
 namespace detail {
+/// The following overload set is a version of `rewriteDescendants` that
+/// operates directly on the AST, rather than generating a Transformer
+/// combinator. It applies `Rule` to all descendants of `Node`, although not
+/// `Node` itself. `Rule` can refer to nodes bound in `Result`.
+///
+/// For example, assuming that "body" is bound to a function body in 
MatchResult
+/// `Results`, this will produce edits to change all appearances of `x` in that
+/// body to `3`.
+/// ```
+/// auto InlineX =
+/// makeRule(declRefExpr(to(varDecl(hasName("x", changeTo(cat("3")));
+/// const auto *Node = Results.Nodes.getNodeAs("body");
+/// auto Edits = rewriteDescendants(*Node, InlineX, Results);
+/// ```
+/// @{
+llvm::Expected>
+rewriteDescendants(const Decl , RewriteRule Rule,
+   const ast_matchers::MatchFinder::MatchResult );
+
+llvm::Expected>
+rewriteDescendants(const Stmt , RewriteRule Rule,
+   const ast_matchers::MatchFinder::MatchResult );
+
+llvm::Expected>
+rewriteDescendants(const TypeLoc , RewriteRule Rule,
+   const ast_matchers::MatchFinder::MatchResult );
+
+llvm::Expected>
+rewriteDescendants(const DynTypedNode , RewriteRule Rule,
+   const ast_matchers::MatchFinder::MatchResult );
+/// @}
+
 /// Builds a single matcher for the rule, covering all of the rule's cases.
 /// Only supports Rules whose cases' matchers share the same base "kind"
 /// (`Stmt`, `Decl`, etc.)  Deprecated: use `buildMatchers` instead, which

diff  --git a/clang/lib/Tooling/Transformer/RewriteRule.cpp 
b/clang/lib/Tooling/Transformer/RewriteRule.cpp
index 594e22f56b87..03921e0ea7de 100644
--- a/clang/lib/Tooling/Transformer/RewriteRule.cpp
+++ b/clang/lib/Tooling/Transformer/RewriteRule.cpp
@@ -242,7 +242,7 @@ class ApplyRuleCallback : public MatchFinder::MatchCallback 
{
 } // namespace
 
 template 
-static llvm::Expected>
+llvm::Expected>
 rewriteDescendantsImpl(const T , RewriteRule Rule,
const MatchResult ) {
   ApplyRuleCallback Callback(std::move(Rule));
@@ -252,10 +252,43 @@ rewriteDescendantsImpl(const T , RewriteRule Rule,
   return std::move(Callback.Edits);
 }
 
+llvm::Expected>
+transformer::detail::rewriteDescendants(const Decl , RewriteRule Rule,
+const MatchResult ) {
+  return rewriteDescendantsImpl(Node, std::move(Rule), Result);
+}
+
+llvm::Expected>
+transformer::detail::rewriteDescendants(const Stmt , RewriteRule Rule,
+const MatchResult ) {
+  return rewriteDescendantsImpl(Node, std::move(Rule), Result);
+}
+
+llvm::Expected>
+transformer::detail::rewriteDescendants(const TypeLoc , RewriteRule Rule,
+const MatchResult ) {
+  return rewriteDescendantsImpl(Node, std::move(Rule), Result);
+}
+
+llvm::Expected>
+transformer::detail::rewriteDescendants(const DynTypedNode ,
+RewriteRule Rule,
+const MatchResult ) {
+  if (const auto *Node = DNode.get())
+return rewriteDescendantsImpl(*Node, std::move(Rule), Result);
+  if (const auto *Node = DNode.get())
+return rewriteDescendantsImpl(*Node, std::move(Rule), Result);
+  if (const auto *Node = DNode.get())
+return rewriteDescendantsImpl(*Node, std::move(Rule), Result);
+
+  return llvm::make_error(
+  llvm::errc::invalid_argument,
+  "type unsupported for recursive rewriting, Kind=" +
+  DNode.getNodeKind().asStringRef());
+}
+
 EditGenerator

[PATCH] D84414: [RISCV] Support Shadow Call Stack

2020-09-03 Thread Z. Zheng via Phabricator via cfe-commits

zzheng added a comment.

ping..

@jrt, @lenary, @asb, IMHO, the patch is in good shape now, all concerns raised 
in comments has been addressed/answered, is there any additional comments 
before we can land it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84414

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

[PATCH] D86048: [AST][RecoveryExpr] Popagate the error-bit from a VarDecl's initializer to DeclRefExpr.

2020-09-03 Thread Sam McCall via Phabricator via cfe-commits

sammccall added a subscriber: rsmith.
sammccall added a comment.

Neither of the testcases look like the right behavior to me, and I think the 
bug is elsewhere in clang.

The crux is we're forcing `decltype(N)` to be a (unique) dependent type, which 
feels wrong.
This isn't specific to error-dependence, the same is true in

  template  void foo() {
constexpr int N = K;
decltype(K) // dependent?
  }

ASTContext::getDecltypeType() has to determine whether 
http://eel.is/c++draft/temp.type#4 applies.
There are three behaviors:

- standard up to C++14: traversing the expr looking for a template param to be 
lexically included (this is my reading of the standard)
- what clang actually does: check instantiation dependence, which I think pulls 
in too many cases
- standard after http://wg21.link/cwg2064: check type dependence

I think it's probably OK to adopt the C++17 behavior for all versions (if I'm 
right that the current behavior is a bug).
@rsmith It's your DR, what do you think :-)

(Per offline discussion, this was reduced from a related bug that didn't 
involve decltype, but I think we should treat that one as distinct)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86048

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

[PATCH] D87066: Thread safety analysis: Improve documentation for scoped capabilities

2020-09-03 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added inline comments.



Comment at: clang/docs/ThreadSafetyAnalysis.rst:922
+// Releases *this and all underlying capabilities, if they are still held.
+// There is no warning on double unlock.
 ~MutexLocker() RELEASE() {

This makes it sound like we're missing a diagnostic, but IIRC, this is intended 
behavior. Maybe we want to say there is purposefully no warning on double 
unlock and why? Or add a FIXME if I'm remembering wrong and this isn't intended 
behavior.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87066

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

[PATCH] D87065: Thread safety analysis: Document how try-acquire is handled

2020-09-03 Thread Aaron Ballman via Phabricator via cfe-commits

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

LGTM, thank you for documenting this, that would be confusing without spelling 
it out explicitly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87065

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

[PATCH] D87064: Thread safety analysis: Test and document release_generic_capability

2020-09-03 Thread Aaron Ballman via Phabricator via cfe-commits

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

LGTM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87064

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

[PATCH] D86508: [clang] improve GCC-compat for C90 __builtin_ functions

2020-09-03 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added a comment.

This looks reasonable to me.




Comment at: clang/docs/LanguageExtensions.rst:2370
 
+* ``bcmp``
 * ``memchr``

Can you mention the deprecation issue here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86508

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

[PATCH] D85351: [Analyzer] Fix for `ExprEngine::computeObjectUnderConstruction()` for base and delegating consturctor initializers

2020-09-03 Thread Balogh , Ádám via Phabricator via cfe-commits

baloghadamsoftware updated this revision to Diff 289703.
baloghadamsoftware added a comment.

Tests separated.


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

https://reviews.llvm.org/D85351

Files:
  clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
  clang/unittests/StaticAnalyzer/TestReturnValueUnderConstruction.cpp

Index: clang/unittests/StaticAnalyzer/TestReturnValueUnderConstruction.cpp
===
--- clang/unittests/StaticAnalyzer/TestReturnValueUnderConstruction.cpp
+++ clang/unittests/StaticAnalyzer/TestReturnValueUnderConstruction.cpp
@@ -51,22 +51,65 @@
 TEST(TestReturnValueUnderConstructionChecker,
  ReturnValueUnderConstructionChecker) {
   EXPECT_TRUE(runCheckerOnCode(
-  R"(class C {
- public:
-   C(int nn): n(nn) {}
-   virtual ~C() {}
- private:
-   int n;
- };
-
- C returnC(int m) {
-   C c(m);
-   return c;
- }
-
- void foo() {
-   C c = returnC(1); 
- })"));
+  R"(class C {
+ public:
+   C(int nn): n(nn) {}
+   virtual ~C() {}
+ private:
+   int n;
+ };
+
+ C returnC(int m) {
+   C c(m);
+   return c;
+ }
+
+ void foo() {
+   C c = returnC(1);
+ })"));
+
+  EXPECT_TRUE(runCheckerOnCode(
+  R"(class C {
+ public:
+   C(int nn): n(nn) {}
+   explicit C(): C(0) {}
+   virtual ~C() {}
+ private:
+   int n;
+ };
+
+ C returnC() {
+   C c;
+   return c;
+ }
+
+ void foo() {
+   C c = returnC();
+ })"));
+
+  EXPECT_TRUE(runCheckerOnCode(
+  R"(class C {
+ public:
+   C(int nn): n(nn) {}
+   virtual ~C() {}
+ private:
+   int n;
+ };
+
+ class D: public C {
+ public:
+   D(int nn): C(nn) {}
+   virtual ~D() {}
+ };
+
+ D returnD(int m) {
+   D d(m);
+   return d;
+ }
+
+ void foo() {
+   D d = returnD(1); 
+ })"));
 }
 
 } // namespace
Index: clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
===
--- clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -132,10 +132,11 @@
 case ConstructionContext::SimpleConstructorInitializerKind: {
   const auto *ICC = cast(CC);
   const auto *Init = ICC->getCXXCtorInitializer();
-  assert(Init->isAnyMemberInitializer());
   const CXXMethodDecl *CurCtor = cast(LCtx->getDecl());
   Loc ThisPtr = SVB.getCXXThis(CurCtor, LCtx->getStackFrame());
   SVal ThisVal = State->getSVal(ThisPtr);
+  if (Init->isBaseInitializer() || Init->isDelegatingInitializer())
+return ThisVal;
 
   const ValueDecl *Field;
   SVal FieldVal;
@@ -364,6 +365,8 @@
 case ConstructionContext::CXX17ElidedCopyConstructorInitializerKind:
 case ConstructionContext::SimpleConstructorInitializerKind: {
   const auto *ICC = cast(CC);
+  const auto *Init = ICC->getCXXCtorInitializer();
+  assert(Init->isAnyMemberInitializer());
   return addObjectUnderConstruction(State, ICC->getCXXCtorInitializer(),
 LCtx, V);
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D83717: [clang-tidy] Add check fo SEI CERT item ENV32-C

2020-09-03 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added inline comments.



Comment at: clang-tools-extra/clang-tidy/cert/ExitHandlerCheck.cpp:37
+return false;
+  constexpr StringRef ExitFunctions[] = {"_Exit", "exit", "quick_exit"};
+  return llvm::is_contained(ExitFunctions, FD->getName());

`abort()` as well?

How about in C++ mode calling `std::terminate()`?

One last class of problem functions are ones that are attributed as not 
returning (this would include user-defined functions).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83717

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

[PATCH] D84604: Thread safety analysis: Consider global variables in scope

2020-09-03 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added inline comments.



Comment at: clang/test/SemaCXX/warn-thread-safety-negative.cpp:87
+Mutex globalMutex;
+void f() EXCLUSIVE_LOCKS_REQUIRED(!globalMutex);
+

Can you add a test that uses `!::globalMutex`? I'd like to verify that lookup 
rules are honored for naming the mutex, so more complex examples with name 
hiding would also be useful.



Comment at: clang/test/SemaCXX/warn-thread-safety-negative.cpp:91
+  Mutex namespaceMutex;
+  void f() EXCLUSIVE_LOCKS_REQUIRED(!namespaceMutex);
+}

Can you also add a test that uses `!ns::namespaceMutex`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84604

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

[PATCH] D67935: Add `#pragma clang deprecated`, used to deprecate macros

2020-09-03 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added a comment.

In D67935#2251145 , @erik.pilkington 
wrote:

> @aaron.ballman: Did you happen to get any feedback on macro attributes? There 
> are a growing number of macros that we'd like to be able to deprecate, and 
> having a workable solution would be useful for us.

Thank you for bringing this back up! I've worked on a patch to add preprocessor 
attributes to clang but have set it aside because it feels like it may be an 
awkward fit because of attribute arguments -- for instance, the preprocessor 
has no type system or AST, so we track values for things with `APValue` objects 
and there is no string `APValue` type, so it would be a fair amount of work to 
support `# [[deprecated("don't use baz")]] define BAZ`, let alone the more 
esoteric situations for arbitrary attributes. Another issue is that the 
preprocessor is sometimes shared between C/C++ frontends and, say, a FORTRAN 
frontend, which could suddenly introduce a new feature into a FORTRAN compiler 
without extra work.

Based on all of that, I think we should go with your approach of using a 
`#pragma` as it does solve a problem and isn't quite as novel. However, I am 
wondering about the design a bit -- why are you using a string literal to 
supply the macro name? It's my understanding that all preprocessing directives 
are executed at the same phase of translation (so you don't have to worry about 
`#define` changing the behavior of `#pragma` or `_Pragma`), so I would have 
expected a design more like:

  #define FOOBAR(x) whatever(x)
  #pragma clang deprecated(FOOBAR, "don't use FOOBAR, it will do bad things")

This feels like a more approachable way to expose the feature, to me, but I 
wonder if I'm missing something.

Other feedback is: should we diagnose if the pp-token for the macro identifier 
doesn't actually match the name of a macro? Or are we allowing constructs like:

  #pragma clang deprecated(FOOBAR, "don't use FOOBAR, twelve is a terrible 
number")
  #define FOOBAR 12

Comment at: clang/lib/Lex/Pragma.cpp:1544
+PP.Lex(Tok);
+if (!PP.FinishLexStringLiteral(Tok, EntityString,
+   "#pragma clang deprecated",

Do we want to allow arbitrary string literals, or only narrow character string 
literals? e.g., should we disallow something like: `#pragma clang 
deprecated(L"oops", U"hahahaha")` ?

Comment at: clang/test/Lexer/pragma-deprecated.c:17-20
+#pragma clang deprecated("\"flerp.h\"", "use \"flarp.h\" instead")
+
+// expected-warning@+1{{'flerp' is deprecated}}
+#pragma clang deprecated("flerp")

What is the utility of unconditionally emitting a diagnostic like this? We've 
already got `#warning`.

Repository:
  rC Clang

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

https://reviews.llvm.org/D67935

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

[PATCH] D86369: [Sema][MSVC] warn at dynamic_cast when /GR- is given

2020-09-03 Thread Hans Wennborg via Phabricator via cfe-commits

hans added a comment.

Okay, almost there..




Comment at: clang/lib/Sema/SemaCast.cpp:897
+  DiagnosticOptions::MSVC;
+if (isMSVC || !DestPointee->isVoidType())
+  Self.Diag(OpRange.getBegin(),

I don't think the if-statement is necessary. I thought we concluded we want to 
warn even for void*? Also note that "isMSVC" here is only checking the driver 
mode, i.e. just the user interface to the compiler. The user could still be 
compiling in MSVC mode.



Comment at: clang/lib/Sema/SemaExprCXX.cpp:649
 
+  // Warns when typeid is used with RTTI disabled.
+  if (!getLangOpts().RTTIData)

s/RTTI/RTTI data/
(the "RTTI disabled" case is on line 646)



Comment at: clang/test/SemaCXX/ms_no_dynamic_cast.cpp:19
+  auto d = dynamic_cast(b); // expected-warning{{dynamic_cast will not 
work since RTTI data is disabled by /GR-}}
+  (void)typeid(int);  // expected-warning{{typeid will not work 
since RTTI data is disabled by /GR-}}
+}

Is the cast to void necessary? Same comment for the next file.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86369

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

[PATCH] D85091: [Sema, CodeGen] Implement [[likely]] and [[unlikely]] in IfStmt

2020-09-03 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added a comment.

In D85091#2252632 , @Mordante wrote:

> In D85091#2250657 , @rsmith wrote:
>
>> Looking specifically for attributes in the 'then' and 'else' cases of an 
>> `if` seems like a fine first pass at this, but I think this is the wrong way 
>> to lower these attributes in the longer term: we should have a uniform 
>> treatment of them that looks for the most recent prior branch and annotates 
>> it with weights. We could do that either by generating LLVM intrinsic calls 
>> that an LLVM pass lowers, or by having the frontend look backwards for the 
>> most recent branch and annotate it. I suppose it's instructive to consider a 
>> case such as:
>>
>>   void mainloop() noexcept; // probably terminates by calling `exit`
>>   void f() {
>> mainloop();
>> [[unlikely]];
>> somethingelse();
>>   }
>>
>> ... where the `[[unlikely]];` should probably cause us to split the 
>> `somethingelse()` out into a separate basic block that we mark as cold, but 
>> should not cause `f()` itself or its call to `mainloop()` to be considered 
>> unlikely -- except in the case where `mainloop()` can be proven to always 
>> terminate, in which case the implication is that it's unlikely that `f()` is 
>> invoked. Cases like this probably need the LLVM intrinsic approach to model 
>> them properly.
>
> We indeed considered similar cases and wondered whether it was really 
> intended to work this way. Since this approach seems a bit hard to explain to 
> users, I changed the code back to only allow the attribute on the 
> substatement of the `if` and `else`. For now I first want to implement the 
> simple approach, which I expect will be the most common use case. Once 
> finished we can see what the next steps will be.

+1 to the cautious approach. While I can understand how to implement what 
Richard is suggesting, I'm not convinced it results in readable code or that 
we're missing important optimization cases by using the more restrictive 
approach, so I'd like to get some field experience with users first.


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

https://reviews.llvm.org/D85091

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

[PATCH] D85091: [Sema, CodeGen] Implement [[likely]] and [[unlikely]] in IfStmt

2020-09-03 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added inline comments.



Comment at: clang/lib/Sema/SemaStmt.cpp:578
+static std::pair
+getLikelihood(const Stmt *Stmt) {
+  if (const auto *AS = dyn_cast(Stmt))

Mordante wrote:
> rsmith wrote:
> > Mordante wrote:
> > > aaron.ballman wrote:
> > > > rsmith wrote:
> > > > > Sema doesn't care about any of this; can you move this code to 
> > > > > CodeGen and remove the code that stores likelihood data in the AST?
> > > > FWIW, I asked for it to be moved out of CodeGen and into Sema because 
> > > > the original implementation in CodeGen was doing a fair amount of work 
> > > > trying to interrogate the AST for this information. Now that we've 
> > > > switched the design to only be on the substatement of an if/else 
> > > > statement (rather than an arbitrary statement), it may be that CodeGen 
> > > > is a better place for this again (and if so, I'm sorry for the churn).
> > > At the moment the Sema cares about it to generate diagnostics about 
> > > conflicting annotations:
> > > ```
> > > if(i) [[ likely]] {}
> > > else [[likely]] {}
> > > ```
> > > This diagnostic only happens for an if statement, for a switch multiple 
> > > values can be considered likely.
> > > Do you prefer to have the diagnostic also in the CodeGen?
> > It looked to me like you'd reached agreement to remove that diagnostic. Are 
> > you intending to keep it?
> > 
> > If so, then I'd suggest you make `getLikelihood` a member of `Stmt` so that 
> > `CodeGen` and the warning here can both easily call it.
> @aaron.ballman I thought we wanted to keep this conflict warning, am I 
> correct?
> I'll add an extra function to the Stmt to test for a conflict and use that 
> for the diagnostic in the Sema. This allows me to use `LH_None` for no 
> attribute or a conflict of attributes in the CodeGen. Then there's no need 
> for `LH_Conflict` and it can be removed from the enumerate.
> @aaron.ballman I thought we wanted to keep this conflict warning, am I 
> correct?

I want to keep the property that any time the user writes an explicit 
annotation in their code but we decide to ignore the annotation (for whatever 
reason), the user gets some sort of diagnostic telling them their expectations 
may not be met. Because we're ignoring the attributes in the case where they're 
identical on both branches, I'd like to keep some form of diagnostic.


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

https://reviews.llvm.org/D85091

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

[PATCH] D86999: getClangStripDependencyFileAdjuster(): Do not remove -M args when using MSVC cl driver

2020-09-03 Thread Hans Wennborg via Phabricator via cfe-commits

hans added inline comments.



Comment at: clang/lib/Tooling/ArgumentsAdjusters.cpp:117
+  // do not remove those when using the cl driver.
+  bool IsDependencyFileArg;
+  if (Arg.startswith("/showIncludes") || Arg.startswith("-showIncludes"))

shivanshu3 wrote:
> hans wrote:
> > Instead of a bool and if below, I'd suggest if-statements and early 
> > continues instead. Breaking up the old if-statement is nice though, and 
> > maybe the comment from above about what flags to ignore could be moved down 
> > here to those if statements. For example:
> > 
> > 
> > ```
> > // -M flags blah blah
> > if (Arg.startswith("-M") && !UsingClDriver)
> >   continue;
> > // MSVC flags blah blah
> > if (Arg.startswith("/showIncludes") || Arg.startswith("-showIncludes"))
> >   continue;
> > AdjustedArgs.push_back(Args[i]);
> > ```
> I realized that with my original change, we would skip the next argument 
> under cl driver mode when -MT was used, which would be wrong. The next 
> argument should only be skipped when `IsDependencyFileArg` is true. So I 
> think it might be cleaner to keep that extra boolean so the code is easy to 
> read and understand. What do you think?
I think having the boolean variable is still more confusing (it adds more state 
to keep track of) than just handling the cases with if-statements and early 
continue.

How about:

```
// -M flags that take an arg..
if (!UsingClDriver && (Arg == "-MF" || Arg == "-MT" || Arg == "-MQ")) {
  ++i;
  continue;
}
// -M flags blah blah
if (!UsingClDriver && Arg.startswith("-M"))
  continue;
// MSVC flags blah blah
if (Arg.startswith("/showIncludes") || Arg.startswith("-showIncludes"))
  continue;

AdjustedArgs.push_back(Args[i]);
```

?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86999

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

1 2 >

1 - 100 of 116 matches

Mail list logo