[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

[Yuanfang Chen via Phabricator via cfe-commits]

ychen added inline comments.



Comment at: clang/lib/CodeGen/CGCoroutine.cpp:437-475
+void emitDynamicAlignedDealloc(CodeGenFunction ,
+   llvm::BasicBlock *AlignedFreeBB,
+   llvm::CallInst *CoroFree) {
+  llvm::CallInst *Dealloc = nullptr;
+  for (llvm::User *U : CoroFree->users()) {
+if (auto *CI = dyn_cast(U))
+  if (CI->getParent() == CGF.Builder.GetInsertBlock())

ChuanqiXu wrote:
> We don't need this in this patch.
Do you mean `// Match size_t argument with the one used during allocation.` or 
the function `emitDynamicAlignedDealloc`? I think either is needed here. Could 
you please elaborate? 



Comment at: clang/lib/CodeGen/CGCoroutine.cpp:555-558
+  explicit CallCoroDelete(Stmt *DeallocStmt, Stmt *AlignedDeallocStmt,
+  bool DynamicAlignedDealloc)
+  : Deallocate(DeallocStmt), AlignedDeallocate(AlignedDeallocStmt),
+DynamicAlignedDealloc(DynamicAlignedDealloc) {}

ChuanqiXu wrote:
> Do we still need this change?
Nope



Comment at: clang/lib/CodeGen/CGCoroutine.cpp:743-744
+CGM.getIntrinsic(llvm::Intrinsic::coro_align, SizeTy));
+auto *AlignedUpAddr = EmitBuiltinAlignTo(AlignedAllocateCall, CoroAlign,
+ S.getAlignedAllocate(), true);
+// rawFrame = frame;

ChuanqiXu wrote:
> ychen wrote:
> > ChuanqiXu wrote:
> > > Maybe we could calculate it in place instead of trying to call a function 
> > > which is not designed for llvm::value*. It looks like the calculation 
> > > isn't too complex.
> > I'm open to not calling `EmitBuiltinAlignTo`, which basically inline the 
> > useful parts of `EmitBuiltinAlignTo`.  The initial intention is code 
> > sharing and easy readability. What's the benefit of not calling it?
> Reusing code is good. But my main concern is that the design for the 
> interfaces. The current design smells bad to me. Another reason for implement 
> it in place is I think it is not very complex and easy to understand.
> 
> Another option I got is to implement `EmitBuitinAlign` in LLVM (someplace 
> like `Local`), then the CodeGenFunction:: EmitBuitinAlign and this function 
> could use it.
> Reusing code is good. But my main concern is that the design for the 
> interfaces. The current design smells bad to me. Another reason for implement 
> it in place is I think it is not very complex and easy to understand.
> 
> Another option I got is to implement `EmitBuitinAlign` in LLVM (someplace 
> like `Local`), then the CodeGenFunction:: EmitBuitinAlign and this function 
> could use it.





Comment at: clang/lib/CodeGen/CodeGenFunction.h:1920-1921
 
+  llvm::Value *EmitBuiltinAlignTo(void *Args, const Expr *E, bool AlignUp);
+
 public:

ChuanqiXu wrote:
> ychen wrote:
> > ChuanqiXu wrote:
> > > We shouldn't add this interface. The actual type for the first argument 
> > > is BuiltinAlignArgs*, which defined in .cpp files. The signature is 
> > > confusing.
> > This is a private function, supposedly only meaningful for the 
> > implementation.  In that situation do you think it's bad? 
> It makes no sense to me that we can add interfaces casually if it is private. 
> For the users of Clang/LLVM, it may be OK since they wouldn't look into the 
> details. But for the developers, it matters. For example, I must be very 
> confused when I see this signature. Why is the type of `Args` is void*? 
> What's the type should I passed in? The smell is really bad.
> It makes no sense to me that we can add interfaces casually if it is private. 
> For the users of Clang/LLVM, it may be OK since they wouldn't look into the 
> details. But for the developers, it matters. For example, I must be very 
> confused when I see this signature. Why is the type of `Args` is void*? 
> What's the type should I passed in? The smell is really bad.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97915

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

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

[Yuanfang Chen via Phabricator via cfe-commits]

ychen updated this revision to Diff 352908.
ychen marked 2 inline comments as done.
ychen added a comment.

- Not use `void *` in EmitBuiltinAlignTo signature.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97915

Files:
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CGCoroutine.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/test/CodeGenCoroutines/coro-alloc.cpp
  clang/test/CodeGenCoroutines/coro-cleanup.cpp
  clang/test/CodeGenCoroutines/coro-gro.cpp
  llvm/docs/Coroutines.rst
  llvm/include/llvm/IR/Intrinsics.td
  llvm/lib/Transforms/Coroutines/CoroFrame.cpp
  llvm/lib/Transforms/Coroutines/CoroInstr.h
  llvm/lib/Transforms/Coroutines/CoroInternal.h
  llvm/lib/Transforms/Coroutines/CoroSplit.cpp
  llvm/lib/Transforms/Coroutines/Coroutines.cpp
  llvm/test/Transforms/Coroutines/coro-frame-overalign.ll

Index: llvm/test/Transforms/Coroutines/coro-frame-overalign.ll
===
--- /dev/null
+++ llvm/test/Transforms/Coroutines/coro-frame-overalign.ll
@@ -0,0 +1,78 @@
+; Check that `llvm.coro.align`, `llvm.coro.raw.frame.ptr.offset` and
+; `@llvm.coro.raw.frame.ptr.alloca` are lowered correctly.
+; RUN: opt < %s -passes=coro-split -S | FileCheck %s
+
+%PackedStruct = type <{ i64 }>
+
+declare void @consume(%PackedStruct*, i32, i32, i8**)
+declare void @consume2(i32, i32)
+
+define i8* @f() "coroutine.presplit"="1" {
+entry:
+  %data = alloca %PackedStruct, align 32
+  %id = call token @llvm.coro.id(i32 16, i8* null, i8* null, i8* null)
+  %size = call i32 @llvm.coro.size.i32()
+  %alloc = call i8* @malloc(i32 %size)
+  %hdl = call i8* @llvm.coro.begin(token %id, i8* %alloc)
+  %align = call i32 @llvm.coro.align.i32()
+  %offset = call i32 @llvm.coro.raw.frame.ptr.offset.i32()
+  %addr = call i8** @llvm.coro.raw.frame.ptr.addr()
+  call void @consume(%PackedStruct* %data, i32 %align, i32 %offset, i8** %addr)
+  %0 = call i8 @llvm.coro.suspend(token none, i1 false)
+  switch i8 %0, label %suspend [i8 0, label %resume
+i8 1, label %cleanup]
+resume:
+  br label %cleanup
+
+cleanup:
+  %align2 = call i32 @llvm.coro.align.i32()
+  %offset2 = call i32 @llvm.coro.raw.frame.ptr.offset.i32()
+  call void @consume2(i32 %align2, i32 %offset2)
+  %mem = call i8* @llvm.coro.free(token %id, i8* %hdl)
+  call void @free(i8* %mem)
+  br label %suspend
+suspend:
+  call i1 @llvm.coro.end(i8* %hdl, i1 0)
+  ret i8* %hdl
+}
+
+; See if the raw frame address was inserted into the frame.
+; CHECK-LABEL: %f.Frame = type { void (%f.Frame*)*, void (%f.Frame*)*, i8*, i1, [7 x i8], %PackedStruct }
+
+; See if we used correct index to access frame addr field (field 2).
+; CHECK-LABEL: @f(
+; CHECK: %alloc.frame.ptr = alloca i8*, align 8
+; CHECK: %[[FIELD:.+]] = getelementptr inbounds %f.Frame, %f.Frame* %FramePtr, i32 0, i32 2
+; CHECK: %[[ADDR:.+]] = load i8*, i8** %alloc.frame.ptr, align 8
+; CHECK: store i8* %[[ADDR]], i8** %[[FIELD]], align 8
+; CHECK: %[[DATA:.+]] = getelementptr inbounds %f.Frame, %f.Frame* %FramePtr, i32 0, i32 5
+; CHECK: call void @consume(%PackedStruct* %[[DATA]], i32 32, i32 16, i8** %[[FIELD]])
+; CHECK: ret i8*
+
+; See if `llvm.coro.align` and `llvm.coro.raw.frame.ptr.offset` are lowered
+; correctly during deallocation.
+; CHECK-LABEL: @f.destroy(
+; CHECK: call void @consume2(i32 32, i32 16)
+; CHECK: call void @free(i8* %{{.*}})
+
+; CHECK-LABEL: @f.cleanup(
+; CHECK: call void @consume2(i32 32, i32 16)
+; CHECK: call void @free(i8*
+
+declare i8* @llvm.coro.free(token, i8*)
+declare i32 @llvm.coro.size.i32()
+declare i32 @llvm.coro.align.i32()
+declare i32 @llvm.coro.raw.frame.ptr.offset.i32()
+declare i8** @llvm.coro.raw.frame.ptr.addr()
+declare i8  @llvm.coro.suspend(token, i1)
+declare void @llvm.coro.resume(i8*)
+declare void @llvm.coro.destroy(i8*)
+
+declare token @llvm.coro.id(i32, i8*, i8*, i8*)
+declare i1 @llvm.coro.alloc(token)
+declare i8* @llvm.coro.begin(token, i8*)
+declare i1 @llvm.coro.end(i8*, i1)
+
+declare noalias i8* @malloc(i32)
+declare double @print(double)
+declare void @free(i8*)
Index: llvm/lib/Transforms/Coroutines/Coroutines.cpp
===
--- llvm/lib/Transforms/Coroutines/Coroutines.cpp
+++ llvm/lib/Transforms/Coroutines/Coroutines.cpp
@@ -234,6 +234,9 @@
   Shape.CoroBegin = nullptr;
   Shape.CoroEnds.clear();
   Shape.CoroSizes.clear();
+  Shape.CoroAligns.clear();
+  Shape.CoroRawFramePtrOffsets.clear();
+  Shape.CoroRawFramePtrAddrs.clear();
   Shape.CoroSuspends.clear();
 
   Shape.FrameTy = nullptr;
@@ -268,6 +271,15 @@
   case Intrinsic::coro_size:
 CoroSizes.push_back(cast(II));
 break;
+  case Intrinsic::coro_align:
+CoroAligns.push_back(cast(II));
+break;
+  case Intrinsic::coro_raw_frame_ptr_offset:
+CoroRawFramePtrOffsets.push_back(cast(II));
+break;
+  

[clang] 6aaf4fa - Bring our handling of -Wframe-larger-than more in line with GCC.

[Richard Smith via cfe-commits]

Author: Richard Smith
Date: 2021-06-17T20:29:13-07:00
New Revision: 6aaf4fa2885600b0e31042071ad06f78218ab0f2

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

LOG: Bring our handling of -Wframe-larger-than more in line with GCC.

Support -Wno-frame-larger-than (with no =) and make it properly
interoperate with -Wframe-larger-than. Reject -Wframe-larger-than with
no argument.

We continue to support Clang's old spelling, -Wframe-larger-than=, for
compatibility with existing users of that facility.

In passing, stop the driver from accepting and ignoring
-fwarn-stack-size and make it a cc1-only flag as intended.

Added: 


Modified: 
clang/include/clang/Basic/DiagnosticFrontendKinds.td
clang/include/clang/Basic/DiagnosticGroups.td
clang/include/clang/Driver/Options.td
clang/lib/Driver/ToolChains/Clang.cpp
clang/test/Driver/Wframe-larger-than.c
clang/test/Frontend/backend-diagnostic.c
clang/test/Misc/backend-stack-frame-diagnostics.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticFrontendKinds.td 
b/clang/include/clang/Basic/DiagnosticFrontendKinds.td
index 5122770316cde..e68058dd19b5b 100644
--- a/clang/include/clang/Basic/DiagnosticFrontendKinds.td
+++ b/clang/include/clang/Basic/DiagnosticFrontendKinds.td
@@ -26,9 +26,9 @@ def err_fe_cannot_link_module : Error<"cannot link module 
'%0': %1">,
   DefaultFatal;
 
 def warn_fe_frame_larger_than : Warning<"stack frame size of %0 bytes in %q1">,
-BackendInfo, InGroup;
+BackendInfo, InGroup;
 def warn_fe_backend_frame_larger_than: Warning<"%0">,
-BackendInfo, InGroup;
+BackendInfo, InGroup;
 def err_fe_backend_frame_larger_than: Error<"%0">, BackendInfo;
 def note_fe_backend_frame_larger_than: Note<"%0">, BackendInfo;
 

diff  --git a/clang/include/clang/Basic/DiagnosticGroups.td 
b/clang/include/clang/Basic/DiagnosticGroups.td
index b4be0fcfb454f..ca8e05f27fc5f 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -1192,7 +1192,9 @@ def OpenMP : DiagGroup<"openmp", [
 // Backend warnings.
 def BackendInlineAsm : DiagGroup<"inline-asm">;
 def BackendSourceMgr : DiagGroup<"source-mgr">;
-def BackendFrameLargerThanEQ : DiagGroup<"frame-larger-than=">;
+def BackendFrameLargerThan : DiagGroup<"frame-larger-than">;
+// Compatibility flag name from old versions of Clang.
+def : DiagGroup<"frame-larger-than=", [BackendFrameLargerThan]>;
 def BackendPlugin : DiagGroup<"backend-plugin">;
 def RemarkBackendPlugin : DiagGroup<"remark-backend-plugin">;
 def BackendOptimizationRemark : DiagGroup<"pass">;

diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index 151968de1789e..0ccf5ef891990 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -1488,9 +1488,6 @@ defm cxx_static_destructors : 
BoolFOption<"c++-static-destructors",
   PosFlag>;
 def fsymbol_partition_EQ : Joined<["-"], "fsymbol-partition=">, Group,
   Flags<[CC1Option]>, MarshallingInfoString>;
-def fwarn_stack_size_EQ : Joined<["-"], "fwarn-stack-size=">, Group,
-  Flags<[CC1Option]>,
-  MarshallingInfoInt, "UINT_MAX">;
 
 defm memory_profile : OptInFFlag<"memory-profile", "Enable", "Disable", " heap 
memory profiling">;
 def fmemory_profile_EQ : Joined<["-"], "fmemory-profile=">,
@@ -2590,7 +2587,12 @@ def Wlarge_by_value_copy_EQ : Joined<["-"], 
"Wlarge-by-value-copy=">, Flags<[CC1
 // Just silence warnings about -Wlarger-than for now.
 def Wlarger_than_EQ : Joined<["-"], "Wlarger-than=">, 
Group;
 def Wlarger_than_ : Joined<["-"], "Wlarger-than-">, Alias;
-def Wframe_larger_than_EQ : Joined<["-"], "Wframe-larger-than=">, 
Group, Flags<[NoXarchOption]>;
+
+// This is converted to -fwarn-stack-size=N and also passed through by the 
driver.
+// FIXME: The driver should strip out the = when passing W_value_Group 
through.
+def Wframe_larger_than_EQ : Joined<["-"], "Wframe-larger-than=">, 
Group,
+Flags<[NoXarchOption, CC1Option]>;
+def Wframe_larger_than : Flag<["-"], "Wframe-larger-than">, 
Alias;
 
 def : Flag<["-"], "fterminated-vtables">, Alias;
 defm threadsafe_statics : BoolFOption<"threadsafe-statics",
@@ -5047,6 +5049,9 @@ def fverify_debuginfo_preserve_export
"into specified (JSON) file (should be abs path as we use "
"append mode to insert new JSON objects).">,
   MarshallingInfoString>;
+def fwarn_stack_size_EQ
+: Joined<["-"], "fwarn-stack-size=">,
+  MarshallingInfoInt, "UINT_MAX">;
 // The driver option takes the key as a parameter to the -msign-return-address=
 // and -mbranch-protection= options, but CC1 has a separate option so we
 // don't have to parse the parameter twice.

diff  --git 

[PATCH] D104505: [HIP] Defer operator overloading errors

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl created this revision.
yaxunl added reviewers: tra, rjmccall, rsmith.
yaxunl requested review of this revision.

nvcc does not diagnose overloading resolution diagnostics
if it happens in functions not emitted, e.g., if a device
function calls a host function, it is not diagnosed in
host compilation. clang implemented a similar feature
under option -fgpu-defer-diags.

Although clang is able to defer overloading resolution
diagnostics for common functions. It does not defer
overloading resolution caused diagnostics for overloaded
operators. This patch extends the existing deferred
diagnostic mechanism and defers a diagnostic caused
by overloaded operator.


https://reviews.llvm.org/D104505

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Overload.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/test/SemaCUDA/deferred-oeverload.cu

Index: clang/test/SemaCUDA/deferred-oeverload.cu
===
--- clang/test/SemaCUDA/deferred-oeverload.cu
+++ clang/test/SemaCUDA/deferred-oeverload.cu
@@ -31,12 +31,20 @@
 __host__ void callee5(float); // com-note {{candidate function}}
 __host__ void callee5(double); // com-note {{candidate function}}
 
+// When '<<` operator is called by a device function, there is error for 'invalid operands'.
+// It should be deferred since it involves wrong-sided candidates.
+struct S {
+  __host__ S  <<(int i); // dev-note {{candidate function not viable}}
+};
+
 __host__ void hf() {
  callee(1); // host-error {{call to 'callee' is ambiguous}}
  callee2();
  callee3();
  callee4(); // com-error {{no matching function for call to 'callee4'}}
  callee5(1); // com-error {{call to 'callee5' is ambiguous}}
+ S s;
+ s << 1;
  undeclared_func(); // com-error {{use of undeclared identifier 'undeclared_func'}}
 }
 
@@ -45,6 +53,8 @@
  callee2(); // dev-error {{no matching function for call to 'callee2'}}
  callee3(); // dev-error {{no matching function for call to 'callee3'}}
  callee4(); // com-error {{no matching function for call to 'callee4'}}
+ S s;
+ s << 1;// dev-error {{invalid operands to binary expression}}
 }
 
 struct A { int x; typedef int isA; };
Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -11641,7 +11641,8 @@
 CompleteCandidates(S, OCD_AllCandidates, Args, OpLoc, [](auto ) {
   return (Cand.Viable == false &&
   Cand.FailureKind == ovl_fail_bad_target) ||
- (Cand.Function->template hasAttr() &&
+ (Cand.Function &&
+  Cand.Function->template hasAttr() &&
   Cand.Function->template hasAttr());
 });
 DeferHint = !WrongSidedCands.empty();
@@ -13820,6 +13821,8 @@
   StringRef OpcStr = BinaryOperator::getOpcodeStr(Opc);
   auto Cands = CandidateSet.CompleteCandidates(*this, OCD_AllCandidates,
Args, OpLoc);
+  DeferDiagsRAII DDR(*this,
+ CandidateSet.shouldDeferDiags(*this, Args, OpLoc));
   if (Args[0]->getType()->isRecordType() &&
   Opc >= BO_Assign && Opc <= BO_OrAssign) {
 Diag(OpLoc,  diag::err_ovl_no_viable_oper)
Index: clang/lib/Sema/Sema.cpp
===
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -1790,7 +1790,7 @@
   bool IsError = Diags.getDiagnosticIDs()->isDefaultMappingAsError(DiagID);
   bool ShouldDefer = getLangOpts().CUDA && LangOpts.GPUDeferDiag &&
  DiagnosticIDs::isDeferrable(DiagID) &&
- (DeferHint || !IsError);
+ (DeferHint || DeferDiags || !IsError);
   auto SetIsLastErrorImmediate = [&](bool Flag) {
 if (IsError)
   IsLastErrorImmediate = Flag;
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -1769,6 +1769,22 @@
   /// Build a partial diagnostic.
   PartialDiagnostic PDiag(unsigned DiagID = 0); // in SemaInternal.h
 
+  /// Whether deferrable diagnostics should be deferred.
+  bool DeferDiags = false;
+
+  /// RAII class to control scope of DeferDiags.
+  class DeferDiagsRAII {
+Sema 
+bool SavedDeferDiags = false;
+
+  public:
+DeferDiagsRAII(Sema &_S, bool DeferDiags)
+: S(_S), SavedDeferDiags(S.DeferDiags) {
+  S.DeferDiags = DeferDiags;
+}
+~DeferDiagsRAII() { S.DeferDiags = SavedDeferDiags; }
+  };
+
   /// Whether uncompilable error has occurred. This includes error happens
   /// in deferred diagnostics.
   bool hasUncompilableErrorOccurred() const;
Index: clang/include/clang/Sema/Overload.h

[PATCH] D104350: [clang][AST] Make `getLocalOrImportedSubmoduleID` work with const `Module*`. NFC.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

Thanks for the review!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104350

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

[PATCH] D103930: [clang][HeaderSearch] Fix implicit module when using header maps

[Andrew Gallagher via Phabricator via cfe-commits]

andrewjcg updated this revision to Diff 352898.
andrewjcg added a comment.

capitalize param


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103930

Files:
  clang/lib/Lex/HeaderSearch.cpp
  clang/test/Modules/Inputs/implicit-module-header-maps/a.h
  clang/test/Modules/Inputs/implicit-module-header-maps/a.hmap.json
  clang/test/Modules/Inputs/implicit-module-header-maps/a.module.modulemap
  clang/test/Modules/Inputs/implicit-module-header-maps/b.hmap.json
  clang/test/Modules/implicit-module-header-maps.cpp

Index: clang/test/Modules/implicit-module-header-maps.cpp
===
--- /dev/null
+++ clang/test/Modules/implicit-module-header-maps.cpp
@@ -0,0 +1,32 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: cd %t
+//
+// RUN: %hmaptool write %S/Inputs/implicit-module-header-maps/a.hmap.json %t/hmap
+//
+// RUN: mkdir -p %t/After
+// RUN: cp %S/Inputs/implicit-module-header-maps/a.h %t/After/Mapping.h
+// RUN: cp %S/Inputs/implicit-module-header-maps/a.module.modulemap %t/module.modulemap
+//
+// RUN: %clang -Rmodule-build -fmodules -fimplicit-modules -fimplicit-module-maps -fmodule-map-file=%t/module.modulemap -fsyntax-only %s -I %t/hmap
+//
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: cd %t
+//
+// RUN: sed -e "s|OUTPUTS_DIR|%t|g" %S/Inputs/implicit-module-header-maps/b.hmap.json > %t/hmap.json
+// RUN: %hmaptool write %t/hmap.json %t/hmap
+//
+// RUN: mkdir -p %t/After
+// RUN: cp %S/Inputs/implicit-module-header-maps/a.h %t/After/Mapping.h
+// RUN: cp %S/Inputs/implicit-module-header-maps/a.module.modulemap %t/module.modulemap
+//
+// RUN: %clang -Rmodule-build -fmodules -fimplicit-modules -fimplicit-module-maps -fmodule-map-file=%t/module.modulemap -fsyntax-only %s -I %t/hmap
+
+#define FOO
+// This include will fail if:
+// 1) modules are't used, as the `FOO` define will propagate into the included
+//header and trip a `#error`, or
+// 2) header maps aren't usesd, as the header name doesn't exist and relies on
+//the header map to remap it to the real header.
+#include "Before/Mapping.h"
Index: clang/test/Modules/Inputs/implicit-module-header-maps/b.hmap.json
===
--- /dev/null
+++ clang/test/Modules/Inputs/implicit-module-header-maps/b.hmap.json
@@ -0,0 +1,6 @@
+{
+  "mappings" :
+{
+ "Before/Mapping.h" : "OUTPUTS_DIR/After/Mapping.h"
+}
+}
Index: clang/test/Modules/Inputs/implicit-module-header-maps/a.module.modulemap
===
--- /dev/null
+++ clang/test/Modules/Inputs/implicit-module-header-maps/a.module.modulemap
@@ -0,0 +1,3 @@
+module a {
+  header "After/Mapping.h"
+}
Index: clang/test/Modules/Inputs/implicit-module-header-maps/a.hmap.json
===
--- /dev/null
+++ clang/test/Modules/Inputs/implicit-module-header-maps/a.hmap.json
@@ -0,0 +1,7 @@
+{
+  "mappings" :
+{
+ "Before/Mapping.h" : "After/Mapping.h",
+ "After/Mapping.h" : "After/Mapping.h"
+}
+}
Index: clang/test/Modules/Inputs/implicit-module-header-maps/a.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/implicit-module-header-maps/a.h
@@ -0,0 +1,3 @@
+#ifdef FOO
+#error foo
+#endif
Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -417,7 +417,8 @@
 
   IsInHeaderMap = true;
 
-  auto FixupSearchPath = [&]() {
+  auto FixupSearchPathAndFindUsableModule =
+  [&](auto File) -> Optional {
 if (SearchPath) {
   StringRef SearchPathRef(getName());
   SearchPath->clear();
@@ -427,6 +428,12 @@
   RelativePath->clear();
   RelativePath->append(Filename.begin(), Filename.end());
 }
+if (!HS.findUsableModuleForHeader(
+(), File.getFileEntry().getDir(),
+RequestingModule, SuggestedModule, isSystemHeaderDirectory())) {
+  return None;
+}
+return File;
   };
 
   // Check if the headermap maps the filename to a framework include
@@ -437,12 +444,10 @@
 Filename = StringRef(MappedName.begin(), MappedName.size());
 Optional Result = HM->LookupFile(Filename, HS.getFileMgr());
 if (Result) {
-  FixupSearchPath();
-  return *Result;
+  return FixupSearchPathAndFindUsableModule(*Result);
 }
   } else if (auto Res = HS.getFileMgr().getOptionalFileRef(Dest)) {
-FixupSearchPath();
-return *Res;
+return FixupSearchPathAndFindUsableModule(*Res);
   }
 
   return None;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D104500: DRAFT: [clang] Apply P1825 as DR for all modes below C++20.

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov updated this revision to Diff 352894.
mizvekov added a comment.

set parent revision.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104500

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaStmt.cpp
  clang/test/CXX/class/class.init/class.copy.elision/p3.cpp
  clang/test/SemaCXX/P1155.cpp
  clang/test/SemaCXX/conversion-function.cpp
  clang/test/SemaCXX/warn-return-std-move.cpp
  clang/test/SemaObjCXX/block-capture.mm

Index: clang/test/SemaObjCXX/block-capture.mm
===
--- clang/test/SemaObjCXX/block-capture.mm
+++ clang/test/SemaObjCXX/block-capture.mm
@@ -1,7 +1,7 @@
-// RUN: %clang_cc1 -std=c++2b -fsyntax-only -fobjc-arc -fblocks   -verify=cxx98_2b,cxx20_2b,cxx2b %s
-// RUN: %clang_cc1 -std=c++20 -fsyntax-only -fobjc-arc -fblocks   -verify=cxx98_2b,cxx20_2b   %s
-// RUN: %clang_cc1 -std=c++11 -fsyntax-only -fobjc-arc -fblocks   -verify=cxx98_2b,cxx98_11   %s
-// RUN: %clang_cc1 -std=c++98 -fsyntax-only -fobjc-arc -fblocks -Wno-c++11-extensions -verify=cxx98_2b,cxx98_11   %s
+// RUN: %clang_cc1 -std=c++2b -fsyntax-only -fobjc-arc -fblocks   -verify=cxx98_2b,cxx2b %s
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -fobjc-arc -fblocks   -verify=cxx98_2b   %s
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -fobjc-arc -fblocks   -verify=cxx98_2b   %s
+// RUN: %clang_cc1 -std=c++98 -fsyntax-only -fobjc-arc -fblocks -Wno-c++11-extensions -verify=cxx98_2b   %s
 
 #define TEST(T) void test_##T() { \
   __block T x;\
@@ -37,31 +37,31 @@
 
 struct ConvertingRVRef {
   ConvertingRVRef();
-  ConvertingRVRef(ConvertingRVRef &) = delete; // cxx98_11-note {{marked deleted here}}
+  ConvertingRVRef(ConvertingRVRef &) = delete;
 
   struct X {};
   ConvertingRVRef(X &&);
   operator X() const & = delete;
   operator X() &&;
 };
-TEST(ConvertingRVRef); // cxx98_11-error {{call to deleted constructor}}
+TEST(ConvertingRVRef);
 
 struct ConvertingCLVRef {
   ConvertingCLVRef();
   ConvertingCLVRef(ConvertingCLVRef &);
 
   struct X {};
-  ConvertingCLVRef(X &&); // cxx20_2b-note {{passing argument to parameter here}}
+  ConvertingCLVRef(X &&); // cxx98_2b-note {{passing argument to parameter here}}
   operator X() const &;
-  operator X() && = delete; // cxx20_2b-note {{marked deleted here}}
+  operator X() && = delete; // cxx98_2b-note {{marked deleted here}}
 };
-TEST(ConvertingCLVRef); // cxx20_2b-error {{invokes a deleted function}}
+TEST(ConvertingCLVRef); // cxx98_2b-error {{invokes a deleted function}}
 
 struct SubSubMove {};
 struct SubMove : SubSubMove {
   SubMove();
-  SubMove(SubMove &) = delete; // cxx98_11-note {{marked deleted here}}
+  SubMove(SubMove &) = delete;
 
   SubMove(SubSubMove &&);
 };
-TEST(SubMove); // cxx98_11-error {{call to deleted constructor}}
+TEST(SubMove);
Index: clang/test/SemaCXX/warn-return-std-move.cpp
===
--- clang/test/SemaCXX/warn-return-std-move.cpp
+++ /dev/null
@@ -1,351 +0,0 @@
-// RUN: %clang_cc1 -std=c++2b -fsyntax-only -verify=cxx20_2b,cxx2b -fcxx-exceptions -Wreturn-std-move %s
-// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=cxx20_2b   -fcxx-exceptions -Wreturn-std-move %s
-// RUN: %clang_cc1 -std=c++17 -fsyntax-only -verify=cxx11_17   -fcxx-exceptions -Wreturn-std-move %s
-// RUN: %clang_cc1 -std=c++14 -fsyntax-only -verify=cxx11_17   -fcxx-exceptions -Wreturn-std-move %s
-// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify=cxx11_17   -fcxx-exceptions -Wreturn-std-move %s
-
-// RUN: %clang_cc1 -std=c++17 -fsyntax-only -fcxx-exceptions -Wreturn-std-move -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s -check-prefix=CHECK
-// RUN: %clang_cc1 -std=c++14 -fsyntax-only -fcxx-exceptions -Wreturn-std-move -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s -check-prefix=CHECK
-// RUN: %clang_cc1 -std=c++11 -fsyntax-only -fcxx-exceptions -Wreturn-std-move -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s -check-prefix=CHECK
-
-// definitions for std::move
-namespace std {
-inline namespace foo {
-template  struct remove_reference { typedef T type; };
-template  struct remove_reference { typedef T type; };
-template  struct remove_reference { typedef T type; };
-
-template  typename remove_reference::type &(T &);
-} // namespace foo
-} // namespace std
-
-struct Instrument {
-Instrument() {}
-Instrument(Instrument&&) { /* MOVE */ }
-Instrument(const Instrument&) { /* COPY */ }
-};
-struct ConvertFromBase { Instrument i; };
-struct ConvertFromDerived { Instrument i; };
-struct Base {
-Instrument i;
-operator ConvertFromBase() const& { return ConvertFromBase{i}; }
-operator ConvertFromBase() && { return ConvertFromBase{std::move(i)}; }
-};

[PATCH] D104500: DRAFT: [clang] Apply P1825 as DR for all modes below C++20.

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov created this revision.
mizvekov requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Signed-off-by: Matheus Izvekov 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D104500

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaStmt.cpp
  clang/test/CXX/class/class.init/class.copy.elision/p3.cpp
  clang/test/SemaCXX/P1155.cpp
  clang/test/SemaCXX/conversion-function.cpp
  clang/test/SemaCXX/warn-return-std-move.cpp
  clang/test/SemaObjCXX/block-capture.mm

Index: clang/test/SemaObjCXX/block-capture.mm
===
--- clang/test/SemaObjCXX/block-capture.mm
+++ clang/test/SemaObjCXX/block-capture.mm
@@ -1,7 +1,7 @@
-// RUN: %clang_cc1 -std=c++2b -fsyntax-only -fobjc-arc -fblocks   -verify=cxx98_2b,cxx20_2b,cxx2b %s
-// RUN: %clang_cc1 -std=c++20 -fsyntax-only -fobjc-arc -fblocks   -verify=cxx98_2b,cxx20_2b   %s
-// RUN: %clang_cc1 -std=c++11 -fsyntax-only -fobjc-arc -fblocks   -verify=cxx98_2b,cxx98_11   %s
-// RUN: %clang_cc1 -std=c++98 -fsyntax-only -fobjc-arc -fblocks -Wno-c++11-extensions -verify=cxx98_2b,cxx98_11   %s
+// RUN: %clang_cc1 -std=c++2b -fsyntax-only -fobjc-arc -fblocks   -verify=cxx98_2b,cxx2b %s
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -fobjc-arc -fblocks   -verify=cxx98_2b   %s
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -fobjc-arc -fblocks   -verify=cxx98_2b   %s
+// RUN: %clang_cc1 -std=c++98 -fsyntax-only -fobjc-arc -fblocks -Wno-c++11-extensions -verify=cxx98_2b   %s
 
 #define TEST(T) void test_##T() { \
   __block T x;\
@@ -37,31 +37,31 @@
 
 struct ConvertingRVRef {
   ConvertingRVRef();
-  ConvertingRVRef(ConvertingRVRef &) = delete; // cxx98_11-note {{marked deleted here}}
+  ConvertingRVRef(ConvertingRVRef &) = delete;
 
   struct X {};
   ConvertingRVRef(X &&);
   operator X() const & = delete;
   operator X() &&;
 };
-TEST(ConvertingRVRef); // cxx98_11-error {{call to deleted constructor}}
+TEST(ConvertingRVRef);
 
 struct ConvertingCLVRef {
   ConvertingCLVRef();
   ConvertingCLVRef(ConvertingCLVRef &);
 
   struct X {};
-  ConvertingCLVRef(X &&); // cxx20_2b-note {{passing argument to parameter here}}
+  ConvertingCLVRef(X &&); // cxx98_2b-note {{passing argument to parameter here}}
   operator X() const &;
-  operator X() && = delete; // cxx20_2b-note {{marked deleted here}}
+  operator X() && = delete; // cxx98_2b-note {{marked deleted here}}
 };
-TEST(ConvertingCLVRef); // cxx20_2b-error {{invokes a deleted function}}
+TEST(ConvertingCLVRef); // cxx98_2b-error {{invokes a deleted function}}
 
 struct SubSubMove {};
 struct SubMove : SubSubMove {
   SubMove();
-  SubMove(SubMove &) = delete; // cxx98_11-note {{marked deleted here}}
+  SubMove(SubMove &) = delete;
 
   SubMove(SubSubMove &&);
 };
-TEST(SubMove); // cxx98_11-error {{call to deleted constructor}}
+TEST(SubMove);
Index: clang/test/SemaCXX/warn-return-std-move.cpp
===
--- clang/test/SemaCXX/warn-return-std-move.cpp
+++ /dev/null
@@ -1,351 +0,0 @@
-// RUN: %clang_cc1 -std=c++2b -fsyntax-only -verify=cxx20_2b,cxx2b -fcxx-exceptions -Wreturn-std-move %s
-// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=cxx20_2b   -fcxx-exceptions -Wreturn-std-move %s
-// RUN: %clang_cc1 -std=c++17 -fsyntax-only -verify=cxx11_17   -fcxx-exceptions -Wreturn-std-move %s
-// RUN: %clang_cc1 -std=c++14 -fsyntax-only -verify=cxx11_17   -fcxx-exceptions -Wreturn-std-move %s
-// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify=cxx11_17   -fcxx-exceptions -Wreturn-std-move %s
-
-// RUN: %clang_cc1 -std=c++17 -fsyntax-only -fcxx-exceptions -Wreturn-std-move -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s -check-prefix=CHECK
-// RUN: %clang_cc1 -std=c++14 -fsyntax-only -fcxx-exceptions -Wreturn-std-move -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s -check-prefix=CHECK
-// RUN: %clang_cc1 -std=c++11 -fsyntax-only -fcxx-exceptions -Wreturn-std-move -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s -check-prefix=CHECK
-
-// definitions for std::move
-namespace std {
-inline namespace foo {
-template  struct remove_reference { typedef T type; };
-template  struct remove_reference { typedef T type; };
-template  struct remove_reference { typedef T type; };
-
-template  typename remove_reference::type &(T &);
-} // namespace foo
-} // namespace std
-
-struct Instrument {
-Instrument() {}
-Instrument(Instrument&&) { /* MOVE */ }
-Instrument(const Instrument&) { /* COPY */ }
-};
-struct ConvertFromBase { Instrument i; };
-struct ConvertFromDerived { Instrument i; };
-struct Base {
-Instrument i;
-operator ConvertFromBase() const& { return ConvertFromBase{i}; }
-operator ConvertFromBase() && { return 

[clang] 05d0f1a - Frontend: Respect -fno-temp-file when creating a PCH

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

Author: Zachary Henkel
Date: 2021-06-17T18:34:10-07:00
New Revision: 05d0f1a8ea012a6b7b8ea65893ec4121106444b5

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

LOG: Frontend: Respect -fno-temp-file when creating a PCH

When creating a PCH file the use of a temp file will be dictated by the
presence or absence of the -fno-temp-file flag. Creating a module file
will always use a temp file via the new ForceUseTemporary flag.

This fixes bug 50033.

Added: 


Modified: 
clang/include/clang/Frontend/CompilerInstance.h
clang/lib/Frontend/CompilerInstance.cpp
clang/lib/Frontend/FrontendActions.cpp

Removed: 




diff  --git a/clang/include/clang/Frontend/CompilerInstance.h 
b/clang/include/clang/Frontend/CompilerInstance.h
index 9d6dd8fa1006f..861b15020329b 100644
--- a/clang/include/clang/Frontend/CompilerInstance.h
+++ b/clang/include/clang/Frontend/CompilerInstance.h
@@ -696,15 +696,13 @@ class CompilerInstance : public ModuleLoader {
   /// The files created by this are usually removed on signal, and, depending
   /// on FrontendOptions, may also use a temporary file (that is, the data is
   /// written to a temporary file which will atomically replace the target
-  /// output on success). If a client (like libclang) needs to disable
-  /// RemoveFileOnSignal, temporary files will be forced on.
+  /// output on success).
   ///
   /// \return - Null on error.
-  std::unique_ptr
-  createDefaultOutputFile(bool Binary = true, StringRef BaseInput = "",
-  StringRef Extension = "",
-  bool RemoveFileOnSignal = true,
-  bool CreateMissingDirectories = false);
+  std::unique_ptr createDefaultOutputFile(
+  bool Binary = true, StringRef BaseInput = "", StringRef Extension = "",
+  bool RemoveFileOnSignal = true, bool CreateMissingDirectories = false,
+  bool ForceUseTemporary = false);
 
   /// Create a new output file, optionally deriving the output path name, and
   /// add it to the list of tracked output files.

diff  --git a/clang/lib/Frontend/CompilerInstance.cpp 
b/clang/lib/Frontend/CompilerInstance.cpp
index 78c7d84bbef45..063384130f730 100644
--- a/clang/lib/Frontend/CompilerInstance.cpp
+++ b/clang/lib/Frontend/CompilerInstance.cpp
@@ -743,11 +743,9 @@ void CompilerInstance::clearOutputFiles(bool EraseFiles) {
   }
 }
 
-std::unique_ptr
-CompilerInstance::createDefaultOutputFile(bool Binary, StringRef InFile,
-  StringRef Extension,
-  bool RemoveFileOnSignal,
-  bool CreateMissingDirectories) {
+std::unique_ptr CompilerInstance::createDefaultOutputFile(
+bool Binary, StringRef InFile, StringRef Extension, bool 
RemoveFileOnSignal,
+bool CreateMissingDirectories, bool ForceUseTemporary) {
   StringRef OutputPath = getFrontendOpts().OutputFile;
   Optional> PathStorage;
   if (OutputPath.empty()) {
@@ -760,9 +758,8 @@ CompilerInstance::createDefaultOutputFile(bool Binary, 
StringRef InFile,
 }
   }
 
-  // Force a temporary file if RemoveFileOnSignal was disabled.
   return createOutputFile(OutputPath, Binary, RemoveFileOnSignal,
-  getFrontendOpts().UseTemporary || 
!RemoveFileOnSignal,
+  getFrontendOpts().UseTemporary || ForceUseTemporary,
   CreateMissingDirectories);
 }
 

diff  --git a/clang/lib/Frontend/FrontendActions.cpp 
b/clang/lib/Frontend/FrontendActions.cpp
index 6df57cbb45ae0..c6ebbdc8c04e1 100644
--- a/clang/lib/Frontend/FrontendActions.cpp
+++ b/clang/lib/Frontend/FrontendActions.cpp
@@ -239,7 +239,8 @@ 
GenerateModuleFromModuleMapAction::CreateOutputFile(CompilerInstance ,
   // Because this is exposed via libclang we must disable RemoveFileOnSignal.
   return CI.createDefaultOutputFile(/*Binary=*/true, InFile, /*Extension=*/"",
 /*RemoveFileOnSignal=*/false,
-/*CreateMissingDirectories=*/true);
+/*CreateMissingDirectories=*/true,
+/*ForceUseTemporary=*/true);
 }
 
 bool GenerateModuleInterfaceAction::BeginSourceFileAction(



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

[PATCH] D104299: Handle interactions between reserved identifier and user-defined suffixes

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

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

Thanks, looks good. Just some minor comments.




Comment at: clang/include/clang/Sema/Sema.h:4132
+  bool checkLiteralOperatorId(const CXXScopeSpec , const UnqualifiedId ,
+  bool isUDSuffix);
   LiteralOperatorLookupResult

Style nit: please start variables with uppercase letters.



Comment at: clang/lib/AST/Decl.cpp:1084-1086
   if (!II)
 if (const auto *FD = dyn_cast(this))
   II = FD->getLiteralIdentifier();

Can you now delete this along with the added code below?



Comment at: clang/lib/AST/Decl.cpp:1091
 
+  // already checked at lexing time
+  if (getDeclName().getCXXLiteralIdentifier())

Style nit: comments should start with a capital letter and end in a period.



Comment at: clang/lib/Sema/SemaExprCXX.cpp:489-499
+  if (!isUDSuffix) {
+// [over.literal] p8
+IdentifierInfo *II = Name.Identifier;
+auto Status = II->isReserved(PP.getLangOpts());
+auto Loc = Name.getBeginLoc();
+if (Status != ReservedIdentifierStatus::NotReserved &&
+!PP.getSourceManager().isInSystemHeader(Loc)) {

Hmm, interesting. This differs from the behavior of regular identifiers in that 
it will warn on both declarations and uses of reserved literal operator names. 
But I think that's actually a desirable difference! For example:

```
int _Foo(); // should warn here (if not in system header)
int k1 = _Foo(); // no need to warn here, and we shouldn't if the previous line 
is in a system header

int operator""_Foo();
int k2 = operator"" _Foo(); // should warn here, regardless of whether the 
previous line is in a system header
```

Given that each appearance can make a different choice in this regard, and that 
the problem can be fixed locally by removing the space, warning on each 
appearance seems like the right approach.



Comment at: clang/lib/Sema/SemaExprCXX.cpp:490
+  if (!isUDSuffix) {
+// [over.literal] p8
+IdentifierInfo *II = Name.Identifier;

It's useful to also include a brief quotation of the relevant text and a 
standard version, since paragraphs get moved around and renumbered over time.



Comment at: clang/lib/Sema/SemaExprCXX.cpp:497
+  Diag(Loc, diag::warn_reserved_extern_symbol)
+  << II << static_cast(Status);
+}

Can we produce a `FixItHint` to remove the space? (That might require the 
parser to pass through a little more information, such as the location for the 
end of the final string literal token, so we can reconstruct the space range 
here.)


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

https://reviews.llvm.org/D104299

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

[PATCH] D104494: [dfsan] Replace dfs$ prefix with .dfsan suffix

[George Balatsouras via Phabricator via cfe-commits]

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



Comment at: llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp:1134
+  Asm.replace(Pos, 1, Suffix + "@");
+}
 GV->getParent()->setModuleInlineAsm(Asm);

stephan.yichao.zhao wrote:
> Based on http://web.mit.edu/rhel-doc/3/rhel-as-en-3/symver.html, there must 
> be a @ in the .symver line after the first match.
> Please change  Pos != std::string::npos to be like
> ```
> Pos = Asm.find("@", Pos);
> assert(Pos != std::string::npos);
> ```
Done. Used `report_fatal_error` instead of `assert` since this can be triggered 
by user input. Ideally, I think we should be using some recoverable error 
mechanism, but since we're not doing it elsewhere, I'm not going to introduce 
this with this change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104494

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

[PATCH] D104350: [clang][AST] Make `getLocalOrImportedSubmoduleID` work with const `Module*`. NFC.

[Volodymyr Sapsai via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG722c51473c7a: [clang][AST] Make 
`getLocalOrImportedSubmoduleID` work with const `Module*`. (authored by 
vsapsai).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104350

Files:
  clang/include/clang/Serialization/ASTWriter.h
  clang/lib/Serialization/ASTWriter.cpp


Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -2476,11 +2476,11 @@
   }
 }
 
-unsigned ASTWriter::getLocalOrImportedSubmoduleID(Module *Mod) {
+unsigned ASTWriter::getLocalOrImportedSubmoduleID(const Module *Mod) {
   if (!Mod)
 return 0;
 
-  llvm::DenseMap::iterator Known = SubmoduleIDs.find(Mod);
+  auto Known = SubmoduleIDs.find(Mod);
   if (Known != SubmoduleIDs.end())
 return Known->second;
 
Index: clang/include/clang/Serialization/ASTWriter.h
===
--- clang/include/clang/Serialization/ASTWriter.h
+++ clang/include/clang/Serialization/ASTWriter.h
@@ -450,7 +450,7 @@
 
   /// A mapping from each known submodule to its ID number, which will
   /// be a positive integer.
-  llvm::DenseMap SubmoduleIDs;
+  llvm::DenseMap SubmoduleIDs;
 
   /// A list of the module file extension writers.
   std::vector>
@@ -671,7 +671,7 @@
   /// Retrieve or create a submodule ID for this module, or return 0 if
   /// the submodule is neither local (a submodle of the currently-written 
module)
   /// nor from an imported module.
-  unsigned getLocalOrImportedSubmoduleID(Module *Mod);
+  unsigned getLocalOrImportedSubmoduleID(const Module *Mod);
 
   /// Note that the identifier II occurs at the given offset
   /// within the identifier table.


Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -2476,11 +2476,11 @@
   }
 }
 
-unsigned ASTWriter::getLocalOrImportedSubmoduleID(Module *Mod) {
+unsigned ASTWriter::getLocalOrImportedSubmoduleID(const Module *Mod) {
   if (!Mod)
 return 0;
 
-  llvm::DenseMap::iterator Known = SubmoduleIDs.find(Mod);
+  auto Known = SubmoduleIDs.find(Mod);
   if (Known != SubmoduleIDs.end())
 return Known->second;
 
Index: clang/include/clang/Serialization/ASTWriter.h
===
--- clang/include/clang/Serialization/ASTWriter.h
+++ clang/include/clang/Serialization/ASTWriter.h
@@ -450,7 +450,7 @@
 
   /// A mapping from each known submodule to its ID number, which will
   /// be a positive integer.
-  llvm::DenseMap SubmoduleIDs;
+  llvm::DenseMap SubmoduleIDs;
 
   /// A list of the module file extension writers.
   std::vector>
@@ -671,7 +671,7 @@
   /// Retrieve or create a submodule ID for this module, or return 0 if
   /// the submodule is neither local (a submodle of the currently-written module)
   /// nor from an imported module.
-  unsigned getLocalOrImportedSubmoduleID(Module *Mod);
+  unsigned getLocalOrImportedSubmoduleID(const Module *Mod);
 
   /// Note that the identifier II occurs at the given offset
   /// within the identifier table.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 722c514 - [clang][AST] Make `getLocalOrImportedSubmoduleID` work with const `Module*`. NFC.

[Volodymyr Sapsai via cfe-commits]

Author: Volodymyr Sapsai
Date: 2021-06-17T17:29:12-07:00
New Revision: 722c51473c7a3bfe13d929734615a3b46d44c09a

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

LOG: [clang][AST] Make `getLocalOrImportedSubmoduleID` work with const 
`Module*`. NFC.

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

Added: 


Modified: 
clang/include/clang/Serialization/ASTWriter.h
clang/lib/Serialization/ASTWriter.cpp

Removed: 




diff  --git a/clang/include/clang/Serialization/ASTWriter.h 
b/clang/include/clang/Serialization/ASTWriter.h
index a7a659637902e..e4d92f0b0515c 100644
--- a/clang/include/clang/Serialization/ASTWriter.h
+++ b/clang/include/clang/Serialization/ASTWriter.h
@@ -450,7 +450,7 @@ class ASTWriter : public ASTDeserializationListener,
 
   /// A mapping from each known submodule to its ID number, which will
   /// be a positive integer.
-  llvm::DenseMap SubmoduleIDs;
+  llvm::DenseMap SubmoduleIDs;
 
   /// A list of the module file extension writers.
   std::vector>
@@ -671,7 +671,7 @@ class ASTWriter : public ASTDeserializationListener,
   /// Retrieve or create a submodule ID for this module, or return 0 if
   /// the submodule is neither local (a submodle of the currently-written 
module)
   /// nor from an imported module.
-  unsigned getLocalOrImportedSubmoduleID(Module *Mod);
+  unsigned getLocalOrImportedSubmoduleID(const Module *Mod);
 
   /// Note that the identifier II occurs at the given offset
   /// within the identifier table.

diff  --git a/clang/lib/Serialization/ASTWriter.cpp 
b/clang/lib/Serialization/ASTWriter.cpp
index 4cdcf53775de1..ca169c010555c 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -2476,11 +2476,11 @@ void 
ASTWriter::WritePreprocessorDetail(PreprocessingRecord ,
   }
 }
 
-unsigned ASTWriter::getLocalOrImportedSubmoduleID(Module *Mod) {
+unsigned ASTWriter::getLocalOrImportedSubmoduleID(const Module *Mod) {
   if (!Mod)
 return 0;
 
-  llvm::DenseMap::iterator Known = SubmoduleIDs.find(Mod);
+  auto Known = SubmoduleIDs.find(Mod);
   if (Known != SubmoduleIDs.end())
 return Known->second;
 



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

[PATCH] D104387: [clang-cl] Implement /external:I, /external:env, and EXTERNAL_INCLUDE support (PR36003)

[Nico Weber via Phabricator via cfe-commits]

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

That's a good argument. lgtm.




Comment at: clang/lib/Driver/ToolChains/MSVC.cpp:1273
+
+if (include_var) {
+  StringRef(*include_var)

Why not keep the definition of include_var in the if condition like it was on 
the rhs? (And do it for ext_include_var too)? They're only used in the 
respective if body, right? i.e. like so:

```
if (llvm::Optional include_var =
llvm::sys::Process::GetEnv("INCLUDE")) {
  StringRef(*include_var)
  .split(Dirs, ";", /*MaxSplit=*/-1, /*KeepEmpty=*/false);
}
if (llvm::Optional ext_include_var =
llvm::sys::Process::GetEnv("EXTERNAL_INCLUDE")) {
  StringRef(*ext_include_var)
  .split(Dirs, ";", /*MaxSplit=*/-1, /*KeepEmpty=*/false);
}
```

(Might even do `for (auto s : {"INCLUDE", "EXTERNAL_INCLUDE"}) ...` but that 
makes the FIXME harder to do later, so maybe don't do that part :) )



Comment at: clang/lib/Driver/ToolChains/MSVC.cpp:1255
+}
+  }
+

hans wrote:
> thakis wrote:
> > `/external:env` should happen after winsysroot I think. sysroots try to 
> > make builds hermetic and env vars defeat that.
> > 
> > I.e. this flag is more like the env var reads below than imsvc above – it 
> > reads the env mostly, instead of being a flag mostly.
> Hmm. I guess it depends on how people will use this flag, which isn't 
> entirely clear. 
> 
> The way I think about it, users might use this to point to third-party 
> libraries besides the system headers. For that reason it's different from the 
> env var reads below, which are about finding the system headers.
> 
> In particular, I don't think /nostdlibinc (/X) should disable these flags, 
> which is why I put it before that check.
> 
> I don't think people would combine this with /winsysroot, but if they want 
> to, I'm not sure we should prevent it.
I don't know what people in general want, but _I_ definitely want a mode that 
makes the compiler not read any env vars :) But fair enough, this flag 
explicitly opts in to reading env vars, so if I don't want that I can just not 
pass this flag.


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

https://reviews.llvm.org/D104387

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

[PATCH] D104494: [dfsan] Replace dfs$ prefix with .dfsan suffix

[stephan.yichao.zhao via Phabricator via cfe-commits]

stephan.yichao.zhao added inline comments.



Comment at: llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp:1134
+  Asm.replace(Pos, 1, Suffix + "@");
+}
 GV->getParent()->setModuleInlineAsm(Asm);

Based on http://web.mit.edu/rhel-doc/3/rhel-as-en-3/symver.html, there must be 
a @ in the .symver line after the first match.
Please change  Pos != std::string::npos to be like
```
Pos = Asm.find("@", Pos);
assert(Pos != std::string::npos);
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104494

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

[PATCH] D104494: [dfsan] Replace dfs$ prefix with .dfsan suffix

[Kostya Serebryany via Phabricator via cfe-commits]

kcc added a comment.

Yey, great idea! :) 
(I am not reviewing the code; but the change looks straightforward)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104494

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

[PATCH] D98726: [analyzer] Enabling MallocChecker to take up after SmartPtrModelling

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

xazax.hun added a comment.

We probably did not update this PR with the discussions we had offline. 
Basically we had a bug with a sink node due to calling into a destructor of 
`unique_ptr`. The catch is that the ctor was evalcalled, so the store did not 
have the correct bindings. Thus, after inlining the dtor the analyzer though we 
are reading garbage values from memory and a sink node was generated. 
SuppressOnSink machinery suppressed the leak warning.

The solution here is probably to EvalCall on the dtor as well so the analyzer 
does not end up thinking the code is reading garbage values from memory.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98726

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

[PATCH] D104300: [analyzer] Handle std::swap for std::unique_ptr

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

xazax.hun added inline comments.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h:435
 
+  void markNotInteresting(SymbolRef sym);
+

Bikeshedding: I wonder if we prefer `Uninteresting` to `NotInteresting`. Or 
alternatively, if we want to emphasize this is only the lack of 
interestingness, we could name it `removeInterestingness`. I do not have strong 
feelings about any of the options, I was just wondering if any of you has a 
preference.



Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:191
+// Check the first arg, if it is of std::unique_ptr type.
+assert(Call.getNumArgs() == 2 && "std::swap should have two arguments");
+const Expr *FirstArg = Call.getArgExpr(0);

I wonder about the value of this assertion. Shouldn`t 
`Call.isCalled(StdSwapCall)` already validate the number of arguments?



Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:193
+const Expr *FirstArg = Call.getArgExpr(0);
+if (!smartptr::isStdSmartPtr(FirstArg->getType()->getAsCXXRecordDecl())) {
+  return false;

Nit: we usually omit braces for simple `if` statements.



Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:458-467
+if (BR.isInteresting(FirstThisRegion) &&
+!BR.isInteresting(SecondThisRegion)) {
+  BR.markInteresting(SecondThisRegion);
+  BR.markNotInteresting(FirstThisRegion);
+}
+if (BR.isInteresting(SecondThisRegion) &&
+!BR.isInteresting(FirstThisRegion)) {

vsavchenko wrote:
> nit: these two pieces of code are very much the same
I guess we could make `markInteresting` take an optional bool and swap the 
interestingness unconditionally. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104300

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

[PATCH] D104475: [Clang][Codegen] emit noprofile IR Fn Attr for new Fn Attr no_profile

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

In D104475#2825795 , @MaskRay wrote:

> In D104475#2825772 , 
> @nickdesaulniers wrote:
>
>> In D104475#2825711 , @MaskRay 
>> wrote:
>>
>>> So if we don't want to offer guarantee for IR/C/C++ attributes, we can 
>>> document that users may need to additionally specify 
>>> `__attribute__((noinline))` to suppress inlining.
>>
>> I don't generally like that approach:
>
> If a guarantee would cause inferior optimization or we cannot think of all 
> the complication/fallout initially, I'd prefer that we keep the initial 
> semantics narrow.

The guarantee is more important than the optimization.

> If we cannot make a promise, don't provide a promise.
> No-suppressing inlining makes the attribute freely composable with other 
> attributes (https://lists.llvm.org/pipermail/llvm-dev/2021-April/150062.html )

IIUC, isn't @jdoerfert arguing that `optnone` and `noinline` being composable 
is _bad_?

>> 1. it's not easy for developers to validate their call call chains to ensure 
>> that a caller with a restrictive function attribute doesn't have 
>> unrestricted callers inlined into the restricted caller.
>> 2. that behavior opposes the principle of least surprise.
>
> Disagree. If the user wants a function not to be inlined, let them add 
> `__attribute__((noinline))`.
>
>> 3. We don't have a statement attribute for call sites to say "please don't 
>> inline this call" which would be fine grain. noinline is a course-grain 
>> hammer; what if we want to inline a callee into most callers but not this 
>> one caller that has such a restricted function attribute?
>
>
>
>> See also D91816  where I took the 
>> conservative approach for `no_stack_protector` and simply chose not to 
>> perform such inline substitution on caller and callee function attribute 
>> mismatch.  I find this behavior to be the least surprising, and the 
>> developer is provided with introspection as to why the compile did not 
>> perform such inlining via `-Rpass=inline` flag.
>
> I think D91816  was an unfortunate case. 
> That would require the inliner to understand every possible attribute.

Hyperbole. Only a few such function attributes would need such restrictions.

> That was the practical approach back then because it is not easy for the 
> kernel to cope.
> For new things we should strive for making the kernel do the right thing.
>
> For example, if a kernel function can sometimes be inlinable, sometimes not: 
> add a alias and add `__attribute__((noinline))` to that alias.

Interesting idea, but `noinline` is straight up broken in LLVM: 
https://godbolt.org/z/MoE1a7c3v. The Linux kernel relies on `noinline` meaning 
don't perform inline substitution.  That violates the principal of least 
surprise.  When a developer uses `noinline`, `no_stack_protector`, or 
`no_profile`, they mean DO NOT inline, insert a stack protector, or insert 
profiling/coverage instrumentation EVER.

Also debugging to find out that inlining was the cause of such violated 
expectations sucks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104475

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

[PATCH] D103750: [analyzer] Handle std::make_unique for SmartPtrModeling

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

xazax.hun added a comment.

I believe there are a couple of comments that are done but not marked 
accordingly. 
I agree with Artem, if we could craft code that fails due to not calling ctors, 
we should probably include them in the tests, even if they don't reflect the 
desired behavior they are a great source of documentation what is missing.




Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:151
+  const auto  = FD->getTemplateSpecializationArgs()->asArray();
+  if (TemplateArgs.size() == 0)
+return {};

Nit: consider `TemplateArgs.empty()`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103750

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

[PATCH] D98799: [UniqueLinkageName] Use consistent checks when mangling symbo linkage name and debug linkage name.

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added a comment.

I think these places have to be fixed too:
https://github.com/llvm/llvm-project/blob/bf9f21a28be171dc500cc68b4cb1fcd3fc33f229/clang/lib/CodeGen/CGBuiltin.cpp#L1705
https://github.com/llvm/llvm-project/blob/bf9f21a28be171dc500cc68b4cb1fcd3fc33f229/clang/lib/CodeGen/CGNonTrivialStruct.cpp#L476
https://github.com/llvm/llvm-project/blob/bf9f21a28be171dc500cc68b4cb1fcd3fc33f229/clang/lib/CodeGen/CGObjC.cpp#L3692
https://github.com/llvm/llvm-project/blob/bf9f21a28be171dc500cc68b4cb1fcd3fc33f229/clang/lib/CodeGen/CGObjC.cpp#L3776
https://github.com/llvm/llvm-project/blob/bf9f21a28be171dc500cc68b4cb1fcd3fc33f229/clang/lib/CodeGen/ItaniumCXXABI.cpp#L2618

This one is okay since it's creating a function that doesn't take any 
parameters:
https://github.com/llvm/llvm-project/blob/bf9f21a28be171dc500cc68b4cb1fcd3fc33f229/clang/lib/CodeGen/CGStmtOpenMP.cpp#L449


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98799

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

[PATCH] D88666: DirectoryWatcher: add an implementation for Windows

[Adrian McCarthy via Phabricator via cfe-commits]

amccarth added a comment.

In D88666#2825557 , @stella.stamenova 
wrote:

> One thing we've run into in the past is that running some of these tests **on 
> a drive other than the OS drive** can cause weird failure. I am not sure if 
> that may be the case here as well.

As long as it's a local drive, that _shouldn't_ be a problem.  I always run 
tests on a different drive than the OS system drive.

If it's a network drive, then, yeah, that would likely be a problem.  If I 
recall correctly, ReadDirectoryChangesW has substantial limitations when 
pointed at a remote drive.  The implementation should probably check that and 
signal an "unsupported" error.

Also note that Stella's sample log looks slightly different than the failures 
we were reproducing.  It's almost as if the initial scan never finished.  I 
haven't looked at that code, but I wonder if the file iteration is stuck in 
some kind of loop due to links or mount points or something.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88666

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

[PATCH] D104475: [Clang][Codegen] emit noprofile IR Fn Attr for new Fn Attr no_profile

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

In D104475#2825772 , @nickdesaulniers 
wrote:

> In D104475#2825711 , @MaskRay wrote:
>
>> So if we don't want to offer guarantee for IR/C/C++ attributes, we can 
>> document that users may need to additionally specify 
>> `__attribute__((noinline))` to suppress inlining.
>
> I don't generally like that approach:

If a guarantee would cause inferior optimization or we cannot think of all the 
fallout initially, I'd prefer that we keep the initial semantics narrow.
If we cannot make a promise, don't provide a promise.
No-suppressing inlining makes the attribute freely composable with other 
attributes (https://lists.llvm.org/pipermail/llvm-dev/2021-April/150062.html )

> 1. it's not easy for developers to validate their call call chains to ensure 
> that a caller with a restrictive function attribute doesn't have unrestricted 
> callers inlined into the restricted caller.
> 2. that behavior opposes the principle of least surprise.

Disagree. If the user wants a function not to be inlined, let them add 
`__attribute__((noinline))`.

> 3. We don't have a statement attribute for call sites to say "please don't 
> inline this call" which would be fine grain. noinline is a course-grain 
> hammer; what if we want to inline a callee into most callers but not this one 
> caller that has such a restricted function attribute?



> See also D91816  where I took the 
> conservative approach for `no_stack_protector` and simply chose not to 
> perform such inline substitution on caller and callee function attribute 
> mismatch.  I find this behavior to be the least surprising, and the developer 
> is provided with introspection as to why the compile did not perform such 
> inlining via `-Rpass=inline` flag.

I think D91816  was an unfortunate case. That 
would require the inliner to understand every possible attribute. That was the 
practical approach back then because it is not easy for the kernel to cope.
For new things we should strive for making the kernel do the right thing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104475

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

[PATCH] D104465: [clang][deps] Prevent PCH validation failures by padding minimized files

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

dexonsmith added a comment.

In D104465#2825343 , @dexonsmith 
wrote:

> A few other ideas:
>
> 1. Disable PCH validation when minimizing. (Is that less robust than your 
> current workaround? If so, can you explain why?)
> 2. Use the original PCH header in the scanning `-cc1`s (translate 
> `-include-pch` to `-include`) and switch back in the generated `-cc1`s (back 
> to `-include-pch`).
> 3. Embed instructions in the PCH for how to build it, and make a "minimized" 
> version of the PCH.

Two more options to consider:

4. If a compilation uses a PCH, use the original files for any input file 
transitively referenced by the PCH. Can be done by using a filesystem overlay 
that skips over the minimization layer for those files, or could tell the 
minimization layer not to minimize those files. You can figure out the files by 
adding a free function that preloads the PCH and extracts out all the input 
files.
5. Add support for building/using a PCH, and only support PCHes that are built 
this way. E.g., add a `-include-pch-auto` option or something. Scanner would 
use `-include`, spit out a command for building a PCH using explicit modules, 
and the generated `-cc1`s would use `-include-pch` to refer to it. This way all 
the modules are generated "in house".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104465

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

[PATCH] D104381: [analyzer] Added a test case for PR46264

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

@NoQ:

> I guess there might be some value in building an older Clang from around when 
> the bug was reported so that to see if the crash was indeed there and we're 
> not just reproducing it wrong. It would also allow us to understand where we 
> fixed it through bisecting. But I don't insist.

Bisecting through a year+ of commits is painful. I thought about this but 
didn't dare. What I can do is to check the failure on a issue-date commit.




Comment at: clang/test/Analysis/diagnostics/PR46264.cpp:4
+// PR46264
+// This case shall not warn about dereference of void*
+namespace ns1 {

NoQ wrote:
> I think you mean this case shall not crash with an assertion failure.
> 
> "Warn" exclusively means "display a warning to the user".
I'll make changes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104381

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

[PATCH] D104475: [Clang][Codegen] emit noprofile IR Fn Attr for new Fn Attr no_profile

[Marco Elver via Phabricator via cfe-commits]

melver added a comment.

In D104475#2825772 , @nickdesaulniers 
wrote:

> In D104475#2825711 , @MaskRay wrote:
>
>> So if we don't want to offer guarantee for IR/C/C++ attributes, we can 
>> document that users may need to additionally specify 
>> `__attribute__((noinline))` to suppress inlining.
>
> I don't generally like that approach:
>
> 1. it's not easy for developers to validate their call call chains to ensure 
> that a caller with a restrictive function attribute doesn't have unrestricted 
> callers inlined into the restricted caller.
> 2. that behavior opposes the principle of least surprise.
> 3. We don't have a statement attribute for call sites to say "please don't 
> inline this call" which would be fine grain. noinline is a course-grain 
> hammer; what if we want to inline a callee into most callers but not this one 
> caller that has such a restricted function attribute?
>
> See also D91816  where I took the 
> conservative approach for `no_stack_protector` and simply chose not to 
> perform such inline substitution on caller and callee function attribute 
> mismatch.  I find this behavior to be the least surprising, and the developer 
> is provided with introspection as to why the compile did not perform such 
> inlining via `-Rpass=inline` flag.

Agree. I think we just raced to say the same thing. ;-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104475

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

[PATCH] D104475: [Clang][Codegen] emit noprofile IR Fn Attr for new Fn Attr no_profile

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

In D104475#2825711 , @MaskRay wrote:

> So if we don't want to offer guarantee for IR/C/C++ attributes, we can 
> document that users may need to additionally specify 
> `__attribute__((noinline))` to suppress inlining.

I don't generally like that approach:

1. it's not easy for developers to validate their call call chains to ensure 
that a caller with a restrictive function attribute doesn't have unrestricted 
callers inlined into the restricted caller.
2. that behavior opposes the principle of least surprise.
3. We don't have a statement attribute for call sites to say "please don't 
inline this call" which would be fine grain. noinline is a course-grain hammer; 
what if we want to inline a callee into most callers but not this one caller 
that has such a restricted function attribute?

See also D91816  where I took the conservative 
approach for `no_stack_protector` and simply chose not to perform such inline 
substitution on caller and callee function attribute mismatch.  I find this 
behavior to be the least surprising, and the developer is provided with 
introspection as to why the compile did not perform such inlining via 
`-Rpass=inline` flag.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104475

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

[PATCH] D104475: [Clang][Codegen] emit noprofile IR Fn Attr for new Fn Attr no_profile

[Marco Elver via Phabricator via cfe-commits]

melver added a comment.

In D104475#2825711 , @MaskRay wrote:

> In D104475#2825666 , @melver wrote:
>
>> In D104475#2825297 , @MaskRay 
>> wrote:
>>
>>> Should `no_profile` specify the inlining behavior? Though `no_sanitize_*` 
>>> don't specify that, either.
>>
>> I think it's somehow implicit, but I can't quite say how. There are some 
>> tests that check this, e.g.
>> compiler-rt/test/asan/TestCases/inline.cpp
>> [...]
>> noinstr does add noinline, but other uses of the attribute alone might not. 
>> But in the end inlining is a red herring, it just seems to be the easiest 
>> way to enforce the promised contract.  See 
>> https://lore.kernel.org/lkml/canpmjnnrz5ovkb6pe7k6gjfogbht_zhypkng9ad+kjndzk7...@mail.gmail.com/
>> [...]
>
> Thanks for the comments.  Recently there is a discussion about LLVM IR 
> function attribute `nointeropt` (similar to a debugging-only GCC function 
> attribute `noipa`).
> People tend to think attributes should be orthogonal. I think not suppressing 
> inlining for the IR attribute `no_profile` is nice.
> Ideally the GNU function attribute (C/C++) does not diverge much from the IR 
> semantics.
> So if we don't want to offer guarantee for IR/C/C++ attributes, we can 
> document that users may need to additionally specify 
> `__attribute__((noinline))` to suppress inlining.

As long as it remains practical. I don't want us ending up with a mess of an 
attribute like we once had for KASAN: __no_kasan_or_inline -- because 'static 
inline __no_sanitize_address foo(void) { ... }' did the wrong thing, because a) 
attribute was incompatible with 'inline', and b) caused instrumentation if it 
was inlined (AFAIK that behaviour is fixed and was a GCC-only bug).

Essentially, as long as no_foo means that code in function where no_foo is 
specified is never instrumented by foo, then we're good. If that code is 
inlined or not doesn't matter, as long as the contract is honored. If that's 
not the case we have a problem because we need ugly workarounds for what are 
essentially compiler bugs (contract not honored).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104475

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

[PATCH] D99810: [ifs] Prepare llvm-ifs for elfabi/ifs merging.

[Haowei Wu via Phabricator via cfe-commits]

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



Comment at: llvm/include/llvm/InterfaceStub/ELFObjHandler.h:25
 
 namespace elfabi {
 

phosek wrote:
> Shouldn't this be in namespace `ifs`?
This was renamed to ifs in D100139.  Rename it here will need some changes in 
elfabi code, which is unnecessary (and will be deleted in D100139)



Comment at: llvm/lib/InterfaceStub/IFSStub.cpp:65
+uint8_t elfabi::convertIFSBitWidthToELF(IFSBitWidthType BitWidth) {
+  return BitWidth == IFSBitWidthType::IFS32 ? ELF::ELFCLASS32 : 
ELF::ELFCLASS64;
+}

phosek wrote:
> I'd consider using `switch` here so if someone adds a new entry to the 
> `IFSBitWidthType` enum, it gets caught by the compiler.
I changed it to switch and I added `llvm_unreachable("unkown bitwidth");` to 
cases where an unknown bitwidth or endianness is used. Please let me know if it 
is OK to do so.

The Unknown enum is reserved for ifs file that has unknown bitwidth or 
endianness field and will trigger ifs tool's early termination. So it shouldn't 
be seen when IFS library is writing an ELF file.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99810

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

[PATCH] D104491: [Docs][Clang][Attr] mark no_stack_protector+no_sanitize GCC compatible

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers created this revision.
nickdesaulniers added reviewers: void, melver.
Herald added a reviewer: aaron.ballman.
nickdesaulniers requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

GCC has supported the function attributes
__attribute__((no_stack_protector)) since GCC 11 and
__attribute__((no_sanitize(""))) since GCC 8.

Signed-off-by: Nick Desaulniers 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D104491

Files:
  clang/include/clang/Basic/Attr.td


Index: clang/include/clang/Basic/Attr.td
===
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -1979,7 +1979,7 @@
 def : MutualExclusions<[AlwaysInline, NotTailCalled]>;
 
 def NoStackProtector : InheritableAttr {
-  let Spellings = [Clang<"no_stack_protector">];
+  let Spellings = [GCC<"no_stack_protector">];
   let Subjects = SubjectList<[Function]>;
   let Documentation = [NoStackProtectorDocs];
   let SimpleHandler = 1;
@@ -2888,7 +2888,7 @@
 }
 
 def NoSanitize : InheritableAttr {
-  let Spellings = [Clang<"no_sanitize">];
+  let Spellings = [GCC<"no_sanitize">];
   let Args = [VariadicStringArgument<"Sanitizers">];
   let Subjects = SubjectList<[Function, ObjCMethod, GlobalVar], ErrorDiag>;
   let Documentation = [NoSanitizeDocs];


Index: clang/include/clang/Basic/Attr.td
===
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -1979,7 +1979,7 @@
 def : MutualExclusions<[AlwaysInline, NotTailCalled]>;
 
 def NoStackProtector : InheritableAttr {
-  let Spellings = [Clang<"no_stack_protector">];
+  let Spellings = [GCC<"no_stack_protector">];
   let Subjects = SubjectList<[Function]>;
   let Documentation = [NoStackProtectorDocs];
   let SimpleHandler = 1;
@@ -2888,7 +2888,7 @@
 }
 
 def NoSanitize : InheritableAttr {
-  let Spellings = [Clang<"no_sanitize">];
+  let Spellings = [GCC<"no_sanitize">];
   let Args = [VariadicStringArgument<"Sanitizers">];
   let Subjects = SubjectList<[Function, ObjCMethod, GlobalVar], ErrorDiag>;
   let Documentation = [NoSanitizeDocs];
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D99797: [analyzer] Implemented RangeSet::Factory::unite function to handle intersections and adjacency

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov updated this revision to Diff 352851.
ASDenysPetrov added a comment.

Updated. Removed `F` as flag. Replaced `goto` with closure. Detailed comments 
and fixed typos.

@vsavchenko made changes according your suggestions.


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

https://reviews.llvm.org/D99797

Files:
  
clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h
  clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
  clang/unittests/StaticAnalyzer/RangeSetTest.cpp

Index: clang/unittests/StaticAnalyzer/RangeSetTest.cpp
===
--- clang/unittests/StaticAnalyzer/RangeSetTest.cpp
+++ clang/unittests/StaticAnalyzer/RangeSetTest.cpp
@@ -35,12 +35,34 @@
   const RangeSet ) {
   return OS << toString(Set);
 }
+// We need it here for better fail diagnostics from gtest.
+LLVM_ATTRIBUTE_UNUSED static std::ostream <<(std::ostream ,
+  const Range ) {
+  return OS << toString(R);
+}
 
 } // namespace ento
 } // namespace clang
 
 namespace {
 
+template  struct TestValues {
+  static constexpr T MIN = std::numeric_limits::min();
+  static constexpr T MAX = std::numeric_limits::max();
+  // MID is a value in the middle of the range
+  // which unary minus does not affect on,
+  // e.g. int8/int32(0), uint8(128), uint32(2147483648).
+  static constexpr T MID =
+  std::is_signed::value ? 0 : ~(static_cast(-1) / static_cast(2));
+  static constexpr T A = MID - (MAX - MID) / 3 * 2;
+  static constexpr T B = MID - (MAX - MID) / 3;
+  static constexpr T C = -B;
+  static constexpr T D = -A;
+
+  static_assert(MIN < A && A < B && B < MID && MID < C && C < D && D < MAX,
+"Values shall be in an ascending order");
+};
+
 template  class RangeSetTest : public testing::Test {
 public:
   // Init block
@@ -55,24 +77,11 @@
   using RawRange = std::pair;
   using RawRangeSet = std::initializer_list;
 
-  static constexpr BaseType getMin() {
-return std::numeric_limits::min();
-  }
-  static constexpr BaseType getMax() {
-return std::numeric_limits::max();
-  }
-  static constexpr BaseType getMid() {
-return isSigned() ? 0 : ~(fromInt(-1) / fromInt(2));
-  }
-
-  static constexpr bool isSigned() { return std::is_signed::value; }
-  static constexpr BaseType fromInt(int X) { return static_cast(X); }
-
-  static llvm::APSInt Base;
   const llvm::APSInt (BaseType X) {
-llvm::APSInt Dummy = Base;
-Dummy = X;
-return BVF.getValue(Dummy);
+static llvm::APSInt Base{sizeof(BaseType) * 8,
+ std::is_unsigned::value};
+Base = X;
+return BVF.getValue(Base);
   }
 
   Range from(const RawRange ) {
@@ -160,7 +169,7 @@
 
   void checkAdd(RawRangeSet RawLHS, RawRangeSet RawRHS,
 RawRangeSet RawExpected) {
-wrap(::checkAddImpl, RawRHS, RawLHS, RawExpected);
+wrap(::checkAddImpl, RawLHS, RawRHS, RawExpected);
   }
 
   void checkAdd(RawRangeSet RawLHS, BaseType RawRHS, RawRangeSet RawExpected) {
@@ -168,6 +177,29 @@
  RawExpected);
   }
 
+  template 
+  void checkUniteImpl(RangeSet LHS, RHSType RHS, RangeSet Expected) {
+RangeSet Result = F.unite(LHS, RHS);
+EXPECT_EQ(Result, Expected)
+<< "while uniting " << toString(LHS) << " and " << toString(RHS);
+  }
+
+  void checkUnite(RawRangeSet RawLHS, RawRange RawRHS,
+  RawRangeSet RawExpected) {
+wrap(::checkUniteImpl, RawLHS, RawRHS, RawExpected);
+  }
+
+  void checkUnite(RawRangeSet RawLHS, RawRangeSet RawRHS,
+  RawRangeSet RawExpected) {
+wrap(::checkUniteImpl, RawLHS, RawRHS, RawExpected);
+  }
+
+  void checkUnite(RawRangeSet RawLHS, BaseType RawRHS,
+  RawRangeSet RawExpected) {
+wrap(::checkUniteImpl, RawLHS, RawRHS,
+ RawExpected);
+  }
+
   void checkDeleteImpl(const llvm::APSInt , RangeSet From,
RangeSet Expected) {
 RangeSet Result = F.deletePoint(From, Point);
@@ -183,29 +215,19 @@
 
 } // namespace
 
-template 
-llvm::APSInt RangeSetTest::Base{sizeof(BaseType) * 8, !isSigned()};
-
 using IntTypes = ::testing::Types;
 TYPED_TEST_SUITE(RangeSetTest, IntTypes, );
 
 TYPED_TEST(RangeSetTest, RangeSetNegateTest) {
-  // Use next values of the range {MIN, A, B, MID, C, D, MAX}.
-
-  constexpr TypeParam MIN = TestFixture::getMin();
-  constexpr TypeParam MAX = TestFixture::getMax();
-  // MID is a value in the middle of the range
-  // which unary minus does not affect on,
-  // e.g. int8/int32(0), uint8(128), uint32(2147483648).
-  constexpr TypeParam MID = TestFixture::getMid();
-  constexpr TypeParam A = MID - TestFixture::fromInt(42 + 42);
-  constexpr TypeParam B = MID - TestFixture::fromInt(42);
-  constexpr TypeParam C = -B;
-  constexpr TypeParam D = -A;
-
-  static_assert(MIN < A && A < B && B < MID && MID < C && C < D && D < MAX,
-  

[PATCH] D104475: [Clang][Codegen] emit noprofile IR Fn Attr for new Fn Attr no_profile

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

In D104475#2825666 , @melver wrote:

> In D104475#2825297 , @MaskRay wrote:
>
>> Should `no_profile` specify the inlining behavior? Though `no_sanitize_*` 
>> don't specify that, either.
>
> I think it's somehow implicit, but I can't quite say how. There are some 
> tests that check this, e.g.
> compiler-rt/test/asan/TestCases/inline.cpp
> [...]
> noinstr does add noinline, but other uses of the attribute alone might not. 
> But in the end inlining is a red herring, it just seems to be the easiest way 
> to enforce the promised contract.  See 
> https://lore.kernel.org/lkml/canpmjnnrz5ovkb6pe7k6gjfogbht_zhypkng9ad+kjndzk7...@mail.gmail.com/
> [...]

Thanks for the comments.  Recently there is a discussion about LLVM IR function 
attribute `nointeropt` (similar to a debugging-only GCC function attribute 
`noipa`).
People tend to think attributes should be orthogonal. I think not suppressing 
for the IR attribute `no_profile` is nice.
The GNU function attribute (C/C++) may be less clear. We can document that 
users may need to additionally specify `noinline` to suppress inlining.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104475

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

[PATCH] D104155: Add documentation for -fsanitize-address-use-after-return.

[Kevin Athey via Phabricator via cfe-commits]

kda updated this revision to Diff 352849.
kda marked an inline comment as done.
kda added a comment.
Herald added a subscriber: dang.

- Responding to comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104155

Files:
  clang/docs/AddressSanitizer.rst
  clang/docs/UsersManual.rst
  clang/include/clang/Driver/Options.td


Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -1587,7 +1587,7 @@
   : Joined<["-"], "fsanitize-address-use-after-return=">,
 MetaVarName<"">,
 Flags<[CC1Option]>,
-HelpText<"Select the mode of detecting stack use-after-return in 
AddressSanitizer">,
+HelpText<"Select the mode of detecting stack use-after-return in 
AddressSanitizer: never | runtime (default) | always">,
 Group,
 Values<"never,runtime,always">,
 NormalizedValuesScope<"llvm::AsanDetectStackUseAfterReturnMode">,
Index: clang/docs/UsersManual.rst
===
--- clang/docs/UsersManual.rst
+++ clang/docs/UsersManual.rst
@@ -3724,6 +3724,8 @@
   Enable linker dead stripping of globals in 
AddressSanitizer
   -fsanitize-address-poison-custom-array-cookie
   Enable poisoning array cookies when using custom 
operator new[] in AddressSanitizer
+  -fsanitize-address-use-after-return=
+  Select the mode of detecting stack 
use-after-return in AddressSanitizer: never | runtime (default) | always
   -fsanitize-address-use-after-scope
   Enable use-after-scope detection in 
AddressSanitizer
   -fsanitize-address-use-odr-indicator
Index: clang/docs/AddressSanitizer.rst
===
--- clang/docs/AddressSanitizer.rst
+++ clang/docs/AddressSanitizer.rst
@@ -14,8 +14,9 @@
 
 * Out-of-bounds accesses to heap, stack and globals
 * Use-after-free
-* Use-after-return (runtime flag 
`ASAN_OPTIONS=detect_stack_use_after_return=1`)
-* Use-after-scope (clang flag `-fsanitize-address-use-after-scope`)
+* Use-after-return (clang flag 
``-fsanitize-address-use-after-return=(never|runtime|always)`` default: 
``runtime``)
+* Enable ``runtime`` with: ``ASAN_OPTIONS=detect_stack_use_after_return=1``
+* Use-after-scope (clang flag ``-fsanitize-address-use-after-scope``)
 * Double-free, invalid free
 * Memory leaks (experimental)
 
@@ -136,6 +137,27 @@
 
 Note that this option is not supported on macOS.
 
+Stack Use After Return (UAR)
+
+
+AddressSanitizer (``-fsanitize=address``) can optionally detect stack use after
+return problems.
+This is available by default, or explicitly
+(``-fsanitize-address-use-after-return=runtime``).
+To enable this check at runtime, set the environment variable
+``ASAN_OPTIONS=detect_stack_use_after_return=1``.
+
+Enabling this check (``-fsanitize-address-use-after-return=always``) will
+reduce code size.  The code size may be reduced further by completely
+eliminating this check (``-fsanitize-address-use-after-return=never``).
+
+To summarize: ``-fsanitize-address-use-after-return=``
+  * ``never``: Completely disables detection of UAR errors (reduces code size).
+  * ``runtime``: Adds the code for detection, but must be enabled via the
+runtime environment (``ASAN_OPTIONS=detect_stack_use_after_return=1``).
+  * ``always``: Enables detection of UAR errors in all cases. (reduces code
+size, but not as much as ``never``).
+
 Memory leak detection
 -
 


Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -1587,7 +1587,7 @@
   : Joined<["-"], "fsanitize-address-use-after-return=">,
 MetaVarName<"">,
 Flags<[CC1Option]>,
-HelpText<"Select the mode of detecting stack use-after-return in AddressSanitizer">,
+HelpText<"Select the mode of detecting stack use-after-return in AddressSanitizer: never | runtime (default) | always">,
 Group,
 Values<"never,runtime,always">,
 NormalizedValuesScope<"llvm::AsanDetectStackUseAfterReturnMode">,
Index: clang/docs/UsersManual.rst
===
--- clang/docs/UsersManual.rst
+++ clang/docs/UsersManual.rst
@@ -3724,6 +3724,8 @@
   Enable linker dead stripping of globals in AddressSanitizer
   -fsanitize-address-poison-custom-array-cookie
   Enable poisoning array cookies when using custom operator new[] in AddressSanitizer
+  -fsanitize-address-use-after-return=
+  Select the mode of detecting stack use-after-return 

[PATCH] D103930: [clang][HeaderSearch] Fix implicit module when using header maps

[Bruno Cardoso Lopes via Phabricator via cfe-commits]

bruno added a comment.

@dexonsmith and @jansvoboda11 thanks for the fast reply and the extra testing.




Comment at: clang/lib/Lex/HeaderSearch.cpp:421
+  auto FixupSearchPathAndFindUsableModule =
+  [&](auto file) -> Optional {
 if (SearchPath) {

Can you capitalize the first letter in the param?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103930

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

[PATCH] D104248: [compiler-rt][hwasan] Refactor Thread::Init

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added a comment.

Will submit around end of day if there's no more comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104248

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

[PATCH] D104475: [Clang][Codegen] emit noprofile IR Fn Attr for new Fn Attr no_profile

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers updated this revision to Diff 352844.
nickdesaulniers marked 3 inline comments as done.
nickdesaulniers added a comment.

- fix td files and test CHECKs


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104475

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/test/CodeGen/fprofile.c
  clang/test/Misc/pragma-attribute-supported-attributes-list.test


Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -99,6 +99,7 @@
 // CHECK-NEXT: NoMerge (SubjectMatchRule_function)
 // CHECK-NEXT: NoMicroMips (SubjectMatchRule_function)
 // CHECK-NEXT: NoMips16 (SubjectMatchRule_function)
+// CHECK-NEXT: NoProfileFunction (SubjectMatchRule_function)
 // CHECK-NEXT: NoSanitize (SubjectMatchRule_function, 
SubjectMatchRule_objc_method, SubjectMatchRule_variable_is_global)
 // CHECK-NEXT: NoSanitizeSpecific (SubjectMatchRule_function, 
SubjectMatchRule_variable_is_global)
 // CHECK-NEXT: NoSpeculativeLoadHardening (SubjectMatchRule_function, 
SubjectMatchRule_objc_method)
Index: clang/test/CodeGen/fprofile.c
===
--- /dev/null
+++ clang/test/CodeGen/fprofile.c
@@ -0,0 +1,21 @@
+// RUN: %clang_cc1 -fprofile-instrument=llvm -disable-llvm-passes \
+// RUN:   -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -fprofile-instrument=csllvm -disable-llvm-passes \
+// RUN:   -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -fprofile-instrument=clang -disable-llvm-passes \
+// RUN:   -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -fprofile-arcs -disable-llvm-passes \
+// RUN:   -emit-llvm -o - %s | FileCheck %s
+int g(int);
+
+void __attribute__((no_profile)) no_instr() {
+// CHECK: define {{.*}} void @no_instr() [[ATTR:#[0-9]+]]
+}
+
+void instr(void) {
+// CHECK: define {{.*}} void @instr() [[ATTR2:#[0-9]+]]
+}
+// CHECK: attributes [[ATTR]] = {{.*}} noprofile
+// CHECK: attributes [[ATTR2]] = {
+// CHECK-NOT: noprofile
+// CHECK: }
Index: clang/lib/CodeGen/CodeGenFunction.cpp
===
--- clang/lib/CodeGen/CodeGenFunction.cpp
+++ clang/lib/CodeGen/CodeGenFunction.cpp
@@ -893,6 +893,9 @@
   if (D && D->hasAttr())
 Fn->addFnAttr("cfi-canonical-jump-table");
 
+  if (D && D->hasAttr())
+Fn->addFnAttr(llvm::Attribute::NoProfile);
+
   if (getLangOpts().OpenCL) {
 // Add metadata for a kernel function.
 if (const FunctionDecl *FD = dyn_cast_or_null(D))
Index: clang/include/clang/Basic/AttrDocs.td
===
--- clang/include/clang/Basic/AttrDocs.td
+++ clang/include/clang/Basic/AttrDocs.td
@@ -2559,6 +2559,17 @@
   }];
 }
 
+def NoProfileDocs : Documentation {
+  let Category = DocCatFunction;
+  let Content = [{
+Use the ``no_profile`` attribute on a function declaration to denote that the
+compiler should not instrument the function with profile related
+instrumentation, such as via the
+``-fprofile-generate`` / ``-fprofile-instr-generate`` /
+``-fcs-profile-generate`` / ``-fprofile-arcs`` flags.
+}];
+}
+
 def NoSanitizeDocs : Documentation {
   let Category = DocCatFunction;
   let Content = [{
Index: clang/include/clang/Basic/Attr.td
===
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -1970,6 +1970,13 @@
   let SimpleHandler = 1;
 }
 
+def NoProfileFunction : InheritableAttr {
+  let Spellings = [Clang<"no_profile">];
+  let Subjects = SubjectList<[Function]>;
+  let Documentation = [NoProfileDocs];
+  let SimpleHandler = 1;
+}
+
 def NotTailCalled : InheritableAttr {
   let Spellings = [Clang<"not_tail_called">];
   let Subjects = SubjectList<[Function]>;


Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -99,6 +99,7 @@
 // CHECK-NEXT: NoMerge (SubjectMatchRule_function)
 // CHECK-NEXT: NoMicroMips (SubjectMatchRule_function)
 // CHECK-NEXT: NoMips16 (SubjectMatchRule_function)
+// CHECK-NEXT: NoProfileFunction (SubjectMatchRule_function)
 // CHECK-NEXT: NoSanitize (SubjectMatchRule_function, SubjectMatchRule_objc_method, SubjectMatchRule_variable_is_global)
 // CHECK-NEXT: NoSanitizeSpecific (SubjectMatchRule_function, SubjectMatchRule_variable_is_global)
 // CHECK-NEXT: NoSpeculativeLoadHardening (SubjectMatchRule_function, SubjectMatchRule_objc_method)
Index: clang/test/CodeGen/fprofile.c

[PATCH] D104475: [Clang][Codegen] emit noprofile IR Fn Attr for new Fn Attr no_profile

[Marco Elver via Phabricator via cfe-commits]

melver added a comment.

In D104475#2825297 , @MaskRay wrote:

> Should `no_profile` specify the inlining behavior? Though `no_sanitize_*` 
> don't specify that, either.

I think it's somehow implicit, but I can't quite say how. There are some tests 
that check this, e.g.
compiler-rt/test/asan/TestCases/inline.cpp

> As I commented on https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80223 , the 
> Linux kernel specifies `noinline` so the exact inlining behavior doesn't 
> matter.
>
>   #define noinstr \
>   noinline notrace __attribute((__section__(".noinstr.text")))\
>   __no_kcsan __no_sanitize_address

noinstr does add noinline, but other uses of the attribute alone might not. But 
in the end inlining is a red herring, it just seems to be the easiest way to 
enforce the promised contract.  See 
https://lore.kernel.org/lkml/canpmjnnrz5ovkb6pe7k6gjfogbht_zhypkng9ad+kjndzk7...@mail.gmail.com/

In particular:

> But the contract of
> "no_sanitize" is "that a particular instrumentation or set of
> instrumentations should not be applied". That contract is broken if a
> function is instrumented, however that may happen.

Also always_inline needs checking, because it's a little special:

 - An __always_inline function inlined into a __no_sanitize function is not 
 instrumented
 - An __always_inline function inlined into an instrumented function is 
 instrumented

This was also previously discussed here: 
https://lore.kernel.org/lkml/CANpmjNMTsY_8241bS7=xafqvzhflrvekv_um4aduwe_kh3r...@mail.gmail.com/


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104475

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

[PATCH] D104459: [clang][lex] Ensure minimizer output is never larger than input

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

dexonsmith added a comment.

Another thing to consider: a client that wants the minimized source to be "no 
bigger than" the original source can use the original source if it ends up 
growing. For example, https://reviews.llvm.org/D104462 could check the 
resulting size, and just return the original source in the (extremely 
unlikely?) corner case where the minimized sources are bigger.

In that case, we wouldn't need this patch's assertion.

I bet some of these changes are good to do anyway (like dropping the 
`/*invalid*/` comment, which was useful as a canary-in-the-coal-mine when 
testing the prototype on large bodies of sources (and maybe has outlived its 
usefulness)), but you could skip the fiddly changes here that complicate the 
logic and make the output less canonical and harder to read, like "maintain 0 
whitespace after macro names" and "maintain missing newline at EOF".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104459

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

[PATCH] D104342: [IR] convert warn-stack-size from module attr to fn attr

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers updated this revision to Diff 352841.
nickdesaulniers marked an inline comment as done.
nickdesaulniers added a comment.

- verifier checks, inliner test, use base 10 radix


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104342

Files:
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/Frontend/fwarn-stack-size.c
  llvm/docs/LangRef.rst
  llvm/include/llvm/IR/Module.h
  llvm/lib/CodeGen/PrologEpilogInserter.cpp
  llvm/lib/IR/Module.cpp
  llvm/lib/IR/Verifier.cpp
  llvm/test/CodeGen/ARM/warn-stack.ll
  llvm/test/CodeGen/X86/warn-stack.ll
  llvm/test/Linker/warn-stack-frame.ll
  llvm/test/Transforms/Inline/warn-stack-size.ll
  llvm/test/Verifier/invalid-warn-stack-size.ll

Index: llvm/test/Verifier/invalid-warn-stack-size.ll
===
--- /dev/null
+++ llvm/test/Verifier/invalid-warn-stack-size.ll
@@ -0,0 +1,10 @@
+; RUN: not opt -passes=verify %s -disable-output 2>&1 | FileCheck %s
+define void @foo() "warn-stack-size"="42" { ret void }
+define void @bar() "warn-stack-size"="-1" { ret void }
+define void @baz() "warn-stack-size"="9" { ret void }
+define void @qux() "warn-stack-size"="a lot lol" { ret void }
+
+; CHECK-NOT: "warn-stack-size" takes an unsigned integer: 42
+; CHECK: "warn-stack-size" takes an unsigned integer: -1
+; CHECK: "warn-stack-size" takes an unsigned integer: 9
+; CHECK: "warn-stack-size" takes an unsigned integer: a lot lol
Index: llvm/test/Transforms/Inline/warn-stack-size.ll
===
--- /dev/null
+++ llvm/test/Transforms/Inline/warn-stack-size.ll
@@ -0,0 +1,24 @@
+; RUN: opt -passes=inline -S %s | FileCheck %s
+
+; The caller should NOT inherit the callee's "warn-stack-size" value.
+define void @foo() "warn-stack-size"="42" { ret void }
+; CHECK: @foo(
+; CHECK-NEXT:ret void
+;
+define void @bar() "warn-stack-size"="7" {
+; CHECK: @bar() [[ATTR0:#[0-9]+]] {
+; CHECK-NEXT:ret void
+;
+  call void @foo()
+  ret void
+}
+define void @baz() "warn-stack-size"="99" {
+; CHECK: @baz() [[ATTR1:#[0-9]+]] {
+; CHECK-NEXT:ret void
+;
+  call void @bar()
+  ret void
+}
+
+; CHECK: attributes [[ATTR0]] = {{.*}} "warn-stack-size"="7"
+; CHECK: attributes [[ATTR1]] = {{.*}} "warn-stack-size"="99"
Index: llvm/test/Linker/warn-stack-frame.ll
===
--- llvm/test/Linker/warn-stack-frame.ll
+++ /dev/null
@@ -1,16 +0,0 @@
-; RUN: split-file %s %t
-; RUN: llvm-link %t/main.ll %t/match.ll
-; RUN: not llvm-link %t/main.ll %t/mismatch.ll 2>&1 | \
-; RUN:   FileCheck --check-prefix=CHECK-MISMATCH %s
-
-; CHECK-MISMATCH: error: linking module flags 'warn-stack-size': IDs have conflicting values
-
-;--- main.ll
-!llvm.module.flags = !{!0}
-!0 = !{i32 1, !"warn-stack-size", i32 80}
-;--- match.ll
-!llvm.module.flags = !{!0}
-!0 = !{i32 1, !"warn-stack-size", i32 80}
-;--- mismatch.ll
-!llvm.module.flags = !{!0}
-!0 = !{i32 1, !"warn-stack-size", i32 81}
Index: llvm/test/CodeGen/X86/warn-stack.ll
===
--- llvm/test/CodeGen/X86/warn-stack.ll
+++ llvm/test/CodeGen/X86/warn-stack.ll
@@ -4,7 +4,7 @@
 ; 
 
 ; CHECK-NOT: nowarn
-define void @nowarn() nounwind ssp {
+define void @nowarn() nounwind ssp "warn-stack-size"="80" {
 entry:
   %buffer = alloca [12 x i8], align 1
   %arraydecay = getelementptr inbounds [12 x i8], [12 x i8]* %buffer, i64 0, i64 0
@@ -13,7 +13,7 @@
 }
 
 ; CHECK: warning: stack size limit exceeded (88) in warn
-define void @warn() nounwind ssp {
+define void @warn() nounwind ssp "warn-stack-size"="80" {
 entry:
   %buffer = alloca [80 x i8], align 1
   %arraydecay = getelementptr inbounds [80 x i8], [80 x i8]* %buffer, i64 0, i64 0
@@ -22,6 +22,3 @@
 }
 
 declare void @doit(i8*)
-
-!llvm.module.flags = !{!0}
-!0 = !{i32 1, !"warn-stack-size", i32 80}
Index: llvm/test/CodeGen/ARM/warn-stack.ll
===
--- llvm/test/CodeGen/ARM/warn-stack.ll
+++ llvm/test/CodeGen/ARM/warn-stack.ll
@@ -4,7 +4,7 @@
 ; 
 
 ; CHECK-NOT: nowarn
-define void @nowarn() nounwind ssp "frame-pointer"="all" {
+define void @nowarn() nounwind ssp "frame-pointer"="all" "warn-stack-size"="80" {
 entry:
   %buffer = alloca [12 x i8], align 1
   %arraydecay = getelementptr inbounds [12 x i8], [12 x i8]* %buffer, i64 0, i64 0
@@ -13,7 +13,7 @@
 }
 
 ; CHECK: warning: stack size limit exceeded (92) in warn
-define void @warn() nounwind ssp "frame-pointer"="all" {
+define void @warn() nounwind ssp "frame-pointer"="all" "warn-stack-size"="80" {
 entry:
   %buffer = alloca [80 x i8], align 1
   %arraydecay = getelementptr inbounds [80 x i8], [80 x i8]* %buffer, i64 0, i64 0
@@ -22,6 +22,3 @@
 }
 
 declare void @doit(i8*)
-
-!llvm.module.flags = 

[PATCH] D104351: [HeaderSearch] Use `isImport` only for imported headers and not for `#pragma once`.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

In D104351#2824505 , @jansvoboda11 
wrote:

> This seems reasonable on first look. Can you add a test that demonstrates the 
> problem this patch solves?

There is no externally observable change that can be tested, I'm only making 
sure the existing tests keep passing. In the subsequent commit I change the 
restoration of `isImport` flag from .pcm because it reflects how a header is 
used, not an inherent property of a header that stays the same regardless of 
the header usage. To make things easier to implement I've decided to separate 
these two qualities of a header. And I think it fits names better, like 
`hasFileBeenImported`. With this change we don't need to add in the comments 
that it means hasFileBeenImportedOrHasPragmaOnce.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104351

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

[PATCH] D104342: [IR] convert warn-stack-size from module attr to fn attr

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D104342#2825357 , @nickdesaulniers 
wrote:

> In D104342#2820811 , @dblaikie 
> wrote:
>
>> what're the rules about module level attributes like this? For instance: 
>> Does this affect/hinder inlining (if the attributes don't match between the 
>> caller and callee)? Other situations that might matter?
>
> I think you meant s/module level/function level/?  That's a good question, 
> one I had to think about a little bit.  Here's my thoughts on the behavior 
> this should exhibit, please let me know if you agree.
>
> When using `-Wframe-larger-than=` per TU, the developer wants to 
> be alerted if any stack frame exceeds a threshold. The Linux kernel's use 
> case is that the kernel's stack is limited to (usually) two pages (`ulimit 
> -s`; typically 8KiB, but different architectures do support non-4KiB page 
> sizes), so functions using more than 1KiB of stack are usually indicative of 
> large objects being stack allocated that should have been heap allocated.
>
> Currently, in C (and C with GNU extensions), there is no way to describe to 
> the compiler function-grain specific values for `-Wframe-larger-than=`; 
> rather than fine grain per function control, we only have coarse grain TU 
> control.
>
> So in the general case (non-LTO), we can only perform inline substitution at 
> call sites visible from callers' definitions. Because there's no GNU C 
> function attribute to change the current value of `-Wframe-larger-than=`, 
> it's not possible for that value to differ between caller and callee.  But 
> with LTO; shit gets weird.
>
> Suddenly now with LTO, we have cross TU (cross Module) visibility into call 
> sites, so we can inline across TU/Module boundaries.  Thus we can have an IR 
> intermediary object with a call site where the caller's value of 
> `-Wframe-larger-than=` differs from the callees!  So the question is what 
> should happen in such a case?
>
> The extremely conservative approach which we have done in the past for 
> certain mismatched function attributes is to simply not perform inline 
> substitution, if we have no other options.  This adds overhead to the inliner 
> to check the XOR of attribute lists of the caller and callee for each call 
> site.
>
> But I *think* (and am open to sugguestions) that we should:
>
> 1. permit inline substitution
> 2. the caller's value of `"warn-stack-size"=` IR Fn Attr wins
>
> I think this is ok because: if caller is defined in TU1 with 
> `-Wframe-larger-than=` distinct from callee defined in TU2 with a different 
> value of `-Wframe-larger-than=`, then we don't care what callee's value was. 
> callee may even be DCE'd if it's inlined into a lone call site.  I'd expect 
> in such cases that callee's value was larger than caller's, in which case 
> callee should be attributed `no_inline` for LTO if the tighter threshold for 
> caller now warns.  If callee's value was smaller than callers and we 
> performed inline substitution, I think that's also perfectly fine, caller 
> should not become "more strict."
>
> Generally in the Linux kernel, we see a common value of 
> `-Wframe-larger-than=` throughout most of the TUs, with only a few generally 
> having a larger value to relax constraints a little. (Also, those relaxations 
> are questionable, given the intent of `-Wframe-larger-than=` use in the 
> kernel in the first place).
>
> Let me add such a test to encode that intention; though I don't know yet 
> what's involved/possible to implement. Let's see.

Sure, that all sounds pretty reasonable to me - mostly I was curious what the 
existing/default behavior is (if we do nothing other than what's already in 
this patch, how does the inliner handle different values/mismatched presence of 
warn-stack-size attributes, for instance) - to check that whatever it does 
seems reasonable/acceptable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104342

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

[PATCH] D93528: [clang-format] Add basic support for formatting JSON

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added a reviewer: HazardyKnusperkeks.
HazardyKnusperkeks accepted this revision.
HazardyKnusperkeks added a comment.
This revision is now accepted and ready to land.

Nice trick! :)


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

https://reviews.llvm.org/D93528

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

[PATCH] D104462: [clang][lex] Add minimizer option to pad the output to the input size

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added a comment.

I agree with Duncan, it would be good to avoid modifying the existing test 
cases if possible


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104462

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

[PATCH] D104424: [Sema] Fix for PR50741

[Abhinav Gaba via Phabricator via cfe-commits]

abhinavgaba added a comment.

Thanks for fixing this, @chrish_ericsson_atx!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104424

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

[PATCH] D104484: [clang] Add cc1 option for dumping layout for all complete types

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

dexonsmith added inline comments.



Comment at: clang/lib/AST/Decl.cpp:4585
+
+  ASTContext =getASTContext();
+  if (Ctx.getLangOpts().DumpRecordLayoutsComplete) {

Nit: please add whitespace around `=`. (Have you run git-clang-format? That 
should fix this.)



Comment at: clang/lib/AST/Decl.cpp:4586
+  ASTContext =getASTContext();
+  if (Ctx.getLangOpts().DumpRecordLayoutsComplete) {
+const ASTRecordLayout  = Ctx.getASTRecordLayout(this);

Probably this deserves a comment, explaining that computing the record layout 
has a side effect of dumping it.

Nit: I think we usually skip braces for one-line `if` statements.



Comment at: clang/lib/AST/Decl.cpp:4587
+  if (Ctx.getLangOpts().DumpRecordLayoutsComplete) {
+const ASTRecordLayout  = Ctx.getASTRecordLayout(this);
+  }

Might be reasonable to cast to `(void)` rather than assigning to an ignored 
variable.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104484

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

[PATCH] D88666: DirectoryWatcher: add an implementation for Windows

[Stella Stamenova via Phabricator via cfe-commits]

stella.stamenova added a comment.

This is the best I can do from the online builds. I'll try and repro locally as 
well:

  FAIL: Clang-Unit :: 
DirectoryWatcher/./DirectoryWatcherTests.exe/DirectoryWatcherTest.InitialScanAsync
 (75980 of 75980)
   TEST 'Clang-Unit :: 
DirectoryWatcher/./DirectoryWatcherTests.exe/DirectoryWatcherTest.InitialScanAsync'
 FAILED 
  Script:
  --
  
D:\a\_work\1\b\llvm\Debug\tools\clang\unittests\DirectoryWatcher\.\DirectoryWatcherTests.exe
 --gtest_filter=DirectoryWatcherTest.InitialScanAsync
  --
  Note: Google Test filter = DirectoryWatcherTest.InitialScanAsync
  
  [==] Running 1 test from 1 test suite.
  
  [--] Global test environment set-up.
  
  [--] 1 test from DirectoryWatcherTest
  
  [ RUN  ] DirectoryWatcherTest.InitialScanAsync
  
  
  
  Testing:  0.. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90.. Terminate batch 
job (Y/N)? 
interrupted by user, skipping remaining tests

One thing we've run into in the past is that running some of these tests on a 
drive other than the OS drive can cause weird failure. I am not sure if that 
may be the case here as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88666

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

[PATCH] D104484: [clang] Add cc1 option for dumping layout for all complete types

[David Tenty via Phabricator via cfe-commits]

daltenty created this revision.
Herald added subscribers: dexonsmith, dang.
daltenty requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This change adds an option which, in addition to dumping the record
layout as is done by -fdump-record-layouts, causes us to compute the
layout for all complete record types (rather than the as-needed basis
which is usually done by clang), so that we will dump them as well.
This is useful if we are looking for layout differences across large
code bases without needing to instantiate every type we are interested in.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D104484

Files:
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/AST/Decl.cpp
  clang/test/Layout/dump-complete.cpp


Index: clang/test/Layout/dump-complete.cpp
===
--- /dev/null
+++ clang/test/Layout/dump-complete.cpp
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -emit-llvm-only -fdump-record-layouts-complete %s | 
FileCheck %s
+
+struct a {
+  int x;
+};
+
+struct b {
+  char y;
+} foo;
+
+class c {};
+
+class d;
+
+// CHECK:  0 | struct a
+// CHECK:  0 | struct b
+// CHECK:  0 | class c
+// CHECK-NOT:  0 | class d
Index: clang/lib/AST/Decl.cpp
===
--- clang/lib/AST/Decl.cpp
+++ clang/lib/AST/Decl.cpp
@@ -4581,6 +4581,11 @@
 void RecordDecl::completeDefinition() {
   assert(!isCompleteDefinition() && "Cannot redefine record!");
   TagDecl::completeDefinition();
+
+  ASTContext =getASTContext();
+  if (Ctx.getLangOpts().DumpRecordLayoutsComplete) {
+const ASTRecordLayout  = Ctx.getASTRecordLayout(this);
+  }
 }
 
 /// isMsStruct - Get whether or not this record uses ms_struct layout.
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -5371,10 +5371,13 @@
 def fdump_record_layouts_simple : Flag<["-"], "fdump-record-layouts-simple">,
   HelpText<"Dump record layout information in a simple form used for testing">,
   MarshallingInfoFlag>;
+def fdump_record_layouts_complete : Flag<["-"], 
"fdump-record-layouts-complete">,
+  HelpText<"Dump record layout information for all complete types">,
+  MarshallingInfoFlag>;
 def fdump_record_layouts : Flag<["-"], "fdump-record-layouts">,
   HelpText<"Dump record layout information">,
   MarshallingInfoFlag>,
-  ImpliedByAnyOf<[fdump_record_layouts_simple.KeyPath]>;
+  ImpliedByAnyOf<[fdump_record_layouts_simple.KeyPath, 
fdump_record_layouts_complete.KeyPath]>;
 def fix_what_you_can : Flag<["-"], "fix-what-you-can">,
   HelpText<"Apply fix-it advice even in the presence of unfixable errors">,
   MarshallingInfoFlag>;
Index: clang/include/clang/Basic/LangOptions.def
===
--- clang/include/clang/Basic/LangOptions.def
+++ clang/include/clang/Basic/LangOptions.def
@@ -266,6 +266,7 @@
 BENIGN_LANGOPT(ElideConstructors , 1, 1, "C++ copy constructor elision")
 BENIGN_LANGOPT(DumpRecordLayouts , 1, 0, "dumping the layout of IRgen'd 
records")
 BENIGN_LANGOPT(DumpRecordLayoutsSimple , 1, 0, "dumping the layout of IRgen'd 
records in a simple form")
+BENIGN_LANGOPT(DumpRecordLayoutsComplete , 1, 0, "dumping the AST layout of 
all complete records")
 BENIGN_LANGOPT(DumpVTableLayouts , 1, 0, "dumping the layouts of emitted 
vtables")
 LANGOPT(NoConstantCFStrings , 1, 0, "no constant CoreFoundation strings")
 BENIGN_LANGOPT(InlineVisibilityHidden , 1, 0, "hidden visibility for inline 
C++ methods")


Index: clang/test/Layout/dump-complete.cpp
===
--- /dev/null
+++ clang/test/Layout/dump-complete.cpp
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -emit-llvm-only -fdump-record-layouts-complete %s | FileCheck %s
+
+struct a {
+  int x;
+};
+
+struct b {
+  char y;
+} foo;
+
+class c {};
+
+class d;
+
+// CHECK:  0 | struct a
+// CHECK:  0 | struct b
+// CHECK:  0 | class c
+// CHECK-NOT:  0 | class d
Index: clang/lib/AST/Decl.cpp
===
--- clang/lib/AST/Decl.cpp
+++ clang/lib/AST/Decl.cpp
@@ -4581,6 +4581,11 @@
 void RecordDecl::completeDefinition() {
   assert(!isCompleteDefinition() && "Cannot redefine record!");
   TagDecl::completeDefinition();
+
+  ASTContext =getASTContext();
+  if (Ctx.getLangOpts().DumpRecordLayoutsComplete) {
+const ASTRecordLayout  = Ctx.getASTRecordLayout(this);
+  }
 }
 
 /// isMsStruct - Get whether or not this record uses ms_struct layout.
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ 

[PATCH] D103855: [clang] Exclude function pointers on DefaultedComparisonAnalyzer

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

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

Thanks, let's go with this for now.

Discussion in WG21 seems to be heading towards removing the builtin candidates 
for `operator<=>`, `operator<`, `operator<=`, `operator>`, and `operator>=` for 
function pointers, but we'll probably not have a firm decision on that for 
quite a while.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103855

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

[PATCH] D104058: ThinLTO: Fix inline assembly references to static functions with CFI

[Sami Tolvanen via Phabricator via cfe-commits]

samitolvanen updated this revision to Diff 352825.
samitolvanen added a comment.

Moved the test to llvm/test/Transforms/ThinLTOBitcodeWriter.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104058

Files:
  llvm/lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp
  llvm/test/Transforms/ThinLTOBitcodeWriter/cfi-icall-static-inline-asm.ll


Index: llvm/test/Transforms/ThinLTOBitcodeWriter/cfi-icall-static-inline-asm.ll
===
--- /dev/null
+++ llvm/test/Transforms/ThinLTOBitcodeWriter/cfi-icall-static-inline-asm.ll
@@ -0,0 +1,19 @@
+; RUN: opt -thinlto-bc -thinlto-split-lto-unit -o - %s | llvm-modextract -b -n 
0 -o - | llvm-dis | FileCheck %s
+
+; CHECK: @a = internal alias {{.*}}@a.[[HASH:[0-9a-f]+]]
+
+define void @b() {
+  %f = alloca void ()*, align 8
+  ; CHECK: store{{.*}} @a.[[HASH]],{{.*}} %f
+  store void ()* @a, void ()** %f, align 8
+  ; CHECK: %1 = call void ()* asm sideeffect "leaq a(%rip)
+  %1 = call void ()* asm sideeffect "leaq a(%rip), $0\0A\09", 
"=r,~{dirflag},~{fpsr},~{flags}"()
+  ret void
+}
+
+; CHECK: define{{.*}} @a.[[HASH]](){{.*}} !type
+define internal void @a() !type !0 {
+  ret void
+}
+
+!0 = !{i64 0, !"typeid1"}
Index: llvm/lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp
===
--- llvm/lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp
+++ llvm/lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp
@@ -69,6 +69,15 @@
   ImportGV->setName(NewName);
   ImportGV->setVisibility(GlobalValue::HiddenVisibility);
 }
+
+if (Function *F = dyn_cast()) {
+  // Create a local alias with the original name to avoid breaking
+  // references from inline assembly.
+  GlobalAlias *A =
+  GlobalAlias::create(F->getValueType(), F->getAddressSpace(),
+  GlobalValue::InternalLinkage, Name, F, );
+  appendToCompilerUsed(ExportM, A);
+}
   }
 
   if (!RenamedComdats.empty())


Index: llvm/test/Transforms/ThinLTOBitcodeWriter/cfi-icall-static-inline-asm.ll
===
--- /dev/null
+++ llvm/test/Transforms/ThinLTOBitcodeWriter/cfi-icall-static-inline-asm.ll
@@ -0,0 +1,19 @@
+; RUN: opt -thinlto-bc -thinlto-split-lto-unit -o - %s | llvm-modextract -b -n 0 -o - | llvm-dis | FileCheck %s
+
+; CHECK: @a = internal alias {{.*}}@a.[[HASH:[0-9a-f]+]]
+
+define void @b() {
+  %f = alloca void ()*, align 8
+  ; CHECK: store{{.*}} @a.[[HASH]],{{.*}} %f
+  store void ()* @a, void ()** %f, align 8
+  ; CHECK: %1 = call void ()* asm sideeffect "leaq a(%rip)
+  %1 = call void ()* asm sideeffect "leaq a(%rip), $0\0A\09", "=r,~{dirflag},~{fpsr},~{flags}"()
+  ret void
+}
+
+; CHECK: define{{.*}} @a.[[HASH]](){{.*}} !type
+define internal void @a() !type !0 {
+  ret void
+}
+
+!0 = !{i64 0, !"typeid1"}
Index: llvm/lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp
===
--- llvm/lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp
+++ llvm/lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp
@@ -69,6 +69,15 @@
   ImportGV->setName(NewName);
   ImportGV->setVisibility(GlobalValue::HiddenVisibility);
 }
+
+if (Function *F = dyn_cast()) {
+  // Create a local alias with the original name to avoid breaking
+  // references from inline assembly.
+  GlobalAlias *A =
+  GlobalAlias::create(F->getValueType(), F->getAddressSpace(),
+  GlobalValue::InternalLinkage, Name, F, );
+  appendToCompilerUsed(ExportM, A);
+}
   }
 
   if (!RenamedComdats.empty())
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D103930: [clang][HeaderSearch] Fix implicit module when using header maps

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

dexonsmith added inline comments.



Comment at: clang/test/Modules/implicit-module-header-maps.cpp:27
+#define FOO
+// This include will fail if modules weren't used.  The include name itself
+// doesn't exist and relies on the header map to remap it to the real header.

andrewjcg wrote:
> andrewjcg wrote:
> > dexonsmith wrote:
> > > Do you really mean if modules aren't used, or do you mean if header maps 
> > > aren't used?
> > > 
> > > (I think we want to find the same headers on disk whether or not modules 
> > > are on... if this patch changes that, then I guess I'm not totally 
> > > understanding why...)
> > Ahh, meant if header maps aren't used.  Will fix.
> Ah no wait.  The include should fail if either header maps or modules isn't 
> used (header maps required for the remapping and modules required to prevent 
> the `FOO` define from propagating into the included header and triggering the 
> `#error`).  
> 
> Will update the comment.
Got it, thanks, the comment is clear now!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103930

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

[PATCH] D104342: [IR] convert warn-stack-size from module attr to fn attr

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added inline comments.



Comment at: llvm/lib/CodeGen/PrologEpilogInserter.cpp:282
+.getValueAsString()
+.getAsInteger(0, Threshold);
   if (StackSize > Threshold) {

nickdesaulniers wrote:
> dblaikie wrote:
> > I guess the 0 value here is the default value if the value can't be parsed 
> > as an integer? Is that desirable? I guess maybe we should ignore it (use 
> > UINT_MAX here instead, maybe) and fail in the verifier.
> > 
> > But I guess if we fail in the verifier, then it doesn't really 
> > matter/shouldn't be tested what the behavior is here when presented with 
> > invalid IR.
> > 
> > (but this is a divergence from the module flag handling, which looks like 
> > it does silently ignore non-numeric values, by using UINT_MAX)
> IIUC, the first parameter to `getAsInteger` is the `Radix`, not the default 
> value on failure to parse. But it does return `true` on error, so I should 
> check that here.
> 
> I also should add a verifier check for this new function attribute.  While 
> the "string key equals string value" attributes are quite flexible, it would 
> be good to have some rigidity in requiring the string value to be parseable 
> as an unsigned int.
Oh, I should use base 10 as the radix, otherwise it will try to parse hex and 
binary literals.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104342

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

[PATCH] D103930: [clang][HeaderSearch] Fix implicit module when using header maps

[Andrew Gallagher via Phabricator via cfe-commits]

andrewjcg updated this revision to Diff 352824.
andrewjcg added a comment.

fix comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103930

Files:
  clang/lib/Lex/HeaderSearch.cpp
  clang/test/Modules/Inputs/implicit-module-header-maps/a.h
  clang/test/Modules/Inputs/implicit-module-header-maps/a.hmap.json
  clang/test/Modules/Inputs/implicit-module-header-maps/a.module.modulemap
  clang/test/Modules/Inputs/implicit-module-header-maps/b.hmap.json
  clang/test/Modules/implicit-module-header-maps.cpp

Index: clang/test/Modules/implicit-module-header-maps.cpp
===
--- /dev/null
+++ clang/test/Modules/implicit-module-header-maps.cpp
@@ -0,0 +1,32 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: cd %t
+//
+// RUN: %hmaptool write %S/Inputs/implicit-module-header-maps/a.hmap.json %t/hmap
+//
+// RUN: mkdir -p %t/After
+// RUN: cp %S/Inputs/implicit-module-header-maps/a.h %t/After/Mapping.h
+// RUN: cp %S/Inputs/implicit-module-header-maps/a.module.modulemap %t/module.modulemap
+//
+// RUN: %clang -Rmodule-build -fmodules -fimplicit-modules -fimplicit-module-maps -fmodule-map-file=%t/module.modulemap -fsyntax-only %s -I %t/hmap
+//
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: cd %t
+//
+// RUN: sed -e "s|OUTPUTS_DIR|%t|g" %S/Inputs/implicit-module-header-maps/b.hmap.json > %t/hmap.json
+// RUN: %hmaptool write %t/hmap.json %t/hmap
+//
+// RUN: mkdir -p %t/After
+// RUN: cp %S/Inputs/implicit-module-header-maps/a.h %t/After/Mapping.h
+// RUN: cp %S/Inputs/implicit-module-header-maps/a.module.modulemap %t/module.modulemap
+//
+// RUN: %clang -Rmodule-build -fmodules -fimplicit-modules -fimplicit-module-maps -fmodule-map-file=%t/module.modulemap -fsyntax-only %s -I %t/hmap
+
+#define FOO
+// This include will fail if:
+// 1) modules are't used, as the `FOO` define will propagate into the included
+//header and trip a `#error`, or
+// 2) header maps aren't usesd, as the header name doesn't exist and relies on
+//the header map to remap it to the real header.
+#include "Before/Mapping.h"
Index: clang/test/Modules/Inputs/implicit-module-header-maps/b.hmap.json
===
--- /dev/null
+++ clang/test/Modules/Inputs/implicit-module-header-maps/b.hmap.json
@@ -0,0 +1,6 @@
+{
+  "mappings" :
+{
+ "Before/Mapping.h" : "OUTPUTS_DIR/After/Mapping.h"
+}
+}
Index: clang/test/Modules/Inputs/implicit-module-header-maps/a.module.modulemap
===
--- /dev/null
+++ clang/test/Modules/Inputs/implicit-module-header-maps/a.module.modulemap
@@ -0,0 +1,3 @@
+module a {
+  header "After/Mapping.h"
+}
Index: clang/test/Modules/Inputs/implicit-module-header-maps/a.hmap.json
===
--- /dev/null
+++ clang/test/Modules/Inputs/implicit-module-header-maps/a.hmap.json
@@ -0,0 +1,7 @@
+{
+  "mappings" :
+{
+ "Before/Mapping.h" : "After/Mapping.h",
+ "After/Mapping.h" : "After/Mapping.h"
+}
+}
Index: clang/test/Modules/Inputs/implicit-module-header-maps/a.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/implicit-module-header-maps/a.h
@@ -0,0 +1,3 @@
+#ifdef FOO
+#error foo
+#endif
Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -417,7 +417,8 @@
 
   IsInHeaderMap = true;
 
-  auto FixupSearchPath = [&]() {
+  auto FixupSearchPathAndFindUsableModule =
+  [&](auto file) -> Optional {
 if (SearchPath) {
   StringRef SearchPathRef(getName());
   SearchPath->clear();
@@ -427,6 +428,12 @@
   RelativePath->clear();
   RelativePath->append(Filename.begin(), Filename.end());
 }
+if (!HS.findUsableModuleForHeader(
+(), file.getFileEntry().getDir(),
+RequestingModule, SuggestedModule, isSystemHeaderDirectory())) {
+  return None;
+}
+return file;
   };
 
   // Check if the headermap maps the filename to a framework include
@@ -437,12 +444,10 @@
 Filename = StringRef(MappedName.begin(), MappedName.size());
 Optional Result = HM->LookupFile(Filename, HS.getFileMgr());
 if (Result) {
-  FixupSearchPath();
-  return *Result;
+  return FixupSearchPathAndFindUsableModule(*Result);
 }
   } else if (auto Res = HS.getFileMgr().getOptionalFileRef(Dest)) {
-FixupSearchPath();
-return *Res;
+return FixupSearchPathAndFindUsableModule(*Res);
   }
 
   return None;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D103930: [clang][HeaderSearch] Fix implicit module when using header maps

[Andrew Gallagher via Phabricator via cfe-commits]

andrewjcg added inline comments.



Comment at: clang/test/Modules/implicit-module-header-maps.cpp:27
+#define FOO
+// This include will fail if modules weren't used.  The include name itself
+// doesn't exist and relies on the header map to remap it to the real header.

andrewjcg wrote:
> dexonsmith wrote:
> > Do you really mean if modules aren't used, or do you mean if header maps 
> > aren't used?
> > 
> > (I think we want to find the same headers on disk whether or not modules 
> > are on... if this patch changes that, then I guess I'm not totally 
> > understanding why...)
> Ahh, meant if header maps aren't used.  Will fix.
Ah no wait.  The include should fail if either header maps or modules isn't 
used (header maps required for the remapping and modules required to prevent 
the `FOO` define from propagating into the included header and triggering the 
`#error`).  

Will update the comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103930

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

[PATCH] D103930: [clang][HeaderSearch] Fix implicit module when using header maps

[Andrew Gallagher via Phabricator via cfe-commits]

andrewjcg added inline comments.



Comment at: clang/test/Modules/implicit-module-header-maps.cpp:27
+#define FOO
+// This include will fail if modules weren't used.  The include name itself
+// doesn't exist and relies on the header map to remap it to the real header.

dexonsmith wrote:
> Do you really mean if modules aren't used, or do you mean if header maps 
> aren't used?
> 
> (I think we want to find the same headers on disk whether or not modules are 
> on... if this patch changes that, then I guess I'm not totally understanding 
> why...)
Ahh, meant if header maps aren't used.  Will fix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103930

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

[PATCH] D104350: [clang][AST] Make `getLocalOrImportedSubmoduleID` work with const `Module*`. NFC.

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

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

LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104350

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

[PATCH] D99005: [clang] Implement P2266 Simpler implicit move

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov updated this revision to Diff 352821.
mizvekov added a comment.

.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99005

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaCoroutine.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/CXX/class/class.init/class.copy.elision/p3.cpp
  clang/test/CXX/dcl.dcl/dcl.spec/dcl.type/dcl.spec.auto/p7-cxx14.cpp
  clang/test/CXX/drs/dr3xx.cpp
  clang/test/CXX/expr/expr.prim/expr.prim.lambda/p4-cxx14.cpp
  clang/test/CXX/temp/temp.decls/temp.mem/p5.cpp
  clang/test/SemaCXX/constant-expression-cxx11.cpp
  clang/test/SemaCXX/constant-expression-cxx14.cpp
  clang/test/SemaCXX/coroutine-rvo.cpp
  clang/test/SemaCXX/coroutines.cpp
  clang/test/SemaCXX/deduced-return-type-cxx14.cpp
  clang/test/SemaCXX/return-stack-addr.cpp
  clang/test/SemaCXX/warn-return-std-move.cpp
  clang/test/SemaObjCXX/block-capture.mm
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -1278,6 +1278,11 @@
   https://wg21.link/p1102r2;>P1102R2
   Clang 13
 
+
+  Simpler implicit move
+  https://wg21.link/p2266r1;>P2266R1
+  Clang 13
+
 
 
 
Index: clang/test/SemaObjCXX/block-capture.mm
===
--- clang/test/SemaObjCXX/block-capture.mm
+++ clang/test/SemaObjCXX/block-capture.mm
@@ -1,6 +1,7 @@
-// RUN: %clang_cc1 -std=c++20 -fsyntax-only -fobjc-arc -fblocks   -verify=cxx98_20,cxx20%s
-// RUN: %clang_cc1 -std=c++11 -fsyntax-only -fobjc-arc -fblocks   -verify=cxx98_20,cxx98_11 %s
-// RUN: %clang_cc1 -std=c++98 -fsyntax-only -fobjc-arc -fblocks -Wno-c++11-extensions -verify=cxx98_20,cxx98_11 %s
+// RUN: %clang_cc1 -std=c++2b -fsyntax-only -fobjc-arc -fblocks   -verify=cxx98_2b,cxx20_2b,cxx2b %s
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -fobjc-arc -fblocks   -verify=cxx98_2b,cxx20_2b   %s
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -fobjc-arc -fblocks   -verify=cxx98_2b,cxx98_11   %s
+// RUN: %clang_cc1 -std=c++98 -fsyntax-only -fobjc-arc -fblocks -Wno-c++11-extensions -verify=cxx98_2b,cxx98_11   %s
 
 #define TEST(T) void test_##T() { \
   __block T x;\
@@ -8,17 +9,17 @@
 }
 
 struct CopyOnly {
-  CopyOnly();
-  CopyOnly(CopyOnly &);
+  CopyOnly();   // cxx2b-note {{not viable}}
+  CopyOnly(CopyOnly &); // cxx2b-note {{not viable}}
 };
-TEST(CopyOnly);
+TEST(CopyOnly); // cxx2b-error {{no matching constructor}}
 
 struct CopyNoMove {
   CopyNoMove();
   CopyNoMove(CopyNoMove &);
-  CopyNoMove(CopyNoMove &&) = delete; // cxx98_20-note {{marked deleted here}}
+  CopyNoMove(CopyNoMove &&) = delete; // cxx98_2b-note {{marked deleted here}}
 };
-TEST(CopyNoMove); // cxx98_20-error {{call to deleted constructor}}
+TEST(CopyNoMove); // cxx98_2b-error {{call to deleted constructor}}
 
 struct MoveOnly {
   MoveOnly();
@@ -30,9 +31,9 @@
 struct NoCopyNoMove {
   NoCopyNoMove();
   NoCopyNoMove(NoCopyNoMove &) = delete;
-  NoCopyNoMove(NoCopyNoMove &&) = delete; // cxx98_20-note {{marked deleted here}}
+  NoCopyNoMove(NoCopyNoMove &&) = delete; // cxx98_2b-note {{marked deleted here}}
 };
-TEST(NoCopyNoMove); // cxx98_20-error {{call to deleted constructor}}
+TEST(NoCopyNoMove); // cxx98_2b-error {{call to deleted constructor}}
 
 struct ConvertingRVRef {
   ConvertingRVRef();
@@ -50,11 +51,11 @@
   ConvertingCLVRef(ConvertingCLVRef &);
 
   struct X {};
-  ConvertingCLVRef(X &&); // cxx20-note {{passing argument to parameter here}}
+  ConvertingCLVRef(X &&); // cxx20_2b-note {{passing argument to parameter here}}
   operator X() const &;
-  operator X() && = delete; // cxx20-note {{marked deleted here}}
+  operator X() && = delete; // cxx20_2b-note {{marked deleted here}}
 };
-TEST(ConvertingCLVRef); // cxx20-error {{invokes a deleted function}}
+TEST(ConvertingCLVRef); // cxx20_2b-error {{invokes a deleted function}}
 
 struct SubSubMove {};
 struct SubMove : SubSubMove {
Index: clang/test/SemaCXX/warn-return-std-move.cpp
===
--- clang/test/SemaCXX/warn-return-std-move.cpp
+++ clang/test/SemaCXX/warn-return-std-move.cpp
@@ -1,8 +1,8 @@
-// RUN: %clang_cc1 -std=c++2b -fsyntax-only -verify=cxx20_2b -fcxx-exceptions -Wreturn-std-move %s
-// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=cxx20_2b -fcxx-exceptions -Wreturn-std-move %s
-// RUN: %clang_cc1 -std=c++17 -fsyntax-only -verify=cxx11_17 -fcxx-exceptions -Wreturn-std-move %s
-// RUN: %clang_cc1 -std=c++14 -fsyntax-only -verify=cxx11_17 -fcxx-exceptions -Wreturn-std-move %s
-// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify=cxx11_17 

[PATCH] D99005: [clang] Implement P2266 Simpler implicit move

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov updated this revision to Diff 352818.
mizvekov added a comment.

- Change block capture NRVO semantics to match P2266 
 in C++2b mode.
- Update cxx_status.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99005

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaCoroutine.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/CXX/class/class.init/class.copy.elision/p3.cpp
  clang/test/CXX/dcl.dcl/dcl.spec/dcl.type/dcl.spec.auto/p7-cxx14.cpp
  clang/test/CXX/drs/dr3xx.cpp
  clang/test/CXX/expr/expr.prim/expr.prim.lambda/p4-cxx14.cpp
  clang/test/CXX/temp/temp.decls/temp.mem/p5.cpp
  clang/test/SemaCXX/constant-expression-cxx11.cpp
  clang/test/SemaCXX/constant-expression-cxx14.cpp
  clang/test/SemaCXX/coroutine-rvo.cpp
  clang/test/SemaCXX/coroutines.cpp
  clang/test/SemaCXX/deduced-return-type-cxx14.cpp
  clang/test/SemaCXX/return-stack-addr.cpp
  clang/test/SemaCXX/warn-return-std-move.cpp
  clang/test/SemaObjCXX/block-capture.mm
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -1278,6 +1278,11 @@
   https://wg21.link/p1102r2;>P1102R2
   Clang 13
 
+
+  Simpler implicit move 
+  https://wg21.link/p2266r1;>P2266R1
+  Clang 13
+
 
 
 
Index: clang/test/SemaObjCXX/block-capture.mm
===
--- clang/test/SemaObjCXX/block-capture.mm
+++ clang/test/SemaObjCXX/block-capture.mm
@@ -1,6 +1,7 @@
-// RUN: %clang_cc1 -std=c++20 -fsyntax-only -fobjc-arc -fblocks   -verify=cxx98_20,cxx20%s
-// RUN: %clang_cc1 -std=c++11 -fsyntax-only -fobjc-arc -fblocks   -verify=cxx98_20,cxx98_11 %s
-// RUN: %clang_cc1 -std=c++98 -fsyntax-only -fobjc-arc -fblocks -Wno-c++11-extensions -verify=cxx98_20,cxx98_11 %s
+// RUN: %clang_cc1 -std=c++2b -fsyntax-only -fobjc-arc -fblocks   -verify=cxx98_2b,cxx20_2b,cxx2b %s
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -fobjc-arc -fblocks   -verify=cxx98_2b,cxx20_2b   %s
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -fobjc-arc -fblocks   -verify=cxx98_2b,cxx98_11   %s
+// RUN: %clang_cc1 -std=c++98 -fsyntax-only -fobjc-arc -fblocks -Wno-c++11-extensions -verify=cxx98_2b,cxx98_11   %s
 
 #define TEST(T) void test_##T() { \
   __block T x;\
@@ -8,17 +9,17 @@
 }
 
 struct CopyOnly {
-  CopyOnly();
-  CopyOnly(CopyOnly &);
+  CopyOnly();   // cxx2b-note {{not viable}}
+  CopyOnly(CopyOnly &); // cxx2b-note {{not viable}}
 };
-TEST(CopyOnly);
+TEST(CopyOnly); // cxx2b-error {{no matching constructor}}
 
 struct CopyNoMove {
   CopyNoMove();
   CopyNoMove(CopyNoMove &);
-  CopyNoMove(CopyNoMove &&) = delete; // cxx98_20-note {{marked deleted here}}
+  CopyNoMove(CopyNoMove &&) = delete; // cxx98_2b-note {{marked deleted here}}
 };
-TEST(CopyNoMove); // cxx98_20-error {{call to deleted constructor}}
+TEST(CopyNoMove); // cxx98_2b-error {{call to deleted constructor}}
 
 struct MoveOnly {
   MoveOnly();
@@ -30,9 +31,9 @@
 struct NoCopyNoMove {
   NoCopyNoMove();
   NoCopyNoMove(NoCopyNoMove &) = delete;
-  NoCopyNoMove(NoCopyNoMove &&) = delete; // cxx98_20-note {{marked deleted here}}
+  NoCopyNoMove(NoCopyNoMove &&) = delete; // cxx98_2b-note {{marked deleted here}}
 };
-TEST(NoCopyNoMove); // cxx98_20-error {{call to deleted constructor}}
+TEST(NoCopyNoMove); // cxx98_2b-error {{call to deleted constructor}}
 
 struct ConvertingRVRef {
   ConvertingRVRef();
@@ -50,11 +51,11 @@
   ConvertingCLVRef(ConvertingCLVRef &);
 
   struct X {};
-  ConvertingCLVRef(X &&); // cxx20-note {{passing argument to parameter here}}
+  ConvertingCLVRef(X &&); // cxx20_2b-note {{passing argument to parameter here}}
   operator X() const &;
-  operator X() && = delete; // cxx20-note {{marked deleted here}}
+  operator X() && = delete; // cxx20_2b-note {{marked deleted here}}
 };
-TEST(ConvertingCLVRef); // cxx20-error {{invokes a deleted function}}
+TEST(ConvertingCLVRef); // cxx20_2b-error {{invokes a deleted function}}
 
 struct SubSubMove {};
 struct SubMove : SubSubMove {
Index: clang/test/SemaCXX/warn-return-std-move.cpp
===
--- clang/test/SemaCXX/warn-return-std-move.cpp
+++ clang/test/SemaCXX/warn-return-std-move.cpp
@@ -1,8 +1,8 @@
-// RUN: %clang_cc1 -std=c++2b -fsyntax-only -verify=cxx20_2b -fcxx-exceptions -Wreturn-std-move %s
-// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=cxx20_2b -fcxx-exceptions -Wreturn-std-move %s
-// RUN: %clang_cc1 -std=c++17 -fsyntax-only -verify=cxx11_17 -fcxx-exceptions -Wreturn-std-move %s
-// RUN: %clang_cc1 -std=c++14 

[PATCH] D104462: [clang][lex] Add minimizer option to pad the output to the input size

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

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

I'm not sure D104465  (the motivation for 
this patch) is the right approach, but we can have that discussion over there.




Comment at: clang/unittests/Lex/DependencyDirectivesSourceMinimizerTest.cpp:24
+  SourceLocation(),
+  /*PadOutputToInputSize=*/true);
+}

Why turn this on all the time? That adds a lot of noise below. Can you instead 
just turn it on for specific targeted tests?

Also, maybe I just missed it, but I don't see any tests that check the 
behaviour of this flag.



Comment at: 
clang/unittests/Lex/DependencyDirectivesSourceMinimizerTest.cpp:113-115
   ASSERT_FALSE(
   minimizeSourceToDependencyDirectives("#define MACRO\n\n\n", Out));
+  EXPECT_EQ("#define MACRO", rtrim(Out));

I'm uncomfortable dropping the check that `#define MACRO\n\n\n` turns into 
precisely `#define MACRO\n`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104462

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

[PATCH] D104342: [IR] convert warn-stack-size from module attr to fn attr

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

In D104342#2820811 , @dblaikie wrote:

> what're the rules about module level attributes like this? For instance: Does 
> this affect/hinder inlining (if the attributes don't match between the caller 
> and callee)? Other situations that might matter?

I think you meant s/module level/function level/?  That's a good question, one 
I had to think about a little bit.  Here's my thoughts on the behavior this 
should exhibit, please let me know if you agree.

When using `-Wframe-larger-than=` per TU, the developer wants to be 
alerted if any stack frame exceeds a threshold. The Linux kernel's use case is 
that the kernel's stack is limited to (usually) two pages (`ulimit -s`; 
typically 8KiB, but different architectures do support non-4KiB page sizes), so 
functions using more than 1KiB of stack are usually indicative of large objects 
being stack allocated that should have been heap allocated.

Currently, in C (and C with GNU extensions), there is no way to describe to the 
compiler function-grain specific values for `-Wframe-larger-than=`; rather than 
fine grain per function control, we only have coarse grain TU control.

So in the general case (non-LTO), we can only perform inline substitution at 
call sites visible from callers' definitions. Because there's no GNU C function 
attribute to change the current value of `-Wframe-larger-than=`, it's not 
possible for that value to differ between caller and callee.  But with LTO; 
shit gets weird.

Suddenly now with LTO, we have cross TU (cross Module) visibility into call 
sites, so we can inline across TU/Module boundaries.  Thus we can have an IR 
intermediary object with a call site where the caller's value of 
`-Wframe-larger-than=` differs from the callees!  So the question is what 
should happen in such a case?

The extremely conservative approach which we have done in the past for certain 
mismatched function attributes is to simply not perform inline substitution, if 
we have no other options.  This adds overhead to the inliner to check the XOR 
of attribute lists of the caller and callee for each call site.

But I *think* (and am open to sugguestions) that we should:

1. permit inline substitution
2. the caller's value of `"warn-stack-size"=` IR Fn Attr wins

I think this is ok because: if caller is defined in TU1 with 
`-Wframe-larger-than=` distinct from callee defined in TU2 with a different 
value of `-Wframe-larger-than=`, then we don't care what callee's value was. 
callee may even be DCE'd if it's inlined into a lone call site.  I'd expect in 
such cases that callee's value was larger than caller's, in which case callee 
should be attributed `no_inline` for LTO if the tighter threshold for caller 
now warns.  If callee's value was smaller than callers and we performed inline 
substitution, I think that's also perfectly fine, caller should not become 
"more strict."

Generally in the Linux kernel, we see a common value of `-Wframe-larger-than=` 
throughout most of the TUs, with only a few generally having a larger value to 
relax constraints a little. (Also, those relaxations are questionable, given 
the intent of `-Wframe-larger-than=` use in the kernel in the first place).

Let me add such a test to encode that intention; though I don't know yet what's 
involved/possible to implement. Let's see.




Comment at: llvm/lib/CodeGen/PrologEpilogInserter.cpp:282
+.getValueAsString()
+.getAsInteger(0, Threshold);
   if (StackSize > Threshold) {

dblaikie wrote:
> I guess the 0 value here is the default value if the value can't be parsed as 
> an integer? Is that desirable? I guess maybe we should ignore it (use 
> UINT_MAX here instead, maybe) and fail in the verifier.
> 
> But I guess if we fail in the verifier, then it doesn't really 
> matter/shouldn't be tested what the behavior is here when presented with 
> invalid IR.
> 
> (but this is a divergence from the module flag handling, which looks like it 
> does silently ignore non-numeric values, by using UINT_MAX)
IIUC, the first parameter to `getAsInteger` is the `Radix`, not the default 
value on failure to parse. But it does return `true` on error, so I should 
check that here.

I also should add a verifier check for this new function attribute.  While the 
"string key equals string value" attributes are quite flexible, it would be 
good to have some rigidity in requiring the string value to be parseable as an 
unsigned int.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104342

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

[PATCH] D102875: [PowerPC] Add PowerPC compare and multiply related builtins and instrinsics for XL compatibility

[Lei Huang via Phabricator via cfe-commits]

lei added inline comments.



Comment at: clang/test/CodeGen/builtins-ppc-xlcompat-compare-64bit-only.c:8
+// RUN: not %clang_cc1 -triple powerpc-unknown-aix \
+// RUN:   -emit-llvm %s -o - -target-cpu pwr9 2>&1 | FileCheck %s 
-check-prefix=CHECK-32
+

`-check-prefix=CHECK-32` => `--check-prefix=CHECK-32`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102875

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

[PATCH] D102875: [PowerPC] Add PowerPC compare and multiply related builtins and instrinsics for XL compatibility

[Lei Huang via Phabricator via cfe-commits]

lei added inline comments.



Comment at: clang/test/CodeGen/builtins-ppc-xlcompat-compare-64bit-only.c:6
+// RUN: %clang_cc1 -triple powerpc64-unknown-aix \
+// RUN:   -emit-llvm %s -o - -target-cpu pwr9 | FileCheck %s 
--check-prefix=CHECK-64
+// RUN: not %clang_cc1 -triple powerpc-unknown-aix \

why not just use default for 64BIT behaviour?  No need for 
`--check-prefix=CHECK-64`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102875

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

[PATCH] D104475: [Clang][Codegen] emit noprofile IR Fn Attr for new Fn Attr no_profile

[Bill Wendling via Phabricator via cfe-commits]

void added inline comments.



Comment at: clang/include/clang/Basic/Attr.td:1974
+def NoProfileFunction : InheritableAttr {
+  let Spellings = [GCC<"no_profile">];
+  let Subjects = SubjectList<[Function]>;

Is this an attribute that exists in GCC? I couldn't find it...



Comment at: clang/include/clang/Basic/AttrDocs.td:2565
+  let Content = [{
+Use the ``no_profile`` attribute on a function to declaration to denote that
+the compiler should not instrument the function with profile related

s/function to declaration/function declaration/


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104475

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

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

[Yuanfang Chen via Phabricator via cfe-commits]

ychen updated this revision to Diff 352810.
ychen added a comment.

- Rebase correctly


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97915

Files:
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CGCoroutine.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/test/CodeGenCoroutines/coro-alloc.cpp
  clang/test/CodeGenCoroutines/coro-cleanup.cpp
  clang/test/CodeGenCoroutines/coro-gro.cpp
  llvm/docs/Coroutines.rst
  llvm/include/llvm/IR/Intrinsics.td
  llvm/lib/Transforms/Coroutines/CoroFrame.cpp
  llvm/lib/Transforms/Coroutines/CoroInstr.h
  llvm/lib/Transforms/Coroutines/CoroInternal.h
  llvm/lib/Transforms/Coroutines/CoroSplit.cpp
  llvm/lib/Transforms/Coroutines/Coroutines.cpp
  llvm/test/Transforms/Coroutines/coro-frame-overalign.ll

Index: llvm/test/Transforms/Coroutines/coro-frame-overalign.ll
===
--- /dev/null
+++ llvm/test/Transforms/Coroutines/coro-frame-overalign.ll
@@ -0,0 +1,78 @@
+; Check that `llvm.coro.align`, `llvm.coro.raw.frame.ptr.offset` and
+; `@llvm.coro.raw.frame.ptr.alloca` are lowered correctly.
+; RUN: opt < %s -passes=coro-split -S | FileCheck %s
+
+%PackedStruct = type <{ i64 }>
+
+declare void @consume(%PackedStruct*, i32, i32, i8**)
+declare void @consume2(i32, i32)
+
+define i8* @f() "coroutine.presplit"="1" {
+entry:
+  %data = alloca %PackedStruct, align 32
+  %id = call token @llvm.coro.id(i32 16, i8* null, i8* null, i8* null)
+  %size = call i32 @llvm.coro.size.i32()
+  %alloc = call i8* @malloc(i32 %size)
+  %hdl = call i8* @llvm.coro.begin(token %id, i8* %alloc)
+  %align = call i32 @llvm.coro.align.i32()
+  %offset = call i32 @llvm.coro.raw.frame.ptr.offset.i32()
+  %addr = call i8** @llvm.coro.raw.frame.ptr.addr()
+  call void @consume(%PackedStruct* %data, i32 %align, i32 %offset, i8** %addr)
+  %0 = call i8 @llvm.coro.suspend(token none, i1 false)
+  switch i8 %0, label %suspend [i8 0, label %resume
+i8 1, label %cleanup]
+resume:
+  br label %cleanup
+
+cleanup:
+  %align2 = call i32 @llvm.coro.align.i32()
+  %offset2 = call i32 @llvm.coro.raw.frame.ptr.offset.i32()
+  call void @consume2(i32 %align2, i32 %offset2)
+  %mem = call i8* @llvm.coro.free(token %id, i8* %hdl)
+  call void @free(i8* %mem)
+  br label %suspend
+suspend:
+  call i1 @llvm.coro.end(i8* %hdl, i1 0)
+  ret i8* %hdl
+}
+
+; See if the raw frame address was inserted into the frame.
+; CHECK-LABEL: %f.Frame = type { void (%f.Frame*)*, void (%f.Frame*)*, i8*, i1, [7 x i8], %PackedStruct }
+
+; See if we used correct index to access frame addr field (field 2).
+; CHECK-LABEL: @f(
+; CHECK: %alloc.frame.ptr = alloca i8*, align 8
+; CHECK: %[[FIELD:.+]] = getelementptr inbounds %f.Frame, %f.Frame* %FramePtr, i32 0, i32 2
+; CHECK: %[[ADDR:.+]] = load i8*, i8** %alloc.frame.ptr, align 8
+; CHECK: store i8* %[[ADDR]], i8** %[[FIELD]], align 8
+; CHECK: %[[DATA:.+]] = getelementptr inbounds %f.Frame, %f.Frame* %FramePtr, i32 0, i32 5
+; CHECK: call void @consume(%PackedStruct* %[[DATA]], i32 32, i32 16, i8** %[[FIELD]])
+; CHECK: ret i8*
+
+; See if `llvm.coro.align` and `llvm.coro.raw.frame.ptr.offset` are lowered
+; correctly during deallocation.
+; CHECK-LABEL: @f.destroy(
+; CHECK: call void @consume2(i32 32, i32 16)
+; CHECK: call void @free(i8* %{{.*}})
+
+; CHECK-LABEL: @f.cleanup(
+; CHECK: call void @consume2(i32 32, i32 16)
+; CHECK: call void @free(i8*
+
+declare i8* @llvm.coro.free(token, i8*)
+declare i32 @llvm.coro.size.i32()
+declare i32 @llvm.coro.align.i32()
+declare i32 @llvm.coro.raw.frame.ptr.offset.i32()
+declare i8** @llvm.coro.raw.frame.ptr.addr()
+declare i8  @llvm.coro.suspend(token, i1)
+declare void @llvm.coro.resume(i8*)
+declare void @llvm.coro.destroy(i8*)
+
+declare token @llvm.coro.id(i32, i8*, i8*, i8*)
+declare i1 @llvm.coro.alloc(token)
+declare i8* @llvm.coro.begin(token, i8*)
+declare i1 @llvm.coro.end(i8*, i1)
+
+declare noalias i8* @malloc(i32)
+declare double @print(double)
+declare void @free(i8*)
Index: llvm/lib/Transforms/Coroutines/Coroutines.cpp
===
--- llvm/lib/Transforms/Coroutines/Coroutines.cpp
+++ llvm/lib/Transforms/Coroutines/Coroutines.cpp
@@ -234,6 +234,9 @@
   Shape.CoroBegin = nullptr;
   Shape.CoroEnds.clear();
   Shape.CoroSizes.clear();
+  Shape.CoroAligns.clear();
+  Shape.CoroRawFramePtrOffsets.clear();
+  Shape.CoroRawFramePtrAddrs.clear();
   Shape.CoroSuspends.clear();
 
   Shape.FrameTy = nullptr;
@@ -268,6 +271,15 @@
   case Intrinsic::coro_size:
 CoroSizes.push_back(cast(II));
 break;
+  case Intrinsic::coro_align:
+CoroAligns.push_back(cast(II));
+break;
+  case Intrinsic::coro_raw_frame_ptr_offset:
+CoroRawFramePtrOffsets.push_back(cast(II));
+break;
+  case Intrinsic::coro_raw_frame_ptr_addr:
+

[PATCH] D104465: [clang][deps] Prevent PCH validation failures by padding minimized files

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

dexonsmith added a comment.

> This patch fixes dependency scanning of a TU with prebuilt modular/PCH 
> dependencies.
>
> Such modules/PCH might have been built using non-minimized files, while 
> dependency scanner does use the minimized versions of source files. Implicit 
> build doesn't like the discrepancy in file sizes and errors out.
>
> The issues is worked around by padding the minimized files with whitespace in 
> such scenarios. This approach throws away one advantage of source 
> minimization (the memory savings), but still keeps the benefits of faster 
> preprocessing/lexing.

I'm not sure this is the right approach.

- Don't PCHs sometimes validate based on hashes of inputs instead of size? At 
least, that has been discussed, and this patch would block making the change.
- Can PCHs embed inputs, like modules can? How would that work (or not) 
minimized sources generally?
- What happens if the PCH has a delayed diagnostic, triggered when parsing the 
next thing? Will the fix-it point in the wrong place? (Are there other things 
that depend on reading the original sources when reading a PCH?)

A few other ideas:

1. Disable PCH validation when minimizing. (Is that less robust than your 
current workaround? If so, can you explain why?)
2. Use the original PCH header in the scanning `-cc1`s (translate 
`-include-pch` to `-include`) and switch back in the generated `-cc1`s (back to 
`-include-pch`).
3. Embed instructions in the PCH for how to build it, and make a "minimized" 
version of the PCH.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104465

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

[PATCH] D103750: [analyzer] Handle std::make_unique for SmartPtrModeling

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

In D103750#2823741 , @RedDocMD wrote:

> I would suppose that constructor calls are properly handled. (ie, member 
> constructors are called properly).

Do the above tests pass when your new `evalCall` modeling is enabled?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103750

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

[PATCH] D88666: DirectoryWatcher: add an implementation for Windows

[Stella Stamenova via Phabricator via cfe-commits]

stella.stamenova added a comment.

In D88666#2825300 , @compnerd wrote:

> Interesting, are the logs from the runs available?  I have run the test 
> ~1 times locally and its been stable.  Perhaps the logs can show what is 
> going on.

Unfortunately, no logs :(. I can try to repro locally later today if I have 
time.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88666

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

[PATCH] D88666: DirectoryWatcher: add an implementation for Windows

[Saleem Abdulrasool via Phabricator via cfe-commits]

compnerd added a comment.

Interesting, are the logs from the runs available?  I have run the test ~1 
times locally and its been stable.  Perhaps the logs can show what is going on.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88666

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

[PATCH] D104475: [Clang][Codegen] emit noprofile IR Fn Attr for new Fn Attr no_profile

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

Should `no_profile` specify the inlining behavior? Though `no_sanitize_*` don't 
specify that, either.

As I commented on https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80223 , the 
Linux kernel specifies `noinline` so the exact inlining behavior doesn't matter.

  #define noinstr \
  noinline notrace __attribute((__section__(".noinstr.text")))\
  __no_kcsan __no_sanitize_address




Comment at: clang/test/CodeGen/fprofile.c:19
+// CHECK: attributes [[ATTR]] = {{.*}} noprofile
+// CHECK-NOT: attributes [[ATTR2]] = {{.*}} noprofile

```
// CHECK: attributes [[ATTR2]] = {
// CHECK-NOT: noprofile
// CHECK: }
```

otherwise the test may become stale if the order of ATTR and ATTR2 shuffles.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104475

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

[PATCH] D88666: DirectoryWatcher: add an implementation for Windows

[Stella Stamenova via Phabricator via cfe-commits]

stella.stamenova added a comment.

We still occasionally (every couple of runs) see these tests hang on Windows in 
both Debug and Release. Unfortunately, I don't have access to the machines 
running the tests to debug the tests while they are hanging and I haven't had a 
chance to try to reproduce locally.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88666

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

[PATCH] D104253: [Clang][Codegen] emit noprofile IR Fn Attr for no_instrument_function Fn Attr

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers abandoned this revision.
nickdesaulniers added a comment.

Jotting down driver to front end flag translations:

-fprofile-generate -> -fprofile-instrument=llvm
-fprofile-instr-generate -> -fprofile-instrument=clang
-fcs-profile-generate -> -fprofile-instrument=csllvm
-fprofile-arcs -> -fprofile-arcs

In D104253#2819995 , @MaskRay wrote:

> For the function attributing disabling instrumentation of 
> -fprofile-generate/-fprofile-instr-generate/-fcs-profile-generate/-fprofile-arcs,
>  how about `no_profile`?

Sure, abandoning. Prefer D104475 .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104253

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

[PATCH] D104459: [clang][lex] Ensure minimizer output is never larger than input

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

dexonsmith added a comment.

> This patch ensures the output of source minimizer is never larger than the 
> input. This property will be handy in a follow up patch that makes it 
> possible to pad the minimized output to the size of the original input.

I suppose when I wrote first wrote this I was thinking of a secondary purpose 
of canonicalizing the sources; in which case, adding a trailing newline (for 
example) seems like feature rather than a bug. I'm not too attached to that, 
but I am curious for more context about why it's important to be able to pad 
the file to the same size, and whether that indicates a bug that should be 
fixed somewhere else instead. (I'll read the follow-up patches in the stack...)




Comment at: clang/unittests/Lex/DependencyDirectivesSourceMinimizerTest.cpp:149
   ASSERT_FALSE(minimizeSourceToDependencyDirectives("#define MACRO((a))", 
Out));
-  EXPECT_STREQ("#define MACRO(/* invalid */\n", Out.data());
+  EXPECT_STREQ("#define MACRO\n", Out.data());
 

I'm not comfortable with this change... the idea of the `(/*invalid*/` was to 
guarantee that an invalid macro argument list didn't get minimized to something 
valid.

Perhaps the result could be `"#define MACRO(\n"`, and just strip the comment?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104459

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

[PATCH] D103750: [analyzer] Handle std::make_unique for SmartPtrModeling

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: clang/test/Analysis/smart-ptr-text-output.cpp:3
+// RUN:  -analyzer-checker=core,cplusplus.Move,alpha.cplusplus.SmartPtr\
+// RUN:  -analyzer-config 
cplusplus.SmartPtrModeling:ModelSmartPtrDereference=true\
+// RUN:  -analyzer-output=text -std=c++20 %s -verify=expected

RedDocMD wrote:
> NoQ wrote:
> > Could you double-check that this flag correctly disables all the newly 
> > introduced modeling so that it wasn't accidentally enabled for all users 
> > before it's ready?
> Yup. There are 133 notes and warnings (just search for "// expected-" and see 
> the count). And on setting the flag to `false`, there are 133 errors.
> So it works.
I mean, specifically the modeling added by this patch? I see you wrote an 
entire checker callback that doesn't seem to have an analyzer-config option 
check anywhere in it. Simply having different results isn't sufficient to 
verify this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103750

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

[PATCH] D104475: [Clang][Codegen] emit noprofile IR Fn Attr for new Fn Attr no_profile

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers created this revision.
nickdesaulniers added reviewers: MaskRay, melver, void, davidxl, vsk, phosek.
Herald added a reviewer: aaron.ballman.
Herald added subscribers: wenlei, jdoerfert.
nickdesaulniers requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

noprofile IR attribute already exists to prevent profiling with PGO;
emit that when a function uses the newly added no_profile function
attribute.

The Linux kernel would like to avoid compiler generated code in
functions annotated with such attribute. We already respect this for
libcalls to fentry() and mcount().

Alternate implementation to: https://reviews.llvm.org/D104253
Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80223
Link: 
https://lore.kernel.org/lkml/cakwvodmpti93n2l0_yqkrzldmpxzror7zggszonyaw2pgsh...@mail.gmail.com/


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D104475

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/test/CodeGen/fprofile.c
  clang/test/Misc/pragma-attribute-supported-attributes-list.test


Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -99,6 +99,7 @@
 // CHECK-NEXT: NoMerge (SubjectMatchRule_function)
 // CHECK-NEXT: NoMicroMips (SubjectMatchRule_function)
 // CHECK-NEXT: NoMips16 (SubjectMatchRule_function)
+// CHECK-NEXT: NoProfileFunction (SubjectMatchRule_function)
 // CHECK-NEXT: NoSanitize (SubjectMatchRule_function, 
SubjectMatchRule_objc_method, SubjectMatchRule_variable_is_global)
 // CHECK-NEXT: NoSanitizeSpecific (SubjectMatchRule_function, 
SubjectMatchRule_variable_is_global)
 // CHECK-NEXT: NoSpeculativeLoadHardening (SubjectMatchRule_function, 
SubjectMatchRule_objc_method)
Index: clang/test/CodeGen/fprofile.c
===
--- /dev/null
+++ clang/test/CodeGen/fprofile.c
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -fprofile-instrument=llvm -disable-llvm-passes \
+// RUN:   -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -fprofile-instrument=csllvm -disable-llvm-passes \
+// RUN:   -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -fprofile-instrument=clang -disable-llvm-passes \
+// RUN:   -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -fprofile-arcs -disable-llvm-passes \
+// RUN:   -emit-llvm -o - %s | FileCheck %s
+int g(int);
+
+void __attribute__((no_profile)) no_instr() {
+// CHECK: define {{.*}} void @no_instr() [[ATTR:#[0-9]+]]
+}
+
+void instr(void) {
+// CHECK: define {{.*}} void @instr() [[ATTR2:#[0-9]+]]
+}
+// CHECK: attributes [[ATTR]] = {{.*}} noprofile
+// CHECK-NOT: attributes [[ATTR2]] = {{.*}} noprofile
Index: clang/lib/CodeGen/CodeGenFunction.cpp
===
--- clang/lib/CodeGen/CodeGenFunction.cpp
+++ clang/lib/CodeGen/CodeGenFunction.cpp
@@ -893,6 +893,9 @@
   if (D && D->hasAttr())
 Fn->addFnAttr("cfi-canonical-jump-table");
 
+  if (D && D->hasAttr())
+Fn->addFnAttr(llvm::Attribute::NoProfile);
+
   if (getLangOpts().OpenCL) {
 // Add metadata for a kernel function.
 if (const FunctionDecl *FD = dyn_cast_or_null(D))
Index: clang/include/clang/Basic/AttrDocs.td
===
--- clang/include/clang/Basic/AttrDocs.td
+++ clang/include/clang/Basic/AttrDocs.td
@@ -2559,6 +2559,17 @@
   }];
 }
 
+def NoProfileDocs : Documentation {
+  let Category = DocCatFunction;
+  let Content = [{
+Use the ``no_profile`` attribute on a function to declaration to denote that
+the compiler should not instrument the function with profile related
+instrumentation, such as via the
+``-fprofile-generate`` / ``-fprofile-instr-generate`` /
+``-fcs-profile-generate`` / ``-fprofile-arcs`` flags.
+}];
+}
+
 def NoSanitizeDocs : Documentation {
   let Category = DocCatFunction;
   let Content = [{
Index: clang/include/clang/Basic/Attr.td
===
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -1970,6 +1970,13 @@
   let SimpleHandler = 1;
 }
 
+def NoProfileFunction : InheritableAttr {
+  let Spellings = [GCC<"no_profile">];
+  let Subjects = SubjectList<[Function]>;
+  let Documentation = [NoProfileDocs];
+  let SimpleHandler = 1;
+}
+
 def NotTailCalled : InheritableAttr {
   let Spellings = [Clang<"not_tail_called">];
   let Subjects = SubjectList<[Function]>;


Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ 

[PATCH] D104381: [analyzer] Added a test case for PR46264

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Thanks!

I guess there might be some value in building an older Clang from around when 
the bug was reported so that to see if the crash was indeed there and we're not 
just reproducing it wrong. It would also allow us to understand where we fixed 
it through bisecting. But I don't insist.




Comment at: clang/test/Analysis/diagnostics/PR46264.cpp:4
+// PR46264
+// This case shall not warn about dereference of void*
+namespace ns1 {

I think you mean this case shall not crash with an assertion failure.

"Warn" exclusively means "display a warning to the user".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104381

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

[PATCH] D103936: [compiler-rt][hwasan] Define fuchsia implementations of required hwasan functions

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added a comment.

*ping* Anything else that should be updated?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103936

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

[PATCH] D103544: [compiler-rt][Fuchsia] Disable interceptors while enabling new/delete replacements

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added a comment.

*ping*


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103544

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

[PATCH] D99381: [compiler-rt][hwasan] Add Fuchsia-specific sanitizer platform limits

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added a comment.

*ping*


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99381

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

[PATCH] D103930: [clang][HeaderSearch] Fix implicit module when using header maps

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

dexonsmith added a subscriber: arphaman.
dexonsmith added a comment.

In D103930#2820725 , @bruno wrote:

> Thanks for working on this, comments inline. @vsapsai @jansvoboda11 
> @dexonsmith any headermap related concerns on your side?

@jansvoboda11, I think it'd be prudent for us to test this patch out internally 
before it's landed, since I don't really trust that the existing unit tests 
cover all the interactions between header maps and modules. Might you be able 
to coordinate something with @arphaman?




Comment at: clang/test/Modules/implicit-module-header-maps.cpp:27
+#define FOO
+// This include will fail if modules weren't used.  The include name itself
+// doesn't exist and relies on the header map to remap it to the real header.

Do you really mean if modules aren't used, or do you mean if header maps aren't 
used?

(I think we want to find the same headers on disk whether or not modules are 
on... if this patch changes that, then I guess I'm not totally understanding 
why...)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103930

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

[PATCH] D104285: [analyzer] Retrieve value by direct index from list initialization of constant array declaration.

[Chris Hamilton via Phabricator via cfe-commits]

chrish_ericsson_atx added a comment.

In D104285#2823305 , 
@chrish_ericsson_atx wrote:

> I'm not sure about whether or not this patch would only work for constant 
> arrays with initializer lists.  If it does only work for such arrays, then I 
> wonder whether the fix is broad enough -- I haven't tested (yet), but I think 
> the presence of the initializer list in the test case is not necessary to 
> trigger the spurious warnings about garbage/undefined values.  I'll try it 
> tomorrow morning...

I tested with an unpatched build using a reproducer without an initializer 
list, and didn't get the spurious warnings -- so your approach seems correct to 
me now.   I've also tested your patch and I believe it gives correct behavior.




Comment at: clang/lib/AST/Type.cpp:149
+Extents.push_back(*CAT->getSize().getRawData());
+  } while (CAT = dyn_cast(CAT->getElementType()));
+  return Extents;




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104285

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

[PATCH] D104198: [Matrix] Add documentation for compound assignment and type conversion of matrix types

[Saurabh Jha via Phabricator via cfe-commits]

SaurabhJha updated this revision to Diff 352783.
SaurabhJha added a comment.

Address comment: replace scalar variables by values


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104198

Files:
  clang/docs/LanguageExtensions.rst


Index: clang/docs/LanguageExtensions.rst
===
--- clang/docs/LanguageExtensions.rst
+++ clang/docs/LanguageExtensions.rst
@@ -523,6 +523,64 @@
 return a + b * c;
   }
 
+The matrix type extension also supports operations between a matrix and a 
scalar.
+
+.. code-block:: c++
+
+  typedef float m4x4_t __attribute__((matrix_type(4, 4)));
+
+  m4x4_t f(m4x4_t a) {
+  return (a + 23) * 12;
+  }
+
+The matrix type extension supports division between a matrix and a scalar but 
not between a matrix and a matrix.
+
+.. code-block:: c++
+  typedef float m4x4_t __attribute__((matrix_type(4, 4)));
+
+  m4x4_t f(m4x4_t a) {
+a = a / 3.0;
+return a;
+  }
+
+The matrix type extension supports compound assignments for addition, 
subtraction, and multiplication between matrices
+and between a matrix and a scalar, provided their types are consistent.
+
+.. code-block:: c++
+
+  typedef float m4x4_t __attribute__((matrix_type(4, 4)));
+
+  m4x4_t f(m4x4_t a, m4x4_t b) {
+a += b;
+a -= b;
+a *= b;
+a += 23;
+a -= 12;
+return a;
+  }
+
+The matrix type extension supports explicit casts. The casts we support are 
C-style casts in C and C++ and
+static casts. Implicit type conversion between matrix types is not allowed.
+
+.. code-block:: c++
+
+  typedef int ix5x5 __attribute__((matrix_type(5, 5)));
+  typedef float fx5x5 __attribute__((matrix_type(5, 5)));
+
+  fx5x5 f1(ix5x5 i, fx5x5 f) {
+return (fx5x5) i;
+  }
+
+
+  template 
+  using matrix_4_4 = X __attribute__((matrix_type(4, 4)));
+
+  void f2() {
+matrix_5_5 d;
+matrix_5_5 i;
+i = (matrix_5_5)d;
+i = static_cast>(d);
+  }
 
 Half-Precision Floating Point
 =


Index: clang/docs/LanguageExtensions.rst
===
--- clang/docs/LanguageExtensions.rst
+++ clang/docs/LanguageExtensions.rst
@@ -523,6 +523,64 @@
 return a + b * c;
   }
 
+The matrix type extension also supports operations between a matrix and a scalar.
+
+.. code-block:: c++
+
+  typedef float m4x4_t __attribute__((matrix_type(4, 4)));
+
+  m4x4_t f(m4x4_t a) {
+  return (a + 23) * 12;
+  }
+
+The matrix type extension supports division between a matrix and a scalar but not between a matrix and a matrix.
+
+.. code-block:: c++
+  typedef float m4x4_t __attribute__((matrix_type(4, 4)));
+
+  m4x4_t f(m4x4_t a) {
+a = a / 3.0;
+return a;
+  }
+
+The matrix type extension supports compound assignments for addition, subtraction, and multiplication between matrices
+and between a matrix and a scalar, provided their types are consistent.
+
+.. code-block:: c++
+
+  typedef float m4x4_t __attribute__((matrix_type(4, 4)));
+
+  m4x4_t f(m4x4_t a, m4x4_t b) {
+a += b;
+a -= b;
+a *= b;
+a += 23;
+a -= 12;
+return a;
+  }
+
+The matrix type extension supports explicit casts. The casts we support are C-style casts in C and C++ and
+static casts. Implicit type conversion between matrix types is not allowed.
+
+.. code-block:: c++
+
+  typedef int ix5x5 __attribute__((matrix_type(5, 5)));
+  typedef float fx5x5 __attribute__((matrix_type(5, 5)));
+
+  fx5x5 f1(ix5x5 i, fx5x5 f) {
+return (fx5x5) i;
+  }
+
+
+  template 
+  using matrix_4_4 = X __attribute__((matrix_type(4, 4)));
+
+  void f2() {
+matrix_5_5 d;
+matrix_5_5 i;
+i = (matrix_5_5)d;
+i = static_cast>(d);
+  }
 
 Half-Precision Floating Point
 =
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D104314: [AIX] Remove --as-needed passing into aix linker

[Jason Liu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG4e2aee8d3bab: [AIX] Remove --as-needed passing into aix 
linker (authored by jasonliu).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104314

Files:
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/test/Driver/aix-ld.c

Index: clang/test/Driver/aix-ld.c
===
--- clang/test/Driver/aix-ld.c
+++ clang/test/Driver/aix-ld.c
@@ -20,7 +20,9 @@
 // CHECK-LD32-NOT: "-lc++"
 // CHECK-LD32-NOT: "-lc++abi"
 // CHECK-LD32: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc.a"
+// CHECK-LD32-NOT: "--as-needed"
 // CHECK-LD32: "-lunwind"
+// CHECK-LD32-NOT: "--no-as-needed"
 // CHECK-LD32-NOT: "-lm"
 // CHECK-LD32: "-lc"
 
@@ -43,7 +45,9 @@
 // CHECK-LD64-NOT: "-lc++"
 // CHECK-LD64-NOT: "-lc++abi"
 // CHECK-LD64: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc64.a"
+// CHECK-LD64-NOT: "--as-needed"
 // CHECK-LD64: "-lunwind"
+// CHECK-LD64-NOT: "--no-as-needed"
 // CHECK-LD64-NOT: "-lm"
 // CHECK-LD64: "-lc"
 
@@ -67,7 +71,9 @@
 // CHECK-LD32-PTHREAD-NOT: "-lc++"
 // CHECK-LD32-PTHREAD-NOT: "-lc++abi"
 // CHECK-LD32-PTHREAD: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc.a"
+// CHECK-LD32-PTHREAD-NOT: "--as-needed"
 // CHECK-LD32-PTHREAD: "-lunwind"
+// CHECK-LD32-PTHREAD-NOT: "--no-as-needed"
 // CHECK-LD32-PTHREAD: "-lpthreads"
 // CHECK-LD32-PTHREAD-NOT: "-lm"
 // CHECK-LD32-PTHREAD: "-lc"
@@ -92,7 +98,9 @@
 // CHECK-LD64-PTHREAD-NOT: "-lc++"
 // CHECK-LD64-PTHREAD-NOT: "-lc++abi"
 // CHECK-LD64-PTHREAD: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc64.a"
+// CHECK-LD64-PTHREAD-NOT: "--as-needed"
 // CHECK-LD64-PTHREAD: "-lunwind"
+// CHECK-LD64-PTHREAD-NOT: "--no-as-needed"
 // CHECK-LD64-PTHREAD: "-lpthreads"
 // CHECK-LD64-PTHREAD-NOT: "-lm"
 // CHECK-LD64-PTHREAD: "-lc"
@@ -117,7 +125,9 @@
 // CHECK-LD32-PROF-NOT: "-lc++"
 // CHECK-LD32-PROF-NOT: "-lc++abi"
 // CHECK-LD32-PROF: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc.a"
+// CHECK-LD32-PROF-NOT: "--as-needed"
 // CHECK-LD32-PROF: "-lunwind"
+// CHECK-LD32-PROF-NOT: "--no-as-needed"
 // CHECK-LD32-PROF-NOT: "-lm"
 // CHECK-LD32-PROF: "-lc"
 
@@ -141,7 +151,9 @@
 // CHECK-LD64-GPROF-NOT: "-lc++"
 // CHECK-LD64-GPROF-NOT: "-lc++abi"
 // CHECK-LD64-GPROF: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc64.a"
+// CHECK-LD64-GPROF-NOT: "--as-needed"
 // CHECK-LD64-GPROF: "-lunwind"
+// CHECK-LD64-GPROF-NOT: "--no-as-needed"
 // CHECK-LD64-GPROF-NOT: "-lm"
 // CHECK-LD64-GPROF: "-lc"
 
@@ -165,7 +177,9 @@
 // CHECK-LD32-STATIC-NOT: "-lc++"
 // CHECK-LD32-STATIC-NOT: "-lc++abi"
 // CHECK-LD32-STATIC: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc.a"
+// CHECK-LD32-STATIC-NOT: "--as-needed"
 // CHECK-LD32-STATIC-NOT: "-lunwind"
+// CHECK-LD32-STATIC-NOT: "--no-as-needed"
 // CHECK-LD32-STATIC-NOT: "-lm"
 // CHECK-LD32-STATIC: "-lc"
 
@@ -190,7 +204,9 @@
 // CHECK-LD32-LIBP-NOT: "-lc++"
 // CHECK-LD32-LIBP-NOT: "-lc++abi"
 // CHECK-LD32-LIBP: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc.a"
+// CHECK-LD32-LIBP-NOT: "--as-needed"
 // CHECK-LD32-LIBP: "-lunwind"
+// CHECK-LD32-LIBP-NOT: "--no-as-needed"
 // CHECK-LD32-LIBP-NOT: "-lm"
 // CHECK-LD32-LIBP: "-lc"
 
@@ -215,7 +231,9 @@
 // CHECK-LD32-NO-STD-LIB-NOT: "-lc++"
 // CHECK-LD32-NO-STD-LIB-NOT: "-lc++abi"
 // CHECK-LD32-NO-STD-LIB-NOT: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc.a"
+// CHECK-LD32-NO-STD-LIB-NOT: "--as-needed"
 // CHECK-LD32-NO-STD-LIB-NOT: "-lunwind"
+// CHECK-LD32-NO-STD-LIB-NOT: "--no-as-needed"
 // CHECK-LD32-NO-STD-LIB-NOT: "-lpthreads"
 // CHECK-LD32-NO-STD-LIB-NOT: "-lm"
 // CHECK-LD32-NO-STD-LIB-NOT: "-lc"
@@ -241,7 +259,9 @@
 // CHECK-LD64-NO-DEFAULT-LIBS-NOT: "-lc++"
 // CHECK-LD64-NO-DEFAULT-LIBS-NOT: "-lc++abi"
 // CHECK-LD64-NO-DEFAULT-LIBS-NOT: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc64.a"
+// CHECK-LD64-NO-DEFAULT-LIBS-NOT: "--as-needed"
 // CHECK-LD64-NO-DEFAULT-LIBS-NOT: "-lunwind"
+// CHECK-LD64-NO-DEFAULT-LIBS-NOT: "--no-as-needed"
 // CHECK-LD64-NO-DEFAULT-LIBS-NOT: "-lpthreads"
 // CHECK-LD64-NO-DEFAULT-LIBS-NOT: "-lm"
 // CHECK-LD64-NO-DEFAULT-LIBS-NOT: "-lc"
@@ -268,7 +288,9 @@
 // CHECK-LD32-ARG-ORDER-NOT: "-lc++"
 // CHECK-LD32-ARG-ORDER-NOT: "-lc++abi"
 // CHECK-LD32-ARG-ORDER: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc.a"
+// CHECK-LD32-ARG-ORDER-NOT: "--as-needed"
 // 

[clang] 4e2aee8 - [AIX] Remove --as-needed passing into aix linker

[via cfe-commits]

Author: jasonliu
Date: 2021-06-17T17:16:41Z
New Revision: 4e2aee8d3bab6010420d9be96480f1d8ae0f35c1

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

LOG: [AIX] Remove --as-needed passing into aix linker

Summary:
AIX does not support --as-needed linker options. Remove that option from
aix linker when -lunwind is needed.
For unwinder library, nothing special is needed because by default aix
linker has the as-needed effect for library that's an archive (which is
the case for libunwind on AIX).

Reviewed By: daltenty

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

Added: 


Modified: 
clang/lib/Driver/ToolChains/CommonArgs.cpp
clang/test/Driver/aix-ld.c

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp 
b/clang/lib/Driver/ToolChains/CommonArgs.cpp
index 2c176b207bc1d..cfda0ff1852c3 100644
--- a/clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -738,6 +738,9 @@ static bool addSanitizerDynamicList(const ToolChain , 
const ArgList ,
 }
 
 static const char *getAsNeededOption(const ToolChain , bool as_needed) {
+  assert(!TC.getTriple().isOSAIX() &&
+ "AIX linker does not support any form of --as-needed option yet.");
+
   // While the Solaris 11.2 ld added --as-needed/--no-as-needed as aliases
   // for the native forms -z ignore/-z record, they are missing in Illumos,
   // so always use the native form.
@@ -1423,7 +1426,8 @@ static void AddUnwindLibrary(const ToolChain , const 
Driver ,
 
   LibGccType LGT = getLibGccType(TC, D, Args);
   bool AsNeeded = LGT == LibGccType::UnspecifiedLibGcc &&
-  !TC.getTriple().isAndroid() && !TC.getTriple().isOSCygMing();
+  !TC.getTriple().isAndroid() &&
+  !TC.getTriple().isOSCygMing() && !TC.getTriple().isOSAIX();
   if (AsNeeded)
 CmdArgs.push_back(getAsNeededOption(TC, true));
 

diff  --git a/clang/test/Driver/aix-ld.c b/clang/test/Driver/aix-ld.c
index 62f152fd0eb69..c66b235cb1e2c 100644
--- a/clang/test/Driver/aix-ld.c
+++ b/clang/test/Driver/aix-ld.c
@@ -20,7 +20,9 @@
 // CHECK-LD32-NOT: "-lc++"
 // CHECK-LD32-NOT: "-lc++abi"
 // CHECK-LD32: 
"[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc.a"
+// CHECK-LD32-NOT: "--as-needed"
 // CHECK-LD32: "-lunwind"
+// CHECK-LD32-NOT: "--no-as-needed"
 // CHECK-LD32-NOT: "-lm"
 // CHECK-LD32: "-lc"
 
@@ -43,7 +45,9 @@
 // CHECK-LD64-NOT: "-lc++"
 // CHECK-LD64-NOT: "-lc++abi"
 // CHECK-LD64: 
"[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc64.a"
+// CHECK-LD64-NOT: "--as-needed"
 // CHECK-LD64: "-lunwind"
+// CHECK-LD64-NOT: "--no-as-needed"
 // CHECK-LD64-NOT: "-lm"
 // CHECK-LD64: "-lc"
 
@@ -67,7 +71,9 @@
 // CHECK-LD32-PTHREAD-NOT: "-lc++"
 // CHECK-LD32-PTHREAD-NOT: "-lc++abi"
 // CHECK-LD32-PTHREAD: 
"[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc.a"
+// CHECK-LD32-PTHREAD-NOT: "--as-needed"
 // CHECK-LD32-PTHREAD: "-lunwind"
+// CHECK-LD32-PTHREAD-NOT: "--no-as-needed"
 // CHECK-LD32-PTHREAD: "-lpthreads"
 // CHECK-LD32-PTHREAD-NOT: "-lm"
 // CHECK-LD32-PTHREAD: "-lc"
@@ -92,7 +98,9 @@
 // CHECK-LD64-PTHREAD-NOT: "-lc++"
 // CHECK-LD64-PTHREAD-NOT: "-lc++abi"
 // CHECK-LD64-PTHREAD: 
"[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc64.a"
+// CHECK-LD64-PTHREAD-NOT: "--as-needed"
 // CHECK-LD64-PTHREAD: "-lunwind"
+// CHECK-LD64-PTHREAD-NOT: "--no-as-needed"
 // CHECK-LD64-PTHREAD: "-lpthreads"
 // CHECK-LD64-PTHREAD-NOT: "-lm"
 // CHECK-LD64-PTHREAD: "-lc"
@@ -117,7 +125,9 @@
 // CHECK-LD32-PROF-NOT: "-lc++"
 // CHECK-LD32-PROF-NOT: "-lc++abi"
 // CHECK-LD32-PROF: 
"[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc.a"
+// CHECK-LD32-PROF-NOT: "--as-needed"
 // CHECK-LD32-PROF: "-lunwind"
+// CHECK-LD32-PROF-NOT: "--no-as-needed"
 // CHECK-LD32-PROF-NOT: "-lm"
 // CHECK-LD32-PROF: "-lc"
 
@@ -141,7 +151,9 @@
 // CHECK-LD64-GPROF-NOT: "-lc++"
 // CHECK-LD64-GPROF-NOT: "-lc++abi"
 // CHECK-LD64-GPROF: 
"[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc64.a"
+// CHECK-LD64-GPROF-NOT: "--as-needed"
 // CHECK-LD64-GPROF: "-lunwind"
+// CHECK-LD64-GPROF-NOT: "--no-as-needed"
 // CHECK-LD64-GPROF-NOT: "-lm"
 // CHECK-LD64-GPROF: "-lc"
 
@@ -165,7 +177,9 @@
 // CHECK-LD32-STATIC-NOT: "-lc++"
 // CHECK-LD32-STATIC-NOT: "-lc++abi"
 // CHECK-LD32-STATIC: 
"[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc.a"
+// CHECK-LD32-STATIC-NOT: "--as-needed"
 // CHECK-LD32-STATIC-NOT: "-lunwind"
+// CHECK-LD32-STATIC-NOT: "--no-as-needed"
 // CHECK-LD32-STATIC-NOT: "-lm"
 

[PATCH] D101976: [OpenMP] Unified entry point for SPMD & generic kernels in the device RTL

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert updated this revision to Diff 352776.
jdoerfert added a comment.

Fix tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101976

Files:
  clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
  clang/lib/CodeGen/CGOpenMPRuntimeGPU.h
  clang/test/OpenMP/amdgcn_target_codegen.cpp
  clang/test/OpenMP/assumes_include_nvptx.cpp
  clang/test/OpenMP/declare_target_codegen_globalization.cpp
  clang/test/OpenMP/nvptx_SPMD_codegen.cpp
  clang/test/OpenMP/nvptx_data_sharing.cpp
  clang/test/OpenMP/nvptx_distribute_parallel_generic_mode_codegen.cpp
  clang/test/OpenMP/nvptx_force_full_runtime_SPMD_codegen.cpp
  clang/test/OpenMP/nvptx_lambda_capturing.cpp
  clang/test/OpenMP/nvptx_multi_target_parallel_codegen.cpp
  clang/test/OpenMP/nvptx_nested_parallel_codegen.cpp
  clang/test/OpenMP/nvptx_parallel_codegen.cpp
  clang/test/OpenMP/nvptx_parallel_for_codegen.cpp
  clang/test/OpenMP/nvptx_target_codegen.cpp
  clang/test/OpenMP/nvptx_target_firstprivate_codegen.cpp
  clang/test/OpenMP/nvptx_target_parallel_codegen.cpp
  clang/test/OpenMP/nvptx_target_parallel_num_threads_codegen.cpp
  clang/test/OpenMP/nvptx_target_parallel_proc_bind_codegen.cpp
  clang/test/OpenMP/nvptx_target_parallel_reduction_codegen.cpp
  clang/test/OpenMP/nvptx_target_parallel_reduction_codegen_tbaa_PR46146.cpp
  clang/test/OpenMP/nvptx_target_printf_codegen.c
  clang/test/OpenMP/nvptx_target_simd_codegen.cpp
  clang/test/OpenMP/nvptx_target_teams_codegen.cpp
  clang/test/OpenMP/nvptx_target_teams_distribute_codegen.cpp
  clang/test/OpenMP/nvptx_target_teams_distribute_parallel_for_codegen.cpp
  
clang/test/OpenMP/nvptx_target_teams_distribute_parallel_for_generic_mode_codegen.cpp
  clang/test/OpenMP/nvptx_target_teams_distribute_parallel_for_simd_codegen.cpp
  clang/test/OpenMP/nvptx_target_teams_distribute_simd_codegen.cpp
  clang/test/OpenMP/nvptx_teams_codegen.cpp
  clang/test/OpenMP/nvptx_teams_reduction_codegen.cpp
  clang/test/OpenMP/remarks_parallel_in_multiple_target_state_machines.c
  clang/test/OpenMP/remarks_parallel_in_target_state_machine.c
  clang/test/OpenMP/target_parallel_debug_codegen.cpp
  clang/test/OpenMP/target_parallel_for_debug_codegen.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
  llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
  llvm/test/Transforms/OpenMP/single_threaded_execution.ll
  openmp/libomptarget/deviceRTLs/common/include/target.h
  openmp/libomptarget/deviceRTLs/common/src/omptarget.cu
  openmp/libomptarget/deviceRTLs/common/src/parallel.cu
  openmp/libomptarget/deviceRTLs/interface.h
  openmp/libomptarget/deviceRTLs/nvptx/src/target_impl.cu

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

[PATCH] D104311: [clang] Fix a race condition in the build of clangInterpreter

[Stella Stamenova via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG734d688fbce8: [clang] Fix a race condition in the build of 
clangInterpreter (authored by stella.stamenova).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104311

Files:
  clang/lib/Interpreter/CMakeLists.txt


Index: clang/lib/Interpreter/CMakeLists.txt
===
--- clang/lib/Interpreter/CMakeLists.txt
+++ clang/lib/Interpreter/CMakeLists.txt
@@ -12,6 +12,9 @@
   IncrementalParser.cpp
   Interpreter.cpp
 
+  DEPENDS
+  intrinsics_gen
+
   LINK_LIBS
   clangAST
   clangAnalysis


Index: clang/lib/Interpreter/CMakeLists.txt
===
--- clang/lib/Interpreter/CMakeLists.txt
+++ clang/lib/Interpreter/CMakeLists.txt
@@ -12,6 +12,9 @@
   IncrementalParser.cpp
   Interpreter.cpp
 
+  DEPENDS
+  intrinsics_gen
+
   LINK_LIBS
   clangAST
   clangAnalysis
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 734d688 - [clang] Fix a race condition in the build of clangInterpreter

[Stella Stamenova via cfe-commits]

Author: Stella Stamenova
Date: 2021-06-17T10:03:33-07:00
New Revision: 734d688fbce8a453aa61764b9b5a43b26455dc0d

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

LOG: [clang] Fix a race condition in the build of clangInterpreter

The library depends on Attributes.inc, so it has to depend on the 
intrinsics_gen target

Reviewed By: v.g.vassilev

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

Added: 


Modified: 
clang/lib/Interpreter/CMakeLists.txt

Removed: 




diff  --git a/clang/lib/Interpreter/CMakeLists.txt 
b/clang/lib/Interpreter/CMakeLists.txt
index e08779945b5fc..88a0a716e1269 100644
--- a/clang/lib/Interpreter/CMakeLists.txt
+++ b/clang/lib/Interpreter/CMakeLists.txt
@@ -12,6 +12,9 @@ add_clang_library(clangInterpreter
   IncrementalParser.cpp
   Interpreter.cpp
 
+  DEPENDS
+  intrinsics_gen
+
   LINK_LIBS
   clangAST
   clangAnalysis



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

[PATCH] D93528: [clang-format] Add basic support for formatting JSON

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay updated this revision to Diff 352773.
MyDeveloperDay added a comment.

JSON strings can't be split when the line is too long like c++ strings


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

https://reviews.llvm.org/D93528

Files:
  clang/docs/ClangFormat.rst
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/Format.cpp
  clang/lib/Format/TokenAnnotator.cpp
  clang/tools/clang-format/ClangFormat.cpp
  clang/unittests/Format/CMakeLists.txt
  clang/unittests/Format/FormatTestJson.cpp

Index: clang/unittests/Format/FormatTestJson.cpp
===
--- /dev/null
+++ clang/unittests/Format/FormatTestJson.cpp
@@ -0,0 +1,117 @@
+//===- unittest/Format/FormatTestJson.cpp - Formatting tests for Json -===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "FormatTestUtils.h"
+#include "clang/Format/Format.h"
+#include "llvm/Support/Debug.h"
+#include "gtest/gtest.h"
+
+#define DEBUG_TYPE "format-test-json"
+
+namespace clang {
+namespace format {
+
+class FormatTestJson : public ::testing::Test {
+protected:
+  static std::string format(llvm::StringRef Code, unsigned Offset,
+unsigned Length, const FormatStyle ) {
+LLVM_DEBUG(llvm::errs() << "---\n");
+LLVM_DEBUG(llvm::errs() << Code << "\n\n");
+
+tooling::Replacements Replaces;
+
+// Mock up what ClangFormat.cpp will do for JSON by adding a variable
+// to trick JSON into being JavaScript
+if (Style.isJson()) {
+  auto Err = Replaces.add(
+  tooling::Replacement(tooling::Replacement("", 0, 0, "x = ")));
+  if (Err) {
+llvm::errs() << "Bad Json variable insertion\n";
+  }
+}
+auto ChangedCode = applyAllReplacements(Code, Replaces);
+if (!ChangedCode) {
+  llvm::errs() << "Bad Json varibale replacement\n";
+}
+StringRef NewCode = *ChangedCode;
+
+std::vector Ranges(1, tooling::Range(0, NewCode.size()));
+Replaces = reformat(Style, NewCode, Ranges);
+auto Result = applyAllReplacements(NewCode, Replaces);
+EXPECT_TRUE(static_cast(Result));
+LLVM_DEBUG(llvm::errs() << "\n" << *Result << "\n\n");
+return *Result;
+  }
+
+  static std::string
+  format(llvm::StringRef Code,
+ const FormatStyle  = getLLVMStyle(FormatStyle::LK_Json)) {
+return format(Code, 0, Code.size(), Style);
+  }
+
+  static FormatStyle getStyleWithColumns(unsigned ColumnLimit) {
+FormatStyle Style = getLLVMStyle(FormatStyle::LK_Json);
+Style.ColumnLimit = ColumnLimit;
+return Style;
+  }
+
+  static void
+  verifyFormat(llvm::StringRef Code,
+   const FormatStyle  = getLLVMStyle(FormatStyle::LK_Json)) {
+EXPECT_EQ(Code.str(), format(Code, Style)) << "Expected code is not stable";
+EXPECT_EQ(Code.str(), format(test::messUp(Code), Style));
+  }
+};
+
+TEST_F(FormatTestJson, JsonRecord) {
+  verifyFormat("{}");
+  verifyFormat("{\n"
+   "  \"name\": 1\n"
+   "}");
+  verifyFormat("{\n"
+   "  \"name\": \"Foo\"\n"
+   "}");
+}
+
+TEST_F(FormatTestJson, JsonArray) {
+  verifyFormat("[]");
+  verifyFormat("[\n"
+   "  1\n"
+   "]");
+  verifyFormat("[\n"
+   "  1,\n"
+   "  2\n"
+   "]");
+  verifyFormat("[\n"
+   "  {},\n"
+   "  {}\n"
+   "]");
+  verifyFormat("[\n"
+   "  {\n"
+   "\"name\": 1\n"
+   "  },\n"
+   "  {}\n"
+   "]");
+}
+
+TEST_F(FormatTestJson, JsonNoStringSplit) {
+  FormatStyle Style = getLLVMStyle(FormatStyle::LK_Json);
+  Style.IndentWidth = 4;
+  verifyFormat("[\n"
+   "{\n"
+   ""
+   "\"n\":"
+   " \"foo oo\"\n"
+   "},\n"
+   "{}\n"
+   "]",
+   Style);
+}
+
+} // namespace format
+} // end namespace clang
Index: clang/unittests/Format/CMakeLists.txt
===
--- clang/unittests/Format/CMakeLists.txt
+++ clang/unittests/Format/CMakeLists.txt
@@ -9,6 +9,7 @@
   FormatTestCSharp.cpp
   FormatTestJS.cpp
   FormatTestJava.cpp
+  FormatTestJson.cpp
   FormatTestObjC.cpp
   FormatTestProto.cpp
   FormatTestRawStrings.cpp
Index: clang/tools/clang-format/ClangFormat.cpp
===
--- clang/tools/clang-format/ClangFormat.cpp
+++ clang/tools/clang-format/ClangFormat.cpp
@@ -411,6 +411,17 @@
   unsigned 

[PATCH] D104096: [Clang-Format] Add ReferenceAlignment directive

[Seraphime Kirkovski (VMware) via Phabricator via cfe-commits]

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

Rebased and fixed warning. My local compiler was yelling at me for adding a 
default case to switch which already covers everything.

For the commit:

Author: Seraphime Kirkovski
email: skirkovski at vmware.com


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104096

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

[PATCH] D104376: [clangd] Correct SelectionTree behavior around anonymous field access.

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clang-tools-extra/clangd/Selection.cpp:71
+  // so we reduce its range to match the CXXConstructExpr.
+  // (It's not clear that changing the clang AST would be correct in general).
+  if (const auto *ME = N.get()) {

hokein wrote:
> I still think it is worth to explore this direction (will take a look on this 
> if I have time), but also ok with the current approach. 
SG, this isn't urgent so I'll leave this open while out on vacation. I agree 
it's suspect/suggestive that we need to override the AST source range in 
exactly one case.

My concern was conceptual: in `A().b` if you want to describe where the anon 
field ref happens, I'd pick `.` or `b` so placing those outside the range is 
suspect. Conversely a range of `A()` seems weird because that's a perfectly 
valid expression with no dereferencing.

OTOH if you squint I guess this looks a bit like an implicit cast from the `A` 
to the anon member. So maybe it's fine and just a question of mental model.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104376

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

[PATCH] D104096: [Clang-Format] Add ReferenceAlignment directive

[Seraphime Kirkovski (VMware) via Phabricator via cfe-commits]

skirkovski updated this revision to Diff 352753.
skirkovski added a comment.

Rebase and fix warning


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104096

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

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -1230,6 +1230,98 @@
getLLVMStyleWithColumns(62));
 }
 
+TEST_F(FormatTest, SeparatePointerReferenceAlignment) {
+  FormatStyle Style = getLLVMStyle();
+  // Check first the default LLVM style
+  // Style.PointerAlignment = FormatStyle::PAS_Right;
+  // Style.ReferenceAlignment = FormatStyle::RAS_Pointer;
+  verifyFormat("int *f1(int *a, int , int &);", Style);
+  verifyFormat("int (int &, int *a, int );", Style);
+  verifyFormat("int &(int , int &, int *a);", Style);
+  verifyFormat("int *f1(int ) const &;", Style);
+  verifyFormat("int *f1(int ) const & = 0;", Style);
+  verifyFormat("int *a = f1();", Style);
+  verifyFormat("int  = f2();", Style);
+  verifyFormat("int & = f3();", Style);
+
+  Style.AlignConsecutiveDeclarations = FormatStyle::ACS_Consecutive;
+  verifyFormat("Const unsigned int *c;\n"
+   "const unsigned int *d;\n"
+   "Const unsigned int \n"
+   "const unsigned int \n"
+   "const unsigned&\n"
+   "Const unsigned  h;",
+   Style);
+
+  Style.PointerAlignment = FormatStyle::PAS_Left;
+  Style.ReferenceAlignment = FormatStyle::RAS_Pointer;
+  verifyFormat("int* f1(int* a, int& b, int&& c);", Style);
+  verifyFormat("int& f2(int&& c, int* a, int& b);", Style);
+  verifyFormat("int&& f3(int& b, int&& c, int* a);", Style);
+  verifyFormat("int* f1(int& a) const& = 0;", Style);
+  verifyFormat("int* a = f1();", Style);
+  verifyFormat("int& b = f2();", Style);
+  verifyFormat("int&& c = f3();", Style);
+
+  Style.AlignConsecutiveDeclarations = FormatStyle::ACS_Consecutive;
+  verifyFormat("Const unsigned int* c;\n"
+   "const unsigned int* d;\n"
+   "Const unsigned int& e;\n"
+   "const unsigned int& f;\n"
+   "const unsigned&&g;\n"
+   "Const unsigned  h;",
+   Style);
+
+  Style.PointerAlignment = FormatStyle::PAS_Right;
+  Style.ReferenceAlignment = FormatStyle::RAS_Left;
+  verifyFormat("int *f1(int *a, int& b, int&& c);", Style);
+  verifyFormat("int& f2(int&& c, int *a, int& b);", Style);
+  verifyFormat("int&& f3(int& b, int&& c, int *a);", Style);
+  verifyFormat("int *a = f1();", Style);
+  verifyFormat("int& b = f2();", Style);
+  verifyFormat("int&& c = f3();", Style);
+
+  Style.AlignConsecutiveDeclarations = FormatStyle::ACS_Consecutive;
+  verifyFormat("Const unsigned int *c;\n"
+   "const unsigned int *d;\n"
+   "Const unsigned int& e;\n"
+   "const unsigned int& f;\n"
+   "const unsigned  g;\n"
+   "Const unsigned  h;",
+   Style);
+
+  Style.PointerAlignment = FormatStyle::PAS_Left;
+  Style.ReferenceAlignment = FormatStyle::RAS_Middle;
+  verifyFormat("int* f1(int* a, int & b, int && c);", Style);
+  verifyFormat("int & f2(int && c, int* a, int & b);", Style);
+  verifyFormat("int && f3(int & b, int && c, int* a);", Style);
+  verifyFormat("int* a = f1();", Style);
+  verifyFormat("int & b = f2();", Style);
+  verifyFormat("int && c = f3();", Style);
+
+  Style.AlignConsecutiveDeclarations = FormatStyle::ACS_Consecutive;
+  verifyFormat("Const unsigned int*  c;\n"
+   "const unsigned int*  d;\n"
+   "Const unsigned int & e;\n"
+   "const unsigned int & f;\n"
+   "const unsigned &&g;\n"
+   "Const unsigned   h;",
+   Style);
+
+  Style.PointerAlignment = FormatStyle::PAS_Middle;
+  Style.ReferenceAlignment = FormatStyle::RAS_Right;
+  verifyFormat("int * f1(int * a, int , int &);", Style);
+  verifyFormat("int (int &, int * a, int );", Style);
+  verifyFormat("int &(int , int &, int * a);", Style);
+  verifyFormat("int * a = f1();", Style);
+  verifyFormat("int  = f2();", Style);
+  verifyFormat("int & = f3();", Style);
+
+  // FIXME: we don't handle this yet, so output may be arbitrary until it's
+  // specifically handled
+  // verifyFormat("int Add2(BTree * , char * szToAdd)", Style);
+}
+
 TEST_F(FormatTest, FormatsForLoop) {
   verifyFormat(
   "for (int VeryVeryLongLoopVariable = 0; VeryVeryLongLoopVariable < 10;\n"
@@ -17454,6 +17546,15 @@
   FormatStyle::PAS_Right);
   CHECK_PARSE("PointerAlignment: Middle", PointerAlignment,
   

[PATCH] D104058: ThinLTO: Fix inline assembly references to static functions with CFI

[Peter Collingbourne via Phabricator via cfe-commits]

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

LGTM




Comment at: clang/test/CodeGen/thinlto-cfi-icall-static-inline-asm.c:3
+
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm-bc -flto=thin 
-fsanitize=cfi-icall -fsplit-lto-unit -o - %s | llvm-dis - | FileCheck %s
+

Can the test be moved to `llvm/test/Transforms/ThinLTOBitcodeWriter` and made 
to use `opt -thinlto-bc`?



Comment at: llvm/lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp:79
+  GlobalValue::InternalLinkage, Name, F, );
+  appendToCompilerUsed(ExportM, A);
+}

samitolvanen wrote:
> Note that I'm adding the alias to llvm.compiler.used because it's otherwise 
> removed during optimization. Is there are better way to accomplish this?
Not as far as I know, that's what I'd recommend.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104058

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

[PATCH] D93528: [clang-format] Add basic support for formatting JSON

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay updated this revision to Diff 352750.
MyDeveloperDay added a comment.

Missing new test file


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

https://reviews.llvm.org/D93528

Files:
  clang/docs/ClangFormat.rst
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/TokenAnnotator.cpp
  clang/tools/clang-format/ClangFormat.cpp
  clang/unittests/Format/CMakeLists.txt
  clang/unittests/Format/FormatTestJson.cpp

Index: clang/unittests/Format/FormatTestJson.cpp
===
--- /dev/null
+++ clang/unittests/Format/FormatTestJson.cpp
@@ -0,0 +1,130 @@
+//===- unittest/Format/FormatTestJson.cpp - Formatting tests for Json -===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "FormatTestUtils.h"
+#include "clang/Format/Format.h"
+#include "llvm/Support/Debug.h"
+#include "gtest/gtest.h"
+
+#define DEBUG_TYPE "format-test-json"
+
+namespace clang {
+namespace format {
+
+class FormatTestJson : public ::testing::Test {
+protected:
+  static std::string format(llvm::StringRef Code, unsigned Offset,
+unsigned Length, const FormatStyle ) {
+LLVM_DEBUG(llvm::errs() << "---\n");
+LLVM_DEBUG(llvm::errs() << Code << "\n\n");
+
+tooling::Replacements Replaces;
+
+// Mock up what ClangFormat.cpp will do for JSON by adding a variable
+// to trick JSON into being JavaScript
+if (Style.isJson()) {
+  auto Err = Replaces.add(
+  tooling::Replacement(tooling::Replacement("", 0, 0, "x = ")));
+  if (Err) {
+llvm::errs() << "Bad Json variable insertion\n";
+  }
+}
+auto ChangedCode = applyAllReplacements(Code, Replaces);
+if (!ChangedCode) {
+  llvm::errs() << "Bad Json varibale replacement\n";
+}
+StringRef NewCode = *ChangedCode;
+
+std::vector Ranges(1, tooling::Range(0, NewCode.size()));
+Replaces = reformat(Style, NewCode, Ranges);
+auto Result = applyAllReplacements(NewCode, Replaces);
+EXPECT_TRUE(static_cast(Result));
+LLVM_DEBUG(llvm::errs() << "\n" << *Result << "\n\n");
+return *Result;
+  }
+
+  static std::string
+  format(llvm::StringRef Code,
+ const FormatStyle  = getLLVMStyle(FormatStyle::LK_Json)) {
+return format(Code, 0, Code.size(), Style);
+  }
+
+  static FormatStyle getStyleWithColumns(unsigned ColumnLimit) {
+FormatStyle Style = getLLVMStyle(FormatStyle::LK_Json);
+Style.ColumnLimit = ColumnLimit;
+return Style;
+  }
+
+  static void
+  verifyFormat(llvm::StringRef Code,
+   const FormatStyle  = getLLVMStyle(FormatStyle::LK_Json)) {
+EXPECT_EQ(Code.str(), format(Code, Style)) << "Expected code is not stable";
+EXPECT_EQ(Code.str(), format(test::messUp(Code), Style));
+  }
+};
+
+TEST_F(FormatTestJson, JsonRecord) {
+  verifyFormat("{}");
+  verifyFormat("{\n"
+   "  \"name\": 1\n"
+   "}");
+  verifyFormat("{\n"
+   "  \"name\": \"Foo\"\n"
+   "}");
+}
+
+TEST_F(FormatTestJson, JsonArray) {
+  verifyFormat("[]");
+  verifyFormat("[\n"
+   "  1\n"
+   "]");
+  verifyFormat("[\n"
+   "  1,\n"
+   "  2\n"
+   "]");
+  verifyFormat("[\n"
+   "  {},\n"
+   "  {}\n"
+   "]");
+  verifyFormat("[\n"
+   "  {\n"
+   "\"name\": 1\n"
+   "  },\n"
+   "  {}\n"
+   "]");
+}
+
+TEST_F(FormatTestJson, JsonIndent) {
+  FormatStyle Style = getLLVMStyle(FormatStyle::LK_Json);
+  Style.IndentWidth = 4;
+  verifyFormat("[]", Style);
+  verifyFormat("[\n"
+   "10\n"
+   "]",
+   Style);
+  verifyFormat("[\n"
+   "1,\n"
+   "2\n"
+   "]",
+   Style);
+  verifyFormat("[\n"
+   "{},\n"
+   "{}\n"
+   "]",
+   Style);
+  verifyFormat("[\n"
+   "{\n"
+   "\"name\": 1\n"
+   "},\n"
+   "{}\n"
+   "]",
+   Style);
+}
+
+} // namespace format
+} // end namespace clang
Index: clang/unittests/Format/CMakeLists.txt
===
--- clang/unittests/Format/CMakeLists.txt
+++ clang/unittests/Format/CMakeLists.txt
@@ -9,6 +9,7 @@
   FormatTestCSharp.cpp
   FormatTestJS.cpp
   FormatTestJava.cpp
+  FormatTestJson.cpp
   FormatTestObjC.cpp
   FormatTestProto.cpp
   FormatTestRawStrings.cpp
Index: clang/tools/clang-format/ClangFormat.cpp

[PATCH] D93528: [clang-format] Add basic support for formatting JSON

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay updated this revision to Diff 352744.
MyDeveloperDay added a comment.

I have reworked this patch to utilise the fact that clang-format can format a 
JSON object in javascript reasonably well.

This technique adds for JSON files only a replacement to the front of the JSON 
transforming:

  {
  "A": 1,
  "B": [ 2,3 ]
  }

into

  x = {
  "A": 1,
  "B": [ 2,3 ]
  }

This is then parsed by clang-format the a more traditional sense, allowing a 
similar technique as would be used when formatting the code, (but with some 
JSON specific rules, to make it follow primarily the style used by 
https://github.com/kapilratnani/JSON-Viewer (Notepad++ plugin, which at least 
I've used extensively)

After formatter, a subsequent replacement is merged to remove the x = from the 
front of the json, rendering it back to its original form.

  {
  "A": 1,
  "B": [ 2,3 ]
  }

I prefer to use this approach rather than our Support/JSON libraries which 
aren't really setup for formatting options, this give us greater flexibility 
later on if users want custom rules from brace wrapping.


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

https://reviews.llvm.org/D93528

Files:
  clang/docs/ClangFormat.rst
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/TokenAnnotator.cpp
  clang/tools/clang-format/ClangFormat.cpp
  clang/unittests/Format/CMakeLists.txt

Index: clang/unittests/Format/CMakeLists.txt
===
--- clang/unittests/Format/CMakeLists.txt
+++ clang/unittests/Format/CMakeLists.txt
@@ -9,6 +9,7 @@
   FormatTestCSharp.cpp
   FormatTestJS.cpp
   FormatTestJava.cpp
+  FormatTestJson.cpp
   FormatTestObjC.cpp
   FormatTestProto.cpp
   FormatTestRawStrings.cpp
Index: clang/tools/clang-format/ClangFormat.cpp
===
--- clang/tools/clang-format/ClangFormat.cpp
+++ clang/tools/clang-format/ClangFormat.cpp
@@ -411,6 +411,17 @@
   unsigned CursorPosition = Cursor;
   Replacements Replaces = sortIncludes(*FormatStyle, Code->getBuffer(), Ranges,
AssumedFileName, );
+
+  // To format JSON insert a variable to trick the code into thinking its
+  // JavaScript.
+  if (FormatStyle->isJson()) {
+auto Err = Replaces.add(tooling::Replacement(
+tooling::Replacement(AssumedFileName, 0, 0, "x = ")));
+if (Err) {
+  llvm::errs() << "Bad Json variable insertion\n";
+}
+  }
+
   auto ChangedCode = tooling::applyAllReplacements(Code->getBuffer(), Replaces);
   if (!ChangedCode) {
 llvm::errs() << llvm::toString(ChangedCode.takeError()) << "\n";
@@ -506,7 +517,8 @@
   cl::SetVersionPrinter(PrintVersion);
   cl::ParseCommandLineOptions(
   argc, argv,
-  "A tool to format C/C++/Java/JavaScript/Objective-C/Protobuf/C# code.\n\n"
+  "A tool to format C/C++/Java/JavaScript/JSON/Objective-C/Protobuf/C# "
+  "code.\n\n"
   "If no arguments are specified, it formats the code from standard input\n"
   "and writes the result to the standard output.\n"
   "If s are given, it reformats the files. If -i is specified\n"
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2900,6 +2900,8 @@
   const FormatToken ) {
   if (Left.is(tok::kw_return) && Right.isNot(tok::semi))
 return true;
+  if (Style.isJson() && Left.is(tok::string_literal) && Right.is(tok::colon))
+return false;
   if (Left.is(Keywords.kw_assert) && Style.Language == FormatStyle::LK_Java)
 return true;
   if (Style.ObjCSpaceAfterProperty && Line.Type == LT_ObjCProperty &&
@@ -3221,6 +3223,9 @@
 // and "%d %d"
 if (Left.is(tok::numeric_constant) && Right.is(tok::percent))
   return HasExistingWhitespace();
+  } else if (Style.isJson()) {
+if (Right.is(tok::colon))
+  return false;
   } else if (Style.isCSharp()) {
 // Require spaces around '{' and  before '}' unless they appear in
 // interpolated strings. Interpolated strings are merged into a single token
@@ -3695,6 +3700,26 @@
   return true;
   }
 
+  // Basic JSON newline processing.
+  if (Style.isJson()) {
+// Always break after a JSON record opener.
+// {
+// }
+if (Left.is(TT_DictLiteral) && Left.is(tok::l_brace))
+  return true;
+// Always break after a JSON array opener.
+// [
+// ]
+if (Left.is(TT_ArrayInitializerLSquare) && Left.is(tok::l_square) &&
+!Right.is(tok::r_square))
+  return true;
+// Always break afer successive entries.
+// 1,
+// 2
+if (Left.is(tok::comma))
+  return true;
+  }
+
   // If the last token before a '}', ']', or ')' is a comma or a trailing
   // comment, the intention is to 

[PATCH] D103930: [clang][HeaderSearch] Fix implicit module when using header maps

[Andrew Gallagher via Phabricator via cfe-commits]

andrewjcg updated this revision to Diff 352740.
andrewjcg added a comment.

fix sed for windows test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103930

Files:
  clang/lib/Lex/HeaderSearch.cpp
  clang/test/Modules/Inputs/implicit-module-header-maps/a.h
  clang/test/Modules/Inputs/implicit-module-header-maps/a.hmap.json
  clang/test/Modules/Inputs/implicit-module-header-maps/a.module.modulemap
  clang/test/Modules/Inputs/implicit-module-header-maps/b.hmap.json
  clang/test/Modules/implicit-module-header-maps.cpp

Index: clang/test/Modules/implicit-module-header-maps.cpp
===
--- /dev/null
+++ clang/test/Modules/implicit-module-header-maps.cpp
@@ -0,0 +1,29 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: cd %t
+//
+// RUN: %hmaptool write %S/Inputs/implicit-module-header-maps/a.hmap.json %t/hmap
+//
+// RUN: mkdir -p %t/After
+// RUN: cp %S/Inputs/implicit-module-header-maps/a.h %t/After/Mapping.h
+// RUN: cp %S/Inputs/implicit-module-header-maps/a.module.modulemap %t/module.modulemap
+//
+// RUN: %clang -Rmodule-build -fmodules -fimplicit-modules -fimplicit-module-maps -fmodule-map-file=%t/module.modulemap -fsyntax-only %s -I %t/hmap
+//
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: cd %t
+//
+// RUN: sed -e "s|OUTPUTS_DIR|%t|g" %S/Inputs/implicit-module-header-maps/b.hmap.json > %t/hmap.json
+// RUN: %hmaptool write %t/hmap.json %t/hmap
+//
+// RUN: mkdir -p %t/After
+// RUN: cp %S/Inputs/implicit-module-header-maps/a.h %t/After/Mapping.h
+// RUN: cp %S/Inputs/implicit-module-header-maps/a.module.modulemap %t/module.modulemap
+//
+// RUN: %clang -Rmodule-build -fmodules -fimplicit-modules -fimplicit-module-maps -fmodule-map-file=%t/module.modulemap -fsyntax-only %s -I %t/hmap
+
+#define FOO
+// This include will fail if modules weren't used.  The include name itself
+// doesn't exist and relies on the header map to remap it to the real header.
+#include "Before/Mapping.h"
Index: clang/test/Modules/Inputs/implicit-module-header-maps/b.hmap.json
===
--- /dev/null
+++ clang/test/Modules/Inputs/implicit-module-header-maps/b.hmap.json
@@ -0,0 +1,6 @@
+{
+  "mappings" :
+{
+ "Before/Mapping.h" : "OUTPUTS_DIR/After/Mapping.h"
+}
+}
Index: clang/test/Modules/Inputs/implicit-module-header-maps/a.module.modulemap
===
--- /dev/null
+++ clang/test/Modules/Inputs/implicit-module-header-maps/a.module.modulemap
@@ -0,0 +1,3 @@
+module a {
+  header "After/Mapping.h"
+}
Index: clang/test/Modules/Inputs/implicit-module-header-maps/a.hmap.json
===
--- /dev/null
+++ clang/test/Modules/Inputs/implicit-module-header-maps/a.hmap.json
@@ -0,0 +1,7 @@
+{
+  "mappings" :
+{
+ "Before/Mapping.h" : "After/Mapping.h",
+ "After/Mapping.h" : "After/Mapping.h"
+}
+}
Index: clang/test/Modules/Inputs/implicit-module-header-maps/a.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/implicit-module-header-maps/a.h
@@ -0,0 +1,3 @@
+#ifdef FOO
+#error foo
+#endif
Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -417,7 +417,8 @@
 
   IsInHeaderMap = true;
 
-  auto FixupSearchPath = [&]() {
+  auto FixupSearchPathAndFindUsableModule =
+  [&](auto file) -> Optional {
 if (SearchPath) {
   StringRef SearchPathRef(getName());
   SearchPath->clear();
@@ -427,6 +428,12 @@
   RelativePath->clear();
   RelativePath->append(Filename.begin(), Filename.end());
 }
+if (!HS.findUsableModuleForHeader(
+(), file.getFileEntry().getDir(),
+RequestingModule, SuggestedModule, isSystemHeaderDirectory())) {
+  return None;
+}
+return file;
   };
 
   // Check if the headermap maps the filename to a framework include
@@ -437,12 +444,10 @@
 Filename = StringRef(MappedName.begin(), MappedName.size());
 Optional Result = HM->LookupFile(Filename, HS.getFileMgr());
 if (Result) {
-  FixupSearchPath();
-  return *Result;
+  return FixupSearchPathAndFindUsableModule(*Result);
 }
   } else if (auto Res = HS.getFileMgr().getOptionalFileRef(Dest)) {
-FixupSearchPath();
-return *Res;
+return FixupSearchPathAndFindUsableModule(*Res);
   }
 
   return None;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D103962: [C++4OpenCL] Fix qualifiers check on binding references to temporaries

[Anastasia Stulova via Phabricator via cfe-commits]

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

LGTM! Yes it seems the only situation where the check would evaluate to a 
different value with and without your patch is when we have `RefRelationship == 
Sema::Ref_Related` as false while the address space is not a superset then we 
would get an incorrect diagnostic. But I feel this would be diagnosed elsewhere 
before we end up in this check.

FYI I was trying the following test case:

  void square(int num) {
private int i = 0;
private const float  = ++i;
  }

but it also fails with your patch which makes me feel there is a bug elsewhere 
too:

`error: reference of type 'const __private float &__private' cannot bind to a 
temporary object because of address space mismatch`

However I do believe that the fix makes sense because that particular check was 
intended for when `RefRelationship == Sema::Ref_Related` evaluates to true.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103962

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

[PATCH] D103929: [clang] P2085R0: Consistent defaulted comparisons

[Alexandru Octavian Buțiu via Phabricator via cfe-commits]

predator5047 added a comment.

ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103929

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

[PATCH] D103930: [clang][HeaderSearch] Fix implicit module when using header maps

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 added a comment.

I don't have any comments regarding header maps.




Comment at: clang/test/Modules/implicit-module-header-maps.cpp:17
+//
+// RUN: sed -e "s:OUTPUTS_DIR:%t:g" 
%S/Inputs/implicit-module-header-maps/b.hmap.json > %t/hmap.json
+// RUN: %hmaptool write %t/hmap.json %t/hmap

The Windows CI is failing here, you should use a separator in `sed` that can't 
appear in `%t`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103930

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

[PATCH] D104459: [clang][lex] Ensure minimizer output is never larger than input

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 updated this revision to Diff 352726.
jansvoboda11 added a comment.

Fix assert wording


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104459

Files:
  clang/lib/Lex/DependencyDirectivesSourceMinimizer.cpp
  clang/unittests/Lex/DependencyDirectivesSourceMinimizerTest.cpp

Index: clang/unittests/Lex/DependencyDirectivesSourceMinimizerTest.cpp
===
--- clang/unittests/Lex/DependencyDirectivesSourceMinimizerTest.cpp
+++ clang/unittests/Lex/DependencyDirectivesSourceMinimizerTest.cpp
@@ -94,7 +94,7 @@
 
   ASSERT_FALSE(
   minimizeSourceToDependencyDirectives("#define MACRO", Out, Tokens));
-  EXPECT_STREQ("#define MACRO\n", Out.data());
+  EXPECT_STREQ("#define MACRO", Out.data());
   ASSERT_EQ(2u, Tokens.size());
   ASSERT_EQ(pp_define, Tokens.front().K);
 }
@@ -123,7 +123,7 @@
   SmallVector Out;
 
   ASSERT_FALSE(minimizeSourceToDependencyDirectives("#define MACRO()", Out));
-  EXPECT_STREQ("#define MACRO()\n", Out.data());
+  EXPECT_STREQ("#define MACRO()", Out.data());
 
   ASSERT_FALSE(
   minimizeSourceToDependencyDirectives("#define MACRO(a, b...)", Out));
@@ -131,7 +131,7 @@
 
   ASSERT_FALSE(
   minimizeSourceToDependencyDirectives("#define MACRO content", Out));
-  EXPECT_STREQ("#define MACRO content\n", Out.data());
+  EXPECT_STREQ("#define MACRO content", Out.data());
 
   ASSERT_FALSE(minimizeSourceToDependencyDirectives(
   "#define MACRO   con  tent   ", Out));
@@ -146,14 +146,14 @@
   SmallVector Out;
 
   ASSERT_FALSE(minimizeSourceToDependencyDirectives("#define MACRO((a))", Out));
-  EXPECT_STREQ("#define MACRO(/* invalid */\n", Out.data());
+  EXPECT_STREQ("#define MACRO\n", Out.data());
 
   ASSERT_FALSE(minimizeSourceToDependencyDirectives("#define MACRO(", Out));
-  EXPECT_STREQ("#define MACRO(/* invalid */\n", Out.data());
+  EXPECT_STREQ("#define MACRO\n", Out.data());
 
   ASSERT_FALSE(
   minimizeSourceToDependencyDirectives("#define MACRO(a * b)", Out));
-  EXPECT_STREQ("#define MACRO(/* invalid */\n", Out.data());
+  EXPECT_STREQ("#define MACRO\n", Out.data());
 }
 
 TEST(MinimizeSourceToDependencyDirectivesTest, DefineHorizontalWhitespace) {
@@ -259,7 +259,7 @@
   SmallVector Out;
 
   ASSERT_FALSE(minimizeSourceToDependencyDirectives("#define AND&\n", Out));
-  EXPECT_STREQ("#define AND &\n", Out.data());
+  EXPECT_STREQ("#define AND&\n", Out.data());
 
   ASSERT_FALSE(minimizeSourceToDependencyDirectives("#define AND\\\n"
 "&\n",
Index: clang/lib/Lex/DependencyDirectivesSourceMinimizer.cpp
===
--- clang/lib/Lex/DependencyDirectivesSourceMinimizer.cpp
+++ clang/lib/Lex/DependencyDirectivesSourceMinimizer.cpp
@@ -93,6 +93,7 @@
   void printDirectiveBody(const char *, const char *const End);
   void printAdjacentMacroArgs(const char *, const char *const End);
   LLVM_NODISCARD bool printMacroArgs(const char *, const char *const End);
+  void maybePrintNewLine(const char *, const char *const End);
 
   /// Reports a diagnostic if the diagnostic engine is provided. Always returns
   /// true at the end.
@@ -446,13 +447,15 @@
   }
 }
 
-static void skipWhitespace(const char *, const char *const End) {
+static size_t skipWhitespace(const char *, const char *const End) {
+  const char *FirstBefore = First;
+
   for (;;) {
 assert(First <= End);
 skipOverSpaces(First, End);
 
 if (End - First < 2)
-  return;
+  break;
 
 if (First[0] == '\\' && isVerticalWhitespace(First[1])) {
   skipNewline(++First, End);
@@ -461,21 +464,23 @@
 
 // Check for a non-comment character.
 if (First[0] != '/')
-  return;
+  break;
 
 // "// ...".
 if (First[1] == '/') {
   skipLineComment(First, End);
-  return;
+  break;
 }
 
 // Cannot be a comment.
 if (First[1] != '*')
-  return;
+  break;
 
 // "/*...*/".
 skipBlockComment(First, End);
   }
+
+  return First - FirstBefore;
 }
 
 void Minimizer::printAdjacentModuleNameParts(const char *,
@@ -519,7 +524,7 @@
   printToNewline(First, End);
   while (Out.back() == ' ')
 Out.pop_back();
-  put('\n');
+  maybePrintNewLine(First, End);
 }
 
 LLVM_NODISCARD static const char *lexRawIdentifier(const char *First,
@@ -595,6 +600,12 @@
   }
 }
 
+void Minimizer::maybePrintNewLine(const char *, const char *const End) {
+  // Only print newline if doing so won't make the output larger than the input.
+  if (Last != End)
+put('\n');
+}
+
 /// Looks for an identifier starting from Last.
 ///
 /// Updates "First" to just past the next identifier, if any.  Returns true iff
@@ -704,15 +715,17 @@
   // Be robust to bad macro arguments, since they can show up in disabled
   // code.
   Out.resize(Size);
-  append("(/* invalid */\n");
+  

[PATCH] D104465: [clang][deps] Prevent PCH validation failures by padding minimized files

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 created this revision.
jansvoboda11 added reviewers: Bigcheese, dexonsmith, arphaman.
jansvoboda11 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This patch fixes dependency scanning of a TU with prebuilt modular/PCH 
dependencies.

Such modules/PCH might have been built using non-minimized files, while 
dependency scanner does use the minimized versions of source files. Implicit 
build doesn't like the discrepancy in file sizes and errors out.

The issues is worked around by padding the minimized files with whitespace in 
such scenarios. This approach throws away one advantage of source minimization 
(the memory savings), but still keeps the benefits of faster 
preprocessing/lexing.

The padding solution comes with a 14% runtime regression compared to not 
padding the inputs. (I tested this by forcing padding and running 
`clang-scan-deps -j 20` on LLVM's `compile_commands.json` with modules 
disabled.) Interestingly, padding only with newlines (compared to spaces), the 
regression is 39%.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D104465

Files:
  clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
  clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
  clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
  clang/test/ClangScanDeps/modules-pch.c

Index: clang/test/ClangScanDeps/modules-pch.c
===
--- clang/test/ClangScanDeps/modules-pch.c
+++ clang/test/ClangScanDeps/modules-pch.c
@@ -6,7 +6,7 @@
 // RUN: sed "s|DIR|%/t|g" %S/Inputs/modules-pch/cdb_pch.json > %t/cdb.json
 // RUN: echo -%t > %t/result_pch.json
 // RUN: clang-scan-deps -compilation-database %t/cdb.json -format experimental-full \
-// RUN:   -generate-modules-path-args -module-files-dir %t/build -mode preprocess >> %t/result_pch.json
+// RUN:   -generate-modules-path-args -module-files-dir %t/build >> %t/result_pch.json
 // RUN: cat %t/result_pch.json | sed 's:\?:/:g' | FileCheck %s -check-prefix=CHECK-PCH
 //
 // Check we didn't build the PCH during dependency scanning.
@@ -127,9 +127,8 @@
 //
 // RUN: sed "s|DIR|%/t|g" %S/Inputs/modules-pch/cdb_tu.json > %t/cdb.json
 // RUN: echo -%t > %t/result_tu.json
-// FIXME: Make this work with '-mode preprocess-minimized-sources'.
 // RUN: clang-scan-deps -compilation-database %t/cdb.json -format experimental-full \
-// RUN:   -generate-modules-path-args -module-files-dir %t/build -mode preprocess >> %t/result_tu.json
+// RUN:   -generate-modules-path-args -module-files-dir %t/build >> %t/result_tu.json
 // RUN: cat %t/result_tu.json | sed 's:\?:/:g' | FileCheck %s -check-prefix=CHECK-TU
 //
 // CHECK-TU:  -[[PREFIX:.*]]
@@ -193,7 +192,7 @@
 // RUN: sed "s|DIR|%/t|g" %S/Inputs/modules-pch/cdb_tu_with_common.json > %t/cdb.json
 // RUN: echo -%t > %t/result_tu_with_common.json
 // RUN: clang-scan-deps -compilation-database %t/cdb.json -format experimental-full \
-// RUN:   -generate-modules-path-args -module-files-dir %t/build -mode preprocess >> %t/result_tu_with_common.json
+// RUN:   -generate-modules-path-args -module-files-dir %t/build >> %t/result_tu_with_common.json
 // RUN: cat %t/result_tu_with_common.json | sed 's:\?:/:g' | FileCheck %s -check-prefix=CHECK-TU-WITH-COMMON
 //
 // CHECK-TU-WITH-COMMON:  -[[PREFIX:.*]]
Index: clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
===
--- clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
+++ clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
@@ -139,6 +139,15 @@
   FileMgr->setVirtualFileSystem(createVFSFromCompilerInvocation(
   CI, Compiler.getDiagnostics(), DepFS));
 
+  // If we are about to import a PCH, make sure minimized files are padded
+  // to sizes of the originals. The PCH might have been built from
+  // non-minimized files and any disagreement on the file size causes issues
+  // in the implicit build.
+  // TODO: Consider padding only files that contributed to the PCH and its
+  // modules.
+  DepFS->PadMinimizedToOriginalSize =
+  !Compiler.getPreprocessorOpts().ImplicitPCHInclude.empty();
+
   // Pass the skip mappings which should speed up excluded conditional block
   // skipping in the preprocessor.
   if (PPSkipMappings)
Index: clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
===
--- clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
+++ clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
@@ -15,8 +15,10 @@
 using namespace tooling;
 using namespace dependencies;
 
-CachedFileSystemEntry CachedFileSystemEntry::createFileEntry(
-StringRef Filename, llvm::vfs::FileSystem , bool Minimize) {

[PATCH] D103191: [OpenCL] Add support of __opencl_c_program_scope_global_variables feature macro

[Anton Zabaznov via Phabricator via cfe-commits]

azabaznov updated this revision to Diff 352723.
azabaznov added a comment.

Restructured test, added comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103191

Files:
  clang/include/clang/Basic/OpenCLOptions.h
  clang/lib/Sema/SemaDecl.cpp
  clang/test/CodeGenOpenCL/addr-space-struct-arg.cl
  clang/test/SemaOpenCL/storageclass.cl

Index: clang/test/SemaOpenCL/storageclass.cl
===
--- clang/test/SemaOpenCL/storageclass.cl
+++ clang/test/SemaOpenCL/storageclass.cl
@@ -1,28 +1,91 @@
 // RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -cl-std=CL1.2
-
+// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -cl-std=CL3.0 -cl-ext=-__opencl_c_program_scope_global_variables -DNO_GENERIC_AS
+// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -cl-std=CL3.0 -cl-ext=+__opencl_c_program_scope_global_variables -DNO_GENERIC_AS
 static constant int G1 = 0;
 constant int G2 = 0;
-int G3 = 0;// expected-error{{program scope variable must reside in constant address space}}
-global int G4 = 0; // expected-error{{program scope variable must reside in constant address space}}
 
-static float g_implicit_static_var = 0; // expected-error {{program scope variable must reside in constant address space}}
+int G3 = 0;
+#ifndef __opencl_c_program_scope_global_variables
+// expected-error@-2 {{program scope variable must reside in constant address space}}
+#endif
+
+global int G4 = 0;
+#ifndef __opencl_c_program_scope_global_variables
+// expected-error@-2 {{program scope variable must reside in constant address space}}
+#endif
+
+static float g_implicit_static_var = 0;
+#ifndef __opencl_c_program_scope_global_variables
+// expected-error@-2 {{program scope variable must reside in constant address space}}
+#endif
+
 static constant float g_constant_static_var = 0;
-static global float g_global_static_var = 0;   // expected-error {{program scope variable must reside in constant address space}}
-static local float g_local_static_var = 0; // expected-error {{program scope variable must reside in constant address space}}
-static private float g_private_static_var = 0; // expected-error {{program scope variable must reside in constant address space}}
-static generic float g_generic_static_var = 0; // expected-error{{OpenCL C version 1.2 does not support the 'generic' type qualifier}} // expected-error {{program scope variable must reside in constant address space}}
 
-extern float g_implicit_extern_var; // expected-error {{extern variable must reside in constant address space}}
+static global float g_global_static_var = 0;
+#ifndef __opencl_c_program_scope_global_variables
+// expected-error@-2 {{program scope variable must reside in constant address space}}
+#endif
+
+static local float g_local_static_var = 0;
+#ifndef __opencl_c_program_scope_global_variables
+// expected-error@-2 {{program scope variable must reside in constant address space}}
+#else
+// expected-error@-4 {{program scope variable must reside in global or constant address space}}
+#endif
+
+static private float g_private_static_var = 0;
+#ifndef __opencl_c_program_scope_global_variables
+// expected-error@-2 {{program scope variable must reside in constant address space}}
+#else
+// expected-error@-4 {{program scope variable must reside in global or constant address space}}
+#endif
+
+static generic float g_generic_static_var = 0; // expected-error-re {{OpenCL C version {{1.2|3.0}} does not support the 'generic' type qualifier}}
+#ifndef __opencl_c_program_scope_global_variables
+// expected-error@-2 {{program scope variable must reside in constant address space}}
+#endif
+
+extern float g_implicit_extern_var;
+#ifndef __opencl_c_program_scope_global_variables
+// expected-error@-2 {{extern variable must reside in constant address space}}
+#endif
+
 extern constant float g_constant_extern_var;
-extern global float g_global_extern_var;   // expected-error {{extern variable must reside in constant address space}}
-extern local float g_local_extern_var; // expected-error {{extern variable must reside in constant address space}}
-extern private float g_private_extern_var; // expected-error {{extern variable must reside in constant address space}}
-extern generic float g_generic_extern_var; // expected-error{{OpenCL C version 1.2 does not support the 'generic' type qualifier}} // expected-error {{extern variable must reside in constant address space}}
+
+extern global float g_global_extern_var;
+#ifndef __opencl_c_program_scope_global_variables
+// expected-error@-2 {{extern variable must reside in constant address space}}
+#endif
+
+extern local float g_local_extern_var;
+#ifndef __opencl_c_program_scope_global_variables
+// expected-error@-2 {{extern variable must reside in constant address space}}
+#else
+// expected-error@-4 {{extern variable must reside in global or constant address space}}
+#endif

[PATCH] D104455: [clangd] Explicitly fail if the file passed to --check is not valid.

[Haojian Wu via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG6765b9c3f119: [clangd] Explicitly fail if the file passed to 
--check is not valid. (authored by hokein).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104455

Files:
  clang-tools-extra/clangd/tool/ClangdMain.cpp


Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -909,7 +909,11 @@
 
   if (CheckFile.getNumOccurrences()) {
 llvm::SmallString<256> Path;
-llvm::sys::fs::real_path(CheckFile, Path, /*expand_tilde=*/true);
+if (auto Error =
+llvm::sys::fs::real_path(CheckFile, Path, /*expand_tilde=*/true)) {
+  elog("Failed to resolve path {0}: {1}", CheckFile, Error.message());
+  return 1;
+}
 log("Entering check mode (no LSP server)");
 uint32_t Begin = 0, End = std::numeric_limits::max();
 if (!CheckFileLines.empty()) {


Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -909,7 +909,11 @@
 
   if (CheckFile.getNumOccurrences()) {
 llvm::SmallString<256> Path;
-llvm::sys::fs::real_path(CheckFile, Path, /*expand_tilde=*/true);
+if (auto Error =
+llvm::sys::fs::real_path(CheckFile, Path, /*expand_tilde=*/true)) {
+  elog("Failed to resolve path {0}: {1}", CheckFile, Error.message());
+  return 1;
+}
 log("Entering check mode (no LSP server)");
 uint32_t Begin = 0, End = std::numeric_limits::max();
 if (!CheckFileLines.empty()) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits