[PATCH] D89055: [analyzer] Wrong type cast occures during pointer dereferencing after type punning

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

@NoQ

> The new code should obviously be restricted into evalCastFromLoc() because if 
> it's a region it's a Loc.

The first I tryed was `evalCastFromLoc()`, but it turned out that `SVal` which 
binds to a pointer can be `NonLoc` as well through violation of pointing levels.
Look here:

  void foo(int** p) { // here is a two-level pointer
 *(int*)p = 42; // pretend as a one-level pointer, dereference it and 
assign a number
 *p; // dereferencing once gives a `nonloc::ConcreteInt`
  }
  
  P.S. I can have miscomprehension of this, due to a lack of experience, but 
this is what I observed.

> Shouldn't we do `castRegion()` unconditionally,

I've also tryed and got 10+ tests unpassed. Didn't dig deeper, just refused 
this idea.

> Can we also use `evalCast()` ?

Do you mean to move `castRegion()` to `evalCast()` instead of `dispatchCast()`. 
I can investigate this.


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

https://reviews.llvm.org/D89055

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

[PATCH] D74130: [clang] fix consteval call in default arguments

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

rsmith added inline comments.



Comment at: clang/lib/Sema/Sema.cpp:1026
 
+  HandleImmediateInvocations(ExprEvalContexts.back());
+

Tyker wrote:
> rsmith wrote:
> > What do we need this for? If I'm understanding the patch correctly, I think 
> > the only way we should propagate immediate invocations upwards should be 
> > from the `ExpressionEvaluationContext` of a lambda default argument into 
> > the `ExpressionEvaluationContext` of the lambda itself, so it should never 
> > reach the top level.
> This is fixing a separate bug that consteval call in to top-level 
> ExprEvalContext are not handled.
> like this:
> ```
> struct S {
>   consteval S(bool b) {if (b) throw;}
> };
> S s2(true);
> ```
> this emits no error.
> because the immediate invocation are added to global scope and the global 
> scope is never poped via PopExpressionEvaluationContext.
In this case, what's supposed to happen is that we create an 
`ExpressionEvaluationContext` for the initializer of `s2`. See 
`Sema::ActOnCXXEnterDeclInitializer` / `Sema::ActOnCXXExitDeclInitializer`. I 
guess that's not working somehow?

Ah, the problem is that we leave the context too early in 
`Parser::ParseDeclarationAfterDeclaratorAndAttributes`. We call 
`InitScope.pop()` before we add the initializer to the declaration, which is 
where we actually finish building the initialization full-expression -- that 
seems wrong in general. (We're also missing an `InitializerScopeRAII` object 
around the call to `ActOnUninitializedDecl` in the case where there is no 
initializer -- we need an `ExpressionEvaluationContext` for the 
default-initialization in that case too.)



Comment at: clang/lib/Sema/SemaExpr.cpp:16232
   SemaRef.Diag(Note.first, Note.second);
+CE->markFailed();
 return;

Changing the dependence flags on an expression isn't safe to do this late: the 
`Error` dependence bit should be propagated to all transitive enclosing AST 
nodes when they're created, which has already happened at this point. If we 
need a way to represent an expression that's supposed to be a constant 
expression but isn't constant, perhaps adding a separate flag to `ConstantExpr` 
would be reasonable.



Comment at: clang/lib/Sema/SemaLambda.cpp:897-901
+ExprEvalContexts.back().ReferenceToConsteval.insert(
+LSI->ReferenceToConsteval.begin(), LSI->ReferenceToConsteval.end());
+ExprEvalContexts.back().ImmediateInvocationCandidates.append(
+LSI->ImmediateInvocationCandidates.begin(),
+LSI->ImmediateInvocationCandidates.end());

Thanks! I think this can be cleaned up a little bit further:

* Move the handling of lambda parameter scopes from 
`HandleImmediateInvocations` to `PopExpressionEvaluationContext`
* Change `HandleImmediateInvocations` to take the `ReferenceToConsteval` and 
`ImmediateInvocationCandidates` data instead of an 
`ExpressionEvaluationContextRecord`
* Maybe consider wrapping `ReferenceToConsteval` and 
`ImmediateInvocationCandidates` in a struct to make it a bit easier to pass 
them around as a set.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74130

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

[PATCH] D89636: [AMDGPU] Extend hip-toolchin-features.hip test

[Tony Tye via Phabricator via cfe-commits]

t-tye created this revision.
t-tye added reviewers: kzhuravl, scott.linder, rampitec.
Herald added subscribers: cfe-commits, tpr, dstuttard, yaxunl.
Herald added a project: clang.
t-tye requested review of this revision.
Herald added a subscriber: wdng.

- Extend hip-toolchin-features.hip to also check the lld attributes are passed 
correctly.

- Add check for cumode attributes.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D89636

Files:
  clang/test/Driver/hip-toolchain-features.hip


Index: clang/test/Driver/hip-toolchain-features.hip
===
--- clang/test/Driver/hip-toolchain-features.hip
+++ clang/test/Driver/hip-toolchain-features.hip
@@ -11,6 +11,8 @@
 
 // XNACK: {{.*}}clang{{.*}}"-target-feature" "+xnack"
 // NOXNACK: {{.*}}clang{{.*}}"-target-feature" "-xnack"
+// XNACK: {{.*}}lld{{.*}} "-plugin-opt=-mattr=+xnack"
+// NOXNACK: {{.*}}lld{{.*}} "-plugin-opt=-mattr=-xnack"
 
 // RUN: %clang -### -target x86_64-linux-gnu -fgpu-rdc -nogpulib \
 // RUN:   --cuda-gpu-arch=gfx908:sram-ecc+ %s \
@@ -21,6 +23,20 @@
 
 // SRAM: {{.*}}clang{{.*}}"-target-feature" "+sram-ecc"
 // NOSRAM: {{.*}}clang{{.*}}"-target-feature" "-sram-ecc"
+// SRAM: {{.*}}lld{{.*}} "-plugin-opt=-mattr=+sram-ecc"
+// NOTSRAM: {{.*}}lld{{.*}} "-plugin-opt=-mattr=-sram-ecc"
+
+// RUN: %clang -### -target x86_64-linux-gnu -fgpu-rdc -nogpulib \
+// RUN:   --cuda-gpu-arch=gfx1010 %s \
+// RUN:   -mcumode  2>&1 | FileCheck %s -check-prefix=CUMODE
+// RUN: %clang -### -target x86_64-linux-gnu -fgpu-rdc -nogpulib \
+// RUN:   --cuda-gpu-arch=gfx1010 %s \
+// RUN:   -mno-cumode  2>&1 | FileCheck %s -check-prefix=NOTCUMODE
+
+// CUMODE: {{.*}}clang{{.*}}"-target-feature" "+cumode"
+// NOTCUMODE: {{.*}}clang{{.*}}"-target-feature" "-cumode"
+// CUMODE: {{.*}}lld{{.*}} "-plugin-opt=-mattr=+cumode"
+// NOTCUMODE: {{.*}}lld{{.*}} "-plugin-opt=-mattr=-cumode"
 
 // RUN: %clang -### -target x86_64-linux-gnu -fgpu-rdc -nogpulib \
 // RUN:   --cuda-gpu-arch=gfx908:xnack+:sram-ecc+ %s \


Index: clang/test/Driver/hip-toolchain-features.hip
===
--- clang/test/Driver/hip-toolchain-features.hip
+++ clang/test/Driver/hip-toolchain-features.hip
@@ -11,6 +11,8 @@
 
 // XNACK: {{.*}}clang{{.*}}"-target-feature" "+xnack"
 // NOXNACK: {{.*}}clang{{.*}}"-target-feature" "-xnack"
+// XNACK: {{.*}}lld{{.*}} "-plugin-opt=-mattr=+xnack"
+// NOXNACK: {{.*}}lld{{.*}} "-plugin-opt=-mattr=-xnack"
 
 // RUN: %clang -### -target x86_64-linux-gnu -fgpu-rdc -nogpulib \
 // RUN:   --cuda-gpu-arch=gfx908:sram-ecc+ %s \
@@ -21,6 +23,20 @@
 
 // SRAM: {{.*}}clang{{.*}}"-target-feature" "+sram-ecc"
 // NOSRAM: {{.*}}clang{{.*}}"-target-feature" "-sram-ecc"
+// SRAM: {{.*}}lld{{.*}} "-plugin-opt=-mattr=+sram-ecc"
+// NOTSRAM: {{.*}}lld{{.*}} "-plugin-opt=-mattr=-sram-ecc"
+
+// RUN: %clang -### -target x86_64-linux-gnu -fgpu-rdc -nogpulib \
+// RUN:   --cuda-gpu-arch=gfx1010 %s \
+// RUN:   -mcumode  2>&1 | FileCheck %s -check-prefix=CUMODE
+// RUN: %clang -### -target x86_64-linux-gnu -fgpu-rdc -nogpulib \
+// RUN:   --cuda-gpu-arch=gfx1010 %s \
+// RUN:   -mno-cumode  2>&1 | FileCheck %s -check-prefix=NOTCUMODE
+
+// CUMODE: {{.*}}clang{{.*}}"-target-feature" "+cumode"
+// NOTCUMODE: {{.*}}clang{{.*}}"-target-feature" "-cumode"
+// CUMODE: {{.*}}lld{{.*}} "-plugin-opt=-mattr=+cumode"
+// NOTCUMODE: {{.*}}lld{{.*}} "-plugin-opt=-mattr=-cumode"
 
 // RUN: %clang -### -target x86_64-linux-gnu -fgpu-rdc -nogpulib \
 // RUN:   --cuda-gpu-arch=gfx908:xnack+:sram-ecc+ %s \
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D89087: [MemProf] Pass down memory profile name with optional path from clang

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson updated this revision to Diff 298849.
tejohnson added a comment.
Herald added a project: Sanitizers.
Herald added a subscriber: Sanitizers.

As discussed in review for D89086 , move test 
changes to this patch since they are only required here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89087

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/CodeGenOptions.h
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Driver/SanitizerArgs.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/memory-profile-filename.c
  clang/test/Driver/fmemprof.cpp
  compiler-rt/test/memprof/TestCases/atexit_stats.cpp
  compiler-rt/test/memprof/TestCases/dump_process_map.cpp
  compiler-rt/test/memprof/TestCases/log_path_test.cpp
  compiler-rt/test/memprof/TestCases/malloc-size-too-big.cpp
  compiler-rt/test/memprof/TestCases/mem_info_cache_entries.cpp
  compiler-rt/test/memprof/TestCases/print_miss_rate.cpp
  compiler-rt/test/memprof/TestCases/stress_dtls.c
  compiler-rt/test/memprof/TestCases/test_malloc_load_store.c
  compiler-rt/test/memprof/TestCases/test_memintrin.cpp
  compiler-rt/test/memprof/TestCases/test_new_load_store.cpp
  compiler-rt/test/memprof/TestCases/test_terse.cpp
  compiler-rt/test/memprof/TestCases/unaligned_loads_and_stores.cpp
  llvm/lib/Transforms/Instrumentation/MemProfiler.cpp
  llvm/test/Instrumentation/HeapProfiler/filename.ll

Index: llvm/test/Instrumentation/HeapProfiler/filename.ll
===
--- /dev/null
+++ llvm/test/Instrumentation/HeapProfiler/filename.ll
@@ -0,0 +1,15 @@
+; Test to ensure that the filename provided by clang in the module flags
+; metadata results in the expected __memprof_profile_filename insertion.
+
+; RUN: opt < %s -mtriple=x86_64-unknown-linux -memprof -memprof-module -S | FileCheck --check-prefixes=CHECK %s
+
+define i32 @main() {
+entry:
+  ret i32 0
+}
+
+!llvm.module.flags = !{!0}
+!0 = !{i32 1, !"MemProfProfileFilename", !"/tmp/memprof.profraw"}
+
+; CHECK: $__memprof_profile_filename = comdat any
+; CHECK: @__memprof_profile_filename = constant [21 x i8] c"/tmp/memprof.profraw\00", comdat
Index: llvm/lib/Transforms/Instrumentation/MemProfiler.cpp
===
--- llvm/lib/Transforms/Instrumentation/MemProfiler.cpp
+++ llvm/lib/Transforms/Instrumentation/MemProfiler.cpp
@@ -60,6 +60,8 @@
 constexpr char MemProfShadowMemoryDynamicAddress[] =
 "__memprof_shadow_memory_dynamic_address";
 
+constexpr char MemProfFilenameVar[] = "__memprof_profile_filename";
+
 // Command-line flags.
 
 static cl::opt ClInsertVersionCheck(
@@ -486,6 +488,26 @@
   IRB.CreateStore(ShadowValue, ShadowAddr);
 }
 
+// Create the variable for the profile file name.
+void createProfileFileNameVar(Module ) {
+  const MDString *MemProfFilename =
+  dyn_cast_or_null(M.getModuleFlag("MemProfProfileFilename"));
+  if (!MemProfFilename)
+return;
+  assert(!MemProfFilename->getString().empty() &&
+ "Unexpected MemProfProfileFilename metadata with empty string");
+  Constant *ProfileNameConst = ConstantDataArray::getString(
+  M.getContext(), MemProfFilename->getString(), true);
+  GlobalVariable *ProfileNameVar = new GlobalVariable(
+  M, ProfileNameConst->getType(), /*isConstant=*/true,
+  GlobalValue::WeakAnyLinkage, ProfileNameConst, MemProfFilenameVar);
+  Triple TT(M.getTargetTriple());
+  if (TT.supportsCOMDAT()) {
+ProfileNameVar->setLinkage(GlobalValue::ExternalLinkage);
+ProfileNameVar->setComdat(M.getOrInsertComdat(MemProfFilenameVar));
+  }
+}
+
 bool ModuleMemProfiler::instrumentModule(Module ) {
   // Create a module constructor.
   std::string MemProfVersion = std::to_string(LLVM_MEM_PROFILER_VERSION);
@@ -500,6 +522,8 @@
   const uint64_t Priority = getCtorAndDtorPriority(TargetTriple);
   appendToGlobalCtors(M, MemProfCtorFunction, Priority);
 
+  createProfileFileNameVar(M);
+
   return true;
 }
 
Index: compiler-rt/test/memprof/TestCases/unaligned_loads_and_stores.cpp
===
--- compiler-rt/test/memprof/TestCases/unaligned_loads_and_stores.cpp
+++ compiler-rt/test/memprof/TestCases/unaligned_loads_and_stores.cpp
@@ -1,4 +1,4 @@
-// RUN: %clangxx_memprof -O0 %s -o %t && %run %t 2>&1 | FileCheck %s
+// RUN: %clangxx_memprof -O0 %s -o %t && %env_memprof_opts=log_path=stderr %run %t 2>&1 | FileCheck %s
 
 // This is actually:
 //  Memory allocation stack id = STACKID
Index: compiler-rt/test/memprof/TestCases/test_terse.cpp
===
--- compiler-rt/test/memprof/TestCases/test_terse.cpp
+++ 

Re: [Lldb-commits] Upcoming upgrade of LLVM buildbot

[Galina Kistanova via cfe-commits]

Thanks everyone for keeping your annotated builders in the staging area!
Much appreciated.

Please feel free to move all the green builders back to the production. It
has a new AnnotatedCommand now.

Thanks

Galina


On Thu, Oct 15, 2020 at 12:46 AM Vitaly Buka  wrote:

> Ok, I can switch them back to staging. However today's staging was
> frequently offline.
>
>
> On Wed, 14 Oct 2020 at 21:44, Galina Kistanova 
> wrote:
>
>> Thanks everyone!
>>
>> I would like to keep the bots in the staging till Friday. And if
>> everything is good, then apply the changes to the production and let you
>> move all the green bots back there.
>>
>> Thanks
>>
>> Galina
>>
>>
>> On Tue, Oct 13, 2020 at 10:43 PM Vitaly Buka 
>> wrote:
>>
>>> They do on staging.
>>> http://lab.llvm.org:8014/#/builders/sanitizer-windows
>>> http://lab.llvm.org:8014/#/builders/clang-x64-windows-msvc
>>>
>>> Galina, when do you plan to push this to the primary server?
>>> If it's a few days, I'd rather keep bots on staging to have colors.
>>>
>>>
>>>
>>> On Tue, 13 Oct 2020 at 11:11, Reid Kleckner  wrote:
>>>
 FWIW, I don't see any issues with my two bots that use buildbot
 annotated commands:
 http://lab.llvm.org:8011/#/builders/sanitizer-windows
 http://lab.llvm.org:8011/#/builders/clang-x64-windows-msvc
 The individual steps don't highlight as green or red, but that's OK for
 now.

 On Mon, Oct 12, 2020 at 7:19 PM Galina Kistanova 
 wrote:

> We have a better version of AnnotatedCommand on the staging. It should
> be a functional equivalent of the old one.
> We need to stress test it well before moving to the production build
> bot.
>
> For that we need all sanitizer + other bots which use the
> AnnotatedCommand directly or indirectly moved temporarily to the staging.
>
> Please let me know when that could be arranged.
>
> Thanks
>
> Galina
>
> On Mon, Oct 12, 2020 at 11:39 AM Reid Kleckner  wrote:
>
>> On Wed, Oct 7, 2020 at 4:32 PM Galina Kistanova via lldb-commits <
>> lldb-comm...@lists.llvm.org> wrote:
>>
>>> They are online now -
>>> http://lab.llvm.org:8011/#/waterfall?tags=sanitizer
>>>
>>> AnnotatedCommand has severe design conflict with the new buildbot.
>>> We have changed it to be safe and still do something useful, but it
>>> will need more love and care.
>>>
>>> Please let me know if you have some spare time to work on porting
>>> AnnotatedCommand.
>>>
>>
>> That's unfortunate, it would've been good to know that earlier. I and
>> another team member have spent a fair amount of time porting things to 
>> use
>> more AnnotatedCommand steps, because it gives us the flexibility to test
>> steps locally and make changes to the steps without restarting the 
>> buildbot
>> master. IMO that is the Right Way to define steps: a script that you can
>> run locally on a machine that satisfies the OS and dep requirements of 
>> the
>> script.
>>
>> I am restarting the two bots that I am responsible for, and may need
>> some help debugging further issues soon. I'll let you know.
>>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D81678: Introduce noundef attribute at call sites for stricter poison analysis

[Evgenii Stepanov via Phabricator via cfe-commits]

eugenis updated this revision to Diff 298692.
eugenis added a comment.

fix type size check for vscale types


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81678

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/attr-noundef.cpp
  clang/test/CodeGen/indirect-noundef.cpp
  clang/test/lit.cfg.py
  llvm/utils/update_cc_test_checks.py

Index: llvm/utils/update_cc_test_checks.py
===
--- llvm/utils/update_cc_test_checks.py
+++ llvm/utils/update_cc_test_checks.py
@@ -28,9 +28,9 @@
 from UpdateTestChecks import common
 
 SUBST = {
-'%clang': [],
-'%clang_cc1': ['-cc1'],
-'%clangxx': ['--driver-mode=g++'],
+'%clang': ['-Xclang -disable-noundef-analysis'],
+'%clang_cc1': ['-cc1', '-disable-noundef-analysis'],
+'%clangxx': ['--driver-mode=g++', '-disable-noundef-analysis'],
 }
 
 def get_line2spell_and_mangled(args, clang_args):
@@ -161,8 +161,8 @@
   try:
 builtin_include_dir = subprocess.check_output(
   [args.clang, '-print-file-name=include']).decode().strip()
-SUBST['%clang_cc1'] = ['-cc1', '-internal-isystem', builtin_include_dir,
-   '-nostdsysteminc']
+SUBST['%clang_cc1'] += ['-internal-isystem', builtin_include_dir,
+'-nostdsysteminc']
   except subprocess.CalledProcessError:
 common.warn('Could not determine clang builtins directory, some tests '
 'might not update correctly.')
Index: clang/test/lit.cfg.py
===
--- clang/test/lit.cfg.py
+++ clang/test/lit.cfg.py
@@ -41,7 +41,8 @@
 
 llvm_config.use_default_substitutions()
 
-llvm_config.use_clang()
+llvm_config.use_clang(cc_additional_flags=['-Xclang -disable-noundef-analysis'],
+  cc1_additional_flags=['-disable-noundef-analysis'])
 
 config.substitutions.append(
 ('%src_include_dir', config.clang_src_dir + '/include'))
Index: clang/test/CodeGen/indirect-noundef.cpp
===
--- /dev/null
+++ clang/test/CodeGen/indirect-noundef.cpp
@@ -0,0 +1,34 @@
+// RUN: %clang_bin -cc1 -x c++ -triple x86_64-unknown-unknown -O0 -emit-llvm -o - %s | FileCheck %s
+
+union u1 {
+  int val;
+};
+
+// CHECK: @indirect_callee_int_ptr = global i32 (i32)*
+int (*indirect_callee_int_ptr)(int);
+// CHECK: @indirect_callee_union_ptr = global i32 (i32)*
+union u1 (*indirect_callee_union_ptr)(union u1);
+
+// CHECK-LABEL: define noundef i32 @{{.*}}indirect_callee_int{{.*}}(i32 noundef %
+int indirect_callee_int(int a) { return a; }
+// CHECK-LABEL: define i32 @{{.*}}indirect_callee_union{{.*}}(i32 %
+union u1 indirect_callee_union(union u1 a) {
+  return a;
+}
+
+int main() {
+  // CHECK: call noundef i32 @{{.*}}indirect_callee_int{{.*}}(i32 noundef 0)
+  indirect_callee_int(0);
+  // CHECK: call i32 @{{.*}}indirect_callee_union{{.*}}(i32 %
+  indirect_callee_union((union u1){0});
+
+  indirect_callee_int_ptr = indirect_callee_int;
+  indirect_callee_union_ptr = indirect_callee_union;
+
+  // CHECK: call noundef i32 %{{.*}}(i32 noundef 0)
+  indirect_callee_int_ptr(0);
+  // CHECK: call i32 %{{.*}}(i32 %
+  indirect_callee_union_ptr((union u1){});
+
+  return 0;
+}
Index: clang/test/CodeGen/attr-noundef.cpp
===
--- /dev/null
+++ clang/test/CodeGen/attr-noundef.cpp
@@ -0,0 +1,163 @@
+// RUN: %clang_bin -cc1 -triple x86_64-gnu-linux -x c++ -S -emit-llvm %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-INTEL
+// RUN: %clang_bin -cc1 -triple x86_64-gnu-linux -x c++ -S -emit-llvm %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-INTEL
+// RUN: %clang_bin -cc1 -triple aarch64-gnu-linux -x c++ -S -emit-llvm %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-AARCH
+
+// Passing structs by value
+// TODO: No structs may currently be marked noundef
+
+namespace check_structs {
+struct Trivial {
+  int a;
+};
+Trivial ret_trivial() { return {}; }
+void pass_trivial(Trivial e) {}
+// CHECK-INTEL: define i32 @{{.*}}ret_trivial
+// CHECK-AARCH: define i64 @{{.*}}ret_trivial
+// CHECK-INTEL: define void @{{.*}}pass_trivial{{.*}}(i32 %
+// CHECK-AARCH: define void @{{.*}}pass_trivial{{.*}}(i64 %
+
+struct NoCopy {
+  int a;
+  NoCopy(NoCopy &) = delete;
+};
+NoCopy ret_nocopy() { return {}; }
+void pass_nocopy(NoCopy e) {}
+// CHECK: define {{(dso_local)?}}void @{{.*}}ret_nocopy{{.*}}(%"struct.check_structs::NoCopy"* noalias sret align 4 %
+// CHECK: define void @{{.*}}pass_nocopy{{.*}}(%"struct.check_structs::NoCopy"* noundef %
+
+struct Huge {
+  int a[1024];
+};

[PATCH] D89464: Revert "[clang-format] Fix AlignConsecutive on PP blocks"

[Sylvestre Ledru via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGb9e789447f14: Revert [clang-format] Fix 
AlignConsecutive on PP blocks (authored by sylvestre.ledru).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89464

Files:
  clang/lib/Format/FormatToken.h
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/lib/Format/WhitespaceManager.cpp
  clang/unittests/Format/FormatTest.cpp
  clang/unittests/Format/FormatTestComments.cpp

Index: clang/unittests/Format/FormatTestComments.cpp
===
--- clang/unittests/Format/FormatTestComments.cpp
+++ clang/unittests/Format/FormatTestComments.cpp
@@ -2783,7 +2783,7 @@
 
   // Checks an edge case in preprocessor handling.
   // These comments should *not* be aligned
-  EXPECT_EQ(
+  EXPECT_NE( // change for EQ when fixed
   "#if FOO\n"
   "#else\n"
   "long a; // Line about a\n"
@@ -2801,6 +2801,24 @@
  "long b_long_name; // Line about b\n"
  "#endif\n",
  getLLVMStyleWithColumns(80)));
+
+  // bug 47589
+  EXPECT_EQ(
+  "namespace m {\n\n"
+  "#define FOO_GLOBAL 0  // Global scope.\n"
+  "#define FOO_LINKLOCAL 1   // Link-local scope.\n"
+  "#define FOO_SITELOCAL 2   // Site-local scope (deprecated).\n"
+  "#define FOO_UNIQUELOCAL 3 // Unique local\n"
+  "#define FOO_NODELOCAL 4   // Loopback\n\n"
+  "} // namespace m\n",
+  format("namespace m {\n\n"
+ "#define FOO_GLOBAL 0   // Global scope.\n"
+ "#define FOO_LINKLOCAL 1  // Link-local scope.\n"
+ "#define FOO_SITELOCAL 2  // Site-local scope (deprecated).\n"
+ "#define FOO_UNIQUELOCAL 3 // Unique local\n"
+ "#define FOO_NODELOCAL 4  // Loopback\n\n"
+ "} // namespace m\n",
+ getLLVMStyleWithColumns(80)));
 }
 
 TEST_F(FormatTestComments, AlignsBlockCommentDecorations) {
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -12254,26 +12254,28 @@
Alignment);
 
   // Bug 25167
-  verifyFormat("#if A\n"
-   "#else\n"
-   "int  = 12;\n"
-   "#endif\n"
-   "#if B\n"
-   "#else\n"
-   "int a = 12;\n"
-   "#endif\n",
-   Alignment);
-  verifyFormat("enum foo {\n"
-   "#if A\n"
-   "#else\n"
-   "   = 12;\n"
-   "#endif\n"
-   "#if B\n"
-   "#else\n"
-   "  a = 12;\n"
-   "#endif\n"
-   "};\n",
-   Alignment);
+  /* Uncomment when fixed
+verifyFormat("#if A\n"
+ "#else\n"
+ "int  = 12;\n"
+ "#endif\n"
+ "#if B\n"
+ "#else\n"
+ "int a = 12;\n"
+ "#endif\n",
+ Alignment);
+verifyFormat("enum foo {\n"
+ "#if A\n"
+ "#else\n"
+ "   = 12;\n"
+ "#endif\n"
+ "#if B\n"
+ "#else\n"
+ "  a = 12;\n"
+ "#endif\n"
+ "};\n",
+ Alignment);
+  */
 
   EXPECT_EQ("int a = 5;\n"
 "\n"
Index: clang/lib/Format/WhitespaceManager.cpp
===
--- clang/lib/Format/WhitespaceManager.cpp
+++ clang/lib/Format/WhitespaceManager.cpp
@@ -411,11 +411,9 @@
 if (Changes[i].NewlinesBefore != 0) {
   CommasBeforeMatch = 0;
   EndOfSequence = i;
-  // If there is a blank line, there is a forced-align-break (eg,
-  // preprocessor), or if the last line didn't contain any matching token,
-  // the sequence ends here.
-  if (Changes[i].NewlinesBefore > 1 ||
-  Changes[i].Tok->MustBreakAlignBefore || !FoundMatchOnLine)
+  // If there is a blank line, or if the last line didn't contain any
+  // matching token, the sequence ends here.
+  if (Changes[i].NewlinesBefore > 1 || !FoundMatchOnLine)
 AlignCurrentSequence();
 
   FoundMatchOnLine = false;
@@ -726,8 +724,6 @@
 if (Changes[i].StartOfBlockComment)
   continue;
 Newlines += Changes[i].NewlinesBefore;
-if (Changes[i].Tok->MustBreakAlignBefore)
-  BreakBeforeNext = true;
 if (!Changes[i].IsTrailingComment)
   continue;
 
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -3046,7 +3046,6 @@
   }
   FormatTok = Tokens->getNextToken();
   

[clang] b9e7894 - Revert "[clang-format] Fix AlignConsecutive on PP blocks"

[Sylvestre Ledru via cfe-commits]

Author: Sylvestre Ledru
Date: 2020-10-17T19:52:51+02:00
New Revision: b9e789447f14c0330edd22c82746af29e7c3b259

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

LOG: Revert "[clang-format] Fix AlignConsecutive on PP blocks"

This reverts commit b2eb439317576ce718193763c12bff9fccdfc166.

Caused the regression:
https://bugs.llvm.org/show_bug.cgi?id=47589

Reviewed By: MyDeveloperDay

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

Added: 


Modified: 
clang/lib/Format/FormatToken.h
clang/lib/Format/UnwrappedLineParser.cpp
clang/lib/Format/WhitespaceManager.cpp
clang/unittests/Format/FormatTest.cpp
clang/unittests/Format/FormatTestComments.cpp

Removed: 




diff  --git a/clang/lib/Format/FormatToken.h b/clang/lib/Format/FormatToken.h
index 9cc65bb11b54..a3cb81f1b1ef 100644
--- a/clang/lib/Format/FormatToken.h
+++ b/clang/lib/Format/FormatToken.h
@@ -206,12 +206,11 @@ class AnnotatedLine;
 struct FormatToken {
   FormatToken()
   : HasUnescapedNewline(false), IsMultiline(false), IsFirst(false),
-MustBreakBefore(false), MustBreakAlignBefore(false),
-IsUnterminatedLiteral(false), CanBreakBefore(false),
-ClosesTemplateDeclaration(false), StartsBinaryExpression(false),
-EndsBinaryExpression(false), PartOfMultiVariableDeclStmt(false),
-ContinuesLineCommentSection(false), Finalized(false),
-BlockKind(BK_Unknown), Decision(FD_Unformatted),
+MustBreakBefore(false), IsUnterminatedLiteral(false),
+CanBreakBefore(false), ClosesTemplateDeclaration(false),
+StartsBinaryExpression(false), EndsBinaryExpression(false),
+PartOfMultiVariableDeclStmt(false), ContinuesLineCommentSection(false),
+Finalized(false), BlockKind(BK_Unknown), Decision(FD_Unformatted),
 PackingKind(PPK_Inconclusive), Type(TT_Unknown) {}
 
   /// The \c Token.
@@ -248,12 +247,6 @@ struct FormatToken {
   /// before the token.
   unsigned MustBreakBefore : 1;
 
-  /// Whether to not align across this token
-  ///
-  /// This happens for example when a preprocessor directive ended directly
-  /// before the token, but very rarely otherwise.
-  unsigned MustBreakAlignBefore : 1;
-
   /// Set to \c true if this token is an unterminated literal.
   unsigned IsUnterminatedLiteral : 1;
 

diff  --git a/clang/lib/Format/UnwrappedLineParser.cpp 
b/clang/lib/Format/UnwrappedLineParser.cpp
index ab1f647481d0..044f034013dc 100644
--- a/clang/lib/Format/UnwrappedLineParser.cpp
+++ b/clang/lib/Format/UnwrappedLineParser.cpp
@@ -3046,7 +3046,6 @@ void UnwrappedLineParser::readToken(int LevelDifference) {
   }
   FormatTok = Tokens->getNextToken();
   FormatTok->MustBreakBefore = true;
-  FormatTok->MustBreakAlignBefore = true;
 }
 
 if (!PPStack.empty() && (PPStack.back().Kind == PP_Unreachable) &&
@@ -3071,7 +3070,6 @@ void UnwrappedLineParser::pushToken(FormatToken *Tok) {
   Line->Tokens.push_back(UnwrappedLineNode(Tok));
   if (MustBreakBeforeNextToken) {
 Line->Tokens.back().Tok->MustBreakBefore = true;
-Line->Tokens.back().Tok->MustBreakAlignBefore = true;
 MustBreakBeforeNextToken = false;
   }
 }

diff  --git a/clang/lib/Format/WhitespaceManager.cpp 
b/clang/lib/Format/WhitespaceManager.cpp
index 2d479817118d..9e47b49da3d0 100644
--- a/clang/lib/Format/WhitespaceManager.cpp
+++ b/clang/lib/Format/WhitespaceManager.cpp
@@ -411,11 +411,9 @@ static unsigned AlignTokens(const FormatStyle , F 
&,
 if (Changes[i].NewlinesBefore != 0) {
   CommasBeforeMatch = 0;
   EndOfSequence = i;
-  // If there is a blank line, there is a forced-align-break (eg,
-  // preprocessor), or if the last line didn't contain any matching token,
-  // the sequence ends here.
-  if (Changes[i].NewlinesBefore > 1 ||
-  Changes[i].Tok->MustBreakAlignBefore || !FoundMatchOnLine)
+  // If there is a blank line, or if the last line didn't contain any
+  // matching token, the sequence ends here.
+  if (Changes[i].NewlinesBefore > 1 || !FoundMatchOnLine)
 AlignCurrentSequence();
 
   FoundMatchOnLine = false;
@@ -726,8 +724,6 @@ void WhitespaceManager::alignTrailingComments() {
 if (Changes[i].StartOfBlockComment)
   continue;
 Newlines += Changes[i].NewlinesBefore;
-if (Changes[i].Tok->MustBreakAlignBefore)
-  BreakBeforeNext = true;
 if (!Changes[i].IsTrailingComment)
   continue;
 

diff  --git a/clang/unittests/Format/FormatTest.cpp 
b/clang/unittests/Format/FormatTest.cpp
index 27e76d40d125..7686e252f7c2 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -12254,26 +12254,28 @@ TEST_F(FormatTest, AlignConsecutiveAssignments) {
Alignment);
 
   // Bug 25167
- 

[PATCH] D88381: [Flang][Driver] Add PrintPreprocessed FrontendAction

[sameeran joshi via Phabricator via cfe-commits]

sameeranjoshi added a comment.

Thank you, this patch looks easy to understand as it's clearly separated 
from(`D87989`) the infrastructure changes needed for frontend actions.
A few comments inline.




Comment at: flang/include/flang/Frontend/CompilerInstance.h:85
+  /// Return parsing to be used by Actions.
+  Fortran::parser::Parsing () const { return *parsing_; }
+

If I am correct this seems to be an accessor member function and it should 
follow point 3 from flang style guide  mentioned at 
https://github.com/llvm/llvm-project/blob/master/flang/docs/C++style.md#naming

I am not aware if driver related patches follow llvm-project style.



Comment at: flang/include/flang/Frontend/CompilerInvocation.h:12
 #include "flang/Frontend/FrontendOptions.h"
+#include "flang/Parser/parsing.h"
 #include "clang/Basic/Diagnostic.h"

`Nit:` I believe `clang-format` is missing.



Comment at: flang/lib/Frontend/CMakeLists.txt:17
   FortranParser
+  FortranSemantics
   clangBasic

I believe this patch is a parsing and preprocessing related patch, is this used 
somewhere or should this be removed for now?



Comment at: flang/lib/Frontend/CompilerInstance.cpp:118
+  // Fortran::parser::option
+  // Set defaults for Parser, as it use as flang options
+  // Default consistent with the temporary driver in f18/f18.cpp

May be you meant `as it is used`?



Comment at: flang/lib/Frontend/FrontendAction.cpp:8
 
//===--===//
-
 #include "flang/Frontend/FrontendAction.h"

undo



Comment at: flang/lib/Frontend/FrontendAction.cpp:57
+  // Read files, scan and run preprocessor
+  // Needed by all next fases of the frontend
+  GetCompilerInstance().GetParsing().Prescan(path, options);

`Nit:` Converts to `phases` in english according to my browser. :)



Comment at: flang/lib/Frontend/FrontendActions.cpp:62
+std::unique_ptr os;
+os = ci.CreateDefaultOutputFile(
+/*Binary=*/true, /*InFile=*/GetCurrentFileOrBufferName());

https://github.com/llvm/llvm-project/blob/master/flang/docs/C++style.md#c-language
point 4 
Can you use `braced initializers` ?

A more better version I think would be to simplify this to 
```
if (auto os{ci.CreateDefaultOutputFile(/*Binary=*/true, 
/*InFile=*/GetCurrentFileOrBufferName())}){
(*os) << out.str();
} else {
  llvm::errs() << "Unable to create the output file\n";
  return;
}
```



Comment at: flang/test/Frontend/Inputs/hello-world.c:1
+a #include int main() {
+  // printf() displays the string inside quotation

a - ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88381

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

[PATCH] D87565: [Sema] Improve const_cast conformance to N4261

[Mark de Wever via Phabricator via cfe-commits]

Mordante updated this revision to Diff 298838.
Mordante marked an inline comment as done.
Mordante added a comment.

Removes the support for the pointer types.

I adjusted the unit test for `void f()`, but I think it would be better remove 
this test now pointers are no longer allowed. Do you agree?


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

https://reviews.llvm.org/D87565

Files:
  clang/lib/Sema/SemaCast.cpp
  clang/test/SemaCXX/const-cast.cpp


Index: clang/test/SemaCXX/const-cast.cpp
===
--- clang/test/SemaCXX/const-cast.cpp
+++ clang/test/SemaCXX/const-cast.cpp
@@ -1,6 +1,9 @@
 // RUN: %clang_cc1 -fsyntax-only -verify %s
 // RUN: %clang_cc1 -fsyntax-only -verify -std=c++98 %s
 // RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++14 %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++17 %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++20 %s
 
 struct A {};
 
@@ -80,3 +83,35 @@
 
 template 
 char *PR21845() { return const_cast((void)T::x); } // expected-error 
{{const_cast from 'void' to 'char *' is not allowed}}
+
+#if __cplusplus >= 201103L
+namespace N4261 {
+typedef int *A[3];
+typedef const int *const CA[3];
+
+CA & = A{};
+A & = const_cast(CA{}); // expected-error {{const_cast to 'N4261::A' 
(aka 'int *[3]'), which is not a reference, pointer-to-object, or 
pointer-to-data-member}}
+
+A & = const_cast(CA{});
+
+void f() {
+  typedef void (*F)();
+  F & = F{};
+  (void)const_cast(F{}); // expected-error {{const_cast to 'F' (aka 'void 
(*)()'), which is not a reference, pointer-to-object, or 
pointer-to-data-member}}
+  (void)const_cast(F{}); // expected-error {{const_cast from rvalue to 
reference type 'F &&' (aka 'void (*&&)()')}}
+
+  class C;
+  typedef void (C::*CF)();
+  CF & = CF{};
+  (void)const_cast(CF{}); // expected-error {{const_cast to 'CF' (aka 
'void (C::*)()'), which is not a reference, pointer-to-object, or 
pointer-to-data-member}}
+  (void)const_cast(CF{}); // expected-error {{const_cast from rvalue to 
reference type 'CF &&' (aka 'void (C::*&&)()')}}
+
+  typedef int C::*CM;
+  CM & = CM{};
+  (void)const_cast(CM{});
+  (void)const_cast(CM{}); // expected-error {{const_cast from rvalue to 
reference type 'CM &&' (aka 'int C::*&&')}}
+}
+
+} // namespace N4261
+
+#endif
Index: clang/lib/Sema/SemaCast.cpp
===
--- clang/lib/Sema/SemaCast.cpp
+++ clang/lib/Sema/SemaCast.cpp
@@ -1780,7 +1780,7 @@
   QualType SrcType = SrcExpr.get()->getType();
   bool NeedToMaterializeTemporary = false;
 
-  if (const ReferenceType *DestTypeTmp =DestType->getAs()) {
+  if (const ReferenceType *DestTypeTmp = DestType->getAs()) {
 // C++11 5.2.11p4:
 //   if a pointer to T1 can be explicitly converted to the type "pointer to
 //   T2" using a const_cast, then the following conversions can also be
@@ -1801,16 +1801,28 @@
 }
 
 if (isa(DestTypeTmp) && SrcExpr.get()->isRValue()) {
-  if (!SrcType->isRecordType()) {
+  // C++17 [expr.const.cast]p3
+  // For two similar types T1 and T2, a prvalue of type T1 may be 
explicitly
+  // converted to the type T2 using a const_cast if, considering the
+  // cv-decompositions of both types, each P1i is the same as P2i for all 
i.
+  // The result of a const_cast refers to the original entity.
+  //
+  // The test for the similarity is done later in this function. Here it
+  // only avoids issuing an diagnostic for possible similar types.
+  //
+  // [expr.const.cast]p4
+  // The result of a reference const_cast refers to the original object if
+  // the operand is a glvalue and to the result of applying the temporary
+  // materialization conversion otherwise.
+  NeedToMaterializeTemporary =
+  SrcType->isRecordType() || SrcType->isArrayType();
+
+  if (!NeedToMaterializeTemporary) {
 // Cannot const_cast non-class prvalue to rvalue reference type. But if
 // this is C-style, static_cast can do this.
 msg = diag::err_bad_cxx_cast_rvalue;
 return TC_NotApplicable;
   }
-
-  // Materialize the class prvalue so that the const_cast can bind a
-  // reference to it.
-  NeedToMaterializeTemporary = true;
 }
 
 // It's not completely clear under the standard whether we can


Index: clang/test/SemaCXX/const-cast.cpp
===
--- clang/test/SemaCXX/const-cast.cpp
+++ clang/test/SemaCXX/const-cast.cpp
@@ -1,6 +1,9 @@
 // RUN: %clang_cc1 -fsyntax-only -verify %s
 // RUN: %clang_cc1 -fsyntax-only -verify -std=c++98 %s
 // RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++14 %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++17 %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++20 %s
 
 struct A {};
 
@@ -80,3 

[PATCH] D87974: [Builtin] Add __builtin_zero_non_value_bits.

[Zoe Carver via Phabricator via cfe-commits]

zoecarver added a comment.

@jfb (and others), friendly ping. Are there any other changes (especially to 
the logic) that you'd like me to make?




Comment at: clang/lib/CodeGen/CGBuiltin.cpp:1647
+QualType Ty) {
+  auto *I8Ptr = CGF.Builder.CreateBitCast(Ptr, CGF.Int8PtrTy);
+  auto *Zero = ConstantInt::get(CGF.Int8Ty, 0);

jfb wrote:
> I'd like to hear @rsmith's thoughts on this approach, in particular w.r.t. 
> aliasing concerns. I also wonder if below the GEPs should be inbounds, 
> depending on how they're created.
Hmm, I don't know much about it but, I think these could be inbound. Because we 
will never actually go beyond the size of the llvm type. 



Comment at: clang/test/CodeGenCXX/builtin-zero-non-value-bits-codegen.cpp:46
+void test(Baz *baz) {
+  __builtin_zero_non_value_bits(baz);
+}

zoecarver wrote:
> zoecarver wrote:
> > jfb wrote:
> > > It would be useful to see a test for arrays with a type that contains 
> > > tail padding.
> > Hmm, this test case doesn't seem to be working. I'll investigate further. 
> OK, I've added that. Just to clarify, you mean a type that contains a 
> constant array type of types with tail padding (i.e., `Bar [2]`)?
Let me know if there are more codegen tests you'd like me to add. Happy to add 
them. (Maybe one for vector types, or volatile, or one that doesn't have an 
non-value bits?)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87974

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

[PATCH] D87565: [Sema] Improve const_cast conformance to N4261

[Mark de Wever via Phabricator via cfe-commits]

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



Comment at: clang/lib/Sema/SemaCast.cpp:1817-1819
+  NeedToMaterializeTemporary =
+  SrcType->isRecordType() || SrcType->isArrayType() ||
+  SrcType->isFunctionPointerType() || SrcType->isMemberPointerType();

rsmith wrote:
> It looks like GCC allowing a cast from `T*` to `T*&&` is just a bug in their 
> implementation. Consider:
> 
> ```
> using T = int*;
> T & = const_cast(T{}); // GCC accepts
> T & = const_cast(T()); // GCC rejects
> ```
> 
> ... and the same behavior seems to show up for all scalar types: they permit 
> `const_cast` from `T{}` but not from `T()` when `T` is `int`, `int*`,  
> This doesn't seem like good behavior to follow. I think we should either 
> implement the current direction of 1965 (that is, only allow classes and 
> arrays here) or leave the current behavior (following the standard as-is) 
> alone.
> It looks like GCC allowing a cast from `T*` to `T*&&` is just a bug in their 
> implementation. Consider:
> 
> ```
> using T = int*;
> T & = const_cast(T{}); // GCC accepts
> T & = const_cast(T()); // GCC rejects
> ```
> 
> ... and the same behavior seems to show up for all scalar types: they permit 
> `const_cast` from `T{}` but not from `T()` when `T` is `int`, `int*`, 

Interesting I hadn't test with `T()` only with `T{}`

> This doesn't seem like good behavior to follow.

Agreed.

> I think we should either implement the current direction of 1965 (that is, 
> only allow classes and arrays here)

I'll then limit the patch to array types (next to the already allowed record 
types.

> or leave the current behavior (following the standard as-is) alone.

Now I'm somewhat confused, because I thought arrays are allowed in the standard 
http://eel.is/c++draft/expr.const.cast#3 as introduced by N4261.



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

https://reviews.llvm.org/D87565

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

[PATCH] D24688: Introduce "hosted" module flag.

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

dexonsmith requested changes to this revision.
dexonsmith added a comment.
This revision now requires changes to proceed.
Herald added subscribers: kerbowa, steven_wu, atanasyan, zzheng, MaskRay, 
fedor.sergeev, hiraditya, jvesely, sdardis, emaste.
Herald added a reviewer: espindola.
Herald added a project: LLVM.

You may have abandoned this, but marking as needing changes to get this off my 
queue.


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

https://reviews.llvm.org/D24688

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

[PATCH] D45639: [Driver] Support default libc++ library location on Darwin

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

dexonsmith resigned from this revision.
dexonsmith added a reviewer: Bigcheese.
dexonsmith added a subscriber: Bigcheese.
dexonsmith added a comment.
Herald added a subscriber: dexonsmith.

If this is still relevant, I’m happy for @ldionne and @Bigcheese to make the 
call.


Repository:
  rC Clang

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

https://reviews.llvm.org/D45639

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

[PATCH] D89055: [analyzer] Wrong type cast occures during pointer dereferencing after type punning

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Aha, great, sounds like all casting can be made more correct, not just casting 
on store retrieve! Maybe it's worth celebrating through extra unittests on 
`evalCast()`. Thank you for looking into this.

The new code should obviously be restricted into `evalCastFromLoc()` because if 
it's a region it's a `Loc`.

You didn't need to move `castRegion()`: `StoreManager` is available in 
`SValBuilder` through `this->StateMgr`. I don't have a strong preference on 
where `castRegion()` should live; the original idea was that `StoreManager` 
should be allowed to have an opinion on this matter because different store 
managers would handle that differently; however, as of now there's only one 
store manager and there doesn't seem to be an interest in introducing more of 
them whereas the abstraction has already leaked all over the place anyway.

I still have questions about the extra if-statements that you've added. 
Shouldn't we do `castRegion()` unconditionally, given that we're trying to cast 
a region and that function presumably does exactly that? I just want to avoid 
these situations:

F13337846: photo_2020-10-16_14-19-19.jpg 

In D89055#2320363 , @NoQ wrote:

> `dispatchCast()` was supposed to be the ultimate method to do so

Ugh, sorry, no, that's `evalCast()`. Like `evalBinOp()` etc. My bad. Can we 
also use `evalCast()`?


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

https://reviews.llvm.org/D89055

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

[PATCH] D87279: [clang] Fix handling of physical registers in inline assembly operands.

[Jonas Paulsson via Phabricator via cfe-commits]

jonpa updated this revision to Diff 298819.
jonpa added a comment.

Sorry, here's the test case included as well...


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

https://reviews.llvm.org/D87279

Files:
  clang/lib/CodeGen/CGStmt.cpp
  clang/test/CodeGen/aarch64-inline-asm.c
  clang/test/CodeGen/systemz-inline-asm-02.c
  clang/test/CodeGen/systemz-inline-asm.c

Index: clang/test/CodeGen/systemz-inline-asm.c
===
--- clang/test/CodeGen/systemz-inline-asm.c
+++ clang/test/CodeGen/systemz-inline-asm.c
@@ -129,3 +129,17 @@
 // CHECK: [[RESULT:%.*]] = tail call fp128 asm "axbr $0, $2", "=f,0,f"(fp128 %f, fp128 %g)
 // CHECK: store fp128 [[RESULT]], fp128* [[DEST]]
 }
+
+// Test that there are no tied physreg uses. TwoAddress pass cannot deal with them.
+int test_physregs(void) {
+  // CHECK-LABEL: define signext i32 @test_physregs()
+  register int l __asm__("r7") = 0;
+
+  // CHECK: call i32 asm "lr $0, $1", "={r7},{r7}"
+  __asm__("lr %0, %1" : "+r"(l));
+
+  // CHECK: call i32 asm "$0 $1 $2", "={r7},{r7},{r7}"
+  __asm__("%0 %1 %2" : "+r"(l) : "r"(l));
+
+  return l;
+}
Index: clang/test/CodeGen/systemz-inline-asm-02.c
===
--- /dev/null
+++ clang/test/CodeGen/systemz-inline-asm-02.c
@@ -0,0 +1,13 @@
+// RUN: not %clang_cc1 -triple s390x-linux-gnu -O2 -emit-llvm -o - %s 2>&1 \
+// RUN:  | FileCheck %s
+// REQUIRES: systemz-registered-target
+
+// Test that an error is given if a physreg is defined by multiple operands.
+int test_physreg_defs(void) {
+  register int l __asm__("r7") = 0;
+
+  // CHECK: error: multiple outputs to hard register: r7
+  __asm__("" : "+r"(l), "=r"(l));
+
+  return l;
+}
Index: clang/test/CodeGen/aarch64-inline-asm.c
===
--- clang/test/CodeGen/aarch64-inline-asm.c
+++ clang/test/CodeGen/aarch64-inline-asm.c
@@ -74,3 +74,9 @@
 asm volatile("mov r0, r1\n");
 // CHECK: call void asm sideeffect "mov r0, r1\0A", ""()
 }
+
+void test_tied_earlyclobber(void) {
+  register int a asm("x1");
+  asm("" : "+"(a));
+  // CHECK: call i32 asm "", "=&{x1},0"(i32 %0)
+}
Index: clang/lib/CodeGen/CGStmt.cpp
===
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -21,6 +21,7 @@
 #include "clang/Basic/PrettyStackTrace.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/TargetInfo.h"
+#include "llvm/ADT/SmallSet.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/IR/DataLayout.h"
 #include "llvm/IR/InlineAsm.h"
@@ -1836,7 +1837,8 @@
 static std::string
 AddVariableConstraints(const std::string , const Expr ,
const TargetInfo , CodeGenModule ,
-   const AsmStmt , const bool EarlyClobber) {
+   const AsmStmt , const bool EarlyClobber,
+   std::string *GCCReg = nullptr) {
   const DeclRefExpr *AsmDeclRef = dyn_cast();
   if (!AsmDeclRef)
 return Constraint;
@@ -1861,6 +1863,8 @@
   }
   // Canonicalize the register here before returning it.
   Register = Target.getNormalizedGCCRegisterName(Register);
+  if (GCCReg != nullptr)
+*GCCReg = Register.str();
   return (EarlyClobber ? "&{" : "{") + Register.str() + "}";
 }
 
@@ -2059,6 +2063,9 @@
   // Keep track of out constraints for tied input operand.
   std::vector OutputConstraints;
 
+  // Keep track of defined physregs.
+  llvm::SmallSet PhysRegOutputs;
+
   // An inline asm can be marked readonly if it meets the following conditions:
   //  - it doesn't have any sideeffects
   //  - it doesn't clobber memory
@@ -2078,9 +2085,15 @@
 const Expr *OutExpr = S.getOutputExpr(i);
 OutExpr = OutExpr->IgnoreParenNoopCasts(getContext());
 
+std::string GCCReg;
 OutputConstraint = AddVariableConstraints(OutputConstraint, *OutExpr,
   getTarget(), CGM, S,
-  Info.earlyClobber());
+  Info.earlyClobber(),
+  );
+// Give an error on multiple outputs to same physreg.
+if (!GCCReg.empty() && !PhysRegOutputs.insert(GCCReg).second)
+  CGM.Error(S.getAsmLoc(), "multiple outputs to hard register: " + GCCReg);
+
 OutputConstraints.push_back(OutputConstraint);
 LValue Dest = EmitLValue(OutExpr);
 if (!Constraints.empty())
@@ -2167,7 +2180,8 @@
 LargestVectorWidth =
 std::max((uint64_t)LargestVectorWidth,
  VT->getPrimitiveSizeInBits().getKnownMinSize());
-  if (Info.allowsRegister())
+  // Only tie earlyclobber physregs.
+  if (Info.allowsRegister() && (GCCReg.empty() || Info.earlyClobber()))
 InOutConstraints += llvm::utostr(i);
   else
 InOutConstraints += OutputConstraint;

[PATCH] D74130: [clang] fix consteval call in default arguments

[Tyker via Phabricator via cfe-commits]

Tyker updated this revision to Diff 298818.
Tyker marked 9 inline comments as done.
Tyker added a comment.

addressed comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74130

Files:
  clang/include/clang/AST/Expr.h
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/ScopeInfo.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaLambda.cpp
  clang/test/SemaCXX/cxx2a-consteval.cpp

Index: clang/test/SemaCXX/cxx2a-consteval.cpp
===
--- clang/test/SemaCXX/cxx2a-consteval.cpp
+++ clang/test/SemaCXX/cxx2a-consteval.cpp
@@ -258,6 +258,26 @@
   return f(0);  
 };
 
+consteval int f1() {
+// expected-note@-1+ {{declared here}}
+  return 0;
+}
+consteval auto g() { return f1; }
+consteval int h(int (*p)() = g()) { return p(); }
+int h1(int (*p)() = g()) { return p(); }
+// expected-error@-1 {{is not a constant expression}}
+// expected-note@-2 {{pointer to a consteval}}
+
+constexpr auto e = g();
+// expected-error@-1 {{call to consteval function}}
+// expected-note@-2 {{is not a constant expression}}
+
+auto l = [](int (*p)() = g()) { return p(); };
+// expected-error@-1 {{is not a constant expression}}
+// expected-note@-2 {{pointer to a consteval}}
+
+auto l2 = [](int (*p)() = g()) consteval { return p(); };
+
 }
 
 namespace std {
@@ -594,3 +614,42 @@
 }
 
 } // namespace special_ctor
+
+namespace top_level {
+struct S {
+  consteval S() {}
+  int a;
+// expected-note@-1 {{subobject declared here}}
+};
+
+S s; // expected-error {{is not a constant expression}}
+// expected-note@-1 {{is not initialized}}
+
+struct S1 {
+  consteval S1() {}
+  int a = 0;
+};
+
+S1 s1;
+
+}
+
+namespace unevaluated {
+struct N {
+  constexpr N() {}
+  N(N const&) = delete;
+};
+template  constexpr void bad_assert_copyable() { T t; T t2 = t; }
+using ineffective = decltype(bad_assert_copyable());
+
+template  consteval void assert_copyable() { T t; T t2 = t; }
+using check = decltype(assert_copyable());
+}
+
+// namespace error {
+
+// consteval void check(bool b) { if (!b) throw; }
+// constinit int x = true ? 1 : (check(false), 2);
+// constinit int x1 = (check(false), 2);
+
+// }
Index: clang/lib/Sema/SemaLambda.cpp
===
--- clang/lib/Sema/SemaLambda.cpp
+++ clang/lib/Sema/SemaLambda.cpp
@@ -891,6 +891,16 @@
   LambdaScopeInfo *const LSI = getCurLambda();
   assert(LSI && "LambdaScopeInfo should be on stack!");
 
+  /// If the lambda is consteval the paramters will be evaluated in constant
+  /// context immediate invocations don't need to be kept.
+  if (ParamInfo.getDeclSpec().getConstexprSpecifier() != CSK_consteval) {
+ExprEvalContexts.back().ReferenceToConsteval.insert(
+LSI->ReferenceToConsteval.begin(), LSI->ReferenceToConsteval.end());
+ExprEvalContexts.back().ImmediateInvocationCandidates.append(
+LSI->ImmediateInvocationCandidates.begin(),
+LSI->ImmediateInvocationCandidates.end());
+  }
+
   // Determine if we're within a context where we know that the lambda will
   // be dependent, because there are template parameters in scope.
   bool KnownDependent;
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -16183,7 +16183,7 @@
 
 ExprResult Sema::CheckForImmediateInvocation(ExprResult E, FunctionDecl *Decl) {
   if (!E.isUsable() || !Decl || !Decl->isConsteval() || isConstantEvaluated() ||
-  RebuildingImmediateInvocation)
+  RebuildingImmediateInvocation || isUnevaluatedContext())
 return E;
 
   /// Opportunistically remove the callee from ReferencesToConsteval if we can.
@@ -16229,6 +16229,7 @@
 SemaRef.Diag(CE->getBeginLoc(), diag::err_invalid_consteval_call) << FD;
 for (auto  : Notes)
   SemaRef.Diag(Note.first, Note.second);
+CE->markFailed();
 return;
   }
   CE->MoveIntoResult(Eval.Val, SemaRef.getASTContext());
@@ -16312,13 +16313,25 @@
   It->getPointer()->setSubExpr(Res.get());
 }
 
-static void
-HandleImmediateInvocations(Sema ,
-   Sema::ExpressionEvaluationContextRecord ) {
+void Sema::HandleImmediateInvocations(
+Sema::ExpressionEvaluationContextRecord ) {
   if ((Rec.ImmediateInvocationCandidates.size() == 0 &&
Rec.ReferenceToConsteval.size() == 0) ||
-  SemaRef.RebuildingImmediateInvocation)
+  RebuildingImmediateInvocation || isUnevaluatedContext())
+return;
+
+  /// If the scope is lambda params we don't know wether the lambda is consteval
+  /// yet. So we store all the needed information in the lambda and resolve it
+  /// later.
+  if (Rec.ExprContext ==
+  Sema::ExpressionEvaluationContextRecord::EK_LambdaParam) {
+

[PATCH] D74130: [clang] fix consteval call in default arguments

[Tyker via Phabricator via cfe-commits]

Tyker added inline comments.



Comment at: clang/lib/Sema/Sema.cpp:1026
 
+  HandleImmediateInvocations(ExprEvalContexts.back());
+

rsmith wrote:
> What do we need this for? If I'm understanding the patch correctly, I think 
> the only way we should propagate immediate invocations upwards should be from 
> the `ExpressionEvaluationContext` of a lambda default argument into the 
> `ExpressionEvaluationContext` of the lambda itself, so it should never reach 
> the top level.
This is fixing a separate bug that consteval call in to top-level 
ExprEvalContext are not handled.
like this:
```
struct S {
  consteval S(bool b) {if (b) throw;}
};
S s2(true);
```
this emits no error.
because the immediate invocation are added to global scope and the global scope 
is never poped via PopExpressionEvaluationContext.



Comment at: clang/lib/Sema/SemaDeclCXX.cpp:17045-17048
+  bool HasConstantInitializer = false;
+  if (auto *VD = dyn_cast(D))
+HasConstantInitializer = VD->isConstexpr() || VD->hasAttr();
+  isConstantEvaluatedOverride = HasConstantInitializer;

rsmith wrote:
> This needs a comment explaining what's going on: why are `constexpr` / 
> `constinit` variables special here, but (for example) a variable of type 
> `const int` is not?
> 
> Actually, I think this change might not be correct. Consider the following:
> 
> ```
> consteval void check(bool b) { if (!b) throw; }
> constinit int x = true ? 1 : (check(false), 2);
> ```
> 
> This code is ill-formed: the immediate invocation of `check(false)` is not a 
> constant expression. But I think with this code change, we won't require 
> `check(false)` to be a constant expression, instead deferring to checking 
> whether the initializer of `x` as a whole is a constant expression (which it 
> is!). So we'll accept invalid code.
this fixes an issue where we would generate 2 error when we couldn't evaluate a 
consteval call inside the initialization of a constexpr/constinit 
variable.
```
consteval void check(bool b) { if (!b) throw; }
constinit int x = (check(false), 2);
```
one of those error is "call to consteval function 'check' is not a constant 
expression"
the second is "variable does not have a constant initializer"

i fixed it by marking the expression as failed when the consteval call fails.



Comment at: clang/lib/Sema/SemaDeclCXX.cpp:17062
 ExitDeclaratorContext(S);
+  isConstantEvaluatedOverride = false;
 }

rsmith wrote:
> Do we need to restore the old value here, rather than resetting to `false`? 
> In a case like:
> 
> ```
> consteval int g() { return 0; }
> constexpr int a = ({ constexpr int b = 0; g(); });
> ```
> 
> it looks like we'd lose the 'override' flag at the end of the definition of 
> `b`.
> 
> Generally I find the idea of using a global override flag like this to be 
> dubious: there are a lot of ways in which we switch context during `Sema`, 
> and we'd need to make sure we switch out this context at those times too. If 
> we can avoid using a global state flag for this (for example by carrying this 
> on the `ExpressionEvaluationContextRecord`, that'd be a lot less worrying.
i changed approach and this isn't needed anymore.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74130

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

[clang] 5564ee4 - Revert "Register TargetCXXABI.def as a textual header"

[Dave Lee via cfe-commits]

Author: Dave Lee
Date: 2020-10-17T00:15:34-07:00
New Revision: 5564ee495bddeb12d3a9a617631cfa478cf67959

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

LOG: Revert "Register TargetCXXABI.def as a textual header"

Unbreak module builds.

TargetCXXABI.def has been removed in a revert: 
79829a47040512fe54001db839ac59146ca55aec.

This reverts commit 0ff9116b36781d6fa61c25841edd53dc8f366bec.

Added: 


Modified: 
clang/include/clang/module.modulemap

Removed: 




diff  --git a/clang/include/clang/module.modulemap 
b/clang/include/clang/module.modulemap
index cee7ed3d7de3..13d4dbf9dc2e 100644
--- a/clang/include/clang/module.modulemap
+++ b/clang/include/clang/module.modulemap
@@ -63,7 +63,6 @@ module Clang_Basic {
   textual header "Basic/OpenMPKinds.def"
   textual header "Basic/OperatorKinds.def"
   textual header "Basic/Sanitizers.def"
-  textual header "Basic/TargetCXXABI.def"
   textual header "Basic/TokenKinds.def"
   textual header "Basic/X86Target.def"
 



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

[PATCH] D89332: [clang-tidy] performance-unnecessary-copy-initialization: Always allow std::function to be copied.

[Nathan James via Phabricator via cfe-commits]

njames93 added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp:409
+
+namespace std {
+

gribozavr2 wrote:
> flx wrote:
> > gribozavr2 wrote:
> > > Could you add a nested inline namespace to better imitate what 
> > > declarations look like in libc++?
> > I'm not sure I follow.  I looked through the other tests that declare a std 
> > function and copied the declaration from modernize-avoid-bind.cpp.
> libc++ declarations look like this:
> 
> ```
> namespace std {
> inline namespace __1 {
> template<...> struct function...
> } // __1
> } // std
> ```
> 
> The inline namespace in the middle often trips up declaration matching in 
> checkers. And yes, many other tests don't imitate this pattern, and are often 
> broken with libc++. Those tests should be improved.
@flx Thats the reason why its advisable to use `Decl::isInStdNamespace` as it 
will handle inline namespaces.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89332

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

[PATCH] D89608: Make sure Objective-C category support in IncludeSorter handles top-level imports

[Dmitri Gribenko via Phabricator via cfe-commits]

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

Any chance of adding a test?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89608

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

[PATCH] D89332: [clang-tidy] performance-unnecessary-copy-initialization: Always allow std::function to be copied.

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr2 added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp:409
+
+namespace std {
+

flx wrote:
> gribozavr2 wrote:
> > Could you add a nested inline namespace to better imitate what declarations 
> > look like in libc++?
> I'm not sure I follow.  I looked through the other tests that declare a std 
> function and copied the declaration from modernize-avoid-bind.cpp.
libc++ declarations look like this:

```
namespace std {
inline namespace __1 {
template<...> struct function...
} // __1
} // std
```

The inline namespace in the middle often trips up declaration matching in 
checkers. And yes, many other tests don't imitate this pattern, and are often 
broken with libc++. Those tests should be improved.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89332

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

[PATCH] D87830: [clang-tidy][test] Allow empty checks in check_clang_tidy.py

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

Probably not quite as verbose but should do the job

  // RUN: clang-tidy %s --checks=-*,my-check-to-test --warnings-as-errors=* -- 
-std=c++11


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87830

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

[PATCH] D89332: [clang-tidy] performance-unnecessary-copy-initialization: Always allow std::function to be copied.

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

How does this type alias and typedef, In theory that should also be handled.

  using Functor = std::function<...>;
  Functor Copy = Orig; // No warning.




Comment at: 
clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp:39-44
+AST_MATCHER(NamedDecl, isStdFunction) {
+  // First use the fast getName() method to avoid unnecessary calls to the
+  // slow getQualifiedNameAsString().
+  return Node.getName() == "function" &&
+ Node.getQualifiedNameAsString() == "std::function";
+}

It's better to use `node.isInStdNamespace()` instead of checking the qualified 
name as its designed for that very purpose. Is should probably take a 
`CXXRecordDecl` instead of a `NamedDecl` aswell.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89332

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

[PATCH] D66324: clang-misexpect: Profile Guided Validation of Performance Annotations in LLVM

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

In D66324#2336186 , @phosek wrote:

> I apologize for the late response, I somehow missed the earlier responses. We 
> have successfully used this feature in Fuchsia and found it useful, but I 
> agree that the issues raised need to be addressed. Unfortunately @paulkirth 
> is no longer working on this project. I hope that someone from our team can 
> take a look but it might take a few weeks. If you prefer, we could revert 
> this change and then reland an improved version in the future?

I would very much prefer *NOT* not revert if someone is going to step up to 
work on these problems soon (within next 4 weeks?).

That being said, in light of that bug, my original doubts about the underlying 
data type (a novel `MD_misexpect`,
with structure different from `MD_prof`) have reappeared with double strength. 
I really think they should share underlying type.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D66324

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