[PATCH] D75332: [clang-tidy] Add module for llvm-libc and restrict-system-libc-header-check.

[Nathan James via Phabricator via cfe-commits]

njames93 added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/llvmlibc/RestrictSystemLibcHeadersCheck.cpp:40
+SrcMgr::CharacteristicKind FileType) {
+  if (SrcMgr::isSystem(FileType)) {
+if (!SM.isInMainFile(HashLoc)) {

abrachet wrote:
> Could you whitelist the freestanding/compiler provided headers like stddef, 
> stdatomic...
Or have a user configurable whitelist 



Comment at: 
clang-tools-extra/clang-tidy/llvmlibc/RestrictSystemLibcHeadersCheck.cpp:42
+if (!SM.isInMainFile(HashLoc)) {
+  auto D = Check.diag(
+  HashLoc,

Don't use auto when the type isn't an iterator or spelled out on the 
initialisation. 



Comment at: 
clang-tools-extra/clang-tidy/llvmlibc/RestrictSystemLibcHeadersCheck.cpp:47
+} else {
+  auto D = Check.diag(HashLoc, "system libc header %0 not allowed");
+  D << FileName;

Ditto


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75332



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

[clang] 624dbfc - [Coroutines][New pass manager] Move CoroElide pass to right position

[Jun Ma via cfe-commits]

Author: Jun Ma
Date: 2020-03-01T21:48:24+08:00
New Revision: 624dbfcc1b81520d2211d43a759f817d0b131de0

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

LOG: [Coroutines][New pass manager] Move CoroElide pass to right position

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

Added: 


Modified: 
clang/test/CodeGenCoroutines/coro-newpm-pipeline.cpp
llvm/lib/Passes/PassBuilder.cpp
llvm/test/Transforms/Coroutines/ex2.ll
llvm/test/Transforms/Coroutines/ex3.ll

Removed: 




diff  --git a/clang/test/CodeGenCoroutines/coro-newpm-pipeline.cpp 
b/clang/test/CodeGenCoroutines/coro-newpm-pipeline.cpp
index aed2cf13f892..cea71a1acc6b 100644
--- a/clang/test/CodeGenCoroutines/coro-newpm-pipeline.cpp
+++ b/clang/test/CodeGenCoroutines/coro-newpm-pipeline.cpp
@@ -14,14 +14,14 @@
 // The first coro-split pass enqueues a second run of the entire CGSCC 
pipeline.
 // CHECK: Starting CGSCC pass manager run.
 // CHECK: Running pass: CoroSplitPass on (_Z3foov)
-// CHECK: Running pass:{{.*}}CoroElidePass{{.*}} on (_Z3foov)
+// CHECK: Running pass:{{.*}}CoroElidePass{{.*}} on {{.*}}_Z3foov{{.*}}
 // CHECK: Finished CGSCC pass manager run.
 //
 // The second coro-split pass splits coroutine 'foo' into funclets
 // 'foo.resume', 'foo.destroy', and 'foo.cleanup'.
 // CHECK: Starting CGSCC pass manager run.
 // CHECK: Running pass: CoroSplitPass on (_Z3foov)
-// CHECK: Running pass:{{.*}}CoroElidePass{{.*}} on (_Z3foov)
+// CHECK: Running pass:{{.*}}CoroElidePass{{.*}} on {{.*}}_Z3foov{{.*}}
 // CHECK: Finished CGSCC pass manager run.
 //
 // CHECK: Running pass:{{.*}}CoroCleanupPass

diff  --git a/llvm/lib/Passes/PassBuilder.cpp b/llvm/lib/Passes/PassBuilder.cpp
index f4806b2db3c8..eb5b3a61fa89 100644
--- a/llvm/lib/Passes/PassBuilder.cpp
+++ b/llvm/lib/Passes/PassBuilder.cpp
@@ -561,6 +561,9 @@ 
PassBuilder::buildFunctionSimplificationPipeline(OptimizationLevel Level,
 EnableMSSALoopDependency, DebugLogging));
   }
 
+  if (PTO.Coroutines)
+FPM.addPass(CoroElidePass());
+
   for (auto &C : ScalarOptimizerLateEPCallbacks)
 C(FPM, Level);
 
@@ -847,10 +850,8 @@ 
PassBuilder::buildModuleSimplificationPipeline(OptimizationLevel Level,
 
   MainCGPipeline.addPass(AttributorCGSCCPass());
 
-  if (PTO.Coroutines) {
+  if (PTO.Coroutines)
 MainCGPipeline.addPass(CoroSplitPass());
-MainCGPipeline.addPass(createCGSCCToFunctionPassAdaptor(CoroElidePass()));
-  }
 
   // Now deduce any function attributes based in the current code.
   MainCGPipeline.addPass(PostOrderFunctionAttrsPass());

diff  --git a/llvm/test/Transforms/Coroutines/ex2.ll 
b/llvm/test/Transforms/Coroutines/ex2.ll
index cd7d8d2a20ed..584bc909a4eb 100644
--- a/llvm/test/Transforms/Coroutines/ex2.ll
+++ b/llvm/test/Transforms/Coroutines/ex2.ll
@@ -40,8 +40,14 @@ entry:
   %hdl = call i8* @f(i32 4)
   call void @llvm.coro.resume(i8* %hdl)
   call void @llvm.coro.resume(i8* %hdl)
+  %to = icmp eq i8* %hdl, null
+  br i1 %to, label %return, label %destroy
+destroy:
   call void @llvm.coro.destroy(i8* %hdl)
+  br label %return
+return:
   ret i32 0
+; CHECK-NOT:  call i8* @CustomAlloc
 ; CHECK:  call void @print(i32 4)
 ; CHECK-NEXT: call void @print(i32 5)
 ; CHECK-NEXT: call void @print(i32 6)

diff  --git a/llvm/test/Transforms/Coroutines/ex3.ll 
b/llvm/test/Transforms/Coroutines/ex3.ll
index 50ce19e26372..85cf53fb576d 100644
--- a/llvm/test/Transforms/Coroutines/ex3.ll
+++ b/llvm/test/Transforms/Coroutines/ex3.ll
@@ -6,11 +6,17 @@ define i8* @f(i32 %n) {
 entry:
   %id = call token @llvm.coro.id(i32 0, i8* null, i8* null, i8* null)
   %size = call i32 @llvm.coro.size.i32()
+  %need.dyn.alloc = call i1 @llvm.coro.alloc(token %id)
+  br i1 %need.dyn.alloc, label %dyn.alloc, label %coro.begin
+dyn.alloc:
   %alloc = call i8* @malloc(i32 %size)
-  %hdl = call noalias i8* @llvm.coro.begin(token %id, i8* %alloc)
+  br label %coro.begin
+coro.begin:
+  %phi = phi i8* [ null, %entry ], [ %alloc, %dyn.alloc ]
+  %hdl = call noalias i8* @llvm.coro.begin(token %id, i8* %phi)
   br label %loop
 loop:
-  %n.val = phi i32 [ %n, %entry ], [ %inc, %loop.resume ]
+  %n.val = phi i32 [ %n, %coro.begin ], [ %inc, %loop.resume ]
   call void @print(i32 %n.val) #4
   %0 = call i8 @llvm.coro.suspend(token none, i1 false)
   switch i8 %0, label %suspend [i8 0, label %loop.resume
@@ -37,8 +43,15 @@ entry:
   %hdl = call i8* @f(i32 4)
   call void @llvm.coro.resume(i8* %hdl)
   call void @llvm.coro.resume(i8* %hdl)
+  %c = ptrtoint i8* %hdl to i64
+  %to = icmp eq i64 %c, 0
+  br i1 %to, label %return, label %destroy
+destroy:
   call void @llvm.coro.destroy(i8* %hdl)
+  br label %return
+return:
   ret i32 0
+; CHECK-NOT:  i8* @malloc
 ; CHECK:  call void @print(i32 4)
 ; CHECK-NEXT: call void @print(i32 -5)
 ; CHECK-NEX

[PATCH] D75423: [OpenCL] Mark pointers to constant address space as invariant

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl created this revision.
yaxunl added reviewers: rjmccall, Anastasia, bader.

OpenCL constant address space is immutable, therefore pointers to constant 
address space can
be marked with llvm.invariant.start permanently.

This should allow more optimization opportunities in LLVM passes.


https://reviews.llvm.org/D75423

Files:
  clang/lib/CodeGen/CGDeclCXX.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/test/CodeGenOpenCL/invariant.cl
  clang/test/CodeGenOpenCL/printf.cl

Index: clang/test/CodeGenOpenCL/printf.cl
===
--- clang/test/CodeGenOpenCL/printf.cl
+++ clang/test/CodeGenOpenCL/printf.cl
@@ -12,25 +12,25 @@
 
 
 // ALL-LABEL: @test_printf_float2(
-// FP64: %call = call spir_func i32 (i8 addrspace(2)*, ...) @printf(i8 addrspace(2)* getelementptr inbounds ([7 x i8], [7 x i8] addrspace(2)* @.str, i32 0, i32 0), <2 x float> %0)
+// FP64: %call = call spir_func i32 (i8 addrspace(2)*, ...) @printf(i8 addrspace(2)* getelementptr inbounds ([7 x i8], [7 x i8] addrspace(2)* @.str, i32 0, i32 0), <2 x float>
 
 
-// NOFP64:  call spir_func i32 (i8 addrspace(2)*, ...) @printf(i8 addrspace(2)* getelementptr inbounds ([7 x i8], [7 x i8] addrspace(2)* @.str, i32 0, i32 0), <2 x float> %0)
+// NOFP64:  call spir_func i32 (i8 addrspace(2)*, ...) @printf(i8 addrspace(2)* getelementptr inbounds ([7 x i8], [7 x i8] addrspace(2)* @.str, i32 0, i32 0), <2 x float>
 kernel void test_printf_float2(float2 arg) {
   printf("%v2hlf", arg);
 }
 
 // ALL-LABEL: @test_printf_half2(
-// FP64:  %call = call spir_func i32 (i8 addrspace(2)*, ...) @printf(i8 addrspace(2)* getelementptr inbounds ([6 x i8], [6 x i8] addrspace(2)* @.str.1, i32 0, i32 0), <2 x half> %0)
+// FP64:  %call = call spir_func i32 (i8 addrspace(2)*, ...) @printf(i8 addrspace(2)* getelementptr inbounds ([6 x i8], [6 x i8] addrspace(2)* @.str.1, i32 0, i32 0), <2 x half>
 
-// NOFP64:  %call = call spir_func i32 (i8 addrspace(2)*, ...) @printf(i8 addrspace(2)* getelementptr inbounds ([6 x i8], [6 x i8] addrspace(2)* @.str.1, i32 0, i32 0), <2 x half> %0)
+// NOFP64:  %call = call spir_func i32 (i8 addrspace(2)*, ...) @printf(i8 addrspace(2)* getelementptr inbounds ([6 x i8], [6 x i8] addrspace(2)* @.str.1, i32 0, i32 0), <2 x half>
 kernel void test_printf_half2(half2 arg) {
   printf("%v2hf", arg);
 }
 
 #ifdef cl_khr_fp64
 // FP64-LABEL: @test_printf_double2(
-// FP64: call spir_func i32 (i8 addrspace(2)*, ...) @printf(i8 addrspace(2)* getelementptr inbounds ([6 x i8], [6 x i8] addrspace(2)* @.str.2, i32 0, i32 0), <2 x double> %0)
+// FP64: call spir_func i32 (i8 addrspace(2)*, ...) @printf(i8 addrspace(2)* getelementptr inbounds ([6 x i8], [6 x i8] addrspace(2)* @.str.2, i32 0, i32 0), <2 x double>
 kernel void test_printf_double2(double2 arg) {
   printf("%v2lf", arg);
 }
Index: clang/test/CodeGenOpenCL/invariant.cl
===
--- /dev/null
+++ clang/test/CodeGenOpenCL/invariant.cl
@@ -0,0 +1,38 @@
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -O3 -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple spir -O3 -emit-llvm -o - %s | FileCheck %s
+
+typedef struct {
+  int a;
+  char b;
+} X;
+
+constant X x = {0, 'a'};
+
+constant char* constant p = &(x.b);
+
+constant X* foo();
+
+// CHECK-LABEL: test1
+// CHECK: llvm.invariant.start
+char test1() {
+  return x.b;
+}
+
+// CHECK-LABEL: test2
+// CHECK: llvm.invariant.start
+char test2() {
+  return *p;
+}
+
+// CHECK-LABEL: test3
+// CHECK: llvm.invariant.start
+char test3(constant X *x) {
+  constant char *p = &(x->b);
+  return *p;
+}
+
+// CHECK-LABEL: test4
+// CHECK: llvm.invariant.start
+char test4() {
+  return foo()->b;
+}
Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -4070,7 +4070,7 @@
 llvm::GlobalVariable *GV);
 
   // Emit an @llvm.invariant.start call for the given memory region.
-  void EmitInvariantStart(llvm::Constant *Addr, CharUnits Size);
+  void EmitInvariantStart(llvm::Value *Addr, CharUnits Size);
 
   /// EmitCXXGlobalVarDeclInit - Create the initializer for a C++
   /// variable with global storage.
Index: clang/lib/CodeGen/CGExprScalar.cpp
===
--- clang/lib/CodeGen/CGExprScalar.cpp
+++ clang/lib/CodeGen/CGExprScalar.cpp
@@ -262,7 +262,17 @@
   const BinOpInfo &Info);
 
   Value *EmitLoadOfLValue(LValue LV, SourceLocation Loc) {
-return CGF.EmitLoadOfLValue(LV, Loc).getScalarVal();
+auto *V = CGF.EmitLoadOfLValue(LV, Loc).getScalarVal();
+// Mark a pointer to OpenCL constant address space as invariant.
+auto QT = LV.getType();
+if (QT->isPointerType()) {
+  auto PointeeTy = QT->getPointeeType();
+  i

[PATCH] D75402: [HIP] Make sure, unused hip-pinned-shadow global var is kept within device code

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

LGTM. Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75402



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

[PATCH] D75255: [ARM,MVE] Add ACLE intrinsics for VCVT[ANPM] family.

[Dave Green via Phabricator via cfe-commits]

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

Sound good to me


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75255



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

[PATCH] D72857: [SYCL] Driver option to enable SYCL mode and select SYCL version

[Alexey Bader via Phabricator via cfe-commits]

bader added a comment.

In D72857#1895649 , @bader wrote:

> In D72857#1895625 , @thakis wrote:
>
> > This landed here: 
> > https://github.com/llvm/llvm-project/commit/bd97704eb5a95ecb048ce343c1a4be5d94e5
> >
> > It broke tests on mac: http://45.33.8.238/mac/9011/step_7.txt
> >
> > Please take a look, and if it takes a while please revert while you 
> > investigate.
>
>
> @thakis, thank for letting me know. I've reverted it at 
> 740ed617f7d4d16e7883636c5eff994f8be7eba4 
> . Sorry 
> for inconvenience.


@thakis, could you help to find the logs for the "build" step?
I don't have access to a Mac system and I can't reproduce the problem on my 
Linux system, but just looking at the log it looks like that changes to 
clang/include/clang/Driver/Options.td were not applied.

  :7:42: note: possible intended match here
  clang: warning: argument unused during compilation: '-fsycl' 
[-Wunused-command-line-argument]

I'd like to make sure that clang is built with the patch applied.

I also noticed that on your system uses gn instead of cmake. Can it be that 
there is a missing dependency in GN files, which lead to skipping clang 
re-build?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72857



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

[PATCH] D68165: [analyzer][MallocChecker][NFC] Split checkPostCall up, deploy CallDescriptionMap

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus marked 5 inline comments as done.
Szelethus added a comment.
Herald added subscribers: martong, steakhal.

I've spent a lot of time on this code, and I don't think it'd be a great idea 
to do everything in a single step. Collapsing `CallExpr, ProgramState` into 
`CallEvent` is a shockingly a non-trivial task. While I'm happy to not commit 
this patch before uploading a solution to that problem, I'd prefer (and I think 
you would too!) to do them in followup patches.

The point of this revision was to drop in `CallDescriptionMap` -- I like to 
think that it introduces no new problems, it just doesn't solve many existing 
ones :^)

edit: I will soon address the inlines and other comments with an update


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

https://reviews.llvm.org/D68165



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

[clang-tools-extra] 21390ea - [ADT][NFC] SCCIterator: Change hasLoop() to hasCycle()

[Stefanos Baziotis via cfe-commits]

Author: Stefanos Baziotis
Date: 2020-03-01T19:17:21+02:00
New Revision: 21390eab4c05e0ed7e7d13ada9e85f62b87ea484

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

LOG: [ADT][NFC] SCCIterator: Change hasLoop() to hasCycle()

Added: 


Modified: 
clang-tools-extra/clang-tidy/misc/NoRecursionCheck.cpp
llvm/include/llvm/ADT/SCCIterator.h
llvm/lib/IR/ModuleSummaryIndex.cpp
llvm/tools/opt/PrintSCC.cpp

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/misc/NoRecursionCheck.cpp 
b/clang-tools-extra/clang-tidy/misc/NoRecursionCheck.cpp
index d382501d191e..05007e5d85a3 100644
--- a/clang-tools-extra/clang-tidy/misc/NoRecursionCheck.cpp
+++ b/clang-tools-extra/clang-tidy/misc/NoRecursionCheck.cpp
@@ -264,7 +264,7 @@ void NoRecursionCheck::check(const MatchFinder::MatchResult 
&Result) {
   for (llvm::scc_iterator SCCI = llvm::scc_begin(&CG),
SCCE = llvm::scc_end(&CG);
SCCI != SCCE; ++SCCI) {
-if (!SCCI.hasLoop()) // We only care about cycles, not standalone nodes.
+if (!SCCI.hasCycle()) // We only care about cycles, not standalone nodes.
   continue;
 handleSCC(*SCCI);
   }

diff  --git a/llvm/include/llvm/ADT/SCCIterator.h 
b/llvm/include/llvm/ADT/SCCIterator.h
index 1e642b9f75d3..8a7c0a78a0fc 100644
--- a/llvm/include/llvm/ADT/SCCIterator.h
+++ b/llvm/include/llvm/ADT/SCCIterator.h
@@ -124,11 +124,11 @@ class scc_iterator : public iterator_facade_base<
 return CurrentSCC;
   }
 
-  /// Test if the current SCC has a loop.
+  /// Test if the current SCC has a cycle.
   ///
   /// If the SCC has more than one node, this is trivially true.  If not, it 
may
-  /// still contain a loop if the node has an edge back to itself.
-  bool hasLoop() const;
+  /// still contain a cycle if the node has an edge back to itself.
+  bool hasCycle() const;
 
   /// This informs the \c scc_iterator that the specified \c Old node
   /// has been deleted, and \c New is to be used in its place.
@@ -212,7 +212,7 @@ template  void scc_iterator::GetNextSCC() {
 }
 
 template 
-bool scc_iterator::hasLoop() const {
+bool scc_iterator::hasCycle() const {
 assert(!CurrentSCC.empty() && "Dereferencing END SCC iterator!");
 if (CurrentSCC.size() > 1)
   return true;

diff  --git a/llvm/lib/IR/ModuleSummaryIndex.cpp 
b/llvm/lib/IR/ModuleSummaryIndex.cpp
index 13a685e1609e..158369a80aca 100644
--- a/llvm/lib/IR/ModuleSummaryIndex.cpp
+++ b/llvm/lib/IR/ModuleSummaryIndex.cpp
@@ -300,7 +300,7 @@ void ModuleSummaryIndex::dumpSCCs(raw_ostream &O) {
   if (V.getSummaryList().size())
 F = cast(V.getSummaryList().front().get());
   O << " " << (F == nullptr ? "External" : "") << " " << 
utostr(V.getGUID())
-<< (I.hasLoop() ? " (has loop)" : "") << "\n";
+<< (I.hasCycle() ? " (has cycle)" : "") << "\n";
 }
 O << "}\n";
   }

diff  --git a/llvm/tools/opt/PrintSCC.cpp b/llvm/tools/opt/PrintSCC.cpp
index 419886d6cc60..5ab4a00552f3 100644
--- a/llvm/tools/opt/PrintSCC.cpp
+++ b/llvm/tools/opt/PrintSCC.cpp
@@ -79,7 +79,7 @@ bool CFGSCC::runOnFunction(Function &F) {
 for (std::vector::const_iterator I = nextSCC.begin(),
E = nextSCC.end(); I != E; ++I)
   errs() << (*I)->getName() << ", ";
-if (nextSCC.size() == 1 && SCCI.hasLoop())
+if (nextSCC.size() == 1 && SCCI.hasCycle())
   errs() << " (Has self-loop).";
   }
   errs() << "\n";
@@ -101,7 +101,7 @@ bool CallGraphSCC::runOnModule(Module &M) {
E = nextSCC.end(); I != E; ++I)
   errs() << ((*I)->getFunction() ? (*I)->getFunction()->getName()
  : "external node") << ", ";
-if (nextSCC.size() == 1 && SCCI.hasLoop())
+if (nextSCC.size() == 1 && SCCI.hasCycle())
   errs() << " (Has self-loop).";
   }
   errs() << "\n";



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

[PATCH] D74801: [ADT][NFC] SCCIterator: Change hasLoop() to hasCycle()

[Stefanos Baziotis via Phabricator via cfe-commits]

baziotis added a comment.

I committed that 
(https://reviews.llvm.org/rG21390eab4c05e0ed7e7d13ada9e85f62b87ea484) but as it 
seems, I should have added the differential division in the commit message. 
I'll try the next commit to be better.


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

https://reviews.llvm.org/D74801



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

[PATCH] D75153: [ThinLTO] Allow usage of all SMT threads in the system

[Mehdi AMINI via Phabricator via cfe-commits]

mehdi_amini added a comment.

> /opt:lldltojobs=N -- limit usage to N threads, but constrained by usage of 
> heavyweight_hardware_concurrency().

I really dislike this behavior: this seems user hostile to me. I would either:

- honor the user request (eventually display a warning), this is in line with 
other system behavior like `ninja -j N` for instance.
- reject the user request

If you want such a behavior, then it should be another flag which express it in 
the name like `opt:lldltomaxnumjobs`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75153



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

[PATCH] D75153: [ThinLTO] Allow usage of all SMT threads in the system

[Alexandre Ganea via Phabricator via cfe-commits]

aganea added a comment.

@mehdi_amini Agreed. In that case, I just need to slightly change this function 

 to ensure threads are equally dispatched on all CPU sockets regardless of the 
thread strategy.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75153



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

[PATCH] D68165: [analyzer][MallocChecker][NFC] Split checkPostCall up, deploy CallDescriptionMap

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus updated this revision to Diff 247512.
Szelethus added a reviewer: balazske.
Szelethus set the repository for this revision to rG LLVM Github Monorepo.
Szelethus added a comment.

Address inlines.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68165

Files:
  clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp

Index: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -61,6 +61,7 @@
 #include "clang/StaticAnalyzer/Core/PathSensitive/DynamicSize.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState_Fwd.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
@@ -92,8 +93,6 @@
   AF_InnerBuffer
 };
 
-struct MemFunctionInfoTy;
-
 } // end of anonymous namespace
 
 /// Print names of allocators and deallocators.
@@ -262,47 +261,6 @@
 
 REGISTER_MAP_WITH_PROGRAMSTATE(ReallocPairs, SymbolRef, ReallocPair)
 
-//===--===//
-// Kinds of memory operations, information about resource managing functions.
-//===--===//
-
-namespace {
-
-struct MemFunctionInfoTy {
-  /// The value of the MallocChecker:Optimistic is stored in this variable.
-  ///
-  /// In pessimistic mode, the checker assumes that it does not know which
-  /// functions might free the memory.
-  /// In optimistic mode, the checker assumes that all user-defined functions
-  /// which might free a pointer are annotated.
-  DefaultBool ShouldIncludeOwnershipAnnotatedFunctions;
-
-  CallDescription CD_alloca{{"alloca"}, 1}, CD_win_alloca{{"_alloca"}, 1},
-  CD_malloc{{"malloc"}, 1}, CD_BSD_malloc{{"malloc"}, 3},
-  CD_free{{"free"}, 1}, CD_realloc{{"realloc"}, 2},
-  CD_calloc{{"calloc"}, 2}, CD_valloc{{"valloc"}, 1},
-  CD_reallocf{{"reallocf"}, 2}, CD_strndup{{"strndup"}, 2},
-  CD_strdup{{"strdup"}, 1}, CD_win_strdup{{"_strdup"}, 1},
-  CD_kmalloc{{"kmalloc"}, 2}, CD_if_nameindex{{"if_nameindex"}, 1},
-  CD_if_freenameindex{{"if_freenameindex"}, 1}, CD_wcsdup{{"wcsdup"}, 1},
-  CD_win_wcsdup{{"_wcsdup"}, 1}, CD_kfree{{"kfree"}, 2},
-  CD_g_malloc{{"g_malloc"}, 1}, CD_g_malloc0{{"g_malloc0"}, 1},
-  CD_g_realloc{{"g_realloc"}, 2}, CD_g_try_malloc{{"g_try_malloc"}, 1},
-  CD_g_try_malloc0{{"g_try_malloc0"}, 1},
-  CD_g_try_realloc{{"g_try_realloc"}, 2}, CD_g_free{{"g_free"}, 1},
-  CD_g_memdup{{"g_memdup"}, 2}, CD_g_malloc_n{{"g_malloc_n"}, 2},
-  CD_g_malloc0_n{{"g_malloc0_n"}, 2}, CD_g_realloc_n{{"g_realloc_n"}, 3},
-  CD_g_try_malloc_n{{"g_try_malloc_n"}, 2},
-  CD_g_try_malloc0_n{{"g_try_malloc0_n"}, 2},
-  CD_g_try_realloc_n{{"g_try_realloc_n"}, 3};
-
-  bool isMemFunction(const CallEvent &Call) const;
-  bool isCMemFunction(const CallEvent &Call) const;
-  bool isCMemFreeFunction(const CallEvent &Call) const;
-  bool isCMemAllocFunction(const CallEvent &Call) const;
-};
-} // end of anonymous namespace
-
 /// Tells if the callee is one of the builtin new/delete operators, including
 /// placement operators and other standard overloads.
 static bool isStandardNewDelete(const FunctionDecl *FD);
@@ -326,7 +284,11 @@
  check::PreStmt, check::PostStmt,
  check::PostObjCMessage, check::Location, eval::Assume> {
 public:
-  MemFunctionInfoTy MemFunctionInfo;
+  /// In pessimistic mode, the checker assumes that it does not know which
+  /// functions might free the memory.
+  /// In optimistic mode, the checker assumes that all user-defined functions
+  /// which might free a pointer are annotated.
+  DefaultBool ShouldIncludeOwnershipAnnotatedFunctions;
 
   /// Many checkers are essentially built into this one, so enabling them will
   /// make MallocChecker perform additional modeling and reporting.
@@ -386,6 +348,74 @@
   mutable std::unique_ptr BT_OffsetFree[CK_NumCheckKinds];
   mutable std::unique_ptr BT_UseZerroAllocated[CK_NumCheckKinds];
 
+#define CHECK_FN(NAME) \
+  void NAME(CheckerContext &C, const CallExpr *CE, ProgramStateRef State) const;
+
+  template 
+  void checkRealloc(CheckerContext &C, const CallExpr *CE,
+ProgramStateRef State) const;
+
+  CHECK_FN(checkBasicAlloc)
+  CHECK_FN(checkKernelMalloc)
+  CHECK_FN(checkCalloc)
+  CHECK_FN(checkFree)
+  CHECK_FN(checkAlloca)
+  CHECK_FN(checkStrdup)
+  CHECK_FN(checkIfNameIndex)
+  CHECK_FN(checkIfFreeNameIndex)
+  CHECK_FN(checkCXXNewOrCXXDelete)
+  CHECK_FN(checkGMalloc0

[PATCH] D68165: [analyzer][MallocChecker][NFC] Split checkPostCall up, deploy CallDescriptionMap

[Kristóf Umann via Phabricator via cfe-commits]

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



Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:378-379
+
+  using CheckFn = void (MallocChecker::*)(CheckerContext &C, const CallExpr 
*CE,
+  ProgramStateRef State) const;
+

NoQ wrote:
> baloghadamsoftware wrote:
> > Szelethus wrote:
> > > Szelethus wrote:
> > > > NoQ wrote:
> > > > > Whenever i see a (`CE`, `State`) pair, it screams `CallEvent` to me. 
> > > > > That said, i'm worried that `State` in these callbacks isn't 
> > > > > necessarily equal to `C.getState()` (the latter, by the way, is 
> > > > > always equal to the `CallEvent`'s `.getState()` - that's a relief, 
> > > > > right?), so if you'll ever be in the mood to check that, that'd be 
> > > > > great :)
> > > > It should be always equal to it. I'll change it.
> > > Hmmm, I tried making this take a (`CallEvent`, `CheckerContext`), but it 
> > > bloated the code for little gain, since every handler function would 
> > > start with the retrieval of the state and the call expression. That said, 
> > > I could cascade these down to `FreeMemAux`, `MallocMemAux`, etc, to also 
> > > take this pair instead, but I'll be honest, I don't see point until 
> > > something actually breaks.
> > This is the standard way in the checkers: almost every handler function 
> > starts with the retrieval of the state from the `CheckerContext`. The only 
> > reason for an extra `State` parameter is that sometimes we create more 
> > states in the lower level functions but only add the final one to the 
> > `CheckerContext` as a new transition. Does something like this happen here?
> I agree with @baloghadamsoftware here. If you pass `State` explicitly, it 
> looks like an indication for the reader that this `State` may be different 
> from `C.getState()`. If in practice they're the same, i'd rather do 
> `C.getState()` in every method than confuse the reader.
> 
> Also do you remember what makes us query `CallExpr` so often? Given that 
> `CallEvent` includes `CallExpr`, we should be able to expose everything we 
> need as `CallEvent` methods. Say, we should be able to replace 
> `MallocMemAux(C, CE, CE->getArg(0), ...)` with `MallocMemAux(C, Call, 
> Call.getArgExpr(0), ...)` and only do `Call.getOriginExpr()` once when we 
> report a bug.
I'll upload followups where I'm addressing these issues -- to keep things 
simple, I decided against increasing this patch's scope. I won't commit until 
those are also accepted.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68165



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

[PATCH] D73052: [clang-tidy] RenamerClangTidy now renames dependent member expr when the member can be resolved

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73052



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

[PATCH] D74801: [ADT][NFC] SCCIterator: Change hasLoop() to hasCycle()

[Michael Kruse via Phabricator via cfe-commits]

Meinersbur closed this revision.
Meinersbur added a comment.

In D74801#189 , @baziotis wrote:

> I committed that 
> (https://reviews.llvm.org/rG21390eab4c05e0ed7e7d13ada9e85f62b87ea484) but as 
> it seems, I should have added the differential division in the commit 
> message. I'll try the next commit to be better.


If that happens, this Phabricator review does not close automatically. It has 
to be closed manually using the "Add Action..." dropdown.


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

https://reviews.llvm.org/D74801



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

[PATCH] D69573: [clang-format] [PR36294] AlwaysBreakAfterReturnType works incorrectly for some operator functions

[Sylvestre Ledru via Phabricator via cfe-commits]

sylvestre.ledru added subscribers: hans, sylvestre.ledru.
sylvestre.ledru added a comment.

@MyDeveloperDay @hans what about adding this to the release notes?
I was trying clang-format 10 on Firefox code base and I noticed this change 
which isn't documented in
https://prereleases.llvm.org/10.0.0/rc1/tools/clang/docs/ReleaseNotes.html#clang-format
(I can do it if you want)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69573



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

[PATCH] D67052: Add reference type transformation builtins

[Eric Fiselier via Phabricator via cfe-commits]

EricWF added a comment.

LGTM once all of the inline comments are addressed.

After that, this is ready to land.




Comment at: clang/lib/AST/ItaniumMangle.cpp:3412
 break;
+  case UnaryTransformType::RemoveReferenceType:
+Out << "3err";

zoecarver wrote:
> Eric, I looked at your comment, but it seems that we no longer unary type 
> transformations that way. @rsmith is this correct? 
@rsmith Can you double check the mangling changes?



Comment at: clang/lib/Sema/SemaType.cpp:1624
   }
+  case DeclSpec::TST_removeReferenceType:
+  case DeclSpec::TST_removeCV:

Why can't we share the implementation with `TST_underlyingType`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67052



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

[PATCH] D72911: clang-format: fix spacing in `operator const char*()`

[Sylvestre Ledru via Phabricator via cfe-commits]

sylvestre.ledru added subscribers: hans, sylvestre.ledru.
sylvestre.ledru added a comment.

@krasimir @MyDeveloperDay @hans Looks like it is a regression from 
https://reviews.llvm.org/D72911
and the fix isn't in 10.0rc2.
Should we take it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72911



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

[PATCH] D41223: [libc++] Fix PR35491 - std::array of zero-size doesn't work with non-default constructible types.

[Eric Fiselier via Phabricator via cfe-commits]

EricWF added a comment.

In D41223#1899394 , @jtbandes wrote:

> The lack of `_LIBCPP_CONSTEXPR_AFTER_CXX14` on the `array` 
> specialization's `begin()`, `end()`, and other methods seems to be a bug: 
> https://stackoverflow.com/questions/60462569/empty-stdarrayt-0-doesnt-have-constexpr-begin


That bug is llvm.org/PR40124.

There are good reasons why it hasn't been fixed yet.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D41223



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

[PATCH] D75410: [clang-format] Fixed BraceWrapping.AfterExternBlock

[MyDeveloperDay via Phabricator via cfe-commits]

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

This needs a full context diff, before we can even look at it properly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75410



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

[PATCH] D75429: [clangd] DefineOutline removes `override` specified from overridden methods.

[Nathan James via Phabricator via cfe-commits]

njames93 created this revision.
Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman, jkorous, 
MaskRay, ilya-biryukov.
Herald added a project: clang.
njames93 edited the summary of this revision.
njames93 added a project: clang-tools-extra.

The define out of line refactor tool previously would copy the override 
specifier into the out of line definition. 
This results in malformed code as the specifiers aren't allowed outside the 
class definition.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D75429

Files:
  clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
  clang-tools-extra/clangd/unittests/TweakTests.cpp


Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -2068,6 +2068,24 @@
   };)cpp",
   "Foo::Foo(int z) __attribute__((weak)) : bar(2){}\n",
   },
+  // Overridden Methods
+  {
+  R"cpp(
+struct A {
+  virtual void foo() = 0;
+};
+struct B : A {
+  void fo^o() override {}
+};)cpp",
+  R"cpp(
+struct A {
+  virtual void foo() = 0;
+};
+struct B : A {
+  void foo() override ;
+};)cpp",
+  "void B::foo()  {}\n",
+  },
   };
   for (const auto &Case : Cases) {
 SCOPED_TRACE(Case.Test);
Index: clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
@@ -16,6 +16,7 @@
 #include "SourceCode.h"
 #include "refactor/Tweak.h"
 #include "clang/AST/ASTTypeTraits.h"
+#include "clang/AST/Attr.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclBase.h"
 #include "clang/AST/DeclCXX.h"
@@ -211,6 +212,18 @@
 }
   }
 
+  // Removes the override specifier if it exists as it doesn't belong in the
+  // function out-of-line definition.
+  if (FD->hasAttr()) {
+const auto *Override = FD->getAttr();
+assert(Override);
+CharSourceRange DelRange =
+CharSourceRange::getTokenRange(Override->getLocation());
+if (auto Err =
+QualifierInsertions.add(tooling::Replacement(SM, DelRange, "")))
+  Errors = llvm::joinErrors(std::move(Errors), std::move(Err));
+  }
+
   if (Errors)
 return std::move(Errors);
   return getFunctionSourceAfterReplacements(FD, QualifierInsertions);


Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -2068,6 +2068,24 @@
   };)cpp",
   "Foo::Foo(int z) __attribute__((weak)) : bar(2){}\n",
   },
+  // Overridden Methods
+  {
+  R"cpp(
+struct A {
+  virtual void foo() = 0;
+};
+struct B : A {
+  void fo^o() override {}
+};)cpp",
+  R"cpp(
+struct A {
+  virtual void foo() = 0;
+};
+struct B : A {
+  void foo() override ;
+};)cpp",
+  "void B::foo()  {}\n",
+  },
   };
   for (const auto &Case : Cases) {
 SCOPED_TRACE(Case.Test);
Index: clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
@@ -16,6 +16,7 @@
 #include "SourceCode.h"
 #include "refactor/Tweak.h"
 #include "clang/AST/ASTTypeTraits.h"
+#include "clang/AST/Attr.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclBase.h"
 #include "clang/AST/DeclCXX.h"
@@ -211,6 +212,18 @@
 }
   }
 
+  // Removes the override specifier if it exists as it doesn't belong in the
+  // function out-of-line definition.
+  if (FD->hasAttr()) {
+const auto *Override = FD->getAttr();
+assert(Override);
+CharSourceRange DelRange =
+CharSourceRange::getTokenRange(Override->getLocation());
+if (auto Err =
+QualifierInsertions.add(tooling::Replacement(SM, DelRange, "")))
+  Errors = llvm::joinErrors(std::move(Errors), std::move(Err));
+  }
+
   if (Errors)
 return std::move(Errors);
   return getFunctionSourceAfterReplacements(FD, QualifierInsertions);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D75410: [clang-format] Fixed BraceWrapping.AfterExternBlock

[Marcus Johnson via Phabricator via cfe-commits]

MarcusJohnson91 updated this revision to Diff 247524.
MarcusJohnson91 added a comment.

Full Context Diff (I think?)


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

https://reviews.llvm.org/D75410

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -2462,14 +2462,18 @@
 
   Style.BraceWrapping.AfterExternBlock = true;
   Style.BraceWrapping.SplitEmptyRecord = false;
-  verifyFormat("extern \"C\"\n"
-   "{}",
-   Style);
-  verifyFormat("extern \"C\"\n"
-   "{\n"
+  verifyFormat("extern \"C\" {}", Style);
+  verifyFormat("extern \"C\" {\n"
"  int foo();\n"
"}",
Style);
+
+  Style.BraceWrapping.AfterExternBlock = false;
+  verifyFormat("extern \"C\" {}", Style);
+  verifyFormat("extern \"C\" {\n"
+   "int foo();\n"
+   "}",
+   Style);
 }
 
 TEST_F(FormatTest, FormatsInlineASM) {
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1086,7 +1086,6 @@
   nextToken();
   if (FormatTok->Tok.is(tok::l_brace)) {
 if (Style.BraceWrapping.AfterExternBlock) {
-  addUnwrappedLine();
   parseBlock(/*MustBeDeclaration=*/true);
 } else {
   parseBlock(/*MustBeDeclaration=*/true, /*AddLevel=*/false);
Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -977,6 +977,29 @@
 ///   } catch () {
 ///   }
 /// \endcode
+/// \code
+///  true:
+/// #ifdef __cplusplus
+/// extern "C" {
+/// #endif
+///
+/// void f(void);
+///
+/// #ifdef __cplusplus
+/// }
+/// #endif
+///
+/// false:
+/// #ifdef __cplusplus
+/// extern "C" {
+/// #endif
+///
+/// void f(void);
+///
+/// #ifdef __cplusplus
+/// }
+/// #endif
+/// \endcode
 bool BeforeCatch;
 /// Wrap before ``else``.
 /// \code
Index: clang/docs/ClangFormatStyleOptions.rst
===
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -935,6 +935,28 @@
   extern "C" {
   int foo();
   }
+  
+  true:
+  #ifdef __cplusplus
+  extern "C" {
+  #endif
+  
+  void f(void);
+  
+  #ifdef __cplusplus
+  }
+  #endif
+  
+  false:
+  #ifdef __cplusplus
+  extern "C" {
+  #endif
+  
+  void f(void);
+  
+  #ifdef __cplusplus
+  }
+  #endif
 
   * ``bool BeforeCatch`` Wrap before ``catch``.
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D75430: [analyzer][NFC] Introduce CXXDeallocatorCall, deploy it in MallocChecker

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus created this revision.
Szelethus added reviewers: balazske, martong, NoQ, xazax.hun, rnkovacs, 
dcoughlin, baloghadamsoftware.
Herald added subscribers: cfe-commits, steakhal, Charusso, gamesh411, dkrupp, 
donat.nagy, mikhail.ramalho, a.sidorin, szepet, whisperity.
Herald added a project: clang.
Szelethus added a parent revision: D68165: [analyzer][MallocChecker][NFC] Split 
checkPostCall up, deploy CallDescriptionMap.

This is my first ever patch touching `ExprEngine`, sorry if the code causes 
lasting damage to the reader.

One of the pain points in simplifying `MallocChecker`s interface by gradually 
changing to `CallEvent` is that a variety of C++ allocation and deallocation 
functionalities are modeled through `preStmt<...>` where `CallEvent` is 
unavailable, and a single one of these callbacks can prevent a mass parameter 
change.

This patch introduces a new `CallEvent`, `CXXDeallocatorCall`, which happens 
//after// `preStmt`, and can completely replace that callback as 
demonstrated. Note how you can retrieve this call through `preCall`, yet there 
is no `postCall` to pair it with -- the reason behind this is that neither does 
`preStmt` have a `postStmt` pair. But I have no 
clue why that is :^)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D75430

Files:
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
  clang/lib/StaticAnalyzer/Checkers/AnalysisOrderChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
  clang/test/Analysis/analyzer-config.c
  clang/test/Analysis/operator-delete-analysis-order.cpp

Index: clang/test/Analysis/operator-delete-analysis-order.cpp
===
--- /dev/null
+++ clang/test/Analysis/operator-delete-analysis-order.cpp
@@ -0,0 +1,67 @@
+// RUN: %clang_analyze_cc1 -std=c++11 -fblocks -verify %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=debug.AnalysisOrder \
+// RUN:   -analyzer-config debug.AnalysisOrder:PreStmtCXXDeleteExpr=true \
+// RUN:   -analyzer-config debug.AnalysisOrder:PostStmtCXXDeleteExpr=true \
+// RUN:   -analyzer-config debug.AnalysisOrder:PreCall=true \
+// RUN:   -analyzer-config debug.AnalysisOrder:PostCall=true \
+// RUN:   2>&1 | FileCheck %s
+
+// expected-no-diagnostics
+
+namespace fundamental_dealloc {
+void f() {
+  int *p = new int;
+  delete p;
+}
+} // namespace fundamental_dealloc
+
+namespace record_dealloc {
+struct S {
+  int x, y;
+};
+
+void f() {
+  S *s = new S;
+  delete s;
+}
+} // namespace record_dealloc
+
+namespace nontrivial_destructor {
+struct NontrivialDtor {
+  int x, y;
+
+  ~NontrivialDtor() {
+// Casually mine bitcoin.
+  }
+};
+
+void f() {
+  NontrivialDtor *s = new NontrivialDtor;
+  delete s;
+}
+} // namespace nontrivial_destructor
+
+// Mind that the results here are in reverse order compared how functions are
+// defined in this file.
+
+// CHECK:  PreCall (operator new)
+// CHECK-NEXT: PostCall (operator new)
+// CHECK-NEXT: PreCall (nontrivial_destructor::NontrivialDtor::NontrivialDtor)
+// CHECK-NEXT: PostCall (nontrivial_destructor::NontrivialDtor::NontrivialDtor)
+// CHECK-NEXT: PreCall (nontrivial_destructor::NontrivialDtor::~NontrivialDtor)
+// CHECK-NEXT: PostCall (nontrivial_destructor::NontrivialDtor::~NontrivialDtor)
+// CHECK-NEXT: PreStmt
+// CHECK-NEXT: PreCall (operator delete)
+
+// CHECK:  PreCall (operator new)
+// CHECK-NEXT: PostCall (operator new)
+// CHECK-NEXT: PreCall (record_dealloc::S::S)
+// CHECK-NEXT: PostCall (record_dealloc::S::S)
+// CHECK-NEXT: PreStmt
+// CHECK-NEXT: PreCall (operator delete)
+
+// CHECK:  PreCall (operator new)
+// CHECK-NEXT: PostCall (operator new)
+// CHECK-NEXT: PreStmt
+// CHECK-NEXT: PreCall (operator delete)
Index: clang/test/Analysis/analyzer-config.c
===
--- clang/test/Analysis/analyzer-config.c
+++ clang/test/Analysis/analyzer-config.c
@@ -36,17 +36,22 @@
 // CHECK-NEXT: deadcode.DeadStores:WarnForDeadNestedAssignments = true
 // CHECK-NEXT: debug.AnalysisOrder:* = false
 // CHECK-NEXT: debug.AnalysisOrder:Bind = false
+// CHECK-NEXT: debug.AnalysisOrder:EndAnalysis = false
 // CHECK-NEXT: debug.AnalysisOrder:EndFunction = false
 // CHECK-NEXT: debug.AnalysisOrder:LiveSymbols = false
 // CHECK-NEXT: debug.AnalysisOrder:NewAllocator = false
 // CHECK-NEXT: debug.AnalysisOrder:PointerEscape = false
 // CHECK-NEXT: debug.AnalysisOrder:PostCall = false
 // CHECK-NEXT: debug.AnalysisOrder:PostStmtArraySubscriptExpr = false
+// CHECK-NEXT: debug.AnalysisOrder:PostStmtCXXConstructExpr = false
+// CHECK-NEXT: debug.AnalysisOrder:PostStmtCXXDeleteExpr = false
 // CHECK-NEXT: debug.AnalysisOrder:PostStmtCXXNewExpr = false
 // CHECK-NEXT: debug.AnalysisOrder:PostStmtCastExpr = false
 // CHECK-NEXT: debug.AnalysisOrder:PostStmtOffsetOfExpr = f

[PATCH] D72874: [clangd] Add a textual fallback for go-to-definition

[Nathan Ridge via Phabricator via cfe-commits]

nridge updated this revision to Diff 247529.
nridge added a comment.

Address some review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72874

Files:
  clang-tools-extra/clangd/SourceCode.cpp
  clang-tools-extra/clangd/SourceCode.h
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/unittests/XRefsTests.cpp

Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -586,6 +586,52 @@
   }
 }
 
+TEST(LocateSymbol, Textual) {
+  const char *Tests[] = {
+  R"cpp(// Comment
+struct [[MyClass]] {};
+// Comment mentioning M^yClass
+  )cpp",
+  R"cpp(// String
+struct [[MyClass]] {};
+const char* s = "String literal mentioning M^yClass";
+  )cpp",
+  R"cpp(// Invalid code
+/*error-ok*/
+int [[myFunction]](int);
+int var = m^yFunction();
+  )cpp",
+  R"cpp(// Dependent type
+struct Foo {
+  void [[uniqueMethodName]]();
+};
+template 
+void f(T t) {
+  t->u^niqueMethodName();
+}
+  )cpp"};
+
+  for (const char *Test : Tests) {
+Annotations T(Test);
+llvm::Optional WantDecl;
+if (!T.ranges().empty())
+  WantDecl = T.range();
+
+auto TU = TestTU::withCode(T.code());
+
+auto AST = TU.build();
+auto Index = TU.index();
+auto Results = locateSymbolAt(AST, T.point(), Index.get());
+
+if (!WantDecl) {
+  EXPECT_THAT(Results, IsEmpty()) << Test;
+} else {
+  ASSERT_THAT(Results, ::testing::SizeIs(1)) << Test;
+  EXPECT_EQ(Results[0].PreferredDeclaration.range, *WantDecl) << Test;
+}
+  }
+}
+
 TEST(LocateSymbol, Ambiguous) {
   auto T = Annotations(R"cpp(
 struct Foo {
@@ -660,6 +706,27 @@
Sym("baz", T.range("StaticOverload2";
 }
 
+TEST(LocateSymbol, TextualAmbiguous) {
+  auto T = Annotations(R"cpp(
+struct Foo {
+  void $FooLoc[[uniqueMethodName]]();
+};
+struct Bar {
+  void $BarLoc[[uniqueMethodName]]();
+};
+template 
+void f(T t) {
+  t->u^niqueMethodName();
+}
+  )cpp");
+  auto TU = TestTU::withCode(T.code());
+  auto AST = TU.build();
+  auto Index = TU.index();
+  EXPECT_THAT(locateSymbolAt(AST, T.point(), Index.get()),
+  UnorderedElementsAre(Sym("uniqueMethodName", T.range("FooLoc")),
+   Sym("uniqueMethodName", T.range("BarLoc";
+}
+
 TEST(LocateSymbol, TemplateTypedefs) {
   auto T = Annotations(R"cpp(
 template  struct function {};
Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -10,9 +10,11 @@
 #include "CodeCompletionStrings.h"
 #include "FindSymbols.h"
 #include "FindTarget.h"
+#include "FuzzyMatch.h"
 #include "Logger.h"
 #include "ParsedAST.h"
 #include "Protocol.h"
+#include "Quality.h"
 #include "Selection.h"
 #include "SourceCode.h"
 #include "URI.h"
@@ -45,6 +47,7 @@
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/raw_ostream.h"
+#include 
 
 namespace clang {
 namespace clangd {
@@ -191,6 +194,116 @@
   return Result;
 }
 
+bool isLikelyToBeIdentifier(StringRef Word) {
+  // Word contains underscore
+  if (Word.contains('_')) {
+return true;
+  }
+  // Word contains capital letter other than at beginning
+  if (Word.substr(1).find_if([](char C) { return std::isupper(C); }) !=
+  StringRef::npos) {
+return true;
+  }
+  // FIXME: There are other signals we could listen for.
+  // Some of these require inspecting the surroundings of the word as well.
+  //   - mid-sentence Capitalization
+  //   - markup like quotes / backticks / brackets / "\p"
+  //   - word is used as an identifier in nearby token (very close if very
+  // short, anywhere in file if longer)
+  //   - word has a qualifier (foo::bar)
+  return false;
+}
+
+using ScoredLocatedSymbol = std::pair;
+struct ScoredSymbolGreater {
+  bool operator()(const ScoredLocatedSymbol &L, const ScoredLocatedSymbol &R) {
+if (L.first != R.first)
+  return L.first > R.first;
+return L.second.Name < R.second.Name; // Earlier name is better.
+  }
+};
+
+std::vector navigationFallback(ParsedAST &AST,
+  const SymbolIndex *Index,
+  Position Pos,
+  const std::string &MainFilePath) {
+  const auto &SM = AST.getSourceManager();
+  auto SourceRange = getWordAtPosition(Pos, SM, AST.getLangOpts());
+  auto QueryString = toSourceCode(SM, SourceRange);
+  if (!isLikely

[PATCH] D72874: [clangd] Add a textual fallback for go-to-definition

[Nathan Ridge via Phabricator via cfe-commits]

nridge added a comment.

I've addressed some of the review comments, with a view to getting something 
minimal we can land, and improve on in follow-up changes.

Mostly, I focused on the suggestions which reduce the number of results. I've 
left other suggestions which increase the number of results (e.g. handling 
non-indexed symbols) for follow-ups.

In D72874#1831977 , @sammccall wrote:

> - only trigger when there's *some* positive signal for the word.
>   - Markup like quotes/backticks/brackets/`\p`
>   - weird case like `lowerCamel`, `UpperCamel`, `CAPS`, `mid-sentence 
> Capitalization`, `under_scores`.
>   - use of the word as a token in nearby code (very close if very short, 
> anywhere in file if longer)
>   - (maybe you want to support `ns::Qualifiers`?)


I currently handle `lowerCamel`, `UpperCamel`, `CAPS`, and `under_scores`. I've 
left the others as follow-ups.

> - post-filter aggressively - only return exact name matches (I think 
> including case).

Done.

> - call `fuzzyFind` directly and set `ProximityPath`

Done.

> as well as the enclosing scopes from lexing. For extra strictness consider 
> AnyScope=false

I haven't done this yet, do you think it's important for an initial landing?

If so, could you mention what API you had in mind for determining "enclosing 
scopes from lexing"?

I had in mind using something like `SelectionTree` and collecting any 
`RecordDecl`s or `NamespaceDecl`s on the path from the common ancestor to the 
TU, but that's technically not "from lexing", so perhaps you have something 
else in mind.

> - if you get more than 3 results, and none from current file, maybe don't 
> return anything, as confidence is too low. Or try a stricter query...

I implemented this, but my testing shows this causes a lot of results for class 
names to be excluded. The reason appears to be that `fuzzyFind()` returns the 
class and each of its constructors as distinct results, so if a class has more 
than two constructors, we'll have more than 3 results (and typically the class 
is declared in a different file).

Should we try to handle this case specifically (collapse a class name and its 
construtors to a single result), or should we reconsider this filtering 
criterion? It's not exactly clear to me what sort of bad behaviour it's 
intended to weed out.

> - handle the most common case of non-indexable symbols (local symbols) by 
> running the query against the closest occurrence of the token in code.

I've left this as a follow-up.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72874



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

[PATCH] D75431: [analyzer][NFC] Merge checkNewAllocator's paramaters into CXXAllocatorCall

[Kristóf Umann via Phabricator via cfe-commits]

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

Party based on this thread: 
http://lists.llvm.org/pipermail/cfe-dev/2020-February/064754.html.

This patch merges two of `CXXAllocatorCall`'s parameters, so that we are able 
to supply a `CallEvent` object to `check::NewAllocatorCall` (see the 
description of D75430  to see why this would 
be great).

One of the things mentioned by @NoQ was the following:

> I think at this point we might actually do a good job sorting out this 
> `check::NewAllocator` issue because we have a "separate" "Environment" to 
> hold the other `SVal`, which is "objects under construction"! - so we should 
> probably simply teach `CXXAllocatorCall` to extract the value from the 
> objects-under-construction trait of the program state and we're good.

I had `MallocChecker` in my crosshair for now, so I admittedly threw together 
something as a proof of concept. Now that I know that this effort is worth 
pursuing though, I'll happily look for a solution better then demonstrated in 
this patch.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D75431

Files:
  clang/include/clang/StaticAnalyzer/Core/Checker.h
  clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
  clang/lib/StaticAnalyzer/Checkers/AnalysisOrderChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
  clang/lib/StaticAnalyzer/Core/CheckerManager.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp

Index: clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
===
--- clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
+++ clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
@@ -10,17 +10,18 @@
 //
 //===--===//
 
-#include "clang/AST/Decl.h"
-#include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
 #include "PrettyStackTraceLocationContext.h"
 #include "clang/AST/CXXInheritance.h"
+#include "clang/AST/Decl.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/Analysis/Analyses/LiveVariables.h"
 #include "clang/Analysis/ConstructionContext.h"
 #include "clang/StaticAnalyzer/Core/CheckerManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
 #include "llvm/ADT/SmallSet.h"
 #include "llvm/ADT/Statistic.h"
+#include "llvm/Support/Casting.h"
 #include "llvm/Support/SaveAndRestore.h"
 
 using namespace clang;
@@ -324,17 +325,14 @@
 CallEventRef<> UpdatedCall = Call.cloneWithState(CEEState);
 
 ExplodedNodeSet DstPostCall;
-if (const CXXNewExpr *CNE = dyn_cast_or_null(CE)) {
+if (llvm::isa_and_nonnull(CE)) {
   ExplodedNodeSet DstPostPostCallCallback;
   getCheckerManager().runCheckersForPostCall(DstPostPostCallCallback,
  CEENode, *UpdatedCall, *this,
  /*wasInlined=*/true);
-  for (auto I : DstPostPostCallCallback) {
+  for (ExplodedNode *I : DstPostPostCallCallback) {
 getCheckerManager().runCheckersForNewAllocator(
-CNE,
-*getObjectUnderConstruction(I->getState(), CNE,
-calleeCtx->getParent()),
-DstPostCall, I, *this,
+cast(*UpdatedCall), DstPostCall, I, *this,
 /*wasInlined=*/true);
   }
 } else {
@@ -590,7 +588,7 @@
   // If there were other constructors called for object-type arguments
   // of this call, clean them up.
   ExplodedNodeSet dstArgumentCleanup;
-  for (auto I : dstCallEvaluated)
+  for (ExplodedNode *I : dstCallEvaluated)
 finishArgumentConstruction(dstArgumentCleanup, I, Call);
 
   ExplodedNodeSet dstPostCall;
@@ -604,7 +602,7 @@
 
   // Run pointerEscape callback with the newly conjured symbols.
   SmallVector, 8> Escaped;
-  for (auto I : dstPostCall) {
+  for (ExplodedNode *I : dstPostCall) {
 NodeBuilder B(I, Dst, *currBldrCtx);
 ProgramStateRef State = I->getState();
 Escaped.clear();
@@ -742,7 +740,7 @@
 const ConstructionContext *CC = CCE ? CCE->getConstructionContext()
 : nullptr;
 
-if (CC && isa(CC) &&
+if (llvm::isa_and_nonnull(CC) &&
 !Opts.MayInlineCXXAllocator)
   return CIP_DisallowedOnce;
 
Index: clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
===
--- clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ clang/lib/StaticAnalyzer

[PATCH] D75429: [clangd] DefineOutline removes `override` specified from overridden methods.

[Nathan James via Phabricator via cfe-commits]

njames93 updated this revision to Diff 247533.
njames93 added a comment.

- Extend to add virtual and final support


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75429

Files:
  clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
  clang-tools-extra/clangd/unittests/TweakTests.cpp

Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -2068,6 +2068,52 @@
   };)cpp",
   "Foo::Foo(int z) __attribute__((weak)) : bar(2){}\n",
   },
+  // Overridden Methods
+  {
+  R"cpp(
+struct A {
+  virtual void f^oo() {}
+};)cpp",
+  R"cpp(
+struct A {
+  virtual void foo() ;
+};)cpp",
+  " void A::foo() {}\n",
+  },
+  {
+  R"cpp(
+struct A {
+  virtual void foo() = 0;
+};
+struct B : A {
+  void fo^o() override {}
+};)cpp",
+  R"cpp(
+struct A {
+  virtual void foo() = 0;
+};
+struct B : A {
+  void foo() override ;
+};)cpp",
+  "void B::foo()  {}\n",
+  },
+  {
+  R"cpp(
+struct A {
+  virtual void foo() = 0;
+};
+struct B : A {
+  void fo^o() final {}
+};)cpp",
+  R"cpp(
+struct A {
+  virtual void foo() = 0;
+};
+struct B : A {
+  void foo() final ;
+};)cpp",
+  "void B::foo()  {}\n",
+  },
   };
   for (const auto &Case : Cases) {
 SCOPED_TRACE(Case.Test);
Index: clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
@@ -16,6 +16,7 @@
 #include "SourceCode.h"
 #include "refactor/Tweak.h"
 #include "clang/AST/ASTTypeTraits.h"
+#include "clang/AST/Attr.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclBase.h"
 #include "clang/AST/DeclCXX.h"
@@ -211,6 +212,34 @@
 }
   }
 
+  // Remove the virtual, override and final specifiers.
+  for (auto *Attr : FD->getAttrs()) {
+if (isa(Attr) || isa(Attr)) {
+  CharSourceRange DelRange =
+  CharSourceRange::getTokenRange(Attr->getLocation());
+  if (auto Err =
+  QualifierInsertions.add(tooling::Replacement(SM, DelRange, "")))
+Errors = llvm::joinErrors(std::move(Errors), std::move(Err));
+}
+  }
+  if (FD->isVirtualAsWritten()) {
+SourceRange SpecRange{FD->getBeginLoc(), FD->getLocation()};
+bool Any = false;
+// Clang allows duplicating virtual specifiers so check for multiple
+// occurances.
+for (const syntax::Token &Tok : TokBuf.expandedTokens(SpecRange)) {
+  if (Tok.kind() == tok::kw_virtual) {
+Any = true;
+CharSourceRange DelRange =
+CharSourceRange::getTokenRange(Tok.location());
+if (auto Err =
+QualifierInsertions.add(tooling::Replacement(SM, DelRange, "")))
+  Errors = llvm::joinErrors(std::move(Errors), std::move(Err));
+  }
+}
+assert(Any); // Ensure at least one virtual was found
+  }
+
   if (Errors)
 return std::move(Errors);
   return getFunctionSourceAfterReplacements(FD, QualifierInsertions);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D74801: [ADT][NFC] SCCIterator: Change hasLoop() to hasCycle()

[Stefanos Baziotis via Phabricator via cfe-commits]

baziotis added a comment.

> If that happens, this Phabricator review does not close automatically. It has 
> to be closed manually using the "Add Action..." dropdown.

Noted.


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

https://reviews.llvm.org/D74801



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

[PATCH] D75432: [analyzer][NFC][MallocChecker] Convert many parameters into CallEvent

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus created this revision.
Szelethus added reviewers: NoQ, xazax.hun, baloghadamsoftware, dcoughlin, 
rnkovacs, balazske, martong.
Szelethus added a project: clang.
Herald added subscribers: cfe-commits, steakhal, Charusso, gamesh411, dkrupp, 
donat.nagy, mikhail.ramalho, a.sidorin, szepet, whisperity.
Szelethus added a parent revision: D75431: [analyzer][NFC] Merge 
checkNewAllocator's paramaters into CXXAllocatorCall.

Exactly what it says on the tin! This is clearly not the end of the road in 
this direction, the parameters could be merged far more with the use of 
`CallEvent` or a better value type in the `CallDescriptionMap`, but this was 
shockingly difficult enough on its own. I expect that simplifying the file 
further will be far easier moving forward.

The end goal is to research how we could create a more mature checker 
interaction infrastructure for more complicated C++ modeling, and I'm pretty 
sure that being able successfully split up our giants is the first step in this 
direction.

Also... as to why I added so much `LLVM_UNREACHABLE` annotations, I'll provide 
the following image:

F11463946: image.png 

One could explain why this restroom looks like this -- but you could guess what 
the horrifying story must have been :^)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D75432

Files:
  clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp

Index: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -47,7 +47,10 @@
 #include "AllocationState.h"
 #include "InterCheckerAPI.h"
 #include "clang/AST/Attr.h"
+#include "clang/AST/Expr.h"
+#include "clang/AST/ExprCXX.h"
 #include "clang/AST/ParentMap.h"
+#include "clang/Basic/LLVM.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/TargetInfo.h"
 #include "clang/Lex/Lexer.h"
@@ -62,10 +65,12 @@
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState_Fwd.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/SVals.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringExtras.h"
+#include "llvm/Support/Compiler.h"
 #include "llvm/Support/ErrorHandling.h"
 #include 
 #include 
@@ -265,7 +270,7 @@
 /// placement operators and other standard overloads.
 static bool isStandardNewDelete(const FunctionDecl *FD);
 static bool isStandardNewDelete(const CallEvent &Call) {
-  if (!Call.getDecl())
+  if (!Call.getDecl() || !isa(Call.getDecl()))
 return false;
   return isStandardNewDelete(cast(Call.getDecl()));
 }
@@ -280,9 +285,8 @@
 : public Checker,
  check::EndFunction, check::PreCall, check::PostCall,
- check::PostStmt, check::NewAllocator,
- check::PostStmt, check::PostObjCMessage,
- check::Location, eval::Assume> {
+ check::NewAllocator, check::PostStmt,
+ check::PostObjCMessage, check::Location, eval::Assume> {
 public:
   /// In pessimistic mode, the checker assumes that it does not know which
   /// functions might free the memory.
@@ -311,7 +315,6 @@
 
   void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
   void checkPostCall(const CallEvent &Call, CheckerContext &C) const;
-  void checkPostStmt(const CXXNewExpr *NE, CheckerContext &C) const;
   void checkNewAllocator(const CXXAllocatorCall &Call, CheckerContext &C) const;
   void checkPostObjCMessage(const ObjCMethodCall &Call, CheckerContext &C) const;
   void checkPostStmt(const BlockExpr *BE, CheckerContext &C) const;
@@ -347,11 +350,10 @@
   mutable std::unique_ptr BT_UseZerroAllocated[CK_NumCheckKinds];
 
 #define CHECK_FN(NAME) \
-  void NAME(CheckerContext &C, const CallExpr *CE, ProgramStateRef State) const;
+  void NAME(const CallEvent &Call, CheckerContext &C) const;
 
   template 
-  void checkRealloc(CheckerContext &C, const CallExpr *CE,
-ProgramStateRef State) const;
+  void checkRealloc(const CallEvent &Call, CheckerContext &C) const;
 
   CHECK_FN(checkBasicAlloc)
   CHECK_FN(checkKernelMalloc)
@@ -369,8 +371,8 @@
   CHECK_FN(checkReallocN)
   CHECK_FN(checkOwnershipAttr)
 
-  using CheckFn = void (MallocChecker::*)(CheckerContext &C, const CallExpr *CE,
-  ProgramStateRef State) const;
+  using CheckFn = void (MallocChecker::*)(const CallEvent &Call,
+  CheckerContext &C) const;
 
   CallDescriptionMap FreeingMemFnMap{
   {{"realloc", 2}, &MallocChecker::checkRealloc},
@@ -419,8 +4

[PATCH] D75429: [clangd] DefineOutline removes `override` specified from overridden methods.

[Nathan James via Phabricator via cfe-commits]

njames93 updated this revision to Diff 247535.
njames93 added a comment.

- Made the replacements macro safe


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75429

Files:
  clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
  clang-tools-extra/clangd/unittests/TweakTests.cpp

Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -2068,6 +2068,52 @@
   };)cpp",
   "Foo::Foo(int z) __attribute__((weak)) : bar(2){}\n",
   },
+  // Overridden Methods
+  {
+  R"cpp(
+struct A {
+  virtual void f^oo() {}
+};)cpp",
+  R"cpp(
+struct A {
+  virtual void foo() ;
+};)cpp",
+  " void A::foo() {}\n",
+  },
+  {
+  R"cpp(
+struct A {
+  virtual void foo() = 0;
+};
+struct B : A {
+  void fo^o() override {}
+};)cpp",
+  R"cpp(
+struct A {
+  virtual void foo() = 0;
+};
+struct B : A {
+  void foo() override ;
+};)cpp",
+  "void B::foo()  {}\n",
+  },
+  {
+  R"cpp(
+struct A {
+  virtual void foo() = 0;
+};
+struct B : A {
+  void fo^o() final {}
+};)cpp",
+  R"cpp(
+struct A {
+  virtual void foo() = 0;
+};
+struct B : A {
+  void foo() final ;
+};)cpp",
+  "void B::foo()  {}\n",
+  },
   };
   for (const auto &Case : Cases) {
 SCOPED_TRACE(Case.Test);
Index: clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
@@ -16,6 +16,7 @@
 #include "SourceCode.h"
 #include "refactor/Tweak.h"
 #include "clang/AST/ASTTypeTraits.h"
+#include "clang/AST/Attr.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclBase.h"
 #include "clang/AST/DeclCXX.h"
@@ -211,6 +212,57 @@
 }
   }
 
+  // Remove the virtual, override and final specifiers.
+  for (auto *Attr : FD->getAttrs()) {
+if (isa(Attr) || isa(Attr)) {
+  assert(Attr->getLocation().isValid());
+  if (Attr->getLocation().isMacroID()) {
+Errors = llvm::joinErrors(
+std::move(Errors),
+llvm::createStringError(
+llvm::inconvertibleErrorCode(),
+isa(Attr)
+? "define outline: Can't move out of line as function has "
+  "a macro override specifier"
+: "define outline: Can't move out of line as function has "
+  "a macro final specifier"));
+continue;
+  }
+  CharSourceRange DelRange =
+  CharSourceRange::getTokenRange(Attr->getLocation());
+  if (auto Err =
+  QualifierInsertions.add(tooling::Replacement(SM, DelRange, "")))
+Errors = llvm::joinErrors(std::move(Errors), std::move(Err));
+}
+  }
+  if (FD->isVirtualAsWritten()) {
+SourceRange SpecRange{FD->getBeginLoc(), FD->getLocation()};
+bool Any = false;
+// Clang allows duplicating virtual specifiers so check for multiple
+// occurances.
+for (const syntax::Token &Tok : TokBuf.expandedTokens(SpecRange)) {
+  if (Tok.kind() == tok::kw_virtual) {
+assert(Tok.location().isValid());
+if (Tok.location().isMacroID()) {
+  Errors =
+  llvm::joinErrors(std::move(Errors),
+   llvm::createStringError(
+   llvm::inconvertibleErrorCode(),
+   "define outline: Can't move out of line as "
+   "function has a macro virtual specifier"));
+  continue;
+}
+Any = true;
+CharSourceRange DelRange =
+CharSourceRange::getTokenRange(Tok.location());
+if (auto Err =
+QualifierInsertions.add(tooling::Replacement(SM, DelRange, "")))
+  Errors = llvm::joinErrors(std::move(Errors), std::move(Err));
+  }
+}
+assert(Any); // Ensure at least one virtual was found
+  }
+
   if (Errors)
 return std::move(Errors);
   return getFunctionSourceAfterReplacements(FD, QualifierInsertions);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D75429: [clangd] DefineOutline removes `override` specified from overridden methods.

[Nathan James via Phabricator via cfe-commits]

njames93 updated this revision to Diff 247538.
njames93 added a comment.

- Fix assertion when no Attrs are present


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75429

Files:
  clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
  clang-tools-extra/clangd/unittests/TweakTests.cpp

Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -2068,6 +2068,52 @@
   };)cpp",
   "Foo::Foo(int z) __attribute__((weak)) : bar(2){}\n",
   },
+  // Overridden Methods
+  {
+  R"cpp(
+struct A {
+  virtual void f^oo() {}
+};)cpp",
+  R"cpp(
+struct A {
+  virtual void foo() ;
+};)cpp",
+  " void A::foo() {}\n",
+  },
+  {
+  R"cpp(
+struct A {
+  virtual void foo() = 0;
+};
+struct B : A {
+  void fo^o() override {}
+};)cpp",
+  R"cpp(
+struct A {
+  virtual void foo() = 0;
+};
+struct B : A {
+  void foo() override ;
+};)cpp",
+  "void B::foo()  {}\n",
+  },
+  {
+  R"cpp(
+struct A {
+  virtual void foo() = 0;
+};
+struct B : A {
+  void fo^o() final {}
+};)cpp",
+  R"cpp(
+struct A {
+  virtual void foo() = 0;
+};
+struct B : A {
+  void foo() final ;
+};)cpp",
+  "void B::foo()  {}\n",
+  },
   };
   for (const auto &Case : Cases) {
 SCOPED_TRACE(Case.Test);
Index: clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
@@ -16,6 +16,7 @@
 #include "SourceCode.h"
 #include "refactor/Tweak.h"
 #include "clang/AST/ASTTypeTraits.h"
+#include "clang/AST/Attr.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclBase.h"
 #include "clang/AST/DeclCXX.h"
@@ -211,6 +212,60 @@
 }
   }
 
+  // Remove the virtual, override and final specifiers.
+  if (FD->hasAttrs()) {
+for (auto *Attr : FD->getAttrs()) {
+  if (isa(Attr) || isa(Attr)) {
+assert(Attr->getLocation().isValid());
+if (Attr->getLocation().isMacroID()) {
+  Errors = llvm::joinErrors(
+  std::move(Errors),
+  llvm::createStringError(llvm::inconvertibleErrorCode(),
+  isa(Attr)
+  ? "define outline: Can't move out of "
+"line as function has "
+"a macro `override` specifier"
+  : "define outline: Can't move out of "
+"line as function has "
+"a macro `final` specifier"));
+  continue;
+}
+CharSourceRange DelRange =
+CharSourceRange::getTokenRange(Attr->getLocation());
+if (auto Err =
+QualifierInsertions.add(tooling::Replacement(SM, DelRange, "")))
+  Errors = llvm::joinErrors(std::move(Errors), std::move(Err));
+  }
+}
+  }
+  if (FD->isVirtualAsWritten()) {
+SourceRange SpecRange{FD->getBeginLoc(), FD->getLocation()};
+bool Any = false;
+// Clang allows duplicating virtual specifiers so check for multiple
+// occurances.
+for (const syntax::Token &Tok : TokBuf.expandedTokens(SpecRange)) {
+  if (Tok.kind() == tok::kw_virtual) {
+assert(Tok.location().isValid());
+if (Tok.location().isMacroID()) {
+  Errors =
+  llvm::joinErrors(std::move(Errors),
+   llvm::createStringError(
+   llvm::inconvertibleErrorCode(),
+   "define outline: Can't move out of line as "
+   "function has a macro `virtual` specifier"));
+  continue;
+}
+Any = true;
+CharSourceRange DelRange =
+CharSourceRange::getTokenRange(Tok.location());
+if (auto Err =
+QualifierInsertions.add(tooling::Replacement(SM, DelRange, "")))
+  Errors = llvm::joinErrors(std::move(Errors), std::move(Err));
+  }
+}
+assert(Any); // Ensure at least one virtual was found
+  }
+
   if (Errors)
 return std::move(Errors);
   return getFunctionSourceAfterReplacements(FD, QualifierInsertions);

[PATCH] D75323: Support relative dest paths in headermap files

[Simon Que via Phabricator via cfe-commits]

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



Comment at: clang/test/Headers/headermap_relpath.cpp:8
+
+// RUN: cd %S; %clang_cc1 -I %S/Inputs/empty.hmap %s
+

takuto.ikuta wrote:
> generate header map using hmaptool like
> https://github.com/llvm/llvm-project/blob/e32ff096858578f526b6d05ab97c8f083f2e1834/clang/test/Preprocessor/include-header-missing-in-framework-with-headermap.c#L2
> ?
OK. Should this test also be moved to under clang/test/Preprocessor?


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

https://reviews.llvm.org/D75323



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

[PATCH] D75323: Support relative dest paths in headermap files

[Simon Que via Phabricator via cfe-commits]

sque updated this revision to Diff 247540.
sque marked an inline comment as done.
sque added a comment.

Changed to generate hmap file using hmaptool.


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

https://reviews.llvm.org/D75323

Files:
  clang/lib/Lex/HeaderSearch.cpp
  clang/test/Headers/Inputs/headermap_relpath/empty.hmap.json
  clang/test/Headers/Inputs/include/empty.h
  clang/test/Headers/headermap_relpath.cpp


Index: clang/test/Headers/headermap_relpath.cpp
===
--- /dev/null
+++ clang/test/Headers/headermap_relpath.cpp
@@ -0,0 +1,13 @@
+// Test relative include paths in headermap files
+// == 
//
+
+// Inputs/empty.hmap contains: "empty/empty.h" -> "Inputs/include/empty.h"
+// Must run in the same directory as this file, so that the relative path can 
be
+// found.
+
+// RUN: rm -f %t.hmap
+// RUN: %hmaptool write %S/Inputs/headermap_relpath/empty.hmap.json %t.hmap
+// RUN: %clang_cc1 -fsyntax-only -I %t.hmap -I %S/Inputs %s
+
+#include 
+int main() { return 0; }
Index: clang/test/Headers/Inputs/include/empty.h
===
--- /dev/null
+++ clang/test/Headers/Inputs/include/empty.h
@@ -0,0 +1 @@
+
Index: clang/test/Headers/Inputs/headermap_relpath/empty.hmap.json
===
--- /dev/null
+++ clang/test/Headers/Inputs/headermap_relpath/empty.hmap.json
@@ -0,0 +1 @@
+{"mappings":{"empty/empty.h":"include/empty.h"}}
Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -415,7 +415,9 @@
   FixupSearchPath();
   return *Result;
 }
-  } else if (auto Res = HS.getFileMgr().getOptionalFileRef(Dest)) {
+// If lookup failed, fall back to file lookup using relative path directly.
+  }
+  if (auto Res = HS.getFileMgr().getOptionalFileRef(Dest)) {
 FixupSearchPath();
 return *Res;
   }


Index: clang/test/Headers/headermap_relpath.cpp
===
--- /dev/null
+++ clang/test/Headers/headermap_relpath.cpp
@@ -0,0 +1,13 @@
+// Test relative include paths in headermap files
+// == //
+
+// Inputs/empty.hmap contains: "empty/empty.h" -> "Inputs/include/empty.h"
+// Must run in the same directory as this file, so that the relative path can be
+// found.
+
+// RUN: rm -f %t.hmap
+// RUN: %hmaptool write %S/Inputs/headermap_relpath/empty.hmap.json %t.hmap
+// RUN: %clang_cc1 -fsyntax-only -I %t.hmap -I %S/Inputs %s
+
+#include 
+int main() { return 0; }
Index: clang/test/Headers/Inputs/include/empty.h
===
--- /dev/null
+++ clang/test/Headers/Inputs/include/empty.h
@@ -0,0 +1 @@
+
Index: clang/test/Headers/Inputs/headermap_relpath/empty.hmap.json
===
--- /dev/null
+++ clang/test/Headers/Inputs/headermap_relpath/empty.hmap.json
@@ -0,0 +1 @@
+{"mappings":{"empty/empty.h":"include/empty.h"}}
Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -415,7 +415,9 @@
   FixupSearchPath();
   return *Result;
 }
-  } else if (auto Res = HS.getFileMgr().getOptionalFileRef(Dest)) {
+// If lookup failed, fall back to file lookup using relative path directly.
+  }
+  if (auto Res = HS.getFileMgr().getOptionalFileRef(Dest)) {
 FixupSearchPath();
 return *Res;
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D75373: [Clang] Fix Hurd toolchain class hierarchy

[Alexandre Ganea via Phabricator via cfe-commits]

aganea updated this revision to Diff 247542.
aganea edited the summary of this revision.
aganea added a comment.

The patch did not make sense conceptually. Hurd is not Linux. I think now it 
makes more sense.


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

https://reviews.llvm.org/D75373

Files:
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/lib/Driver/ToolChains/Gnu.h
  clang/lib/Driver/ToolChains/Hurd.cpp
  clang/lib/Driver/ToolChains/Hurd.h
  clang/lib/Driver/ToolChains/Linux.cpp
  clang/lib/Driver/ToolChains/Linux.h

Index: clang/lib/Driver/ToolChains/Linux.h
===
--- clang/lib/Driver/ToolChains/Linux.h
+++ clang/lib/Driver/ToolChains/Linux.h
@@ -42,7 +42,9 @@
 llvm::opt::ArgStringList &CmdArgs) const override;
   virtual std::string computeSysRoot() const;
 
-  virtual std::string getDynamicLinker(const llvm::opt::ArgList &Args) const;
+  std::string getDynamicLinker(const llvm::opt::ArgList &Args) const override;
+
+  void addExtraOpts(llvm::opt::ArgStringList &CmdArgs) const override;
 
   std::vector ExtraOpts;
 
Index: clang/lib/Driver/ToolChains/Linux.cpp
===
--- clang/lib/Driver/ToolChains/Linux.cpp
+++ clang/lib/Driver/ToolChains/Linux.cpp
@@ -986,3 +986,8 @@
 Twine("-u", llvm::getInstrProfRuntimeHookVarName(;
   ToolChain::addProfileRTLibs(Args, CmdArgs);
 }
+
+void Linux::addExtraOpts(llvm::opt::ArgStringList &CmdArgs) const {
+  for (const auto &Opt : ExtraOpts)
+CmdArgs.push_back(Opt.c_str());
+}
\ No newline at end of file
Index: clang/lib/Driver/ToolChains/Hurd.h
===
--- clang/lib/Driver/ToolChains/Hurd.h
+++ clang/lib/Driver/ToolChains/Hurd.h
@@ -27,9 +27,9 @@
   AddClangSystemIncludeArgs(const llvm::opt::ArgList &DriverArgs,
 llvm::opt::ArgStringList &CC1Args) const override;
 
-  virtual std::string computeSysRoot() const;
+  std::string computeSysRoot() const;
 
-  virtual std::string getDynamicLinker(const llvm::opt::ArgList &Args) const;
+  std::string getDynamicLinker(const llvm::opt::ArgList &Args) const override;
 
   std::vector ExtraOpts;
 
Index: clang/lib/Driver/ToolChains/Hurd.cpp
===
--- clang/lib/Driver/ToolChains/Hurd.cpp
+++ clang/lib/Driver/ToolChains/Hurd.cpp
@@ -61,8 +61,7 @@
   return Triple.isArch32Bit() ? "lib" : "lib64";
 }
 
-Hurd::Hurd(const Driver &D, const llvm::Triple &Triple,
-   const ArgList &Args)
+Hurd::Hurd(const Driver &D, const llvm::Triple &Triple, const ArgList &Args)
 : Generic_ELF(D, Triple, Args) {
   std::string SysRoot = computeSysRoot();
   path_list &Paths = getFilePaths();
Index: clang/lib/Driver/ToolChains/Gnu.h
===
--- clang/lib/Driver/ToolChains/Gnu.h
+++ clang/lib/Driver/ToolChains/Gnu.h
@@ -356,6 +356,12 @@
   void addClangTargetOptions(const llvm::opt::ArgList &DriverArgs,
  llvm::opt::ArgStringList &CC1Args,
  Action::OffloadKind DeviceOffloadKind) const override;
+
+  virtual std::string getDynamicLinker(const llvm::opt::ArgList &Args) const {
+return {};
+  }
+
+  virtual void addExtraOpts(llvm::opt::ArgStringList &CmdArgs) const {}
 };
 
 } // end namespace toolchains
Index: clang/lib/Driver/ToolChains/Gnu.cpp
===
--- clang/lib/Driver/ToolChains/Gnu.cpp
+++ clang/lib/Driver/ToolChains/Gnu.cpp
@@ -309,7 +309,7 @@
   }
 }
 
-static bool getPIE(const ArgList &Args, const toolchains::Linux &ToolChain) {
+static bool getPIE(const ArgList &Args, const ToolChain &TC) {
   if (Args.hasArg(options::OPT_shared) || Args.hasArg(options::OPT_static) ||
   Args.hasArg(options::OPT_r) || Args.hasArg(options::OPT_static_pie))
 return false;
@@ -317,17 +317,16 @@
   Arg *A = Args.getLastArg(options::OPT_pie, options::OPT_no_pie,
options::OPT_nopie);
   if (!A)
-return ToolChain.isPIEDefault();
+return TC.isPIEDefault();
   return A->getOption().matches(options::OPT_pie);
 }
 
-static bool getStaticPIE(const ArgList &Args,
- const toolchains::Linux &ToolChain) {
+static bool getStaticPIE(const ArgList &Args, const ToolChain &TC) {
   bool HasStaticPIE = Args.hasArg(options::OPT_static_pie);
   // -no-pie is an alias for -nopie. So, handling -nopie takes care of
   // -no-pie as well.
   if (HasStaticPIE && Args.hasArg(options::OPT_nopie)) {
-const Driver &D = ToolChain.getDriver();
+const Driver &D = TC.getDriver();
 const llvm::opt::OptTable &Opts = D.getOpts();
 const char *StaticPIEName = Opts.getOptionName(options::OPT_static_pie);
 const char *NoPIEName = Opts.getOptionName(options::OPT_nopie);
@@ -346,8 +345,8 @@
   

[PATCH] D73462: [dwarf-5] Support DebugInfo for Defaulted parameters for C++ templates

[Awanish Pandey via Phabricator via cfe-commits]

awpandey updated this revision to Diff 247546.

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

https://reviews.llvm.org/D73462

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/test/CodeGenCXX/debug-info-template-parameter.cpp
  llvm/include/llvm/IR/DIBuilder.h
  llvm/include/llvm/IR/DebugInfoMetadata.h
  llvm/lib/AsmParser/LLParser.cpp
  llvm/lib/Bitcode/Reader/MetadataLoader.cpp
  llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
  llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
  llvm/lib/IR/AsmWriter.cpp
  llvm/lib/IR/DIBuilder.cpp
  llvm/lib/IR/DebugInfoMetadata.cpp
  llvm/lib/IR/LLVMContextImpl.h
  llvm/test/Assembler/DITemplateParameter.ll
  llvm/test/Bitcode/DITemplateParameter-5.0.ll
  llvm/test/Bitcode/DITemplateParameter-5.0.ll.bc
  llvm/test/DebugInfo/X86/debug-info-template-parameter.ll
  llvm/unittests/IR/MetadataTest.cpp

Index: llvm/unittests/IR/MetadataTest.cpp
===
--- llvm/unittests/IR/MetadataTest.cpp
+++ llvm/unittests/IR/MetadataTest.cpp
@@ -2076,17 +2076,19 @@
 TEST_F(DITemplateTypeParameterTest, get) {
   StringRef Name = "template";
   DIType *Type = getBasicType("basic");
+  bool defaulted = false;
 
-  auto *N = DITemplateTypeParameter::get(Context, Name, Type);
+  auto *N = DITemplateTypeParameter::get(Context, Name, Type, defaulted);
 
   EXPECT_EQ(dwarf::DW_TAG_template_type_parameter, N->getTag());
   EXPECT_EQ(Name, N->getName());
   EXPECT_EQ(Type, N->getType());
-  EXPECT_EQ(N, DITemplateTypeParameter::get(Context, Name, Type));
+  EXPECT_EQ(N, DITemplateTypeParameter::get(Context, Name, Type, defaulted));
 
-  EXPECT_NE(N, DITemplateTypeParameter::get(Context, "other", Type));
-  EXPECT_NE(N,
-DITemplateTypeParameter::get(Context, Name, getBasicType("other")));
+  EXPECT_NE(N, DITemplateTypeParameter::get(Context, "other", Type, defaulted));
+  EXPECT_NE(N, DITemplateTypeParameter::get(Context, Name,
+getBasicType("other"), defaulted));
+  EXPECT_NE(N, DITemplateTypeParameter::get(Context, Name, Type, true));
 
   TempDITemplateTypeParameter Temp = N->clone();
   EXPECT_EQ(N, MDNode::replaceWithUniqued(std::move(Temp)));
@@ -2098,24 +2100,31 @@
   unsigned Tag = dwarf::DW_TAG_template_value_parameter;
   StringRef Name = "template";
   DIType *Type = getBasicType("basic");
+  bool defaulted = false;
   Metadata *Value = getConstantAsMetadata();
 
-  auto *N = DITemplateValueParameter::get(Context, Tag, Name, Type, Value);
+  auto *N =
+  DITemplateValueParameter::get(Context, Tag, Name, Type, defaulted, Value);
   EXPECT_EQ(Tag, N->getTag());
   EXPECT_EQ(Name, N->getName());
   EXPECT_EQ(Type, N->getType());
   EXPECT_EQ(Value, N->getValue());
-  EXPECT_EQ(N, DITemplateValueParameter::get(Context, Tag, Name, Type, Value));
+  EXPECT_EQ(N, DITemplateValueParameter::get(Context, Tag, Name, Type,
+ defaulted, Value));
 
   EXPECT_NE(N, DITemplateValueParameter::get(
Context, dwarf::DW_TAG_GNU_template_template_param, Name,
-   Type, Value));
-  EXPECT_NE(N,
-DITemplateValueParameter::get(Context, Tag, "other", Type, Value));
+   Type, defaulted, Value));
+  EXPECT_NE(N, DITemplateValueParameter::get(Context, Tag, "other", Type,
+ defaulted, Value));
   EXPECT_NE(N, DITemplateValueParameter::get(Context, Tag, Name,
- getBasicType("other"), Value));
-  EXPECT_NE(N, DITemplateValueParameter::get(Context, Tag, Name, Type,
- getConstantAsMetadata()));
+ getBasicType("other"), defaulted,
+ Value));
+  EXPECT_NE(N,
+DITemplateValueParameter::get(Context, Tag, Name, Type, defaulted,
+  getConstantAsMetadata()));
+  EXPECT_NE(
+  N, DITemplateValueParameter::get(Context, Tag, Name, Type, true, Value));
 
   TempDITemplateValueParameter Temp = N->clone();
   EXPECT_EQ(N, MDNode::replaceWithUniqued(std::move(Temp)));
Index: llvm/test/DebugInfo/X86/debug-info-template-parameter.ll
===
--- /dev/null
+++ llvm/test/DebugInfo/X86/debug-info-template-parameter.ll
@@ -0,0 +1,90 @@
+; RUN: %llc_dwarf  %s -filetype=obj -o - | llvm-dwarfdump -v - | FileCheck %s
+
+; C++ source to regenerate:
+
+;template 
+;class foo {
+;};
+;
+;int main() {
+; foo f1;
+; foo<> f2;
+; return 0;
+;}
+
+; $ clang++ -O0 -gdwarf-5 -S -gdwarf-5 test.cpp 
+
+; CHECK: .debug_abbrev contents:
+; CHECK: DW_AT_default_value DW_FORM_flag_present
+
+; CHECK: debug_info contents:
+
+; CHECK: DW_AT_name {{.*}} "foo"
+; CHECK: DW_AT_type {{.*}} "int"
+; CHECK-NEXT: DW_AT_name {{.*}} "T"
+; CHECK-NOT: DW_AT_default_value
+; CHECK: DW_AT_type {{.*}} "int"
+

[PATCH] D75423: [OpenCL] Mark pointers to constant address space as invariant

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:172
+  if (llvm::Constant *C = dyn_cast(Addr))
+Cast = llvm::ConstantExpr::getBitCast(C, ObjectPtr[0]);
+  else

This check is already done by `CreateBitCast`.



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:176
+  llvm::Value *Args[2] = {llvm::ConstantInt::getSigned(Int64Ty, Width), Cast};
   Builder.CreateCall(InvariantStart, Args);
 }

I know this is a pre-existing code pattern, but there's an overload of 
`CreateCall` that just takes two arguments directly; please switch the code to 
use that.



Comment at: clang/lib/CodeGen/CGExpr.cpp:1255
   ApplyDebugLocation DL(*this, E);
+  LValue Ret;
   switch (E->getStmtClass()) {

This is not the way to do this, but fortunately it's unnecessary anyway.  You 
can just put the invariant-load metadata on any loads from the constant address 
space in the functions called by `EmitLoadOfLValue`.


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

https://reviews.llvm.org/D75423



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

[PATCH] D75387: [Sema] Look through OpaqueValueExpr when checking implicit conversions

[John McCall via Phabricator via cfe-commits]

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

Seems reasonable.


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

https://reviews.llvm.org/D75387



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

[PATCH] D75323: Support relative dest paths in headermap files

[Simon Que via Phabricator via cfe-commits]

sque updated this revision to Diff 247554.
sque added a comment.

Delete unnecessary comment


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

https://reviews.llvm.org/D75323

Files:
  clang/lib/Lex/HeaderSearch.cpp
  clang/test/Headers/Inputs/headermap_relpath/empty.hmap.json
  clang/test/Headers/Inputs/include/empty.h
  clang/test/Headers/headermap_relpath.cpp


Index: clang/test/Headers/headermap_relpath.cpp
===
--- /dev/null
+++ clang/test/Headers/headermap_relpath.cpp
@@ -0,0 +1,9 @@
+// Test relative include paths in headermap files
+// == 
//
+
+// RUN: rm -f %t.hmap
+// RUN: %hmaptool write %S/Inputs/headermap_relpath/empty.hmap.json %t.hmap
+// RUN: %clang_cc1 -fsyntax-only -I %t.hmap -I %S/Inputs %s
+
+#include 
+int main() { return 0; }
Index: clang/test/Headers/Inputs/include/empty.h
===
--- /dev/null
+++ clang/test/Headers/Inputs/include/empty.h
@@ -0,0 +1 @@
+
Index: clang/test/Headers/Inputs/headermap_relpath/empty.hmap.json
===
--- /dev/null
+++ clang/test/Headers/Inputs/headermap_relpath/empty.hmap.json
@@ -0,0 +1 @@
+{"mappings":{"empty/empty.h":"include/empty.h"}}
Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -415,7 +415,9 @@
   FixupSearchPath();
   return *Result;
 }
-  } else if (auto Res = HS.getFileMgr().getOptionalFileRef(Dest)) {
+// If lookup failed, fall back to file lookup using relative path directly.
+  }
+  if (auto Res = HS.getFileMgr().getOptionalFileRef(Dest)) {
 FixupSearchPath();
 return *Res;
   }


Index: clang/test/Headers/headermap_relpath.cpp
===
--- /dev/null
+++ clang/test/Headers/headermap_relpath.cpp
@@ -0,0 +1,9 @@
+// Test relative include paths in headermap files
+// == //
+
+// RUN: rm -f %t.hmap
+// RUN: %hmaptool write %S/Inputs/headermap_relpath/empty.hmap.json %t.hmap
+// RUN: %clang_cc1 -fsyntax-only -I %t.hmap -I %S/Inputs %s
+
+#include 
+int main() { return 0; }
Index: clang/test/Headers/Inputs/include/empty.h
===
--- /dev/null
+++ clang/test/Headers/Inputs/include/empty.h
@@ -0,0 +1 @@
+
Index: clang/test/Headers/Inputs/headermap_relpath/empty.hmap.json
===
--- /dev/null
+++ clang/test/Headers/Inputs/headermap_relpath/empty.hmap.json
@@ -0,0 +1 @@
+{"mappings":{"empty/empty.h":"include/empty.h"}}
Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -415,7 +415,9 @@
   FixupSearchPath();
   return *Result;
 }
-  } else if (auto Res = HS.getFileMgr().getOptionalFileRef(Dest)) {
+// If lookup failed, fall back to file lookup using relative path directly.
+  }
+  if (auto Res = HS.getFileMgr().getOptionalFileRef(Dest)) {
 FixupSearchPath();
 return *Res;
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] c2b437d - [DebugInfo][clang][DWARF5]: Added support for debuginfo generation for defaulted parameters

[Sourabh Singh Tomar via cfe-commits]

Author: Awanish Pandey
Date: 2020-03-02T12:33:05+05:30
New Revision: c2b437d53d40b6dc5603c97f527398f477d9c5f1

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

LOG: [DebugInfo][clang][DWARF5]: Added support for debuginfo generation for 
defaulted parameters
in C++ templates.

Summary:
This patch adds support for debuginfo generation for defaulted
parameters in clang and also extends corresponding DebugMetadata/IR to support 
this feature.

Reviewers: probinson, aprantl, dblaikie

Reviewed By: aprantl, dblaikie

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

Added: 
llvm/test/Assembler/DITemplateParameter.ll
llvm/test/Bitcode/DITemplateParameter-5.0.ll

Modified: 
clang/lib/CodeGen/CGDebugInfo.cpp
llvm/include/llvm/IR/DIBuilder.h
llvm/include/llvm/IR/DebugInfoMetadata.h
llvm/lib/AsmParser/LLParser.cpp
llvm/lib/Bitcode/Reader/MetadataLoader.cpp
llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
llvm/lib/IR/AsmWriter.cpp
llvm/lib/IR/DIBuilder.cpp
llvm/lib/IR/DebugInfoMetadata.cpp
llvm/lib/IR/LLVMContextImpl.h
llvm/unittests/IR/MetadataTest.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGDebugInfo.cpp 
b/clang/lib/CodeGen/CGDebugInfo.cpp
index e171082942f6..404ecfa975a1 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -1787,18 +1787,36 @@ CGDebugInfo::CollectTemplateParams(const 
TemplateParameterList *TPList,
   for (unsigned i = 0, e = TAList.size(); i != e; ++i) {
 const TemplateArgument &TA = TAList[i];
 StringRef Name;
+bool defaultParameter = false;
 if (TPList)
   Name = TPList->getParam(i)->getName();
 switch (TA.getKind()) {
 case TemplateArgument::Type: {
   llvm::DIType *TTy = getOrCreateType(TA.getAsType(), Unit);
-  TemplateParams.push_back(
-  DBuilder.createTemplateTypeParameter(TheCU, Name, TTy));
+
+  if (TPList)
+if (auto *templateType =
+dyn_cast_or_null(TPList->getParam(i)))
+  if (templateType->hasDefaultArgument())
+defaultParameter =
+templateType->getDefaultArgument() == TA.getAsType();
+
+  TemplateParams.push_back(DBuilder.createTemplateTypeParameter(
+  TheCU, Name, TTy, defaultParameter));
+
 } break;
 case TemplateArgument::Integral: {
   llvm::DIType *TTy = getOrCreateType(TA.getIntegralType(), Unit);
+  if (TPList)
+if (auto *templateType =
+dyn_cast_or_null(TPList->getParam(i)))
+  if (templateType->hasDefaultArgument())
+defaultParameter =
+templateType->getDefaultArgument()->EvaluateKnownConstInt(
+CGM.getContext()) == TA.getAsIntegral();
+
   TemplateParams.push_back(DBuilder.createTemplateValueParameter(
-  TheCU, Name, TTy,
+  TheCU, Name, TTy, defaultParameter,
   llvm::ConstantInt::get(CGM.getLLVMContext(), TA.getAsIntegral(;
 } break;
 case TemplateArgument::Declaration: {
@@ -1837,7 +1855,7 @@ CGDebugInfo::CollectTemplateParams(const 
TemplateParameterList *TPList,
 V = V->stripPointerCasts();
   }
   TemplateParams.push_back(DBuilder.createTemplateValueParameter(
-  TheCU, Name, TTy, cast_or_null(V)));
+  TheCU, Name, TTy, defaultParameter, 
cast_or_null(V)));
 } break;
 case TemplateArgument::NullPtr: {
   QualType T = TA.getNullPtrType();
@@ -1855,8 +1873,8 @@ CGDebugInfo::CollectTemplateParams(const 
TemplateParameterList *TPList,
   V = CGM.getCXXABI().EmitNullMemberPointer(MPT);
   if (!V)
 V = llvm::ConstantInt::get(CGM.Int8Ty, 0);
-  TemplateParams.push_back(
-  DBuilder.createTemplateValueParameter(TheCU, Name, TTy, V));
+  TemplateParams.push_back(DBuilder.createTemplateValueParameter(
+  TheCU, Name, TTy, defaultParameter, V));
 } break;
 case TemplateArgument::Template:
   TemplateParams.push_back(DBuilder.createTemplateTemplateParameter(
@@ -1877,7 +1895,7 @@ CGDebugInfo::CollectTemplateParams(const 
TemplateParameterList *TPList,
   assert(V && "Expression in template argument isn't constant");
   llvm::DIType *TTy = getOrCreateType(T, Unit);
   TemplateParams.push_back(DBuilder.createTemplateValueParameter(
-  TheCU, Name, TTy, V->stripPointerCasts()));
+  TheCU, Name, TTy, defaultParameter, V->stripPointerCasts()));
 } break;
 // And the following should never occur:
 case TemplateArgument::TemplateExpansion:

diff  --git a/llvm/include/llvm/IR/DIBuilder.h 
b/llvm/include/llvm/IR/DIBuilder.h
index cbc96a483f91..604740dcdfb3 100644
--- a/llvm/include/llvm/IR/DIBuilder.h
+++ b/llvm/include/llvm/IR/DIBuilder.h
@@ -443,19 +443,22 @@ namespace llvm {
   

[PATCH] D75323: Support relative dest paths in headermap files

[Takuto Ikuta via Phabricator via cfe-commits]

takuto.ikuta added inline comments.



Comment at: clang/test/Headers/Inputs/headermap_relpath/empty.hmap.json:1
+{"mappings":{"empty/empty.h":"include/empty.h"}}

better to format like other hmap.json?


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

https://reviews.llvm.org/D75323



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

[PATCH] D73462: [dwarf-5] Support DebugInfo for Defaulted parameters for C++ templates

[Sourabh Singh Tomar via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGc2b437d53d40: [DebugInfo][clang][DWARF5]: Added support for 
debuginfo generation for… (authored by awpandey, committed by SouraVX).

Changed prior to commit:
  https://reviews.llvm.org/D73462?vs=247546&id=247557#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73462

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  llvm/include/llvm/IR/DIBuilder.h
  llvm/include/llvm/IR/DebugInfoMetadata.h
  llvm/lib/AsmParser/LLParser.cpp
  llvm/lib/Bitcode/Reader/MetadataLoader.cpp
  llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
  llvm/lib/IR/AsmWriter.cpp
  llvm/lib/IR/DIBuilder.cpp
  llvm/lib/IR/DebugInfoMetadata.cpp
  llvm/lib/IR/LLVMContextImpl.h
  llvm/test/Assembler/DITemplateParameter.ll
  llvm/test/Bitcode/DITemplateParameter-5.0.ll
  llvm/unittests/IR/MetadataTest.cpp

Index: llvm/unittests/IR/MetadataTest.cpp
===
--- llvm/unittests/IR/MetadataTest.cpp
+++ llvm/unittests/IR/MetadataTest.cpp
@@ -2076,17 +2076,19 @@
 TEST_F(DITemplateTypeParameterTest, get) {
   StringRef Name = "template";
   DIType *Type = getBasicType("basic");
+  bool defaulted = false;
 
-  auto *N = DITemplateTypeParameter::get(Context, Name, Type);
+  auto *N = DITemplateTypeParameter::get(Context, Name, Type, defaulted);
 
   EXPECT_EQ(dwarf::DW_TAG_template_type_parameter, N->getTag());
   EXPECT_EQ(Name, N->getName());
   EXPECT_EQ(Type, N->getType());
-  EXPECT_EQ(N, DITemplateTypeParameter::get(Context, Name, Type));
+  EXPECT_EQ(N, DITemplateTypeParameter::get(Context, Name, Type, defaulted));
 
-  EXPECT_NE(N, DITemplateTypeParameter::get(Context, "other", Type));
-  EXPECT_NE(N,
-DITemplateTypeParameter::get(Context, Name, getBasicType("other")));
+  EXPECT_NE(N, DITemplateTypeParameter::get(Context, "other", Type, defaulted));
+  EXPECT_NE(N, DITemplateTypeParameter::get(Context, Name,
+getBasicType("other"), defaulted));
+  EXPECT_NE(N, DITemplateTypeParameter::get(Context, Name, Type, true));
 
   TempDITemplateTypeParameter Temp = N->clone();
   EXPECT_EQ(N, MDNode::replaceWithUniqued(std::move(Temp)));
@@ -2098,24 +2100,31 @@
   unsigned Tag = dwarf::DW_TAG_template_value_parameter;
   StringRef Name = "template";
   DIType *Type = getBasicType("basic");
+  bool defaulted = false;
   Metadata *Value = getConstantAsMetadata();
 
-  auto *N = DITemplateValueParameter::get(Context, Tag, Name, Type, Value);
+  auto *N =
+  DITemplateValueParameter::get(Context, Tag, Name, Type, defaulted, Value);
   EXPECT_EQ(Tag, N->getTag());
   EXPECT_EQ(Name, N->getName());
   EXPECT_EQ(Type, N->getType());
   EXPECT_EQ(Value, N->getValue());
-  EXPECT_EQ(N, DITemplateValueParameter::get(Context, Tag, Name, Type, Value));
+  EXPECT_EQ(N, DITemplateValueParameter::get(Context, Tag, Name, Type,
+ defaulted, Value));
 
   EXPECT_NE(N, DITemplateValueParameter::get(
Context, dwarf::DW_TAG_GNU_template_template_param, Name,
-   Type, Value));
-  EXPECT_NE(N,
-DITemplateValueParameter::get(Context, Tag, "other", Type, Value));
+   Type, defaulted, Value));
+  EXPECT_NE(N, DITemplateValueParameter::get(Context, Tag, "other", Type,
+ defaulted, Value));
   EXPECT_NE(N, DITemplateValueParameter::get(Context, Tag, Name,
- getBasicType("other"), Value));
-  EXPECT_NE(N, DITemplateValueParameter::get(Context, Tag, Name, Type,
- getConstantAsMetadata()));
+ getBasicType("other"), defaulted,
+ Value));
+  EXPECT_NE(N,
+DITemplateValueParameter::get(Context, Tag, Name, Type, defaulted,
+  getConstantAsMetadata()));
+  EXPECT_NE(
+  N, DITemplateValueParameter::get(Context, Tag, Name, Type, true, Value));
 
   TempDITemplateValueParameter Temp = N->clone();
   EXPECT_EQ(N, MDNode::replaceWithUniqued(std::move(Temp)));
Index: llvm/test/Bitcode/DITemplateParameter-5.0.ll
===
--- /dev/null
+++ llvm/test/Bitcode/DITemplateParameter-5.0.ll
@@ -0,0 +1,69 @@
+; RUN: llvm-dis -o - %s.bc | FileCheck %s
+
+; ModuleID = '/dir/test.cpp'
+source_filename = "test.cpp"
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+%class.foo = type { i8 }
+%class.foo.0 = type { i8 }
+; Function Attrs: noinline norecurse nounwind optnone uwtable
+define dso_local i32 @main() #0 !dbg !7 {
+entry:
+  %retval = alloca i32, align 4
+  %f1 = alloca %class.foo, align 1
+  %f

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

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: clang/docs/LanguageExtensions.rst:3055
+same spelling and syntax.  For pragmas specified at file scope, a stack
+is supported so that the pragma float_control settings can be pushed or popped.
+

`pragma float_control` should be in backticks.

Throughout this documentation, when referring to command-line options, please 
spell them the way they're actually spelled on the command line, i.e. with a 
dash.



Comment at: clang/include/clang/AST/Stmt.h:1104
+static_assert(sizeof(*this) <= 16,
   "changing bitfields changed sizeof(Stmt)");
 static_assert(sizeof(*this) % alignof(void *) == 0,

What's happening here is exactly what this assertion is supposed to prevent.   
If you need more bits in one of these classes (I assume it's 
`CXXOperatorCallExpr`), you need to either make a field in the actual class or 
investigate more arcane mechanisms like  trailing storage to reduce the normal 
impact.  The latter is probably unnecessary for `CXXOperatorCallExpr`.



Comment at: clang/include/clang/Basic/DiagnosticParseKinds.td:1138
+def err_pragma_float_control_unknown_kind : Error<
+  "unknown kind of pragma float_control">;
 // - #pragma pointers_to_members

Maybe "operation" would be a better user-facing name than "kind"?   Also, this 
diagnostic is more specific but less helpful than the diagnostic just above.



Comment at: clang/include/clang/Basic/LangOptions.def:196
+COMPATIBLE_LANGOPT(AllowRecip, 1, 0, "Permit Floating Point 
reciprocal")
+COMPATIBLE_LANGOPT(ApproxFunc, 1, 0, "Permit Floating Point 
approximation")
 

Please align the commas.

Would it make more sense to just store an `FPOptions` in `LangOptions` instead 
of breaking all of the bits down separately?

We may need to reconsider at some point whether any of these are really 
"compatible" language options.  Headers can contain inline code, and we 
shouldn't compile that incorrectly just because we reused a module we built 
under different language settings.  Although... maybe we can figure out a way 
to store just the ways that an expression's context overrides the default 
semantics and then merge those semantics into the default set for the 
translation unit; that would make them actually compatible.  Of course, it 
would also require more bits in expressions where it matters, and you might 
need to investigate trailing storage at that point.



Comment at: clang/include/clang/Basic/LangOptions.h:468
+  bool allowReciprocal() const { return allow_reciprocal; }
+  bool approxFunc() const  { return approx_func; }
+

Somewhere in this type, it should be obvious where I can go in order to 
understand what any of these flags means precisely.  Ideally that would be 
reinforced by the method names, instead of using non-term-of-art abbreviations 
like "reassoc".



Comment at: clang/include/clang/Basic/LangOptions.h:517
+approx_func = ((I>>13) & 1);
   }
 

The more conventional method names here would an instance method called 
something like `getAsOpaqueInt` and then a static method called something like 
`getFromOpaqueInt`.



Comment at: clang/lib/CodeGen/CGExprScalar.cpp:462
+  return StmtVisitor::Visit(E);
+}
 return StmtVisitor::Visit(E);

You can override `VisitBinOp` and just do this in that case.  But why does it 
need to be done at this level at all, setting global state on the builder for 
all emissions, instead of in the leaves where we know we're emitting 
floating-point operations?  This is adding a lot of overhead in some of the 
most commonly-exercised code paths in IRGen, but building FP expressions is 
relatively uncommon.  I would definitely prefer a little bit of repetitive code 
over burdening the common case this much.  It might also be nice to figure out 
when this is unnecessary.

Also, please extract a function to make FastMathFlags from FPOptions; you'll 
need it elsewhere, e.g. in CGExprComplex.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72841



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

[PATCH] D75323: Support relative dest paths in headermap files

[Simon Que via Phabricator via cfe-commits]

sque updated this revision to Diff 247558.
sque added a comment.

Format json according to RFC 8259 (https://jsonformatter.curiousconcept.com/)


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

https://reviews.llvm.org/D75323

Files:
  clang/lib/Lex/HeaderSearch.cpp
  clang/test/Headers/Inputs/headermap_relpath/empty.hmap.json
  clang/test/Headers/Inputs/include/empty.h
  clang/test/Headers/headermap_relpath.cpp


Index: clang/test/Headers/headermap_relpath.cpp
===
--- /dev/null
+++ clang/test/Headers/headermap_relpath.cpp
@@ -0,0 +1,9 @@
+// Test relative include paths in headermap files
+// == 
//
+
+// RUN: rm -f %t.hmap
+// RUN: %hmaptool write %S/Inputs/headermap_relpath/empty.hmap.json %t.hmap
+// RUN: %clang_cc1 -fsyntax-only -I %t.hmap -I %S/Inputs %s
+
+#include 
+int main() { return 0; }
Index: clang/test/Headers/Inputs/include/empty.h
===
--- /dev/null
+++ clang/test/Headers/Inputs/include/empty.h
@@ -0,0 +1 @@
+
Index: clang/test/Headers/Inputs/headermap_relpath/empty.hmap.json
===
--- /dev/null
+++ clang/test/Headers/Inputs/headermap_relpath/empty.hmap.json
@@ -0,0 +1,5 @@
+{
+  "mappings":{
+"empty/empty.h":"include/empty.h"
+  }
+}
Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -415,7 +415,9 @@
   FixupSearchPath();
   return *Result;
 }
-  } else if (auto Res = HS.getFileMgr().getOptionalFileRef(Dest)) {
+// If lookup failed, fall back to file lookup using relative path directly.
+  }
+  if (auto Res = HS.getFileMgr().getOptionalFileRef(Dest)) {
 FixupSearchPath();
 return *Res;
   }


Index: clang/test/Headers/headermap_relpath.cpp
===
--- /dev/null
+++ clang/test/Headers/headermap_relpath.cpp
@@ -0,0 +1,9 @@
+// Test relative include paths in headermap files
+// == //
+
+// RUN: rm -f %t.hmap
+// RUN: %hmaptool write %S/Inputs/headermap_relpath/empty.hmap.json %t.hmap
+// RUN: %clang_cc1 -fsyntax-only -I %t.hmap -I %S/Inputs %s
+
+#include 
+int main() { return 0; }
Index: clang/test/Headers/Inputs/include/empty.h
===
--- /dev/null
+++ clang/test/Headers/Inputs/include/empty.h
@@ -0,0 +1 @@
+
Index: clang/test/Headers/Inputs/headermap_relpath/empty.hmap.json
===
--- /dev/null
+++ clang/test/Headers/Inputs/headermap_relpath/empty.hmap.json
@@ -0,0 +1,5 @@
+{
+  "mappings":{
+"empty/empty.h":"include/empty.h"
+  }
+}
Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -415,7 +415,9 @@
   FixupSearchPath();
   return *Result;
 }
-  } else if (auto Res = HS.getFileMgr().getOptionalFileRef(Dest)) {
+// If lookup failed, fall back to file lookup using relative path directly.
+  }
+  if (auto Res = HS.getFileMgr().getOptionalFileRef(Dest)) {
 FixupSearchPath();
 return *Res;
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D74361: [Clang] Undef attribute for global variables

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In D74361#1896982 , @JonChesterfield 
wrote:

> In D74361#1879559 , @rjmccall wrote:
>
> > This will need to impact static analysis and the AST, I think.  Local 
> > variables can't be redeclared, but global variables can, so disallowing 
> > initializers syntactically when the attribute is present doesn't 
> > necessarily stop other declarations from defining the variable.  Also, you 
> > need to make the attribute mark something as a definition, the same way 
> > that e.g. the alias attribute does.  Also, this probably ought to disable 
> > the default-initialization of non-trivial types in C++, in case that's not 
> > already done.
>
>
> I would like this to be the case but am having a tough time understanding how 
> sema works. VarDecl::hasInit() looked promising but doesn't appear to 
> indicate whether there is a syntactic initialiser (e.g. = 10) present. I will 
> continue to investigate. Codegen appears to be working fine.


Looks like you figured this out.

> In D74361#1880904 , @jfb wrote:
> 
>> The current uninitialized attribute fits the model C and C++ follow for 
>> attributes: they have no semantic meaning for a valid program, in that a 
>> compiler can just ignore them and (barring undefined behavior) there's no 
>> observable effect to the program. This updated semantic changes the behavior 
>> of a valid C and C++ program, because the standards specify the value of 
>> uninitialized globals and TLS. I'd much rather have a separate attribute, 
>> say `no_zero_init`, to explicitly say what this does.
> 
> 
> This proposed attribute can similarly be ignored without observable semantic 
> effect. Instead of an IR undef variable, we would have an IR zeroinitialized 
> variable, which is a legitimate implementation choice for undef. Adding the 
> attribute to an existing program will, in general, change its meaning - but 
> that's also true of other attributes.

I agree with this reasoning, but since you seem willing to change the attribute 
name, the point is now moot.




Comment at: clang/include/clang/Basic/Attr.td:3436
+  let Subjects = SubjectList<[GlobalVar]>;
+  let Documentation = [Undocumented];
+}

We try to always add documentation for any new attribute.

I'm not sure I like the new name; it doesn't read right to me.  Maybe 
`loader_uninitialized` makes the intent clear enough?

Thinking more about it, I agree with you that this is orthogonal to C++ 
initialization.  Users on targets like yours probably ought to be able to 
disable C++ initialization without having to disable zero-initialization, or 
vice-versa.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74361



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